ASoC: SOF: imx: Remove the use of dev_err_probe()

[PATCH] ASoC: SOF: imx: Remove the use of dev_err_probe()

Posted by Xichao Zhao 1 month, 2 weeks ago

The dev_err_probe() doesn't do anything when error is '-ENOMEM'.
Therefore, remove the useless call to dev_err_probe(), and just
return the value instead.

Signed-off-by: Xichao Zhao <zhao.xichao@vivo.com>
---
 sound/soc/sof/imx/imx-common.c | 4 ++--
 sound/soc/sof/imx/imx8.c       | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sof/imx/imx-common.c b/sound/soc/sof/imx/imx-common.c
index f00b381cec3b..4523621cda10 100644
--- a/sound/soc/sof/imx/imx-common.c
+++ b/sound/soc/sof/imx/imx-common.c
@@ -354,8 +354,8 @@ static int imx_probe(struct snd_sof_dev *sdev)
 
 	common = devm_kzalloc(sdev->dev, sizeof(*common), GFP_KERNEL);
 	if (!common)
-		return dev_err_probe(sdev->dev, -ENOMEM,
-				     "failed to allocate common data\n");
+		return -ENOMEM;
+
 	sdev->pdata->hw_pdata = common;
 
 	common->ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp",
diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
index b73dd91bd529..7e9eab2e3034 100644
--- a/sound/soc/sof/imx/imx8.c
+++ b/sound/soc/sof/imx/imx8.c
@@ -171,8 +171,7 @@ static int imx8m_probe(struct snd_sof_dev *sdev)
 
 	chip = devm_kzalloc(sdev->dev, sizeof(*chip), GFP_KERNEL);
 	if (!chip)
-		return dev_err_probe(sdev->dev, -ENOMEM,
-				     "failed to allocate chip data\n");
+		return -ENOMEM;
 
 	chip->dap = devm_ioremap(sdev->dev, IMX8M_DAP_DEBUG, IMX8M_DAP_DEBUG_SIZE);
 	if (!chip->dap)
-- 
2.34.1

Re: [PATCH] ASoC: SOF: imx: Remove the use of dev_err_probe()

Posted by Mark Brown 1 month, 1 week ago

On Tue, 19 Aug 2025 19:26:54 +0800, Xichao Zhao wrote:
> The dev_err_probe() doesn't do anything when error is '-ENOMEM'.
> Therefore, remove the useless call to dev_err_probe(), and just
> return the value instead.
> 
> 

Applied to

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

Thanks!

[1/1] ASoC: SOF: imx: Remove the use of dev_err_probe()
      commit: f840737d1746398c2993be34bfdc80bdc19ecae2

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

Re: [PATCH] ASoC: SOF: imx: Remove the use of dev_err_probe()

Posted by Laurentiu Mihalcea 1 month, 2 weeks ago

On 8/19/2025 2:26 PM, Xichao Zhao wrote:
> The dev_err_probe() doesn't do anything when error is '-ENOMEM'.
> Therefore, remove the useless call to dev_err_probe(), and just
> return the value instead.
>
> Signed-off-by: Xichao Zhao <zhao.xichao@vivo.com>
> ---
>  sound/soc/sof/imx/imx-common.c | 4 ++--
>  sound/soc/sof/imx/imx8.c       | 3 +--
>  2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/sound/soc/sof/imx/imx-common.c b/sound/soc/sof/imx/imx-common.c
> index f00b381cec3b..4523621cda10 100644
> --- a/sound/soc/sof/imx/imx-common.c
> +++ b/sound/soc/sof/imx/imx-common.c
> @@ -354,8 +354,8 @@ static int imx_probe(struct snd_sof_dev *sdev)
>  
>  	common = devm_kzalloc(sdev->dev, sizeof(*common), GFP_KERNEL);
>  	if (!common)
> -		return dev_err_probe(sdev->dev, -ENOMEM,
> -				     "failed to allocate common data\n");
> +		return -ENOMEM;
> +
>  	sdev->pdata->hw_pdata = common;
>  
>  	common->ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp",
> diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
> index b73dd91bd529..7e9eab2e3034 100644
> --- a/sound/soc/sof/imx/imx8.c
> +++ b/sound/soc/sof/imx/imx8.c
> @@ -171,8 +171,7 @@ static int imx8m_probe(struct snd_sof_dev *sdev)
>  
>  	chip = devm_kzalloc(sdev->dev, sizeof(*chip), GFP_KERNEL);
>  	if (!chip)
> -		return dev_err_probe(sdev->dev, -ENOMEM,
> -				     "failed to allocate chip data\n");
> +		return -ENOMEM;
>  
>  	chip->dap = devm_ioremap(sdev->dev, IMX8M_DAP_DEBUG, IMX8M_DAP_DEBUG_SIZE);
>  	if (!chip->dap)


Good stuff, thanks! Maybe it would be worth mentioning in the commit

message that generic messages on OOM errors such as these ones are unneeded

since the kernel is already verbose enough? This seems to be the reason why

dev_err_probe() doesn't do anything on -ENOMEM anyways.


Either way:

Reviewed-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

Re: [PATCH] ASoC: SOF: imx: Remove the use of dev_err_probe()

Posted by Daniel Baluta 1 month, 2 weeks ago

On Tue, Aug 19, 2025 at 2:29 PM Xichao Zhao <zhao.xichao@vivo.com> wrote:
>
> The dev_err_probe() doesn't do anything when error is '-ENOMEM'.
> Therefore, remove the useless call to dev_err_probe(), and just
> return the value instead.
>
> Signed-off-by: Xichao Zhao <zhao.xichao@vivo.com>

NACK on this.

Quote from dev_err_probe documentation:

```
 Using this helper in your probe function is totally fine even if @err
 * is known to never be -EPROBE_DEFER.
 * The benefit compared to a normal dev_err() is the standardized format
 * of the error code, which is emitted symbolically (i.e. you get "EAGAIN"
 * instead of "-35"), and having the error code returned allows more
 * compact error paths.
```

Thanks,
Daniel.

Re: [PATCH] ASoC: SOF: imx: Remove the use of dev_err_probe()

Posted by Laurentiu Mihalcea 1 month, 2 weeks ago

On 8/20/2025 10:04 AM, Daniel Baluta wrote:
> On Tue, Aug 19, 2025 at 2:29 PM Xichao Zhao <zhao.xichao@vivo.com> wrote:
>> The dev_err_probe() doesn't do anything when error is '-ENOMEM'.
>> Therefore, remove the useless call to dev_err_probe(), and just
>> return the value instead.
>>
>> Signed-off-by: Xichao Zhao <zhao.xichao@vivo.com>
> NACK on this.
>
> Quote from dev_err_probe documentation:
>
> ```
>  Using this helper in your probe function is totally fine even if @err
>  * is known to never be -EPROBE_DEFER.
>  * The benefit compared to a normal dev_err() is the standardized format
>  * of the error code, which is emitted symbolically (i.e. you get "EAGAIN"
>  * instead of "-35"), and having the error code returned allows more
>  * compact error paths.
> ```


AFAIK the kernel logs are verbose enough on OOM errors, which makes prints

such as the ones removed by this series unneeded (have a look at [1]). Normally,

you'd get a warning from checkpatch but I think these logs slipped through the

cracks because the error messages do not quite match the expected format?


[1]: https://lkml.org/lkml/2014/6/10/382

Re: [PATCH] ASoC: SOF: imx: Remove the use of dev_err_probe()

Posted by Mark Brown 1 month, 2 weeks ago

On Wed, Aug 20, 2025 at 12:24:40PM +0300, Laurentiu Mihalcea wrote:
> On 8/20/2025 10:04 AM, Daniel Baluta wrote:
> > On Tue, Aug 19, 2025 at 2:29 PM Xichao Zhao <zhao.xichao@vivo.com> wrote:

> >> The dev_err_probe() doesn't do anything when error is '-ENOMEM'.
> >> Therefore, remove the useless call to dev_err_probe(), and just
> >> return the value instead.

> > NACK on this.

> > Quote from dev_err_probe documentation:
> >
> > ```
> >  Using this helper in your probe function is totally fine even if @err

> AFAIK the kernel logs are verbose enough on OOM errors, which makes prints
> 
> such as the ones removed by this series unneeded (have a look at [1]). Normally,
> 
> you'd get a warning from checkpatch but I think these logs slipped through the
> 
> cracks because the error messages do not quite match the expected format?

Indeed, there's generally no need for any additional logging on OOM and
if the code is fixed there's no possibility of deferral.