[PATCH v2] PM / devfreq: sun8i-a33-mbus: Simplify by using more devm functions

[PATCH v2] PM / devfreq: sun8i-a33-mbus: Simplify by using more devm functions

[Uwe Kleine-König]

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Use devm allocators for enabling the bus clock and
clk_rate_exclusive_get(). This simplifies error handling and the remove
callback.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
Hello,

On Wed, May 14, 2025 at 01:25:51AM +0900, Chanwoo Choi wrote in the
thread at
https://lore.kernel.org/linux-pm/CAGTfZH2uk-6-R5QNZwsssQ1UtnqexKeo=e=SsXB26NMwqf8c9g@mail.gmail.com:
> Instead of fix-up patch, I'd like you to send the v2 of patch[1].
> [1] https://patchwork.kernel.org/project/linux-pm/patch/20241111112237.336310-2-u.kleine-koenig@baylibre.com/

Ah, then I misunderstood. Here comes the requested v2. The changes since
(implicit) v1 are:

 - Fix the build failure that I introduced by forgetting to add the dev
   parameter to devm_clk_rate_exclusive_get(dev, priv->clk_mbus);

 - Added the Reviewed-by tag for Chen-Yu.

 - Actually compile tested. I would have sworn I did that for v1
   already, but that is proven to be a perjury now. Don't know how that
   slipped through.

Best regards and again sorry for the mess
Uwe

 drivers/devfreq/sun8i-a33-mbus.c | 38 ++++++++------------------------
 1 file changed, 9 insertions(+), 29 deletions(-)

diff --git a/drivers/devfreq/sun8i-a33-mbus.c b/drivers/devfreq/sun8i-a33-mbus.c
index 7c6ae91ede1f..4bd5657558d6 100644
--- a/drivers/devfreq/sun8i-a33-mbus.c
+++ b/drivers/devfreq/sun8i-a33-mbus.c
@@ -360,7 +360,7 @@ static int sun8i_a33_mbus_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->reg_mbus))
 		return PTR_ERR(priv->reg_mbus);
 
-	priv->clk_bus = devm_clk_get(dev, "bus");
+	priv->clk_bus = devm_clk_get_enabled(dev, "bus");
 	if (IS_ERR(priv->clk_bus))
 		return dev_err_probe(dev, PTR_ERR(priv->clk_bus),
 				     "failed to get bus clock\n");
@@ -375,24 +375,15 @@ static int sun8i_a33_mbus_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(priv->clk_mbus),
 				     "failed to get mbus clock\n");
 
-	ret = clk_prepare_enable(priv->clk_bus);
-	if (ret)
-		return dev_err_probe(dev, ret,
-				     "failed to enable bus clock\n");
-
 	/* Lock the DRAM clock rate to keep priv->nominal_bw in sync. */
-	ret = clk_rate_exclusive_get(priv->clk_dram);
-	if (ret) {
-		err = "failed to lock dram clock rate\n";
-		goto err_disable_bus;
-	}
+	ret = devm_clk_rate_exclusive_get(dev, priv->clk_dram);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to lock dram clock rate\n");
 
 	/* Lock the MBUS clock rate to keep MBUS_TMR_PERIOD in sync. */
-	ret = clk_rate_exclusive_get(priv->clk_mbus);
-	if (ret) {
-		err = "failed to lock mbus clock rate\n";
-		goto err_unlock_dram;
-	}
+	ret = devm_clk_rate_exclusive_get(dev, priv->clk_mbus);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to lock mbus clock rate\n");
 
 	priv->gov_data.upthreshold	= 10;
 	priv->gov_data.downdifferential	=  5;
@@ -405,10 +396,8 @@ static int sun8i_a33_mbus_probe(struct platform_device *pdev)
 	priv->profile.max_state		= max_state;
 
 	ret = devm_pm_opp_set_clkname(dev, "dram");
-	if (ret) {
-		err = "failed to add OPP table\n";
-		goto err_unlock_mbus;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to add OPP table\n");
 
 	base_freq = clk_get_rate(clk_get_parent(priv->clk_dram));
 	for (i = 0; i < max_state; ++i) {
@@ -448,12 +437,6 @@ static int sun8i_a33_mbus_probe(struct platform_device *pdev)
 
 err_remove_opps:
 	dev_pm_opp_remove_all_dynamic(dev);
-err_unlock_mbus:
-	clk_rate_exclusive_put(priv->clk_mbus);
-err_unlock_dram:
-	clk_rate_exclusive_put(priv->clk_dram);
-err_disable_bus:
-	clk_disable_unprepare(priv->clk_bus);
 
 	return dev_err_probe(dev, ret, err);
 }
@@ -472,9 +455,6 @@ static void sun8i_a33_mbus_remove(struct platform_device *pdev)
 		dev_warn(dev, "failed to restore DRAM frequency: %d\n", ret);
 
 	dev_pm_opp_remove_all_dynamic(dev);
-	clk_rate_exclusive_put(priv->clk_mbus);
-	clk_rate_exclusive_put(priv->clk_dram);
-	clk_disable_unprepare(priv->clk_bus);
 }
 
 static const struct sun8i_a33_mbus_variant sun50i_a64_mbus = {

base-commit: 92a09c47464d040866cf2b4cd052bc60555185fb
-- 
2.47.2

Re: [PATCH v2] PM / devfreq: sun8i-a33-mbus: Simplify by using more devm functions

[Corentin Labbe]

Le Tue, May 13, 2025 at 10:39:02PM +0200, Uwe Kleine-König a écrit :
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Use devm allocators for enabling the bus clock and
> clk_rate_exclusive_get(). This simplifies error handling and the remove
> callback.
> 
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---

Tested-by: Corentin LABBE <clabbe.montjoie@gmail.com>
Tested-on: sun8i-a33-olinuxino

Thanks
Regards

Re: [PATCH v2] PM / devfreq: sun8i-a33-mbus: Simplify by using more devm functions

[Chanwoo Choi]

On Wed, May 14, 2025 at 5:39 AM Uwe Kleine-König
<u.kleine-koenig@baylibre.com> wrote:
>
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit

Applied it after removing the above unneeded information.

Thanks,

>
> Use devm allocators for enabling the bus clock and
> clk_rate_exclusive_get(). This simplifies error handling and the remove
> callback.
>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> Hello,
>
> On Wed, May 14, 2025 at 01:25:51AM +0900, Chanwoo Choi wrote in the
> thread at
> https://lore.kernel.org/linux-pm/CAGTfZH2uk-6-R5QNZwsssQ1UtnqexKeo=e=SsXB26NMwqf8c9g@mail.gmail.com:
> > Instead of fix-up patch, I'd like you to send the v2 of patch[1].
> > [1] https://patchwork.kernel.org/project/linux-pm/patch/20241111112237.336310-2-u.kleine-koenig@baylibre.com/
>
> Ah, then I misunderstood. Here comes the requested v2. The changes since
> (implicit) v1 are:
>
>  - Fix the build failure that I introduced by forgetting to add the dev
>    parameter to devm_clk_rate_exclusive_get(dev, priv->clk_mbus);
>
>  - Added the Reviewed-by tag for Chen-Yu.
>
>  - Actually compile tested. I would have sworn I did that for v1
>    already, but that is proven to be a perjury now. Don't know how that
>    slipped through.
>
> Best regards and again sorry for the mess
> Uwe
>
>  drivers/devfreq/sun8i-a33-mbus.c | 38 ++++++++------------------------
>  1 file changed, 9 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/devfreq/sun8i-a33-mbus.c b/drivers/devfreq/sun8i-a33-mbus.c
> index 7c6ae91ede1f..4bd5657558d6 100644
> --- a/drivers/devfreq/sun8i-a33-mbus.c
> +++ b/drivers/devfreq/sun8i-a33-mbus.c
> @@ -360,7 +360,7 @@ static int sun8i_a33_mbus_probe(struct platform_device *pdev)
>         if (IS_ERR(priv->reg_mbus))
>                 return PTR_ERR(priv->reg_mbus);
>
> -       priv->clk_bus = devm_clk_get(dev, "bus");
> +       priv->clk_bus = devm_clk_get_enabled(dev, "bus");
>         if (IS_ERR(priv->clk_bus))
>                 return dev_err_probe(dev, PTR_ERR(priv->clk_bus),
>                                      "failed to get bus clock\n");
> @@ -375,24 +375,15 @@ static int sun8i_a33_mbus_probe(struct platform_device *pdev)
>                 return dev_err_probe(dev, PTR_ERR(priv->clk_mbus),
>                                      "failed to get mbus clock\n");
>
> -       ret = clk_prepare_enable(priv->clk_bus);
> -       if (ret)
> -               return dev_err_probe(dev, ret,
> -                                    "failed to enable bus clock\n");
> -
>         /* Lock the DRAM clock rate to keep priv->nominal_bw in sync. */
> -       ret = clk_rate_exclusive_get(priv->clk_dram);
> -       if (ret) {
> -               err = "failed to lock dram clock rate\n";
> -               goto err_disable_bus;
> -       }
> +       ret = devm_clk_rate_exclusive_get(dev, priv->clk_dram);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "failed to lock dram clock rate\n");
>
>         /* Lock the MBUS clock rate to keep MBUS_TMR_PERIOD in sync. */
> -       ret = clk_rate_exclusive_get(priv->clk_mbus);
> -       if (ret) {
> -               err = "failed to lock mbus clock rate\n";
> -               goto err_unlock_dram;
> -       }
> +       ret = devm_clk_rate_exclusive_get(dev, priv->clk_mbus);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "failed to lock mbus clock rate\n");
>
>         priv->gov_data.upthreshold      = 10;
>         priv->gov_data.downdifferential =  5;
> @@ -405,10 +396,8 @@ static int sun8i_a33_mbus_probe(struct platform_device *pdev)
>         priv->profile.max_state         = max_state;
>
>         ret = devm_pm_opp_set_clkname(dev, "dram");
> -       if (ret) {
> -               err = "failed to add OPP table\n";
> -               goto err_unlock_mbus;
> -       }
> +       if (ret)
> +               return dev_err_probe(dev, ret, "failed to add OPP table\n");
>
>         base_freq = clk_get_rate(clk_get_parent(priv->clk_dram));
>         for (i = 0; i < max_state; ++i) {
> @@ -448,12 +437,6 @@ static int sun8i_a33_mbus_probe(struct platform_device *pdev)
>
>  err_remove_opps:
>         dev_pm_opp_remove_all_dynamic(dev);
> -err_unlock_mbus:
> -       clk_rate_exclusive_put(priv->clk_mbus);
> -err_unlock_dram:
> -       clk_rate_exclusive_put(priv->clk_dram);
> -err_disable_bus:
> -       clk_disable_unprepare(priv->clk_bus);
>
>         return dev_err_probe(dev, ret, err);
>  }
> @@ -472,9 +455,6 @@ static void sun8i_a33_mbus_remove(struct platform_device *pdev)
>                 dev_warn(dev, "failed to restore DRAM frequency: %d\n", ret);
>
>         dev_pm_opp_remove_all_dynamic(dev);
> -       clk_rate_exclusive_put(priv->clk_mbus);
> -       clk_rate_exclusive_put(priv->clk_dram);
> -       clk_disable_unprepare(priv->clk_bus);
>  }
>
>  static const struct sun8i_a33_mbus_variant sun50i_a64_mbus = {
>
> base-commit: 92a09c47464d040866cf2b4cd052bc60555185fb
> --
> 2.47.2
>


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v2] PM / devfreq: sun8i-a33-mbus: Simplify by using more devm functions

[Uwe Kleine-König]

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

Hello,

On Thu, May 15, 2025 at 11:38:11PM +0900, Chanwoo Choi wrote:
> On Wed, May 14, 2025 at 5:39 AM Uwe Kleine-König
> <u.kleine-koenig@baylibre.com> wrote:
> >
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> 
> Applied it after removing the above unneeded information.

Oh indeed. That happend because I added a newline in the header part of
the patch mail. Thanks for fixing.

Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]