* [PATCH v2] iio: pressure: bmp280: use devm action and remove labels from probe
@ 2019-10-07 2:41 Bartosz Golaszewski
2019-10-12 13:37 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Bartosz Golaszewski @ 2019-10-07 2:41 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler
Cc: linux-iio, linux-kernel, Bartosz Golaszewski
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
We can drop some duplicate code if we use devm_action for disabling
regulators and pm and the managed variant of iio_device_register().
This allows us to completely remove all remove() callbacks from both
i2c and spi code.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
v1 -> v2:
- squash the patches using devm_iio_device_register() and devm_action
to keep the changes bisectable
drivers/iio/pressure/bmp280-core.c | 62 +++++++++++++++---------------
drivers/iio/pressure/bmp280-i2c.c | 6 ---
drivers/iio/pressure/bmp280-spi.c | 6 ---
drivers/iio/pressure/bmp280.h | 1 -
4 files changed, 30 insertions(+), 45 deletions(-)
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index c2988dbdb1a7..79254dd26dfd 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -984,6 +984,22 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
return 0;
}
+static void bmp280_pm_disable(void *data)
+{
+ struct device *dev = data;
+
+ pm_runtime_get_sync(dev);
+ pm_runtime_put_noidle(dev);
+ pm_runtime_disable(dev);
+}
+
+static void bmp280_regulators_disable(void *data)
+{
+ struct regulator_bulk_data *supplies = data;
+
+ regulator_bulk_disable(BMP280_NUM_SUPPLIES, supplies);
+}
+
int bmp280_common_probe(struct device *dev,
struct regmap *regmap,
unsigned int chip,
@@ -1055,6 +1071,11 @@ int bmp280_common_probe(struct device *dev,
return ret;
}
+ ret = devm_add_action_or_reset(dev, bmp280_regulators_disable,
+ data->supplies);
+ if (ret)
+ return ret;
+
/* Wait to make sure we started up properly */
usleep_range(data->start_up_time, data->start_up_time + 100);
@@ -1069,17 +1090,16 @@ int bmp280_common_probe(struct device *dev,
data->regmap = regmap;
ret = regmap_read(regmap, BMP280_REG_ID, &chip_id);
if (ret < 0)
- goto out_disable_regulators;
+ return ret;
if (chip_id != chip) {
dev_err(dev, "bad chip id: expected %x got %x\n",
chip, chip_id);
- ret = -EINVAL;
- goto out_disable_regulators;
+ return -EINVAL;
}
ret = data->chip_info->chip_config(data);
if (ret < 0)
- goto out_disable_regulators;
+ return ret;
dev_set_drvdata(dev, indio_dev);
@@ -1093,14 +1113,14 @@ int bmp280_common_probe(struct device *dev,
if (ret < 0) {
dev_err(data->dev,
"failed to read calibration coefficients\n");
- goto out_disable_regulators;
+ return ret;
}
} else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
if (ret < 0) {
dev_err(data->dev,
"failed to read calibration coefficients\n");
- goto out_disable_regulators;
+ return ret;
}
}
@@ -1112,7 +1132,7 @@ int bmp280_common_probe(struct device *dev,
if (irq > 0 || (chip_id == BMP180_CHIP_ID)) {
ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
if (ret)
- goto out_disable_regulators;
+ return ret;
}
/* Enable runtime PM */
@@ -1127,36 +1147,14 @@ int bmp280_common_probe(struct device *dev,
pm_runtime_use_autosuspend(dev);
pm_runtime_put(dev);
- ret = iio_device_register(indio_dev);
+ ret = devm_add_action_or_reset(dev, bmp280_pm_disable, dev);
if (ret)
- goto out_runtime_pm_disable;
-
- return 0;
+ return ret;
-out_runtime_pm_disable:
- pm_runtime_get_sync(data->dev);
- pm_runtime_put_noidle(data->dev);
- pm_runtime_disable(data->dev);
-out_disable_regulators:
- regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
- return ret;
+ return devm_iio_device_register(dev, indio_dev);
}
EXPORT_SYMBOL(bmp280_common_probe);
-int bmp280_common_remove(struct device *dev)
-{
- struct iio_dev *indio_dev = dev_get_drvdata(dev);
- struct bmp280_data *data = iio_priv(indio_dev);
-
- iio_device_unregister(indio_dev);
- pm_runtime_get_sync(data->dev);
- pm_runtime_put_noidle(data->dev);
- pm_runtime_disable(data->dev);
- regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
- return 0;
-}
-EXPORT_SYMBOL(bmp280_common_remove);
-
#ifdef CONFIG_PM
static int bmp280_runtime_suspend(struct device *dev)
{
diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
index acd9a3784fb4..3109c8e2cc11 100644
--- a/drivers/iio/pressure/bmp280-i2c.c
+++ b/drivers/iio/pressure/bmp280-i2c.c
@@ -38,11 +38,6 @@ static int bmp280_i2c_probe(struct i2c_client *client,
client->irq);
}
-static int bmp280_i2c_remove(struct i2c_client *client)
-{
- return bmp280_common_remove(&client->dev);
-}
-
static const struct acpi_device_id bmp280_acpi_i2c_match[] = {
{"BMP0280", BMP280_CHIP_ID },
{"BMP0180", BMP180_CHIP_ID },
@@ -82,7 +77,6 @@ static struct i2c_driver bmp280_i2c_driver = {
.pm = &bmp280_dev_pm_ops,
},
.probe = bmp280_i2c_probe,
- .remove = bmp280_i2c_remove,
.id_table = bmp280_i2c_id,
};
module_i2c_driver(bmp280_i2c_driver);
diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
index 9d57b7a3b134..625b86878ad8 100644
--- a/drivers/iio/pressure/bmp280-spi.c
+++ b/drivers/iio/pressure/bmp280-spi.c
@@ -86,11 +86,6 @@ static int bmp280_spi_probe(struct spi_device *spi)
spi->irq);
}
-static int bmp280_spi_remove(struct spi_device *spi)
-{
- return bmp280_common_remove(&spi->dev);
-}
-
static const struct of_device_id bmp280_of_spi_match[] = {
{ .compatible = "bosch,bmp085", },
{ .compatible = "bosch,bmp180", },
@@ -118,7 +113,6 @@ static struct spi_driver bmp280_spi_driver = {
},
.id_table = bmp280_spi_id,
.probe = bmp280_spi_probe,
- .remove = bmp280_spi_remove,
};
module_spi_driver(bmp280_spi_driver);
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index eda50ef65706..57ba0e85db91 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -112,7 +112,6 @@ int bmp280_common_probe(struct device *dev,
unsigned int chip,
const char *name,
int irq);
-int bmp280_common_remove(struct device *dev);
/* PM ops */
extern const struct dev_pm_ops bmp280_dev_pm_ops;
--
2.23.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] iio: pressure: bmp280: use devm action and remove labels from probe
2019-10-07 2:41 [PATCH v2] iio: pressure: bmp280: use devm action and remove labels from probe Bartosz Golaszewski
@ 2019-10-12 13:37 ` Jonathan Cameron
2019-10-21 12:47 ` Bartosz Golaszewski
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2019-10-12 13:37 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
linux-iio, linux-kernel, Bartosz Golaszewski
On Mon, 7 Oct 2019 04:41:31 +0200
Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> We can drop some duplicate code if we use devm_action for disabling
> regulators and pm and the managed variant of iio_device_register().
>
> This allows us to completely remove all remove() callbacks from both
> i2c and spi code.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
This is on top of the bulk regulator patch which is awaiting precusors
getting to my upstream. I'll hold this one as well on that.
If it looks like I've forgotten it then give me a poke.
Thanks,
Jonathan
> ---
> v1 -> v2:
> - squash the patches using devm_iio_device_register() and devm_action
> to keep the changes bisectable
>
> drivers/iio/pressure/bmp280-core.c | 62 +++++++++++++++---------------
> drivers/iio/pressure/bmp280-i2c.c | 6 ---
> drivers/iio/pressure/bmp280-spi.c | 6 ---
> drivers/iio/pressure/bmp280.h | 1 -
> 4 files changed, 30 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index c2988dbdb1a7..79254dd26dfd 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -984,6 +984,22 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
> return 0;
> }
>
> +static void bmp280_pm_disable(void *data)
> +{
> + struct device *dev = data;
> +
> + pm_runtime_get_sync(dev);
> + pm_runtime_put_noidle(dev);
> + pm_runtime_disable(dev);
> +}
> +
> +static void bmp280_regulators_disable(void *data)
> +{
> + struct regulator_bulk_data *supplies = data;
> +
> + regulator_bulk_disable(BMP280_NUM_SUPPLIES, supplies);
> +}
> +
> int bmp280_common_probe(struct device *dev,
> struct regmap *regmap,
> unsigned int chip,
> @@ -1055,6 +1071,11 @@ int bmp280_common_probe(struct device *dev,
> return ret;
> }
>
> + ret = devm_add_action_or_reset(dev, bmp280_regulators_disable,
> + data->supplies);
> + if (ret)
> + return ret;
> +
> /* Wait to make sure we started up properly */
> usleep_range(data->start_up_time, data->start_up_time + 100);
>
> @@ -1069,17 +1090,16 @@ int bmp280_common_probe(struct device *dev,
> data->regmap = regmap;
> ret = regmap_read(regmap, BMP280_REG_ID, &chip_id);
> if (ret < 0)
> - goto out_disable_regulators;
> + return ret;
> if (chip_id != chip) {
> dev_err(dev, "bad chip id: expected %x got %x\n",
> chip, chip_id);
> - ret = -EINVAL;
> - goto out_disable_regulators;
> + return -EINVAL;
> }
>
> ret = data->chip_info->chip_config(data);
> if (ret < 0)
> - goto out_disable_regulators;
> + return ret;
>
> dev_set_drvdata(dev, indio_dev);
>
> @@ -1093,14 +1113,14 @@ int bmp280_common_probe(struct device *dev,
> if (ret < 0) {
> dev_err(data->dev,
> "failed to read calibration coefficients\n");
> - goto out_disable_regulators;
> + return ret;
> }
> } else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
> ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
> if (ret < 0) {
> dev_err(data->dev,
> "failed to read calibration coefficients\n");
> - goto out_disable_regulators;
> + return ret;
> }
> }
>
> @@ -1112,7 +1132,7 @@ int bmp280_common_probe(struct device *dev,
> if (irq > 0 || (chip_id == BMP180_CHIP_ID)) {
> ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
> if (ret)
> - goto out_disable_regulators;
> + return ret;
> }
>
> /* Enable runtime PM */
> @@ -1127,36 +1147,14 @@ int bmp280_common_probe(struct device *dev,
> pm_runtime_use_autosuspend(dev);
> pm_runtime_put(dev);
>
> - ret = iio_device_register(indio_dev);
> + ret = devm_add_action_or_reset(dev, bmp280_pm_disable, dev);
> if (ret)
> - goto out_runtime_pm_disable;
> -
> - return 0;
> + return ret;
>
> -out_runtime_pm_disable:
> - pm_runtime_get_sync(data->dev);
> - pm_runtime_put_noidle(data->dev);
> - pm_runtime_disable(data->dev);
> -out_disable_regulators:
> - regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> - return ret;
> + return devm_iio_device_register(dev, indio_dev);
> }
> EXPORT_SYMBOL(bmp280_common_probe);
>
> -int bmp280_common_remove(struct device *dev)
> -{
> - struct iio_dev *indio_dev = dev_get_drvdata(dev);
> - struct bmp280_data *data = iio_priv(indio_dev);
> -
> - iio_device_unregister(indio_dev);
> - pm_runtime_get_sync(data->dev);
> - pm_runtime_put_noidle(data->dev);
> - pm_runtime_disable(data->dev);
> - regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> - return 0;
> -}
> -EXPORT_SYMBOL(bmp280_common_remove);
> -
> #ifdef CONFIG_PM
> static int bmp280_runtime_suspend(struct device *dev)
> {
> diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
> index acd9a3784fb4..3109c8e2cc11 100644
> --- a/drivers/iio/pressure/bmp280-i2c.c
> +++ b/drivers/iio/pressure/bmp280-i2c.c
> @@ -38,11 +38,6 @@ static int bmp280_i2c_probe(struct i2c_client *client,
> client->irq);
> }
>
> -static int bmp280_i2c_remove(struct i2c_client *client)
> -{
> - return bmp280_common_remove(&client->dev);
> -}
> -
> static const struct acpi_device_id bmp280_acpi_i2c_match[] = {
> {"BMP0280", BMP280_CHIP_ID },
> {"BMP0180", BMP180_CHIP_ID },
> @@ -82,7 +77,6 @@ static struct i2c_driver bmp280_i2c_driver = {
> .pm = &bmp280_dev_pm_ops,
> },
> .probe = bmp280_i2c_probe,
> - .remove = bmp280_i2c_remove,
> .id_table = bmp280_i2c_id,
> };
> module_i2c_driver(bmp280_i2c_driver);
> diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
> index 9d57b7a3b134..625b86878ad8 100644
> --- a/drivers/iio/pressure/bmp280-spi.c
> +++ b/drivers/iio/pressure/bmp280-spi.c
> @@ -86,11 +86,6 @@ static int bmp280_spi_probe(struct spi_device *spi)
> spi->irq);
> }
>
> -static int bmp280_spi_remove(struct spi_device *spi)
> -{
> - return bmp280_common_remove(&spi->dev);
> -}
> -
> static const struct of_device_id bmp280_of_spi_match[] = {
> { .compatible = "bosch,bmp085", },
> { .compatible = "bosch,bmp180", },
> @@ -118,7 +113,6 @@ static struct spi_driver bmp280_spi_driver = {
> },
> .id_table = bmp280_spi_id,
> .probe = bmp280_spi_probe,
> - .remove = bmp280_spi_remove,
> };
> module_spi_driver(bmp280_spi_driver);
>
> diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> index eda50ef65706..57ba0e85db91 100644
> --- a/drivers/iio/pressure/bmp280.h
> +++ b/drivers/iio/pressure/bmp280.h
> @@ -112,7 +112,6 @@ int bmp280_common_probe(struct device *dev,
> unsigned int chip,
> const char *name,
> int irq);
> -int bmp280_common_remove(struct device *dev);
>
> /* PM ops */
> extern const struct dev_pm_ops bmp280_dev_pm_ops;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] iio: pressure: bmp280: use devm action and remove labels from probe
2019-10-12 13:37 ` Jonathan Cameron
@ 2019-10-21 12:47 ` Bartosz Golaszewski
2019-10-22 10:15 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Bartosz Golaszewski @ 2019-10-21 12:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
linux-iio, Linux Kernel Mailing List, Bartosz Golaszewski
sob., 12 paź 2019 o 15:37 Jonathan Cameron <jic23@kernel.org> napisał(a):
>
> On Mon, 7 Oct 2019 04:41:31 +0200
> Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > We can drop some duplicate code if we use devm_action for disabling
> > regulators and pm and the managed variant of iio_device_register().
> >
> > This allows us to completely remove all remove() callbacks from both
> > i2c and spi code.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> This is on top of the bulk regulator patch which is awaiting precusors
> getting to my upstream. I'll hold this one as well on that.
>
> If it looks like I've forgotten it then give me a poke.
>
Hi Jonathan,
gentle poke after v5.4-rc4. Only one of the three patches is in next
so far, the one using bulk regulators is missing too besides this one.
Best regards,
Bartosz Golaszewski
> Thanks,
>
> Jonathan
>
> > ---
> > v1 -> v2:
> > - squash the patches using devm_iio_device_register() and devm_action
> > to keep the changes bisectable
> >
> > drivers/iio/pressure/bmp280-core.c | 62 +++++++++++++++---------------
> > drivers/iio/pressure/bmp280-i2c.c | 6 ---
> > drivers/iio/pressure/bmp280-spi.c | 6 ---
> > drivers/iio/pressure/bmp280.h | 1 -
> > 4 files changed, 30 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > index c2988dbdb1a7..79254dd26dfd 100644
> > --- a/drivers/iio/pressure/bmp280-core.c
> > +++ b/drivers/iio/pressure/bmp280-core.c
> > @@ -984,6 +984,22 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
> > return 0;
> > }
> >
> > +static void bmp280_pm_disable(void *data)
> > +{
> > + struct device *dev = data;
> > +
> > + pm_runtime_get_sync(dev);
> > + pm_runtime_put_noidle(dev);
> > + pm_runtime_disable(dev);
> > +}
> > +
> > +static void bmp280_regulators_disable(void *data)
> > +{
> > + struct regulator_bulk_data *supplies = data;
> > +
> > + regulator_bulk_disable(BMP280_NUM_SUPPLIES, supplies);
> > +}
> > +
> > int bmp280_common_probe(struct device *dev,
> > struct regmap *regmap,
> > unsigned int chip,
> > @@ -1055,6 +1071,11 @@ int bmp280_common_probe(struct device *dev,
> > return ret;
> > }
> >
> > + ret = devm_add_action_or_reset(dev, bmp280_regulators_disable,
> > + data->supplies);
> > + if (ret)
> > + return ret;
> > +
> > /* Wait to make sure we started up properly */
> > usleep_range(data->start_up_time, data->start_up_time + 100);
> >
> > @@ -1069,17 +1090,16 @@ int bmp280_common_probe(struct device *dev,
> > data->regmap = regmap;
> > ret = regmap_read(regmap, BMP280_REG_ID, &chip_id);
> > if (ret < 0)
> > - goto out_disable_regulators;
> > + return ret;
> > if (chip_id != chip) {
> > dev_err(dev, "bad chip id: expected %x got %x\n",
> > chip, chip_id);
> > - ret = -EINVAL;
> > - goto out_disable_regulators;
> > + return -EINVAL;
> > }
> >
> > ret = data->chip_info->chip_config(data);
> > if (ret < 0)
> > - goto out_disable_regulators;
> > + return ret;
> >
> > dev_set_drvdata(dev, indio_dev);
> >
> > @@ -1093,14 +1113,14 @@ int bmp280_common_probe(struct device *dev,
> > if (ret < 0) {
> > dev_err(data->dev,
> > "failed to read calibration coefficients\n");
> > - goto out_disable_regulators;
> > + return ret;
> > }
> > } else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
> > ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
> > if (ret < 0) {
> > dev_err(data->dev,
> > "failed to read calibration coefficients\n");
> > - goto out_disable_regulators;
> > + return ret;
> > }
> > }
> >
> > @@ -1112,7 +1132,7 @@ int bmp280_common_probe(struct device *dev,
> > if (irq > 0 || (chip_id == BMP180_CHIP_ID)) {
> > ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
> > if (ret)
> > - goto out_disable_regulators;
> > + return ret;
> > }
> >
> > /* Enable runtime PM */
> > @@ -1127,36 +1147,14 @@ int bmp280_common_probe(struct device *dev,
> > pm_runtime_use_autosuspend(dev);
> > pm_runtime_put(dev);
> >
> > - ret = iio_device_register(indio_dev);
> > + ret = devm_add_action_or_reset(dev, bmp280_pm_disable, dev);
> > if (ret)
> > - goto out_runtime_pm_disable;
> > -
> > - return 0;
> > + return ret;
> >
> > -out_runtime_pm_disable:
> > - pm_runtime_get_sync(data->dev);
> > - pm_runtime_put_noidle(data->dev);
> > - pm_runtime_disable(data->dev);
> > -out_disable_regulators:
> > - regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> > - return ret;
> > + return devm_iio_device_register(dev, indio_dev);
> > }
> > EXPORT_SYMBOL(bmp280_common_probe);
> >
> > -int bmp280_common_remove(struct device *dev)
> > -{
> > - struct iio_dev *indio_dev = dev_get_drvdata(dev);
> > - struct bmp280_data *data = iio_priv(indio_dev);
> > -
> > - iio_device_unregister(indio_dev);
> > - pm_runtime_get_sync(data->dev);
> > - pm_runtime_put_noidle(data->dev);
> > - pm_runtime_disable(data->dev);
> > - regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> > - return 0;
> > -}
> > -EXPORT_SYMBOL(bmp280_common_remove);
> > -
> > #ifdef CONFIG_PM
> > static int bmp280_runtime_suspend(struct device *dev)
> > {
> > diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
> > index acd9a3784fb4..3109c8e2cc11 100644
> > --- a/drivers/iio/pressure/bmp280-i2c.c
> > +++ b/drivers/iio/pressure/bmp280-i2c.c
> > @@ -38,11 +38,6 @@ static int bmp280_i2c_probe(struct i2c_client *client,
> > client->irq);
> > }
> >
> > -static int bmp280_i2c_remove(struct i2c_client *client)
> > -{
> > - return bmp280_common_remove(&client->dev);
> > -}
> > -
> > static const struct acpi_device_id bmp280_acpi_i2c_match[] = {
> > {"BMP0280", BMP280_CHIP_ID },
> > {"BMP0180", BMP180_CHIP_ID },
> > @@ -82,7 +77,6 @@ static struct i2c_driver bmp280_i2c_driver = {
> > .pm = &bmp280_dev_pm_ops,
> > },
> > .probe = bmp280_i2c_probe,
> > - .remove = bmp280_i2c_remove,
> > .id_table = bmp280_i2c_id,
> > };
> > module_i2c_driver(bmp280_i2c_driver);
> > diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
> > index 9d57b7a3b134..625b86878ad8 100644
> > --- a/drivers/iio/pressure/bmp280-spi.c
> > +++ b/drivers/iio/pressure/bmp280-spi.c
> > @@ -86,11 +86,6 @@ static int bmp280_spi_probe(struct spi_device *spi)
> > spi->irq);
> > }
> >
> > -static int bmp280_spi_remove(struct spi_device *spi)
> > -{
> > - return bmp280_common_remove(&spi->dev);
> > -}
> > -
> > static const struct of_device_id bmp280_of_spi_match[] = {
> > { .compatible = "bosch,bmp085", },
> > { .compatible = "bosch,bmp180", },
> > @@ -118,7 +113,6 @@ static struct spi_driver bmp280_spi_driver = {
> > },
> > .id_table = bmp280_spi_id,
> > .probe = bmp280_spi_probe,
> > - .remove = bmp280_spi_remove,
> > };
> > module_spi_driver(bmp280_spi_driver);
> >
> > diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> > index eda50ef65706..57ba0e85db91 100644
> > --- a/drivers/iio/pressure/bmp280.h
> > +++ b/drivers/iio/pressure/bmp280.h
> > @@ -112,7 +112,6 @@ int bmp280_common_probe(struct device *dev,
> > unsigned int chip,
> > const char *name,
> > int irq);
> > -int bmp280_common_remove(struct device *dev);
> >
> > /* PM ops */
> > extern const struct dev_pm_ops bmp280_dev_pm_ops;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] iio: pressure: bmp280: use devm action and remove labels from probe
2019-10-21 12:47 ` Bartosz Golaszewski
@ 2019-10-22 10:15 ` Jonathan Cameron
2019-10-31 6:46 ` Bartosz Golaszewski
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2019-10-22 10:15 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
linux-iio, Linux Kernel Mailing List, Bartosz Golaszewski
On Mon, 21 Oct 2019 14:47:18 +0200
Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> sob., 12 paź 2019 o 15:37 Jonathan Cameron <jic23@kernel.org> napisał(a):
> >
> > On Mon, 7 Oct 2019 04:41:31 +0200
> > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >
> > > We can drop some duplicate code if we use devm_action for disabling
> > > regulators and pm and the managed variant of iio_device_register().
> > >
> > > This allows us to completely remove all remove() callbacks from both
> > > i2c and spi code.
> > >
> > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > This is on top of the bulk regulator patch which is awaiting precusors
> > getting to my upstream. I'll hold this one as well on that.
> >
> > If it looks like I've forgotten it then give me a poke.
> >
>
> Hi Jonathan,
>
> gentle poke after v5.4-rc4. Only one of the three patches is in next
> so far, the one using bulk regulators is missing too besides this one.
Thanks for the reminder.
Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to play with it.
Thanks,
Jonathan
>
> Best regards,
> Bartosz Golaszewski
>
> > Thanks,
> >
> > Jonathan
> >
> > > ---
> > > v1 -> v2:
> > > - squash the patches using devm_iio_device_register() and devm_action
> > > to keep the changes bisectable
> > >
> > > drivers/iio/pressure/bmp280-core.c | 62 +++++++++++++++---------------
> > > drivers/iio/pressure/bmp280-i2c.c | 6 ---
> > > drivers/iio/pressure/bmp280-spi.c | 6 ---
> > > drivers/iio/pressure/bmp280.h | 1 -
> > > 4 files changed, 30 insertions(+), 45 deletions(-)
> > >
> > > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > > index c2988dbdb1a7..79254dd26dfd 100644
> > > --- a/drivers/iio/pressure/bmp280-core.c
> > > +++ b/drivers/iio/pressure/bmp280-core.c
> > > @@ -984,6 +984,22 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
> > > return 0;
> > > }
> > >
> > > +static void bmp280_pm_disable(void *data)
> > > +{
> > > + struct device *dev = data;
> > > +
> > > + pm_runtime_get_sync(dev);
> > > + pm_runtime_put_noidle(dev);
> > > + pm_runtime_disable(dev);
> > > +}
> > > +
> > > +static void bmp280_regulators_disable(void *data)
> > > +{
> > > + struct regulator_bulk_data *supplies = data;
> > > +
> > > + regulator_bulk_disable(BMP280_NUM_SUPPLIES, supplies);
> > > +}
> > > +
> > > int bmp280_common_probe(struct device *dev,
> > > struct regmap *regmap,
> > > unsigned int chip,
> > > @@ -1055,6 +1071,11 @@ int bmp280_common_probe(struct device *dev,
> > > return ret;
> > > }
> > >
> > > + ret = devm_add_action_or_reset(dev, bmp280_regulators_disable,
> > > + data->supplies);
> > > + if (ret)
> > > + return ret;
> > > +
> > > /* Wait to make sure we started up properly */
> > > usleep_range(data->start_up_time, data->start_up_time + 100);
> > >
> > > @@ -1069,17 +1090,16 @@ int bmp280_common_probe(struct device *dev,
> > > data->regmap = regmap;
> > > ret = regmap_read(regmap, BMP280_REG_ID, &chip_id);
> > > if (ret < 0)
> > > - goto out_disable_regulators;
> > > + return ret;
> > > if (chip_id != chip) {
> > > dev_err(dev, "bad chip id: expected %x got %x\n",
> > > chip, chip_id);
> > > - ret = -EINVAL;
> > > - goto out_disable_regulators;
> > > + return -EINVAL;
> > > }
> > >
> > > ret = data->chip_info->chip_config(data);
> > > if (ret < 0)
> > > - goto out_disable_regulators;
> > > + return ret;
> > >
> > > dev_set_drvdata(dev, indio_dev);
> > >
> > > @@ -1093,14 +1113,14 @@ int bmp280_common_probe(struct device *dev,
> > > if (ret < 0) {
> > > dev_err(data->dev,
> > > "failed to read calibration coefficients\n");
> > > - goto out_disable_regulators;
> > > + return ret;
> > > }
> > > } else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
> > > ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
> > > if (ret < 0) {
> > > dev_err(data->dev,
> > > "failed to read calibration coefficients\n");
> > > - goto out_disable_regulators;
> > > + return ret;
> > > }
> > > }
> > >
> > > @@ -1112,7 +1132,7 @@ int bmp280_common_probe(struct device *dev,
> > > if (irq > 0 || (chip_id == BMP180_CHIP_ID)) {
> > > ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
> > > if (ret)
> > > - goto out_disable_regulators;
> > > + return ret;
> > > }
> > >
> > > /* Enable runtime PM */
> > > @@ -1127,36 +1147,14 @@ int bmp280_common_probe(struct device *dev,
> > > pm_runtime_use_autosuspend(dev);
> > > pm_runtime_put(dev);
> > >
> > > - ret = iio_device_register(indio_dev);
> > > + ret = devm_add_action_or_reset(dev, bmp280_pm_disable, dev);
> > > if (ret)
> > > - goto out_runtime_pm_disable;
> > > -
> > > - return 0;
> > > + return ret;
> > >
> > > -out_runtime_pm_disable:
> > > - pm_runtime_get_sync(data->dev);
> > > - pm_runtime_put_noidle(data->dev);
> > > - pm_runtime_disable(data->dev);
> > > -out_disable_regulators:
> > > - regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> > > - return ret;
> > > + return devm_iio_device_register(dev, indio_dev);
> > > }
> > > EXPORT_SYMBOL(bmp280_common_probe);
> > >
> > > -int bmp280_common_remove(struct device *dev)
> > > -{
> > > - struct iio_dev *indio_dev = dev_get_drvdata(dev);
> > > - struct bmp280_data *data = iio_priv(indio_dev);
> > > -
> > > - iio_device_unregister(indio_dev);
> > > - pm_runtime_get_sync(data->dev);
> > > - pm_runtime_put_noidle(data->dev);
> > > - pm_runtime_disable(data->dev);
> > > - regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> > > - return 0;
> > > -}
> > > -EXPORT_SYMBOL(bmp280_common_remove);
> > > -
> > > #ifdef CONFIG_PM
> > > static int bmp280_runtime_suspend(struct device *dev)
> > > {
> > > diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
> > > index acd9a3784fb4..3109c8e2cc11 100644
> > > --- a/drivers/iio/pressure/bmp280-i2c.c
> > > +++ b/drivers/iio/pressure/bmp280-i2c.c
> > > @@ -38,11 +38,6 @@ static int bmp280_i2c_probe(struct i2c_client *client,
> > > client->irq);
> > > }
> > >
> > > -static int bmp280_i2c_remove(struct i2c_client *client)
> > > -{
> > > - return bmp280_common_remove(&client->dev);
> > > -}
> > > -
> > > static const struct acpi_device_id bmp280_acpi_i2c_match[] = {
> > > {"BMP0280", BMP280_CHIP_ID },
> > > {"BMP0180", BMP180_CHIP_ID },
> > > @@ -82,7 +77,6 @@ static struct i2c_driver bmp280_i2c_driver = {
> > > .pm = &bmp280_dev_pm_ops,
> > > },
> > > .probe = bmp280_i2c_probe,
> > > - .remove = bmp280_i2c_remove,
> > > .id_table = bmp280_i2c_id,
> > > };
> > > module_i2c_driver(bmp280_i2c_driver);
> > > diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
> > > index 9d57b7a3b134..625b86878ad8 100644
> > > --- a/drivers/iio/pressure/bmp280-spi.c
> > > +++ b/drivers/iio/pressure/bmp280-spi.c
> > > @@ -86,11 +86,6 @@ static int bmp280_spi_probe(struct spi_device *spi)
> > > spi->irq);
> > > }
> > >
> > > -static int bmp280_spi_remove(struct spi_device *spi)
> > > -{
> > > - return bmp280_common_remove(&spi->dev);
> > > -}
> > > -
> > > static const struct of_device_id bmp280_of_spi_match[] = {
> > > { .compatible = "bosch,bmp085", },
> > > { .compatible = "bosch,bmp180", },
> > > @@ -118,7 +113,6 @@ static struct spi_driver bmp280_spi_driver = {
> > > },
> > > .id_table = bmp280_spi_id,
> > > .probe = bmp280_spi_probe,
> > > - .remove = bmp280_spi_remove,
> > > };
> > > module_spi_driver(bmp280_spi_driver);
> > >
> > > diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> > > index eda50ef65706..57ba0e85db91 100644
> > > --- a/drivers/iio/pressure/bmp280.h
> > > +++ b/drivers/iio/pressure/bmp280.h
> > > @@ -112,7 +112,6 @@ int bmp280_common_probe(struct device *dev,
> > > unsigned int chip,
> > > const char *name,
> > > int irq);
> > > -int bmp280_common_remove(struct device *dev);
> > >
> > > /* PM ops */
> > > extern const struct dev_pm_ops bmp280_dev_pm_ops;
> >
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] iio: pressure: bmp280: use devm action and remove labels from probe
2019-10-22 10:15 ` Jonathan Cameron
@ 2019-10-31 6:46 ` Bartosz Golaszewski
2019-11-02 14:22 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Bartosz Golaszewski @ 2019-10-31 6:46 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
linux-iio, Linux Kernel Mailing List, Bartosz Golaszewski
wt., 22 paź 2019 o 12:15 Jonathan Cameron <jic23@kernel.org> napisał(a):
>
> On Mon, 21 Oct 2019 14:47:18 +0200
> Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > sob., 12 paź 2019 o 15:37 Jonathan Cameron <jic23@kernel.org> napisał(a):
> > >
> > > On Mon, 7 Oct 2019 04:41:31 +0200
> > > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
> > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > >
> > > > We can drop some duplicate code if we use devm_action for disabling
> > > > regulators and pm and the managed variant of iio_device_register().
> > > >
> > > > This allows us to completely remove all remove() callbacks from both
> > > > i2c and spi code.
> > > >
> > > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > This is on top of the bulk regulator patch which is awaiting precusors
> > > getting to my upstream. I'll hold this one as well on that.
> > >
> > > If it looks like I've forgotten it then give me a poke.
> > >
> >
> > Hi Jonathan,
> >
> > gentle poke after v5.4-rc4. Only one of the three patches is in next
> > so far, the one using bulk regulators is missing too besides this one.
> Thanks for the reminder.
>
> Applied to the togreg branch of iio.git and pushed out as testing for the
> autobuilders to play with it.
>
Hi,
I don't see it in the togreg branch of the iio tree - did something go
wrong with testing?
Bart
> Thanks,
>
> Jonathan
> >
> > Best regards,
> > Bartosz Golaszewski
> >
> > > Thanks,
> > >
> > > Jonathan
> > >
> > > > ---
> > > > v1 -> v2:
> > > > - squash the patches using devm_iio_device_register() and devm_action
> > > > to keep the changes bisectable
> > > >
> > > > drivers/iio/pressure/bmp280-core.c | 62 +++++++++++++++---------------
> > > > drivers/iio/pressure/bmp280-i2c.c | 6 ---
> > > > drivers/iio/pressure/bmp280-spi.c | 6 ---
> > > > drivers/iio/pressure/bmp280.h | 1 -
> > > > 4 files changed, 30 insertions(+), 45 deletions(-)
> > > >
> > > > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > > > index c2988dbdb1a7..79254dd26dfd 100644
> > > > --- a/drivers/iio/pressure/bmp280-core.c
> > > > +++ b/drivers/iio/pressure/bmp280-core.c
> > > > @@ -984,6 +984,22 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
> > > > return 0;
> > > > }
> > > >
> > > > +static void bmp280_pm_disable(void *data)
> > > > +{
> > > > + struct device *dev = data;
> > > > +
> > > > + pm_runtime_get_sync(dev);
> > > > + pm_runtime_put_noidle(dev);
> > > > + pm_runtime_disable(dev);
> > > > +}
> > > > +
> > > > +static void bmp280_regulators_disable(void *data)
> > > > +{
> > > > + struct regulator_bulk_data *supplies = data;
> > > > +
> > > > + regulator_bulk_disable(BMP280_NUM_SUPPLIES, supplies);
> > > > +}
> > > > +
> > > > int bmp280_common_probe(struct device *dev,
> > > > struct regmap *regmap,
> > > > unsigned int chip,
> > > > @@ -1055,6 +1071,11 @@ int bmp280_common_probe(struct device *dev,
> > > > return ret;
> > > > }
> > > >
> > > > + ret = devm_add_action_or_reset(dev, bmp280_regulators_disable,
> > > > + data->supplies);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > /* Wait to make sure we started up properly */
> > > > usleep_range(data->start_up_time, data->start_up_time + 100);
> > > >
> > > > @@ -1069,17 +1090,16 @@ int bmp280_common_probe(struct device *dev,
> > > > data->regmap = regmap;
> > > > ret = regmap_read(regmap, BMP280_REG_ID, &chip_id);
> > > > if (ret < 0)
> > > > - goto out_disable_regulators;
> > > > + return ret;
> > > > if (chip_id != chip) {
> > > > dev_err(dev, "bad chip id: expected %x got %x\n",
> > > > chip, chip_id);
> > > > - ret = -EINVAL;
> > > > - goto out_disable_regulators;
> > > > + return -EINVAL;
> > > > }
> > > >
> > > > ret = data->chip_info->chip_config(data);
> > > > if (ret < 0)
> > > > - goto out_disable_regulators;
> > > > + return ret;
> > > >
> > > > dev_set_drvdata(dev, indio_dev);
> > > >
> > > > @@ -1093,14 +1113,14 @@ int bmp280_common_probe(struct device *dev,
> > > > if (ret < 0) {
> > > > dev_err(data->dev,
> > > > "failed to read calibration coefficients\n");
> > > > - goto out_disable_regulators;
> > > > + return ret;
> > > > }
> > > > } else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
> > > > ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
> > > > if (ret < 0) {
> > > > dev_err(data->dev,
> > > > "failed to read calibration coefficients\n");
> > > > - goto out_disable_regulators;
> > > > + return ret;
> > > > }
> > > > }
> > > >
> > > > @@ -1112,7 +1132,7 @@ int bmp280_common_probe(struct device *dev,
> > > > if (irq > 0 || (chip_id == BMP180_CHIP_ID)) {
> > > > ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
> > > > if (ret)
> > > > - goto out_disable_regulators;
> > > > + return ret;
> > > > }
> > > >
> > > > /* Enable runtime PM */
> > > > @@ -1127,36 +1147,14 @@ int bmp280_common_probe(struct device *dev,
> > > > pm_runtime_use_autosuspend(dev);
> > > > pm_runtime_put(dev);
> > > >
> > > > - ret = iio_device_register(indio_dev);
> > > > + ret = devm_add_action_or_reset(dev, bmp280_pm_disable, dev);
> > > > if (ret)
> > > > - goto out_runtime_pm_disable;
> > > > -
> > > > - return 0;
> > > > + return ret;
> > > >
> > > > -out_runtime_pm_disable:
> > > > - pm_runtime_get_sync(data->dev);
> > > > - pm_runtime_put_noidle(data->dev);
> > > > - pm_runtime_disable(data->dev);
> > > > -out_disable_regulators:
> > > > - regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> > > > - return ret;
> > > > + return devm_iio_device_register(dev, indio_dev);
> > > > }
> > > > EXPORT_SYMBOL(bmp280_common_probe);
> > > >
> > > > -int bmp280_common_remove(struct device *dev)
> > > > -{
> > > > - struct iio_dev *indio_dev = dev_get_drvdata(dev);
> > > > - struct bmp280_data *data = iio_priv(indio_dev);
> > > > -
> > > > - iio_device_unregister(indio_dev);
> > > > - pm_runtime_get_sync(data->dev);
> > > > - pm_runtime_put_noidle(data->dev);
> > > > - pm_runtime_disable(data->dev);
> > > > - regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> > > > - return 0;
> > > > -}
> > > > -EXPORT_SYMBOL(bmp280_common_remove);
> > > > -
> > > > #ifdef CONFIG_PM
> > > > static int bmp280_runtime_suspend(struct device *dev)
> > > > {
> > > > diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
> > > > index acd9a3784fb4..3109c8e2cc11 100644
> > > > --- a/drivers/iio/pressure/bmp280-i2c.c
> > > > +++ b/drivers/iio/pressure/bmp280-i2c.c
> > > > @@ -38,11 +38,6 @@ static int bmp280_i2c_probe(struct i2c_client *client,
> > > > client->irq);
> > > > }
> > > >
> > > > -static int bmp280_i2c_remove(struct i2c_client *client)
> > > > -{
> > > > - return bmp280_common_remove(&client->dev);
> > > > -}
> > > > -
> > > > static const struct acpi_device_id bmp280_acpi_i2c_match[] = {
> > > > {"BMP0280", BMP280_CHIP_ID },
> > > > {"BMP0180", BMP180_CHIP_ID },
> > > > @@ -82,7 +77,6 @@ static struct i2c_driver bmp280_i2c_driver = {
> > > > .pm = &bmp280_dev_pm_ops,
> > > > },
> > > > .probe = bmp280_i2c_probe,
> > > > - .remove = bmp280_i2c_remove,
> > > > .id_table = bmp280_i2c_id,
> > > > };
> > > > module_i2c_driver(bmp280_i2c_driver);
> > > > diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
> > > > index 9d57b7a3b134..625b86878ad8 100644
> > > > --- a/drivers/iio/pressure/bmp280-spi.c
> > > > +++ b/drivers/iio/pressure/bmp280-spi.c
> > > > @@ -86,11 +86,6 @@ static int bmp280_spi_probe(struct spi_device *spi)
> > > > spi->irq);
> > > > }
> > > >
> > > > -static int bmp280_spi_remove(struct spi_device *spi)
> > > > -{
> > > > - return bmp280_common_remove(&spi->dev);
> > > > -}
> > > > -
> > > > static const struct of_device_id bmp280_of_spi_match[] = {
> > > > { .compatible = "bosch,bmp085", },
> > > > { .compatible = "bosch,bmp180", },
> > > > @@ -118,7 +113,6 @@ static struct spi_driver bmp280_spi_driver = {
> > > > },
> > > > .id_table = bmp280_spi_id,
> > > > .probe = bmp280_spi_probe,
> > > > - .remove = bmp280_spi_remove,
> > > > };
> > > > module_spi_driver(bmp280_spi_driver);
> > > >
> > > > diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> > > > index eda50ef65706..57ba0e85db91 100644
> > > > --- a/drivers/iio/pressure/bmp280.h
> > > > +++ b/drivers/iio/pressure/bmp280.h
> > > > @@ -112,7 +112,6 @@ int bmp280_common_probe(struct device *dev,
> > > > unsigned int chip,
> > > > const char *name,
> > > > int irq);
> > > > -int bmp280_common_remove(struct device *dev);
> > > >
> > > > /* PM ops */
> > > > extern const struct dev_pm_ops bmp280_dev_pm_ops;
> > >
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] iio: pressure: bmp280: use devm action and remove labels from probe
2019-10-31 6:46 ` Bartosz Golaszewski
@ 2019-11-02 14:22 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2019-11-02 14:22 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
linux-iio, Linux Kernel Mailing List, Bartosz Golaszewski
On Thu, 31 Oct 2019 07:46:51 +0100
Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> wt., 22 paź 2019 o 12:15 Jonathan Cameron <jic23@kernel.org> napisał(a):
> >
> > On Mon, 21 Oct 2019 14:47:18 +0200
> > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > > sob., 12 paź 2019 o 15:37 Jonathan Cameron <jic23@kernel.org> napisał(a):
> > > >
> > > > On Mon, 7 Oct 2019 04:41:31 +0200
> > > > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > >
> > > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > > >
> > > > > We can drop some duplicate code if we use devm_action for disabling
> > > > > regulators and pm and the managed variant of iio_device_register().
> > > > >
> > > > > This allows us to completely remove all remove() callbacks from both
> > > > > i2c and spi code.
> > > > >
> > > > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > > This is on top of the bulk regulator patch which is awaiting precusors
> > > > getting to my upstream. I'll hold this one as well on that.
> > > >
> > > > If it looks like I've forgotten it then give me a poke.
> > > >
> > >
> > > Hi Jonathan,
> > >
> > > gentle poke after v5.4-rc4. Only one of the three patches is in next
> > > so far, the one using bulk regulators is missing too besides this one.
> > Thanks for the reminder.
> >
> > Applied to the togreg branch of iio.git and pushed out as testing for the
> > autobuilders to play with it.
> >
>
> Hi,
>
> I don't see it in the togreg branch of the iio tree - did something go
> wrong with testing?
I tend to only get to my laptop ever few days + 0-day has taken to timing
out half the time at the moment (which means you end up waiting another day
for late running tests to complete). So can be a week or more before things
make it to the togreg branch. Sometimes I forget entirely and it only
happens when I get a pull request ready.
Anyhow, should be in Greg's tree and linux-next by now.
Thanks,
Jonathan
>
> Bart
>
> > Thanks,
> >
> > Jonathan
> > >
> > > Best regards,
> > > Bartosz Golaszewski
> > >
> > > > Thanks,
> > > >
> > > > Jonathan
> > > >
> > > > > ---
> > > > > v1 -> v2:
> > > > > - squash the patches using devm_iio_device_register() and devm_action
> > > > > to keep the changes bisectable
> > > > >
> > > > > drivers/iio/pressure/bmp280-core.c | 62 +++++++++++++++---------------
> > > > > drivers/iio/pressure/bmp280-i2c.c | 6 ---
> > > > > drivers/iio/pressure/bmp280-spi.c | 6 ---
> > > > > drivers/iio/pressure/bmp280.h | 1 -
> > > > > 4 files changed, 30 insertions(+), 45 deletions(-)
> > > > >
> > > > > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > > > > index c2988dbdb1a7..79254dd26dfd 100644
> > > > > --- a/drivers/iio/pressure/bmp280-core.c
> > > > > +++ b/drivers/iio/pressure/bmp280-core.c
> > > > > @@ -984,6 +984,22 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
> > > > > return 0;
> > > > > }
> > > > >
> > > > > +static void bmp280_pm_disable(void *data)
> > > > > +{
> > > > > + struct device *dev = data;
> > > > > +
> > > > > + pm_runtime_get_sync(dev);
> > > > > + pm_runtime_put_noidle(dev);
> > > > > + pm_runtime_disable(dev);
> > > > > +}
> > > > > +
> > > > > +static void bmp280_regulators_disable(void *data)
> > > > > +{
> > > > > + struct regulator_bulk_data *supplies = data;
> > > > > +
> > > > > + regulator_bulk_disable(BMP280_NUM_SUPPLIES, supplies);
> > > > > +}
> > > > > +
> > > > > int bmp280_common_probe(struct device *dev,
> > > > > struct regmap *regmap,
> > > > > unsigned int chip,
> > > > > @@ -1055,6 +1071,11 @@ int bmp280_common_probe(struct device *dev,
> > > > > return ret;
> > > > > }
> > > > >
> > > > > + ret = devm_add_action_or_reset(dev, bmp280_regulators_disable,
> > > > > + data->supplies);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > /* Wait to make sure we started up properly */
> > > > > usleep_range(data->start_up_time, data->start_up_time + 100);
> > > > >
> > > > > @@ -1069,17 +1090,16 @@ int bmp280_common_probe(struct device *dev,
> > > > > data->regmap = regmap;
> > > > > ret = regmap_read(regmap, BMP280_REG_ID, &chip_id);
> > > > > if (ret < 0)
> > > > > - goto out_disable_regulators;
> > > > > + return ret;
> > > > > if (chip_id != chip) {
> > > > > dev_err(dev, "bad chip id: expected %x got %x\n",
> > > > > chip, chip_id);
> > > > > - ret = -EINVAL;
> > > > > - goto out_disable_regulators;
> > > > > + return -EINVAL;
> > > > > }
> > > > >
> > > > > ret = data->chip_info->chip_config(data);
> > > > > if (ret < 0)
> > > > > - goto out_disable_regulators;
> > > > > + return ret;
> > > > >
> > > > > dev_set_drvdata(dev, indio_dev);
> > > > >
> > > > > @@ -1093,14 +1113,14 @@ int bmp280_common_probe(struct device *dev,
> > > > > if (ret < 0) {
> > > > > dev_err(data->dev,
> > > > > "failed to read calibration coefficients\n");
> > > > > - goto out_disable_regulators;
> > > > > + return ret;
> > > > > }
> > > > > } else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
> > > > > ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
> > > > > if (ret < 0) {
> > > > > dev_err(data->dev,
> > > > > "failed to read calibration coefficients\n");
> > > > > - goto out_disable_regulators;
> > > > > + return ret;
> > > > > }
> > > > > }
> > > > >
> > > > > @@ -1112,7 +1132,7 @@ int bmp280_common_probe(struct device *dev,
> > > > > if (irq > 0 || (chip_id == BMP180_CHIP_ID)) {
> > > > > ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
> > > > > if (ret)
> > > > > - goto out_disable_regulators;
> > > > > + return ret;
> > > > > }
> > > > >
> > > > > /* Enable runtime PM */
> > > > > @@ -1127,36 +1147,14 @@ int bmp280_common_probe(struct device *dev,
> > > > > pm_runtime_use_autosuspend(dev);
> > > > > pm_runtime_put(dev);
> > > > >
> > > > > - ret = iio_device_register(indio_dev);
> > > > > + ret = devm_add_action_or_reset(dev, bmp280_pm_disable, dev);
> > > > > if (ret)
> > > > > - goto out_runtime_pm_disable;
> > > > > -
> > > > > - return 0;
> > > > > + return ret;
> > > > >
> > > > > -out_runtime_pm_disable:
> > > > > - pm_runtime_get_sync(data->dev);
> > > > > - pm_runtime_put_noidle(data->dev);
> > > > > - pm_runtime_disable(data->dev);
> > > > > -out_disable_regulators:
> > > > > - regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> > > > > - return ret;
> > > > > + return devm_iio_device_register(dev, indio_dev);
> > > > > }
> > > > > EXPORT_SYMBOL(bmp280_common_probe);
> > > > >
> > > > > -int bmp280_common_remove(struct device *dev)
> > > > > -{
> > > > > - struct iio_dev *indio_dev = dev_get_drvdata(dev);
> > > > > - struct bmp280_data *data = iio_priv(indio_dev);
> > > > > -
> > > > > - iio_device_unregister(indio_dev);
> > > > > - pm_runtime_get_sync(data->dev);
> > > > > - pm_runtime_put_noidle(data->dev);
> > > > > - pm_runtime_disable(data->dev);
> > > > > - regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> > > > > - return 0;
> > > > > -}
> > > > > -EXPORT_SYMBOL(bmp280_common_remove);
> > > > > -
> > > > > #ifdef CONFIG_PM
> > > > > static int bmp280_runtime_suspend(struct device *dev)
> > > > > {
> > > > > diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
> > > > > index acd9a3784fb4..3109c8e2cc11 100644
> > > > > --- a/drivers/iio/pressure/bmp280-i2c.c
> > > > > +++ b/drivers/iio/pressure/bmp280-i2c.c
> > > > > @@ -38,11 +38,6 @@ static int bmp280_i2c_probe(struct i2c_client *client,
> > > > > client->irq);
> > > > > }
> > > > >
> > > > > -static int bmp280_i2c_remove(struct i2c_client *client)
> > > > > -{
> > > > > - return bmp280_common_remove(&client->dev);
> > > > > -}
> > > > > -
> > > > > static const struct acpi_device_id bmp280_acpi_i2c_match[] = {
> > > > > {"BMP0280", BMP280_CHIP_ID },
> > > > > {"BMP0180", BMP180_CHIP_ID },
> > > > > @@ -82,7 +77,6 @@ static struct i2c_driver bmp280_i2c_driver = {
> > > > > .pm = &bmp280_dev_pm_ops,
> > > > > },
> > > > > .probe = bmp280_i2c_probe,
> > > > > - .remove = bmp280_i2c_remove,
> > > > > .id_table = bmp280_i2c_id,
> > > > > };
> > > > > module_i2c_driver(bmp280_i2c_driver);
> > > > > diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
> > > > > index 9d57b7a3b134..625b86878ad8 100644
> > > > > --- a/drivers/iio/pressure/bmp280-spi.c
> > > > > +++ b/drivers/iio/pressure/bmp280-spi.c
> > > > > @@ -86,11 +86,6 @@ static int bmp280_spi_probe(struct spi_device *spi)
> > > > > spi->irq);
> > > > > }
> > > > >
> > > > > -static int bmp280_spi_remove(struct spi_device *spi)
> > > > > -{
> > > > > - return bmp280_common_remove(&spi->dev);
> > > > > -}
> > > > > -
> > > > > static const struct of_device_id bmp280_of_spi_match[] = {
> > > > > { .compatible = "bosch,bmp085", },
> > > > > { .compatible = "bosch,bmp180", },
> > > > > @@ -118,7 +113,6 @@ static struct spi_driver bmp280_spi_driver = {
> > > > > },
> > > > > .id_table = bmp280_spi_id,
> > > > > .probe = bmp280_spi_probe,
> > > > > - .remove = bmp280_spi_remove,
> > > > > };
> > > > > module_spi_driver(bmp280_spi_driver);
> > > > >
> > > > > diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> > > > > index eda50ef65706..57ba0e85db91 100644
> > > > > --- a/drivers/iio/pressure/bmp280.h
> > > > > +++ b/drivers/iio/pressure/bmp280.h
> > > > > @@ -112,7 +112,6 @@ int bmp280_common_probe(struct device *dev,
> > > > > unsigned int chip,
> > > > > const char *name,
> > > > > int irq);
> > > > > -int bmp280_common_remove(struct device *dev);
> > > > >
> > > > > /* PM ops */
> > > > > extern const struct dev_pm_ops bmp280_dev_pm_ops;
> > > >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-02 14:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-07 2:41 [PATCH v2] iio: pressure: bmp280: use devm action and remove labels from probe Bartosz Golaszewski
2019-10-12 13:37 ` Jonathan Cameron
2019-10-21 12:47 ` Bartosz Golaszewski
2019-10-22 10:15 ` Jonathan Cameron
2019-10-31 6:46 ` Bartosz Golaszewski
2019-11-02 14:22 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).