Quoting Gwendal Grignou (2021-02-08 20:07:19)
> On Mon, Feb 8, 2021 at 6:53 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> >
> > Quoting Stephen Boyd (2021-02-08 18:38:20)
> > >
> > > 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.
> Splitting the patch in 2 does not make sense given
> of_property_read_variable_u32_array has no equivalent in the
> device_property_read_ realm.
> That function introduced in the first patch would be removed in the
> second patch.
> I will update the commit message to indicate this patch takes care of
> the "semtech,combined-sensors" variable array for both DT and ACPI.
>
I suggest we apply this patch and send it back to stable trees. I didn't
notice because I was setting combined-sensors to 3, and that is the
default value and it was getting removed from the default register
values all the time, even though the read of the DT property was
failing. With this change we'll always read the array and if the length
is less than zero or more than the size of the array we'll break early
and get out of there. Otherwise we'll continue on and build a bitmap out
of the values in the array to compare and set the reg_def->def bits to.
Importantly we don't mask out the settings from the default register
values until we successfully read the property.
After this change you can introduce the device_property_read_*() APIs to
add the new feature. But this is a fix that needs to be backported.
----8<-----
diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
index 6a04959df35e..6ba063e51af5 100644
--- a/drivers/iio/proximity/sx9310.c
+++ b/drivers/iio/proximity/sx9310.c
@@ -1236,15 +1236,17 @@ sx9310_get_default_reg(struct sx9310_data *data, int i,
reg_def->def |= SX9310_REG_PROX_CTRL2_SHIELDEN_GROUND;
}
+ ret = of_property_read_variable_u32_array(np, "semtech,combined-sensors",
+ combined, 0, ARRAY_SIZE(combined));
+ if (ret < 0)
+ break;
+
reg_def->def &= ~SX9310_REG_PROX_CTRL2_COMBMODE_MASK;
- of_property_read_u32_array(np, "semtech,combined-sensors",
- combined, ARRAY_SIZE(combined));
for (i = 0; i < ARRAY_SIZE(combined); i++) {
- if (combined[i] <= SX9310_NUM_CHANNELS)
+ if (combined[i] < SX9310_NUM_CHANNELS)
comb_mask |= BIT(combined[i]);
}
- comb_mask &= 0xf;
if (comb_mask == (BIT(3) | BIT(2) | BIT(1) | BIT(0)))
reg_def->def |= SX9310_REG_PROX_CTRL2_COMBMODE_CS0_CS1_CS2_CS3;
else if (comb_mask == (BIT(1) | BIT(2)))
