On Mo, Aug 13, 2018 at 8:53 PM, Brandon Maier
<brandon.maier@xxxxxxxxxxxxxxxxxxx> wrote:
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).
Yes, that was wishful thinking, I tested it and the first GPIO bank can
be addressed but the second one can't in this setup. Just setting base
to -1 takes care of the base thing.
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.
Given that it is encouraged to switch to GPIO_GENERIC, it should
certainly have facilities to handle multi-bank gpios. I see myself
qualified to rewrite xilinx-gpio, but to be honest, changing mmgpio
which is used all over the place is something that I'm not comfortable
with, at my level of GPIO expertise.
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.
I guess this would be the quick fix, but it is up to Xilinx to change
the interpretation of their device tree (given that most device trees
are automatically generated by their tools).
...
+ 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.
Ok, thanks I'll do that in my next iteration.
