: Eliminate the TG3_FLAG_5701_REG_WRITE_BUG flag.

PATCH Applied Michael Chan 4 participants 4 messages May 5, 2007

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Michael Chan

author

2007-05-05 00:07 UTC #0 lore raw

PATCH 7/11 : Eliminate the TG3_FLAG_5701_REG_WRITE_BUG flag.

[TG3]: Eliminate the TG3_FLAG_5701_REG_WRITE_BUG flag.

This patch removes the use of the TG3_FLAG_5701_REG_WRITE_BUG flag.

Signed-off-by Matt Carlson <[email protected]>

Signed-off-by Michael Chan <[email protected]>

drivers/net/tg3.c +11 ~~-12~~

index 84fc498..23a9477 100644
@@ -10673,17 +10673,6 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
 	if (GET_CHIP_REV(tp->pci_chip_rev_id) == CHIPREV_5700_BX)
 		tp->tg3_flags |= TG3_FLAG_TXD_MBOX_HWBUG;
 
-	/* Back to back register writes can cause problems on this chip,
-	 * the workaround is to read back all reg writes except those to
-	 * mailbox regs.  See tg3_write_indirect_reg32().
-	 *
-	 * PCI Express 5750_A0 rev chips need this workaround too.
-	 */
-	if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5701 ||
-	    ((tp->tg3_flags2 & TG3_FLG2_PCI_EXPRESS) &&
-	     tp->pci_chip_rev_id == CHIPREV_ID_5750_A0))
-		tp->tg3_flags |= TG3_FLAG_5701_REG_WRITE_BUG;
-
 	if ((pci_state_reg & PCISTATE_BUS_SPEED_HIGH) != 0)
 		tp->tg3_flags |= TG3_FLAG_PCI_HIGH_SPEED;
 	if ((pci_state_reg & PCISTATE_BUS_32BIT) != 0)

@@ -10707,8 +10696,18 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
 	/* Various workaround register access methods */
 	if (tp->tg3_flags & TG3_FLAG_PCIX_TARGET_HWBUG)
 		tp->write32 = tg3_write_indirect_reg32;
