MIME-Version: 1.0
References: <20230125221416.3058051-1-robh@kernel.org> <Y9Hce2OqJOB/uiyM@nixie71>
In-Reply-To: <Y9Hce2OqJOB/uiyM@nixie71>
From:   Rob Herring <robh@kernel.org>
Date:   Wed, 25 Jan 2023 21:10:33 -0600
Message-ID: <CAL_JsqJACct1J2T357V6CSQWsRP1P7bFpNCmippbYf2rYdd2Rw@mail.gmail.com>
Subject: Re: [PATCH] dt-bindings: input: azoteq: Fix differing types
To:     Jeff LaBundy <jeff@labundy.com>
Cc:     Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        linux-input@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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?

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

Rob