Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <51308ed3-09c1-6bf2-190b-306abd42f3da@intel.com>
Date:   Thu, 12 May 2022 10:09:09 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.0
Subject: Re: [Intel-wired-lan] [PATCH] ixgbe: Manual AN-37 for troublesome
 link partners for X550 SFI
Content-Language: en-US
From:   Tony Nguyen <anthony.l.nguyen@intel.com>
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>
References: <20220316192710.9947-1-jeffd@silicom-usa.com>
 <56cd0dde-600d-1bb0-1555-e66de8c37236@intel.com>
In-Reply-To: <56cd0dde-600d-1bb0-1555-e66de8c37236@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cUJ1S2d1K3ZKd3VUTk55WTBYL25qSUo2aEU3S1hnV3ROTVlVa0ovN0F0Yi81?=
 =?utf-8?B?RC91RDM3MGZXRU00TkE4ZXpPaURzL1RQNldsTVZOeEhCaW5ubTkyMmhVRjMr?=
 =?utf-8?B?aENncUQ3bGJwZStnMm9paGtNUjBjQjJMalV1MnV3MFA3UTd0WVJUU0QybFR1?=
 =?utf-8?B?ZE9PWjQ3Y3pna3g3eExaWnRHam45RlJCd0o2OEdGNnc5anEvTzkwbXMyOGtI?=
 =?utf-8?B?ZUhKZGNIb0FmWTVHY2dLdmZTMUZvMUkrRk5zM09WZjNBa21BSVJkTnBCeEpO?=
 =?utf-8?B?b3pabk84cVloemRWWFdwU1piaUVOeTdLUzhoV0tXN0xzc2ticDlrL0Q4UkdP?=
 =?utf-8?B?bDh2b2ZlZXJYVlYvbDhYcnppMzQrR21FdzRvYzIxeWk2T0FJZXJPYTFxaVBz?=
 =?utf-8?B?K1NEWnVwb3l1a0tKNHNKMHhlRE9ZY3pXTkpVTXEySy85bjlrQldxS0pUekpj?=
 =?utf-8?B?WlpUcDNGazFZTS9ieEI1dUx0eU1CYzcvQS9Wd2tkNGtTRHdCUFM5dm5Tc21t?=
 =?utf-8?B?a3pzNjQyZ01IbXhHcFY0RHk4UnFMdU9Qby82VmJudUpXbHpTa3VQT29jeGJ2?=
 =?utf-8?B?ajArc1pOK2cwNlY5dW84REx0SmZwWEYzOUJ1eGpsVWxDMTlXRVpMaEVEYUZS?=
 =?utf-8?B?dHRYeTFtSTM2aVNvQkU1bzRud1JYclhOajBHeTViRXdCNWQ5anNjWGpoeC95?=
 =?utf-8?B?MHNpeWlvYmhIN0xaVmptSjJrRnp0RGJVdUJiTVFRY1ZQeW1zR3hBQnQvNFZq?=
 =?utf-8?B?Qkh0RFJMNEJFVmloTXd6cGxLeFA0NkhDVEdVZ1paRmJ1dDBPWXpia2JBL0g3?=
 =?utf-8?B?cDh1cWEwbFRPaFFOd090ajk3MTFVdXJuYXhVYkNoVlVTb0NTeE1ZUmFYV2VD?=
 =?utf-8?B?Z2YxMHFTQllBY0VnWkhBOXBYK2hFamlPNnBITmtTUC92NTRXczF3eThZRlZs?=
 =?utf-8?B?YmpGamlqM3VnbHJ4cVNmL0NyTWlneFIyL2p5eVdoMVhnT0MzdktkWkYyRksv?=
 =?utf-8?B?bzhWYkZ2ZzlMekRtdWo3V2h4WjJBWHdtbk9XR1ZPUko5Mlh3MzlpdDVvbTAx?=
 =?utf-8?B?aS9HbjNKRTdKOE95ZHZPTDZ4eDczVjVSNmgwaW1uVXM3Wm1Gd3prREt1eWJB?=
 =?utf-8?B?QkwrTTk1eml6Q3RoekxScThadEh2b2U4MUhYUW9ncjBVcGM5VmVnTFpnSWVS?=
 =?utf-8?B?cE9uUDdDSU1QMmllTXBaOU8rS1VLNXN6SGEwZWY4cXN6bFdOZ0N1bitpWlk3?=
 =?utf-8?B?eFhlWER1cy9iK0I2RW1QZHdVV0FXQVFDQkNqWFRlS1cxS05KK1NrYmdLVVdK?=
 =?utf-8?B?VjZMMnpsZmt2TVplVytzWTNNcFJrMDJDNTQyQ1RzUGlGYWZ6UmcrTFc5N1Qx?=
 =?utf-8?B?TTB1Wmp0eUIxaUVWRWZpMDFMNVlodFR1NldVUGhqWnZnUHo3VFEzKzU0N3pX?=
 =?utf-8?B?UVkvNDg2WjZmdXdzTlhXZWl1TjZKNEZvcnBpUWY4eTRwd2NzU2pnNytLV0Ur?=
 =?utf-8?B?TjJkS1BYL29ncjYvSmRjMjlxUkFHN2hNRWI5K2NOU3p4ZUZCZkR6bFdlM1l2?=
 =?utf-8?B?MzVoUCttK0M3eEVTSFIvcVQrZGQ4WWMyeVd3MGs1YVRGaG90WFcxMjZoTmJT?=
 =?utf-8?B?K1lpb0JxWTFaU2hhRnR3dGd3R3JoZHdUZTNPUVQ1VE9IVkYrR2FPam1nd1JR?=
 =?utf-8?B?L1lyVzk3QzBVb0NET2RCMUUzZmZtVkFib1llcmZsOG1IV3FTZE95aEpmT05P?=
 =?utf-8?B?T2pGbkxUd2xuME9DSjRqVlJCRWVsQm1wTUV2NDB0REJqS3NEYUZhdUVKZ2VB?=
 =?utf-8?B?WDlpMTNkYnNlSWg5TW1SaGdIMlpSSVNyZ01JZTB5d0p4SnRzbHpLZHM0NElk?=
 =?utf-8?B?M3JGOVJxVi9EY2JISTVlTy9zRksxV254TXlLRHFHbXpRczlsV0lIck50S3Zj?=
 =?utf-8?B?WHluVkxFdWI2QVk4MmR2VW1SNDdleXdFQ1lFOHJVRWpMeGJzYVRMaU52SjQy?=
 =?utf-8?B?WS81QU13V1BnSnNHUzgxaURZMHdkV0oxRWZkWGczK2JUYXg4dHhHRndBUzRx?=
 =?utf-8?B?UFZnTE5KRHhHTHI4N1loUHRZWU9QWU4xNk9NcUp2Nk8vZTg4M0RpRzhoa1Ax?=
 =?utf-8?B?RFV1Yk1oOGVNM0dQdFc3SDR3MDE1SE56SzRYeWoraXJHQk1Pd2NTTVZEcm92?=
 =?utf-8?B?QTYwSGp1QXAwN2ZuZzV4aVlua3pSYThKOEdBWE9IUGhUaE8zY0dXemU1bXl6?=
 =?utf-8?B?SkR1d0ZOV1I1Z1dnc0pMM0RPYzR4N2U4ZCttdW5MQzB4cmZlNThNUjVhT1ZT?=
 =?utf-8?B?dXd5NHkwUWlMblQ1b0hsdmMxUHgxd0xkbEJXTm50V3hlRHl1VkRiUjlsMDhO?=
 =?utf-8?Q?eqKQRD8NXUVd0ako=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 19495779-6842-43d4-5c32-08da343a2288
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3224.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 May 2022 17:09:12.0174
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: kWnYFUG4xn6OcDjz6unlsri7mppN7VHXrfXDgWn3oIcoi6lMuEgzHeoSvK9/DtwB0ZRt66SfozcY35iaRaO9dzU7dYIBhVdQ0u6+VyJ7Ses=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4602
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_HI,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


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