Received-SPF: pass (google.com: domain of linux-kernel+bounces-171684-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <5eaeb35e-2740-4b47-ae42-75eadb6f0be5@intel.com>
Date: Tue, 7 May 2024 17:20:11 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH V3 net 1/7] net: hns3: using user configure after hardware
 reset
Content-Language: en-US
To: Jijie Shao <shaojijie@huawei.com>, <yisen.zhuang@huawei.com>,
	<salil.mehta@huawei.com>, <davem@davemloft.net>, <edumazet@google.com>,
	<kuba@kernel.org>, <pabeni@redhat.com>, <jiri@resnulli.us>,
	<horms@kernel.org>
CC: <shenjian15@huawei.com>, <wangjie125@huawei.com>,
	<liuyonglong@huawei.com>, <chenhao418@huawei.com>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
References: <20240507134224.2646246-1-shaojijie@huawei.com>
 <20240507134224.2646246-2-shaojijie@huawei.com>
From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
In-Reply-To: <20240507134224.2646246-2-shaojijie@huawei.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?YlowSUdzTm90SXBWQnNVM1dBWTY1amVROXFQSDZRTlJIbUJQMzdRTGRlTWMy?=
 =?utf-8?B?S2ZsMWQ5dFdzUHd3RSt3Mzd2MVplTGZPd3B5RUtMS2N1eE8zOU1wUmdHRThH?=
 =?utf-8?B?YmZtUmc2eURENndQS213ZU11R2ozZGJtbmkvQkFhc3llK3JlSUVrS3hzdTRu?=
 =?utf-8?B?cXNMTm92YjVlQVIrZkNKVDhpOGlic0V6MDFsenRsdEZiaHNpYUl4QVJwVE92?=
 =?utf-8?B?RGMyNGRSUVhWNkEwNlYyd1NmWUpXZVcvMmh5OWhZUlVCVlJvajJOenFNYUJl?=
 =?utf-8?B?MVJ4TE5qL0ZYUVBJdDFyZE5EelE1QnRqNWU1SjZKTnA2cVB4UlhXN2F1d0Qr?=
 =?utf-8?B?OVRjbGRsbURVTFdtZXcxZEVQT29MS3p4YVBmekJOWS9yVDR6ZktKNHFOQVJD?=
 =?utf-8?B?cHg2MnpRbUNtYmcvMDhNQzNrL1IzY1dNcjNHT2hzN3l2S1NMS3VkWlAyRTBC?=
 =?utf-8?B?Y3Y4TGxHc3hiRDJ3UEJZZnlFS2lPTDlmd0N0OHA5SEpaSnVPc3U3V3pQLzFZ?=
 =?utf-8?B?WG9ZTXpWV3A4eWFFeStreTRlVDNmNER4QjVMblNpcVNRbDE3aDZvcTJkSjZu?=
 =?utf-8?B?aEZuSk43ZVdBYmZNVGJQY3dNcFdtY2lFUEhxZG1TR0swZFFJOUlUS2hGVWwz?=
 =?utf-8?B?Tk5hTmJUUWJEN1JIalNkeWtzUmpGa2prL2lRK2JRSTIvejZsUEh0RVhQYXA4?=
 =?utf-8?B?YzZDa3F2dVJhRnJiMmVTYVRIODk1NVAxUnR0UUVHanFKMFp4Z1pMUnNuYnlE?=
 =?utf-8?B?S3hKWWR0ZkZpYmhxaVFIdldjNHZWTlNuZWpMRTM1MGFpWEVPaWtUUDAreGZS?=
 =?utf-8?B?K1FxNE5iNkFreVNpRTlSYzRzNEN1My9OU1JDS3B1amp2MzJ3SWNhZlprYWwr?=
 =?utf-8?B?UE43MDA2dStoQmRVNDJFT2c5VTdTNk1FcEFrRy9NNFZhY3ZSQlM5MWR0STIx?=
 =?utf-8?B?WWd5T0gzeWZkcFQ2YUoxZ0FWSG80Wlc1QlpnZ1pSbUJQRGYzbCsxT0k2dUxZ?=
 =?utf-8?B?NDBLTzdZRTRzU3VrZ2x0c25rL21BSUhMVk04NndIMkVORXNRRVZHbWJ2Ykw5?=
 =?utf-8?B?dTlDcXNNY2l3MUNaWmdEVy9ESDZKTjhrdDJuTU56QjkwY3NuUnBFQSsxaVdr?=
 =?utf-8?B?MWt1RUsrd3pJdExkaXNEN00xWmpBM0ZBYkMzSWpkR2M5akdrd1Z0eDFsWHh1?=
 =?utf-8?B?N1NrZXZUN0ZBK0czY0J4QW1GL0xMa1JSelFoY01Qdm95TC8rTTVPSktFSmNG?=
 =?utf-8?B?SXg0Z281Z01SV094QmFlZGtOL2p0WlFGMWJCR1gzT2s5UGdNaW5MZzVzNUo3?=
 =?utf-8?B?RUxLcTEwYnpMOHNkZjF5cUV2WTJORGZNTVFXN3pheS9nUTlCeVBVQVNCREpO?=
 =?utf-8?B?UnR4ZGMzQXhzR0JxMjluOU5BemhadkEvYkVUZ3BzMzBHNjI1WmVuY1FwM3hy?=
 =?utf-8?B?QnFoL29ZYVBud3dLTjB0cy8xaTB3V0hPU0l3eFRaWTJWektRUGlrT2MzeHBp?=
 =?utf-8?B?clhVNzRGb3ZTSnNVRXo0dklSRlRhVy9XTXlrSGYzL1R5NVZoYmhJZlhEeDVT?=
 =?utf-8?B?MUhlcERTZVo0c2gwLzFzUzFia3cxOXFIRk1UbVh6d1U2a0NKMit3ZC9DN1R0?=
 =?utf-8?B?TklTRldualJET0NjMStoaC94RFIzWWVSNUZMcUFkSFNuSmdRT3ViSGJ2WWxm?=
 =?utf-8?B?TlU4VFdkQUlVOWFiU0xGN1RxSHFyOExNYzhWa2hQcXdzNjVIOFE0SzJKWUgy?=
 =?utf-8?B?bGV0NHNLQ3FDWEE1b05lTStxRURnUXhFN0w5OTl4RDBBdzE3Zjl0WW0xUVJv?=
 =?utf-8?B?eUNoaWd3NDY0UmN5c2hBUnF1bzM1OUVEQVkzLzB5Tmovb2VhcHNoRjBiYjYz?=
 =?utf-8?B?am5HMlNkQmNtV0RRaEFMY212bUZxR210c1ZHS1BoRmhOQ3cwQTBKZWpXRXdT?=
 =?utf-8?B?SDV3d1VTODFvbEQyNDdqSVNocGNuaFh4ekZ4ay9mYlk1TzFvWGhVRi9nMFlo?=
 =?utf-8?B?Z2FZTlFXZzdGNVVZNnlHaUdaNEFST043LytPWHVOalpxTEIyeVJzYVFJY21w?=
 =?utf-8?B?bndxRlgwdDh0UjRnU0F4Z2J3RHlUVTMrd3RpQ25GMFMvc3RhQ2Y1MEdLeU5S?=
 =?utf-8?B?dXU0TmVsZkc5dnZKMUZKN1ZPcS9WVWUyREFRMEhScXJ3SFhHRmx5ZDZVaGNz?=
 =?utf-8?B?NEE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: f7e05bfe-33ca-4f09-7237-08dc6ea936a2
