diff mbox

[0/5] GPIO OMAP driver changes for v3.16

Message ID CABxcv=miFY3gE6HOU5ovWwCzgC6Un3ANh_Weh_bPCb7OoYQ+pA@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Javier Martinez Canillas April 10, 2014, 8:17 p.m. UTC 
Hello Aaro,

Thanks a lot for testing the series!

On Thu, Apr 10, 2014 at 9:30 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> Hi,
>
> On Thu, Apr 10, 2014 at 07:29:26PM +0200, Linus Walleij wrote:
>> On Sun, Apr 6, 2014 at 4:58 PM, Javier Martinez Canillas
>> <javier.martinez@collabora.co.uk> wrote:
>>
>> > Now that you have sent your changes for v3.15 to Torvalds, here are some
>> > changes for the OMAP GPIO driver targeted to v3.16. Mostly improvements
>> > so nothing here is -rc material.
>>
>> I like this series so I have applied them for v3.16, pending some ACK
>> from Kevin &| Santosh.
>
> I tried these patches on OMAP1 on top of today's Torvalds git
> (4ba85265790ba3681deeaf73f018c0eb829a7341).
>
> On Amstrad E3 I'm getting the following logs:
>
> [    0.156491] omap_gpio omap_gpio.0: Runtime PM disabled, clock forced on.
> [    0.164604] genirq: Setting trigger mode 0 for irq 64 failed (gpio_irq_type+0x0/0x1f0)
> [    0.165418] genirq: Setting trigger mode 0 for irq 65 failed (gpio_irq_type+0x0/0x1f0)
> [    0.166133] genirq: Setting trigger mode 0 for irq 66 failed (gpio_irq_type+0x0/0x1f0)
> [    0.166838] genirq: Setting trigger mode 0 for irq 67 failed (gpio_irq_type+0x0/0x1f0)
> [...]
> [    0.182856] genirq: Setting trigger mode 0 for irq 79 failed (gpio_irq_type+0x0/0x1f0)
> [    0.186887] omap_gpio omap_gpio.1: Could not get gpio dbck
> [    0.189308] genirq: Setting trigger mode 0 for irq 95 failed (gpio_irq_type+0x0/0x1f0)
> [...]
> [    0.203121] genirq: Setting trigger mode 0 for irq 110 failed (gpio_irq_type+0x0/0x1f0)
>
> However it still seems to work. The serio is only GPIO IRQ and it
> triggers when I press the external keyboard.
>
> The same happens also on Nokia 770:
>
> [    0.118896] genirq: Setting trigger mode 0 for irq 128 failed (gpio_irq_type+0x0/0x220)
> [    0.119201] genirq: Setting trigger mode 0 for irq 129 failed (gpio_irq_type+0x0/0x220)
> [...]
> [    0.124999] genirq: Setting trigger mode 0 for irq 143 failed (gpio_irq_type
> +0x0/0x220)
> [    0.126831] omap_gpio omap_gpio.1: Could not get gpio dbck
> [    0.127258] OMAP GPIO hardware version 1.1
> [    0.127624] omap_gpio omap_gpio.2: Could not get gpio dbck
> [    0.128204] omap_gpio omap_gpio.3: Could not get gpio dbck
> [    0.128753] omap_gpio omap_gpio.4: Could not get gpio dbck
>
> Here also GPIO IRQs (touchscreen, Retu) still work.
>
> A.

I don't have those errors when booting on my DM3730 IGEPv2 board but
it seems that for some reason on omap1  __irq_set_trigger() complains
when IRQ_TYPE_NONE is used as a default flag when calling
gpiochip_irqchip_add()

Could you please test the following patch and tell me if your board
still works and makes the errors go away?

%d\n", ret);

Best regards,
Javier

Comments

Aaro Koskinen April 10, 2014, 9:22 p.m. UTC | #1 
Hi,

On Thu, Apr 10, 2014 at 10:17:44PM +0200, Javier Martinez Canillas wrote:
> > The same happens also on Nokia 770:
> >
> > [    0.118896] genirq: Setting trigger mode 0 for irq 128 failed (gpio_irq_type+0x0/0x220)
> 
> I don't have those errors when booting on my DM3730 IGEPv2 board but
> it seems that for some reason on omap1  __irq_set_trigger() complains
> when IRQ_TYPE_NONE is used as a default flag when calling
> gpiochip_irqchip_add()
> 
> Could you please test the following patch and tell me if your board
> still works and makes the errors go away?

Now it complains about mode 8...

[    0.118835] genirq: Setting trigger mode 8 for irq 128 failed (gpio_irq_type
+0x0/0x220)

A.

> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 8cc9e91..5bc8aec 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1122,7 +1122,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank)
> 
>         ret = gpiochip_irqchip_add(&bank->chip, &gpio_irq_chip,
>                                    irq_base, gpio_irq_handler,
> -                                  IRQ_TYPE_NONE);
> +                                  IRQ_TYPE_LEVEL_LOW);
> 
>         if (ret) {
>                 dev_err(bank->dev, "Couldn't add irqchip to gpiochip
> %d\n", ret);
> 
> Best regards,
> Javier
Javier Martinez Canillas April 11, 2014, 3:03 p.m. UTC | #2 
Hello Aaro,

On Thu, Apr 10, 2014 at 11:22 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> Hi,
>
> On Thu, Apr 10, 2014 at 10:17:44PM +0200, Javier Martinez Canillas wrote:
>> > The same happens also on Nokia 770:
>> >
>> > [    0.118896] genirq: Setting trigger mode 0 for irq 128 failed (gpio_irq_type+0x0/0x220)
>>
>> I don't have those errors when booting on my DM3730 IGEPv2 board but
>> it seems that for some reason on omap1  __irq_set_trigger() complains
>> when IRQ_TYPE_NONE is used as a default flag when calling
>> gpiochip_irqchip_add()
>>
>> Could you please test the following patch and tell me if your board
>> still works and makes the errors go away?
>
> Now it complains about mode 8...
>
> [    0.118835] genirq: Setting trigger mode 8 for irq 128 failed (gpio_irq_type
> +0x0/0x220)
>
> A.
>
>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> index 8cc9e91..5bc8aec 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -1122,7 +1122,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank)
>>
>>         ret = gpiochip_irqchip_add(&bank->chip, &gpio_irq_chip,
>>                                    irq_base, gpio_irq_handler,
>> -                                  IRQ_TYPE_NONE);
>> +                                  IRQ_TYPE_LEVEL_LOW);
>>
>>         if (ret) {
>>                 dev_err(bank->dev, "Couldn't add irqchip to gpiochip
>> %d\n", ret);
>>
>> Best regards,
>> Javier

Thanks for testing. Unfortunately I'm out of ideas on why that error
could be shown and I don't have a way to further debug it without an
omap1 board. I wonder why that pr_err() message is shown or why it is
still working when an error happens.

Maybe Linus or Santosh could give us a hint on what is happening here.

Best regards,
Javier
Linus Walleij April 22, 2014, 1 p.m. UTC | #3 
On Fri, Apr 11, 2014 at 5:03 PM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Aaro,
>
> On Thu, Apr 10, 2014 at 11:22 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
>> Hi,
>>
>> On Thu, Apr 10, 2014 at 10:17:44PM +0200, Javier Martinez Canillas wrote:
>>> > The same happens also on Nokia 770:
>>> >
>>> > [    0.118896] genirq: Setting trigger mode 0 for irq 128 failed (gpio_irq_type+0x0/0x220)
>>>
>>> I don't have those errors when booting on my DM3730 IGEPv2 board but
>>> it seems that for some reason on omap1  __irq_set_trigger() complains
>>> when IRQ_TYPE_NONE is used as a default flag when calling
>>> gpiochip_irqchip_add()
>>>
>>> Could you please test the following patch and tell me if your board
>>> still works and makes the errors go away?
>>
>> Now it complains about mode 8...
>>
>> [    0.118835] genirq: Setting trigger mode 8 for irq 128 failed (gpio_irq_type
>> +0x0/0x220)
>>
>> A.
>>
>>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>>> index 8cc9e91..5bc8aec 100644
>>> --- a/drivers/gpio/gpio-omap.c
>>> +++ b/drivers/gpio/gpio-omap.c
>>> @@ -1122,7 +1122,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank)
>>>
>>>         ret = gpiochip_irqchip_add(&bank->chip, &gpio_irq_chip,
>>>                                    irq_base, gpio_irq_handler,
>>> -                                  IRQ_TYPE_NONE);
>>> +                                  IRQ_TYPE_LEVEL_LOW);
>>>
>>>         if (ret) {
>>>                 dev_err(bank->dev, "Couldn't add irqchip to gpiochip
>>> %d\n", ret);
>>>
>>> Best regards,
>>> Javier
>
> Thanks for testing. Unfortunately I'm out of ideas on why that error
> could be shown and I don't have a way to further debug it without an
> omap1 board. I wonder why that pr_err() message is shown or why it is
> still working when an error happens.
>
> Maybe Linus or Santosh could give us a hint on what is happening here.

Isn't an edge IRQ more apropriate as default then?

The code contains this:

    if (!bank->regs->leveldetect0 &&
        (type & (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH)))
        return -EINVAL;

Meaning sometimes the banks don't support level IRQs.

Yours,
Linus Walleij
Javier Martinez Canillas April 23, 2014, 9:48 p.m. UTC | #4 
Hello Aaro,

On Thu, Apr 10, 2014 at 11:22 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> Hi,
>
> On Thu, Apr 10, 2014 at 10:17:44PM +0200, Javier Martinez Canillas wrote:
>> > The same happens also on Nokia 770:
>> >
>> > [    0.118896] genirq: Setting trigger mode 0 for irq 128 failed (gpio_irq_type+0x0/0x220)
>>
>> I don't have those errors when booting on my DM3730 IGEPv2 board but
>> it seems that for some reason on omap1  __irq_set_trigger() complains
>> when IRQ_TYPE_NONE is used as a default flag when calling
>> gpiochip_irqchip_add()
>>
>> Could you please test the following patch and tell me if your board
>> still works and makes the errors go away?
>
> Now it complains about mode 8...
>
> [    0.118835] genirq: Setting trigger mode 8 for irq 128 failed (gpio_irq_type
> +0x0/0x220)
>
> A.
>

On thread [1] was reported a regression on a Sitara AM335x board
because the irq .map function handler used on the gpiolib called
irq_set_type() for each pin on the bank.

In the case of that board, a GPIO pin is used to control the SDRAM
termination regulator and calling irq_set_type() sets the GPIO as
input thus making the board to fail on boot. But this may also explain
why you had those error logs on your OMAP1 board when applying these
patches since that changes GPIO OMAP semantics when the driver
expected irq_set_type() to only be called when a IRQ is requested.

After some discussion Linus proposed a patch [2] which I think should
also fix your issue. Can you please give it a try and provide your
Tested-by for that patch?

Thanks a lot and best regards,
Javier

[1]: http://marc.info/?t=139817273800014&r=1&w=2
[2] https://patchwork.kernel.org/patch/4041881/
diff mbox

Patch

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 8cc9e91..5bc8aec 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1122,7 +1122,7 @@  static int omap_gpio_chip_init(struct gpio_bank *bank)

        ret = gpiochip_irqchip_add(&bank->chip, &gpio_irq_chip,
                                   irq_base, gpio_irq_handler,
-                                  IRQ_TYPE_NONE);
+                                  IRQ_TYPE_LEVEL_LOW);

        if (ret) {
                dev_err(bank->dev, "Couldn't add irqchip to gpiochip