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

[Brian Norris]

Hi all,

On Mon, Dec 12, 2016 at 08:47:06AM -0600, Rob Herring wrote:

[Snip Benjamin's proposal; I agree we don't really want a multi-level DT
layout purely for making the driver look a little nicer (I'm not sure
this would really be nicer anyway). And I think, as Rob notes here, our
disagreement is smaller than appears. But I might be wrong.]

> Anyway, we're only debating this:

OK, so I think we might have a consensus of sorts? I'll describe it
here, in case I'm wrong. Otherwise, I'll send another rev.

>          i2c-hid-dev@2c {
>                  compatible = "wacom,w9013", "hid-over-i2c";

I plan to document the above, but not treat "wacom,w9013" specially in
the driver, apart from possibly listing it in the driver of_match_table.
This was mentioned by Dmitry earlier, and I didn't see any objection.

(Note that there are problems with module autoload when using multiple
compatible strings like above. May not be supremely relevant to the
documentation, but it *is* practically important.)

>                  reg = <0x2c>;
>                  hid-descr-addr = <0x0020>;
>                  interrupt-parent = <&gpx3>;
>                  interrupts = <3 2>;
>                  vdd-supply = <sth>;

Document and support 'vdd-supply', optionally.

>                  init-delay-ms = <100>;

Per Rob's mention below, support this as 'post-power-on-delay-ms',
optionally.

We can use either of these properties on any device, with the
intention that if there are future needs for divergent bindings, the
aforementioned compatible property can help us differentiate.

>          };
> 
> vs.
> 
>          i2c-hid-dev@2c {
>                  compatible = "hid-over-i2c";
>                  reg = <0x2c>;
>                  hid-descr-addr = <0x0020>;
>                  interrupt-parent = <&gpx3>;
>                  interrupts = <3 2>;
>                  vdd-supply = <sth>;
>                  init-delay-ms = <100>;
>          };
> 
> My only other nit is use "post-power-on-delay-ms" which is already a
> defined property name rather than "init-delay-ms".

Any objections? Speak now or forever [1] hold your peace.

Brian

[1] Who am I kidding? There's always room for more paint on the
bikeshed.