diff mbox series

[v3,08/10] gpio: aggregator: handle runtime registration of gpio_desc in gpiochip_fwd

Message ID 20250416-aaeon-up-board-pinctrl-support-v3-8-f40776bd06ee@bootlin.com
State New
Headers show
Series Add pinctrl support for the AAEON UP board FPGA | expand

Commit Message

Thomas Richard April 16, 2025, 2:08 p.m. UTC 
Add request() callback to check if the GPIO descriptor was well registered
in the gpiochip_fwd before to use it. This is done to handle the case
where GPIO descriptor is added at runtime in the forwarder.

If at least one GPIO descriptor was not added before the forwarder
registration, we assume the forwarder can sleep as if a GPIO is added at
runtime it may sleep.

Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
---
 drivers/gpio/gpio-aggregator.c | 27 +++++++++++++++++++++------
 include/linux/gpio/forwarder.h |  2 ++
 2 files changed, 23 insertions(+), 6 deletions(-)

Comments

Andy Shevchenko April 17, 2025, 5:38 p.m. UTC | #1 
On Wed, Apr 16, 2025 at 04:08:16PM +0200, Thomas Richard wrote:
> Add request() callback to check if the GPIO descriptor was well registered
> in the gpiochip_fwd before to use it. This is done to handle the case
> where GPIO descriptor is added at runtime in the forwarder.
> 
> If at least one GPIO descriptor was not added before the forwarder
> registration, we assume the forwarder can sleep as if a GPIO is added at
> runtime it may sleep.

...

> +int gpio_fwd_request(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
> +
> +	return fwd->descs[offset] ? 0 : -ENODEV;

ENODEV? Hmm... Why has that error code been chosen?

> +}

...

>  int gpio_fwd_register(struct gpiochip_fwd *fwd)
>  {
>  	struct gpio_chip *chip = &fwd->chip;
> +	unsigned int ndescs = 0, i;

Slightly better (from maintenance perspective) to decouple assignment as it's
not a standalone function that just counts them. So it means some code may
appear in between and in long-term someone might screw up with the initial
value for that.

>  	int error;

	ndescs = 0;

> +	for (i = 0; i < chip->ngpio; i++)
> +		if (fwd->descs[i])
> +			ndescs++;
> +
> +	/*
> +	 * Some gpio_desc were not registers. They will be registered at runtime

registered

> +	 * but we have to suppose they can sleep.
> +	 */
> +	if (ndescs != chip->ngpio)
> +		chip->can_sleep = true;
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index 5743f84e1310..0fb890f0793d 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -260,6 +260,14 @@  static void __exit gpio_aggregator_remove_all(void)
 
 #define fwd_tmp_size(ngpios)	(BITS_TO_LONGS((ngpios)) + (ngpios))
 
+int gpio_fwd_request(struct gpio_chip *chip, unsigned int offset)
+{
+	struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
+
+	return fwd->descs[offset] ? 0 : -ENODEV;
+}
+EXPORT_SYMBOL_NS_GPL(gpio_fwd_request, "GPIO_FORWARDER");
+
 int gpio_fwd_get_direction(struct gpio_chip *chip, unsigned int offset)
 {
 	struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
@@ -513,6 +521,7 @@  devm_gpio_fwd_alloc(struct device *dev, unsigned int ngpios)
 	chip->label = label;
 	chip->parent = dev;
 	chip->owner = THIS_MODULE;
+	chip->request = gpio_fwd_request;
 	chip->get_direction = gpio_fwd_get_direction;
 	chip->direction_input = gpio_fwd_direction_input;
 	chip->direction_output = gpio_fwd_direction_output;
@@ -531,7 +540,6 @@  EXPORT_SYMBOL_NS_GPL(devm_gpio_fwd_alloc, "GPIO_FORWARDER");
 int gpio_fwd_add_gpio_desc(struct gpiochip_fwd *fwd, struct gpio_desc *desc,
 			   unsigned int offset)
 {
-	struct gpio_chip *parent = gpiod_to_chip(desc);
 	struct gpio_chip *chip = &fwd->chip;
 
 	if (offset > chip->ngpio)
@@ -543,15 +551,10 @@  int gpio_fwd_add_gpio_desc(struct gpiochip_fwd *fwd, struct gpio_desc *desc,
 	/*
 	 * If any of the GPIO lines are sleeping, then the entire forwarder
 	 * will be sleeping.
-	 * If any of the chips support .set_config(), then the forwarder will
-	 * support setting configs.
 	 */
 	if (gpiod_cansleep(desc))
 		chip->can_sleep = true;
 
-	if (parent && parent->set_config)
-		chip->set_config = gpio_fwd_set_config;
-
 	fwd->descs[offset] = desc;
 
 	dev_dbg(chip->parent, "%u => gpio %d irq %d\n", offset,
@@ -564,8 +567,20 @@  EXPORT_SYMBOL_NS_GPL(gpio_fwd_add_gpio_desc, "GPIO_FORWARDER");
 int gpio_fwd_register(struct gpiochip_fwd *fwd)
 {
 	struct gpio_chip *chip = &fwd->chip;
+	unsigned int ndescs = 0, i;
 	int error;
 
+	for (i = 0; i < chip->ngpio; i++)
+		if (fwd->descs[i])
+			ndescs++;
+
+	/*
+	 * Some gpio_desc were not registers. They will be registered at runtime
+	 * but we have to suppose they can sleep.
+	 */
+	if (ndescs != chip->ngpio)
+		chip->can_sleep = true;
+
 	if (chip->can_sleep)
 		mutex_init(&fwd->mlock);
 	else
diff --git a/include/linux/gpio/forwarder.h b/include/linux/gpio/forwarder.h
index 78adfe6f162f..f3fba3c39bda 100644
--- a/include/linux/gpio/forwarder.h
+++ b/include/linux/gpio/forwarder.h
@@ -27,6 +27,8 @@  struct gpiochip_fwd {
 	unsigned long tmp[];		/* values and descs for multiple ops */
 };
 
+int gpio_fwd_request(struct gpio_chip *chip, unsigned int offset);
+
 int gpio_fwd_get_direction(struct gpio_chip *chip, unsigned int offset);
 
 int gpio_fwd_direction_input(struct gpio_chip *chip, unsigned int offset);