[PATCH v1 0/4] gpio: make gpiod_count() API consistent

[PATCH v1 0/4] gpio: make gpiod_count() API consistent

[Andy Shevchenko]

There are three possibilities in gpiod_count(): ACPI, OF, and
platform data.

Some of them return 0, which requires to be handled separately, though
developers rather lazy and just shadow an actual error code.

Let's make this API consistent by not allowing 0 in returned value.

There are luckily only 3 users right now, one of them handles this
properly, the rest is converted in this series.

Series is supposed to go through GPIO tree.

Andy Shevchenko (4):
  gpio: acpi: Don't return 0 on acpi_gpio_count()
  gpio: of: Don't return 0 on dt_gpio_count()
  platform/x86: surface3_button: Propagate error from gpiod_count()
  Input: soc_button_array - Propagate error from gpiod_count()

 drivers/gpio/gpiolib-acpi.c            | 4 ++--
 drivers/gpio/gpiolib.c                 | 4 ++--
 drivers/input/misc/soc_button_array.c  | 5 +++--
 drivers/platform/x86/surface3_button.c | 5 +++--
 4 files changed, 10 insertions(+), 8 deletions(-)

-- 
2.11.0

[PATCH v1 1/4] gpio: acpi: Don't return 0 on acpi_gpio_count()

[Andy Shevchenko]

It's unusual to have error checking like (ret <= 0) in cases when
counting GPIO resources. In case when it's mandatory we propagate the
error (-ENOENT), otherwise we don't use the result.

This makes consistent behaviour across all possible variants called in
gpiod_count().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 9b37a3692b3f..d7a22c470312 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1067,7 +1067,7 @@ int acpi_gpio_count(struct device *dev, const char *con_id)
 					break;
 				}
 		}
-		if (count >= 0)
+		if (count > 0)
 			break;
 	}
 
@@ -1083,7 +1083,7 @@ int acpi_gpio_count(struct device *dev, const char *con_id)
 		if (crs_count > 0)
 			count = crs_count;
 	}
-	return count;
+	return count ? count : -ENOENT;
 }
 
 struct acpi_crs_lookup {
-- 
2.11.0

Re: [PATCH v1 1/4] gpio: acpi: Don't return 0 on acpi_gpio_count()

[Linus Walleij]

On Mon, Feb 20, 2017 at 5:15 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> It's unusual to have error checking like (ret <= 0) in cases when
> counting GPIO resources. In case when it's mandatory we propagate the
> error (-ENOENT), otherwise we don't use the result.
>
> This makes consistent behaviour across all possible variants called in
> gpiod_count().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Mika/Rafael, can you look at this patch?

(Andy: sorry for late reply, busy merge window...)

Yours,
Linus Walleij

Re: [PATCH v1 1/4] gpio: acpi: Don't return 0 on acpi_gpio_count()

[Mika Westerberg]

On Tue, Mar 14, 2017 at 10:43:02AM +0100, Linus Walleij wrote:
> On Mon, Feb 20, 2017 at 5:15 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > It's unusual to have error checking like (ret <= 0) in cases when
> > counting GPIO resources. In case when it's mandatory we propagate the
> > error (-ENOENT), otherwise we don't use the result.
> >
> > This makes consistent behaviour across all possible variants called in
> > gpiod_count().
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Mika/Rafael, can you look at this patch?

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH v1 1/4] gpio: acpi: Don't return 0 on acpi_gpio_count()

[Linus Walleij]

On Mon, Feb 20, 2017 at 5:15 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> It's unusual to have error checking like (ret <= 0) in cases when
> counting GPIO resources. In case when it's mandatory we propagate the
> error (-ENOENT), otherwise we don't use the result.
>
> This makes consistent behaviour across all possible variants called in
> gpiod_count().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Patch applied with Mika's ACK.

Yours,
Linus Walleij

[PATCH v1 2/4] gpio: of: Don't return 0 on dt_gpio_count()

[Andy Shevchenko]

It's unusual to have error checking like (ret <= 0) in cases when
counting GPIO resources. In case when it's mandatory we propagate the
error (-ENOENT), otherwise we don't use the result.

This makes consistent behaviour across all possible variants called in
gpiod_count().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 8b4d721d6d63..f8ee417de0b7 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3122,10 +3122,10 @@ static int dt_gpio_count(struct device *dev, const char *con_id)
 				 gpio_suffixes[i]);
 
 		ret = of_gpio_named_count(dev->of_node, propname);
-		if (ret >= 0)
+		if (ret > 0)
 			break;
 	}
-	return ret;
+	return ret ? ret : -ENOENT;
 }
 
 static int platform_gpio_count(struct device *dev, const char *con_id)
-- 
2.11.0

Re: [PATCH v1 2/4] gpio: of: Don't return 0 on dt_gpio_count()

[Linus Walleij]

On Mon, Feb 20, 2017 at 5:15 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> It's unusual to have error checking like (ret <= 0) in cases when
> counting GPIO resources. In case when it's mandatory we propagate the
> error (-ENOENT), otherwise we don't use the result.
>
> This makes consistent behaviour across all possible variants called in
> gpiod_count().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Patch applied.

Yours,
Linus Walleij

[PATCH v1 3/4] platform/x86: surface3_button: Propagate error from gpiod_count()

[Andy Shevchenko]

Since gpiod_count() does not return 0 anymore, we don't need to shadow
its error code and would safely propagate to the user.

While here, replace second parameter by NULL in order to prevent side
effects on _DSD enabled firmware.

Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/platform/x86/surface3_button.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/surface3_button.c b/drivers/platform/x86/surface3_button.c
index 8bfd7f613d36..57f51476bb65 100644
--- a/drivers/platform/x86/surface3_button.c
+++ b/drivers/platform/x86/surface3_button.c
@@ -196,9 +196,10 @@ static int surface3_button_probe(struct i2c_client *client,
 		    strlen(SURFACE_BUTTON_OBJ_NAME)))
 		return -ENODEV;
 
-	if (gpiod_count(dev, KBUILD_MODNAME) <= 0) {
+	error = gpiod_count(dev, NULL);
+	if (error < 0) {
 		dev_dbg(dev, "no GPIO attached, ignoring...\n");
-		return -ENODEV;
+		return error;
 	}
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
-- 
2.11.0

Re: [PATCH v1 3/4] platform/x86: surface3_button: Propagate error from gpiod_count()

[Linus Walleij]

On Mon, Feb 20, 2017 at 5:15 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Since gpiod_count() does not return 0 anymore, we don't need to shadow
> its error code and would safely propagate to the user.
>
> While here, replace second parameter by NULL in order to prevent side
> effects on _DSD enabled firmware.
>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

As I understand it Andy, you're more or less default-maintainer for
drivers/platform/x86 so I can just merge this patch into the
GPIO tree with the rest?

Yours,
Linus Walleij

Re: [PATCH v1 3/4] platform/x86: surface3_button: Propagate error from gpiod_count()

[Andy Shevchenko]

On Tue, 2017-03-14 at 10:46 +0100, Linus Walleij wrote:
> On Mon, Feb 20, 2017 at 5:15 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > Since gpiod_count() does not return 0 anymore, we don't need to
> > shadow
> > its error code and would safely propagate to the user.
> > 
> > While here, replace second parameter by NULL in order to prevent
> > side
> > effects on _DSD enabled firmware.
> > 
> > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> As I understand it Andy, you're more or less default-maintainer for
> drivers/platform/x86 so I can just merge this patch into the
> GPIO tree with the rest?

Correct!

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v1 3/4] platform/x86: surface3_button: Propagate error from gpiod_count()

[Linus Walleij]

On Mon, Feb 20, 2017 at 5:15 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Since gpiod_count() does not return 0 anymore, we don't need to shadow
> its error code and would safely propagate to the user.
>
> While here, replace second parameter by NULL in order to prevent side
> effects on _DSD enabled firmware.
>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Patch applied to the GPIO tree.

Yours,
Linus Walleij

[PATCH v1 4/4] Input: soc_button_array - Propagate error from gpiod_count()

[Andy Shevchenko]

Since gpiod_count() does not return 0 anymore, we don't need to shadow
its error code and would safely propagate to the user.

While here, replace second parameter by NULL in order to prevent side
effects on _DSD enabled firmware.

Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/input/misc/soc_button_array.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index ddb2f22fca7a..c3b8e1fb4699 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -169,9 +169,10 @@ static int soc_button_probe(struct platform_device *pdev)
 
 	button_info = (struct soc_button_info *)id->driver_data;
 
-	if (gpiod_count(dev, KBUILD_MODNAME) <= 0) {
+	error = gpiod_count(dev, NULL);
+	if (error < 0) {
 		dev_dbg(dev, "no GPIO attached, ignoring...\n");
-		return -ENODEV;
+		return error;
 	}
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
-- 
2.11.0

Re: [PATCH v1 4/4] Input: soc_button_array - Propagate error from gpiod_count()

[Dmitry Torokhov]

On Mon, Feb 20, 2017 at 06:15:49PM +0200, Andy Shevchenko wrote:
> Since gpiod_count() does not return 0 anymore, we don't need to shadow
> its error code and would safely propagate to the user.
> 
> While here, replace second parameter by NULL in order to prevent side
> effects on _DSD enabled firmware.
> 
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> ---
>  drivers/input/misc/soc_button_array.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
> index ddb2f22fca7a..c3b8e1fb4699 100644
> --- a/drivers/input/misc/soc_button_array.c
> +++ b/drivers/input/misc/soc_button_array.c
> @@ -169,9 +169,10 @@ static int soc_button_probe(struct platform_device *pdev)
>  
>  	button_info = (struct soc_button_info *)id->driver_data;
>  
> -	if (gpiod_count(dev, KBUILD_MODNAME) <= 0) {
> +	error = gpiod_count(dev, NULL);
> +	if (error < 0) {
>  		dev_dbg(dev, "no GPIO attached, ignoring...\n");
> -		return -ENODEV;
> +		return error;
>  	}
>  
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> -- 
> 2.11.0
> 

-- 
Dmitry

Re: [PATCH v1 4/4] Input: soc_button_array - Propagate error from gpiod_count()

[Linus Walleij]

On Mon, Feb 20, 2017 at 5:15 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Since gpiod_count() does not return 0 anymore, we don't need to shadow
> its error code and would safely propagate to the user.
>
> While here, replace second parameter by NULL in order to prevent side
> effects on _DSD enabled firmware.
>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Patch applied with Dmitry's ACK.

Yours,
Linus Walleij

Re: [PATCH v1 0/4] gpio: make gpiod_count() API consistent

[Benjamin Tissoires]

On Feb 20 2017 or thereabouts, Andy Shevchenko wrote:
> There are three possibilities in gpiod_count(): ACPI, OF, and
> platform data.
> 
> Some of them return 0, which requires to be handled separately, though
> developers rather lazy and just shadow an actual error code.
> 
> Let's make this API consistent by not allowing 0 in returned value.
> 
> There are luckily only 3 users right now, one of them handles this
> properly, the rest is converted in this series.
> 
> Series is supposed to go through GPIO tree.
> 
> Andy Shevchenko (4):
>   gpio: acpi: Don't return 0 on acpi_gpio_count()
>   gpio: of: Don't return 0 on dt_gpio_count()
>   platform/x86: surface3_button: Propagate error from gpiod_count()
>   Input: soc_button_array - Propagate error from gpiod_count()

Not sure if this still matters, but still:
Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

> 
>  drivers/gpio/gpiolib-acpi.c            | 4 ++--
>  drivers/gpio/gpiolib.c                 | 4 ++--
>  drivers/input/misc/soc_button_array.c  | 5 +++--
>  drivers/platform/x86/surface3_button.c | 5 +++--
>  4 files changed, 10 insertions(+), 8 deletions(-)
> 
> -- 
> 2.11.0
> 

Re: [PATCH v1 0/4] gpio: make gpiod_count() API consistent

[Andy Shevchenko]

On Mon, 2017-02-27 at 09:27 +0100, Benjamin Tissoires wrote:
> On Feb 20 2017 or thereabouts, Andy Shevchenko wrote:
> > There are three possibilities in gpiod_count(): ACPI, OF, and
> > platform data.
> > 
> > Some of them return 0, which requires to be handled separately,
> > though
> > developers rather lazy and just shadow an actual error code.
> > 
> > Let's make this API consistent by not allowing 0 in returned value.
> > 
> > There are luckily only 3 users right now, one of them handles this
> > properly, the rest is converted in this series.
> > 
> > Series is supposed to go through GPIO tree.
> > 
> > Andy Shevchenko (4):
> >   gpio: acpi: Don't return 0 on acpi_gpio_count()
> >   gpio: of: Don't return 0 on dt_gpio_count()
> >   platform/x86: surface3_button: Propagate error from gpiod_count()
> >   Input: soc_button_array - Propagate error from gpiod_count()
> 
> Not sure if this still matters, but still:
> Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

I'm sure it is.

Linus, is your plan to go through queue after merge window is closed?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v1 0/4] gpio: make gpiod_count() API consistent

[Linus Walleij]

On Tue, Feb 28, 2017 at 10:48 AM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Mon, 2017-02-27 at 09:27 +0100, Benjamin Tissoires wrote:
>> On Feb 20 2017 or thereabouts, Andy Shevchenko wrote:
>> > There are three possibilities in gpiod_count(): ACPI, OF, and
>> > platform data.
>> >
>> > Some of them return 0, which requires to be handled separately,
>> > though
>> > developers rather lazy and just shadow an actual error code.
>> >
>> > Let's make this API consistent by not allowing 0 in returned value.
>> >
>> > There are luckily only 3 users right now, one of them handles this
>> > properly, the rest is converted in this series.
>> >
>> > Series is supposed to go through GPIO tree.
>> >
>> > Andy Shevchenko (4):
>> >   gpio: acpi: Don't return 0 on acpi_gpio_count()
>> >   gpio: of: Don't return 0 on dt_gpio_count()
>> >   platform/x86: surface3_button: Propagate error from gpiod_count()
>> >   Input: soc_button_array - Propagate error from gpiod_count()
>>
>> Not sure if this still matters, but still:
>> Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> I'm sure it is.
>
> Linus, is your plan to go through queue after merge window is closed?

Yes sorry for the delay, it was a busy merge window etc.

I'd like to have some nod from Mika/Rafael that this is what
we want to do. If I don't hear anything I guess I will just
merge them, it looks right to me.

Yours,
Linus Walleij