Re: [net-next PATCH 06/14] net: phy: at803x: move at8031 specific data out of generic at803x_priv

[Russell King (Oracle)]

On Wed, Nov 29, 2023 at 03:12:11AM +0100, Christian Marangi wrote:
> Rework everything related to specific at8031 function to specific
> function and allocate the 2 bool, is_1000basex and is_fiber and the
> regulator structs to a dedicated qca8031_data struct.
> 
> This is needed to keep at803x functions more generic and detach them
> from specific check of at8031/33 PHY.
> 
> Out of all the reworked functions, only config_aneg required some code
> duplication with how the mdix config is handled.
> 
> This also reduces the generic at803x_priv struct by removing variables
> only used by at8031 PHY.

You are changing the order that register writes happen, e.g. for the
set_wol() method. at803x_set_wol() very clearly does stuff like
configuring the ethernet MAC address _before_ enabling WoL, and that
can fail. Your new code enables WoL and then calls at803x_set_wol().
If at803x_set_wol() fails (e.g. because of an invalid MAC address)
you leave WoL enabled. This is a change of behaviour.

I haven't checked anything else, but given the above, I think you
need to think more about how you make this change, and check
whether there are any other similar issues.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!