Re: [PATCH 1/6] spi: fsl-dspi: Use devm_spi_register_controller()

[Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>]

On Tue, Sep 22, 2020 at 10:44:37AM +0100, Mark Brown wrote:
> On Tue, Sep 22, 2020 at 11:32:23AM +0200, Sascha Hauer wrote:
> 
> > @@ -1440,9 +1440,6 @@ static int dspi_remove(struct platform_device *pdev)
> >  	struct spi_controller *ctlr = platform_get_drvdata(pdev);
> >  	struct fsl_dspi *dspi = spi_controller_get_devdata(ctlr);
> >  
> > -	/* Disconnect from the SPI framework */
> > -	spi_unregister_controller(dspi->ctlr);
> > -
> >  	/* Disable RX and TX */
> >  	regmap_update_bits(dspi->regmap, SPI_MCR,
> >  			   SPI_MCR_DIS_TXF | SPI_MCR_DIS_RXF,
> 
> Is this fix safe - what happens if we start another transaction between
> disabling RX/TX and the unregistration taking effect?  Similar concerns
> apply to some of the other patches.

I asked myself the same question until I saw that a dozen of other drivers
also do it like this.

I don't think it's safe. Currently we only have spi_unregister_controller()
which both unregisters child SPI devices and frees the SPI controller along
with the driver data.
There are many drivers have something in their remove function like
disabling the hardware, clocks, freeing interrupts. They do it either
before calling spi_unregister_controller(), in which case messages may be
queued on a teared down hardware, or they do it after calling
spi_unregister_controller(), in which case they operate on already freed
data. IMO this only works when all drivers are fully managed with devm_*
functions and runtime pm so that the remove functions for all drivers
become empty.  Until this happens spi_unregister_controller() has to be
split up in a function actually unregistering the controller and another
one freeing the resources.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |