Re: [Intel-wired-lan] [PATCH iwl-next 6/8] ixgbe: extract ixgbe_restart_auto_neg() to avoid code duplication

[Simon Horman <horms@kernel.org>]

On Fri, May 08, 2026 at 05:12:24AM +0200, Aleksandr Loktionov wrote:
> From: Jakub Chylkowski <jakubx.chylkowski@intel.com>
> 
> Both ixgbe_setup_phy_link_generic() and ixgbe_setup_phy_link_tnx()
> end with the same three-line sequence that reads MDIO_CTRL1, sets
> the MDIO_AN_CTRL1_RESTART bit, and writes MDIO_CTRL1 back.
> 
> Factor it out into a static helper ixgbe_restart_auto_neg() and call
> it from both sites.
> 
> While at it, also check the return value of phy.ops.read_reg() in the
> helper and skip the write on failure.  The original inlined code
> ignored the read result and would OR MDIO_AN_CTRL1_RESTART into a
> stale autoneg_reg value (left over from the prior MDIO_AN_ADVERTISE
> write) and unconditionally write it back to MDIO_CTRL1 if the read
> failed.  This is a small behavioral change: on read_reg() failure the
> restart write is now skipped instead of being issued with a
> potentially garbage value.
> 
> Signed-off-by: Jakub Chylkowski <jakubx.chylkowski@intel.com>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

Reviewed-by: Simon Horman <horms@kernel.org>

FWIIW, the AI-generated review of this patch available on sashiko.dev
flags that similar problems wrt write on failre exist earlier on in
ixgbe_setup_phy_link_generic(). It may be good to address this area
more holistically as a follow-up. (I am not suggesting increasing
the scope of this patch/patch-set.)

...