Skip to contentSkip to conversation

tg3: Fix INTx fallback when MSI fails

PATCH Superseded Andre Detsch 3 participants 6 messages Apr 16, 2010 – Apr 26, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 tg3: Fix INTx fallback when MSI fails

tg3: Fix INTx fallback when MSI fails

MSI setup changes the value of some key attributes of struct tg3 *tp. These attributes must be taken into account and restored before we try to do a new request_irq for INTx fallback.

In powerpc, the original code was leading to an EINVAL return within request_irq, because the driver was trying to use the disabled MSI virtual irq number instead of tp->pdev->irq.

---
Tested on powerpc, but should be safe for other architectures as well.

drivers/net/tg3.c +3 -0
Index: linux-2.6.34-rc4/drivers/net/tg3.c
===================================================================
@@ -8633,6 +8633,9 @@ static int tg3_test_msi(struct tg3 *tp)
 	pci_disable_msi(tp->pdev);
 
 	tp->tg3_flags2 &= ~TG3_FLG2_USING_MSI;
+	tp->irq_cnt = 1;
+	tp->napi[0].irq_vec = tp->pdev->irq;
+	tp->dev->real_num_tx_queues = 1;
 
 	err = tg3_request_irq(tp, 0);
 	if (err)
maintainer
#1 lore raw

From: Andre Detsch <[email protected]>
Date: Fri, 16 Apr 2010 10:15:11 -0300

Quoted 11 lines

tg3: Fix INTx fallback when MSI fails

MSI setup changes the value of some key attributes of struct tg3 *tp. These attributes must be taken into account and restored before we try to do a new request_irq for INTx fallback.

In powerpc, the original code was leading to an EINVAL return within request_irq, because the driver was trying to use the disabled MSI virtual irq number instead of tp->pdev->irq.

Broadcom folks, please review, thanks.

#2 lore raw

PATCH v2 1/1 tg3: Fix INTx fallback when MSI fails

tg3: Fix INTx fallback when MSI fails

MSI setup changes the value of irq_vec in struct tg3 *tp. This attribute must be taken into account and restored before we try to do a new request_irq for INTx fallback.

In powerpc, the original code was leading to an EINVAL return within request_irq, because the driver was trying to use the disabled MSI virtual irq number instead of tp->pdev->irq.

---
Tested on powerpc, but should be safe for other architectures as well.
v2: removed unnecessary changes

drivers/net/tg3.c +1 -0
Index: linux-2.6.34-rc5/drivers/net/tg3.c
===================================================================
@@ -8633,6 +8633,7 @@ static int tg3_test_msi(struct tg3 *tp)
 	pci_disable_msi(tp->pdev);
 
 	tp->tg3_flags2 &= ~TG3_FLG2_USING_MSI;
+	tp->napi[0].irq_vec = tp->pdev->irq;
 
 	err = tg3_request_irq(tp, 0);
 	if (err)
maintainer
#3 lore raw

From: Andre Detsch <[email protected]>
Date: Mon, 26 Apr 2010 14:27:07 -0300

Quoted 11 lines

tg3: Fix INTx fallback when MSI fails

MSI setup changes the value of irq_vec in struct tg3 *tp. This attribute must be taken into account and restored before we try to do a new request_irq for INTx fallback.

In powerpc, the original code was leading to an EINVAL return within request_irq, because the driver was trying to use the disabled MSI virtual irq number instead of tp->pdev->irq.

Broadcom folks, I've already asked you to review Andre's original patch.

If you can't be bothered to look at and ACK/NACK a simple fix like this for 10 days, then I'm just going to apply Andre's patch as-is since it looks right to me.

If the idea was to bundle this up into a set of 15 patches you guys already have ready to bomb at me, I absolutely do not want you guys to operate that way. It doesn't work. You should be able to ACK a simple patch like this as soon as possible so it can be integrated upstream in the most expediant manner possible.

Thanks.

Acked-by
#4 lore raw
Quoted 21 lines · David Miller

From: Andre Detsch <[email protected]>
Date: Mon, 26 Apr 2010 14:27:07 -0300

Quoted 11 lines

tg3: Fix INTx fallback when MSI fails

MSI setup changes the value of irq_vec in struct tg3 *tp. This attribute must be taken into account and restored before we try to do a new request_irq for INTx fallback.

In powerpc, the original code was leading to an EINVAL return within request_irq, because the driver was trying to use the disabled MSI virtual irq number instead of tp->pdev->irq.

Broadcom folks, I've already asked you to review Andre's original patch.

If you can't be bothered to look at and ACK/NACK a simple fix like this for 10 days, then I'm just going to apply Andre's patch as-is since it looks right to me.

Quoted 8 lines

If the idea was to bundle this up into a set of 15 patches you guys already have ready to bomb at me, I absolutely do not want you guys to operate that way. It doesn't work. You should be able to ACK a simple patch like this as soon as possible so it can be integrated upstream in the most expediant manner possible.

Thanks.

Sorry, Matt just got back from vacation. I discussed with Andre earlier today to come up with the new v2 patch. So this patch has my: