Re: [PATCH net v3] net: dsa: mxl-gsw1xx: manually clear RANEG bit

[Andrew Lunn <andrew@lunn.ch>]

On Mon, Dec 08, 2025 at 12:47:51PM +0000, Russell King (Oracle) wrote:
> On Mon, Dec 08, 2025 at 01:27:04AM +0000, Daniel Golle wrote:
> >  static void gsw1xx_remove(struct mdio_device *mdiodev)
> >  {
> >  	struct gswip_priv *priv = dev_get_drvdata(&mdiodev->dev);
> > +	struct gsw1xx_priv *gsw1xx_priv;
> >  
> >  	if (!priv)
> >  		return;
> >  
> > +	gsw1xx_priv = container_of(priv, struct gsw1xx_priv, gswip);
> > +	cancel_delayed_work_sync(&gsw1xx_priv->clear_raneg);
> > +
> >  	gswip_disable_switch(priv);
> >  
> >  	dsa_unregister_switch(priv->ds);
> 
> Can we please pay attention to ->remove methods, and code them properly
> please?
> 
> There are two golden rules of driver programming.
> 
> 1. Do not publish the device during probe until hardware setup is
>    complete. If you publish before hardware setup is complete, userspace
>    is free to race with the hardware setup and start using the device.
>    This is especially true of recent systems which use hotplug events
>    via udev and systemd to do stuff.
> 
> 2. Do not start tearing down a device until the user interfaces have
>    been unpublished. Similar to (1), while the user interface is
>    published, uesrspace is completely free to interact with the device
>    in any way it sees fit.
> 
> In this case, what I'm concerned with is the call above to
> cancel_delayed_work_sync() before dsa_unregister_switch(). While
> cancel_delayed_work_sync() will stop this work and wait for the handler
> to finish running before returning (which is safe) there is a window
> between this call and dsa_unregister_switch() where the user _could_
> issue a badly timed ethtool command which invokes
> gsw1xx_pcs_an_restart(), which would re-schedule the delayed work,
> thus undoing the cancel_delayed_work_sync() effect in this path.

And this is why is was pushing for the much simpler msleep(10), or
io_poll.h polling to see if it self clears. It is hard to get that
wrong, where as delayed work is much easier to get wrong.

	Andrew