linux-next: build failure after merge of the gpio tree

linux-next: build failure after merge of the gpio tree

[Stephen Rothwell]

Hi Linus,

After merging the gpio tree, today's linux-next build (x86_64
allmodconfig) failed like this:

drivers/regulator/rk808-regulator.c: In function 'rk808_regulator_dt_parse_pdata':
drivers/regulator/rk808-regulator.c:543:24: error: too few arguments to function 'gpiod_get_index'
   pdata->dvs_gpio[i] = gpiod_get_index(client_dev, "dvs", i);
                        ^
In file included from include/asm-generic/gpio.h:13:0,
                 from include/linux/gpio.h:51,
                 from drivers/regulator/rk808-regulator.c:20:
include/linux/gpio/consumer.h:53:32: note: declared here
 struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
                                ^

Caused by commit

  bad47ad2eef3 ("regulator: rk808: fixed the overshoot when adjust voltage")

from the regulator tree interactings with commit

  b17d1bf16cc7 ("gpio: make flags mandatory for gpiod_get functions")

from the gpio tree.

I added teh following merge fix patch:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 21 Jul 2015 13:23:56 +1000
Subject: [PATCH] regulator: rk808: fix up for gpiod_get_index() API change

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/regulator/rk808-regulator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/rk808-regulator.c b/drivers/regulator/rk808-regulator.c
index ca913fd15598..cfec52c1a4a2 100644
--- a/drivers/regulator/rk808-regulator.c
+++ b/drivers/regulator/rk808-regulator.c
@@ -540,7 +540,7 @@ static int rk808_regulator_dt_parse_pdata(struct device *dev,
 		goto dt_parse_end;
 
 	for (i = 0; i < ARRAY_SIZE(pdata->dvs_gpio); i++) {
-		pdata->dvs_gpio[i] = gpiod_get_index(client_dev, "dvs", i);
+		pdata->dvs_gpio[i] = gpiod_get_index(client_dev, "dvs", i, GPIOD_ASIS);
 		if (IS_ERR(pdata->dvs_gpio[i])) {
 			dev_warn(dev, "there is no dvs%d gpio\n", i);
 			continue;
-- 
2.1.4

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[PATCH] regulator: rk808: make better use of the gpiod API

[Uwe Kleine-König]

The gpiod functions include variants for managed gpiod resources. Use it
to simplify the remove function.

As the driver handles a device node without a specification of dvs gpios
just fine, additionally use the variant of gpiod_get exactly for this
use case. This makes error checking more strict.

As a third benefit this patch makes the driver use the flags parameter
of gpiod_get* which will not be optional any more after 4.2 and so
prevents a build failure when the respective gpiod commit is merged.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

this is the more complete fix of the issue that Stephen found while creating
next-20150721 (see commit f51ec04cf8be9f7ef795f1f39ada17c19f725650).

Best regards
Uwe

 drivers/regulator/rk808-regulator.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/regulator/rk808-regulator.c b/drivers/regulator/rk808-regulator.c
index ca913fd15598..3738e49beb75 100644
--- a/drivers/regulator/rk808-regulator.c
+++ b/drivers/regulator/rk808-regulator.c
@@ -94,7 +94,7 @@ static int rk808_buck1_2_get_voltage_sel_regmap(struct regulator_dev *rdev)
 	unsigned int val;
 	int ret;
 
-	if (IS_ERR(gpio) || gpiod_get_value(gpio) == 0)
+	if (!gpio || gpiod_get_value(gpio) == 0)
 		return regulator_get_voltage_sel_regmap(rdev);
 
 	ret = regmap_read(rdev->regmap,
@@ -168,7 +168,7 @@ static int rk808_buck1_2_set_voltage_sel(struct regulator_dev *rdev,
 	unsigned old_sel;
 	int ret, gpio_level;
 
-	if (IS_ERR(gpio))
+	if (!gpio)
 		return rk808_buck1_2_i2c_set_voltage_sel(rdev, sel);
 
 	gpio_level = gpiod_get_value(gpio);
@@ -205,7 +205,7 @@ static int rk808_buck1_2_set_voltage_time_sel(struct regulator_dev *rdev,
 	struct gpio_desc *gpio = pdata->dvs_gpio[id];
 
 	/* if there is no dvs1/2 pin, we don't need wait extra time here. */
-	if (IS_ERR(gpio))
+	if (!gpio)
 		return 0;
 
 	return regulator_set_voltage_time_sel(rdev, old_selector, new_selector);
@@ -540,14 +540,19 @@ static int rk808_regulator_dt_parse_pdata(struct device *dev,
 		goto dt_parse_end;
 
 	for (i = 0; i < ARRAY_SIZE(pdata->dvs_gpio); i++) {
-		pdata->dvs_gpio[i] = gpiod_get_index(client_dev, "dvs", i);
+		pdata->dvs_gpio[i] =
+			devm_gpiod_get_index_optional(client_dev, "dvs", i,
+						      GPIOD_OUT_LOW);
 		if (IS_ERR(pdata->dvs_gpio[i])) {
+			dev_err(dev, "failed to get dvs%d gpio\n", i);
+			return PTR_ERR(pdata->dvs_gpio[i]);
+		}
+
+		if (!pdata->dvs_gpio[i]) {
 			dev_warn(dev, "there is no dvs%d gpio\n", i);
 			continue;
 		}
 
-		gpiod_direction_output(pdata->dvs_gpio[i], 0);
-
 		tmp = i ? RK808_DVS2_POL : RK808_DVS1_POL;
 		ret = regmap_update_bits(map, RK808_IO_POL_REG, tmp,
 				gpiod_is_active_low(pdata->dvs_gpio[i]) ?
@@ -561,14 +566,6 @@ dt_parse_end:
 
 static int rk808_regulator_remove(struct platform_device *pdev)
 {
-	struct rk808_regulator_data *pdata = platform_get_drvdata(pdev);
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(pdata->dvs_gpio); i++) {
-		if (!IS_ERR(pdata->dvs_gpio[i]))
-			gpiod_put(pdata->dvs_gpio[i]);
-	}
-
 	return 0;
 }
 
-- 
2.1.4

Re: [PATCH] regulator: rk808: make better use of the gpiod API

[Linus Walleij]

On Tue, Jul 21, 2015 at 8:59 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:

> The gpiod functions include variants for managed gpiod resources. Use it
> to simplify the remove function.
>
> As the driver handles a device node without a specification of dvs gpios
> just fine, additionally use the variant of gpiod_get exactly for this
> use case. This makes error checking more strict.
>
> As a third benefit this patch makes the driver use the flags parameter
> of gpiod_get* which will not be optional any more after 4.2 and so
> prevents a build failure when the respective gpiod commit is merged.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
>
> this is the more complete fix of the issue that Stephen found while creating
> next-20150721 (see commit f51ec04cf8be9f7ef795f1f39ada17c19f725650).

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

Mark, please apply this.

Yours,
Linus Walleij

Re: [PATCH] regulator: rk808: make better use of the gpiod API

[Mark Brown]

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

On Tue, Jul 21, 2015 at 11:56:49AM +0200, Linus Walleij wrote:
> On Tue, Jul 21, 2015 at 8:59 AM, Uwe Kleine-König

> > The gpiod functions include variants for managed gpiod resources. Use it
> > to simplify the remove function.

> > As the driver handles a device node without a specification of dvs gpios
> > just fine, additionally use the variant of gpiod_get exactly for this
> > use case. This makes error checking more strict.

> > As a third benefit this patch makes the driver use the flags parameter
> > of gpiod_get* which will not be optional any more after 4.2 and so
> > prevents a build failure when the respective gpiod commit is merged.

> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Mark, please apply this.

...and reverted since it doesn't build as it's adding a use of a new API
which doesn't actually exist in Linus' tree yet (hopefully it's in
-next).  I need a tag to pull if I'm going to use this patch.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] regulator: rk808: make better use of the gpiod API

[Krzysztof Kozlowski]

2015-07-21 15:59 GMT+09:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> The gpiod functions include variants for managed gpiod resources. Use it
> to simplify the remove function.
>
> As the driver handles a device node without a specification of dvs gpios
> just fine, additionally use the variant of gpiod_get exactly for this
> use case. This makes error checking more strict.
>
> As a third benefit this patch makes the driver use the flags parameter
> of gpiod_get* which will not be optional any more after 4.2 and so
> prevents a build failure when the respective gpiod commit is merged.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
>
> this is the more complete fix of the issue that Stephen found while creating
> next-20150721 (see commit f51ec04cf8be9f7ef795f1f39ada17c19f725650).
>
> Best regards
> Uwe
>
>  drivers/regulator/rk808-regulator.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/regulator/rk808-regulator.c b/drivers/regulator/rk808-regulator.c
> index ca913fd15598..3738e49beb75 100644
> --- a/drivers/regulator/rk808-regulator.c
> +++ b/drivers/regulator/rk808-regulator.c
> @@ -94,7 +94,7 @@ static int rk808_buck1_2_get_voltage_sel_regmap(struct regulator_dev *rdev)
>         unsigned int val;
>         int ret;
>
> -       if (IS_ERR(gpio) || gpiod_get_value(gpio) == 0)
> +       if (!gpio || gpiod_get_value(gpio) == 0)
>                 return regulator_get_voltage_sel_regmap(rdev);
>
>         ret = regmap_read(rdev->regmap,
> @@ -168,7 +168,7 @@ static int rk808_buck1_2_set_voltage_sel(struct regulator_dev *rdev,
>         unsigned old_sel;
>         int ret, gpio_level;
>
> -       if (IS_ERR(gpio))
> +       if (!gpio)
>                 return rk808_buck1_2_i2c_set_voltage_sel(rdev, sel);
>
>         gpio_level = gpiod_get_value(gpio);
> @@ -205,7 +205,7 @@ static int rk808_buck1_2_set_voltage_time_sel(struct regulator_dev *rdev,
>         struct gpio_desc *gpio = pdata->dvs_gpio[id];
>
>         /* if there is no dvs1/2 pin, we don't need wait extra time here. */
> -       if (IS_ERR(gpio))
> +       if (!gpio)
>                 return 0;
>
>         return regulator_set_voltage_time_sel(rdev, old_selector, new_selector);
> @@ -540,14 +540,19 @@ static int rk808_regulator_dt_parse_pdata(struct device *dev,
>                 goto dt_parse_end;
>
>         for (i = 0; i < ARRAY_SIZE(pdata->dvs_gpio); i++) {
> -               pdata->dvs_gpio[i] = gpiod_get_index(client_dev, "dvs", i);
> +               pdata->dvs_gpio[i] =
> +                       devm_gpiod_get_index_optional(client_dev, "dvs", i,
> +                                                     GPIOD_OUT_LOW);
>                 if (IS_ERR(pdata->dvs_gpio[i])) {
> +                       dev_err(dev, "failed to get dvs%d gpio\n", i);

Missing of_node_put() from of_get_child_by_name() called before.

> +                       return PTR_ERR(pdata->dvs_gpio[i]);
> +               }
> +
> +               if (!pdata->dvs_gpio[i]) {
>                         dev_warn(dev, "there is no dvs%d gpio\n", i);
>                         continue;
>                 }
>
> -               gpiod_direction_output(pdata->dvs_gpio[i], 0);
> -
>                 tmp = i ? RK808_DVS2_POL : RK808_DVS1_POL;
>                 ret = regmap_update_bits(map, RK808_IO_POL_REG, tmp,
>                                 gpiod_is_active_low(pdata->dvs_gpio[i]) ?
> @@ -561,14 +566,6 @@ dt_parse_end:
>
>  static int rk808_regulator_remove(struct platform_device *pdev)
>  {
> -       struct rk808_regulator_data *pdata = platform_get_drvdata(pdev);
> -       int i;
> -
> -       for (i = 0; i < ARRAY_SIZE(pdata->dvs_gpio); i++) {
> -               if (!IS_ERR(pdata->dvs_gpio[i]))
> -                       gpiod_put(pdata->dvs_gpio[i]);
> -       }
> -
>         return 0;
>  }

The function looks empty so it can be removed entirely.

Best regards,
Krzysztof

Re: [PATCH] regulator: rk808: make better use of the gpiod API

[Uwe Kleine-König]

On Tue, Jul 21, 2015 at 12:04:15PM +0100, Mark Brown wrote:
> On Tue, Jul 21, 2015 at 11:56:49AM +0200, Linus Walleij wrote:
> > On Tue, Jul 21, 2015 at 8:59 AM, Uwe Kleine-König
> 
> > > The gpiod functions include variants for managed gpiod resources. Use it
> > > to simplify the remove function.
> 
> > > As the driver handles a device node without a specification of dvs gpios
> > > just fine, additionally use the variant of gpiod_get exactly for this
> > > use case. This makes error checking more strict.
> 
> > > As a third benefit this patch makes the driver use the flags parameter
> > > of gpiod_get* which will not be optional any more after 4.2 and so
> > > prevents a build failure when the respective gpiod commit is merged.
> 
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > 
> > Mark, please apply this.
> 
> ...and reverted since it doesn't build as it's adding a use of a new API
> which doesn't actually exist in Linus' tree yet (hopefully it's in
> -next).  I need a tag to pull if I'm going to use this patch.
If you're refering to the build bot warning I got, too, then the problem
is just a missing #include and the problem existed already before e.g.
for gpiod_get_value with the .config used by the build bot.

In reply to this mail I'll send a fixup patch for the missing include
and a reworked version of my patch that fixes the reference leak pointed
out by Krzysztof Kozlowski.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] regulator: rk808: make better use of the gpiod API

[Uwe Kleine-König]

Hello,

On Tue, Jul 21, 2015 at 10:09:32PM +0900, Krzysztof Kozlowski wrote:
> 2015-07-21 15:59 GMT+09:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> > @@ -540,14 +540,19 @@ static int rk808_regulator_dt_parse_pdata(struct device *dev,
> >                 goto dt_parse_end;
> >
> >         for (i = 0; i < ARRAY_SIZE(pdata->dvs_gpio); i++) {
> > -               pdata->dvs_gpio[i] = gpiod_get_index(client_dev, "dvs", i);
> > +               pdata->dvs_gpio[i] =
> > +                       devm_gpiod_get_index_optional(client_dev, "dvs", i,
> > +                                                     GPIOD_OUT_LOW);
> >                 if (IS_ERR(pdata->dvs_gpio[i])) {
> > +                       dev_err(dev, "failed to get dvs%d gpio\n", i);
> 
> Missing of_node_put() from of_get_child_by_name() called before.
Good catch, thanks.

> > @@ -561,14 +566,6 @@ dt_parse_end:
> >
> >  static int rk808_regulator_remove(struct platform_device *pdev)
> >  {
> > -       struct rk808_regulator_data *pdata = platform_get_drvdata(pdev);
> > -       int i;
> > -
> > -       for (i = 0; i < ARRAY_SIZE(pdata->dvs_gpio); i++) {
> > -               if (!IS_ERR(pdata->dvs_gpio[i]))
> > -                       gpiod_put(pdata->dvs_gpio[i]);
> > -       }
> > -
> >         return 0;
> >  }
> 
> The function looks empty so it can be removed entirely.
I assumed that not having a remove function makes the device not
detachable. Not sure about that.
Looking at the code I found that not having a remove function can yield
surprises, though. If your driver has a probe but no remove function the
platform bus glue calls

	dev_pm_domain_attach(_dev, true);

at probe time, but not

	dev_pm_domain_detach(_dev, true);

at remove. I admit I don't know about that dev_pm_domain stuff, but it
looks wrong to only have one but not the other. Greg?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] regulator: rk808: make better use of the gpiod API

[Mark Brown]

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

On Tue, Jul 21, 2015 at 04:35:24PM +0200, Uwe Kleine-König wrote:
> On Tue, Jul 21, 2015 at 10:09:32PM +0900, Krzysztof Kozlowski wrote:

> > The function looks empty so it can be removed entirely.

> I assumed that not having a remove function makes the device not
> detachable. Not sure about that.

No, of course not - the remove function is completely optional.

> Looking at the code I found that not having a remove function can yield
> surprises, though. If your driver has a probe but no remove function the
> platform bus glue calls

> 	dev_pm_domain_attach(_dev, true);

> at probe time, but not

> 	dev_pm_domain_detach(_dev, true);

> at remove. I admit I don't know about that dev_pm_domain stuff, but it
> looks wrong to only have one but not the other. Greg?

That looks like a bug, yes.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH v2 2/2] regulator: rk808: make better use of the gpiod API

[Uwe Kleine-König]

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

The gpiod functions include variants for managed gpiod resources. Use it
to simplify the remove function.

As the driver handles a device node without a specification of dvs gpios
just fine, additionally use the variant of gpiod_get exactly for this
use case. This makes error checking more strict.

As a third benefit this patch makes the driver use the flags parameter
of gpiod_get* which will not be optional any more after 4.2 and so
prevents a build failure when the respective gpiod commit is merged.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
note the above mentioned gpiod change is already in next, so the driver
fails to build there.

Changes since (implicit) v1, sent with
Message-Id: 1437461993-14860-1-git-send-email-u.kleine-koenig@pengutronix.de:

 - Assert that of_node_put is called in error path to not leak a reference
   and drop now empty remove callback.
   Thanks to Krzysztof Kozlowski for catching.


Best regards
Uwe
 drivers/regulator/rk808-regulator.c | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/regulator/rk808-regulator.c b/drivers/regulator/rk808-regulator.c
index ac9436d2ea8d..d86a3dcd61e2 100644
--- a/drivers/regulator/rk808-regulator.c
+++ b/drivers/regulator/rk808-regulator.c
@@ -95,7 +95,7 @@ static int rk808_buck1_2_get_voltage_sel_regmap(struct regulator_dev *rdev)
 	unsigned int val;
 	int ret;
 
-	if (IS_ERR(gpio) || gpiod_get_value(gpio) == 0)
+	if (!gpio || gpiod_get_value(gpio) == 0)
 		return regulator_get_voltage_sel_regmap(rdev);
 
 	ret = regmap_read(rdev->regmap,
@@ -169,7 +169,7 @@ static int rk808_buck1_2_set_voltage_sel(struct regulator_dev *rdev,
 	unsigned old_sel;
 	int ret, gpio_level;
 
-	if (IS_ERR(gpio))
+	if (!gpio)
 		return rk808_buck1_2_i2c_set_voltage_sel(rdev, sel);
 
 	gpio_level = gpiod_get_value(gpio);
@@ -206,7 +206,7 @@ static int rk808_buck1_2_set_voltage_time_sel(struct regulator_dev *rdev,
 	struct gpio_desc *gpio = pdata->dvs_gpio[id];
 
 	/* if there is no dvs1/2 pin, we don't need wait extra time here. */
-	if (IS_ERR(gpio))
+	if (!gpio)
 		return 0;
 
 	return regulator_set_voltage_time_sel(rdev, old_selector, new_selector);
@@ -541,14 +541,20 @@ static int rk808_regulator_dt_parse_pdata(struct device *dev,
 		goto dt_parse_end;
 
 	for (i = 0; i < ARRAY_SIZE(pdata->dvs_gpio); i++) {
-		pdata->dvs_gpio[i] = gpiod_get_index(client_dev, "dvs", i);
+		pdata->dvs_gpio[i] =
+			devm_gpiod_get_index_optional(client_dev, "dvs", i,
+						      GPIOD_OUT_LOW);
 		if (IS_ERR(pdata->dvs_gpio[i])) {
+			ret = PTR_ERR(pdata->dvs_gpio[i]);
+			dev_err(dev, "failed to get dvs%d gpio (%d)\n", i, ret);
+			goto dt_parse_end;
+		}
+
+		if (!pdata->dvs_gpio[i]) {
 			dev_warn(dev, "there is no dvs%d gpio\n", i);
 			continue;
 		}
 
-		gpiod_direction_output(pdata->dvs_gpio[i], 0);
-
 		tmp = i ? RK808_DVS2_POL : RK808_DVS1_POL;
 		ret = regmap_update_bits(map, RK808_IO_POL_REG, tmp,
 				gpiod_is_active_low(pdata->dvs_gpio[i]) ?
@@ -560,19 +566,6 @@ dt_parse_end:
 	return ret;
 }
 
-static int rk808_regulator_remove(struct platform_device *pdev)
-{
-	struct rk808_regulator_data *pdata = platform_get_drvdata(pdev);
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(pdata->dvs_gpio); i++) {
-		if (!IS_ERR(pdata->dvs_gpio[i]))
-			gpiod_put(pdata->dvs_gpio[i]);
-	}
-
-	return 0;
-}
-
 static int rk808_regulator_probe(struct platform_device *pdev)
 {
 	struct rk808 *rk808 = dev_get_drvdata(pdev->dev.parent);
@@ -619,7 +612,6 @@ static int rk808_regulator_probe(struct platform_device *pdev)
 
 static struct platform_driver rk808_regulator_driver = {
 	.probe = rk808_regulator_probe,
-	.remove = rk808_regulator_remove,
 	.driver = {
 		.name = "rk808-regulator",
 		.owner = THIS_MODULE,
-- 
2.1.4

Re: [PATCH] regulator: rk808: make better use of the gpiod API

[Krzysztof Kozlowski]

On 21.07.2015 23:41, Mark Brown wrote:
> On Tue, Jul 21, 2015 at 04:35:24PM +0200, Uwe Kleine-König wrote:
>> On Tue, Jul 21, 2015 at 10:09:32PM +0900, Krzysztof Kozlowski wrote:
> 
>>> The function looks empty so it can be removed entirely.
> 
>> I assumed that not having a remove function makes the device not
>> detachable. Not sure about that.
> 
> No, of course not - the remove function is completely optional.
> 
>> Looking at the code I found that not having a remove function can yield
>> surprises, though. If your driver has a probe but no remove function the
>> platform bus glue calls
> 
>> 	dev_pm_domain_attach(_dev, true);
> 
>> at probe time, but not
> 
>> 	dev_pm_domain_detach(_dev, true);
> 
>> at remove. I admit I don't know about that dev_pm_domain stuff, but it
>> looks wrong to only have one but not the other. Greg?
> 
> That looks like a bug, yes.

Cc: linux-pm, Kevin, Rafael, Ulf

I agree, device should be detached from domain regardless of presence of
remove callback.

Documentation (like Documentation/driver-model/driver.txt) does not
mention that remove callback is necessary for unbinding devices. There
is no sense in storing empty removal callbacks.

Best regards,
Krzysztof

Re: [PATCH v2 2/2] regulator: rk808: make better use of the gpiod API

[Krzysztof Kozlowski]

2015-07-21 23:46 GMT+09:00 Uwe Kleine-König <uwe@kleine-koenig.org>:
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>
> The gpiod functions include variants for managed gpiod resources. Use it
> to simplify the remove function.
>
> As the driver handles a device node without a specification of dvs gpios
> just fine, additionally use the variant of gpiod_get exactly for this
> use case. This makes error checking more strict.
>
> As a third benefit this patch makes the driver use the flags parameter
> of gpiod_get* which will not be optional any more after 4.2 and so
> prevents a build failure when the respective gpiod commit is merged.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> note the above mentioned gpiod change is already in next, so the driver
> fails to build there.
>
> Changes since (implicit) v1, sent with
> Message-Id: 1437461993-14860-1-git-send-email-u.kleine-koenig@pengutronix.de:
>
>  - Assert that of_node_put is called in error path to not leak a reference
>    and drop now empty remove callback.
>    Thanks to Krzysztof Kozlowski for catching.
>
>
> Best regards
> Uwe
>  drivers/regulator/rk808-regulator.c | 32 ++++++++++++--------------------
>  1 file changed, 12 insertions(+), 20 deletions(-)

Looks good now:
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [PATCH] regulator: rk808: make better use of the gpiod API

[Uwe Kleine-König]

Hello,

On Wed, Jul 22, 2015 at 09:13:51AM +0900, Krzysztof Kozlowski wrote:
> On 21.07.2015 23:41, Mark Brown wrote:
> > That looks like a bug, yes.
> 
> Cc: linux-pm, Kevin, Rafael, Ulf
Good idea. Note I already sent a patch to Greg that you might want to
look at, too:
	http://article.gmane.org/gmane.linux.kernel/2001345

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |