[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

[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

[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 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 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 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
>
>