diff mbox

[2/2] Documentation: DT: bindings: input: Add documentation for cyttsp5

Message ID 20170529144538.29187-3-mylene.josserand@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mylene JOSSERAND May 29, 2017, 2:45 p.m. UTC 
Add the Cypress TrueTouch Generation 5 touchscreen device tree bindings
documentation. It can use I2C or SPI bus.
This touchscreen can handle some defined zone that are designed and
sent as button. To be able to customize the keycode sent, the
"linux,code" property in a "button" sub-node can be used.

Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
---
 .../bindings/input/touchscreen/cyttsp5.txt         | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt

Comments

Rob Herring June 7, 2017, 8:26 p.m. UTC | #1 
On Mon, May 29, 2017 at 04:45:38PM +0200, Mylène Josserand wrote:
> Add the Cypress TrueTouch Generation 5 touchscreen device tree bindings
> documentation. It can use I2C or SPI bus.
> This touchscreen can handle some defined zone that are designed and
> sent as button. To be able to customize the keycode sent, the
> "linux,code" property in a "button" sub-node can be used.

"documentation" twice in the subject makes for a long subject. 
The preferred subject prefix is "dt-bindings: input: ..."

> 
> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> ---
>  .../bindings/input/touchscreen/cyttsp5.txt         | 55 ++++++++++++++++++++++

cypress,cyttsp5.txt matching the compatible is preferred.

>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt
> new file mode 100644
> index 000000000000..713a377b5039
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt
> @@ -0,0 +1,55 @@
> +* Cypress cyttsp touchscreen controller, generation 5
> +
> +Required properties:
> + - compatible		: must be "cypress,cyttsp5"
> + - reg			: Device I2C address or SPI chip select number
> + - interrupt-parent	: the phandle for the gpio controller
> +			  (see interrupt binding[0]).
> + - interrupts		: (gpio) interrupt to which the chip is connected
> +			  (see interrupt binding[0]).
> +
> +Optional properties (many of them coming from touchscreen binding[1]):
> + - reset-gpios		: the reset gpio the chip is connected to
> +			  (see GPIO binding[2] for more details).
> + - touchscreen-size-x	: horizontal resolution of touchscreen (in pixels)

Just "see ./touchscreen.txt" is enough description.

