Skip to contentSkip to conversation

- e1000_ethtool.c - convert macros to functions

PATCH In review Joe Perches 2 participants 7 messages Oct 31, 2007 – Nov 1, 2007

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Joe Perches

author
#0 lore raw

PATCH 1/1 - e1000_ethtool.c - convert macros to functions

Convert REG_PATTERN_TEST and REG_SET_AND_CHECK macros to functions Reduces x86 defconfig image by about 3k

compiled, untested (no hardware)

Signed-off-by Joe Perches <[email protected]>

New:

$ size vmlinux
   text    data     bss     dec     hex filename
4792735  490626  606208 5889569  59de21 vmlinux

Current:

$ size vmlinux
   text    data     bss     dec     hex filename
4795759  490626  606208 5892593  59e9f1 vmlinux

---

 drivers/net/e1000/e1000_ethtool.c |  185 +++++++++++++++++++++++++------------
 drivers/net/e1000/e1000_osdep.h   |   42 +++++----
 2 files changed, 149 insertions(+), 78 deletions(-)
drivers/net/e1000/e1000_ethtool.c +125 -60
index 667f18b..2627395 100644
@@ -728,37 +728,45 @@ err_setup:
 	return err;
 }
 
-#define REG_PATTERN_TEST(R, M, W)                                              \
-{                                                                              \
-	uint32_t pat, val;                                                     \
-	const uint32_t test[] = 					       \
-		{0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};              \
-	for (pat = 0; pat < ARRAY_SIZE(test); pat++) {           	       \
-		E1000_WRITE_REG(&adapter->hw, R, (test[pat] & W));             \
-		val = E1000_READ_REG(&adapter->hw, R);                         \
-		if (val != (test[pat] & W & M)) {                              \
-			DPRINTK(DRV, ERR, "pattern test reg %04X failed: got " \
-			        "0x%08X expected 0x%08X\n",                    \
-			        E1000_##R, val, (test[pat] & W & M));          \
-			*data = (adapter->hw.mac_type < e1000_82543) ?         \
-				E1000_82542_##R : E1000_##R;                   \
-			return 1;                                              \
-		}                                                              \
-	}                                                                      \
+static bool reg_pattern_test(struct e1000_adapter *adapter, uint64_t *data,
+			     int reg, uint32_t mask, uint32_t write)
+{
+	static const uint32_t test[] =
+		{0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};
+	uint8_t __iomem *address = adapter->hw.hw_addr + reg;
+	uint32_t read;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(test); i++) {
+		writel(write & test[i], address);
+		read = readl(address);
+		if (read != (write & test[i] & mask)) {
+			DPRINTK(DRV, ERR, "pattern test reg %04X failed: "
+				"got 0x%08X expected 0x%08X\n",
+			        reg, read, (write & test[i] & mask));
+			*data = reg;
+			return true;
+		}
+	}
+	return false;
 }
 
-#define REG_SET_AND_CHECK(R, M, W)                                             \
-{                                                                              \
-	uint32_t val;                                                          \
-	E1000_WRITE_REG(&adapter->hw, R, W & M);                               \
-	val = E1000_READ_REG(&adapter->hw, R);                                 \
-	if ((W & M) != (val & M)) {                                            \
-		DPRINTK(DRV, ERR, "set/check reg %04X test failed: got 0x%08X "\
-		        "expected 0x%08X\n", E1000_##R, (val & M), (W & M));   \
-		*data = (adapter->hw.mac_type < e1000_82543) ?                 \
-			E1000_82542_##R : E1000_##R;                           \
-		return 1;                                                      \
-	}                                                                      \
+static bool reg_set_and_check(struct e1000_adapter *adapter, uint64_t *data,
+			      int reg, uint32_t mask, uint32_t write)
+{
+	uint8_t __iomem *address = adapter->hw.hw_addr + reg;
+	uint32_t read;
+
+	writel(write & mask, address);
+	read = readl(address);
+	if ((read & mask) != (write & mask)) {
+		DPRINTK(DRV, ERR, "set/check reg %04X test failed: "
+			"got 0x%08X expected 0x%08X\n",
+			reg, (read & mask), (write & mask));
+		*data = reg;
+		return true;
+	}
+	return false;
 }
 
 static int

@@ -800,58 +808,115 @@ e1000_reg_test(struct e1000_adapter *adapter, uint64_t *data)
 	E1000_WRITE_REG(&adapter->hw, STATUS, before);
 
 	if (adapter->hw.mac_type != e1000_ich8lan) {
-		REG_PATTERN_TEST(FCAL, 0xFFFFFFFF, 0xFFFFFFFF);
-		REG_PATTERN_TEST(FCAH, 0x0000FFFF, 0xFFFFFFFF);
-		REG_PATTERN_TEST(FCT, 0x0000FFFF, 0xFFFFFFFF);
-		REG_PATTERN_TEST(VET, 0x0000FFFF, 0xFFFFFFFF);
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, FCAL),
+				     0xFFFFFFFF, 0xFFFFFFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, FCAH),
+				     0x0000FFFF, 0xFFFFFFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, FCT),
+				     0x0000FFFF, 0xFFFFFFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, VET),
+				     0x0000FFFF, 0xFFFFFFFF))
+			return 1;
 	}
 
-	REG_PATTERN_TEST(RDTR, 0x0000FFFF, 0xFFFFFFFF);
-	REG_PATTERN_TEST(RDBAH, 0xFFFFFFFF, 0xFFFFFFFF);
-	REG_PATTERN_TEST(RDLEN, 0x000FFF80, 0x000FFFFF);
-	REG_PATTERN_TEST(RDH, 0x0000FFFF, 0x0000FFFF);
-	REG_PATTERN_TEST(RDT, 0x0000FFFF, 0x0000FFFF);
-	REG_PATTERN_TEST(FCRTH, 0x0000FFF8, 0x0000FFF8);
-	REG_PATTERN_TEST(FCTTV, 0x0000FFFF, 0x0000FFFF);
-	REG_PATTERN_TEST(TIPG, 0x3FFFFFFF, 0x3FFFFFFF);
-	REG_PATTERN_TEST(TDBAH, 0xFFFFFFFF, 0xFFFFFFFF);
-	REG_PATTERN_TEST(TDLEN, 0x000FFF80, 0x000FFFFF);
+	if (reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDTR),
+			     0x0000FFFF, 0xFFFFFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDBAH),
+			     0xFFFFFFFF, 0xFFFFFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDLEN),
+			     0x000FFF80, 0x000FFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDH),
+			     0x0000FFFF, 0x0000FFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDT),
+			     0x0000FFFF, 0x0000FFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, FCRTH),
+			     0x0000FFF8, 0x0000FFF8) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, FCTTV),
+			     0x0000FFFF, 0x0000FFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, TIPG),
+			     0x3FFFFFFF, 0x3FFFFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, TDBAH),
+			     0xFFFFFFFF, 0xFFFFFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, TDLEN),
+			     0x000FFF80, 0x000FFFFF))
+		return 1;
 
-	REG_SET_AND_CHECK(RCTL, 0xFFFFFFFF, 0x00000000);
+	if (reg_set_and_check(adapter, data, E1000_REG(&adapter->hw, RCTL),
+			      0xFFFFFFFF, 0x00000000))
+		return 1;
 
 	before = (adapter->hw.mac_type == e1000_ich8lan ?
 	          0x06C3B33E : 0x06DFB3FE);
-	REG_SET_AND_CHECK(RCTL, before, 0x003FFFFB);
-	REG_SET_AND_CHECK(TCTL, 0xFFFFFFFF, 0x00000000);
+	if (reg_set_and_check(adapter, data, E1000_REG(&adapter->hw, RCTL),
+			      before, 0x003FFFFB) ||
+	    reg_set_and_check(adapter, data, E1000_REG(&adapter->hw, TCTL),
+			      0xFFFFFFFF, 0x00000000))
+		return 1;
 
 	if (adapter->hw.mac_type >= e1000_82543) {
 
-		REG_SET_AND_CHECK(RCTL, before, 0xFFFFFFFF);
-		REG_PATTERN_TEST(RDBAL, 0xFFFFFFF0, 0xFFFFFFFF);
-		if (adapter->hw.mac_type != e1000_ich8lan)
-			REG_PATTERN_TEST(TXCW, 0xC000FFFF, 0x0000FFFF);
-		REG_PATTERN_TEST(TDBAL, 0xFFFFFFF0, 0xFFFFFFFF);
-		REG_PATTERN_TEST(TIDV, 0x0000FFFF, 0x0000FFFF);
+		if (reg_set_and_check(adapter, data,
+				      E1000_REG(&adapter->hw, RCTL),
+				      before, 0xFFFFFFFF))
+			return 1;
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, RDBAL),
+				     0xFFFFFFF0, 0xFFFFFFFF))
+			return 1;
+		if (adapter->hw.mac_type != e1000_ich8lan) {
+			if (reg_pattern_test(adapter, data,
+					     E1000_REG(&adapter->hw, TXCW),
+					     0xC000FFFF, 0x0000FFFF))
+				return 1;
+		}
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, TDBAL),
+				     0xFFFFFFF0, 0xFFFFFFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, TIDV),
+				     0x0000FFFF, 0x0000FFFF))
+			return 1;
 		value = (adapter->hw.mac_type == e1000_ich8lan ?
 		         E1000_RAR_ENTRIES_ICH8LAN : E1000_RAR_ENTRIES);
 		for (i = 0; i < value; i++) {
-			REG_PATTERN_TEST(RA + (((i << 1) + 1) << 2), 0x8003FFFF,
-			                 0xFFFFFFFF);
+			if (reg_pattern_test(adapter, data,
+					     E1000_REG(&adapter->hw, RA) +
+					     (((i << 1) + 1) << 2),
+					     0x8003FFFF, 0xFFFFFFFF))
+				return 1;
 		}
 
 	} else {
 
-		REG_SET_AND_CHECK(RCTL, 0xFFFFFFFF, 0x01FFFFFF);
-		REG_PATTERN_TEST(RDBAL, 0xFFFFF000, 0xFFFFFFFF);
-		REG_PATTERN_TEST(TXCW, 0x0000FFFF, 0x0000FFFF);
-		REG_PATTERN_TEST(TDBAL, 0xFFFFF000, 0xFFFFFFFF);
+		if (reg_set_and_check(adapter, data,
+				      E1000_REG(&adapter->hw, RCTL),
+				      0xFFFFFFFF, 0x01FFFFFF))
+			return 1;
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, RDBAL),
+				     0xFFFFF000, 0xFFFFFFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, TXCW),
+				     0x0000FFFF, 0x0000FFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, TDBAL),
+				     0xFFFFF000, 0xFFFFFFFF))
+			return 1;
 
 	}
 
 	value = (adapter->hw.mac_type == e1000_ich8lan ?
 			E1000_MC_TBL_SIZE_ICH8LAN : E1000_MC_TBL_SIZE);
-	for (i = 0; i < value; i++)
-		REG_PATTERN_TEST(MTA + (i << 2), 0xFFFFFFFF, 0xFFFFFFFF);
+	for (i = 0; i < value; i++) {
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, MTA) + (i << 2),
+				     0xFFFFFFFF, 0xFFFFFFFF))
+			return 1;
+	}
 
 	*data = 0;
 	return 0;
drivers/net/e1000/e1000_osdep.h +24 -18

48 lines not on this page · raw message

#1 lore raw
Quoted 61 lines · Joe Perches

Convert REG_PATTERN_TEST and REG_SET_AND_CHECK macros to functions Reduces x86 defconfig image by about 3k

compiled, untested (no hardware)

Signed-off-by Joe Perches <[email protected]>

New:

$ size vmlinux
   text    data     bss     dec     hex filename
4792735  490626  606208 5889569  59de21 vmlinux

Current:

$ size vmlinux
   text    data     bss     dec     hex filename
4795759  490626  606208 5892593  59e9f1 vmlinux

---

 drivers/net/e1000/e1000_ethtool.c |  185 +++++++++++++++++++++++++------------
 drivers/net/e1000/e1000_osdep.h   |   42 +++++----
 2 files changed, 149 insertions(+), 78 deletions(-)
drivers/net/e1000/e1000_ethtool.c +11 -17
index 667f18b..2627395 100644
@@ -728,37 +728,45 @@ err_setup:
 	return err;
 }
 
-#define REG_PATTERN_TEST(R, M, W)                                              \
-{                                                                              \
-	uint32_t pat, val;                                                     \
-	const uint32_t test[] = 					       \
-		{0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};              \
-	for (pat = 0; pat < ARRAY_SIZE(test); pat++) {           	       \
-		E1000_WRITE_REG(&adapter->hw, R, (test[pat] & W));             \
-		val = E1000_READ_REG(&adapter->hw, R);                         \
-		if (val != (test[pat] & W & M)) {                              \
-			DPRINTK(DRV, ERR, "pattern test reg %04X failed: got " \
-			        "0x%08X expected 0x%08X\n",                    \
-			        E1000_##R, val, (test[pat] & W & M));          \
-			*data = (adapter->hw.mac_type < e1000_82543) ?         \
-				E1000_82542_##R : E1000_##R;                   \
-			return 1;                                              \
-		}                                                              \
-	}                                                                      \
+static bool reg_pattern_test(struct e1000_adapter *adapter, uint64_t *data,
+			     int reg, uint32_t mask, uint32_t write)
+{
+	static const uint32_t test[] =
+		{0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};
+	uint8_t __iomem *address = adapter->hw.hw_addr + reg;
+	uint32_t read;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(test); i++) {
+		writel(write & test[i], address);

drivers/net/e1000/e1000_ethtool.c err_setup: · line 751

Quoted 10 lines
drivers/net/e1000/e1000_ethtool.c +10 -0
+		read = readl(address);
+		if (read != (write & test[i] & mask)) {
+			DPRINTK(DRV, ERR, "pattern test reg %04X failed: "
+				"got 0x%08X expected 0x%08X\n",
+			        reg, read, (write & test[i] & mask));
+			*data = reg;
+			return true;
+		}
+	}
+	return false;

that's not a bad idea, however see below:

Quoted 44 lines
drivers/net/e1000/e1000_ethtool.c +19 -16
 }
 
-#define REG_SET_AND_CHECK(R, M, W)                                             \
-{                                                                              \
-	uint32_t val;                                                          \
-	E1000_WRITE_REG(&adapter->hw, R, W & M);                               \
-	val = E1000_READ_REG(&adapter->hw, R);                                 \
-	if ((W & M) != (val & M)) {                                            \
-		DPRINTK(DRV, ERR, "set/check reg %04X test failed: got 0x%08X "\
-		        "expected 0x%08X\n", E1000_##R, (val & M), (W & M));   \
-		*data = (adapter->hw.mac_type < e1000_82543) ?                 \
-			E1000_82542_##R : E1000_##R;                           \
-		return 1;                                                      \
-	}                                                                      \
+static bool reg_set_and_check(struct e1000_adapter *adapter, uint64_t *data,
+			      int reg, uint32_t mask, uint32_t write)
+{
+	uint8_t __iomem *address = adapter->hw.hw_addr + reg;
+	uint32_t read;
+
+	writel(write & mask, address);
+	read = readl(address);
+	if ((read & mask) != (write & mask)) {
+		DPRINTK(DRV, ERR, "set/check reg %04X test failed: "
+			"got 0x%08X expected 0x%08X\n",
+			reg, (read & mask), (write & mask));
+		*data = reg;
+		return true;
+	}
+	return false;
 }
 
 static int

@@ -800,58 +808,115 @@ e1000_reg_test(struct e1000_adapter *adapter, uint64_t *data)
 	E1000_WRITE_REG(&adapter->hw, STATUS, before);
 
 	if (adapter->hw.mac_type != e1000_ich8lan) {
-		REG_PATTERN_TEST(FCAL, 0xFFFFFFFF, 0xFFFFFFFF);
-		REG_PATTERN_TEST(FCAH, 0x0000FFFF, 0xFFFFFFFF);
-		REG_PATTERN_TEST(FCT, 0x0000FFFF, 0xFFFFFFFF);
-		REG_PATTERN_TEST(VET, 0x0000FFFF, 0xFFFFFFFF);
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, FCAL),
+				     0xFFFFFFFF, 0xFFFFFFFF) ||

drivers/net/e1000/e1000_ethtool.c e1000_reg_test(struct e1000_adapter *adapter, uint64_t *data) · line 823

Quoted 10 lines
drivers/net/e1000/e1000_ethtool.c +10 -0
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, FCAH),
+				     0x0000FFFF, 0xFFFFFFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, FCT),
+				     0x0000FFFF, 0xFFFFFFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, VET),
+				     0x0000FFFF, 0xFFFFFFFF))
+			return 1;

can't we keep the macro here (and just make it call the function instead of expanding). the resulting code is much more lenghty and contains all these logic traps that the previous code didn't have.

just have the macro expand to if (reg_pattern_test(...)) return 1) and you don't need to change any of the calling lines.

Quoted 166 lines
drivers/net/e1000/e1000_ethtool.c +75 -27
 	}
 
-	REG_PATTERN_TEST(RDTR, 0x0000FFFF, 0xFFFFFFFF);
-	REG_PATTERN_TEST(RDBAH, 0xFFFFFFFF, 0xFFFFFFFF);
-	REG_PATTERN_TEST(RDLEN, 0x000FFF80, 0x000FFFFF);
-	REG_PATTERN_TEST(RDH, 0x0000FFFF, 0x0000FFFF);
-	REG_PATTERN_TEST(RDT, 0x0000FFFF, 0x0000FFFF);
-	REG_PATTERN_TEST(FCRTH, 0x0000FFF8, 0x0000FFF8);
-	REG_PATTERN_TEST(FCTTV, 0x0000FFFF, 0x0000FFFF);
-	REG_PATTERN_TEST(TIPG, 0x3FFFFFFF, 0x3FFFFFFF);
-	REG_PATTERN_TEST(TDBAH, 0xFFFFFFFF, 0xFFFFFFFF);
-	REG_PATTERN_TEST(TDLEN, 0x000FFF80, 0x000FFFFF);
+	if (reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDTR),
+			     0x0000FFFF, 0xFFFFFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDBAH),
+			     0xFFFFFFFF, 0xFFFFFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDLEN),
+			     0x000FFF80, 0x000FFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDH),
+			     0x0000FFFF, 0x0000FFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDT),
+			     0x0000FFFF, 0x0000FFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, FCRTH),
+			     0x0000FFF8, 0x0000FFF8) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, FCTTV),
+			     0x0000FFFF, 0x0000FFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, TIPG),
+			     0x3FFFFFFF, 0x3FFFFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, TDBAH),
+			     0xFFFFFFFF, 0xFFFFFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, TDLEN),
+			     0x000FFF80, 0x000FFFFF))
+		return 1;
 
-	REG_SET_AND_CHECK(RCTL, 0xFFFFFFFF, 0x00000000);
+	if (reg_set_and_check(adapter, data, E1000_REG(&adapter->hw, RCTL),
+			      0xFFFFFFFF, 0x00000000))
+		return 1;
 
 	before = (adapter->hw.mac_type == e1000_ich8lan ?
 	          0x06C3B33E : 0x06DFB3FE);
-	REG_SET_AND_CHECK(RCTL, before, 0x003FFFFB);
-	REG_SET_AND_CHECK(TCTL, 0xFFFFFFFF, 0x00000000);
+	if (reg_set_and_check(adapter, data, E1000_REG(&adapter->hw, RCTL),
+			      before, 0x003FFFFB) ||
+	    reg_set_and_check(adapter, data, E1000_REG(&adapter->hw, TCTL),
+			      0xFFFFFFFF, 0x00000000))
+		return 1;
 
 	if (adapter->hw.mac_type >= e1000_82543) {
 
-		REG_SET_AND_CHECK(RCTL, before, 0xFFFFFFFF);
-		REG_PATTERN_TEST(RDBAL, 0xFFFFFFF0, 0xFFFFFFFF);
-		if (adapter->hw.mac_type != e1000_ich8lan)
-			REG_PATTERN_TEST(TXCW, 0xC000FFFF, 0x0000FFFF);
-		REG_PATTERN_TEST(TDBAL, 0xFFFFFFF0, 0xFFFFFFFF);
-		REG_PATTERN_TEST(TIDV, 0x0000FFFF, 0x0000FFFF);
+		if (reg_set_and_check(adapter, data,
+				      E1000_REG(&adapter->hw, RCTL),
+				      before, 0xFFFFFFFF))
+			return 1;
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, RDBAL),
+				     0xFFFFFFF0, 0xFFFFFFFF))
+			return 1;
+		if (adapter->hw.mac_type != e1000_ich8lan) {
+			if (reg_pattern_test(adapter, data,
+					     E1000_REG(&adapter->hw, TXCW),
+					     0xC000FFFF, 0x0000FFFF))
+				return 1;
+		}
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, TDBAL),
+				     0xFFFFFFF0, 0xFFFFFFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, TIDV),
+				     0x0000FFFF, 0x0000FFFF))
+			return 1;
 		value = (adapter->hw.mac_type == e1000_ich8lan ?
 		         E1000_RAR_ENTRIES_ICH8LAN : E1000_RAR_ENTRIES);
 		for (i = 0; i < value; i++) {
-			REG_PATTERN_TEST(RA + (((i << 1) + 1) << 2), 0x8003FFFF,
-			                 0xFFFFFFFF);
+			if (reg_pattern_test(adapter, data,
+					     E1000_REG(&adapter->hw, RA) +
+					     (((i << 1) + 1) << 2),
+					     0x8003FFFF, 0xFFFFFFFF))
+				return 1;
 		}
 
 	} else {
 
-		REG_SET_AND_CHECK(RCTL, 0xFFFFFFFF, 0x01FFFFFF);
-		REG_PATTERN_TEST(RDBAL, 0xFFFFF000, 0xFFFFFFFF);
-		REG_PATTERN_TEST(TXCW, 0x0000FFFF, 0x0000FFFF);
-		REG_PATTERN_TEST(TDBAL, 0xFFFFF000, 0xFFFFFFFF);
+		if (reg_set_and_check(adapter, data,
+				      E1000_REG(&adapter->hw, RCTL),
+				      0xFFFFFFFF, 0x01FFFFFF))
+			return 1;
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, RDBAL),
+				     0xFFFFF000, 0xFFFFFFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, TXCW),
+				     0x0000FFFF, 0x0000FFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, TDBAL),
+				     0xFFFFF000, 0xFFFFFFFF))
+			return 1;
 
 	}
 
 	value = (adapter->hw.mac_type == e1000_ich8lan ?
 			E1000_MC_TBL_SIZE_ICH8LAN : E1000_MC_TBL_SIZE);
