From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
To: Julia Lawall <julia.lawall@inria.fr>
Cc: johan@kernel.org, gregkh@linuxfoundation.org, vireshk@kernel.org,
elder@kernel.org, greybus-dev@lists.linaro.org,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
outreachy@lists.linux.dev
Subject: Re: [PATCH v3] staging: greybus: replace PTR_ERR() with %pe in error messages
Date: Thu, 15 Oct 2026 10:23:28 +0530 [thread overview]
Message-ID: <atBcSEXZzZ0pryDg@user> (raw)
In-Reply-To: <c42bf562-913d-c449-4c7a-5f67bd08f9d@inria.fr>
On Wed, Oct 14, 2026 at 10:21:22PM -0400, Julia Lawall wrote:
>
>
> On Thu, 15 Oct 2026, pavankumaryalagada@gmail.com wrote:
>
> > From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> >
> > Use %pe to print error pointers directly and display their
> > error names.
> >
> > 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>
> > ---
> > 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/arche-apb-ctrl.c | 18 +++++++++---------
> > drivers/staging/greybus/arche-platform.c | 8 ++++----
> > drivers/staging/greybus/fw-core.c | 16 ++++++++--------
> > drivers/staging/greybus/uart.c | 2 +-
> > 4 files changed, 22 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/staging/greybus/arche-apb-ctrl.c b/drivers/staging/greybus/arche-apb-ctrl.c
> > index 4302c2226126..fa4d2d88dc3f 100644
> > --- a/drivers/staging/greybus/arche-apb-ctrl.c
> > +++ b/drivers/staging/greybus/arche-apb-ctrl.c
> > @@ -145,8 +145,8 @@ static int fw_flashing_seq(struct platform_device *pdev)
> > apb->spi_en = devm_gpiod_get(dev, "spi-en", flags);
> > if (IS_ERR(apb->spi_en)) {
> > ret = PTR_ERR(apb->spi_en);
> > - dev_err(dev, "Failed requesting SPI bus en GPIO: %d\n",
> > - ret);
> > + dev_err(dev, "Failed requesting SPI bus en GPIO: %pe\n",
> > + apb->spi_en);
> > return ret;
>
> Maybe it's for after this series gets accepted, but you don't really need
> to initialize ret here. You can just return PTR_ERR(apb->spi_en).
I'll return PTR_ERR() directly where possible and keep ret assignments
where the value is needed for further error handling.
>
> You can also check whether the ret variable is needed at all in this
> function (I don't know).
I'll keep ret in fw_flashing_seq() since it's used for regulator operations,
and remove it from apb_ctrl_get_fw_data() since it's only used for returning
error codes.
Thanks,
Pavan
>
> > }
> > }
> > @@ -323,14 +323,14 @@ static int apb_ctrl_get_fw_data(struct platform_device *pdev,
> > apb->resetn = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > if (IS_ERR(apb->resetn)) {
> > ret = PTR_ERR(apb->resetn);
> > - dev_err(dev, "Failed requesting reset GPIO: %d\n", ret);
> > + dev_err(dev, "Failed requesting reset GPIO: %pe\n", apb->resetn);
> > return ret;
> > }
> >
> > apb->boot_ret = devm_gpiod_get(dev, "boot-ret", GPIOD_OUT_LOW);
> > if (IS_ERR(apb->boot_ret)) {
> > ret = PTR_ERR(apb->boot_ret);
> > - dev_err(dev, "Failed requesting bootret GPIO: %d\n", ret);
> > + dev_err(dev, "Failed requesting bootret GPIO: %pe\n", apb->boot_ret);
> > return ret;
> > }
>
> Same here and in the subsequent ones.
>
> > @@ -338,7 +338,7 @@ static int apb_ctrl_get_fw_data(struct platform_device *pdev,
> > apb->pwroff = devm_gpiod_get_optional(dev, "pwr-off", GPIOD_IN);
> > if (IS_ERR(apb->pwroff)) {
> > ret = PTR_ERR(apb->pwroff);
> > - dev_err(dev, "Failed requesting pwroff_n GPIO: %d\n", ret);
> > + dev_err(dev, "Failed requesting pwroff_n GPIO: %pe\n", apb->pwroff);
> > return ret;
> > }
> >
> > @@ -346,14 +346,14 @@ static int apb_ctrl_get_fw_data(struct platform_device *pdev,
> > apb->clk_en = devm_gpiod_get_optional(dev, "clock-en", GPIOD_OUT_LOW);
> > if (IS_ERR(apb->clk_en)) {
> > ret = PTR_ERR(apb->clk_en);
> > - dev_err(dev, "Failed requesting APB clock en GPIO: %d\n", ret);
> > + dev_err(dev, "Failed requesting APB clock en GPIO: %pe\n", apb->clk_en);
> > return ret;
> > }
> >
> > apb->pwrdn = devm_gpiod_get(dev, "pwr-down", GPIOD_OUT_LOW);
> > if (IS_ERR(apb->pwrdn)) {
> > ret = PTR_ERR(apb->pwrdn);
> > - dev_warn(dev, "Failed requesting power down GPIO: %d\n", ret);
> > + dev_warn(dev, "Failed requesting power down GPIO: %pe\n", apb->pwrdn);
> > return ret;
> > }
> >
> > @@ -368,12 +368,12 @@ static int apb_ctrl_get_fw_data(struct platform_device *pdev,
> >
> > apb->pinctrl = devm_pinctrl_get(&pdev->dev);
> > if (IS_ERR(apb->pinctrl)) {
> > - dev_err(&pdev->dev, "could not get pinctrl handle\n");
> > + dev_err(&pdev->dev, "could not get pinctrl handle: %pe\n", apb->pinctrl);
> > return PTR_ERR(apb->pinctrl);
> > }
> > apb->pin_default = pinctrl_lookup_state(apb->pinctrl, "default");
> > if (IS_ERR(apb->pin_default)) {
> > - dev_err(&pdev->dev, "could not get default pin state\n");
> > + dev_err(&pdev->dev, "could not get default pin state: %pe\n", apb->pin_default);
> > return PTR_ERR(apb->pin_default);
> > }
>
> These two are different from the others.
>
> julia
>
> > diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c
> > index de5de59ea8ab..a97ac59c6f01 100644
> > --- a/drivers/staging/greybus/arche-platform.c
> > +++ b/drivers/staging/greybus/arche-platform.c
> > @@ -445,7 +445,7 @@ static int arche_platform_probe(struct platform_device *pdev)
> > arche_pdata->svc_reset = devm_gpiod_get(dev, "svc,reset", flags);
> > if (IS_ERR(arche_pdata->svc_reset)) {
> > ret = PTR_ERR(arche_pdata->svc_reset);
> > - dev_err(dev, "failed to request svc-reset GPIO: %d\n", ret);
> > + dev_err(dev, "failed to request svc-reset GPIO: %pe\n", arche_pdata->svc_reset);
> > return ret;
> > }
> > arche_platform_set_state(arche_pdata, ARCHE_PLATFORM_STATE_OFF);
> > @@ -454,7 +454,7 @@ static int arche_platform_probe(struct platform_device *pdev)
> > GPIOD_OUT_LOW);
> > if (IS_ERR(arche_pdata->svc_sysboot)) {
> > ret = PTR_ERR(arche_pdata->svc_sysboot);
> > - dev_err(dev, "failed to request sysboot0 GPIO: %d\n", ret);
> > + dev_err(dev, "failed to request sysboot0 GPIO: %pe\n", arche_pdata->svc_sysboot);
> > return ret;
> > }
> >
> > @@ -471,7 +471,7 @@ static int arche_platform_probe(struct platform_device *pdev)
> > arche_pdata->svc_ref_clk = devm_clk_get(dev, "svc_ref_clk");
> > if (IS_ERR(arche_pdata->svc_ref_clk)) {
> > ret = PTR_ERR(arche_pdata->svc_ref_clk);
> > - dev_err(dev, "failed to get svc_ref_clk: %d\n", ret);
> > + dev_err(dev, "failed to get svc_ref_clk: %pe\n", arche_pdata->svc_ref_clk);
> > return ret;
> > }
> >
> > @@ -484,7 +484,7 @@ static int arche_platform_probe(struct platform_device *pdev)
> > GPIOD_IN);
> > if (IS_ERR(arche_pdata->wake_detect)) {
> > ret = PTR_ERR(arche_pdata->wake_detect);
> > - dev_err(dev, "Failed requesting wake_detect GPIO: %d\n", ret);
> > + dev_err(dev, "Failed requesting wake_detect GPIO: %pe\n", arche_pdata->wake_detect);
> > return ret;
> > }
> >
> > 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;
> > }
> > diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
> > index 172cf900eaf0..59a112cf47a0 100644
> > --- a/drivers/staging/greybus/uart.c
> > +++ b/drivers/staging/greybus/uart.c
> > @@ -951,7 +951,7 @@ static int gb_tty_init(void)
> > TTY_DRIVER_DYNAMIC_DEV |
> > TTY_DRIVER_RESET_SAVED_TERMIOS);
> > 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
> >
> >
> >
prev parent reply other threads:[~2026-10-15 4:53 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-15 1:56 [PATCH v3] staging: greybus: replace PTR_ERR() with %pe in error messages pavankumaryalagada
2026-10-15 2:21 ` Julia Lawall
2026-10-15 4:53 ` Yalagada Pavan Kumar [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=atBcSEXZzZ0pryDg@user \
--to=pavankumaryalagada@gmail.com \
--cc=elder@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=greybus-dev@lists.linaro.org \
--cc=johan@kernel.org \
--cc=julia.lawall@inria.fr \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=outreachy@lists.linux.dev \
--cc=vireshk@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.