Re: [WIP] Port xilinx gpio to GPIO_GENERIC

[Brandon Maier <brandon.maier@xxxxxxxxxxxxxxxxxxx>]

On Sat, Aug 11, 2018 at 2:37 PM, Hedges  Alexander
<ahedges@xxxxxxxxxxxxxxx> wrote:
> Hi,
>
> This is my first try switching gpio-xilinx to GPIO_GENERIC. Most of the work
> here is based on the EP93xx GPIO which is the only example I found using
> multiple banks and bgpio_init.
>
> The way this sets the gpio up is that with respect to the device tree, both
> banks can be addressed in the same &gpio (I hope, I havent tested addressing the
> second bank yet, mainly because no leds are mapped to it yet). But in the sysfs
> (in /sys/class/gpio/), there are two entries, one for each bank.


Aren't we going to have the same problem? Both gpiochips are still
under the same devicetree phandle, so there's no way to select which
"bank" we want. This patch hardcodes each chip's base to 0 or 32, but
this won't work because to my knowledge there's no way to fetch a
devicetree gpio by its base (And manually setting the gpio base is
considered "deprecated" according to the description of struct
gpio_chip).

Unfortunately the way mmgpio is written now, I don't see a way we can
have two mmgpio's share a single gpio_chip. We'd need to rework mmgpio
to decouple all the gpio handling from the gpio_chip.

Probably the simplest solution would be to add a sub-node in the
devicetree and register the second bank under that. The node/phandle
of a gpiochip can be changed by overriding the gpio_chip's of_node
before registration.

...
> +       status = bgpio_init(gc, dev, 4,
> +                           mmio_base + XGPIO_DATA_OFFSET,
> +                           NULL,
> +                           NULL,
> +                           mmio_base + XGPIO_TRI_OFFSET,
> +                           NULL,
> +                           0);
> +       if (status) {
> +               return status;
> +       }
> +       gc->label = bank_name[bank];
> +       gc->ngpio = gpio_width;
> +       gc->base = base;
> +       gc->bgpio_data = gpio_state;
> +       gc->bgpio_dir = gpio_dir;

The state of bgpio_data and bgpio_dir don't get written to hardware
until a user sets a gpio in the respective bank. So we'll still need
something like xgpio_save_regs() to program the initial state.