-	for (i = 0; i < value; i++)
-		REG_PATTERN_TEST(MTA + (i << 2), 0xFFFFFFFF, 0xFFFFFFFF);
+	for (i = 0; i < value; i++) {
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, MTA) + (i << 2),
+				     0xFFFFFFFF, 0xFFFFFFFF))
+			return 1;
+	}
 
 	*data = 0;
 	return 0;
drivers/net/e1000/e1000_osdep.h +14 -18
index 10af742..7d70eb6 100644
@@ -61,24 +61,30 @@ typedef enum {
 #define DEBUGOUT3 DEBUGOUT2
 #define DEBUGOUT7 DEBUGOUT3
 
-
-#define E1000_WRITE_REG(a, reg, value) ( \
-    writel((value), ((a)->hw_addr + \
-        (((a)->mac_type >= e1000_82543) ? E1000_##reg : E1000_82542_##reg))))
-
-#define E1000_READ_REG(a, reg) ( \
-    readl((a)->hw_addr + \
-        (((a)->mac_type >= e1000_82543) ? E1000_##reg : E1000_82542_##reg)))
-
-#define E1000_WRITE_REG_ARRAY(a, reg, offset, value) ( \
-    writel((value), ((a)->hw_addr + \
-        (((a)->mac_type >= e1000_82543) ? E1000_##reg : E1000_82542_##reg) + \
-        ((offset) << 2))))
-
-#define E1000_READ_REG_ARRAY(a, reg, offset) ( \
-    readl((a)->hw_addr + \
-        (((a)->mac_type >= e1000_82543) ? E1000_##reg : E1000_82542_##reg) + \
-        ((offset) << 2)))
+#define E1000_REG(a, reg) \
+	(((a)->mac_type >= e1000_82543) ? E1000_##reg : E1000_82542_##reg)
+
+#define E1000_WRITE_REG(a, reg, value)					\
+	(writel((value), ((a)->hw_addr +				\
+			  (((a)->mac_type >= e1000_82543)		\
+			   ? E1000_##reg : E1000_82542_##reg))))
+
+#define E1000_READ_REG(a, reg)				\
+	(readl((a)->hw_addr +				\
+	       (((a)->mac_type >= e1000_82543)		\
+		? E1000_##reg : E1000_82542_##reg)))
+
+#define E1000_WRITE_REG_ARRAY(a, reg, offset, value)	       \

drivers/net/e1000/e1000_osdep.h typedef enum { · line 87

Quoted 10 lines
drivers/net/e1000/e1000_osdep.h +10 -0
+	(writel((value), ((a)->hw_addr +		       \
+			  (((a)->mac_type >= e1000_82543)      \
+			   ? E1000_##reg : E1000_82542_##reg) +\
+			  ((offset) << 2))))
+
+#define E1000_READ_REG_ARRAY(a, reg, offset)		\
+	(readl((a)->hw_addr +				\
+	       (((a)->mac_type >= e1000_82543)		\
+		? E1000_##reg : E1000_82542_##reg) +	\
+	       ((offset) << 2)))

did you have to change these macro's ?

also, I'm a bit inclined to prefer a patch for e1000e for now as we're about to move the pci-express hardware over, but we can certainly merge something like this in e1000 after the move as well.

Auke

author
#2 lore raw
Quoted 7 lines · Auke

Joe Perches wrote:
that's not a bad idea, however see below:
can't we keep the macro here (and just make it call the function instead of expanding). the resulting code is much more lenghty and contains all these logic traps that the previous code didn't have.
just have the macro expand to if (reg_pattern_test(...)) return 1) and you don't need to change any of the calling lines.

You could define something like:

#define REG_PATTERN_TEST(reg, mask, write) \
	if (reg_pattern_test(adapter, data, \
			     E1000_REG(&adapter->hw, reg), \
			     mask, write)) \
		return 1;

But isn't the macro with an embedded return a bit too obfuscating?

Quoted 7 lines

drivers/net/e1000/e1000_osdep.h typedef enum { · line 87

Quoted 5 lines
drivers/net/e1000/e1000_osdep.h +5 -0
+#define E1000_READ_REG_ARRAY(a, reg, offset)		\
+	(readl((a)->hw_addr +				\
+	       (((a)->mac_type >= e1000_82543)		\
+		? E1000_##reg : E1000_82542_##reg) +	\
+	       ((offset) << 2)))

did you have to change these macro's ?

No. Your choice to keep/remove.
I did want to use the E1000_REG or a new E1000_REG_ADDR macro.

Quoted 3 lines

also, I'm a bit inclined to prefer a patch for e1000e for now as we're about to move the pci-express hardware over, but we can certainly merge something like this in e1000 after the move as well.

Simple enough.

When is e1000e scheduled to be part of defconfig?

cheers, Joe

#3 lore raw
Quoted 11 lines · Joe Perches
Quoted 7 lines · Auke

Joe Perches wrote:
that's not a bad idea, however see below:
can't we keep the macro here (and just make it call the function instead of expanding). the resulting code is much more lenghty and contains all these logic traps that the previous code didn't have.
just have the macro expand to if (reg_pattern_test(...)) return 1) and you don't need to change any of the calling lines.

You could define something like:

Quoted 7 lines
#define REG_PATTERN_TEST(reg, mask, write) \
	if (reg_pattern_test(adapter, data, \
			     E1000_REG(&adapter->hw, reg), \
			     mask, write)) \
		return 1;

But isn't the macro with an embedded return a bit too obfuscating?

in this case it saves a bunch of time reading and makes the code actually more readable and robust IMO. Now we're explicitly mobming out on an error. with your patch we need to assure that for every call to reg_pattern_test we check the return value.

Quoted 10 lines
Quoted 6 lines

drivers/net/e1000/e1000_osdep.h typedef enum { · line 87

Quoted 5 lines
drivers/net/e1000/e1000_osdep.h +5 -0
+#define E1000_READ_REG_ARRAY(a, reg, offset)		\
+	(readl((a)->hw_addr +				\
+	       (((a)->mac_type >= e1000_82543)		\
+		? E1000_##reg : E1000_82542_##reg) +	\
+	       ((offset) << 2)))

did you have to change these macro's ?

No. Your choice to keep/remove.
I did want to use the E1000_REG or a new E1000_REG_ADDR macro.

Quoted 7 lines
Quoted 3 lines

also, I'm a bit inclined to prefer a patch for e1000e for now as we're about to move the pci-express hardware over, but we can certainly merge something like this in e1000 after the move as well.

Simple enough.

When is e1000e scheduled to be part of defconfig?

I don't know about defconfig, but you can already get e1000 in 2.6.24rc1...

Auke

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

author
#4 lore raw
In reply to Kok, Auke
Quoted 3 lines · Auke
Quoted 2 lines · Joe Perches

Convert REG_PATTERN_TEST and REG_SET_AND_CHECK macros to functions Reduces x86 defconfig image by about 3k

Convert REG_PATTERN_TEST and REG_SET_AND_CHECK macros to functions Reduces x86 defconfig image by about 3k

compiled, untested (no hardware)

Signed-off-by Joe Perches <[email protected]>

New:

$ size vmlinux
   text    data     bss     dec     hex filename
4792735  490626  606208 5889569  59de21 vmlinux

Current:

$ size vmlinux
   text    data     bss     dec     hex filename
4795759  490626  606208 5892593  59e9f1 vmlinux

Changed since last submission:

Fixed existing controler->controller typo Added E1000_REG_ADDR macro
Added E1000_FLASH_ADDR macro
The #define E1000_PHY_CTRL conflicted with the PHY_CTRL expansion when using the E1000_REG_ADDR macro and was renamed to E1000_PHY_CTRL_CSR

Signed-off-by Joe Perches <[email protected]>

---

 drivers/net/e1000/e1000_ethtool.c |  185 +++++++++++++++++++++++++------------
 drivers/net/e1000/e1000_hw.c      |   20 ++--
 drivers/net/e1000/e1000_hw.h      |    6 +-
 drivers/net/e1000/e1000_osdep.h   |   70 +++++++--------
 4 files changed, 170 insertions(+), 111 deletions(-)
drivers/net/e1000/e1000_ethtool.c +125 -60
index 667f18b..2627395 100644
@@ -728,37 +728,45 @@ err_setup:
 	return err;
 }
 
-#define REG_PATTERN_TEST(R, M, W)                                              \
-{                                                                              \
-	uint32_t pat, val;                                                     \
-	const uint32_t test[] = 					       \
-		{0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};              \
-	for (pat = 0; pat < ARRAY_SIZE(test); pat++) {           	       \
-		E1000_WRITE_REG(&adapter->hw, R, (test[pat] & W));             \
-		val = E1000_READ_REG(&adapter->hw, R);                         \
-		if (val != (test[pat] & W & M)) {                              \
-			DPRINTK(DRV, ERR, "pattern test reg %04X failed: got " \
-			        "0x%08X expected 0x%08X\n",                    \
-			        E1000_##R, val, (test[pat] & W & M));          \
-			*data = (adapter->hw.mac_type < e1000_82543) ?         \
-				E1000_82542_##R : E1000_##R;                   \
-			return 1;                                              \
-		}                                                              \
-	}                                                                      \
+static bool reg_pattern_test(struct e1000_adapter *adapter, uint64_t *data,
+			     int reg, uint32_t mask, uint32_t write)
+{
+	static const uint32_t test[] =
+		{0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};
+	uint8_t __iomem *address = adapter->hw.hw_addr + reg;
+	uint32_t read;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(test); i++) {
+		writel(write & test[i], address);
+		read = readl(address);
+		if (read != (write & test[i] & mask)) {
+			DPRINTK(DRV, ERR, "pattern test reg %04X failed: "
+				"got 0x%08X expected 0x%08X\n",
+			        reg, read, (write & test[i] & mask));
+			*data = reg;
+			return true;
+		}
+	}
+	return false;
 }
 
-#define REG_SET_AND_CHECK(R, M, W)                                             \
-{                                                                              \
-	uint32_t val;                                                          \
-	E1000_WRITE_REG(&adapter->hw, R, W & M);                               \
-	val = E1000_READ_REG(&adapter->hw, R);                                 \
-	if ((W & M) != (val & M)) {                                            \
-		DPRINTK(DRV, ERR, "set/check reg %04X test failed: got 0x%08X "\
-		        "expected 0x%08X\n", E1000_##R, (val & M), (W & M));   \
-		*data = (adapter->hw.mac_type < e1000_82543) ?                 \
-			E1000_82542_##R : E1000_##R;                           \
-		return 1;                                                      \
-	}                                                                      \
+static bool reg_set_and_check(struct e1000_adapter *adapter, uint64_t *data,
+			      int reg, uint32_t mask, uint32_t write)
+{
+	uint8_t __iomem *address = adapter->hw.hw_addr + reg;
+	uint32_t read;
+
+	writel(write & mask, address);
+	read = readl(address);
+	if ((read & mask) != (write & mask)) {
+		DPRINTK(DRV, ERR, "set/check reg %04X test failed: "
+			"got 0x%08X expected 0x%08X\n",
+			reg, (read & mask), (write & mask));
+		*data = reg;
+		return true;
+	}
+	return false;
 }
 
 static int

@@ -800,58 +808,115 @@ e1000_reg_test(struct e1000_adapter *adapter, uint64_t *data)
 	E1000_WRITE_REG(&adapter->hw, STATUS, before);
 
 	if (adapter->hw.mac_type != e1000_ich8lan) {
-		REG_PATTERN_TEST(FCAL, 0xFFFFFFFF, 0xFFFFFFFF);
-		REG_PATTERN_TEST(FCAH, 0x0000FFFF, 0xFFFFFFFF);
-		REG_PATTERN_TEST(FCT, 0x0000FFFF, 0xFFFFFFFF);
-		REG_PATTERN_TEST(VET, 0x0000FFFF, 0xFFFFFFFF);
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, FCAL),
+				     0xFFFFFFFF, 0xFFFFFFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, FCAH),
+				     0x0000FFFF, 0xFFFFFFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, FCT),
+				     0x0000FFFF, 0xFFFFFFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, VET),
+				     0x0000FFFF, 0xFFFFFFFF))
+			return 1;
 	}
 
-	REG_PATTERN_TEST(RDTR, 0x0000FFFF, 0xFFFFFFFF);
-	REG_PATTERN_TEST(RDBAH, 0xFFFFFFFF, 0xFFFFFFFF);
-	REG_PATTERN_TEST(RDLEN, 0x000FFF80, 0x000FFFFF);
-	REG_PATTERN_TEST(RDH, 0x0000FFFF, 0x0000FFFF);
-	REG_PATTERN_TEST(RDT, 0x0000FFFF, 0x0000FFFF);
-	REG_PATTERN_TEST(FCRTH, 0x0000FFF8, 0x0000FFF8);
-	REG_PATTERN_TEST(FCTTV, 0x0000FFFF, 0x0000FFFF);
-	REG_PATTERN_TEST(TIPG, 0x3FFFFFFF, 0x3FFFFFFF);
-	REG_PATTERN_TEST(TDBAH, 0xFFFFFFFF, 0xFFFFFFFF);
-	REG_PATTERN_TEST(TDLEN, 0x000FFF80, 0x000FFFFF);
+	if (reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDTR),
+			     0x0000FFFF, 0xFFFFFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDBAH),
+			     0xFFFFFFFF, 0xFFFFFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDLEN),
+			     0x000FFF80, 0x000FFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDH),
+			     0x0000FFFF, 0x0000FFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, RDT),
+			     0x0000FFFF, 0x0000FFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, FCRTH),
+			     0x0000FFF8, 0x0000FFF8) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, FCTTV),
+			     0x0000FFFF, 0x0000FFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, TIPG),
+			     0x3FFFFFFF, 0x3FFFFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, TDBAH),
+			     0xFFFFFFFF, 0xFFFFFFFF) ||
+	    reg_pattern_test(adapter, data, E1000_REG(&adapter->hw, TDLEN),
+			     0x000FFF80, 0x000FFFFF))
+		return 1;
 
-	REG_SET_AND_CHECK(RCTL, 0xFFFFFFFF, 0x00000000);
+	if (reg_set_and_check(adapter, data, E1000_REG(&adapter->hw, RCTL),
+			      0xFFFFFFFF, 0x00000000))
+		return 1;
 
 	before = (adapter->hw.mac_type == e1000_ich8lan ?
 	          0x06C3B33E : 0x06DFB3FE);
-	REG_SET_AND_CHECK(RCTL, before, 0x003FFFFB);
-	REG_SET_AND_CHECK(TCTL, 0xFFFFFFFF, 0x00000000);
+	if (reg_set_and_check(adapter, data, E1000_REG(&adapter->hw, RCTL),
+			      before, 0x003FFFFB) ||
+	    reg_set_and_check(adapter, data, E1000_REG(&adapter->hw, TCTL),
+			      0xFFFFFFFF, 0x00000000))
+		return 1;
 
 	if (adapter->hw.mac_type >= e1000_82543) {
 
-		REG_SET_AND_CHECK(RCTL, before, 0xFFFFFFFF);
-		REG_PATTERN_TEST(RDBAL, 0xFFFFFFF0, 0xFFFFFFFF);
-		if (adapter->hw.mac_type != e1000_ich8lan)
-			REG_PATTERN_TEST(TXCW, 0xC000FFFF, 0x0000FFFF);
-		REG_PATTERN_TEST(TDBAL, 0xFFFFFFF0, 0xFFFFFFFF);
-		REG_PATTERN_TEST(TIDV, 0x0000FFFF, 0x0000FFFF);
+		if (reg_set_and_check(adapter, data,
+				      E1000_REG(&adapter->hw, RCTL),
+				      before, 0xFFFFFFFF))
+			return 1;
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, RDBAL),
+				     0xFFFFFFF0, 0xFFFFFFFF))
+			return 1;
+		if (adapter->hw.mac_type != e1000_ich8lan) {
+			if (reg_pattern_test(adapter, data,
+					     E1000_REG(&adapter->hw, TXCW),
+					     0xC000FFFF, 0x0000FFFF))
+				return 1;
+		}
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, TDBAL),
+				     0xFFFFFFF0, 0xFFFFFFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, TIDV),
+				     0x0000FFFF, 0x0000FFFF))
+			return 1;
 		value = (adapter->hw.mac_type == e1000_ich8lan ?
 		         E1000_RAR_ENTRIES_ICH8LAN : E1000_RAR_ENTRIES);
 		for (i = 0; i < value; i++) {
-			REG_PATTERN_TEST(RA + (((i << 1) + 1) << 2), 0x8003FFFF,
-			                 0xFFFFFFFF);
+			if (reg_pattern_test(adapter, data,
+					     E1000_REG(&adapter->hw, RA) +
+					     (((i << 1) + 1) << 2),
+					     0x8003FFFF, 0xFFFFFFFF))
+				return 1;
 		}
 
 	} else {
 
-		REG_SET_AND_CHECK(RCTL, 0xFFFFFFFF, 0x01FFFFFF);
-		REG_PATTERN_TEST(RDBAL, 0xFFFFF000, 0xFFFFFFFF);
-		REG_PATTERN_TEST(TXCW, 0x0000FFFF, 0x0000FFFF);
-		REG_PATTERN_TEST(TDBAL, 0xFFFFF000, 0xFFFFFFFF);
+		if (reg_set_and_check(adapter, data,
+				      E1000_REG(&adapter->hw, RCTL),
+				      0xFFFFFFFF, 0x01FFFFFF))
+			return 1;
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, RDBAL),
+				     0xFFFFF000, 0xFFFFFFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, TXCW),
+				     0x0000FFFF, 0x0000FFFF) ||
+		    reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, TDBAL),
+				     0xFFFFF000, 0xFFFFFFFF))
+			return 1;
 
 	}
 
 	value = (adapter->hw.mac_type == e1000_ich8lan ?
 			E1000_MC_TBL_SIZE_ICH8LAN : E1000_MC_TBL_SIZE);
-	for (i = 0; i < value; i++)
-		REG_PATTERN_TEST(MTA + (i << 2), 0xFFFFFFFF, 0xFFFFFFFF);
+	for (i = 0; i < value; i++) {
+		if (reg_pattern_test(adapter, data,
+				     E1000_REG(&adapter->hw, MTA) + (i << 2),
+				     0xFFFFFFFF, 0xFFFFFFFF))
+			return 1;
+	}
 
 	*data = 0;
 	return 0;
drivers/net/e1000/e1000_hw.c +10 -10
index 7c6888c..7af6b6f 100644
@@ -3947,8 +3947,8 @@ e1000_phy_powerdown_workaround(struct e1000_hw *hw)
 
     do {
         /* Disable link */
-        reg = E1000_READ_REG(hw, PHY_CTRL);
-        E1000_WRITE_REG(hw, PHY_CTRL, reg | E1000_PHY_CTRL_GBE_DISABLE |
+        reg = E1000_READ_REG(hw, PHY_CTRL_CSR);
+        E1000_WRITE_REG(hw, PHY_CTRL_CSR, reg | E1000_PHY_CTRL_GBE_DISABLE |
                         E1000_PHY_CTRL_NOND0A_GBE_DISABLE);
 
         /* Write VR power-down enable - bits 9:8 should be 10b */

@@ -4023,8 +4023,8 @@ e1000_kumeran_lock_loss_workaround(struct e1000_hw *hw)
             mdelay(5);
         }
         /* Disable GigE link negotiation */
-        reg = E1000_READ_REG(hw, PHY_CTRL);
-        E1000_WRITE_REG(hw, PHY_CTRL, reg | E1000_PHY_CTRL_GBE_DISABLE |
+        reg = E1000_READ_REG(hw, PHY_CTRL_CSR);
+        E1000_WRITE_REG(hw, PHY_CTRL_CSR, reg | E1000_PHY_CTRL_GBE_DISABLE |
                         E1000_PHY_CTRL_NOND0A_GBE_DISABLE);
 
         /* unable to acquire PCS lock */

@@ -7243,7 +7243,7 @@ e1000_set_d3_lplu_state(struct e1000_hw *hw,
         /* MAC writes into PHY register based on the state transition
          * and start auto-negotiation. SW driver can overwrite the settings
          * in CSR PHY power control E1000_PHY_CTRL register. */
-        phy_ctrl = E1000_READ_REG(hw, PHY_CTRL);
+        phy_ctrl = E1000_READ_REG(hw, PHY_CTRL_CSR);
     } else {
         ret_val = e1000_read_phy_reg(hw, IGP02E1000_PHY_POWER_MGMT, &phy_data);
         if (ret_val)

@@ -7260,7 +7260,7 @@ e1000_set_d3_lplu_state(struct e1000_hw *hw,
         } else {
             if (hw->mac_type == e1000_ich8lan) {
                 phy_ctrl &= ~E1000_PHY_CTRL_NOND0A_LPLU;
-                E1000_WRITE_REG(hw, PHY_CTRL, phy_ctrl);
+                E1000_WRITE_REG(hw, PHY_CTRL_CSR, phy_ctrl);
             } else {
                 phy_data &= ~IGP02E1000_PM_D3_LPLU;
                 ret_val = e1000_write_phy_reg(hw, IGP02E1000_PHY_POWER_MGMT,

@@ -7311,7 +7311,7 @@ e1000_set_d3_lplu_state(struct e1000_hw *hw,
         } else {
             if (hw->mac_type == e1000_ich8lan) {
                 phy_ctrl |= E1000_PHY_CTRL_NOND0A_LPLU;
-                E1000_WRITE_REG(hw, PHY_CTRL, phy_ctrl);
+                E1000_WRITE_REG(hw, PHY_CTRL_CSR, phy_ctrl);
             } else {
                 phy_data |= IGP02E1000_PM_D3_LPLU;
                 ret_val = e1000_write_phy_reg(hw, IGP02E1000_PHY_POWER_MGMT,

@@ -7362,7 +7362,7 @@ e1000_set_d0_lplu_state(struct e1000_hw *hw,
         return E1000_SUCCESS;
 
     if (hw->mac_type == e1000_ich8lan) {
-        phy_ctrl = E1000_READ_REG(hw, PHY_CTRL);
+        phy_ctrl = E1000_READ_REG(hw, PHY_CTRL_CSR);
     } else {
         ret_val = e1000_read_phy_reg(hw, IGP02E1000_PHY_POWER_MGMT, &phy_data);
         if (ret_val)

@@ -7372,7 +7372,7 @@ e1000_set_d0_lplu_state(struct e1000_hw *hw,
     if (!active) {
         if (hw->mac_type == e1000_ich8lan) {
             phy_ctrl &= ~E1000_PHY_CTRL_D0A_LPLU;
-            E1000_WRITE_REG(hw, PHY_CTRL, phy_ctrl);
+            E1000_WRITE_REG(hw, PHY_CTRL_CSR, phy_ctrl);
         } else {
             phy_data &= ~IGP02E1000_PM_D0_LPLU;
             ret_val = e1000_write_phy_reg(hw, IGP02E1000_PHY_POWER_MGMT, phy_data);

@@ -7413,7 +7413,7 @@ e1000_set_d0_lplu_state(struct e1000_hw *hw,
 
         if (hw->mac_type == e1000_ich8lan) {
             phy_ctrl |= E1000_PHY_CTRL_D0A_LPLU;
-            E1000_WRITE_REG(hw, PHY_CTRL, phy_ctrl);
+            E1000_WRITE_REG(hw, PHY_CTRL_CSR, phy_ctrl);
         } else {
             phy_data |= IGP02E1000_PM_D0_LPLU;
             ret_val = e1000_write_phy_reg(hw, IGP02E1000_PHY_POWER_MGMT, phy_data);
drivers/net/e1000/e1000_hw.h +3 -3
index a2a86c5..7fa4b60 100644
@@ -41,7 +41,7 @@ struct e1000_hw;
 struct e1000_hw_stats;
 
 /* Enumerated types specific to the e1000 hardware */
-/* Media Access Controlers */
+/* Media Access Controllers */
 typedef enum {
     e1000_undefined = 0,
     e1000_82542_rev2_0,

@@ -922,7 +922,7 @@ struct e1000_ffvt_entry {
 #define E1000_LEDCTL   0x00E00  /* LED Control - RW */
 #define E1000_EXTCNF_CTRL  0x00F00  /* Extended Configuration Control */
 #define E1000_EXTCNF_SIZE  0x00F08  /* Extended Configuration Size */
-#define E1000_PHY_CTRL     0x00F10  /* PHY Control Register in CSR */
+#define E1000_PHY_CTRL_CSR 0x00F10  /* PHY Control Register in CSR */
 #define FEXTNVM_SW_CONFIG  0x0001
 #define E1000_PBA      0x01000  /* Packet Buffer Allocation - RW */
 #define E1000_PBS      0x01008  /* Packet Buffer Size */

@@ -1178,7 +1178,7 @@ struct e1000_ffvt_entry {
 #define E1000_82542_FLOP     E1000_FLOP
 #define E1000_82542_EXTCNF_CTRL  E1000_EXTCNF_CTRL
 #define E1000_82542_EXTCNF_SIZE  E1000_EXTCNF_SIZE
-#define E1000_82542_PHY_CTRL E1000_PHY_CTRL
+#define E1000_82542_PHY_CTRL_CSR E1000_PHY_CTRL_CSR
 #define E1000_82542_ERT      E1000_ERT
 #define E1000_82542_RXDCTL   E1000_RXDCTL
 #define E1000_82542_RXDCTL1  E1000_RXDCTL1
drivers/net/e1000/e1000_osdep.h +32 -38

92 lines not on this page · raw message

author
#5 lore raw

PATCH 1/1 - e1000e/ethtool.c - convert macros to functions

Add functions for reg_pattern_test and reg_set_and check Changed macros to use these functions

Compiled x86, untested

Size decreased ~2K

old:

$ size drivers/net/e1000e/ethtool.o
   text    data     bss     dec     hex filename
  14461       0       0   14461    387d drivers/net/e1000e/ethtool.o

new:

$ size drivers/net/e1000e/ethtool.o
   text    data     bss     dec     hex filename
  12498       0       0   12498    30d2 drivers/net/e1000e/ethtool.o
Signed-off-by Joe Perches <[email protected]>

---

 drivers/net/e1000e/ethtool.c |   78 +++++++++++++++++++++++++----------------
 1 files changed, 47 insertions(+), 31 deletions(-)
drivers/net/e1000e/ethtool.c +47 -31
index 6a39784..225db17 100644
@@ -691,41 +691,57 @@ err_setup:
 	return err;
 }
 
-#define REG_PATTERN_TEST(R, M, W) REG_PATTERN_TEST_ARRAY(R, 0, M, W)
-#define REG_PATTERN_TEST_ARRAY(reg, offset, mask, writeable)		      \
-{									      \
-	u32 _pat;							      \
-	u32 _value;							      \
-	u32 _test[] = {0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};	      \
-	for (_pat = 0; _pat < ARRAY_SIZE(_test); _pat++) {		      \
-		E1000_WRITE_REG_ARRAY(hw, reg, offset,	      \
-				      (_test[_pat] & writeable));	      \
-		_value = E1000_READ_REG_ARRAY(hw, reg, offset);     \
-		if (_value != (_test[_pat] & writeable & mask)) {	      \
-			ndev_err(netdev, "pattern test reg %04X "             \
-				 "failed: got 0x%08X expected 0x%08X\n",      \
-				 reg + offset,  \
-				 value, (_test[_pat] & writeable & mask));    \
-			*data = reg;					      \
-			return 1;					      \
-		}							      \
-	}								      \
+bool reg_pattern_test_array(struct e1000_adapter *adapter, u64 *data, 
+			    int reg, int offset, u32 mask, u32 write)
+{
+	int i;
+	u32 read;
+	static const u32 test[] = 
+		{0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};
+	for (i = 0; i < ARRAY_SIZE(test); i++) {
+		E1000_WRITE_REG_ARRAY(&adapter->hw, reg, offset,
+				      (test[i] & write));
+		read = E1000_READ_REG_ARRAY(&adapter->hw, reg, offset);
+		if (read != (test[i] & write & mask)) {
+			ndev_err(adapter->netdev, "pattern test reg %04X "
+				 "failed: got 0x%08X expected 0x%08X\n",
+				 reg + offset,
+				 read, (test[i] & write & mask));
+			*data = reg;
+			return true;
+		}
+	}
+	return false;
 }
 
-#define REG_SET_AND_CHECK(R, M, W)					      \
-{									      \
-	u32 _value;							      \
-	__ew32(hw, R, W & M);						\
-	_value = __er32(hw, R);						\
-	if ((W & M) != (_value & M)) {					      \
-		ndev_err(netdev, "set/check reg %04X test failed: "           \
-			 "got 0x%08X expected 0x%08X\n", R, (_value & M),     \
-			 (W & M));					      \
-		*data = R;						      \
-		return 1;						      \
-	}								      \
+#define REG_PATTERN_TEST(R, M, W) \
+	if (reg_pattern_test_array(adapter, data, R, 0, M, W)) \
+		return 1;
+
+#define REG_PATTERN_TEST_ARRAY(R, offset, M, W) \
+	if (reg_pattern_test_array(adapter, data, R, offset, M, W)) \
+		return 1;
+
+static bool reg_set_and_check(struct e1000_adapter *adapter, u64 *data,
+			      int reg, u32 mask, u32 write)
+{
+	u32 read;
+	__ew32(&adapter->hw, reg, write & mask);
+	read = __er32(&adapter->hw, reg);
+	if ((write & mask) != (read & mask)) {
+		ndev_err(adapter->netdev, "set/check reg %04X test failed: "
+			 "got 0x%08X expected 0x%08X\n", reg, (read & mask),
+			 (write & mask));
+		*data = reg;
+		return true;
+	}
+	return false;
 }
 
+#define REG_SET_AND_CHECK(R, M, W) \
+	if (reg_set_and_check(adapter, data, R, M, W)) \
+		return 1;
+
 static int e1000_reg_test(struct e1000_adapter *adapter, u64 *data)
 {
 	struct e1000_hw *hw = &adapter->hw;

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

#6 lore raw

Re: [PATCH] - e1000e/ethtool.c - convert macros to functions

Quoted 13 lines · Joe Perches

Add functions for reg_pattern_test and reg_set_and check Changed macros to use these functions

Compiled x86, untested

Size decreased ~2K

old:

$ size drivers/net/e1000e/ethtool.o
   text    data     bss     dec     hex filename
  14461       0       0   14461    387d drivers/net/e1000e/ethtool.o
Quoted 8 lines

new:

$ size drivers/net/e1000e/ethtool.o
   text    data     bss     dec     hex filename
  12498       0       0   12498    30d2 drivers/net/e1000e/ethtool.o
Signed-off-by Joe Perches <[email protected]>

ok, this looks great.

I'm going to have this tested a bit before I pass it on to Jeff Garzik.

Thanks!

Auke

Quoted 102 lines

---

 drivers/net/e1000e/ethtool.c |   78 +++++++++++++++++++++++++----------------
 1 files changed, 47 insertions(+), 31 deletions(-)
drivers/net/e1000e/ethtool.c +47 -32
index 6a39784..225db17 100644
@@ -691,41 +691,57 @@ err_setup:
 	return err;
 }
 
-#define REG_PATTERN_TEST(R, M, W) REG_PATTERN_TEST_ARRAY(R, 0, M, W)
-#define REG_PATTERN_TEST_ARRAY(reg, offset, mask, writeable)		      \
-{									      \
-	u32 _pat;							      \
-	u32 _value;							      \
-	u32 _test[] = {0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};	      \
-	for (_pat = 0; _pat < ARRAY_SIZE(_test); _pat++) {		      \
-		E1000_WRITE_REG_ARRAY(hw, reg, offset,	      \
-				      (_test[_pat] & writeable));	      \
-		_value = E1000_READ_REG_ARRAY(hw, reg, offset);     \
-		if (_value != (_test[_pat] & writeable & mask)) {	      \
-			ndev_err(netdev, "pattern test reg %04X "             \
-				 "failed: got 0x%08X expected 0x%08X\n",      \
-				 reg + offset,  \
-				 value, (_test[_pat] & writeable & mask));    \
-			*data = reg;					      \
-			return 1;					      \
-		}							      \
-	}								      \
+bool reg_pattern_test_array(struct e1000_adapter *adapter, u64 *data, 
+			    int reg, int offset, u32 mask, u32 write)
+{
+	int i;
+	u32 read;
+	static const u32 test[] = 
+		{0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};
+	for (i = 0; i < ARRAY_SIZE(test); i++) {
+		E1000_WRITE_REG_ARRAY(&adapter->hw, reg, offset,
+				      (test[i] & write));
+		read = E1000_READ_REG_ARRAY(&adapter->hw, reg, offset);
+		if (read != (test[i] & write & mask)) {
+			ndev_err(adapter->netdev, "pattern test reg %04X "
+				 "failed: got 0x%08X expected 0x%08X\n",
+				 reg + offset,
+				 read, (test[i] & write & mask));
+			*data = reg;
+			return true;
+		}
+	}
+	return false;
 }
 
-#define REG_SET_AND_CHECK(R, M, W)					      \
-{									      \
-	u32 _value;							      \
-	__ew32(hw, R, W & M);						\
-	_value = __er32(hw, R);						\
-	if ((W & M) != (_value & M)) {					      \
-		ndev_err(netdev, "set/check reg %04X test failed: "           \
-			 "got 0x%08X expected 0x%08X\n", R, (_value & M),     \
-			 (W & M));					      \
-		*data = R;						      \
-		return 1;						      \
-	}								      \
+#define REG_PATTERN_TEST(R, M, W) \
+	if (reg_pattern_test_array(adapter, data, R, 0, M, W)) \
+		return 1;
+
+#define REG_PATTERN_TEST_ARRAY(R, offset, M, W) \
+	if (reg_pattern_test_array(adapter, data, R, offset, M, W)) \
+		return 1;
+
+static bool reg_set_and_check(struct e1000_adapter *adapter, u64 *data,
+			      int reg, u32 mask, u32 write)
+{
+	u32 read;
+	__ew32(&adapter->hw, reg, write & mask);
+	read = __er32(&adapter->hw, reg);
+	if ((write & mask) != (read & mask)) {
+		ndev_err(adapter->netdev, "set/check reg %04X test failed: "
+			 "got 0x%08X expected 0x%08X\n", reg, (read & mask),
+			 (write & mask));
+		*data = reg;
+		return true;
+	}
+	return false;
 }
 
+#define REG_SET_AND_CHECK(R, M, W) \
+	if (reg_set_and_check(adapter, data, R, M, W)) \
+		return 1;
+
 static int e1000_reg_test(struct e1000_adapter *adapter, u64 *data)
 {
 	struct e1000_hw *hw = &adapter->hw;
 
 
-
Quoted 3 lines

To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/