| Message ID | 20210322073220.1637117-3-gwendal@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: sx9310: Support ACPI properties | expand |
On Mon, Mar 22, 2021 at 9:33 AM Gwendal Grignou <gwendal@chromium.org> wrote: > > Use device_property_read_uXX to support both device tree and ACPI > bindings when reading the properties we need to configure the SAR > sensor. > > Signed-off-by: Gwendal Grignou <gwendal@chromium.org> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > --- > Changes in v6: > Use proper function to gather the number of elements in an array. > > Changes in v5: > Split in 2 patches, one for fixing access to propery array, the other > to support ACPI. > > drivers/iio/proximity/sx9310.c | 26 +++++++++++--------------- > 1 file changed, 11 insertions(+), 15 deletions(-) > > diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c > index 289c76bb3b024..3817a5714aab3 100644 > --- a/drivers/iio/proximity/sx9310.c > +++ b/drivers/iio/proximity/sx9310.c > @@ -20,6 +20,7 @@ > #include <linux/mod_devicetable.h> > #include <linux/module.h> > #include <linux/pm.h> > +#include <linux/property.h> > #include <linux/regmap.h> > #include <linux/regulator/consumer.h> > #include <linux/slab.h> > @@ -1213,10 +1214,9 @@ static int sx9310_init_compensation(struct iio_dev *indio_dev) > } > > static const struct sx9310_reg_default * > -sx9310_get_default_reg(struct sx9310_data *data, int idx, > +sx9310_get_default_reg(struct device *dev, int idx, > struct sx9310_reg_default *reg_def) > { > - const struct device_node *np = data->client->dev.of_node; There may be an issue with this. 'np' was coming from the parent device (which is an i2c device) So, 'data->client->dev' should be the same pointer as 'indio_dev->dev.parent' Now, it's a reference to 'indio_dev->dev', so it's probably going to miss a lot of DT properties. Maybe an alternative would be to do: const struct device *dev = &data->client->dev; [1] Or instead of pass '&data->client->dev' or 'indio_dev->dev.parent' when calling sx9310_get_default_reg() Though [1] looks good to me as well. > u32 combined[SX9310_NUM_CHANNELS]; > u32 start = 0, raw = 0, pos = 0; > unsigned long comb_mask = 0; > @@ -1224,21 +1224,17 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, > const char *res; > > memcpy(reg_def, &sx9310_default_regs[idx], sizeof(*reg_def)); > - if (!np) > - return reg_def; > - > switch (reg_def->reg) { > case SX9310_REG_PROX_CTRL2: > - if (of_property_read_bool(np, "semtech,cs0-ground")) { > + if (device_property_read_bool(dev, "semtech,cs0-ground")) { > reg_def->def &= ~SX9310_REG_PROX_CTRL2_SHIELDEN_MASK; > reg_def->def |= SX9310_REG_PROX_CTRL2_SHIELDEN_GROUND; > } > > - count = of_property_count_elems_of_size(np, "semtech,combined-sensors", > - sizeof(u32)); > + count = device_property_count_u32(dev, "semtech,combined-sensors"); > if (count > 0 && count <= ARRAY_SIZE(combined)) { > - ret = of_property_read_u32_array(np, "semtech,combined-sensors", > - combined, count); > + ret = device_property_read_u32_array(dev, "semtech,combined-sensors", > + combined, count); > if (ret) > break; > } else { > @@ -1270,7 +1266,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, > > break; > case SX9310_REG_PROX_CTRL4: > - ret = of_property_read_string(np, "semtech,resolution", &res); > + ret = device_property_read_string(dev, "semtech,resolution", &res); > if (ret) > break; > > @@ -1294,7 +1290,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, > > break; > case SX9310_REG_PROX_CTRL5: > - ret = of_property_read_u32(np, "semtech,startup-sensor", &start); > + ret = device_property_read_u32(dev, "semtech,startup-sensor", &start); > if (ret) { > start = FIELD_GET(SX9310_REG_PROX_CTRL5_STARTUPSENS_MASK, > reg_def->def); > @@ -1304,7 +1300,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, > reg_def->def |= FIELD_PREP(SX9310_REG_PROX_CTRL5_STARTUPSENS_MASK, > start); > > - ret = of_property_read_u32(np, "semtech,proxraw-strength", &raw); > + ret = device_property_read_u32(dev, "semtech,proxraw-strength", &raw); > if (ret) { > raw = FIELD_GET(SX9310_REG_PROX_CTRL5_RAWFILT_MASK, > reg_def->def); > @@ -1317,7 +1313,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, > raw); > break; > case SX9310_REG_PROX_CTRL7: > - ret = of_property_read_u32(np, "semtech,avg-pos-strength", &pos); > + ret = device_property_read_u32(dev, "semtech,avg-pos-strength", &pos); > if (ret) > break; > > @@ -1353,7 +1349,7 @@ static int sx9310_init_device(struct iio_dev *indio_dev) > > /* Program some sane defaults. */ > for (i = 0; i < ARRAY_SIZE(sx9310_default_regs); i++) { > - initval = sx9310_get_default_reg(data, i, &tmp); > + initval = sx9310_get_default_reg(&indio_dev->dev, i, &tmp); > ret = regmap_write(data->regmap, initval->reg, initval->def); > if (ret) > return ret; > -- > 2.31.0.291.g576ba9dcdaf-goog >
On Mon, Mar 22, 2021 at 2:54 AM Alexandru Ardelean <ardeleanalex@gmail.com> wrote: > > On Mon, Mar 22, 2021 at 9:33 AM Gwendal Grignou <gwendal@chromium.org> wrote: > > > > Use device_property_read_uXX to support both device tree and ACPI > > bindings when reading the properties we need to configure the SAR > > sensor. > > > > Signed-off-by: Gwendal Grignou <gwendal@chromium.org> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > --- > > Changes in v6: > > Use proper function to gather the number of elements in an array. > > > > Changes in v5: > > Split in 2 patches, one for fixing access to propery array, the other > > to support ACPI. > > > > drivers/iio/proximity/sx9310.c | 26 +++++++++++--------------- > > 1 file changed, 11 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c > > index 289c76bb3b024..3817a5714aab3 100644 > > --- a/drivers/iio/proximity/sx9310.c > > +++ b/drivers/iio/proximity/sx9310.c > > @@ -20,6 +20,7 @@ > > #include <linux/mod_devicetable.h> > > #include <linux/module.h> > > #include <linux/pm.h> > > +#include <linux/property.h> > > #include <linux/regmap.h> > > #include <linux/regulator/consumer.h> > > #include <linux/slab.h> > > @@ -1213,10 +1214,9 @@ static int sx9310_init_compensation(struct iio_dev *indio_dev) > > } > > > > static const struct sx9310_reg_default * > > -sx9310_get_default_reg(struct sx9310_data *data, int idx, > > +sx9310_get_default_reg(struct device *dev, int idx, > > struct sx9310_reg_default *reg_def) > > { > > - const struct device_node *np = data->client->dev.of_node; > > There may be an issue with this. > 'np' was coming from the parent device (which is an i2c device) > > So, 'data->client->dev' should be the same pointer as 'indio_dev->dev.parent' > > Now, it's a reference to 'indio_dev->dev', so it's probably going to > miss a lot of DT properties. > Maybe an alternative would be to do: > > const struct device *dev = &data->client->dev; [1] > > Or instead of pass '&data->client->dev' or 'indio_dev->dev.parent' > when calling sx9310_get_default_reg() > > Though [1] looks good to me as well. Indeed, the current code does not work with device-tree bindings. The proposed fix solves the issue, sending v7. Thanks, Gwendal. > > > u32 combined[SX9310_NUM_CHANNELS]; > > u32 start = 0, raw = 0, pos = 0; > > unsigned long comb_mask = 0; > > @@ -1224,21 +1224,17 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, > > const char *res; > > > > memcpy(reg_def, &sx9310_default_regs[idx], sizeof(*reg_def)); > > - if (!np) > > - return reg_def; > > - > > switch (reg_def->reg) { > > case SX9310_REG_PROX_CTRL2: > > - if (of_property_read_bool(np, "semtech,cs0-ground")) { > > + if (device_property_read_bool(dev, "semtech,cs0-ground")) { > > reg_def->def &= ~SX9310_REG_PROX_CTRL2_SHIELDEN_MASK; > > reg_def->def |= SX9310_REG_PROX_CTRL2_SHIELDEN_GROUND; > > } > > > > - count = of_property_count_elems_of_size(np, "semtech,combined-sensors", > > - sizeof(u32)); > > + count = device_property_count_u32(dev, "semtech,combined-sensors"); > > if (count > 0 && count <= ARRAY_SIZE(combined)) { > > - ret = of_property_read_u32_array(np, "semtech,combined-sensors", > > - combined, count); > > + ret = device_property_read_u32_array(dev, "semtech,combined-sensors", > > + combined, count); > > if (ret) > > break; > > } else { > > @@ -1270,7 +1266,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, > > > > break; > > case SX9310_REG_PROX_CTRL4: > > - ret = of_property_read_string(np, "semtech,resolution", &res); > > + ret = device_property_read_string(dev, "semtech,resolution", &res); > > if (ret) > > break; > > > > @@ -1294,7 +1290,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, > > > > break; > > case SX9310_REG_PROX_CTRL5: > > - ret = of_property_read_u32(np, "semtech,startup-sensor", &start); > > + ret = device_property_read_u32(dev, "semtech,startup-sensor", &start); > > if (ret) { > > start = FIELD_GET(SX9310_REG_PROX_CTRL5_STARTUPSENS_MASK, > > reg_def->def); > > @@ -1304,7 +1300,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, > > reg_def->def |= FIELD_PREP(SX9310_REG_PROX_CTRL5_STARTUPSENS_MASK, > > start); > > > > - ret = of_property_read_u32(np, "semtech,proxraw-strength", &raw); > > + ret = device_property_read_u32(dev, "semtech,proxraw-strength", &raw); > > if (ret) { > > raw = FIELD_GET(SX9310_REG_PROX_CTRL5_RAWFILT_MASK, > > reg_def->def); > > @@ -1317,7 +1313,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, > > raw); > > break; > > case SX9310_REG_PROX_CTRL7: > > - ret = of_property_read_u32(np, "semtech,avg-pos-strength", &pos); > > + ret = device_property_read_u32(dev, "semtech,avg-pos-strength", &pos); > > if (ret) > > break; > > > > @@ -1353,7 +1349,7 @@ static int sx9310_init_device(struct iio_dev *indio_dev) > > > > /* Program some sane defaults. */ > > for (i = 0; i < ARRAY_SIZE(sx9310_default_regs); i++) { > > - initval = sx9310_get_default_reg(data, i, &tmp); > > + initval = sx9310_get_default_reg(&indio_dev->dev, i, &tmp); > > ret = regmap_write(data->regmap, initval->reg, initval->def); > > if (ret) > > return ret; > > -- > > 2.31.0.291.g576ba9dcdaf-goog > >
diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c index 289c76bb3b024..3817a5714aab3 100644 --- a/drivers/iio/proximity/sx9310.c +++ b/drivers/iio/proximity/sx9310.c @@ -20,6 +20,7 @@ #include <linux/mod_devicetable.h> #include <linux/module.h> #include <linux/pm.h> +#include <linux/property.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> #include <linux/slab.h> @@ -1213,10 +1214,9 @@ static int sx9310_init_compensation(struct iio_dev *indio_dev) } static const struct sx9310_reg_default * -sx9310_get_default_reg(struct sx9310_data *data, int idx, +sx9310_get_default_reg(struct device *dev, int idx, struct sx9310_reg_default *reg_def) { - const struct device_node *np = data->client->dev.of_node; u32 combined[SX9310_NUM_CHANNELS]; u32 start = 0, raw = 0, pos = 0; unsigned long comb_mask = 0; @@ -1224,21 +1224,17 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, const char *res; memcpy(reg_def, &sx9310_default_regs[idx], sizeof(*reg_def)); - if (!np) - return reg_def; - switch (reg_def->reg) { case SX9310_REG_PROX_CTRL2: - if (of_property_read_bool(np, "semtech,cs0-ground")) { + if (device_property_read_bool(dev, "semtech,cs0-ground")) { reg_def->def &= ~SX9310_REG_PROX_CTRL2_SHIELDEN_MASK; reg_def->def |= SX9310_REG_PROX_CTRL2_SHIELDEN_GROUND; } - count = of_property_count_elems_of_size(np, "semtech,combined-sensors", - sizeof(u32)); + count = device_property_count_u32(dev, "semtech,combined-sensors"); if (count > 0 && count <= ARRAY_SIZE(combined)) { - ret = of_property_read_u32_array(np, "semtech,combined-sensors", - combined, count); + ret = device_property_read_u32_array(dev, "semtech,combined-sensors", + combined, count); if (ret) break; } else { @@ -1270,7 +1266,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, break; case SX9310_REG_PROX_CTRL4: - ret = of_property_read_string(np, "semtech,resolution", &res); + ret = device_property_read_string(dev, "semtech,resolution", &res); if (ret) break; @@ -1294,7 +1290,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, break; case SX9310_REG_PROX_CTRL5: - ret = of_property_read_u32(np, "semtech,startup-sensor", &start); + ret = device_property_read_u32(dev, "semtech,startup-sensor", &start); if (ret) { start = FIELD_GET(SX9310_REG_PROX_CTRL5_STARTUPSENS_MASK, reg_def->def); @@ -1304,7 +1300,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, reg_def->def |= FIELD_PREP(SX9310_REG_PROX_CTRL5_STARTUPSENS_MASK, start); - ret = of_property_read_u32(np, "semtech,proxraw-strength", &raw); + ret = device_property_read_u32(dev, "semtech,proxraw-strength", &raw); if (ret) { raw = FIELD_GET(SX9310_REG_PROX_CTRL5_RAWFILT_MASK, reg_def->def); @@ -1317,7 +1313,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int idx, raw); break; case SX9310_REG_PROX_CTRL7: - ret = of_property_read_u32(np, "semtech,avg-pos-strength", &pos); + ret = device_property_read_u32(dev, "semtech,avg-pos-strength", &pos); if (ret) break; @@ -1353,7 +1349,7 @@ static int sx9310_init_device(struct iio_dev *indio_dev) /* Program some sane defaults. */ for (i = 0; i < ARRAY_SIZE(sx9310_default_regs); i++) { - initval = sx9310_get_default_reg(data, i, &tmp); + initval = sx9310_get_default_reg(&indio_dev->dev, i, &tmp); ret = regmap_write(data->regmap, initval->reg, initval->def); if (ret) return ret;