linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
To: Linus Walleij <linus.walleij@linaro.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Vignesh R <vigneshr@ti.com>, Yong Li <yong.b.li@intel.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>, Wolfram Sang <wsa@the-dreams.de>
Cc: linux-i2c <linux-i2c@vger.kernel.org>,
	linux-gpio <linux-gpio@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>
Subject: [PATCH] gpio: pca953x: fix an incorrect lockdep warning
Date: Fri, 16 Sep 2016 16:18:04 +0200	[thread overview]
Message-ID: <1474035484-25134-1-git-send-email-bgolaszewski@baylibre.com> (raw)

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

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.
 	 */
-- 
2.7.4


             reply	other threads:[~2016-09-16 14:18 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-16 14:18 Bartosz Golaszewski [this message]
2016-09-16 14:33 ` [PATCH] gpio: pca953x: fix an incorrect lockdep warning Peter Rosin
2016-09-16 14:40   ` Bartosz Golaszewski
2016-09-16 16:04 ` Bartosz Golaszewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1474035484-25134-1-git-send-email-bgolaszewski@baylibre.com \
    --to=bgolaszewski@baylibre.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=geert+renesas@glider.be \
    --cc=gnurou@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=vigneshr@ti.com \
    --cc=wsa@the-dreams.de \
    --cc=yong.b.li@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).