LEDS: S3C24XX generate name if none given

LEDS: S3C24XX generate name if none given

[Ben Dooks]

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

Signed-off-by: Ben Dooks <ben-linux@fluff.org>

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) {

Re: LEDS: S3C24XX generate name if none given

[Robert P. J. Day]

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 <ben-linux@fluff.org>
>
> 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

Re: LEDS: S3C24XX generate name if none given

[Richard Purdie]

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 <ben-linux@fluff.org>
> 
> 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

Re: LEDS: S3C24XX generate name if none given

[Ben Dooks]

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 <ben-linux@fluff.org>
> > 
> > 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.

Re: LEDS: S3C24XX generate name if none given

[Richard Purdie]

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 <ben-linux@fluff.org>
> > > 
>
> Ok, how about using kstrdup() on the name, like this:

Much better, thanks.

Acked-by: Richard Purdie <rpurdie@rpsys.net>

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) {
> 
> 

Re: LEDS: S3C24XX generate name if none given

[Andrew Morton]

> On Mon, 15 Jan 2007 15:42:21 +0000 Ben Dooks <ben@trinity.fluff.org> 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.