[PATCH 1/2] spi: Make spi_add_lock a per-controller lock

* [PATCH 1/2] spi: Make spi_add_lock a per-controller lock
@ 2021-10-13 13:37 Uwe Kleine-König
  2021-10-13 13:37 ` [PATCH 2/2] spi-mux: Fix false-positive lockdep splats Uwe Kleine-König
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2021-10-13 13:37 UTC (permalink / raw)
  To: Mark Brown; +Cc: kernel, linux-spi

The spi_add_lock that is removed with this change was held when
spi_add_device() called device_add() (via __spi_add_device()).

In the case where the added device is an spi-mux calling device_add()
might result in calling the spi-mux's probe function which adds another
controller and for that spi_add_device() might be called. This results
in a dead-lock.

To circumvent this deadlock replace the global spi_add_lock with a lock
per controller.

The biggest part of this patch was authored by Mark Brown.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/spi/spi.c       | 17 ++++++-----------
 include/linux/spi/spi.h |  4 ++++
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 50591de16e0f..193e3264f7eb 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -477,12 +477,6 @@ static LIST_HEAD(spi_controller_list);
  */
 static DEFINE_MUTEX(board_lock);
 
-/*
- * Prevents addition of devices with same chip select and
- * addition of devices below an unregistering controller.
- */
-static DEFINE_MUTEX(spi_add_lock);
-
 /**
  * spi_alloc_device - Allocate a new SPI device
  * @ctlr: Controller to which device is connected
@@ -635,9 +629,9 @@ static int spi_add_device(struct spi_device *spi)
 	/* Set the bus ID string */
 	spi_dev_set_name(spi);
 
-	mutex_lock(&spi_add_lock);
+	mutex_lock(&ctlr->add_lock);
 	status = __spi_add_device(spi);
-	mutex_unlock(&spi_add_lock);
+	mutex_unlock(&ctlr->add_lock);
 	return status;
 }
 
@@ -656,7 +650,7 @@ static int spi_add_device_locked(struct spi_device *spi)
 	/* Set the bus ID string */
 	spi_dev_set_name(spi);
 
-	WARN_ON(!mutex_is_locked(&spi_add_lock));
+	WARN_ON(!mutex_is_locked(&ctlr->add_lock));
 	return __spi_add_device(spi);
 }
 
@@ -2909,6 +2903,7 @@ int spi_register_controller(struct spi_controller *ctlr)
 	spin_lock_init(&ctlr->bus_lock_spinlock);
 	mutex_init(&ctlr->bus_lock_mutex);
 	mutex_init(&ctlr->io_mutex);
+	mutex_init(&ctlr->add_lock);
 	ctlr->bus_lock_flag = 0;
 	init_completion(&ctlr->xfer_completion);
 	if (!ctlr->max_dma_len)
@@ -3045,7 +3040,7 @@ void spi_unregister_controller(struct spi_controller *ctlr)
 
 	/* Prevent addition of new devices, unregister existing ones */
 	if (IS_ENABLED(CONFIG_SPI_DYNAMIC))
-		mutex_lock(&spi_add_lock);
+		mutex_lock(&ctlr->add_lock);
 
 	device_for_each_child(&ctlr->dev, NULL, __unregister);
 
@@ -3076,7 +3071,7 @@ void spi_unregister_controller(struct spi_controller *ctlr)
 	mutex_unlock(&board_lock);
 
 	if (IS_ENABLED(CONFIG_SPI_DYNAMIC))
-		mutex_unlock(&spi_add_lock);
+		mutex_unlock(&ctlr->add_lock);
 }
 EXPORT_SYMBOL_GPL(spi_unregister_controller);
 
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 29e21d49aafc..5b61abe679f1 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -349,6 +349,7 @@ extern struct spi_device *spi_new_ancillary_device(struct spi_device *spi, u8 ch
  * @max_message_size: function that returns the max message size for
  *	a &spi_device; may be %NULL, so the default %SIZE_MAX will be used.
  * @io_mutex: mutex for physical bus access
+ * @add_lock: mutex to avoid adding two devices on the same CS
  * @bus_lock_spinlock: spinlock for SPI bus locking
  * @bus_lock_mutex: mutex for exclusion of multiple callers
  * @bus_lock_flag: indicates that the SPI bus is locked for exclusive use
@@ -527,6 +528,9 @@ struct spi_controller {
 	/* I/O mutex */
 	struct mutex		io_mutex;
 
+	/* Used to avoid adding two devices on the same CS line */
+	struct mutex		add_lock;
+
 	/* lock and mutex for SPI bus locking */
 	spinlock_t		bus_lock_spinlock;
 	struct mutex		bus_lock_mutex;

base-commit: da21fde0fdb393c2fbe0ae0735cc826cd55fd46f
-- 
2.30.2


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