lkml.org 
[lkml]   [2020]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC PATCH] spi: Take the SPI IO-mutex in the spi_setup() method
    On Tue, Nov 17, 2020 at 12:56:44PM +0200, Andy Shevchenko wrote:
    > On Tue, Nov 17, 2020 at 11:45 AM Serge Semin
    > <Sergey.Semin@baikalelectronics.ru> wrote:
    > >
    > > I've discovered that due to the recent commit 49d7d695ca4b ("spi: dw:
    > > Explicitly de-assert CS on SPI transfer completion") a concurrent usage of
    > > the spidev devices with different chip-selects causes the "SPI transfer
    > > timed out" error.
    >
    > I'll read this later...
    >
    > > + mutex_lock(&spi->controller->io_mutex);
    > > +
    > > if (spi->controller->setup)
    > > status = spi->controller->setup(spi);
    > >
    > > if (spi->controller->auto_runtime_pm && spi->controller->set_cs) {
    > > status = pm_runtime_get_sync(spi->controller->dev.parent);
    >

    > I didn't check what this lock is protecting,

    It is used to protect the SPI io operations. So it's locked only
    during the SPI memory operations and the SPI-message execution. That's
    the time when the core toggles the controller chip-selects by calling
    the spi_set_cs() method and the set_cs callback.

    > but have you checked all
    > PM runtime callbacks if they are not taking the lock. When you call PM
    > runtime functions with 'sync' it may include a lot of work, some of
    > which may sleep (not a problem for mutex) and may take arbitrary locks
    > (might be a deadlock in case of trying the same lock).

    Yeah, I understand that. Simple grepping hasn't showed anyone else but
    the SPI-core using it. So unless the controllers PM methods also call
    spi_setup() or request SPI-transfers, there shouldn't be a deadlock.
    Moreover as I can see from the __spi_pump_messages() method the
    IO-mutex is locked during the sync-suffixed PM-methods invocation.
    AFAICS locking io_mutex around the PM-methods here shouldn't cause
    problems. But of course testing it in various platforms/controllers is
    always welcome.

    -Sergey

    >
    > > if (status < 0) {
    > > + mutex_unlock(&spi->controller->io_mutex);
    > > pm_runtime_put_noidle(spi->controller->dev.parent);
    > > dev_err(&spi->controller->dev, "Failed to power device: %d\n",
    > > status);
    > > @@ -3354,6 +3357,8 @@ int spi_setup(struct spi_device *spi)
    > > spi_set_cs(spi, false);
    > > }
    > >
    > > + mutex_unlock(&spi->controller->io_mutex);
    > > +
    >
    >
    > --
    > With Best Regards,
    > Andy Shevchenko

    \
     
     \ /
      Last update: 2020-11-17 12:30    [W:2.853 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site