netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578
  2014-12-15  8:39 [PATCH " Zhu Yanjun
@ 2014-12-15  8:39 ` Zhu Yanjun
  0 siblings, 0 replies; 11+ messages in thread
From: Zhu Yanjun @ 2014-12-15  8:39 UTC (permalink / raw)
  To: netdev, w, zyjzyj2000
  Cc: Zhu Yanjun, Bruce Allan, Jeff Kirsher, David S. Miller

2.6.x kernels require a similar logic change as commit 901b2b95
[e1000e: reset MAC-PHY interconnect on 82577/82578] introduces
for newer kernels.

During Sx->S0 transitions, the interconnect between the MAC and PHY on
82577/82578 can remain in SMBus mode instead of transitioning to the
PCIe-like mode required during normal operation.  Toggling the LANPHYPC
Value bit essentially resets the interconnect forcing it to the correct
mode.

after review of all intel drivers, found several instances where
drivers had the incorrect pattern of:
memory mapped write();
delay();

which should always be:
memory mapped write();
write flush(); /* aka memory mapped read */
delay();

explanation:
The reason for including the flush is that writes can be held
(posted) in PCI/PCIe bridges, but the read always has to complete
synchronously and therefore has to flush all pending writes to a
device.  If a write is held and followed by a delay, the delay
means nothing because the write may not have reached hardware
(maybe even not until the next read)

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
---
 drivers/net/e1000e/defines.h |  2 ++
 drivers/net/e1000e/ich8lan.c | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/net/e1000e/defines.h b/drivers/net/e1000e/defines.h
index 1190167..52283a6 100644
--- a/drivers/net/e1000e/defines.h
+++ b/drivers/net/e1000e/defines.h
@@ -214,6 +214,8 @@
 #define E1000_CTRL_SPD_1000 0x00000200  /* Force 1Gb */
 #define E1000_CTRL_FRCSPD   0x00000800  /* Force Speed */
 #define E1000_CTRL_FRCDPX   0x00001000  /* Force Duplex */
+#define E1000_CTRL_LANPHYPC_OVERRIDE 0x00010000 /* SW control of LANPHYPC */
+#define E1000_CTRL_LANPHYPC_VALUE    0x00020000 /* SW value of LANPHYPC */
 #define E1000_CTRL_SWDPIN0  0x00040000  /* SWDPIN 0 value */
 #define E1000_CTRL_SWDPIN1  0x00080000  /* SWDPIN 1 value */
 #define E1000_CTRL_SWDPIO0  0x00400000  /* SWDPIN 0 Input or output */
diff --git a/drivers/net/e1000e/ich8lan.c b/drivers/net/e1000e/ich8lan.c
index de39f9a..020657c 100644
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@ -88,6 +88,8 @@
 
 
 #define E1000_ICH_FWSM_RSPCIPHY	0x00000040 /* Reset PHY on PCI Reset */
+/* FW established a valid mode */ 
+#define E1000_ICH_FWSM_FW_VALID                0x00008000
 
 #define E1000_ICH_MNG_IAMT_MODE		0x2
 
@@ -260,6 +262,7 @@ static inline void __ew32flash(struct e1000_hw *hw, unsigned long reg, u32 val)
 static s32 e1000_init_phy_params_pchlan(struct e1000_hw *hw)
 {
 	struct e1000_phy_info *phy = &hw->phy;
+	u32 ctrl;
 	s32 ret_val = 0;
 
 	phy->addr                     = 1;
@@ -274,6 +277,23 @@ static s32 e1000_init_phy_params_pchlan(struct e1000_hw *hw)
 	phy->ops.write_phy_reg_locked = e1000_write_phy_reg_hv_locked;
 	phy->autoneg_mask             = AUTONEG_ADVERTISE_SPEED_DEFAULT;
 
+	if (!(er32(FWSM) & E1000_ICH_FWSM_FW_VALID)) {
+		/*
+		 * The MAC-PHY interconnect may still be in SMBus mode
+		 * after Sx->S0.  Toggle the LANPHYPC Value bit to force
+		 * the interconnect to PCIe mode, but only if there is no
+		 * firmware present otherwise firmware will have done it.
+		*/
+		ctrl = er32(CTRL);
+		ctrl |=  E1000_CTRL_LANPHYPC_OVERRIDE;
+		ctrl &= ~E1000_CTRL_LANPHYPC_VALUE;
+		ew32(CTRL, ctrl);
+		e1e_flush();
+		udelay(10);
+		ctrl &= ~E1000_CTRL_LANPHYPC_OVERRIDE;
+		ew32(CTRL, ctrl);
+		msleep(50);
+	}
 	/*
 	 * Reset the PHY before any acccess to it.  Doing so, ensures that
 	 * the PHY is in a known good state before we read/write PHY registers.
-- 
1.9.1

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

* [PATCH V2 0/5] e1000e: fix nic not boot after rebooting
@ 2014-12-16 10:28 Zhu Yanjun
  2014-12-16 10:28 ` [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578 Zhu Yanjun
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Zhu Yanjun @ 2014-12-16 10:28 UTC (permalink / raw)
  To: netdev, w, zyjzyj2000; +Cc: Zhu Yanjun

With kernel 2.6.x, e1000e with 82577/8/9 sometimes will not boot
after rebooting. 

If a kernel 2.6.x board with 82577/8/9 e1000e nic is rebooted for 
100 times, there are 7~8 times that 82577/8/9 e1000e nic will not boot 
normally.

V2:
Follow the advice from Willy, the wrong upstream commit IDs in these
5 patches commit messages are fixed.

Zhu Yanjun (5):
  e1000e: reset MAC-PHY interconnect on 82577/82578
  e1000e: workaround EEPROM configuration change on 82579 on kernel
    2.6.x
  e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked
  e1000e: update workaround for 82579 intermittently disabled during
    S0->Sx
  e1000e: cleanup use of check_reset_block function pointer

 drivers/net/e1000e/defines.h |  2 ++
 drivers/net/e1000e/hw.h      |  1 +
 drivers/net/e1000e/ich8lan.c | 37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+)

-- 
1.9.1

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

* [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578
  2014-12-16 10:28 [PATCH V2 0/5] e1000e: fix nic not boot after rebooting Zhu Yanjun
@ 2014-12-16 10:28 ` Zhu Yanjun
  2014-12-16 10:48   ` David Laight
  2014-12-16 12:12   ` Jeff Kirsher
  2014-12-16 10:28 ` [PATCH 2/5] e1000e: workaround EEPROM configuration change on 82579 on kernel 2.6.x Zhu Yanjun
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 11+ messages in thread
From: Zhu Yanjun @ 2014-12-16 10:28 UTC (permalink / raw)
  To: netdev, w, zyjzyj2000
  Cc: Zhu Yanjun, Bruce Allan, Jeff Kirsher, David S. Miller

2.6.x kernels require a similar logic change as commit 6dfaa76 
[e1000e: reset MAC-PHY interconnect on 82577/82578] introduces
for newer kernels.

During Sx->S0 transitions, the interconnect between the MAC and PHY on
82577/82578 can remain in SMBus mode instead of transitioning to the
PCIe-like mode required during normal operation.  Toggling the LANPHYPC
Value bit essentially resets the interconnect forcing it to the correct
mode.

after review of all intel drivers, found several instances where
drivers had the incorrect pattern of:
memory mapped write();
delay();

which should always be:
memory mapped write();
write flush(); /* aka memory mapped read */
delay();

explanation:
The reason for including the flush is that writes can be held
(posted) in PCI/PCIe bridges, but the read always has to complete
synchronously and therefore has to flush all pending writes to a
device.  If a write is held and followed by a delay, the delay
means nothing because the write may not have reached hardware
(maybe even not until the next read)

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
---
 drivers/net/e1000e/defines.h |  2 ++
 drivers/net/e1000e/ich8lan.c | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/net/e1000e/defines.h b/drivers/net/e1000e/defines.h
index 1190167..52283a6 100644
--- a/drivers/net/e1000e/defines.h
+++ b/drivers/net/e1000e/defines.h
@@ -214,6 +214,8 @@
 #define E1000_CTRL_SPD_1000 0x00000200  /* Force 1Gb */
 #define E1000_CTRL_FRCSPD   0x00000800  /* Force Speed */
 #define E1000_CTRL_FRCDPX   0x00001000  /* Force Duplex */
+#define E1000_CTRL_LANPHYPC_OVERRIDE 0x00010000 /* SW control of LANPHYPC */
+#define E1000_CTRL_LANPHYPC_VALUE    0x00020000 /* SW value of LANPHYPC */
 #define E1000_CTRL_SWDPIN0  0x00040000  /* SWDPIN 0 value */
 #define E1000_CTRL_SWDPIN1  0x00080000  /* SWDPIN 1 value */
 #define E1000_CTRL_SWDPIO0  0x00400000  /* SWDPIN 0 Input or output */
diff --git a/drivers/net/e1000e/ich8lan.c b/drivers/net/e1000e/ich8lan.c
index de39f9a..020657c 100644
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@ -88,6 +88,8 @@
 
 
 #define E1000_ICH_FWSM_RSPCIPHY	0x00000040 /* Reset PHY on PCI Reset */
+/* FW established a valid mode */ 
+#define E1000_ICH_FWSM_FW_VALID                0x00008000
 
 #define E1000_ICH_MNG_IAMT_MODE		0x2
 
@@ -260,6 +262,7 @@ static inline void __ew32flash(struct e1000_hw *hw, unsigned long reg, u32 val)
 static s32 e1000_init_phy_params_pchlan(struct e1000_hw *hw)
 {
 	struct e1000_phy_info *phy = &hw->phy;
+	u32 ctrl;
 	s32 ret_val = 0;
 
 	phy->addr                     = 1;
@@ -274,6 +277,23 @@ static s32 e1000_init_phy_params_pchlan(struct e1000_hw *hw)
 	phy->ops.write_phy_reg_locked = e1000_write_phy_reg_hv_locked;
 	phy->autoneg_mask             = AUTONEG_ADVERTISE_SPEED_DEFAULT;
 
+	if (!(er32(FWSM) & E1000_ICH_FWSM_FW_VALID)) {
+		/*
+		 * The MAC-PHY interconnect may still be in SMBus mode
+		 * after Sx->S0.  Toggle the LANPHYPC Value bit to force
+		 * the interconnect to PCIe mode, but only if there is no
+		 * firmware present otherwise firmware will have done it.
+		*/
+		ctrl = er32(CTRL);
+		ctrl |=  E1000_CTRL_LANPHYPC_OVERRIDE;
+		ctrl &= ~E1000_CTRL_LANPHYPC_VALUE;
+		ew32(CTRL, ctrl);
+		e1e_flush();
+		udelay(10);
+		ctrl &= ~E1000_CTRL_LANPHYPC_OVERRIDE;
+		ew32(CTRL, ctrl);
+		msleep(50);
+	}
 	/*
 	 * Reset the PHY before any acccess to it.  Doing so, ensures that
 	 * the PHY is in a known good state before we read/write PHY registers.
-- 
1.9.1

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

* [PATCH 2/5] e1000e: workaround EEPROM configuration change on 82579 on kernel 2.6.x
  2014-12-16 10:28 [PATCH V2 0/5] e1000e: fix nic not boot after rebooting Zhu Yanjun
  2014-12-16 10:28 ` [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578 Zhu Yanjun
@ 2014-12-16 10:28 ` Zhu Yanjun
  2014-12-16 10:28 ` [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked Zhu Yanjun
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Zhu Yanjun @ 2014-12-16 10:28 UTC (permalink / raw)
  To: netdev, w, zyjzyj2000; +Cc: Zhu Yanjun, Bruce Allan, Jeff Kirsher

2.6.x kernels require a similar logic change as commit 62bc813 
[e1000e: workaround EEPROM configuration change on 82579] introduces
for newer kernels.

An update to the EEPROM on 82579 will extend a delay in hardware to fix an
issue with WoL not working after a G3->S5 transition which is unrelated to
the driver.  However, this extended delay conflicts with nominal operation
of the device when it is initialized by the driver and after every reset
of the hardware (i.e. the driver starts configuring the device before the
hardware is done with it's own configuration work).  The workaround for
when the driver is in control of the device is to tell the hardware after
every reset the configuration delay should be the original shorter one.

Some pre-existing variables are renamed generically to be re-used with
new register accesses.

[e1000_toggle_lanphypc_value_ich8lan does not exist. Its implementations
exist in e1000_init_phy_params_pchlan. Renamed variables remain unchanged]

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
---
 drivers/net/e1000e/hw.h      |  1 +
 drivers/net/e1000e/ich8lan.c | 17 +++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/e1000e/hw.h b/drivers/net/e1000e/hw.h
index 11f3b7c..b055d78 100644
--- a/drivers/net/e1000e/hw.h
+++ b/drivers/net/e1000e/hw.h
@@ -60,6 +60,7 @@ enum e1e_registers {
 	E1000_FEXTNVM  = 0x00028, /* Future Extended NVM - RW */
 	E1000_FCT      = 0x00030, /* Flow Control Type - RW */
 	E1000_VET      = 0x00038, /* VLAN Ether Type - RW */
+	E1000_FEXTNVM3 = 0x0003C, /* Future Extended NVM 3 - RW */
 	E1000_ICR      = 0x000C0, /* Interrupt Cause Read - R/clr */
 	E1000_ITR      = 0x000C4, /* Interrupt Throttling Rate - RW */
 	E1000_ICS      = 0x000C8, /* Interrupt Cause Set - WO */
diff --git a/drivers/net/e1000e/ich8lan.c b/drivers/net/e1000e/ich8lan.c
index 020657c..c4b2d15 100644
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@ -108,6 +108,9 @@
 #define E1000_FEXTNVM_SW_CONFIG		1
 #define E1000_FEXTNVM_SW_CONFIG_ICH8M (1 << 27) /* Bit redefined for ICH8M :/ */
 
+#define E1000_FEXTNVM3_PHY_CFG_COUNTER_MASK    0x0C000000
+#define E1000_FEXTNVM3_PHY_CFG_COUNTER_50MSEC  0x08000000
+
 #define PCIE_ICH8_SNOOP_ALL		PCIE_NO_SNOOP_ALL
 
 #define E1000_ICH_RAR_ENTRIES		7
@@ -278,6 +281,12 @@ static s32 e1000_init_phy_params_pchlan(struct e1000_hw *hw)
 	phy->autoneg_mask             = AUTONEG_ADVERTISE_SPEED_DEFAULT;
 
 	if (!(er32(FWSM) & E1000_ICH_FWSM_FW_VALID)) {
+		/*Set Phy Config Counter to 50msec */
+		ctrl = er32(FEXTNVM3);
+		ctrl &= ~E1000_FEXTNVM3_PHY_CFG_COUNTER_MASK;
+		ctrl |= E1000_FEXTNVM3_PHY_CFG_COUNTER_50MSEC;
+		ew32(FEXTNVM3, ctrl);
+
 		/*
 		 * The MAC-PHY interconnect may still be in SMBus mode
 		 * after Sx->S0.  Toggle the LANPHYPC Value bit to force
@@ -2685,6 +2694,14 @@ static s32 e1000_reset_hw_ich8lan(struct e1000_hw *hw)
 	ew32(CTRL, (ctrl | E1000_CTRL_RST));
 	msleep(20);
 
+	/* Set Phy Config Counter to 50msec */
+	if (hw->mac.type == e1000_pch2lan) {
+		u32 phycc_reg = er32(FEXTNVM3);
+		phycc_reg &= ~E1000_FEXTNVM3_PHY_CFG_COUNTER_MASK;
+		phycc_reg |= E1000_FEXTNVM3_PHY_CFG_COUNTER_50MSEC;
+		ew32(FEXTNVM3, phycc_reg);
+	}
+
 	if (!ret_val)
 		e1000_release_swflag_ich8lan(hw);
 
-- 
1.9.1

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

* [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked
  2014-12-16 10:28 [PATCH V2 0/5] e1000e: fix nic not boot after rebooting Zhu Yanjun
  2014-12-16 10:28 ` [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578 Zhu Yanjun
  2014-12-16 10:28 ` [PATCH 2/5] e1000e: workaround EEPROM configuration change on 82579 on kernel 2.6.x Zhu Yanjun
@ 2014-12-16 10:28 ` Zhu Yanjun
  2014-12-16 10:28 ` [PATCH 4/5] e1000e: update workaround for 82579 intermittently disabled during S0->Sx Zhu Yanjun
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Zhu Yanjun @ 2014-12-16 10:28 UTC (permalink / raw)
  To: netdev, w, zyjzyj2000; +Cc: Zhu Yanjun, Bruce Allan, Jeff Kirsher

2.6.x kernels require a similar logic change as commit 6cc7aae 
[e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked]
introduces for newer kernels.

When PHY reset is intentionally blocked on 82577/8/9, do not toggle the
LANPHYPC value bit (essentially performing a hard power reset of the
device) otherwise the PHY can be put into an unknown state.

Cleanup whitespace in the same function.

[yanjun.zhu: whitespace remains unchanged]

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
---
 drivers/net/e1000e/ich8lan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/e1000e/ich8lan.c b/drivers/net/e1000e/ich8lan.c
index c4b2d15..8c7e4aa 100644
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@ -280,7 +280,8 @@ static s32 e1000_init_phy_params_pchlan(struct e1000_hw *hw)
 	phy->ops.write_phy_reg_locked = e1000_write_phy_reg_hv_locked;
 	phy->autoneg_mask             = AUTONEG_ADVERTISE_SPEED_DEFAULT;
 
-	if (!(er32(FWSM) & E1000_ICH_FWSM_FW_VALID)) {
+	if (!(er32(FWSM) & E1000_ICH_FWSM_FW_VALID) && 
+		!e1000_check_reset_block(hw)) {
 		/*Set Phy Config Counter to 50msec */
 		ctrl = er32(FEXTNVM3);
 		ctrl &= ~E1000_FEXTNVM3_PHY_CFG_COUNTER_MASK;
-- 
1.9.1

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

* [PATCH 4/5] e1000e: update workaround for 82579 intermittently disabled during S0->Sx
  2014-12-16 10:28 [PATCH V2 0/5] e1000e: fix nic not boot after rebooting Zhu Yanjun
                   ` (2 preceding siblings ...)
  2014-12-16 10:28 ` [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked Zhu Yanjun
@ 2014-12-16 10:28 ` Zhu Yanjun
  2014-12-16 10:28 ` [PATCH 5/5] e1000e: cleanup use of check_reset_block function pointer Zhu Yanjun
  2015-05-14 17:10 ` [PATCH V2 0/5] e1000e: fix nic not boot after rebooting Willy Tarreau
  5 siblings, 0 replies; 11+ messages in thread
From: Zhu Yanjun @ 2014-12-16 10:28 UTC (permalink / raw)
  To: netdev, w, zyjzyj2000; +Cc: Zhu Yanjun, Bruce Allan, Jeff Kirsher

2.6.x kernels require a similar logic change as commit 90b8298 
[e1000e: update workaround for 82579 intermittently disabled during S0->Sx]
introduces for newer kernels.

The workaround which toggles the LANPHYPC (LAN PHY Power Control) value bit
to force the MAC-Phy interconnect into PCIe mode from SMBus mode during
driver load and resume should always be done except if PHY resets are
blocked by the Manageability Engine (ME).  Previously, the toggle was done
only if PHY resets are blocked and the ME was disabled.

The rest of the patch is just indentation changes as a consequence of the
updated workaround.

[yanjun.zhu: indentation changes are removed.
function e1000_init_phy_workarounds_pchlan does not exist]

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
---
 drivers/net/e1000e/ich8lan.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/e1000e/ich8lan.c b/drivers/net/e1000e/ich8lan.c
index 8c7e4aa..0da2c2c 100644
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@ -280,8 +280,7 @@ static s32 e1000_init_phy_params_pchlan(struct e1000_hw *hw)
 	phy->ops.write_phy_reg_locked = e1000_write_phy_reg_hv_locked;
 	phy->autoneg_mask             = AUTONEG_ADVERTISE_SPEED_DEFAULT;
 
-	if (!(er32(FWSM) & E1000_ICH_FWSM_FW_VALID) && 
-		!e1000_check_reset_block(hw)) {
+	if (!e1000_check_reset_block(hw)) {
 		/*Set Phy Config Counter to 50msec */
 		ctrl = er32(FEXTNVM3);
 		ctrl &= ~E1000_FEXTNVM3_PHY_CFG_COUNTER_MASK;
-- 
1.9.1

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

* [PATCH 5/5] e1000e: cleanup use of check_reset_block function pointer
  2014-12-16 10:28 [PATCH V2 0/5] e1000e: fix nic not boot after rebooting Zhu Yanjun
                   ` (3 preceding siblings ...)
  2014-12-16 10:28 ` [PATCH 4/5] e1000e: update workaround for 82579 intermittently disabled during S0->Sx Zhu Yanjun
@ 2014-12-16 10:28 ` Zhu Yanjun
  2015-05-14 17:10 ` [PATCH V2 0/5] e1000e: fix nic not boot after rebooting Willy Tarreau
  5 siblings, 0 replies; 11+ messages in thread
From: Zhu Yanjun @ 2014-12-16 10:28 UTC (permalink / raw)
  To: netdev, w, zyjzyj2000; +Cc: Zhu Yanjun, Bruce Allan, Jeff Kirsher

2.6.x kernels require a similar logic change as commit 44abd5c 
[e1000e: cleanup use of check_reset_block function pointer] introduces
for newer kernels.

Replace e1000_check_reset_block() inline function with calls to the PHY ops
check_reset_block function pointer.

[yanjun.zhu: only modifications in function e1000_init_phy_params_pchlan will
be backported. Others remain unchanged]

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
---
 drivers/net/e1000e/ich8lan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/e1000e/ich8lan.c b/drivers/net/e1000e/ich8lan.c
index 0da2c2c..732cd48 100644
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@ -280,7 +280,7 @@ static s32 e1000_init_phy_params_pchlan(struct e1000_hw *hw)
 	phy->ops.write_phy_reg_locked = e1000_write_phy_reg_hv_locked;
 	phy->autoneg_mask             = AUTONEG_ADVERTISE_SPEED_DEFAULT;
 
-	if (!e1000_check_reset_block(hw)) {
+	if (!hw->phy.ops.check_reset_block(hw)) {
 		/*Set Phy Config Counter to 50msec */
 		ctrl = er32(FEXTNVM3);
 		ctrl &= ~E1000_FEXTNVM3_PHY_CFG_COUNTER_MASK;
-- 
1.9.1

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

* RE: [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578
  2014-12-16 10:28 ` [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578 Zhu Yanjun
@ 2014-12-16 10:48   ` David Laight
  2014-12-16 12:12   ` Jeff Kirsher
  1 sibling, 0 replies; 11+ messages in thread
From: David Laight @ 2014-12-16 10:48 UTC (permalink / raw)
  To: 'Zhu Yanjun', netdev@vger.kernel.org, w@1wt.eu
  Cc: Zhu Yanjun, Bruce Allan, Jeff Kirsher, David S. Miller

From: Zhu Yanjun
> 2.6.x kernels require a similar logic change as commit 6dfaa76
> [e1000e: reset MAC-PHY interconnect on 82577/82578] introduces
> for newer kernels.
> 
> During Sx->S0 transitions, the interconnect between the MAC and PHY on
> 82577/82578 can remain in SMBus mode instead of transitioning to the
> PCIe-like mode required during normal operation.  Toggling the LANPHYPC
> Value bit essentially resets the interconnect forcing it to the correct
> mode.
> 
> after review of all intel drivers, found several instances where
> drivers had the incorrect pattern of:
> memory mapped write();
> delay();
> 
> which should always be:
> memory mapped write();
> write flush(); /* aka memory mapped read */
> delay();

Probably not only the intel drivers.
I'd bet that a lot of the delay() calls are wrong.
 
> explanation:
> The reason for including the flush is that writes can be held
> (posted) in PCI/PCIe bridges, but the read always has to complete
> synchronously and therefore has to flush all pending writes to a
> device.  If a write is held and followed by a delay, the delay
> means nothing because the write may not have reached hardware
> (maybe even not until the next read)

It might even be that the 'write flush' (ie a read) guarantees to
generates a long enough delay that the delay() itself isn't needed.

	David

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

* Re: [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578
  2014-12-16 10:28 ` [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578 Zhu Yanjun
  2014-12-16 10:48   ` David Laight
@ 2014-12-16 12:12   ` Jeff Kirsher
  2014-12-16 12:17     ` Willy Tarreau
  1 sibling, 1 reply; 11+ messages in thread
From: Jeff Kirsher @ 2014-12-16 12:12 UTC (permalink / raw)
  To: Zhu Yanjun, stable, Willy Tarreau
  Cc: netdev, w, Zhu Yanjun, Bruce Allan, David S. Miller

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

On Tue, 2014-12-16 at 18:28 +0800, Zhu Yanjun wrote:
> 2.6.x kernels require a similar logic change as commit 6dfaa76 
> [e1000e: reset MAC-PHY interconnect on 82577/82578] introduces
> for newer kernels.
> 
> During Sx->S0 transitions, the interconnect between the MAC and PHY on
> 82577/82578 can remain in SMBus mode instead of transitioning to the
> PCIe-like mode required during normal operation.  Toggling the
> LANPHYPC
> Value bit essentially resets the interconnect forcing it to the
> correct
> mode.
> 
> after review of all intel drivers, found several instances where
> drivers had the incorrect pattern of:
> memory mapped write();
> delay();
> 
> which should always be:
> memory mapped write();
> write flush(); /* aka memory mapped read */
> delay();
> 
> explanation:
> The reason for including the flush is that writes can be held
> (posted) in PCI/PCIe bridges, but the read always has to complete
> synchronously and therefore has to flush all pending writes to a
> device.  If a write is held and followed by a delay, the delay
> means nothing because the write may not have reached hardware
> (maybe even not until the next read)
> 
> Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
> ---
>  drivers/net/e1000e/defines.h |  2 ++
>  drivers/net/e1000e/ich8lan.c | 20 ++++++++++++++++++++
>  2 files changed, 22 insertions(+)

To be clear, Zhu is wanting this applied to stable trees (yet did not CC
stable@vger.kernel.org ).

Willy- I am fine with this series being applied to stable.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578
  2014-12-16 12:12   ` Jeff Kirsher
@ 2014-12-16 12:17     ` Willy Tarreau
  0 siblings, 0 replies; 11+ messages in thread
From: Willy Tarreau @ 2014-12-16 12:17 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: Zhu Yanjun, stable, netdev, Zhu Yanjun, Bruce Allan,
	David S. Miller

On Tue, Dec 16, 2014 at 04:12:06AM -0800, Jeff Kirsher wrote:
> On Tue, 2014-12-16 at 18:28 +0800, Zhu Yanjun wrote:
> > 2.6.x kernels require a similar logic change as commit 6dfaa76 
> > [e1000e: reset MAC-PHY interconnect on 82577/82578] introduces
> > for newer kernels.
> > 
> > During Sx->S0 transitions, the interconnect between the MAC and PHY on
> > 82577/82578 can remain in SMBus mode instead of transitioning to the
> > PCIe-like mode required during normal operation.  Toggling the
> > LANPHYPC
> > Value bit essentially resets the interconnect forcing it to the
> > correct
> > mode.
> > 
> > after review of all intel drivers, found several instances where
> > drivers had the incorrect pattern of:
> > memory mapped write();
> > delay();
> > 
> > which should always be:
> > memory mapped write();
> > write flush(); /* aka memory mapped read */
> > delay();
> > 
> > explanation:
> > The reason for including the flush is that writes can be held
> > (posted) in PCI/PCIe bridges, but the read always has to complete
> > synchronously and therefore has to flush all pending writes to a
> > device.  If a write is held and followed by a delay, the delay
> > means nothing because the write may not have reached hardware
> > (maybe even not until the next read)
> > 
> > Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
> > ---
> >  drivers/net/e1000e/defines.h |  2 ++
> >  drivers/net/e1000e/ich8lan.c | 20 ++++++++++++++++++++
> >  2 files changed, 22 insertions(+)
> 
> To be clear, Zhu is wanting this applied to stable trees (yet did not CC
> stable@vger.kernel.org ).
> 
> Willy- I am fine with this series being applied to stable.

OK, thanks Jeff! I'm queuing the patches now.

Best regards,
Willy

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

* Re: [PATCH V2 0/5] e1000e: fix nic not boot after rebooting
  2014-12-16 10:28 [PATCH V2 0/5] e1000e: fix nic not boot after rebooting Zhu Yanjun
                   ` (4 preceding siblings ...)
  2014-12-16 10:28 ` [PATCH 5/5] e1000e: cleanup use of check_reset_block function pointer Zhu Yanjun
@ 2015-05-14 17:10 ` Willy Tarreau
  5 siblings, 0 replies; 11+ messages in thread
From: Willy Tarreau @ 2015-05-14 17:10 UTC (permalink / raw)
  To: Zhu Yanjun; +Cc: netdev, Zhu Yanjun

Hi,

On Tue, Dec 16, 2014 at 06:28:15PM +0800, Zhu Yanjun wrote:
> With kernel 2.6.x, e1000e with 82577/8/9 sometimes will not boot
> after rebooting. 
> 
> If a kernel 2.6.x board with 82577/8/9 e1000e nic is rebooted for 
> 100 times, there are 7~8 times that 82577/8/9 e1000e nic will not boot 
> normally.
> 
> V2:
> Follow the advice from Willy, the wrong upstream commit IDs in these
> 5 patches commit messages are fixed.
> 
> Zhu Yanjun (5):
>   e1000e: reset MAC-PHY interconnect on 82577/82578
>   e1000e: workaround EEPROM configuration change on 82579 on kernel
>     2.6.x
>   e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked
>   e1000e: update workaround for 82579 intermittently disabled during
>     S0->Sx
>   e1000e: cleanup use of check_reset_block function pointer

I queued this series for 2.6.32.66 but found that it doesn't build
because patch 2 adds this test :

@@ -2685,6 +2694,14 @@ static s32 e1000_reset_hw_ich8lan(struct e1000_hw *hw)
        ew32(CTRL, (ctrl | E1000_CTRL_RST));                            
        msleep(20);

+       /* Set Phy Config Counter to 50msec */
+       if (hw->mac.type == e1000_pch2lan) {
+               u32 phycc_reg = er32(FEXTNVM3);
+               phycc_reg &= ~E1000_FEXTNVM3_PHY_CFG_COUNTER_MASK;
+               phycc_reg |= E1000_FEXTNVM3_PHY_CFG_COUNTER_50MSEC;
+               ew32(FEXTNVM3, phycc_reg);
+       }
+

And e1000_pch2lan doesn't appear in the enum in 2.6.32 hence this
type is never assigned. Thus I'm wondering if I should just drop
this test (trivial) or if there are other parts that may not be
relevant to this kernel version.

Any hint will be greatly appreciated.

Thanks!
Willy

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

end of thread, other threads:[~2015-05-14 17:10 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-16 10:28 [PATCH V2 0/5] e1000e: fix nic not boot after rebooting Zhu Yanjun
2014-12-16 10:28 ` [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578 Zhu Yanjun
2014-12-16 10:48   ` David Laight
2014-12-16 12:12   ` Jeff Kirsher
2014-12-16 12:17     ` Willy Tarreau
2014-12-16 10:28 ` [PATCH 2/5] e1000e: workaround EEPROM configuration change on 82579 on kernel 2.6.x Zhu Yanjun
2014-12-16 10:28 ` [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked Zhu Yanjun
2014-12-16 10:28 ` [PATCH 4/5] e1000e: update workaround for 82579 intermittently disabled during S0->Sx Zhu Yanjun
2014-12-16 10:28 ` [PATCH 5/5] e1000e: cleanup use of check_reset_block function pointer Zhu Yanjun
2015-05-14 17:10 ` [PATCH V2 0/5] e1000e: fix nic not boot after rebooting Willy Tarreau
  -- strict thread matches above, loose matches on Subject: below --
2014-12-15  8:39 [PATCH " Zhu Yanjun
2014-12-15  8:39 ` [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578 Zhu Yanjun

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).