Re: [PATCH 3/5] gpio/omap: Add DT support to GPIO driver

[Stephen Warren <swarren@wwwdotorg.org>]

On 04/10/2013 12:12 PM, Linus Walleij wrote:
> On Fri, Mar 29, 2013 at 6:01 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 03/27/2013 02:55 PM, Linus Walleij wrote:
>>> On Wed, Mar 27, 2013 at 5:09 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> This is the case for some SMSC911x clients like the snowball
>>> routing it to a GPIO, whereas I think the RealView and Integrator/CP
>>> has a dedicated IRQ line for it on the FPGA so it's a good example.
>>>
>>> In such cases the right thing to do is for the platform
>>> code populating the platform data with the int irq field, or device tree
>>> core, or whatever piece of code that knows about
>>> the actual GPIO shall start from the GPIO, request it and
>>> then convert it to an IRQ.
>>
>> So board code could easily do that; plenty of opportunity to put
>> whatever custom code is needed right there.
>>
>> For DT core code to do that, we'd need some alternative way of
>> specifying interrupts. That would change the DT bindings for interrupts.
>> I don't think we can do that...
> 
> Sorry, I'm probably not knowledgeable about DT to understand this.
> The information for what GPIO corresponds to what IRQ is
> present in the device tree, is it not?

No, I don't think so. DT treats the IRQ and GPIO numbering spaces as
entirely unrelated things, just like the kernel APIs do.

The fact that some DT nodes (device/drivers) happen to be both GPIO and
IRQ providers (implement gpio_chip and irq_chip) and happen to use the
same numbering scheme for both GPIO and IRQ IDs is purely an
implementation detail within individual drivers (or rather, of specific
DT binding definitions really), and not a general truism.

> If the information is there, whether to convert from IRQ to GPIO
> or from GPIO to IRQ is a technicality and any order should be
> feasible in some way?

There isn't always a unique 1:1 mapping between GPIOs and IRQs. Put
another way, a single GPIO would likely only ever trigger a single IRQ,
but a single IRQ might easily be triggered by any number of GPIOs. This
is exactly why the function irq_to_gpio() isn't something one should use
any more. I think there was an effort to outright remove the API,
although it doesn't look like that's entirely complete yet. I guess you
know this given your mentions of problem with gpio_to_irq() later on, so
I'm not quite sure what your paragraph above was supposed to mean.

> I just can't get the necessity to do it one way preferred over the
> other through my head, sorry...
> 
>>> If it seems like identical boilerplate in every machine we can
>>> simply centralize it to gpiolib. Something like
>>>
>>> int gpio_add_single_irq_resource(struct platform_device *pdev, int
>>> gpio, unsigned flags)
>> ... [code to convert GPIO to IRQ and create IORESOURCE_IRQ from it]
>>
>>> int gpio_populate_irq_resource(struct platform_device *pdev, int gpio,
>>>                               unsigned flags, struct resource *r)
>> ... [code to create IORESOURCE_IRQ from the IRQ]
>>
>> ...
>>> gpio_populate_irq_resource(&ethernet, 17,
>>>      IORESOURCE_IRQ_HIGHEDGE, &ethernet_res[1]);
>>> platform_device_register(&ethernet);
>>>
>>> That populates the device with the single GPIO IRQ from
>>> the GPIO number in a smooth way.
>>>
>>> Of course it has to be done after the GPIO driver is up
>>> and running so may require some other refactoring,
>>> but it should work.
>>
>> That latter issue also somewhat scuppers this approach; then you have to
>> somehow run a bunch of your board file code inter-mixed with various
>> driver probe() calls. That will quickly get nasy.
> 
> No, just use a module_init() for the above code in the board
> file and it will defer just like any other initcall.

Using initcalls to order things is rather fragile. Perhaps it could work
out OK with board files, but it's certainly something that's frowned
upon now for DT-based systems, since deferred probe solves it much more
generally, and without any maintenance issues (e.g. continual
re-shuffling of the initcall order, not having enough initcall levels if
you end up with more dependencies being added, etc.).

>> And it doesn't address how the DT core will know when to call
>> gpio_add_single_irq_resource() vs. gpio_populate_irq_resource() without
>> changing the DT binding for interrupts.
> 
> Is it not possible to do this in
> drivers/gpio/gpiolib-of.c: of_gpiochip_add(), if the DT *know*
> which GPIOs will be used as plain IRQs? That is the point with
> the gpio hogs I talk about ...

I suppose that if you modified the device tree bindings (schema
definitions), you could have every GPIO provider DT node indicate which
GPIOs were used as IRQs instead. However,

a) This would be an incompatible change to the DT bindings, and they're
supposed to be a stable ABI; old DTBs should work unmodified with new
kernels.

b) This would place GPIO usage information in multiple places. Right
now, GPIO consumers list which GPIO(s) they use. The same goes for IRQs.
If a GPIO provider started listing which GPIOs were really IRQs, then
that'd mean configuring each GPIO-as-an-IRQ in two places; once in the
client node to indicate which IRQ it should hook/register, and once in
the provider node to indicate that the GPIO is really an IRQ. Two places
is more hassle for people to implement, and more opportunity for mistakes.