| Message ID | 20241010210030.33309-8-vassilisamir@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | : chemical: bme680: 2nd set of clean and add | expand |
On Thu, Oct 10, 2024 at 11:00:24PM +0200, vamoirid wrote: > From: Vasileios Amoiridis <vassilisamir@gmail.com> > > Add support for the regulators described in the dt-binding. ... > +static const char *const bme680_supply_names[] = { > + "vdd", "vddio" Leave trailing comma. > +}; ... > + devm_add_action_or_reset(dev, bme680_regulators_disable, data->supplies); No error check?! Why?
On Fri, Oct 11, 2024 at 01:05:06PM +0300, Andy Shevchenko wrote: > On Thu, Oct 10, 2024 at 11:00:24PM +0200, vamoirid wrote: > > From: Vasileios Amoiridis <vassilisamir@gmail.com> > > > > Add support for the regulators described in the dt-binding. > > ... > > > +static const char *const bme680_supply_names[] = { > > + "vdd", "vddio" > > Leave trailing comma. > ACK. > > +}; > > ... > > > + devm_add_action_or_reset(dev, bme680_regulators_disable, data->supplies); > > No error check?! Why? > You are right, I should put it, I probably didnit see it! > -- > With Best Regards, > Andy Shevchenko > > Cheers, Vasilis
On Thu, 10 Oct 2024 23:00:24 +0200 vamoirid <vassilisamir@gmail.com> wrote: > From: Vasileios Amoiridis <vassilisamir@gmail.com> > > Add support for the regulators described in the dt-binding. > > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> > --- > drivers/iio/chemical/bme680_core.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c > index dedb7edaf43d..a2039b966f20 100644 > --- a/drivers/iio/chemical/bme680_core.c > +++ b/drivers/iio/chemical/bme680_core.c > @@ -15,6 +15,7 @@ > #include <linux/log2.h> > #include <linux/module.h> > #include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > > #include <linux/iio/iio.h> > #include <linux/iio/sysfs.h> > @@ -100,6 +101,12 @@ enum bme680_op_mode { > BME680_FORCED, > }; > > +static const char *const bme680_supply_names[] = { > + "vdd", "vddio" > +}; > + > +#define BME680_NUM_SUPPLIES ARRAY_SIZE(bme680_supply_names) > + > struct bme680_data { > struct regmap *regmap; > struct bme680_calib bme680; > @@ -110,6 +117,8 @@ struct bme680_data { > u16 heater_dur; > u16 heater_temp; > > + struct regulator_bulk_data supplies[BME680_NUM_SUPPLIES]; > + > union { > u8 buf[3]; > unsigned int check; > @@ -857,6 +866,13 @@ static const struct iio_info bme680_info = { > .attrs = &bme680_attribute_group, > }; > > +static void bme680_regulators_disable(void *data) > +{ > + struct regulator_bulk_data *supplies = data; > + > + regulator_bulk_disable(BME680_NUM_SUPPLIES, supplies); > +} > + > int bme680_core_probe(struct device *dev, struct regmap *regmap, > const char *name) > { > @@ -885,6 +901,20 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap, > data->heater_temp = 320; /* degree Celsius */ > data->heater_dur = 150; /* milliseconds */ > > + regulator_bulk_set_supply_names(data->supplies, bme680_supply_names, > + BME680_NUM_SUPPLIES); > + ret = devm_regulator_bulk_get(dev, BME680_NUM_SUPPLIES, data->supplies); devm_regulator_bulk_get_enable() should replace all this with functionally equivalent code. You can also have the supplies list local as you don't then need to hang on to a copy. > + if (ret) > + return dev_err_probe(dev, ret, "failed to get regulators\n"); > + > + ret = regulator_bulk_enable(BME680_NUM_SUPPLIES, data->supplies); > + if (ret) > + return dev_err_probe(dev, ret, "failed to enable regulators\n"); > + > + devm_add_action_or_reset(dev, bme680_regulators_disable, data->supplies); > + > + fsleep(BME680_STARTUP_TIME_US); > + > ret = regmap_write(regmap, BME680_REG_SOFT_RESET, BME680_CMD_SOFTRESET); > if (ret < 0) > return dev_err_probe(dev, ret, "Failed to reset chip\n");
diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c index dedb7edaf43d..a2039b966f20 100644 --- a/drivers/iio/chemical/bme680_core.c +++ b/drivers/iio/chemical/bme680_core.c @@ -15,6 +15,7 @@ #include <linux/log2.h> #include <linux/module.h> #include <linux/regmap.h> +#include <linux/regulator/consumer.h> #include <linux/iio/iio.h> #include <linux/iio/sysfs.h> @@ -100,6 +101,12 @@ enum bme680_op_mode { BME680_FORCED, }; +static const char *const bme680_supply_names[] = { + "vdd", "vddio" +}; + +#define BME680_NUM_SUPPLIES ARRAY_SIZE(bme680_supply_names) + struct bme680_data { struct regmap *regmap; struct bme680_calib bme680; @@ -110,6 +117,8 @@ struct bme680_data { u16 heater_dur; u16 heater_temp; + struct regulator_bulk_data supplies[BME680_NUM_SUPPLIES]; + union { u8 buf[3]; unsigned int check; @@ -857,6 +866,13 @@ static const struct iio_info bme680_info = { .attrs = &bme680_attribute_group, }; +static void bme680_regulators_disable(void *data) +{ + struct regulator_bulk_data *supplies = data; + + regulator_bulk_disable(BME680_NUM_SUPPLIES, supplies); +} + int bme680_core_probe(struct device *dev, struct regmap *regmap, const char *name) { @@ -885,6 +901,20 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap, data->heater_temp = 320; /* degree Celsius */ data->heater_dur = 150; /* milliseconds */ + regulator_bulk_set_supply_names(data->supplies, bme680_supply_names, + BME680_NUM_SUPPLIES); + ret = devm_regulator_bulk_get(dev, BME680_NUM_SUPPLIES, data->supplies); + if (ret) + return dev_err_probe(dev, ret, "failed to get regulators\n"); + + ret = regulator_bulk_enable(BME680_NUM_SUPPLIES, data->supplies); + if (ret) + return dev_err_probe(dev, ret, "failed to enable regulators\n"); + + devm_add_action_or_reset(dev, bme680_regulators_disable, data->supplies); + + fsleep(BME680_STARTUP_TIME_US); + ret = regmap_write(regmap, BME680_REG_SOFT_RESET, BME680_CMD_SOFTRESET); if (ret < 0) return dev_err_probe(dev, ret, "Failed to reset chip\n");