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

[Jon Hunter <jon-hunter@ti.com>]

On 04/16/2013 05:11 PM, Stephen Warren wrote:
> On 04/16/2013 01:27 PM, Jon Hunter wrote:
>>
>> On 04/16/2013 01:40 PM, Stephen Warren wrote:
>>> On 04/15/2013 05:04 PM, Jon Hunter wrote:
> ...
>>>> If some driver is calling gpio_request() directly, then they will most
>>>> likely just call gpio_to_irq() when requesting the interrupt and so the
>>>> xlate function would not be called in this case (mmc drivers are a good
>>>> example). So I don't see that as being a problem. In fact that's the
>>>> benefit of this approach as AFAICT it solves this problem.
>>>
>>> Oh. That assumption seems very fragile. What about drivers that actually
>>> do have platform data (or DT bindings) that provide both the IRQ and
>>> GPIO IDs, and hence don't use gpio_to_irq()? That's entirely possible.
>>
>> Right. In the DT case though, if someone does provide the IRQ and GPIO
>> IDs then at least they would use a different xlate function. Another
>> option to consider would be defining the #interrupt-cells = <3> where we
>> would have ...
>>
>> cell-#1 --> IRQ domain ID
>> cell-#2 --> Trigger type
>> cell-#3 --> GPIO ID
>>
>> Then we could have a generic xlate for 3 cells that would also request
>> the GPIO. Again not sure if people are against a gpio being requested in
>> the xlate but just an idea. Or given that irq_of_parse_and_map() calls
>> the xlate, we could have this function call gpio_request() if the
>> interrupt controller is a gpio and there are 3 cells.
> 
> I rather dislike this approach since:
>
> a) It requires changes to the DT bindings, which are already defined.
> Admittedly it's backwards-compatible, but still.
> 
> b) There isn't really any need for the DT to represent this; the
> GPIO+IRQ driver itself already knows which IRQ ID is which GPIO ID and
> vice-versa (if the HW has such a concept), so there's no need for the DT
> to contain this information. This seems like pushing Linux's internal
> requirements into the design of the DT binding.

Yes, so the only alternative is to use irq_to_gpio to avoid this.

> c) I have the feeling that hooking the of_xlate function for this is a
> bit of an abuse of the function.

I was wondering about that. So I was grep'ing through the various xlate
implementations and found this [1]. Also you may recall that in the
of_dma_simple_xlate() we call the dma_request_channel() to allocate the
channel, which is very similar. However, I don't wish to get a
reputation as abusing APIs so would be good to know if this really is
abuse or not ;-)

Cheers
Jon

[1] http://permalink.gmane.org/gmane.linux.ports.arm.kernel/195124