[PATCH] gpio: pca953x: add a comment explaining the need for a lockdep subclass

[PATCH] gpio: pca953x: add a comment explaining the need for a lockdep subclass

[Bartosz Golaszewski]

Since there's an rc8 I thought I'd send a follow-up patch to the
series addressing the lockdep false positive in pca953x.

The reason for setting the subclass in the probe function is not
explained in the code nor is it obvious at first glance. This patch
adds a comment explaining the problem.

Rebased on top of current i2c/for-next.

Bartosz Golaszewski (1):
  gpio: pca953x: add a comment explaining the need for a lockdep
    subclass

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

-- 
2.7.4

[PATCH] gpio: pca953x: add a comment explaining the need for a lockdep subclass

[Bartosz Golaszewski]

This is a follow-up to commit 559b46990e76 ("gpio: pca953x: fix an
incorrect lockdep warning"). The reason for calling
lockdep_set_subclass() in pca953x_probe() is not explained in
the code.

Add a comment describing the problem, partial solution and required
future extensions.

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

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 018f39c..f306909 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -786,6 +786,22 @@ static int pca953x_probe(struct i2c_client *client,
 	chip->chip_type = PCA_CHIP_TYPE(chip->driver_data);
 
 	mutex_init(&chip->i2c_lock);
+	/*
+	 * In case we have an i2c-mux controlled by a GPIO provided by an
+	 * expander using the same driver higher on the device tree, read the
+	 * i2c adapter nesting depth and use the retrieved value as lockdep
+	 * subclass for chip->i2c_lock.
+	 *
+	 * REVISIT: This solution is not complete. It protects us from lockdep
+	 * false positives when the expander controlling the i2c-mux is on
+	 * a different level on the device tree, but not when it's on the same
+	 * level on a different branch (in which case the subclass number
+	 * would be the same).
+	 *
+	 * TODO: Once a correct solution is developed, a similar fix should be
+	 * applied to all other i2c-controlled GPIO expanders (and potentially
+	 * regmap-i2c).
+	 */
 	lockdep_set_subclass(&chip->i2c_lock,
 			     i2c_adapter_depth(client->adapter));
 
-- 
2.7.4

Re: [PATCH] gpio: pca953x: add a comment explaining the need for a lockdep subclass

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 553 bytes --]

On Mon, Sep 26, 2016 at 11:54:15AM +0200, Bartosz Golaszewski wrote:
> This is a follow-up to commit 559b46990e76 ("gpio: pca953x: fix an
> incorrect lockdep warning"). The reason for calling
> lockdep_set_subclass() in pca953x_probe() is not explained in
> the code.
> 
> Add a comment describing the problem, partial solution and required
> future extensions.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Acked-by: Wolfram Sang <wsa@the-dreams.de>

Linus, because of dependencies, I should probably pick it up?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] gpio: pca953x: add a comment explaining the need for a lockdep subclass

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 660 bytes --]

On Mon, Sep 26, 2016 at 12:00:30PM +0200, Wolfram Sang wrote:
> On Mon, Sep 26, 2016 at 11:54:15AM +0200, Bartosz Golaszewski wrote:
> > This is a follow-up to commit 559b46990e76 ("gpio: pca953x: fix an
> > incorrect lockdep warning"). The reason for calling
> > lockdep_set_subclass() in pca953x_probe() is not explained in
> > the code.
> > 
> > Add a comment describing the problem, partial solution and required
> > future extensions.
> > 
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Acked-by: Wolfram Sang <wsa@the-dreams.de>
> 
> Linus, because of dependencies, I should probably pick it up?

Linus, ping!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] gpio: pca953x: add a comment explaining the need for a lockdep subclass

[Bartosz Golaszewski]

2016-09-29 8:06 GMT+02:00 Wolfram Sang <wsa@the-dreams.de>:
> On Mon, Sep 26, 2016 at 12:00:30PM +0200, Wolfram Sang wrote:
>> On Mon, Sep 26, 2016 at 11:54:15AM +0200, Bartosz Golaszewski wrote:
>> > This is a follow-up to commit 559b46990e76 ("gpio: pca953x: fix an
>> > incorrect lockdep warning"). The reason for calling
>> > lockdep_set_subclass() in pca953x_probe() is not explained in
>> > the code.
>> >
>> > Add a comment describing the problem, partial solution and required
>> > future extensions.
>> >
>> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>
>> Acked-by: Wolfram Sang <wsa@the-dreams.de>
>>
>> Linus, because of dependencies, I should probably pick it up?
>
> Linus, ping!
>

Hi Wolfram,

this patch will not apply to the gpio tree. If Linus is too busy to
comment, maybe you could pick it up anyway - it doesn't change
anything in terms of functionality and it's better to have the code
commented when it's not obvious what it does.

Thanks,
Bartosz

Re: [PATCH] gpio: pca953x: add a comment explaining the need for a lockdep subclass

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1237 bytes --]

On Thu, Sep 29, 2016 at 09:56:58AM +0200, Bartosz Golaszewski wrote:
> 2016-09-29 8:06 GMT+02:00 Wolfram Sang <wsa@the-dreams.de>:
> > On Mon, Sep 26, 2016 at 12:00:30PM +0200, Wolfram Sang wrote:
> >> On Mon, Sep 26, 2016 at 11:54:15AM +0200, Bartosz Golaszewski wrote:
> >> > This is a follow-up to commit 559b46990e76 ("gpio: pca953x: fix an
> >> > incorrect lockdep warning"). The reason for calling
> >> > lockdep_set_subclass() in pca953x_probe() is not explained in
> >> > the code.
> >> >
> >> > Add a comment describing the problem, partial solution and required
> >> > future extensions.
> >> >
> >> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >>
> >> Acked-by: Wolfram Sang <wsa@the-dreams.de>
> >>
> >> Linus, because of dependencies, I should probably pick it up?
> >
> > Linus, ping!
> >
> 
> Hi Wolfram,
> 
> this patch will not apply to the gpio tree. If Linus is too busy to
> comment, maybe you could pick it up anyway - it doesn't change
> anything in terms of functionality and it's better to have the code
> commented when it's not obvious what it does.

I want to pick it up and am likely to do so anyway, but to be formally
perfect, I need an ack from Linus.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] gpio: pca953x: add a comment explaining the need for a lockdep subclass

[Linus Walleij]

On Mon, Sep 26, 2016 at 12:00 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Mon, Sep 26, 2016 at 11:54:15AM +0200, Bartosz Golaszewski wrote:
>> This is a follow-up to commit 559b46990e76 ("gpio: pca953x: fix an
>> incorrect lockdep warning"). The reason for calling
>> lockdep_set_subclass() in pca953x_probe() is not explained in
>> the code.
>>
>> Add a comment describing the problem, partial solution and required
>> future extensions.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Acked-by: Wolfram Sang <wsa@the-dreams.de>
>
> Linus, because of dependencies, I should probably pick it up?

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

Sorry was travelling.

Yours,
Linus Walleij

Re: [PATCH] gpio: pca953x: add a comment explaining the need for a lockdep subclass

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 477 bytes --]

On Mon, Sep 26, 2016 at 11:54:15AM +0200, Bartosz Golaszewski wrote:
> This is a follow-up to commit 559b46990e76 ("gpio: pca953x: fix an
> incorrect lockdep warning"). The reason for calling
> lockdep_set_subclass() in pca953x_probe() is not explained in
> the code.
> 
> Add a comment describing the problem, partial solution and required
> future extensions.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]