Re: [PATCH 2/2] gpio: Support cascaded GPIO chip lookup for OF

[Pantelis Antoniou <pantelis.antoniou@konsulko.com>]

Hi Rob,

> On Jun 8, 2016, at 18:18 , Rob Herring <robherring2@gmail.com> wrote:
> 
> On Wed, Jun 8, 2016 at 8:41 AM, Pantelis Antoniou
> <pantelis.antoniou@konsulko.com> wrote:
>> Hi Rob,
>> 
>>> On Jun 8, 2016, at 00:00 , Rob Herring <robherring2@gmail.com> wrote:
>>> 
>>> +Mark R
>>> 
>>> On Fri, Jun 3, 2016 at 3:26 PM, Pantelis Antoniou
>>> <pantelis.antoniou@konsulko.com> wrote:
>>>> In certain cases it makes sense to create cascaded GPIO which
>>>> are not real GPIOs, merely point to the real backend GPIO chip.
>>> 
>>> In what cases? Make it clear what this is for. Connectors of course,
>>> but are there any other use cases you have in mind.
>>> 
>> 
>> Connectors is one obvious use-case. In fact even when there is no
>> connector but there is an obvious interface abstraction point you
>> might want to use it.
>> 
>> For instance a SoC package may have a number of different GPIO
>> controllers (that may or may not use the same IP block). You could
>> abstract all the gpio controllers away in a single GPIO controller
>> block.
> 
> There had better be some good reason besides just wanting to make a
> single virtual controller.
> 

The reason is to forward a gpio reference to a real h/w gpio device
while not having to expose said real gpio device details. 

>>>> In order to support this, cascaded of_xlate lookup need to be
>>>> performed.
>>>> 
>>>> For example let's take a connector that we want to abstract
>>>> having two GPIO pins from different GPIO controllers, connector
>>>> pin #0 connected to gpioA controller with offset 10 and gpioB
>>>> with 4.
>>> 
>>> A connector's GPIO number may or may not be related to connector pins.
>>> 
>> 
>> Obviously, this is just an example.
>> 
>>>> In pseudo DT form this is analogous to:
>>>> 
>>>>       gpioA: gpioa@80000 {
>>>>               compatible = "foocorp,gpio";
>>>>               ...
>>>>       };
>>>> 
>>>>       gpioB: gpiob@80800 {
>>>>               compatible = "foocorp,gpio";
>>>>               ....
>>>>       };
>>>> 
>>>>       gpioC: controller_gpio {
>>>>               compatible = "cascaded-gpio";
>>> 
>>> This compatible is kind of meaningless. I'd expect this to be a
>>> connector compatible.
>>> 
>> 
>> No, because this gpio patch is completely independent of the
>> existence of a connector.
> 
> My point is that "cascaded-gpio" is not something used to parse the
> binding and will never be an accepted compatible string. I know it is
> an example, but you should make that obvious like foocorp is.
> 

It doesn’t parse the binding; the xlate method of a driver does.  

>>>>               gpios = <&gpioA 10>, <&gpioB 5>;
>>> 
>>> As we discussed at ELC, I think this should be modeled after
>>> interrupt-map property like this:
>>> 
>>> gpio-map = <0 0 &soc_gpio 10 0>, <1 0 &soc_gpio 5 0>;
>>> gpio-map-mask = <0xff 0>;
>>> 
>>> This is more flexible, a known pattern, and allows remapping of flag cells.
>>> 
>> 
>> It’s just syntactic sugar. It can work either way.
>> 
>>> Also, we will likely have interrupt capable GPIOs, so they are going
>>> to need interrupt-map anyway.
>>> 
>> 
>> Devices that use interrupts usually convert the GPIO to an interrupt and use it.
>> Since the xlat op will return the real GPIO spec the interrupt conversion will work.
>> 
>> Bare interrupt lines are sort-of out of fashion nowadays I think. I’m eager to be
>> proven wrong though with a recent portable expansion board that uses them.
> 
> Uh, no! Practically every gpio controller is also an interrupt
> controller (in DT) and devices pretty much always see just an
> interrupt line. Just go look at all the I2C devices with an interrupt
> line. Unless devices have some special needs to control the gpio, we
> always use interrupts. Expansion boards may be dealing with the GPIO
> simply because that is the only option for userspace drivers.
> 

Looking into a list of 54 capes for the beaglebone I can only find a single
one that uses an interrupt instead of a gpio, and that can easily be converted
to using the gpio.

The IRQ forwarding case is easier than the GPIO one TBH. The IRQ parsing core
can handle complex cases so adding a new one won’t be a big deal. 

> Rob

Regards

— Pantelis