All of lore.kernel.org
 help / color / mirror / Atom feed
* [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates
@ 2014-01-03  5:18 Jeff Kirsher
  2014-01-03  5:18 ` [net-next v2 01/11] ixbge: Protect ixgbe_down with __IXGBE_DOWN bit Jeff Kirsher
                   ` (10 more replies)
  0 siblings, 11 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-03  5:18 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann, bhutchings

This series contains updates to ixgbe, e1000e and igb.

Most notable are the patches to add Live Error Recovery (LER)
support to the ixgbe driver. This support also improves behavior
in Thunderbolt environments. This involves checking all register
reads for a value of all one's and when that is seen, to read the
status register, which should never properly return all one's, to
confirm whether the received value was correct. When this detects
a removal, the hw_addr field is cleared to indicate the removal.
This then blocks subsequent access to the device registers.

The register access macros have been changed to static inline
functions and all register accesses now use them. A configuration
option is added to allow the LER feature to be disabled. The added
checks on register accesses results in a small increase in cpu 
utilization, so disabling the option makes it possible to avoid 
paying that price in environments that have no need for LER. The
option is enabled by default because the cost is not high and
it makes the driver more robust.

The __IXGBE_DOWN bit is no longer overloaded to also mean that
device removal has been initiated. Now the bit can be used to
protect ixgbe_down from multiple entry via test_and_set_bit. A
needed smp_mb__before_clear_bit was also added.

v2 Changes:
- Use ACCESS_ONCE where needed, thanks to Ben Hutchings
- Fix crash on module removal
- Use boolean values for boolean returns instead of 0 and 1
- Reword Kconfig help text

The following are changes since commit c1ddf295f5183a5189196a8035546842caa2055a:
  net: revert "sched classifier: make cgroup table local"
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master

Jeff Kirsher (1):
  igb: make local functions static and remove dead code

Mark Rustad (8):
  ixbge: Protect ixgbe_down with __IXGBE_DOWN bit
  ixgbe: Indicate removal state explicitly
  ixgbe: Use static inlines instead of macros
  ixgbe: Make ethtool register test use accessors
  ixgbe: Check register reads for adapter removal
  ixgbe: Check for adapter removal on register writes
  ixgbe: Additional adapter removal checks
  ixgbe: Add Live Error Recovery configuration option

Tom Herbert (2):
  net: e1000e calls skb_set_hash
  net: igb calls skb_set_hash

 drivers/net/ethernet/intel/Kconfig               |  11 +++
 drivers/net/ethernet/intel/e1000e/netdev.c       |   2 +-
 drivers/net/ethernet/intel/igb/e1000_82575.c     |   4 +-
 drivers/net/ethernet/intel/igb/e1000_82575.h     |   2 -
 drivers/net/ethernet/intel/igb/e1000_i210.c      |  20 +++--
 drivers/net/ethernet/intel/igb/e1000_i210.h      |   9 --
 drivers/net/ethernet/intel/igb/e1000_phy.c       |  71 ---------------
 drivers/net/ethernet/intel/igb/e1000_phy.h       |   1 -
 drivers/net/ethernet/intel/igb/igb.h             |   2 -
 drivers/net/ethernet/intel/igb/igb_main.c        |   4 +-
 drivers/net/ethernet/intel/igb/igb_ptp.c         |   6 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe.h         |  12 +++
 drivers/net/ethernet/intel/ixgbe/ixgbe_common.h  |  58 +++++++++---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 110 +++++++++++++----------
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c    |  82 ++++++++++++++---
 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c     |   3 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c     |   2 +-
 17 files changed, 229 insertions(+), 170 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [net-next v2 01/11] ixbge: Protect ixgbe_down with __IXGBE_DOWN bit
  2014-01-03  5:18 [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
@ 2014-01-03  5:18 ` Jeff Kirsher
  2014-01-03  5:18 ` [net-next v2 02/11] ixgbe: Indicate removal state explicitly Jeff Kirsher
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-03  5:18 UTC (permalink / raw)
  To: davem; +Cc: Mark Rustad, netdev, gospo, sassmann, Jeff Kirsher

From: Mark Rustad <mark.d.rustad@intel.com>

The ixgbe_down function can now prevent multiple executions by
doing test_and_set_bit on __IXGBE_DOWN. This did not work before
introduction of the __IXGBE_REMOVED bit, because of overloading
of __IXGBE_DOWN. Also add smp_mb__before_clear_bit call before
clearing the __IXGBE_DOWN bit.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index cc06854..90a0b4a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4572,6 +4572,7 @@ static void ixgbe_up_complete(struct ixgbe_adapter *adapter)
 	if (hw->mac.ops.enable_tx_laser)
 		hw->mac.ops.enable_tx_laser(hw);
 
+	smp_mb__before_clear_bit();
 	clear_bit(__IXGBE_DOWN, &adapter->state);
 	ixgbe_napi_enable_all(adapter);
 
@@ -4783,7 +4784,8 @@ void ixgbe_down(struct ixgbe_adapter *adapter)
 	int i;
 
 	/* signal that we are down to the interrupt handler */
-	set_bit(__IXGBE_DOWN, &adapter->state);
+	if (test_and_set_bit(__IXGBE_DOWN, &adapter->state))
+		return; /* do nothing if already down */
 
 	/* disable receives */
 	rxctrl = IXGBE_READ_REG(hw, IXGBE_RXCTRL);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net-next v2 02/11] ixgbe: Indicate removal state explicitly
  2014-01-03  5:18 [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
  2014-01-03  5:18 ` [net-next v2 01/11] ixbge: Protect ixgbe_down with __IXGBE_DOWN bit Jeff Kirsher
@ 2014-01-03  5:18 ` Jeff Kirsher
  2014-01-03  5:18 ` [net-next v2 03/11] ixgbe: Use static inlines instead of macros Jeff Kirsher
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-03  5:18 UTC (permalink / raw)
  To: davem; +Cc: Mark Rustad, netdev, gospo, sassmann, Jeff Kirsher

From: Mark Rustad <mark.d.rustad@intel.com>

Add a bit, __IXGBE_REMOVE, to indicate that the module is being
removed. The __IXGBE_DOWN bit had been overloaded for this purpose,
but that leads to trouble. A few places now check both __IXGBE_DOWN
and __IXGBE_REMOVE. Notably, setting either bit will prevent service
task execution.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 12 ++++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 49531cd..8da263a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -798,6 +798,7 @@ enum ixgbe_state_t {
 	__IXGBE_TESTING,
 	__IXGBE_RESETTING,
 	__IXGBE_DOWN,
+	__IXGBE_REMOVE,
 	__IXGBE_SERVICE_SCHED,
 	__IXGBE_IN_SFP_INIT,
 	__IXGBE_PTP_RUNNING,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 90a0b4a..923b0fa 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -278,6 +278,7 @@ static void ixgbe_check_minimum_link(struct ixgbe_adapter *adapter,
 static void ixgbe_service_event_schedule(struct ixgbe_adapter *adapter)
 {
 	if (!test_bit(__IXGBE_DOWN, &adapter->state) &&
+	    !test_bit(__IXGBE_REMOVE, &adapter->state) &&
 	    !test_and_set_bit(__IXGBE_SERVICE_SCHED, &adapter->state))
 		schedule_work(&adapter->service_task);
 }
@@ -5876,8 +5877,9 @@ static void ixgbe_check_hang_subtask(struct ixgbe_adapter *adapter)
 	u64 eics = 0;
 	int i;
 
-	/* If we're down or resetting, just bail */
+	/* If we're down, removing or resetting, just bail */
 	if (test_bit(__IXGBE_DOWN, &adapter->state) ||
+	    test_bit(__IXGBE_REMOVE, &adapter->state) ||
 	    test_bit(__IXGBE_RESETTING, &adapter->state))
 		return;
 
@@ -6124,8 +6126,9 @@ static void ixgbe_spoof_check(struct ixgbe_adapter *adapter)
  **/
 static void ixgbe_watchdog_subtask(struct ixgbe_adapter *adapter)
 {
-	/* if interface is down do nothing */
+	/* if interface is down, removing or resetting, do nothing */
 	if (test_bit(__IXGBE_DOWN, &adapter->state) ||
+	    test_bit(__IXGBE_REMOVE, &adapter->state) ||
 	    test_bit(__IXGBE_RESETTING, &adapter->state))
 		return;
 
@@ -6343,8 +6346,9 @@ static void ixgbe_reset_subtask(struct ixgbe_adapter *adapter)
 
 	adapter->flags2 &= ~IXGBE_FLAG2_RESET_REQUESTED;
 
-	/* If we're already down or resetting, just bail */
+	/* If we're already down, removing or resetting, just bail */
 	if (test_bit(__IXGBE_DOWN, &adapter->state) ||
+	    test_bit(__IXGBE_REMOVE, &adapter->state) ||
 	    test_bit(__IXGBE_RESETTING, &adapter->state))
 		return;
 
@@ -8219,7 +8223,7 @@ static void ixgbe_remove(struct pci_dev *pdev)
 
 	ixgbe_dbg_adapter_exit(adapter);
 
-	set_bit(__IXGBE_DOWN, &adapter->state);
+	set_bit(__IXGBE_REMOVE, &adapter->state);
 	cancel_work_sync(&adapter->service_task);
 
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net-next v2 03/11] ixgbe: Use static inlines instead of macros
  2014-01-03  5:18 [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
  2014-01-03  5:18 ` [net-next v2 01/11] ixbge: Protect ixgbe_down with __IXGBE_DOWN bit Jeff Kirsher
  2014-01-03  5:18 ` [net-next v2 02/11] ixgbe: Indicate removal state explicitly Jeff Kirsher
@ 2014-01-03  5:18 ` Jeff Kirsher
  2014-01-03  5:28   ` Joe Perches
  2014-01-03  5:18 ` [net-next v2 04/11] ixgbe: Make ethtool register test use accessors Jeff Kirsher
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-03  5:18 UTC (permalink / raw)
  To: davem; +Cc: Mark Rustad, netdev, gospo, sassmann, Jeff Kirsher

From: Mark Rustad <mark.d.rustad@intel.com>

Kernel coding standard prefers static inline functions instead
of macros, so use them for register accessors. This is to prepare
for adding LER, Live Error Recovery, checks to those accessors.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h        |  5 +++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_common.h | 23 ++++++++++++++++-------
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c   |  4 ++--
 3 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 8da263a..199cf74 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -585,6 +585,11 @@ static inline u16 ixgbe_desc_unused(struct ixgbe_ring *ring)
 	return ((ntc > ntu) ? 0 : ring->count) + ntc - ntu - 1;
 }
 
+static inline void ixgbe_write_tail(struct ixgbe_ring *ring, u32 value)
+{
+	writel(value, ring->tail);
+}
+
 #define IXGBE_RX_DESC(R, i)	    \
 	(&(((union ixgbe_adv_rx_desc *)((R)->desc))[i]))
 #define IXGBE_TX_DESC(R, i)	    \
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
index d259dc7..503790a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
@@ -124,22 +124,31 @@ s32 ixgbe_reset_pipeline_82599(struct ixgbe_hw *hw);
 s32 ixgbe_get_thermal_sensor_data_generic(struct ixgbe_hw *hw);
 s32 ixgbe_init_thermal_sensor_thresh_generic(struct ixgbe_hw *hw);
 
-#define IXGBE_WRITE_REG(a, reg, value) writel((value), ((a)->hw_addr + (reg)))
+static inline void IXGBE_WRITE_REG(struct ixgbe_hw *hw, u32 reg, u32 value)
+{
+	writel(value, hw->hw_addr + reg);
+}
 
 #ifndef writeq
 #define writeq(val, addr) writel((u32) (val), addr); \
     writel((u32) (val >> 32), (addr + 4));
 #endif
 
-#define IXGBE_WRITE_REG64(a, reg, value) writeq((value), ((a)->hw_addr + (reg)))
+static inline void IXGBE_WRITE_REG64(struct ixgbe_hw *hw, u32 reg, u64 value)
+{
+	writeq(value, hw->hw_addr + reg);
+}
 
-#define IXGBE_READ_REG(a, reg) readl((a)->hw_addr + (reg))
+static inline u32 IXGBE_READ_REG(struct ixgbe_hw *hw, u32 reg)
+{
+	return readl(hw->hw_addr + reg);
+}
 
-#define IXGBE_WRITE_REG_ARRAY(a, reg, offset, value) (\
-    writel((value), ((a)->hw_addr + (reg) + ((offset) << 2))))
+#define IXGBE_WRITE_REG_ARRAY(a, reg, offset, value) \
+		IXGBE_WRITE_REG((a), (reg) + ((offset) << 2), (value))
 