> + - touchscreen-size-y	: vertical resolution of touchscreen (in pixels)
> + - touchscreen-fuzz-x	: horizontal noise value of the absolute input device
> +			  (in pixels)
> + - touchscreen-fuzz-y	: vertical noise value of the absolute input device
> +			  (in pixels)
> +
> +This touchscreen can handle some buttons that are touchscreen's defined zones.
> +Each button's event can be customized using a sub-node properties:
> +	- linux,code: Keycode to emit.
> +
> +[0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> +[1]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
> +[2]: Documentation/devicetree/bindings/gpio/gpio.txt
> +
> +Example:
> +&i2c0 {
> +	[...]
> +
> +	tsc@24 {

touchscreen@24

> +		compatible = "cypress,cyttsp5";
> +		reg = <0x24>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&tp_reset_ds203>;
> +		interrupt-parent = <&pio>;
> +		interrupts = <1 5 IRQ_TYPE_LEVEL_LOW>;
> +		reset-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>;
> +
> +		button@0 {

unit addresses need a reg property. If 0,1,2 are meaningful numbers for 
the hardware, then it makes sense to add here.

> +			linux,code = <KEY_HOMEPAGE>;
> +		};
> +
> +		button@1 {
> +			linux,code = <KEY_MENU>;
> +		};
> +
> +		button@2 {
> +			linux,code = <KEY_BACK>;
> +		};
> +	};
> +};
> -- 
> 2.11.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov June 7, 2017, 8:40 p.m. UTC | #2 
On Wed, Jun 07, 2017 at 03:26:03PM -0500, Rob Herring wrote:
> On Mon, May 29, 2017 at 04:45:38PM +0200, Mylène Josserand wrote:
> > Add the Cypress TrueTouch Generation 5 touchscreen device tree bindings
> > documentation. It can use I2C or SPI bus.
> > This touchscreen can handle some defined zone that are designed and
> > sent as button. To be able to customize the keycode sent, the
> > "linux,code" property in a "button" sub-node can be used.
> 
> "documentation" twice in the subject makes for a long subject. 
> The preferred subject prefix is "dt-bindings: input: ..."
> 
> > 
> > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> > ---
> >  .../bindings/input/touchscreen/cyttsp5.txt         | 55 ++++++++++++++++++++++
> 
> cypress,cyttsp5.txt matching the compatible is preferred.
> 
> >  1 file changed, 55 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt
> > new file mode 100644
> > index 000000000000..713a377b5039
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt
> > @@ -0,0 +1,55 @@
> > +* Cypress cyttsp touchscreen controller, generation 5
> > +
> > +Required properties:
> > + - compatible		: must be "cypress,cyttsp5"
> > + - reg			: Device I2C address or SPI chip select number
> > + - interrupt-parent	: the phandle for the gpio controller
> > +			  (see interrupt binding[0]).
> > + - interrupts		: (gpio) interrupt to which the chip is connected
> > +			  (see interrupt binding[0]).
> > +
> > +Optional properties (many of them coming from touchscreen binding[1]):
> > + - reset-gpios		: the reset gpio the chip is connected to
> > +			  (see GPIO binding[2] for more details).
> > + - touchscreen-size-x	: horizontal resolution of touchscreen (in pixels)
> 
> Just "see ./touchscreen.txt" is enough description.
> 
> > + - touchscreen-size-y	: vertical resolution of touchscreen (in pixels)
> > + - touchscreen-fuzz-x	: horizontal noise value of the absolute input device
> > +			  (in pixels)
> > + - touchscreen-fuzz-y	: vertical noise value of the absolute input device
> > +			  (in pixels)
> > +
> > +This touchscreen can handle some buttons that are touchscreen's defined zones.
> > +Each button's event can be customized using a sub-node properties:
> > +	- linux,code: Keycode to emit.
> > +
> > +[0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> > +[1]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
> > +[2]: Documentation/devicetree/bindings/gpio/gpio.txt
> > +
> > +Example:
> > +&i2c0 {
> > +	[...]
> > +
> > +	tsc@24 {
> 
> touchscreen@24
> 
> > +		compatible = "cypress,cyttsp5";
> > +		reg = <0x24>;
> > +
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&tp_reset_ds203>;
> > +		interrupt-parent = <&pio>;
> > +		interrupts = <1 5 IRQ_TYPE_LEVEL_LOW>;
> > +		reset-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>;
> > +
> > +		button@0 {
> 
> unit addresses need a reg property. If 0,1,2 are meaningful numbers for 
> the hardware, then it makes sense to add here.

Another option would be just say:

		linux,keycodes = <KEY_HOMEPAGE>, <KEY_MENU>, <KEY_BACK>;

I am wondering though: you read number of button supported by the device
from HID_SYSINFO_BTN_OFFSET, can you also get button assignment form
the device as well?

And the biggest question of all: since you refer to HID descriptors in
your driver, is it a HID device and should it be driven by HID susbystem
instead of relying on a custom driver?

> 
> > +			linux,code = <KEY_HOMEPAGE>;
> > +		};
> > +
> > +		button@1 {
> > +			linux,code = <KEY_MENU>;
> > +		};
> > +
> > +		button@2 {
> > +			linux,code = <KEY_BACK>;
> > +		};
> > +	};
> > +};
> > -- 
> > 2.11.0
> > 

Thanks.
Mylene JOSSERAND June 9, 2017, 11:11 a.m. UTC | #3 
Hi Rob,

Thank you for the review.

On 07/06/2017 22:26, Rob Herring wrote:
> On Mon, May 29, 2017 at 04:45:38PM +0200, Mylène Josserand wrote:
>> Add the Cypress TrueTouch Generation 5 touchscreen device tree bindings
>> documentation. It can use I2C or SPI bus.
>> This touchscreen can handle some defined zone that are designed and
>> sent as button. To be able to customize the keycode sent, the
>> "linux,code" property in a "button" sub-node can be used.
>
> "documentation" twice in the subject makes for a long subject.
> The preferred subject prefix is "dt-bindings: input: ..."
>

Noted, thanks.

>>
>> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
>> ---
>>  .../bindings/input/touchscreen/cyttsp5.txt         | 55 ++++++++++++++++++++++
>
> cypress,cyttsp5.txt matching the compatible is preferred.

ACK

>
>>  1 file changed, 55 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt
>>
>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt
>> new file mode 100644
>> index 000000000000..713a377b5039
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt
>> @@ -0,0 +1,55 @@
>> +* Cypress cyttsp touchscreen controller, generation 5
>> +
>> +Required properties:
>> + - compatible		: must be "cypress,cyttsp5"
>> + - reg			: Device I2C address or SPI chip select number
>> + - interrupt-parent	: the phandle for the gpio controller
>> +			  (see interrupt binding[0]).
>> + - interrupts		: (gpio) interrupt to which the chip is connected
>> +			  (see interrupt binding[0]).
>> +
>> +Optional properties (many of them coming from touchscreen binding[1]):
>> + - reset-gpios		: the reset gpio the chip is connected to
>> +			  (see GPIO binding[2] for more details).
>> + - touchscreen-size-x	: horizontal resolution of touchscreen (in pixels)
>
> Just "see ./touchscreen.txt" is enough description.

Okay

>
>> + - touchscreen-size-y	: vertical resolution of touchscreen (in pixels)
>> + - touchscreen-fuzz-x	: horizontal noise value of the absolute input device
>> +			  (in pixels)
>> + - touchscreen-fuzz-y	: vertical noise value of the absolute input device
>> +			  (in pixels)
>> +
>> +This touchscreen can handle some buttons that are touchscreen's defined zones.
>> +Each button's event can be customized using a sub-node properties:
>> +	- linux,code: Keycode to emit.
>> +
>> +[0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
>> +[1]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
>> +[2]: Documentation/devicetree/bindings/gpio/gpio.txt
>> +
>> +Example:
>> +&i2c0 {
>> +	[...]
>> +
>> +	tsc@24 {
>
> touchscreen@24

ACK

>
>> +		compatible = "cypress,cyttsp5";
>> +		reg = <0x24>;
>> +
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&tp_reset_ds203>;
>> +		interrupt-parent = <&pio>;
>> +		interrupts = <1 5 IRQ_TYPE_LEVEL_LOW>;
>> +		reset-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>;
>> +
>> +		button@0 {
>
> unit addresses need a reg property. If 0,1,2 are meaningful numbers for
> the hardware, then it makes sense to add here.

No, 0,1,2 do not mean anything to the driver.

>
>> +			linux,code = <KEY_HOMEPAGE>;
>> +		};
>> +
>> +		button@1 {
>> +			linux,code = <KEY_MENU>;
>> +		};
>> +
>> +		button@2 {
>> +			linux,code = <KEY_BACK>;
>> +		};
>> +	};
>> +};
>> --
>> 2.11.0
>>

Thanks!

Best regards,
Mylene JOSSERAND June 9, 2017, 11:32 a.m. UTC | #4 
Hi Dmitry,

Thank you for the review!

On 07/06/2017 22:40, Dmitry Torokhov wrote:
> On Wed, Jun 07, 2017 at 03:26:03PM -0500, Rob Herring wrote:
>> On Mon, May 29, 2017 at 04:45:38PM +0200, Mylène Josserand wrote:
>>> Add the Cypress TrueTouch Generation 5 touchscreen device tree bindings
>>> documentation. It can use I2C or SPI bus.
>>> This touchscreen can handle some defined zone that are designed and
>>> sent as button. To be able to customize the keycode sent, the
>>> "linux,code" property in a "button" sub-node can be used.
>>
>> "documentation" twice in the subject makes for a long subject.
>> The preferred subject prefix is "dt-bindings: input: ..."
>>
>>>
>>> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
>>> ---
>>>  .../bindings/input/touchscreen/cyttsp5.txt         | 55 ++++++++++++++++++++++
>>
>> cypress,cyttsp5.txt matching the compatible is preferred.
>>
>>>  1 file changed, 55 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt
>>> new file mode 100644
>>> index 000000000000..713a377b5039
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt
>>> @@ -0,0 +1,55 @@
>>> +* Cypress cyttsp touchscreen controller, generation 5
>>> +
>>> +Required properties:
>>> + - compatible		: must be "cypress,cyttsp5"
>>> + - reg			: Device I2C address or SPI chip select number
>>> + - interrupt-parent	: the phandle for the gpio controller
>>> +			  (see interrupt binding[0]).
>>> + - interrupts		: (gpio) interrupt to which the chip is connected
>>> +			  (see interrupt binding[0]).
>>> +
>>> +Optional properties (many of them coming from touchscreen binding[1]):
>>> + - reset-gpios		: the reset gpio the chip is connected to
>>> +			  (see GPIO binding[2] for more details).
>>> + - touchscreen-size-x	: horizontal resolution of touchscreen (in pixels)
>>
>> Just "see ./touchscreen.txt" is enough description.
>>
>>> + - touchscreen-size-y	: vertical resolution of touchscreen (in pixels)
>>> + - touchscreen-fuzz-x	: horizontal noise value of the absolute input device
>>> +			  (in pixels)
>>> + - touchscreen-fuzz-y	: vertical noise value of the absolute input device
>>> +			  (in pixels)
>>> +
>>> +This touchscreen can handle some buttons that are touchscreen's defined zones.
>>> +Each button's event can be customized using a sub-node properties:
>>> +	- linux,code: Keycode to emit.
>>> +
>>> +[0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
>>> +[1]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
>>> +[2]: Documentation/devicetree/bindings/gpio/gpio.txt
>>> +
>>> +Example:
>>> +&i2c0 {
>>> +	[...]
>>> +
>>> +	tsc@24 {
>>
>> touchscreen@24
>>
>>> +		compatible = "cypress,cyttsp5";
>>> +		reg = <0x24>;
>>> +
>>> +		pinctrl-names = "default";
>>> +		pinctrl-0 = <&tp_reset_ds203>;
>>> +		interrupt-parent = <&pio>;
>>> +		interrupts = <1 5 IRQ_TYPE_LEVEL_LOW>;
>>> +		reset-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>;
>>> +
>>> +		button@0 {
>>
>> unit addresses need a reg property. If 0,1,2 are meaningful numbers for
>> the hardware, then it makes sense to add here.
>
> Another option would be just say:
>
> 		linux,keycodes = <KEY_HOMEPAGE>, <KEY_MENU>, <KEY_BACK>;

Sure. I noticed the "linux,code" property so I used that one but I did 
not see the "linux,keycodes". I guess it is possible to group all the 
keycodes using that property.

>
> I am wondering though: you read number of button supported by the device
> from HID_SYSINFO_BTN_OFFSET, can you also get button assignment form
> the device as well?

For what I know, it is not possible. We can only retrieve the number of 
buttons configured for one device but not keycodes.

>
> And the biggest question of all: since you refer to HID descriptors in
> your driver, is it a HID device and should it be driven by HID susbystem
> instead of relying on a custom driver?

I am not familiar with HID subsytem but I looked to other HID drivers 
and read the kernel documentation.
In the datasheet, there is no reference to the HID specification and, 
for example, there is no GET/SET_REPORT requests. There is one "HID 
descriptor register" but that is all.

I guess that it is not a HID device but do not hesitate to give me hints 
to look at it, if you want me to confirm it.

>
>>
>>> +			linux,code = <KEY_HOMEPAGE>;
>>> +		};
>>> +
>>> +		button@1 {
>>> +			linux,code = <KEY_MENU>;
>>> +		};
>>> +
>>> +		button@2 {
>>> +			linux,code = <KEY_BACK>;
>>> +		};
>>> +	};
>>> +};
>>> --
>>> 2.11.0
>>>
>
> Thanks.
>

Best regards,
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt
new file mode 100644
index 000000000000..713a377b5039
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp5.txt
@@ -0,0 +1,55 @@ 
+* Cypress cyttsp touchscreen controller, generation 5
+
+Required properties:
+ - compatible		: must be "cypress,cyttsp5"
+ - reg			: Device I2C address or SPI chip select number
+ - interrupt-parent	: the phandle for the gpio controller
+			  (see interrupt binding[0]).
+ - interrupts		: (gpio) interrupt to which the chip is connected
+			  (see interrupt binding[0]).
+
+Optional properties (many of them coming from touchscreen binding[1]):
+ - reset-gpios		: the reset gpio the chip is connected to
+			  (see GPIO binding[2] for more details).
+ - touchscreen-size-x	: horizontal resolution of touchscreen (in pixels)
+ - touchscreen-size-y	: vertical resolution of touchscreen (in pixels)
+ - touchscreen-fuzz-x	: horizontal noise value of the absolute input device
+			  (in pixels)
+ - touchscreen-fuzz-y	: vertical noise value of the absolute input device
+			  (in pixels)
+
+This touchscreen can handle some buttons that are touchscreen's defined zones.
+Each button's event can be customized using a sub-node properties:
+	- linux,code: Keycode to emit.
+
+[0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+[1]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
+[2]: Documentation/devicetree/bindings/gpio/gpio.txt
+
+Example:
+&i2c0 {
+	[...]
+
+	tsc@24 {
+		compatible = "cypress,cyttsp5";
+		reg = <0x24>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&tp_reset_ds203>;
+		interrupt-parent = <&pio>;
+		interrupts = <1 5 IRQ_TYPE_LEVEL_LOW>;
+		reset-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>;
+
+		button@0 {
+			linux,code = <KEY_HOMEPAGE>;
+		};
+
+		button@1 {
+			linux,code = <KEY_MENU>;
+		};
+
+		button@2 {
+			linux,code = <KEY_BACK>;
+		};
+	};
+};