* [PATCH 0/2] Two fixes for leds-tca6507
@ 2011-12-19 0:20 NeilBrown
2011-12-19 0:20 ` [PATCH 2/2] leds-tca6507 - fix off by one error NeilBrown
2011-12-19 0:20 ` [PATCH 1/2] leds-tca6507: allow driver to compile when GPIOLIB is not available NeilBrown
0 siblings, 2 replies; 5+ messages in thread
From: NeilBrown @ 2011-12-19 0:20 UTC (permalink / raw)
To: Andrew Morton
Cc: rpurdie, linux-kernel, Randy Dunlap, Stephen Rothwell,
Dan Carpenter
The driver didn't compile on arches which did not define GPIOLIB.
This this by excluding gpio support on those archs.
Reported by: Randy Dunlap <rdunlap@xenotime.net> and
Stephen Rothwell <sfr@canb.auug.org.au>
off-by-one error in choose_times
Reported by: Dan Carpenter <dan.carpenter@oracle.com>
Thanks,
NeilBrown
---
NeilBrown (2):
leds-tca6507 - fix off by one error.
leds-tca6507: allow driver to compile when GPIOLIB is not available.
drivers/leds/leds-tca6507.c | 83 +++++++++++++++++++++++++++++-------------
include/linux/leds-tca6507.h | 3 +-
2 files changed, 59 insertions(+), 27 deletions(-)
--
Signature
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] leds-tca6507: allow driver to compile when GPIOLIB is not available.
2011-12-19 0:20 [PATCH 0/2] Two fixes for leds-tca6507 NeilBrown
2011-12-19 0:20 ` [PATCH 2/2] leds-tca6507 - fix off by one error NeilBrown
@ 2011-12-19 0:20 ` NeilBrown
2011-12-19 20:34 ` Randy Dunlap
1 sibling, 1 reply; 5+ messages in thread
From: NeilBrown @ 2011-12-19 0:20 UTC (permalink / raw)
To: Andrew Morton; +Cc: rpurdie, linux-kernel, Randy Dunlap, Stephen Rothwell
This driver can configure the outputs as GPIO line instead of LEDs. But
that only works if GPIOLIB is available. So make that code conditional
on the library's availability.
Also remove the 'teardown' callback as it is never called and should
never be needed.
Reported-by: Randy Dunlap <rdunlap@xenotime.net>,
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>,
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/leds/leds-tca6507.c | 75 +++++++++++++++++++++++++++++-------------
include/linux/leds-tca6507.h | 3 +-
2 files changed, 53 insertions(+), 25 deletions(-)
diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c
index a7ea4cf..c9cc3f7 100644
--- a/drivers/leds/leds-tca6507.c
+++ b/drivers/leds/leds-tca6507.c
@@ -149,9 +149,11 @@ struct tca6507_chip {
int bank; /* Bank used, or -1 */
int blink; /* 1 if we are hardware-blinking */
} leds[NUM_LEDS];
+#ifdef CONFIG_GPIOLIB
struct gpio_chip gpio;
const char *gpio_name[NUM_LEDS];
int gpio_map[NUM_LEDS];
+#endif
};
static const struct i2c_device_id tca6507_id[] = {
@@ -520,6 +522,7 @@ static int tca6507_blink_set(struct led_classdev *led_cdev,
return 0;
}
+#ifdef CONFIG_GPIOLIB
static void tca6507_gpio_set_value(struct gpio_chip *gc,
unsigned offset, int val)
{
@@ -542,6 +545,51 @@ static int tca6507_gpio_direction_output(struct gpio_chip *gc,
tca6507_gpio_set_value(gc, offset, val);
return 0;
}
+static int tca6507_probe_gpios(struct i2c_client *client,
+ struct tca6507_chip *tca,
+ struct tca6507_platform_data *pdata)
+{
+ int err;
+ int i = 0;
+ int gpios = 0;
+
+ for (i = 0; i < NUM_LEDS; i++)
+ if (pdata->leds.leds[i].name && pdata->leds.leds[i].flags) {
+ /* Configure as a gpio */
+ tca->gpio_name[gpios] = pdata->leds.leds[i].name;
+ tca->gpio_map[gpios] = i;
+ gpios++;
+ }
+
+ if (!gpios)
+ return 0;
+
+ tca->gpio.label = "gpio-tca6507";
+ tca->gpio.names = tca->gpio_name;
+ tca->gpio.ngpio = gpios;
+ tca->gpio.base = pdata->gpio_base;
+ tca->gpio.owner = THIS_MODULE;
+ tca->gpio.direction_output = tca6507_gpio_direction_output;
+ tca->gpio.set = tca6507_gpio_set_value;
+ tca->gpio.dev = &client->dev;
+ err = gpiochip_add(&tca->gpio);
+ if (err) {
+ tca->gpio.ngpio = 0;
+ return err;
+ }
+ if (pdata->setup)
+ pdata->setup(tca->gpio.base, tca->gpio.ngpio);
+ return 0;
+}
+
+#else /* CONFIG_GPIOLIB */
+static int tca6507_probe_gpios(struct i2c_client *client,
+ struct tca6507_chip *tca,
+ struct tca6507_platform_data *pdata)
+{
+ return 0;
+}
+#endif /* CONFIG_GPIOLIB */
static int __devinit tca6507_probe(struct i2c_client *client,
const struct i2c_device_id *id)
@@ -551,7 +599,6 @@ static int __devinit tca6507_probe(struct i2c_client *client,
struct tca6507_platform_data *pdata;
int err;
int i = 0;
- int gpios = 0;
adapter = to_i2c_adapter(client->dev.parent);
pdata = client->dev.platform_data;
@@ -590,30 +637,10 @@ static int __devinit tca6507_probe(struct i2c_client *client,
if (err < 0)
goto exit;
}
- if (pdata->leds.leds[i].name && pdata->leds.leds[i].flags) {
- /* Configure as a gpio */
- tca->gpio_name[gpios] = pdata->leds.leds[i].name;
- tca->gpio_map[gpios] = i;
- gpios++;
- }
- }
- if (gpios) {
- tca->gpio.label = "gpio-tca6507";
- tca->gpio.names = tca->gpio_name;
- tca->gpio.ngpio = gpios;
- tca->gpio.base = pdata->gpio_base;
- tca->gpio.owner = THIS_MODULE;
- tca->gpio.direction_output = tca6507_gpio_direction_output;
- tca->gpio.set = tca6507_gpio_set_value;
- tca->gpio.dev = &client->dev;
- err = gpiochip_add(&tca->gpio);
- if (err) {
- tca->gpio.ngpio = 0;
- goto exit;
- }
- if (pdata->setup)
- pdata->setup(tca->gpio.base, tca->gpio.ngpio);
}
+ err = tca6507_probe_gpios(client, tca, pdata);
+ if (err)
+ goto exit;
i2c_set_clientdata(client, tca);
/* set all registers to known state - zero */
tca->reg_set = 0x7f;
diff --git a/include/linux/leds-tca6507.h b/include/linux/leds-tca6507.h
index 3b8ac62..dcabf4f 100644
--- a/include/linux/leds-tca6507.h
+++ b/include/linux/leds-tca6507.h
@@ -24,9 +24,10 @@
struct tca6507_platform_data {
struct led_platform_data leds;
+#ifdef CONFIG_GPIOLIB
int gpio_base;
void (*setup)(unsigned gpio_base, unsigned ngpio);
- void (*teardown)(unsigned gpio_base, unsigned ngpio);
+#endif
};
#define TCA6507_MAKE_GPIO 1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] leds-tca6507 - fix off by one error.
2011-12-19 0:20 [PATCH 0/2] Two fixes for leds-tca6507 NeilBrown
@ 2011-12-19 0:20 ` NeilBrown
2011-12-19 0:20 ` [PATCH 1/2] leds-tca6507: allow driver to compile when GPIOLIB is not available NeilBrown
1 sibling, 0 replies; 5+ messages in thread
From: NeilBrown @ 2011-12-19 0:20 UTC (permalink / raw)
To: Andrew Morton; +Cc: rpurdie, linux-kernel, Dan Carpenter
When walking the list of possible time codes we can fall off the end.
Fix that, and also improved the commentary.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/leds/leds-tca6507.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c
index c9cc3f7..75dc5e4 100644
--- a/drivers/leds/leds-tca6507.c
+++ b/drivers/leds/leds-tca6507.c
@@ -165,7 +165,8 @@ MODULE_DEVICE_TABLE(i2c, tca6507);
static int choose_times(int msec, int *c1p, int *c2p)
{
/* Chose two timecodes which add to 'msec' as near as possible.
- * The first returned should be the larger.
+ * The first returned should be the larger and is the 'on' of 'off' time.
+ * The second will be used as a 'fade-on' or 'fade-off' time.
* If cannot get within 1/8, fail.
* If two possibilities are equally good (e.g. 512+0, 256+256), choose
* the first pair so there is more change-time visible (i.e. it is softer).
@@ -175,7 +176,10 @@ static int choose_times(int msec, int *c1p, int *c2p)
int tmin = msec * 7 / 8;
int diff = 65536;
- for (c1 = 1; c1 <= TIMECODES; c1++) {
+ /* We start at '1' to ensure we never even think of choosing a
+ * total time of '0'.
+ */
+ for (c1 = 1; c1 < TIMECODES; c1++) {
int t = time_codes[c1];
if (t*2 < tmin)
continue;
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] leds-tca6507: allow driver to compile when GPIOLIB is not available.
2011-12-19 0:20 ` [PATCH 1/2] leds-tca6507: allow driver to compile when GPIOLIB is not available NeilBrown
@ 2011-12-19 20:34 ` Randy Dunlap
2011-12-20 5:37 ` NeilBrown
0 siblings, 1 reply; 5+ messages in thread
From: Randy Dunlap @ 2011-12-19 20:34 UTC (permalink / raw)
To: NeilBrown; +Cc: Andrew Morton, rpurdie, linux-kernel, Stephen Rothwell
On 12/18/2011 04:20 PM, NeilBrown wrote:
> This driver can configure the outputs as GPIO line instead of LEDs. But
> that only works if GPIOLIB is available. So make that code conditional
> on the library's availability.
>
> Also remove the 'teardown' callback as it is never called and should
> never be needed.
>
> Reported-by: Randy Dunlap <rdunlap@xenotime.net>,
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>,
> Signed-off-by: NeilBrown <neilb@suse.de>
Hi Neil,
with linux-next 20111219 and this patch applied, I now get:
drivers/leds/leds-tca6507.c: In function 'tca6507_remove':
drivers/leds/leds-tca6507.c:677:9: error: 'struct tca6507_chip' has no member named 'gpio'
drivers/leds/leds-tca6507.c:678:3: error: implicit declaration of function 'gpiochip_remove'
drivers/leds/leds-tca6507.c:678:33: error: 'struct tca6507_chip' has no member named 'gpio'
when GPIOLIB is not enabled.
> ---
>
> drivers/leds/leds-tca6507.c | 75 +++++++++++++++++++++++++++++-------------
> include/linux/leds-tca6507.h | 3 +-
> 2 files changed, 53 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c
> index a7ea4cf..c9cc3f7 100644
> --- a/drivers/leds/leds-tca6507.c
> +++ b/drivers/leds/leds-tca6507.c
> @@ -149,9 +149,11 @@ struct tca6507_chip {
> int bank; /* Bank used, or -1 */
> int blink; /* 1 if we are hardware-blinking */
> } leds[NUM_LEDS];
> +#ifdef CONFIG_GPIOLIB
> struct gpio_chip gpio;
> const char *gpio_name[NUM_LEDS];
> int gpio_map[NUM_LEDS];
> +#endif
> };
>
> static const struct i2c_device_id tca6507_id[] = {
> @@ -520,6 +522,7 @@ static int tca6507_blink_set(struct led_classdev *led_cdev,
> return 0;
> }
>
> +#ifdef CONFIG_GPIOLIB
> static void tca6507_gpio_set_value(struct gpio_chip *gc,
> unsigned offset, int val)
> {
> @@ -542,6 +545,51 @@ static int tca6507_gpio_direction_output(struct gpio_chip *gc,
> tca6507_gpio_set_value(gc, offset, val);
> return 0;
> }
> +static int tca6507_probe_gpios(struct i2c_client *client,
> + struct tca6507_chip *tca,
> + struct tca6507_platform_data *pdata)
> +{
> + int err;
> + int i = 0;
> + int gpios = 0;
> +
> + for (i = 0; i < NUM_LEDS; i++)
> + if (pdata->leds.leds[i].name && pdata->leds.leds[i].flags) {
> + /* Configure as a gpio */
> + tca->gpio_name[gpios] = pdata->leds.leds[i].name;
> + tca->gpio_map[gpios] = i;
> + gpios++;
> + }
> +
> + if (!gpios)
> + return 0;
> +
> + tca->gpio.label = "gpio-tca6507";
> + tca->gpio.names = tca->gpio_name;
> + tca->gpio.ngpio = gpios;
> + tca->gpio.base = pdata->gpio_base;
> + tca->gpio.owner = THIS_MODULE;
> + tca->gpio.direction_output = tca6507_gpio_direction_output;
> + tca->gpio.set = tca6507_gpio_set_value;
> + tca->gpio.dev = &client->dev;
> + err = gpiochip_add(&tca->gpio);
> + if (err) {
> + tca->gpio.ngpio = 0;
> + return err;
> + }
> + if (pdata->setup)
> + pdata->setup(tca->gpio.base, tca->gpio.ngpio);
> + return 0;
> +}
> +
> +#else /* CONFIG_GPIOLIB */
> +static int tca6507_probe_gpios(struct i2c_client *client,
> + struct tca6507_chip *tca,
> + struct tca6507_platform_data *pdata)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_GPIOLIB */
>
> static int __devinit tca6507_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> @@ -551,7 +599,6 @@ static int __devinit tca6507_probe(struct i2c_client *client,
> struct tca6507_platform_data *pdata;
> int err;
> int i = 0;
> - int gpios = 0;
>
> adapter = to_i2c_adapter(client->dev.parent);
> pdata = client->dev.platform_data;
> @@ -590,30 +637,10 @@ static int __devinit tca6507_probe(struct i2c_client *client,
> if (err < 0)
> goto exit;
> }
> - if (pdata->leds.leds[i].name && pdata->leds.leds[i].flags) {
> - /* Configure as a gpio */
> - tca->gpio_name[gpios] = pdata->leds.leds[i].name;
> - tca->gpio_map[gpios] = i;
> - gpios++;
> - }
> - }
> - if (gpios) {
> - tca->gpio.label = "gpio-tca6507";
> - tca->gpio.names = tca->gpio_name;
> - tca->gpio.ngpio = gpios;
> - tca->gpio.base = pdata->gpio_base;
> - tca->gpio.owner = THIS_MODULE;
> - tca->gpio.direction_output = tca6507_gpio_direction_output;
> - tca->gpio.set = tca6507_gpio_set_value;
> - tca->gpio.dev = &client->dev;
> - err = gpiochip_add(&tca->gpio);
> - if (err) {
> - tca->gpio.ngpio = 0;
> - goto exit;
> - }
> - if (pdata->setup)
> - pdata->setup(tca->gpio.base, tca->gpio.ngpio);
> }
> + err = tca6507_probe_gpios(client, tca, pdata);
> + if (err)
> + goto exit;
> i2c_set_clientdata(client, tca);
> /* set all registers to known state - zero */
> tca->reg_set = 0x7f;
> diff --git a/include/linux/leds-tca6507.h b/include/linux/leds-tca6507.h
> index 3b8ac62..dcabf4f 100644
> --- a/include/linux/leds-tca6507.h
> +++ b/include/linux/leds-tca6507.h
> @@ -24,9 +24,10 @@
>
> struct tca6507_platform_data {
> struct led_platform_data leds;
> +#ifdef CONFIG_GPIOLIB
> int gpio_base;
> void (*setup)(unsigned gpio_base, unsigned ngpio);
> - void (*teardown)(unsigned gpio_base, unsigned ngpio);
> +#endif
> };
>
> #define TCA6507_MAKE_GPIO 1
>
>
> --
--
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] leds-tca6507: allow driver to compile when GPIOLIB is not available.
2011-12-19 20:34 ` Randy Dunlap
@ 2011-12-20 5:37 ` NeilBrown
0 siblings, 0 replies; 5+ messages in thread
From: NeilBrown @ 2011-12-20 5:37 UTC (permalink / raw)
To: Randy Dunlap; +Cc: Andrew Morton, rpurdie, linux-kernel, Stephen Rothwell
[-- Attachment #1: Type: text/plain, Size: 1364 bytes --]
On Mon, 19 Dec 2011 12:34:18 -0800 Randy Dunlap <rdunlap@xenotime.net> wrote:
> On 12/18/2011 04:20 PM, NeilBrown wrote:
> > This driver can configure the outputs as GPIO line instead of LEDs. But
> > that only works if GPIOLIB is available. So make that code conditional
> > on the library's availability.
> >
> > Also remove the 'teardown' callback as it is never called and should
> > never be needed.
> >
> > Reported-by: Randy Dunlap <rdunlap@xenotime.net>,
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>,
> > Signed-off-by: NeilBrown <neilb@suse.de>
>
> Hi Neil,
>
> with linux-next 20111219 and this patch applied, I now get:
>
> drivers/leds/leds-tca6507.c: In function 'tca6507_remove':
> drivers/leds/leds-tca6507.c:677:9: error: 'struct tca6507_chip' has no member named 'gpio'
> drivers/leds/leds-tca6507.c:678:3: error: implicit declaration of function 'gpiochip_remove'
> drivers/leds/leds-tca6507.c:678:33: error: 'struct tca6507_chip' has no member named 'gpio'
>
> when GPIOLIB is not enabled.
Drat - I forgot the 'remove' code.
I cannot easily compile for an ARCH that doesn't include GPIOLIB, but it
seems that if I put
#undef CONFIG_GPIOLIB
at the top of leds-tca6507.c it comes close enough for testing.
So the next version should get that right. I'll repost.
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-12-20 5:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-19 0:20 [PATCH 0/2] Two fixes for leds-tca6507 NeilBrown
2011-12-19 0:20 ` [PATCH 2/2] leds-tca6507 - fix off by one error NeilBrown
2011-12-19 0:20 ` [PATCH 1/2] leds-tca6507: allow driver to compile when GPIOLIB is not available NeilBrown
2011-12-19 20:34 ` Randy Dunlap
2011-12-20 5:37 ` NeilBrown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).