[PATCH] fpga: xilinx-pr-decoupler: Use devm_clk_get_prepared()

[PATCH] fpga: xilinx-pr-decoupler: Use devm_clk_get_prepared()

[Michal Simek]

The driver keeps the "aclk" clock prepared but disabled in its idle
state, toggling only the atomic clk_enable()/clk_disable() around
register accesses in the bridge enable_set/enable_show callbacks.

At probe time this was open-coded as clk_prepare_enable() immediately
followed by clk_disable(), leaving the clock prepared, with a matching
clk_unprepare() in the error path and in remove().

devm_clk_get_prepared() expresses exactly this: it gets and prepares the
clock and unprepares it automatically on driver detach.

Use it to drop the manual prepare/disable dance, the error-path
unprepare, and the now-empty clock teardown in remove().

Signed-off-by: Michal Simek <michal.simek@amd.com>
---

 drivers/fpga/xilinx-pr-decoupler.c | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/fpga/xilinx-pr-decoupler.c b/drivers/fpga/xilinx-pr-decoupler.c
index 6994d68e9036..45b65a3264af 100644
--- a/drivers/fpga/xilinx-pr-decoupler.c
+++ b/drivers/fpga/xilinx-pr-decoupler.c
@@ -118,46 +118,30 @@ static int xlnx_pr_decoupler_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->io_base))
 		return PTR_ERR(priv->io_base);
 
-	priv->clk = devm_clk_get(&pdev->dev, "aclk");
+	priv->clk = devm_clk_get_prepared(&pdev->dev, "aclk");
 	if (IS_ERR(priv->clk))
 		return dev_err_probe(&pdev->dev, PTR_ERR(priv->clk),
 				     "input clock not found\n");
 
-	err = clk_prepare_enable(priv->clk);
-	if (err) {
-		dev_err(&pdev->dev, "unable to enable clock\n");
-		return err;
-	}
-
-	clk_disable(priv->clk);
-
 	br = fpga_bridge_register(&pdev->dev, priv->ipconfig->name,
 				  &xlnx_pr_decoupler_br_ops, priv);
 	if (IS_ERR(br)) {
 		err = PTR_ERR(br);
 		dev_err(&pdev->dev, "unable to register %s",
 			priv->ipconfig->name);
-		goto err_clk;
+		return err;
 	}
 
 	platform_set_drvdata(pdev, br);
 
 	return 0;
-
-err_clk:
-	clk_unprepare(priv->clk);
-
-	return err;
 }
 
 static void xlnx_pr_decoupler_remove(struct platform_device *pdev)
 {
 	struct fpga_bridge *bridge = platform_get_drvdata(pdev);
-	struct xlnx_pr_decoupler_data *p = bridge->priv;
 
 	fpga_bridge_unregister(bridge);
-
-	clk_unprepare(p->clk);
 }
 
 static struct platform_driver xlnx_pr_decoupler_driver = {
---
base-commit: 502d801f0ab03e4f32f9a33d203154ce84887921
branch: xnext/pr-decoupler

-- 
2.43.0

Re: [PATCH] fpga: xilinx-pr-decoupler: Use devm_clk_get_prepared()

[Pandey, Radhey Shyam]

> The driver keeps the "aclk" clock prepared but disabled in its idle
> state, toggling only the atomic clk_enable()/clk_disable() around
> register accesses in the bridge enable_set/enable_show callbacks.
> 
> At probe time this was open-coded as clk_prepare_enable() immediately
> followed by clk_disable(), leaving the clock prepared, with a matching
> clk_unprepare() in the error path and in remove().
> 
> devm_clk_get_prepared() expresses exactly this: it gets and prepares the
> clock and unprepares it automatically on driver detach.
> 
> Use it to drop the manual prepare/disable dance, the error-path
> unprepare, and the now-empty clock teardown in remove().
> 
> Signed-off-by: Michal Simek <michal.simek@amd.com>

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Thanks!

> ---
> 
>   drivers/fpga/xilinx-pr-decoupler.c | 20 ++------------------
>   1 file changed, 2 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/fpga/xilinx-pr-decoupler.c b/drivers/fpga/xilinx-pr-decoupler.c
> index 6994d68e9036..45b65a3264af 100644
> --- a/drivers/fpga/xilinx-pr-decoupler.c
> +++ b/drivers/fpga/xilinx-pr-decoupler.c
> @@ -118,46 +118,30 @@ static int xlnx_pr_decoupler_probe(struct platform_device *pdev)
>   	if (IS_ERR(priv->io_base))
>   		return PTR_ERR(priv->io_base);
>   
> -	priv->clk = devm_clk_get(&pdev->dev, "aclk");
> +	priv->clk = devm_clk_get_prepared(&pdev->dev, "aclk");
>   	if (IS_ERR(priv->clk))
>   		return dev_err_probe(&pdev->dev, PTR_ERR(priv->clk),
>   				     "input clock not found\n");
>   
> -	err = clk_prepare_enable(priv->clk);
> -	if (err) {
> -		dev_err(&pdev->dev, "unable to enable clock\n");
> -		return err;
> -	}
> -
> -	clk_disable(priv->clk);
> -
>   	br = fpga_bridge_register(&pdev->dev, priv->ipconfig->name,
>   				  &xlnx_pr_decoupler_br_ops, priv);
>   	if (IS_ERR(br)) {
>   		err = PTR_ERR(br);
>   		dev_err(&pdev->dev, "unable to register %s",
>   			priv->ipconfig->name);
> -		goto err_clk;
> +		return err;
>   	}
>   
>   	platform_set_drvdata(pdev, br);
>   
>   	return 0;
> -
> -err_clk:
> -	clk_unprepare(priv->clk);
> -
> -	return err;
>   }
>   
>   static void xlnx_pr_decoupler_remove(struct platform_device *pdev)
>   {
>   	struct fpga_bridge *bridge = platform_get_drvdata(pdev);
> -	struct xlnx_pr_decoupler_data *p = bridge->priv;
>   
>   	fpga_bridge_unregister(bridge);
> -
> -	clk_unprepare(p->clk);
>   }
>   
>   static struct platform_driver xlnx_pr_decoupler_driver = {
> ---
> base-commit: 502d801f0ab03e4f32f9a33d203154ce84887921
> branch: xnext/pr-decoupler
>