[PATCH v5] staging: greybus: use %pe in error messages

[PATCH v5] staging: greybus: use %pe in error messages

[pavankumaryalagada]

From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>

Use %pe to print the error pointer directly and display its
error name.

Keep PTR_ERR() calls when the numeric error value is needed
for returning the error code.

Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
Reviewed-by: Alex Elder <elder@riscstar.com>
---
v5:
- Drop arche-apb-ctrl.c and arche-platform.c changes because the
  files were removed.
- keep only the uart.c change, as the fw-core.c changes are already
  present in staging/staging-next.

v4:
- Return error codes directly instead of storing them in ret,
  as suggested by Julia Lawall.
- Remove the unused ret variable.

v3:
- Extend the %pe conversion throughout Greybus, as suggested by Alex Elder.

v2:
- Use %pe in the firmware management case, as suggested by Jeremy Clifton,
  to match the other cases.

v1: https://lore.kernel.org/outreachy/as0nhVbagZa8Birm@user/T/#t
---
 drivers/staging/greybus/uart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
index d47f00751e7a..74c9898c0eed 100644
--- a/drivers/staging/greybus/uart.c
+++ b/drivers/staging/greybus/uart.c
@@ -956,7 +956,7 @@ static int gb_tty_init(void)
 	gb_tty_driver = tty_alloc_driver(GB_NUM_MINORS, TTY_DRIVER_REAL_RAW |
 					 TTY_DRIVER_DYNAMIC_DEV);
 	if (IS_ERR(gb_tty_driver)) {
-		pr_err("Can not allocate tty driver\n");
+		pr_err("Can not allocate tty driver: %pe\n", gb_tty_driver);
 		retval = PTR_ERR(gb_tty_driver);
 		goto fail_unregister_dev;
 	}
-- 
2.43.0

Re: [PATCH v5] staging: greybus: use %pe in error messages

[Johan Hovold]

On Sun, Oct 18, 2026 at 02:19:25PM +0530, pavankumaryalagada@gmail.com wrote:
> From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> 
> Use %pe to print the error pointer directly and display its
> error name.
> 
> Keep PTR_ERR() calls when the numeric error value is needed
> for returning the error code.
> 
> Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> Reviewed-by: Alex Elder <elder@riscstar.com>
> ---
> v5:
> - Drop arche-apb-ctrl.c and arche-platform.c changes because the
>   files were removed.
> - keep only the uart.c change, as the fw-core.c changes are already
>   present in staging/staging-next.

The commit summary (Subject) is a bit misleading now.

> diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
> index d47f00751e7a..74c9898c0eed 100644
> --- a/drivers/staging/greybus/uart.c
> +++ b/drivers/staging/greybus/uart.c
> @@ -956,7 +956,7 @@ static int gb_tty_init(void)
>  	gb_tty_driver = tty_alloc_driver(GB_NUM_MINORS, TTY_DRIVER_REAL_RAW |
>  					 TTY_DRIVER_DYNAMIC_DEV);
>  	if (IS_ERR(gb_tty_driver)) {
> -		pr_err("Can not allocate tty driver\n");
> +		pr_err("Can not allocate tty driver: %pe\n", gb_tty_driver);

There's really no need to print the errno here as it will always be
-ENOMEM if we ever hit it. You could even remove the error message
completely.

>  		retval = PTR_ERR(gb_tty_driver);
>  		goto fail_unregister_dev;
>  	}

Johan

Re: [PATCH v5] staging: greybus: use %pe in error messages

[Yalagada Pavan Kumar]

On Mon, Oct 19, 2026 at 09:16:14AM +0200, Johan Hovold wrote:
> On Sun, Oct 18, 2026 at 02:19:25PM +0530, pavankumaryalagada@gmail.com wrote:
> > From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> > 
> > Use %pe to print the error pointer directly and display its
> > error name.
> > 
> > Keep PTR_ERR() calls when the numeric error value is needed
> > for returning the error code.
> > 
> > Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> > Reviewed-by: Alex Elder <elder@riscstar.com>
> > ---
> > v5:
> > - Drop arche-apb-ctrl.c and arche-platform.c changes because the
> >   files were removed.
> > - keep only the uart.c change, as the fw-core.c changes are already
> >   present in staging/staging-next.
> 
> The commit summary (Subject) is a bit misleading now.
> 
> > diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
> > index d47f00751e7a..74c9898c0eed 100644
> > --- a/drivers/staging/greybus/uart.c
> > +++ b/drivers/staging/greybus/uart.c
> > @@ -956,7 +956,7 @@ static int gb_tty_init(void)
> >  	gb_tty_driver = tty_alloc_driver(GB_NUM_MINORS, TTY_DRIVER_REAL_RAW |
> >  					 TTY_DRIVER_DYNAMIC_DEV);
> >  	if (IS_ERR(gb_tty_driver)) {
> > -		pr_err("Can not allocate tty driver\n");
> > +		pr_err("Can not allocate tty driver: %pe\n", gb_tty_driver);
> 
> There's really no need to print the errno here as it will always be
> -ENOMEM if we ever hit it. You could even remove the error message
> completely.

Since the error message is no longer needed, I will remove it in a separate
patch rather than in this revision.

Thanks,
Pavan

> 
> >  		retval = PTR_ERR(gb_tty_driver);
> >  		goto fail_unregister_dev;
> >  	}
> 
> Johan

Re: [PATCH v5] staging: greybus: use %pe in error messages

[Alex Elder]

On 10/19/26 4:16 AM, Yalagada Pavan Kumar wrote:
> On Mon, Oct 19, 2026 at 09:16:14AM +0200, Johan Hovold wrote:
>> On Sun, Oct 18, 2026 at 02:19:25PM +0530, pavankumaryalagada@gmail.com wrote:
>>> From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
>>>
>>> Use %pe to print the error pointer directly and display its
>>> error name.
>>>
>>> Keep PTR_ERR() calls when the numeric error value is needed
>>> for returning the error code.
>>>
>>> Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
>>> Reviewed-by: Alex Elder <elder@riscstar.com>
>>> ---
>>> v5:
>>> - Drop arche-apb-ctrl.c and arche-platform.c changes because the
>>>    files were removed.
>>> - keep only the uart.c change, as the fw-core.c changes are already
>>>    present in staging/staging-next.
>>
>> The commit summary (Subject) is a bit misleading now.
>>
>>> diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
>>> index d47f00751e7a..74c9898c0eed 100644
>>> --- a/drivers/staging/greybus/uart.c
>>> +++ b/drivers/staging/greybus/uart.c
>>> @@ -956,7 +956,7 @@ static int gb_tty_init(void)
>>>   	gb_tty_driver = tty_alloc_driver(GB_NUM_MINORS, TTY_DRIVER_REAL_RAW |
>>>   					 TTY_DRIVER_DYNAMIC_DEV);
>>>   	if (IS_ERR(gb_tty_driver)) {
>>> -		pr_err("Can not allocate tty driver\n");
>>> +		pr_err("Can not allocate tty driver: %pe\n", gb_tty_driver);
>>
>> There's really no need to print the errno here as it will always be
>> -ENOMEM if we ever hit it. You could even remove the error message
>> completely.
> 
> Since the error message is no longer needed, I will remove it in a separate
> patch rather than in this revision.

So doesn't this mean that your original patch can change from
adding %pe to a single spot, to removing the error message
entirely?

If so, I would prefer you just do version 6 of the patch that
does exactly that.  And if you do that, you can preserve my
Reviewed-by tag.

					-Alex>
> Thanks,
> Pavan
> 
>>
>>>   		retval = PTR_ERR(gb_tty_driver);
>>>   		goto fail_unregister_dev;
>>>   	}
>>
>> Johan