2009-04-07 12:00:34

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: leds: just ignore invalid GPIOs in leds-gpio

On Mon, 6 Apr 2009, Linux Kernel Mailing List wrote:
> Gitweb: http://git.kernel.org/linus/d379ee8acd0719736ee7f1d1ccc3b5765880eaf8
> Commit: d379ee8acd0719736ee7f1d1ccc3b5765880eaf8
> Parent: 7fbc3a9b132e93b2ba1fd889c1ad8a4135731cc3
> Author: David Brownell <[email protected]>
> AuthorDate: Thu Mar 5 16:46:44 2009 -0800
> Committer: Richard Purdie <[email protected]>
> CommitDate: Mon Apr 6 16:06:27 2009 +0100
>
> leds: just ignore invalid GPIOs in leds-gpio
>
> Sometimes it's awkward to make sure that the array in the
> platform_data handed to the leds-gpio driver has only valid
> data ... some leds may not be always available, and coping
> with that currently requires patching or rebuilding the array.
>
> This patch fixes that by making it be OK to pass an invalid
> GPIO (such as "-EINVAL") ... such table entries are skipped.
>
> [[email protected]: adjusted to apply against other led tree changes]
> Signed-off-by: David Brownell <[email protected]>
> Tested-by: Diego Dompe <[email protected]>
> Signed-off-by: Richard Purdie <[email protected]>
> ---
> drivers/leds/leds-gpio.c | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index 8fa352a..102ef4a 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -78,6 +78,13 @@ static int __devinit create_gpio_led(const struct gpio_led *template,
> {
> int ret;
>
> + /* skip leds that aren't available */
> + if (!gpio_is_valid(template->gpio)) {
> + printk(KERN_INFO "Skipping unavilable LED gpio %d (%s)\n",
> + template->gpio, template->name);
> + return;

| drivers/leds/leds-gpio.c:85: warning: 'return' with no value, in function
| returning non-void

So what should we return here? -ENODEV? -EINVAL? Anything else?

> + }
> +
> ret = gpio_request(template->gpio, template->name);
> if (ret < 0)
> return ret;

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010


2009-04-08 00:52:01

by David Brownell

[permalink] [raw]
Subject: Re: leds: just ignore invalid GPIOs in leds-gpio

On Tuesday 07 April 2009, Geert Uytterhoeven wrote:
> | drivers/leds/leds-gpio.c:85: warning: 'return' with no value, in function
> | returning non-void
>
> So what should we return here? -ENODEV? -EINVAL? Anything else?

Success: 0. The point is to ignore them, not fail!


See the appended.

- Dave

========== CUT HERE
From: David Brownell <[email protected]>

Fix build problems with leds-gpio:

CC drivers/leds/leds-gpio.o
drivers/leds/leds-gpio.c: In function 'create_gpio_led':
drivers/leds/leds-gpio.c:85: warning: 'return' with no value, in function returning non-void

Signed-off-by: David Brownell <[email protected]>

---
drivers/leds/leds-gpio.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -82,7 +84,7 @@ static int __devinit create_gpio_led(con
if (!gpio_is_valid(template->gpio)) {
printk(KERN_INFO "Skipping unavilable LED gpio %d (%s)\n",
template->gpio, template->name);
- return;
+ return 0;
}

ret = gpio_request(template->gpio, template->name);

2009-04-08 13:12:24

by Richard Purdie

[permalink] [raw]
Subject: Re: leds: just ignore invalid GPIOs in leds-gpio


On Tue, 2009-04-07 at 17:51 -0700, David Brownell wrote:
> On Tuesday 07 April 2009, Geert Uytterhoeven wrote:
> > | drivers/leds/leds-gpio.c:85: warning: 'return' with no value, in function
> > | returning non-void
> >
> > So what should we return here? -ENODEV? -EINVAL? Anything else?
>
> Success: 0. The point is to ignore them, not fail!
>
>
> See the appended.

Agreed. I wondered about returning ENODEV and then catching it in the
calling functions but that doesn't work out as very clear code. I'll
queue and submit this.

Cheers,

Richard