[PATCH] staging: media: tegra-video: Remove the use of dev_err_probe()

[PATCH] staging: media: tegra-video: Remove the use of dev_err_probe()

[Xichao Zhao]

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>
---
 drivers/staging/media/tegra-video/tegra20.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/tegra-video/tegra20.c b/drivers/staging/media/tegra-video/tegra20.c
index 7b8f8f810b35..ee65e89c119c 100644
--- a/drivers/staging/media/tegra-video/tegra20.c
+++ b/drivers/staging/media/tegra-video/tegra20.c
@@ -255,7 +255,7 @@ static int tegra20_channel_host1x_syncpt_init(struct tegra_vi_channel *chan)
 
 	out_sp = host1x_syncpt_request(&vi->client, HOST1X_SYNCPT_CLIENT_MANAGED);
 	if (!out_sp)
-		return dev_err_probe(vi->dev, -ENOMEM, "failed to request syncpoint\n");
+		return -ENOMEM;
 
 	chan->mw_ack_sp[0] = out_sp;
 
-- 
2.34.1

Re: [PATCH] staging: media: tegra-video: Remove the use of dev_err_probe()

[Luca Ceresoli]

Hello Xichao, Uwe,

+Uwe, author of 2f3cfd2f4b7c ("driver core: Make dev_err_probe() silent
for -ENOMEM").

On Tue, 19 Aug 2025 17:23:30 +0800
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>
> ---
>  drivers/staging/media/tegra-video/tegra20.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/tegra-video/tegra20.c b/drivers/staging/media/tegra-video/tegra20.c
> index 7b8f8f810b35..ee65e89c119c 100644
> --- a/drivers/staging/media/tegra-video/tegra20.c
> +++ b/drivers/staging/media/tegra-video/tegra20.c
> @@ -255,7 +255,7 @@ static int tegra20_channel_host1x_syncpt_init(struct tegra_vi_channel *chan)
>  
>  	out_sp = host1x_syncpt_request(&vi->client, HOST1X_SYNCPT_CLIENT_MANAGED);
>  	if (!out_sp)
> -		return dev_err_probe(vi->dev, -ENOMEM, "failed to request syncpoint\n");
> +		return -ENOMEM;

Thanks for your patch!

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

About dev_err_probe(), I wonder whether it should mention the -ENOMEM
exception in kerneldoc. Is it part of the API "contract" it provides?

It would be good to clarify that, because there are other users of
dev_err_probe(..., -ENOMEM, ...). I counted 80 with a trivial git grep.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] staging: media: tegra-video: Remove the use of dev_err_probe()

[赵西超]

在 2025/8/19 18:28, Luca Ceresoli 写道:
> Hello Xichao, Uwe,
>
> +Uwe, author of 2f3cfd2f4b7c ("driver core: Make dev_err_probe() silent
> for -ENOMEM").
>
> On Tue, 19 Aug 2025 17:23:30 +0800
> 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>
>> ---
>>   drivers/staging/media/tegra-video/tegra20.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/media/tegra-video/tegra20.c b/drivers/staging/media/tegra-video/tegra20.c
>> index 7b8f8f810b35..ee65e89c119c 100644
>> --- a/drivers/staging/media/tegra-video/tegra20.c
>> +++ b/drivers/staging/media/tegra-video/tegra20.c
>> @@ -255,7 +255,7 @@ static int tegra20_channel_host1x_syncpt_init(struct tegra_vi_channel *chan)
>>   
>>   	out_sp = host1x_syncpt_request(&vi->client, HOST1X_SYNCPT_CLIENT_MANAGED);
>>   	if (!out_sp)
>> -		return dev_err_probe(vi->dev, -ENOMEM, "failed to request syncpoint\n");
>> +		return -ENOMEM;
> Thanks for your patch!
>
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>
> About dev_err_probe(), I wonder whether it should mention the -ENOMEM
> exception in kerneldoc. Is it part of the API "contract" it provides?
>
> It would be good to clarify that, because there are other users of
> dev_err_probe(..., -ENOMEM, ...). I counted 80 with a trivial git grep.
>
> Luca
>
Yes, I agree with your point that the -ENOMEM exception should be 
mentioned in the kerneldoc.

This can help other developers. I am currently cleaning up other code 
that uses dev_err_probe(..., -ENOMEM, ...).

Best regards,

Xichao Zhao

Re: [PATCH] staging: media: tegra-video: Remove the use of dev_err_probe()

[Uwe Kleine-König]

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

Hello,

On Tue, Aug 19, 2025 at 12:28:30PM +0200, Luca Ceresoli wrote:
> +Uwe, author of 2f3cfd2f4b7c ("driver core: Make dev_err_probe() silent
> for -ENOMEM").

The main purpose of that commit was to make dev_err_probe() silent if
the error code might be ENOMEM. If it's sensible to call dev_err_probe()
if you know that the error code is ENOMEM is subjective. On one hand
it's explicitly a noop then, but on the other hand it might serve as
code documentation and make it explicit that error messaging is
consistent.

There was also a recent discussion about:

	ret = devm_add_action(...)
	if (ret)
		return dev_err_probe(..., ret, ...);

which is a case of "ret can only be 0 or -ENOMEM", but that's harder to
spot than the patched code below.
		
> On Tue, 19 Aug 2025 17:23:30 +0800
> 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>
> > ---
> >  drivers/staging/media/tegra-video/tegra20.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/media/tegra-video/tegra20.c b/drivers/staging/media/tegra-video/tegra20.c
> > index 7b8f8f810b35..ee65e89c119c 100644
> > --- a/drivers/staging/media/tegra-video/tegra20.c
> > +++ b/drivers/staging/media/tegra-video/tegra20.c
> > @@ -255,7 +255,7 @@ static int tegra20_channel_host1x_syncpt_init(struct tegra_vi_channel *chan)
> >  
> >  	out_sp = host1x_syncpt_request(&vi->client, HOST1X_SYNCPT_CLIENT_MANAGED);
> >  	if (!out_sp)
> > -		return dev_err_probe(vi->dev, -ENOMEM, "failed to request syncpoint\n");
> > +		return -ENOMEM;
> 
> Thanks for your patch!
> 
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> About dev_err_probe(), I wonder whether it should mention the -ENOMEM
> exception in kerneldoc. Is it part of the API "contract" it provides?

In general you're not supposed to wail about -ENOMEM because such a
problem is already "loud". I claim it's fixed that dev_err_probe() is
silent for -ENOMEM. If you consider it helpful to add it to the
documentation, add it.

In this case I agree that dropping dev_err_probe() is sane. In the
devm_add_action() case above I tend to keep it.

Best regards
Uwe

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