[PATCHv2] ASoC: xilinx: formatter_pcm: use more devm in probe

[PATCHv2] ASoC: xilinx: formatter_pcm: use more devm in probe

[Rosen Penev]

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. As a result, move the
clk struct member into probe, which is the only place it's used.

Simplify the probe error paths by returning directly instead of
jumping to clk_err, and use devm_snd_soc_register_component() return
value directly. Also remove error message from ioremap as it throws one
itself. Same with devm_request_irq().

Assisted-by: opencode:deepseek-v4-flash-free
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: add signed-off-by. Further clean up some stuff.
 sound/soc/xilinx/xlnx_formatter_pcm.c | 70 ++++++++-------------------
 1 file changed, 20 insertions(+), 50 deletions(-)

diff --git a/sound/soc/xilinx/xlnx_formatter_pcm.c b/sound/soc/xilinx/xlnx_formatter_pcm.c
index 3d6f1e4046d8..eb878c75e293 100644
--- a/sound/soc/xilinx/xlnx_formatter_pcm.c
+++ b/sound/soc/xilinx/xlnx_formatter_pcm.c
@@ -82,7 +82,6 @@ struct xlnx_pcm_drv_data {
 	int mm2s_irq;
 	struct snd_pcm_substream *play_stream;
 	struct snd_pcm_substream *capture_stream;
-	struct clk *axi_clk;
 	unsigned int sysclk;
 };
 
@@ -596,30 +595,19 @@ static int xlnx_formatter_pcm_probe(struct platform_device *pdev)
 	u32 val;
 	struct xlnx_pcm_drv_data *aud_drv_data;
 	struct device *dev = &pdev->dev;
+	struct clk *axi_clk;
+
+	axi_clk = devm_clk_get_enabled(dev, "s_axi_lite_aclk");
+	if (IS_ERR(axi_clk))
+		return dev_err_probe(dev, PTR_ERR(axi_clk), "failed to get s_axi_lite_aclk");
 
 	aud_drv_data = devm_kzalloc(dev, sizeof(*aud_drv_data), GFP_KERNEL);
 	if (!aud_drv_data)
 		return -ENOMEM;
 
-	aud_drv_data->axi_clk = devm_clk_get(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;
-	}
+	if (IS_ERR(aud_drv_data->mmio))
+		return PTR_ERR(aud_drv_data->mmio);
 
 	val = readl(aud_drv_data->mmio + XLNX_AUD_CORE_CONFIG);
 	if (val & AUD_CFG_MM2S_MASK) {
@@ -628,7 +616,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,
@@ -636,17 +624,14 @@ 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;
-		}
+		if (ret)
+			return ret;
 	}
 	if (val & AUD_CFG_S2MM_MASK) {
 		aud_drv_data->s2mm_presence = true;
@@ -654,7 +639,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,
@@ -662,34 +647,21 @@ 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;
-		}
+		if (ret)
+			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)
@@ -706,8 +678,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[] = {
-- 
2.55.0

Re: [PATCHv2] ASoC: xilinx: formatter_pcm: use more devm in probe

[Michal Simek]

On 8/11/26 20:50, 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. As a result, move the
> clk struct member into probe, which is the only place it's used.
> 
> Simplify the probe error paths by returning directly instead of
> jumping to clk_err, and use devm_snd_soc_register_component() return
> value directly. Also remove error message from ioremap as it throws one
> itself. Same with devm_request_irq().
> 
> Assisted-by: opencode:deepseek-v4-flash-free
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>   v2: add signed-off-by. Further clean up some stuff.
>   sound/soc/xilinx/xlnx_formatter_pcm.c | 70 ++++++++-------------------
>   1 file changed, 20 insertions(+), 50 deletions(-)
> 
> diff --git a/sound/soc/xilinx/xlnx_formatter_pcm.c b/sound/soc/xilinx/xlnx_formatter_pcm.c
> index 3d6f1e4046d8..eb878c75e293 100644
> --- a/sound/soc/xilinx/xlnx_formatter_pcm.c
> +++ b/sound/soc/xilinx/xlnx_formatter_pcm.c
> @@ -82,7 +82,6 @@ struct xlnx_pcm_drv_data {
>   	int mm2s_irq;
>   	struct snd_pcm_substream *play_stream;
>   	struct snd_pcm_substream *capture_stream;
> -	struct clk *axi_clk;
>   	unsigned int sysclk;
>   };
>   
> @@ -596,30 +595,19 @@ static int xlnx_formatter_pcm_probe(struct platform_device *pdev)
>   	u32 val;
>   	struct xlnx_pcm_drv_data *aud_drv_data;
>   	struct device *dev = &pdev->dev;
> +	struct clk *axi_clk;
> +
> +	axi_clk = devm_clk_get_enabled(dev, "s_axi_lite_aclk");
> +	if (IS_ERR(axi_clk))
> +		return dev_err_probe(dev, PTR_ERR(axi_clk), "failed to get s_axi_lite_aclk");
>   
>   	aud_drv_data = devm_kzalloc(dev, sizeof(*aud_drv_data), GFP_KERNEL);
>   	if (!aud_drv_data)
>   		return -ENOMEM;
>   
> -	aud_drv_data->axi_clk = devm_clk_get(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;
> -	}
> +	if (IS_ERR(aud_drv_data->mmio))
> +		return PTR_ERR(aud_drv_data->mmio);
>   
>   	val = readl(aud_drv_data->mmio + XLNX_AUD_CORE_CONFIG);
>   	if (val & AUD_CFG_MM2S_MASK) {
> @@ -628,7 +616,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,
> @@ -636,17 +624,14 @@ 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;
> -		}
> +		if (ret)
> +			return ret;
>   	}
>   	if (val & AUD_CFG_S2MM_MASK) {
>   		aud_drv_data->s2mm_presence = true;
> @@ -654,7 +639,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,
> @@ -662,34 +647,21 @@ 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;
> -		}
> +		if (ret)
> +			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)
> @@ -706,8 +678,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[] = {

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

Thanks,
Michal

Re: [PATCHv2] ASoC: xilinx: formatter_pcm: use more devm in probe

[Mark Brown]

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

On Wed, Aug 12, 2026 at 07:58:23AM +0200, Michal Simek wrote:
> 
> 
> On 8/11/26 20:50, 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. As a result, move the
> > clk struct member into probe, which is the only place it's used.

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

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

Re: [PATCHv2] ASoC: xilinx: formatter_pcm: use more devm in probe

[Michal Simek]

Hi Mark.

On 8/12/26 12:16, Mark Brown wrote:
> On Wed, Aug 12, 2026 at 07:58:23AM +0200, Michal Simek wrote:
>>
>>
>> On 8/11/26 20:50, 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. As a result, move the
>>> clk struct member into probe, which is the only place it's used.
> 
> Please delete unneeded context from mails when replying.  Doing this
> makes it much easier to find your reply in the message, helping ensure
> it won't be missed by people scrolling through the irrelevant quoted
> material.

Do you mean patch content in this case?

Thanks,
Michal

Re: [PATCHv2] ASoC: xilinx: formatter_pcm: use more devm in probe

[Mark Brown]

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

On Wed, Aug 12, 2026 at 01:22:33PM +0200, Michal Simek wrote:
> On 8/12/26 12:16, Mark Brown wrote:

> > Please delete unneeded context from mails when replying.  Doing this
> > makes it much easier to find your reply in the message, helping ensure
> > it won't be missed by people scrolling through the irrelevant quoted
> > material.

> Do you mean patch content in this case?

Yes, we don't need the entire patch (especially not with the only
comment being added at the very end of the mail).

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

Re: [PATCHv2] ASoC: xilinx: formatter_pcm: use more devm in probe

[Vincenzo Frascino]

On 11/08/2026 19:50, 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. As a result, move the
> clk struct member into probe, which is the only place it's used.
> 
> Simplify the probe error paths by returning directly instead of
> jumping to clk_err, and use devm_snd_soc_register_component() return
> value directly. Also remove error message from ioremap as it throws one
> itself. Same with devm_request_irq().
> 
> Assisted-by: opencode:deepseek-v4-flash-free
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

[...]

Re: [PATCHv2] ASoC: xilinx: formatter_pcm: use more devm in probe

[Mark Brown]

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

On Tue, Aug 11, 2026 at 11:50:13AM -0700, 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. As a result, move the
> clk struct member into probe, which is the only place it's used.

This doesn't apply against current code, please check and resend.

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