Message ID | 1364631689.3767.7.camel@mars (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Sat, Mar 30, 2013 at 9:21 AM, Christoph Fritz <chf.fritz@googlemail.com> wrote: > This patch sets gpio #interrupt-cells from a falsely acquired '1' to '2' > referring to Documentation/devicetree/bindings/gpio/gpio-omap.txt: > > The first cell is the GPIO number. > The second cell is used to specify flags: > bits[3:0] trigger type and level flags: > 1 = low-to-high edge triggered. > 2 = high-to-low edge triggered. > 4 = active high level-sensitive. > 8 = active low level-sensitive. > > But using this trigger cell in a board specific devicetree leads to a > non-starting kernel. This is due to not yet enabled gpio-clocks while > kernel/irq/irqdomain.c tries to set this trigger-flag (from the second > interrupt-cell) to gpio-irq-controller. > > Any ideas? Hi Christoph, A call to gpio_request() to enable the GPIO bank is needed before using a GPIO as an IRQ source, otherwise accesses to the GPIO bank registers fails making the kernel to hang. Jon's (added as cc) "gpio/omap: warn if bank is not enabled on setting irq type" patch [1] fixes the issue by warning and returning -EINVAL. This patch will make the kernel to boot but the call to request_irq() will fail of course. For now, the only solution is to call gpio_request() before request_irq() in your platform code or device driver. There is an on going discussion about what's the better way to address this but we still haven't found a good solution to this problem, you can see the last email for this discussion here [2] Also, even when calling gpio_request() before request_irq() this won't work. When specifying the trigger/level flags on the second cell for an GPIO-IRQ, this is not set on the IORESOURCE_IRQ struct resource. The IRQ flag is set on of_irq_to_resource() but it just does: r->flags = IORESOURCE_IRQ; and then the call stack is irq_to_parse_and_map() -> irq_set_irq_type() -> __irq_set_trigger() -> chip->irq_set_type() -> (drivers/gpio/gpio-omap.c) gpio_irq_type(). So, even when gpio_irq_type() receive the correct flags, this are not returned neither stored on the flags member of the IORESOURCE_IRQ struct resource that passed to the drivers in their struct platform_device. I have on my TODO to better investigate if this behavior is intentional or is a bug in the interrupt core but didn't have time to work on this yet. A relevant discussion about this is here [3]. > --- > arch/arm/boot/dts/omap3.dtsi | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi > index 1997b41..e8e6b8f 100644 > --- a/arch/arm/boot/dts/omap3.dtsi > +++ b/arch/arm/boot/dts/omap3.dtsi > @@ -99,7 +99,7 @@ > gpio-controller; > #gpio-cells = <2>; > interrupt-controller; > - #interrupt-cells = <1>; > + #interrupt-cells = <2>; > }; > > gpio2: gpio@49050000 { > @@ -108,7 +108,7 @@ > gpio-controller; > #gpio-cells = <2>; > interrupt-controller; > - #interrupt-cells = <1>; > + #interrupt-cells = <2>; > }; > > gpio3: gpio@49052000 { > @@ -117,7 +117,7 @@ > gpio-controller; > #gpio-cells = <2>; > interrupt-controller; > - #interrupt-cells = <1>; > + #interrupt-cells = <2>; > }; > > gpio4: gpio@49054000 { > @@ -126,7 +126,7 @@ > gpio-controller; > #gpio-cells = <2>; > interrupt-controller; > - #interrupt-cells = <1>; > + #interrupt-cells = <2>; > }; > > gpio5: gpio@49056000 { > @@ -135,7 +135,7 @@ > gpio-controller; > #gpio-cells = <2>; > interrupt-controller; > - #interrupt-cells = <1>; > + #interrupt-cells = <2>; > }; > > gpio6: gpio@49058000 { > @@ -144,7 +144,7 @@ > gpio-controller; > #gpio-cells = <2>; > interrupt-controller; > - #interrupt-cells = <1>; > + #interrupt-cells = <2>; > }; > > uart1: serial@4806a000 { > -- > 1.7.10.4 > > > By the way, Jon has already sent a "ARM: dts: OMAP3+: Correct gpio #interrupts-cells property" patch [4] that changes #interrupt-cells to <2> for all OMAP platforms. Best regards, Javier [1]: https://patchwork.kernel.org/patch/2202511/ [2]: http://www.spinics.net/lists/linux-omap/msg89247.html [3]: https://patchwork.kernel.org/patch/2194911/ [4]: https://patchwork.kernel.org/patch/2278081/
Hi Javier On Sat, 2013-03-30 at 14:18 +0100, Javier Martinez Canillas wrote: > A call to gpio_request() to enable the GPIO bank is needed before > using a GPIO as an IRQ source, otherwise accesses to the GPIO bank > registers fails making the kernel to hang. Yes, that is exactly my problem here. I'm using the GPIO as an IRQ source. > Jon's (added as cc)"gpio/omap: warn if bank is not enabled on setting > irq type" patch [1] fixes the issue by warning and returning -EINVAL. > > This patch will make the kernel to boot but the call to request_irq() > will fail of course. For now, the only solution is to call > gpio_request() before request_irq() in your platform code or device > driver. There is an on going discussion about what's the better way to > address this but we still haven't found a good solution to this > problem, you can see the last email for this discussion here [2] > > Also, even when calling gpio_request() before request_irq() this won't > work. When specifying the trigger/level flags on the second cell for > an GPIO-IRQ, this is not set on the IORESOURCE_IRQ struct resource. > The IRQ flag is set on of_irq_to_resource() but it just does: > > r->flags = IORESOURCE_IRQ; > > and then the call stack is irq_to_parse_and_map() -> > irq_set_irq_type() -> __irq_set_trigger() -> chip->irq_set_type() -> > (drivers/gpio/gpio-omap.c) gpio_irq_type(). > > So, even when gpio_irq_type() receive the correct flags, this are not > returned neither stored on the flags member of the IORESOURCE_IRQ > struct resource that passed to the drivers in their struct > platform_device. As a quick-fix (hack) I wrote directly to the registers in gpio_probe() to enable GPIO banks. I now geht this: > > [ 0.214630] omap_gpio_probe, 1133, CM_CLKSEL_PER 0x48005040: 0x000000ff > > [ 0.214660] omap_gpio_probe, 1136, CM_ICLKEN_PER 0x48005010: 0x0007ffff > > [ 0.214660] omap_gpio_probe, 1139, CM_FCLKEN_PER 0x48005000: 0x0007ffff And it works for me. _But_ when I do enable regulator twl4030 (CONFIG_REGULATOR_TWL4030=y) in my config these registers get reset: [ 2.935455] smsc911x_open, 1537, CM_CLKSEL_PER 0x48005040: 0x000000ff [ 2.942291] smsc911x_open, 1540, CM_ICLKEN_PER 0x48005010: 0x00040fff [ 2.949066] smsc911x_open, 1543, CM_FCLKEN_PER 0x48005000: 0x00000000 And the IRQ source for the network chip (smsc911x) is disabled :-( Do you have any idea how to ("quick") fix this? Thanks -- Christoph
diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi index 1997b41..e8e6b8f 100644 --- a/arch/arm/boot/dts/omap3.dtsi +++ b/arch/arm/boot/dts/omap3.dtsi @@ -99,7 +99,7 @@ gpio-controller; #gpio-cells = <2>; interrupt-controller; - #interrupt-cells = <1>; + #interrupt-cells = <2>; }; gpio2: gpio@49050000 { @@ -108,7 +108,7 @@ gpio-controller; #gpio-cells = <2>; interrupt-controller; - #interrupt-cells = <1>; + #interrupt-cells = <2>; }; gpio3: gpio@49052000 { @@ -117,7 +117,7 @@ gpio-controller; #gpio-cells = <2>; interrupt-controller; - #interrupt-cells = <1>; + #interrupt-cells = <2>; }; gpio4: gpio@49054000 { @@ -126,7 +126,7 @@ gpio-controller; #gpio-cells = <2>; interrupt-controller; - #interrupt-cells = <1>; + #interrupt-cells = <2>; }; gpio5: gpio@49056000 { @@ -135,7 +135,7 @@ gpio-controller; #gpio-cells = <2>; interrupt-controller; - #interrupt-cells = <1>; + #interrupt-cells = <2>; }; gpio6: gpio@49058000 { @@ -144,7 +144,7 @@ gpio-controller; #gpio-cells = <2>; interrupt-controller; - #interrupt-cells = <1>; + #interrupt-cells = <2>; }; uart1: serial@4806a000 {