2007-01-15 12:27:10

by Ben Dooks

[permalink] [raw]
Subject: LEDS: S3C24XX generate name if none given

Generate a name if none is passed to the S3C24XX GPIO LED driver.

Signed-off-by: Ben Dooks <[email protected]>

diff -urpN -X ../dontdiff linux-2.6.19/drivers/leds/leds-s3c24xx.c linux-2.6.19-simtec1p22/drivers/leds/leds-s3c24xx.c
--- linux-2.6.19/drivers/leds/leds-s3c24xx.c 2006-11-29 21:57:37.000000000 +0000
+++ linux-2.6.19-simtec1p22/drivers/leds/leds-s3c24xx.c 2007-01-04 10:22:58.000000000 +0000
@@ -23,6 +23,8 @@
/* our context */

struct s3c24xx_gpio_led {
+ char name[32];
+
struct led_classdev cdev;
struct s3c24xx_led_platdata *pdata;
};
@@ -85,6 +87,14 @@ static int s3c24xx_led_probe(struct plat

led->pdata = pdata;

+ /* create name if we where not passed one */
+
+ if (led->cdev.name == NULL) {
+ snprintf(led->name, sizeof(led->name), "%s.%d",
+ dev->name, dev->id);
+ led->cdev.name = led->name;
+ }
+
/* no point in having a pull-up if we are always driving */

if (pdata->flags & S3C24XX_LEDF_TRISTATE) {


2007-01-15 13:03:15

by Robert P. J. Day

[permalink] [raw]
Subject: Re: LEDS: S3C24XX generate name if none given

On Mon, 15 Jan 2007, Ben Dooks wrote:

> Generate a name if none is passed to the S3C24XX GPIO LED driver.
>
> Signed-off-by: Ben Dooks <[email protected]>
>
> diff -urpN -X ../dontdiff linux-2.6.19/drivers/leds/leds-s3c24xx.c linux-2.6.19-simtec1p22/drivers/leds/leds-s3c24xx.c
> --- linux-2.6.19/drivers/leds/leds-s3c24xx.c 2006-11-29 21:57:37.000000000 +0000
> +++ linux-2.6.19-simtec1p22/drivers/leds/leds-s3c24xx.c 2007-01-04 10:22:58.000000000 +0000
> @@ -23,6 +23,8 @@
> /* our context */
>
> struct s3c24xx_gpio_led {
> + char name[32];
> +
> struct led_classdev cdev;
> struct s3c24xx_led_platdata *pdata;
> };
> @@ -85,6 +87,14 @@ static int s3c24xx_led_probe(struct plat
>
> led->pdata = pdata;
>
> + /* create name if we where not passed one */
^^^^^ "were"


rday

2007-01-15 13:44:55

by Richard Purdie

[permalink] [raw]
Subject: Re: LEDS: S3C24XX generate name if none given

On Mon, 2007-01-15 at 12:26 +0000, Ben Dooks wrote:
> Generate a name if none is passed to the S3C24XX GPIO LED driver.
>
> Signed-off-by: Ben Dooks <[email protected]>
>
> diff -urpN -X ../dontdiff linux-2.6.19/drivers/leds/leds-s3c24xx.c linux-2.6.19-simtec1p22/drivers/leds/leds-s3c24xx.c
> --- linux-2.6.19/drivers/leds/leds-s3c24xx.c 2006-11-29 21:57:37.000000000 +0000
> +++ linux-2.6.19-simtec1p22/drivers/leds/leds-s3c24xx.c 2007-01-04 10:22:58.000000000 +0000
> @@ -23,6 +23,8 @@
> /* our context */
>
> struct s3c24xx_gpio_led {
> + char name[32];
> +
> struct led_classdev cdev;

I'm not that keen on this since it wastes 32 bytes per LED when the
platform code does declare the names. If you're happy with that, its up
to you as the platform maintainer I guess but is there no nicer way to
handle this? I'm mainly concerned about people copying this code into
other drivers as then they'll all end up doing it...

Richard

2007-01-15 15:42:28

by Ben Dooks

[permalink] [raw]
Subject: Re: LEDS: S3C24XX generate name if none given

On Mon, Jan 15, 2007 at 01:44:28PM +0000, Richard Purdie wrote:
> On Mon, 2007-01-15 at 12:26 +0000, Ben Dooks wrote:
> > Generate a name if none is passed to the S3C24XX GPIO LED driver.
> >
> > Signed-off-by: Ben Dooks <[email protected]>
> >
> > diff -urpN -X ../dontdiff linux-2.6.19/drivers/leds/leds-s3c24xx.c linux-2.6.19-simtec1p22/drivers/leds/leds-s3c24xx.c
> > --- linux-2.6.19/drivers/leds/leds-s3c24xx.c 2006-11-29 21:57:37.000000000 +0000
> > +++ linux-2.6.19-simtec1p22/drivers/leds/leds-s3c24xx.c 2007-01-04 10:22:58.000000000 +0000
> > @@ -23,6 +23,8 @@
> > /* our context */
> >
> > struct s3c24xx_gpio_led {
> > + char name[32];
> > +
> > struct led_classdev cdev;
>
> I'm not that keen on this since it wastes 32 bytes per LED when the
> platform code does declare the names. If you're happy with that, its up
> to you as the platform maintainer I guess but is there no nicer way to
> handle this? I'm mainly concerned about people copying this code into
> other drivers as then they'll all end up doing it...

Ok, how about using kstrdup() on the name, like this:

diff -urpN -X ../dontdiff linux-2.6.19/drivers/leds/leds-s3c24xx.c linux-2.6.19-simtec1p22/drivers/leds/leds-s3c24xx.c
--- linux-2.6.19/drivers/leds/leds-s3c24xx.c 2006-11-29 21:57:37.000000000 +0000
+++ linux-2.6.19-simtec1p22/drivers/leds/leds-s3c24xx.c 2007-01-15 15:26:05.000000000 +0000
@@ -59,6 +59,9 @@ static int s3c24xx_led_remove(struct pla
{
struct s3c24xx_gpio_led *led = pdev_to_gpio(dev);

+ if (led->cdev.name != led->pdata.name)
+ kfree(led->cdev.name);
+
led_classdev_unregister(&led->cdev);
kfree(led);

@@ -85,6 +88,15 @@ static int s3c24xx_led_probe(struct plat

led->pdata = pdata;

+ /* create name if we were not passed one */
+
+ if (led->cdev.name == NULL) {
+ char name[64];
+
+ snprintf(name, sizeof(name), "%s.%d", dev->name, dev->id);
+ led->cdev.name = kstrdup(name);
+ }
+
/* no point in having a pull-up if we are always driving */

if (pdata->flags & S3C24XX_LEDF_TRISTATE) {


--
Ben

Q: What's a light-year?
A: One-third less calories than a regular year.

2007-01-16 19:06:28

by Richard Purdie

[permalink] [raw]
Subject: Re: LEDS: S3C24XX generate name if none given

On Mon, 2007-01-15 at 15:42 +0000, Ben Dooks wrote:
> On Mon, Jan 15, 2007 at 01:44:28PM +0000, Richard Purdie wrote:
> > On Mon, 2007-01-15 at 12:26 +0000, Ben Dooks wrote:
> > > Generate a name if none is passed to the S3C24XX GPIO LED driver.
> > >
> > > Signed-off-by: Ben Dooks <[email protected]>
> > >
>
> Ok, how about using kstrdup() on the name, like this:

Much better, thanks.

Acked-by: Richard Purdie <[email protected]>

Creating a git tree to handle the LED patches properly is on my todo
list...

> diff -urpN -X ../dontdiff linux-2.6.19/drivers/leds/leds-s3c24xx.c linux-2.6.19-simtec1p22/drivers/leds/leds-s3c24xx.c
> --- linux-2.6.19/drivers/leds/leds-s3c24xx.c 2006-11-29 21:57:37.000000000 +0000
> +++ linux-2.6.19-simtec1p22/drivers/leds/leds-s3c24xx.c 2007-01-15 15:26:05.000000000 +0000
> @@ -59,6 +59,9 @@ static int s3c24xx_led_remove(struct pla
> {
> struct s3c24xx_gpio_led *led = pdev_to_gpio(dev);
>
> + if (led->cdev.name != led->pdata.name)
> + kfree(led->cdev.name);
> +
> led_classdev_unregister(&led->cdev);
> kfree(led);
>
> @@ -85,6 +88,15 @@ static int s3c24xx_led_probe(struct plat
>
> led->pdata = pdata;
>
> + /* create name if we were not passed one */
> +
> + if (led->cdev.name == NULL) {
> + char name[64];
> +
> + snprintf(name, sizeof(name), "%s.%d", dev->name, dev->id);
> + led->cdev.name = kstrdup(name);
> + }
> +
> /* no point in having a pull-up if we are always driving */
>
> if (pdata->flags & S3C24XX_LEDF_TRISTATE) {
>
>

2007-01-22 19:57:12

by Andrew Morton

[permalink] [raw]
Subject: Re: LEDS: S3C24XX generate name if none given

> On Mon, 15 Jan 2007 15:42:21 +0000 Ben Dooks <[email protected]> wrote:
> On Mon, Jan 15, 2007 at 01:44:28PM +0000, Richard Purdie wrote:
> > On Mon, 2007-01-15 at 12:26 +0000, Ben Dooks wrote:
> > > Generate a name if none is passed to the S3C24XX GPIO LED driver.

Wouldn't it be better to fix the callers to consistently pass in a name
rather than hacking around fixing stuff up in the callee?

> + /* create name if we were not passed one */
> +
> + if (led->cdev.name == NULL) {
> + char name[64];
> +
> + snprintf(name, sizeof(name), "%s.%d", dev->name, dev->id);
> + led->cdev.name = kstrdup(name);
> + }
> +

Take a peek at kasprintf() ;)

Please include full-and-fresh changelog in each iteration of a patch, so
poor patch-takers don't have to go making one up for you, thanks.