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

[Simon Horman]

On Wed, Jul 29, 2026 at 07:45:21PM +0800, Jijie Shao wrote:
> 
> 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 for clarifying.

In that case all seems to be in order to me.

Reviewed-by: Simon Horman <horms@xxxxxxxxxx>