Re: [PATCH v3 04/21] pinctrl: starfive: Add StarFive JHB100 sys0 controller driver

[Linus Walleij]

On Sat, Jul 25, 2026 at 12:25 PM Changhuang Liang
<changhuang.liang@xxxxxxxxxxxxxxxx> wrote:
> > On Wed, Jun 3, 2026 at 7:54 AM Changhuang Liang
> > <changhuang.liang@xxxxxxxxxxxxxxxx> wrote:

> > > +static unsigned int jhb100_gpio_to_pin(struct gpio_chip *gc, unsigned
> > > +int gpio) {
> > > +       struct jhb100_gpio_bank *bank = jhb100_gc_to_bank(gc);
> > > +
> > > +       return bank->id * JHB100_NR_GPIOS_PER_BANK + gpio; }
> >
> > This usually tells me that GPIO_GENERIC can be used but maybe this has been
> > discussed before...
>
> Here, the main purpose is to convert GPIOs into pins for configuring certain pinctrl
> settings. I haven't found the part where GPIO_GENERIC can assist with this.
> Can you point it out for me? Thank you very much.

Normally GPIO chips use the .config() callback when you want to
set pin config for a certain pin in a pinctrl backend.

See Documentation/driver-api/gpio/driver.rst:

"The .set_config() callback uses the same enumerators and configuration
semantics as the generic pin control drivers. This is not a coincidence: it is
possible to assign the .set_config() to the function gpiochip_generic_config()
which will result in pinctrl_gpio_set_config() being called and eventually
ending up in the pin control back-end "behind" the GPIO controller, usually
closer to the actual pins. This way the pin controller can manage the below
listed GPIO configurations.

If a pin controller back-end is used, the GPIO controller or hardware
description needs to provide "GPIO ranges" mapping the GPIO line offsets to pin
numbers on the pin controller so they can properly cross-reference each other."

Maybe this is what you're looking for?

If you define gpio-ranges in your device tree, the GPIO and pin control
core deals with the cross-referencing for you.

> > > +       sfp->num_banks = DIV_ROUND_UP(sfp->ngpios,
> > > + JHB100_NR_GPIOS_PER_BANK);
> > > +
> > > +       for (unsigned int i = 0; i < sfp->num_banks; i++) {
> > > +               if (sfp->ngpios > (i + 1) *
> > JHB100_NR_GPIOS_PER_BANK)
> > > +                       sfp->banks[i].gc.ngpio = (i + 1) *
> > JHB100_NR_GPIOS_PER_BANK;
> > > +               else
> > > +                       sfp->banks[i].gc.ngpio = sfp->ngpios - i *
> > > + JHB100_NR_GPIOS_PER_BANK;
> >
> > This looks completely bananas, shouldn't this be simply:
> >
> > sfp->banks[i].gc.ngpio = JHB100_NR_GPIOS_PER_BANK;
>
> The JHB100 GPIO domain is not fully aligned to 32. For example, in the per0 domain,
> there are 60 GPIOs, so I split it into:
> bank0: 32 GPIOs
> bank1: 28 GPIOs.

At least add that, what you just wrote, as a comment right above this loop.

There is something with this math I think can be made simpler
with bitwise artithmetic. Think about it.

> > > +               sfp->banks[i].gc.request = pinctrl_gpio_request;
> >
> > Use
> > gpiochip_generic_request
> >
> > > +               sfp->banks[i].gc.free = pinctrl_gpio_free;
> >
> > Use
> > gpiochip_generic_free
> >
> > These calls will do what you want, and also check that the right gpio ranges
> > are available.
> >
> > Make sure you add GPIO ranges (the mapping between pin control pins and
> > corresponding GPIO offsets) for this to work properly.
> >
> > I'm pretty sure you can have a generic pin config backend as well.
> >
> > sfp->banks[i].gc.set_config = gpiochip_generic_config;

This is pertaining to what I said above about using the gpio
ranges and .config etc.

> > This will make config calls to the gpiochip call into the pinctrl backend = what
> > you want.

I still think this is what you want.

Yours,
Linus Walleij