Patchwork SBus HME startup failure

login
register
mail settings
Submitter David Miller
Date Feb. 7, 2009, 7:46 a.m.
Message ID <20090206.234600.211802423.davem@davemloft.net>
Download mbox | patch
Permalink /patch/22463/
State Accepted
Delegated to: David Miller
Headers show

Comments

David Miller - Feb. 7, 2009, 7:46 a.m.
Meelis and Eric, does the following patch do the trick?

--------------------
sunhme: Don't match PCI devices in SBUS probe.

Unfortunately, the OF device tree nodes for SBUS and PCI
hme devices have the same device node name on some systems.

So if the name of the parent node isn't 'sbus', skip it.

Based upon an excellent report and detective work by
Meelis Roos and Eric Brower.

Signed-off-by: David S. Miller <davem@davemloft.net>

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Meelis Roos - Feb. 7, 2009, 8:34 a.m.
> Meelis and Eric, does the following patch do the trick?

Yes, thank you very much!

However, do we need to fix the second prpblem too that's now not 
appearing - registering quatrro sbus irqs anyway if some hme was found 
and a quattro HME exists in the system?

> --------------------
> sunhme: Don't match PCI devices in SBUS probe.
> 
> Unfortunately, the OF device tree nodes for SBUS and PCI
> hme devices have the same device node name on some systems.
> 
> So if the name of the parent node isn't 'sbus', skip it.
> 
> Based upon an excellent report and detective work by
> Meelis Roos and Eric Brower.
> 
> Signed-off-by: David S. Miller <davem@davemloft.net>

Tested-by: Meelis Roos <mroos@linux.ee>
David Miller - Feb. 7, 2009, 10:20 a.m.
From: Meelis Roos <mroos@linux.ee>
Date: Sat, 7 Feb 2009 10:34:04 +0200 (EET)

> > Meelis and Eric, does the following patch do the trick?
> 
> Yes, thank you very much!

Thanks for testing.

> However, do we need to fix the second prpblem too that's now not 
> appearing - registering quatrro sbus irqs anyway if some hme was found 
> and a quattro HME exists in the system?

Hmmm... one thing we could do is only register the IRQ if all 4
slots of the per-quattro array are filled in.

Similar checks would be needed in quattro_sbus_free_irqs().

This means we'll also have to add some code to the error paths
of happy_meal_sbus_probe_one() to make it NULL out the
qp->happy_meals[qfe_slot] entry on failure.

Maybe you could work on this patch? :-)

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Meelis Roos - Feb. 7, 2009, 10:23 a.m.
> Maybe you could work on this patch? :-)

OK, will probably try in a couple of days.
Eric Brower - Feb. 8, 2009, 12:15 a.m.
On Fri, Feb 6, 2009 at 11:46 PM, David Miller <davem@davemloft.net> wrote:
>
> Meelis and Eric, does the following patch do the trick?
>
> --------------------
> sunhme: Don't match PCI devices in SBUS probe.
>
> Unfortunately, the OF device tree nodes for SBUS and PCI
> hme devices have the same device node name on some systems.
>
> So if the name of the parent node isn't 'sbus', skip it.
>
> Based upon an excellent report and detective work by
> Meelis Roos and Eric Brower.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> diff --git a/drivers/net/sunhme.c b/drivers/net/sunhme.c
> index 7a72a31..cc4013b 100644
> --- a/drivers/net/sunhme.c
> +++ b/drivers/net/sunhme.c
> @@ -2629,6 +2629,14 @@ static int __devinit happy_meal_sbus_probe_one(struct of_device *op, int is_qfe)
>        int i, qfe_slot = -1;
>        int err = -ENODEV;
>
> +       sbus_dp = to_of_device(op->dev.parent)->node;
> +       if (is_qfe)
> +               sbus_dp = to_of_device(op->dev.parent->parent)->node;
> +
> +       /* We can match PCI devices too, do not accept those here. */
> +       if (strcmp(sbus_dp->name, "sbus"))
> +               return err;
> +
>        if (is_qfe) {
>                qp = quattro_sbus_find(op);
>                if (qp == NULL)
> @@ -2734,10 +2742,6 @@ static int __devinit happy_meal_sbus_probe_one(struct of_device *op, int is_qfe)
>        if (qp != NULL)
>                hp->happy_flags |= HFLAG_QUATTRO;
>
> -       sbus_dp = to_of_device(op->dev.parent)->node;
> -       if (is_qfe)
> -               sbus_dp = to_of_device(op->dev.parent->parent)->node;
> -
>        /* Get the supported DVMA burst sizes from our Happy SBUS. */
>        hp->happy_bursts = of_getintprop_default(sbus_dp,
>                                                 "burst-sizes", 0x00);
>

This works for me as well-- thanks.

I wasn't confident enough that the direct parent of a non-QFE HME
would always be an "sbus" node, though it certainly seems like a valid
assumption.
David Miller - Feb. 8, 2009, 3:09 a.m.
From: Eric Brower <ebrower@gmail.com>
Date: Sat, 7 Feb 2009 16:15:44 -0800

> I wasn't confident enough that the direct parent of a non-QFE HME
> would always be an "sbus" node, though it certainly seems like a valid
> assumption.

It's actually possible for it not to be there.

For example, via XBOX sbus expansion boards, which would add another
level of bus hierarchy.  We don't support those expander devices
currently but...

So the thing to do is to add a "of_find_parent_by_name()" interface
and use that in these kinds of situations.

I'll work on that when I get a chance.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/sunhme.c b/drivers/net/sunhme.c
index 7a72a31..cc4013b 100644
--- a/drivers/net/sunhme.c
+++ b/drivers/net/sunhme.c
@@ -2629,6 +2629,14 @@  static int __devinit happy_meal_sbus_probe_one(struct of_device *op, int is_qfe)
 	int i, qfe_slot = -1;
 	int err = -ENODEV;
 
+	sbus_dp = to_of_device(op->dev.parent)->node;
+	if (is_qfe)
+		sbus_dp = to_of_device(op->dev.parent->parent)->node;
+
+	/* We can match PCI devices too, do not accept those here. */
+	if (strcmp(sbus_dp->name, "sbus"))
+		return err;
+
 	if (is_qfe) {
 		qp = quattro_sbus_find(op);
 		if (qp == NULL)
@@ -2734,10 +2742,6 @@  static int __devinit happy_meal_sbus_probe_one(struct of_device *op, int is_qfe)
 	if (qp != NULL)
 		hp->happy_flags |= HFLAG_QUATTRO;
 
-	sbus_dp = to_of_device(op->dev.parent)->node;
-	if (is_qfe)
-		sbus_dp = to_of_device(op->dev.parent->parent)->node;
-
 	/* Get the supported DVMA burst sizes from our Happy SBUS. */
 	hp->happy_bursts = of_getintprop_default(sbus_dp,
 						 "burst-sizes", 0x00);