[PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Andy Shevchenko]

It feels wrong and actually inconsistent to attach managed resources
to the IIO device object, which is child of the physical device object.
The rest of the ->probe() calls do that against physical device.

Resolve this by reassigning managed resources to the physical device object.

Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: new fix-patch
 drivers/iio/adc/meson_saradc.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 62cc6fb0ef85..4fe6b997cd03 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -650,11 +650,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
 				  void __iomem *base)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	struct clk_init_data init;
 	const char *clk_parents[1];
 
-	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_div",
-				   dev_name(indio_dev->dev.parent));
+	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_div", dev_name(dev));
 	if (!init.name)
 		return -ENOMEM;
 
@@ -670,13 +670,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
 	priv->clk_div.hw.init = &init;
 	priv->clk_div.flags = 0;
 
-	priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
-					      &priv->clk_div.hw);
+	priv->adc_div_clk = devm_clk_register(dev, &priv->clk_div.hw);
 	if (WARN_ON(IS_ERR(priv->adc_div_clk)))
 		return PTR_ERR(priv->adc_div_clk);
 
-	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_en",
-				   dev_name(indio_dev->dev.parent));
+	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_en", dev_name(dev));
 	if (!init.name)
 		return -ENOMEM;
 
@@ -690,7 +688,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
 	priv->clk_gate.bit_idx = __ffs(MESON_SAR_ADC_REG3_CLK_EN);
 	priv->clk_gate.hw.init = &init;
 
-	priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
+	priv->adc_clk = devm_clk_register(dev, &priv->clk_gate.hw);
 	if (WARN_ON(IS_ERR(priv->adc_clk)))
 		return PTR_ERR(priv->adc_clk);
 
-- 
2.35.1

[PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Andy Shevchenko]

It feels wrong and actually inconsistent to attach managed resources
to the IIO device object, which is child of the physical device object.
The rest of the ->probe() calls do that against physical device.

Resolve this by reassigning managed resources to the physical device object.

Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: new fix-patch
 drivers/iio/adc/meson_saradc.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 62cc6fb0ef85..4fe6b997cd03 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -650,11 +650,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
 				  void __iomem *base)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	struct clk_init_data init;
 	const char *clk_parents[1];
 
-	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_div",
-				   dev_name(indio_dev->dev.parent));
+	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_div", dev_name(dev));
 	if (!init.name)
 		return -ENOMEM;
 
@@ -670,13 +670,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
 	priv->clk_div.hw.init = &init;
 	priv->clk_div.flags = 0;
 
-	priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
-					      &priv->clk_div.hw);
+	priv->adc_div_clk = devm_clk_register(dev, &priv->clk_div.hw);
 	if (WARN_ON(IS_ERR(priv->adc_div_clk)))
 		return PTR_ERR(priv->adc_div_clk);
 
-	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_en",
-				   dev_name(indio_dev->dev.parent));
+	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_en", dev_name(dev));
 	if (!init.name)
 		return -ENOMEM;
 
@@ -690,7 +688,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
 	priv->clk_gate.bit_idx = __ffs(MESON_SAR_ADC_REG3_CLK_EN);
 	priv->clk_gate.hw.init = &init;
 
-	priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
+	priv->adc_clk = devm_clk_register(dev, &priv->clk_gate.hw);
 	if (WARN_ON(IS_ERR(priv->adc_clk)))
 		return PTR_ERR(priv->adc_clk);
 
-- 
2.35.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Andy Shevchenko]

It feels wrong and actually inconsistent to attach managed resources
to the IIO device object, which is child of the physical device object.
The rest of the ->probe() calls do that against physical device.

Resolve this by reassigning managed resources to the physical device object.

Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: new fix-patch
 drivers/iio/adc/meson_saradc.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 62cc6fb0ef85..4fe6b997cd03 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -650,11 +650,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
 				  void __iomem *base)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	struct clk_init_data init;
 	const char *clk_parents[1];
 
-	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_div",
-				   dev_name(indio_dev->dev.parent));
+	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_div", dev_name(dev));
 	if (!init.name)
 		return -ENOMEM;
 
@@ -670,13 +670,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
 	priv->clk_div.hw.init = &init;
 	priv->clk_div.flags = 0;
 
-	priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
-					      &priv->clk_div.hw);
+	priv->adc_div_clk = devm_clk_register(dev, &priv->clk_div.hw);
 	if (WARN_ON(IS_ERR(priv->adc_div_clk)))
 		return PTR_ERR(priv->adc_div_clk);
 
-	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_en",
-				   dev_name(indio_dev->dev.parent));
+	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_en", dev_name(dev));
 	if (!init.name)
 		return -ENOMEM;
 
@@ -690,7 +688,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
 	priv->clk_gate.bit_idx = __ffs(MESON_SAR_ADC_REG3_CLK_EN);
 	priv->clk_gate.hw.init = &init;
 
-	priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
+	priv->adc_clk = devm_clk_register(dev, &priv->clk_gate.hw);
 	if (WARN_ON(IS_ERR(priv->adc_clk)))
 		return PTR_ERR(priv->adc_clk);
 
-- 
2.35.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 2/6] iio: adc: meson_saradc: Align messages to be with physical device prefix

[Andy Shevchenko]

Align messages to be printed with the physical device prefix as it's done
everywhere else in this driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: new patch (inspired by previous change)
 drivers/iio/adc/meson_saradc.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 4fe6b997cd03..658047370db0 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -345,6 +345,7 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
 					 int *val)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int regval, fifo_chan, fifo_val, count;
 
 	if (!wait_for_completion_timeout(&priv->done,
@@ -353,16 +354,14 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
 
 	count = meson_sar_adc_get_fifo_count(indio_dev);
 	if (count != 1) {
-		dev_err(&indio_dev->dev,
-			"ADC FIFO has %d element(s) instead of one\n", count);
+		dev_err(dev, "ADC FIFO has %d element(s) instead of one\n", count);
 		return -EINVAL;
 	}
 
 	regmap_read(priv->regmap, MESON_SAR_ADC_FIFO_RD, &regval);
 	fifo_chan = FIELD_GET(MESON_SAR_ADC_FIFO_RD_CHAN_ID_MASK, regval);
 	if (fifo_chan != chan->address) {
-		dev_err(&indio_dev->dev,
-			"ADC FIFO entry belongs to channel %d instead of %lu\n",
+		dev_err(dev, "ADC FIFO entry belongs to channel %d instead of %lu\n",
 			fifo_chan, chan->address);
 		return -EINVAL;
 	}
-- 
2.35.1

[PATCH v3 2/6] iio: adc: meson_saradc: Align messages to be with physical device prefix

[Andy Shevchenko]

Align messages to be printed with the physical device prefix as it's done
everywhere else in this driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: new patch (inspired by previous change)
 drivers/iio/adc/meson_saradc.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 4fe6b997cd03..658047370db0 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -345,6 +345,7 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
 					 int *val)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int regval, fifo_chan, fifo_val, count;
 
 	if (!wait_for_completion_timeout(&priv->done,
@@ -353,16 +354,14 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
 
 	count = meson_sar_adc_get_fifo_count(indio_dev);
 	if (count != 1) {
-		dev_err(&indio_dev->dev,
-			"ADC FIFO has %d element(s) instead of one\n", count);
+		dev_err(dev, "ADC FIFO has %d element(s) instead of one\n", count);
 		return -EINVAL;
 	}
 
 	regmap_read(priv->regmap, MESON_SAR_ADC_FIFO_RD, &regval);
 	fifo_chan = FIELD_GET(MESON_SAR_ADC_FIFO_RD_CHAN_ID_MASK, regval);
 	if (fifo_chan != chan->address) {
-		dev_err(&indio_dev->dev,
-			"ADC FIFO entry belongs to channel %d instead of %lu\n",
+		dev_err(dev, "ADC FIFO entry belongs to channel %d instead of %lu\n",
 			fifo_chan, chan->address);
 		return -EINVAL;
 	}
-- 
2.35.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v3 2/6] iio: adc: meson_saradc: Align messages to be with physical device prefix

[Andy Shevchenko]

Align messages to be printed with the physical device prefix as it's done
everywhere else in this driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: new patch (inspired by previous change)
 drivers/iio/adc/meson_saradc.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 4fe6b997cd03..658047370db0 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -345,6 +345,7 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
 					 int *val)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int regval, fifo_chan, fifo_val, count;
 
 	if (!wait_for_completion_timeout(&priv->done,
@@ -353,16 +354,14 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
 
 	count = meson_sar_adc_get_fifo_count(indio_dev);
 	if (count != 1) {
-		dev_err(&indio_dev->dev,
-			"ADC FIFO has %d element(s) instead of one\n", count);
+		dev_err(dev, "ADC FIFO has %d element(s) instead of one\n", count);
 		return -EINVAL;
 	}
 
 	regmap_read(priv->regmap, MESON_SAR_ADC_FIFO_RD, &regval);
 	fifo_chan = FIELD_GET(MESON_SAR_ADC_FIFO_RD_CHAN_ID_MASK, regval);
 	if (fifo_chan != chan->address) {
-		dev_err(&indio_dev->dev,
-			"ADC FIFO entry belongs to channel %d instead of %lu\n",
+		dev_err(dev, "ADC FIFO entry belongs to channel %d instead of %lu\n",
 			fifo_chan, chan->address);
 		return -EINVAL;
 	}
-- 
2.35.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 3/6] iio: adc: meson_saradc: Convert to use dev_err_probe()

[Andy Shevchenko]

It's fine to call dev_err_probe() in ->probe() when error code is known.
Convert the driver to use dev_err_probe().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: rebased on top of previous changes
 drivers/iio/adc/meson_saradc.c | 52 +++++++++++++---------------------
 1 file changed, 19 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 658047370db0..0d9aef362c5d 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -698,6 +698,7 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
 	u8 *buf, trimming_bits, trimming_mask, upper_adc_val;
+	struct device *dev = indio_dev->dev.parent;
 	struct nvmem_cell *temperature_calib;
 	size_t read_len;
 	int ret;
@@ -714,30 +715,23 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 		if (ret == -ENODEV)
 			return 0;
 
-		return dev_err_probe(indio_dev->dev.parent, ret,
-				     "failed to get temperature_calib cell\n");
+		return dev_err_probe(dev, ret, "failed to get temperature_calib cell\n");
 	}
 
 	priv->tsc_regmap =
 		syscon_regmap_lookup_by_phandle(indio_dev->dev.parent->of_node,
 						"amlogic,hhi-sysctrl");
-	if (IS_ERR(priv->tsc_regmap)) {
-		dev_err(indio_dev->dev.parent,
-			"failed to get amlogic,hhi-sysctrl regmap\n");
-		return PTR_ERR(priv->tsc_regmap);
-	}
+	if (IS_ERR(priv->tsc_regmap))
+		return dev_err_probe(dev, PTR_ERR(priv->tsc_regmap),
+				     "failed to get amlogic,hhi-sysctrl regmap\n");
 
 	read_len = MESON_SAR_ADC_EFUSE_BYTES;
 	buf = nvmem_cell_read(temperature_calib, &read_len);
-	if (IS_ERR(buf)) {
-		dev_err(indio_dev->dev.parent,
-			"failed to read temperature_calib cell\n");
-		return PTR_ERR(buf);
-	} else if (read_len != MESON_SAR_ADC_EFUSE_BYTES) {
+	if (IS_ERR(buf))
+		return dev_err_probe(dev, PTR_ERR(buf), "failed to read temperature_calib cell\n");
+	if (read_len != MESON_SAR_ADC_EFUSE_BYTES) {
 		kfree(buf);
-		dev_err(indio_dev->dev.parent,
-			"invalid read size of temperature_calib cell\n");
-		return -EINVAL;
+		return dev_err_probe(dev, -EINVAL, "invalid read size of temperature_calib cell\n");
 	}
 
 	trimming_bits = priv->param->temperature_trimming_bits;
@@ -762,6 +756,7 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 static int meson_sar_adc_init(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int regval, i, ret;
 
 	/*
@@ -885,18 +880,12 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 	}
 
 	ret = clk_set_parent(priv->adc_sel_clk, priv->clkin);
-	if (ret) {
-		dev_err(indio_dev->dev.parent,
-			"failed to set adc parent to clkin\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to set adc parent to clkin\n");
 
 	ret = clk_set_rate(priv->adc_clk, priv->param->clock_rate);
-	if (ret) {
-		dev_err(indio_dev->dev.parent,
-			"failed to set adc clock rate\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to set adc clock rate\n");
 
 	return 0;
 }
@@ -1183,24 +1172,21 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 {
 	const struct meson_sar_adc_data *match_data;
 	struct meson_sar_adc_priv *priv;
+	struct device *dev = &pdev->dev;
 	struct iio_dev *indio_dev;
 	void __iomem *base;
 	int irq, ret;
 
 	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
-	if (!indio_dev) {
-		dev_err(&pdev->dev, "failed allocating iio device\n");
-		return -ENOMEM;
-	}
+	if (!indio_dev)
+		return dev_err_probe(dev, -ENOMEM, "failed allocating iio device\n");
 
 	priv = iio_priv(indio_dev);
 	init_completion(&priv->done);
 
 	match_data = of_device_get_match_data(&pdev->dev);
-	if (!match_data) {
-		dev_err(&pdev->dev, "failed to get match data\n");
-		return -ENODEV;
-	}
+	if (!match_data)
+		return dev_err_probe(dev, -ENODEV, "failed to get match data\n");
 
 	priv->param = match_data->param;
 
-- 
2.35.1

[PATCH v3 3/6] iio: adc: meson_saradc: Convert to use dev_err_probe()

[Andy Shevchenko]

It's fine to call dev_err_probe() in ->probe() when error code is known.
Convert the driver to use dev_err_probe().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: rebased on top of previous changes
 drivers/iio/adc/meson_saradc.c | 52 +++++++++++++---------------------
 1 file changed, 19 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 658047370db0..0d9aef362c5d 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -698,6 +698,7 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
 	u8 *buf, trimming_bits, trimming_mask, upper_adc_val;
+	struct device *dev = indio_dev->dev.parent;
 	struct nvmem_cell *temperature_calib;
 	size_t read_len;
 	int ret;
@@ -714,30 +715,23 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 		if (ret == -ENODEV)
 			return 0;
 
-		return dev_err_probe(indio_dev->dev.parent, ret,
-				     "failed to get temperature_calib cell\n");
+		return dev_err_probe(dev, ret, "failed to get temperature_calib cell\n");
 	}
 
 	priv->tsc_regmap =
 		syscon_regmap_lookup_by_phandle(indio_dev->dev.parent->of_node,
 						"amlogic,hhi-sysctrl");
-	if (IS_ERR(priv->tsc_regmap)) {
-		dev_err(indio_dev->dev.parent,
-			"failed to get amlogic,hhi-sysctrl regmap\n");
-		return PTR_ERR(priv->tsc_regmap);
-	}
+	if (IS_ERR(priv->tsc_regmap))
+		return dev_err_probe(dev, PTR_ERR(priv->tsc_regmap),
+				     "failed to get amlogic,hhi-sysctrl regmap\n");
 
 	read_len = MESON_SAR_ADC_EFUSE_BYTES;
 	buf = nvmem_cell_read(temperature_calib, &read_len);
-	if (IS_ERR(buf)) {
-		dev_err(indio_dev->dev.parent,
-			"failed to read temperature_calib cell\n");
-		return PTR_ERR(buf);
-	} else if (read_len != MESON_SAR_ADC_EFUSE_BYTES) {
+	if (IS_ERR(buf))
+		return dev_err_probe(dev, PTR_ERR(buf), "failed to read temperature_calib cell\n");
+	if (read_len != MESON_SAR_ADC_EFUSE_BYTES) {
 		kfree(buf);
-		dev_err(indio_dev->dev.parent,
-			"invalid read size of temperature_calib cell\n");
-		return -EINVAL;
+		return dev_err_probe(dev, -EINVAL, "invalid read size of temperature_calib cell\n");
 	}
 
 	trimming_bits = priv->param->temperature_trimming_bits;
@@ -762,6 +756,7 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 static int meson_sar_adc_init(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int regval, i, ret;
 
 	/*
@@ -885,18 +880,12 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 	}
 
 	ret = clk_set_parent(priv->adc_sel_clk, priv->clkin);
-	if (ret) {
-		dev_err(indio_dev->dev.parent,
-			"failed to set adc parent to clkin\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to set adc parent to clkin\n");
 
 	ret = clk_set_rate(priv->adc_clk, priv->param->clock_rate);
-	if (ret) {
-		dev_err(indio_dev->dev.parent,
-			"failed to set adc clock rate\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to set adc clock rate\n");
 
 	return 0;
 }
@@ -1183,24 +1172,21 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 {
 	const struct meson_sar_adc_data *match_data;
 	struct meson_sar_adc_priv *priv;
+	struct device *dev = &pdev->dev;
 	struct iio_dev *indio_dev;
 	void __iomem *base;
 	int irq, ret;
 
 	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
-	if (!indio_dev) {
-		dev_err(&pdev->dev, "failed allocating iio device\n");
-		return -ENOMEM;
-	}
+	if (!indio_dev)
+		return dev_err_probe(dev, -ENOMEM, "failed allocating iio device\n");
 
 	priv = iio_priv(indio_dev);
 	init_completion(&priv->done);
 
 	match_data = of_device_get_match_data(&pdev->dev);
-	if (!match_data) {
-		dev_err(&pdev->dev, "failed to get match data\n");
-		return -ENODEV;
-	}
+	if (!match_data)
+		return dev_err_probe(dev, -ENODEV, "failed to get match data\n");
 
 	priv->param = match_data->param;
 
-- 
2.35.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v3 3/6] iio: adc: meson_saradc: Convert to use dev_err_probe()

[Andy Shevchenko]

It's fine to call dev_err_probe() in ->probe() when error code is known.
Convert the driver to use dev_err_probe().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: rebased on top of previous changes
 drivers/iio/adc/meson_saradc.c | 52 +++++++++++++---------------------
 1 file changed, 19 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 658047370db0..0d9aef362c5d 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -698,6 +698,7 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
 	u8 *buf, trimming_bits, trimming_mask, upper_adc_val;
+	struct device *dev = indio_dev->dev.parent;
 	struct nvmem_cell *temperature_calib;
 	size_t read_len;
 	int ret;
@@ -714,30 +715,23 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 		if (ret == -ENODEV)
 			return 0;
 
-		return dev_err_probe(indio_dev->dev.parent, ret,
-				     "failed to get temperature_calib cell\n");
+		return dev_err_probe(dev, ret, "failed to get temperature_calib cell\n");
 	}
 
 	priv->tsc_regmap =
 		syscon_regmap_lookup_by_phandle(indio_dev->dev.parent->of_node,
 						"amlogic,hhi-sysctrl");
-	if (IS_ERR(priv->tsc_regmap)) {
-		dev_err(indio_dev->dev.parent,
-			"failed to get amlogic,hhi-sysctrl regmap\n");
-		return PTR_ERR(priv->tsc_regmap);
-	}
+	if (IS_ERR(priv->tsc_regmap))
+		return dev_err_probe(dev, PTR_ERR(priv->tsc_regmap),
+				     "failed to get amlogic,hhi-sysctrl regmap\n");
 
 	read_len = MESON_SAR_ADC_EFUSE_BYTES;
 	buf = nvmem_cell_read(temperature_calib, &read_len);
-	if (IS_ERR(buf)) {
-		dev_err(indio_dev->dev.parent,
-			"failed to read temperature_calib cell\n");
-		return PTR_ERR(buf);
-	} else if (read_len != MESON_SAR_ADC_EFUSE_BYTES) {
+	if (IS_ERR(buf))
+		return dev_err_probe(dev, PTR_ERR(buf), "failed to read temperature_calib cell\n");
+	if (read_len != MESON_SAR_ADC_EFUSE_BYTES) {
 		kfree(buf);
-		dev_err(indio_dev->dev.parent,
-			"invalid read size of temperature_calib cell\n");
-		return -EINVAL;
+		return dev_err_probe(dev, -EINVAL, "invalid read size of temperature_calib cell\n");
 	}
 
 	trimming_bits = priv->param->temperature_trimming_bits;
@@ -762,6 +756,7 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 static int meson_sar_adc_init(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int regval, i, ret;
 
 	/*
@@ -885,18 +880,12 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 	}
 
 	ret = clk_set_parent(priv->adc_sel_clk, priv->clkin);
-	if (ret) {
-		dev_err(indio_dev->dev.parent,
-			"failed to set adc parent to clkin\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to set adc parent to clkin\n");
 
 	ret = clk_set_rate(priv->adc_clk, priv->param->clock_rate);
-	if (ret) {
-		dev_err(indio_dev->dev.parent,
-			"failed to set adc clock rate\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to set adc clock rate\n");
 
 	return 0;
 }
@@ -1183,24 +1172,21 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 {
 	const struct meson_sar_adc_data *match_data;
 	struct meson_sar_adc_priv *priv;
+	struct device *dev = &pdev->dev;
 	struct iio_dev *indio_dev;
 	void __iomem *base;
 	int irq, ret;
 
 	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
-	if (!indio_dev) {
-		dev_err(&pdev->dev, "failed allocating iio device\n");
-		return -ENOMEM;
-	}
+	if (!indio_dev)
+		return dev_err_probe(dev, -ENOMEM, "failed allocating iio device\n");
 
 	priv = iio_priv(indio_dev);
 	init_completion(&priv->done);
 
 	match_data = of_device_get_match_data(&pdev->dev);
-	if (!match_data) {
-		dev_err(&pdev->dev, "failed to get match data\n");
-		return -ENODEV;
-	}
+	if (!match_data)
+		return dev_err_probe(dev, -ENODEV, "failed to get match data\n");
 
 	priv->param = match_data->param;
 
-- 
2.35.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 4/6] iio: adc: meson_saradc: Use devm_clk_get_optional()

[Andy Shevchenko]

Replace open coded variants of devm_clk_get_optional().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
v3: no changes

 drivers/iio/adc/meson_saradc.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 0d9aef362c5d..337517a8e1ac 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -1222,23 +1222,13 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(priv->core_clk),
 				     "failed to get core clk\n");
 
-	priv->adc_clk = devm_clk_get(&pdev->dev, "adc_clk");
-	if (IS_ERR(priv->adc_clk)) {
-		if (PTR_ERR(priv->adc_clk) == -ENOENT)
-			priv->adc_clk = NULL;
-		else
-			return dev_err_probe(&pdev->dev, PTR_ERR(priv->adc_clk),
-					     "failed to get adc clk\n");
-	}
+	priv->adc_clk = devm_clk_get_optional(dev, "adc_clk");
+	if (IS_ERR(priv->adc_clk))
+		return dev_err_probe(dev, PTR_ERR(priv->adc_clk), "failed to get adc clk\n");
 
-	priv->adc_sel_clk = devm_clk_get(&pdev->dev, "adc_sel");
-	if (IS_ERR(priv->adc_sel_clk)) {
-		if (PTR_ERR(priv->adc_sel_clk) == -ENOENT)
-			priv->adc_sel_clk = NULL;
-		else
-			return dev_err_probe(&pdev->dev, PTR_ERR(priv->adc_sel_clk),
-					     "failed to get adc_sel clk\n");
-	}
+	priv->adc_sel_clk = devm_clk_get_optional(dev, "adc_sel");
+	if (IS_ERR(priv->adc_sel_clk))
+		return dev_err_probe(dev, PTR_ERR(priv->adc_sel_clk), "failed to get adc_sel clk\n");
 
 	/* on pre-GXBB SoCs the SAR ADC itself provides the ADC clock: */
 	if (!priv->adc_clk) {
-- 
2.35.1

[PATCH v3 4/6] iio: adc: meson_saradc: Use devm_clk_get_optional()

[Andy Shevchenko]

Replace open coded variants of devm_clk_get_optional().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
v3: no changes

 drivers/iio/adc/meson_saradc.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 0d9aef362c5d..337517a8e1ac 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -1222,23 +1222,13 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(priv->core_clk),
 				     "failed to get core clk\n");
 
-	priv->adc_clk = devm_clk_get(&pdev->dev, "adc_clk");
-	if (IS_ERR(priv->adc_clk)) {
-		if (PTR_ERR(priv->adc_clk) == -ENOENT)
-			priv->adc_clk = NULL;
-		else
-			return dev_err_probe(&pdev->dev, PTR_ERR(priv->adc_clk),
-					     "failed to get adc clk\n");
-	}
+	priv->adc_clk = devm_clk_get_optional(dev, "adc_clk");
+	if (IS_ERR(priv->adc_clk))
+		return dev_err_probe(dev, PTR_ERR(priv->adc_clk), "failed to get adc clk\n");
 
-	priv->adc_sel_clk = devm_clk_get(&pdev->dev, "adc_sel");
-	if (IS_ERR(priv->adc_sel_clk)) {
-		if (PTR_ERR(priv->adc_sel_clk) == -ENOENT)
-			priv->adc_sel_clk = NULL;
-		else
-			return dev_err_probe(&pdev->dev, PTR_ERR(priv->adc_sel_clk),
-					     "failed to get adc_sel clk\n");
-	}
+	priv->adc_sel_clk = devm_clk_get_optional(dev, "adc_sel");
+	if (IS_ERR(priv->adc_sel_clk))
+		return dev_err_probe(dev, PTR_ERR(priv->adc_sel_clk), "failed to get adc_sel clk\n");
 
 	/* on pre-GXBB SoCs the SAR ADC itself provides the ADC clock: */
 	if (!priv->adc_clk) {
-- 
2.35.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v3 4/6] iio: adc: meson_saradc: Use devm_clk_get_optional()

[Andy Shevchenko]

Replace open coded variants of devm_clk_get_optional().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
v3: no changes

 drivers/iio/adc/meson_saradc.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 0d9aef362c5d..337517a8e1ac 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -1222,23 +1222,13 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(priv->core_clk),
 				     "failed to get core clk\n");
 
-	priv->adc_clk = devm_clk_get(&pdev->dev, "adc_clk");
-	if (IS_ERR(priv->adc_clk)) {
-		if (PTR_ERR(priv->adc_clk) == -ENOENT)
-			priv->adc_clk = NULL;
-		else
-			return dev_err_probe(&pdev->dev, PTR_ERR(priv->adc_clk),
-					     "failed to get adc clk\n");
-	}
+	priv->adc_clk = devm_clk_get_optional(dev, "adc_clk");
+	if (IS_ERR(priv->adc_clk))
+		return dev_err_probe(dev, PTR_ERR(priv->adc_clk), "failed to get adc clk\n");
 
-	priv->adc_sel_clk = devm_clk_get(&pdev->dev, "adc_sel");
-	if (IS_ERR(priv->adc_sel_clk)) {
-		if (PTR_ERR(priv->adc_sel_clk) == -ENOENT)
-			priv->adc_sel_clk = NULL;
-		else
-			return dev_err_probe(&pdev->dev, PTR_ERR(priv->adc_sel_clk),
-					     "failed to get adc_sel clk\n");
-	}
+	priv->adc_sel_clk = devm_clk_get_optional(dev, "adc_sel");
+	if (IS_ERR(priv->adc_sel_clk))
+		return dev_err_probe(dev, PTR_ERR(priv->adc_sel_clk), "failed to get adc_sel clk\n");
 
 	/* on pre-GXBB SoCs the SAR ADC itself provides the ADC clock: */
 	if (!priv->adc_clk) {
-- 
2.35.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 5/6] iio: adc: meson_saradc: Use temporary variable for struct device

[Andy Shevchenko]

Use temporary variable for struct device to make code neater.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: rebased on top of previous changes

 drivers/iio/adc/meson_saradc.c | 53 ++++++++++++++--------------------
 1 file changed, 22 insertions(+), 31 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 337517a8e1ac..c100f933c12e 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -549,6 +549,7 @@ static int meson_sar_adc_get_sample(struct iio_dev *indio_dev,
 				    int *val)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int ret;
 
 	if (chan->type == IIO_TEMP && !priv->temperature_sensor_calibrated)
@@ -572,8 +573,7 @@ static int meson_sar_adc_get_sample(struct iio_dev *indio_dev,
 	meson_sar_adc_unlock(indio_dev);
 
 	if (ret) {
-		dev_warn(indio_dev->dev.parent,
-			 "failed to read sample for channel %lu: %d\n",
+		dev_warn(dev, "failed to read sample for channel %lu: %d\n",
 			 chan->address, ret);
 		return ret;
 	}
@@ -586,6 +586,7 @@ static int meson_sar_adc_iio_info_read_raw(struct iio_dev *indio_dev,
 					   int *val, int *val2, long mask)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int ret;
 
 	switch (mask) {
@@ -602,9 +603,7 @@ static int meson_sar_adc_iio_info_read_raw(struct iio_dev *indio_dev,
 		if (chan->type == IIO_VOLTAGE) {
 			ret = regulator_get_voltage(priv->vref);
 			if (ret < 0) {
-				dev_err(indio_dev->dev.parent,
-					"failed to get vref voltage: %d\n",
-					ret);
+				dev_err(dev, "failed to get vref voltage: %d\n", ret);
 				return ret;
 			}
 
@@ -703,8 +702,7 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 	size_t read_len;
 	int ret;
 
-	temperature_calib = devm_nvmem_cell_get(indio_dev->dev.parent,
-						"temperature_calib");
+	temperature_calib = devm_nvmem_cell_get(dev, "temperature_calib");
 	if (IS_ERR(temperature_calib)) {
 		ret = PTR_ERR(temperature_calib);
 
@@ -718,9 +716,7 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 		return dev_err_probe(dev, ret, "failed to get temperature_calib cell\n");
 	}
 
-	priv->tsc_regmap =
-		syscon_regmap_lookup_by_phandle(indio_dev->dev.parent->of_node,
-						"amlogic,hhi-sysctrl");
+	priv->tsc_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "amlogic,hhi-sysctrl");
 	if (IS_ERR(priv->tsc_regmap))
 		return dev_err_probe(dev, PTR_ERR(priv->tsc_regmap),
 				     "failed to get amlogic,hhi-sysctrl regmap\n");
@@ -908,6 +904,7 @@ static void meson_sar_adc_set_bandgap(struct iio_dev *indio_dev, bool on_off)
 static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int ret;
 	u32 regval;
 
@@ -917,14 +914,13 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
 
 	ret = regulator_enable(priv->vref);
 	if (ret < 0) {
-		dev_err(indio_dev->dev.parent,
-			"failed to enable vref regulator\n");
+		dev_err(dev, "failed to enable vref regulator\n");
 		goto err_vref;
 	}
 
 	ret = clk_prepare_enable(priv->core_clk);
 	if (ret) {
-		dev_err(indio_dev->dev.parent, "failed to enable core clk\n");
+		dev_err(dev, "failed to enable core clk\n");
 		goto err_core_clk;
 	}
 
@@ -942,7 +938,7 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
 
 	ret = clk_prepare_enable(priv->adc_clk);
 	if (ret) {
-		dev_err(indio_dev->dev.parent, "failed to enable adc clk\n");
+		dev_err(dev, "failed to enable adc clk\n");
 		goto err_adc_clk;
 	}
 
@@ -1177,14 +1173,14 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int irq, ret;
 
-	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
 	if (!indio_dev)
 		return dev_err_probe(dev, -ENOMEM, "failed allocating iio device\n");
 
 	priv = iio_priv(indio_dev);
 	init_completion(&priv->done);
 
-	match_data = of_device_get_match_data(&pdev->dev);
+	match_data = of_device_get_match_data(dev);
 	if (!match_data)
 		return dev_err_probe(dev, -ENODEV, "failed to get match data\n");
 
@@ -1198,29 +1194,25 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-	priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
-					     priv->param->regmap_config);
+	priv->regmap = devm_regmap_init_mmio(dev, base, priv->param->regmap_config);
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
 
-	irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	irq = irq_of_parse_and_map(dev->of_node, 0);
 	if (!irq)
 		return -EINVAL;
 
-	ret = devm_request_irq(&pdev->dev, irq, meson_sar_adc_irq, IRQF_SHARED,
-			       dev_name(&pdev->dev), indio_dev);
+	ret = devm_request_irq(dev, irq, meson_sar_adc_irq, IRQF_SHARED, dev_name(dev), indio_dev);
 	if (ret)
 		return ret;
 
-	priv->clkin = devm_clk_get(&pdev->dev, "clkin");
+	priv->clkin = devm_clk_get(dev, "clkin");
 	if (IS_ERR(priv->clkin))
-		return dev_err_probe(&pdev->dev, PTR_ERR(priv->clkin),
-				     "failed to get clkin\n");
+		return dev_err_probe(dev, PTR_ERR(priv->clkin), "failed to get clkin\n");
 
-	priv->core_clk = devm_clk_get(&pdev->dev, "core");
+	priv->core_clk = devm_clk_get(dev, "core");
 	if (IS_ERR(priv->core_clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(priv->core_clk),
-				     "failed to get core clk\n");
+		return dev_err_probe(dev, PTR_ERR(priv->core_clk), "failed to get core clk\n");
 
 	priv->adc_clk = devm_clk_get_optional(dev, "adc_clk");
 	if (IS_ERR(priv->adc_clk))
@@ -1237,10 +1229,9 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	priv->vref = devm_regulator_get(&pdev->dev, "vref");
+	priv->vref = devm_regulator_get(dev, "vref");
 	if (IS_ERR(priv->vref))
-		return dev_err_probe(&pdev->dev, PTR_ERR(priv->vref),
-				     "failed to get vref regulator\n");
+		return dev_err_probe(dev, PTR_ERR(priv->vref), "failed to get vref regulator\n");
 
 	priv->calibscale = MILLION;
 
@@ -1270,7 +1261,7 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 
 	ret = meson_sar_adc_calib(indio_dev);
 	if (ret)
-		dev_warn(&pdev->dev, "calibration failed\n");
+		dev_warn(dev, "calibration failed\n");
 
 	platform_set_drvdata(pdev, indio_dev);
 
-- 
2.35.1

[PATCH v3 5/6] iio: adc: meson_saradc: Use temporary variable for struct device

[Andy Shevchenko]

Use temporary variable for struct device to make code neater.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: rebased on top of previous changes

 drivers/iio/adc/meson_saradc.c | 53 ++++++++++++++--------------------
 1 file changed, 22 insertions(+), 31 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 337517a8e1ac..c100f933c12e 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -549,6 +549,7 @@ static int meson_sar_adc_get_sample(struct iio_dev *indio_dev,
 				    int *val)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int ret;
 
 	if (chan->type == IIO_TEMP && !priv->temperature_sensor_calibrated)
@@ -572,8 +573,7 @@ static int meson_sar_adc_get_sample(struct iio_dev *indio_dev,
 	meson_sar_adc_unlock(indio_dev);
 
 	if (ret) {
-		dev_warn(indio_dev->dev.parent,
-			 "failed to read sample for channel %lu: %d\n",
+		dev_warn(dev, "failed to read sample for channel %lu: %d\n",
 			 chan->address, ret);
 		return ret;
 	}
@@ -586,6 +586,7 @@ static int meson_sar_adc_iio_info_read_raw(struct iio_dev *indio_dev,
 					   int *val, int *val2, long mask)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int ret;
 
 	switch (mask) {
@@ -602,9 +603,7 @@ static int meson_sar_adc_iio_info_read_raw(struct iio_dev *indio_dev,
 		if (chan->type == IIO_VOLTAGE) {
 			ret = regulator_get_voltage(priv->vref);
 			if (ret < 0) {
-				dev_err(indio_dev->dev.parent,
-					"failed to get vref voltage: %d\n",
-					ret);
+				dev_err(dev, "failed to get vref voltage: %d\n", ret);
 				return ret;
 			}
 
@@ -703,8 +702,7 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 	size_t read_len;
 	int ret;
 
-	temperature_calib = devm_nvmem_cell_get(indio_dev->dev.parent,
-						"temperature_calib");
+	temperature_calib = devm_nvmem_cell_get(dev, "temperature_calib");
 	if (IS_ERR(temperature_calib)) {
 		ret = PTR_ERR(temperature_calib);
 
@@ -718,9 +716,7 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 		return dev_err_probe(dev, ret, "failed to get temperature_calib cell\n");
 	}
 
-	priv->tsc_regmap =
-		syscon_regmap_lookup_by_phandle(indio_dev->dev.parent->of_node,
-						"amlogic,hhi-sysctrl");
+	priv->tsc_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "amlogic,hhi-sysctrl");
 	if (IS_ERR(priv->tsc_regmap))
 		return dev_err_probe(dev, PTR_ERR(priv->tsc_regmap),
 				     "failed to get amlogic,hhi-sysctrl regmap\n");
@@ -908,6 +904,7 @@ static void meson_sar_adc_set_bandgap(struct iio_dev *indio_dev, bool on_off)
 static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int ret;
 	u32 regval;
 
@@ -917,14 +914,13 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
 
 	ret = regulator_enable(priv->vref);
 	if (ret < 0) {
-		dev_err(indio_dev->dev.parent,
-			"failed to enable vref regulator\n");
+		dev_err(dev, "failed to enable vref regulator\n");
 		goto err_vref;
 	}
 
 	ret = clk_prepare_enable(priv->core_clk);
 	if (ret) {
-		dev_err(indio_dev->dev.parent, "failed to enable core clk\n");
+		dev_err(dev, "failed to enable core clk\n");
 		goto err_core_clk;
 	}
 
@@ -942,7 +938,7 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
 
 	ret = clk_prepare_enable(priv->adc_clk);
 	if (ret) {
-		dev_err(indio_dev->dev.parent, "failed to enable adc clk\n");
+		dev_err(dev, "failed to enable adc clk\n");
 		goto err_adc_clk;
 	}
 
@@ -1177,14 +1173,14 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int irq, ret;
 
-	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
 	if (!indio_dev)
 		return dev_err_probe(dev, -ENOMEM, "failed allocating iio device\n");
 
 	priv = iio_priv(indio_dev);
 	init_completion(&priv->done);
 
-	match_data = of_device_get_match_data(&pdev->dev);
+	match_data = of_device_get_match_data(dev);
 	if (!match_data)
 		return dev_err_probe(dev, -ENODEV, "failed to get match data\n");
 
@@ -1198,29 +1194,25 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-	priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
-					     priv->param->regmap_config);
+	priv->regmap = devm_regmap_init_mmio(dev, base, priv->param->regmap_config);
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
 
-	irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	irq = irq_of_parse_and_map(dev->of_node, 0);
 	if (!irq)
 		return -EINVAL;
 
-	ret = devm_request_irq(&pdev->dev, irq, meson_sar_adc_irq, IRQF_SHARED,
-			       dev_name(&pdev->dev), indio_dev);
+	ret = devm_request_irq(dev, irq, meson_sar_adc_irq, IRQF_SHARED, dev_name(dev), indio_dev);
 	if (ret)
 		return ret;
 
-	priv->clkin = devm_clk_get(&pdev->dev, "clkin");
+	priv->clkin = devm_clk_get(dev, "clkin");
 	if (IS_ERR(priv->clkin))
-		return dev_err_probe(&pdev->dev, PTR_ERR(priv->clkin),
-				     "failed to get clkin\n");
+		return dev_err_probe(dev, PTR_ERR(priv->clkin), "failed to get clkin\n");
 
-	priv->core_clk = devm_clk_get(&pdev->dev, "core");
+	priv->core_clk = devm_clk_get(dev, "core");
 	if (IS_ERR(priv->core_clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(priv->core_clk),
-				     "failed to get core clk\n");
+		return dev_err_probe(dev, PTR_ERR(priv->core_clk), "failed to get core clk\n");
 
 	priv->adc_clk = devm_clk_get_optional(dev, "adc_clk");
 	if (IS_ERR(priv->adc_clk))
@@ -1237,10 +1229,9 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	priv->vref = devm_regulator_get(&pdev->dev, "vref");
+	priv->vref = devm_regulator_get(dev, "vref");
 	if (IS_ERR(priv->vref))
-		return dev_err_probe(&pdev->dev, PTR_ERR(priv->vref),
-				     "failed to get vref regulator\n");
+		return dev_err_probe(dev, PTR_ERR(priv->vref), "failed to get vref regulator\n");
 
 	priv->calibscale = MILLION;
 
@@ -1270,7 +1261,7 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 
 	ret = meson_sar_adc_calib(indio_dev);
 	if (ret)
-		dev_warn(&pdev->dev, "calibration failed\n");
+		dev_warn(dev, "calibration failed\n");
 
 	platform_set_drvdata(pdev, indio_dev);
 
-- 
2.35.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v3 5/6] iio: adc: meson_saradc: Use temporary variable for struct device

[Andy Shevchenko]

Use temporary variable for struct device to make code neater.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: rebased on top of previous changes

 drivers/iio/adc/meson_saradc.c | 53 ++++++++++++++--------------------
 1 file changed, 22 insertions(+), 31 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 337517a8e1ac..c100f933c12e 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -549,6 +549,7 @@ static int meson_sar_adc_get_sample(struct iio_dev *indio_dev,
 				    int *val)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int ret;
 
 	if (chan->type == IIO_TEMP && !priv->temperature_sensor_calibrated)
@@ -572,8 +573,7 @@ static int meson_sar_adc_get_sample(struct iio_dev *indio_dev,
 	meson_sar_adc_unlock(indio_dev);
 
 	if (ret) {
-		dev_warn(indio_dev->dev.parent,
-			 "failed to read sample for channel %lu: %d\n",
+		dev_warn(dev, "failed to read sample for channel %lu: %d\n",
 			 chan->address, ret);
 		return ret;
 	}
@@ -586,6 +586,7 @@ static int meson_sar_adc_iio_info_read_raw(struct iio_dev *indio_dev,
 					   int *val, int *val2, long mask)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int ret;
 
 	switch (mask) {
@@ -602,9 +603,7 @@ static int meson_sar_adc_iio_info_read_raw(struct iio_dev *indio_dev,
 		if (chan->type == IIO_VOLTAGE) {
 			ret = regulator_get_voltage(priv->vref);
 			if (ret < 0) {
-				dev_err(indio_dev->dev.parent,
-					"failed to get vref voltage: %d\n",
-					ret);
+				dev_err(dev, "failed to get vref voltage: %d\n", ret);
 				return ret;
 			}
 
@@ -703,8 +702,7 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 	size_t read_len;
 	int ret;
 
-	temperature_calib = devm_nvmem_cell_get(indio_dev->dev.parent,
-						"temperature_calib");
+	temperature_calib = devm_nvmem_cell_get(dev, "temperature_calib");
 	if (IS_ERR(temperature_calib)) {
 		ret = PTR_ERR(temperature_calib);
 
@@ -718,9 +716,7 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
 		return dev_err_probe(dev, ret, "failed to get temperature_calib cell\n");
 	}
 
-	priv->tsc_regmap =
-		syscon_regmap_lookup_by_phandle(indio_dev->dev.parent->of_node,
-						"amlogic,hhi-sysctrl");
+	priv->tsc_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "amlogic,hhi-sysctrl");
 	if (IS_ERR(priv->tsc_regmap))
 		return dev_err_probe(dev, PTR_ERR(priv->tsc_regmap),
 				     "failed to get amlogic,hhi-sysctrl regmap\n");
@@ -908,6 +904,7 @@ static void meson_sar_adc_set_bandgap(struct iio_dev *indio_dev, bool on_off)
 static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
 	int ret;
 	u32 regval;
 
@@ -917,14 +914,13 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
 
 	ret = regulator_enable(priv->vref);
 	if (ret < 0) {
-		dev_err(indio_dev->dev.parent,
-			"failed to enable vref regulator\n");
+		dev_err(dev, "failed to enable vref regulator\n");
 		goto err_vref;
 	}
 
 	ret = clk_prepare_enable(priv->core_clk);
 	if (ret) {
-		dev_err(indio_dev->dev.parent, "failed to enable core clk\n");
+		dev_err(dev, "failed to enable core clk\n");
 		goto err_core_clk;
 	}
 
@@ -942,7 +938,7 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
 
 	ret = clk_prepare_enable(priv->adc_clk);
 	if (ret) {
-		dev_err(indio_dev->dev.parent, "failed to enable adc clk\n");
+		dev_err(dev, "failed to enable adc clk\n");
 		goto err_adc_clk;
 	}
 
@@ -1177,14 +1173,14 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int irq, ret;
 
-	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
 	if (!indio_dev)
 		return dev_err_probe(dev, -ENOMEM, "failed allocating iio device\n");
 
 	priv = iio_priv(indio_dev);
 	init_completion(&priv->done);
 
-	match_data = of_device_get_match_data(&pdev->dev);
+	match_data = of_device_get_match_data(dev);
 	if (!match_data)
 		return dev_err_probe(dev, -ENODEV, "failed to get match data\n");
 
@@ -1198,29 +1194,25 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-	priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
-					     priv->param->regmap_config);
+	priv->regmap = devm_regmap_init_mmio(dev, base, priv->param->regmap_config);
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
 
-	irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	irq = irq_of_parse_and_map(dev->of_node, 0);
 	if (!irq)
 		return -EINVAL;
 
-	ret = devm_request_irq(&pdev->dev, irq, meson_sar_adc_irq, IRQF_SHARED,
-			       dev_name(&pdev->dev), indio_dev);
+	ret = devm_request_irq(dev, irq, meson_sar_adc_irq, IRQF_SHARED, dev_name(dev), indio_dev);
 	if (ret)
 		return ret;
 
-	priv->clkin = devm_clk_get(&pdev->dev, "clkin");
+	priv->clkin = devm_clk_get(dev, "clkin");
 	if (IS_ERR(priv->clkin))
-		return dev_err_probe(&pdev->dev, PTR_ERR(priv->clkin),
-				     "failed to get clkin\n");
+		return dev_err_probe(dev, PTR_ERR(priv->clkin), "failed to get clkin\n");
 
-	priv->core_clk = devm_clk_get(&pdev->dev, "core");
+	priv->core_clk = devm_clk_get(dev, "core");
 	if (IS_ERR(priv->core_clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(priv->core_clk),
-				     "failed to get core clk\n");
+		return dev_err_probe(dev, PTR_ERR(priv->core_clk), "failed to get core clk\n");
 
 	priv->adc_clk = devm_clk_get_optional(dev, "adc_clk");
 	if (IS_ERR(priv->adc_clk))
@@ -1237,10 +1229,9 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	priv->vref = devm_regulator_get(&pdev->dev, "vref");
+	priv->vref = devm_regulator_get(dev, "vref");
 	if (IS_ERR(priv->vref))
-		return dev_err_probe(&pdev->dev, PTR_ERR(priv->vref),
-				     "failed to get vref regulator\n");
+		return dev_err_probe(dev, PTR_ERR(priv->vref), "failed to get vref regulator\n");
 
 	priv->calibscale = MILLION;
 
@@ -1270,7 +1261,7 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 
 	ret = meson_sar_adc_calib(indio_dev);
 	if (ret)
-		dev_warn(&pdev->dev, "calibration failed\n");
+		dev_warn(dev, "calibration failed\n");
 
 	platform_set_drvdata(pdev, indio_dev);
 
-- 
2.35.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC PATCH v3 6/6] iio: adc: meson_saradc: Use regmap_read_poll_timeout() for busy wait

[Andy Shevchenko]

Simplify busy wait stages by using regmap_read_poll_timeout().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: new patch, but RFC, not always the read_poll_timeout() can be used, would
    be nice to have it tested.

 drivers/iio/adc/meson_saradc.c | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index c100f933c12e..c18be3c519af 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -322,22 +322,17 @@ static int meson_sar_adc_calib_val(struct iio_dev *indio_dev, int val)
 static int meson_sar_adc_wait_busy_clear(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
-	int regval, timeout = 10000;
+	int val;
 
 	/*
 	 * NOTE: we need a small delay before reading the status, otherwise
 	 * the sample engine may not have started internally (which would
 	 * seem to us that sampling is already finished).
 	 */
-	do {
-		udelay(1);
-		regmap_read(priv->regmap, MESON_SAR_ADC_REG0, &regval);
-	} while (FIELD_GET(MESON_SAR_ADC_REG0_BUSY_MASK, regval) && timeout--);
-
-	if (timeout < 0)
-		return -ETIMEDOUT;
-
-	return 0;
+	udelay(1);
+	return regmap_read_poll_timeout_atomic(priv->regmap, MESON_SAR_ADC_REG0, val,
+					       !FIELD_GET(MESON_SAR_ADC_REG0_BUSY_MASK, val),
+					       1, 10000);
 }
 
 static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
@@ -489,7 +484,7 @@ static void meson_sar_adc_stop_sample_engine(struct iio_dev *indio_dev)
 static int meson_sar_adc_lock(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
-	int val, timeout = 10000;
+	int val, ret;
 
 	mutex_lock(&indio_dev->mlock);
 
@@ -499,18 +494,18 @@ static int meson_sar_adc_lock(struct iio_dev *indio_dev)
 				   MESON_SAR_ADC_DELAY_KERNEL_BUSY,
 				   MESON_SAR_ADC_DELAY_KERNEL_BUSY);
 
+		udelay(1);
+
 		/*
 		 * wait until BL30 releases it's lock (so we can use the SAR
 		 * ADC)
 		 */
-		do {
-			udelay(1);
-			regmap_read(priv->regmap, MESON_SAR_ADC_DELAY, &val);
-		} while (val & MESON_SAR_ADC_DELAY_BL30_BUSY && timeout--);
-
-		if (timeout < 0) {
+		ret = regmap_read_poll_timeout_atomic(priv->regmap, MESON_SAR_ADC_DELAY, val,
+						      !(val & MESON_SAR_ADC_DELAY_BL30_BUSY),
+						      1, 10000);
+		if (ret) {
 			mutex_unlock(&indio_dev->mlock);
-			return -ETIMEDOUT;
+			return ret;
 		}
 	}
 
-- 
2.35.1

[RFC PATCH v3 6/6] iio: adc: meson_saradc: Use regmap_read_poll_timeout() for busy wait

[Andy Shevchenko]

Simplify busy wait stages by using regmap_read_poll_timeout().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: new patch, but RFC, not always the read_poll_timeout() can be used, would
    be nice to have it tested.

 drivers/iio/adc/meson_saradc.c | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index c100f933c12e..c18be3c519af 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -322,22 +322,17 @@ static int meson_sar_adc_calib_val(struct iio_dev *indio_dev, int val)
 static int meson_sar_adc_wait_busy_clear(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
-	int regval, timeout = 10000;
+	int val;
 
 	/*
 	 * NOTE: we need a small delay before reading the status, otherwise
 	 * the sample engine may not have started internally (which would
 	 * seem to us that sampling is already finished).
 	 */
-	do {
-		udelay(1);
-		regmap_read(priv->regmap, MESON_SAR_ADC_REG0, &regval);
-	} while (FIELD_GET(MESON_SAR_ADC_REG0_BUSY_MASK, regval) && timeout--);
-
-	if (timeout < 0)
-		return -ETIMEDOUT;
-
-	return 0;
+	udelay(1);
+	return regmap_read_poll_timeout_atomic(priv->regmap, MESON_SAR_ADC_REG0, val,
+					       !FIELD_GET(MESON_SAR_ADC_REG0_BUSY_MASK, val),
+					       1, 10000);
 }
 
 static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
@@ -489,7 +484,7 @@ static void meson_sar_adc_stop_sample_engine(struct iio_dev *indio_dev)
 static int meson_sar_adc_lock(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
-	int val, timeout = 10000;
+	int val, ret;
 
 	mutex_lock(&indio_dev->mlock);
 
@@ -499,18 +494,18 @@ static int meson_sar_adc_lock(struct iio_dev *indio_dev)
 				   MESON_SAR_ADC_DELAY_KERNEL_BUSY,
 				   MESON_SAR_ADC_DELAY_KERNEL_BUSY);
 
+		udelay(1);
+
 		/*
 		 * wait until BL30 releases it's lock (so we can use the SAR
 		 * ADC)
 		 */
-		do {
-			udelay(1);
-			regmap_read(priv->regmap, MESON_SAR_ADC_DELAY, &val);
-		} while (val & MESON_SAR_ADC_DELAY_BL30_BUSY && timeout--);
-
-		if (timeout < 0) {
+		ret = regmap_read_poll_timeout_atomic(priv->regmap, MESON_SAR_ADC_DELAY, val,
+						      !(val & MESON_SAR_ADC_DELAY_BL30_BUSY),
+						      1, 10000);
+		if (ret) {
 			mutex_unlock(&indio_dev->mlock);
-			return -ETIMEDOUT;
+			return ret;
 		}
 	}
 
-- 
2.35.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[RFC PATCH v3 6/6] iio: adc: meson_saradc: Use regmap_read_poll_timeout() for busy wait

[Andy Shevchenko]

Simplify busy wait stages by using regmap_read_poll_timeout().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: new patch, but RFC, not always the read_poll_timeout() can be used, would
    be nice to have it tested.

 drivers/iio/adc/meson_saradc.c | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index c100f933c12e..c18be3c519af 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -322,22 +322,17 @@ static int meson_sar_adc_calib_val(struct iio_dev *indio_dev, int val)
 static int meson_sar_adc_wait_busy_clear(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
-	int regval, timeout = 10000;
+	int val;
 
 	/*
 	 * NOTE: we need a small delay before reading the status, otherwise
 	 * the sample engine may not have started internally (which would
 	 * seem to us that sampling is already finished).
 	 */
-	do {
-		udelay(1);
-		regmap_read(priv->regmap, MESON_SAR_ADC_REG0, &regval);
-	} while (FIELD_GET(MESON_SAR_ADC_REG0_BUSY_MASK, regval) && timeout--);
-
-	if (timeout < 0)
-		return -ETIMEDOUT;
-
-	return 0;
+	udelay(1);
+	return regmap_read_poll_timeout_atomic(priv->regmap, MESON_SAR_ADC_REG0, val,
+					       !FIELD_GET(MESON_SAR_ADC_REG0_BUSY_MASK, val),
+					       1, 10000);
 }
 
 static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
@@ -489,7 +484,7 @@ static void meson_sar_adc_stop_sample_engine(struct iio_dev *indio_dev)
 static int meson_sar_adc_lock(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
-	int val, timeout = 10000;
+	int val, ret;
 
 	mutex_lock(&indio_dev->mlock);
 
@@ -499,18 +494,18 @@ static int meson_sar_adc_lock(struct iio_dev *indio_dev)
 				   MESON_SAR_ADC_DELAY_KERNEL_BUSY,
 				   MESON_SAR_ADC_DELAY_KERNEL_BUSY);
 
+		udelay(1);
+
 		/*
 		 * wait until BL30 releases it's lock (so we can use the SAR
 		 * ADC)
 		 */
-		do {
-			udelay(1);
-			regmap_read(priv->regmap, MESON_SAR_ADC_DELAY, &val);
-		} while (val & MESON_SAR_ADC_DELAY_BL30_BUSY && timeout--);
-
-		if (timeout < 0) {
+		ret = regmap_read_poll_timeout_atomic(priv->regmap, MESON_SAR_ADC_DELAY, val,
+						      !(val & MESON_SAR_ADC_DELAY_BL30_BUSY),
+						      1, 10000);
+		if (ret) {
 			mutex_unlock(&indio_dev->mlock);
-			return -ETIMEDOUT;
+			return ret;
 		}
 	}
 
-- 
2.35.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Jonathan Cameron]

On Fri,  3 Jun 2022 12:59:59 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> It feels wrong and actually inconsistent to attach managed resources
> to the IIO device object, which is child of the physical device object.
> The rest of the ->probe() calls do that against physical device.
> 
> Resolve this by reassigning managed resources to the physical device object.
> 
> Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Hi Andy,

This has come up a few times in the past (and we elected to not clean it up
at the time, though it wasn't a decision to never do so!)

It would definitely be wrong if we had another driver binding against
the resulting created device (funnily enough I reported a bug on a driver
doing just that earlier this week), but in this case it's harmless because the
the tear down will occur with a put_device() ultimately calling device_release()
and devres_release_all()

https://elixir.bootlin.com/linux/latest/source/drivers/base/core.c#L2211

Has a comment that covers this case (more or less).
"
	 * Some platform devices are driven without driver attached
	 * and managed resources may have been acquired.  Make sure
	 * all resources are released.
"

Now, I definitely agree with your statement that it's a bit inconsistent to
do this, just not the fixes tag.

One other suggestion below.


> ---
> v3: new fix-patch
>  drivers/iio/adc/meson_saradc.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 62cc6fb0ef85..4fe6b997cd03 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -650,11 +650,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
>  				  void __iomem *base)
>  {
>  	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> +	struct device *dev = indio_dev->dev.parent;

I'd slightly prefer the device was passed in explicitly to this function rather
than using the parent assignment which feels a little fragile. 


>  	struct clk_init_data init;
>  	const char *clk_parents[1];
>  
> -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_div",
> -				   dev_name(indio_dev->dev.parent));
> +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_div", dev_name(dev));
>  	if (!init.name)
>  		return -ENOMEM;
>  
> @@ -670,13 +670,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
>  	priv->clk_div.hw.init = &init;
>  	priv->clk_div.flags = 0;
>  
> -	priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
> -					      &priv->clk_div.hw);
> +	priv->adc_div_clk = devm_clk_register(dev, &priv->clk_div.hw);
>  	if (WARN_ON(IS_ERR(priv->adc_div_clk)))
>  		return PTR_ERR(priv->adc_div_clk);
>  
> -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_en",
> -				   dev_name(indio_dev->dev.parent));
> +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_en", dev_name(dev));
>  	if (!init.name)
>  		return -ENOMEM;
>  
> @@ -690,7 +688,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
>  	priv->clk_gate.bit_idx = __ffs(MESON_SAR_ADC_REG3_CLK_EN);
>  	priv->clk_gate.hw.init = &init;
>  
> -	priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
> +	priv->adc_clk = devm_clk_register(dev, &priv->clk_gate.hw);
>  	if (WARN_ON(IS_ERR(priv->adc_clk)))
>  		return PTR_ERR(priv->adc_clk);
>  

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Jonathan Cameron]

On Fri,  3 Jun 2022 12:59:59 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> It feels wrong and actually inconsistent to attach managed resources
> to the IIO device object, which is child of the physical device object.
> The rest of the ->probe() calls do that against physical device.
> 
> Resolve this by reassigning managed resources to the physical device object.
> 
> Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Hi Andy,

This has come up a few times in the past (and we elected to not clean it up
at the time, though it wasn't a decision to never do so!)

It would definitely be wrong if we had another driver binding against
the resulting created device (funnily enough I reported a bug on a driver
doing just that earlier this week), but in this case it's harmless because the
the tear down will occur with a put_device() ultimately calling device_release()
and devres_release_all()

https://elixir.bootlin.com/linux/latest/source/drivers/base/core.c#L2211

Has a comment that covers this case (more or less).
"
	 * Some platform devices are driven without driver attached
	 * and managed resources may have been acquired.  Make sure
	 * all resources are released.
"

Now, I definitely agree with your statement that it's a bit inconsistent to
do this, just not the fixes tag.

One other suggestion below.


> ---
> v3: new fix-patch
>  drivers/iio/adc/meson_saradc.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 62cc6fb0ef85..4fe6b997cd03 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -650,11 +650,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
>  				  void __iomem *base)
>  {
>  	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> +	struct device *dev = indio_dev->dev.parent;

I'd slightly prefer the device was passed in explicitly to this function rather
than using the parent assignment which feels a little fragile. 


>  	struct clk_init_data init;
>  	const char *clk_parents[1];
>  
> -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_div",
> -				   dev_name(indio_dev->dev.parent));
> +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_div", dev_name(dev));
>  	if (!init.name)
>  		return -ENOMEM;
>  
> @@ -670,13 +670,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
>  	priv->clk_div.hw.init = &init;
>  	priv->clk_div.flags = 0;
>  
> -	priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
> -					      &priv->clk_div.hw);
> +	priv->adc_div_clk = devm_clk_register(dev, &priv->clk_div.hw);
>  	if (WARN_ON(IS_ERR(priv->adc_div_clk)))
>  		return PTR_ERR(priv->adc_div_clk);
>  
> -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_en",
> -				   dev_name(indio_dev->dev.parent));
> +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_en", dev_name(dev));
>  	if (!init.name)
>  		return -ENOMEM;
>  
> @@ -690,7 +688,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
>  	priv->clk_gate.bit_idx = __ffs(MESON_SAR_ADC_REG3_CLK_EN);
>  	priv->clk_gate.hw.init = &init;
>  
> -	priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
> +	priv->adc_clk = devm_clk_register(dev, &priv->clk_gate.hw);
>  	if (WARN_ON(IS_ERR(priv->adc_clk)))
>  		return PTR_ERR(priv->adc_clk);
>  


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Jonathan Cameron]

On Fri,  3 Jun 2022 12:59:59 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> It feels wrong and actually inconsistent to attach managed resources
> to the IIO device object, which is child of the physical device object.
> The rest of the ->probe() calls do that against physical device.
> 
> Resolve this by reassigning managed resources to the physical device object.
> 
> Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Hi Andy,

This has come up a few times in the past (and we elected to not clean it up
at the time, though it wasn't a decision to never do so!)

It would definitely be wrong if we had another driver binding against
the resulting created device (funnily enough I reported a bug on a driver
doing just that earlier this week), but in this case it's harmless because the
the tear down will occur with a put_device() ultimately calling device_release()
and devres_release_all()

https://elixir.bootlin.com/linux/latest/source/drivers/base/core.c#L2211

Has a comment that covers this case (more or less).
"
	 * Some platform devices are driven without driver attached
	 * and managed resources may have been acquired.  Make sure
	 * all resources are released.
"

Now, I definitely agree with your statement that it's a bit inconsistent to
do this, just not the fixes tag.

One other suggestion below.


> ---
> v3: new fix-patch
>  drivers/iio/adc/meson_saradc.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 62cc6fb0ef85..4fe6b997cd03 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -650,11 +650,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
>  				  void __iomem *base)
>  {
>  	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> +	struct device *dev = indio_dev->dev.parent;

I'd slightly prefer the device was passed in explicitly to this function rather
than using the parent assignment which feels a little fragile. 


>  	struct clk_init_data init;
>  	const char *clk_parents[1];
>  
> -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_div",
> -				   dev_name(indio_dev->dev.parent));
> +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_div", dev_name(dev));
>  	if (!init.name)
>  		return -ENOMEM;
>  
> @@ -670,13 +670,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
>  	priv->clk_div.hw.init = &init;
>  	priv->clk_div.flags = 0;
>  
> -	priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
> -					      &priv->clk_div.hw);
> +	priv->adc_div_clk = devm_clk_register(dev, &priv->clk_div.hw);
>  	if (WARN_ON(IS_ERR(priv->adc_div_clk)))
>  		return PTR_ERR(priv->adc_div_clk);
>  
> -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_en",
> -				   dev_name(indio_dev->dev.parent));
> +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_en", dev_name(dev));
>  	if (!init.name)
>  		return -ENOMEM;
>  
> @@ -690,7 +688,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
>  	priv->clk_gate.bit_idx = __ffs(MESON_SAR_ADC_REG3_CLK_EN);
>  	priv->clk_gate.hw.init = &init;
>  
> -	priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
> +	priv->adc_clk = devm_clk_register(dev, &priv->clk_gate.hw);
>  	if (WARN_ON(IS_ERR(priv->adc_clk)))
>  		return PTR_ERR(priv->adc_clk);
>  


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 2/6] iio: adc: meson_saradc: Align messages to be with physical device prefix

[Jonathan Cameron]

On Fri,  3 Jun 2022 13:00:00 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Align messages to be printed with the physical device prefix as it's done
> everywhere else in this driver.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Here I'm fine with the use of the parent as can only happen after all
the setup is done, so it's obvious the parent will be assigned
(some might argue it is obvious in the previous patch, but I had to check
as I couldn't remember when we set it :)

Anyhow, LGTM. 
> ---
> v3: new patch (inspired by previous change)
>  drivers/iio/adc/meson_saradc.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 4fe6b997cd03..658047370db0 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -345,6 +345,7 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
>  					 int *val)
>  {
>  	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> +	struct device *dev = indio_dev->dev.parent;
>  	int regval, fifo_chan, fifo_val, count;
>  
>  	if (!wait_for_completion_timeout(&priv->done,
> @@ -353,16 +354,14 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
>  
>  	count = meson_sar_adc_get_fifo_count(indio_dev);
>  	if (count != 1) {
> -		dev_err(&indio_dev->dev,
> -			"ADC FIFO has %d element(s) instead of one\n", count);
> +		dev_err(dev, "ADC FIFO has %d element(s) instead of one\n", count);
>  		return -EINVAL;
>  	}
>  
>  	regmap_read(priv->regmap, MESON_SAR_ADC_FIFO_RD, &regval);
>  	fifo_chan = FIELD_GET(MESON_SAR_ADC_FIFO_RD_CHAN_ID_MASK, regval);
>  	if (fifo_chan != chan->address) {
> -		dev_err(&indio_dev->dev,
> -			"ADC FIFO entry belongs to channel %d instead of %lu\n",
> +		dev_err(dev, "ADC FIFO entry belongs to channel %d instead of %lu\n",
>  			fifo_chan, chan->address);
>  		return -EINVAL;
>  	}

Re: [PATCH v3 2/6] iio: adc: meson_saradc: Align messages to be with physical device prefix

[Jonathan Cameron]

On Fri,  3 Jun 2022 13:00:00 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Align messages to be printed with the physical device prefix as it's done
> everywhere else in this driver.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Here I'm fine with the use of the parent as can only happen after all
the setup is done, so it's obvious the parent will be assigned
(some might argue it is obvious in the previous patch, but I had to check
as I couldn't remember when we set it :)

Anyhow, LGTM. 
> ---
> v3: new patch (inspired by previous change)
>  drivers/iio/adc/meson_saradc.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 4fe6b997cd03..658047370db0 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -345,6 +345,7 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
>  					 int *val)
>  {
>  	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> +	struct device *dev = indio_dev->dev.parent;
>  	int regval, fifo_chan, fifo_val, count;
>  
>  	if (!wait_for_completion_timeout(&priv->done,
> @@ -353,16 +354,14 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
>  
>  	count = meson_sar_adc_get_fifo_count(indio_dev);
>  	if (count != 1) {
> -		dev_err(&indio_dev->dev,
> -			"ADC FIFO has %d element(s) instead of one\n", count);
> +		dev_err(dev, "ADC FIFO has %d element(s) instead of one\n", count);
>  		return -EINVAL;
>  	}
>  
>  	regmap_read(priv->regmap, MESON_SAR_ADC_FIFO_RD, &regval);
>  	fifo_chan = FIELD_GET(MESON_SAR_ADC_FIFO_RD_CHAN_ID_MASK, regval);
>  	if (fifo_chan != chan->address) {
> -		dev_err(&indio_dev->dev,
> -			"ADC FIFO entry belongs to channel %d instead of %lu\n",
> +		dev_err(dev, "ADC FIFO entry belongs to channel %d instead of %lu\n",
>  			fifo_chan, chan->address);
>  		return -EINVAL;
>  	}


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 2/6] iio: adc: meson_saradc: Align messages to be with physical device prefix

[Jonathan Cameron]

On Fri,  3 Jun 2022 13:00:00 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Align messages to be printed with the physical device prefix as it's done
> everywhere else in this driver.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Here I'm fine with the use of the parent as can only happen after all
the setup is done, so it's obvious the parent will be assigned
(some might argue it is obvious in the previous patch, but I had to check
as I couldn't remember when we set it :)

Anyhow, LGTM. 
> ---
> v3: new patch (inspired by previous change)
>  drivers/iio/adc/meson_saradc.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 4fe6b997cd03..658047370db0 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -345,6 +345,7 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
>  					 int *val)
>  {
>  	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> +	struct device *dev = indio_dev->dev.parent;
>  	int regval, fifo_chan, fifo_val, count;
>  
>  	if (!wait_for_completion_timeout(&priv->done,
> @@ -353,16 +354,14 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
>  
>  	count = meson_sar_adc_get_fifo_count(indio_dev);
>  	if (count != 1) {
> -		dev_err(&indio_dev->dev,
> -			"ADC FIFO has %d element(s) instead of one\n", count);
> +		dev_err(dev, "ADC FIFO has %d element(s) instead of one\n", count);
>  		return -EINVAL;
>  	}
>  
>  	regmap_read(priv->regmap, MESON_SAR_ADC_FIFO_RD, &regval);
>  	fifo_chan = FIELD_GET(MESON_SAR_ADC_FIFO_RD_CHAN_ID_MASK, regval);
>  	if (fifo_chan != chan->address) {
> -		dev_err(&indio_dev->dev,
> -			"ADC FIFO entry belongs to channel %d instead of %lu\n",
> +		dev_err(dev, "ADC FIFO entry belongs to channel %d instead of %lu\n",
>  			fifo_chan, chan->address);
>  		return -EINVAL;
>  	}


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Jonathan Cameron]

On Fri, 3 Jun 2022 17:06:12 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri,  3 Jun 2022 12:59:59 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > It feels wrong and actually inconsistent to attach managed resources
> > to the IIO device object, which is child of the physical device object.
> > The rest of the ->probe() calls do that against physical device.
> > 
> > Resolve this by reassigning managed resources to the physical device object.
> > 
> > Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
> > Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>  
> Hi Andy,
> 
> This has come up a few times in the past (and we elected to not clean it up
> at the time, though it wasn't a decision to never do so!)
> 
> It would definitely be wrong if we had another driver binding against
> the resulting created device (funnily enough I reported a bug on a driver
> doing just that earlier this week), but in this case it's harmless because the
> the tear down will occur with a put_device() ultimately calling device_release()
> and devres_release_all()
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/base/core.c#L2211
> 
> Has a comment that covers this case (more or less).
> "
> 	 * Some platform devices are driven without driver attached
> 	 * and managed resources may have been acquired.  Make sure
> 	 * all resources are released.
> "
> 
> Now, I definitely agree with your statement that it's a bit inconsistent to
> do this, just not the fixes tag.
> 
> One other suggestion below.
> 
> 
> > ---
> > v3: new fix-patch
> >  drivers/iio/adc/meson_saradc.c | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> > index 62cc6fb0ef85..4fe6b997cd03 100644
> > --- a/drivers/iio/adc/meson_saradc.c
> > +++ b/drivers/iio/adc/meson_saradc.c
> > @@ -650,11 +650,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> >  				  void __iomem *base)
> >  {
> >  	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> > +	struct device *dev = indio_dev->dev.parent;  
> 
> I'd slightly prefer the device was passed in explicitly to this function rather
> than using the parent assignment which feels a little fragile. 

Meh, ignore this. I see from one of the later patches, the driver is already
making the assumption this is set in other calls, so we aren't making anything
worse with this change.

Jonathan

> 
> 
> >  	struct clk_init_data init;
> >  	const char *clk_parents[1];
> >  
> > -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_div",
> > -				   dev_name(indio_dev->dev.parent));
> > +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_div", dev_name(dev));
> >  	if (!init.name)
> >  		return -ENOMEM;
> >  
> > @@ -670,13 +670,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> >  	priv->clk_div.hw.init = &init;
> >  	priv->clk_div.flags = 0;
> >  
> > -	priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
> > -					      &priv->clk_div.hw);
> > +	priv->adc_div_clk = devm_clk_register(dev, &priv->clk_div.hw);
> >  	if (WARN_ON(IS_ERR(priv->adc_div_clk)))
> >  		return PTR_ERR(priv->adc_div_clk);
> >  
> > -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_en",
> > -				   dev_name(indio_dev->dev.parent));
> > +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_en", dev_name(dev));
> >  	if (!init.name)
> >  		return -ENOMEM;
> >  
> > @@ -690,7 +688,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> >  	priv->clk_gate.bit_idx = __ffs(MESON_SAR_ADC_REG3_CLK_EN);
> >  	priv->clk_gate.hw.init = &init;
> >  
> > -	priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
> > +	priv->adc_clk = devm_clk_register(dev, &priv->clk_gate.hw);
> >  	if (WARN_ON(IS_ERR(priv->adc_clk)))
> >  		return PTR_ERR(priv->adc_clk);
> >    
> 

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Jonathan Cameron]

On Fri, 3 Jun 2022 17:06:12 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri,  3 Jun 2022 12:59:59 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > It feels wrong and actually inconsistent to attach managed resources
> > to the IIO device object, which is child of the physical device object.
> > The rest of the ->probe() calls do that against physical device.
> > 
> > Resolve this by reassigning managed resources to the physical device object.
> > 
> > Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
> > Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>  
> Hi Andy,
> 
> This has come up a few times in the past (and we elected to not clean it up
> at the time, though it wasn't a decision to never do so!)
> 
> It would definitely be wrong if we had another driver binding against
> the resulting created device (funnily enough I reported a bug on a driver
> doing just that earlier this week), but in this case it's harmless because the
> the tear down will occur with a put_device() ultimately calling device_release()
> and devres_release_all()
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/base/core.c#L2211
> 
> Has a comment that covers this case (more or less).
> "
> 	 * Some platform devices are driven without driver attached
> 	 * and managed resources may have been acquired.  Make sure
> 	 * all resources are released.
> "
> 
> Now, I definitely agree with your statement that it's a bit inconsistent to
> do this, just not the fixes tag.
> 
> One other suggestion below.
> 
> 
> > ---
> > v3: new fix-patch
> >  drivers/iio/adc/meson_saradc.c | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> > index 62cc6fb0ef85..4fe6b997cd03 100644
> > --- a/drivers/iio/adc/meson_saradc.c
> > +++ b/drivers/iio/adc/meson_saradc.c
> > @@ -650,11 +650,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> >  				  void __iomem *base)
> >  {
> >  	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> > +	struct device *dev = indio_dev->dev.parent;  
> 
> I'd slightly prefer the device was passed in explicitly to this function rather
> than using the parent assignment which feels a little fragile. 

Meh, ignore this. I see from one of the later patches, the driver is already
making the assumption this is set in other calls, so we aren't making anything
worse with this change.

Jonathan

> 
> 
> >  	struct clk_init_data init;
> >  	const char *clk_parents[1];
> >  
> > -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_div",
> > -				   dev_name(indio_dev->dev.parent));
> > +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_div", dev_name(dev));
> >  	if (!init.name)
> >  		return -ENOMEM;
> >  
> > @@ -670,13 +670,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> >  	priv->clk_div.hw.init = &init;
> >  	priv->clk_div.flags = 0;
> >  
> > -	priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
> > -					      &priv->clk_div.hw);
> > +	priv->adc_div_clk = devm_clk_register(dev, &priv->clk_div.hw);
> >  	if (WARN_ON(IS_ERR(priv->adc_div_clk)))
> >  		return PTR_ERR(priv->adc_div_clk);
> >  
> > -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_en",
> > -				   dev_name(indio_dev->dev.parent));
> > +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_en", dev_name(dev));
> >  	if (!init.name)
> >  		return -ENOMEM;
> >  
> > @@ -690,7 +688,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> >  	priv->clk_gate.bit_idx = __ffs(MESON_SAR_ADC_REG3_CLK_EN);
> >  	priv->clk_gate.hw.init = &init;
> >  
> > -	priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
> > +	priv->adc_clk = devm_clk_register(dev, &priv->clk_gate.hw);
> >  	if (WARN_ON(IS_ERR(priv->adc_clk)))
> >  		return PTR_ERR(priv->adc_clk);
> >    
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Jonathan Cameron]

On Fri, 3 Jun 2022 17:06:12 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri,  3 Jun 2022 12:59:59 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > It feels wrong and actually inconsistent to attach managed resources
> > to the IIO device object, which is child of the physical device object.
> > The rest of the ->probe() calls do that against physical device.
> > 
> > Resolve this by reassigning managed resources to the physical device object.
> > 
> > Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
> > Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>  
> Hi Andy,
> 
> This has come up a few times in the past (and we elected to not clean it up
> at the time, though it wasn't a decision to never do so!)
> 
> It would definitely be wrong if we had another driver binding against
> the resulting created device (funnily enough I reported a bug on a driver
> doing just that earlier this week), but in this case it's harmless because the
> the tear down will occur with a put_device() ultimately calling device_release()
> and devres_release_all()
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/base/core.c#L2211
> 
> Has a comment that covers this case (more or less).
> "
> 	 * Some platform devices are driven without driver attached
> 	 * and managed resources may have been acquired.  Make sure
> 	 * all resources are released.
> "
> 
> Now, I definitely agree with your statement that it's a bit inconsistent to
> do this, just not the fixes tag.
> 
> One other suggestion below.
> 
> 
> > ---
> > v3: new fix-patch
> >  drivers/iio/adc/meson_saradc.c | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> > index 62cc6fb0ef85..4fe6b997cd03 100644
> > --- a/drivers/iio/adc/meson_saradc.c
> > +++ b/drivers/iio/adc/meson_saradc.c
> > @@ -650,11 +650,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> >  				  void __iomem *base)
> >  {
> >  	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> > +	struct device *dev = indio_dev->dev.parent;  
> 
> I'd slightly prefer the device was passed in explicitly to this function rather
> than using the parent assignment which feels a little fragile. 

Meh, ignore this. I see from one of the later patches, the driver is already
making the assumption this is set in other calls, so we aren't making anything
worse with this change.

Jonathan

> 
> 
> >  	struct clk_init_data init;
> >  	const char *clk_parents[1];
> >  
> > -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_div",
> > -				   dev_name(indio_dev->dev.parent));
> > +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_div", dev_name(dev));
> >  	if (!init.name)
> >  		return -ENOMEM;
> >  
> > @@ -670,13 +670,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> >  	priv->clk_div.hw.init = &init;
> >  	priv->clk_div.flags = 0;
> >  
> > -	priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
> > -					      &priv->clk_div.hw);
> > +	priv->adc_div_clk = devm_clk_register(dev, &priv->clk_div.hw);
> >  	if (WARN_ON(IS_ERR(priv->adc_div_clk)))
> >  		return PTR_ERR(priv->adc_div_clk);
> >  
> > -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_en",
> > -				   dev_name(indio_dev->dev.parent));
> > +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_en", dev_name(dev));
> >  	if (!init.name)
> >  		return -ENOMEM;
> >  
> > @@ -690,7 +688,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> >  	priv->clk_gate.bit_idx = __ffs(MESON_SAR_ADC_REG3_CLK_EN);
> >  	priv->clk_gate.hw.init = &init;
> >  
> > -	priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
> > +	priv->adc_clk = devm_clk_register(dev, &priv->clk_gate.hw);
> >  	if (WARN_ON(IS_ERR(priv->adc_clk)))
> >  		return PTR_ERR(priv->adc_clk);
> >    
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Jonathan Cameron]

On Fri, 3 Jun 2022 17:23:07 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 3 Jun 2022 17:06:12 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Fri,  3 Jun 2022 12:59:59 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >   
> > > It feels wrong and actually inconsistent to attach managed resources
> > > to the IIO device object, which is child of the physical device object.
> > > The rest of the ->probe() calls do that against physical device.
> > > 
> > > Resolve this by reassigning managed resources to the physical device object.
> > > 
> > > Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
> > > Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>    
> > Hi Andy,
> > 
> > This has come up a few times in the past (and we elected to not clean it up
> > at the time, though it wasn't a decision to never do so!)
> > 
> > It would definitely be wrong if we had another driver binding against
> > the resulting created device (funnily enough I reported a bug on a driver
> > doing just that earlier this week), but in this case it's harmless because the
> > the tear down will occur with a put_device() ultimately calling device_release()
> > and devres_release_all()
> > 
> > https://elixir.bootlin.com/linux/latest/source/drivers/base/core.c#L2211
> > 
> > Has a comment that covers this case (more or less).
> > "
> > 	 * Some platform devices are driven without driver attached
> > 	 * and managed resources may have been acquired.  Make sure
> > 	 * all resources are released.
> > "
> > 
> > Now, I definitely agree with your statement that it's a bit inconsistent to
> > do this, just not the fixes tag.
> > 
> > One other suggestion below.

Andy, put a cover letter on these larger series - if nothing else it gives
somewhere convenient for people to give tags for the whole series, or 
maintainer to say what they are doing with it.

Anyhow, I'm fine with the series, but will leave it on list for a while
longer, particularly to get patch 6 some eyes + testing.

Currently I plan to drop the fixes tag from this first patch, but I'm prepared
to be convinced it's a bug fix rather than a consistency cleanup.

Jonathan

> > 
> >   
> > > ---
> > > v3: new fix-patch
> > >  drivers/iio/adc/meson_saradc.c | 12 +++++-------
> > >  1 file changed, 5 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> > > index 62cc6fb0ef85..4fe6b997cd03 100644
> > > --- a/drivers/iio/adc/meson_saradc.c
> > > +++ b/drivers/iio/adc/meson_saradc.c
> > > @@ -650,11 +650,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> > >  				  void __iomem *base)
> > >  {
> > >  	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> > > +	struct device *dev = indio_dev->dev.parent;    
> > 
> > I'd slightly prefer the device was passed in explicitly to this function rather
> > than using the parent assignment which feels a little fragile.   
> 
> Meh, ignore this. I see from one of the later patches, the driver is already
> making the assumption this is set in other calls, so we aren't making anything
> worse with this change.
> 
> Jonathan
> 
> > 
> >   
> > >  	struct clk_init_data init;
> > >  	const char *clk_parents[1];
> > >  
> > > -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_div",
> > > -				   dev_name(indio_dev->dev.parent));
> > > +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_div", dev_name(dev));
> > >  	if (!init.name)
> > >  		return -ENOMEM;
> > >  
> > > @@ -670,13 +670,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> > >  	priv->clk_div.hw.init = &init;
> > >  	priv->clk_div.flags = 0;
> > >  
> > > -	priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
> > > -					      &priv->clk_div.hw);
> > > +	priv->adc_div_clk = devm_clk_register(dev, &priv->clk_div.hw);
> > >  	if (WARN_ON(IS_ERR(priv->adc_div_clk)))
> > >  		return PTR_ERR(priv->adc_div_clk);
> > >  
> > > -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_en",
> > > -				   dev_name(indio_dev->dev.parent));
> > > +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_en", dev_name(dev));
> > >  	if (!init.name)
> > >  		return -ENOMEM;
> > >  
> > > @@ -690,7 +688,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> > >  	priv->clk_gate.bit_idx = __ffs(MESON_SAR_ADC_REG3_CLK_EN);
> > >  	priv->clk_gate.hw.init = &init;
> > >  
> > > -	priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
> > > +	priv->adc_clk = devm_clk_register(dev, &priv->clk_gate.hw);
> > >  	if (WARN_ON(IS_ERR(priv->adc_clk)))
> > >  		return PTR_ERR(priv->adc_clk);
> > >      
> >   
> 

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Jonathan Cameron]

On Fri, 3 Jun 2022 17:23:07 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 3 Jun 2022 17:06:12 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Fri,  3 Jun 2022 12:59:59 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >   
> > > It feels wrong and actually inconsistent to attach managed resources
> > > to the IIO device object, which is child of the physical device object.
> > > The rest of the ->probe() calls do that against physical device.
> > > 
> > > Resolve this by reassigning managed resources to the physical device object.
> > > 
> > > Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
> > > Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>    
> > Hi Andy,
> > 
> > This has come up a few times in the past (and we elected to not clean it up
> > at the time, though it wasn't a decision to never do so!)
> > 
> > It would definitely be wrong if we had another driver binding against
> > the resulting created device (funnily enough I reported a bug on a driver
> > doing just that earlier this week), but in this case it's harmless because the
> > the tear down will occur with a put_device() ultimately calling device_release()
> > and devres_release_all()
> > 
> > https://elixir.bootlin.com/linux/latest/source/drivers/base/core.c#L2211
> > 
> > Has a comment that covers this case (more or less).
> > "
> > 	 * Some platform devices are driven without driver attached
> > 	 * and managed resources may have been acquired.  Make sure
> > 	 * all resources are released.
> > "
> > 
> > Now, I definitely agree with your statement that it's a bit inconsistent to
> > do this, just not the fixes tag.
> > 
> > One other suggestion below.

Andy, put a cover letter on these larger series - if nothing else it gives
somewhere convenient for people to give tags for the whole series, or 
maintainer to say what they are doing with it.

Anyhow, I'm fine with the series, but will leave it on list for a while
longer, particularly to get patch 6 some eyes + testing.

Currently I plan to drop the fixes tag from this first patch, but I'm prepared
to be convinced it's a bug fix rather than a consistency cleanup.

Jonathan

> > 
> >   
> > > ---
> > > v3: new fix-patch
> > >  drivers/iio/adc/meson_saradc.c | 12 +++++-------
> > >  1 file changed, 5 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> > > index 62cc6fb0ef85..4fe6b997cd03 100644
> > > --- a/drivers/iio/adc/meson_saradc.c
> > > +++ b/drivers/iio/adc/meson_saradc.c
> > > @@ -650,11 +650,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> > >  				  void __iomem *base)
> > >  {
> > >  	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> > > +	struct device *dev = indio_dev->dev.parent;    
> > 
> > I'd slightly prefer the device was passed in explicitly to this function rather
> > than using the parent assignment which feels a little fragile.   
> 
> Meh, ignore this. I see from one of the later patches, the driver is already
> making the assumption this is set in other calls, so we aren't making anything
> worse with this change.
> 
> Jonathan
> 
> > 
> >   
> > >  	struct clk_init_data init;
> > >  	const char *clk_parents[1];
> > >  
> > > -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_div",
> > > -				   dev_name(indio_dev->dev.parent));
> > > +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_div", dev_name(dev));
> > >  	if (!init.name)
> > >  		return -ENOMEM;
> > >  
> > > @@ -670,13 +670,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> > >  	priv->clk_div.hw.init = &init;
> > >  	priv->clk_div.flags = 0;
> > >  
> > > -	priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
> > > -					      &priv->clk_div.hw);
> > > +	priv->adc_div_clk = devm_clk_register(dev, &priv->clk_div.hw);
> > >  	if (WARN_ON(IS_ERR(priv->adc_div_clk)))
> > >  		return PTR_ERR(priv->adc_div_clk);
> > >  
> > > -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_en",
> > > -				   dev_name(indio_dev->dev.parent));
> > > +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_en", dev_name(dev));
> > >  	if (!init.name)
> > >  		return -ENOMEM;
> > >  
> > > @@ -690,7 +688,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> > >  	priv->clk_gate.bit_idx = __ffs(MESON_SAR_ADC_REG3_CLK_EN);
> > >  	priv->clk_gate.hw.init = &init;
> > >  
> > > -	priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
> > > +	priv->adc_clk = devm_clk_register(dev, &priv->clk_gate.hw);
> > >  	if (WARN_ON(IS_ERR(priv->adc_clk)))
> > >  		return PTR_ERR(priv->adc_clk);
> > >      
> >   
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Jonathan Cameron]

On Fri, 3 Jun 2022 17:23:07 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 3 Jun 2022 17:06:12 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Fri,  3 Jun 2022 12:59:59 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >   
> > > It feels wrong and actually inconsistent to attach managed resources
> > > to the IIO device object, which is child of the physical device object.
> > > The rest of the ->probe() calls do that against physical device.
> > > 
> > > Resolve this by reassigning managed resources to the physical device object.
> > > 
> > > Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
> > > Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>    
> > Hi Andy,
> > 
> > This has come up a few times in the past (and we elected to not clean it up
> > at the time, though it wasn't a decision to never do so!)
> > 
> > It would definitely be wrong if we had another driver binding against
> > the resulting created device (funnily enough I reported a bug on a driver
> > doing just that earlier this week), but in this case it's harmless because the
> > the tear down will occur with a put_device() ultimately calling device_release()
> > and devres_release_all()
> > 
> > https://elixir.bootlin.com/linux/latest/source/drivers/base/core.c#L2211
> > 
> > Has a comment that covers this case (more or less).
> > "
> > 	 * Some platform devices are driven without driver attached
> > 	 * and managed resources may have been acquired.  Make sure
> > 	 * all resources are released.
> > "
> > 
> > Now, I definitely agree with your statement that it's a bit inconsistent to
> > do this, just not the fixes tag.
> > 
> > One other suggestion below.

Andy, put a cover letter on these larger series - if nothing else it gives
somewhere convenient for people to give tags for the whole series, or 
maintainer to say what they are doing with it.

Anyhow, I'm fine with the series, but will leave it on list for a while
longer, particularly to get patch 6 some eyes + testing.

Currently I plan to drop the fixes tag from this first patch, but I'm prepared
to be convinced it's a bug fix rather than a consistency cleanup.

Jonathan

> > 
> >   
> > > ---
> > > v3: new fix-patch
> > >  drivers/iio/adc/meson_saradc.c | 12 +++++-------
> > >  1 file changed, 5 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> > > index 62cc6fb0ef85..4fe6b997cd03 100644
> > > --- a/drivers/iio/adc/meson_saradc.c
> > > +++ b/drivers/iio/adc/meson_saradc.c
> > > @@ -650,11 +650,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> > >  				  void __iomem *base)
> > >  {
> > >  	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> > > +	struct device *dev = indio_dev->dev.parent;    
> > 
> > I'd slightly prefer the device was passed in explicitly to this function rather
> > than using the parent assignment which feels a little fragile.   
> 
> Meh, ignore this. I see from one of the later patches, the driver is already
> making the assumption this is set in other calls, so we aren't making anything
> worse with this change.
> 
> Jonathan
> 
> > 
> >   
> > >  	struct clk_init_data init;
> > >  	const char *clk_parents[1];
> > >  
> > > -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_div",
> > > -				   dev_name(indio_dev->dev.parent));
> > > +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_div", dev_name(dev));
> > >  	if (!init.name)
> > >  		return -ENOMEM;
> > >  
> > > @@ -670,13 +670,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> > >  	priv->clk_div.hw.init = &init;
> > >  	priv->clk_div.flags = 0;
> > >  
> > > -	priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
> > > -					      &priv->clk_div.hw);
> > > +	priv->adc_div_clk = devm_clk_register(dev, &priv->clk_div.hw);
> > >  	if (WARN_ON(IS_ERR(priv->adc_div_clk)))
> > >  		return PTR_ERR(priv->adc_div_clk);
> > >  
> > > -	init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_en",
> > > -				   dev_name(indio_dev->dev.parent));
> > > +	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_en", dev_name(dev));
> > >  	if (!init.name)
> > >  		return -ENOMEM;
> > >  
> > > @@ -690,7 +688,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
> > >  	priv->clk_gate.bit_idx = __ffs(MESON_SAR_ADC_REG3_CLK_EN);
> > >  	priv->clk_gate.hw.init = &init;
> > >  
> > > -	priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
> > > +	priv->adc_clk = devm_clk_register(dev, &priv->clk_gate.hw);
> > >  	if (WARN_ON(IS_ERR(priv->adc_clk)))
> > >  		return PTR_ERR(priv->adc_clk);
> > >      
> >   
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 2/6] iio: adc: meson_saradc: Align messages to be with physical device prefix

[Andy Shevchenko]

On Fri, Jun 03, 2022 at 05:21:17PM +0100, Jonathan Cameron wrote:
> On Fri,  3 Jun 2022 13:00:00 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > Align messages to be printed with the physical device prefix as it's done
> > everywhere else in this driver.

> Here I'm fine with the use of the parent as can only happen after all
> the setup is done, so it's obvious the parent will be assigned
> (some might argue it is obvious in the previous patch, but I had to check
> as I couldn't remember when we set it :)
> 
> Anyhow, LGTM. 

Thanks!

> > v3: new patch (inspired by previous change)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/6] iio: adc: meson_saradc: Align messages to be with physical device prefix

[Andy Shevchenko]

On Fri, Jun 03, 2022 at 05:21:17PM +0100, Jonathan Cameron wrote:
> On Fri,  3 Jun 2022 13:00:00 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > Align messages to be printed with the physical device prefix as it's done
> > everywhere else in this driver.

> Here I'm fine with the use of the parent as can only happen after all
> the setup is done, so it's obvious the parent will be assigned
> (some might argue it is obvious in the previous patch, but I had to check
> as I couldn't remember when we set it :)
> 
> Anyhow, LGTM. 

Thanks!

> > v3: new patch (inspired by previous change)

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 2/6] iio: adc: meson_saradc: Align messages to be with physical device prefix

[Andy Shevchenko]

On Fri, Jun 03, 2022 at 05:21:17PM +0100, Jonathan Cameron wrote:
> On Fri,  3 Jun 2022 13:00:00 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > Align messages to be printed with the physical device prefix as it's done
> > everywhere else in this driver.

> Here I'm fine with the use of the parent as can only happen after all
> the setup is done, so it's obvious the parent will be assigned
> (some might argue it is obvious in the previous patch, but I had to check
> as I couldn't remember when we set it :)
> 
> Anyhow, LGTM. 

Thanks!

> > v3: new patch (inspired by previous change)

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Andy Shevchenko]

On Fri, Jun 03, 2022 at 05:29:20PM +0100, Jonathan Cameron wrote:
> On Fri, 3 Jun 2022 17:23:07 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:

...

> Andy, put a cover letter on these larger series - if nothing else it gives
> somewhere convenient for people to give tags for the whole series, or 
> maintainer to say what they are doing with it.

I'll try to remember this. The series was started from only a couple of patches
and grew to this big, and I forgot to add a cover letter when it seems not
anymore obvious what has been done.

> Anyhow, I'm fine with the series, but will leave it on list for a while
> longer, particularly to get patch 6 some eyes + testing.
> 
> Currently I plan to drop the fixes tag from this first patch, but I'm prepared
> to be convinced it's a bug fix rather than a consistency cleanup.

Fine with me, thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Andy Shevchenko]

On Fri, Jun 03, 2022 at 05:29:20PM +0100, Jonathan Cameron wrote:
> On Fri, 3 Jun 2022 17:23:07 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:

...

> Andy, put a cover letter on these larger series - if nothing else it gives
> somewhere convenient for people to give tags for the whole series, or 
> maintainer to say what they are doing with it.

I'll try to remember this. The series was started from only a couple of patches
and grew to this big, and I forgot to add a cover letter when it seems not
anymore obvious what has been done.

> Anyhow, I'm fine with the series, but will leave it on list for a while
> longer, particularly to get patch 6 some eyes + testing.
> 
> Currently I plan to drop the fixes tag from this first patch, but I'm prepared
> to be convinced it's a bug fix rather than a consistency cleanup.

Fine with me, thanks!

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Andy Shevchenko]

On Fri, Jun 03, 2022 at 05:29:20PM +0100, Jonathan Cameron wrote:
> On Fri, 3 Jun 2022 17:23:07 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:

...

> Andy, put a cover letter on these larger series - if nothing else it gives
> somewhere convenient for people to give tags for the whole series, or 
> maintainer to say what they are doing with it.

I'll try to remember this. The series was started from only a couple of patches
and grew to this big, and I forgot to add a cover letter when it seems not
anymore obvious what has been done.

> Anyhow, I'm fine with the series, but will leave it on list for a while
> longer, particularly to get patch 6 some eyes + testing.
> 
> Currently I plan to drop the fixes tag from this first patch, but I'm prepared
> to be convinced it's a bug fix rather than a consistency cleanup.

Fine with me, thanks!

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> It feels wrong and actually inconsistent to attach managed resources
> to the IIO device object, which is child of the physical device object.
> The rest of the ->probe() calls do that against physical device.
>
> Resolve this by reassigning managed resources to the physical device object.
>
> Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

I am also fine if the Fixes tag is being dropped - please keep my
Reviewed-by in that case.

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> It feels wrong and actually inconsistent to attach managed resources
> to the IIO device object, which is child of the physical device object.
> The rest of the ->probe() calls do that against physical device.
>
> Resolve this by reassigning managed resources to the physical device object.
>
> Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

I am also fine if the Fixes tag is being dropped - please keep my
Reviewed-by in that case.

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> It feels wrong and actually inconsistent to attach managed resources
> to the IIO device object, which is child of the physical device object.
> The rest of the ->probe() calls do that against physical device.
>
> Resolve this by reassigning managed resources to the physical device object.
>
> Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

I am also fine if the Fixes tag is being dropped - please keep my
Reviewed-by in that case.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 2/6] iio: adc: meson_saradc: Align messages to be with physical device prefix

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Align messages to be printed with the physical device prefix as it's done
> everywhere else in this driver.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Re: [PATCH v3 2/6] iio: adc: meson_saradc: Align messages to be with physical device prefix

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Align messages to be printed with the physical device prefix as it's done
> everywhere else in this driver.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 2/6] iio: adc: meson_saradc: Align messages to be with physical device prefix

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Align messages to be printed with the physical device prefix as it's done
> everywhere else in this driver.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 5/6] iio: adc: meson_saradc: Use temporary variable for struct device

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Use temporary variable for struct device to make code neater.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Re: [PATCH v3 5/6] iio: adc: meson_saradc: Use temporary variable for struct device

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Use temporary variable for struct device to make code neater.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 5/6] iio: adc: meson_saradc: Use temporary variable for struct device

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Use temporary variable for struct device to make code neater.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH v3 6/6] iio: adc: meson_saradc: Use regmap_read_poll_timeout() for busy wait

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Simplify busy wait stages by using regmap_read_poll_timeout().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

> v3: new patch, but RFC, not always the read_poll_timeout() can be used, would
>     be nice to have it tested.
and also:
Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> # GXM VIM2

Re: [RFC PATCH v3 6/6] iio: adc: meson_saradc: Use regmap_read_poll_timeout() for busy wait

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Simplify busy wait stages by using regmap_read_poll_timeout().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

> v3: new patch, but RFC, not always the read_poll_timeout() can be used, would
>     be nice to have it tested.
and also:
Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> # GXM VIM2

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [RFC PATCH v3 6/6] iio: adc: meson_saradc: Use regmap_read_poll_timeout() for busy wait

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Simplify busy wait stages by using regmap_read_poll_timeout().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

> v3: new patch, but RFC, not always the read_poll_timeout() can be used, would
>     be nice to have it tested.
and also:
Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> # GXM VIM2

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 3/6] iio: adc: meson_saradc: Convert to use dev_err_probe()

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> It's fine to call dev_err_probe() in ->probe() when error code is known.
> Convert the driver to use dev_err_probe().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Re: [PATCH v3 3/6] iio: adc: meson_saradc: Convert to use dev_err_probe()

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> It's fine to call dev_err_probe() in ->probe() when error code is known.
> Convert the driver to use dev_err_probe().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 3/6] iio: adc: meson_saradc: Convert to use dev_err_probe()

[Martin Blumenstingl]

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> It's fine to call dev_err_probe() in ->probe() when error code is known.
> Convert the driver to use dev_err_probe().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [RFC PATCH v3 6/6] iio: adc: meson_saradc: Use regmap_read_poll_timeout() for busy wait

[Andy Shevchenko]

On Sun, Jun 05, 2022 at 11:59:53PM +0200, Martin Blumenstingl wrote:
> On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > Simplify busy wait stages by using regmap_read_poll_timeout().
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> 
> > v3: new patch, but RFC, not always the read_poll_timeout() can be used, would
> >     be nice to have it tested.
> and also:
> Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> # GXM VIM2

Thanks for testing!

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH v3 6/6] iio: adc: meson_saradc: Use regmap_read_poll_timeout() for busy wait

[Andy Shevchenko]

On Sun, Jun 05, 2022 at 11:59:53PM +0200, Martin Blumenstingl wrote:
> On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > Simplify busy wait stages by using regmap_read_poll_timeout().
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> 
> > v3: new patch, but RFC, not always the read_poll_timeout() can be used, would
> >     be nice to have it tested.
> and also:
> Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> # GXM VIM2

Thanks for testing!

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH v3 6/6] iio: adc: meson_saradc: Use regmap_read_poll_timeout() for busy wait

[Andy Shevchenko]

On Sun, Jun 05, 2022 at 11:59:53PM +0200, Martin Blumenstingl wrote:
> On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > Simplify busy wait stages by using regmap_read_poll_timeout().
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> 
> > v3: new patch, but RFC, not always the read_poll_timeout() can be used, would
> >     be nice to have it tested.
> and also:
> Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> # GXM VIM2

Thanks for testing!

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Jonathan Cameron]

On Fri, 3 Jun 2022 19:54:43 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Fri, Jun 03, 2022 at 05:29:20PM +0100, Jonathan Cameron wrote:
> > On Fri, 3 Jun 2022 17:23:07 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:  
> 
> ...
> 
> > Andy, put a cover letter on these larger series - if nothing else it gives
> > somewhere convenient for people to give tags for the whole series, or 
> > maintainer to say what they are doing with it.  
> 
> I'll try to remember this. The series was started from only a couple of patches
> and grew to this big, and I forgot to add a cover letter when it seems not
> anymore obvious what has been done.
> 
> > Anyhow, I'm fine with the series, but will leave it on list for a while
> > longer, particularly to get patch 6 some eyes + testing.
> > 
> > Currently I plan to drop the fixes tag from this first patch, but I'm prepared
> > to be convinced it's a bug fix rather than a consistency cleanup.  
> 
> Fine with me, thanks!
> 
Applied to the togreg branch of iio.git and pushed out as testing for all the normal
reasons.

Thanks,

Jonathan

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Jonathan Cameron]

On Fri, 3 Jun 2022 19:54:43 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Fri, Jun 03, 2022 at 05:29:20PM +0100, Jonathan Cameron wrote:
> > On Fri, 3 Jun 2022 17:23:07 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:  
> 
> ...
> 
> > Andy, put a cover letter on these larger series - if nothing else it gives
> > somewhere convenient for people to give tags for the whole series, or 
> > maintainer to say what they are doing with it.  
> 
> I'll try to remember this. The series was started from only a couple of patches
> and grew to this big, and I forgot to add a cover letter when it seems not
> anymore obvious what has been done.
> 
> > Anyhow, I'm fine with the series, but will leave it on list for a while
> > longer, particularly to get patch 6 some eyes + testing.
> > 
> > Currently I plan to drop the fixes tag from this first patch, but I'm prepared
> > to be convinced it's a bug fix rather than a consistency cleanup.  
> 
> Fine with me, thanks!
> 
Applied to the togreg branch of iio.git and pushed out as testing for all the normal
reasons.

Thanks,

Jonathan


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

[Jonathan Cameron]

On Fri, 3 Jun 2022 19:54:43 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Fri, Jun 03, 2022 at 05:29:20PM +0100, Jonathan Cameron wrote:
> > On Fri, 3 Jun 2022 17:23:07 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:  
> 
> ...
> 
> > Andy, put a cover letter on these larger series - if nothing else it gives
> > somewhere convenient for people to give tags for the whole series, or 
> > maintainer to say what they are doing with it.  
> 
> I'll try to remember this. The series was started from only a couple of patches
> and grew to this big, and I forgot to add a cover letter when it seems not
> anymore obvious what has been done.
> 
> > Anyhow, I'm fine with the series, but will leave it on list for a while
> > longer, particularly to get patch 6 some eyes + testing.
> > 
> > Currently I plan to drop the fixes tag from this first patch, but I'm prepared
> > to be convinced it's a bug fix rather than a consistency cleanup.  
> 
> Fine with me, thanks!
> 
Applied to the togreg branch of iio.git and pushed out as testing for all the normal
reasons.

Thanks,

Jonathan


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel