on 2026/6/18 23:34, Simon Horman wrote:

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
net: hns3: differentiate autoneg default values between copper and fiber

This commit addresses a link loss issue during driver initialization on
optical ports connected to forced-mode remote switches. It modifies
hclge_init_ae_dev to set autonegotiation based on media type. Copper ports
retain the AUTONEG_ENABLE default, while optical ports inherit the firmware
pre-configured autoneg setting.

quoted

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c

@@ -11916,6 +11916,9 @@ 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 = hdev->hw.mac.autoneg;
+

[Severity: High]
Does this apply the wrong speed when autonegotiation is disabled on fiber
ports?

During driver initialization, hclge_update_port_info() dynamically probes
the SFP module and updates mac->speed, but does not update mac->req_speed,
which retains its statically configured default capability.

When hdev->hw.mac.autoneg is 0 (forced-mode), req_autoneg becomes 0, and
then hclge_set_autoneg_speed_dup() uses req_speed:

drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c:hclge_set_autoneg_speed_dup() {
...
     if (!hdev->hw.mac.req_autoneg) {
         ret = hclge_cfg_mac_speed_dup_hw(hdev, hdev->hw.mac.req_speed,
...
}

Could this override the dynamically probed SFP speed with the static default
speed?

[Reject] This is initializing the path. mac->speed has been parsed in hclge_configure()
          and an initial value has been assigned to req_speed.

[Severity: High]
Does this unconditionally disable autonegotiation for V1 optical hardware?

Looking at hclge_update_port_info(), it only queries SFP information via
hclge_get_sfp_info() for V2 hardware. For V1 hardware, it falls back to
hclge_get_sfp_speed(), which leaves mac->autoneg uninitialized (0):

drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c:hclge_update_port_info() {
...
     if (hdev->ae_dev->dev_version >= HNAE3_DEVICE_VERSION_V2) {
         speed = mac->speed;
         ret = hclge_get_sfp_info(hdev, mac);
     } else {
         speed = HCLGE_MAC_SPEED_UNKNOWN;
         ret = hclge_get_sfp_speed(hdev, &speed);
     }
...
}

If mac->autoneg is left as 0, wouldn't req_autoneg unconditionally be set
to 0 here, overriding the speed and breaking connectivity for V1 devices?

[Reject] No, this process is the same as before the modification.
        Before the modification, mac->autoneg was not actively assigned a value (it was initialized to 0 by default).

Thanks,
Jijie Shao