Re: [PATCH net-next] net: dsa: Some cleanups in remove code

Your commit prefix does not reflect the fact that you are touching the
vsc73xx driver. Try "net: dsa: vsc73xx: ".

On Tue, Nov 09, 2021 at 12:39:21PM +0100, Uwe Kleine-König wrote:

vsc73xx_remove() returns zero unconditionally and no caller checks the
returned value. So convert the function to return no value.

This I agree with.

For both the platform and the spi variant ..._get_drvdata() will never
return NULL in .remove() because the remove callback is only called after
the probe callback returned successfully and in this case driver data was
set to a non-NULL value.

Have you read the commit message of 0650bf52b31f ("net: dsa: be
compatible with masters which unregister on shutdown")?

To remove the check for dev_get_drvdata == NULL in ->remove, you need to
prove that ->remove will never be called after ->shutdown. For platform
devices this is pretty easy to prove, for SPI devices not so much.
I intentionally kept the code structure the same because code gets
copied around a lot, it is easy to copy from the wrong place.

Also setting driver data to NULL is not necessary, this is already done
in the driver core in __device_release_driver(), so drop this from the
remove callback, too.

And this was also intentional, for visibility more or less. I would like
you to ack that you understand the problems surrounding ->remove/->shutdown
ordering for devices on buses, prior to making seemingly trivial cleanups.

Signed-off-by: Uwe Kleine-König <redacted>
---