[PATCH 0/3] gpiolib: fix a regression introduced by gpio_do_set_config()

[PATCH 0/3] gpiolib: fix a regression introduced by gpio_do_set_config()

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

These three patches fix a regression introduced by commit d90f36851d65
("gpiolib: have a single place of calling set_config()"). We first need
to revert patches that came on top of it, then apply the actual fix.

Bartosz Golaszewski (3):
  Revert "gpiolib: Remove duplicated function gpio_do_set_config()"
  Revert "gpiolib: remove set but not used variable 'config'"
  gpiolib: fix gpio_do_set_config()

 drivers/gpio/gpiolib.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

-- 
2.23.0

[PATCH 1/3] Revert "gpiolib: Remove duplicated function gpio_do_set_config()"

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This reverts commit d18fddff061d2796525e6d4a958cb3d30aed8efd.

This patch came on top of another patch that introduced a regression.
Revert it before addressing the culprit.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/gpio/gpiolib.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 99ac27a72e28..0673daeaca00 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3035,8 +3035,8 @@ EXPORT_SYMBOL_GPL(gpiochip_free_own_desc);
  * rely on gpio_request() having been called beforehand.
  */
 
-static int gpio_set_config(struct gpio_chip *gc, unsigned int offset,
-			   enum pin_config_param mode)
+static int gpio_do_set_config(struct gpio_chip *gc, unsigned int offset,
+			      enum pin_config_param mode)
 {
 	if (!gc->set_config)
 		return -ENOTSUPP;
@@ -3044,6 +3044,25 @@ static int gpio_set_config(struct gpio_chip *gc, unsigned int offset,
 	return gc->set_config(gc, offset, mode);
 }
 
+static int gpio_set_config(struct gpio_chip *gc, unsigned int offset,
+			   enum pin_config_param mode)
+{
+	unsigned arg;
+
+	switch (mode) {
+	case PIN_CONFIG_BIAS_DISABLE:
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+	case PIN_CONFIG_BIAS_PULL_UP:
+		arg = 1;
+		break;
+
+	default:
+		arg = 0;
+	}
+
+	return gpio_do_set_config(gc, offset, mode);
+}
+
 static int gpio_set_bias(struct gpio_chip *chip, struct gpio_desc *desc)
 {
 	int bias = 0;
@@ -3277,7 +3296,7 @@ int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
 	chip = desc->gdev->chip;
 
 	config = pinconf_to_config_packed(PIN_CONFIG_INPUT_DEBOUNCE, debounce);
-	return gpio_set_config(chip, gpio_chip_hwgpio(desc), config);
+	return gpio_do_set_config(chip, gpio_chip_hwgpio(desc), config);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_debounce);
 
@@ -3311,7 +3330,7 @@ int gpiod_set_transitory(struct gpio_desc *desc, bool transitory)
 	packed = pinconf_to_config_packed(PIN_CONFIG_PERSIST_STATE,
 					  !transitory);
 	gpio = gpio_chip_hwgpio(desc);
-	rc = gpio_set_config(chip, gpio, packed);
+	rc = gpio_do_set_config(chip, gpio, packed);
 	if (rc == -ENOTSUPP) {
 		dev_dbg(&desc->gdev->dev, "Persistence not supported for GPIO %d\n",
 				gpio);
-- 
2.23.0

[PATCH 2/3] Revert "gpiolib: remove set but not used variable 'config'"

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This reverts commit e5e42ad224a040f93bf112e96f82b3a0ed97ffab.

This patch came on top of another patch that introduced a regression.
Revert it before addressing the culprit.

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

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 0673daeaca00..13982056c14e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3047,6 +3047,7 @@ static int gpio_do_set_config(struct gpio_chip *gc, unsigned int offset,
 static int gpio_set_config(struct gpio_chip *gc, unsigned int offset,
 			   enum pin_config_param mode)
 {
+	unsigned long config;
 	unsigned arg;
 
 	switch (mode) {
@@ -3060,6 +3061,7 @@ static int gpio_set_config(struct gpio_chip *gc, unsigned int offset,
 		arg = 0;
 	}
 
+	config = PIN_CONF_PACKED(mode, arg);
 	return gpio_do_set_config(gc, offset, mode);
 }
 
-- 
2.23.0

[PATCH 3/3] gpiolib: fix gpio_do_set_config()

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Commit d90f36851d65 ("gpiolib: have a single place of calling
set_config()") introduced a regression where we don't pass the right
variable as argument to the set_config() callback of gpio driver from
gpio_set_config(). After reverting two additional patches that came
on top of it - this addresses the issue by changing the type of the last
argument of gpio_do_set_config() to unsigned long and making sure the
packed config variable is actually used in gpio_set_config().

Fixes: d90f36851d65 ("gpiolib: have a single place of calling set_config()")
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/gpio/gpiolib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 13982056c14e..760ae0707c01 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3036,12 +3036,12 @@ EXPORT_SYMBOL_GPL(gpiochip_free_own_desc);
  */
 
 static int gpio_do_set_config(struct gpio_chip *gc, unsigned int offset,
-			      enum pin_config_param mode)
+			      unsigned long config)
 {
 	if (!gc->set_config)
 		return -ENOTSUPP;
 
-	return gc->set_config(gc, offset, mode);
+	return gc->set_config(gc, offset, config);
 }
 
 static int gpio_set_config(struct gpio_chip *gc, unsigned int offset,
@@ -3062,7 +3062,7 @@ static int gpio_set_config(struct gpio_chip *gc, unsigned int offset,
 	}
 
 	config = PIN_CONF_PACKED(mode, arg);
-	return gpio_do_set_config(gc, offset, mode);
+	return gpio_do_set_config(gc, offset, config);
 }
 
 static int gpio_set_bias(struct gpio_chip *chip, struct gpio_desc *desc)
-- 
2.23.0

Re: [PATCH 0/3] gpiolib: fix a regression introduced by gpio_do_set_config()

[Andy Shevchenko]

On Mon, Feb 03, 2020 at 02:30:23PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> These three patches fix a regression introduced by commit d90f36851d65
> ("gpiolib: have a single place of calling set_config()"). We first need
> to revert patches that came on top of it, then apply the actual fix.

Thank you for addressing this!

It might be good to add Fixes / Depends-on to the first two, but I didn't look
if they are in any of v5.5 or older release.

> Bartosz Golaszewski (3):
>   Revert "gpiolib: Remove duplicated function gpio_do_set_config()"
>   Revert "gpiolib: remove set but not used variable 'config'"
>   gpiolib: fix gpio_do_set_config()
> 
>  drivers/gpio/gpiolib.c | 31 ++++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> -- 
> 2.23.0
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] gpiolib: fix a regression introduced by gpio_do_set_config()

[Bartosz Golaszewski]

pon., 3 lut 2020 o 15:31 Andy Shevchenko
<andriy.shevchenko@linux.intel.com> napisał(a):
>
> On Mon, Feb 03, 2020 at 02:30:23PM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > These three patches fix a regression introduced by commit d90f36851d65
> > ("gpiolib: have a single place of calling set_config()"). We first need
> > to revert patches that came on top of it, then apply the actual fix.
>
> Thank you for addressing this!
>
> It might be good to add Fixes / Depends-on to the first two, but I didn't look
> if they are in any of v5.5 or older release.
>

They're not - the patch in question was merged for v5.6 and then the
"fixes" came on top of it once it got into next. We're fine here IMO.

Bart

Re: [PATCH 0/3] gpiolib: fix a regression introduced by gpio_do_set_config()

[Andy Shevchenko]

On Mon, Feb 03, 2020 at 03:35:41PM +0100, Bartosz Golaszewski wrote:
> pon., 3 lut 2020 o 15:31 Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> napisał(a):
> >
> > On Mon, Feb 03, 2020 at 02:30:23PM +0100, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >
> > > These three patches fix a regression introduced by commit d90f36851d65
> > > ("gpiolib: have a single place of calling set_config()"). We first need
> > > to revert patches that came on top of it, then apply the actual fix.
> >
> > Thank you for addressing this!
> >
> > It might be good to add Fixes / Depends-on to the first two, but I didn't look
> > if they are in any of v5.5 or older release.
> >
> 
> They're not - the patch in question was merged for v5.6 and then the
> "fixes" came on top of it once it got into next. We're fine here IMO.

Good to know.

P.S. A bit of offtopic. Since you are going to send a PR for v5.6-rc2,
     perhaps you can include fixes for MAINTAINERS data base.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] gpiolib: fix a regression introduced by gpio_do_set_config()

[Guenter Roeck]

On Mon, Feb 03, 2020 at 02:30:23PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> These three patches fix a regression introduced by commit d90f36851d65
> ("gpiolib: have a single place of calling set_config()"). We first need
> to revert patches that came on top of it, then apply the actual fix.
> 
> Bartosz Golaszewski (3):
>   Revert "gpiolib: Remove duplicated function gpio_do_set_config()"
>   Revert "gpiolib: remove set but not used variable 'config'"
>   gpiolib: fix gpio_do_set_config()
> 

For the series:

Tested-by: Guenter Roeck <linux@roeck-us.net>

>  drivers/gpio/gpiolib.c | 31 ++++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> -- 
> 2.23.0
> 

Re: [PATCH 0/3] gpiolib: fix a regression introduced by gpio_do_set_config()

[Bartosz Golaszewski]

pon., 3 lut 2020 o 20:14 Guenter Roeck <linux@roeck-us.net> napisał(a):
>
> On Mon, Feb 03, 2020 at 02:30:23PM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > These three patches fix a regression introduced by commit d90f36851d65
> > ("gpiolib: have a single place of calling set_config()"). We first need
> > to revert patches that came on top of it, then apply the actual fix.
> >
> > Bartosz Golaszewski (3):
> >   Revert "gpiolib: Remove duplicated function gpio_do_set_config()"
> >   Revert "gpiolib: remove set but not used variable 'config'"
> >   gpiolib: fix gpio_do_set_config()
> >
>
> For the series:
>
> Tested-by: Guenter Roeck <linux@roeck-us.net>
>

Applied the patches. I'll send them to Linus W shortly.

Bartosz