[PATCH 2.6.11 3/8] tg3: Add msi support for 5750 C0

[PATCH 2.6.11 3/8] tg3: Add msi support for 5750 C0

[Michael Chan]

[-- Attachment #1: Type: text/plain, Size: 410 bytes --]

Add MSI support for 5750 C0 and newer chips. MSI is handled by a new
interrupt service routine that is very similar to the existing
tg3_interrupt() for INTx mode. The MSI version is optimized by not checking
for shared interrupt and not flushing the status block.

The code is also changed to reference the irq from tp->pdev->irq instead of
dev->irq.

Signed-off-by: Michael Chan <mchan@broadcom.com>

[-- Attachment #2: tg3-3.patch --]
[-- Type: application/octet-stream, Size: 4024 bytes --]

diff -Nru 3/drivers/net/tg3.c 4/drivers/net/tg3.c
--- 3/drivers/net/tg3.c	2005-03-14 22:24:35.000000000 -0800
+++ 4/drivers/net/tg3.c	2005-03-15 20:41:51.000000000 -0800
@@ -2876,6 +2876,43 @@
 	return work_exists;
 }
 
+/* MSI ISR - No need to check for interrupt sharing and no need to
+ * flush status block and interrupt mailbox. PCI ordering rules
+ * guarantee that MSI will arrive after the status block.
+ */
+static irqreturn_t tg3_msi(int irq, void *dev_id, struct pt_regs *regs)
+{
+	struct net_device *dev = dev_id;
+	struct tg3 *tp = netdev_priv(dev);
+	struct tg3_hw_status *sblk = tp->hw_status;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tp->lock, flags);
+
+	/*
+	 * writing any value to intr-mbox-0 clears PCI INTA# and
+	 * chip-internal interrupt pending events.
+	 * writing non-zero to intr-mbox-0 additional tells the
+	 * NIC to stop sending us irqs, engaging "in-intr-handler"
+	 * event coalescing.
+	 */
+	tw32_mailbox(MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW, 0x00000001);
+	sblk->status &= ~SD_STATUS_UPDATED;
+
+	if (likely(tg3_has_work(dev, tp)))
+		netif_rx_schedule(dev);		/* schedule NAPI poll */
+	else {
+		/* no work, re-enable interrupts
+		 */
+		tw32_mailbox(MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW,
+			     0x00000000);
+	}
+
+	spin_unlock_irqrestore(&tp->lock, flags);
+
+	return IRQ_RETVAL(1);
+}
+
 static irqreturn_t tg3_interrupt(int irq, void *dev_id, struct pt_regs *regs)
 {
 	struct net_device *dev = dev_id;
@@ -2934,7 +2971,9 @@
 #ifdef CONFIG_NET_POLL_CONTROLLER
 static void tg3_poll_controller(struct net_device *dev)
 {
-	tg3_interrupt(dev->irq, dev, NULL);
+	struct tg3 *tp = netdev_priv(dev);
+
+	tg3_interrupt(tp->pdev->irq, dev, NULL);
 }
 #endif
 
@@ -5700,10 +5739,29 @@
 	if (err)
 		return err;
 
-	err = request_irq(dev->irq, tg3_interrupt,
-			  SA_SHIRQ, dev->name, dev);
+	if ((GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5750) &&
+	    (GET_CHIP_REV(tp->pci_chip_rev_id) != CHIPREV_5750_AX) &&
+	    (GET_CHIP_REV(tp->pci_chip_rev_id) != CHIPREV_5750_BX)) {
+		if (pci_enable_msi(tp->pdev) == 0) {
+			u32 msi_mode;
+
+			msi_mode = tr32(MSGINT_MODE);
+			tw32(MSGINT_MODE, msi_mode | MSGINT_MODE_ENABLE);
+			tp->tg3_flags2 |= TG3_FLG2_USING_MSI;
+		}
+	}
+	if (tp->tg3_flags2 & TG3_FLG2_USING_MSI)
+		err = request_irq(tp->pdev->irq, tg3_msi,
+				  0, dev->name, dev);
+	else
+		err = request_irq(tp->pdev->irq, tg3_interrupt,
+				  SA_SHIRQ, dev->name, dev);
 
 	if (err) {
+		if (tp->tg3_flags2 & TG3_FLG2_USING_MSI) {
+			pci_disable_msi(tp->pdev);
+			tp->tg3_flags2 &= ~TG3_FLG2_USING_MSI;
+		}
 		tg3_free_consistent(tp);
 		return err;
 	}
@@ -5733,7 +5791,11 @@
 	spin_unlock_irq(&tp->lock);
 
 	if (err) {
-		free_irq(dev->irq, dev);
+		free_irq(tp->pdev->irq, dev);
+		if (tp->tg3_flags2 & TG3_FLG2_USING_MSI) {
+			pci_disable_msi(tp->pdev);
+			tp->tg3_flags2 &= ~TG3_FLG2_USING_MSI;
+		}
 		tg3_free_consistent(tp);
 		return err;
 	}
@@ -6008,7 +6070,11 @@
 	spin_unlock(&tp->tx_lock);
 	spin_unlock_irq(&tp->lock);
 
-	free_irq(dev->irq, dev);
+	free_irq(tp->pdev->irq, dev);
+	if (tp->tg3_flags2 & TG3_FLG2_USING_MSI) {
+		pci_disable_msi(tp->pdev);
+		tp->tg3_flags2 &= ~TG3_FLG2_USING_MSI;
+	}
 
 	memcpy(&tp->net_stats_prev, tg3_get_stats(tp->dev),
 	       sizeof(tp->net_stats_prev));
diff -Nru 3/drivers/net/tg3.h 4/drivers/net/tg3.h
--- 3/drivers/net/tg3.h	2005-03-14 22:24:38.000000000 -0800
+++ 4/drivers/net/tg3.h	2005-03-15 20:36:07.000000000 -0800
@@ -140,6 +140,8 @@
 #define   CHIPREV_5703_AX		 0x10
 #define   CHIPREV_5704_AX		 0x20
 #define   CHIPREV_5704_BX		 0x21
+#define   CHIPREV_5750_AX		 0x40
+#define   CHIPREV_5750_BX		 0x41
 #define  GET_METAL_REV(CHIP_REV_ID)	((CHIP_REV_ID) & 0xff)
 #define   METAL_REV_A0			 0x00
 #define   METAL_REV_A1			 0x01
@@ -2097,6 +2099,7 @@
 #define TG3_FLG2_HW_TSO			0x00010000
 #define TG3_FLG2_SERDES_PREEMPHASIS	0x00020000
 #define TG3_FLG2_5705_PLUS		0x00040000
+#define TG3_FLG2_USING_MSI		0x00080000
 
 	u32				split_mode_max_reqs;
 #define SPLIT_MODE_5704_MAX_REQ		3

Re: [PATCH 2.6.11 3/8] tg3: Add msi support for 5750 C0

[Jeff Garzik]

Michael Chan wrote:
> Add MSI support for 5750 C0 and newer chips. MSI is handled by a new
> interrupt service routine that is very similar to the existing
> tg3_interrupt() for INTx mode. The MSI version is optimized by not checking
> for shared interrupt and not flushing the status block.
> 
> The code is also changed to reference the irq from tp->pdev->irq instead of
> dev->irq.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com

I would prefer that a new 'static inline' function was created, 
containing the core interrupt handler code.

This avoids the code duplication you have in this patch, in new function 
tg3_msi().

	Jeff

RE: [PATCH 2.6.11 3/8] tg3: Add msi support for 5750 C0

[Michael Chan]

"Jeff Garzik" wrote:

> 
> I would prefer that a new 'static inline' function was created, 
> containing the core interrupt handler code.
> 
> This avoids the code duplication you have in this patch, in 
> new function 
> tg3_msi().
> 
> 	Jeff
> 

There are enough subtle differences between tg3_interrupt() and tg3_msi()
that makes it impractical to create a core inline function to be shared by
the two. To share a meaningful chunk of the code, it will have to check for
MSI mode and skip some of the register reads. The reason I created tg3_msi()
was to avoid all conditionals.

Michael

Re: [PATCH 2.6.11 3/8] tg3: Add msi support for 5750 C0

[Jeff Garzik]

Michael Chan wrote:
> "Jeff Garzik" wrote:
> 
> 
>>I would prefer that a new 'static inline' function was created, 
>>containing the core interrupt handler code.
>>
>>This avoids the code duplication you have in this patch, in 
>>new function 
>>tg3_msi().
>>
>>	Jeff
>>
> 
> 
> There are enough subtle differences between tg3_interrupt() and tg3_msi()
> that makes it impractical to create a core inline function to be shared by
> the two. To share a meaningful chunk of the code, it will have to check for
> MSI mode and skip some of the register reads. The reason I created tg3_msi()
> was to avoid all conditionals.

After reviewing the existing tg3.c code again, I agree.  Objection 
withdrawn.

	Jeff