devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][BUG] arm/dts: OMAP3: set #interrupt-cells to two
@ 2013-03-30  8:21 Christoph Fritz
  2013-03-30 13:18 ` Javier Martinez Canillas
  0 siblings, 1 reply; 10+ messages in thread
From: Christoph Fritz @ 2013-03-30  8:21 UTC (permalink / raw)
  To: Benoît Cousson, Tony Lindgren, Russell King, linux-omap,
	devicetree-discuss, linux-arm-kernel, Tarun Kanti DebBarma,
	Daniel Mack, Hans J. Koch

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?
---
 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




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC][BUG] arm/dts: OMAP3: set #interrupt-cells to two
  2013-03-30  8:21 [RFC][BUG] arm/dts: OMAP3: set #interrupt-cells to two Christoph Fritz
@ 2013-03-30 13:18 ` Javier Martinez Canillas
  2013-04-01 16:41   ` Christoph Fritz
  0 siblings, 1 reply; 10+ messages in thread
From: Javier Martinez Canillas @ 2013-03-30 13:18 UTC (permalink / raw)
  To: Christoph Fritz
  Cc: Benoît Cousson, Tony Lindgren, Russell King,
	linux-omap@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, Tarun Kanti DebBarma,
	Daniel Mack, Hans J. Koch, Jon Hunter

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/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC][BUG] arm/dts: OMAP3: set #interrupt-cells to two
  2013-03-30 13:18 ` Javier Martinez Canillas
@ 2013-04-01 16:41   ` Christoph Fritz
  2013-04-01 20:05     ` Javier Martinez Canillas
  0 siblings, 1 reply; 10+ messages in thread
From: Christoph Fritz @ 2013-04-01 16:41 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Benoît Cousson, Tony Lindgren, Russell King,
	linux-omap@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, Daniel Mack, Hans J. Koch,
	Jon Hunter, Paul Walmsley, Kevin Hilman, Rajendra Nayak,
	Santosh Shilimkar

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC][BUG] arm/dts: OMAP3: set #interrupt-cells to two
  2013-04-01 16:41   ` Christoph Fritz
@ 2013-04-01 20:05     ` Javier Martinez Canillas
  2013-04-02 15:55       ` Christoph Fritz
  2013-04-13 17:42       ` Christoph Fritz
  0 siblings, 2 replies; 10+ messages in thread
From: Javier Martinez Canillas @ 2013-04-01 20:05 UTC (permalink / raw)
  To: Christoph Fritz
  Cc: Benoît Cousson, Tony Lindgren, Russell King,
	linux-omap@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, Daniel Mack, Hans J. Koch,
	Jon Hunter, Paul Walmsley, Kevin Hilman, Rajendra Nayak,
	Santosh Shilimkar

On Mon, Apr 1, 2013 at 6:41 PM, Christoph Fritz
<chf.fritz@googlemail.com> wrote:
> 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?
>

A quick hack is to call gpio_request() explicitly before calling to
irq_set_type() is made.
I've this patch just to make it work until we find a clean solution:

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 90c15ee..d594e1d 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -14,6 +14,7 @@
  */
 #undef DEBUG

+#include <linux/gpio.h>
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
@@ -1528,6 +1529,11 @@ static int gpmc_probe_dt(struct platform_device *pdev)
                return ret;
        }

+       ret = gpio_request_one(176, GPIOF_IN, "smsc911x irq");
+       if (ret) {
+               pr_err("Failed to request IRQ GPIO%d\n", 176);
+               return ret;
+       }
+
        for_each_node_by_name(child, "nand") {
                ret = gpmc_probe_nand_child(pdev, child);
                if (ret < 0) {

This solves the issue of the non-initialized GPIO bank before that
makes the kernel to hang. Since I've to configure the IRQ polarity as
active low level-sensitive on my board and the flags are not set by
the IRQ core, I've another ugly hack that forces this:

diff --git a/drivers/net/ethernet/smsc/smsc911x.c
b/drivers/net/ethernet/smsc/smsc
index da5cc9a..27e46f9 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2390,6 +2390,9 @@ static int smsc911x_drv_probe(struct
platform_device *pdev)
        pdata = netdev_priv(dev);
        dev->irq = irq_res->start;
-        irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
+       irq_flags = IRQF_TRIGGER_LOW;
        pdata->ioaddr = ioremap_nocache(res->start, res_size);

        pdata->dev = dev;

>  Thanks
>   -- Christoph
>

I hope to find some time this week to work on this and at least find a
solution for the second issue (IORESOURCE_IRQ struct resource flags
not being set).

Best regards,
Javier

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC][BUG] arm/dts: OMAP3: set #interrupt-cells to two
  2013-04-01 20:05     ` Javier Martinez Canillas
@ 2013-04-02 15:55       ` Christoph Fritz
  2013-04-02 16:38         ` Jon Hunter
  2013-04-13 17:42       ` Christoph Fritz
  1 sibling, 1 reply; 10+ messages in thread
From: Christoph Fritz @ 2013-04-02 15:55 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Benoît Cousson, Tony Lindgren, Russell King,
	linux-omap@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, Daniel Mack, Hans J. Koch,
	Jon Hunter, Paul Walmsley, Rajendra Nayak, Santosh Shilimkar

On Mon, 2013-04-01 at 22:05 +0200, Javier Martinez Canillas wrote:

> > 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

to be more specific on this point, this is the patch to enable the
gpio-clocks:

--
Subject: [PATCH] HACK: enable gpio-clocks in gpio-omap probe()

Without this patch setting trigger value from #interrupt-cell two
(smsc911x) fails.
---
 drivers/gpio/gpio-omap.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 159f5c5..720b2e6 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1098,6 +1098,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct gpio_bank *bank;
 	int ret = 0;
+	void __iomem *tmp;
 
 	match = of_match_device(of_match_ptr(omap_gpio_match), dev);
 
@@ -1117,6 +1118,17 @@ static int omap_gpio_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	// TRM: Table 3-242. PER_CM Register Summary
+	tmp = ioremap(0x48005040, 4); //CM_CLKSEL_PER, GPT2 = sys clk
+	writel(0xFF, tmp);
+	iounmap(tmp);
+	tmp = ioremap(0x48005010, 4); //CM_ICLKEN_PER, ICKen GPT2
+	writel(0x7FFFF, tmp);
+	iounmap(tmp);
+	tmp = ioremap(0x48005000, 4); //CM_FCLKEN_PER, GPIOX functional clock is enabled
+	writel(0x7FFFF, tmp);
+	iounmap(tmp);
+
 	bank->irq = res->start;
 	bank->dev = dev;
 	bank->dbck_flag = pdata->dbck_flag;
-- 
1.7.10.4

Is there a better way to do this?


> >
> > 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 :-(

CONFIG_REGULATOR_TWL4030=y  disables the gpio-clocks. Why is that?

> >
> > Do you have any idea how to ("quick") fix this?
> >
> 
> A quick hack is to call gpio_request() explicitly before calling to
> irq_set_type() is made.
> I've this patch just to make it work until we find a clean solution:
> 
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index 90c15ee..d594e1d 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -14,6 +14,7 @@
>   */
>  #undef DEBUG
> 
> +#include <linux/gpio.h>
>  #include <linux/irq.h>
>  #include <linux/kernel.h>
>  #include <linux/init.h>
> @@ -1528,6 +1529,11 @@ static int gpmc_probe_dt(struct platform_device *pdev)
>                 return ret;
>         }
> 
> +       ret = gpio_request_one(176, GPIOF_IN, "smsc911x irq");
> +       if (ret) {
> +               pr_err("Failed to request IRQ GPIO%d\n", 176);
> +               return ret;
> +       }
> +
>         for_each_node_by_name(child, "nand") {
>                 ret = gpmc_probe_nand_child(pdev, child);
>                 if (ret < 0) {
> 
> This solves the issue of the non-initialized GPIO bank before that
> makes the kernel to hang.

Here it does not. A printk shows that I'm not using gpmc at all.
So I added a gpmc node:

+       gpmc: gpmc@0x6E000000 {
+               compatible = "ti,omap3430-gpmc";
+               ti,hwmods = "ti,gpmc";
+               reg = <0x6E000000 0x2000>;
+               gpmc,num-cs = <8>;
+               gpmc,num-waitpins = <2>;
+               #address-cells = <2>;
+               #size-cells = <1>;
+               ranges = <0 0 0x0 0x3FFFFFFF>;
+
+       };

But still, gpmc_probe_dt() isn't called. I maybe have to define some
child nodes but currently I do configure the gpmc in u-boot and try to
avoid kernel-gpmc-config.

>  Since I've to configure the IRQ polarity as
> active low level-sensitive on my board and the flags are not set by
> the IRQ core, I've another ugly hack that forces this:
> 
> diff --git a/drivers/net/ethernet/smsc/smsc911x.c
> b/drivers/net/ethernet/smsc/smsc
> index da5cc9a..27e46f9 100644
> --- a/drivers/net/ethernet/smsc/smsc911x.c
> +++ b/drivers/net/ethernet/smsc/smsc911x.c
> @@ -2390,6 +2390,9 @@ static int smsc911x_drv_probe(struct
> platform_device *pdev)
>         pdata = netdev_priv(dev);
>         dev->irq = irq_res->start;
> -        irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> +       irq_flags = IRQF_TRIGGER_LOW;
>         pdata->ioaddr = ioremap_nocache(res->start, res_size);
> 
>         pdata->dev = dev;

I already did something like that with this patch:

---
Subject: [PATCH] net: smsc911x: adopt pinctrl support

This patch is derived from 2d4b4520a "i2c: omap: adopt pinctrl support":
Some GPIO expanders need some early pin control muxing. Due to
legacy boards sometimes the driver uses subsys_initcall instead of
module_init. This patch takes advantage of defer probe feature
and pin control in order to wait until pin control probing before
GPIO driver probing.

---
 drivers/net/ethernet/smsc/smsc911x.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index da5cc9a..3e3547c 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -59,6 +59,7 @@
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/of_net.h>
+#include <linux/pinctrl/consumer.h>
 #include "smsc911x.h"
 
 #define SMSC_CHIPNAME		"smsc911x"
@@ -144,6 +145,8 @@ struct smsc911x_data {
 
 	/* regulators */
 	struct regulator_bulk_data supplies[SMSC911X_NUM_SUPPLIES];
+
+	struct pinctrl *pins;
 };
 
 /* Easy access to information */
@@ -2433,6 +2436,18 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 	if (retval < 0)
 		goto out_disable_resources;
 
+	pdata->pins = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pdata->pins)) {
+		if (PTR_ERR(pdata->pins) == -EPROBE_DEFER) {
+			retval = -EPROBE_DEFER;
+			goto out_disable_resources;
+		}
+
+		dev_warn(&pdev->dev, "No pins for smsc911x error: %li\n",
+			 PTR_ERR(pdata->pins));
+		pdata->pins = NULL;
+	}
+
 	/* configure irq polarity and type before connecting isr */
 	if (pdata->config.irq_polarity == SMSC911X_IRQ_POLARITY_ACTIVE_HIGH)
 		intcfg |= INT_CFG_IRQ_POL_;
-- 
1.7.10.4

Now I can specify the "IRQF_TRIGGER_LOW" (which is 0x2) in the device
tree thanks to the first patch in this thread:

        lan9221@15000000 {
                compatible = "smsc,lan9221", "smsc,lan9115";
                reg = <0x15000000 0x400>;
                phy-mode = "mii";
                interrupt-parent = <&gpio5>;
                interrupts = <1 0x2>;   /* gpio_129, trigger: falling-edge */
                reg-io-width = <4>;
                vdd33a-supply = <&reg_vcc3>;
                vddvario-supply = <&reg_vcc3>;
                pinctrl-names = "default";
                pinctrl-0 = <&lan9221_pins>;
        };


 Thanks
  -- Christoph



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC][BUG] arm/dts: OMAP3: set #interrupt-cells to two
  2013-04-02 15:55       ` Christoph Fritz
@ 2013-04-02 16:38         ` Jon Hunter
  0 siblings, 0 replies; 10+ messages in thread
