Re: [PATCH V3 net 1/7] net: hns3: using user configure after hardware reset

[Przemek Kitszel <przemyslaw.kitszel@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>