[PATCH 0/3] ASoC: mxs: Improve probe error handling

[PATCH 0/3] ASoC: mxs: Improve probe error handling

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Hi all,

This series improves probe error handling in the MXS ASoC drivers.
Use dev_err_probe() where appropriate and remove redundant dev_err() calls
when the corresponding errors are already reported by the called functions
or further down the call chain.

Compile-tested only.

Best regards, 
Phuc

bui duc phuc (3):
  ASoC: mxs-saif: Use dev_err_probe() for error handling
  ASoC: mxs-saif: Drop redundant probe error messages
  ASoC: mxs-sgtl5000: Drop redundant probe error messages

 sound/soc/mxs/mxs-saif.c     | 21 ++++++---------------
 sound/soc/mxs/mxs-sgtl5000.c |  2 --
 2 files changed, 6 insertions(+), 17 deletions(-)

-- 
2.43.0

[PATCH 1/3] ASoC: mxs-saif: Use dev_err_probe() for error handling

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Replace dev_err() with dev_err_probe() to prevent log spam when probe
returns -EPROBE_DEFER.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/mxs/mxs-saif.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c
index a01a680ad4d7..b877c978a04c 100644
--- a/sound/soc/mxs/mxs-saif.c
+++ b/sound/soc/mxs/mxs-saif.c
@@ -826,12 +826,9 @@ static int mxs_saif_probe(struct platform_device *pdev)
 	mxs_saif[saif->id] = saif;
 
 	saif->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(saif->clk)) {
-		ret = PTR_ERR(saif->clk);
-		dev_err(&pdev->dev, "Cannot get the clock: %d\n",
-			ret);
-		return ret;
-	}
+	if (IS_ERR(saif->clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(saif->clk),
+				     "Cannot get the clock\n");
 
 	saif->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(saif->base))
-- 
2.43.0

Re: [PATCH 1/3] ASoC: mxs-saif: Use dev_err_probe() for error handling

[Frank Li]

On Wed, Aug 12, 2026 at 05:14:16PM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
>
> Replace dev_err() with dev_err_probe() to prevent log spam when probe
> returns -EPROBE_DEFER.
>
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  sound/soc/mxs/mxs-saif.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c
> index a01a680ad4d7..b877c978a04c 100644
> --- a/sound/soc/mxs/mxs-saif.c
> +++ b/sound/soc/mxs/mxs-saif.c
> @@ -826,12 +826,9 @@ static int mxs_saif_probe(struct platform_device *pdev)
>  	mxs_saif[saif->id] = saif;
>
>  	saif->clk = devm_clk_get(&pdev->dev, NULL);
> -	if (IS_ERR(saif->clk)) {
> -		ret = PTR_ERR(saif->clk);
> -		dev_err(&pdev->dev, "Cannot get the clock: %d\n",
> -			ret);
> -		return ret;
> -	}
> +	if (IS_ERR(saif->clk))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(saif->clk),
> +				     "Cannot get the clock\n");
>
>  	saif->base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(saif->base))
> --
> 2.43.0
>
>

[PATCH 2/3] ASoC: mxs-saif: Drop redundant probe error messages

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

The functions called here don't log the error themselves, but the
error is already reported deeper in the call chain, so the dev_err()
calls are redundant and can be removed.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/mxs/mxs-saif.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c
index b877c978a04c..a77cd516a1bf 100644
--- a/sound/soc/mxs/mxs-saif.c
+++ b/sound/soc/mxs/mxs-saif.c
@@ -841,10 +841,8 @@ static int mxs_saif_probe(struct platform_device *pdev)
 	saif->dev = &pdev->dev;
 	ret = devm_request_irq(&pdev->dev, irq, mxs_saif_irq, 0,
 			       dev_name(&pdev->dev), saif);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to request irq\n");
+	if (ret)
 		return ret;
-	}
 
 	platform_set_drvdata(pdev, saif);
 
@@ -857,16 +855,12 @@ static int mxs_saif_probe(struct platform_device *pdev)
 
 	ret = devm_snd_soc_register_component(&pdev->dev, &mxs_saif_component,
 					      &mxs_saif_dai, 1);
-	if (ret) {
-		dev_err(&pdev->dev, "register DAI failed\n");
+	if (ret)
 		return ret;
-	}
 
 	ret = mxs_pcm_platform_register(&pdev->dev);
-	if (ret) {
-		dev_err(&pdev->dev, "register PCM failed: %d\n", ret);
+	if (ret)
 		return ret;
-	}
 
 	return 0;
 }
-- 
2.43.0

Re: [PATCH 2/3] ASoC: mxs-saif: Drop redundant probe error messages

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Low] Removing error logs degrades debugging context, as the underlying functions do not log their specific failures.

Pre-existing issues:
- [High] Clock divider and OF clock provider leak on probe failure, leading to a kernel panic (use-after-unmap) upon subsequent accesses.
- [High] The global `mxs_saif` array retains a dangling pointer when `mxs_saif_probe` fails or the driver is unbound, leading to a potential Use-After-Free.
--

--- Patch [2]: [PATCH 2/3] ASoC: mxs-saif: Drop redundant probe error messages ---
commit d8455dc02bf0f7c00ea57b01724fbc7b2b394e9d
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: mxs-saif: Drop redundant probe error messages

This commit removes error logging messages from the probe function's failure
paths. The commit message states these are redundant because the underlying
functions log the errors themselves.

> commit d8455dc02bf0f7c00ea57b01724fbc7b2b394e9d
> Author: bui duc phuc <phucduc.bui@gmail.com>
> Date:   Wed Aug 12 10:14:17 2026 +0000
> 
>     ASoC: mxs-saif: Drop redundant probe error messages
>     
>     The functions called here don't log the error themselves, but the
>     error is already reported deeper in the call chain, so the dev_err()
>     calls are redundant and can be removed.

[Severity: Low]
Are we sure the underlying functions actually log these errors?

Looking at devm_request_irq(), devm_snd_soc_register_component(), and
mxs_pcm_platform_register(), they do not appear to print error messages when
they fail, except in very specific edge cases.

While the driver core will log a generic failure code on probe exit, does
removing these specific error logs cause the driver to fail silently at these
initialization steps, making it difficult to debug which step failed?

> diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c
> index b877c978a04c6..a77cd516a1bfd 100644
> --- a/sound/soc/mxs/mxs-saif.c
> +++ b/sound/soc/mxs/mxs-saif.c
> @@ -841,10 +841,8 @@ static int mxs_saif_probe(struct platform_device *pdev)
>  	saif->dev = &pdev->dev;
>  	ret = devm_request_irq(&pdev->dev, irq, mxs_saif_irq, 0,
>  			       dev_name(&pdev->dev), saif);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to request irq\n");
> +	if (ret)
>  		return ret;

[Severity: High]
This is a pre-existing issue, but does returning here leave a dangling pointer
in the global mxs_saif array?

Earlier in mxs_saif_probe(), the newly allocated saif context is stored
globally:

mxs_saif_probe()
    mxs_saif[saif->id] = saif;

If the probe function fails here, or if the driver is manually unbound via
sysfs, devres will automatically free the saif structure. However, the driver
never clears mxs_saif[saif->id].

If a user is actively playing audio and the driver is unbound, the ALSA core
unregisters the component but keeps the PCM file descriptor open. When the
stream is subsequently closed, the machine driver's hw_free callback invokes
mxs_saif_put_mclk(), which will dereference the freed mxs_saif pointer.

>  
>  	platform_set_drvdata(pdev, saif);
>  
> @@ -857,16 +855,12 @@ static int mxs_saif_probe(struct platform_device *pdev)
>  
>  	ret = devm_snd_soc_register_component(&pdev->dev, &mxs_saif_component,
>  					      &mxs_saif_dai, 1);
> -	if (ret) {
> -		dev_err(&pdev->dev, "register DAI failed\n");
> +	if (ret)
>  		return ret;

[Severity: High]
This is also a pre-existing issue, but does returning here cause a
use-after-unmap panic if the clock divider was initialized?

When mxs_saif_probe() calls mxs_saif_mclk_init() for saif0, it registers a
clock divider and an OF clock provider. These are not devm-managed and require
explicit unregistration.

The registered clock divider is passed a register pointer mapped via devres:

mxs_saif_mclk_init()
    clk = clk_register_divider(..., saif->base + SAIF_CTRL, ...);

If devm_snd_soc_register_component() fails and we return directly, devres
will automatically unmap saif->base.

Because the clock framework objects are never unregistered, they remain in the
system. Any subsequent access to this clock, such as reading debugfs or a
machine driver interacting with the clock, will dereference the now-unmapped
register pointer and cause a synchronous external abort.

>  
>  	ret = mxs_pcm_platform_register(&pdev->dev);
> -	if (ret) {
> -		dev_err(&pdev->dev, "register PCM failed: %d\n", ret);
> +	if (ret)
>  		return ret;
> -	}
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812101418.37966-1-phucduc.bui@gmail.com?part=2

Re: [PATCH 2/3] ASoC: mxs-saif: Drop redundant probe error messages

[Bui Duc Phuc]

> [Severity: Low]
> Are we sure the underlying functions actually log these errors?
>
> Looking at devm_request_irq(), devm_snd_soc_register_component(), and
> mxs_pcm_platform_register(), they do not appear to print error messages when
> they fail, except in very specific edge cases.
>
> While the driver core will log a generic failure code on probe exit, does
> removing these specific error logs cause the driver to fail silently at these
> initialization steps, making it difficult to debug which step failed?
>

I checked all error paths in the called functions, and they already
report the corresponding errors,
either directly or deeper in the call chain.
Therefore, removing the additional dev_err() calls does not make these
failures silent.

> [Severity: High]
> This is a pre-existing issue, but does returning here leave a dangling pointer
> in the global mxs_saif array?
>
> Earlier in mxs_saif_probe(), the newly allocated saif context is stored
> globally:
>
> mxs_saif_probe()
>     mxs_saif[saif->id] = saif;
>
> If the probe function fails here, or if the driver is manually unbound via
> sysfs, devres will automatically free the saif structure. However, the driver
> never clears mxs_saif[saif->id].
>
> If a user is actively playing audio and the driver is unbound, the ALSA core
> unregisters the component but keeps the PCM file descriptor open. When the
> stream is subsequently closed, the machine driver's hw_free callback invokes
> mxs_saif_put_mclk(), which will dereference the freed mxs_saif pointer.
>
> >
> >       platform_set_drvdata(pdev, saif);
> >
> > @@ -857,16 +855,12 @@ static int mxs_saif_probe(struct platform_device *pdev)
> >
> >       ret = devm_snd_soc_register_component(&pdev->dev, &mxs_saif_component,
> >                                             &mxs_saif_dai, 1);
> > -     if (ret) {
> > -             dev_err(&pdev->dev, "register DAI failed\n");
> > +     if (ret)
> >               return ret;
>
> [Severity: High]
> This is also a pre-existing issue, but does returning here cause a
> use-after-unmap panic if the clock divider was initialized?
>
> When mxs_saif_probe() calls mxs_saif_mclk_init() for saif0, it registers a
> clock divider and an OF clock provider. These are not devm-managed and require
> explicit unregistration.
>
> The registered clock divider is passed a register pointer mapped via devres:
>
> mxs_saif_mclk_init()
>     clk = clk_register_divider(..., saif->base + SAIF_CTRL, ...);
>
> If devm_snd_soc_register_component() fails and we return directly, devres
> will automatically unmap saif->base.
>
> Because the clock framework objects are never unregistered, they remain in the
> system. Any subsequent access to this clock, such as reading debugfs or a
> machine driver interacting with the clock, will dereference the now-unmapped
> register pointer and cause a synchronous external abort.
>
> >
> >       ret = mxs_pcm_platform_register(&pdev->dev);
> > -     if (ret) {
> > -             dev_err(&pdev->dev, "register PCM failed: %d\n", ret);
> > +     if (ret)
> >               return ret;
> > -     }
> >
> >       return 0;
> >  }
>

I confirmed that both issues are pre-existing and are not introduced
by this patch.
They are unrelated to the error-message cleanup in this series.

Re: [PATCH 2/3] ASoC: mxs-saif: Drop redundant probe error messages

[Daniel Baluta]

On 8/12/26 14:28, Bui Duc Phuc wrote:
>> [Severity: Low]
>> Are we sure the underlying functions actually log these errors?
>>
>> Looking at devm_request_irq(), devm_snd_soc_register_component(), and
>> mxs_pcm_platform_register(), they do not appear to print error messages when
>> they fail, except in very specific edge cases.
>>
>> While the driver core will log a generic failure code on probe exit, does
>> removing these specific error logs cause the driver to fail silently at these
>> initialization steps, making it difficult to debug which step failed?
>>
> 
> I checked all error paths in the called functions, and they already
> report the corresponding errors,
> either directly or deeper in the call chain.
> Therefore, removing the additional dev_err() calls does not make these
> failures silent.

Point here is not really that the error is not propagated up in the call chain
but more like if you remove the error messages the user wont' really know exactly
where the failure happened.

Is there a tool that told you to fix these? How did you reached the conclusion that
this patch is needed.


thanks,
Daniel.

Re: [PATCH 2/3] ASoC: mxs-saif: Drop redundant probe error messages

[Bui Duc Phuc]

Hi Daniel,

Thank you for your feedback.

>
> Is there a tool that told you to fix these? How did you reached the conclusion that
> this patch is needed.
>

My main goal was to understand what the underlying core code is
actually doing, so I tend
to trace the call chain and read deeper into the implementation when
reviewing these issues.
I use Bootlin Elixir for this, as it is convenient to access anytime
and anywhere,
even from my phone :

https://elixir.bootlin.com/linux/v7.1.8/C/ident/platform_get_irq_optional

I don't use a tool to detect bugs or generate these patches. The
patches are mainly the result of
reading the code, tracing the actual paths, and then finding issues or
improvements along the way.
Also, through discussions with other maintainers during the review
process, I can sometimes
identify additional issues or areas that could be improved.
For example, this discussion led me to another improvement:

https://lore.kernel.org/all/CAABR9nG2vYKhMKeSxFstCRcHb8Z2FzDTBuQ3PY261Lipx7FfJw@mail.gmail.com/

https://lore.kernel.org/all/95f5d6ec-ecc5-4090-ac04-27cdcdc21b8a@sirena.org.uk/#t

> > I checked all error paths in the called functions, and they already
> > report the corresponding errors,
> > either directly or deeper in the call chain.
> > Therefore, removing the additional dev_err() calls does not make these
> > failures silent.
>
> Point here is not really that the error is not propagated up in the call chain
> but more like if you remove the error messages the user wont' really know exactly
> where the failure happened.

I understand your concern.
I think this comes down to perspective on how much value the outer
message adds.
Let me give a concrete example :

----------------------------------------------------------------
       ret = devm_request_irq(&pdev->dev, irq, mxs_saif_irq, 0,
                               dev_name(&pdev->dev), saif);
-       if (ret) {
-               dev_err(&pdev->dev, "failed to request irq\n");
+       if (ret)
                return ret;
-       }
---------------------------------------------------------------

Here, devm_request_irq() internally calls devm_request_result()

https://elixir.bootlin.com/linux/v7.2-rc7/source/kernel/irq/devres.c#L33

 which already uses dev_err_probe():
--------------------------
dev_err_probe(dev, rc, "request_irq(%u) %ps %ps %s\n",
                         irq, handler, thread_fn, devname ? : "");
-------------------------

Since this lower-level message already logs the exact IRQ number,
handler, thread_fn,
devname, and error code, my thought was that the outer dev_err() might
be redundant
without adding extra context.

That said, I recognize this can be a matter of maintainer preference.
If you feel keeping the explicit dev_err() at the driver level is
still clearer for tracing,
I am completely fine dropping this change from the patch series.

Best regards,
Phuc

[PATCH 3/3] ASoC: mxs-sgtl5000: Drop redundant probe error messages

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Remove the probe error messages to avoid duplicate error reporting,
since the error is already reported by the called functions.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/mxs/mxs-sgtl5000.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/soc/mxs/mxs-sgtl5000.c b/sound/soc/mxs/mxs-sgtl5000.c
index f1c0e612313d..a253a48ca59c 100644
--- a/sound/soc/mxs/mxs-sgtl5000.c
+++ b/sound/soc/mxs/mxs-sgtl5000.c
@@ -155,8 +155,6 @@ static int mxs_sgtl5000_probe(struct platform_device *pdev)
 
 		ret = snd_soc_of_parse_audio_routing(card, "audio-routing");
 		if (ret) {
-			dev_err(&pdev->dev, "failed to parse audio-routing (%d)\n",
-				ret);
 			mxs_saif_put_mclk(0);
 			return ret;
 		}
-- 
2.43.0

Re: [PATCH 3/3] ASoC: mxs-sgtl5000: Drop redundant probe error messages

[Daniel Baluta]

On 8/12/26 13:14, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> Remove the probe error messages to avoid duplicate error reporting,
> since the error is already reported by the called functions.
> 
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>

This looks to be OK. Indeed at various points snd_soc_of_parse_audio_routing prints
an error message if it gets into trouble.

Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>

> ---

>  sound/soc/mxs/mxs-sgtl5000.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/sound/soc/mxs/mxs-sgtl5000.c b/sound/soc/mxs/mxs-sgtl5000.c
> index f1c0e612313d..a253a48ca59c 100644
> --- a/sound/soc/mxs/mxs-sgtl5000.c
> +++ b/sound/soc/mxs/mxs-sgtl5000.c
> @@ -155,8 +155,6 @@ static int mxs_sgtl5000_probe(struct platform_device *pdev)
>  
>  		ret = snd_soc_of_parse_audio_routing(card, "audio-routing");
>  		if (ret) {
> -			dev_err(&pdev->dev, "failed to parse audio-routing (%d)\n",
> -				ret);
>  			mxs_saif_put_mclk(0);
>  			return ret;
>  		}

Re: [PATCH 3/3] ASoC: mxs-sgtl5000: Drop redundant probe error messages

[Frank Li]

On Wed, Aug 12, 2026 at 05:14:18PM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
>
> Remove the probe error messages to avoid duplicate error reporting,
> since the error is already reported by the called functions.
>
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  sound/soc/mxs/mxs-sgtl5000.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/sound/soc/mxs/mxs-sgtl5000.c b/sound/soc/mxs/mxs-sgtl5000.c
> index f1c0e612313d..a253a48ca59c 100644
> --- a/sound/soc/mxs/mxs-sgtl5000.c
> +++ b/sound/soc/mxs/mxs-sgtl5000.c
> @@ -155,8 +155,6 @@ static int mxs_sgtl5000_probe(struct platform_device *pdev)
>
>  		ret = snd_soc_of_parse_audio_routing(card, "audio-routing");
>  		if (ret) {
> -			dev_err(&pdev->dev, "failed to parse audio-routing (%d)\n",
> -				ret);
>  			mxs_saif_put_mclk(0);
>  			return ret;
>  		}
> --
> 2.43.0
>
>

Re: [PATCH 0/3] ASoC: mxs: Improve probe error handling

[Mark Brown]

On Wed, 12 Aug 2026 17:14:15 +0700, phucduc.bui@gmail.com wrote:
> ASoC: mxs: Improve probe error handling
> 
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> Hi all,
> 
> This series improves probe error handling in the MXS ASoC drivers.
> Use dev_err_probe() where appropriate and remove redundant dev_err() calls
> when the corresponding errors are already reported by the called functions
> or further down the call chain.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.3

Thanks!

[1/3] ASoC: mxs-saif: Use dev_err_probe() for error handling
      https://git.kernel.org/broonie/sound/c/b215caca714e
[2/3] ASoC: mxs-saif: Drop redundant probe error messages
      https://git.kernel.org/broonie/sound/c/f0701e5fc299
[3/3] ASoC: mxs-sgtl5000: Drop redundant probe error messages
      https://git.kernel.org/broonie/sound/c/e84c06775d4a

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark