Re: [REGRESSION] gpio: pxa: change initcall level second attempt

2016-01-31 Thread Marcel Ziswiler
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

2016-01-30 Thread Robert Jarzmik
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

2016-01-26 Thread Marcel Ziswiler
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

2016-01-26 Thread Robert Jarzmik
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

2016-01-25 Thread Robert Jarzmik
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

2016-01-23 Thread Marcel Ziswiler
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