If duplex setting is half, mac and phy can not transmit and receive data at the same time, loopback test of serdes and phy will be failed, print message as follow: hns3 0000:35:00.2 eth2: self test start hns3 0000:35:00.2 eth2: link down hns3 0000:35:00.2 eth2: mode 2 recv fail, cnt=0x0, budget=0x1 hns3 0000:35:00.2 eth2: mode 3 recv fail, cnt=0x0, budget=0x1 hns3 0000:35:00.2 eth2: mode 4 recv fail, cnt=0x0, budget=0x1 hns3 0000:35:00.2 eth2: self test end The test result is FAIL The test extra info: External Loopback test 4 App Loopback test 0 Serdes serial Loopback test 3 Serdes parallel Loopback test 3 Phy Loopback test 3 To fix this problem, duplex setting of mac or phy will be set to full before serdes and phy starting loopback test, and restore duplex setting after test is end. Fixes: c39c4d98dc65 ("net: hns3: Add mac loopback selftest support in hns3 driver") Signed-off-by: Jijie Shao --- .../hisilicon/hns3/hns3pf/hclge_main.c | 26 +++++++++++++++++++ .../hisilicon/hns3/hns3pf/hclge_main.h | 1 + 2 files changed, 27 insertions(+) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c index f209a05e2033..78b10f2668e5 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c @@ -7844,8 +7844,15 @@ static int hclge_cfg_common_loopback(struct hclge_dev *hdev, bool en, static int hclge_set_common_loopback(struct hclge_dev *hdev, bool en, enum hnae3_loop loop_mode) { + u8 duplex; int ret; + duplex = en ? DUPLEX_FULL : hdev->hw.mac.duplex; + ret = hclge_cfg_mac_speed_dup_hw(hdev, hdev->hw.mac.speed, duplex, + hdev->hw.mac.lane_num); + if (ret) + return ret; + ret = hclge_cfg_common_loopback(hdev, en, loop_mode); if (ret) return ret; @@ -7871,6 +7878,12 @@ static int hclge_enable_phy_loopback(struct hclge_dev *hdev, return ret; } + hdev->hw.mac.duplex_last = phydev->duplex; + + ret = phy_set_bits(phydev, MII_BMCR, BMCR_FULLDPLX); + if (ret) + return ret; + ret = phy_resume(phydev); if (ret) return ret; @@ -7887,12 +7900,19 @@ static int hclge_disable_phy_loopback(struct hclge_dev *hdev, if (ret) return ret; + if (hdev->hw.mac.duplex_last == DUPLEX_HALF) { + ret = phy_clear_bits(phydev, MII_BMCR, BMCR_FULLDPLX); + if (ret) + return ret; + } + return phy_suspend(phydev); } static int hclge_set_phy_loopback(struct hclge_dev *hdev, bool en) { struct phy_device *phydev = hdev->hw.mac.phydev; + u8 duplex; int ret; if (!phydev) { @@ -7902,6 +7922,12 @@ static int hclge_set_phy_loopback(struct hclge_dev *hdev, bool en) return -ENOTSUPP; } + duplex = en ? DUPLEX_FULL : hdev->hw.mac.duplex; + ret = hclge_cfg_mac_speed_dup_hw(hdev, hdev->hw.mac.speed, duplex, + hdev->hw.mac.lane_num); + if (ret) + return ret; + if (en) ret = hclge_enable_phy_loopback(hdev, phydev); else diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h index 032b472d2368..36f2b06fa17d 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h @@ -283,6 +283,7 @@ struct hclge_mac { u8 autoneg; u8 req_autoneg; u8 duplex; + u8 duplex_last; u8 req_duplex; u8 support_autoneg; u8 speed_type; /* 0: sfp speed, 1: active speed */ -- 2.33.0 Currently, in hclge_mii_ioctl(), the operation to read the PHY register (SIOCGMIIREG) always returns 0. This patch changes the return type of hclge_read_phy_reg(), returning an error code when the function fails. Fixes: 024712f51e57 ("net: hns3: add ioctl support for imp-controlled PHYs") Signed-off-by: Jijie Shao --- drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 3 +-- drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c | 9 ++++++--- drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.h | 2 +- 3 files changed, 8 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 78b10f2668e5..85f07219b8be 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c @@ -9455,8 +9455,7 @@ static int hclge_mii_ioctl(struct hclge_dev *hdev, struct ifreq *ifr, int cmd) /* this command reads phy id and register at the same time */ fallthrough; case SIOCGMIIREG: - data->val_out = hclge_read_phy_reg(hdev, data->reg_num); - return 0; + return hclge_read_phy_reg(hdev, data->reg_num, &data->val_out); case SIOCSMIIREG: return hclge_write_phy_reg(hdev, data->reg_num, data->val_in); diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c index 96553109f44c..cf881108fa57 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c @@ -274,7 +274,7 @@ void hclge_mac_stop_phy(struct hclge_dev *hdev) phy_stop(phydev); } -u16 hclge_read_phy_reg(struct hclge_dev *hdev, u16 reg_addr) +int hclge_read_phy_reg(struct hclge_dev *hdev, u16 reg_addr, u16 *val) { struct hclge_phy_reg_cmd *req; struct hclge_desc desc; @@ -286,11 +286,14 @@ u16 hclge_read_phy_reg(struct hclge_dev *hdev, u16 reg_addr) req->reg_addr = cpu_to_le16(reg_addr); ret = hclge_cmd_send(&hdev->hw, &desc, 1); - if (ret) + if (ret) { dev_err(&hdev->pdev->dev, "failed to read phy reg, ret = %d.\n", ret); + return ret; + } - return le16_to_cpu(req->reg_val); + *val = le16_to_cpu(req->reg_val); + return 0; } int hclge_write_phy_reg(struct hclge_dev *hdev, u16 reg_addr, u16 val) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.h index 4200d0b6d931..21d434c82475 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.h +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.h @@ -13,7 +13,7 @@ int hclge_mac_connect_phy(struct hnae3_handle *handle); void hclge_mac_disconnect_phy(struct hnae3_handle *handle); void hclge_mac_start_phy(struct hclge_dev *hdev); void hclge_mac_stop_phy(struct hclge_dev *hdev); -u16 hclge_read_phy_reg(struct hclge_dev *hdev, u16 reg_addr); +int hclge_read_phy_reg(struct hclge_dev *hdev, u16 reg_addr, u16 *val); int hclge_write_phy_reg(struct hclge_dev *hdev, u16 reg_addr, u16 val); #endif -- 2.33.0 When a reset occurring, it's supposed to recover user's configuration. 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 previous configuration which is not the user's. Therefore, in this case, the network port may not link up. In this patch, saving user configure to req_xxx variable after successful configuration when using kernel PHY or firmware PHY. And then, use the req_xxx variable after hardware reset. Compared to the previous commit "net: hns3: using user configure after hardware reset", it adds handling for kernel PHY and FIBER. Fixes: 05eb60e9648c ("net: hns3: using user configure after hardware reset") Signed-off-by: Jijie Shao --- .../ethernet/hisilicon/hns3/hns3_ethtool.c | 26 ++--- .../hisilicon/hns3/hns3pf/hclge_main.c | 110 +++++++++++++----- .../hisilicon/hns3/hns3pf/hclge_main.h | 2 +- 3 files changed, 95 insertions(+), 43 deletions(-) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c index d5454e126c85..f7ac84840a2f 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c @@ -811,12 +811,11 @@ static int hns3_get_link_ksettings(struct net_device *netdev, } static int hns3_check_ksettings_param(const struct net_device *netdev, - const struct ethtool_link_ksettings *cmd) + const struct ethtool_link_ksettings *cmd, + u8 media_type) { struct hnae3_handle *handle = hns3_get_handle(netdev); const struct hnae3_ae_ops *ops = hns3_get_ops(handle); - u8 module_type = HNAE3_MODULE_TYPE_UNKNOWN; - u8 media_type = HNAE3_MEDIA_TYPE_UNKNOWN; u32 lane_num; u8 autoneg; u32 speed; @@ -836,9 +835,6 @@ static int hns3_check_ksettings_param(const struct net_device *netdev, return 0; } - if (ops->get_media_type) - ops->get_media_type(handle, &media_type, &module_type); - if (cmd->base.duplex == DUPLEX_HALF && media_type != HNAE3_MEDIA_TYPE_COPPER) { netdev_err(netdev, @@ -863,6 +859,8 @@ static int hns3_set_link_ksettings(struct net_device *netdev, struct hnae3_handle *handle = hns3_get_handle(netdev); struct hnae3_ae_dev *ae_dev = hns3_get_ae_dev(handle); const struct hnae3_ae_ops *ops = hns3_get_ops(handle); + u8 module_type = HNAE3_MODULE_TYPE_UNKNOWN; + u8 media_type = HNAE3_MEDIA_TYPE_UNKNOWN; int ret; /* Chip don't support this mode. */ @@ -878,22 +876,20 @@ static int hns3_set_link_ksettings(struct net_device *netdev, cmd->base.autoneg, cmd->base.speed, cmd->base.duplex, cmd->lanes); - /* Only support ksettings_set for netdev with phy attached for now */ - if (netdev->phydev) { - if (cmd->base.speed == SPEED_1000 && - cmd->base.autoneg == AUTONEG_DISABLE) - return -EINVAL; + if (!ops->get_media_type) + return -EOPNOTSUPP; + ops->get_media_type(handle, &media_type, &module_type); - return phy_ethtool_ksettings_set(netdev->phydev, cmd); - } else if (test_bit(HNAE3_DEV_SUPPORT_PHY_IMP_B, ae_dev->caps) && - ops->set_phy_link_ksettings) { + if (media_type == HNAE3_MEDIA_TYPE_COPPER) { + if (!ops->set_phy_link_ksettings) + return -EOPNOTSUPP; return ops->set_phy_link_ksettings(handle, cmd); } if (ae_dev->dev_version < HNAE3_DEVICE_VERSION_V2) return -EOPNOTSUPP; - ret = hns3_check_ksettings_param(netdev, cmd); + ret = hns3_check_ksettings_param(netdev, cmd, media_type); if (ret) return ret; diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c index 85f07219b8be..50150f1cd79c 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c @@ -1574,7 +1574,6 @@ static int hclge_configure(struct hclge_dev *hdev) 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); @@ -2684,6 +2683,7 @@ static int hclge_set_autoneg(struct hnae3_handle *handle, bool enable) { struct hclge_vport *vport = hclge_get_vport(handle); struct hclge_dev *hdev = vport->back; + int ret; if (!hdev->hw.mac.support_autoneg) { if (enable) { @@ -2695,7 +2695,10 @@ static int hclge_set_autoneg(struct hnae3_handle *handle, bool enable) } } - return hclge_set_autoneg_en(hdev, enable); + ret = hclge_set_autoneg_en(hdev, enable); + if (!ret) + hdev->hw.mac.req_autoneg = enable; + return ret; } static int hclge_get_autoneg(struct hnae3_handle *handle) @@ -2957,20 +2960,6 @@ static int hclge_mac_init(struct hclge_dev *hdev) if (!test_bit(HCLGE_STATE_RST_HANDLING, &hdev->state)) hdev->hw.mac.duplex = HCLGE_MAC_FULL; - if (hdev->hw.mac.support_autoneg) { - ret = hclge_set_autoneg_en(hdev, hdev->hw.mac.autoneg); - if (ret) - return ret; - } - - if (!hdev->hw.mac.autoneg) { - ret = hclge_cfg_mac_speed_dup_hw(hdev, hdev->hw.mac.req_speed, - hdev->hw.mac.req_duplex, - hdev->hw.mac.lane_num); - if (ret) - return ret; - } - mac->link = 0; if (mac->user_fec_mode & BIT(HNAE3_FEC_USER_DEF)) { @@ -3358,8 +3347,8 @@ static int hclge_get_phy_link_ksettings(struct hnae3_handle *handle, } static int -hclge_set_phy_link_ksettings(struct hnae3_handle *handle, - const struct ethtool_link_ksettings *cmd) +hclge_ethtool_ksettings_set(struct hnae3_handle *handle, + const struct ethtool_link_ksettings *cmd) { struct hclge_desc desc[HCLGE_PHY_LINK_SETTING_BD_NUM]; struct hclge_vport *vport = hclge_get_vport(handle); @@ -3369,12 +3358,6 @@ hclge_set_phy_link_ksettings(struct hnae3_handle *handle, u32 advertising; int ret; - if (cmd->base.autoneg == AUTONEG_DISABLE && - ((cmd->base.speed != SPEED_100 && cmd->base.speed != SPEED_10) || - (cmd->base.duplex != DUPLEX_HALF && - cmd->base.duplex != DUPLEX_FULL))) - return -EINVAL; - hclge_cmd_setup_basic_desc(&desc[0], HCLGE_OPC_PHY_LINK_KSETTING, false); desc[0].flag |= cpu_to_le16(HCLGE_COMM_CMD_FLAG_NEXT); @@ -3394,6 +3377,33 @@ hclge_set_phy_link_ksettings(struct hnae3_handle *handle, req1->master_slave_cfg = cmd->base.master_slave_cfg; ret = hclge_cmd_send(&hdev->hw, desc, HCLGE_PHY_LINK_SETTING_BD_NUM); + + if (!ret) + linkmode_copy(hdev->hw.mac.advertising, + cmd->link_modes.advertising); + + return ret; +} + +static int +hclge_set_phy_link_ksettings(struct hnae3_handle *handle, + const struct ethtool_link_ksettings *cmd) +{ + struct hclge_vport *vport = hclge_get_vport(handle); + struct hclge_dev *hdev = vport->back; + int ret; + + if (cmd->base.autoneg == AUTONEG_DISABLE && + ((cmd->base.speed != SPEED_100 && cmd->base.speed != SPEED_10) || + (cmd->base.duplex != DUPLEX_HALF && + cmd->base.duplex != DUPLEX_FULL))) + return -EINVAL; + + if (hnae3_dev_phy_imp_supported(hdev)) + ret = hclge_ethtool_ksettings_set(handle, cmd); + else + ret = phy_ethtool_ksettings_set(handle->netdev->phydev, cmd); + if (ret) { dev_err(&hdev->pdev->dev, "failed to set phy link ksettings, ret = %d.\n", ret); @@ -3401,9 +3411,10 @@ hclge_set_phy_link_ksettings(struct hnae3_handle *handle, } 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); + if (cmd->base.speed != SPEED_UNKNOWN) + hdev->hw.mac.req_speed = cmd->base.speed; + if (cmd->base.duplex != DUPLEX_UNKNOWN) + hdev->hw.mac.req_duplex = cmd->base.duplex; return 0; } @@ -11747,6 +11758,27 @@ static int hclge_set_wol(struct hnae3_handle *handle, return ret; } +static int hclge_set_autoneg_speed_dup(struct hclge_dev *hdev) +{ + int ret; + + if (hdev->hw.mac.support_autoneg) { + ret = hclge_set_autoneg_en(hdev, hdev->hw.mac.req_autoneg); + if (ret) + return ret; + } + + if (!hdev->hw.mac.req_autoneg) { + ret = hclge_cfg_mac_speed_dup_hw(hdev, hdev->hw.mac.req_speed, + hdev->hw.mac.req_duplex, + hdev->hw.mac.lane_num); + if (ret) + return ret; + } + + return 0; +} + static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev) { struct pci_dev *pdev = ae_dev->pdev; @@ -11908,6 +11940,23 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev) if (ret) goto err_ptp_uninit; + if (hdev->hw.mac.media_type == HNAE3_MEDIA_TYPE_COPPER) + hdev->hw.mac.req_autoneg = AUTONEG_ENABLE; + else + hdev->hw.mac.req_autoneg = hdev->hw.mac.autoneg; + + /* When lane_num is 0, the firmware will automatically + * select the appropriate lane_num based on the speed. + */ + hdev->hw.mac.lane_num = 0; + + ret = hclge_set_autoneg_speed_dup(hdev); + if (ret) { + dev_err(&pdev->dev, + "failed to set autoneg speed duplex, ret = %d\n", ret); + goto err_ptp_uninit; + } + INIT_KFIFO(hdev->mac_tnl_log); hclge_dcb_ops_set(hdev); @@ -12238,6 +12287,13 @@ static int hclge_reset_ae_dev(struct hnae3_ae_dev *ae_dev) return ret; } + ret = hclge_set_autoneg_speed_dup(hdev); + if (ret) { + dev_err(&pdev->dev, + "failed to set autoneg speed duplex, ret = %d\n", ret); + return ret; + } + ret = hclge_tp_port_init(hdev); if (ret) { dev_err(&pdev->dev, "failed to init tp port, ret = %d\n", diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h index 36f2b06fa17d..f650b07bc739 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h @@ -285,7 +285,7 @@ struct hclge_mac { u8 duplex; u8 duplex_last; u8 req_duplex; - u8 support_autoneg; + u8 support_autoneg; /* for non-copper port */ u8 speed_type; /* 0: sfp speed, 1: active speed */ u8 lane_num; u32 speed; -- 2.33.0