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

Przemek Kitszel
2024-05-07T15:20:27Z

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>