-	else if (tp->tg3_flags & TG3_FLAG_5701_REG_WRITE_BUG)
+	else if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5701 ||
+		 ((tp->tg3_flags2 & TG3_FLG2_PCI_EXPRESS) &&
+		  tp->pci_chip_rev_id == CHIPREV_ID_5750_A0)) {
+		/* Back to back register writes can cause problems on this chip,
+		 * the workaround is to read back all reg writes except those to
+		 * mailbox regs.  See tg3_write_indirect_reg32().
+		 *
+		 * PCI Express 5750_A0 rev chips need this workaround too.
+		 */
 		tp->write32 = tg3_write_flush_reg32;
+	}
+
 
 	if ((tp->tg3_flags & TG3_FLAG_TXD_MBOX_HWBUG) ||
 	    (tp->tg3_flags & TG3_FLAG_MBOX_WRITE_REORDER)) {

drivers/net/tg3.h +0 -1

index dd04a46..c52acb0 100644
@@ -2199,7 +2199,6 @@ struct tg3 {
 #define TG3_FLAG_USE_LINKCHG_REG	0x00000008
 #define TG3_FLAG_USE_MI_INTERRUPT	0x00000010
 #define TG3_FLAG_ENABLE_ASF		0x00000020
-#define TG3_FLAG_5701_REG_WRITE_BUG	0x00000040
 #define TG3_FLAG_POLL_SERDES		0x00000080
 #define TG3_FLAG_MBOX_WRITE_REORDER	0x00000100
 #define TG3_FLAG_PCIX_TARGET_HWBUG	0x00000200

Jeff Garzik

2007-05-05 00:36 UTC #1 lore raw

Quoted 6 lines · Michael Chan

[TG3]: Eliminate the TG3_FLAG_5701_REG_WRITE_BUG flag.
This patch removes the use of the TG3_FLAG_5701_REG_WRITE_BUG flag.
Signed-off-by Matt Carlson <[email protected]>
Signed-off-by Michael Chan <[email protected]>

ACK, though the patch description is a bit unclear /why/ the flag can be eliminated -- even though it is certainly clear from reading the patch that this is a logical simplification.

Christoph Hellwig

2007-05-05 17:25 UTC #2 lore raw

In reply to Michael Chan

drivers/net/tg3.c static int __devinit tg3_get_invariants(struct tg3 *tp) · line 10708

Quoted 10 lines · Michael Chan

drivers/net/tg3.c +9 -0

+	else if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5701 ||
+		 ((tp->tg3_flags2 & TG3_FLG2_PCI_EXPRESS) &&
+		  tp->pci_chip_rev_id == CHIPREV_ID_5750_A0)) {
+		/* Back to back register writes can cause problems on this chip,
+		 * the workaround is to read back all reg writes except those to
+		 * mailbox regs.  See tg3_write_indirect_reg32().
+		 *
+		 * PCI Express 5750_A0 rev chips need this workaround too.
+		 */
 		tp->write32 = tg3_write_flush_reg32;

This comment is a little odd now. What about

/*
* Back to back register writes can cause problems on these
* chips, the workaround is to read back all reg writes
* except those to mailbox regs.
*
* See tg3_write_indirect_reg32().
*/

instead?

David Miller

maintainer

2007-05-05 19:48 UTC #3 lore raw

From: Christoph Hellwig <[email protected]>
Date: Sat, 5 May 2007 10:46:12 +0100

Quoted 22 lines · Michael Chan

Review
Quoted 10 lines
(quoted diff) +9 -0
+	else if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5701 ||
+		 ((tp->tg3_flags2 & TG3_FLG2_PCI_EXPRESS) &&
+		  tp->pci_chip_rev_id == CHIPREV_ID_5750_A0)) {
+		/* Back to back register writes can cause problems on this chip,
+		 * the workaround is to read back all reg writes except those to
+		 * mailbox regs.  See tg3_write_indirect_reg32().
+		 *
+		 * PCI Express 5750_A0 rev chips need this workaround too.
+		 */
 		tp->write32 = tg3_write_flush_reg32;
This comment is a little odd now. What about
/*
* Back to back register writes can cause problems on these
* chips, the workaround is to read back all reg writes
* except those to mailbox regs.
*
* See tg3_write_indirect_reg32().
*/

Quoted 1 line

instead?

Looks nicer to me too. I made this modification and added some more verbiage to the changelog as suggested by Jeff Garzik when I applied this, as follows:

commit 98efd8a6be79550767f5a9be6f3db8e7e9b747da

Author: Matt Carlson <[email protected]>
Date:   Sat May 5 12:47:25 2007 -0700

[TG3]: Eliminate the TG3_FLAG_5701_REG_WRITE_BUG flag.

This patch removes the use of the TG3_FLAG_5701_REG_WRITE_BUG flag. It's logic is only used to set a function pointer and thus the logic can be collapsed and the flag removed.

[ Comment tidy by Christoph Hellwig. -DaveM ]

Signed-off-by Matt Carlson <[email protected]>

Signed-off-by Michael Chan <[email protected]>

drivers/net/tg3.c +12 ~~-12~~

index ff157b7..4154e12 100644
@@ -10673,17 +10673,6 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
 	if (GET_CHIP_REV(tp->pci_chip_rev_id) == CHIPREV_5700_BX)
 		tp->tg3_flags |= TG3_FLAG_TXD_MBOX_HWBUG;
 
-	/* Back to back register writes can cause problems on this chip,
-	 * the workaround is to read back all reg writes except those to
-	 * mailbox regs.  See tg3_write_indirect_reg32().
-	 *
-	 * PCI Express 5750_A0 rev chips need this workaround too.
-	 */
-	if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5701 ||
-	    ((tp->tg3_flags2 & TG3_FLG2_PCI_EXPRESS) &&
-	     tp->pci_chip_rev_id == CHIPREV_ID_5750_A0))
-		tp->tg3_flags |= TG3_FLAG_5701_REG_WRITE_BUG;
-
 	if ((pci_state_reg & PCISTATE_BUS_SPEED_HIGH) != 0)
 		tp->tg3_flags |= TG3_FLAG_PCI_HIGH_SPEED;
 	if ((pci_state_reg & PCISTATE_BUS_32BIT) != 0)

@@ -10707,8 +10696,19 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
 	/* Various workaround register access methods */
 	if (tp->tg3_flags & TG3_FLAG_PCIX_TARGET_HWBUG)
 		tp->write32 = tg3_write_indirect_reg32;
-	else if (tp->tg3_flags & TG3_FLAG_5701_REG_WRITE_BUG)
+	else if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5701 ||
+		 ((tp->tg3_flags2 & TG3_FLG2_PCI_EXPRESS) &&
+		  tp->pci_chip_rev_id == CHIPREV_ID_5750_A0)) {
+		/*
+		 * Back to back register writes can cause problems on these
+		 * chips, the workaround is to read back all reg writes
+		 * except those to mailbox regs.
+		 *
+		 * See tg3_write_indirect_reg32().
+		 */
 		tp->write32 = tg3_write_flush_reg32;
+	}
+
 
 	if ((tp->tg3_flags & TG3_FLAG_TXD_MBOX_HWBUG) ||
 	    (tp->tg3_flags & TG3_FLAG_MBOX_WRITE_REORDER)) {

drivers/net/tg3.h +0 -1

index dd04a46..c52acb0 100644
@@ -2199,7 +2199,6 @@ struct tg3 {
 #define TG3_FLAG_USE_LINKCHG_REG	0x00000008
 #define TG3_FLAG_USE_MI_INTERRUPT	0x00000010
 #define TG3_FLAG_ENABLE_ASF		0x00000020
-#define TG3_FLAG_5701_REG_WRITE_BUG	0x00000040
 #define TG3_FLAG_POLL_SERDES		0x00000080
 #define TG3_FLAG_MBOX_WRITE_REORDER	0x00000100
 #define TG3_FLAG_PCIX_TARGET_HWBUG	0x00000200