From: Jon Hunter @ 2013-04-02 16:38 UTC (permalink / raw)
  To: Christoph Fritz
  Cc: Javier Martinez Canillas, Benoît Cousson, Tony Lindgren,
	Russell King, linux-omap@vger.kernel.org,
	devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, Daniel Mack, Hans J. Koch,
	Paul Walmsley, Rajendra Nayak, Santosh Shilimkar


On 04/02/2013 10:55 AM, Christoph Fritz wrote:
> On Mon, 2013-04-01 at 22:05 +0200, Javier Martinez Canillas wrote:
> 
>>> 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
> 
> to be more specific on this point, this is the patch to enable the
> gpio-clocks:
> 
> --
> Subject: [PATCH] HACK: enable gpio-clocks in gpio-omap probe()
> 
> Without this patch setting trigger value from #interrupt-cell two
> (smsc911x) fails.
> ---
>  drivers/gpio/gpio-omap.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 159f5c5..720b2e6 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1098,6 +1098,7 @@ static int omap_gpio_probe(struct platform_device
> *pdev)
>  	struct resource *res;
>  	struct gpio_bank *bank;
>  	int ret = 0;
> +	void __iomem *tmp;
> 
>  	match = of_match_device(of_match_ptr(omap_gpio_match), dev);
> 
> @@ -1117,6 +1118,17 @@ static int omap_gpio_probe(struct platform_device
> *pdev)
>  		return -ENODEV;
>  	}
> 
> +	// TRM: Table 3-242. PER_CM Register Summary
> +	tmp = ioremap(0x48005040, 4); //CM_CLKSEL_PER, GPT2 = sys clk
> +	writel(0xFF, tmp);
> +	iounmap(tmp);
> +	tmp = ioremap(0x48005010, 4); //CM_ICLKEN_PER, ICKen GPT2
> +	writel(0x7FFFF, tmp);
> +	iounmap(tmp);
> +	tmp = ioremap(0x48005000, 4); //CM_FCLKEN_PER, GPIOX functional clock
> is enabled
> +	writel(0x7FFFF, tmp);
> +	iounmap(tmp);
> +
>  	bank->irq = res->start;
>  	bank->dev = dev;
>  	bank->dbck_flag = pdata->dbck_flag;
> 
> Is there a better way to do this?

A better way to hack this and leave the gpio bank on always would be ...

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index f1fbedb2..0fc75a2 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1181,8 +1181,6 @@ static int omap_gpio_probe(struct platform_device *pdev)
        if (bank->loses_context)
                bank->get_context_loss_count = pdata->get_context_loss_count;
 
-       pm_runtime_put(bank->dev);
-
        list_add_tail(&bank->node, &omap_gpio_list);
 
        return ret;

>>> 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 :-(
> 
> CONFIG_REGULATOR_TWL4030=y  disables the gpio-clocks. Why is that?

Well your hack is completely by-passing pm-runtime and so pm-runtime does not
know that someone has enabled the bank. Therefore, the next time pm_runtime_get()
is called followed by a pm_runtime_put() for the gpio bank it will disable the
bank. By removing the pm_runtime_put() at the end of probe should always keep the
bank enabled.

>>> Do you have any idea how to ("quick") fix this?
>>>
>>
>> A quick hack is to call gpio_request() explicitly before calling to
>> irq_set_type() is made.
>> I've this patch just to make it work until we find a clean solution:
>>
>> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
>> index 90c15ee..d594e1d 100644
>> --- a/arch/arm/mach-omap2/gpmc.c
>> +++ b/arch/arm/mach-omap2/gpmc.c
>> @@ -14,6 +14,7 @@
>>   */
>>  #undef DEBUG
>>
>> +#include <linux/gpio.h>
>>  #include <linux/irq.h>
>>  #include <linux/kernel.h>
>>  #include <linux/init.h>
>> @@ -1528,6 +1529,11 @@ static int gpmc_probe_dt(struct platform_device *pdev)
>>                 return ret;
>>         }
>>
>> +       ret = gpio_request_one(176, GPIOF_IN, "smsc911x irq");
>> +       if (ret) {
>> +               pr_err("Failed to request IRQ GPIO%d\n", 176);
>> +               return ret;
>> +       }
>> +
>>         for_each_node_by_name(child, "nand") {
>>                 ret = gpmc_probe_nand_child(pdev, child);
>>                 if (ret < 0) {
>>
>> This solves the issue of the non-initialized GPIO bank before that
>> makes the kernel to hang.
> 
> Here it does not. A printk shows that I'm not using gpmc at all.
> So I added a gpmc node:
> 
> +       gpmc: gpmc@0x6E000000 {
> +               compatible = "ti,omap3430-gpmc";
> +               ti,hwmods = "ti,gpmc";

This should just be ...

+               ti,hwmods = "gpmc";

> +               reg = <0x6E000000 0x2000>;
> +               gpmc,num-cs = <8>;
> +               gpmc,num-waitpins = <2>;
> +               #address-cells = <2>;
> +               #size-cells = <1>;
> +               ranges = <0 0 0x0 0x3FFFFFFF>;
> +
> +       };
> 
> But still, gpmc_probe_dt() isn't called. I maybe have to define some
> child nodes but currently I do configure the gpmc in u-boot and try to
> avoid kernel-gpmc-config.

Not sure what patches you have, but the plan for DT is that the kernel
configures the gpmc and is not dependent on the bootloader.

Cheers
Jon

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC][BUG] arm/dts: OMAP3: set #interrupt-cells to two
  2013-04-01 20:05     ` Javier Martinez Canillas
  2013-04-02 15:55       ` Christoph Fritz
@ 2013-04-13 17:42       ` Christoph Fritz
  2013-04-13 18:30         ` Javier Martinez Canillas
  1 sibling, 1 reply; 10+ messages in thread
From: Christoph Fritz @ 2013-04-13 17:42 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Benoît Cousson, Tony Lindgren, Russell King,
	linux-omap@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, Daniel Mack, Hans J. Koch,
	Jon Hunter, Paul Walmsley, Kevin Hilman, Rajendra Nayak,
	Santosh Shilimkar

On Mon, 2013-04-01 at 22:05 +0200, Javier Martinez Canillas wrote:
> On Mon, Apr 1, 2013 at 6:41 PM, Christoph Fritz
> > 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?
> >
> 
> A quick hack is to call gpio_request() explicitly before calling to
> irq_set_type() is made.
> I've this patch just to make it work until we find a clean solution:
> 
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index 90c15ee..d594e1d 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -14,6 +14,7 @@
>   */
>  #undef DEBUG
> 
> +#include <linux/gpio.h>
>  #include <linux/irq.h>
>  #include <linux/kernel.h>
>  #include <linux/init.h>
> @@ -1528,6 +1529,11 @@ static int gpmc_probe_dt(struct platform_device *pdev)
>                 return ret;
>         }
> 
> +       ret = gpio_request_one(176, GPIOF_IN, "smsc911x irq");
> +       if (ret) {
> +               pr_err("Failed to request IRQ GPIO%d\n", 176);
> +               return ret;
> +       }
> +
>         for_each_node_by_name(child, "nand") {
>                 ret = gpmc_probe_nand_child(pdev, child);
>                 if (ret < 0) {
> 
> This solves the issue of the non-initialized GPIO bank before that
> makes the kernel to hang. Since I've to configure the IRQ polarity as
> active low level-sensitive on my board and the flags are not set by
> the IRQ core, I've another ugly hack that forces this:
> 
> diff --git a/drivers/net/ethernet/smsc/smsc911x.c
> b/drivers/net/ethernet/smsc/smsc
> index da5cc9a..27e46f9 100644
> --- a/drivers/net/ethernet/smsc/smsc911x.c
> +++ b/drivers/net/ethernet/smsc/smsc911x.c
> @@ -2390,6 +2390,9 @@ static int smsc911x_drv_probe(struct
> platform_device *pdev)
>         pdata = netdev_priv(dev);
>         dev->irq = irq_res->start;
> -        irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
> +       irq_flags = IRQF_TRIGGER_LOW;
>         pdata->ioaddr = ioremap_nocache(res->start, res_size);
> 
>         pdata->dev = dev;
> 
> >  Thanks
> >   -- Christoph
> >
> 
> I hope to find some time this week to work on this and at least find a
> solution for the second issue (IORESOURCE_IRQ struct resource flags
> not being set).

Any updates on this issue?

For me it works when doing this in the device tree:

+&omap3_pmx_wkup {
+	pinctrl-names = "default";
+
+	lan9221_pins: pinmux_lan9221_pins {
+		pinctrl-single,pins = <
+			0x5A 0x104	/* gpio_129, INPUT | MODE4 */
+		>;
+	};
+};
+
<SNIP>
+	lan9221@15000000 {
+		compatible = "smsc,lan9221", "smsc,lan9115";
+		reg = <0x15000000 0x400>;
+		phy-mode = "mii";
+		interrupt-parent = <&gpio5>;
+		interrupts = <1 0x2>;	/* gpio_129, trigger: falling-edge */
+		reg-io-width = <4>;
+		vdd33a-supply = <&reg_vcc3>;
+		vddvario-supply = <&reg_vcc3>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&lan9221_pins>;
+	};

but in arch/arm/mach-omap2/gpmc.c gpmc_probe_dt() I still need this
hack:

+	ret = gpio_request_one(129, GPIOF_IN, "lan9221 irq");
+	if (ret) {
+		pr_err("Failed to request IRQ GPIO%d\n", 129);
+		return ret;
+	}

The following patches (already sent mainline) are also applied:
 "arm/dts: OMAP3: fix pinctrl-single configuration"
 "net: smsc911x: adopt pinctrl support"

 Thanks
  -- Christoph


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC][BUG] arm/dts: OMAP3: set #interrupt-cells to two
  2013-04-13 17:42       ` Christoph Fritz
@ 2013-04-13 18:30         ` Javier Martinez Canillas
  2013-04-13 18:59           ` Christoph Fritz
  0 siblings, 1 reply; 10+ messages in thread
From: Javier Martinez Canillas @ 2013-04-13 18:30 UTC (permalink / raw)
  To: Christoph Fritz
  Cc: Benoît Cousson, Tony Lindgren, Russell King,
	linux-omap@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, Daniel Mack, Hans J. Koch,
	Jon Hunter, Paul Walmsley, Kevin Hilman, Rajendra Nayak,
	Santosh Shilimkar

On Sat, Apr 13, 2013 at 7:42 PM, Christoph Fritz
<chf.fritz@googlemail.com> wrote:
> On Mon, 2013-04-01 at 22:05 +0200, Javier Martinez Canillas wrote:
>> On Mon, Apr 1, 2013 at 6:41 PM, Christoph Fritz
>> > 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?
>> >
>>
>> A quick hack is to call gpio_request() explicitly before calling to
>> irq_set_type() is made.
>> I've this patch just to make it work until we find a clean solution:
>>
>> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
>> index 90c15ee..d594e1d 100644
>> --- a/arch/arm/mach-omap2/gpmc.c
>> +++ b/arch/arm/mach-omap2/gpmc.c
>> @@ -14,6 +14,7 @@
>>   */
>>  #undef DEBUG
>>
>> +#include <linux/gpio.h>
>>  #include <linux/irq.h>
>>  #include <linux/kernel.h>
>>  #include <linux/init.h>
>> @@ -1528,6 +1529,11 @@ static int gpmc_probe_dt(struct platform_device *pdev)
>>                 return ret;
>>         }
>>
>> +       ret = gpio_request_one(176, GPIOF_IN, "smsc911x irq");
>> +       if (ret) {
>> +               pr_err("Failed to request IRQ GPIO%d\n", 176);
>> +               return ret;
>> +       }
>> +
>>         for_each_node_by_name(child, "nand") {
>>                 ret = gpmc_probe_nand_child(pdev, child);
>>                 if (ret < 0) {
>>
>> This solves the issue of the non-initialized GPIO bank before that
>> makes the kernel to hang. Since I've to configure the IRQ polarity as
>> active low level-sensitive on my board and the flags are not set by
>> the IRQ core, I've another ugly hack that forces this:
>>
>> diff --git a/drivers/net/ethernet/smsc/smsc911x.c
>> b/drivers/net/ethernet/smsc/smsc
>> index da5cc9a..27e46f9 100644
>> --- a/drivers/net/ethernet/smsc/smsc911x.c
>> +++ b/drivers/net/ethernet/smsc/smsc911x.c
>> @@ -2390,6 +2390,9 @@ static int smsc911x_drv_probe(struct
>> platform_device *pdev)
>>         pdata = netdev_priv(dev);
>>         dev->irq = irq_res->start;
>> -        irq_flags = irq_res->flags & IRQF_TRIGGER_MASK;
>> +       irq_flags = IRQF_TRIGGER_LOW;
>>         pdata->ioaddr = ioremap_nocache(res->start, res_size);
>>
>>         pdata->dev = dev;
>>
>> >  Thanks
>> >   -- Christoph
>> >
>>
>> I hope to find some time this week to work on this and at least find a
>> solution for the second issue (IORESOURCE_IRQ struct resource flags
>> not being set).
>
> Any updates on this issue?
>

Yes, my last approach to solve the IRQ flags not saved on the
IORESOURCE_IRQ struct resource issue is to add a new
irq_get_trigger_type() function to get the edge/level flags from an
IRQ number and use that function on the smsc911x driver probe function
to get the IRQ flags.

The patch-set is composed of these patches:

[PATCH v2 1/2] genirq: add function to get IRQ edge/level flags [1]
[PATCH 2/2] net: smsc911x: get IRQ flags from chip if not present in
IORESOURCE_IRQ [2]

and the cover letter is this [3]

It would be great if you can test these patches and give some feedback.

> For me it works when doing this in the device tree:
>
> +&omap3_pmx_wkup {
> +       pinctrl-names = "default";
> +
> +       lan9221_pins: pinmux_lan9221_pins {
> +               pinctrl-single,pins = <
> +                       0x5A 0x104      /* gpio_129, INPUT | MODE4 */
> +               >;
> +       };
> +};
> +
> <SNIP>
> +       lan9221@15000000 {
> +               compatible = "smsc,lan9221", "smsc,lan9115";
> +               reg = <0x15000000 0x400>;

If I understood correctly your smsc ethernet chip is connected to the
OMAP through the GPMC, then you should use a chip-select, base address
and size instead of the physical address and size.

Do you have commit 5330dc161 ("ARM: OMAP2+: Add GPMC DT support for
Ethernet child nodes") already on your tree? [4]

> +               phy-mode = "mii";
> +               interrupt-parent = <&gpio5>;
> +               interrupts = <1 0x2>;   /* gpio_129, trigger: falling-edge */

I'm confused here, do you get the IRQ_TYPE_EDGE_FALLING (0x2) trigger
type flag on the smsc911x driver probe function?

> +               reg-io-width = <4>;
> +               vdd33a-supply = <&reg_vcc3>;
> +               vddvario-supply = <&reg_vcc3>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&lan9221_pins>;
> +       };
>
> but in arch/arm/mach-omap2/gpmc.c gpmc_probe_dt() I still need this
> hack:
>
> +       ret = gpio_request_one(129, GPIOF_IN, "lan9221 irq");
> +       if (ret) {
> +               pr_err("Failed to request IRQ GPIO%d\n", 129);
> +               return ret;
> +       }
>

Yes, this hack is still needed until we figure out how to enable the
GPIO bank before a call to request_irq() is made.

> The following patches (already sent mainline) are also applied:
>  "arm/dts: OMAP3: fix pinctrl-single configuration"
>  "net: smsc911x: adopt pinctrl support"
>

Yes, but that patch is not needed anymore from 3.9, look at this [5]

>  Thanks
>   -- Christoph
>

Best regards,
Javier

[1]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg88241.html
[2]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg88225.html
[3]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg88224.html
[4]: https://patchwork.kernel.org/patch/2273851/
[5]: http://marc.info/?l=linux-kernel&m=135887740715083&w=2

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC][BUG] arm/dts: OMAP3: set #interrupt-cells to two
  2013-04-13 18:30         ` Javier Martinez Canillas
@ 2013-04-13 18:59           ` Christoph Fritz
  2013-04-13 21:40             ` Javier Martinez Canillas
  0 siblings, 1 reply; 10+ messages in thread
From: Christoph Fritz @ 2013-04-13 18:59 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Benoît Cousson, Tony Lindgren, Russell King,
	linux-omap@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, Daniel Mack, Hans J. Koch,
	Jon Hunter, Paul Walmsley, Kevin Hilman, Rajendra Nayak,
	Santosh Shilimkar

On Sat, 2013-04-13 at 20:30 +0200, Javier Martinez Canillas wrote:
> On Sat, Apr 13, 2013 at 7:42 PM, Christoph Fritz
> <chf.fritz@googlemail.com> wrote:

> Yes, my last approach to solve the IRQ flags not saved on the
> IORESOURCE_IRQ struct resource issue is to add a new
> irq_get_trigger_type() function to get the edge/level flags from an
> IRQ number and use that function on the smsc911x driver probe function
> to get the IRQ flags.
> 
> The patch-set is composed of these patches:
> 
> [PATCH v2 1/2] genirq: add function to get IRQ edge/level flags [1]
> [PATCH 2/2] net: smsc911x: get IRQ flags from chip if not present in
> IORESOURCE_IRQ [2]
> 
> and the cover letter is this [3]
> 
> It would be great if you can test these patches and give some feedback.
> 
> > For me it works when doing this in the device tree:
> >
> > +&omap3_pmx_wkup {
> > +       pinctrl-names = "default";
> > +
> > +       lan9221_pins: pinmux_lan9221_pins {
> > +               pinctrl-single,pins = <
> > +                       0x5A 0x104      /* gpio_129, INPUT | MODE4 */
> > +               >;
> > +       };
> > +};
> > +
> > <SNIP>
> > +       lan9221@15000000 {
> > +               compatible = "smsc,lan9221", "smsc,lan9115";
> > +               reg = <0x15000000 0x400>;
> 
> If I understood correctly your smsc ethernet chip is connected to the
> OMAP through the GPMC, then you should use a chip-select, base address
> and size instead of the physical address and size.

Yes, it's connected with GPMC.

> Do you have commit 5330dc161 ("ARM: OMAP2+: Add GPMC DT support for
> Ethernet child nodes") already on your tree? [4]

No I haven't.

> 
> > +               phy-mode = "mii";
> > +               interrupt-parent = <&gpio5>;
> > +               interrupts = <1 0x2>;   /* gpio_129, trigger: falling-edge */
> 
> I'm confused here, do you get the IRQ_TYPE_EDGE_FALLING (0x2) trigger
> type flag on the smsc911x driver probe function?

I added printks for irq_res->flags and irq_flags:
[    1.259857] smsc911x_drv_probe, 2396, irq_res->flags 0x00000400
[    1.266113] smsc911x_drv_probe, 2397, irq_flags 0x00000000

So the answer is no. But weird that the smsc911x works nevertheless.

> 
> > +               reg-io-width = <4>;
> > +               vdd33a-supply = <&reg_vcc3>;
> > +               vddvario-supply = <&reg_vcc3>;
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <&lan9221_pins>;
> > +       };
> >
> > but in arch/arm/mach-omap2/gpmc.c gpmc_probe_dt() I still need this
> > hack:
> >
> > +       ret = gpio_request_one(129, GPIOF_IN, "lan9221 irq");
> > +       if (ret) {
> > +               pr_err("Failed to request IRQ GPIO%d\n", 129);
> > +               return ret;
> > +       }
> >
> 
> Yes, this hack is still needed until we figure out how to enable the
> GPIO bank before a call to request_irq() is made.
> 
> > The following patches (already sent mainline) are also applied:
> >  "arm/dts: OMAP3: fix pinctrl-single configuration"
> >  "net: smsc911x: adopt pinctrl support"
> >
> 
> Yes, but that patch is not needed anymore from 3.9, look at this [5]
> 
> [1]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg88241.html
> [2]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg88225.html
> [3]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg88224.html
> [4]: https://patchwork.kernel.org/patch/2273851/
> [5]: http://marc.info/?l=linux-kernel&m=135887740715083&w=2

Thanks
 -- Christoph



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC][BUG] arm/dts: OMAP3: set #interrupt-cells to two
  2013-04-13 18:59           ` Christoph Fritz
@ 2013-04-13 21:40             ` Javier Martinez Canillas
  0 siblings, 0 replies; 10+ messages in thread
From: Javier Martinez Canillas @ 2013-04-13 21:40 UTC (permalink / raw)
  To: Christoph Fritz
  Cc: Benoît Cousson, Tony Lindgren, Russell King,
	linux-omap@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, Daniel Mack, Hans J. Koch,
	Jon Hunter, Paul Walmsley, Kevin Hilman, Rajendra Nayak,
	Santosh Shilimkar

On Sat, Apr 13, 2013 at 8:59 PM, Christoph Fritz
<chf.fritz@googlemail.com> wrote:
> On Sat, 2013-04-13 at 20:30 +0200, Javier Martinez Canillas wrote:
>> On Sat, Apr 13, 2013 at 7:42 PM, Christoph Fritz
>> <chf.fritz@googlemail.com> wrote:
>
>> Yes, my last approach to solve the IRQ flags not saved on the
>> IORESOURCE_IRQ struct resource issue is to add a new
>> irq_get_trigger_type() function to get the edge/level flags from an
>> IRQ number and use that function on the smsc911x driver probe function
>> to get the IRQ flags.
>>
>> The patch-set is composed of these patches:
>>
>> [PATCH v2 1/2] genirq: add function to get IRQ edge/level flags [1]
>> [PATCH 2/2] net: smsc911x: get IRQ flags from chip if not present in
>> IORESOURCE_IRQ [2]
>>
>> and the cover letter is this [3]
>>
>> It would be great if you can test these patches and give some feedback.
>>
>> > For me it works when doing this in the device tree:
>> >
>> > +&omap3_pmx_wkup {
>> > +       pinctrl-names = "default";
>> > +
>> > +       lan9221_pins: pinmux_lan9221_pins {
>> > +               pinctrl-single,pins = <
>> > +                       0x5A 0x104      /* gpio_129, INPUT | MODE4 */
>> > +               >;
>> > +       };
>> > +};
>> > +
>> > <SNIP>
>> > +       lan9221@15000000 {
>> > +               compatible = "smsc,lan9221", "smsc,lan9115";
>> > +               reg = <0x15000000 0x400>;
>>
>> If I understood correctly your smsc ethernet chip is connected to the
>> OMAP through the GPMC, then you should use a chip-select, base address
>> and size instead of the physical address and size.
>
> Yes, it's connected with GPMC.
>
>> Do you have commit 5330dc161 ("ARM: OMAP2+: Add GPMC DT support for
>> Ethernet child nodes") already on your tree? [4]
>
> No I haven't.
>

In that case you should have that commit on your tree. But the GPMC
driver has changed a lot for 3.9 and 3.10, so I recommend you to base
your work on linux-next that has the latest changes.

And then you will need something like this on your DT (in this example
is using the chip-select 5 but adjust according to your board):

gpmc: gpmc@6e000000 {
        compatible = "ti,omap3430-gpmc";
        ti,hwmods = "gpmc";
        reg = <0x6e000000 0x1000>;
        interrupts = <20>;
        gpmc,num-cs = <8>;
        gpmc,num-waitpins = <4>;
        #address-cells = <2>;
        #size-cells = <1>;

        ranges = <5 0 0x2c000000 0x1000000>;

        ethernet@5,0 {
                pinctrl-names = "default";
                pinctrl-0 = <&lan9221_pins>;
                compatible = "smsc,lan9221", "smsc,lan9115";
                reg = <5 0 0xff>;
                bank-width = <2>;

                gpmc,mux-add-data;
                gpmc,cs-on-ns = <0>;
                gpmc,cs-rd-off-ns = <186>;
                gpmc,cs-wr-off-ns = <186>;
                gpmc,adv-on-ns = <12>;
                gpmc,adv-rd-off-ns = <48>;
                gpmc,adv-wr-off-ns = <48>;
                gpmc,oe-on-ns = <54>;
                gpmc,oe-off-ns = <168>;
                gpmc,we-on-ns = <54>;
                gpmc,we-off-ns = <168>;
                gpmc,rd-cycle-ns = <186>;
                gpmc,wr-cycle-ns = <186>;
                gpmc,access-ns = <114>;
                gpmc,page-burst-access-ns = <6>;
                gpmc,bus-turnaround-ns = <12>;
                gpmc,cycle2cycle-delay-ns = <18>;
                gpmc,wr-data-mux-bus-ns = <90>;
                gpmc,wr-access-ns = <186>;
                gpmc,cycle2cycle-samecsen;
                gpmc,cycle2cycle-diffcsen;

                interrupt-parent = <&gpio5>;
                interrupts = <1 0x2>;
                reg-io-width = <4>;
                vdd33a-supply = <&reg_vcc3>;
                vmmc_aux-supply = <&vdd33a>;
                vddvario-supply = <&reg_vcc3>;
        };
};

>>
>> > +               phy-mode = "mii";
>> > +               interrupt-parent = <&gpio5>;
>> > +               interrupts = <1 0x2>;   /* gpio_129, trigger: falling-edge */
>>
>> I'm confused here, do you get the IRQ_TYPE_EDGE_FALLING (0x2) trigger
>> type flag on the smsc911x driver probe function?
>
> I added printks for irq_res->flags and irq_flags:
> [    1.259857] smsc911x_drv_probe, 2396, irq_res->flags 0x00000400
> [    1.266113] smsc911x_drv_probe, 2397, irq_flags 0x00000000
>
> So the answer is no. But weird that the smsc911x works nevertheless.
>

Yes, that's what I thought. You will need patch [1] and [2] to allow
smsc911x driver to get the GPIO-IRQ trigger type flags.

With those patches + linux-next + the device node I posted above +
your gpio_request_one() hack on gpmc_probe_dt() should be enough to
have your smsc ethernet chip working on your board.

>>
>> > +               reg-io-width = <4>;
>> > +               vdd33a-supply = <&reg_vcc3>;
>> > +               vddvario-supply = <&reg_vcc3>;
>> > +               pinctrl-names = "default";
>> > +               pinctrl-0 = <&lan9221_pins>;
>> > +       };
>> >
>> > but in arch/arm/mach-omap2/gpmc.c gpmc_probe_dt() I still need this
>> > hack:
>> >
>> > +       ret = gpio_request_one(129, GPIOF_IN, "lan9221 irq");
>> > +       if (ret) {
>> > +               pr_err("Failed to request IRQ GPIO%d\n", 129);
>> > +               return ret;
>> > +       }
>> >
>>
>> Yes, this hack is still needed until we figure out how to enable the
>> GPIO bank before a call to request_irq() is made.
>>
>> > The following patches (already sent mainline) are also applied:
>> >  "arm/dts: OMAP3: fix pinctrl-single configuration"
>> >  "net: smsc911x: adopt pinctrl support"
>> >
>>
>> Yes, but that patch is not needed anymore from 3.9, look at this [5]
>>
>> [1]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg88241.html
>> [2]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg88225.html
>> [3]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg88224.html
>> [4]: https://patchwork.kernel.org/patch/2273851/
>> [5]: http://marc.info/?l=linux-kernel&m=135887740715083&w=2
>
> Thanks
>  -- Christoph
>
>

Best regards,
Javier

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-04-13 21:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-30  8:21 [RFC][BUG] arm/dts: OMAP3: set #interrupt-cells to two Christoph Fritz
2013-03-30 13:18 ` Javier Martinez Canillas
2013-04-01 16:41   ` Christoph Fritz
2013-04-01 20:05     ` Javier Martinez Canillas
2013-04-02 15:55       ` Christoph Fritz
2013-04-02 16:38         ` Jon Hunter
2013-04-13 17:42       ` Christoph Fritz
2013-04-13 18:30         ` Javier Martinez Canillas
2013-04-13 18:59           ` Christoph Fritz
2013-04-13 21:40             ` Javier Martinez Canillas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).