| Message ID | 20210505164734.175546-1-saravanak@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 27e7db56cf3dffd302bd7ddfacb1d405cf671a2a |
| Headers | show |
| Series | [v2] spi: Don't have controller clean up spi device before driver unbind | expand |
On Wed, May 5, 2021 at 8:26 PM Saravana Kannan <saravanak@google.com> wrote: > > When a spi device is unregistered and triggers a driver unbind, the > driver might need to access the spi device. So, don't have the > controller clean up the spi device before the driver is unbound. Clean > up the spi device after the driver is unbound. > > Fixes: c7299fea6769 ("spi: Fix spi device unregister flow") > Reported-by: Lukas Wunner <lukas@wunner.de> And Suggested-by: Lukas ... > Signed-off-by: Saravana Kannan <saravanak@google.com> ... > Can you test this one your end to make sure you don't have issues > anymore? Do you need a test on my setup? ... > + device_del(&spi->dev); > + spi_cleanup(spi); > + put_device(&spi->dev); This block deserves a comment in the code.
On Wed, May 05, 2021 at 08:53:14PM +0300, Andy Shevchenko wrote: > On Wed, May 5, 2021 at 8:26 PM Saravana Kannan <saravanak@google.com> wrote: > > Can you test this one your end to make sure you don't have issues > > anymore? > Do you need a test on my setup? It wouldn't hurt.
On Tue, May 11, 2021 at 6:58 PM Mark Brown <broonie@kernel.org> wrote: > > On Wed, May 05, 2021 at 08:53:14PM +0300, Andy Shevchenko wrote: > > On Wed, May 5, 2021 at 8:26 PM Saravana Kannan <saravanak@google.com> wrote: > > > > Can you test this one your end to make sure you don't have issues > > > anymore? > > > Do you need a test on my setup? > > It wouldn't hurt. Okay, I have reverted first the "spi: Fix spi device unregister flow" to be sure I can reproduce the lockdep warning, indeed, it's there. After applying the above mentioned patch it's gone. On top I applied this ("spi: Don't have controller clean up spi device before driver unbind") patch to see if there is any changes, nope, seems everything is fine. FWIW, Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
On Wed, 5 May 2021 09:47:34 -0700, Saravana Kannan wrote: > When a spi device is unregistered and triggers a driver unbind, the > driver might need to access the spi device. So, don't have the > controller clean up the spi device before the driver is unbound. Clean > up the spi device after the driver is unbound. Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: Don't have controller clean up spi device before driver unbind commit: 27e7db56cf3dffd302bd7ddfacb1d405cf671a2a All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
On Wed, May 05, 2021 at 09:47:34AM -0700, Saravana Kannan wrote: > When a spi device is unregistered and triggers a driver unbind, the > driver might need to access the spi device. So, don't have the > controller clean up the spi device before the driver is unbound. Clean > up the spi device after the driver is unbound. > > Fixes: c7299fea6769 ("spi: Fix spi device unregister flow") > Reported-by: Lukas Wunner <lukas@wunner.de> > Signed-off-by: Saravana Kannan <saravanak@google.com> > --- > > v1->v2: > - Made the clean up more symmetric. > > Lukas, > > Can you test this one your end to make sure you don't have issues > anymore? Tested-by: Lukas Wunner <lukas@wunner.de> My apologies for the delay, yesterday I was finally able to set up a RasPi in my home office and test the patch. I'm not seeing any issues when unloading/reloading the SPI controller module, so this LGTM. Thanks, Lukas
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 2350d131871b..f23e288e6498 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -714,8 +714,6 @@ void spi_unregister_device(struct spi_device *spi) if (!spi) return; - spi_cleanup(spi); - if (spi->dev.of_node) { of_node_clear_flag(spi->dev.of_node, OF_POPULATED); of_node_put(spi->dev.of_node); @@ -723,7 +721,9 @@ void spi_unregister_device(struct spi_device *spi) if (ACPI_COMPANION(&spi->dev)) acpi_device_clear_enumerated(ACPI_COMPANION(&spi->dev)); device_remove_software_node(&spi->dev); - device_unregister(&spi->dev); + device_del(&spi->dev); + spi_cleanup(spi); + put_device(&spi->dev); } EXPORT_SYMBOL_GPL(spi_unregister_device);
When a spi device is unregistered and triggers a driver unbind, the driver might need to access the spi device. So, don't have the controller clean up the spi device before the driver is unbound. Clean up the spi device after the driver is unbound. Fixes: c7299fea6769 ("spi: Fix spi device unregister flow") Reported-by: Lukas Wunner <lukas@wunner.de> Signed-off-by: Saravana Kannan <saravanak@google.com> --- v1->v2: - Made the clean up more symmetric. Lukas, Can you test this one your end to make sure you don't have issues anymore? Thanks, Saravana drivers/spi/spi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)