[PATCHv2] ASoC: xilinx: formatter_pcm: use more devm in probe
Michal Simek
michal.simek at amd.com
Tue Aug 11 22:58:23 PDT 2026
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 at 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 at amd.com>
Thanks,
Michal
More information about the linux-arm-kernel
mailing list