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

[PATCH V3 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.

The commit referenced in Fixes: added two overwrites in
hclge_init_ae_dev() for non-copper media, so that optical ports
connected to forced-mode remotes inherit the firmware-preset
autoneg and speed instead of the hardcoded defaults:

  req_autoneg = mac.autoneg
  req_speed   = mac.speed         (when autoneg disabled)

The autoneg overwrite keeps existing behavior:
hclge_set_autoneg_speed_dup() already uses mac.autoneg (not
req_autoneg) since it was introduced, so autoneg inheritance from
firmware was already in place. This part is kept.

The speed overwrite, however, introduces the residue: mac.speed
reflects whatever was last programmed into the MAC, and after unload
firmware does not restore the MAC speed to the flash default. So if
the user changed speed via ethtool in a prior load, mac.speed still
carries that value on reload and req_speed inherits it.

Fix by dropping the req_speed overwrite only. req_speed keeps the
firmware default value set in hclge_configure() (cfg.default_speed),
so a reload reverts the speed to default, matching the expectation
that a driver reload resets link configuration.

Trade-off: on optical ports whose firmware default speed does not
match a forced-mode remote, reload now drops the link and the user
must re-apply ethtool configuration. This is acceptable: a driver
reload is expected to reset link configuration, not to inherit
runtime state from before unload. The autoneg inheritance is left
in place as established behavior; changing it is out of scope for
this patch and would itself be a user-perceivable behavior change.

Fixes: d9d349c4e8a0 ("net: hns3: differentiate autoneg default values between copper and fiber")
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
v3:
 - Expand commit message to explain the two overwrites added by the
   Fixes commit: req_autoneg inheritance is kept because it matches
   existing behavior of hclge_set_autoneg_speed_dup(); req_speed
   sync is dropped because it introduces the reported residue.
 - State explicitly that link loss on forced-mode optical ports after
   reload is acceptable, since a driver reload is expected to reset
   link configuration rather than inherit runtime state.
 - Explain why autoneg inheritance is left unchanged: it is
   established behavior, and changing it is out of scope and would
   be user-perceivable.

v2: https://lore.kernel.org/all/20260708140532.1691493-1-shaojijie@huawei.com/
 - 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: 78f75d632f74b8de0f081a128588f7c37d0d1164
-- 
2.33.0

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

[Simon Horman]

On Fri, Jul 24, 2026 at 05:30:36PM +0800, Jijie Shao wrote:
> 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.
> 
> The commit referenced in Fixes: added two overwrites in
> hclge_init_ae_dev() for non-copper media, so that optical ports
> connected to forced-mode remotes inherit the firmware-preset
> autoneg and speed instead of the hardcoded defaults:
> 
>   req_autoneg = mac.autoneg
>   req_speed   = mac.speed         (when autoneg disabled)
> 
> The autoneg overwrite keeps existing behavior:
> hclge_set_autoneg_speed_dup() already uses mac.autoneg (not
> req_autoneg) since it was introduced, so autoneg inheritance from
> firmware was already in place. This part is kept.

The AI-generated review on netdev-ai [1] flags that this isn't strictly
true as req_autoneg does appear to be used in hclge_set_autoneg_speed_dup().
I don't want to nitpick, but perhaps this is worth clarifying.

[1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260724093036.426631-1-shaojijie%40huawei.com

> The speed overwrite, however, introduces the residue: mac.speed
> reflects whatever was last programmed into the MAC, and after unload
> firmware does not restore the MAC speed to the flash default. So if
> the user changed speed via ethtool in a prior load, mac.speed still
> carries that value on reload and req_speed inherits it.
> 
> Fix by dropping the req_speed overwrite only. req_speed keeps the
> firmware default value set in hclge_configure() (cfg.default_speed),
> so a reload reverts the speed to default, matching the expectation
> that a driver reload resets link configuration.
> 
> Trade-off: on optical ports whose firmware default speed does not
> match a forced-mode remote, reload now drops the link and the user
> must re-apply ethtool configuration. This is acceptable: a driver
> reload is expected to reset link configuration, not to inherit
> runtime state from before unload. The autoneg inheritance is left
> in place as established behavior; changing it is out of scope for
> this patch and would itself be a user-perceivable behavior change.
> 
> Fixes: d9d349c4e8a0 ("net: hns3: differentiate autoneg default values between copper and fiber")
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
> v3:
>  - Expand commit message to explain the two overwrites added by the
>    Fixes commit: req_autoneg inheritance is kept because it matches
>    existing behavior of hclge_set_autoneg_speed_dup(); req_speed
>    sync is dropped because it introduces the reported residue.
>  - State explicitly that link loss on forced-mode optical ports after
>    reload is acceptable, since a driver reload is expected to reset
>    link configuration rather than inherit runtime state.
>  - Explain why autoneg inheritance is left unchanged: it is
>    established behavior, and changing it is out of scope and would
>    be user-perceivable.

...

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

[Jijie Shao]

on 2026/7/29 19:03, Simon Horman wrote:
> On Fri, Jul 24, 2026 at 05:30:36PM +0800, Jijie Shao wrote:
>> 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.
>>
>> The commit referenced in Fixes: added two overwrites in
>> hclge_init_ae_dev() for non-copper media, so that optical ports
>> connected to forced-mode remotes inherit the firmware-preset
>> autoneg and speed instead of the hardcoded defaults:
>>
>>    req_autoneg = mac.autoneg
>>    req_speed   = mac.speed         (when autoneg disabled)
>>
>> The autoneg overwrite keeps existing behavior:
>> hclge_set_autoneg_speed_dup() already uses mac.autoneg (not
>> req_autoneg) since it was introduced, so autoneg inheritance from
>> firmware was already in place. This part is kept.
> The AI-generated review on netdev-ai [1] flags that this isn't strictly
> true as req_autoneg does appear to be used in hclge_set_autoneg_speed_dup().
> I don't want to nitpick, but perhaps this is worth clarifying.
>
> [1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260724093036.426631-1-shaojijie%40huawei.com


The intent is to restore the pre-c711f6d1cee9 behavior for non-copper
media, where the helper read mac.autoneg directly and fiber ports
inherited firmware autoneg at init. c711f6d1cee9 switched the helper
to req_autoneg, breaking that inheritance; d9d349c4e8a0 (same patchset)
added the explicit req_autoneg = autoneg copy to restore
it, and also added a req_speed = mac.speed copy.

This patch keeps the req_autoneg copy — load-bearing as the sole
init-time path for non-copper media — and drops the req_speed copy,
which is the residue source. The commit message wording about the
helper reading mac.autoneg describes the pre-c711f6d1cee9 state,
not the current code.

Thanks
Jijie.


>
>> The speed overwrite, however, introduces the residue: mac.speed
>> reflects whatever was last programmed into the MAC, and after unload
>> firmware does not restore the MAC speed to the flash default. So if
>> the user changed speed via ethtool in a prior load, mac.speed still
>> carries that value on reload and req_speed inherits it.
>>
>> Fix by dropping the req_speed overwrite only. req_speed keeps the
>> firmware default value set in hclge_configure() (cfg.default_speed),
>> so a reload reverts the speed to default, matching the expectation
>> that a driver reload resets link configuration.
>>
>> Trade-off: on optical ports whose firmware default speed does not
>> match a forced-mode remote, reload now drops the link and the user
>> must re-apply ethtool configuration. This is acceptable: a driver
>> reload is expected to reset link configuration, not to inherit
>> runtime state from before unload. The autoneg inheritance is left
>> in place as established behavior; changing it is out of scope for
>> this patch and would itself be a user-perceivable behavior change.
>>
>> Fixes: d9d349c4e8a0 ("net: hns3: differentiate autoneg default values between copper and fiber")
>> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
>> ---
>> v3:
>>   - Expand commit message to explain the two overwrites added by the
>>     Fixes commit: req_autoneg inheritance is kept because it matches
>>     existing behavior of hclge_set_autoneg_speed_dup(); req_speed
>>     sync is dropped because it introduces the reported residue.
>>   - State explicitly that link loss on forced-mode optical ports after
>>     reload is acceptable, since a driver reload is expected to reset
>>     link configuration rather than inherit runtime state.
>>   - Explain why autoneg inheritance is left unchanged: it is
>>     established behavior, and changing it is out of scope and would
>>     be user-perceivable.
> ...