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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Dec 6, 2024 at 11:14 AM Claudiu <claudiu.beznea@xxxxxxxxx> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxx>
> ---
>
> 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@xxxxxxxxxxxxxx>

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
>
>





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux