[Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Jeff Daly]

Some (Juniper MX5) SFP link partners exhibit a disinclination to
autonegotiate with X550 configured in SFI mode.  This patch enables
a manual AN-37 restart to work around the problem.

Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |  3 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 50 +++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 2647937f7f4d..dc8a259fda5f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -3705,7 +3705,9 @@ struct ixgbe_info {
 #define IXGBE_KRM_LINK_S1(P)		((P) ? 0x8200 : 0x4200)
 #define IXGBE_KRM_LINK_CTRL_1(P)	((P) ? 0x820C : 0x420C)
 #define IXGBE_KRM_AN_CNTL_1(P)		((P) ? 0x822C : 0x422C)
+#define IXGBE_KRM_AN_CNTL_4(P)		((P) ? 0x8238 : 0x4238)
 #define IXGBE_KRM_AN_CNTL_8(P)		((P) ? 0x8248 : 0x4248)
+#define IXGBE_KRM_PCS_KX_AN(P)		((P) ? 0x9918 : 0x5918)
 #define IXGBE_KRM_SGMII_CTRL(P)		((P) ? 0x82A0 : 0x42A0)
 #define IXGBE_KRM_LP_BASE_PAGE_HIGH(P)	((P) ? 0x836C : 0x436C)
 #define IXGBE_KRM_DSP_TXFFE_STATE_4(P)	((P) ? 0x8634 : 0x4634)
@@ -3715,6 +3717,7 @@ struct ixgbe_info {
 #define IXGBE_KRM_PMD_FLX_MASK_ST20(P)	((P) ? 0x9054 : 0x5054)
 #define IXGBE_KRM_TX_COEFF_CTRL_1(P)	((P) ? 0x9520 : 0x5520)
 #define IXGBE_KRM_RX_ANA_CTL(P)		((P) ? 0x9A00 : 0x5A00)
+#define IXGBE_KRM_FLX_TMRS_CTRL_ST31(P)	((P) ? 0x9180 : 0x5180)
 
 #define IXGBE_KRM_PMD_FLX_MASK_ST20_SFI_10G_DA		~(0x3 << 20)
 #define IXGBE_KRM_PMD_FLX_MASK_ST20_SFI_10G_SR		BIT(20)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
index e4b50c7781ff..f48a422ae83f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
@@ -1725,6 +1725,56 @@ static s32 ixgbe_setup_sfi_x550a(struct ixgbe_hw *hw, ixgbe_link_speed *speed)
 				IXGBE_KRM_PMD_FLX_MASK_ST20(hw->bus.lan_id),
 				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
 
+	/* change mode enforcement rules to hybrid */
+	status = mac->ops.read_iosf_sb_reg(hw,
+				IXGBE_KRM_FLX_TMRS_CTRL_ST31(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+	reg_val |= 0x0400;
+
+	status = mac->ops.write_iosf_sb_reg(hw,
+				IXGBE_KRM_FLX_TMRS_CTRL_ST31(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+
+	/* manually control the config */
+	status = mac->ops.read_iosf_sb_reg(hw,
+				IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+	reg_val |= 0x20002240;
+
+	status = mac->ops.write_iosf_sb_reg(hw,
+				IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+
+	/* move the AN base page values */
+	status = mac->ops.read_iosf_sb_reg(hw,
+				IXGBE_KRM_PCS_KX_AN(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+	reg_val |= 0x1;
+
+	status = mac->ops.write_iosf_sb_reg(hw,
+				IXGBE_KRM_PCS_KX_AN(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+
+	/* set the AN37 over CB mode */
+	status = mac->ops.read_iosf_sb_reg(hw,
+				IXGBE_KRM_AN_CNTL_4(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+	reg_val |= 0x20000000;
+
+	status = mac->ops.write_iosf_sb_reg(hw,
+				IXGBE_KRM_AN_CNTL_4(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+
+	/* restart AN manually */
+	status = mac->ops.read_iosf_sb_reg(hw,
+				IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+	reg_val |= IXGBE_KRM_LINK_CTRL_1_TETH_AN_RESTART;
+
+	status = mac->ops.write_iosf_sb_reg(hw,
+				IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+
 	/* Toggle port SW reset by AN reset. */
 	status = ixgbe_restart_an_internal_phy_x550em(hw);
 
-- 
2.25.1

[Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Jeff Daly]

Some (Juniper MX5) SFP link partners exhibit a disinclination to
autonegotiate with X550 configured in SFI mode.  This patch enables
a manual AN-37 restart to work around the problem.

Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |  3 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 50 +++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 2647937f7f4d..dc8a259fda5f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -3705,7 +3705,9 @@ struct ixgbe_info {
 #define IXGBE_KRM_LINK_S1(P)		((P) ? 0x8200 : 0x4200)
 #define IXGBE_KRM_LINK_CTRL_1(P)	((P) ? 0x820C : 0x420C)
 #define IXGBE_KRM_AN_CNTL_1(P)		((P) ? 0x822C : 0x422C)
+#define IXGBE_KRM_AN_CNTL_4(P)		((P) ? 0x8238 : 0x4238)
 #define IXGBE_KRM_AN_CNTL_8(P)		((P) ? 0x8248 : 0x4248)
+#define IXGBE_KRM_PCS_KX_AN(P)		((P) ? 0x9918 : 0x5918)
 #define IXGBE_KRM_SGMII_CTRL(P)		((P) ? 0x82A0 : 0x42A0)
 #define IXGBE_KRM_LP_BASE_PAGE_HIGH(P)	((P) ? 0x836C : 0x436C)
 #define IXGBE_KRM_DSP_TXFFE_STATE_4(P)	((P) ? 0x8634 : 0x4634)
@@ -3715,6 +3717,7 @@ struct ixgbe_info {
 #define IXGBE_KRM_PMD_FLX_MASK_ST20(P)	((P) ? 0x9054 : 0x5054)
 #define IXGBE_KRM_TX_COEFF_CTRL_1(P)	((P) ? 0x9520 : 0x5520)
 #define IXGBE_KRM_RX_ANA_CTL(P)		((P) ? 0x9A00 : 0x5A00)
+#define IXGBE_KRM_FLX_TMRS_CTRL_ST31(P)	((P) ? 0x9180 : 0x5180)
 
 #define IXGBE_KRM_PMD_FLX_MASK_ST20_SFI_10G_DA		~(0x3 << 20)
 #define IXGBE_KRM_PMD_FLX_MASK_ST20_SFI_10G_SR		BIT(20)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
index e4b50c7781ff..f48a422ae83f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
@@ -1725,6 +1725,56 @@ static s32 ixgbe_setup_sfi_x550a(struct ixgbe_hw *hw, ixgbe_link_speed *speed)
 				IXGBE_KRM_PMD_FLX_MASK_ST20(hw->bus.lan_id),
 				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
 
+	/* change mode enforcement rules to hybrid */
+	status = mac->ops.read_iosf_sb_reg(hw,
+				IXGBE_KRM_FLX_TMRS_CTRL_ST31(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+	reg_val |= 0x0400;
+
+	status = mac->ops.write_iosf_sb_reg(hw,
+				IXGBE_KRM_FLX_TMRS_CTRL_ST31(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+
+	/* manually control the config */
+	status = mac->ops.read_iosf_sb_reg(hw,
+				IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+	reg_val |= 0x20002240;
+
+	status = mac->ops.write_iosf_sb_reg(hw,
+				IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+
+	/* move the AN base page values */
+	status = mac->ops.read_iosf_sb_reg(hw,
+				IXGBE_KRM_PCS_KX_AN(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+	reg_val |= 0x1;
+
+	status = mac->ops.write_iosf_sb_reg(hw,
+				IXGBE_KRM_PCS_KX_AN(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+
+	/* set the AN37 over CB mode */
+	status = mac->ops.read_iosf_sb_reg(hw,
+				IXGBE_KRM_AN_CNTL_4(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+	reg_val |= 0x20000000;
+
+	status = mac->ops.write_iosf_sb_reg(hw,
+				IXGBE_KRM_AN_CNTL_4(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+
+	/* restart AN manually */
+	status = mac->ops.read_iosf_sb_reg(hw,
+				IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+	reg_val |= IXGBE_KRM_LINK_CTRL_1_TETH_AN_RESTART;
+
+	status = mac->ops.write_iosf_sb_reg(hw,
+				IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
+				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+
 	/* Toggle port SW reset by AN reset. */
 	status = ixgbe_restart_an_internal_phy_x550em(hw);
 
-- 
2.25.1

[Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Tony Nguyen]

On 3/16/2022 12:27 PM, Jeff Daly wrote:
> Some (Juniper MX5) SFP link partners exhibit a disinclination to
> autonegotiate with X550 configured in SFI mode.  This patch enables
> a manual AN-37 restart to work around the problem.

Hi Jeff,

I talked to the ixgbe team about this and we need a bit more time to 
look this over. Will keep you updated.

> Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |  3 ++
>   drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 50 +++++++++++++++++++
>   2 files changed, 53 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> index 2647937f7f4d..dc8a259fda5f 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> @@ -3705,7 +3705,9 @@ struct ixgbe_info {
>   #define IXGBE_KRM_LINK_S1(P)		((P) ? 0x8200 : 0x4200)
>   #define IXGBE_KRM_LINK_CTRL_1(P)	((P) ? 0x820C : 0x420C)
>   #define IXGBE_KRM_AN_CNTL_1(P)		((P) ? 0x822C : 0x422C)
> +#define IXGBE_KRM_AN_CNTL_4(P)		((P) ? 0x8238 : 0x4238)
>   #define IXGBE_KRM_AN_CNTL_8(P)		((P) ? 0x8248 : 0x4248)
> +#define IXGBE_KRM_PCS_KX_AN(P)		((P) ? 0x9918 : 0x5918)
>   #define IXGBE_KRM_SGMII_CTRL(P)		((P) ? 0x82A0 : 0x42A0)
>   #define IXGBE_KRM_LP_BASE_PAGE_HIGH(P)	((P) ? 0x836C : 0x436C)
>   #define IXGBE_KRM_DSP_TXFFE_STATE_4(P)	((P) ? 0x8634 : 0x4634)
> @@ -3715,6 +3717,7 @@ struct ixgbe_info {
>   #define IXGBE_KRM_PMD_FLX_MASK_ST20(P)	((P) ? 0x9054 : 0x5054)
>   #define IXGBE_KRM_TX_COEFF_CTRL_1(P)	((P) ? 0x9520 : 0x5520)
>   #define IXGBE_KRM_RX_ANA_CTL(P)		((P) ? 0x9A00 : 0x5A00)
> +#define IXGBE_KRM_FLX_TMRS_CTRL_ST31(P)	((P) ? 0x9180 : 0x5180)
>   
>   #define IXGBE_KRM_PMD_FLX_MASK_ST20_SFI_10G_DA		~(0x3 << 20)
>   #define IXGBE_KRM_PMD_FLX_MASK_ST20_SFI_10G_SR		BIT(20)
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
> index e4b50c7781ff..f48a422ae83f 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
> @@ -1725,6 +1725,56 @@ static s32 ixgbe_setup_sfi_x550a(struct ixgbe_hw *hw, ixgbe_link_speed *speed)
>   				IXGBE_KRM_PMD_FLX_MASK_ST20(hw->bus.lan_id),
>   				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
>   
> +	/* change mode enforcement rules to hybrid */
> +	status = mac->ops.read_iosf_sb_reg(hw,
> +				IXGBE_KRM_FLX_TMRS_CTRL_ST31(hw->bus.lan_id),
> +				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
> +	reg_val |= 0x0400;
> +
> +	status = mac->ops.write_iosf_sb_reg(hw,
> +				IXGBE_KRM_FLX_TMRS_CTRL_ST31(hw->bus.lan_id),
> +				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);

I don't see a need for all the status assignments, they're not being 
used before being overwritten.

Thanks,

Tony

> +	/* manually control the config */
> +	status = mac->ops.read_iosf_sb_reg(hw,
> +				IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
> +				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
> +	reg_val |= 0x20002240;
> +
> +	status = mac->ops.write_iosf_sb_reg(hw,
> +				IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
> +				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
> +
> +	/* move the AN base page values */
> +	status = mac->ops.read_iosf_sb_reg(hw,
> +				IXGBE_KRM_PCS_KX_AN(hw->bus.lan_id),
> +				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
> +	reg_val |= 0x1;
> +
> +	status = mac->ops.write_iosf_sb_reg(hw,
> +				IXGBE_KRM_PCS_KX_AN(hw->bus.lan_id),
> +				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
> +
> +	/* set the AN37 over CB mode */
> +	status = mac->ops.read_iosf_sb_reg(hw,
> +				IXGBE_KRM_AN_CNTL_4(hw->bus.lan_id),
> +				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
> +	reg_val |= 0x20000000;
> +
> +	status = mac->ops.write_iosf_sb_reg(hw,
> +				IXGBE_KRM_AN_CNTL_4(hw->bus.lan_id),
> +				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
> +
> +	/* restart AN manually */
> +	status = mac->ops.read_iosf_sb_reg(hw,
> +				IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
> +				IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
> +	reg_val |= IXGBE_KRM_LINK_CTRL_1_TETH_AN_RESTART;
> +
> +	status = mac->ops.write_iosf_sb_reg(hw,
> +				IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
> +				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
> +
>   	/* Toggle port SW reset by AN reset. */
>   	status = ixgbe_restart_an_internal_phy_x550em(hw);
>   

[Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Skajewski, PiotrX]

Hi Jeff,

This patch has been tested by validation team and appears to be working correctly.
ACK, but first take a stance on the comment form Tony

Piotr
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20220510/519e1000/attachment-0001.html>

[Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Piotr Skajewski]

Hi Jeff,

This patch has been tested by validation team and appears to be working correctly.
ACK, but first take a stance on the comment form Tony

Piotr

[Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Tony Nguyen]

On 3/18/2022 4:47 PM, Tony Nguyen wrote:
> 
> On 3/16/2022 12:27 PM, Jeff Daly wrote:
>> Some (Juniper MX5) SFP link partners exhibit a disinclination to
>> autonegotiate with X550 configured in SFI mode.? This patch enables
>> a manual AN-37 restart to work around the problem.
> 
> Hi Jeff,
> 
> I talked to the ixgbe team about this and we need a bit more time to 
> look this over. Will keep you updated.

Hi Jeff,

Our developer is having some issues responding to this but this patch 
look ok. However, can you please address the unneeded status 
assignments. Also, replace the magic numbers for the register writes 
with meaningful define names.

Thanks,
Tony

> 
>> Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
>> ---
>> ? drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |? 3 ++
>> ? drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 50 +++++++++++++++++++
>> ? 2 files changed, 53 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h 
>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> index 2647937f7f4d..dc8a259fda5f 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> @@ -3705,7 +3705,9 @@ struct ixgbe_info {
>> ? #define IXGBE_KRM_LINK_S1(P)??????? ((P) ? 0x8200 : 0x4200)
>> ? #define IXGBE_KRM_LINK_CTRL_1(P)??? ((P) ? 0x820C : 0x420C)
>> ? #define IXGBE_KRM_AN_CNTL_1(P)??????? ((P) ? 0x822C : 0x422C)
>> +#define IXGBE_KRM_AN_CNTL_4(P)??????? ((P) ? 0x8238 : 0x4238)
>> ? #define IXGBE_KRM_AN_CNTL_8(P)??????? ((P) ? 0x8248 : 0x4248)
>> +#define IXGBE_KRM_PCS_KX_AN(P)??????? ((P) ? 0x9918 : 0x5918)
>> ? #define IXGBE_KRM_SGMII_CTRL(P)??????? ((P) ? 0x82A0 : 0x42A0)
>> ? #define IXGBE_KRM_LP_BASE_PAGE_HIGH(P)??? ((P) ? 0x836C : 0x436C)
>> ? #define IXGBE_KRM_DSP_TXFFE_STATE_4(P)??? ((P) ? 0x8634 : 0x4634)
>> @@ -3715,6 +3717,7 @@ struct ixgbe_info {
>> ? #define IXGBE_KRM_PMD_FLX_MASK_ST20(P)??? ((P) ? 0x9054 : 0x5054)
>> ? #define IXGBE_KRM_TX_COEFF_CTRL_1(P)??? ((P) ? 0x9520 : 0x5520)
>> ? #define IXGBE_KRM_RX_ANA_CTL(P)??????? ((P) ? 0x9A00 : 0x5A00)
>> +#define IXGBE_KRM_FLX_TMRS_CTRL_ST31(P)??? ((P) ? 0x9180 : 0x5180)
>> ? #define IXGBE_KRM_PMD_FLX_MASK_ST20_SFI_10G_DA??????? ~(0x3 << 20)
>> ? #define IXGBE_KRM_PMD_FLX_MASK_ST20_SFI_10G_SR??????? BIT(20)
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c 
>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
>> index e4b50c7781ff..f48a422ae83f 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
>> @@ -1725,6 +1725,56 @@ static s32 ixgbe_setup_sfi_x550a(struct 
>> ixgbe_hw *hw, ixgbe_link_speed *speed)
>> ????????????????? IXGBE_KRM_PMD_FLX_MASK_ST20(hw->bus.lan_id),
>> ????????????????? IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
>> +??? /* change mode enforcement rules to hybrid */
>> +??? status = mac->ops.read_iosf_sb_reg(hw,
>> +??????????????? IXGBE_KRM_FLX_TMRS_CTRL_ST31(hw->bus.lan_id),
>> +??????????????? IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
>> +??? reg_val |= 0x0400;
>> +
>> +??? status = mac->ops.write_iosf_sb_reg(hw,
>> +??????????????? IXGBE_KRM_FLX_TMRS_CTRL_ST31(hw->bus.lan_id),
>> +??????????????? IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
> 
> I don't see a need for all the status assignments, they're not being 
> used before being overwritten.
> 
> Thanks,
> 
> Tony
> 
>> +??? /* manually control the config */
>> +??? status = mac->ops.read_iosf_sb_reg(hw,
>> +??????????????? IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
>> +??????????????? IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
>> +??? reg_val |= 0x20002240;
>> +
>> +??? status = mac->ops.write_iosf_sb_reg(hw,
>> +??????????????? IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
>> +??????????????? IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
>> +
>> +??? /* move the AN base page values */
>> +??? status = mac->ops.read_iosf_sb_reg(hw,
>> +??????????????? IXGBE_KRM_PCS_KX_AN(hw->bus.lan_id),
>> +??????????????? IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
>> +??? reg_val |= 0x1;
>> +
>> +??? status = mac->ops.write_iosf_sb_reg(hw,
>> +??????????????? IXGBE_KRM_PCS_KX_AN(hw->bus.lan_id),
>> +??????????????? IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
>> +
>> +??? /* set the AN37 over CB mode */
>> +??? status = mac->ops.read_iosf_sb_reg(hw,
>> +??????????????? IXGBE_KRM_AN_CNTL_4(hw->bus.lan_id),
>> +??????????????? IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
>> +??? reg_val |= 0x20000000;
>> +
>> +??? status = mac->ops.write_iosf_sb_reg(hw,
>> +??????????????? IXGBE_KRM_AN_CNTL_4(hw->bus.lan_id),
>> +??????????????? IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
>> +
>> +??? /* restart AN manually */
>> +??? status = mac->ops.read_iosf_sb_reg(hw,
>> +??????????????? IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
>> +??????????????? IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
>> +??? reg_val |= IXGBE_KRM_LINK_CTRL_1_TETH_AN_RESTART;
>> +
>> +??? status = mac->ops.write_iosf_sb_reg(hw,
>> +??????????????? IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
>> +??????????????? IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
>> +
>> ????? /* Toggle port SW reset by AN reset. */
>> ????? status = ixgbe_restart_an_internal_phy_x550em(hw);
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Jeff Daly]

I can't replace the hardcoded values with meaningful assignments because I don't know what the bit names are.  This was originally a patch that Intel worked on for Silicom.  I suspect they are all DFT bits and as such are probably not going to be disclosed.

> -----Original Message-----
> From: Tony Nguyen <anthony.l.nguyen@intel.com>
> Sent: Thursday, May 12, 2022 1:09 PM
> To: Jeff Daly <jeffd@silicom-usa.com>; intel-wired-lan@lists.osuosl.org;
> Skajewski, PiotrX <piotrx.skajewski@intel.com>
> Cc: Jakub Kicinski <kuba@kernel.org>; open list:NETWORKING DRIVERS
> <netdev@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>; David S.
> Miller <davem@davemloft.net>
> Subject: Re: [Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link
> partners for X550 SFI
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments.
> 
> 
> On 3/18/2022 4:47 PM, Tony Nguyen wrote:
> >
> > On 3/16/2022 12:27 PM, Jeff Daly wrote:
> >> Some (Juniper MX5) SFP link partners exhibit a disinclination to
> >> autonegotiate with X550 configured in SFI mode.  This patch enables a
> >> manual AN-37 restart to work around the problem.
> >
> > Hi Jeff,
> >
> > I talked to the ixgbe team about this and we need a bit more time to
> > look this over. Will keep you updated.
> 
> Hi Jeff,
> 
> Our developer is having some issues responding to this but this patch look ok.
> However, can you please address the unneeded status assignments. Also,
> replace the magic numbers for the register writes with meaningful define
> names.
> 
> Thanks,
> Tony
> 
> >
> >> Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
> >> ---
> >>   drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |  3 ++
> >>   drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 50 +++++++++++++++++++
> >>   2 files changed, 53 insertions(+)
> >>
> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> >> b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> >> index 2647937f7f4d..dc8a259fda5f 100644
> >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> >> @@ -3705,7 +3705,9 @@ struct ixgbe_info {
> >>   #define IXGBE_KRM_LINK_S1(P)        ((P) ? 0x8200 : 0x4200)
> >>   #define IXGBE_KRM_LINK_CTRL_1(P)    ((P) ? 0x820C : 0x420C)
> >>   #define IXGBE_KRM_AN_CNTL_1(P)        ((P) ? 0x822C : 0x422C)
> >> +#define IXGBE_KRM_AN_CNTL_4(P)        ((P) ? 0x8238 : 0x4238)
> >>   #define IXGBE_KRM_AN_CNTL_8(P)        ((P) ? 0x8248 : 0x4248)
> >> +#define IXGBE_KRM_PCS_KX_AN(P)        ((P) ? 0x9918 : 0x5918)
> >>   #define IXGBE_KRM_SGMII_CTRL(P)        ((P) ? 0x82A0 : 0x42A0)
> >>   #define IXGBE_KRM_LP_BASE_PAGE_HIGH(P)    ((P) ? 0x836C : 0x436C)
> >>   #define IXGBE_KRM_DSP_TXFFE_STATE_4(P)    ((P) ? 0x8634 : 0x4634)
> >> @@ -3715,6 +3717,7 @@ struct ixgbe_info {
> >>   #define IXGBE_KRM_PMD_FLX_MASK_ST20(P)    ((P) ? 0x9054 : 0x5054)
> >>   #define IXGBE_KRM_TX_COEFF_CTRL_1(P)    ((P) ? 0x9520 : 0x5520)
> >>   #define IXGBE_KRM_RX_ANA_CTL(P)        ((P) ? 0x9A00 : 0x5A00)
> >> +#define IXGBE_KRM_FLX_TMRS_CTRL_ST31(P)    ((P) ? 0x9180 : 0x5180)
> >>   #define IXGBE_KRM_PMD_FLX_MASK_ST20_SFI_10G_DA        ~(0x3 << 20)
> >>   #define IXGBE_KRM_PMD_FLX_MASK_ST20_SFI_10G_SR        BIT(20)
> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
> >> b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
> >> index e4b50c7781ff..f48a422ae83f 100644
> >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
> >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
> >> @@ -1725,6 +1725,56 @@ static s32 ixgbe_setup_sfi_x550a(struct
> >> ixgbe_hw *hw, ixgbe_link_speed *speed)
> >>                   IXGBE_KRM_PMD_FLX_MASK_ST20(hw->bus.lan_id),
> >>                   IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
> >> +    /* change mode enforcement rules to hybrid */
> >> +    status = mac->ops.read_iosf_sb_reg(hw,
> >> +                IXGBE_KRM_FLX_TMRS_CTRL_ST31(hw->bus.lan_id),
> >> +                IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
> >> +    reg_val |= 0x0400;
> >> +
> >> +    status = mac->ops.write_iosf_sb_reg(hw,
> >> +                IXGBE_KRM_FLX_TMRS_CTRL_ST31(hw->bus.lan_id),
> >> +                IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
> >
> > I don't see a need for all the status assignments, they're not being
> > used before being overwritten.
> >
> > Thanks,
> >
> > Tony
> >
> >> +    /* manually control the config */
> >> +    status = mac->ops.read_iosf_sb_reg(hw,
> >> +                IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
> >> +                IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
> >> +    reg_val |= 0x20002240;
> >> +
> >> +    status = mac->ops.write_iosf_sb_reg(hw,
> >> +                IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
> >> +                IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
> >> +
> >> +    /* move the AN base page values */
> >> +    status = mac->ops.read_iosf_sb_reg(hw,
> >> +                IXGBE_KRM_PCS_KX_AN(hw->bus.lan_id),
> >> +                IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
> >> +    reg_val |= 0x1;
> >> +
> >> +    status = mac->ops.write_iosf_sb_reg(hw,
> >> +                IXGBE_KRM_PCS_KX_AN(hw->bus.lan_id),
> >> +                IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
> >> +
> >> +    /* set the AN37 over CB mode */
> >> +    status = mac->ops.read_iosf_sb_reg(hw,
> >> +                IXGBE_KRM_AN_CNTL_4(hw->bus.lan_id),
> >> +                IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
> >> +    reg_val |= 0x20000000;
> >> +
> >> +    status = mac->ops.write_iosf_sb_reg(hw,
> >> +                IXGBE_KRM_AN_CNTL_4(hw->bus.lan_id),
> >> +                IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
> >> +
> >> +    /* restart AN manually */
> >> +    status = mac->ops.read_iosf_sb_reg(hw,
> >> +                IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
> >> +                IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
> >> +    reg_val |= IXGBE_KRM_LINK_CTRL_1_TETH_AN_RESTART;
> >> +
> >> +    status = mac->ops.write_iosf_sb_reg(hw,
> >> +                IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
> >> +                IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
> >> +
> >>       /* Toggle port SW reset by AN reset. */
> >>       status = ixgbe_restart_an_internal_phy_x550em(hw);
> > _______________________________________________
> > Intel-wired-lan mailing list
> > Intel-wired-lan@osuosl.org
> > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Tony Nguyen]

On 7/19/2022 6:30 AM, Jeff Daly wrote:
> I can't replace the hardcoded values with meaningful assignments because I don't know what the bit names are.  This was originally a patch that Intel worked on for Silicom.  I suspect they are all DFT bits and as such are probably not going to be disclosed.

Could you re-send the patch? It's many pages back now and, likely, 
easier if you could re-send it.

Thanks,
Tony
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Jeff Daly]

Resubmit commit 565736048bd5 ("ixgbe: Manual AN-37 for troublesome link
partners for X550 SFI")

Some (Juniper MX5) SFP link partners exhibit a disinclination to
autonegotiate with X550 configured in SFI mode.  This patch enables
a manual AN-37 restart to work around the problem.

Resubmitted patch includes a module parameter (default disabled) to
isolate changes.

Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  8 +++
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |  4 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 52 +++++++++++++++++++
 3 files changed, 64 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 8b8404d8c946..ef77df0f94a6 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -157,6 +157,11 @@ module_param(allow_unsupported_sfp, bool, 0444);
 MODULE_PARM_DESC(allow_unsupported_sfp,
 		 "Allow unsupported and untested SFP+ modules on 82599-based adapters");
 
+static bool manual_an37_for_sfi;
+module_param(manual_an37_for_sfi, bool, 0444);
+MODULE_PARM_DESC(manual_an37_for_sfi,
+		 "Manual AN-37 for troublesome link partners for X550 SFI");
+
 #define DEFAULT_MSG_ENABLE (NETIF_MSG_DRV|NETIF_MSG_PROBE|NETIF_MSG_LINK)
 static int debug = -1;
 module_param(debug, int, 0);
@@ -10977,6 +10982,9 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (allow_unsupported_sfp)
 		hw->allow_unsupported_sfp = allow_unsupported_sfp;
 
+	if (manual_an37_for_sfi)
+		hw->manual_an37_for_sfi = manual_an37_for_sfi;
+
 	/* reset_hw fills in the perm_addr as well */
 	hw->phy.reset_if_overtemp = true;
 	err = hw->mac.ops.reset_hw(hw);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 346e3d9114a8..288bb2be3c23 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -3654,6 +3654,7 @@ struct ixgbe_hw {
 	bool				allow_unsupported_sfp;
 	bool				wol_enabled;
 	bool				need_crosstalk_fix;
+	bool				manual_an37_for_sfi;
 };
 
 struct ixgbe_info {
@@ -3675,7 +3676,9 @@ struct ixgbe_info {
 #define IXGBE_KRM_LINK_S1(P)		((P) ? 0x8200 : 0x4200)
 #define IXGBE_KRM_LINK_CTRL_1(P)	((P) ? 0x820C : 0x420C)
 #define IXGBE_KRM_AN_CNTL_1(P)		((P) ? 0x822C : 0x422C)
+#define IXGBE_KRM_AN_CNTL_4(P)		((P) ? 0x8238 : 0x4238)
 #define IXGBE_KRM_AN_CNTL_8(P)		((P) ? 0x8248 : 0x4248)
+#define IXGBE_KRM_PCS_KX_AN(P)		((P) ? 0x9918 : 0x5918)
 #define IXGBE_KRM_SGMII_CTRL(P)		((P) ? 0x82A0 : 0x42A0)
 #define IXGBE_KRM_LP_BASE_PAGE_HIGH(P)	((P) ? 0x836C : 0x436C)
 #define IXGBE_KRM_DSP_TXFFE_STATE_4(P)	((P) ? 0x8634 : 0x4634)
@@ -3685,6 +3688,7 @@ struct ixgbe_info {
 #define IXGBE_KRM_PMD_FLX_MASK_ST20(P)	((P) ? 0x9054 : 0x5054)
 #define IXGBE_KRM_TX_COEFF_CTRL_1(P)	((P) ? 0x9520 : 0x5520)
 #define IXGBE_KRM_RX_ANA_CTL(P)		((P) ? 0x9A00 : 0x5A00)
+#define IXGBE_KRM_FLX_TMRS_CTRL_ST31(P)	((P) ? 0x9180 : 0x5180)
 
 #define IXGBE_KRM_PMD_FLX_MASK_ST20_SFI_10G_DA		~(0x3 << 20)
 #define IXGBE_KRM_PMD_FLX_MASK_ST20_SFI_10G_SR		BIT(20)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
index a5f644934445..e3117ccf092c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
@@ -1726,6 +1726,58 @@ static int ixgbe_setup_sfi_x550a(struct ixgbe_hw *hw, ixgbe_link_speed *speed)
 				IXGBE_KRM_PMD_FLX_MASK_ST20(hw->bus.lan_id),
 				IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
 
+	if (hw->manual_an37_for_sfi) {
+		/* change mode enforcement rules to hybrid */
+		(void)mac->ops.read_iosf_sb_reg(hw,
+			IXGBE_KRM_FLX_TMRS_CTRL_ST31(hw->bus.lan_id),
+			IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+			reg_val |= 0x0400;
+
+		(void)mac->ops.write_iosf_sb_reg(hw,
+			IXGBE_KRM_FLX_TMRS_CTRL_ST31(hw->bus.lan_id),
+			IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+
+		/* manually control the config */
+		(void)mac->ops.read_iosf_sb_reg(hw,
+			IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
+			IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+		reg_val |= 0x20002240;
+
+		(void)mac->ops.write_iosf_sb_reg(hw,
+			IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
+			IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+
+		/* move the AN base page values */
+		(void)mac->ops.read_iosf_sb_reg(hw,
+			IXGBE_KRM_PCS_KX_AN(hw->bus.lan_id),
+			IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+		reg_val |= 0x1;
+
+		(void)mac->ops.write_iosf_sb_reg(hw,
+			IXGBE_KRM_PCS_KX_AN(hw->bus.lan_id),
+			IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+
+		/* set the AN37 over CB mode */
+		(void)mac->ops.read_iosf_sb_reg(hw,
+			IXGBE_KRM_AN_CNTL_4(hw->bus.lan_id),
+			IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+		reg_val |= 0x20000000;
+
+		(void)mac->ops.write_iosf_sb_reg(hw,
+			IXGBE_KRM_AN_CNTL_4(hw->bus.lan_id),
+			IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+
+		/* restart AN manually */
+		(void)mac->ops.read_iosf_sb_reg(hw,
+			IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
+			IXGBE_SB_IOSF_TARGET_KR_PHY, &reg_val);
+		reg_val |= IXGBE_KRM_LINK_CTRL_1_TETH_AN_RESTART;
+
+		(void)mac->ops.write_iosf_sb_reg(hw,
+			IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id),
+			IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val);
+	}
+
 	/* Toggle port SW reset by AN reset. */
 	status = ixgbe_restart_an_internal_phy_x550em(hw);
 
-- 
2.25.1

Re: [Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Andrew Lunn]

On Fri, Sep 06, 2024 at 06:41:45AM -0400, Jeff Daly wrote:
> Resubmit commit 565736048bd5 ("ixgbe: Manual AN-37 for troublesome link
> partners for X550 SFI")
> 
> Some (Juniper MX5) SFP link partners exhibit a disinclination to
> autonegotiate with X550 configured in SFI mode.  This patch enables
> a manual AN-37 restart to work around the problem.
> 
> Resubmitted patch includes a module parameter (default disabled) to
> isolate changes.

Module parameters are not liked in networking code. They are very user
unfriendly, and poorly documented.

Why do you need it? Is this change risky?

	Andrew

Re: [Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Jeff Daly]

 
> On Fri, Sep 06, 2024 at 06:41:45AM -0400, Jeff Daly wrote:
> > Resubmit commit 565736048bd5 ("ixgbe: Manual AN-37 for troublesome
> > link partners for X550 SFI")
> >
> > Some (Juniper MX5) SFP link partners exhibit a disinclination to
> > autonegotiate with X550 configured in SFI mode.  This patch enables a
> > manual AN-37 restart to work around the problem.
> >
> > Resubmitted patch includes a module parameter (default disabled) to
> > isolate changes.
> 
> Module parameters are not liked in networking code. They are very user
> unfriendly, and poorly documented.

Completely understood, which is why the original patch didn't include this.

> 
> Why do you need it? Is this change risky?
> 
>         Andrew

It turns out that the patch works fine for the specific issue it's trying to address (Juniper switch), 
but for (seemingly all) other devices it breaks the autonegotiation.  A few months back it was 
reported that there were issues with Cisco switches (which we didn't have to test with).  The
parameter was added in order to isolate the specific changes from affecting any other hardware. 

Re: [Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Andrew Lunn]

> It turns out that the patch works fine for the specific issue it's trying to address (Juniper switch), 
> but for (seemingly all) other devices it breaks the autonegotiation.

So it sounds like you need to figure out the nitty-gritty details of
what is going on with the Juniper switch. Once you understand that,
you might be able to find a workaround which works for all systems.

    Andrew

Re: [Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Jeff Daly]

> 
> > It turns out that the patch works fine for the specific issue it's
> > trying to address (Juniper switch), but for (seemingly all) other devices it breaks
> the autonegotiation.
> 
> So it sounds like you need to figure out the nitty-gritty details of what is going
> on with the Juniper switch. Once you understand that, you might be able to find
> a workaround which works for all systems.
> 
>     Andrew

This was originally worked out by Doug Boom at Intel.  It had to do with autonegotiation not being the part of the SFP optics when the Denverton X550 Si was released and was thus not POR for DNV.   The Juniper switches however won't exit their AN sequence unless an AN37 transaction is seen.  Other switch vendors recover gracefully when the right encoding is discovered, not using AN37 transactions, but not Juniper.  Since DNV doesn't do AN37 in SFP auto mode, there's an endless loop.   (Technically, the switches *could* be updated to new firmware that should have this capability, but apparently a logistical issue for at least one of our customers.)  

Going back through my emails, Doug did mention that it would possibly cause issues with other switches, but it wasn't anything we, or (until just recently) anyone else had observed.  A quote from Doug:  

"that AN37 fix pretty much only works with the Juniper switches, and can cause problems with other switches."

Initially I wanted to have this patch wrapped in a module parameter to avoid any potential issues, but that was shot down for the same reasons you initially commented about.  The unwrapped patch was accepted however.  It was a couple years before the potential other switch issue actually showed up and the patch was reverted.  Our customer still wants the code in the mainline kernel driver, maintaining a separate patch was not something that was acceptable to them, so we are.  This was all gone around with Intel a couple of years before and the solution for a non-updated Juniper MX5T switch is orthogonal to other switch support, thus the patch with the module parameter.

Re: [Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Andrew Lunn]

> This was originally worked out by Doug Boom at Intel.  It had to do
> with autonegotiation not being the part of the SFP optics when the
> Denverton X550 Si was released and was thus not POR for DNV.  The
> Juniper switches however won't exit their AN sequence unless an AN37
> transaction is seen.

I wounder what 802.3 says about this. I suspect the Juniper switch is
within the standard here, and the x550 is broken.

> Other switch vendors recover gracefully when the right encoding is
> discovered, not using AN37 transactions, but not Juniper.

We have seen similar things in the Linux core PHY handling, but mostly
around 2500BaseX MAC and PHY drivers. A lot of vendors implement what
they call over clocked SGMII, rather than 2500BaseX. But SGMII
signalling makes no sense when overclocked to 2.5GHz, so they just
disable it, leaving no signalling at all. Some 25000BaseX PHYs handle
this, they gracefully fall back to sensible defaults when they
discover they are connected to a broken MAC. Others need telling they
are connected to a broken MAC which does not perform signalling. But
it is easier for a MAC-PHY relationship, everything is on one board,
we know all the details, and can work around the issues.

> Since DNV doesn't do AN37 in SFP auto mode, there's an endless loop.
> (Technically, the switches *could* be updated to new firmware that
> should have this capability, but apparently a logistical issue for
> at least one of our customers.)

I would say that is the wrong solution, i don't think the switch is
doing anything wrong. But the devil is in the details, check 802.3.

> Going back through my emails, Doug did mention that it would possibly cause issues with other switches, but it wasn't anything we, or (until just recently) anyone else had observed.  A quote from Doug:  
> 
> "that AN37 fix pretty much only works with the Juniper switches, and can cause problems with other switches."

LOS from the from the SFP cage will tell you there is something on the
other end of the link. It is not a particularly reliable signal, since
it just means there is light. Is there any indication the link is not
usable? You could wait 10 seconds after LOS is inactive, and if there
is no usable link kick off the workaround. If after 10 seconds the
link is still not usable, turn the workaround off again. Flip flop
every 10 seconds.

Hopefully the initial 10 seconds delay means you won't upset switches
which currently work, and after 10 seconds, you gain a link to
switches that really do expect AN37.

	Andrew

Re: [Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome link partners for X550 SFI

[Jeff Daly]

> > This was originally worked out by Doug Boom at Intel.  It had to do
> > with autonegotiation not being the part of the SFP optics when the
> > Denverton X550 Si was released and was thus not POR for DNV.  The
> > Juniper switches however won't exit their AN sequence unless an AN37
> > transaction is seen.
> 
> I wounder what 802.3 says about this. I suspect the Juniper switch is within the
> standard here, and the x550 is broken.
>

Paraphrasing Doug: X550 on DNV lacks the AN37 to SFI like the other ixgbe devices so AN37 SFI doesn't work with DNV unless both sides force autonegotiation.
The Juniper switch won't exit AN until it sees an AN37 transaction, on stock DNV this won't occur.  There's no timeout with AN37 in the spec so Juniper 
implements the protocol according to spec, but this means with no AN37 coming from DNV it loops forever.  Other vendors (and probably Juniper too) saw the
hole in the spec and have a timeout and some recovery where it locks correctly (not via AN37), which make other switches work ok with DNV, but these still
have the endless loop. 
 
> > Other switch vendors recover gracefully when the right encoding is
> > discovered, not using AN37 transactions, but not Juniper.
> 

Snip

> LOS from the from the SFP cage will tell you there is something on the other end
> of the link. It is not a particularly reliable signal, since it just means there is light.
> Is there any indication the link is not usable? You could wait 10 seconds after
> LOS is inactive, and if there is no usable link kick off the workaround. If after 10
> seconds the link is still not usable, turn the workaround off again. Flip flop every
> 10 seconds.
> 
> Hopefully the initial 10 seconds delay means you won't upset switches which
> currently work, and after 10 seconds, you gain a link to switches that really do
> expect AN37.
> 
>         Andrew

I'll look into this.