lkml.org 
[lkml]   [2021]   [Apr]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] spi: Fix spi device unregister flow
+Cc Lukas

On Tue, Apr 27, 2021 at 2:56 AM Saravana Kannan <saravanak@google.com> wrote:
>
> When an SPI device is unregistered, the spi->controller->cleanup() is
> called in the device's release callback. That's wrong for a couple of
> reasons:
>
> 1. spi_dev_put() can be called before spi_add_device() is called. And
> it's spi_add_device() that calls spi_setup(). This will cause clean()
> to get called without the spi device ever being setup.
>
> 2. There's no guarantee that the controller's driver would be present by
> the time the spi device's release function gets called.
>
> 3. It also causes "sleeping in atomic context" stack dump[1] when device
> link deletion code does a put_device() on the spi device.
>
> Fix these issues by simply moving the cleanup from the device release
> callback to the actual spi_unregister_device() function.
>
> [1] - https://lore.kernel.org/lkml/CAHp75Vc=FCGcUyS0v6fnxme2YJ+qD+Y-hQDQLa2JhWNON9VmsQ@mail.gmail.com/
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
> drivers/spi/spi.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index b08efe88ccd6..7d0d89172a1d 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -47,10 +47,6 @@ static void spidev_release(struct device *dev)
> {
> struct spi_device *spi = to_spi_device(dev);
>
> - /* spi controllers may cleanup for released devices */
> - if (spi->controller->cleanup)
> - spi->controller->cleanup(spi);
> -
> spi_controller_put(spi->controller);
> kfree(spi->driver_override);
> kfree(spi);
> @@ -558,6 +554,12 @@ static int spi_dev_check(struct device *dev, void *data)
> return 0;
> }
>
> +static void spi_cleanup(struct spi_device *spi)
> +{
> + if (spi->controller->cleanup)
> + spi->controller->cleanup(spi);
> +}
> +
> /**
> * spi_add_device - Add spi_device allocated with spi_alloc_device
> * @spi: spi_device to register
> @@ -622,11 +624,13 @@ int spi_add_device(struct spi_device *spi)
>
> /* Device may be bound to an active driver when this returns */
> status = device_add(&spi->dev);
> - if (status < 0)
> + if (status < 0) {
> dev_err(dev, "can't add %s, status %d\n",
> dev_name(&spi->dev), status);
> - else
> + spi_cleanup(spi);
> + } else {
> dev_dbg(dev, "registered child %s\n", dev_name(&spi->dev));
> + }
>
> done:
> mutex_unlock(&spi_add_lock);
> @@ -713,6 +717,8 @@ 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);
> --
> 2.31.1.498.g6c1eba8ee3d-goog
>


--
With Best Regards,
Andy Shevchenko

\
 
 \ /
  Last update: 2021-04-27 08:53    [W:0.145 / U:0.460 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site