-#define IXGBE_READ_REG_ARRAY(a, reg, offset) (\
-    readl((a)->hw_addr + (reg) + ((offset) << 2)))
+#define IXGBE_READ_REG_ARRAY(a, reg, offset) \
+		IXGBE_READ_REG((a), (reg) + ((offset) << 2))
 
 #define IXGBE_WRITE_FLUSH(a) IXGBE_READ_REG(a, IXGBE_STATUS)
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 923b0fa..4d71277 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1315,7 +1315,7 @@ static inline void ixgbe_release_rx_desc(struct ixgbe_ring *rx_ring, u32 val)
 	 * such as IA-64).
 	 */
 	wmb();
-	writel(val, rx_ring->tail);
+	ixgbe_write_tail(rx_ring, val);
 }
 
 static bool ixgbe_alloc_mapped_page(struct ixgbe_ring *rx_ring,
@@ -6699,7 +6699,7 @@ static void ixgbe_tx_map(struct ixgbe_ring *tx_ring,
 	tx_ring->next_to_use = i;
 
 	/* notify HW of packet */
-	writel(i, tx_ring->tail);
+	ixgbe_write_tail(tx_ring, i);
 
 	return;
 dma_error:
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net-next v2 04/11] ixgbe: Make ethtool register test use accessors
  2014-01-03  5:18 [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
                   ` (2 preceding siblings ...)
  2014-01-03  5:18 ` [net-next v2 03/11] ixgbe: Use static inlines instead of macros Jeff Kirsher
@ 2014-01-03  5:18 ` Jeff Kirsher
  2014-01-03  5:18 ` [net-next v2 05/11] ixgbe: Check register reads for adapter removal Jeff Kirsher
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-03  5:18 UTC (permalink / raw)
  To: davem; +Cc: Mark Rustad, netdev, gospo, sassmann, Jeff Kirsher

From: Mark Rustad <mark.d.rustad@intel.com>

Make the ethtool register test use the normal register accessor
functions. Also eliminate macros used for calling register test
functions to make error exits clearer. Use boolean values for
boolean returns instead of 0 and 1.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 88 +++++++++++-------------
 1 file changed, 42 insertions(+), 46 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 4e7c9b0..412cb1a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -1343,54 +1343,41 @@ static bool reg_pattern_test(struct ixgbe_adapter *adapter, u64 *data, int reg,
 		0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};
 
 	for (pat = 0; pat < ARRAY_SIZE(test_pattern); pat++) {
-		before = readl(adapter->hw.hw_addr + reg);
-		writel((test_pattern[pat] & write),
-		       (adapter->hw.hw_addr + reg));
-		val = readl(adapter->hw.hw_addr + reg);
+		before = IXGBE_READ_REG(&adapter->hw, reg);
+		IXGBE_WRITE_REG(&adapter->hw, reg, test_pattern[pat] & write);
+		val = IXGBE_READ_REG(&adapter->hw, reg);
 		if (val != (test_pattern[pat] & write & mask)) {
 			e_err(drv, "pattern test reg %04X failed: got "
 			      "0x%08X expected 0x%08X\n",
 			      reg, val, (test_pattern[pat] & write & mask));
 			*data = reg;
-			writel(before, adapter->hw.hw_addr + reg);
-			return 1;
+			IXGBE_WRITE_REG(&adapter->hw, reg, before);
+			return true;
 		}
-		writel(before, adapter->hw.hw_addr + reg);
+		IXGBE_WRITE_REG(&adapter->hw, reg, before);
 	}
-	return 0;
+	return false;
 }
 
 static bool reg_set_and_check(struct ixgbe_adapter *adapter, u64 *data, int reg,
 			      u32 mask, u32 write)
 {
 	u32 val, before;
-	before = readl(adapter->hw.hw_addr + reg);
-	writel((write & mask), (adapter->hw.hw_addr + reg));
-	val = readl(adapter->hw.hw_addr + reg);
+
+	before = IXGBE_READ_REG(&adapter->hw, reg);
+	IXGBE_WRITE_REG(&adapter->hw, reg, write & mask);
+	val = IXGBE_READ_REG(&adapter->hw, reg);
 	if ((write & mask) != (val & mask)) {
 		e_err(drv, "set/check reg %04X test failed: got 0x%08X "
 		      "expected 0x%08X\n", reg, (val & mask), (write & mask));
 		*data = reg;
-		writel(before, (adapter->hw.hw_addr + reg));
-		return 1;
+		IXGBE_WRITE_REG(&adapter->hw, reg, before);
+		return true;
 	}
-	writel(before, (adapter->hw.hw_addr + reg));
-	return 0;
+	IXGBE_WRITE_REG(&adapter->hw, reg, before);
+	return false;
 }
 
-#define REG_PATTERN_TEST(reg, mask, write)				      \
-	do {								      \
-		if (reg_pattern_test(adapter, data, reg, mask, write))	      \
-			return 1;					      \
-	} while (0)							      \
-
-
-#define REG_SET_AND_CHECK(reg, mask, write)				      \
-	do {								      \
-		if (reg_set_and_check(adapter, data, reg, mask, write))	      \
-			return 1;					      \
-	} while (0)							      \
-
 static int ixgbe_reg_test(struct ixgbe_adapter *adapter, u64 *data)
 {
 	const struct ixgbe_reg_test *test;
@@ -1438,38 +1425,47 @@ static int ixgbe_reg_test(struct ixgbe_adapter *adapter, u64 *data)
 	 */
 	while (test->reg) {
 		for (i = 0; i < test->array_len; i++) {
+			bool b = false;
+
 			switch (test->test_type) {
 			case PATTERN_TEST:
-				REG_PATTERN_TEST(test->reg + (i * 0x40),
-						 test->mask,
-						 test->write);
+				b = reg_pattern_test(adapter, data,
+						     test->reg + (i * 0x40),
+						     test->mask,
+						     test->write);
 				break;
 			case SET_READ_TEST:
-				REG_SET_AND_CHECK(test->reg + (i * 0x40),
-						  test->mask,
-						  test->write);
+				b = reg_set_and_check(adapter, data,
+						      test->reg + (i * 0x40),
+						      test->mask,
+						      test->write);
 				break;
 			case WRITE_NO_TEST:
-				writel(test->write,
-				       (adapter->hw.hw_addr + test->reg)
-				       + (i * 0x40));
+				IXGBE_WRITE_REG(&adapter->hw,
+						test->reg + (i * 0x40),
+						test->write);
 				break;
 			case TABLE32_TEST:
-				REG_PATTERN_TEST(test->reg + (i * 4),
-						 test->mask,
-						 test->write);
+				b = reg_pattern_test(adapter, data,
+						     test->reg + (i * 4),
+						     test->mask,
+						     test->write);
 				break;
 			case TABLE64_TEST_LO:
-				REG_PATTERN_TEST(test->reg + (i * 8),
-						 test->mask,
-						 test->write);
+				b = reg_pattern_test(adapter, data,
+						     test->reg + (i * 8),
+						     test->mask,
+						     test->write);
 				break;
 			case TABLE64_TEST_HI:
-				REG_PATTERN_TEST((test->reg + 4) + (i * 8),
-						 test->mask,
-						 test->write);
+				b = reg_pattern_test(adapter, data,
+						     (test->reg + 4) + (i * 8),
+						     test->mask,
+						     test->write);
 				break;
 			}
+			if (b)
+				return 1;
 		}
 		test++;
 	}
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net-next v2 05/11] ixgbe: Check register reads for adapter removal
  2014-01-03  5:18 [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
                   ` (3 preceding siblings ...)
  2014-01-03  5:18 ` [net-next v2 04/11] ixgbe: Make ethtool register test use accessors Jeff Kirsher
@ 2014-01-03  5:18 ` Jeff Kirsher
  2014-01-03  5:18 ` [net-next v2 06/11] ixgbe: Check for adapter removal on register writes Jeff Kirsher
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-03  5:18 UTC (permalink / raw)
  To: davem; +Cc: Mark Rustad, netdev, gospo, sassmann, bhutchings, Jeff Kirsher

From: Mark Rustad <mark.d.rustad@intel.com>

When CONFIG_IXGBE_LER is defined, check all register reads for
adapter removal by checking the status register after any register
read that returns 0xFFFFFFFF. Since the status register will never
return 0xFFFFFFFF unless the adapter is removed, such a value from
a status register read confirms the removal.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h        |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_common.h | 21 +++++++++++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c   | 40 ++++++++++++++++++++++---
 3 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 199cf74..002c9e2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -747,6 +747,7 @@ struct ixgbe_adapter {
 #ifdef IXGBE_FCOE
 	struct ixgbe_fcoe fcoe;
 #endif /* IXGBE_FCOE */
+	u8 __iomem *io_addr; /* Mainly for iounmap use */
 	u32 wol;
 
 	u16 bd_number;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
index 503790a..551025a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
@@ -124,6 +124,15 @@ s32 ixgbe_reset_pipeline_82599(struct ixgbe_hw *hw);
 s32 ixgbe_get_thermal_sensor_data_generic(struct ixgbe_hw *hw);
 s32 ixgbe_init_thermal_sensor_thresh_generic(struct ixgbe_hw *hw);
 
+#define IXGBE_FAILED_READ_REG 0xffffffffU
+
+#ifdef CONFIG_IXGBE_LER
+void ixgbe_check_remove(struct ixgbe_hw *hw, u32 reg);
+#define IXGBE_REMOVED(a) unlikely(!(a))
+#else
+#define IXGBE_REMOVED(a) (0)
+#endif /* CONFIG_IXGBE_LER */
+
 static inline void IXGBE_WRITE_REG(struct ixgbe_hw *hw, u32 reg, u32 value)
 {
 	writel(value, hw->hw_addr + reg);
@@ -141,7 +150,19 @@ static inline void IXGBE_WRITE_REG64(struct ixgbe_hw *hw, u32 reg, u64 value)
 
 static inline u32 IXGBE_READ_REG(struct ixgbe_hw *hw, u32 reg)
 {
+#ifdef CONFIG_IXGBE_LER
+	u8 __iomem *reg_addr = ACCESS_ONCE(hw->hw_addr);
+	u32 value;
+
+	if (IXGBE_REMOVED(reg_addr))
+		return IXGBE_FAILED_READ_REG;
+	value = readl(reg_addr + reg);
+	if (unlikely(value == IXGBE_FAILED_READ_REG))
+		ixgbe_check_remove(hw, reg);
+	return value;
+#else
 	return readl(hw->hw_addr + reg);
+#endif /* CONFIG_IXGBE_LER */
 }
 
 #define IXGBE_WRITE_REG_ARRAY(a, reg, offset, value) \
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 4d71277..0f0ca66 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -283,6 +283,37 @@ static void ixgbe_service_event_schedule(struct ixgbe_adapter *adapter)
 		schedule_work(&adapter->service_task);
 }
 
+#ifdef CONFIG_IXGBE_LER
+static void ixgbe_remove_adapter(struct ixgbe_hw *hw)
+{
+	struct ixgbe_adapter *adapter = hw->back;
+
+	if (!hw->hw_addr)
+		return;
+	hw->hw_addr = NULL;
+	e_dev_err("Adapter removed\n");
+}
+
+void ixgbe_check_remove(struct ixgbe_hw *hw, u32 reg)
+{
+	u32 value;
+
+	/* The following check not only optimizes a bit by not
+	 * performing a read on the status register when the
+	 * register just read was a status register read that
+	 * returned IXGBE_FAILED_READ_REG. It also blocks any
+	 * potential recursion.
+	 */
+	if (reg == IXGBE_STATUS) {
+		ixgbe_remove_adapter(hw);
+		return;
+	}
+	value = IXGBE_READ_REG(hw, IXGBE_STATUS);
+	if (value == IXGBE_FAILED_READ_REG)
+		ixgbe_remove_adapter(hw);
+}
+#endif /* CONFIG_IXGBE_LER */
+
 static void ixgbe_service_event_complete(struct ixgbe_adapter *adapter)
 {
 	BUG_ON(!test_bit(__IXGBE_SERVICE_SCHED, &adapter->state));
@@ -2970,7 +3001,7 @@ void ixgbe_configure_tx_ring(struct ixgbe_adapter *adapter,
 			ring->count * sizeof(union ixgbe_adv_tx_desc));
 	IXGBE_WRITE_REG(hw, IXGBE_TDH(reg_idx), 0);
 	IXGBE_WRITE_REG(hw, IXGBE_TDT(reg_idx), 0);
-	ring->tail = hw->hw_addr + IXGBE_TDT(reg_idx);
+	ring->tail = adapter->io_addr + IXGBE_TDT(reg_idx);
 
 	/*
 	 * set WTHRESH to encourage burst writeback, it should not be set
@@ -3373,7 +3404,7 @@ void ixgbe_configure_rx_ring(struct ixgbe_adapter *adapter,
 			ring->count * sizeof(union ixgbe_adv_rx_desc));
 	IXGBE_WRITE_REG(hw, IXGBE_RDH(reg_idx), 0);
 	IXGBE_WRITE_REG(hw, IXGBE_RDT(reg_idx), 0);
-	ring->tail = hw->hw_addr + IXGBE_RDT(reg_idx);
+	ring->tail = adapter->io_addr + IXGBE_RDT(reg_idx);
 
 	ixgbe_configure_srrctl(adapter, ring);
 	ixgbe_configure_rscctl(adapter, ring);
@@ -7887,6 +7918,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	hw->hw_addr = ioremap(pci_resource_start(pdev, 0),
 			      pci_resource_len(pdev, 0));
+	adapter->io_addr = hw->hw_addr;
 	if (!hw->hw_addr) {
 		err = -EIO;
 		goto err_ioremap;
@@ -8195,7 +8227,7 @@ err_register:
 err_sw_init:
 	ixgbe_disable_sriov(adapter);
 	adapter->flags2 &= ~IXGBE_FLAG2_SEARCH_FOR_SFP;
-	iounmap(hw->hw_addr);
+	iounmap(adapter->io_addr);
 err_ioremap:
 	free_netdev(netdev);
 err_alloc_etherdev:
@@ -8262,7 +8294,7 @@ static void ixgbe_remove(struct pci_dev *pdev)
 	kfree(adapter->ixgbe_ieee_ets);
 
 #endif
-	iounmap(adapter->hw.hw_addr);
+	iounmap(adapter->io_addr);
 	pci_release_selected_regions(pdev, pci_select_bars(pdev,
 				     IORESOURCE_MEM));
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net-next v2 06/11] ixgbe: Check for adapter removal on register writes
  2014-01-03  5:18 [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
                   ` (4 preceding siblings ...)
  2014-01-03  5:18 ` [net-next v2 05/11] ixgbe: Check register reads for adapter removal Jeff Kirsher
@ 2014-01-03  5:18 ` Jeff Kirsher
  2014-01-03  5:18 ` [net-next v2 07/11] ixgbe: Additional adapter removal checks Jeff Kirsher
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-03  5:18 UTC (permalink / raw)
  To: davem; +Cc: Mark Rustad, netdev, gospo, sassmann, bhutchings, Jeff Kirsher

From: Mark Rustad <mark.d.rustad@intel.com>

Prevent writes to an adapter that has been detected as removed
by a previous failing read. This also fixes some include file
ordering confusion that this patch revealed.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h        |  5 +++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_common.h | 12 ++++++++++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c   |  6 ++++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c    |  3 +--
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c    |  2 +-
 5 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 002c9e2..8a4b03b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -257,6 +257,9 @@ struct ixgbe_ring {
 	};
 	unsigned long last_rx_timestamp;
 	unsigned long state;
+#ifdef CONFIG_IXGBE_LER
+	u8 __iomem **adapter_present;	/* Points to hw_addr in ixgbe_hw */
+#endif /* CONFIG_IXGBE_LER */
 	u8 __iomem *tail;
 	dma_addr_t dma;			/* phys. address of descriptor ring */
 	unsigned int size;		/* length in bytes */
@@ -587,6 +590,8 @@ static inline u16 ixgbe_desc_unused(struct ixgbe_ring *ring)
 
 static inline void ixgbe_write_tail(struct ixgbe_ring *ring, u32 value)
 {
+	if (IXGBE_REMOVED(*ring->adapter_present))
+		return;
 	writel(value, ring->tail);
 }
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
index 551025a..b07a887 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
@@ -135,7 +135,11 @@ void ixgbe_check_remove(struct ixgbe_hw *hw, u32 reg);
 
 static inline void IXGBE_WRITE_REG(struct ixgbe_hw *hw, u32 reg, u32 value)
 {
-	writel(value, hw->hw_addr + reg);
+	u8 __iomem *reg_addr = ACCESS_ONCE(hw->hw_addr);
+
+	if (IXGBE_REMOVED(reg_addr))
+		return;
+	writel(value, reg_addr + reg);
 }
 
 #ifndef writeq
@@ -145,7 +149,11 @@ static inline void IXGBE_WRITE_REG(struct ixgbe_hw *hw, u32 reg, u32 value)
 
 static inline void IXGBE_WRITE_REG64(struct ixgbe_hw *hw, u32 reg, u64 value)
 {
-	writeq(value, hw->hw_addr + reg);
+	u8 __iomem *reg_addr = ACCESS_ONCE(hw->hw_addr);
+
+	if (IXGBE_REMOVED(reg_addr))
+		return;
+	writeq(value, reg_addr + reg);
 }
 
 static inline u32 IXGBE_READ_REG(struct ixgbe_hw *hw, u32 reg)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 0f0ca66..22e51d1 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3001,6 +3001,9 @@ void ixgbe_configure_tx_ring(struct ixgbe_adapter *adapter,
 			ring->count * sizeof(union ixgbe_adv_tx_desc));
 	IXGBE_WRITE_REG(hw, IXGBE_TDH(reg_idx), 0);
 	IXGBE_WRITE_REG(hw, IXGBE_TDT(reg_idx), 0);
+#ifdef CONFIG_IXGBE_LER
+	ring->adapter_present = &hw->hw_addr;
+#endif /* CONFIG_IXGBE_LER */
 	ring->tail = adapter->io_addr + IXGBE_TDT(reg_idx);
 
 	/*
@@ -3404,6 +3407,9 @@ void ixgbe_configure_rx_ring(struct ixgbe_adapter *adapter,
 			ring->count * sizeof(union ixgbe_adv_rx_desc));
 	IXGBE_WRITE_REG(hw, IXGBE_RDH(reg_idx), 0);
 	IXGBE_WRITE_REG(hw, IXGBE_RDT(reg_idx), 0);
+#ifdef CONFIG_IXGBE_LER
+	ring->adapter_present = &hw->hw_addr;
+#endif /* CONFIG_IXGBE_LER */
 	ring->tail = adapter->io_addr + IXGBE_RDT(reg_idx);
 
 	ixgbe_configure_srrctl(adapter, ring);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c
index d4a64e6..cc3101a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c
@@ -27,8 +27,7 @@
 
 #include <linux/pci.h>
 #include <linux/delay.h>
-#include "ixgbe_type.h"
-#include "ixgbe_common.h"
+#include "ixgbe.h"
 #include "ixgbe_mbx.h"
 
 /**
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
index 39217e5..132557c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
@@ -29,7 +29,7 @@
 #include <linux/delay.h>
 #include <linux/sched.h>
 
-#include "ixgbe_common.h"
+#include "ixgbe.h"
 #include "ixgbe_phy.h"
 
 static void ixgbe_i2c_start(struct ixgbe_hw *hw);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net-next v2 07/11] ixgbe: Additional adapter removal checks
  2014-01-03  5:18 [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
                   ` (5 preceding siblings ...)
  2014-01-03  5:18 ` [net-next v2 06/11] ixgbe: Check for adapter removal on register writes Jeff Kirsher
@ 2014-01-03  5:18 ` Jeff Kirsher
  2014-01-03  5:18 ` [net-next v2 08/11] ixgbe: Add Live Error Recovery configuration option Jeff Kirsher
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-03  5:18 UTC (permalink / raw)
  To: davem; +Cc: Mark Rustad, netdev, gospo, sassmann, Jeff Kirsher

From: Mark Rustad <mark.d.rustad@intel.com>

Additional checks are needed for a detected removal not to cause
problems. Some involve simply avoiding a lot of stuff that can't
do anything good, and also cases where the phony return value can
cause problems. In addition, down the adapter when the removal is
sensed.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 22 ++++++++++++++++++++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c    | 16 ++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 412cb1a..0899b1d 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -1342,6 +1342,10 @@ static bool reg_pattern_test(struct ixgbe_adapter *adapter, u64 *data, int reg,
 	static const u32 test_pattern[] = {
 		0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};
 
+	if (IXGBE_REMOVED(adapter->hw.hw_addr)) {
+		*data = 1;
+		return 1;
+	}
 	for (pat = 0; pat < ARRAY_SIZE(test_pattern); pat++) {
 		before = IXGBE_READ_REG(&adapter->hw, reg);
 		IXGBE_WRITE_REG(&adapter->hw, reg, test_pattern[pat] & write);
@@ -1364,6 +1368,10 @@ static bool reg_set_and_check(struct ixgbe_adapter *adapter, u64 *data, int reg,
 {
 	u32 val, before;
 
+	if (IXGBE_REMOVED(adapter->hw.hw_addr)) {
+		*data = 1;
+		return 1;
+	}
 	before = IXGBE_READ_REG(&adapter->hw, reg);
 	IXGBE_WRITE_REG(&adapter->hw, reg, write & mask);
 	val = IXGBE_READ_REG(&adapter->hw, reg);
@@ -1384,6 +1392,11 @@ static int ixgbe_reg_test(struct ixgbe_adapter *adapter, u64 *data)
 	u32 value, before, after;
 	u32 i, toggle;
 
+	if (IXGBE_REMOVED(adapter->hw.hw_addr)) {
+		e_err(drv, "Adapter removed - register test blocked\n");
+		*data = 1;
+		return 1;
+	}
 	switch (adapter->hw.mac.type) {
 	case ixgbe_mac_82598EB:
 		toggle = 0x7FFFF3FF;
@@ -1950,6 +1963,15 @@ static void ixgbe_diag_test(struct net_device *netdev,
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 	bool if_running = netif_running(netdev);
 
+	if (IXGBE_REMOVED(adapter->hw.hw_addr)) {
+		e_err(hw, "Adapter removed - test blocked\n");
+		data[0] = 1;
+		data[1] = 1;
+		data[2] = 1;
+		data[3] = 1;
+		eth_test->flags |= ETH_TEST_FL_FAILED;
+		return;
+	}
 	set_bit(__IXGBE_TESTING, &adapter->state);
 	if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
 		struct ixgbe_hw *hw = &adapter->hw;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 22e51d1..4801390 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -292,6 +292,7 @@ static void ixgbe_remove_adapter(struct ixgbe_hw *hw)
 		return;
 	hw->hw_addr = NULL;
 	e_dev_err("Adapter removed\n");
+	ixgbe_service_event_schedule(adapter);
 }
 
 void ixgbe_check_remove(struct ixgbe_hw *hw, u32 reg)
@@ -3343,6 +3344,8 @@ static void ixgbe_rx_desc_queue_enable(struct ixgbe_adapter *adapter,
 	u32 rxdctl;
 	u8 reg_idx = ring->reg_idx;
 
+	if (IXGBE_REMOVED(hw->hw_addr))
+		return;
 	/* RXDCTL.EN will return 0 on 82598 if link is down, so skip it */
 	if (hw->mac.type == ixgbe_mac_82598EB &&
 	    !(IXGBE_READ_REG(hw, IXGBE_LINKS) & IXGBE_LINKS_UP))
@@ -3367,6 +3370,8 @@ void ixgbe_disable_rx_queue(struct ixgbe_adapter *adapter,
 	u32 rxdctl;
 	u8 reg_idx = ring->reg_idx;
 
+	if (IXGBE_REMOVED(hw->hw_addr))
+		return;
 	rxdctl = IXGBE_READ_REG(hw, IXGBE_RXDCTL(reg_idx));
 	rxdctl &= ~IXGBE_RXDCTL_ENABLE;
 
@@ -4695,6 +4700,8 @@ void ixgbe_reset(struct ixgbe_adapter *adapter)
 	struct ixgbe_hw *hw = &adapter->hw;
 	int err;
 
+	if (IXGBE_REMOVED(hw->hw_addr))
+		return;
 	/* lock SFP init bit to prevent race conditions with the watchdog */
 	while (test_and_set_bit(__IXGBE_IN_SFP_INIT, &adapter->state))
 		usleep_range(1000, 2000);
@@ -6405,6 +6412,15 @@ static void ixgbe_service_task(struct work_struct *work)
 	struct ixgbe_adapter *adapter = container_of(work,
 						     struct ixgbe_adapter,
 						     service_task);
+	if (IXGBE_REMOVED(adapter->hw.hw_addr)) {
+		if (!test_bit(__IXGBE_DOWN, &adapter->state)) {
+			rtnl_lock();
+			ixgbe_down(adapter);
+			rtnl_unlock();
+		}
+		ixgbe_service_event_complete(adapter);
+		return;
+	}
 	ixgbe_reset_subtask(adapter);
 	ixgbe_sfp_detection_subtask(adapter);
 	ixgbe_sfp_link_config_subtask(adapter);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net-next v2 08/11] ixgbe: Add Live Error Recovery configuration option
  2014-01-03  5:18 [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
                   ` (6 preceding siblings ...)
  2014-01-03  5:18 ` [net-next v2 07/11] ixgbe: Additional adapter removal checks Jeff Kirsher
@ 2014-01-03  5:18 ` Jeff Kirsher
  2014-01-04  1:43   ` David Miller
  2014-01-03  5:18 ` [net-next v2 09/11] net: e1000e calls skb_set_hash Jeff Kirsher
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-03  5:18 UTC (permalink / raw)
  To: davem; +Cc: Mark Rustad, netdev, gospo, sassmann, bhutchings, Jeff Kirsher

From: Mark Rustad <mark.d.rustad@intel.com>

CONFIG_IXGBE_LER is added to control LER, Live Error Recovery,
support. This avoids possible crashes in the driver when a device
is suddenly removed, which is possible in Thunderbolt and other
environments. Turning this off will save a little CPU utilization.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/Kconfig | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
index 94adb89..791897d 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -192,6 +192,17 @@ config IXGBE
 	  To compile this driver as a module, choose M here. The module
 	  will be called ixgbe.
 
+config IXGBE_LER
+	bool "Intel(R) 10GbE PCI Express LER support"
+	default y
+	depends on IXGBE
+	---help---
+	  This option makes the driver more robust in the face of sudden
+	  device removals, which can happen in Thunderbolt and other
+	  environments.  Say N to disable Live Error Recovery (LER)
+	  checks to save a little CPU utilization. If in doubt,
+	  say Y.
+
 config IXGBE_HWMON
 	bool "Intel(R) 10GbE PCI Express adapters HWMON support"
 	default y
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net-next v2 09/11] net: e1000e calls skb_set_hash
  2014-01-03  5:18 [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
                   ` (7 preceding siblings ...)
  2014-01-03  5:18 ` [net-next v2 08/11] ixgbe: Add Live Error Recovery configuration option Jeff Kirsher
@ 2014-01-03  5:18 ` Jeff Kirsher
  2014-01-03  5:18 ` [net-next v2 10/11] net: igb " Jeff Kirsher
  2014-01-03  5:18 ` [net-next v2 11/11] igb: make local functions static and remove dead code Jeff Kirsher
  10 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-03  5:18 UTC (permalink / raw)
  To: davem; +Cc: Tom Herbert, netdev, gospo, sassmann, Jeff Kirsher

From: Tom Herbert <therbert@google.com>

Drivers should call skb_set_hash to set the hash and its type
in an skbuff.

Signed-off-by: Tom Herbert <therbert@google.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 051d158..098fb7d 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -885,7 +885,7 @@ static inline void e1000_rx_hash(struct net_device *netdev, __le32 rss,
 				 struct sk_buff *skb)
 {
 	if (netdev->features & NETIF_F_RXHASH)
-		skb->rxhash = le32_to_cpu(rss);
+		skb_set_hash(skb, le32_to_cpu(rss), PKT_HASH_TYPE_L3);
 }
 
 /**
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net-next v2 10/11] net: igb calls skb_set_hash
  2014-01-03  5:18 [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
                   ` (8 preceding siblings ...)
  2014-01-03  5:18 ` [net-next v2 09/11] net: e1000e calls skb_set_hash Jeff Kirsher
@ 2014-01-03  5:18 ` Jeff Kirsher
  2014-01-03  5:18 ` [net-next v2 11/11] igb: make local functions static and remove dead code Jeff Kirsher
  10 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-03  5:18 UTC (permalink / raw)
  To: davem; +Cc: Tom Herbert, netdev, gospo, sassmann, Jeff Kirsher

From: Tom Herbert <therbert@google.com>

Drivers should call skb_set_hash to set the hash and its type
in an skbuff.

Signed-off-by: Tom Herbert <therbert@google.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 46d31a4..a01bdca 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6620,7 +6620,9 @@ static inline void igb_rx_hash(struct igb_ring *ring,
 			       struct sk_buff *skb)
 {
 	if (ring->netdev->features & NETIF_F_RXHASH)
-		skb->rxhash = le32_to_cpu(rx_desc->wb.lower.hi_dword.rss);
+		skb_set_hash(skb,
+			     le32_to_cpu(rx_desc->wb.lower.hi_dword.rss),
+			     PKT_HASH_TYPE_L3);
 }
 
 /**
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net-next v2 11/11] igb: make local functions static and remove dead code
  2014-01-03  5:18 [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
                   ` (9 preceding siblings ...)
  2014-01-03  5:18 ` [net-next v2 10/11] net: igb " Jeff Kirsher
@ 2014-01-03  5:18 ` Jeff Kirsher
  10 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-03  5:18 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann

Based on Stephen Hemminger's original patch.
Make local functions static, and remove unused functions.

Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_82575.c |  4 +-
 drivers/net/ethernet/intel/igb/e1000_82575.h |  2 -
 drivers/net/ethernet/intel/igb/e1000_i210.c  | 20 ++++----
 drivers/net/ethernet/intel/igb/e1000_i210.h  |  9 ----
 drivers/net/ethernet/intel/igb/e1000_phy.c   | 71 ----------------------------
 drivers/net/ethernet/intel/igb/e1000_phy.h   |  1 -
 drivers/net/ethernet/intel/igb/igb.h         |  2 -
 drivers/net/ethernet/intel/igb/igb_ptp.c     |  6 ++-
 8 files changed, 17 insertions(+), 98 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c b/drivers/net/ethernet/intel/igb/e1000_82575.c
index 06df692..0ee7049 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.c
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.c
@@ -2720,7 +2720,7 @@ static const u8 e1000_emc_therm_limit[4] = {
  *
  *  Updates the temperatures in mac.thermal_sensor_data
  **/
-s32 igb_get_thermal_sensor_data_generic(struct e1000_hw *hw)
+static s32 igb_get_thermal_sensor_data_generic(struct e1000_hw *hw)
 {
 	s32 status = E1000_SUCCESS;
 	u16 ets_offset;
@@ -2774,7 +2774,7 @@ s32 igb_get_thermal_sensor_data_generic(struct e1000_hw *hw)
  *  Sets the thermal sensor thresholds according to the NVM map
  *  and save off the threshold and location values into mac.thermal_sensor_data
  **/
-s32 igb_init_thermal_sensor_thresh_generic(struct e1000_hw *hw)
+static s32 igb_init_thermal_sensor_thresh_generic(struct e1000_hw *hw)
 {
 	s32 status = E1000_SUCCESS;
 	u16 ets_offset;
diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.h b/drivers/net/ethernet/intel/igb/e1000_82575.h
index 8c24377..622d80d 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.h
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.h
@@ -266,8 +266,6 @@ u16 igb_rxpbs_adjust_82580(u32 data);
 s32 igb_read_emi_reg(struct e1000_hw *, u16 addr, u16 *data);
 s32 igb_set_eee_i350(struct e1000_hw *);
 s32 igb_set_eee_i354(struct e1000_hw *);
-s32 igb_init_thermal_sensor_thresh_generic(struct e1000_hw *);
-s32 igb_get_thermal_sensor_data_generic(struct e1000_hw *hw);
 
 #define E1000_I2C_THERMAL_SENSOR_ADDR	0xF8
 #define E1000_EMC_INTERNAL_DATA		0x00
diff --git a/drivers/net/ethernet/intel/igb/e1000_i210.c b/drivers/net/ethernet/intel/igb/e1000_i210.c
index 0c03933..9f32c78 100644
--- a/drivers/net/ethernet/intel/igb/e1000_i210.c
+++ b/drivers/net/ethernet/intel/igb/e1000_i210.c
@@ -35,6 +35,8 @@
 #include "e1000_hw.h"
 #include "e1000_i210.h"
 
+static s32 igb_update_flash_i210(struct e1000_hw *hw);
+
 /**
  * igb_get_hw_semaphore_i210 - Acquire hardware semaphore
  *  @hw: pointer to the HW structure
@@ -111,7 +113,7 @@ static s32 igb_get_hw_semaphore_i210(struct e1000_hw *hw)
  *  Return successful if access grant bit set, else clear the request for
  *  EEPROM access and return -E1000_ERR_NVM (-1).
  **/
-s32 igb_acquire_nvm_i210(struct e1000_hw *hw)
+static s32 igb_acquire_nvm_i210(struct e1000_hw *hw)
 {
 	return igb_acquire_swfw_sync_i210(hw, E1000_SWFW_EEP_SM);
 }
@@ -123,7 +125,7 @@ s32 igb_acquire_nvm_i210(struct e1000_hw *hw)
  *  Stop any current commands to the EEPROM and clear the EEPROM request bit,
  *  then release the semaphores acquired.
  **/
-void igb_release_nvm_i210(struct e1000_hw *hw)
+static void igb_release_nvm_i210(struct e1000_hw *hw)
 {
 	igb_release_swfw_sync_i210(hw, E1000_SWFW_EEP_SM);
 }
@@ -206,8 +208,8 @@ void igb_release_swfw_sync_i210(struct e1000_hw *hw, u16 mask)
  *  Reads a 16 bit word from the Shadow Ram using the EERD register.
  *  Uses necessary synchronization semaphores.
  **/
-s32 igb_read_nvm_srrd_i210(struct e1000_hw *hw, u16 offset, u16 words,
-			     u16 *data)
+static s32 igb_read_nvm_srrd_i210(struct e1000_hw *hw, u16 offset, u16 words,
+				  u16 *data)
 {
 	s32 status = E1000_SUCCESS;
 	u16 i, count;
@@ -306,8 +308,8 @@ out:
  *  If error code is returned, data and Shadow RAM may be inconsistent - buffer
  *  partially written.
  **/
-s32 igb_write_nvm_srwr_i210(struct e1000_hw *hw, u16 offset, u16 words,
-			      u16 *data)
+static s32 igb_write_nvm_srwr_i210(struct e1000_hw *hw, u16 offset, u16 words,
+				   u16 *data)
 {
 	s32 status = E1000_SUCCESS;
 	u16 i, count;
@@ -555,7 +557,7 @@ s32 igb_read_invm_version(struct e1000_hw *hw,
  *  Calculates the EEPROM checksum by reading/adding each word of the EEPROM
  *  and then verifies that the sum of the EEPROM is equal to 0xBABA.
  **/
-s32 igb_validate_nvm_checksum_i210(struct e1000_hw *hw)
+static s32 igb_validate_nvm_checksum_i210(struct e1000_hw *hw)
 {
 	s32 status = E1000_SUCCESS;
 	s32 (*read_op_ptr)(struct e1000_hw *, u16, u16, u16 *);
@@ -590,7 +592,7 @@ s32 igb_validate_nvm_checksum_i210(struct e1000_hw *hw)
  *  up to the checksum.  Then calculates the EEPROM checksum and writes the
  *  value to the EEPROM. Next commit EEPROM data onto the Flash.
  **/
-s32 igb_update_nvm_checksum_i210(struct e1000_hw *hw)
+static s32 igb_update_nvm_checksum_i210(struct e1000_hw *hw)
 {
 	s32 ret_val = E1000_SUCCESS;
 	u16 checksum = 0;
@@ -684,7 +686,7 @@ bool igb_get_flash_presence_i210(struct e1000_hw *hw)
  *  @hw: pointer to the HW structure
  *
  **/
-s32 igb_update_flash_i210(struct e1000_hw *hw)
+static s32 igb_update_flash_i210(struct e1000_hw *hw)
 {
 	s32 ret_val = E1000_SUCCESS;
 	u32 flup;
diff --git a/drivers/net/ethernet/intel/igb/e1000_i210.h b/drivers/net/ethernet/intel/igb/e1000_i210.h
index 2d91371..a068866 100644
--- a/drivers/net/ethernet/intel/igb/e1000_i210.h
+++ b/drivers/net/ethernet/intel/igb/e1000_i210.h
@@ -28,17 +28,8 @@
 #ifndef _E1000_I210_H_
 #define _E1000_I210_H_
 
-s32 igb_update_flash_i210(struct e1000_hw *hw);
-s32 igb_update_nvm_checksum_i210(struct e1000_hw *hw);
-s32 igb_validate_nvm_checksum_i210(struct e1000_hw *hw);
-s32 igb_write_nvm_srwr_i210(struct e1000_hw *hw, u16 offset, u16 words,
-			    u16 *data);
-s32 igb_read_nvm_srrd_i210(struct e1000_hw *hw, u16 offset, u16 words,
-			   u16 *data);
 s32 igb_acquire_swfw_sync_i210(struct e1000_hw *hw, u16 mask);
 void igb_release_swfw_sync_i210(struct e1000_hw *hw, u16 mask);
-s32 igb_acquire_nvm_i210(struct e1000_hw *hw);
-void igb_release_nvm_i210(struct e1000_hw *hw);
 s32 igb_valid_led_default_i210(struct e1000_hw *hw, u16 *data);
 s32 igb_read_invm_version(struct e1000_hw *hw,
 			  struct e1000_fw_version *invm_ver);
diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.c b/drivers/net/ethernet/intel/igb/e1000_phy.c
index ad2b74d..c0f2a16 100644
--- a/drivers/net/ethernet/intel/igb/e1000_phy.c
+++ b/drivers/net/ethernet/intel/igb/e1000_phy.c
@@ -394,77 +394,6 @@ s32 igb_read_sfp_data_byte(struct e1000_hw *hw, u16 offset, u8 *data)
 }
 
 /**
- *  e1000_write_sfp_data_byte - Writes SFP module data.
- *  @hw: pointer to the HW structure
- *  @offset: byte location offset to write to
- *  @data: data to write
- *
- *  Writes one byte to SFP module data stored
- *  in SFP resided EEPROM memory or SFP diagnostic area.
- *  Function should be called with
- *  E1000_I2CCMD_SFP_DATA_ADDR(<byte offset>) for SFP module database access
- *  E1000_I2CCMD_SFP_DIAG_ADDR(<byte offset>) for SFP diagnostics parameters
- *  access
- **/
-s32 e1000_write_sfp_data_byte(struct e1000_hw *hw, u16 offset, u8 data)
-{
-	u32 i = 0;
-	u32 i2ccmd = 0;
-	u32 data_local = 0;
-
-	if (offset > E1000_I2CCMD_SFP_DIAG_ADDR(255)) {
-		hw_dbg("I2CCMD command address exceeds upper limit\n");
-		return -E1000_ERR_PHY;
-	}
-	/* The programming interface is 16 bits wide
-	 * so we need to read the whole word first
-	 * then update appropriate byte lane and write
-	 * the updated word back.
-	 */
-	/* Set up Op-code, EEPROM Address,in the I2CCMD
-	 * register. The MAC will take care of interfacing
-	 * with an EEPROM to write the data given.
-	 */
-	i2ccmd = ((offset << E1000_I2CCMD_REG_ADDR_SHIFT) |
-		  E1000_I2CCMD_OPCODE_READ);
-	/* Set a command to read single word */
-	wr32(E1000_I2CCMD, i2ccmd);
-	for (i = 0; i < E1000_I2CCMD_PHY_TIMEOUT; i++) {
-		udelay(50);
-		/* Poll the ready bit to see if lastly
-		 * launched I2C operation completed
-		 */
-		i2ccmd = rd32(E1000_I2CCMD);
-		if (i2ccmd & E1000_I2CCMD_READY) {
-			/* Check if this is READ or WRITE phase */
-			if ((i2ccmd & E1000_I2CCMD_OPCODE_READ) ==
-			    E1000_I2CCMD_OPCODE_READ) {
-				/* Write the selected byte
-				 * lane and update whole word
-				 */
-				data_local = i2ccmd & 0xFF00;
-				data_local |= data;
-				i2ccmd = ((offset <<
-					E1000_I2CCMD_REG_ADDR_SHIFT) |
-					E1000_I2CCMD_OPCODE_WRITE | data_local);
-				wr32(E1000_I2CCMD, i2ccmd);
-			} else {
-				break;
-			}
-		}
-	}
-	if (!(i2ccmd & E1000_I2CCMD_READY)) {
-		hw_dbg("I2CCMD Write did not complete\n");
-		return -E1000_ERR_PHY;
-	}
-	if (i2ccmd & E1000_I2CCMD_ERROR) {
-		hw_dbg("I2CCMD Error bit set\n");
-		return -E1000_ERR_PHY;
-	}
-	return 0;
-}
-
-/**
  *  igb_read_phy_reg_igp - Read igp PHY register
  *  @hw: pointer to the HW structure
  *  @offset: register offset to be read
diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.h b/drivers/net/ethernet/intel/igb/e1000_phy.h
index 6a0873f..55b3f8c 100644
--- a/drivers/net/ethernet/intel/igb/e1000_phy.h
+++ b/drivers/net/ethernet/intel/igb/e1000_phy.h
@@ -70,7 +70,6 @@ s32  igb_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data);
 s32  igb_read_phy_reg_i2c(struct e1000_hw *hw, u32 offset, u16 *data);
 s32  igb_write_phy_reg_i2c(struct e1000_hw *hw, u32 offset, u16 data);
 s32  igb_read_sfp_data_byte(struct e1000_hw *hw, u16 offset, u8 *data);
-s32  e1000_write_sfp_data_byte(struct e1000_hw *hw, u16 offset, u8 data);
 s32  igb_copper_link_setup_82580(struct e1000_hw *hw);
 s32  igb_get_phy_info_82580(struct e1000_hw *hw);
 s32  igb_phy_force_speed_duplex_82580(struct e1000_hw *hw);
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index ccf472f..65f3c77 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -525,9 +525,7 @@ void igb_set_fw_version(struct igb_adapter *);
 void igb_ptp_init(struct igb_adapter *adapter);
 void igb_ptp_stop(struct igb_adapter *adapter);
 void igb_ptp_reset(struct igb_adapter *adapter);
-void igb_ptp_tx_work(struct work_struct *work);
 void igb_ptp_rx_hang(struct igb_adapter *adapter);
-void igb_ptp_tx_hwtstamp(struct igb_adapter *adapter);
 void igb_ptp_rx_rgtstamp(struct igb_q_vector *q_vector, struct sk_buff *skb);
 void igb_ptp_rx_pktstamp(struct igb_q_vector *q_vector, unsigned char *va,
 			 struct sk_buff *skb);
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 5a54e3d..d9f3976 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -75,6 +75,8 @@
 #define INCVALUE_82576			(16 << IGB_82576_TSYNC_SHIFT)
 #define IGB_NBITS_82580			40
 
+static void igb_ptp_tx_hwtstamp(struct igb_adapter *adapter);
+
 /* SYSTIM read access for the 82576 */
 static cycle_t igb_ptp_read_82576(const struct cyclecounter *cc)
 {
@@ -372,7 +374,7 @@ static int igb_ptp_enable(struct ptp_clock_info *ptp,
  * This work function polls the TSYNCTXCTL valid bit to determine when a
  * timestamp has been taken for the current stored skb.
  **/
-void igb_ptp_tx_work(struct work_struct *work)
+static void igb_ptp_tx_work(struct work_struct *work)
 {
 	struct igb_adapter *adapter = container_of(work, struct igb_adapter,
 						   ptp_tx_work);
@@ -466,7 +468,7 @@ void igb_ptp_rx_hang(struct igb_adapter *adapter)
  * available, then it must have been for this skb here because we only
  * allow only one such packet into the queue.
  **/
-void igb_ptp_tx_hwtstamp(struct igb_adapter *adapter)
+static void igb_ptp_tx_hwtstamp(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	struct skb_shared_hwtstamps shhwtstamps;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [net-next v2 03/11] ixgbe: Use static inlines instead of macros
  2014-01-03  5:18 ` [net-next v2 03/11] ixgbe: Use static inlines instead of macros Jeff Kirsher
@ 2014-01-03  5:28   ` Joe Perches
  2014-01-03 18:31     ` Rustad, Mark D
  0 siblings, 1 reply; 15+ messages in thread
From: Joe Perches @ 2014-01-03  5:28 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, Mark Rustad, netdev, gospo, sassmann

On Thu, 2014-01-02 at 21:18 -0800, Jeff Kirsher wrote:
> From: Mark Rustad <mark.d.rustad@intel.com>
[]
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
[]
> @@ -124,22 +124,31 @@ s32 ixgbe_reset_pipeline_82599(struct ixgbe_hw *hw);
>  #ifndef writeq
>  #define writeq(val, addr) writel((u32) (val), addr); \
>      writel((u32) (val >> 32), (addr + 4));
>  #endif

This is unchanged, but it would be nicer with a do {} while.

#ifndef writeq
#define writeq(val, addr)				\
do {							\
	writel((u32)(val), addr);			\
	writel((u32)((val) >> 32), (addr + 4));		\
} while (0)

Even then, this could be nicer as an inline too.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [net-next v2 03/11] ixgbe: Use static inlines instead of macros
  2014-01-03  5:28   ` Joe Perches
@ 2014-01-03 18:31     ` Rustad, Mark D
  0 siblings, 0 replies; 15+ messages in thread
From: Rustad, Mark D @ 2014-01-03 18:31 UTC (permalink / raw)
  To: Joe Perches
  Cc: Kirsher, Jeffrey T, David Miller, <netdev@vger.kernel.org>,
	gospo, sassmann

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

On Jan 2, 2014, at 9:28 PM, Joe Perches <joe@perches.com> wrote:

> On Thu, 2014-01-02 at 21:18 -0800, Jeff Kirsher wrote:
>> From: Mark Rustad <mark.d.rustad@intel.com>
> []
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
> []
>> @@ -124,22 +124,31 @@ s32 ixgbe_reset_pipeline_82599(struct ixgbe_hw *hw);
>> #ifndef writeq
>> #define writeq(val, addr) writel((u32) (val), addr); \
>>     writel((u32) (val >> 32), (addr + 4));
>> #endif
> 
> This is unchanged, but it would be nicer with a do {} while.

Yes, it is definitely a trap for a macro to directly generate two statements like this. The #ifdef check seemed a little dubious to me when I first saw it, but at the time I chose to stick to what I had to do and then forgot to revisit this.

> #ifndef writeq
> #define writeq(val, addr)				\
> do {							\
> 	writel((u32)(val), addr);			\
> 	writel((u32)((val) >> 32), (addr + 4));		\
> } while (0)

Now that I have looked into it, I only fear becoming a "mucking foron" by touching it at all! :-) (grep the alpha arch code) Lacking an ARCH_HAS macro for detecting writeq, it does appear that this check is currently the way to do it, though it probably makes no sense whatsoever to compile this driver for any architecture that is not defining writeq. I wonder how many of these sequences were added in support of randconfig?

> Even then, this could be nicer as an inline too.

It does appear that the other places checking for writeq are being implemented with static inlines, so that should be the preferred method here as well.

-- 
Mark Rustad, Networking Division, Intel Corporation


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [net-next v2 08/11] ixgbe: Add Live Error Recovery configuration option
  2014-01-03  5:18 ` [net-next v2 08/11] ixgbe: Add Live Error Recovery configuration option Jeff Kirsher
@ 2014-01-04  1:43   ` David Miller
  0 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2014-01-04  1:43 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: mark.d.rustad, netdev, gospo, sassmann, bhutchings

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu,  2 Jan 2014 21:18:27 -0800

> From: Mark Rustad <mark.d.rustad@intel.com>
> 
> CONFIG_IXGBE_LER is added to control LER, Live Error Recovery,
> support. This avoids possible crashes in the driver when a device
> is suddenly removed, which is possible in Thunderbolt and other
> environments. Turning this off will save a little CPU utilization.
> 
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Compared to the cost of the MMIO register accesses themselves,
the cost of these checks will be negligible.

I also can't think of any distribution which would not enable
this option, so it's basically pointless.

Please remove it and just enable this new code unconditionally.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-01-04  1:43 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-03  5:18 [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
2014-01-03  5:18 ` [net-next v2 01/11] ixbge: Protect ixgbe_down with __IXGBE_DOWN bit Jeff Kirsher
2014-01-03  5:18 ` [net-next v2 02/11] ixgbe: Indicate removal state explicitly Jeff Kirsher
2014-01-03  5:18 ` [net-next v2 03/11] ixgbe: Use static inlines instead of macros Jeff Kirsher
2014-01-03  5:28   ` Joe Perches
2014-01-03 18:31     ` Rustad, Mark D
2014-01-03  5:18 ` [net-next v2 04/11] ixgbe: Make ethtool register test use accessors Jeff Kirsher
2014-01-03  5:18 ` [net-next v2 05/11] ixgbe: Check register reads for adapter removal Jeff Kirsher
2014-01-03  5:18 ` [net-next v2 06/11] ixgbe: Check for adapter removal on register writes Jeff Kirsher
2014-01-03  5:18 ` [net-next v2 07/11] ixgbe: Additional adapter removal checks Jeff Kirsher
2014-01-03  5:18 ` [net-next v2 08/11] ixgbe: Add Live Error Recovery configuration option Jeff Kirsher
2014-01-04  1:43   ` David Miller
2014-01-03  5:18 ` [net-next v2 09/11] net: e1000e calls skb_set_hash Jeff Kirsher
2014-01-03  5:18 ` [net-next v2 10/11] net: igb " Jeff Kirsher
2014-01-03  5:18 ` [net-next v2 11/11] igb: make local functions static and remove dead code Jeff Kirsher

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.