diff mbox

gpio: pca953x: fix an incorrect lockdep warning

Message ID 1474035484-25134-1-git-send-email-bgolaszewski@baylibre.com
State New
Headers show

Commit Message

Bartosz Golaszewski Sept. 16, 2016, 2:18 p.m. UTC
If an I2C GPIO multiplexer is driven by a GPIO provided by an expander
when there's a second expander using the same device driver on one of
the I2C bus segments, lockdep prints a deadlock warning when trying to
set the direction or the value of the GPIOs provided by the second
expander.

The below diagram presents the setup:

                                               - - - - -
 -------             ---------  Bus segment 1 |         |
|       |           |         |---------------  Devices
|       | SCL/SDA   |         |               |         |
| Linux |-----------| I2C MUX |                - - - - -
|       |    |      |         | Bus segment 2
|       |    |      |         |-------------------
 -------     |       ---------                    |
             |           |                    - - - - -
        ------------     | MUX GPIO          |         |
       |            |    |                     Devices
       |    GPIO    |    |                   |         |
       | Expander 1 |----                     - - - - -
       |            |                             |
        ------------                              | SCL/SDA
                                                  |
                                             ------------
                                            |            |
                                            |    GPIO    |
                                            | Expander 2 |
                                            |            |
                                             ------------

The reason for lockdep warning is that we take the chip->i2c_lock in
pca953x_gpio_set_value() or pca953x_gpio_direction_output() and then
come right back to pca953x_gpio_set_value() when the GPIO mux kicks
in. The locks actually protect different expanders, but for lockdep
both are of the same class, so it says:

  Possible unsafe locking scenario:

        CPU0
        ----
   lock(&chip->i2c_lock);
   lock(&chip->i2c_lock);

  *** DEADLOCK ***

  May be due to missing lock nesting notation

In order to get rid of the warning, check if the i2c adapter of the
expander is multiplexed (by checking if it has a parent adapter) and,
if so, set a different lock subclass for chip->i2c_lock.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

---
Note: a similar issue would occur with other gpio expanders under
similar circumstances. If this patch get's merged, I'll prepare
a common solution for all gpio drivers which use an internal i2c lock.

 drivers/gpio/gpio-pca953x.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 02f2a56..2d49b25 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -787,6 +787,18 @@  static int pca953x_probe(struct i2c_client *client,
 
 	mutex_init(&chip->i2c_lock);
 
+	/*
+	 * If the i2c adapter we're connected to is multiplexed (which is
+	 * indicated by it having a parent adapter) we need to use a
+	 * different lock subclass. It's caused by the fact that in a rare
+	 * case of using an i2c-gpio multiplexer controlled by a gpio
+	 * provided by an expander using the same driver, lockdep would
+	 * incorrectly detect a deadlock, since we'd take a second lock
+	 * of the same class without releasing the first one.
+	 */
+	if (i2c_parent_is_i2c_adapter(client->adapter))
+		lockdep_set_subclass(&chip->i2c_lock, SINGLE_DEPTH_NESTING);
+
 	/* initialize cached registers from their original values.
 	 * we can't share this chip with another i2c master.
 	 */