Re: [PATCH v2 1/2] devicetree: i2c-hid: Add Wacom digitizer + regulator support

From: Benjamin Tissoires
Date: Thu Dec 01 2016 - 09:34:47 EST


On Nov 30 2016 or thereabouts, Brian Norris wrote:
> From: Caesar Wang <wxt@xxxxxxxxxxxxxx>
>
> Add a compatible string and regulator property for Wacom W9103
> digitizer. Its VDD supply may need to be enabled before using it.
>
> Signed-off-by: Caesar Wang <wxt@xxxxxxxxxxxxxx>
> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> Cc: Jiri Kosina <jikos@xxxxxxxxxx>
> Cc: linux-input@xxxxxxxxxxxxxxx
> Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx>
> ---
> v1 was a few months back. I finally got around to rewriting it based on
> DT binding feedback.
>
> v2:
> * add compatible property for wacom
> * name the regulator property specifically (VDD)
>
> Documentation/devicetree/bindings/input/hid-over-i2c.txt | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.txt b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> index 488edcb264c4..eb98054e60c9 100644
> --- a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> +++ b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> @@ -11,12 +11,16 @@ If this binding is used, the kernel module i2c-hid will handle the communication
> with the device and the generic hid core layer will handle the protocol.
>
> Required properties:
> -- compatible: must be "hid-over-i2c"
> +- compatible: must be "hid-over-i2c", or a device-specific string like:
> + * "wacom,w9013"

NACK on this one.

After re-reading the v1 submission I realized Rob asked for this change,
but I strongly disagree.

HID over I2C is a generic protocol, in the same way HID over USB is. We
can not start adding device specifics here, this is opening the can of
worms. If the device is a HID one, nothing else should matter. The rest
(description of the device, name, etc...) is all provided by the
protocol.

> - reg: i2c slave address
> - hid-descr-addr: HID descriptor address
> - interrupt-parent: the phandle for the interrupt controller
> - interrupts: interrupt line
>
> +Optional properties:
> +- vdd-supply: phandle of the regulator that provides the supply voltage.

Agree on this one however.

Cheers,
Benjamin

> +
> Example:
>
> i2c-hid-dev@2c {
> --
> 2.8.0.rc3.226.g39d4020
>