[PATCH v4 05/12] gpio: aggregator: refactor the code to add GPIO desc in the forwarder

Thomas Richard posted 12 patches 9 months, 2 weeks ago
There is a newer version of this series
[PATCH v4 05/12] gpio: aggregator: refactor the code to add GPIO desc in the forwarder
Posted by Thomas Richard 9 months, 2 weeks ago
Create a dedicated function to add a GPIO desc in the forwarder. Instead of
passing an array of GPIO desc, now the GPIO desc are passed on by one to
the forwarder.

Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
---
 drivers/gpio/gpio-aggregator.c | 57 +++++++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index 4268ef94914d..78bb84ca8a05 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -509,6 +509,10 @@ devm_gpiochip_fwd_alloc(struct device *dev, unsigned int ngpios)
 	if (!fwd)
 		return ERR_PTR(-ENOMEM);
 
+	fwd->descs = devm_kcalloc(dev, ngpios, sizeof(*fwd->descs), GFP_KERNEL);
+	if (!fwd->descs)
+		return ERR_PTR(-ENOMEM);
+
 	chip = &fwd->chip;
 
 	chip->label = label;
@@ -528,6 +532,39 @@ devm_gpiochip_fwd_alloc(struct device *dev, unsigned int ngpios)
 	return fwd;
 }
 
+static int gpiochip_fwd_gpio_add(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)
+		return -EINVAL;
+
+	if (fwd->descs[offset])
+		return -EEXIST;
+
+	/*
+	 * 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,
+		desc_to_gpio(desc), gpiod_to_irq(desc));
+
+	return 0;
+}
+
 /**
  * gpiochip_fwd_create() - Create a new GPIO forwarder
  * @dev: Parent device pointer
@@ -559,26 +596,12 @@ static struct gpiochip_fwd *gpiochip_fwd_create(struct device *dev,
 
 	chip = &fwd->chip;
 
-	/*
-	 * 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.
-	 */
 	for (i = 0; i < ngpios; i++) {
-		struct gpio_chip *parent = gpiod_to_chip(descs[i]);
-
-		dev_dbg(dev, "%u => gpio %d irq %d\n", i,
-			desc_to_gpio(descs[i]), gpiod_to_irq(descs[i]));
-
-		if (gpiod_cansleep(descs[i]))
-			chip->can_sleep = true;
-		if (parent && parent->set_config)
-			chip->set_config = gpio_fwd_set_config;
+		error = gpiochip_fwd_gpio_add(fwd, descs[i], i);
+		if (error)
+			return ERR_PTR(error);
 	}
 
-	fwd->descs = descs;
-
 	if (chip->can_sleep)
 		mutex_init(&fwd->mlock);
 	else

-- 
2.39.5
Re: [PATCH v4 05/12] gpio: aggregator: refactor the code to add GPIO desc in the forwarder
Posted by Andy Shevchenko 9 months, 2 weeks ago
On Tue, Apr 29, 2025 at 5:08 PM Thomas Richard
<thomas.richard@bootlin.com> wrote:
>
> Create a dedicated function to add a GPIO desc in the forwarder. Instead of
> passing an array of GPIO desc, now the GPIO desc are passed on by one to

GPIO desc:s
GPIO desc:s

> the forwarder.

...

> +static int gpiochip_fwd_gpio_add(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)
> +               return -EINVAL;

> +       if (fwd->descs[offset])
> +               return -EEXIST;

If we still need this, it shouldn't be in this patch. This change
doesn't require the check.

> +       /*
> +        * 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,
> +               desc_to_gpio(desc), gpiod_to_irq(desc));
> +
> +       return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko