Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1660188 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/2] Documentation: DT: bindings: input: Add  documentation for cyttsp5 Rob Herring <robh@kernel.org> - 2017-06-07 22:30 +0200
    Re: [PATCH 2/2] Documentation: DT: bindings: input: Add  documentation for cyttsp5 Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-06-07 22:50 +0200
      Re: [PATCH 2/2] Documentation: DT: bindings: input: Add documentation  for cyttsp5 Mylene Josserand <mylene.josserand@free-electrons.com> - 2017-06-09 13:40 +0200
    Re: [PATCH 2/2] Documentation: DT: bindings: input: Add documentation  for cyttsp5 Mylene Josserand <mylene.josserand@free-electrons.com> - 2017-06-09 13:20 +0200

#1660188 — Re: [PATCH 2/2] Documentation: DT: bindings: input: Add documentation for cyttsp5

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
> 

[toc] | [next] | [standalone]

#1660205

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.

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1662329 — Re: [PATCH 2/2] Documentation: DT: bindings: input: Add documentation for cyttsp5

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,

-- 
Mylène Josserand, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1662311 — Re: [PATCH 2/2] Documentation: DT: bindings: input: Add documentation for cyttsp5

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,

-- 
Mylène Josserand, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web