Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1485148 > unrolled thread

[PATCH v2 4/4] gpio: pca953x: fix an incorrect lockdep warning

Started byBartosz Golaszewski <bgolaszewski@baylibre.com>
First post2016-09-16 18:10 +0200
Last post2016-09-23 10:20 +0200
Articles 3 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH v2 4/4] gpio: pca953x: fix an incorrect lockdep warning Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-09-16 18:10 +0200
    Re: [PATCH v2 4/4] gpio: pca953x: fix an incorrect lockdep warning Wolfram Sang <wsa@the-dreams.de> - 2016-09-21 07:50 +0200
      Re: [PATCH v2 4/4] gpio: pca953x: fix an incorrect lockdep warning Linus Walleij <linus.walleij@linaro.org> - 2016-09-23 10:20 +0200

#1485148 — [PATCH v2 4/4] gpio: pca953x: fix an incorrect lockdep warning

FromBartosz Golaszewski <bgolaszewski@baylibre.com>
Date2016-09-16 18:10 +0200
Subject[PATCH v2 4/4] gpio: pca953x: fix an incorrect lockdep warning
Message-ID<si3S9-6Ya-9@gated-at.bofh.it>
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, retrieve the adapter nesting depth
and use it as lockdep subclass for chip->i2c_lock.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/gpio/gpio-pca953x.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 02f2a56..892dc04 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -786,6 +786,8 @@ static int pca953x_probe(struct i2c_client *client,
 	chip->chip_type = PCA_CHIP_TYPE(chip->driver_data);
 
 	mutex_init(&chip->i2c_lock);
+	lockdep_set_subclass(&chip->i2c_lock,
+			     i2c_adapter_depth(client->adapter));
 
 	/* initialize cached registers from their original values.
 	 * we can't share this chip with another i2c master.
-- 
2.7.4

[toc] | [next] | [standalone]


#1487817

FromWolfram Sang <wsa@the-dreams.de>
Date2016-09-21 07:50 +0200
Message-ID<sjIzT-4Vo-9@gated-at.bofh.it>
In reply to#1485148

[Multipart message — attachments visible in raw view] — view raw

On Fri, Sep 16, 2016 at 06:02:45PM +0200, Bartosz Golaszewski wrote:
> 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, retrieve the adapter nesting depth
> and use it as lockdep subclass for chip->i2c_lock.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Linus, we'd like that in 4.9. Can I get your ack for the gpio part?

[toc] | [prev] | [next] | [standalone]


#1489834

FromLinus Walleij <linus.walleij@linaro.org>
Date2016-09-23 10:20 +0200
Message-ID<sktSa-1fi-23@gated-at.bofh.it>
In reply to#1487817
On Wed, Sep 21, 2016 at 7:45 AM, Wolfram Sang <wsa@the-dreams.de> wrote:

>> In order to get rid of the warning, retrieve the adapter nesting depth
>> and use it as lockdep subclass for chip->i2c_lock.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Linus, we'd like that in 4.9. Can I get your ack for the gpio part?

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Sorry for not attending quick enough, I was down in the
block layer.

Yours,
Linus Walleij

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web