All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] e1000e: fix nic not boot after rebooting
@ 2014-12-15  8:39 Zhu Yanjun
  2014-12-15  8:39 ` [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578 Zhu Yanjun
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Zhu Yanjun @ 2014-12-15  8:39 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.

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] 13+ messages in thread

* [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578
  2014-12-15  8:39 [PATCH 0/5] e1000e: fix nic not boot after rebooting Zhu Yanjun
@ 2014-12-15  8:39 ` Zhu Yanjun
  2014-12-15  8:39 ` [PATCH 2/5] e1000e: workaround EEPROM configuration change on 82579 on kernel 2.6.x Zhu Yanjun
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ 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] 13+ messages in thread

* [PATCH 2/5] e1000e: workaround EEPROM configuration change on 82579 on kernel 2.6.x
  2014-12-15  8:39 [PATCH 0/5] e1000e: fix nic not boot after rebooting Zhu Yanjun
  2014-12-15  8:39 ` [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578 Zhu Yanjun
@ 2014-12-15  8:39 ` Zhu Yanjun
  2014-12-15  8:39 ` [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked Zhu Yanjun
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ 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

2.6.x kernels require a similar logic change as commit 62f1d8d1
[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] 13+ messages in thread

* [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked
  2014-12-15  8:39 [PATCH 0/5] e1000e: fix nic not boot after rebooting Zhu Yanjun
  2014-12-15  8:39 ` [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578 Zhu Yanjun
  2014-12-15  8:39 ` [PATCH 2/5] e1000e: workaround EEPROM configuration change on 82579 on kernel 2.6.x Zhu Yanjun
@ 2014-12-15  8:39 ` Zhu Yanjun
  2014-12-15 12:01   ` Sergei Shtylyov
  2014-12-15  8:39 ` [PATCH 4/5] e1000e: update workaround for 82579 intermittently disabled during S0->Sx Zhu Yanjun
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ 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

2.6.x kernels require a similar logic change as commit b7d6e335
[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] 13+ messages in thread

* [PATCH 4/5] e1000e: update workaround for 82579 intermittently disabled during S0->Sx
  2014-12-15  8:39 [PATCH 0/5] e1000e: fix nic not boot after rebooting Zhu Yanjun
                   ` (2 preceding siblings ...)
  2014-12-15  8:39 ` [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked Zhu Yanjun
@ 2014-12-15  8:39 ` Zhu Yanjun
  2014-12-15  8:39 ` [PATCH 5/5] e1000e: cleanup use of check_reset_block function pointer Zhu Yanjun
  2014-12-15  8:48 ` [PATCH 0/5] e1000e: fix nic not boot after rebooting Willy Tarreau
  5 siblings, 0 replies; 13+ 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

2.6.x kernels require a similar logic change as commit c6f8b74f
[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] 13+ messages in thread

* [PATCH 5/5] e1000e: cleanup use of check_reset_block function pointer
  2014-12-15  8:39 [PATCH 0/5] e1000e: fix nic not boot after rebooting Zhu Yanjun
                   ` (3 preceding siblings ...)
  2014-12-15  8:39 ` [PATCH 4/5] e1000e: update workaround for 82579 intermittently disabled during S0->Sx Zhu Yanjun
@ 2014-12-15  8:39 ` Zhu Yanjun
  2014-12-15  8:48 ` [PATCH 0/5] e1000e: fix nic not boot after rebooting Willy Tarreau
  5 siblings, 0 replies; 13+ 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

2.6.x kernels require a similar logic change as commit 13ca85e0
[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] 13+ messages in thread

* Re: [PATCH 0/5] e1000e: fix nic not boot after rebooting
  2014-12-15  8:39 [PATCH 0/5] e1000e: fix nic not boot after rebooting Zhu Yanjun
                   ` (4 preceding siblings ...)
  2014-12-15  8:39 ` [PATCH 5/5] e1000e: cleanup use of check_reset_block function pointer Zhu Yanjun
@ 2014-12-15  8:48 ` Willy Tarreau
  5 siblings, 0 replies; 13+ messages in thread
From: Willy Tarreau @ 2014-12-15  8:48 UTC (permalink / raw)
  To: Zhu Yanjun, Bruce Allan, Jeff Kirsher; +Cc: netdev, Zhu Yanjun

Hello,

On Mon, Dec 15, 2014 at 04:39:09PM +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.
> 
> 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

Great, thanks! Jeff/Bruce, do you have any objection against me applying
these fixes to 2.6.32 ?

Thanks,
Willy

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

* Re: [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked
  2014-12-15  8:39 ` [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked Zhu Yanjun
@ 2014-12-15 12:01   ` Sergei Shtylyov
  2014-12-15 12:16     ` Willy Tarreau
  0 siblings, 1 reply; 13+ messages in thread
From: Sergei Shtylyov @ 2014-12-15 12:01 UTC (permalink / raw)
  To: Zhu Yanjun, netdev, w; +Cc: Zhu Yanjun, Bruce Allan, Jeff Kirsher

Hello.

On 12/15/2014 11:39 AM, Zhu Yanjun wrote:

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

    Hm, so is this patch to 2.6.x-stable kernels or a recent kernel?
If the former, you should follow the rules in 
Documentation/stable_kernel_rules.txt.

> 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>

    So, is this your patch, or Bruce's? If the latter, you should add:

From: Bruce Allan <bruce.w.allan@intel.com>

at the start of the change log.

> 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>

WBR, Sergei

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

* Re: [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked
  2014-12-15 12:01   ` Sergei Shtylyov
@ 2014-12-15 12:16     ` Willy Tarreau
  2014-12-15 13:21       ` Zhu, Yanjun
  0 siblings, 1 reply; 13+ messages in thread
From: Willy Tarreau @ 2014-12-15 12:16 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: Zhu Yanjun, netdev, Zhu Yanjun, Bruce Allan, Jeff Kirsher

Hello,

On Mon, Dec 15, 2014 at 03:01:48PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 12/15/2014 11:39 AM, Zhu Yanjun wrote:
> 
> >2.6.x kernels require a similar logic change as commit b7d6e335
> >[e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked]
> >introduces for newer kernels.
> 
>    Hm, so is this patch to 2.6.x-stable kernels or a recent kernel?
> If the former, you should follow the rules in 
> Documentation/stable_kernel_rules.txt.

I don't see anything there that does not comply with the rules. This
patchset looks fine and acceptable to me, I'll just wait a bit so that
if either Jeff or Bruce rejects it I respect their wish.

However you just made me realize that I can't find commit b7d6e335.
Zhu, please double-check that this commit (or an equivalent) was merged
upstream, *this* it a prerequisite for going into -stable.

> >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>
> 
>    So, is this your patch, or Bruce's? If the latter, you should add:
> 
> From: Bruce Allan <bruce.w.allan@intel.com>
> 
> at the start of the change log.
>
> >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>

The case where a commit gets backported is always ambiguous, especially
when the context changes a lot. I'm personally in favor of keeping the
original signed-of-by chain related to the original fix, and adding some
text between it and the backporter's s-o-b indicating that the changes
were, so that original authors do not get blamed for mistakes.

Best regards,
Willy

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

* RE: [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked
  2014-12-15 12:16     ` Willy Tarreau
@ 2014-12-15 13:21       ` Zhu, Yanjun
  2014-12-15 13:33         ` Willy Tarreau
  0 siblings, 1 reply; 13+ messages in thread
From: Zhu, Yanjun @ 2014-12-15 13:21 UTC (permalink / raw)
  To: Willy Tarreau, Sergei Shtylyov
  Cc: Zhu Yanjun, netdev, ALLAN, BRUCE, KIRSHER, JEFFREY

Hi, Willy

Thanks for your reply.

This patch "[PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked" exists in tag v3.1.

Please follow these steps, this patch can be found:

1. git checkout -f v3.1

2. git log -p drivers/net/e1000e/ich8lan.c

3. search "b7d6e335"

Then we will find this patch.

Best Regards!
Zhu Yanjun

________________________________________
From: Willy Tarreau [w@1wt.eu]
Sent: Monday, December 15, 2014 4:16 AM
To: Sergei Shtylyov
Cc: Zhu Yanjun; netdev@vger.kernel.org; Zhu, Yanjun; ALLAN, BRUCE; KIRSHER, JEFFREY
Subject: Re: [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked

Hello,

On Mon, Dec 15, 2014 at 03:01:48PM +0300, Sergei Shtylyov wrote:
> Hello.
>
> On 12/15/2014 11:39 AM, Zhu Yanjun wrote:
>
> >2.6.x kernels require a similar logic change as commit b7d6e335
> >[e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked]
> >introduces for newer kernels.
>
>    Hm, so is this patch to 2.6.x-stable kernels or a recent kernel?
> If the former, you should follow the rules in
> Documentation/stable_kernel_rules.txt.

I don't see anything there that does not comply with the rules. This
patchset looks fine and acceptable to me, I'll just wait a bit so that
if either Jeff or Bruce rejects it I respect their wish.

However you just made me realize that I can't find commit b7d6e335.
Zhu, please double-check that this commit (or an equivalent) was merged
upstream, *this* it a prerequisite for going into -stable.



> >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>
>
>    So, is this your patch, or Bruce's? If the latter, you should add:
>
> From: Bruce Allan <bruce.w.allan@intel.com>
>
> at the start of the change log.
>
> >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>

The case where a commit gets backported is always ambiguous, especially
when the context changes a lot. I'm personally in favor of keeping the
original signed-of-by chain related to the original fix, and adding some
text between it and the backporter's s-o-b indicating that the changes
were, so that original authors do not get blamed for mistakes.

Best regards,
Willy

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

* Re: [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked
  2014-12-15 13:21       ` Zhu, Yanjun
@ 2014-12-15 13:33         ` Willy Tarreau
  2014-12-16  2:08           ` yzhu1
  0 siblings, 1 reply; 13+ messages in thread
From: Willy Tarreau @ 2014-12-15 13:33 UTC (permalink / raw)
  To: Zhu, Yanjun
  Cc: Sergei Shtylyov, Zhu Yanjun, netdev, ALLAN, BRUCE, KIRSHER, JEFFREY

On Mon, Dec 15, 2014 at 01:21:43PM +0000, Zhu, Yanjun wrote:
> Hi, Willy
> 
> Thanks for your reply.
> 
> This patch "[PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked" exists in tag v3.1.
> 
> Please follow these steps, this patch can be found:
> 
> 1. git checkout -f v3.1
> 
> 2. git log -p drivers/net/e1000e/ich8lan.c
> 
> 3. search "b7d6e335"
> 
> Then we will find this patch.

Ah it's because you truncated the commit ID from the right instead of from
the left. Truncated commit IDs are valid from the left, not from the right.
In your case, the commit is 6cc7aaed70c96c3933fbacbad582fc79b7d6e335
("e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked"), so
the truncated ID is 6cc7aae, not b7d6e335. It's important to fix that in
your commit messages so that a "git show" works correctly (it failed for me
for this precise reason).

Thanks,
Willy

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

* Re: [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked
  2014-12-15 13:33         ` Willy Tarreau
@ 2014-12-16  2:08           ` yzhu1
  0 siblings, 0 replies; 13+ messages in thread
From: yzhu1 @ 2014-12-16  2:08 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Sergei Shtylyov, Zhu Yanjun, netdev, ALLAN, BRUCE, KIRSHER, JEFFREY

Hi, Willy

Thanks for your reply.

I will fix it and send V2.

Best Regards!
Zhu Yanjun
On 12/15/2014 09:33 PM, Willy Tarreau wrote:
> On Mon, Dec 15, 2014 at 01:21:43PM +0000, Zhu, Yanjun wrote:
>> Hi, Willy
>>
>> Thanks for your reply.
>>
>> This patch "[PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked" exists in tag v3.1.
>>
>> Please follow these steps, this patch can be found:
>>
>> 1. git checkout -f v3.1
>>
>> 2. git log -p drivers/net/e1000e/ich8lan.c
>>
>> 3. search "b7d6e335"
>>
>> Then we will find this patch.
> Ah it's because you truncated the commit ID from the right instead of from
> the left. Truncated commit IDs are valid from the left, not from the right.
> In your case, the commit is 6cc7aaed70c96c3933fbacbad582fc79b7d6e335
> ("e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked"), so
> the truncated ID is 6cc7aae, not b7d6e335. It's important to fix that in
> your commit messages so that a "git show" works correctly (it failed for me
> for this precise reason).
>
> Thanks,
> Willy
>

^ permalink raw reply	[flat|nested] 13+ 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 " Zhu Yanjun
@ 2014-12-16 10:28 ` Zhu Yanjun
  0 siblings, 0 replies; 13+ 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] 13+ messages in thread

end of thread, other threads:[~2014-12-16 10:28 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-15  8:39 [PATCH 0/5] e1000e: fix nic not boot after rebooting Zhu Yanjun
2014-12-15  8:39 ` [PATCH 1/5] e1000e: reset MAC-PHY interconnect on 82577/82578 Zhu Yanjun
2014-12-15  8:39 ` [PATCH 2/5] e1000e: workaround EEPROM configuration change on 82579 on kernel 2.6.x Zhu Yanjun
2014-12-15  8:39 ` [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked Zhu Yanjun
2014-12-15 12:01   ` Sergei Shtylyov
2014-12-15 12:16     ` Willy Tarreau
2014-12-15 13:21       ` Zhu, Yanjun
2014-12-15 13:33         ` Willy Tarreau
2014-12-16  2:08           ` yzhu1
2014-12-15  8:39 ` [PATCH 4/5] e1000e: update workaround for 82579 intermittently disabled during S0->Sx Zhu Yanjun
2014-12-15  8:39 ` [PATCH 5/5] e1000e: cleanup use of check_reset_block function pointer Zhu Yanjun
2014-12-15  8:48 ` [PATCH 0/5] e1000e: fix nic not boot after rebooting Willy Tarreau
2014-12-16 10:28 [PATCH V2 " Zhu Yanjun
2014-12-16 10:28 ` [PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked Zhu Yanjun

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.