linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] iio: pressure: bmp280: code shrink
@ 2019-10-02  8:57 Bartosz Golaszewski
  2019-10-02  8:57 ` [PATCH 1/4] iio: pressure: bmp280: use bulk regulator ops Bartosz Golaszewski
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2019-10-02  8:57 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>

This series contains a couple changes that shrink the driver both in
terms of LOC as well as actual generated code.

Bartosz Golaszewski (4):
  iio: pressure: bmp280: use bulk regulator ops
  iio: pressure: bmp280: use devm_iio_device_register()
  iio: pressure: bmp280: remove stray newline
  iio: pressure: bmp280: use devm action and remove labels from probe

 drivers/iio/pressure/bmp280-core.c | 114 +++++++++++++----------------
 drivers/iio/pressure/bmp280-i2c.c  |   6 --
 drivers/iio/pressure/bmp280-spi.c  |   6 --
 drivers/iio/pressure/bmp280.h      |   1 -
 4 files changed, 51 insertions(+), 76 deletions(-)

-- 
2.23.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] iio: pressure: bmp280: use bulk regulator ops
  2019-10-02  8:57 [PATCH 0/4] iio: pressure: bmp280: code shrink Bartosz Golaszewski
@ 2019-10-02  8:57 ` Bartosz Golaszewski
  2019-10-02 13:06   ` kbuild test robot
  2019-10-06  9:50   ` Jonathan Cameron
  2019-10-02  8:57 ` [PATCH 2/4] iio: pressure: bmp280: use devm_iio_device_register() Bartosz Golaszewski
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2019-10-02  8:57 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>

The vddd and vdda supplies are always operated on together. We can
shrink the code a bit by using the bulk regulator helpers.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/iio/pressure/bmp280-core.c | 69 +++++++++++++-----------------
 1 file changed, 30 insertions(+), 39 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 8d0f15f27dc5..c21f8ce7b09c 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -74,6 +74,12 @@ struct bmp280_calib {
 	s8  H6;
 };
 
+static const char *const bmp280_supply_names[] = {
+	"vddd", "vdda"
+};
+
+#define BMP280_NUM_SUPPLIES ARRAY_SIZE(bmp280_supply_names)
+
 struct bmp280_data {
 	struct device *dev;
 	struct mutex lock;
@@ -85,8 +91,7 @@ struct bmp280_data {
 		struct bmp180_calib bmp180;
 		struct bmp280_calib bmp280;
 	} calib;
-	struct regulator *vddd;
-	struct regulator *vdda;
+	struct regulator_bulk_data supplies[BMP280_NUM_SUPPLIES];
 	unsigned int start_up_time; /* in microseconds */
 
 	/* log of base 2 of oversampling rate */
@@ -1033,27 +1038,23 @@ int bmp280_common_probe(struct device *dev,
 	}
 
 	/* Bring up regulators */
-	data->vddd = devm_regulator_get(dev, "vddd");
-	if (IS_ERR(data->vddd)) {
-		dev_err(dev, "failed to get VDDD regulator\n");
-		return PTR_ERR(data->vddd);
-	}
-	ret = regulator_enable(data->vddd);
+	regulator_bulk_set_supply_names(data->supplies,
+					bmp280_supply_names,
+					BMP280_NUM_SUPPLIES);
+
+	ret = devm_regulator_bulk_get(dev,
+				      BMP280_NUM_SUPPLIES, data->supplies);
 	if (ret) {
-		dev_err(dev, "failed to enable VDDD regulator\n");
+		dev_err(dev, "failed to get regulators\n");
 		return ret;
 	}
-	data->vdda = devm_regulator_get(dev, "vdda");
-	if (IS_ERR(data->vdda)) {
-		dev_err(dev, "failed to get VDDA regulator\n");
-		ret = PTR_ERR(data->vdda);
-		goto out_disable_vddd;
-	}
-	ret = regulator_enable(data->vdda);
+
+	ret = regulator_bulk_enable(BMP280_NUM_SUPPLIES, data->supplies);
 	if (ret) {
-		dev_err(dev, "failed to enable VDDA regulator\n");
-		goto out_disable_vddd;
+		dev_err(dev, "failed to enable regulators\n");
+		return ret;
 	}
+
 	/* Wait to make sure we started up properly */
 	usleep_range(data->start_up_time, data->start_up_time + 100);
 
@@ -1068,17 +1069,17 @@ 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_vdda;
+		goto out_disable_regulators;
 	if (chip_id != chip) {
 		dev_err(dev, "bad chip id: expected %x got %x\n",
 			chip, chip_id);
 		ret = -EINVAL;
-		goto out_disable_vdda;
+		goto out_disable_regulators;
 	}
 
 	ret = data->chip_info->chip_config(data);
 	if (ret < 0)
-		goto out_disable_vdda;
+		goto out_disable_regulators;
 
 	dev_set_drvdata(dev, indio_dev);
 
@@ -1092,14 +1093,14 @@ int bmp280_common_probe(struct device *dev,
 		if (ret < 0) {
 			dev_err(data->dev,
 				"failed to read calibration coefficients\n");
-			goto out_disable_vdda;
+			goto out_disable_regulators;
 		}
 	} 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_vdda;
+			goto out_disable_regulators;
 		}
 	}
 
@@ -1111,7 +1112,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_vdda;
+			goto out_disable_regulators;
 	}
 
 	/* Enable runtime PM */
@@ -1137,10 +1138,8 @@ int bmp280_common_probe(struct device *dev,
 	pm_runtime_get_sync(data->dev);
 	pm_runtime_put_noidle(data->dev);
 	pm_runtime_disable(data->dev);
-out_disable_vdda:
-	regulator_disable(data->vdda);
-out_disable_vddd:
-	regulator_disable(data->vddd);
+out_disable_regulators:
+	regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
 	return ret;
 }
 EXPORT_SYMBOL(bmp280_common_probe);
@@ -1154,8 +1153,7 @@ int bmp280_common_remove(struct device *dev)
 	pm_runtime_get_sync(data->dev);
 	pm_runtime_put_noidle(data->dev);
 	pm_runtime_disable(data->dev);
-	regulator_disable(data->vdda);
-	regulator_disable(data->vddd);
+	regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
 	return 0;
 }
 EXPORT_SYMBOL(bmp280_common_remove);
@@ -1165,12 +1163,8 @@ static int bmp280_runtime_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct bmp280_data *data = iio_priv(indio_dev);
-	int ret;
 
-	ret = regulator_disable(data->vdda);
-	if (ret)
-		return ret;
-	return regulator_disable(data->vddd);
+	return regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
 }
 
 static int bmp280_runtime_resume(struct device *dev)
@@ -1179,10 +1173,7 @@ static int bmp280_runtime_resume(struct device *dev)
 	struct bmp280_data *data = iio_priv(indio_dev);
 	int ret;
 
-	ret = regulator_enable(data->vddd);
-	if (ret)
-		return ret;
-	ret = regulator_enable(data->vdda);
+	ret = regulator_bulk_enable(BMP280_NUM_SUPPLIES, data->supplies);
 	if (ret)
 		return ret;
 	usleep_range(data->start_up_time, data->start_up_time + 100);
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] iio: pressure: bmp280: use devm_iio_device_register()
  2019-10-02  8:57 [PATCH 0/4] iio: pressure: bmp280: code shrink Bartosz Golaszewski
  2019-10-02  8:57 ` [PATCH 1/4] iio: pressure: bmp280: use bulk regulator ops Bartosz Golaszewski
@ 2019-10-02  8:57 ` Bartosz Golaszewski
  2019-10-06  9:56   ` Jonathan Cameron
  2019-10-02  8:57 ` [PATCH 3/4] iio: pressure: bmp280: remove stray newline Bartosz Golaszewski
  2019-10-02  8:57 ` [PATCH 4/4] iio: pressure: bmp280: use devm action and remove labels from probe Bartosz Golaszewski
  3 siblings, 1 reply; 13+ messages in thread
From: Bartosz Golaszewski @ 2019-10-02  8:57 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 use the managed variant of iio_device_register() and remove
the corresponding unregister operation from the remove callback.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/iio/pressure/bmp280-core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index c21f8ce7b09c..f22400e1e98f 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -1127,7 +1127,7 @@ int bmp280_common_probe(struct device *dev,
 	pm_runtime_use_autosuspend(dev);
 	pm_runtime_put(dev);
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(dev, indio_dev);
 	if (ret)
 		goto out_runtime_pm_disable;
 
@@ -1149,7 +1149,6 @@ 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);
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] iio: pressure: bmp280: remove stray newline
  2019-10-02  8:57 [PATCH 0/4] iio: pressure: bmp280: code shrink Bartosz Golaszewski
  2019-10-02  8:57 ` [PATCH 1/4] iio: pressure: bmp280: use bulk regulator ops Bartosz Golaszewski
  2019-10-02  8:57 ` [PATCH 2/4] iio: pressure: bmp280: use devm_iio_device_register() Bartosz Golaszewski
@ 2019-10-02  8:57 ` Bartosz Golaszewski
  2019-10-06  9:57   ` Jonathan Cameron
  2019-10-02  8:57 ` [PATCH 4/4] iio: pressure: bmp280: use devm action and remove labels from probe Bartosz Golaszewski
  3 siblings, 1 reply; 13+ messages in thread
From: Bartosz Golaszewski @ 2019-10-02  8:57 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>

Remove a stray newline from the probe callback.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/iio/pressure/bmp280-core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index f22400e1e98f..fdbd3bc27921 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -1131,7 +1131,6 @@ int bmp280_common_probe(struct device *dev,
 	if (ret)
 		goto out_runtime_pm_disable;
 
-
 	return 0;
 
 out_runtime_pm_disable:
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] iio: pressure: bmp280: use devm action and remove labels from probe
  2019-10-02  8:57 [PATCH 0/4] iio: pressure: bmp280: code shrink Bartosz Golaszewski
                   ` (2 preceding siblings ...)
  2019-10-02  8:57 ` [PATCH 3/4] iio: pressure: bmp280: remove stray newline Bartosz Golaszewski
@ 2019-10-02  8:57 ` Bartosz Golaszewski
  2019-10-06  9:58   ` Jonathan Cameron
  3 siblings, 1 reply; 13+ messages in thread
From: Bartosz Golaszewski @ 2019-10-02  8:57 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. This allows us to completely remove all remove()
callbacks from both i2c and spi code.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/iio/pressure/bmp280-core.c | 61 +++++++++++++++---------------
 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(+), 44 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index fdbd3bc27921..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,35 +1147,14 @@ int bmp280_common_probe(struct device *dev,
 	pm_runtime_use_autosuspend(dev);
 	pm_runtime_put(dev);
 
-	ret = devm_iio_device_register(dev, 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);
-
-	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] 13+ messages in thread

* Re: [PATCH 1/4] iio: pressure: bmp280: use bulk regulator ops
  2019-10-02  8:57 ` [PATCH 1/4] iio: pressure: bmp280: use bulk regulator ops Bartosz Golaszewski
@ 2019-10-02 13:06   ` kbuild test robot
  2019-10-02 15:57     ` Bartosz Golaszewski
  2019-10-06  9:50   ` Jonathan Cameron
  1 sibling, 1 reply; 13+ messages in thread
From: kbuild test robot @ 2019-10-02 13:06 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: kbuild-all, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, linux-iio, linux-kernel,
	Bartosz Golaszewski

[-- Attachment #1: Type: text/plain, Size: 6996 bytes --]

Hi Bartosz,

I love your patch! Yet something to improve:

[auto build test ERROR on iio/togreg]
[cannot apply to v5.4-rc1 next-20191002]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Bartosz-Golaszewski/iio-pressure-bmp280-code-shrink/20191002-194508
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=sh 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/iio/pressure/bmp280-core.c: In function 'bmp280_common_probe':
>> drivers/iio/pressure/bmp280-core.c:1041:2: error: implicit declaration of function 'regulator_bulk_set_supply_names'; did you mean 'regulator_bulk_register_supply_alias'? [-Werror=implicit-function-declaration]
     regulator_bulk_set_supply_names(data->supplies,
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     regulator_bulk_register_supply_alias
   cc1: some warnings being treated as errors

vim +1041 drivers/iio/pressure/bmp280-core.c

   986	
   987	int bmp280_common_probe(struct device *dev,
   988				struct regmap *regmap,
   989				unsigned int chip,
   990				const char *name,
   991				int irq)
   992	{
   993		int ret;
   994		struct iio_dev *indio_dev;
   995		struct bmp280_data *data;
   996		unsigned int chip_id;
   997		struct gpio_desc *gpiod;
   998	
   999		indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
  1000		if (!indio_dev)
  1001			return -ENOMEM;
  1002	
  1003		data = iio_priv(indio_dev);
  1004		mutex_init(&data->lock);
  1005		data->dev = dev;
  1006	
  1007		indio_dev->dev.parent = dev;
  1008		indio_dev->name = name;
  1009		indio_dev->channels = bmp280_channels;
  1010		indio_dev->info = &bmp280_info;
  1011		indio_dev->modes = INDIO_DIRECT_MODE;
  1012	
  1013		switch (chip) {
  1014		case BMP180_CHIP_ID:
  1015			indio_dev->num_channels = 2;
  1016			data->chip_info = &bmp180_chip_info;
  1017			data->oversampling_press = ilog2(8);
  1018			data->oversampling_temp = ilog2(1);
  1019			data->start_up_time = 10000;
  1020			break;
  1021		case BMP280_CHIP_ID:
  1022			indio_dev->num_channels = 2;
  1023			data->chip_info = &bmp280_chip_info;
  1024			data->oversampling_press = ilog2(16);
  1025			data->oversampling_temp = ilog2(2);
  1026			data->start_up_time = 2000;
  1027			break;
  1028		case BME280_CHIP_ID:
  1029			indio_dev->num_channels = 3;
  1030			data->chip_info = &bme280_chip_info;
  1031			data->oversampling_press = ilog2(16);
  1032			data->oversampling_humid = ilog2(16);
  1033			data->oversampling_temp = ilog2(2);
  1034			data->start_up_time = 2000;
  1035			break;
  1036		default:
  1037			return -EINVAL;
  1038		}
  1039	
  1040		/* Bring up regulators */
> 1041		regulator_bulk_set_supply_names(data->supplies,
  1042						bmp280_supply_names,
  1043						BMP280_NUM_SUPPLIES);
  1044	
  1045		ret = devm_regulator_bulk_get(dev,
  1046					      BMP280_NUM_SUPPLIES, data->supplies);
  1047		if (ret) {
  1048			dev_err(dev, "failed to get regulators\n");
  1049			return ret;
  1050		}
  1051	
  1052		ret = regulator_bulk_enable(BMP280_NUM_SUPPLIES, data->supplies);
  1053		if (ret) {
  1054			dev_err(dev, "failed to enable regulators\n");
  1055			return ret;
  1056		}
  1057	
  1058		/* Wait to make sure we started up properly */
  1059		usleep_range(data->start_up_time, data->start_up_time + 100);
  1060	
  1061		/* Bring chip out of reset if there is an assigned GPIO line */
  1062		gpiod = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
  1063		/* Deassert the signal */
  1064		if (!IS_ERR(gpiod)) {
  1065			dev_info(dev, "release reset\n");
  1066			gpiod_set_value(gpiod, 0);
  1067		}
  1068	
  1069		data->regmap = regmap;
  1070		ret = regmap_read(regmap, BMP280_REG_ID, &chip_id);
  1071		if (ret < 0)
  1072			goto out_disable_regulators;
  1073		if (chip_id != chip) {
  1074			dev_err(dev, "bad chip id: expected %x got %x\n",
  1075				chip, chip_id);
  1076			ret = -EINVAL;
  1077			goto out_disable_regulators;
  1078		}
  1079	
  1080		ret = data->chip_info->chip_config(data);
  1081		if (ret < 0)
  1082			goto out_disable_regulators;
  1083	
  1084		dev_set_drvdata(dev, indio_dev);
  1085	
  1086		/*
  1087		 * Some chips have calibration parameters "programmed into the devices'
  1088		 * non-volatile memory during production". Let's read them out at probe
  1089		 * time once. They will not change.
  1090		 */
  1091		if (chip_id  == BMP180_CHIP_ID) {
  1092			ret = bmp180_read_calib(data, &data->calib.bmp180);
  1093			if (ret < 0) {
  1094				dev_err(data->dev,
  1095					"failed to read calibration coefficients\n");
  1096				goto out_disable_regulators;
  1097			}
  1098		} else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
  1099			ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
  1100			if (ret < 0) {
  1101				dev_err(data->dev,
  1102					"failed to read calibration coefficients\n");
  1103				goto out_disable_regulators;
  1104			}
  1105		}
  1106	
  1107		/*
  1108		 * Attempt to grab an optional EOC IRQ - only the BMP085 has this
  1109		 * however as it happens, the BMP085 shares the chip ID of BMP180
  1110		 * so we look for an IRQ if we have that.
  1111		 */
  1112		if (irq > 0 || (chip_id  == BMP180_CHIP_ID)) {
  1113			ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
  1114			if (ret)
  1115				goto out_disable_regulators;
  1116		}
  1117	
  1118		/* Enable runtime PM */
  1119		pm_runtime_get_noresume(dev);
  1120		pm_runtime_set_active(dev);
  1121		pm_runtime_enable(dev);
  1122		/*
  1123		 * Set autosuspend to two orders of magnitude larger than the
  1124		 * start-up time.
  1125		 */
  1126		pm_runtime_set_autosuspend_delay(dev, data->start_up_time / 10);
  1127		pm_runtime_use_autosuspend(dev);
  1128		pm_runtime_put(dev);
  1129	
  1130		ret = iio_device_register(indio_dev);
  1131		if (ret)
  1132			goto out_runtime_pm_disable;
  1133	
  1134	
  1135		return 0;
  1136	
  1137	out_runtime_pm_disable:
  1138		pm_runtime_get_sync(data->dev);
  1139		pm_runtime_put_noidle(data->dev);
  1140		pm_runtime_disable(data->dev);
  1141	out_disable_regulators:
  1142		regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
  1143		return ret;
  1144	}
  1145	EXPORT_SYMBOL(bmp280_common_probe);
  1146	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51717 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] iio: pressure: bmp280: use bulk regulator ops
  2019-10-02 13:06   ` kbuild test robot
@ 2019-10-02 15:57     ` Bartosz Golaszewski
  2019-10-06  9:49       ` Jonathan Cameron
  0 siblings, 1 reply; 13+ messages in thread
From: Bartosz Golaszewski @ 2019-10-02 15:57 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, linux-iio, Linux Kernel Mailing List,
	Bartosz Golaszewski

śr., 2 paź 2019 o 15:06 kbuild test robot <lkp@intel.com> napisał(a):
>
> Hi Bartosz,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on iio/togreg]
> [cannot apply to v5.4-rc1 next-20191002]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url:    https://github.com/0day-ci/linux/commits/Bartosz-Golaszewski/iio-pressure-bmp280-code-shrink/20191002-194508
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> config: sh-allmodconfig (attached as .config)
> compiler: sh4-linux-gcc (GCC) 7.4.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=7.4.0 make.cross ARCH=sh
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>    drivers/iio/pressure/bmp280-core.c: In function 'bmp280_common_probe':
> >> drivers/iio/pressure/bmp280-core.c:1041:2: error: implicit declaration of function 'regulator_bulk_set_supply_names'; did you mean 'regulator_bulk_register_supply_alias'? [-Werror=implicit-function-declaration]
>      regulator_bulk_set_supply_names(data->supplies,
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      regulator_bulk_register_supply_alias
>    cc1: some warnings being treated as errors
>

This function has been introduced in commit d0087e72710c ("regulator:
provide regulator_bulk_set_supply_names()") and released in v5.4-rc1
but it's not present in this tree. In other words: a false positive.

Bart

> vim +1041 drivers/iio/pressure/bmp280-core.c
>
>    986
>    987  int bmp280_common_probe(struct device *dev,
>    988                          struct regmap *regmap,
>    989                          unsigned int chip,
>    990                          const char *name,
>    991                          int irq)
>    992  {
>    993          int ret;
>    994          struct iio_dev *indio_dev;
>    995          struct bmp280_data *data;
>    996          unsigned int chip_id;
>    997          struct gpio_desc *gpiod;
>    998
>    999          indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
>   1000          if (!indio_dev)
>   1001                  return -ENOMEM;
>   1002
>   1003          data = iio_priv(indio_dev);
>   1004          mutex_init(&data->lock);
>   1005          data->dev = dev;
>   1006
>   1007          indio_dev->dev.parent = dev;
>   1008          indio_dev->name = name;
>   1009          indio_dev->channels = bmp280_channels;
>   1010          indio_dev->info = &bmp280_info;
>   1011          indio_dev->modes = INDIO_DIRECT_MODE;
>   1012
>   1013          switch (chip) {
>   1014          case BMP180_CHIP_ID:
>   1015                  indio_dev->num_channels = 2;
>   1016                  data->chip_info = &bmp180_chip_info;
>   1017                  data->oversampling_press = ilog2(8);
>   1018                  data->oversampling_temp = ilog2(1);
>   1019                  data->start_up_time = 10000;
>   1020                  break;
>   1021          case BMP280_CHIP_ID:
>   1022                  indio_dev->num_channels = 2;
>   1023                  data->chip_info = &bmp280_chip_info;
>   1024                  data->oversampling_press = ilog2(16);
>   1025                  data->oversampling_temp = ilog2(2);
>   1026                  data->start_up_time = 2000;
>   1027                  break;
>   1028          case BME280_CHIP_ID:
>   1029                  indio_dev->num_channels = 3;
>   1030                  data->chip_info = &bme280_chip_info;
>   1031                  data->oversampling_press = ilog2(16);
>   1032                  data->oversampling_humid = ilog2(16);
>   1033                  data->oversampling_temp = ilog2(2);
>   1034                  data->start_up_time = 2000;
>   1035                  break;
>   1036          default:
>   1037                  return -EINVAL;
>   1038          }
>   1039
>   1040          /* Bring up regulators */
> > 1041          regulator_bulk_set_supply_names(data->supplies,
>   1042                                          bmp280_supply_names,
>   1043                                          BMP280_NUM_SUPPLIES);
>   1044
>   1045          ret = devm_regulator_bulk_get(dev,
>   1046                                        BMP280_NUM_SUPPLIES, data->supplies);
>   1047          if (ret) {
>   1048                  dev_err(dev, "failed to get regulators\n");
>   1049                  return ret;
>   1050          }
>   1051
>   1052          ret = regulator_bulk_enable(BMP280_NUM_SUPPLIES, data->supplies);
>   1053          if (ret) {
>   1054                  dev_err(dev, "failed to enable regulators\n");
>   1055                  return ret;
>   1056          }
>   1057
>   1058          /* Wait to make sure we started up properly */
>   1059          usleep_range(data->start_up_time, data->start_up_time + 100);
>   1060
>   1061          /* Bring chip out of reset if there is an assigned GPIO line */
>   1062          gpiod = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
>   1063          /* Deassert the signal */
>   1064          if (!IS_ERR(gpiod)) {
>   1065                  dev_info(dev, "release reset\n");
>   1066                  gpiod_set_value(gpiod, 0);
>   1067          }
>   1068
>   1069          data->regmap = regmap;
>   1070          ret = regmap_read(regmap, BMP280_REG_ID, &chip_id);
>   1071          if (ret < 0)
>   1072                  goto out_disable_regulators;
>   1073          if (chip_id != chip) {
>   1074                  dev_err(dev, "bad chip id: expected %x got %x\n",
>   1075                          chip, chip_id);
>   1076                  ret = -EINVAL;
>   1077                  goto out_disable_regulators;
>   1078          }
>   1079
>   1080          ret = data->chip_info->chip_config(data);
>   1081          if (ret < 0)
>   1082                  goto out_disable_regulators;
>   1083
>   1084          dev_set_drvdata(dev, indio_dev);
>   1085
>   1086          /*
>   1087           * Some chips have calibration parameters "programmed into the devices'
>   1088           * non-volatile memory during production". Let's read them out at probe
>   1089           * time once. They will not change.
>   1090           */
>   1091          if (chip_id  == BMP180_CHIP_ID) {
>   1092                  ret = bmp180_read_calib(data, &data->calib.bmp180);
>   1093                  if (ret < 0) {
>   1094                          dev_err(data->dev,
>   1095                                  "failed to read calibration coefficients\n");
>   1096                          goto out_disable_regulators;
>   1097                  }
>   1098          } else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
>   1099                  ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
>   1100                  if (ret < 0) {
>   1101                          dev_err(data->dev,
>   1102                                  "failed to read calibration coefficients\n");
>   1103                          goto out_disable_regulators;
>   1104                  }
>   1105          }
>   1106
>   1107          /*
>   1108           * Attempt to grab an optional EOC IRQ - only the BMP085 has this
>   1109           * however as it happens, the BMP085 shares the chip ID of BMP180
>   1110           * so we look for an IRQ if we have that.
>   1111           */
>   1112          if (irq > 0 || (chip_id  == BMP180_CHIP_ID)) {
>   1113                  ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
>   1114                  if (ret)
>   1115                          goto out_disable_regulators;
>   1116          }
>   1117
>   1118          /* Enable runtime PM */
>   1119          pm_runtime_get_noresume(dev);
>   1120          pm_runtime_set_active(dev);
>   1121          pm_runtime_enable(dev);
>   1122          /*
>   1123           * Set autosuspend to two orders of magnitude larger than the
>   1124           * start-up time.
>   1125           */
>   1126          pm_runtime_set_autosuspend_delay(dev, data->start_up_time / 10);
>   1127          pm_runtime_use_autosuspend(dev);
>   1128          pm_runtime_put(dev);
>   1129
>   1130          ret = iio_device_register(indio_dev);
>   1131          if (ret)
>   1132                  goto out_runtime_pm_disable;
>   1133
>   1134
>   1135          return 0;
>   1136
>   1137  out_runtime_pm_disable:
>   1138          pm_runtime_get_sync(data->dev);
>   1139          pm_runtime_put_noidle(data->dev);
>   1140          pm_runtime_disable(data->dev);
>   1141  out_disable_regulators:
>   1142          regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
>   1143          return ret;
>   1144  }
>   1145  EXPORT_SYMBOL(bmp280_common_probe);
>   1146
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] iio: pressure: bmp280: use bulk regulator ops
  2019-10-02 15:57     ` Bartosz Golaszewski
@ 2019-10-06  9:49       ` Jonathan Cameron
  2019-10-22 10:03         ` Jonathan Cameron
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2019-10-06  9:49 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: kbuild test robot, kbuild-all, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio,
	Linux Kernel Mailing List, Bartosz Golaszewski

On Wed, 2 Oct 2019 17:57:30 +0200
Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> śr., 2 paź 2019 o 15:06 kbuild test robot <lkp@intel.com> napisał(a):
> >
> > Hi Bartosz,
> >
> > I love your patch! Yet something to improve:
> >
> > [auto build test ERROR on iio/togreg]
> > [cannot apply to v5.4-rc1 next-20191002]
> > [if your patch is applied to the wrong git tree, please drop us a note to help
> > improve the system. BTW, we also suggest to use '--base' option to specify the
> > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> >
> > url:    https://github.com/0day-ci/linux/commits/Bartosz-Golaszewski/iio-pressure-bmp280-code-shrink/20191002-194508
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> > config: sh-allmodconfig (attached as .config)
> > compiler: sh4-linux-gcc (GCC) 7.4.0
> > reproduce:
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         GCC_VERSION=7.4.0 make.cross ARCH=sh
> >
> > If you fix the issue, kindly add following tag
> > Reported-by: kbuild test robot <lkp@intel.com>
> >
> > All errors (new ones prefixed by >>):
> >
> >    drivers/iio/pressure/bmp280-core.c: In function 'bmp280_common_probe':  
> > >> drivers/iio/pressure/bmp280-core.c:1041:2: error: implicit declaration of function 'regulator_bulk_set_supply_names'; did you mean 'regulator_bulk_register_supply_alias'? [-Werror=implicit-function-declaration]  
> >      regulator_bulk_set_supply_names(data->supplies,
> >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >      regulator_bulk_register_supply_alias
> >    cc1: some warnings being treated as errors
> >  
> 
> This function has been introduced in commit d0087e72710c ("regulator:
> provide regulator_bulk_set_supply_names()") and released in v5.4-rc1
> but it's not present in this tree. In other words: a false positive.
Kind of handy to known though ;)  My tree doesn't contain it yet
either.  That should be fixed later this week after a pull request
and rebase.  I'll not be applying this series until after that.

Thanks,

Jonathan

> 
> Bart
> 
> > vim +1041 drivers/iio/pressure/bmp280-core.c
> >
> >    986
> >    987  int bmp280_common_probe(struct device *dev,
> >    988                          struct regmap *regmap,
> >    989                          unsigned int chip,
> >    990                          const char *name,
> >    991                          int irq)
> >    992  {
> >    993          int ret;
> >    994          struct iio_dev *indio_dev;
> >    995          struct bmp280_data *data;
> >    996          unsigned int chip_id;
> >    997          struct gpio_desc *gpiod;
> >    998
> >    999          indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> >   1000          if (!indio_dev)
> >   1001                  return -ENOMEM;
> >   1002
> >   1003          data = iio_priv(indio_dev);
> >   1004          mutex_init(&data->lock);
> >   1005          data->dev = dev;
> >   1006
> >   1007          indio_dev->dev.parent = dev;
> >   1008          indio_dev->name = name;
> >   1009          indio_dev->channels = bmp280_channels;
> >   1010          indio_dev->info = &bmp280_info;
> >   1011          indio_dev->modes = INDIO_DIRECT_MODE;
> >   1012
> >   1013          switch (chip) {
> >   1014          case BMP180_CHIP_ID:
> >   1015                  indio_dev->num_channels = 2;
> >   1016                  data->chip_info = &bmp180_chip_info;
> >   1017                  data->oversampling_press = ilog2(8);
> >   1018                  data->oversampling_temp = ilog2(1);
> >   1019                  data->start_up_time = 10000;
> >   1020                  break;
> >   1021          case BMP280_CHIP_ID:
> >   1022                  indio_dev->num_channels = 2;
> >   1023                  data->chip_info = &bmp280_chip_info;
> >   1024                  data->oversampling_press = ilog2(16);
> >   1025                  data->oversampling_temp = ilog2(2);
> >   1026                  data->start_up_time = 2000;
> >   1027                  break;
> >   1028          case BME280_CHIP_ID:
> >   1029                  indio_dev->num_channels = 3;
> >   1030                  data->chip_info = &bme280_chip_info;
> >   1031                  data->oversampling_press = ilog2(16);
> >   1032                  data->oversampling_humid = ilog2(16);
> >   1033                  data->oversampling_temp = ilog2(2);
> >   1034                  data->start_up_time = 2000;
> >   1035                  break;
> >   1036          default:
> >   1037                  return -EINVAL;
> >   1038          }
> >   1039
> >   1040          /* Bring up regulators */  
> > > 1041          regulator_bulk_set_supply_names(data->supplies,  
> >   1042                                          bmp280_supply_names,
> >   1043                                          BMP280_NUM_SUPPLIES);
> >   1044
> >   1045          ret = devm_regulator_bulk_get(dev,
> >   1046                                        BMP280_NUM_SUPPLIES, data->supplies);
> >   1047          if (ret) {
> >   1048                  dev_err(dev, "failed to get regulators\n");
> >   1049                  return ret;
> >   1050          }
> >   1051
> >   1052          ret = regulator_bulk_enable(BMP280_NUM_SUPPLIES, data->supplies);
> >   1053          if (ret) {
> >   1054                  dev_err(dev, "failed to enable regulators\n");
> >   1055                  return ret;
> >   1056          }
> >   1057
> >   1058          /* Wait to make sure we started up properly */
> >   1059          usleep_range(data->start_up_time, data->start_up_time + 100);
> >   1060
> >   1061          /* Bring chip out of reset if there is an assigned GPIO line */
> >   1062          gpiod = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> >   1063          /* Deassert the signal */
> >   1064          if (!IS_ERR(gpiod)) {
> >   1065                  dev_info(dev, "release reset\n");
> >   1066                  gpiod_set_value(gpiod, 0);
> >   1067          }
> >   1068
> >   1069          data->regmap = regmap;
> >   1070          ret = regmap_read(regmap, BMP280_REG_ID, &chip_id);
> >   1071          if (ret < 0)
> >   1072                  goto out_disable_regulators;
> >   1073          if (chip_id != chip) {
> >   1074                  dev_err(dev, "bad chip id: expected %x got %x\n",
> >   1075                          chip, chip_id);
> >   1076                  ret = -EINVAL;
> >   1077                  goto out_disable_regulators;
> >   1078          }
> >   1079
> >   1080          ret = data->chip_info->chip_config(data);
> >   1081          if (ret < 0)
> >   1082                  goto out_disable_regulators;
> >   1083
> >   1084          dev_set_drvdata(dev, indio_dev);
> >   1085
> >   1086          /*
> >   1087           * Some chips have calibration parameters "programmed into the devices'
> >   1088           * non-volatile memory during production". Let's read them out at probe
> >   1089           * time once. They will not change.
> >   1090           */
> >   1091          if (chip_id  == BMP180_CHIP_ID) {
> >   1092                  ret = bmp180_read_calib(data, &data->calib.bmp180);
> >   1093                  if (ret < 0) {
> >   1094                          dev_err(data->dev,
> >   1095                                  "failed to read calibration coefficients\n");
> >   1096                          goto out_disable_regulators;
> >   1097                  }
> >   1098          } else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
> >   1099                  ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
> >   1100                  if (ret < 0) {
> >   1101                          dev_err(data->dev,
> >   1102                                  "failed to read calibration coefficients\n");
> >   1103                          goto out_disable_regulators;
> >   1104                  }
> >   1105          }
> >   1106
> >   1107          /*
> >   1108           * Attempt to grab an optional EOC IRQ - only the BMP085 has this
> >   1109           * however as it happens, the BMP085 shares the chip ID of BMP180
> >   1110           * so we look for an IRQ if we have that.
> >   1111           */
> >   1112          if (irq > 0 || (chip_id  == BMP180_CHIP_ID)) {
> >   1113                  ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
> >   1114                  if (ret)
> >   1115                          goto out_disable_regulators;
> >   1116          }
> >   1117
> >   1118          /* Enable runtime PM */
> >   1119          pm_runtime_get_noresume(dev);
> >   1120          pm_runtime_set_active(dev);
> >   1121          pm_runtime_enable(dev);
> >   1122          /*
> >   1123           * Set autosuspend to two orders of magnitude larger than the
> >   1124           * start-up time.
> >   1125           */
> >   1126          pm_runtime_set_autosuspend_delay(dev, data->start_up_time / 10);
> >   1127          pm_runtime_use_autosuspend(dev);
> >   1128          pm_runtime_put(dev);
> >   1129
> >   1130          ret = iio_device_register(indio_dev);
> >   1131          if (ret)
> >   1132                  goto out_runtime_pm_disable;
> >   1133
> >   1134
> >   1135          return 0;
> >   1136
> >   1137  out_runtime_pm_disable:
> >   1138          pm_runtime_get_sync(data->dev);
> >   1139          pm_runtime_put_noidle(data->dev);
> >   1140          pm_runtime_disable(data->dev);
> >   1141  out_disable_regulators:
> >   1142          regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> >   1143          return ret;
> >   1144  }
> >   1145  EXPORT_SYMBOL(bmp280_common_probe);
> >   1146
> >
> > ---
> > 0-DAY kernel test infrastructure                Open Source Technology Center
> > https://lists.01.org/pipermail/kbuild-all                   Intel Corporation  


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] iio: pressure: bmp280: use bulk regulator ops
  2019-10-02  8:57 ` [PATCH 1/4] iio: pressure: bmp280: use bulk regulator ops Bartosz Golaszewski
  2019-10-02 13:06   ` kbuild test robot
@ 2019-10-06  9:50   ` Jonathan Cameron
  1 sibling, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2019-10-06  9:50 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	linux-iio, linux-kernel, Bartosz Golaszewski

On Wed,  2 Oct 2019 10:57:56 +0200
Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> The vddd and vdda supplies are always operated on together. We can
> shrink the code a bit by using the bulk regulator helpers.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Looks good. Will pick up once I have the precursors in my branch.

Thanks,

Jonathan

> ---
>  drivers/iio/pressure/bmp280-core.c | 69 +++++++++++++-----------------
>  1 file changed, 30 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 8d0f15f27dc5..c21f8ce7b09c 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -74,6 +74,12 @@ struct bmp280_calib {
>  	s8  H6;
>  };
>  
> +static const char *const bmp280_supply_names[] = {
> +	"vddd", "vdda"
> +};
> +
> +#define BMP280_NUM_SUPPLIES ARRAY_SIZE(bmp280_supply_names)
> +
>  struct bmp280_data {
>  	struct device *dev;
>  	struct mutex lock;
> @@ -85,8 +91,7 @@ struct bmp280_data {
>  		struct bmp180_calib bmp180;
>  		struct bmp280_calib bmp280;
>  	} calib;
> -	struct regulator *vddd;
> -	struct regulator *vdda;
> +	struct regulator_bulk_data supplies[BMP280_NUM_SUPPLIES];
>  	unsigned int start_up_time; /* in microseconds */
>  
>  	/* log of base 2 of oversampling rate */
> @@ -1033,27 +1038,23 @@ int bmp280_common_probe(struct device *dev,
>  	}
>  
>  	/* Bring up regulators */
> -	data->vddd = devm_regulator_get(dev, "vddd");
> -	if (IS_ERR(data->vddd)) {
> -		dev_err(dev, "failed to get VDDD regulator\n");
> -		return PTR_ERR(data->vddd);
> -	}
> -	ret = regulator_enable(data->vddd);
> +	regulator_bulk_set_supply_names(data->supplies,
> +					bmp280_supply_names,
> +					BMP280_NUM_SUPPLIES);
> +
> +	ret = devm_regulator_bulk_get(dev,
> +				      BMP280_NUM_SUPPLIES, data->supplies);
>  	if (ret) {
> -		dev_err(dev, "failed to enable VDDD regulator\n");
> +		dev_err(dev, "failed to get regulators\n");
>  		return ret;
>  	}
> -	data->vdda = devm_regulator_get(dev, "vdda");
> -	if (IS_ERR(data->vdda)) {
> -		dev_err(dev, "failed to get VDDA regulator\n");
> -		ret = PTR_ERR(data->vdda);
> -		goto out_disable_vddd;
> -	}
> -	ret = regulator_enable(data->vdda);
> +
> +	ret = regulator_bulk_enable(BMP280_NUM_SUPPLIES, data->supplies);
>  	if (ret) {
> -		dev_err(dev, "failed to enable VDDA regulator\n");
> -		goto out_disable_vddd;
> +		dev_err(dev, "failed to enable regulators\n");
> +		return ret;
>  	}
> +
>  	/* Wait to make sure we started up properly */
>  	usleep_range(data->start_up_time, data->start_up_time + 100);
>  
> @@ -1068,17 +1069,17 @@ 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_vdda;
> +		goto out_disable_regulators;
>  	if (chip_id != chip) {
>  		dev_err(dev, "bad chip id: expected %x got %x\n",
>  			chip, chip_id);
>  		ret = -EINVAL;
> -		goto out_disable_vdda;
> +		goto out_disable_regulators;
>  	}
>  
>  	ret = data->chip_info->chip_config(data);
>  	if (ret < 0)
> -		goto out_disable_vdda;
> +		goto out_disable_regulators;
>  
>  	dev_set_drvdata(dev, indio_dev);
>  
> @@ -1092,14 +1093,14 @@ int bmp280_common_probe(struct device *dev,
>  		if (ret < 0) {
>  			dev_err(data->dev,
>  				"failed to read calibration coefficients\n");
> -			goto out_disable_vdda;
> +			goto out_disable_regulators;
>  		}
>  	} 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_vdda;
> +			goto out_disable_regulators;
>  		}
>  	}
>  
> @@ -1111,7 +1112,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_vdda;
> +			goto out_disable_regulators;
>  	}
>  
>  	/* Enable runtime PM */
> @@ -1137,10 +1138,8 @@ int bmp280_common_probe(struct device *dev,
>  	pm_runtime_get_sync(data->dev);
>  	pm_runtime_put_noidle(data->dev);
>  	pm_runtime_disable(data->dev);
> -out_disable_vdda:
> -	regulator_disable(data->vdda);
> -out_disable_vddd:
> -	regulator_disable(data->vddd);
> +out_disable_regulators:
> +	regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
>  	return ret;
>  }
>  EXPORT_SYMBOL(bmp280_common_probe);
> @@ -1154,8 +1153,7 @@ int bmp280_common_remove(struct device *dev)
>  	pm_runtime_get_sync(data->dev);
>  	pm_runtime_put_noidle(data->dev);
>  	pm_runtime_disable(data->dev);
> -	regulator_disable(data->vdda);
> -	regulator_disable(data->vddd);
> +	regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
>  	return 0;
>  }
>  EXPORT_SYMBOL(bmp280_common_remove);
> @@ -1165,12 +1163,8 @@ static int bmp280_runtime_suspend(struct device *dev)
>  {
>  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct bmp280_data *data = iio_priv(indio_dev);
> -	int ret;
>  
> -	ret = regulator_disable(data->vdda);
> -	if (ret)
> -		return ret;
> -	return regulator_disable(data->vddd);
> +	return regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
>  }
>  
>  static int bmp280_runtime_resume(struct device *dev)
> @@ -1179,10 +1173,7 @@ static int bmp280_runtime_resume(struct device *dev)
>  	struct bmp280_data *data = iio_priv(indio_dev);
>  	int ret;
>  
> -	ret = regulator_enable(data->vddd);
> -	if (ret)
> -		return ret;
> -	ret = regulator_enable(data->vdda);
> +	ret = regulator_bulk_enable(BMP280_NUM_SUPPLIES, data->supplies);
>  	if (ret)
>  		return ret;
>  	usleep_range(data->start_up_time, data->start_up_time + 100);


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] iio: pressure: bmp280: use devm_iio_device_register()
  2019-10-02  8:57 ` [PATCH 2/4] iio: pressure: bmp280: use devm_iio_device_register() Bartosz Golaszewski
@ 2019-10-06  9:56   ` Jonathan Cameron
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2019-10-06  9:56 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	linux-iio, linux-kernel, Bartosz Golaszewski

On Wed,  2 Oct 2019 10:57:57 +0200
Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> We can use the managed variant of iio_device_register() and remove
> the corresponding unregister operation from the remove callback.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This results in a race where the regulators are powered down before
we remove the userspace interfaces.  All sorts of fun can therefore
occur...

If we fix that with some devm_add_action_or_reset fun then there
is still the fact that we loose the 'obviously correct' property
of the remove being a mirror of the probe because the ordering
wrt to runtime_pm is different.  

So I'd leave this one alone.

Thanks,

Jonathan


> ---
>  drivers/iio/pressure/bmp280-core.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index c21f8ce7b09c..f22400e1e98f 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -1127,7 +1127,7 @@ int bmp280_common_probe(struct device *dev,
>  	pm_runtime_use_autosuspend(dev);
>  	pm_runtime_put(dev);
>  
> -	ret = iio_device_register(indio_dev);
> +	ret = devm_iio_device_register(dev, indio_dev);
>  	if (ret)
>  		goto out_runtime_pm_disable;
>  
> @@ -1149,7 +1149,6 @@ 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);


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] iio: pressure: bmp280: remove stray newline
  2019-10-02  8:57 ` [PATCH 3/4] iio: pressure: bmp280: remove stray newline Bartosz Golaszewski
@ 2019-10-06  9:57   ` Jonathan Cameron
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2019-10-06  9:57 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	linux-iio, linux-kernel, Bartosz Golaszewski

On Wed,  2 Oct 2019 10:57:58 +0200
Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Remove a stray newline from the probe callback.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Applied to the togreg branch of iio.git and pushed out as testing for
no particular reason (well for the other patches in that tree ;)

Thanks,

Jonathan

> ---
>  drivers/iio/pressure/bmp280-core.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index f22400e1e98f..fdbd3bc27921 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -1131,7 +1131,6 @@ int bmp280_common_probe(struct device *dev,
>  	if (ret)
>  		goto out_runtime_pm_disable;
>  
> -
>  	return 0;
>  
>  out_runtime_pm_disable:


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] iio: pressure: bmp280: use devm action and remove labels from probe
  2019-10-02  8:57 ` [PATCH 4/4] iio: pressure: bmp280: use devm action and remove labels from probe Bartosz Golaszewski
@ 2019-10-06  9:58   ` Jonathan Cameron
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2019-10-06  9:58 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	linux-iio, linux-kernel, Bartosz Golaszewski

On Wed,  2 Oct 2019 10:57:59 +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. This allows us to completely remove all remove()
> callbacks from both i2c and spi code.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Ah. I should read all the patches but that would be far too much like hard
work.  Roll this one and patch 2 together and we should have satisfied the
issues I had with that.

Thanks,

Jonathan

> ---
>  drivers/iio/pressure/bmp280-core.c | 61 +++++++++++++++---------------
>  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(+), 44 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index fdbd3bc27921..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,35 +1147,14 @@ int bmp280_common_probe(struct device *dev,
>  	pm_runtime_use_autosuspend(dev);
>  	pm_runtime_put(dev);
>  
> -	ret = devm_iio_device_register(dev, 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);
> -
> -	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] 13+ messages in thread

* Re: [PATCH 1/4] iio: pressure: bmp280: use bulk regulator ops
  2019-10-06  9:49       ` Jonathan Cameron
@ 2019-10-22 10:03         ` Jonathan Cameron
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2019-10-22 10:03 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: kbuild test robot, kbuild-all, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio,
	Linux Kernel Mailing List, Bartosz Golaszewski

On Sun, 6 Oct 2019 10:49:18 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Wed, 2 Oct 2019 17:57:30 +0200
> Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> 
> > śr., 2 paź 2019 o 15:06 kbuild test robot <lkp@intel.com> napisał(a):  
> > >
> > > Hi Bartosz,
> > >
> > > I love your patch! Yet something to improve:
> > >
> > > [auto build test ERROR on iio/togreg]
> > > [cannot apply to v5.4-rc1 next-20191002]
> > > [if your patch is applied to the wrong git tree, please drop us a note to help
> > > improve the system. BTW, we also suggest to use '--base' option to specify the
> > > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> > >
> > > url:    https://github.com/0day-ci/linux/commits/Bartosz-Golaszewski/iio-pressure-bmp280-code-shrink/20191002-194508
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> > > config: sh-allmodconfig (attached as .config)
> > > compiler: sh4-linux-gcc (GCC) 7.4.0
> > > reproduce:
> > >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         # save the attached .config to linux build tree
> > >         GCC_VERSION=7.4.0 make.cross ARCH=sh
> > >
> > > If you fix the issue, kindly add following tag
> > > Reported-by: kbuild test robot <lkp@intel.com>
> > >
> > > All errors (new ones prefixed by >>):
> > >
> > >    drivers/iio/pressure/bmp280-core.c: In function 'bmp280_common_probe':    
> > > >> drivers/iio/pressure/bmp280-core.c:1041:2: error: implicit declaration of function 'regulator_bulk_set_supply_names'; did you mean 'regulator_bulk_register_supply_alias'? [-Werror=implicit-function-declaration]    
> > >      regulator_bulk_set_supply_names(data->supplies,
> > >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >      regulator_bulk_register_supply_alias
> > >    cc1: some warnings being treated as errors
> > >    
> > 
> > This function has been introduced in commit d0087e72710c ("regulator:
> > provide regulator_bulk_set_supply_names()") and released in v5.4-rc1
> > but it's not present in this tree. In other words: a false positive.  
> Kind of handy to known though ;)  My tree doesn't contain it yet
> either.  That should be fixed later this week after a pull request
> and rebase.  I'll not be applying this series until after that.
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> 
> Thanks,
> 
> Jonathan
> 
> > 
> > Bart
> >   
> > > vim +1041 drivers/iio/pressure/bmp280-core.c
> > >
> > >    986
> > >    987  int bmp280_common_probe(struct device *dev,
> > >    988                          struct regmap *regmap,
> > >    989                          unsigned int chip,
> > >    990                          const char *name,
> > >    991                          int irq)
> > >    992  {
> > >    993          int ret;
> > >    994          struct iio_dev *indio_dev;
> > >    995          struct bmp280_data *data;
> > >    996          unsigned int chip_id;
> > >    997          struct gpio_desc *gpiod;
> > >    998
> > >    999          indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> > >   1000          if (!indio_dev)
> > >   1001                  return -ENOMEM;
> > >   1002
> > >   1003          data = iio_priv(indio_dev);
> > >   1004          mutex_init(&data->lock);
> > >   1005          data->dev = dev;
> > >   1006
> > >   1007          indio_dev->dev.parent = dev;
> > >   1008          indio_dev->name = name;
> > >   1009          indio_dev->channels = bmp280_channels;
> > >   1010          indio_dev->info = &bmp280_info;
> > >   1011          indio_dev->modes = INDIO_DIRECT_MODE;
> > >   1012
> > >   1013          switch (chip) {
> > >   1014          case BMP180_CHIP_ID:
> > >   1015                  indio_dev->num_channels = 2;
> > >   1016                  data->chip_info = &bmp180_chip_info;
> > >   1017                  data->oversampling_press = ilog2(8);
> > >   1018                  data->oversampling_temp = ilog2(1);
> > >   1019                  data->start_up_time = 10000;
> > >   1020                  break;
> > >   1021          case BMP280_CHIP_ID:
> > >   1022                  indio_dev->num_channels = 2;
> > >   1023                  data->chip_info = &bmp280_chip_info;
> > >   1024                  data->oversampling_press = ilog2(16);
> > >   1025                  data->oversampling_temp = ilog2(2);
> > >   1026                  data->start_up_time = 2000;
> > >   1027                  break;
> > >   1028          case BME280_CHIP_ID:
> > >   1029                  indio_dev->num_channels = 3;
> > >   1030                  data->chip_info = &bme280_chip_info;
> > >   1031                  data->oversampling_press = ilog2(16);
> > >   1032                  data->oversampling_humid = ilog2(16);
> > >   1033                  data->oversampling_temp = ilog2(2);
> > >   1034                  data->start_up_time = 2000;
> > >   1035                  break;
> > >   1036          default:
> > >   1037                  return -EINVAL;
> > >   1038          }
> > >   1039
> > >   1040          /* Bring up regulators */    
> > > > 1041          regulator_bulk_set_supply_names(data->supplies,    
> > >   1042                                          bmp280_supply_names,
> > >   1043                                          BMP280_NUM_SUPPLIES);
> > >   1044
> > >   1045          ret = devm_regulator_bulk_get(dev,
> > >   1046                                        BMP280_NUM_SUPPLIES, data->supplies);
> > >   1047          if (ret) {
> > >   1048                  dev_err(dev, "failed to get regulators\n");
> > >   1049                  return ret;
> > >   1050          }
> > >   1051
> > >   1052          ret = regulator_bulk_enable(BMP280_NUM_SUPPLIES, data->supplies);
> > >   1053          if (ret) {
> > >   1054                  dev_err(dev, "failed to enable regulators\n");
> > >   1055                  return ret;
> > >   1056          }
> > >   1057
> > >   1058          /* Wait to make sure we started up properly */
> > >   1059          usleep_range(data->start_up_time, data->start_up_time + 100);
> > >   1060
> > >   1061          /* Bring chip out of reset if there is an assigned GPIO line */
> > >   1062          gpiod = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> > >   1063          /* Deassert the signal */
> > >   1064          if (!IS_ERR(gpiod)) {
> > >   1065                  dev_info(dev, "release reset\n");
> > >   1066                  gpiod_set_value(gpiod, 0);
> > >   1067          }
> > >   1068
> > >   1069          data->regmap = regmap;
> > >   1070          ret = regmap_read(regmap, BMP280_REG_ID, &chip_id);
> > >   1071          if (ret < 0)
> > >   1072                  goto out_disable_regulators;
> > >   1073          if (chip_id != chip) {
> > >   1074                  dev_err(dev, "bad chip id: expected %x got %x\n",
> > >   1075                          chip, chip_id);
> > >   1076                  ret = -EINVAL;
> > >   1077                  goto out_disable_regulators;
> > >   1078          }
> > >   1079
> > >   1080          ret = data->chip_info->chip_config(data);
> > >   1081          if (ret < 0)
> > >   1082                  goto out_disable_regulators;
> > >   1083
> > >   1084          dev_set_drvdata(dev, indio_dev);
> > >   1085
> > >   1086          /*
> > >   1087           * Some chips have calibration parameters "programmed into the devices'
> > >   1088           * non-volatile memory during production". Let's read them out at probe
> > >   1089           * time once. They will not change.
> > >   1090           */
> > >   1091          if (chip_id  == BMP180_CHIP_ID) {
> > >   1092                  ret = bmp180_read_calib(data, &data->calib.bmp180);
> > >   1093                  if (ret < 0) {
> > >   1094                          dev_err(data->dev,
> > >   1095                                  "failed to read calibration coefficients\n");
> > >   1096                          goto out_disable_regulators;
> > >   1097                  }
> > >   1098          } else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
> > >   1099                  ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
> > >   1100                  if (ret < 0) {
> > >   1101                          dev_err(data->dev,
> > >   1102                                  "failed to read calibration coefficients\n");
> > >   1103                          goto out_disable_regulators;
> > >   1104                  }
> > >   1105          }
> > >   1106
> > >   1107          /*
> > >   1108           * Attempt to grab an optional EOC IRQ - only the BMP085 has this
> > >   1109           * however as it happens, the BMP085 shares the chip ID of BMP180
> > >   1110           * so we look for an IRQ if we have that.
> > >   1111           */
> > >   1112          if (irq > 0 || (chip_id  == BMP180_CHIP_ID)) {
> > >   1113                  ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
> > >   1114                  if (ret)
> > >   1115                          goto out_disable_regulators;
> > >   1116          }
> > >   1117
> > >   1118          /* Enable runtime PM */
> > >   1119          pm_runtime_get_noresume(dev);
> > >   1120          pm_runtime_set_active(dev);
> > >   1121          pm_runtime_enable(dev);
> > >   1122          /*
> > >   1123           * Set autosuspend to two orders of magnitude larger than the
> > >   1124           * start-up time.
> > >   1125           */
> > >   1126          pm_runtime_set_autosuspend_delay(dev, data->start_up_time / 10);
> > >   1127          pm_runtime_use_autosuspend(dev);
> > >   1128          pm_runtime_put(dev);
> > >   1129
> > >   1130          ret = iio_device_register(indio_dev);
> > >   1131          if (ret)
> > >   1132                  goto out_runtime_pm_disable;
> > >   1133
> > >   1134
> > >   1135          return 0;
> > >   1136
> > >   1137  out_runtime_pm_disable:
> > >   1138          pm_runtime_get_sync(data->dev);
> > >   1139          pm_runtime_put_noidle(data->dev);
> > >   1140          pm_runtime_disable(data->dev);
> > >   1141  out_disable_regulators:
> > >   1142          regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> > >   1143          return ret;
> > >   1144  }
> > >   1145  EXPORT_SYMBOL(bmp280_common_probe);
> > >   1146
> > >
> > > ---
> > > 0-DAY kernel test infrastructure                Open Source Technology Center
> > > https://lists.01.org/pipermail/kbuild-all                   Intel Corporation    
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-10-22 10:03 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-02  8:57 [PATCH 0/4] iio: pressure: bmp280: code shrink Bartosz Golaszewski
2019-10-02  8:57 ` [PATCH 1/4] iio: pressure: bmp280: use bulk regulator ops Bartosz Golaszewski
2019-10-02 13:06   ` kbuild test robot
2019-10-02 15:57     ` Bartosz Golaszewski
2019-10-06  9:49       ` Jonathan Cameron
2019-10-22 10:03         ` Jonathan Cameron
2019-10-06  9:50   ` Jonathan Cameron
2019-10-02  8:57 ` [PATCH 2/4] iio: pressure: bmp280: use devm_iio_device_register() Bartosz Golaszewski
2019-10-06  9:56   ` Jonathan Cameron
2019-10-02  8:57 ` [PATCH 3/4] iio: pressure: bmp280: remove stray newline Bartosz Golaszewski
2019-10-06  9:57   ` Jonathan Cameron
2019-10-02  8:57 ` [PATCH 4/4] iio: pressure: bmp280: use devm action and remove labels from probe Bartosz Golaszewski
2019-10-06  9:58   ` 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).