| Message ID | 20201115205745.618455-3-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3,v3] iio: accel: bmc150-accel: Add DT bindings | expand |
On Sun, 15 Nov 2020 21:57:45 +0100 Linus Walleij <linus.walleij@linaro.org> wrote: > These Bosch accelerometers have two supplies, VDD and VDDIO. > Add some rudimentary support to obtain and enable these > regulators during probe() and disable them during remove() > or on the errorpath. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> All looks good to me. Will leave it for a bit longer though to let Rob have a chance to look at the dt binding doc. As ever, if it seems to have gotten lost it is probably worth poking me. Thanks, Jonathan > --- > ChangeLog v2->v3: > - Resend with the rest. > ChangeLog v1->v2: > - Use dev_err_probe() to simplify the errorpath. > --- > drivers/iio/accel/bmc150-accel-core.c | 35 +++++++++++++++++++++++++-- > 1 file changed, 33 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c > index 01dbc01e328f..caa984612e88 100644 > --- a/drivers/iio/accel/bmc150-accel-core.c > +++ b/drivers/iio/accel/bmc150-accel-core.c > @@ -28,6 +28,7 @@ > #include <linux/iio/trigger_consumer.h> > #include <linux/iio/triggered_buffer.h> > #include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > > #include "bmc150-accel.h" > > @@ -184,6 +185,7 @@ enum bmc150_accel_trigger_id { > > struct bmc150_accel_data { > struct regmap *regmap; > + struct regulator_bulk_data regulators[2]; > int irq; > struct bmc150_accel_interrupt interrupts[BMC150_ACCEL_INTERRUPTS]; > struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS]; > @@ -1591,10 +1593,33 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq, > &data->orientation); > if (ret) > return ret; > + /* > + * VDD is the analog and digital domain voltage supply > + * VDDIO is the digital I/O voltage supply > + */ > + data->regulators[0].supply = "vdd"; > + data->regulators[1].supply = "vddio"; > + ret = devm_regulator_bulk_get(dev, > + ARRAY_SIZE(data->regulators), > + data->regulators); > + if (ret) > + return dev_err_probe(dev, ret, "failed to get regulators\n"); > + > + ret = regulator_bulk_enable(ARRAY_SIZE(data->regulators), > + data->regulators); > + if (ret) { > + dev_err(dev, "failed to enable regulators: %d\n", ret); > + return ret; > + } > + /* > + * 2ms or 3ms power-on time according to datasheets, let's better > + * be safe than sorry and set this delay to 5ms. > + */ > + msleep(5); > > ret = bmc150_accel_chip_init(data); > if (ret < 0) > - return ret; > + goto err_disable_regulators; > > mutex_init(&data->mutex); > > @@ -1611,7 +1636,7 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq, > &bmc150_accel_buffer_ops); > if (ret < 0) { > dev_err(dev, "Failed: iio triggered buffer setup\n"); > - return ret; > + goto err_disable_regulators; > } > > if (data->irq > 0) { > @@ -1672,6 +1697,9 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq, > bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1); > err_buffer_cleanup: > iio_triggered_buffer_cleanup(indio_dev); > +err_disable_regulators: > + regulator_bulk_disable(ARRAY_SIZE(data->regulators), > + data->regulators); > > return ret; > } > @@ -1696,6 +1724,9 @@ int bmc150_accel_core_remove(struct device *dev) > bmc150_accel_set_mode(data, BMC150_ACCEL_SLEEP_MODE_DEEP_SUSPEND, 0); > mutex_unlock(&data->mutex); > > + regulator_bulk_disable(ARRAY_SIZE(data->regulators), > + data->regulators); > + > return 0; > } > EXPORT_SYMBOL_GPL(bmc150_accel_core_remove);
On Sat, Nov 21, 2020 at 5:45 PM Jonathan Cameron <jic23@kernel.org> wrote: > On Sun, 15 Nov 2020 21:57:45 +0100 > Linus Walleij <linus.walleij@linaro.org> wrote: > > > These Bosch accelerometers have two supplies, VDD and VDDIO. > > Add some rudimentary support to obtain and enable these > > regulators during probe() and disable them during remove() > > or on the errorpath. > > > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > All looks good to me. Will leave it for a bit longer though to let > Rob have a chance to look at the dt binding doc. > > As ever, if it seems to have gotten lost it is probably worth > poking me. It's been another week and the bindings are simple enough I think? There has been plenty of time to review these bindings as they are now in v3 even, but it's a soft measure, I know there is usually this YAML-bot that complains if something is way out of line at least. Yours, Linus Walleij
On Sat, 28 Nov 2020 12:44:26 +0100 Linus Walleij <linus.walleij@linaro.org> wrote: > On Sat, Nov 21, 2020 at 5:45 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Sun, 15 Nov 2020 21:57:45 +0100 > > Linus Walleij <linus.walleij@linaro.org> wrote: > > > > > These Bosch accelerometers have two supplies, VDD and VDDIO. > > > Add some rudimentary support to obtain and enable these > > > regulators during probe() and disable them during remove() > > > or on the errorpath. > > > > > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > > All looks good to me. Will leave it for a bit longer though to let > > Rob have a chance to look at the dt binding doc. > > > > As ever, if it seems to have gotten lost it is probably worth > > poking me. > > It's been another week and the bindings are simple enough > I think? There has been plenty of time to review these > bindings as they are now in v3 even, but it's a soft measure, > I know there is usually this YAML-bot that complains if > something is way out of line at least. Agreed. No need to bother Rob with this one if he's busy. It's had 2 weeks in it's final patch version. Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to poke at it. Thanks, Jonathan > > Yours, > Linus Walleij
diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c index 01dbc01e328f..caa984612e88 100644 --- a/drivers/iio/accel/bmc150-accel-core.c +++ b/drivers/iio/accel/bmc150-accel-core.c @@ -28,6 +28,7 @@ #include <linux/iio/trigger_consumer.h> #include <linux/iio/triggered_buffer.h> #include <linux/regmap.h> +#include <linux/regulator/consumer.h> #include "bmc150-accel.h" @@ -184,6 +185,7 @@ enum bmc150_accel_trigger_id { struct bmc150_accel_data { struct regmap *regmap; + struct regulator_bulk_data regulators[2]; int irq; struct bmc150_accel_interrupt interrupts[BMC150_ACCEL_INTERRUPTS]; struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS]; @@ -1591,10 +1593,33 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq, &data->orientation); if (ret) return ret; + /* + * VDD is the analog and digital domain voltage supply + * VDDIO is the digital I/O voltage supply + */ + data->regulators[0].supply = "vdd"; + data->regulators[1].supply = "vddio"; + ret = devm_regulator_bulk_get(dev, + ARRAY_SIZE(data->regulators), + data->regulators); + if (ret) + return dev_err_probe(dev, ret, "failed to get regulators\n"); + + ret = regulator_bulk_enable(ARRAY_SIZE(data->regulators), + data->regulators); + if (ret) { + dev_err(dev, "failed to enable regulators: %d\n", ret); + return ret; + } + /* + * 2ms or 3ms power-on time according to datasheets, let's better + * be safe than sorry and set this delay to 5ms. + */ + msleep(5); ret = bmc150_accel_chip_init(data); if (ret < 0) - return ret; + goto err_disable_regulators; mutex_init(&data->mutex); @@ -1611,7 +1636,7 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq, &bmc150_accel_buffer_ops); if (ret < 0) { dev_err(dev, "Failed: iio triggered buffer setup\n"); - return ret; + goto err_disable_regulators; } if (data->irq > 0) { @@ -1672,6 +1697,9 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq, bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1); err_buffer_cleanup: iio_triggered_buffer_cleanup(indio_dev); +err_disable_regulators: + regulator_bulk_disable(ARRAY_SIZE(data->regulators), + data->regulators); return ret; } @@ -1696,6 +1724,9 @@ int bmc150_accel_core_remove(struct device *dev) bmc150_accel_set_mode(data, BMC150_ACCEL_SLEEP_MODE_DEEP_SUSPEND, 0); mutex_unlock(&data->mutex); + regulator_bulk_disable(ARRAY_SIZE(data->regulators), + data->regulators); + return 0; } EXPORT_SYMBOL_GPL(bmc150_accel_core_remove);
These Bosch accelerometers have two supplies, VDD and VDDIO. Add some rudimentary support to obtain and enable these regulators during probe() and disable them during remove() or on the errorpath. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- ChangeLog v2->v3: - Resend with the rest. ChangeLog v1->v2: - Use dev_err_probe() to simplify the errorpath. --- drivers/iio/accel/bmc150-accel-core.c | 35 +++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-)