Hi Lee,
lee.jones@xxxxxxxxxx wrote on Thu, 23 Sep 2021 10:11:13 +0100:
> On Thu, 23 Sep 2021, Miquel Raynal wrote:
>
> > Hi Lee,
> >
> > lee.jones@xxxxxxxxxx wrote on Wed, 22 Sep 2021 16:57:10 +0100:
> >
> > > On Wed, 15 Sep 2021, Miquel Raynal wrote:
> > >
> > > > There was in the past a typo in the coordinate readouts property. The
> > > > bindings have been updated, the touchscreen driver as well and now
> > > > supports both. However, the MFD driver that is in charge of verifying
> > > > the validity of the property only checks the bogus one. Add support for
> > > > the correctly spelled DT property.
> > > >
> > > > Fixes: c9aeb249bf72 ("Input: ti_am335x_tsc - fix spelling mistake in TSC/ADC DT binding")
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> > > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > > > ---
> > > > drivers/mfd/ti_am335x_tscadc.c | 8 +++++++-
> > > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> > > > index 155a8ed879b3..aa46ed669016 100644
> > > > --- a/drivers/mfd/ti_am335x_tscadc.c
> > > > +++ b/drivers/mfd/ti_am335x_tscadc.c
> > > > @@ -144,8 +144,14 @@ static int ti_tscadc_probe(struct platform_device *pdev)
> > > > if (tscadc->data->has_tsc) {
> > > > node = of_get_child_by_name(pdev->dev.of_node, "tsc");
> > > > of_property_read_u32(node, "ti,wires", &tscmag_wires);
> > > > - of_property_read_u32(node, "ti,coordiante-readouts", &readouts);
> > > > + err = of_property_read_u32(node, "ti,coordinate-readouts",
> > > > + &readouts);
> > > > + if (err < 0)
> > > > + of_property_read_u32(node, "ti,coordiante-readouts",
> > > > + &readouts);
> > > > +
> > >
> > > How long are you proposing that we support this churn?
> >
> > Well, I am not proposing anything, I am just "fixing" the driver so
> > that it fits the bindings :) Given the fact that at the end of this
> > series there is a patch that changes the "coordiante" typo to
> > "coordinate" in a device tree source file, I believe it is still too
> > soon...
>
> If this is something you're changing in this set, please reconsider.
>
> I'd rather have a slightly misspelled documented property than being
> forced to support 2 for any length of time.
I am not suggesting anything here:
- "coordiante" was introduced back in 2014.
- "coordinate" was introduced shortly later.
- "coordiante" was considered part of the "stable API" back in 2014 and
even though the right spelling got introduced very shortly after, DT
stability rules wanted us to support it forever.
- The touchscreen driver has immediately be fixed to support both but
not the MFD driver, and this does not make any sense!
We *should* either support only one property (1) or support both in the
two drivers (2), but supporting the two in one driver and supporting
only one in the other one does not make any sense (given the fact that
these two drives are tied together, the touchscreen driver does not
exist without the MFD driver). That is what I am fixing here.
Is #1 valid? Theoretically it's the best scenario. In practice it is
not (yet) possible because the two versions are still used in the
mainline device trees:
$ git grep coordiante-readouts -- arch/arm/boot/dts/ | wc -l
1
$ git grep coordinate-readouts -- arch/arm/boot/dts/ | wc -l
5
So in this series I am fixing the MFD driver to be sure it handles
correctly the correctly spelled DT property which is used by 5 boards
since at least 6 years and I am also updating the remaining DT to use
the correctly spelled property as well.
I suppose we could propose to drop support for the "coordiante"
version of that property in a few years but if we decide to do it right
now we're definitely gonna break users.
Thanks,
Miquèl
