[PATCH v2 1/2] devicetree: i2c-hid: Add Wacom digitizer + regulator support
robh at kernel.org
Mon Dec 5 15:59:08 PST 2016
On Thu, Dec 01, 2016 at 09:24:50AM -0800, Brian Norris wrote:
> Hi Benjamin and Rob,
> On Thu, Dec 01, 2016 at 03:34:34PM +0100, Benjamin Tissoires wrote:
> > On Nov 30 2016 or thereabouts, Brian Norris wrote:
> > > From: Caesar Wang <wxt at rock-chips.com>
> > >
> > > 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 at rock-chips.com>
> > > Cc: Rob Herring <robh+dt at kernel.org>
> > > Cc: Jiri Kosina <jikos at kernel.org>
> > > Cc: linux-input at vger.kernel.org
> > > Signed-off-by: Brian Norris <briannorris at chromium.org>
> > > ---
> > > 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.
> I should have spoken up when Rob made the suggestion, because I more or
> less agree with Benjamin here. I don't really see why this needs to have
> a specialized compatible string, as the property is still fairly
> generic, and the entire device handling is via a generic protocol. The
> fact that we manage its power via a regulator is not very
It doesn't matter that the protocol is generic. The device attached and
the implementation is not. Implementations have been known to have
bugs/quirks (generally speaking, not HID over I2C in particular). There
are also things outside the scope of what is 'hid-over-i2c' like what's
needed to power-on the device which this patch clearly show.
This is no different than a panel attached via LVDS, eDP, etc., or
USB/PCIe device hard-wired on a board. They all use standard protocols
and all need additional data to describe them. Of course, adding a
single property for a delay would not be a big deal, but it's never
ending. Next you need multiple supplies, GPIO controls, mutiple
delays... This has been discussed to death already. As Thierry Reding
said, you're not special.
Now if you want to make 'hid-over-i2c' a fallback to 'wacom,w9013', I'm
fine with that.
More information about the Linux-rockchip