* [PATCH] fix Quattro HME irq registration on proble failures
@ 2009-02-07 23:12 Meelis Roos
2009-02-10 20:12 ` Meelis Roos
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Meelis Roos @ 2009-02-07 23:12 UTC (permalink / raw)
To: sparclinux
> 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.
Here is a preliminary patch, with some doubts:
* should we really panic if request_irq fails in
quattro_sbus_register_irqs()? Better propagate the error up. However, do
we still need to return success if any IRQ was registered correctly
(like the logic of of_register_driver one step before)?
Should this be a different patch/commit?
* happy_meal_open() does request_irq for PCI cards and non-quattro SBus
cards, so the printk on failure may lie about SBus? Or did I misread the
code?
But here is the patch with description:
---------------------
Currently, the sunhme driver installs SBus Quattro interrupt handler
when at least one HME card was initialized correctly and at least one
Quattro card is present. This breaks when a Quattro card fails
initialization for whatever reason - IRQ is registered and OOPS happens
when it fires.
The solution, as suggested by David Miller, was to keep track which
cards of the Quattro bundles have been initialized, and request/free the
Quattro IRQ only when all four devices have been successfully
initialized.
The patch only touches SBus initialization - PCI init already resets the
card pointer to NULL on init failure.
The patch has been tested on Sun E3500 with SBus and PCI single HME
cards and one PCI Quattro HME card in a situation where any PCI card
failed init when the SBus routines tried to init them by mistake.
Additionally it replaces Quattro request_irq panic with error return -
if this card fails to work, at least let the others work. This change is
untested.
Signed-off-by: Meelis Roos <mroos@linux.ee>
diff --git a/drivers/net/sunhme.c b/drivers/net/sunhme.c
index 7a72a31..8887ed9 100644
--- a/drivers/net/sunhme.c
+++ b/drivers/net/sunhme.c
@@ -2543,25 +2543,36 @@ static struct quattro * __devinit quattro_sbus_find(struct of_device *child)
}
/* After all quattro cards have been probed, we call these functions
- * to register the IRQ handlers.
+ * to register the IRQ handlers for the cards that have been
+ * successfully probed and skip the cards that failed to initialize
*/
-static void __init quattro_sbus_register_irqs(void)
+static int __init quattro_sbus_register_irqs(void)
{
struct quattro *qp;
for (qp = qfe_sbus_list; qp != NULL; qp = qp->next) {
struct of_device *op = qp->quattro_dev;
int err;
+ int qfe_slot, skip = 0;
+
+ for (qfe_slot = 0; qfe_slot < 4; qfe_slot++) {
+ if (qp->happy_meals[qfe_slot] = NULL) {
+ skip = 1;
+ }
+ }
+ if (skip) continue;
err = request_irq(op->irqs[0],
quattro_sbus_interrupt,
IRQF_SHARED, "Quattro",
qp);
if (err != 0) {
- printk(KERN_ERR "Quattro: Fatal IRQ registery error %d.\n", err);
- panic("QFE request irq");
+ printk(KERN_ERR "Quattro HME: IRQ registration error %d.\n", err);
+ return err;
}
}
+
+ return 0;
}
static void quattro_sbus_free_irqs(void)
@@ -2570,6 +2581,14 @@ static void quattro_sbus_free_irqs(void)
for (qp = qfe_sbus_list; qp != NULL; qp = qp->next) {
struct of_device *op = qp->quattro_dev;
+ int qfe_slot, skip = 0;
+
+ for (qfe_slot = 0; qfe_slot < 4; qfe_slot++) {
+ if (qp->happy_meals[qfe_slot] = NULL) {
+ skip = 1;
+ }
+ }
+ if (skip) continue;
free_irq(op->irqs[0], qp);
}
@@ -2824,6 +2843,10 @@ err_out_iounmap:
if (hp->tcvregs)
of_iounmap(&op->resource[4], hp->tcvregs, TCVR_REG_SIZE);
+ if (qp != NULL ) {
+ qp->happy_meals[qfe_slot] = NULL;
+ }
+
err_out_free_netdev:
free_netdev(dev);
@@ -3281,7 +3304,7 @@ static int __init happy_meal_sbus_init(void)
err = of_register_driver(&hme_sbus_driver, &of_bus_type);
if (!err)
- quattro_sbus_register_irqs();
+ err = quattro_sbus_register_irqs();
return err;
}
--
Meelis Roos (mroos@linux.ee)
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] fix Quattro HME irq registration on proble failures
2009-02-07 23:12 [PATCH] fix Quattro HME irq registration on proble failures Meelis Roos
@ 2009-02-10 20:12 ` Meelis Roos
2009-02-10 23:25 ` David Miller
2009-02-11 1:32 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Meelis Roos @ 2009-02-10 20:12 UTC (permalink / raw)
To: sparclinux
> The patch has been tested on Sun E3500 with SBus and PCI single HME
> cards and one PCI Quattro HME card in a situation where any PCI card
> failed init when the SBus routines tried to init them by mistake.
Also tested on E450 with PCI HME and PCI Quad HME.
Dave, any opinions?
--
Meelis Roos (mroos@linux.ee)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fix Quattro HME irq registration on proble failures
2009-02-07 23:12 [PATCH] fix Quattro HME irq registration on proble failures Meelis Roos
2009-02-10 20:12 ` Meelis Roos
@ 2009-02-10 23:25 ` David Miller
2009-02-11 1:32 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2009-02-10 23:25 UTC (permalink / raw)
To: sparclinux
From: Meelis Roos <mroos@linux.ee>
Date: Tue, 10 Feb 2009 22:12:09 +0200 (EET)
> > The patch has been tested on Sun E3500 with SBus and PCI single HME
> > cards and one PCI Quattro HME card in a situation where any PCI card
> > failed init when the SBus routines tried to init them by mistake.
>
> Also tested on E450 with PCI HME and PCI Quad HME.
>
> Dave, any opinions?
I'm just real busy, I'll look over your patch soon.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fix Quattro HME irq registration on proble failures
2009-02-07 23:12 [PATCH] fix Quattro HME irq registration on proble failures Meelis Roos
2009-02-10 20:12 ` Meelis Roos
2009-02-10 23:25 ` David Miller
@ 2009-02-11 1:32 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2009-02-11 1:32 UTC (permalink / raw)
To: sparclinux
From: Meelis Roos <mroos@linux.ee>
Date: Sun, 8 Feb 2009 01:12:28 +0200 (EET)
> Currently, the sunhme driver installs SBus Quattro interrupt handler
> when at least one HME card was initialized correctly and at least one
> Quattro card is present. This breaks when a Quattro card fails
> initialization for whatever reason - IRQ is registered and OOPS happens
> when it fires.
>
> The solution, as suggested by David Miller, was to keep track which
> cards of the Quattro bundles have been initialized, and request/free the
> Quattro IRQ only when all four devices have been successfully
> initialized.
>
> The patch only touches SBus initialization - PCI init already resets the
> card pointer to NULL on init failure.
>
> The patch has been tested on Sun E3500 with SBus and PCI single HME
> cards and one PCI Quattro HME card in a situation where any PCI card
> failed init when the SBus routines tried to init them by mistake.
>
> Additionally it replaces Quattro request_irq panic with error return -
> if this card fails to work, at least let the others work. This change is
> untested.
>
> Signed-off-by: Meelis Roos <mroos@linux.ee>
This patch looks great, I made some minor coding style fixups
and updated to commit log message to indicate that you did
test this change also on an E450 system.
Now, to answer your questions:
> * should we really panic if request_irq fails in
> quattro_sbus_register_irqs()? Better propagate the error up. However, do
> we still need to return success if any IRQ was registered correctly
> (like the logic of of_register_driver one step before)?
Yes, propagating the error is much better. The panic() is overboard.
> * happy_meal_open() does request_irq for PCI cards and non-quattro SBus
> cards, so the printk on failure may lie about SBus? Or did I misread the
> code?
It's half lying :-) In some of the cases this request_irq() would be
for SBUS (non-quattro only) and for others it would be for PCI
(quattro or not).
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-02-11 1:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-07 23:12 [PATCH] fix Quattro HME irq registration on proble failures Meelis Roos
2009-02-10 20:12 ` Meelis Roos
2009-02-10 23:25 ` David Miller
2009-02-11 1:32 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.