On Tue, May 4, 2021 at 10:48 PM Lukas Wunner <lukas@xxxxxxxxx> wrote: > > On Tue, May 04, 2021 at 08:14:16PM -0700, Saravana Kannan wrote: > > @@ -415,6 +421,7 @@ static int spi_remove(struct device *dev) > > ERR_PTR(ret)); > > } > > > > + spi_cleanup(to_spi_device(dev)); > > dev_pm_domain_detach(dev, true); > > > > return 0; > > Unfortunately this doesn't look right: spi_remove() is run on > driver unbind of the spi_device. With the above change, > ->setup is called on spi_device addition and ->cleanup is called > on unbind, which is obviously assymetric. What can happen > here is that a slave-specific controller_state is allocated on > spi_device addition, then on unbind that controller_state is freed > and on a subsequent rebind it won't be recreated because ->setup > isn't run on spi_device ->probe. Good point! > > As I've written yesterday, calling spi_cleanup() in > spi_unregister_device() should be fine if you move it to the end > of the function, but before the final put_device(). For that, > you need to open code the calls to device_del() and put_device() > that happen in device_unregister() so far. Yeah, I saw that email after I sent out this patch. I'll send out a v2. -Saravana