Subject: Re: [PATCH 2/2] Documentation: DT: bindings: input: Add documentation
 for cyttsp5
To: Rob Herring <robh@kernel.org>
References: <20170529144538.29187-1-mylene.josserand@free-electrons.com>
 <20170529144538.29187-3-mylene.josserand@free-electrons.com>
 <20170607202603.f7qh46gjkjrxx3cp@rob-hp-laptop>
Cc: dmitry.torokhov@gmail.com, mark.rutland@arm.com,
        linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
        devicetree@vger.kernel.org, thomas.petazzoni@free-electrons.com,
        maxime.ripard@free-electrons.com
From: Mylene Josserand <mylene.josserand@free-electrons.com>
Message-ID: <703c687b-6a5e-0d23-07ca-82323b330763@free-electrons.com>
Date: Fri, 9 Jun 2017 13:11:57 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <20170607202603.f7qh46gjkjrxx3cp@rob-hp-laptop>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3582
Lines: 123

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