diff mbox series

[v2,04/15] iio: adc: rzg2l_adc: Simplify the runtime PM code

Message ID 20241206111337.726244-5-claudiu.beznea.uj@bp.renesas.com (mailing list archive)
State Accepted
Headers show
Series iio: adc: rzg2l_adc: Add support for RZ/G3S | expand

Commit Message

Claudiu Beznea Dec. 6, 2024, 11:13 a.m. UTC 
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

All Renesas SoCs using the rzg2l_adc driver manage ADC clocks through PM
domains. Calling pm_runtime_{resume_and_get, put_sync}() implicitly sets
the state of the clocks. As a result, the code in the rzg2l_adc driver that
explicitly manages ADC clocks can be removed, leading to simpler and
cleaner implementation.

Additionally, replace the use of rzg2l_adc_set_power() with direct PM
runtime API calls to further simplify and clean up the code.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- rebased on top of patch 02/15 from this version

 drivers/iio/adc/rzg2l_adc.c | 96 ++++++++-----------------------------
 1 file changed, 20 insertions(+), 76 deletions(-)

Comments

Lad, Prabhakar Dec. 8, 2024, 8:23 p.m. UTC | #1 
On Fri, Dec 6, 2024 at 11:14 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> All Renesas SoCs using the rzg2l_adc driver manage ADC clocks through PM
> domains. Calling pm_runtime_{resume_and_get, put_sync}() implicitly sets
> the state of the clocks. As a result, the code in the rzg2l_adc driver that
> explicitly manages ADC clocks can be removed, leading to simpler and
> cleaner implementation.
>
> Additionally, replace the use of rzg2l_adc_set_power() with direct PM
> runtime API calls to further simplify and clean up the code.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - rebased on top of patch 02/15 from this version
>
>  drivers/iio/adc/rzg2l_adc.c | 96 ++++++++-----------------------------
>  1 file changed, 20 insertions(+), 76 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> index c0c13e99aa92..780cb927eab1 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -8,7 +8,6 @@
>   */
>
>  #include <linux/bitfield.h>
> -#include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/delay.h>
>  #include <linux/iio/iio.h>
> @@ -69,8 +68,6 @@ struct rzg2l_adc_data {
>
>  struct rzg2l_adc {
>         void __iomem *base;
> -       struct clk *pclk;
> -       struct clk *adclk;
>         struct reset_control *presetn;
>         struct reset_control *adrstn;
>         struct completion completion;
> @@ -188,29 +185,18 @@ static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
>         return 0;
>  }
>
> -static int rzg2l_adc_set_power(struct iio_dev *indio_dev, bool on)
> -{
> -       struct device *dev = indio_dev->dev.parent;
> -
> -       if (on)
> -               return pm_runtime_resume_and_get(dev);
> -
> -       return pm_runtime_put_sync(dev);
> -}
> -
>  static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc, u8 ch)
>  {
> +       struct device *dev = indio_dev->dev.parent;
>         int ret;
>
> -       ret = rzg2l_adc_set_power(indio_dev, true);
> +       ret = pm_runtime_resume_and_get(dev);
>         if (ret)
>                 return ret;
>
>         ret = rzg2l_adc_conversion_setup(adc, ch);
> -       if (ret) {
> -               rzg2l_adc_set_power(indio_dev, false);
> -               return ret;
> -       }
> +       if (ret)
> +               goto rpm_put;
>
>         reinit_completion(&adc->completion);
>
> @@ -219,12 +205,14 @@ static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc
>         if (!wait_for_completion_timeout(&adc->completion, RZG2L_ADC_TIMEOUT)) {
>                 rzg2l_adc_writel(adc, RZG2L_ADINT,
>                                  rzg2l_adc_readl(adc, RZG2L_ADINT) & ~RZG2L_ADINT_INTEN_MASK);
> -               rzg2l_adc_start_stop(adc, false);
> -               rzg2l_adc_set_power(indio_dev, false);
> -               return -ETIMEDOUT;
> +               ret = -ETIMEDOUT;
>         }
>
> -       return rzg2l_adc_set_power(indio_dev, false);
> +       rzg2l_adc_start_stop(adc, false);
> +
> +rpm_put:
> +       pm_runtime_put_sync(dev);
> +       return ret;
>  }
>
>  static int rzg2l_adc_read_raw(struct iio_dev *indio_dev,
> @@ -348,13 +336,13 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
>         return 0;
>  }
>
> -static int rzg2l_adc_hw_init(struct rzg2l_adc *adc)
> +static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
>  {
>         int timeout = 5;
>         u32 reg;
>         int ret;
>
> -       ret = clk_prepare_enable(adc->pclk);
> +       ret = pm_runtime_resume_and_get(dev);
>         if (ret)
>                 return ret;
>
> @@ -392,25 +380,10 @@ static int rzg2l_adc_hw_init(struct rzg2l_adc *adc)
>         rzg2l_adc_writel(adc, RZG2L_ADM(3), reg);
>
>  exit_hw_init:
> -       clk_disable_unprepare(adc->pclk);
> -
> +       pm_runtime_put_sync(dev);
>         return ret;
>  }
>
> -static void rzg2l_adc_pm_runtime_disable(void *data)
> -{
> -       struct device *dev = data;
> -
> -       pm_runtime_disable(dev->parent);
> -}
> -
> -static void rzg2l_adc_pm_runtime_set_suspended(void *data)
> -{
> -       struct device *dev = data;
> -
> -       pm_runtime_set_suspended(dev->parent);
> -}
> -
>  static int rzg2l_adc_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
> @@ -435,14 +408,6 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
>         if (IS_ERR(adc->base))
>                 return PTR_ERR(adc->base);
>
> -       adc->pclk = devm_clk_get(dev, "pclk");
> -       if (IS_ERR(adc->pclk))
> -               return dev_err_probe(dev, PTR_ERR(adc->pclk), "Failed to get pclk");
> -
> -       adc->adclk = devm_clk_get(dev, "adclk");
> -       if (IS_ERR(adc->adclk))
> -               return dev_err_probe(dev, PTR_ERR(adc->adclk), "Failed to get adclk");
> -
>         adc->adrstn = devm_reset_control_get_exclusive_deasserted(dev, "adrst-n");
>         if (IS_ERR(adc->adrstn))
>                 return dev_err_probe(dev, PTR_ERR(adc->adrstn), "failed to get/deassert adrst-n\n");
> @@ -453,7 +418,13 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
>                                      "failed to get/deassert presetn\n");
>         }
>
> -       ret = rzg2l_adc_hw_init(adc);
> +       ret = devm_pm_runtime_enable(dev);
> +       if (ret)
> +               return ret;
> +
> +       platform_set_drvdata(pdev, indio_dev);
> +
> +       ret = rzg2l_adc_hw_init(dev, adc);
>         if (ret)
>                 return dev_err_probe(&pdev->dev, ret, "failed to initialize ADC HW, %d\n", ret);
>
> @@ -468,26 +439,12 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
>
>         init_completion(&adc->completion);
>
> -       platform_set_drvdata(pdev, indio_dev);
> -
>         indio_dev->name = DRIVER_NAME;
>         indio_dev->info = &rzg2l_adc_iio_info;
>         indio_dev->modes = INDIO_DIRECT_MODE;
>         indio_dev->channels = adc->data->channels;
>         indio_dev->num_channels = adc->data->num_channels;
>
> -       pm_runtime_set_suspended(dev);
> -       ret = devm_add_action_or_reset(&pdev->dev,
> -                                      rzg2l_adc_pm_runtime_set_suspended, &indio_dev->dev);
> -       if (ret)
> -               return ret;
> -
> -       pm_runtime_enable(dev);
> -       ret = devm_add_action_or_reset(&pdev->dev,
> -                                      rzg2l_adc_pm_runtime_disable, &indio_dev->dev);
> -       if (ret)
> -               return ret;
> -
>         return devm_iio_device_register(dev, indio_dev);
>  }
>
> @@ -503,8 +460,6 @@ static int __maybe_unused rzg2l_adc_pm_runtime_suspend(struct device *dev)
>         struct rzg2l_adc *adc = iio_priv(indio_dev);
>
>         rzg2l_adc_pwr(adc, false);
> -       clk_disable_unprepare(adc->adclk);
> -       clk_disable_unprepare(adc->pclk);
>
>         return 0;
>  }
> @@ -513,17 +468,6 @@ static int __maybe_unused rzg2l_adc_pm_runtime_resume(struct device *dev)
>  {
>         struct iio_dev *indio_dev = dev_get_drvdata(dev);
>         struct rzg2l_adc *adc = iio_priv(indio_dev);
> -       int ret;
> -
> -       ret = clk_prepare_enable(adc->pclk);
> -       if (ret)
> -               return ret;
> -
> -       ret = clk_prepare_enable(adc->adclk);
> -       if (ret) {
> -               clk_disable_unprepare(adc->pclk);
> -               return ret;
> -       }
>
>         rzg2l_adc_pwr(adc, true);
>
> --
> 2.39.2
>
>
diff mbox series

Patch

diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
index c0c13e99aa92..780cb927eab1 100644
--- a/drivers/iio/adc/rzg2l_adc.c
+++ b/drivers/iio/adc/rzg2l_adc.c
@@ -8,7 +8,6 @@ 
  */
 
 #include <linux/bitfield.h>
-#include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/delay.h>
 #include <linux/iio/iio.h>
@@ -69,8 +68,6 @@  struct rzg2l_adc_data {
 
 struct rzg2l_adc {
 	void __iomem *base;
-	struct clk *pclk;
-	struct clk *adclk;
 	struct reset_control *presetn;
 	struct reset_control *adrstn;
 	struct completion completion;
@@ -188,29 +185,18 @@  static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
 	return 0;
 }
 
-static int rzg2l_adc_set_power(struct iio_dev *indio_dev, bool on)
-{
-	struct device *dev = indio_dev->dev.parent;
-
-	if (on)
-		return pm_runtime_resume_and_get(dev);
-
-	return pm_runtime_put_sync(dev);
-}
-
 static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc, u8 ch)
 {
+	struct device *dev = indio_dev->dev.parent;
 	int ret;
 
-	ret = rzg2l_adc_set_power(indio_dev, true);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret)
 		return ret;
 
 	ret = rzg2l_adc_conversion_setup(adc, ch);
-	if (ret) {
-		rzg2l_adc_set_power(indio_dev, false);
-		return ret;
-	}
+	if (ret)
+		goto rpm_put;
 
 	reinit_completion(&adc->completion);
 
@@ -219,12 +205,14 @@  static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc
 	if (!wait_for_completion_timeout(&adc->completion, RZG2L_ADC_TIMEOUT)) {
 		rzg2l_adc_writel(adc, RZG2L_ADINT,
 				 rzg2l_adc_readl(adc, RZG2L_ADINT) & ~RZG2L_ADINT_INTEN_MASK);
-		rzg2l_adc_start_stop(adc, false);
-		rzg2l_adc_set_power(indio_dev, false);
-		return -ETIMEDOUT;
+		ret = -ETIMEDOUT;
 	}
 
-	return rzg2l_adc_set_power(indio_dev, false);
+	rzg2l_adc_start_stop(adc, false);
+
+rpm_put:
+	pm_runtime_put_sync(dev);
+	return ret;
 }
 
 static int rzg2l_adc_read_raw(struct iio_dev *indio_dev,
@@ -348,13 +336,13 @@  static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
 	return 0;
 }
 
-static int rzg2l_adc_hw_init(struct rzg2l_adc *adc)
+static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
 {
 	int timeout = 5;
 	u32 reg;
 	int ret;
 
-	ret = clk_prepare_enable(adc->pclk);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret)
 		return ret;
 
@@ -392,25 +380,10 @@  static int rzg2l_adc_hw_init(struct rzg2l_adc *adc)
 	rzg2l_adc_writel(adc, RZG2L_ADM(3), reg);
 
 exit_hw_init:
-	clk_disable_unprepare(adc->pclk);
-
+	pm_runtime_put_sync(dev);
 	return ret;
 }
 
-static void rzg2l_adc_pm_runtime_disable(void *data)
-{
-	struct device *dev = data;
-
-	pm_runtime_disable(dev->parent);
-}
-
-static void rzg2l_adc_pm_runtime_set_suspended(void *data)
-{
-	struct device *dev = data;
-
-	pm_runtime_set_suspended(dev->parent);
-}
-
 static int rzg2l_adc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -435,14 +408,6 @@  static int rzg2l_adc_probe(struct platform_device *pdev)
 	if (IS_ERR(adc->base))
 		return PTR_ERR(adc->base);
 
-	adc->pclk = devm_clk_get(dev, "pclk");
-	if (IS_ERR(adc->pclk))
-		return dev_err_probe(dev, PTR_ERR(adc->pclk), "Failed to get pclk");
-
-	adc->adclk = devm_clk_get(dev, "adclk");
-	if (IS_ERR(adc->adclk))
-		return dev_err_probe(dev, PTR_ERR(adc->adclk), "Failed to get adclk");
-
 	adc->adrstn = devm_reset_control_get_exclusive_deasserted(dev, "adrst-n");
 	if (IS_ERR(adc->adrstn))
 		return dev_err_probe(dev, PTR_ERR(adc->adrstn), "failed to get/deassert adrst-n\n");
@@ -453,7 +418,13 @@  static int rzg2l_adc_probe(struct platform_device *pdev)
 				     "failed to get/deassert presetn\n");
 	}
 
-	ret = rzg2l_adc_hw_init(adc);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	ret = rzg2l_adc_hw_init(dev, adc);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret, "failed to initialize ADC HW, %d\n", ret);
 
@@ -468,26 +439,12 @@  static int rzg2l_adc_probe(struct platform_device *pdev)
 
 	init_completion(&adc->completion);
 
-	platform_set_drvdata(pdev, indio_dev);
-
 	indio_dev->name = DRIVER_NAME;
 	indio_dev->info = &rzg2l_adc_iio_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = adc->data->channels;
 	indio_dev->num_channels = adc->data->num_channels;
 
-	pm_runtime_set_suspended(dev);
-	ret = devm_add_action_or_reset(&pdev->dev,
-				       rzg2l_adc_pm_runtime_set_suspended, &indio_dev->dev);
-	if (ret)
-		return ret;
-
-	pm_runtime_enable(dev);
-	ret = devm_add_action_or_reset(&pdev->dev,
-				       rzg2l_adc_pm_runtime_disable, &indio_dev->dev);
-	if (ret)
-		return ret;
-
 	return devm_iio_device_register(dev, indio_dev);
 }
 
@@ -503,8 +460,6 @@  static int __maybe_unused rzg2l_adc_pm_runtime_suspend(struct device *dev)
 	struct rzg2l_adc *adc = iio_priv(indio_dev);
 
 	rzg2l_adc_pwr(adc, false);
-	clk_disable_unprepare(adc->adclk);
-	clk_disable_unprepare(adc->pclk);
 
 	return 0;
 }
@@ -513,17 +468,6 @@  static int __maybe_unused rzg2l_adc_pm_runtime_resume(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct rzg2l_adc *adc = iio_priv(indio_dev);
-	int ret;
-
-	ret = clk_prepare_enable(adc->pclk);
-	if (ret)
-		return ret;
-
-	ret = clk_prepare_enable(adc->adclk);
-	if (ret) {
-		clk_disable_unprepare(adc->pclk);
-		return ret;
-	}
 
 	rzg2l_adc_pwr(adc, true);