On Mon, Feb 8, 2021 at 6:38 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
>
> 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.
This is how the code has been written: from the documentation (see
commit b31384fa5de37a):
+ * of_property_read_u64_array - Find and read an array of 64 bit
integers [same for 32 bit arrays]
+ * from a property.
+ *
[...]
+ * @sz: number of array elements to read
+ *
+ * Search for a property in a device node and read 64-bit value(s) from
+ * it. Returns 0 on success,[...], and -EOVERFLOW if the
+ * property data isn't large enough.
If the given array is too big, an error is returned. This makes sense
since the elements that can not be read will have undefined values.
Once again, the method of using count=device_property_count_u32() then
device_property_read_u32_array(..., count) is used throughout the
kernel.
Gwendal.
