Re: [PATCH v1 1/2] pinctrl: Add pinmux & GPIO controller driver for new SoC

From: Linus Walleij
Date: Wed Oct 16 2019 - 08:05:29 EST


On Thu, Oct 10, 2019 at 6:35 AM Tanwar, Rahul
<rahul.tanwar@xxxxxxxxxxxxxxx> wrote:

> My understanding is
> that GPIO_GENERIC can support a max of 64 consecutive bits representing
> GPIO lines.

Correct, it also demand that all GPIOs are accessed in a single
register of 8, 16, 32 or 64 bits.

> We have more than 100 GPIO's and they are spread out across
> 4 different banks with non consecutive registers i.e. DATA_IN_0~31@offset0x0,
> DATA_IN_32~63@offset0x100 and so on. In other words, i think we can not
> support memory mapped GPIO controller.

But why can't you just create one device and thus one gpio_chip
per bank?

This is what most drivers do, it also often makes things easier.

The main reason for not doing this is usually something like
that some registers are shared inbetween the banks, or their
registers are mingled in the same MMIO region or so.

If they are just in different memory chunks then create
one device per bank, and all gets much simpler.

> As mentioned above, we cannot use GPIO_GENERIC. And there was no specific
> reason/motivation for us to use regmap-mmio because most of the driver's
> that we referenced were using readl()/write(). Do you recommend us to remove
> readl()/writel() and switch to regmap-mmio based design ?

I don't really know, usually whatever makes for the simplest
code. But check if you can use GPIO_GENERIC first.

> Just to clarify, we have 4 different GPIO banks and each GPIO bank is
> implemented as a separate gpio_chip. So we have 4 instances of gpio_desc
> each one having its own gpio_chip. What i mean to say is that gpio_desc
> is actually not a per-line state container, its a per gpio_chip container.

As you're already creating one gpio_chip per bank using GPIO_GENERIC
should be simple.

Just reference the memory cells directly when calling bgpio_init() for each
mmio address, you're already half done.

Yours,
Linus Walleij