* [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).