OMAP5912 boot broken by "gpio/omap: don't create an IRQ mapping for every GPIO on DT"

[Santosh Shilimkar]

Javier,

On Monday 29 July 2013 05:39 AM, Javier Martinez Canillas wrote:
> On 07/29/2013 11:19 AM, Javier Martinez Canillas wrote:
>> On 07/29/2013 11:01 AM, Linus Walleij wrote:
>>> Hi Paul,
>>>
>>> On Mon, Jul 29, 2013 at 9:52 AM, Paul Walmsley <paul at pwsan.com> wrote:
>>>
>>>> your commit 0e970cec05635adbe7b686063e2548a8e4afb8f4 ("gpio/omap: don't
>>>> create an IRQ mapping for every GPIO on DT") breaks the boot on the
>>>> OMAP5912 OSK:
>>>
>>> I'm contemplating just reverting this whole series, as I didn't like
>>> the approach from the beginning and it has exploded in exactly
>>> the way I thought it would.
>>>
>>> If we revert these three patches:
>>>
>>> commit 949eb1a4d29dc75e0b5b16b03747886b52ecf854
>>> "gpio/omap: fix build error when OF_GPIO is not defined."
>>> commit b4419e1a15905191661ffe75ba2f9e649f5d565e
>>> "gpio/omap: auto request GPIO as input if used as IRQ via DT"
>>> commit 0e970cec05635adbe7b686063e2548a8e4afb8f4
>>> "gpio/omap: don't create an IRQ mapping for every GPIO on DT"
>>>
>>> Does the OMAP1 boot again after this?
>>>
>>> I think it's a way better idea to proceed with input-hogs on the gpiochip
>>> DT node and use that to get auto-request on the GPIO lines that
>>> will be used as IRQs only.
>>>
>>> Yours,
>>> Linus Walleij
>>>
>>
>> Hi Paul,
>>
>> I've looked at this and it seems that irq_create_mapping() does not call the
>> irq_domain_ops .map function handler since OMAP1 still uses legacy domain
>> mapping. I don't have an OMAP1 platform to test but could you please see if the
>> following patch [1] makes your OMAP1 platforms to boot again?
>>
>> But I agree with Linus and probably we should just go and revert the whole
>> series since it is very hard to get it right. In another thread a user reported
>> that this change also broke his DTS tree.
>>
>> I really tried to get this right without breaking anything but there are just
>> too many OMAP platforms behaving differently and most OMAP drivers are only half
>> converted to DT so this is really a can of worms.
>>
>> Thanks a lot and sorry for the inconvenience,
>> Javier
>>
>> [1]:
>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> index c57244e..f1c6da8 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -1090,8 +1090,18 @@ static void omap_gpio_chip_init(struct gpio_bank *bank)
>>          * are used as interrupts.
>>          */
>>         if (!omap_gpio_chip_boot_dt(&bank->chip))
>> -               for (j = 0; j < bank->width; j++)
>> -                       irq_create_mapping(bank->domain, j);
>> +               for (j = 0; j < bank->width; j++) {
>> +                       int irq = irq_create_mapping(bank->domain, j);
>> +                       irq_set_lockdep_class(irq, &gpio_lock_class);
>> +                       irq_set_chip_data(irq, bank);
>> +                       if (bank->is_mpuio) {
>> +                               omap_mpuio_alloc_gc(bank, irq, bank->width);
>> +                       } else {
>> +                               irq_set_chip_and_handler(irq, &gpio_irq_chip,
>> +                                                        handle_simple_irq);
>> +                               set_irq_flags(irq, IRQF_VALID);
>> +                       }
>> +               }
>>         irq_set_chained_handler(bank->irq, gpio_irq_handler);
>>         irq_set_handler_data(bank->irq, bank);
>>
> 
> In case this solves Paul issue, a cleaned patch with a commit message is [2].
> But we should decide if is better to fix this or just drop the patches and go
> with Linus' input-hogs idea to do the GPIO auto request.
> 
> Santosh, Kevin, Grant, what do you think we should do?
> 
With some helps from MMC and other guys, we validated the Linus's tip which includes
your patches. It actually doesn't break anything and as OMAP hsmmc maintainer
clarified, the cd-gpios isn't supported yet for DT. While supporting that it
can use appropriate binding whichever works.

But with OMAP1 breakage reported by Paul, I think we are not left with choice
but to revert those commits. We *must* respect rc rules for the fixes.
*No regression*

Thanks for your hardwork to cook up those patches but now Linus's W proposal
is going to be generic, hopefully the issue can be address better. Till
then we can't get the ethernet support.

Linus,
I guess you have already gone ahead on revert path as seen from other email.
Lets just make that official. We are sorry for cause you trouble over a
week-end.

Regards,
Santosh