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

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xxxxxxx>
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