Re: [PATCH] ASoC: xilinx: formatter_pcm: use devm_clk_get_enabled and simplify probe

[Michal Simek <michal.simek@amd.com>]

On 8/7/26 01:38, Rosen Penev wrote:
> Use devm_clk_get_enabled() to let the clock be enabled automatically
> and cleaned up on unbind, dropping the manual clk_prepare_enable() and
> clk_disable_unprepare() calls in probe and remove.
> 
> Simplify the probe error paths by returning directly instead of
> jumping to clk_err, and use devm_snd_soc_register_component() return
> value directly.
> 
> Assisted-by: opencode:deepseek-v4-flash-free
> ---
>   sound/soc/xilinx/xlnx_formatter_pcm.c | 47 ++++++++-------------------
>   1 file changed, 13 insertions(+), 34 deletions(-)
> 
> diff --git a/sound/soc/xilinx/xlnx_formatter_pcm.c b/sound/soc/xilinx/xlnx_formatter_pcm.c
> index b50306b0fc06..948bc604738a 100644
> --- a/sound/soc/xilinx/xlnx_formatter_pcm.c
> +++ b/sound/soc/xilinx/xlnx_formatter_pcm.c
> @@ -593,24 +593,17 @@ static int xlnx_formatter_pcm_probe(struct platform_device *pdev)
>   	if (!aud_drv_data)
>   		return -ENOMEM;
>   
> -	aud_drv_data->axi_clk = devm_clk_get(dev, "s_axi_lite_aclk");
> +	aud_drv_data->axi_clk = devm_clk_get_enabled(dev, "s_axi_lite_aclk");
>   	if (IS_ERR(aud_drv_data->axi_clk)) {
>   		ret = PTR_ERR(aud_drv_data->axi_clk);
>   		dev_err(dev, "failed to get s_axi_lite_aclk(%d)\n", ret);
>   		return ret;
>   	}
> -	ret = clk_prepare_enable(aud_drv_data->axi_clk);
> -	if (ret) {
> -		dev_err(dev,
> -			"failed to enable s_axi_lite_aclk(%d)\n", ret);
> -		return ret;
> -	}
>   
>   	aud_drv_data->mmio = devm_platform_ioremap_resource(pdev, 0);
>   	if (IS_ERR(aud_drv_data->mmio)) {
>   		dev_err(dev, "audio formatter ioremap failed\n");
> -		ret = PTR_ERR(aud_drv_data->mmio);
> -		goto clk_err;
> +		return PTR_ERR(aud_drv_data->mmio);
>   	}
>   
>   	val = readl(aud_drv_data->mmio + XLNX_AUD_CORE_CONFIG);
> @@ -620,7 +613,7 @@ static int xlnx_formatter_pcm_probe(struct platform_device *pdev)
>   					       XLNX_MM2S_OFFSET);
>   		if (ret) {
>   			dev_err(dev, "audio formatter reset failed\n");
> -			goto clk_err;
> +			return ret;
>   		}
>   		xlnx_formatter_disable_irqs(aud_drv_data->mmio +
>   					    XLNX_MM2S_OFFSET,
> @@ -628,16 +621,15 @@ static int xlnx_formatter_pcm_probe(struct platform_device *pdev)
>   
>   		aud_drv_data->mm2s_irq = platform_get_irq_byname(pdev,
>   								 "irq_mm2s");
> -		if (aud_drv_data->mm2s_irq < 0) {
> -			ret = aud_drv_data->mm2s_irq;
> -			goto clk_err;
> -		}
> +		if (aud_drv_data->mm2s_irq < 0)
> +			return aud_drv_data->mm2s_irq;
> +
>   		ret = devm_request_irq(dev, aud_drv_data->mm2s_irq,
>   				       xlnx_mm2s_irq_handler, 0,
>   				       "xlnx_formatter_pcm_mm2s_irq", aud_drv_data);
>   		if (ret) {
>   			dev_err(dev, "xlnx audio mm2s irq request failed\n");
> -			goto clk_err;
> +			return ret;
>   		}
>   	}
>   	if (val & AUD_CFG_S2MM_MASK) {
> @@ -646,7 +638,7 @@ static int xlnx_formatter_pcm_probe(struct platform_device *pdev)
>   					       XLNX_S2MM_OFFSET);
>   		if (ret) {
>   			dev_err(dev, "audio formatter reset failed\n");
> -			goto clk_err;
> +			return ret;
>   		}
>   		xlnx_formatter_disable_irqs(aud_drv_data->mmio +
>   					    XLNX_S2MM_OFFSET,
> @@ -654,34 +646,23 @@ static int xlnx_formatter_pcm_probe(struct platform_device *pdev)
>   
>   		aud_drv_data->s2mm_irq = platform_get_irq_byname(pdev,
>   								 "irq_s2mm");
> -		if (aud_drv_data->s2mm_irq < 0) {
> -			ret = aud_drv_data->s2mm_irq;
> -			goto clk_err;
> -		}
> +		if (aud_drv_data->s2mm_irq < 0)
> +			return aud_drv_data->s2mm_irq;
> +
>   		ret = devm_request_irq(dev, aud_drv_data->s2mm_irq,
>   				       xlnx_s2mm_irq_handler, 0,
>   				       "xlnx_formatter_pcm_s2mm_irq",
>   				       aud_drv_data);
>   		if (ret) {
>   			dev_err(dev, "xlnx audio s2mm irq request failed\n");
> -			goto clk_err;
> +			return ret;
>   		}
>   	}
>   
>   	dev_set_drvdata(dev, aud_drv_data);
>   
> -	ret = devm_snd_soc_register_component(dev, &xlnx_asoc_component,
> +	return devm_snd_soc_register_component(dev, &xlnx_asoc_component,
>   					      NULL, 0);
> -	if (ret) {
> -		dev_err(dev, "pcm platform device register failed\n");
> -		goto clk_err;
> -	}
> -
> -	return 0;
> -
> -clk_err:
> -	clk_disable_unprepare(aud_drv_data->axi_clk);
> -	return ret;
>   }
>   
>   static void xlnx_formatter_pcm_remove(struct platform_device *pdev)
> @@ -698,8 +679,6 @@ static void xlnx_formatter_pcm_remove(struct platform_device *pdev)
>   
>   	if (ret)
>   		dev_err(&pdev->dev, "audio formatter reset failed\n");
> -
> -	clk_disable_unprepare(adata->axi_clk);
>   }
>   
>   static const struct of_device_id xlnx_formatter_pcm_of_match[] = {

As Mark mentioned SOB is missing but the patch is correct.
When SOB is added feel free to add
Reviewed-by: Michal Simek <michal.simek@amd.com>

Thanks,
Michal