* [net-next 1/9] ixgbe: Use static inlines instead of macros
2014-01-02 9:33 [net-next 0/9][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
@ 2014-01-02 9:33 ` Jeff Kirsher
2014-01-02 9:33 ` [net-next 2/9] ixgbe: Make ethtool register test use accessors Jeff Kirsher
` (8 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-02 9:33 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>
Tested-by: Phil Schmitt <phillip.j.schmitt@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 49531cd..9e9b8d0 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 cc06854..752c20b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1314,7 +1314,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,
@@ -6693,7 +6693,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 2/9] ixgbe: Make ethtool register test use accessors
2014-01-02 9:33 [net-next 0/9][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
2014-01-02 9:33 ` [net-next 1/9] ixgbe: Use static inlines instead of macros Jeff Kirsher
@ 2014-01-02 9:33 ` Jeff Kirsher
2014-01-02 9:33 ` [net-next 3/9] ixgbe: Check register reads for adapter removal Jeff Kirsher
` (7 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-02 9:33 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.
Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 80 +++++++++++-------------
1 file changed, 38 insertions(+), 42 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 4e7c9b0..70f2ccc 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -1343,19 +1343,18 @@ 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);
+ IXGBE_WRITE_REG(&adapter->hw, reg, before);
return 1;
}
- writel(before, adapter->hw.hw_addr + reg);
+ IXGBE_WRITE_REG(&adapter->hw, reg, before);
}
return 0;
}
@@ -1364,33 +1363,21 @@ 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));
+ IXGBE_WRITE_REG(&adapter->hw, reg, before);
return 1;
}
- writel(before, (adapter->hw.hw_addr + reg));
+ IXGBE_WRITE_REG(&adapter->hw, reg, before);
return 0;
}
-#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 3/9] ixgbe: Check register reads for adapter removal
2014-01-02 9:33 [net-next 0/9][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
2014-01-02 9:33 ` [net-next 1/9] ixgbe: Use static inlines instead of macros Jeff Kirsher
2014-01-02 9:33 ` [net-next 2/9] ixgbe: Make ethtool register test use accessors Jeff Kirsher
@ 2014-01-02 9:33 ` Jeff Kirsher
2014-01-02 18:10 ` Ben Hutchings
2014-01-02 9:33 ` [net-next 4/9] ixgbe: Check for adapter removal on register writes Jeff Kirsher
` (6 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-02 9:33 UTC (permalink / raw)
To: davem; +Cc: Mark Rustad, netdev, gospo, sassmann, 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>
Tested-by: Phil Schmitt <phillip.j.schmitt@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 9e9b8d0..ad34757 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..7626c84 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 = 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 752c20b..1100315 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -282,6 +282,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));
@@ -2969,7 +3000,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
@@ -3372,7 +3403,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);
@@ -7881,6 +7912,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;
@@ -8189,7 +8221,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:
@@ -8256,7 +8288,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* Re: [net-next 3/9] ixgbe: Check register reads for adapter removal
2014-01-02 9:33 ` [net-next 3/9] ixgbe: Check register reads for adapter removal Jeff Kirsher
@ 2014-01-02 18:10 ` Ben Hutchings
0 siblings, 0 replies; 15+ messages in thread
From: Ben Hutchings @ 2014-01-02 18:10 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, Mark Rustad, netdev, gospo, sassmann
On Thu, 2014-01-02 at 01:33 -0800, Jeff Kirsher wrote:
> 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>
> Tested-by: Phil Schmitt <phillip.j.schmitt@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 9e9b8d0..ad34757 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
[...]
> @@ -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 = hw->hw_addr;
This should have an ACCESS_ONCE() so the compiler definitely won't read
hw->hw_addr a second time after it's been validated.
Ben.
> + 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 */
> }
[...]
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [net-next 4/9] ixgbe: Check for adapter removal on register writes
2014-01-02 9:33 [net-next 0/9][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
` (2 preceding siblings ...)
2014-01-02 9:33 ` [net-next 3/9] ixgbe: Check register reads for adapter removal Jeff Kirsher
@ 2014-01-02 9:33 ` Jeff Kirsher
2014-01-02 18:12 ` Ben Hutchings
2014-01-02 9:33 ` [net-next 5/9] ixgbe: Additional adapter removal checks Jeff Kirsher
` (5 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-02 9:33 UTC (permalink / raw)
To: davem; +Cc: Mark Rustad, netdev, gospo, sassmann, 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>
Tested-by: Phil Schmitt <phillip.j.schmitt@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 ad34757..d0a514b 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 7626c84..e426fe2 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 = 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 = 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 1100315..c415d71 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3000,6 +3000,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);
/*
@@ -3403,6 +3406,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* Re: [net-next 4/9] ixgbe: Check for adapter removal on register writes
2014-01-02 9:33 ` [net-next 4/9] ixgbe: Check for adapter removal on register writes Jeff Kirsher
@ 2014-01-02 18:12 ` Ben Hutchings
0 siblings, 0 replies; 15+ messages in thread
From: Ben Hutchings @ 2014-01-02 18:12 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, Mark Rustad, netdev, gospo, sassmann
On Thu, 2014-01-02 at 01:33 -0800, Jeff Kirsher wrote:
> 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>
> Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
[...]
> --- 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 = 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 = hw->hw_addr;
> +
> + if (IXGBE_REMOVED(reg_addr))
> + return;
> + writeq(value, reg_addr + reg);
> }
[...]
These should also use ACCESS_ONCE() to read hw->hw_addr.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [net-next 5/9] ixgbe: Additional adapter removal checks
2014-01-02 9:33 [net-next 0/9][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
` (3 preceding siblings ...)
2014-01-02 9:33 ` [net-next 4/9] ixgbe: Check for adapter removal on register writes Jeff Kirsher
@ 2014-01-02 9:33 ` Jeff Kirsher
2014-01-02 9:33 ` [net-next 6/9] ixgbe: Add Live Error Recovery configuration option Jeff Kirsher
` (4 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-02 9:33 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>
Tested-by: Phil Schmitt <phillip.j.schmitt@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 70f2ccc..be319ef 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 c415d71..b672b85 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -291,6 +291,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)
@@ -3342,6 +3343,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))
@@ -3366,6 +3369,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;
@@ -4693,6 +4698,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);
@@ -6399,6 +6406,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 6/9] ixgbe: Add Live Error Recovery configuration option
2014-01-02 9:33 [net-next 0/9][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
` (4 preceding siblings ...)
2014-01-02 9:33 ` [net-next 5/9] ixgbe: Additional adapter removal checks Jeff Kirsher
@ 2014-01-02 9:33 ` Jeff Kirsher
2014-01-02 18:15 ` Ben Hutchings
2014-01-02 9:33 ` [net-next 7/9] net: e1000e calls skb_set_hash Jeff Kirsher
` (3 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-02 9:33 UTC (permalink / raw)
To: davem; +Cc: Mark Rustad, netdev, gospo, sassmann, Jeff Kirsher
From: Mark Rustad <mark.d.rustad@intel.com>
CONFIG_IXGBE_LER is added to control LER, Live Error Recovery
support to save a little CPU utilization in environments that
don't support LER. Thunderbolt environments also benefit from
having this enabled.
Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@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..f1646e4 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---
+ Say N to disable Live Error Recovery (LER) checks to save a
+ little CPU utilization. Thunderbolt environments also benefit
+ from having this enabled. Non-LER environments only suffer a
+ little increased CPU utilization by leaving this enabled. 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* Re: [net-next 6/9] ixgbe: Add Live Error Recovery configuration option
2014-01-02 9:33 ` [net-next 6/9] ixgbe: Add Live Error Recovery configuration option Jeff Kirsher
@ 2014-01-02 18:15 ` Ben Hutchings
0 siblings, 0 replies; 15+ messages in thread
From: Ben Hutchings @ 2014-01-02 18:15 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, Mark Rustad, netdev, gospo, sassmann
On Thu, 2014-01-02 at 01:33 -0800, Jeff Kirsher wrote:
> From: Mark Rustad <mark.d.rustad@intel.com>
>
> CONFIG_IXGBE_LER is added to control LER, Live Error Recovery
> support to save a little CPU utilization in environments that
> don't support LER. Thunderbolt environments also benefit from
> having this enabled.
[...]
This is a strange way of explaining it:
'Disabling it does A. Enabling it also does B.'
There's no 'also' here; users have to trade-off A against B.
The Kconfig help text has the same problem.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [net-next 7/9] net: e1000e calls skb_set_hash
2014-01-02 9:33 [net-next 0/9][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
` (5 preceding siblings ...)
2014-01-02 9:33 ` [net-next 6/9] ixgbe: Add Live Error Recovery configuration option Jeff Kirsher
@ 2014-01-02 9:33 ` Jeff Kirsher
2014-01-02 9:33 ` [net-next 8/9] net: igb " Jeff Kirsher
` (2 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-02 9:33 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 8/9] net: igb calls skb_set_hash
2014-01-02 9:33 [net-next 0/9][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
` (6 preceding siblings ...)
2014-01-02 9:33 ` [net-next 7/9] net: e1000e calls skb_set_hash Jeff Kirsher
@ 2014-01-02 9:33 ` Jeff Kirsher
2014-01-02 9:33 ` [net-next 9/9] igb: make local functions static and remove dead code Jeff Kirsher
2014-01-02 18:21 ` [net-next 0/9][pull request] Intel Wired LAN Driver Updates Ben Hutchings
9 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-02 9:33 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 9/9] igb: make local functions static and remove dead code
2014-01-02 9:33 [net-next 0/9][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
` (7 preceding siblings ...)
2014-01-02 9:33 ` [net-next 8/9] net: igb " Jeff Kirsher
@ 2014-01-02 9:33 ` Jeff Kirsher
2014-01-02 18:21 ` [net-next 0/9][pull request] Intel Wired LAN Driver Updates Ben Hutchings
9 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2014-01-02 9:33 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 0/9][pull request] Intel Wired LAN Driver Updates
2014-01-02 9:33 [net-next 0/9][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
` (8 preceding siblings ...)
2014-01-02 9:33 ` [net-next 9/9] igb: make local functions static and remove dead code Jeff Kirsher
@ 2014-01-02 18:21 ` Ben Hutchings
2014-01-02 19:06 ` Rustad, Mark D
9 siblings, 1 reply; 15+ messages in thread
From: Ben Hutchings @ 2014-01-02 18:21 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, netdev, gospo, sassmann
On Thu, 2014-01-02 at 01:33 -0800, Jeff Kirsher wrote:
> This series contains updates to ixgbe, e1000e and igb.
>
> Most notably is Mark Rustad's ixgbe patches to add the Live Error
> Recovery option to ixgbe as well as additional cleanups to the driver.
[...]
The Live Error Recovery option is pretty odd; it looks like a
combination of PCI hotplug and EEH. Why is it not integrated with
either of those?
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [net-next 0/9][pull request] Intel Wired LAN Driver Updates
2014-01-02 18:21 ` [net-next 0/9][pull request] Intel Wired LAN Driver Updates Ben Hutchings
@ 2014-01-02 19:06 ` Rustad, Mark D
0 siblings, 0 replies; 15+ messages in thread
From: Rustad, Mark D @ 2014-01-02 19:06 UTC (permalink / raw)
To: Ben Hutchings
Cc: Kirsher, Jeffrey T, David Miller, <netdev@vger.kernel.org>,
gospo@redhat.com, sassmann@redhat.com
[-- Attachment #1: Type: text/plain, Size: 1447 bytes --]
On Jan 2, 2014, at 10:21 AM, Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Thu, 2014-01-02 at 01:33 -0800, Jeff Kirsher wrote:
>> This series contains updates to ixgbe, e1000e and igb.
>>
>> Most notably is Mark Rustad's ixgbe patches to add the Live Error
>> Recovery option to ixgbe as well as additional cleanups to the driver.
> [...]
>
> The Live Error Recovery option is pretty odd; it looks like a
> combination of PCI hotplug and EEH. Why is it not integrated with
> either of those?
It certainly is related to both of those. The issue being addressed here is specific to this driver in that the driver can be the first to get a read return of all one’s when a device is suddenly removed. This code allows the driver to determine whether such a return is valid, or is a sign of the device having been removed. Without those checks, there are paths where the driver will crash in such a case. This code will race with hotplug, so it allows hotplug to drive the logical removal, but it makes the driver “safe” in the mean time.
Thanks for the comments. You are absolutely right about the need for ACCESS_ONCE and better Kconfig help text.
> Ben.
>
> --
> Ben Hutchings, Staff Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
--
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