Re: [PATCH] iio: sx9310: Support ACPI property

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Stephen Boyd (2021-02-08 18:38:20)
> Quoting Gwendal Grignou (2021-02-08 18:36:19)
> > On Fri, Feb 5, 2021 at 5:41 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> > >
> > > Quoting Gwendal Grignou (2021-02-05 13:49:12)
> > > > On Tue, Feb 2, 2021 at 11:17 AM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> > > > >
> > > > > Quoting Gwendal Grignou (2021-02-01 22:45:41)
> > > > > > diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
> > > > > > index 37fd0b65a0140..1a8a441c9774d 100644
> > > > > > --- a/drivers/iio/proximity/sx9310.c
> > > > > > +++ b/drivers/iio/proximity/sx9310.c
> > > > This method of asking first for the number of element and a second
> > > > time for the values is already used at different places in the kernel:
> > > > drivers/power/supply/gpio-charger.c : see init_charge_current_limit()
> > > > or  madera_get_variable_u32_array insound/soc/codecs/madera.c.
> > >
> > > Sure it's used but that doesn't really mean it's a good pattern to copy.
> > > If the number is more than 4 I'd say we should ignore it and move on,
> > > but if it's less than 4 is an error returned?
> > >
> > > >
> > > > However, it could use device_property_count_u32(...), which is more
> > > > readable than device_property_count_u32(..., NULL,0).
> > > >
> > >
> > > How about
> > >
> > >   ret = device_property_read_u32_array(dev, "semtech,combined-sensors", combined, ARRAY_SIZE(combined));
> > >   if (ret)
> > >         break; /* Must have overflowed or not existed; ignore */
> > >
> > >   for (i = 0; i < ARRAY_SIZE(combined); i++)
> > >
> > > and then it should work as before?
> > It will not work:
> > If the DTD has a valid array of only 3 elements (for instance  [CS0,
> > CS1, CS2] as in
> >          Package (0x02)
> >          {
> >              "semtech,combined-sensors",
> >              Package (0x03)
> >              {
> >                  Zero,
> >                  One,
> >                  0x02
> >              }
> >          },
> > )
> > 
> > device_property_read_u32_array(...., 4) will return -EOVERFLOW and we
> > will use the default in the driver, which we do not want.
> > 
> 
> Isn't that a bug in the ACPI property reading code? 3 doesn't overflow 4
> so I'm lost why it would return an error code to indicate it can't fit
> the whole property into an array that is one size larger.

Or it's a bug in the driver because it's assuming that it can read the
DT property out even when it is only length 1 when the property read
should be variable length. Can you split this into two and fix the
underlying problem with the read of the array not matching the length of
the property? I think it needs to be
of_property_read_variable_u32_array() with 1 and 4 for min and max.




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux