All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] iio: adc: cc10001: Devm conversion
@ 2022-10-16 17:09 Jonathan Cameron
  2022-10-16 17:09 ` [PATCH 1/5] iio: adc: cc10001: Add local struct device *dev variable to avoid repitition Jonathan Cameron
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Jonathan Cameron @ 2022-10-16 17:09 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron, Phani Movva, Naidu Tellapati

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

A very simple example of how using devm_ managed calls for everything
can reduce complexity error handling and removal ordering in a driver.

Note I don't have one of these to test so if anyone has a chance to do
so or give these a quick look at that would be much appreciated.
Note this is a fairly old driver, so relative unlikely original authors
still have access.

Cc: Phani Movva <Phani.Movva@imgtec.com>
Cc: Naidu Tellapati <naidu.tellapati@imgtec.com>

Jonathan Cameron (5):
  iio: adc: cc10001: Add local struct device *dev variable to avoid
    repitition
  iio: adc: cc10001: Add devm_add_action_or_reset() to disable
    regulator.
  iio: adc: cc10001: Use devm_clk_get_enabled() to avoid boilerplate.
  iio: adc: cc10001: Use devm_ to call device power down.
  iio: adc: cc10001: Switch remaining IIO calls in probe to devm_ forms.

 drivers/iio/adc/cc10001_adc.c | 89 +++++++++++++----------------------
 1 file changed, 34 insertions(+), 55 deletions(-)

-- 
2.37.2


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

* [PATCH 1/5] iio: adc: cc10001: Add local struct device *dev variable to avoid repitition
  2022-10-16 17:09 [PATCH 0/5] iio: adc: cc10001: Devm conversion Jonathan Cameron
@ 2022-10-16 17:09 ` Jonathan Cameron
  2022-10-16 17:09 ` [PATCH 2/5] iio: adc: cc10001: Add devm_add_action_or_reset() to disable regulator Jonathan Cameron
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2022-10-16 17:09 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

There are lots of uses of this in probe() and we are about to introduce
some more, so add a local variable to simplify this.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/cc10001_adc.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/adc/cc10001_adc.c b/drivers/iio/adc/cc10001_adc.c
index e16ac935693b..eeaea1362ed1 100644
--- a/drivers/iio/adc/cc10001_adc.c
+++ b/drivers/iio/adc/cc10001_adc.c
@@ -307,14 +307,15 @@ static int cc10001_adc_channel_init(struct iio_dev *indio_dev,
 
 static int cc10001_adc_probe(struct platform_device *pdev)
 {
-	struct device_node *node = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
 	struct cc10001_adc_device *adc_dev;
 	unsigned long adc_clk_rate;
 	struct iio_dev *indio_dev;
 	unsigned long channel_map;
 	int ret;
 
-	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*adc_dev));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*adc_dev));
 	if (indio_dev == NULL)
 		return -ENOMEM;
 
@@ -326,7 +327,7 @@ static int cc10001_adc_probe(struct platform_device *pdev)
 		channel_map &= ~ret;
 	}
 
-	adc_dev->reg = devm_regulator_get(&pdev->dev, "vref");
+	adc_dev->reg = devm_regulator_get(dev, "vref");
 	if (IS_ERR(adc_dev->reg))
 		return PTR_ERR(adc_dev->reg);
 
@@ -334,7 +335,7 @@ static int cc10001_adc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	indio_dev->name = dev_name(&pdev->dev);
+	indio_dev->name = dev_name(dev);
 	indio_dev->info = &cc10001_adc_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
@@ -344,23 +345,23 @@ static int cc10001_adc_probe(struct platform_device *pdev)
 		goto err_disable_reg;
 	}
 
-	adc_dev->adc_clk = devm_clk_get(&pdev->dev, "adc");
+	adc_dev->adc_clk = devm_clk_get(dev, "adc");
 	if (IS_ERR(adc_dev->adc_clk)) {
-		dev_err(&pdev->dev, "failed to get the clock\n");
+		dev_err(dev, "failed to get the clock\n");
 		ret = PTR_ERR(adc_dev->adc_clk);
 		goto err_disable_reg;
 	}
 
 	ret = clk_prepare_enable(adc_dev->adc_clk);
 	if (ret) {
-		dev_err(&pdev->dev, "failed to enable the clock\n");
+		dev_err(dev, "failed to enable the clock\n");
 		goto err_disable_reg;
 	}
 
 	adc_clk_rate = clk_get_rate(adc_dev->adc_clk);
 	if (!adc_clk_rate) {
 		ret = -EINVAL;
-		dev_err(&pdev->dev, "null clock rate!\n");
+		dev_err(dev, "null clock rate!\n");
 		goto err_disable_clk;
 	}
 
-- 
2.37.2


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

* [PATCH 2/5] iio: adc: cc10001: Add devm_add_action_or_reset() to disable regulator.
  2022-10-16 17:09 [PATCH 0/5] iio: adc: cc10001: Devm conversion Jonathan Cameron
  2022-10-16 17:09 ` [PATCH 1/5] iio: adc: cc10001: Add local struct device *dev variable to avoid repitition Jonathan Cameron
@ 2022-10-16 17:09 ` Jonathan Cameron
  2022-10-16 17:09 ` [PATCH 3/5] iio: adc: cc10001: Use devm_clk_get_enabled() to avoid boilerplate Jonathan Cameron
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2022-10-16 17:09 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

As the voltage of this regulator is queried, we cannot use the
devm_regulator_get_enable() call and have to role our own disable.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/cc10001_adc.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/adc/cc10001_adc.c b/drivers/iio/adc/cc10001_adc.c
index eeaea1362ed1..4f42ceb40ded 100644
--- a/drivers/iio/adc/cc10001_adc.c
+++ b/drivers/iio/adc/cc10001_adc.c
@@ -305,6 +305,11 @@ static int cc10001_adc_channel_init(struct iio_dev *indio_dev,
 	return 0;
 }
 
+static void cc10001_reg_disable(void *priv)
+{
+	regulator_disable(priv);
+}
+
 static int cc10001_adc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -335,27 +340,28 @@ static int cc10001_adc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ret = devm_add_action_or_reset(dev, cc10001_reg_disable, adc_dev->reg);
+	if (ret)
+		return ret;
+
 	indio_dev->name = dev_name(dev);
 	indio_dev->info = &cc10001_adc_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
 	adc_dev->reg_base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(adc_dev->reg_base)) {
-		ret = PTR_ERR(adc_dev->reg_base);
-		goto err_disable_reg;
-	}
+	if (IS_ERR(adc_dev->reg_base))
+		return PTR_ERR(adc_dev->reg_base);
 
 	adc_dev->adc_clk = devm_clk_get(dev, "adc");
 	if (IS_ERR(adc_dev->adc_clk)) {
 		dev_err(dev, "failed to get the clock\n");
-		ret = PTR_ERR(adc_dev->adc_clk);
-		goto err_disable_reg;
+		return PTR_ERR(adc_dev->adc_clk);
 	}
 
 	ret = clk_prepare_enable(adc_dev->adc_clk);
 	if (ret) {
 		dev_err(dev, "failed to enable the clock\n");
-		goto err_disable_reg;
+		return ret;
 	}
 
 	adc_clk_rate = clk_get_rate(adc_dev->adc_clk);
@@ -400,8 +406,6 @@ static int cc10001_adc_probe(struct platform_device *pdev)
 	iio_triggered_buffer_cleanup(indio_dev);
 err_disable_clk:
 	clk_disable_unprepare(adc_dev->adc_clk);
-err_disable_reg:
-	regulator_disable(adc_dev->reg);
 	return ret;
 }
 
@@ -414,7 +418,6 @@ static int cc10001_adc_remove(struct platform_device *pdev)
 	iio_device_unregister(indio_dev);
 	iio_triggered_buffer_cleanup(indio_dev);
 	clk_disable_unprepare(adc_dev->adc_clk);
-	regulator_disable(adc_dev->reg);
 
 	return 0;
 }
-- 
2.37.2


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

* [PATCH 3/5] iio: adc: cc10001: Use devm_clk_get_enabled() to avoid boilerplate.
  2022-10-16 17:09 [PATCH 0/5] iio: adc: cc10001: Devm conversion Jonathan Cameron
  2022-10-16 17:09 ` [PATCH 1/5] iio: adc: cc10001: Add local struct device *dev variable to avoid repitition Jonathan Cameron
  2022-10-16 17:09 ` [PATCH 2/5] iio: adc: cc10001: Add devm_add_action_or_reset() to disable regulator Jonathan Cameron
@ 2022-10-16 17:09 ` Jonathan Cameron
  2022-10-31 11:19   ` Sa, Nuno
  2022-10-16 17:09 ` [PATCH 4/5] iio: adc: cc10001: Use devm_ to call device power down Jonathan Cameron
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2022-10-16 17:09 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

As this driver just enables clock in probe() and disables in remove()
we can use this new function to replace boilerplate and simplify
error paths.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/cc10001_adc.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/adc/cc10001_adc.c b/drivers/iio/adc/cc10001_adc.c
index 4f42ceb40ded..332f0e06369f 100644
--- a/drivers/iio/adc/cc10001_adc.c
+++ b/drivers/iio/adc/cc10001_adc.c
@@ -352,23 +352,16 @@ static int cc10001_adc_probe(struct platform_device *pdev)
 	if (IS_ERR(adc_dev->reg_base))
 		return PTR_ERR(adc_dev->reg_base);
 
-	adc_dev->adc_clk = devm_clk_get(dev, "adc");
+	adc_dev->adc_clk = devm_clk_get_enabled(dev, "adc");
 	if (IS_ERR(adc_dev->adc_clk)) {
 		dev_err(dev, "failed to get the clock\n");
 		return PTR_ERR(adc_dev->adc_clk);
 	}
 
-	ret = clk_prepare_enable(adc_dev->adc_clk);
-	if (ret) {
-		dev_err(dev, "failed to enable the clock\n");
-		return ret;
-	}
-
 	adc_clk_rate = clk_get_rate(adc_dev->adc_clk);
 	if (!adc_clk_rate) {
-		ret = -EINVAL;
 		dev_err(dev, "null clock rate!\n");
-		goto err_disable_clk;
+		return -EINVAL;
 	}
 
 	adc_dev->eoc_delay_ns = NSEC_PER_SEC / adc_clk_rate;
@@ -385,14 +378,14 @@ static int cc10001_adc_probe(struct platform_device *pdev)
 	/* Setup the ADC channels available on the device */
 	ret = cc10001_adc_channel_init(indio_dev, channel_map);
 	if (ret < 0)
-		goto err_disable_clk;
+		return ret;
 
 	mutex_init(&adc_dev->lock);
 
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
 					 &cc10001_adc_trigger_h, NULL);
 	if (ret < 0)
-		goto err_disable_clk;
+		return ret;
 
 	ret = iio_device_register(indio_dev);
 	if (ret < 0)
@@ -404,8 +397,6 @@ static int cc10001_adc_probe(struct platform_device *pdev)
 
 err_cleanup_buffer:
 	iio_triggered_buffer_cleanup(indio_dev);
-err_disable_clk:
-	clk_disable_unprepare(adc_dev->adc_clk);
 	return ret;
 }
 
@@ -417,7 +408,6 @@ static int cc10001_adc_remove(struct platform_device *pdev)
 	cc10001_adc_power_down(adc_dev);
 	iio_device_unregister(indio_dev);
 	iio_triggered_buffer_cleanup(indio_dev);
-	clk_disable_unprepare(adc_dev->adc_clk);
 
 	return 0;
 }
-- 
2.37.2


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

* [PATCH 4/5] iio: adc: cc10001: Use devm_ to call device power down.
  2022-10-16 17:09 [PATCH 0/5] iio: adc: cc10001: Devm conversion Jonathan Cameron
                   ` (2 preceding siblings ...)
  2022-10-16 17:09 ` [PATCH 3/5] iio: adc: cc10001: Use devm_clk_get_enabled() to avoid boilerplate Jonathan Cameron
@ 2022-10-16 17:09 ` Jonathan Cameron
  2022-10-16 17:09 ` [PATCH 5/5] iio: adc: cc10001: Switch remaining IIO calls in probe to devm_ forms Jonathan Cameron
  2022-10-29 12:43 ` [PATCH 0/5] iio: adc: cc10001: Devm conversion Jonathan Cameron
  5 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2022-10-16 17:09 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

It is presumably safe to call the powerdown whether or not we are
in the commented shared state (the driver always did this).

The power down was previously out of order wrt to the probe() function
so move using devm_ will ensure it occurs after the userspace interfaces
are removed.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/cc10001_adc.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/cc10001_adc.c b/drivers/iio/adc/cc10001_adc.c
index 332f0e06369f..b0daaec7ff16 100644
--- a/drivers/iio/adc/cc10001_adc.c
+++ b/drivers/iio/adc/cc10001_adc.c
@@ -310,6 +310,11 @@ static void cc10001_reg_disable(void *priv)
 	regulator_disable(priv);
 }
 
+static void cc10001_pd_cb(void *priv)
+{
+	cc10001_adc_power_down(priv);
+}
+
 static int cc10001_adc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -375,6 +380,9 @@ static int cc10001_adc_probe(struct platform_device *pdev)
 	if (adc_dev->shared)
 		cc10001_adc_power_up(adc_dev);
 
+	ret = devm_add_action_or_reset(dev, cc10001_pd_cb, adc_dev);
+	if (ret)
+		return ret;
 	/* Setup the ADC channels available on the device */
 	ret = cc10001_adc_channel_init(indio_dev, channel_map);
 	if (ret < 0)
@@ -405,7 +413,6 @@ static int cc10001_adc_remove(struct platform_device *pdev)
 	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
 	struct cc10001_adc_device *adc_dev = iio_priv(indio_dev);
 
-	cc10001_adc_power_down(adc_dev);
 	iio_device_unregister(indio_dev);
 	iio_triggered_buffer_cleanup(indio_dev);
 
-- 
2.37.2


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

* [PATCH 5/5] iio: adc: cc10001: Switch remaining IIO calls in probe to devm_ forms.
  2022-10-16 17:09 [PATCH 0/5] iio: adc: cc10001: Devm conversion Jonathan Cameron
                   ` (3 preceding siblings ...)
  2022-10-16 17:09 ` [PATCH 4/5] iio: adc: cc10001: Use devm_ to call device power down Jonathan Cameron
@ 2022-10-16 17:09 ` Jonathan Cameron
  2022-10-31 11:17   ` Sa, Nuno
  2022-10-29 12:43 ` [PATCH 0/5] iio: adc: cc10001: Devm conversion Jonathan Cameron
  5 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2022-10-16 17:09 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

As everything else is now handled by devm managed releases the
triggered buffer setup and IIO device registration can also be
moved over to their devm forms allowing dropping of remove().

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/cc10001_adc.c | 28 +++-------------------------
 1 file changed, 3 insertions(+), 25 deletions(-)

diff --git a/drivers/iio/adc/cc10001_adc.c b/drivers/iio/adc/cc10001_adc.c
index b0daaec7ff16..ab71004ea8f1 100644
--- a/drivers/iio/adc/cc10001_adc.c
+++ b/drivers/iio/adc/cc10001_adc.c
@@ -390,33 +390,12 @@ static int cc10001_adc_probe(struct platform_device *pdev)
 
 	mutex_init(&adc_dev->lock);
 
-	ret = iio_triggered_buffer_setup(indio_dev, NULL,
-					 &cc10001_adc_trigger_h, NULL);
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
+					      &cc10001_adc_trigger_h, NULL);
 	if (ret < 0)
 		return ret;
 
-	ret = iio_device_register(indio_dev);
-	if (ret < 0)
-		goto err_cleanup_buffer;
-
-	platform_set_drvdata(pdev, indio_dev);
-
-	return 0;
-
-err_cleanup_buffer:
-	iio_triggered_buffer_cleanup(indio_dev);
-	return ret;
-}
-
-static int cc10001_adc_remove(struct platform_device *pdev)
-{
-	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
-	struct cc10001_adc_device *adc_dev = iio_priv(indio_dev);
-
-	iio_device_unregister(indio_dev);
-	iio_triggered_buffer_cleanup(indio_dev);
-
-	return 0;
+	return devm_iio_device_register(dev, indio_dev);
 }
 
 static const struct of_device_id cc10001_adc_dt_ids[] = {
@@ -431,7 +410,6 @@ static struct platform_driver cc10001_adc_driver = {
 		.of_match_table = cc10001_adc_dt_ids,
 	},
 	.probe	= cc10001_adc_probe,
-	.remove	= cc10001_adc_remove,
 };
 module_platform_driver(cc10001_adc_driver);
 
-- 
2.37.2


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

* Re: [PATCH 0/5] iio: adc: cc10001: Devm conversion
  2022-10-16 17:09 [PATCH 0/5] iio: adc: cc10001: Devm conversion Jonathan Cameron
                   ` (4 preceding siblings ...)
  2022-10-16 17:09 ` [PATCH 5/5] iio: adc: cc10001: Switch remaining IIO calls in probe to devm_ forms Jonathan Cameron
@ 2022-10-29 12:43 ` Jonathan Cameron
  2022-10-31 11:19   ` Sa, Nuno
  5 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2022-10-29 12:43 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron, Phani Movva, Naidu Tellapati

On Sun, 16 Oct 2022 18:09:45 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> A very simple example of how using devm_ managed calls for everything
> can reduce complexity error handling and removal ordering in a driver.
> 
> Note I don't have one of these to test so if anyone has a chance to do
> so or give these a quick look at that would be much appreciated.
> Note this is a fairly old driver, so relative unlikely original authors
> still have access.
> 
> Cc: Phani Movva <Phani.Movva@imgtec.com>
> Cc: Naidu Tellapati <naidu.tellapati@imgtec.com>

If anyone has time for a quick glance at this it would be much appreciated!

Old maintainer issue of who is the fall back reviewer for the maintainers
own patches to old drivers, where the authors etc have probably long since
moved on.

Jonathan
> 
> Jonathan Cameron (5):
>   iio: adc: cc10001: Add local struct device *dev variable to avoid
>     repitition
>   iio: adc: cc10001: Add devm_add_action_or_reset() to disable
>     regulator.
>   iio: adc: cc10001: Use devm_clk_get_enabled() to avoid boilerplate.
>   iio: adc: cc10001: Use devm_ to call device power down.
>   iio: adc: cc10001: Switch remaining IIO calls in probe to devm_ forms.
> 
>  drivers/iio/adc/cc10001_adc.c | 89 +++++++++++++----------------------
>  1 file changed, 34 insertions(+), 55 deletions(-)
> 


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

* RE: [PATCH 5/5] iio: adc: cc10001: Switch remaining IIO calls in probe to devm_ forms.
  2022-10-16 17:09 ` [PATCH 5/5] iio: adc: cc10001: Switch remaining IIO calls in probe to devm_ forms Jonathan Cameron
@ 2022-10-31 11:17   ` Sa, Nuno
  0 siblings, 0 replies; 11+ messages in thread
From: Sa, Nuno @ 2022-10-31 11:17 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio; +Cc: Jonathan Cameron



> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Sunday, October 16, 2022 7:10 PM
> To: linux-iio@vger.kernel.org
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Subject: [PATCH 5/5] iio: adc: cc10001: Switch remaining IIO calls in probe to
> devm_ forms.
> 
> [External]
> 
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> As everything else is now handled by devm managed releases the
> triggered buffer setup and IIO device registration can also be
> moved over to their devm forms allowing dropping of remove().
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/adc/cc10001_adc.c | 28 +++-------------------------
>  1 file changed, 3 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/iio/adc/cc10001_adc.c b/drivers/iio/adc/cc10001_adc.c
> index b0daaec7ff16..ab71004ea8f1 100644
> --- a/drivers/iio/adc/cc10001_adc.c
> +++ b/drivers/iio/adc/cc10001_adc.c
> @@ -390,33 +390,12 @@ static int cc10001_adc_probe(struct
> platform_device *pdev)
> 
>  	mutex_init(&adc_dev->lock);
> 
> -	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> -					 &cc10001_adc_trigger_h, NULL);
> +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
> +					      &cc10001_adc_trigger_h, NULL);
>  	if (ret < 0)
>  		return ret;
> 
> -	ret = iio_device_register(indio_dev);
> -	if (ret < 0)
> -		goto err_cleanup_buffer;
> -
> -	platform_set_drvdata(pdev, indio_dev);
> -

I wonder if it's obvious to everyone that removing this call as a consequence
of not needing  .remove() is clear enough that it does not deserve a mention
in the commit message?

Note that I don't find it particular important so I will still ack it and leave it
up to you :)


- Nuno Sá

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

* RE: [PATCH 3/5] iio: adc: cc10001: Use devm_clk_get_enabled() to avoid boilerplate.
  2022-10-16 17:09 ` [PATCH 3/5] iio: adc: cc10001: Use devm_clk_get_enabled() to avoid boilerplate Jonathan Cameron
@ 2022-10-31 11:19   ` Sa, Nuno
  0 siblings, 0 replies; 11+ messages in thread
From: Sa, Nuno @ 2022-10-31 11:19 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio; +Cc: Jonathan Cameron



> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Sunday, October 16, 2022 7:10 PM
> To: linux-iio@vger.kernel.org
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Subject: [PATCH 3/5] iio: adc: cc10001: Use devm_clk_get_enabled() to avoid
> boilerplate.
> 
> [External]
> 
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> As this driver just enables clock in probe() and disables in remove()
> we can use this new function to replace boilerplate and simplify
> error paths.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/adc/cc10001_adc.c | 18 ++++--------------
>  1 file changed, 4 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/iio/adc/cc10001_adc.c b/drivers/iio/adc/cc10001_adc.c
> index 4f42ceb40ded..332f0e06369f 100644
> --- a/drivers/iio/adc/cc10001_adc.c
> +++ b/drivers/iio/adc/cc10001_adc.c
> @@ -352,23 +352,16 @@ static int cc10001_adc_probe(struct
> platform_device *pdev)
>  	if (IS_ERR(adc_dev->reg_base))
>  		return PTR_ERR(adc_dev->reg_base);
> 
> -	adc_dev->adc_clk = devm_clk_get(dev, "adc");
> +	adc_dev->adc_clk = devm_clk_get_enabled(dev, "adc");
>  	if (IS_ERR(adc_dev->adc_clk)) {
>  		dev_err(dev, "failed to get the clock\n");
>  		return PTR_ERR(adc_dev->adc_clk);
>  	}
> 

I guess we could also tweak the err() message as we loose
"failed to enable the clock"...

- Nuno Sá


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

* RE: [PATCH 0/5] iio: adc: cc10001: Devm conversion
  2022-10-29 12:43 ` [PATCH 0/5] iio: adc: cc10001: Devm conversion Jonathan Cameron
@ 2022-10-31 11:19   ` Sa, Nuno
  2022-11-06 11:48     ` Jonathan Cameron
  0 siblings, 1 reply; 11+ messages in thread
From: Sa, Nuno @ 2022-10-31 11:19 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: Jonathan Cameron, Phani Movva, Naidu Tellapati



> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, October 29, 2022 2:44 PM
> To: linux-iio@vger.kernel.org
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>; Phani Movva
> <Phani.Movva@imgtec.com>; Naidu Tellapati <naidu.tellapati@imgtec.com>
> Subject: Re: [PATCH 0/5] iio: adc: cc10001: Devm conversion
> 
> [External]
> 
> On Sun, 16 Oct 2022 18:09:45 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > A very simple example of how using devm_ managed calls for everything
> > can reduce complexity error handling and removal ordering in a driver.
> >
> > Note I don't have one of these to test so if anyone has a chance to do
> > so or give these a quick look at that would be much appreciated.
> > Note this is a fairly old driver, so relative unlikely original authors
> > still have access.
> >
> > Cc: Phani Movva <Phani.Movva@imgtec.com>
> > Cc: Naidu Tellapati <naidu.tellapati@imgtec.com>
> 
> If anyone has time for a quick glance at this it would be much appreciated!
> 
> Old maintainer issue of who is the fall back reviewer for the maintainers
> own patches to old drivers, where the authors etc have probably long since
> moved on.
> 
> Jonathan
> >
> > Jonathan Cameron (5):
> >   iio: adc: cc10001: Add local struct device *dev variable to avoid
> >     repitition
> >   iio: adc: cc10001: Add devm_add_action_or_reset() to disable
> >     regulator.
> >   iio: adc: cc10001: Use devm_clk_get_enabled() to avoid boilerplate.
> >   iio: adc: cc10001: Use devm_ to call device power down.
> >   iio: adc: cc10001: Switch remaining IIO calls in probe to devm_ forms.
> >
> >  drivers/iio/adc/cc10001_adc.c | 89 +++++++++++++----------------------
> >  1 file changed, 34 insertions(+), 55 deletions(-)
> >

Only minor comments, so feel free to add:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

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

* Re: [PATCH 0/5] iio: adc: cc10001: Devm conversion
  2022-10-31 11:19   ` Sa, Nuno
@ 2022-11-06 11:48     ` Jonathan Cameron
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2022-11-06 11:48 UTC (permalink / raw)
  To: Sa, Nuno; +Cc: linux-iio, Jonathan Cameron, Phani Movva, Naidu Tellapati

On Mon, 31 Oct 2022 11:19:19 +0000
"Sa, Nuno" <Nuno.Sa@analog.com> wrote:

> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Saturday, October 29, 2022 2:44 PM
> > To: linux-iio@vger.kernel.org
> > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>; Phani Movva
> > <Phani.Movva@imgtec.com>; Naidu Tellapati <naidu.tellapati@imgtec.com>
> > Subject: Re: [PATCH 0/5] iio: adc: cc10001: Devm conversion
> > 
> > [External]
> > 
> > On Sun, 16 Oct 2022 18:09:45 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >   
> > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > >
> > > A very simple example of how using devm_ managed calls for everything
> > > can reduce complexity error handling and removal ordering in a driver.
> > >
> > > Note I don't have one of these to test so if anyone has a chance to do
> > > so or give these a quick look at that would be much appreciated.
> > > Note this is a fairly old driver, so relative unlikely original authors
> > > still have access.
> > >
> > > Cc: Phani Movva <Phani.Movva@imgtec.com>
> > > Cc: Naidu Tellapati <naidu.tellapati@imgtec.com>  
> > 
> > If anyone has time for a quick glance at this it would be much appreciated!
> > 
> > Old maintainer issue of who is the fall back reviewer for the maintainers
> > own patches to old drivers, where the authors etc have probably long since
> > moved on.
> > 
> > Jonathan  
> > >
> > > Jonathan Cameron (5):
> > >   iio: adc: cc10001: Add local struct device *dev variable to avoid
> > >     repitition
> > >   iio: adc: cc10001: Add devm_add_action_or_reset() to disable
> > >     regulator.
> > >   iio: adc: cc10001: Use devm_clk_get_enabled() to avoid boilerplate.
> > >   iio: adc: cc10001: Use devm_ to call device power down.
> > >   iio: adc: cc10001: Switch remaining IIO calls in probe to devm_ forms.
> > >
> > >  drivers/iio/adc/cc10001_adc.c | 89 +++++++++++++----------------------
> > >  1 file changed, 34 insertions(+), 55 deletions(-)
> > >  
> 
> Only minor comments, so feel free to add:
> 
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>

Tweaked as suggested whilst applying.  Thanks!

Applied to the togreg branch of iio.git and pushed out as testing for all the
normal reasons.

Thanks,

Jonathan

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

end of thread, other threads:[~2022-11-06 11:49 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-16 17:09 [PATCH 0/5] iio: adc: cc10001: Devm conversion Jonathan Cameron
2022-10-16 17:09 ` [PATCH 1/5] iio: adc: cc10001: Add local struct device *dev variable to avoid repitition Jonathan Cameron
2022-10-16 17:09 ` [PATCH 2/5] iio: adc: cc10001: Add devm_add_action_or_reset() to disable regulator Jonathan Cameron
2022-10-16 17:09 ` [PATCH 3/5] iio: adc: cc10001: Use devm_clk_get_enabled() to avoid boilerplate Jonathan Cameron
2022-10-31 11:19   ` Sa, Nuno
2022-10-16 17:09 ` [PATCH 4/5] iio: adc: cc10001: Use devm_ to call device power down Jonathan Cameron
2022-10-16 17:09 ` [PATCH 5/5] iio: adc: cc10001: Switch remaining IIO calls in probe to devm_ forms Jonathan Cameron
2022-10-31 11:17   ` Sa, Nuno
2022-10-29 12:43 ` [PATCH 0/5] iio: adc: cc10001: Devm conversion Jonathan Cameron
2022-10-31 11:19   ` Sa, Nuno
2022-11-06 11:48     ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.