Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH] [RFC] gpiolib: introduce gpio_name() helper
@ 2026-06-29 13:56 Arnd Bergmann
  2026-06-29 14:10 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Arnd Bergmann @ 2026-06-29 13:56 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski
  Cc: Arnd Bergmann, Marcel Holtmann, MyungJoo Ham, Chanwoo Choi,
	Geert Uytterhoeven, Andy Shevchenko, Dmitry Torokhov, Ulf Hansson,
	linux-bluetooth, linux-kernel, linux-gpio, dri-devel, linux-i2c,
	linux-iio, linux-input, linux-mmc, linux-arm-kernel, linux-pm,
	linux-usb

From: Arnd Bergmann <arnd@arndb.de>

Most remaining users of desc_to_gpio() only call it for printing debug
information.

Replace this with a new gpiod_name() helper that returns the
gpio_desc->name string after checking the gpio_desc pointer.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---

Not sure if this the way we want to take this, or if the gpio name is
an appropriate replacement in debug printk.

Since most of the callers of desc_to_gpio() and gpio_to_desc() are
otherwise in drivers that already depend on CONFIG_GPIOLIB_LEGACY and
include linux/gpio/legacy.h, only a handful of instances remain that
are otherwise in files that otherwise only use the descriptor interfaces:

arch/arm/mach-pxa/pxa27x.c:	reset_gpio = desc_to_gpio(gpiod);
arch/arm/plat-orion/gpio.c:	unsigned gpio = desc_to_gpio(desc);
drivers/gpio/gpio-nomadik.c:		mode = nmk_prcm_gpiocr_get_mode(pctldev, desc_to_gpio(desc));
drivers/gpio/gpiolib-acpi-core.c:			desc = gpio_to_desc(agpio->pin_table[pin_index]);
drivers/gpio/gpiolib-cdev.c:	hte_init_line_attr(hdesc, desc_to_gpio(line->desc), flags, NULL,
drivers/gpio/gpiolib-sysfs.c:	desc = gpio_to_desc(gpio);
drivers/gpio/gpiolib-sysfs.c:						   desc_to_gpio(desc));
drivers/gpio/gpiolib.c:	trace_gpio_direction(desc_to_gpio(desc), 1, ret);
drivers/input/misc/soc_button_array.c:	*gpio_ret = desc_to_gpio(desc);
drivers/pinctrl/core.c:			gdev = gpiod_to_gpio_device(gpio_to_desc(gpio_num));
drivers/platform/x86/x86-android-tablets/core.c: * 2. Calling desc_to_gpio() to get an old style GPIO number for gpio-keys
drivers/soc/fsl/qe/gpio.c:	gpio_num = desc_to_gpio(gpiod);
---
 drivers/bluetooth/hci_intel.c             |  4 ++--
 drivers/extcon/extcon-rtk-type-c.c        |  4 ++--
 drivers/gpio/gpio-aggregator.c            |  4 ++--
 drivers/gpio/gpiolib.c                    | 13 +++++++++++++
 drivers/gpu/drm/bridge/analogix/anx7625.c |  6 +++---
 drivers/i2c/busses/i2c-gpio.c             |  4 ++--
 drivers/iio/accel/mma9551.c               |  4 ++--
 drivers/iio/humidity/dht11.c              |  2 +-
 drivers/input/touchscreen/edt-ft5x06.c    |  6 +++---
 drivers/input/touchscreen/hycon-hy46xx.c  |  5 ++---
 drivers/mmc/host/atmel-mci.c              |  8 ++++----
 drivers/power/supply/bq24257_charger.c    |  2 +-
 drivers/usb/gadget/udc/at91_udc.c         |  4 ++--
 include/linux/gpio/consumer.h             |  8 ++++++++
 14 files changed, 47 insertions(+), 27 deletions(-)

diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
index c31105b91e47..2e6ebc152bcb 100644
--- a/drivers/bluetooth/hci_intel.c
+++ b/drivers/bluetooth/hci_intel.c
@@ -1176,8 +1176,8 @@ static int intel_probe(struct platform_device *pdev)
 	list_add_tail(&idev->list, &intel_device_list);
 	mutex_unlock(&intel_device_list_lock);
 
-	dev_info(&pdev->dev, "registered, gpio(%d)/irq(%d).\n",
-		 desc_to_gpio(idev->reset), idev->irq);
+	dev_info(&pdev->dev, "registered, gpio(%s)/irq(%d).\n",
+		 gpiod_name(idev->reset), idev->irq);
 
 	return 0;
 }
diff --git a/drivers/extcon/extcon-rtk-type-c.c b/drivers/extcon/extcon-rtk-type-c.c
index 82b60b927e41..fb57e9d7ddb6 100644
--- a/drivers/extcon/extcon-rtk-type-c.c
+++ b/drivers/extcon/extcon-rtk-type-c.c
@@ -1356,8 +1356,8 @@ static int extcon_rtk_type_c_probe(struct platform_device *pdev)
 				(int)PTR_ERR(gpio));
 		} else {
 			type_c->rd_ctrl_gpio_desc = gpio;
-			dev_dbg(dev, "%s get rd-ctrl-gpios (id=%d) OK\n",
-				__func__, desc_to_gpio(gpio));
+			dev_dbg(dev, "%s get rd-ctrl-gpios (id=%s) OK\n",
+				__func__, gpiod_name(gpio));
 		}
 	}
 
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index bc6699a821ee..27df680fbdbb 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -758,8 +758,8 @@ int gpiochip_fwd_desc_add(struct gpiochip_fwd *fwd, struct gpio_desc *desc,
 
 	fwd->descs[offset] = desc;
 
-	dev_dbg(chip->parent, "%u => gpio %d irq %d\n", offset,
-		desc_to_gpio(desc), gpiod_to_irq(desc));
+	dev_dbg(chip->parent, "%u => gpio %s irq %d\n", offset,
+		gpiod_name(desc), gpiod_to_irq(desc));
 
 	return 0;
 }
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 1f498d6c8c68..00de24db74a5 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4122,6 +4122,19 @@ int gpiod_set_consumer_name(struct gpio_desc *desc, const char *name)
 }
 EXPORT_SYMBOL_GPL(gpiod_set_consumer_name);
 
+/**
+ * gpiod_name() - get a name to print for a gpio descriptor
+ * @desc: gpio or NULL pointer to query
+ *
+ * Returns:
+ * The desc->name field or a dummy string for unknown GPIOs.
+ */
+const char *gpiod_name(const struct gpio_desc *desc)
+{
+	return desc ? desc->name : "(no gpio)";
+}
+EXPORT_SYMBOL_GPL(gpiod_name);
+
 /**
  * gpiod_is_shared() - check if this GPIO can be shared by multiple consumers
  * @desc: GPIO to inspect
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index fffcd6154c71..5dce097d4045 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1448,9 +1448,9 @@ static void anx7625_init_gpio(struct anx7625_data *platform)
 
 	if (platform->pdata.gpio_p_on && platform->pdata.gpio_reset) {
 		platform->pdata.low_power_mode = 1;
-		DRM_DEV_DEBUG_DRIVER(dev, "low power mode, pon %d, reset %d.\n",
-				     desc_to_gpio(platform->pdata.gpio_p_on),
-				     desc_to_gpio(platform->pdata.gpio_reset));
+		DRM_DEV_DEBUG_DRIVER(dev, "low power mode, pon %s, reset %s.\n",
+				     gpiod_name(platform->pdata.gpio_p_on),
+				     gpiod_name(platform->pdata.gpio_reset));
 	} else {
 		platform->pdata.low_power_mode = 0;
 		DRM_DEV_DEBUG_DRIVER(dev, "not low power mode.\n");
diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index f4355b17bfbf..4c320a833d9e 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -440,8 +440,8 @@ static int i2c_gpio_probe(struct platform_device *pdev)
 	 * get accessors to get the actual name of the GPIO line,
 	 * from the descriptor, then provide that instead.
 	 */
-	dev_info(dev, "using lines %u (SDA) and %u (SCL%s)\n",
-		 desc_to_gpio(priv->sda), desc_to_gpio(priv->scl),
+	dev_info(dev, "using lines %s (SDA) and %s (SCL%s)\n",
+		 gpiod_name(priv->sda), gpiod_name(priv->scl),
 		 pdata->scl_is_output_only
 		 ? ", no clock stretching" : "");
 
diff --git a/drivers/iio/accel/mma9551.c b/drivers/iio/accel/mma9551.c
index 020370b0ec07..b9d1fc3caf83 100644
--- a/drivers/iio/accel/mma9551.c
+++ b/drivers/iio/accel/mma9551.c
@@ -426,8 +426,8 @@ static int mma9551_gpio_probe(struct iio_dev *indio_dev)
 			return ret;
 		}
 
-		dev_dbg(dev, "gpio resource, no:%d irq:%d\n",
-			desc_to_gpio(gpio), data->irqs[i]);
+		dev_dbg(dev, "gpio resource, no:%s irq:%d\n",
+			gpiod_name(gpio), data->irqs[i]);
 	}
 
 	return 0;
diff --git a/drivers/iio/humidity/dht11.c b/drivers/iio/humidity/dht11.c
index 980cb946bbf7..ca6b8c53e462 100644
--- a/drivers/iio/humidity/dht11.c
+++ b/drivers/iio/humidity/dht11.c
@@ -305,7 +305,7 @@ static int dht11_probe(struct platform_device *pdev)
 
 	dht11->irq = gpiod_to_irq(dht11->gpiod);
 	if (dht11->irq < 0) {
-		dev_err(dev, "GPIO %d has no interrupt\n", desc_to_gpio(dht11->gpiod));
+		dev_err(dev, "GPIO %s has no interrupt\n", gpiod_name(dht11->gpiod));
 		return -EINVAL;
 	}
 
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index d3b1177185a3..2d31c77614b0 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1326,10 +1326,10 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client)
 	edt_ft5x06_ts_prepare_debugfs(tsdata);
 
 	dev_dbg(&client->dev,
-		"EDT FT5x06 initialized: IRQ %d, WAKE pin %d, Reset pin %d.\n",
+		"EDT FT5x06 initialized: IRQ %d, WAKE pin %s, Reset pin %s.\n",
 		client->irq,
-		tsdata->wake_gpio ? desc_to_gpio(tsdata->wake_gpio) : -1,
-		tsdata->reset_gpio ? desc_to_gpio(tsdata->reset_gpio) : -1);
+		gpiod_name(tsdata->wake_gpio),
+		gpiod_name(tsdata->reset_gpio));
 
 	return 0;
 }
diff --git a/drivers/input/touchscreen/hycon-hy46xx.c b/drivers/input/touchscreen/hycon-hy46xx.c
index 1513f20cbf51..797667c5dd99 100644
--- a/drivers/input/touchscreen/hycon-hy46xx.c
+++ b/drivers/input/touchscreen/hycon-hy46xx.c
@@ -528,9 +528,8 @@ static int hycon_hy46xx_probe(struct i2c_client *client)
 		return error;
 
 	dev_dbg(&client->dev,
-		"HYCON HY46XX initialized: IRQ %d, Reset pin %d.\n",
-		client->irq,
-		tsdata->reset_gpio ? desc_to_gpio(tsdata->reset_gpio) : -1);
+		"HYCON HY46XX initialized: IRQ %d, Reset pin %s.\n",
+		client->irq, gpiod_name(tsdata->reset_gpio));
 
 	return 0;
 }
diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 3b4928f5b9b2..b21820564315 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -2255,11 +2255,11 @@ static int atmci_init_slot(struct atmel_mci *host,
 	slot->sdio_irq = sdio_irq;
 
 	dev_dbg(&mmc->class_dev,
-	        "slot[%u]: bus_width=%u, detect_pin=%d, "
-		"detect_is_active_high=%s, wp_pin=%d\n",
-		id, slot_data->bus_width, desc_to_gpio(slot_data->detect_pin),
+	        "slot[%u]: bus_width=%u, detect_pin=%s, "
+		"detect_is_active_high=%s, wp_pin=%s\n",
+		id, slot_data->bus_width, gpiod_name(slot_data->detect_pin),
 		str_true_false(!gpiod_is_active_low(slot_data->detect_pin)),
-		desc_to_gpio(slot_data->wp_pin));
+		gpiod_name(slot_data->wp_pin));
 
 	mmc->ops = &atmci_ops;
 	mmc->f_min = DIV_ROUND_UP(host->bus_hz, 512);
diff --git a/drivers/power/supply/bq24257_charger.c b/drivers/power/supply/bq24257_charger.c
index 72f1bfea8d54..b756bab74eec 100644
--- a/drivers/power/supply/bq24257_charger.c
+++ b/drivers/power/supply/bq24257_charger.c
@@ -868,7 +868,7 @@ static void bq24257_pg_gpio_probe(struct bq24257_device *bq)
 	}
 
 	if (bq->pg)
-		dev_dbg(bq->dev, "probed PG pin = %d\n", desc_to_gpio(bq->pg));
+		dev_dbg(bq->dev, "probed PG pin = %s\n", gpiod_name(bq->pg));
 }
 
 static int bq24257_fw_probe(struct bq24257_device *bq)
diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
index 5aa360ba4f03..099313604387 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -1896,8 +1896,8 @@ static int at91udc_probe(struct platform_device *pdev)
 					gpiod_to_irq(udc->board.vbus_pin),
 					at91_vbus_irq, 0, driver_name, udc);
 			if (retval) {
-				DBG("request vbus irq %d failed\n",
-				    desc_to_gpio(udc->board.vbus_pin));
+				DBG("request vbus irq %s failed\n",
+				    gpiod_name(udc->board.vbus_pin));
 				goto err_unprepare_iclk;
 			}
 		}
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index fceeefd5f893..9c91fae62f8c 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -172,6 +172,8 @@ int gpiod_set_consumer_name(struct gpio_desc *desc, const char *name);
 
 bool gpiod_is_shared(const struct gpio_desc *desc);
 
+const char *gpiod_name(const struct gpio_desc *desc);
+
 /* Convert between the old gpio_ and new gpiod_ interfaces */
 struct gpio_desc *gpio_to_desc(unsigned gpio);
 int desc_to_gpio(const struct gpio_desc *desc);
@@ -538,6 +540,12 @@ static inline bool gpiod_is_shared(const struct gpio_desc *desc)
 	return false;
 }
 
+static inline const char *gpiod_name(const struct gpio_desc *desc)
+{
+	WARN_ON(desc);
+	return "(no gpio)";
+}
+
 static inline struct gpio_desc *gpio_to_desc(unsigned gpio)
 {
 	return NULL;
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] [RFC] gpiolib: introduce gpio_name() helper
  2026-06-29 13:56 [PATCH] [RFC] gpiolib: introduce gpio_name() helper Arnd Bergmann
@ 2026-06-29 14:10 ` sashiko-bot
  2026-06-29 15:23 ` Andy Shevchenko
  2026-06-29 15:29 ` Geert Uytterhoeven
  2 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-06-29 14:10 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: dmitry.torokhov, linux-input, dri-devel

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Uncanceled timer in the probe error path leads to a Use-After-Free when the timer fires.
- [High] Uncanceled delayed work in the probe error path and early IRQ request lead to Use-After-Free.
- [High] The return value of extcon_rtk_type_c_edev_register() is assigned but never checked, leading to silent failures.
--

commit fd9fdd473dd5d54a8f94bae5e74133accbec590a
Author: Arnd Bergmann <arnd@arndb.de>

gpiolib: introduce gpio_name() helper

Most remaining users of desc_to_gpio() only call it for printing debug
information.

Replace this with a new gpiod_name() helper that returns the
gpio_desc->name string after checking the gpio_desc pointer.

> diff --git a/drivers/extcon/extcon-rtk-type-c.c b/drivers/extcon/extcon-rtk-type-c.c
> index 82b60b927e41a..fb57e9d7ddb63 100644
> --- a/drivers/extcon/extcon-rtk-type-c.c
> +++ b/drivers/extcon/extcon-rtk-type-c.c
> @@ -1356,8 +1356,8 @@ static int extcon_rtk_type_c_probe(struct platform_device *pdev)
>  				(int)PTR_ERR(gpio));
>  		} else {
>  			type_c->rd_ctrl_gpio_desc = gpio;
> -			dev_dbg(dev, "%s get rd-ctrl-gpios (id=%d) OK\n",
> -				__func__, desc_to_gpio(gpio));
> +			dev_dbg(dev, "%s get rd-ctrl-gpios (id=%s) OK\n",
> +				__func__, gpiod_name(gpio));
>  		}
>  	}

[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free if
extcon_rtk_type_c_init() fails.

In extcon_rtk_type_c_probe(), the code requests an IRQ before the delayed
work is initialized:

	ret = devm_request_irq(dev, type_c->irq, type_c_detect_irq, ...);
	...
	INIT_DELAYED_WORK(&type_c->delayed_work, host_device_switch);

	ret = extcon_rtk_type_c_init(type_c);
	if (ret) {
		dev_err(dev, "%s failed to init type_c\n", __func__);
		goto err;
	}

Could an early interrupt schedule the uninitialized work?

Also, if extcon_rtk_type_c_init() fails, the code jumps to the err label
and returns without canceling the delayed work. Since the structure is
freed by devres, will this cause a use-after-free when the work executes?

[Severity: High]
This is also a pre-existing issue, but further down in the same function,
the return value of extcon_rtk_type_c_edev_register() is assigned to ret
but is not checked:

	ret = extcon_rtk_type_c_edev_register(type_c);

	create_debug_files(type_c);

	return 0;

Could this mask a registration failure, since the function unconditionally
returns 0?

[ ... ]

> diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
> index 5aa360ba4f039..0993136043870 100644
> --- a/drivers/usb/gadget/udc/at91_udc.c
> +++ b/drivers/usb/gadget/udc/at91_udc.c
> @@ -1896,8 +1896,8 @@ static int at91udc_probe(struct platform_device *pdev)
>  					gpiod_to_irq(udc->board.vbus_pin),
>  					at91_vbus_irq, 0, driver_name, udc);
>  			if (retval) {
> -				DBG("request vbus irq %d failed\n",
> -				    desc_to_gpio(udc->board.vbus_pin));
> +				DBG("request vbus irq %s failed\n",
> +				    gpiod_name(udc->board.vbus_pin));
>  				goto err_unprepare_iclk;
>  			}
>  		}

[Severity: High]
This is a pre-existing issue, but there appears to be a missing timer cleanup
in the probe error path.

In at91udc_probe(), when vbus_polled is true, a timer is armed:

		if (udc->board.vbus_polled) {
			INIT_WORK(&udc->vbus_timer_work, at91_vbus_timer_work);
			timer_setup(&udc->vbus_timer, at91_vbus_timer, 0);
			mod_timer(&udc->vbus_timer,
				  jiffies + VBUS_POLL_TIMEOUT);
		}

If usb_add_gadget_udc() fails later, the code jumps to err_unprepare_iclk
and returns without deleting the timer:

	retval = usb_add_gadget_udc(dev, &udc->gadget);
	if (retval)
		goto err_unprepare_iclk;
	...
err_unprepare_iclk:
	clk_unprepare(udc->iclk);
err_unprepare_fclk:
	clk_unprepare(udc->fclk);

	return retval;

If the probe fails, devres will free the udc structure. Does this mean the
timer can still fire and access the freed udc memory, causing a
use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629135917.1308621-1-arnd@kernel.org?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] [RFC] gpiolib: introduce gpio_name() helper
  2026-06-29 13:56 [PATCH] [RFC] gpiolib: introduce gpio_name() helper Arnd Bergmann
  2026-06-29 14:10 ` sashiko-bot
@ 2026-06-29 15:23 ` Andy Shevchenko
  2026-06-29 15:45   ` Arnd Bergmann
  2026-06-29 15:29 ` Geert Uytterhoeven
  2 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2026-06-29 15:23 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linus Walleij, Bartosz Golaszewski, Arnd Bergmann,
	Marcel Holtmann, MyungJoo Ham, Chanwoo Choi, Geert Uytterhoeven,
	Andy Shevchenko, Dmitry Torokhov, Ulf Hansson, linux-bluetooth,
	linux-kernel, linux-gpio, dri-devel, linux-i2c, linux-iio,
	linux-input, linux-mmc, linux-arm-kernel, linux-pm, linux-usb

On Mon, Jun 29, 2026 at 03:56:29PM +0200, Arnd Bergmann wrote:

> Most remaining users of desc_to_gpio() only call it for printing debug
> information.
> 
> Replace this with a new gpiod_name() helper that returns the
> gpio_desc->name string after checking the gpio_desc pointer.

Oh, that's nice!

...

> +/**
> + * gpiod_name() - get a name to print for a gpio descriptor
> + * @desc: gpio or NULL pointer to query
> + *
> + * Returns:
> + * The desc->name field or a dummy string for unknown GPIOs.
> + */
> +const char *gpiod_name(const struct gpio_desc *desc)
> +{
> +	return desc ? desc->name : "(no gpio)";

Can we get into here with wrong (error pointer descriptor)? Shouldn't you call
one of validate_desc() / VALIDATE_DESC()?

Also not sure if "(no gpio)" is a good choice. "not requested"? "not provided"?

> +}

...

> +static inline const char *gpiod_name(const struct gpio_desc *desc)
> +{
> +	WARN_ON(desc);
> +	return "(no gpio)";

Hmm... This will be a second copy with a slight potential of going apart from
the other case. Perhaps a #define? (Yes, yes, I understand that there are pros
and cons, in particular readability with define is questionable.)

> +}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] [RFC] gpiolib: introduce gpio_name() helper
  2026-06-29 13:56 [PATCH] [RFC] gpiolib: introduce gpio_name() helper Arnd Bergmann
  2026-06-29 14:10 ` sashiko-bot
  2026-06-29 15:23 ` Andy Shevchenko
@ 2026-06-29 15:29 ` Geert Uytterhoeven
  2026-06-29 15:48   ` Arnd Bergmann
  2 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2026-06-29 15:29 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linus Walleij, Bartosz Golaszewski, Arnd Bergmann,
	Marcel Holtmann, MyungJoo Ham, Chanwoo Choi, Geert Uytterhoeven,
	Andy Shevchenko, Dmitry Torokhov, Ulf Hansson, linux-bluetooth,
	linux-kernel, linux-gpio, dri-devel, linux-i2c, linux-iio,
	linux-input, linux-mmc, linux-arm-kernel, linux-pm, linux-usb

Hi Arnd,

On Mon, 29 Jun 2026 at 15:59, Arnd Bergmann <arnd@kernel.org> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Most remaining users of desc_to_gpio() only call it for printing debug
> information.
>
> Replace this with a new gpiod_name() helper that returns the
> gpio_desc->name string after checking the gpio_desc pointer.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks for your patch!

> Not sure if this the way we want to take this, or if the gpio name is
> an appropriate replacement in debug printk.
>
> Since most of the callers of desc_to_gpio() and gpio_to_desc() are
> otherwise in drivers that already depend on CONFIG_GPIOLIB_LEGACY and
> include linux/gpio/legacy.h, only a handful of instances remain that
> are otherwise in files that otherwise only use the descriptor interfaces:

> --- a/drivers/gpio/gpio-aggregator.c
> +++ b/drivers/gpio/gpio-aggregator.c
> @@ -758,8 +758,8 @@ int gpiochip_fwd_desc_add(struct gpiochip_fwd *fwd, struct gpio_desc *desc,
>
>         fwd->descs[offset] = desc;
>
> -       dev_dbg(chip->parent, "%u => gpio %d irq %d\n", offset,
> -               desc_to_gpio(desc), gpiod_to_irq(desc));
> +       dev_dbg(chip->parent, "%u => gpio %s irq %d\n", offset,
> +               gpiod_name(desc), gpiod_to_irq(desc));
>
>         return 0;
>  }

Before, this printed:

    gpio-aggregator gpio-aggregator.1: 0 => gpio 589 irq 188
    gpio-aggregator gpio-aggregator.1: 1 => gpio 590 irq 189

After, this prints:

    gpio-aggregator gpio-aggregator.1: 0 => gpio (null) irq 188
    gpio-aggregator gpio-aggregator.1: 1 => gpio (null) irq 189

Same results for instantiation using sysfs or configfs[1], although
the latter does have optional support for specifying the name.

[1] Documentation/admin-guide/gpio/gpio-aggregator.rst

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] [RFC] gpiolib: introduce gpio_name() helper
  2026-06-29 15:23 ` Andy Shevchenko
@ 2026-06-29 15:45   ` Arnd Bergmann
  0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2026-06-29 15:45 UTC (permalink / raw)
  To: Andy Shevchenko, Arnd Bergmann
  Cc: Linus Walleij, Bartosz Golaszewski, Marcel Holtmann, MyungJoo Ham,
	Chanwoo Choi, Geert Uytterhoeven, Andy Shevchenko,
	Dmitry Torokhov, Ulf Hansson, linux-bluetooth, linux-kernel,
	open list:GPIO SUBSYSTEM, dri-devel, linux-i2c, linux-iio,
	linux-input, linux-mmc @ vger . kernel . org, linux-arm-kernel,
	linux-pm, linux-usb

On Mon, Jun 29, 2026, at 17:23, Andy Shevchenko wrote:
> On Mon, Jun 29, 2026 at 03:56:29PM +0200, Arnd Bergmann wrote:
>> +const char *gpiod_name(const struct gpio_desc *desc)
>> +{
>> +	return desc ? desc->name : "(no gpio)";
>
> Can we get into here with wrong (error pointer descriptor)? Shouldn't you call
> one of validate_desc() / VALIDATE_DESC()?

Since all the callers previously call desc_to_gpio and that does
not even check desc at all, it would be a preexisting bug if
any caller passed an error pointer.

I added the NULL pointer check since many callers had that
part originally, like

      tsdata->wake_gpio ? desc_to_gpio(tsdata->wake_gpio) : -1,

> Also not sure if "(no gpio)" is a good choice. "not requested"? "not provided"?

Any of those seem fine to me, not sure.

>> +static inline const char *gpiod_name(const struct gpio_desc *desc)
>> +{
>> +	WARN_ON(desc);
>> +	return "(no gpio)";
>
> Hmm... This will be a second copy with a slight potential of going apart from
> the other case. Perhaps a #define? (Yes, yes, I understand that there are pros
> and cons, in particular readability with define is questionable.)

I was mostly trying to optimize for consistency with the other
stub functions here. Since it should not actually be used at all
without gpiolib, returning NULL or an empty string here would
also work.

     Arnd

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] [RFC] gpiolib: introduce gpio_name() helper
  2026-06-29 15:29 ` Geert Uytterhoeven
@ 2026-06-29 15:48   ` Arnd Bergmann
  0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2026-06-29 15:48 UTC (permalink / raw)
  To: Geert Uytterhoeven, Arnd Bergmann
  Cc: Linus Walleij, Bartosz Golaszewski, Marcel Holtmann, MyungJoo Ham,
	Chanwoo Choi, Geert Uytterhoeven, Andy Shevchenko,
	Dmitry Torokhov, Ulf Hansson, linux-bluetooth, linux-kernel,
	open list:GPIO SUBSYSTEM, dri-devel, linux-i2c, linux-iio,
	linux-input, linux-mmc @ vger . kernel . org, linux-arm-kernel,
	linux-pm, linux-usb

On Mon, Jun 29, 2026, at 17:29, Geert Uytterhoeven wrote:
> On Mon, 29 Jun 2026 at 15:59, Arnd Bergmann <arnd@kernel.org> wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> Most remaining users of desc_to_gpio() only call it for printing debug
>> information.
>>
>> Replace this with a new gpiod_name() helper that returns the
>> gpio_desc->name string after checking the gpio_desc pointer.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Thanks for your patch!

Thanks for testing!

>> --- a/drivers/gpio/gpio-aggregator.c
>> +++ b/drivers/gpio/gpio-aggregator.c
>> @@ -758,8 +758,8 @@ int gpiochip_fwd_desc_add(struct gpiochip_fwd *fwd, struct gpio_desc *desc,
>>
>>         fwd->descs[offset] = desc;
>>
>> -       dev_dbg(chip->parent, "%u => gpio %d irq %d\n", offset,
>> -               desc_to_gpio(desc), gpiod_to_irq(desc));
>> +       dev_dbg(chip->parent, "%u => gpio %s irq %d\n", offset,
>> +               gpiod_name(desc), gpiod_to_irq(desc));
>>
>>         return 0;
>>  }
>
> Before, this printed:
>
>     gpio-aggregator gpio-aggregator.1: 0 => gpio 589 irq 188
>     gpio-aggregator gpio-aggregator.1: 1 => gpio 590 irq 189
>
> After, this prints:
>
>     gpio-aggregator gpio-aggregator.1: 0 => gpio (null) irq 188
>     gpio-aggregator gpio-aggregator.1: 1 => gpio (null) irq 189
>
> Same results for instantiation using sysfs or configfs[1], although
> the latter does have optional support for specifying the name.

I wonder how many of the other instances have the same problem
then. Would it be appropriate for gpiochip_fwd_desc_add() to set
a name itself to address this one?

       Arnd

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-29 15:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-29 13:56 [PATCH] [RFC] gpiolib: introduce gpio_name() helper Arnd Bergmann
2026-06-29 14:10 ` sashiko-bot
2026-06-29 15:23 ` Andy Shevchenko
2026-06-29 15:45   ` Arnd Bergmann
2026-06-29 15:29 ` Geert Uytterhoeven
2026-06-29 15:48   ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox