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

Andre Przywara
2026-03-15T00:43:58Z

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];
>