Re: [REGRESSION] gpio: pxa: change initcall level second attempt
Hi Robert On Sun, 2016-01-31 at 00:20 +0100, Robert Jarzmik wrote: > Marcel, would you try the patch here after ? > I have tested it on my cm-x300 with a devicetree build, let's see if > that is a > solution to your issue. Yes, that indeed cuts it nicely. Thanks! > Cheers. > > -- > Robert > > ---8<--- > From 5901e6d55061c0cd627cfbf090ef6362c712b3c8 Mon Sep 17 00:00:00 > 2001 > From: Robert Jarzmik > Date: Sun, 31 Jan 2016 00:06:21 +0100 > Subject: [PATCH] net: ethernet: davicom: fix devicetree irq resource > > The dm9000 driver doesn't work in at least one device-tree > configuration, spitting an error message on irq resource : > [1.062495] dm9000 800.ethernet: insufficient resources > [1.068439] dm9000 800.ethernet: not found (-2). > [1.073451] dm9000: probe of 800.ethernet failed with error -2 > > The reason behind is that the interrupt might be provided by a gpio > controller, not probed when dm9000 is probed, and needing the probe > deferral mechanism to apply. > > Currently, the interrupt is directly taken from resources. This patch > changes this to use the more generic platform_get_irq(), which > handles > the deferral. > > Moreover, since commit Fixes: 7085a7401ba5 ("drivers: platform: parse > IRQ flags from resources"), the interrupt trigger flags are honored > in > platform_get_irq(), so remove the needless code in dm9000. > > Signed-off-by: Robert Jarzmik > --- > drivers/net/ethernet/davicom/dm9000.c | 28 ++--- > --- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/ethernet/davicom/dm9000.c > b/drivers/net/ethernet/davicom/dm9000.c > index cf94b72..22e1a9d 100644 > --- a/drivers/net/ethernet/davicom/dm9000.c > +++ b/drivers/net/ethernet/davicom/dm9000.c > @@ -128,7 +128,6 @@ struct board_info { > struct resource *data_res; > struct resource *addr_req; /* resources requested > */ > struct resource *data_req; > - struct resource *irq_res; > > int irq_wake; > > @@ -1300,18 +1299,14 @@ static int > dm9000_open(struct net_device *dev) > { > struct board_info *db = netdev_priv(dev); > - unsigned long irqflags = db->irq_res->flags & > IRQF_TRIGGER_MASK; > + unsigned long irqflags = 0; > > if (netif_msg_ifup(db)) > dev_dbg(db->dev, "enabling %s\n", dev->name); > > - /* If there is no IRQ type specified, default to something > that > - * may work, and tell the user that this is a problem */ > - > - if (irqflags == IRQF_TRIGGER_NONE) > - irqflags = irq_get_trigger_type(dev->irq); > - > - if (irqflags == IRQF_TRIGGER_NONE) > + /* If there is no IRQ type specified, tell the user that > this is a > + * problem */ > + if (irq_get_trigger_type(dev->irq) == IRQF_TRIGGER_NONE) > dev_warn(db->dev, "WARNING: no IRQ resource flags > set.\n"); > > irqflags |= IRQF_SHARED; > @@ -1500,15 +1495,21 @@ dm9000_probe(struct platform_device *pdev) > > db->addr_res = platform_get_resource(pdev, IORESOURCE_MEM, > 0); > db->data_res = platform_get_resource(pdev, IORESOURCE_MEM, > 1); > - db->irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, > 0); > > - if (db->addr_res == NULL || db->data_res == NULL || > - db->irq_res == NULL) { > - dev_err(db->dev, "insufficient resources\n"); > + if (db->addr_res == NULL || db->data_res == NULL) { > + dev_err(db->dev, "insufficient resources addr=%p > data=%p\n", > + db->addr_res, db->data_res); > ret = -ENOENT; > goto out; > } > > + ndev->irq = platform_get_irq(pdev, 0); > + if (ndev->irq <= 0) { > + dev_err(db->dev, "interrupt ressource unavailable: > %d\n", > + ndev->irq); > + return ndev->irq; > + } > + > db->irq_wake = platform_get_irq(pdev, 1); > if (db->irq_wake >= 0) { > dev_dbg(db->dev, "wakeup irq %d\n", db->irq_wake); > @@ -1570,7 +1571,6 @@ dm9000_probe(struct platform_device *pdev) > > /* fill in parameters for net-dev structure */ > ndev->base_addr = (unsigned long)db->io_addr; > - ndev->irq = db->irq_res->start; > > /* ensure at least we have a default set of IO routines */ > dm9000_set_io(db, iosize); Acked-by: Marcel Ziswiler Cheers Marcel
Re: [REGRESSION] gpio: pxa: change initcall level second attempt
Marcel, would you try the patch here after ? I have tested it on my cm-x300 with a devicetree build, let's see if that is a solution to your issue. Cheers. -- Robert ---8<--- >From 5901e6d55061c0cd627cfbf090ef6362c712b3c8 Mon Sep 17 00:00:00 2001 From: Robert Jarzmik Date: Sun, 31 Jan 2016 00:06:21 +0100 Subject: [PATCH] net: ethernet: davicom: fix devicetree irq resource The dm9000 driver doesn't work in at least one device-tree configuration, spitting an error message on irq resource : [1.062495] dm9000 800.ethernet: insufficient resources [1.068439] dm9000 800.ethernet: not found (-2). [1.073451] dm9000: probe of 800.ethernet failed with error -2 The reason behind is that the interrupt might be provided by a gpio controller, not probed when dm9000 is probed, and needing the probe deferral mechanism to apply. Currently, the interrupt is directly taken from resources. This patch changes this to use the more generic platform_get_irq(), which handles the deferral. Moreover, since commit Fixes: 7085a7401ba5 ("drivers: platform: parse IRQ flags from resources"), the interrupt trigger flags are honored in platform_get_irq(), so remove the needless code in dm9000. Signed-off-by: Robert Jarzmik --- drivers/net/ethernet/davicom/dm9000.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/net/ethernet/davicom/dm9000.c b/drivers/net/ethernet/davicom/dm9000.c index cf94b72..22e1a9d 100644 --- a/drivers/net/ethernet/davicom/dm9000.c +++ b/drivers/net/ethernet/davicom/dm9000.c @@ -128,7 +128,6 @@ struct board_info { struct resource *data_res; struct resource *addr_req; /* resources requested */ struct resource *data_req; - struct resource *irq_res; int irq_wake; @@ -1300,18 +1299,14 @@ static int dm9000_open(struct net_device *dev) { struct board_info *db = netdev_priv(dev); - unsigned long irqflags = db->irq_res->flags & IRQF_TRIGGER_MASK; + unsigned long irqflags = 0; if (netif_msg_ifup(db)) dev_dbg(db->dev, "enabling %s\n", dev->name); - /* If there is no IRQ type specified, default to something that -* may work, and tell the user that this is a problem */ - - if (irqflags == IRQF_TRIGGER_NONE) - irqflags = irq_get_trigger_type(dev->irq); - - if (irqflags == IRQF_TRIGGER_NONE) + /* If there is no IRQ type specified, tell the user that this is a +* problem */ + if (irq_get_trigger_type(dev->irq) == IRQF_TRIGGER_NONE) dev_warn(db->dev, "WARNING: no IRQ resource flags set.\n"); irqflags |= IRQF_SHARED; @@ -1500,15 +1495,21 @@ dm9000_probe(struct platform_device *pdev) db->addr_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); db->data_res = platform_get_resource(pdev, IORESOURCE_MEM, 1); - db->irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); - if (db->addr_res == NULL || db->data_res == NULL || - db->irq_res == NULL) { - dev_err(db->dev, "insufficient resources\n"); + if (db->addr_res == NULL || db->data_res == NULL) { + dev_err(db->dev, "insufficient resources addr=%p data=%p\n", + db->addr_res, db->data_res); ret = -ENOENT; goto out; } + ndev->irq = platform_get_irq(pdev, 0); + if (ndev->irq <= 0) { + dev_err(db->dev, "interrupt ressource unavailable: %d\n", + ndev->irq); + return ndev->irq; + } + db->irq_wake = platform_get_irq(pdev, 1); if (db->irq_wake >= 0) { dev_dbg(db->dev, "wakeup irq %d\n", db->irq_wake); @@ -1570,7 +1571,6 @@ dm9000_probe(struct platform_device *pdev) /* fill in parameters for net-dev structure */ ndev->base_addr = (unsigned long)db->io_addr; - ndev->irq = db->irq_res->start; /* ensure at least we have a default set of IO routines */ dm9000_set_io(db, iosize); -- 2.1.4
Re: [REGRESSION] gpio: pxa: change initcall level second attempt
On Tue, 2016-01-26 at 21:41 +0100, Robert Jarzmik wrote: > Robert Jarzmik writes: > > > > Have you seen this as well or do you know how exactly that should > > > be > > > worked around? > > Hi Marcel, > > > > I haven't seen that before on my devicetree boards, I will try this > > evening on > > top of next-20160125. > > > > Could you activate the debug logs in drivers/of/irq.c please, and > > send me > > privately your boot dmesg ? And I'd like to see your board .dts > > file and your > > .config also, to compare with mine for the mioa701. > > > > I think in your dm9000 case we have this callstack : > > - of_irq_get() > > of_irq_parse_one() => fails, for an unknown reason to me > > => I would have expected it return 0 > > => the following irq_find_host() would > > return > > -EPROBE_DEFER, that's what I'd expect > > irq_create_of_mapping() > > irq_create_fwspec_mapping() > > => error message > > > > What is probable is that gpio-pxa was not probed yet, and this > > triggers the > > error. What I don't understand is why you don't end up with > > -EPROBE_DEFER, > > that's why I'd like to have your logs. > > BTW, would you try also with the patch at the end of this mail > applied ? Just to > verify a wild guess. Thanks, but unfortunately that did not change anything on the issue. > Cheers.
Re: [REGRESSION] gpio: pxa: change initcall level second attempt
Robert Jarzmik writes: >> Have you seen this as well or do you know how exactly that should be >> worked around? > Hi Marcel, > > I haven't seen that before on my devicetree boards, I will try this evening on > top of next-20160125. > > Could you activate the debug logs in drivers/of/irq.c please, and send me > privately your boot dmesg ? And I'd like to see your board .dts file and your > .config also, to compare with mine for the mioa701. > > I think in your dm9000 case we have this callstack : > - of_irq_get() > of_irq_parse_one() => fails, for an unknown reason to me > => I would have expected it return 0 > => the following irq_find_host() would return >-EPROBE_DEFER, that's what I'd expect > irq_create_of_mapping() >irq_create_fwspec_mapping() > => error message > > What is probable is that gpio-pxa was not probed yet, and this triggers the > error. What I don't understand is why you don't end up with -EPROBE_DEFER, > that's why I'd like to have your logs. BTW, would you try also with the patch at the end of this mail applied ? Just to verify a wild guess. Cheers. -- Robert ---8<--- diff --git a/drivers/net/ethernet/davicom/dm9000.c b/drivers/net/ethernet/davicom/dm9000.c index cf94b72dbacd..2c532011ae8e 100644 --- a/drivers/net/ethernet/davicom/dm9000.c +++ b/drivers/net/ethernet/davicom/dm9000.c @@ -1509,6 +1509,10 @@ dm9000_probe(struct platform_device *pdev) goto out; } + ndev->irq = platform_get_irq(pdev, 0); + if (ndev->irq < 0) + return ndev->irq; + db->irq_wake = platform_get_irq(pdev, 1); if (db->irq_wake >= 0) { dev_dbg(db->dev, "wakeup irq %d\n", db->irq_wake); @@ -1570,7 +1574,6 @@ dm9000_probe(struct platform_device *pdev) /* fill in parameters for net-dev structure */ ndev->base_addr = (unsigned long)db->io_addr; - ndev->irq = db->irq_res->start; /* ensure at least we have a default set of IO routines */ dm9000_set_io(db, iosize);
Re: [REGRESSION] gpio: pxa: change initcall level second attempt
Marcel Ziswiler writes: > Hi Robert > > I tried latest next-20160122 on Colibri PXA270 with a previously > working device tree and got the following DM9000 Ethernet driver issue: > > [1.062495] dm9000 800.ethernet: insufficient resources > [1.068439] dm9000 800.ethernet: not found (-2). > [1.073451] dm9000: probe of 800.ethernet failed with error -2 > > Digging deeper I debugged it to a missing interrupt ressource. A > subsequent git bisect blamed your patch changing the initcall level and > indeed just reverting that made it all work again. > > I then also noticed the following message upon boot in the failing case > which is probably related: > > [0.175995] irq: no irq domain found for /pxabus/gpio@40e0 ! > > Have you seen this as well or do you know how exactly that should be > worked around? Hi Marcel, I haven't seen that before on my devicetree boards, I will try this evening on top of next-20160125. Could you activate the debug logs in drivers/of/irq.c please, and send me privately your boot dmesg ? And I'd like to see your board .dts file and your .config also, to compare with mine for the mioa701. I think in your dm9000 case we have this callstack : - of_irq_get() of_irq_parse_one() => fails, for an unknown reason to me => I would have expected it return 0 => the following irq_find_host() would return -EPROBE_DEFER, that's what I'd expect irq_create_of_mapping() irq_create_fwspec_mapping() => error message What is probable is that gpio-pxa was not probed yet, and this triggers the error. What I don't understand is why you don't end up with -EPROBE_DEFER, that's why I'd like to have your logs. Cheers. -- Robert
[REGRESSION] gpio: pxa: change initcall level second attempt
Hi Robert I tried latest next-20160122 on Colibri PXA270 with a previously working device tree and got the following DM9000 Ethernet driver issue: [1.062495] dm9000 800.ethernet: insufficient resources [1.068439] dm9000 800.ethernet: not found (-2). [1.073451] dm9000: probe of 800.ethernet failed with error -2 Digging deeper I debugged it to a missing interrupt ressource. A subsequent git bisect blamed your patch changing the initcall level and indeed just reverting that made it all work again. I then also noticed the following message upon boot in the failing case which is probably related: [0.175995] irq: no irq domain found for /pxabus/gpio@40e0 ! Have you seen this as well or do you know how exactly that should be worked around? Cheers Marcel