Re: [PATCH] dt-bindings: input: azoteq: Fix differing types

[Jeff LaBundy <jeff@labundy.com>]

Hi Rob,

On Wed, Jan 25, 2023 at 09:10:33PM -0600, Rob Herring wrote:
> On Wed, Jan 25, 2023 at 7:51 PM Jeff LaBundy <jeff@labundy.com> wrote:
> >
> > Hi Rob,
> >
> > On Wed, Jan 25, 2023 at 04:14:16PM -0600, Rob Herring wrote:
> > > 'azoteq,ati-base' and 'azoteq,thresh' properties are defined in multiple
> > > bindings, but have differing types defined. Both 'uint32' and
> > > 'uint32-array' are used. Unify these to use 'uint32-array' everywhere.
> > >
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> >
> > Thank you for the patch. While this is a step forward in moving toward
> > a common binding for this vendor like we have discussed in the past, I
> > do not agree with this approach and will instead propose an alternative
> > that accomplishes the same goal.
> >
> > For all of these devices, a single sensing channel takes a base and a
> > threshold property. IQS626A is unique in that a fixed number of channels
> > form a trackpad, and I decided at the time to simply define the base and
> > target properties for all channels as a uint32-array.
> >
> > For all other existing drivers, as well as others coming down the pipe,
> > base and threshold are uint32s. I find it confusing to redefine all of
> > those as single-element arrays, especially on account of one device.
> >
> > In hindsight, a better design would have been to define a child node
> > for each channel under the trackpad node, with each child node accepting
> > a uint32 base and threshold. That would follow other devices, be more
> > representative of the actual hardware, and keep the definitions the same
> > across each binding.
> >
> > So, that's what I propose to do here instead. I happen to have a fix in
> > review [1] that addresses a bug related to this parsing code, and would
> > be happy to build this solution on top assuming it can wait until the
> > next cycle. Does this compromise sound OK?
> 
> Shrug
> 
> How exactly are you going to change an existing property and not break
> existing users? Or are there not any users?

There are no known users at this time.

> 
> We have the same properties with 2 definitions. At the end of the day,
> I just want to fix that...

Agreed on all counts. I've folded my proposal into the existing fix for
this driver and sent [1] for your consideration.

[1] https://patchwork.kernel.org/patch/13119464/

> 
> Rob

Kind regards,
Jeff LaBundy