[PATCH V2 net] net: hns3: fix speed configuration residue after driver reload

[PATCH V2 net] net: hns3: fix speed configuration residue after driver reload

[Jijie Shao]

After setting a 100G optical port to 40G via ethtool and reloading
the driver, the port remains at 40G instead of reverting to the
firmware default speed of 100G.

In hclge_init_ae_dev(), hclge_update_port_info() reads mac.speed
from hardware, which reflects the last user configuration (e.g.
ethtool changes), not the firmware default. When req_speed is
overwritten with this value, hclge_set_autoneg_speed_dup() re-applies
the stale speed instead of the firmware default on non-copper media.

Fix by removing the req_speed overwrite in hclge_init_ae_dev(),
keeping only the req_autoneg synchronization. This ensures req_speed
retains the firmware default value set during hclge_configure().

Fixes: d9d349c4e8a0 ("net: hns3: differentiate autoneg default values between copper and fiber")
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
v2:
 - Discard v1 and resend with the correct "net" prefix in the subject. 
   Apologies for the noise. No code changes.
---
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index fc8587c80813..164c3ecf195c 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -9498,12 +9498,8 @@ 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) {
+	if (hdev->hw.mac.media_type != HNAE3_MEDIA_TYPE_COPPER)
 		hdev->hw.mac.req_autoneg = hdev->hw.mac.autoneg;
-		if (hdev->hw.mac.autoneg == AUTONEG_DISABLE &&
-		    hdev->hw.mac.speed != SPEED_UNKNOWN)
-			hdev->hw.mac.req_speed = hdev->hw.mac.speed;
-	}
 
 	ret = hclge_set_autoneg_speed_dup(hdev);
 	if (ret) {

base-commit: 235acadd310533ba386ae61ad155b72bee381559
-- 
2.33.0

Re: [PATCH V2 net] net: hns3: fix speed configuration residue after driver reload

[Paolo Abeni]

From: AI Reviewer <ai@example.com>

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://netdev-ai.bots.linux.dev/sashiko/
---
net: hns3: fix speed configuration residue after driver reload

This change drops the req_speed overwrite in hclge_init_ae_dev() so
req_speed keeps the firmware default set during hclge_configure()
after a driver reload, and only req_autoneg continues to be
synchronized from mac.autoneg on non-copper media.

> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> index fc8587c80813..164c3ecf195c 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> @@ -9498,12 +9498,8 @@ 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) {
> +	if (hdev->hw.mac.media_type != HNAE3_MEDIA_TYPE_COPPER)
>  		hdev->hw.mac.req_autoneg = hdev->hw.mac.autoneg;
> -		if (hdev->hw.mac.autoneg == AUTONEG_DISABLE &&
> -		    hdev->hw.mac.speed != SPEED_UNKNOWN)
> -			hdev->hw.mac.req_speed = hdev->hw.mac.speed;
> -	}

[High]
Could removing this block reintroduce the scenario that d9d349c4e8a0
("net: hns3: differentiate autoneg default values between copper and
fiber") was written to handle?

The Fixes-tagged commit added this req_speed overwrite specifically for
optical ports where firmware presets autoneg=DISABLE at a specific
forced speed (a case its commit message describes as common in data
centers where the remote switch runs in forced mode).

There appear to be two distinct notions of "firmware default" involved:

  - cfg.default_speed read from flash static parameters via
    hclge_get_cfg() and stored into req_speed by hclge_configure()

  - the live firmware/SFP state read via hclge_get_sfp_info() into
    mac.autoneg and mac.speed

On a fiber port where the flash default_speed is 100G but firmware has
preset the port to forced 25G to match a fixed-speed remote, after this
change:

    hdev->hw.mac.req_autoneg = hdev->hw.mac.autoneg;  /* = DISABLE */
    /* req_speed still holds flash default 100G */

hclge_set_autoneg_speed_dup() would then see !req_autoneg and call:

    hclge_cfg_mac_speed_dup_hw(hdev, hdev->hw.mac.req_speed=100G, ...);

forcing 100G against a 25G-only peer.

Is that scenario still handled, or is it expected not to occur in
practice?

[Medium]
The commit message reasons about mac.speed carrying over user ethtool
changes across a driver reload:

  "In hclge_init_ae_dev(), hclge_update_port_info() reads mac.speed
   from hardware, which reflects the last user configuration (e.g.
   ethtool changes), not the firmware default."

Does the same reasoning apply to mac.autoneg?

hclge_get_sfp_info() sets mac->autoneg from the live firmware state, so
a prior "ethtool -s ethX autoneg off speed 40000" on a 100G optical
port would leave firmware reporting autoneg=DISABLE after reload.

After this change, req_speed correctly reverts to the flash default
100G, but this line still runs:

    hdev->hw.mac.req_autoneg = hdev->hw.mac.autoneg;

so req_autoneg stays at AUTONEG_DISABLE rather than reverting to the
AUTONEG_ENABLE default that hclge_configure() would set for fiber.

Why is speed residue fixed while autoneg residue is preserved?

>  
>  	ret = hclge_set_autoneg_speed_dup(hdev);
>  	if (ret) {
-- 
This is an AI-generated review.

Re: [PATCH V2 net] net: hns3: fix speed configuration residue after driver reload

[Jijie Shao]

on 2026/7/21 19:05, Paolo Abeni wrote:
> From: AI Reviewer <ai@example.com>
>
> 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://netdev-ai.bots.linux.dev/sashiko/
> ---
> net: hns3: fix speed configuration residue after driver reload
>
> This change drops the req_speed overwrite in hclge_init_ae_dev() so
> req_speed keeps the firmware default set during hclge_configure()
> after a driver reload, and only req_autoneg continues to be
> synchronized from mac.autoneg on non-copper media.
>
>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
>> index fc8587c80813..164c3ecf195c 100644
>> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
>> @@ -9498,12 +9498,8 @@ 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) {
>> +	if (hdev->hw.mac.media_type != HNAE3_MEDIA_TYPE_COPPER)
>>   		hdev->hw.mac.req_autoneg = hdev->hw.mac.autoneg;
>> -		if (hdev->hw.mac.autoneg == AUTONEG_DISABLE &&
>> -		    hdev->hw.mac.speed != SPEED_UNKNOWN)
>> -			hdev->hw.mac.req_speed = hdev->hw.mac.speed;
>> -	}
> [High]
> Could removing this block reintroduce the scenario that d9d349c4e8a0
> ("net: hns3: differentiate autoneg default values between copper and
> fiber") was written to handle?
>
> The Fixes-tagged commit added this req_speed overwrite specifically for
> optical ports where firmware presets autoneg=DISABLE at a specific
> forced speed (a case its commit message describes as common in data
> centers where the remote switch runs in forced mode).
>
> There appear to be two distinct notions of "firmware default" involved:
>
>    - cfg.default_speed read from flash static parameters via
>      hclge_get_cfg() and stored into req_speed by hclge_configure()
>
>    - the live firmware/SFP state read via hclge_get_sfp_info() into
>      mac.autoneg and mac.speed
>
> On a fiber port where the flash default_speed is 100G but firmware has
> preset the port to forced 25G to match a fixed-speed remote, after this
> change:
>
>      hdev->hw.mac.req_autoneg = hdev->hw.mac.autoneg;  /* = DISABLE */
>      /* req_speed still holds flash default 100G */
>
> hclge_set_autoneg_speed_dup() would then see !req_autoneg and call:
>
>      hclge_cfg_mac_speed_dup_hw(hdev, hdev->hw.mac.req_speed=100G, ...);
>
> forcing 100G against a 25G-only peer.
>
> Is that scenario still handled, or is it expected not to occur in
> practice?

We want driver reload to reset MAC speed to the firmware default.
If that doesn't match a forced-mode remote on an optical port,
link drops and the user reconfigures with ethtool. That's fine.

The req_speed = mac.speed line was added in the V2 of the June
series to avoid that link drop, but it caused the actual reported
bug: speed stays at the last user setting across reload. This
patch drops that line and goes back to the V1 behavior, which
only synced req_autoneg.

   V1:
   https://lore.kernel.org/all/20260617112721.75186-5-shaojijie@huawei.com/

   V2:
   https://lore.kernel.org/all/20260624141319.271439-5-shaojijie@huawei.com/

If we need to cover the forced-mode optical case without
reintroducing the residue, that's a separate problem — a one-line
req_speed = mac.speed can't tell user-configured speed apart
from firmware-preset speed.

>
> [Medium]
> The commit message reasons about mac.speed carrying over user ethtool
> changes across a driver reload:
>
>    "In hclge_init_ae_dev(), hclge_update_port_info() reads mac.speed
>     from hardware, which reflects the last user configuration (e.g.
>     ethtool changes), not the firmware default."
>
> Does the same reasoning apply to mac.autoneg?
>
> hclge_get_sfp_info() sets mac->autoneg from the live firmware state, so
> a prior "ethtool -s ethX autoneg off speed 40000" on a 100G optical
> port would leave firmware reporting autoneg=DISABLE after reload.
>
> After this change, req_speed correctly reverts to the flash default
> 100G, but this line still runs:
>
>      hdev->hw.mac.req_autoneg = hdev->hw.mac.autoneg;
>
> so req_autoneg stays at AUTONEG_DISABLE rather than reverting to the
> AUTONEG_ENABLE default that hclge_configure() would set for fiber.
>
> Why is speed residue fixed while autoneg residue is preserved?

The asymmetry is on purpose. req_autoneg has been inherited from 
mac.autoneg since long before this work; that behavior has always been 
there and is left unchanged to avoid surprising users. req_speed = 
mac.speed was added in June V2 and is what caused the reported residue, 
so dropping it is the fix. I'll send a v3 with the commit message noting 
that link loss on forced-mode optical ports after reload is expected. 
Best, Jijie