[net-next-2.6 PATCH] ixgbe: fix for link failure on SFP+ DA cables

[net-next-2.6 PATCH] ixgbe: fix for link failure on SFP+ DA cables

[Jeff Kirsher]

From: Don Skidmore <donald.c.skidmore@intel.com>

This patch helps prevent FW/SW semaphore collision from leading
to link establishment failure.  The collision might mess up the
PHY registers so we reset the PHY.  However there are SFI/KR areas
in the PHY that are not reset with a Reset_AN so we need to change
LMS to reset it.  Also wait until AN state machine is AN_GOOD

Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_82599.c |   28 +++++++++++++++++++++++++---
 drivers/net/ixgbe/ixgbe_type.h  |    3 +++
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_82599.c b/drivers/net/ixgbe/ixgbe_82599.c
index e34643e..26b8ceb 100644
--- a/drivers/net/ixgbe/ixgbe_82599.c
+++ b/drivers/net/ixgbe/ixgbe_82599.c
@@ -90,6 +90,8 @@ static void ixgbe_init_mac_link_ops_82599(struct ixgbe_hw *hw)
 static s32 ixgbe_setup_sfp_modules_82599(struct ixgbe_hw *hw)
 {
 	s32 ret_val = 0;
+	u32 reg_anlp1 = 0;
+	u32 i = 0;
 	u16 list_offset, data_offset, data_value;
 
 	if (hw->phy.sfp_type != ixgbe_sfp_type_unknown) {
@@ -116,14 +118,34 @@ static s32 ixgbe_setup_sfp_modules_82599(struct ixgbe_hw *hw)
 			IXGBE_WRITE_FLUSH(hw);
 			hw->eeprom.ops.read(hw, ++data_offset, &data_value);
 		}
-		/* Now restart DSP by setting Restart_AN */
-		IXGBE_WRITE_REG(hw, IXGBE_AUTOC,
-		    (IXGBE_READ_REG(hw, IXGBE_AUTOC) | IXGBE_AUTOC_AN_RESTART));
 
 		/* Release the semaphore */
 		ixgbe_release_swfw_sync(hw, IXGBE_GSSR_MAC_CSR_SM);
 		/* Delay obtaining semaphore again to allow FW access */
 		msleep(hw->eeprom.semaphore_delay);
+
+		/* Now restart DSP by setting Restart_AN and clearing LMS */
+		IXGBE_WRITE_REG(hw, IXGBE_AUTOC, ((IXGBE_READ_REG(hw,
+		                IXGBE_AUTOC) & ~IXGBE_AUTOC_LMS_MASK) |
+		                IXGBE_AUTOC_AN_RESTART));
+
+		/* Wait for AN to leave state 0 */
+		for (i = 0; i < 10; i++) {
+			msleep(4);
+			reg_anlp1 = IXGBE_READ_REG(hw, IXGBE_ANLP1);
+			if (reg_anlp1 & IXGBE_ANLP1_AN_STATE_MASK)
+				break;
+		}
+		if (!(reg_anlp1 & IXGBE_ANLP1_AN_STATE_MASK)) {
+			hw_dbg(hw, "sfp module setup not complete\n");
+			ret_val = IXGBE_ERR_SFP_SETUP_NOT_COMPLETE;
+			goto setup_sfp_out;
+		}
+
+		/* Restart DSP by setting Restart_AN and return to SFI mode */
+		IXGBE_WRITE_REG(hw, IXGBE_AUTOC, (IXGBE_READ_REG(hw,
+		                IXGBE_AUTOC) | IXGBE_AUTOC_LMS_10G_SERIAL |
+		                IXGBE_AUTOC_AN_RESTART));
 	}
 
 setup_sfp_out:
diff --git a/drivers/net/ixgbe/ixgbe_type.h b/drivers/net/ixgbe/ixgbe_type.h
index 42c6073..20e1f03 100644
--- a/drivers/net/ixgbe/ixgbe_type.h
+++ b/drivers/net/ixgbe/ixgbe_type.h
@@ -1467,6 +1467,8 @@
 #define IXGBE_ANLP1_PAUSE               0x0C00
 #define IXGBE_ANLP1_SYM_PAUSE           0x0400
 #define IXGBE_ANLP1_ASM_PAUSE           0x0800
+#define IXGBE_ANLP1_AN_STATE_MASK       0x000f0000
+
 
 /* SW Semaphore Register bitmasks */
 #define IXGBE_SWSM_SMBI 0x00000001 /* Driver Semaphore bit */
@@ -2634,6 +2636,7 @@ struct ixgbe_info {
 #define IXGBE_ERR_NO_SPACE                      -25
 #define IXGBE_ERR_OVERTEMP                      -26
 #define IXGBE_ERR_RAR_INDEX                     -27
+#define IXGBE_ERR_SFP_SETUP_NOT_COMPLETE        -30
 #define IXGBE_NOT_IMPLEMENTED                   0x7FFFFFFF
 
 #endif /* _IXGBE_TYPE_H_ */

Re: [net-next-2.6 PATCH] ixgbe: fix for link failure on SFP+ DA cables

[Joe Perches]

On Wed, 2010-12-01 at 22:59 -0800, Jeff Kirsher wrote:
> This patch helps prevent FW/SW semaphore collision from leading
> to link establishment failure.  The collision might mess up the
> PHY registers so we reset the PHY.  However there are SFI/KR areas
> in the PHY that are not reset with a Reset_AN so we need to change
> LMS to reset it.  Also wait until AN state machine is AN_GOOD
[]
>  drivers/net/ixgbe/ixgbe_82599.c |   28 +++++++++++++++++++++++++---
> diff --git a/drivers/net/ixgbe/ixgbe_82599.c b/drivers/net/ixgbe/ixgbe_82599.c
[]
> @@ -116,14 +118,34 @@ static s32 ixgbe_setup_sfp_modules_82599(struct ixgbe_hw *hw)
>  			IXGBE_WRITE_FLUSH(hw);
>  			hw->eeprom.ops.read(hw, ++data_offset, &data_value);
>  		}
> -		/* Now restart DSP by setting Restart_AN */
> -		IXGBE_WRITE_REG(hw, IXGBE_AUTOC,
> -		    (IXGBE_READ_REG(hw, IXGBE_AUTOC) | IXGBE_AUTOC_AN_RESTART));
>  
>  		/* Release the semaphore */
>  		ixgbe_release_swfw_sync(hw, IXGBE_GSSR_MAC_CSR_SM);
>  		/* Delay obtaining semaphore again to allow FW access */
>  		msleep(hw->eeprom.semaphore_delay);

might these msleeps be usleep_range?

[]

> +		/* Wait for AN to leave state 0 */
> +		for (i = 0; i < 10; i++) {
> +			msleep(4);

Perhaps usleep_range?

Maybe all the msleep uses < 20ms in the intel drivers?

$ grep -nPr --include=*.[ch] "msleep\s*\(\s*1?\d\s*\)" drivers/net/ig* drivers/net/ixg* drivers/net/e100* | wc -l
123

Maybe something like doubling the sleep value
for the upper bound range?

Here's a little script that does that.

$ grep -nPrl --include=*.[ch] "msleep\s*\(\s*1?\d\s*\)" \
	drivers/net/ig* drivers/net/ixg* drivers/net/e100* | \
xargs perl -p -i -e 's/msleep\s*\(\s*(1?\d)\s*\)/"usleep_range\(${1}000, " . scalar($1) * 2 . "000\)"/ge'

RE: [net-next-2.6 PATCH] ixgbe: fix for link failure on SFP+ DA cables

[Skidmore, Donald C]

>-----Original Message-----
>From: Joe Perches [mailto:joe@perches.com]
>Sent: Wednesday, December 01, 2010 11:42 PM
>To: Kirsher, Jeffrey T
>Cc: davem@davemloft.net; netdev@vger.kernel.org; gospo@redhat.com;
>bphilips@novell.com; Skidmore, Donald C
>Subject: Re: [net-next-2.6 PATCH] ixgbe: fix for link failure on SFP+ DA
>cables
>
>On Wed, 2010-12-01 at 22:59 -0800, Jeff Kirsher wrote:
>> This patch helps prevent FW/SW semaphore collision from leading
>> to link establishment failure.  The collision might mess up the
>> PHY registers so we reset the PHY.  However there are SFI/KR areas
>> in the PHY that are not reset with a Reset_AN so we need to change
>> LMS to reset it.  Also wait until AN state machine is AN_GOOD
>[]
>>  drivers/net/ixgbe/ixgbe_82599.c |   28 +++++++++++++++++++++++++---
>> diff --git a/drivers/net/ixgbe/ixgbe_82599.c
>b/drivers/net/ixgbe/ixgbe_82599.c
>[]
>> @@ -116,14 +118,34 @@ static s32 ixgbe_setup_sfp_modules_82599(struct
>ixgbe_hw *hw)
>>  			IXGBE_WRITE_FLUSH(hw);
>>  			hw->eeprom.ops.read(hw, ++data_offset, &data_value);
>>  		}
>> -		/* Now restart DSP by setting Restart_AN */
>> -		IXGBE_WRITE_REG(hw, IXGBE_AUTOC,
>> -		    (IXGBE_READ_REG(hw, IXGBE_AUTOC) |
>IXGBE_AUTOC_AN_RESTART));
>>
>>  		/* Release the semaphore */
>>  		ixgbe_release_swfw_sync(hw, IXGBE_GSSR_MAC_CSR_SM);
>>  		/* Delay obtaining semaphore again to allow FW access */
>>  		msleep(hw->eeprom.semaphore_delay);
>
>might these msleeps be usleep_range?
>
>[]
>
>> +		/* Wait for AN to leave state 0 */
>> +		for (i = 0; i < 10; i++) {
>> +			msleep(4);
>
>Perhaps usleep_range?
>
>Maybe all the msleep uses < 20ms in the intel drivers?
>
>$ grep -nPr --include=*.[ch] "msleep\s*\(\s*1?\d\s*\)" drivers/net/ig*
>drivers/net/ixg* drivers/net/e100* | wc -l
>123
>
>Maybe something like doubling the sleep value
>for the upper bound range?
>
>Here's a little script that does that.
>
>$ grep -nPrl --include=*.[ch] "msleep\s*\(\s*1?\d\s*\)" \
>	drivers/net/ig* drivers/net/ixg* drivers/net/e100* | \
>xargs perl -p -i -e 's/msleep\s*\(\s*(1?\d)\s*\)/"usleep_range\(${1}000, "
>. scalar($1) * 2 . "000\)"/ge'
>

Good point about the msleeps.  We do have a plan to change all the <20 ones to usleep_range I just haven't gotten around to make the patch yet.  I just wasn't positive on what our upper range would need to be for each case and wanted to look at each individually to make sure.  Your suggestion may well end up being what we end up trying (doubling the upper bound). :)

I kept msleeps in this patch as I wanted to be consistent when doing the usleep_range changeover and the longer delay really doesn't hurt here as the sleep value is really only the minimum needed.  I do promises that the other patch is coming soon. :)

Thanks,
-Don