On 04/15/2013 04:40 PM, Jon Hunter wrote:
>
> On 04/15/2013 11:58 AM, Stephen Warren wrote:
>> On 04/14/2013 02:53 PM, Linus Walleij wrote:
>>> On Sun, Apr 14, 2013 at 3:35 AM, Javier Martinez Canillas
>>> <martinez.javier@xxxxxxxxx> wrote:
>>>
>>>> Is the following inlined patch [1] what you were thinking that would
>>>> be the right approach?
>>>
>>> This looks sort of OK, but I'm still struggling with the question of
>>> what we could do to help other implementations facing the same issue.
>>>
>>> This is a pretty hard design pattern to properly replicate in every such
>>> driver is it not?
>>
>> Well, instead of adding .request_irq() to the irqchip, and then making
>> each driver call gpio_request() from the implementation, perhaps you
>> could add a .irq_to_gpio() to the irqchip, have the IRQ core call that,
>> and if it gets back a non-error response, the IRQ core could call
>> gpio_request(). That means that the change to each GPIO+IRQ driver is
>> simply to implement a standalone data transformation function
>> .irq_to_gpio().
>
> I am still concerned about the case where a driver may have already
> called gpio_request() and then calls request_irq(). I think that the
> solution needs to handle cases where the driver may or may not call
> gpio_request() to allocate the gpio.
>
> Although it could be argued that this is problem is not DT specific,
> it does become a bigger problem to handle in the case of DT. Therefore,
> I am wondering if we should just focus on the DT case for now.
>
>> Now, this does re-introduce irq_to_gpio() in some way, but with the
>> following advantages:
>>
>> 1) The implementation is per-controller, not a single global function,
>> so isn't introducing any kind of centralized mapping scheme again.
>>
>> 2) This irq-chip-specific .irq_to_gpio() would only be implemented for
>> IRQ+GPIO chips that actually have a 1:1 mapping between GPIOs and IRQs.
>> Its potential existence doesn't imply that all IRQ chips need implement
>> this; it would be very specifically be for this one particular case.
>>
>> So, I think it's reasonable to introduce this.
>
> How about using the gpio irq domain xlate function?
>
> Typically, in DT land a device using a gpio as an interrupt source
> will have something like the following ...
>
> eth@0 {
> compatible = "ks8851";
> ...
> interrupt-parent = <&gpio2>;
> interrupts = <2 8>; /* gpio line 34, low triggered */
> };
>
> ... or ...
>
> mmc {
> label = "pandaboard::status2";
> gpios = <&gpio1 8 0>;
> ...
> };
>
> Both these devices are using a gpio as an interrupt source, but the mmc
> driver is requesting the gpio directly. In the first case the xlate
> function for the gpio irq domain will be called where as it is not used
> in the 2nd case. Therefore, we could add a custom xlate function. For
> example ...
>
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 53bb8d5..caaeab2 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1085,6 +1085,33 @@ static void omap_gpio_chip_init(struct gpio_bank *bank)
> irq_set_handler_data(bank->irq, bank);
> }
>
> +
> +int omap_irq_domain_xlate(struct irq_domain *d, struct device_node *ctrlr,
> + const u32 *intspec, unsigned int intsize,
> + unsigned long *out_hwirq, unsigned int *out_type)
> +{
> + struct gpio_bank *bank;
> + int irq;
> +
> + if (WARN_ON(intsize < 1))
> + return -EINVAL;
> +
> + irq = irq_find_mapping(d, intspec[0]);
> + bank = irq_get_chip_data(irq);
> + if (!bank)
> + return -ENODEV;
> +
> + gpio_request_one(irq_to_gpio(bank, intspec[0]), GPIOF_IN, ctrlr->name);
By the way, I know that I should check the return code here, but this
was just an example. Also I don't think using ctrlr->name here works
either as this is just "gpio".
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
