Re: [PATCH 1/3] input: touchscreen: ar1021_i2c: add support for AR1020

From: Rob Herring
Date: Thu Apr 13 2017 - 16:35:44 EST


On Thu, Apr 13, 2017 at 08:38:39AM +0200, Martin Kepplinger wrote:
>
>
> On 2017-04-12 17:40, Dmitry Torokhov wrote:
> > Hi Martin,
> >
> > On Tue, Apr 11, 2017 at 12:27:57PM +0200, Martin Kepplinger wrote:
> >> ar1021_i2c simply also supports the ar1020 device we use. This is tested.
> >> They also share the same datasheet:
> >>
> >> http://ww1.microchip.com/downloads/en/DeviceDoc/40001393C.pdf
> >>
> >> We differentiate not only to make it obvious that we support both devices,
> >> but also to be able to implement the few model specific things in the
> >> future.

[...]

> >
> > I do not see where you handle ar1020 differently from ar1021. If devices
> > are compatible, you do not need to add a new compatible to the driver,
> > simply use it in the binding:
> >
> > compatible = "microchip,ar1020-i2c", "microchip,ar1021-i2c";
> >
> > Thanks.
> >
>
> Why would you use "microchip,ar1020-i2c" in the dts if it's not
> available? people don't obviously see, by grepping or reading,
> that they have a compatible driver. ... or did I get you wrong?
>
> I don't handle anything differently now. Factory reset has to be done
> differntly though, as one example. So it'd be nice to have the option
> to add data.

Having both lets you use the current driver now and you can match on the
more specific compatible string when and if you need to. That's exactly
why we have multiple compatibles. I could imagine the driver never
supports factory reset. Add both to comments or kconfig help if you want
to make it clear that both devices are supported.

Rob