[RFC PATCH] pinctrl: sunxi: convert to GPIO_GENERIC

[RFC PATCH] pinctrl: sunxi: convert to GPIO_GENERIC

[Andre Przywara]

Allwinner SoCs combine pinmuxing and GPIO control in one device/MMIO
register frame. So far we were instantiating one GPIO chip per pinctrl
device, which covers multiple banks of up to 32 GPIO pins per bank. The
GPIO numbers were set to match the absolute pin numbers, even across the
typically two instances of the pinctrl device.

Convert the GPIO part of the sunxi pinctrl over to use the gpio_generic
framework. This alone allows to remove some sunxi specific code, which
is replaced with the existing generic code. This will become even more
useful with the upcoming A733 support, which adds set and clear
registers for the output.
As a side effect this also changes the GPIO device and number
allocation: Each bank is now represented by its own gpio_chip, with only
as many pins as there are actually implemented. The numbering is left up
to the kernel (.base = -1).

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/pinctrl/sunxi/Kconfig         |   1 +
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 245 ++++++++++++--------------
 drivers/pinctrl/sunxi/pinctrl-sunxi.h |  11 +-
 3 files changed, 124 insertions(+), 133 deletions(-)

diff --git a/drivers/pinctrl/sunxi/Kconfig b/drivers/pinctrl/sunxi/Kconfig
index dc62eba96348e..5905810dbf398 100644
--- a/drivers/pinctrl/sunxi/Kconfig
+++ b/drivers/pinctrl/sunxi/Kconfig
@@ -4,6 +4,7 @@ if ARCH_SUNXI
 config PINCTRL_SUNXI
 	bool
 	select PINMUX
+	select GPIO_GENERIC
 	select GENERIC_PINCONF
 	select GPIOLIB
 
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index 48434292a39b5..4235f9feff00d 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -13,6 +13,7 @@
 #include <linux/clk.h>
 #include <linux/export.h>
 #include <linux/gpio/driver.h>
+#include <linux/gpio/generic.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/irqchip/chained_irq.h>
@@ -86,17 +87,6 @@ static void sunxi_mux_reg(const struct sunxi_pinctrl *pctl,
 	*mask  = (BIT(MUX_FIELD_WIDTH) - 1) << *shift;
 }
 
-static void sunxi_data_reg(const struct sunxi_pinctrl *pctl,
-			   u32 pin, u32 *reg, u32 *shift, u32 *mask)
-{
-	u32 offset = pin % PINS_PER_BANK * DATA_FIELD_WIDTH;
-
-	*reg   = sunxi_bank_offset(pctl, pin) + DATA_REGS_OFFSET +
-		 offset / BITS_PER_TYPE(u32) * sizeof(u32);
-	*shift = offset % BITS_PER_TYPE(u32);
-	*mask  = (BIT(DATA_FIELD_WIDTH) - 1) << *shift;
-}
-
 static void sunxi_dlevel_reg(const struct sunxi_pinctrl *pctl,
 			     u32 pin, u32 *reg, u32 *shift, u32 *mask)
 {
@@ -930,99 +920,22 @@ static const struct pinmux_ops sunxi_pmx_ops = {
 	.strict			= true,
 };
 
-static int sunxi_pinctrl_gpio_direction_input(struct gpio_chip *chip,
-					unsigned offset)
-{
-	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
-
-	return sunxi_pmx_gpio_set_direction(pctl->pctl_dev, NULL,
-					    chip->base + offset, true);
-}
-
-static int sunxi_pinctrl_gpio_get(struct gpio_chip *chip, unsigned offset)
-{
-	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
-	bool set_mux = pctl->desc->irq_read_needs_mux &&
-		gpiochip_line_is_irq(chip, offset);
-	u32 pin = offset + chip->base;
-	u32 reg, shift, mask, val;
-
-	sunxi_data_reg(pctl, offset, &reg, &shift, &mask);
-
-	if (set_mux)
-		sunxi_pmx_set(pctl->pctl_dev, pin, SUN4I_FUNC_INPUT);
-
-	val = (readl(pctl->membase + reg) & mask) >> shift;
-
-	if (set_mux)
-		sunxi_pmx_set(pctl->pctl_dev, pin, SUN4I_FUNC_IRQ);
-
-	return val;
-}
-
-static int sunxi_pinctrl_gpio_set(struct gpio_chip *chip, unsigned int offset,
-				  int value)
-{
-	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
-	u32 reg, shift, mask, val;
-	unsigned long flags;
-
-	sunxi_data_reg(pctl, offset, &reg, &shift, &mask);
-
-	raw_spin_lock_irqsave(&pctl->lock, flags);
-
-	val = readl(pctl->membase + reg);
-
-	if (value)
-		val |= mask;
-	else
-		val &= ~mask;
-
-	writel(val, pctl->membase + reg);
-
-	raw_spin_unlock_irqrestore(&pctl->lock, flags);
-
-	return 0;
-}
-
-static int sunxi_pinctrl_gpio_direction_output(struct gpio_chip *chip,
-					unsigned offset, int value)
-{
-	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
-
-	sunxi_pinctrl_gpio_set(chip, offset, value);
-	return sunxi_pmx_gpio_set_direction(pctl->pctl_dev, NULL,
-					    chip->base + offset, false);
-}
-
-static int sunxi_pinctrl_gpio_of_xlate(struct gpio_chip *gc,
-				const struct of_phandle_args *gpiospec,
-				u32 *flags)
-{
-	int pin, base;
-
-	base = PINS_PER_BANK * gpiospec->args[0];
-	pin = base + gpiospec->args[1];
-
-	if (pin > gc->ngpio)
-		return -EINVAL;
-
-	if (flags)
-		*flags = gpiospec->args[2];
-
-	return pin;
-}
-
 static int sunxi_pinctrl_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
 {
 	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
 	struct sunxi_desc_function *desc;
-	unsigned pinnum = pctl->desc->pin_base + offset;
-	unsigned irqnum;
+	unsigned int pinnum, irqnum, i;
 
 	if (offset >= chip->ngpio)
 		return -ENXIO;
 
+	for (i = 0; i < SUNXI_PINCTRL_MAX_BANKS; i++)
+		if (pctl->banks[i].chip.gc.base == chip->base)
+			break;
+	if (i == SUNXI_PINCTRL_MAX_BANKS)
+		return -EINVAL;
+	pinnum = pctl->desc->pin_base + i * PINS_PER_BANK + offset;
+
 	desc = sunxi_pinctrl_desc_find_function_by_pin(pctl, pinnum, "irq");
 	if (!desc)
 		return -EINVAL;
@@ -1039,18 +952,19 @@ static int sunxi_pinctrl_irq_request_resources(struct irq_data *d)
 {
 	struct sunxi_pinctrl *pctl = irq_data_get_irq_chip_data(d);
 	struct sunxi_desc_function *func;
-	int ret;
+	int pinnum = pctl->irq_array[d->hwirq], ret;
+	int bank = (pinnum - pctl->desc->pin_base) / PINS_PER_BANK;
 
-	func = sunxi_pinctrl_desc_find_function_by_pin(pctl,
-					pctl->irq_array[d->hwirq], "irq");
+	func = sunxi_pinctrl_desc_find_function_by_pin(pctl, pinnum, "irq");
 	if (!func)
 		return -EINVAL;
 
-	ret = gpiochip_lock_as_irq(pctl->chip,
-			pctl->irq_array[d->hwirq] - pctl->desc->pin_base);
+	ret = gpiochip_lock_as_irq(&pctl->banks[bank].chip.gc,
+				   d->hwirq % IRQ_PER_BANK);
 	if (ret) {
 		dev_err(pctl->dev, "unable to lock HW IRQ %lu for IRQ\n",
 			irqd_to_hwirq(d));
+
 		return ret;
 	}
 
@@ -1063,9 +977,10 @@ static int sunxi_pinctrl_irq_request_resources(struct irq_data *d)
 static void sunxi_pinctrl_irq_release_resources(struct irq_data *d)
 {
 	struct sunxi_pinctrl *pctl = irq_data_get_irq_chip_data(d);
+	int pinnum = pctl->irq_array[d->hwirq] - pctl->desc->pin_base;
+	struct gpio_chip *gc = &pctl->banks[pinnum / PINS_PER_BANK].chip.gc;
 
-	gpiochip_unlock_as_irq(pctl->chip,
-			      pctl->irq_array[d->hwirq] - pctl->desc->pin_base);
+	gpiochip_unlock_as_irq(gc, pinnum);
 }
 
 static int sunxi_pinctrl_irq_set_type(struct irq_data *d, unsigned int type)
@@ -1493,6 +1408,84 @@ static int sunxi_pinctrl_setup_debounce(struct sunxi_pinctrl *pctl,
 	return 0;
 }
 
+static bool sunxi_of_node_instance_match(struct gpio_chip *chip, unsigned int i)
+{
+	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
+
+	if (i >= SUNXI_PINCTRL_MAX_BANKS)
+		return false;
+
+	return (chip->base == pctl->banks[i].chip.gc.base);
+}
+
+static int sunxi_num_pins_of_bank(struct sunxi_pinctrl *pctl, int bank)
+{
+	int max = -1, i;
+
+	for (i = 0; i < pctl->desc->npins; i++) {
+		int pinnum = pctl->desc->pins[i].pin.number - pctl->desc->pin_base;
+
+		if (pinnum / PINS_PER_BANK < bank)
+			continue;
+		if (pinnum / PINS_PER_BANK > bank)
+			break;
+		if (pinnum % PINS_PER_BANK > max)
+			max = pinnum % PINS_PER_BANK;
+	}
+
+	return max + 1;
+}
+
+static int sunxi_gpio_add_bank(struct sunxi_pinctrl *pctl, int index)
+{
+	char bank_name = 'A' + index + pctl->desc->pin_base / PINS_PER_BANK;
+	struct sunxi_gpio_bank *bank = &pctl->banks[index];
+	struct gpio_generic_chip_config config;
+	struct gpio_chip *gc = &bank->chip.gc;
+	int ngpio, ret;
+
+	ngpio = sunxi_num_pins_of_bank(pctl, index);
+	bank->pctl = pctl;
+	bank->base = pctl->membase + index * pctl->bank_mem_size;
+	if (!ngpio) {
+		gc->owner = THIS_MODULE;
+		gc->ngpio = 0;
+		gc->base = -1;
+		gc->of_gpio_n_cells = 3;
+
+		return 0;
+	}
+
+	config = (struct gpio_generic_chip_config) {
+		.dev = pctl->dev,
+		.sz = 4,
+		.dat = bank->base + DATA_REGS_OFFSET,
+		.set = bank->base + DATA_REGS_OFFSET,
+		.clr = NULL,
+		.flags = GPIO_GENERIC_READ_OUTPUT_REG_SET |
+			 GPIO_GENERIC_PINCTRL_BACKEND,
+	};
+
+	ret = gpio_generic_chip_init(&bank->chip, &config);
+	if (ret)
+		return dev_err_probe(pctl->dev, ret,
+				     "failed to init generic gpio chip\n");
+
+	gc->owner		= THIS_MODULE;
+	gc->label		= devm_kasprintf(pctl->dev, GFP_KERNEL,
+						 "%s-P%c", gc->label,
+						 bank_name);
+	gc->ngpio		= ngpio;
+	gc->base		= -1;
+	gc->of_gpio_n_cells	= 3;
+	gc->of_node_instance_match = sunxi_of_node_instance_match;
+	gc->set_config		= gpiochip_generic_config;
+	gc->to_irq		= sunxi_pinctrl_gpio_to_irq;
+	gc->can_sleep		= false;
+
+	return gpiochip_add_data(gc, pctl);
+}
+
 int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
 				  const struct sunxi_pinctrl_desc *desc,
 				  unsigned long flags)
@@ -1503,6 +1496,7 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
 	struct sunxi_pinctrl *pctl;
 	struct pinmux_ops *pmxops;
 	int i, ret, last_pin, pin_idx;
+	int gpio_banks;
 	struct clk *clk;
 
 	pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL);
@@ -1590,38 +1584,23 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
 		return PTR_ERR(pctl->pctl_dev);
 	}
 
-	pctl->chip = devm_kzalloc(&pdev->dev, sizeof(*pctl->chip), GFP_KERNEL);
-	if (!pctl->chip)
-		return -ENOMEM;
-
-	last_pin = pctl->desc->pins[pctl->desc->npins - 1].pin.number;
-	pctl->chip->owner = THIS_MODULE;
-	pctl->chip->request = gpiochip_generic_request;
-	pctl->chip->free = gpiochip_generic_free;
-	pctl->chip->set_config = gpiochip_generic_config;
-	pctl->chip->direction_input = sunxi_pinctrl_gpio_direction_input;
-	pctl->chip->direction_output = sunxi_pinctrl_gpio_direction_output;
-	pctl->chip->get = sunxi_pinctrl_gpio_get;
-	pctl->chip->set = sunxi_pinctrl_gpio_set;
-	pctl->chip->of_xlate = sunxi_pinctrl_gpio_of_xlate;
-	pctl->chip->to_irq = sunxi_pinctrl_gpio_to_irq;
-	pctl->chip->of_gpio_n_cells = 3;
-	pctl->chip->can_sleep = false;
-	pctl->chip->ngpio = round_up(last_pin, PINS_PER_BANK) -
-			    pctl->desc->pin_base;
-	pctl->chip->label = dev_name(&pdev->dev);
-	pctl->chip->parent = &pdev->dev;
-	pctl->chip->base = pctl->desc->pin_base;
-
-	ret = gpiochip_add_data(pctl->chip, pctl);
-	if (ret)
-		return ret;
+	last_pin = pctl->desc->pins[pctl->desc->npins - 1].pin.number -
+		   pctl->desc->pin_base;
+	for (gpio_banks = 0;
+	     gpio_banks <= last_pin / PINS_PER_BANK;
+	     gpio_banks++) {
+		ret = sunxi_gpio_add_bank(pctl, gpio_banks);
+		if (ret)
+			goto gpiochip_error;
+	}
 
 	for (i = 0; i < pctl->desc->npins; i++) {
 		const struct sunxi_desc_pin *pin = pctl->desc->pins + i;
+		int bank = (pin->pin.number - pctl->desc->pin_base) / PINS_PER_BANK;
+		struct gpio_chip *gc = &pctl->banks[bank].chip.gc;
 
-		ret = gpiochip_add_pin_range(pctl->chip, dev_name(&pdev->dev),
-					     pin->pin.number - pctl->desc->pin_base,
+		ret = gpiochip_add_pin_range(gc, dev_name(&pdev->dev),
+					     pin->pin.number % PINS_PER_BANK,
 					     pin->pin.number, 1);
 		if (ret)
 			goto gpiochip_error;
@@ -1690,6 +1669,8 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
 	return 0;
 
 gpiochip_error:
-	gpiochip_remove(pctl->chip);
+	while (--gpio_banks >= 0)
+		gpiochip_remove(&pctl->banks[gpio_banks].chip.gc);
+
 	return ret;
 }
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
index ad26e4de16a85..085131caa02fe 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
@@ -14,6 +14,7 @@
 #define __PINCTRL_SUNXI_H
 
 #include <linux/kernel.h>
+#include <linux/gpio/generic.h>
 #include <linux/spinlock.h>
 
 #define PA_BASE	0
@@ -159,9 +160,17 @@ struct sunxi_pinctrl_regulator {
 	refcount_t		refcount;
 };
 
+struct sunxi_pinctrl;
+
+struct sunxi_gpio_bank {
+	struct gpio_generic_chip chip;
+	struct sunxi_pinctrl *pctl;
+	void __iomem *base;
+};
+
 struct sunxi_pinctrl {
 	void __iomem			*membase;
-	struct gpio_chip		*chip;
+	struct sunxi_gpio_bank		banks[SUNXI_PINCTRL_MAX_BANKS];
 	const struct sunxi_pinctrl_desc	*desc;
 	struct device			*dev;
 	struct sunxi_pinctrl_regulator	regulators[11];
-- 
2.46.4

Re: [RFC PATCH] pinctrl: sunxi: convert to GPIO_GENERIC

[Icenowy Zheng]

在 2026-03-13五的 01:06 +0100，Andre Przywara写道：
> Allwinner SoCs combine pinmuxing and GPIO control in one device/MMIO
> register frame. So far we were instantiating one GPIO chip per
> pinctrl
> device, which covers multiple banks of up to 32 GPIO pins per bank.
> The
> GPIO numbers were set to match the absolute pin numbers, even across
> the
> typically two instances of the pinctrl device.
> 
> Convert the GPIO part of the sunxi pinctrl over to use the
> gpio_generic
> framework. This alone allows to remove some sunxi specific code,
> which
> is replaced with the existing generic code. This will become even
> more
> useful with the upcoming A733 support, which adds set and clear
> registers for the output.
> As a side effect this also changes the GPIO device and number
> allocation: Each bank is now represented by its own gpio_chip, with
> only
> as many pins as there are actually implemented. The numbering is left
> up

Ah, is this a userspace API break?

Sincerely,
Icenowy

> to the kernel (.base = -1).
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/pinctrl/sunxi/Kconfig         |   1 +
>  drivers/pinctrl/sunxi/pinctrl-sunxi.c | 245 ++++++++++++------------
> --
>  drivers/pinctrl/sunxi/pinctrl-sunxi.h |  11 +-
>  3 files changed, 124 insertions(+), 133 deletions(-)
> 
> diff --git a/drivers/pinctrl/sunxi/Kconfig
> b/drivers/pinctrl/sunxi/Kconfig
> index dc62eba96348e..5905810dbf398 100644
> --- a/drivers/pinctrl/sunxi/Kconfig
> +++ b/drivers/pinctrl/sunxi/Kconfig
> @@ -4,6 +4,7 @@ if ARCH_SUNXI
>  config PINCTRL_SUNXI
>  	bool
>  	select PINMUX
> +	select GPIO_GENERIC
>  	select GENERIC_PINCONF
>  	select GPIOLIB
>  
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 48434292a39b5..4235f9feff00d 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -13,6 +13,7 @@
>  #include <linux/clk.h>
>  #include <linux/export.h>
>  #include <linux/gpio/driver.h>
> +#include <linux/gpio/generic.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/irqchip/chained_irq.h>
> @@ -86,17 +87,6 @@ static void sunxi_mux_reg(const struct
> sunxi_pinctrl *pctl,
>  	*mask  = (BIT(MUX_FIELD_WIDTH) - 1) << *shift;
>  }
>  
> -static void sunxi_data_reg(const struct sunxi_pinctrl *pctl,
> -			   u32 pin, u32 *reg, u32 *shift, u32 *mask)
> -{
> -	u32 offset = pin % PINS_PER_BANK * DATA_FIELD_WIDTH;
> -
> -	*reg   = sunxi_bank_offset(pctl, pin) + DATA_REGS_OFFSET +
> -		 offset / BITS_PER_TYPE(u32) * sizeof(u32);
> -	*shift = offset % BITS_PER_TYPE(u32);
> -	*mask  = (BIT(DATA_FIELD_WIDTH) - 1) << *shift;
> -}
> -
>  static void sunxi_dlevel_reg(const struct sunxi_pinctrl *pctl,
>  			     u32 pin, u32 *reg, u32 *shift, u32
> *mask)
>  {
> @@ -930,99 +920,22 @@ static const struct pinmux_ops sunxi_pmx_ops =
> {
>  	.strict			= true,
>  };
>  
> -static int sunxi_pinctrl_gpio_direction_input(struct gpio_chip
> *chip,
> -					unsigned offset)
> -{
> -	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
> -
> -	return sunxi_pmx_gpio_set_direction(pctl->pctl_dev, NULL,
> -					    chip->base + offset,
> true);
> -}
> -
> -static int sunxi_pinctrl_gpio_get(struct gpio_chip *chip, unsigned
> offset)
> -{
> -	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
> -	bool set_mux = pctl->desc->irq_read_needs_mux &&
> -		gpiochip_line_is_irq(chip, offset);
> -	u32 pin = offset + chip->base;
> -	u32 reg, shift, mask, val;
> -
> -	sunxi_data_reg(pctl, offset, &reg, &shift, &mask);
> -
> -	if (set_mux)
> -		sunxi_pmx_set(pctl->pctl_dev, pin,
> SUN4I_FUNC_INPUT);
> -
> -	val = (readl(pctl->membase + reg) & mask) >> shift;
> -
> -	if (set_mux)
> -		sunxi_pmx_set(pctl->pctl_dev, pin, SUN4I_FUNC_IRQ);
> -
> -	return val;
> -}
> -
> -static int sunxi_pinctrl_gpio_set(struct gpio_chip *chip, unsigned
> int offset,
> -				  int value)
> -{
> -	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
> -	u32 reg, shift, mask, val;
> -	unsigned long flags;
> -
> -	sunxi_data_reg(pctl, offset, &reg, &shift, &mask);
> -
> -	raw_spin_lock_irqsave(&pctl->lock, flags);
> -
> -	val = readl(pctl->membase + reg);
> -
> -	if (value)
> -		val |= mask;
> -	else
> -		val &= ~mask;
> -
> -	writel(val, pctl->membase + reg);
> -
> -	raw_spin_unlock_irqrestore(&pctl->lock, flags);
> -
> -	return 0;
> -}
> -
> -static int sunxi_pinctrl_gpio_direction_output(struct gpio_chip
> *chip,
> -					unsigned offset, int value)
> -{
> -	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
> -
> -	sunxi_pinctrl_gpio_set(chip, offset, value);
> -	return sunxi_pmx_gpio_set_direction(pctl->pctl_dev, NULL,
> -					    chip->base + offset,
> false);
> -}
> -
> -static int sunxi_pinctrl_gpio_of_xlate(struct gpio_chip *gc,
> -				const struct of_phandle_args
> *gpiospec,
> -				u32 *flags)
> -{
> -	int pin, base;
> -
> -	base = PINS_PER_BANK * gpiospec->args[0];
> -	pin = base + gpiospec->args[1];
> -
> -	if (pin > gc->ngpio)
> -		return -EINVAL;
> -
> -	if (flags)
> -		*flags = gpiospec->args[2];
> -
> -	return pin;
> -}
> -
>  static int sunxi_pinctrl_gpio_to_irq(struct gpio_chip *chip,
> unsigned offset)
>  {
>  	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
>  	struct sunxi_desc_function *desc;
> -	unsigned pinnum = pctl->desc->pin_base + offset;
> -	unsigned irqnum;
> +	unsigned int pinnum, irqnum, i;
>  
>  	if (offset >= chip->ngpio)
>  		return -ENXIO;
>  
> +	for (i = 0; i < SUNXI_PINCTRL_MAX_BANKS; i++)
> +		if (pctl->banks[i].chip.gc.base == chip->base)
> +			break;
> +	if (i == SUNXI_PINCTRL_MAX_BANKS)
> +		return -EINVAL;
> +	pinnum = pctl->desc->pin_base + i * PINS_PER_BANK + offset;
> +
>  	desc = sunxi_pinctrl_desc_find_function_by_pin(pctl, pinnum,
> "irq");
>  	if (!desc)
>  		return -EINVAL;
> @@ -1039,18 +952,19 @@ static int
> sunxi_pinctrl_irq_request_resources(struct irq_data *d)
>  {
>  	struct sunxi_pinctrl *pctl = irq_data_get_irq_chip_data(d);
>  	struct sunxi_desc_function *func;
> -	int ret;
> +	int pinnum = pctl->irq_array[d->hwirq], ret;
> +	int bank = (pinnum - pctl->desc->pin_base) / PINS_PER_BANK;
>  
> -	func = sunxi_pinctrl_desc_find_function_by_pin(pctl,
> -					pctl->irq_array[d->hwirq],
> "irq");
> +	func = sunxi_pinctrl_desc_find_function_by_pin(pctl, pinnum,
> "irq");
>  	if (!func)
>  		return -EINVAL;
>  
> -	ret = gpiochip_lock_as_irq(pctl->chip,
> -			pctl->irq_array[d->hwirq] - pctl->desc-
> >pin_base);
> +	ret = gpiochip_lock_as_irq(&pctl->banks[bank].chip.gc,
> +				   d->hwirq % IRQ_PER_BANK);
>  	if (ret) {
>  		dev_err(pctl->dev, "unable to lock HW IRQ %lu for
> IRQ\n",
>  			irqd_to_hwirq(d));
> +
>  		return ret;
>  	}
>  
> @@ -1063,9 +977,10 @@ static int
> sunxi_pinctrl_irq_request_resources(struct irq_data *d)
>  static void sunxi_pinctrl_irq_release_resources(struct irq_data *d)
>  {
>  	struct sunxi_pinctrl *pctl = irq_data_get_irq_chip_data(d);
> +	int pinnum = pctl->irq_array[d->hwirq] - pctl->desc-
> >pin_base;
> +	struct gpio_chip *gc = &pctl->banks[pinnum /
> PINS_PER_BANK].chip.gc;
>  
> -	gpiochip_unlock_as_irq(pctl->chip,
> -			      pctl->irq_array[d->hwirq] - pctl-
> >desc->pin_base);
> +	gpiochip_unlock_as_irq(gc, pinnum);
>  }
>  
>  static int sunxi_pinctrl_irq_set_type(struct irq_data *d, unsigned
> int type)
> @@ -1493,6 +1408,84 @@ static int sunxi_pinctrl_setup_debounce(struct
> sunxi_pinctrl *pctl,
>  	return 0;
>  }
>  
> +static bool sunxi_of_node_instance_match(struct gpio_chip *chip,
> unsigned int i)
> +{
> +	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
> +
> +	if (i >= SUNXI_PINCTRL_MAX_BANKS)
> +		return false;
> +
> +	return (chip->base == pctl->banks[i].chip.gc.base);
> +}
> +
> +static int sunxi_num_pins_of_bank(struct sunxi_pinctrl *pctl, int
> bank)
> +{
> +	int max = -1, i;
> +
> +	for (i = 0; i < pctl->desc->npins; i++) {
> +		int pinnum = pctl->desc->pins[i].pin.number - pctl-
> >desc->pin_base;
> +
> +		if (pinnum / PINS_PER_BANK < bank)
> +			continue;
> +		if (pinnum / PINS_PER_BANK > bank)
> +			break;
> +		if (pinnum % PINS_PER_BANK > max)
> +			max = pinnum % PINS_PER_BANK;
> +	}
> +
> +	return max + 1;
> +}
> +
> +static int sunxi_gpio_add_bank(struct sunxi_pinctrl *pctl, int
> index)
> +{
> +	char bank_name = 'A' + index + pctl->desc->pin_base /
> PINS_PER_BANK;
> +	struct sunxi_gpio_bank *bank = &pctl->banks[index];
> +	struct gpio_generic_chip_config config;
> +	struct gpio_chip *gc = &bank->chip.gc;
> +	int ngpio, ret;
> +
> +	ngpio = sunxi_num_pins_of_bank(pctl, index);
> +	bank->pctl = pctl;
> +	bank->base = pctl->membase + index * pctl->bank_mem_size;
> +	if (!ngpio) {
> +		gc->owner = THIS_MODULE;
> +		gc->ngpio = 0;
> +		gc->base = -1;
> +		gc->of_gpio_n_cells = 3;
> +
> +		return 0;
> +	}
> +
> +	config = (struct gpio_generic_chip_config) {
> +		.dev = pctl->dev,
> +		.sz = 4,
> +		.dat = bank->base + DATA_REGS_OFFSET,
> +		.set = bank->base + DATA_REGS_OFFSET,
> +		.clr = NULL,
> +		.flags = GPIO_GENERIC_READ_OUTPUT_REG_SET |
> +			 GPIO_GENERIC_PINCTRL_BACKEND,
> +	};
> +
> +	ret = gpio_generic_chip_init(&bank->chip, &config);
> +	if (ret)
> +		return dev_err_probe(pctl->dev, ret,
> +				     "failed to init generic gpio
> chip\n");
> +
> +	gc->owner		= THIS_MODULE;
> +	gc->label		= devm_kasprintf(pctl->dev,
> GFP_KERNEL,
> +						 "%s-P%c", gc-
> >label,
> +						 bank_name);
> +	gc->ngpio		= ngpio;
> +	gc->base		= -1;
> +	gc->of_gpio_n_cells	= 3;
> +	gc->of_node_instance_match = sunxi_of_node_instance_match;
> +	gc->set_config		= gpiochip_generic_config;
> +	gc->to_irq		= sunxi_pinctrl_gpio_to_irq;
> +	gc->can_sleep		= false;
> +
> +	return gpiochip_add_data(gc, pctl);
> +}
> +
>  int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
>  				  const struct sunxi_pinctrl_desc
> *desc,
>  				  unsigned long flags)
> @@ -1503,6 +1496,7 @@ int sunxi_pinctrl_init_with_flags(struct
> platform_device *pdev,
>  	struct sunxi_pinctrl *pctl;
>  	struct pinmux_ops *pmxops;
>  	int i, ret, last_pin, pin_idx;
> +	int gpio_banks;
>  	struct clk *clk;
>  
>  	pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL);
> @@ -1590,38 +1584,23 @@ int sunxi_pinctrl_init_with_flags(struct
> platform_device *pdev,
>  		return PTR_ERR(pctl->pctl_dev);
>  	}
>  
> -	pctl->chip = devm_kzalloc(&pdev->dev, sizeof(*pctl->chip),
> GFP_KERNEL);
> -	if (!pctl->chip)
> -		return -ENOMEM;
> -
> -	last_pin = pctl->desc->pins[pctl->desc->npins -
> 1].pin.number;
> -	pctl->chip->owner = THIS_MODULE;
> -	pctl->chip->request = gpiochip_generic_request;
> -	pctl->chip->free = gpiochip_generic_free;
> -	pctl->chip->set_config = gpiochip_generic_config;
> -	pctl->chip->direction_input =
> sunxi_pinctrl_gpio_direction_input;
> -	pctl->chip->direction_output =
> sunxi_pinctrl_gpio_direction_output;
> -	pctl->chip->get = sunxi_pinctrl_gpio_get;
> -	pctl->chip->set = sunxi_pinctrl_gpio_set;
> -	pctl->chip->of_xlate = sunxi_pinctrl_gpio_of_xlate;
> -	pctl->chip->to_irq = sunxi_pinctrl_gpio_to_irq;
> -	pctl->chip->of_gpio_n_cells = 3;
> -	pctl->chip->can_sleep = false;
> -	pctl->chip->ngpio = round_up(last_pin, PINS_PER_BANK) -
> -			    pctl->desc->pin_base;
> -	pctl->chip->label = dev_name(&pdev->dev);
> -	pctl->chip->parent = &pdev->dev;
> -	pctl->chip->base = pctl->desc->pin_base;
> -
> -	ret = gpiochip_add_data(pctl->chip, pctl);
> -	if (ret)
> -		return ret;
> +	last_pin = pctl->desc->pins[pctl->desc->npins -
> 1].pin.number -
> +		   pctl->desc->pin_base;
> +	for (gpio_banks = 0;
> +	     gpio_banks <= last_pin / PINS_PER_BANK;
> +	     gpio_banks++) {
> +		ret = sunxi_gpio_add_bank(pctl, gpio_banks);
> +		if (ret)
> +			goto gpiochip_error;
> +	}
>  
>  	for (i = 0; i < pctl->desc->npins; i++) {
>  		const struct sunxi_desc_pin *pin = pctl->desc->pins
> + i;
> +		int bank = (pin->pin.number - pctl->desc->pin_base)
> / PINS_PER_BANK;
> +		struct gpio_chip *gc = &pctl->banks[bank].chip.gc;
>  
> -		ret = gpiochip_add_pin_range(pctl->chip,
> dev_name(&pdev->dev),
> -					     pin->pin.number - pctl-
> >desc->pin_base,
> +		ret = gpiochip_add_pin_range(gc, dev_name(&pdev-
> >dev),
> +					     pin->pin.number %
> PINS_PER_BANK,
>  					     pin->pin.number, 1);
>  		if (ret)
>  			goto gpiochip_error;
> @@ -1690,6 +1669,8 @@ int sunxi_pinctrl_init_with_flags(struct
> platform_device *pdev,
>  	return 0;
>  
>  gpiochip_error:
> -	gpiochip_remove(pctl->chip);
> +	while (--gpio_banks >= 0)
> +		gpiochip_remove(&pctl->banks[gpio_banks].chip.gc);
> +
>  	return ret;
>  }
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> index ad26e4de16a85..085131caa02fe 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> @@ -14,6 +14,7 @@
>  #define __PINCTRL_SUNXI_H
>  
>  #include <linux/kernel.h>
> +#include <linux/gpio/generic.h>
>  #include <linux/spinlock.h>
>  
>  #define PA_BASE	0
> @@ -159,9 +160,17 @@ struct sunxi_pinctrl_regulator {
>  	refcount_t		refcount;
>  };
>  
> +struct sunxi_pinctrl;
> +
> +struct sunxi_gpio_bank {
> +	struct gpio_generic_chip chip;
> +	struct sunxi_pinctrl *pctl;
> +	void __iomem *base;
> +};
> +
>  struct sunxi_pinctrl {
>  	void __iomem			*membase;
> -	struct gpio_chip		*chip;
> +	struct
> sunxi_gpio_bank		banks[SUNXI_PINCTRL_MAX_BANKS];
>  	const struct sunxi_pinctrl_desc	*desc;
>  	struct device			*dev;
>  	struct sunxi_pinctrl_regulator	regulators[11];

Re: [RFC PATCH] pinctrl: sunxi: convert to GPIO_GENERIC

[Chen-Yu Tsai]

On Sat, Mar 14, 2026 at 1:14 PM Icenowy Zheng <uwu@icenowy.me> wrote:
>
> 在 2026-03-13五的 01:06 +0100，Andre Przywara写道：
> > Allwinner SoCs combine pinmuxing and GPIO control in one device/MMIO
> > register frame. So far we were instantiating one GPIO chip per
> > pinctrl
> > device, which covers multiple banks of up to 32 GPIO pins per bank.
> > The
> > GPIO numbers were set to match the absolute pin numbers, even across
> > the
> > typically two instances of the pinctrl device.
> >
> > Convert the GPIO part of the sunxi pinctrl over to use the
> > gpio_generic
> > framework. This alone allows to remove some sunxi specific code,
> > which
> > is replaced with the existing generic code. This will become even
> > more
> > useful with the upcoming A733 support, which adds set and clear
> > registers for the output.
> > As a side effect this also changes the GPIO device and number
> > allocation: Each bank is now represented by its own gpio_chip, with
> > only
> > as many pins as there are actually implemented. The numbering is left
> > up
>
> Ah, is this a userspace API break?

Unfortunately, yes. This means the easily computable numbers that one can
use with the (deprecated) sysfs interface is gone, and also the pins are
now split amongst multiple gpiochip instances.

However if someone wanted the old "one gpiochip for one PIO instance with
evenly spaced banks" scheme, I suppose we could put together something
with the GPIO aggregator driver? It won't have same base pin number though.


ChenYu

Re: [RFC PATCH] pinctrl: sunxi: convert to GPIO_GENERIC

[Jernej Škrabec]

Dne sobota, 14. marec 2026 ob 09:38:11 Srednjeevropski standardni čas je Chen-Yu Tsai napisal(a):
> On Sat, Mar 14, 2026 at 1:14 PM Icenowy Zheng <uwu@icenowy.me> wrote:
> >
> > 在 2026-03-13五的 01:06 +0100，Andre Przywara写道：
> > > Allwinner SoCs combine pinmuxing and GPIO control in one device/MMIO
> > > register frame. So far we were instantiating one GPIO chip per
> > > pinctrl
> > > device, which covers multiple banks of up to 32 GPIO pins per bank.
> > > The
> > > GPIO numbers were set to match the absolute pin numbers, even across
> > > the
> > > typically two instances of the pinctrl device.
> > >
> > > Convert the GPIO part of the sunxi pinctrl over to use the
> > > gpio_generic
> > > framework. This alone allows to remove some sunxi specific code,
> > > which
> > > is replaced with the existing generic code. This will become even
> > > more
> > > useful with the upcoming A733 support, which adds set and clear
> > > registers for the output.
> > > As a side effect this also changes the GPIO device and number
> > > allocation: Each bank is now represented by its own gpio_chip, with
> > > only
> > > as many pins as there are actually implemented. The numbering is left
> > > up
> >
> > Ah, is this a userspace API break?
> 
> Unfortunately, yes. This means the easily computable numbers that one can
> use with the (deprecated) sysfs interface is gone, and also the pins are
> now split amongst multiple gpiochip instances.

I don't mind this at all for new SoCs, e.g. A733, but not really for already
supported SoCs.

> 
> However if someone wanted the old "one gpiochip for one PIO instance with
> evenly spaced banks" scheme, I suppose we could put together something
> with the GPIO aggregator driver? It won't have same base pin number though.

IIUC, this can be instantiated only via sysfs or configfs?

Best regards,
Jernej

Re: [RFC PATCH] pinctrl: sunxi: convert to GPIO_GENERIC

[Chen-Yu Tsai]

On Fri, Mar 13, 2026 at 8:08 AM Andre Przywara <andre.przywara@arm.com> wrote:
>
> Allwinner SoCs combine pinmuxing and GPIO control in one device/MMIO
> register frame. So far we were instantiating one GPIO chip per pinctrl
> device, which covers multiple banks of up to 32 GPIO pins per bank. The
> GPIO numbers were set to match the absolute pin numbers, even across the
> typically two instances of the pinctrl device.
>
> Convert the GPIO part of the sunxi pinctrl over to use the gpio_generic
> framework. This alone allows to remove some sunxi specific code, which
> is replaced with the existing generic code. This will become even more
> useful with the upcoming A733 support, which adds set and clear
> registers for the output.
> As a side effect this also changes the GPIO device and number
> allocation: Each bank is now represented by its own gpio_chip, with only
> as many pins as there are actually implemented. The numbering is left up
> to the kernel (.base = -1).
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/pinctrl/sunxi/Kconfig         |   1 +
>  drivers/pinctrl/sunxi/pinctrl-sunxi.c | 245 ++++++++++++--------------
>  drivers/pinctrl/sunxi/pinctrl-sunxi.h |  11 +-
>  3 files changed, 124 insertions(+), 133 deletions(-)
>
> diff --git a/drivers/pinctrl/sunxi/Kconfig b/drivers/pinctrl/sunxi/Kconfig
> index dc62eba96348e..5905810dbf398 100644
> --- a/drivers/pinctrl/sunxi/Kconfig
> +++ b/drivers/pinctrl/sunxi/Kconfig
> @@ -4,6 +4,7 @@ if ARCH_SUNXI
>  config PINCTRL_SUNXI
>         bool
>         select PINMUX
> +       select GPIO_GENERIC
>         select GENERIC_PINCONF
>         select GPIOLIB
>
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 48434292a39b5..4235f9feff00d 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c

[...]

>  static int sunxi_pinctrl_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
>  {
>         struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
>         struct sunxi_desc_function *desc;
> -       unsigned pinnum = pctl->desc->pin_base + offset;
> -       unsigned irqnum;
> +       unsigned int pinnum, irqnum, i;
>
>         if (offset >= chip->ngpio)
>                 return -ENXIO;
>
> +       for (i = 0; i < SUNXI_PINCTRL_MAX_BANKS; i++)
> +               if (pctl->banks[i].chip.gc.base == chip->base)

Can't you simply compare the instance?

    if (&pctl->bankd[i].chip.gc == chip)

> +                       break;
> +       if (i == SUNXI_PINCTRL_MAX_BANKS)
> +               return -EINVAL;
> +       pinnum = pctl->desc->pin_base + i * PINS_PER_BANK + offset;
> +
>         desc = sunxi_pinctrl_desc_find_function_by_pin(pctl, pinnum, "irq");
>         if (!desc)
>                 return -EINVAL;
> @@ -1039,18 +952,19 @@ static int sunxi_pinctrl_irq_request_resources(struct irq_data *d)
>  {
>         struct sunxi_pinctrl *pctl = irq_data_get_irq_chip_data(d);
>         struct sunxi_desc_function *func;
> -       int ret;
> +       int pinnum = pctl->irq_array[d->hwirq], ret;
> +       int bank = (pinnum - pctl->desc->pin_base) / PINS_PER_BANK;
>
> -       func = sunxi_pinctrl_desc_find_function_by_pin(pctl,
> -                                       pctl->irq_array[d->hwirq], "irq");
> +       func = sunxi_pinctrl_desc_find_function_by_pin(pctl, pinnum, "irq");
>         if (!func)
>                 return -EINVAL;
>
> -       ret = gpiochip_lock_as_irq(pctl->chip,
> -                       pctl->irq_array[d->hwirq] - pctl->desc->pin_base);
> +       ret = gpiochip_lock_as_irq(&pctl->banks[bank].chip.gc,
> +                                  d->hwirq % IRQ_PER_BANK);
>         if (ret) {
>                 dev_err(pctl->dev, "unable to lock HW IRQ %lu for IRQ\n",
>                         irqd_to_hwirq(d));
> +
>                 return ret;
>         }
>
> @@ -1063,9 +977,10 @@ static int sunxi_pinctrl_irq_request_resources(struct irq_data *d)
>  static void sunxi_pinctrl_irq_release_resources(struct irq_data *d)
>  {
>         struct sunxi_pinctrl *pctl = irq_data_get_irq_chip_data(d);
> +       int pinnum = pctl->irq_array[d->hwirq] - pctl->desc->pin_base;
> +       struct gpio_chip *gc = &pctl->banks[pinnum / PINS_PER_BANK].chip.gc;
>
> -       gpiochip_unlock_as_irq(pctl->chip,
> -                             pctl->irq_array[d->hwirq] - pctl->desc->pin_base);
> +       gpiochip_unlock_as_irq(gc, pinnum);
>  }
>
>  static int sunxi_pinctrl_irq_set_type(struct irq_data *d, unsigned int type)
> @@ -1493,6 +1408,84 @@ static int sunxi_pinctrl_setup_debounce(struct sunxi_pinctrl *pctl,
>         return 0;
>  }
>
> +static bool sunxi_of_node_instance_match(struct gpio_chip *chip, unsigned int i)
> +{
> +       struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
> +
> +       if (i >= SUNXI_PINCTRL_MAX_BANKS)
> +               return false;
> +
> +       return (chip->base == pctl->banks[i].chip.gc.base);

Same here.

> +}
> +
> +static int sunxi_num_pins_of_bank(struct sunxi_pinctrl *pctl, int bank)

IMHO num_pins_in_bank would be better.

And I think having a comment above saying this returns the *actual* number
of valid pins would help.

Or just call it sunxi_num_valid_pins_in_bank?

> +{
> +       int max = -1, i;
> +
> +       for (i = 0; i < pctl->desc->npins; i++) {
> +               int pinnum = pctl->desc->pins[i].pin.number - pctl->desc->pin_base;
> +
> +               if (pinnum / PINS_PER_BANK < bank)
> +                       continue;
> +               if (pinnum / PINS_PER_BANK > bank)
> +                       break;
> +               if (pinnum % PINS_PER_BANK > max)
> +                       max = pinnum % PINS_PER_BANK;

This doesn't work for existing sun5i platforms, which have pins non-existent
on some variants, so we end up with holes in each bank.

Instead we have to actually calculate the number of valid pins.

> +       }
> +
> +       return max + 1;
> +}
> +
> +static int sunxi_gpio_add_bank(struct sunxi_pinctrl *pctl, int index)
> +{
> +       char bank_name = 'A' + index + pctl->desc->pin_base / PINS_PER_BANK;
> +       struct sunxi_gpio_bank *bank = &pctl->banks[index];
> +       struct gpio_generic_chip_config config;
> +       struct gpio_chip *gc = &bank->chip.gc;
> +       int ngpio, ret;
> +
> +       ngpio = sunxi_num_pins_of_bank(pctl, index);
> +       bank->pctl = pctl;
> +       bank->base = pctl->membase + index * pctl->bank_mem_size;
> +       if (!ngpio) {
> +               gc->owner = THIS_MODULE;
> +               gc->ngpio = 0;
> +               gc->base = -1;
> +               gc->of_gpio_n_cells = 3;
> +
> +               return 0;
> +       }
> +
> +       config = (struct gpio_generic_chip_config) {
> +               .dev = pctl->dev,
> +               .sz = 4,
> +               .dat = bank->base + DATA_REGS_OFFSET,
> +               .set = bank->base + DATA_REGS_OFFSET,
> +               .clr = NULL,
> +               .flags = GPIO_GENERIC_READ_OUTPUT_REG_SET |
> +                        GPIO_GENERIC_PINCTRL_BACKEND,
> +       };
> +
> +       ret = gpio_generic_chip_init(&bank->chip, &config);
> +       if (ret)
> +               return dev_err_probe(pctl->dev, ret,
> +                                    "failed to init generic gpio chip\n");

Generic GPIO assumes that the GPIO pin range starts from 0, and is contiguous.
This breaks down with the sun5i and sun6i families. For example, on the A31s,
there is no PC16 ~ PC23, nor PH0 ~ PH8, just to show a few.

> +       gc->owner               = THIS_MODULE;
> +       gc->label               = devm_kasprintf(pctl->dev, GFP_KERNEL,
> +                                                "%s-P%c", gc->label,
> +                                                bank_name);
> +       gc->ngpio               = ngpio;

Also set gc->offset?

> +       gc->base                = -1;
> +       gc->of_gpio_n_cells     = 3;
> +       gc->of_node_instance_match = sunxi_of_node_instance_match;
> +       gc->set_config          = gpiochip_generic_config;
> +       gc->to_irq              = sunxi_pinctrl_gpio_to_irq;
> +       gc->can_sleep           = false;
> +
> +       return gpiochip_add_data(gc, pctl);

Can we switch to devm_gpiochip_add_data() instead? It simplifies the
teardown as well.

> +}
> +
>  int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
>                                   const struct sunxi_pinctrl_desc *desc,
>                                   unsigned long flags)
> @@ -1503,6 +1496,7 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
>         struct sunxi_pinctrl *pctl;
>         struct pinmux_ops *pmxops;
>         int i, ret, last_pin, pin_idx;
> +       int gpio_banks;
>         struct clk *clk;
>
>         pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL);
> @@ -1590,38 +1584,23 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
>                 return PTR_ERR(pctl->pctl_dev);
>         }
>
> -       pctl->chip = devm_kzalloc(&pdev->dev, sizeof(*pctl->chip), GFP_KERNEL);
> -       if (!pctl->chip)
> -               return -ENOMEM;
> -
> -       last_pin = pctl->desc->pins[pctl->desc->npins - 1].pin.number;
> -       pctl->chip->owner = THIS_MODULE;
> -       pctl->chip->request = gpiochip_generic_request;
> -       pctl->chip->free = gpiochip_generic_free;
> -       pctl->chip->set_config = gpiochip_generic_config;
> -       pctl->chip->direction_input = sunxi_pinctrl_gpio_direction_input;
> -       pctl->chip->direction_output = sunxi_pinctrl_gpio_direction_output;
> -       pctl->chip->get = sunxi_pinctrl_gpio_get;
> -       pctl->chip->set = sunxi_pinctrl_gpio_set;
> -       pctl->chip->of_xlate = sunxi_pinctrl_gpio_of_xlate;
> -       pctl->chip->to_irq = sunxi_pinctrl_gpio_to_irq;
> -       pctl->chip->of_gpio_n_cells = 3;
> -       pctl->chip->can_sleep = false;
> -       pctl->chip->ngpio = round_up(last_pin, PINS_PER_BANK) -
> -                           pctl->desc->pin_base;
> -       pctl->chip->label = dev_name(&pdev->dev);
> -       pctl->chip->parent = &pdev->dev;
> -       pctl->chip->base = pctl->desc->pin_base;
> -
> -       ret = gpiochip_add_data(pctl->chip, pctl);
> -       if (ret)
> -               return ret;
> +       last_pin = pctl->desc->pins[pctl->desc->npins - 1].pin.number -
> +                  pctl->desc->pin_base;
> +       for (gpio_banks = 0;

If you switch to devm_gpiochip_add_data() above, you won't need
gpiochip_remove() below, and you can declare |gpio_banks| inline here in
the for statement.

> +            gpio_banks <= last_pin / PINS_PER_BANK;
> +            gpio_banks++) {
> +               ret = sunxi_gpio_add_bank(pctl, gpio_banks);
> +               if (ret)
> +                       goto gpiochip_error;
> +       }
>
>         for (i = 0; i < pctl->desc->npins; i++) {
>                 const struct sunxi_desc_pin *pin = pctl->desc->pins + i;
> +               int bank = (pin->pin.number - pctl->desc->pin_base) / PINS_PER_BANK;
> +               struct gpio_chip *gc = &pctl->banks[bank].chip.gc;
>
> -               ret = gpiochip_add_pin_range(pctl->chip, dev_name(&pdev->dev),
> -                                            pin->pin.number - pctl->desc->pin_base,
> +               ret = gpiochip_add_pin_range(gc, dev_name(&pdev->dev),
> +                                            pin->pin.number % PINS_PER_BANK,
>                                              pin->pin.number, 1);
>                 if (ret)
>                         goto gpiochip_error;
> @@ -1690,6 +1669,8 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
>         return 0;
>
>  gpiochip_error:
> -       gpiochip_remove(pctl->chip);
> +       while (--gpio_banks >= 0)
> +               gpiochip_remove(&pctl->banks[gpio_banks].chip.gc);
> +
>         return ret;
>  }
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> index ad26e4de16a85..085131caa02fe 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> @@ -14,6 +14,7 @@
>  #define __PINCTRL_SUNXI_H
>
>  #include <linux/kernel.h>
> +#include <linux/gpio/generic.h>

gpio comes before kernel?

And maybe we should try to stop including the massive kernel.h header.


Thanks
ChenYu

>  #include <linux/spinlock.h>
>
>  #define PA_BASE        0
> @@ -159,9 +160,17 @@ struct sunxi_pinctrl_regulator {
>         refcount_t              refcount;
>  };
>
> +struct sunxi_pinctrl;
> +
> +struct sunxi_gpio_bank {
> +       struct gpio_generic_chip chip;
> +       struct sunxi_pinctrl *pctl;
> +       void __iomem *base;
> +};
> +
>  struct sunxi_pinctrl {
>         void __iomem                    *membase;
> -       struct gpio_chip                *chip;
> +       struct sunxi_gpio_bank          banks[SUNXI_PINCTRL_MAX_BANKS];
>         const struct sunxi_pinctrl_desc *desc;
>         struct device                   *dev;
>         struct sunxi_pinctrl_regulator  regulators[11];
> --
> 2.46.4
>
>

Re: [RFC PATCH] pinctrl: sunxi: convert to GPIO_GENERIC

[Andre Przywara]

On Sat, 14 Mar 2026 13:14:25 +0800
Icenowy Zheng <uwu@icenowy.me> wrote:

> 在 2026-03-13五的 01:06 +0100，Andre Przywara写道：
> > Allwinner SoCs combine pinmuxing and GPIO control in one device/MMIO
> > register frame. So far we were instantiating one GPIO chip per
> > pinctrl
> > device, which covers multiple banks of up to 32 GPIO pins per bank.
> > The
> > GPIO numbers were set to match the absolute pin numbers, even across
> > the
> > typically two instances of the pinctrl device.
> > 
> > Convert the GPIO part of the sunxi pinctrl over to use the
> > gpio_generic
> > framework. This alone allows to remove some sunxi specific code,
> > which
> > is replaced with the existing generic code. This will become even
> > more
> > useful with the upcoming A733 support, which adds set and clear
> > registers for the output.
> > As a side effect this also changes the GPIO device and number
> > allocation: Each bank is now represented by its own gpio_chip, with
> > only
> > as many pins as there are actually implemented. The numbering is left
> > up  
> 
> Ah, is this a userspace API break?

Was that ever a guaranteed user space API? Or just something
that everyone relied on because it was always the same (until it
wasn't)? Similar to /dev/mmcblk0 being the SD card?
And ignoring the ill-fated old-style sysfs interface for now, how does
this work with libgpiod? Would it still use the absolute pin numbers?

I mean looking at that warning about the forced GPIO numbering we
get, using base = -1 seems to be the recommended way?

Cheers,
Andre

> Sincerely,
> Icenowy
> 
> > to the kernel (.base = -1).
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  drivers/pinctrl/sunxi/Kconfig         |   1 +
> >  drivers/pinctrl/sunxi/pinctrl-sunxi.c | 245 ++++++++++++------------
> > --
> >  drivers/pinctrl/sunxi/pinctrl-sunxi.h |  11 +-
> >  3 files changed, 124 insertions(+), 133 deletions(-)
> > 
> > diff --git a/drivers/pinctrl/sunxi/Kconfig
> > b/drivers/pinctrl/sunxi/Kconfig
> > index dc62eba96348e..5905810dbf398 100644
> > --- a/drivers/pinctrl/sunxi/Kconfig
> > +++ b/drivers/pinctrl/sunxi/Kconfig
> > @@ -4,6 +4,7 @@ if ARCH_SUNXI
> >  config PINCTRL_SUNXI
> >  	bool
> >  	select PINMUX
> > +	select GPIO_GENERIC
> >  	select GENERIC_PINCONF
> >  	select GPIOLIB
> >  
> > diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> > b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> > index 48434292a39b5..4235f9feff00d 100644
> > --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> > +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/clk.h>
> >  #include <linux/export.h>
> >  #include <linux/gpio/driver.h>
> > +#include <linux/gpio/generic.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> >  #include <linux/irqchip/chained_irq.h>
> > @@ -86,17 +87,6 @@ static void sunxi_mux_reg(const struct
> > sunxi_pinctrl *pctl,
> >  	*mask  = (BIT(MUX_FIELD_WIDTH) - 1) << *shift;
> >  }
> >  
> > -static void sunxi_data_reg(const struct sunxi_pinctrl *pctl,
> > -			   u32 pin, u32 *reg, u32 *shift, u32 *mask)
> > -{
> > -	u32 offset = pin % PINS_PER_BANK * DATA_FIELD_WIDTH;
> > -
> > -	*reg   = sunxi_bank_offset(pctl, pin) + DATA_REGS_OFFSET +
> > -		 offset / BITS_PER_TYPE(u32) * sizeof(u32);
> > -	*shift = offset % BITS_PER_TYPE(u32);
> > -	*mask  = (BIT(DATA_FIELD_WIDTH) - 1) << *shift;
> > -}
> > -
> >  static void sunxi_dlevel_reg(const struct sunxi_pinctrl *pctl,
> >  			     u32 pin, u32 *reg, u32 *shift, u32
> > *mask)
> >  {
> > @@ -930,99 +920,22 @@ static const struct pinmux_ops sunxi_pmx_ops =
> > {
> >  	.strict			= true,
> >  };
> >  
> > -static int sunxi_pinctrl_gpio_direction_input(struct gpio_chip
> > *chip,
> > -					unsigned offset)
> > -{
> > -	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
> > -
> > -	return sunxi_pmx_gpio_set_direction(pctl->pctl_dev, NULL,
> > -					    chip->base + offset,
> > true);
> > -}
> > -
> > -static int sunxi_pinctrl_gpio_get(struct gpio_chip *chip, unsigned
> > offset)
> > -{
> > -	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
> > -	bool set_mux = pctl->desc->irq_read_needs_mux &&
> > -		gpiochip_line_is_irq(chip, offset);
> > -	u32 pin = offset + chip->base;
> > -	u32 reg, shift, mask, val;
> > -
> > -	sunxi_data_reg(pctl, offset, &reg, &shift, &mask);
> > -
> > -	if (set_mux)
> > -		sunxi_pmx_set(pctl->pctl_dev, pin,
> > SUN4I_FUNC_INPUT);
> > -
> > -	val = (readl(pctl->membase + reg) & mask) >> shift;
> > -
> > -	if (set_mux)
> > -		sunxi_pmx_set(pctl->pctl_dev, pin, SUN4I_FUNC_IRQ);
> > -
> > -	return val;
> > -}
> > -
> > -static int sunxi_pinctrl_gpio_set(struct gpio_chip *chip, unsigned
> > int offset,
> > -				  int value)
> > -{
> > -	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
> > -	u32 reg, shift, mask, val;
> > -	unsigned long flags;
> > -
> > -	sunxi_data_reg(pctl, offset, &reg, &shift, &mask);
> > -
> > -	raw_spin_lock_irqsave(&pctl->lock, flags);
> > -
> > -	val = readl(pctl->membase + reg);
> > -
> > -	if (value)
> > -		val |= mask;
> > -	else
> > -		val &= ~mask;
> > -
> > -	writel(val, pctl->membase + reg);
> > -
> > -	raw_spin_unlock_irqrestore(&pctl->lock, flags);
> > -
> > -	return 0;
> > -}
> > -
> > -static int sunxi_pinctrl_gpio_direction_output(struct gpio_chip
> > *chip,
> > -					unsigned offset, int value)
> > -{
> > -	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
> > -
> > -	sunxi_pinctrl_gpio_set(chip, offset, value);
> > -	return sunxi_pmx_gpio_set_direction(pctl->pctl_dev, NULL,
> > -					    chip->base + offset,
> > false);
> > -}
> > -
> > -static int sunxi_pinctrl_gpio_of_xlate(struct gpio_chip *gc,
> > -				const struct of_phandle_args
> > *gpiospec,
> > -				u32 *flags)
> > -{
> > -	int pin, base;
> > -
> > -	base = PINS_PER_BANK * gpiospec->args[0];
> > -	pin = base + gpiospec->args[1];
> > -
> > -	if (pin > gc->ngpio)
> > -		return -EINVAL;
> > -
> > -	if (flags)
> > -		*flags = gpiospec->args[2];
> > -
> > -	return pin;
> > -}
> > -
> >  static int sunxi_pinctrl_gpio_to_irq(struct gpio_chip *chip,
> > unsigned offset)
> >  {
> >  	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
> >  	struct sunxi_desc_function *desc;
> > -	unsigned pinnum = pctl->desc->pin_base + offset;
> > -	unsigned irqnum;
> > +	unsigned int pinnum, irqnum, i;
> >  
> >  	if (offset >= chip->ngpio)
> >  		return -ENXIO;
> >  
> > +	for (i = 0; i < SUNXI_PINCTRL_MAX_BANKS; i++)
> > +		if (pctl->banks[i].chip.gc.base == chip->base)
> > +			break;
> > +	if (i == SUNXI_PINCTRL_MAX_BANKS)
> > +		return -EINVAL;
> > +	pinnum = pctl->desc->pin_base + i * PINS_PER_BANK + offset;
> > +
> >  	desc = sunxi_pinctrl_desc_find_function_by_pin(pctl, pinnum,
> > "irq");
> >  	if (!desc)
> >  		return -EINVAL;
> > @@ -1039,18 +952,19 @@ static int
> > sunxi_pinctrl_irq_request_resources(struct irq_data *d)
> >  {
> >  	struct sunxi_pinctrl *pctl = irq_data_get_irq_chip_data(d);
> >  	struct sunxi_desc_function *func;
> > -	int ret;
> > +	int pinnum = pctl->irq_array[d->hwirq], ret;
> > +	int bank = (pinnum - pctl->desc->pin_base) / PINS_PER_BANK;
> >  
> > -	func = sunxi_pinctrl_desc_find_function_by_pin(pctl,
> > -					pctl->irq_array[d->hwirq],
> > "irq");
> > +	func = sunxi_pinctrl_desc_find_function_by_pin(pctl, pinnum,
> > "irq");
> >  	if (!func)
> >  		return -EINVAL;
> >  
> > -	ret = gpiochip_lock_as_irq(pctl->chip,
> > -			pctl->irq_array[d->hwirq] - pctl->desc-  
> > >pin_base);  
> > +	ret = gpiochip_lock_as_irq(&pctl->banks[bank].chip.gc,
> > +				   d->hwirq % IRQ_PER_BANK);
> >  	if (ret) {
> >  		dev_err(pctl->dev, "unable to lock HW IRQ %lu for
> > IRQ\n",
> >  			irqd_to_hwirq(d));
> > +
> >  		return ret;
> >  	}
> >  
> > @@ -1063,9 +977,10 @@ static int
> > sunxi_pinctrl_irq_request_resources(struct irq_data *d)
> >  static void sunxi_pinctrl_irq_release_resources(struct irq_data *d)
> >  {
> >  	struct sunxi_pinctrl *pctl = irq_data_get_irq_chip_data(d);
> > +	int pinnum = pctl->irq_array[d->hwirq] - pctl->desc-  
> > >pin_base;  
> > +	struct gpio_chip *gc = &pctl->banks[pinnum /
> > PINS_PER_BANK].chip.gc;
> >  
> > -	gpiochip_unlock_as_irq(pctl->chip,
> > -			      pctl->irq_array[d->hwirq] - pctl-  
> > >desc->pin_base);  
> > +	gpiochip_unlock_as_irq(gc, pinnum);
> >  }
> >  
> >  static int sunxi_pinctrl_irq_set_type(struct irq_data *d, unsigned
> > int type)
> > @@ -1493,6 +1408,84 @@ static int sunxi_pinctrl_setup_debounce(struct
> > sunxi_pinctrl *pctl,
> >  	return 0;
> >  }
> >  
> > +static bool sunxi_of_node_instance_match(struct gpio_chip *chip,
> > unsigned int i)
> > +{
> > +	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
> > +
> > +	if (i >= SUNXI_PINCTRL_MAX_BANKS)
> > +		return false;
> > +
> > +	return (chip->base == pctl->banks[i].chip.gc.base);
> > +}
> > +
> > +static int sunxi_num_pins_of_bank(struct sunxi_pinctrl *pctl, int
> > bank)
> > +{
> > +	int max = -1, i;
> > +
> > +	for (i = 0; i < pctl->desc->npins; i++) {
> > +		int pinnum = pctl->desc->pins[i].pin.number - pctl-  
> > >desc->pin_base;  
> > +
> > +		if (pinnum / PINS_PER_BANK < bank)
> > +			continue;
> > +		if (pinnum / PINS_PER_BANK > bank)
> > +			break;
> > +		if (pinnum % PINS_PER_BANK > max)
> > +			max = pinnum % PINS_PER_BANK;
> > +	}
> > +
> > +	return max + 1;
> > +}
> > +
> > +static int sunxi_gpio_add_bank(struct sunxi_pinctrl *pctl, int
> > index)
> > +{
> > +	char bank_name = 'A' + index + pctl->desc->pin_base /
> > PINS_PER_BANK;
> > +	struct sunxi_gpio_bank *bank = &pctl->banks[index];
> > +	struct gpio_generic_chip_config config;
> > +	struct gpio_chip *gc = &bank->chip.gc;
> > +	int ngpio, ret;
> > +
> > +	ngpio = sunxi_num_pins_of_bank(pctl, index);
> > +	bank->pctl = pctl;
> > +	bank->base = pctl->membase + index * pctl->bank_mem_size;
> > +	if (!ngpio) {
> > +		gc->owner = THIS_MODULE;
> > +		gc->ngpio = 0;
> > +		gc->base = -1;
> > +		gc->of_gpio_n_cells = 3;
> > +
> > +		return 0;
> > +	}
> > +
> > +	config = (struct gpio_generic_chip_config) {
> > +		.dev = pctl->dev,
> > +		.sz = 4,
> > +		.dat = bank->base + DATA_REGS_OFFSET,
> > +		.set = bank->base + DATA_REGS_OFFSET,
> > +		.clr = NULL,
> > +		.flags = GPIO_GENERIC_READ_OUTPUT_REG_SET |
> > +			 GPIO_GENERIC_PINCTRL_BACKEND,
> > +	};
> > +
> > +	ret = gpio_generic_chip_init(&bank->chip, &config);
> > +	if (ret)
> > +		return dev_err_probe(pctl->dev, ret,
> > +				     "failed to init generic gpio
> > chip\n");
> > +
> > +	gc->owner		= THIS_MODULE;
> > +	gc->label		= devm_kasprintf(pctl->dev,
> > GFP_KERNEL,
> > +						 "%s-P%c", gc-  
> > >label,  
> > +						 bank_name);
> > +	gc->ngpio		= ngpio;
> > +	gc->base		= -1;
> > +	gc->of_gpio_n_cells	= 3;
> > +	gc->of_node_instance_match = sunxi_of_node_instance_match;
> > +	gc->set_config		= gpiochip_generic_config;
> > +	gc->to_irq		= sunxi_pinctrl_gpio_to_irq;
> > +	gc->can_sleep		= false;
> > +
> > +	return gpiochip_add_data(gc, pctl);
> > +}
> > +
> >  int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
> >  				  const struct sunxi_pinctrl_desc
> > *desc,
> >  				  unsigned long flags)
> > @@ -1503,6 +1496,7 @@ int sunxi_pinctrl_init_with_flags(struct
> > platform_device *pdev,
> >  	struct sunxi_pinctrl *pctl;
> >  	struct pinmux_ops *pmxops;
> >  	int i, ret, last_pin, pin_idx;
> > +	int gpio_banks;
> >  	struct clk *clk;
> >  
> >  	pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL);
> > @@ -1590,38 +1584,23 @@ int sunxi_pinctrl_init_with_flags(struct
> > platform_device *pdev,
> >  		return PTR_ERR(pctl->pctl_dev);
> >  	}
> >  
> > -	pctl->chip = devm_kzalloc(&pdev->dev, sizeof(*pctl->chip),
> > GFP_KERNEL);
> > -	if (!pctl->chip)
> > -		return -ENOMEM;
> > -
> > -	last_pin = pctl->desc->pins[pctl->desc->npins -
> > 1].pin.number;
> > -	pctl->chip->owner = THIS_MODULE;
> > -	pctl->chip->request = gpiochip_generic_request;
> > -	pctl->chip->free = gpiochip_generic_free;
> > -	pctl->chip->set_config = gpiochip_generic_config;
> > -	pctl->chip->direction_input =
> > sunxi_pinctrl_gpio_direction_input;
> > -	pctl->chip->direction_output =
> > sunxi_pinctrl_gpio_direction_output;
> > -	pctl->chip->get = sunxi_pinctrl_gpio_get;
> > -	pctl->chip->set = sunxi_pinctrl_gpio_set;
> > -	pctl->chip->of_xlate = sunxi_pinctrl_gpio_of_xlate;
> > -	pctl->chip->to_irq = sunxi_pinctrl_gpio_to_irq;
> > -	pctl->chip->of_gpio_n_cells = 3;
> > -	pctl->chip->can_sleep = false;
> > -	pctl->chip->ngpio = round_up(last_pin, PINS_PER_BANK) -
> > -			    pctl->desc->pin_base;
> > -	pctl->chip->label = dev_name(&pdev->dev);
> > -	pctl->chip->parent = &pdev->dev;
> > -	pctl->chip->base = pctl->desc->pin_base;
> > -
> > -	ret = gpiochip_add_data(pctl->chip, pctl);
> > -	if (ret)
> > -		return ret;
> > +	last_pin = pctl->desc->pins[pctl->desc->npins -
> > 1].pin.number -
> > +		   pctl->desc->pin_base;
> > +	for (gpio_banks = 0;
> > +	     gpio_banks <= last_pin / PINS_PER_BANK;
> > +	     gpio_banks++) {
> > +		ret = sunxi_gpio_add_bank(pctl, gpio_banks);
> > +		if (ret)
> > +			goto gpiochip_error;
> > +	}
> >  
> >  	for (i = 0; i < pctl->desc->npins; i++) {
> >  		const struct sunxi_desc_pin *pin = pctl->desc->pins
> > + i;
> > +		int bank = (pin->pin.number - pctl->desc->pin_base)
> > / PINS_PER_BANK;
> > +		struct gpio_chip *gc = &pctl->banks[bank].chip.gc;
> >  
> > -		ret = gpiochip_add_pin_range(pctl->chip,
> > dev_name(&pdev->dev),
> > -					     pin->pin.number - pctl-  
> > >desc->pin_base,  
> > +		ret = gpiochip_add_pin_range(gc, dev_name(&pdev-  
> > >dev),  
> > +					     pin->pin.number %
> > PINS_PER_BANK,
> >  					     pin->pin.number, 1);
> >  		if (ret)
> >  			goto gpiochip_error;
> > @@ -1690,6 +1669,8 @@ int sunxi_pinctrl_init_with_flags(struct
> > platform_device *pdev,
> >  	return 0;
> >  
> >  gpiochip_error:
> > -	gpiochip_remove(pctl->chip);
> > +	while (--gpio_banks >= 0)
> > +		gpiochip_remove(&pctl->banks[gpio_banks].chip.gc);
> > +
> >  	return ret;
> >  }
> > diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> > b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> > index ad26e4de16a85..085131caa02fe 100644
> > --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> > +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> > @@ -14,6 +14,7 @@
> >  #define __PINCTRL_SUNXI_H
> >  
> >  #include <linux/kernel.h>
> > +#include <linux/gpio/generic.h>
> >  #include <linux/spinlock.h>
> >  
> >  #define PA_BASE	0
> > @@ -159,9 +160,17 @@ struct sunxi_pinctrl_regulator {
> >  	refcount_t		refcount;
> >  };
> >  
> > +struct sunxi_pinctrl;
> > +
> > +struct sunxi_gpio_bank {
> > +	struct gpio_generic_chip chip;
> > +	struct sunxi_pinctrl *pctl;
> > +	void __iomem *base;
> > +};
> > +
> >  struct sunxi_pinctrl {
> >  	void __iomem			*membase;
> > -	struct gpio_chip		*chip;
> > +	struct
> > sunxi_gpio_bank		banks[SUNXI_PINCTRL_MAX_BANKS];
> >  	const struct sunxi_pinctrl_desc	*desc;
> >  	struct device			*dev;
> >  	struct sunxi_pinctrl_regulator	regulators[11];  
> 

Re: [RFC PATCH] pinctrl: sunxi: convert to GPIO_GENERIC

[Bartosz Golaszewski]

On Sun, 15 Mar 2026 01:42:05 +0100, Andre Przywara
<andre.przywara@arm.com> said:
> On Sat, 14 Mar 2026 13:14:25 +0800
> Icenowy Zheng <uwu@icenowy.me> wrote:
>
>> 在 2026-03-13五的 01:06 +0100，Andre Przywara写道：
>> > Allwinner SoCs combine pinmuxing and GPIO control in one device/MMIO
>> > register frame. So far we were instantiating one GPIO chip per
>> > pinctrl
>> > device, which covers multiple banks of up to 32 GPIO pins per bank.
>> > The
>> > GPIO numbers were set to match the absolute pin numbers, even across
>> > the
>> > typically two instances of the pinctrl device.
>> >
>> > Convert the GPIO part of the sunxi pinctrl over to use the
>> > gpio_generic
>> > framework. This alone allows to remove some sunxi specific code,
>> > which
>> > is replaced with the existing generic code. This will become even
>> > more
>> > useful with the upcoming A733 support, which adds set and clear
>> > registers for the output.
>> > As a side effect this also changes the GPIO device and number
>> > allocation: Each bank is now represented by its own gpio_chip, with
>> > only
>> > as many pins as there are actually implemented. The numbering is left
>> > up
>>
>> Ah, is this a userspace API break?
>
> Was that ever a guaranteed user space API? Or just something
> that everyone relied on because it was always the same (until it
> wasn't)? Similar to /dev/mmcblk0 being the SD card?
> And ignoring the ill-fated old-style sysfs interface for now, how does
> this work with libgpiod? Would it still use the absolute pin numbers?
>
> I mean looking at that warning about the forced GPIO numbering we
> get, using base = -1 seems to be the recommended way?
>

My view is this: for libgpiod, there are no absolute GPIO numbers and the
ordering of GPIO chip character devices is not guaranteed.

For sysfs: userspace may rightfully expect the global numbering to stay the
same. I don't like it but this is the kernel policy.

Here however, there's another thing: the fact that a single GPIO chips is now
split into several. I don't mind it but if someone complains, it would have
to be reverted.

Bartosz

Re: [RFC PATCH] pinctrl: sunxi: convert to GPIO_GENERIC

[Linus Walleij]

On Fri, Mar 13, 2026 at 1:08 AM Andre Przywara <andre.przywara@arm.com> wrote:

> Allwinner SoCs combine pinmuxing and GPIO control in one device/MMIO
> register frame. So far we were instantiating one GPIO chip per pinctrl
> device, which covers multiple banks of up to 32 GPIO pins per bank. The
> GPIO numbers were set to match the absolute pin numbers, even across the
> typically two instances of the pinctrl device.
>
> Convert the GPIO part of the sunxi pinctrl over to use the gpio_generic
> framework. This alone allows to remove some sunxi specific code, which
> is replaced with the existing generic code. This will become even more
> useful with the upcoming A733 support, which adds set and clear
> registers for the output.
> As a side effect this also changes the GPIO device and number
> allocation: Each bank is now represented by its own gpio_chip, with only
> as many pins as there are actually implemented. The numbering is left up
> to the kernel (.base = -1).
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

You could add that this makes the driver benefit of the
.get/set_multiple() accelerated implementations of the generic
GPIO MMIO driver as well.

> +static int sunxi_gpio_add_bank(struct sunxi_pinctrl *pctl, int index)
> +{
> +       char bank_name = 'A' + index + pctl->desc->pin_base / PINS_PER_BANK;
> +       struct sunxi_gpio_bank *bank = &pctl->banks[index];
> +       struct gpio_generic_chip_config config;
> +       struct gpio_chip *gc = &bank->chip.gc;
> +       int ngpio, ret;
> +
> +       ngpio = sunxi_num_pins_of_bank(pctl, index);
> +       bank->pctl = pctl;
> +       bank->base = pctl->membase + index * pctl->bank_mem_size;
> +       if (!ngpio) {
> +               gc->owner = THIS_MODULE;
> +               gc->ngpio = 0;
> +               gc->base = -1;
> +               gc->of_gpio_n_cells = 3;
> +
> +               return 0;
> +       }

Add some comment about what is going on here, I suspect you
are flagging the gpio_chip as unused, you could just not assign
anything I think?

> +       config = (struct gpio_generic_chip_config) {
> +               .dev = pctl->dev,
> +               .sz = 4,
> +               .dat = bank->base + DATA_REGS_OFFSET,
> +               .set = bank->base + DATA_REGS_OFFSET,
> +               .clr = NULL,
> +               .flags = GPIO_GENERIC_READ_OUTPUT_REG_SET |
> +                        GPIO_GENERIC_PINCTRL_BACKEND,
> +       };
> +
> +       ret = gpio_generic_chip_init(&bank->chip, &config);
> +       if (ret)
> +               return dev_err_probe(pctl->dev, ret,
> +                                    "failed to init generic gpio chip\n");
> +
> +       gc->owner               = THIS_MODULE;
> +       gc->label               = devm_kasprintf(pctl->dev, GFP_KERNEL,
> +                                                "%s-P%c", gc->label,
> +                                                bank_name);
> +       gc->ngpio               = ngpio;
> +       gc->base                = -1;
> +       gc->of_gpio_n_cells     = 3;

Nice use of the three-cell GPIO OF parser!

Overall this is a nice change and makes the kernel a
better place. I would apply a non-RFC version as soon
as we can agree on the path forward.

Yours,
Linus Walleij

Re: [RFC PATCH] pinctrl: sunxi: convert to GPIO_GENERIC

[Linus Walleij]

On Sun, Mar 15, 2026 at 1:44 AM Andre Przywara <andre.przywara@arm.com> wrote:
> On Sat, 14 Mar 2026 13:14:25 +0800
> Icenowy Zheng <uwu@icenowy.me> wrote:

> > Ah, is this a userspace API break?
>
> Was that ever a guaranteed user space API? Or just something
> that everyone relied on because it was always the same (until it
> wasn't)? Similar to /dev/mmcblk0 being the SD card?

As Torvalds has pointed out, if it has users (that complain) it's an ABI.

However if a tree falls in the forest and no-one is there to hear it
fall, it is a philosophical question whether it makes a sound or not.
https://en.wikipedia.org/wiki/If_a_tree_falls_in_a_forest_and_no_one_is_around_to_hear_it,_does_it_make_a_sound%3F

The ABI has been broken since the first introduction of dynamic
GPIO devices, because anything specifying -1 as base will
get some random semi-predictable global GPIO numbers.

> And ignoring the ill-fated old-style sysfs interface for now, how does
> this work with libgpiod? Would it still use the absolute pin numbers?

No.

There is also a new sysfs that enables you to export chip-local
lines, and this is what you get with CONFIG_GPIO_SYSFS.
The old ABI is available behind CONFIG_GPIO_SYSFS_LEGACY
and if the users don't enable this, then they don't have a
problem with this patch.

> I mean looking at that warning about the forced GPIO numbering we
> get, using base = -1 seems to be the recommended way?

It is.

Unless there are users that love the old ABI (for Sunxi, I *doubt it*)
I think the patch should be applied.

Yours,
Linus Walleij