Re: [RFC PATCH net 2/5] net: dsa: be compatible with masters which unregister on shutdown

From: Vladimir Oltean <olteanv@gmail.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Vladimir Oltean <vladimir.oltean@nxp.com>,
	netdev@vger.kernel.org, Florian Fainelli <f.fainelli@gmail.com>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Kurt Kanzenbach <kurt@linutronix.de>,
	Hauke Mehrtens <hauke@hauke-m.de>,
	Woojung Huh <woojung.huh@microchip.com>,
	UNGLinuxDriver@microchip.com, Sean Wang <sean.wang@mediatek.com>,
	Landen Chao <Landen.Chao@mediatek.com>,
	DENG Qingfang <dqfext@gmail.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	George McCollister <george.mccollister@gmail.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	Oleksij Rempel <linux@rempel-privat.de>,
	Michael Grzeschik <m.grzeschik@pengutronix.de>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,
	Lino Sanfilippo <LinoSanfilippo@gmx.de>
Subject: Re: [RFC PATCH net 2/5] net: dsa: be compatible with masters which unregister on shutdown
Date: Mon, 13 Sep 2021 16:31:30 +0300	[thread overview]
Message-ID: <20210913133130.ohk4co56v4mtljyk@skbuf> (raw)
In-Reply-To: <YT9QwOA2DxaXNsfw@lunn.ch>

On Mon, Sep 13, 2021 at 03:23:12PM +0200, Andrew Lunn wrote:
> > I will fix this when I send the v2 patch, but I will not send that now,
> > as I would like to get some feedback on the approach first.
> 
> It would of been nice to have less boilerplate code, but the basic
> idea seems O.K.
> 
> Have you tested it with a D in DSA system?

To various degrees.

I cannot easily patch DSA masters to just implement ->shutdown as
->remove so as to reproduce Lino's case with the Raspberry Pi, but I did
perform basic regression-testing on:

- the Bluebox 3 board with the 2x SJA1110 switches in a "real" DSA multi
  switch tree setup, with dpaa2-eth as the master and drivers/spi/spi-sc18is602.c
  as the SPI controller

- the weird board with disjoint DSA trees comprised of 2x SJA1105
  switches hanging off of the internal Felix/Ocelot switch of the
  LS1028A which in itself has the fsl-enetc driver as its master. Here I
  could test the fsl-enetc driver with and without the ->shutdown method.
  I also tested with and without dspi_shutdown so as to walk through
  both the sja1105's shutdown and remove methods.

- the Turris MOX board where I did not notice any issues during
  regression testing. The only new message is that the link of the DSA
  interfaces goes down, this is because the net devices are actually
  unregistered on shutdown.

It would be possible to have less boilerplate code, by implementing the
DSA shutdown procedure as dsa_unregister_switch itself.

For buses where the ->remove and ->shutdown have the same prototype
(they both return void), like PCI, the code added is minimal (although
we still need to add the "if this then not that" scheme, to avoid the
function body getting executed twice). For the other buses, there would
still need to be a separate shutdown method, which calls the remove
method. Although in principle, this also has functional consequences
which I am not sure whether I like or not. To walk the full-blown unbind
code path or to do a shutdown with the minimal necessities?