linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] spi: Don't have controller clean up spi device before driver unbind
@ 2021-05-05 16:47 Saravana Kannan
  2021-05-05 17:53 ` Andy Shevchenko
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Saravana Kannan @ 2021-05-05 16:47 UTC (permalink / raw)
  To: Mark Brown, Saravana Kannan
  Cc: Lukas Wunner, kernel-team, linux-spi, linux-kernel

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(-)

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);
 
-- 
2.31.1.527.g47e6f16901-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] spi: Don't have controller clean up spi device before driver unbind
  2021-05-05 16:47 [PATCH v2] spi: Don't have controller clean up spi device before driver unbind Saravana Kannan
@ 2021-05-05 17:53 ` Andy Shevchenko
  2021-05-11 15:57   ` Mark Brown
  2021-05-14 15:22 ` Mark Brown
  2021-05-21  5:09 ` Lukas Wunner
  2 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2021-05-05 17:53 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Mark Brown, Lukas Wunner, Android Kernel Team, linux-spi,
	Linux Kernel Mailing List

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.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] spi: Don't have controller clean up spi device before driver unbind
  2021-05-05 17:53 ` Andy Shevchenko
@ 2021-05-11 15:57   ` Mark Brown
  2021-05-11 20:30     ` Andy Shevchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2021-05-11 15:57 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Saravana Kannan, Lukas Wunner, Android Kernel Team, linux-spi,
	Linux Kernel Mailing List

[-- Attachment #1: Type: text/plain, Size: 282 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] spi: Don't have controller clean up spi device before driver unbind
  2021-05-11 15:57   ` Mark Brown
@ 2021-05-11 20:30     ` Andy Shevchenko
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2021-05-11 20:30 UTC (permalink / raw)
  To: Mark Brown
  Cc: Saravana Kannan, Lukas Wunner, Android Kernel Team, linux-spi,
	Linux Kernel Mailing List

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>


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] spi: Don't have controller clean up spi device before driver unbind
  2021-05-05 16:47 [PATCH v2] spi: Don't have controller clean up spi device before driver unbind Saravana Kannan
  2021-05-05 17:53 ` Andy Shevchenko
@ 2021-05-14 15:22 ` Mark Brown
  2021-05-21  5:09 ` Lukas Wunner
  2 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2021-05-14 15:22 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Mark Brown, kernel-team, linux-kernel, linux-spi, Lukas Wunner

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] spi: Don't have controller clean up spi device before driver unbind
  2021-05-05 16:47 [PATCH v2] spi: Don't have controller clean up spi device before driver unbind Saravana Kannan
  2021-05-05 17:53 ` Andy Shevchenko
  2021-05-14 15:22 ` Mark Brown
@ 2021-05-21  5:09 ` Lukas Wunner
  2 siblings, 0 replies; 6+ messages in thread
From: Lukas Wunner @ 2021-05-21  5:09 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Mark Brown, kernel-team, linux-spi, linux-kernel, Andy Shevchenko

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-05-21  5:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-05 16:47 [PATCH v2] spi: Don't have controller clean up spi device before driver unbind Saravana Kannan
2021-05-05 17:53 ` Andy Shevchenko
2021-05-11 15:57   ` Mark Brown
2021-05-11 20:30     ` Andy Shevchenko
2021-05-14 15:22 ` Mark Brown
2021-05-21  5:09 ` Lukas Wunner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).