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

[Vladimir Oltean <olteanv@gmail.com>]

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 <u.kleine-koenig@pengutronix.de>
> ---