* [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 = <®_vcc3>;
vddvario-supply = <®_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 = <®_vcc3>;
+ vddvario-supply = <®_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 = <®_vcc3>;
> + vddvario-supply = <®_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 = <®_vcc3>;
> > + vddvario-supply = <®_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 = <®_vcc3>;
vmmc_aux-supply = <&vdd33a>;
vddvario-supply = <®_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 = <®_vcc3>;
>> > + vddvario-supply = <®_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).