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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Y9RSGa4tCz50NRye@nixie71>
Date:   Fri, 27 Jan 2023 16:37:13 -0600
From:   Jeff LaBundy <jeff@...undy.com>
To:     Rob Herring <robh@...nel.org>
Cc:     Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-input@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: input: azoteq: Fix differing types

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@...undy.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@...nel.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