Re: [PATCH 2/2] gpio: add support for GPIO controller on Siflower SoCs

[Chuanhong Guo <gch981213@gmail.com>]

Hi Linus!

On Sat, Dec 28, 2024 at 12:52 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Hi Chuanhong,
>
> thanks for your patch!
>
> I think it is better to split the driver instances into 4, one for each
> physical block, as explained in the binding patch.
>
> On Wed, Dec 25, 2024 at 4:59 AM Chuanhong Guo <gch981213@gmail.com> wrote:
>
> > From: Qingfang Deng <qingfang.deng@siflower.com.cn>
> >
> > Add a driver for the GPIO controller on Siflower SoCs.
> > This controller is found on all current Siflower MIPS and RISC-V
> > chips including SF19A2890, SF21A6826 and SF21H8898.
> >
> > Signed-off-by: Qingfang Deng <qingfang.deng@siflower.com.cn>
> > Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
>
>
>
> > +config GPIO_SIFLOWER
> > +       tristate "SiFlower GPIO support"
> > +       depends on OF_GPIO
> > +       depends on MIPS || RISCV || COMPILE_TEST
> > +       select GPIOLIB_IRQCHIP
>
> select GPIO_GENERIC
>
> As you have only a set of 32-bit registers to handle for each
> instance, then some IRQs on top, you can untilize the MMIO
> library.
>
> > +#define GPIO_IR(n)     (0x40 * (n) + 0x00)
> > +#define GPIO_OR(n)     (0x40 * (n) + 0x04)
> > +#define GPIO_OEN(n)    (0x40 * (n) + 0x08)
> > +#define GPIO_IMR(n)    (0x40 * (n) + 0x0c)
> > +#define GPIO_GPIMR(n)  (0x40 * (n) + 0x10)
> > +#define GPIO_PIR(n)    (0x40 * (n) + 0x14)
> > +#define GPIO_ITR(n)    (0x40 * (n) + 0x18)
> > +#define GPIO_IFR(n)    (0x40 * (n) + 0x1c)
> > +#define GPIO_ICR(n)    (0x40 * (n) + 0x20)

This set of registers is for a single pin, not a multiple-pin block.

> > +#define GPIO_GPxIR(n)  (0x4 * (n) + 0x4000)
>
> Just define GPIO_IR 0, GPIO_OR 4, etc.
>
> Look up the GPIO_GPIR register separately from the
> device tree and use it without offset.
>
> Always register 16 GPIO lines unless ngpios is set.
>
> Look at example drivers such as
> drivers/gpio/gpio-pl061.c
> drivers/gpio/gpio-ftgpio010.c
> on how to use the MMIO helper library to implement
> a simple driver for one instance reusing the common code.

In my case the MMIO library doesn't seem really helpful since that's
for the more common case where multiple GPIO pins share one set of
registers.

-- 
Regards,
Chuanhong Guo