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

[Linus Walleij]

On Fri, Mar 13, 2026 at 1:08 AM Andre Przywara <andre.przywara@xxxxxxx> 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@xxxxxxx>

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