On Tue, Oct 13, 2026 at 03:11:51PM -0500, Alex Elder wrote:

On 10/12/26 9:24 PM, pavankumaryalagada@gmail.com wrote:

...

From: Yalagada Pavan Kumar pavankumaryalagada@gmail.com

Use %pe to print error pointers directly and display their error names.

Keep the other PTR_ERR() call because its return value is needed for ret.

Signed-off-by: Yalagada Pavan Kumar pavankumaryalagada@gmail.com

Generally I think what you're doing is OK. It looks like %pe is better than just %d, because it prints the error *name* for the pointer-coded error.

But I have two general questions about this effort:

• There are other places in the Greybus code (even in just the staging source files) where this exact same thing could be done. Why limit this to "fw-core.c" only?

I initially limited the change to "fw-core.c" because I was focusing on the error messages in that file.

• Do you think that all Greybus error messages should be converted to use %pe (passing ERR_PTR() if necessary) instead of %d for showing the error number?

Using %pe prints the error name instead of just the error number, which makes the error messages easier to understand.

For example, the current message is:

failed to create download connection (-12)

with %pe, it becomes:

failed to create download connection (-ENOMEM)

I think the second message is easier to understand because it shows the error name instead of just error number.

I also found other PTR_ERR() calls in Greybus. some of them are used to return error codes and should remain unchanged.

For example, in fw-core.c:

if (IS_ERR(connection)) { ret = PTR_ERR(connection); dev_err(&bundle->dev, "failed to create management connection (%pe)\n", connection); goto err_destroy_connections; }

Here, PTR_ERR(connection) is needed to store the error code in ret for the error-handling path, while passing connection to %pe makes the error message more readable.

I think your suggested change is a net positive, but doing it in some spots but not others means it is adding a bit of inconsistency.

I agree that similar cases elsewhere in Greybus are worth checking to keep error reporting consistent. I will look at the other Greybus files for cases where using %pe would be appropriate.

Would you prefer me to extend this change to the other relevant Greybus files, or keep this patch focused on fw-core.c and submit a separate patch for the remaining changes?

Thanks for the suggestions.

Pavan

			-Alex

...

---

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/fw-core.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/greybus/fw-core.c b/drivers/staging/greybus/fw-core.c index 2016a74f137f..78321be80013 100644 --- a/drivers/staging/greybus/fw-core.c +++ b/drivers/staging/greybus/fw-core.c @@ -93,8 +93,8 @@ static int gb_fw_core_probe(struct gb_bundle *bundle, if (IS_ERR(connection)) { ret = PTR_ERR(connection); dev_err(&bundle->dev,

• 			"failed to create management connection (%d)\n",
  

• 			ret);
  

• 			"failed to create management connection (%pe)\n",
  

• 			connection);
  	goto err_destroy_connections;
  }
  

@@ -112,8 +112,8 @@ static int gb_fw_core_probe(struct gb_bundle *bundle, connection = gb_connection_create(bundle, cport_id, gb_fw_download_request_handler); if (IS_ERR(connection)) {

• 		dev_err(&bundle->dev, "failed to create download connection (%ld)\n",
  

• 			PTR_ERR(connection));
  

• 		dev_err(&bundle->dev, "failed to create download connection (%pe)\n",
  

• 			connection);
  } else {
  	fw_core->download_connection = connection;
  }
  

@@ -131,8 +131,8 @@ static int gb_fw_core_probe(struct gb_bundle *bundle, connection = gb_connection_create(bundle, cport_id, NULL); if (IS_ERR(connection)) {

• 		dev_err(&bundle->dev, "failed to create SPI connection (%ld)\n",
  

• 			PTR_ERR(connection));
  

• 		dev_err(&bundle->dev, "failed to create SPI connection (%pe)\n",
  

• 			connection);
  } else {
  	fw_core->spi_connection = connection;
  }
  

@@ -149,8 +149,8 @@ static int gb_fw_core_probe(struct gb_bundle *bundle, connection = gb_connection_create(bundle, cport_id, NULL); if (IS_ERR(connection)) {

• 		dev_err(&bundle->dev, "failed to create Authentication connection (%ld)\n",
  

• 			PTR_ERR(connection));
  

• 		dev_err(&bundle->dev, "failed to create Authentication connection (%pe)\n",
  

• 			connection);
  } else {
  	fw_core->cap_connection = connection;
  }