Re: [WIP] Port xilinx gpio to GPIO_GENERIC

[Michal Simek <michal.simek@xxxxxxxxxx>]

On 14.8.2018 10:19, Alexander Hedges wrote:
> 
> 
> 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).

We are using 3 gpio cell format where one cell is used for bank address.
As I said I can't see any issue with allocating one chip with bank0 +
bank1 gpio pins.
I vote for not using sub-nodes in this case.

Thanks,
Michal