X-MS-Exchange-CrossTenant-AuthSource: MN6PR11MB8102.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 May 2024 15:20:22.6023
 (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: 89nCVFZs3XtnvRswh0e/DzoeG10i0NvO0bW5tjutr51nDhuSwbALlXQ0s3uVoVGz+n7gTbXv7SoSnPTXaYoXDUyJ6GK3e8v275rU40cNn9o=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB7177
X-OriginatorOrg: intel.com

On 5/7/24 15:42, Jijie Shao wrote:
> From: Peiyang Wang <wangpeiyang1@huawei.com>
> 
> When a reset occurring, it's supposed to recover user's configuration.
> Currently, the port info(speed, duplex and autoneg) is stored in hclge_mac
> and will be scheduled updated. Consider the case that reset was happened
> consecutively. During the first reset, the port info is configured with
> a temporary value cause the PHY is reset and looking for best link config.
> Second reset start and use pervious configuration which is not the user's.

nit: for future submissions please run your commit messages through
spellchecker

> The specific process is as follows:
> 
> +------+               +----+                +----+
> | USER |               | PF |                | HW |
> +---+--+               +-+--+                +-+--+
>      |  ethtool --reset   |                     |
>      +------------------->|    reset command    |
>      |  ethtool --reset   +-------------------->|
>      +------------------->|                     +---+
>      |                    +---+                 |   |
>      |                    |   |reset currently  |   | HW RESET
>      |                    |   |and wait to do   |   |
>      |                    |<--+                 |   |
>      |                    | send pervious cfg   |<--+
>      |                    | (1000M FULL AN_ON)  |
>      |                    +-------------------->|
>      |                    | read cfg(time task) |
>      |                    | (10M HALF AN_OFF)   +---+
>      |                    |<--------------------+   | cfg take effect
>      |                    |    reset command    |<--+
>      |                    +-------------------->|
>      |                    |                     +---+
>      |                    | send pervious cfg   |   | HW RESET
>      |                    | (10M HALF AN_OFF)   |<--+
>      |                    +-------------------->|
>      |                    | read cfg(time task) |
>      |                    |  (10M HALF AN_OFF)  +---+
>      |                    |<--------------------+   | cfg take effect
>      |                    |                     |   |
>      |                    | read cfg(time task) |<--+
>      |                    |  (10M HALF AN_OFF)  |
>      |                    |<--------------------+
>      |                    |                     |
>      v                    v                     v
> 
> To avoid aboved situation, this patch introduced req_speed, req_duplex,
> req_autoneg to store user's configuration and it only be used after
> hardware reset and to recover user's configuration
> 
> Fixes: f5f2b3e4dcc0 ("net: hns3: add support for imp-controlled PHYs")
> Signed-off-by: Peiyang Wang <wangpeiyang1@huawei.com>
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
>   .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c   | 15 +++++++++------
>   .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h   |  3 +++
>   2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> index ff6a2ed23ddb..8043f1795dc7 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> @@ -1537,6 +1537,9 @@ static int hclge_configure(struct hclge_dev *hdev)
>   			cfg.default_speed, ret);
>   		return ret;
>   	}
> +	hdev->hw.mac.req_speed = hdev->hw.mac.speed;
> +	hdev->hw.mac.req_autoneg = AUTONEG_ENABLE;
> +	hdev->hw.mac.req_duplex = DUPLEX_FULL;
>   
>   	hclge_parse_link_mode(hdev, cfg.speed_ability);
>   
> @@ -3342,9 +3345,9 @@ hclge_set_phy_link_ksettings(struct hnae3_handle *handle,
>   		return ret;
>   	}
>   
> -	hdev->hw.mac.autoneg = cmd->base.autoneg;
> -	hdev->hw.mac.speed = cmd->base.speed;
> -	hdev->hw.mac.duplex = cmd->base.duplex;
> +	hdev->hw.mac.req_autoneg = cmd->base.autoneg;
> +	hdev->hw.mac.req_speed = cmd->base.speed;
> +	hdev->hw.mac.req_duplex = cmd->base.duplex;
>   	linkmode_copy(hdev->hw.mac.advertising, cmd->link_modes.advertising);
>   
>   	return 0;
> @@ -3377,9 +3380,9 @@ static int hclge_tp_port_init(struct hclge_dev *hdev)
>   	if (!hnae3_dev_phy_imp_supported(hdev))
>   		return 0;
>   
> -	cmd.base.autoneg = hdev->hw.mac.autoneg;
> -	cmd.base.speed = hdev->hw.mac.speed;
> -	cmd.base.duplex = hdev->hw.mac.duplex;
> +	cmd.base.autoneg = hdev->hw.mac.req_autoneg;
> +	cmd.base.speed = hdev->hw.mac.req_speed;
> +	cmd.base.duplex = hdev->hw.mac.req_duplex;
>   	linkmode_copy(cmd.link_modes.advertising, hdev->hw.mac.advertising);
>   
>   	return hclge_set_phy_link_ksettings(&hdev->vport->nic, &cmd);
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
> index e821dd2f1528..e3c69be8256f 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
> @@ -279,11 +279,14 @@ struct hclge_mac {
>   	u8 media_type;	/* port media type, e.g. fibre/copper/backplane */
>   	u8 mac_addr[ETH_ALEN];
>   	u8 autoneg;
> +	u8 req_autoneg;
>   	u8 duplex;
> +	u8 req_duplex;
>   	u8 support_autoneg;
>   	u8 speed_type;	/* 0: sfp speed, 1: active speed */
>   	u8 lane_num;
>   	u32 speed;
> +	u32 req_speed;
>   	u32 max_speed;
>   	u32 speed_ability; /* speed ability supported by current media */
>   	u32 module_type; /* sub media type, e.g. kr/cr/sr/lr */

Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>