* [PATCH 1/4] leds-lp5523: add channel name in the platform data
@ 2012-08-21 9:03 Kim, Milo
2012-08-22 2:42 ` Bryan Wu
0 siblings, 1 reply; 4+ messages in thread
From: Kim, Milo @ 2012-08-21 9:03 UTC (permalink / raw)
To: Bryan Wu; +Cc: Richard Purdie, linux-kernel, linux-leds, mathias.nyman
The name of each led channel is configurable.
If the name is NULL, just use the channel id for making the channel name
Signed-off-by: Milo(Woogyom) Kim <milo.kim@ti.com>
---
Documentation/leds/leds-lp5523.txt | 7 +++++--
drivers/leds/leds-lp5523.c | 10 +++++++---
include/linux/leds-lp5523.h | 1 +
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/Documentation/leds/leds-lp5523.txt b/Documentation/leds/leds-lp5523.txt
index fad2feb..e958d9a 100644
--- a/Documentation/leds/leds-lp5523.txt
+++ b/Documentation/leds/leds-lp5523.txt
@@ -10,7 +10,9 @@ Contact: Samu Onkalo (samu.p.onkalo-at-nokia.com)
Description
-----------
LP5523 can drive up to 9 channels. Leds can be controlled directly via
-the led class control interface. Channels have generic names:
+the led class control interface.
+The name of each channel is configurable in the platform data.
+If the name is NULL, channels have generic names:
lp5523:channelx where x is 0...8
The chip provides 3 engines. Each engine can control channels without
@@ -46,12 +48,13 @@ Note - chan_nr can have values between 0 and 8.
static struct lp5523_led_config lp5523_led_config[] = {
{
+ .name = "red",
.chan_nr = 0,
.led_current = 50,
.max_current = 130,
},
...
- }, {
+ {
.chan_nr = 8,
.led_current = 50,
.max_current = 130,
diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
index fbc12ac..9fd9a92 100644
--- a/drivers/leds/leds-lp5523.c
+++ b/drivers/leds/leds-lp5523.c
@@ -846,10 +846,14 @@ static int __devinit lp5523_init_led(struct lp5523_led *led, struct device *dev,
return -EINVAL;
}
- snprintf(name, sizeof(name), "%s:channel%d",
- pdata->label ?: "lp5523", chan);
+ if (pdata->led_config[chan].name) {
+ led->cdev.name = pdata->led_config[chan].name;
+ } else {
+ snprintf(name, sizeof(name), "%s:channel%d",
+ pdata->label ?: "lp5523", chan);
+ led->cdev.name = name;
+ }
- led->cdev.name = name;
led->cdev.brightness_set = lp5523_set_brightness;
res = led_classdev_register(dev, &led->cdev);
if (res < 0) {
diff --git a/include/linux/leds-lp5523.h b/include/linux/leds-lp5523.h
index 2694289..727877f 100644
--- a/include/linux/leds-lp5523.h
+++ b/include/linux/leds-lp5523.h
@@ -26,6 +26,7 @@
/* See Documentation/leds/leds-lp5523.txt */
struct lp5523_led_config {
+ const char *name;
u8 chan_nr;
u8 led_current; /* mA x10, 0 if led is not connected */
u8 max_current;
--
1.7.2.5
Best Regards,
Milo
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 1/4] leds-lp5523: add channel name in the platform data
2012-08-21 9:03 [PATCH 1/4] leds-lp5523: add channel name in the platform data Kim, Milo
@ 2012-08-22 2:42 ` Bryan Wu
2012-08-22 5:43 ` Kim, Milo
0 siblings, 1 reply; 4+ messages in thread
From: Bryan Wu @ 2012-08-22 2:42 UTC (permalink / raw)
To: Kim, Milo; +Cc: Richard Purdie, linux-kernel, linux-leds, mathias.nyman
On Tue, Aug 21, 2012 at 5:03 PM, Kim, Milo <Milo.Kim@ti.com> wrote:
> The name of each led channel is configurable.
> If the name is NULL, just use the channel id for making the channel name
>
> Signed-off-by: Milo(Woogyom) Kim <milo.kim@ti.com>
> ---
> Documentation/leds/leds-lp5523.txt | 7 +++++--
> drivers/leds/leds-lp5523.c | 10 +++++++---
> include/linux/leds-lp5523.h | 1 +
> 3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/leds/leds-lp5523.txt b/Documentation/leds/leds-lp5523.txt
> index fad2feb..e958d9a 100644
> --- a/Documentation/leds/leds-lp5523.txt
> +++ b/Documentation/leds/leds-lp5523.txt
> @@ -10,7 +10,9 @@ Contact: Samu Onkalo (samu.p.onkalo-at-nokia.com)
> Description
> -----------
> LP5523 can drive up to 9 channels. Leds can be controlled directly via
> -the led class control interface. Channels have generic names:
> +the led class control interface.
> +The name of each channel is configurable in the platform data.
> +If the name is NULL, channels have generic names:
> lp5523:channelx where x is 0...8
>
> The chip provides 3 engines. Each engine can control channels without
> @@ -46,12 +48,13 @@ Note - chan_nr can have values between 0 and 8.
>
> static struct lp5523_led_config lp5523_led_config[] = {
> {
> + .name = "red",
> .chan_nr = 0,
> .led_current = 50,
> .max_current = 130,
> },
> ...
> - }, {
> + {
> .chan_nr = 8,
> .led_current = 50,
> .max_current = 130,
> diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
> index fbc12ac..9fd9a92 100644
> --- a/drivers/leds/leds-lp5523.c
> +++ b/drivers/leds/leds-lp5523.c
> @@ -846,10 +846,14 @@ static int __devinit lp5523_init_led(struct lp5523_led *led, struct device *dev,
> return -EINVAL;
> }
>
> - snprintf(name, sizeof(name), "%s:channel%d",
> - pdata->label ?: "lp5523", chan);
> + if (pdata->led_config[chan].name) {
> + led->cdev.name = pdata->led_config[chan].name;
> + } else {
> + snprintf(name, sizeof(name), "%s:channel%d",
> + pdata->label ?: "lp5523", chan);
Why we need a test here "pdata->label ?:"? From the document, looks
like we use lp5523:channel%d as default.
> + led->cdev.name = name;
> + }
>
> - led->cdev.name = name;
> led->cdev.brightness_set = lp5523_set_brightness;
> res = led_classdev_register(dev, &led->cdev);
> if (res < 0) {
> diff --git a/include/linux/leds-lp5523.h b/include/linux/leds-lp5523.h
> index 2694289..727877f 100644
> --- a/include/linux/leds-lp5523.h
> +++ b/include/linux/leds-lp5523.h
> @@ -26,6 +26,7 @@
> /* See Documentation/leds/leds-lp5523.txt */
>
> struct lp5523_led_config {
> + const char *name;
> u8 chan_nr;
> u8 led_current; /* mA x10, 0 if led is not connected */
> u8 max_current;
> --
> 1.7.2.5
>
>
> Best Regards,
> Milo
>
>
--
Bryan Wu <bryan.wu@canonical.com>
Kernel Developer +86.186-168-78255 Mobile
Canonical Ltd. www.canonical.com
Ubuntu - Linux for human beings | www.ubuntu.com
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH 1/4] leds-lp5523: add channel name in the platform data
2012-08-22 2:42 ` Bryan Wu
@ 2012-08-22 5:43 ` Kim, Milo
2012-08-22 7:12 ` Bryan Wu
0 siblings, 1 reply; 4+ messages in thread
From: Kim, Milo @ 2012-08-22 5:43 UTC (permalink / raw)
To: Bryan Wu; +Cc: Richard Purdie, linux-kernel, linux-leds
> > LP5523 can drive up to 9 channels. Leds can be controlled directly
> via
> > -the led class control interface. Channels have generic names:
> > +the led class control interface.
> > +The name of each channel is configurable in the platform data.
> > +If the name is NULL, channels have generic names:
> > lp5523:channelx where x is 0...8
> Why we need a test here "pdata->label ?:"? From the document, looks
> like we use lp5523:channel%d as default.
To define the channel name, we have three options.
a) using 'name' field for each channel
b) using 'label' field without 'name'
c) default : neither data is defined
So I would change the description as below.
Signed-off-by: Milo(Woogyom) Kim <milo.kim@ti.com>
---
Documentation/leds/leds-lp5523.txt | 18 ++++++++++++++++--
1 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/Documentation/leds/leds-lp5523.txt b/Documentation/leds/leds-lp5523.txt
index fad2feb..120fdfe 100644
--- a/Documentation/leds/leds-lp5523.txt
+++ b/Documentation/leds/leds-lp5523.txt
@@ -10,8 +10,22 @@ Contact: Samu Onkalo (samu.p.onkalo-at-nokia.com)
Description
-----------
LP5523 can drive up to 9 channels. Leds can be controlled directly via
-the led class control interface. Channels have generic names:
-lp5523:channelx where x is 0...8
+the led class control interface.
+The name of each channel is configurable in the platform data - name and label.
+There are three options to make the channel name.
+
+a) Define the 'name' in the platform data
+To make specific channel name, then use 'name' platform data.
+/sys/class/leds/R1 (name: 'R1')
+/sys/class/leds/B1 (name: 'B1')
+
+b) Use the 'label' with no 'name' field
+For one device name with channel number, then use 'label'.
+/sys/class/leds/RGB:channelN (label: 'RGB', N: 0 ~ 8)
+
+c) Default
+If both fields are NULL, 'lp5523' is used by default.
+/sys/class/leds/lp5523:channelN (N: 0 ~ 8)
The chip provides 3 engines. Each engine can control channels without
interaction from the main CPU. Details of the micro engine code can be found
--
1.7.2.5
Best Regards,
Milo
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/4] leds-lp5523: add channel name in the platform data
2012-08-22 5:43 ` Kim, Milo
@ 2012-08-22 7:12 ` Bryan Wu
0 siblings, 0 replies; 4+ messages in thread
From: Bryan Wu @ 2012-08-22 7:12 UTC (permalink / raw)
To: Kim, Milo; +Cc: Richard Purdie, linux-kernel, linux-leds
On Wed, Aug 22, 2012 at 1:43 PM, Kim, Milo <Milo.Kim@ti.com> wrote:
>> > LP5523 can drive up to 9 channels. Leds can be controlled directly
>> via
>> > -the led class control interface. Channels have generic names:
>> > +the led class control interface.
>> > +The name of each channel is configurable in the platform data.
>> > +If the name is NULL, channels have generic names:
>> > lp5523:channelx where x is 0...8
>
>> Why we need a test here "pdata->label ?:"? From the document, looks
>> like we use lp5523:channel%d as default.
>
> To define the channel name, we have three options.
> a) using 'name' field for each channel
> b) using 'label' field without 'name'
> c) default : neither data is defined
>
> So I would change the description as below.
>
Thanks for this clarification. I'm fine with this, please resubmit patches.
-Bryan
> Signed-off-by: Milo(Woogyom) Kim <milo.kim@ti.com>
> ---
> Documentation/leds/leds-lp5523.txt | 18 ++++++++++++++++--
> 1 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/leds/leds-lp5523.txt b/Documentation/leds/leds-lp5523.txt
> index fad2feb..120fdfe 100644
> --- a/Documentation/leds/leds-lp5523.txt
> +++ b/Documentation/leds/leds-lp5523.txt
> @@ -10,8 +10,22 @@ Contact: Samu Onkalo (samu.p.onkalo-at-nokia.com)
> Description
> -----------
> LP5523 can drive up to 9 channels. Leds can be controlled directly via
> -the led class control interface. Channels have generic names:
> -lp5523:channelx where x is 0...8
> +the led class control interface.
> +The name of each channel is configurable in the platform data - name and label.
> +There are three options to make the channel name.
> +
> +a) Define the 'name' in the platform data
> +To make specific channel name, then use 'name' platform data.
> +/sys/class/leds/R1 (name: 'R1')
> +/sys/class/leds/B1 (name: 'B1')
> +
> +b) Use the 'label' with no 'name' field
> +For one device name with channel number, then use 'label'.
> +/sys/class/leds/RGB:channelN (label: 'RGB', N: 0 ~ 8)
> +
> +c) Default
> +If both fields are NULL, 'lp5523' is used by default.
> +/sys/class/leds/lp5523:channelN (N: 0 ~ 8)
>
> The chip provides 3 engines. Each engine can control channels without
> interaction from the main CPU. Details of the micro engine code can be found
> --
> 1.7.2.5
>
>
> Best Regards,
> Milo
>
>
>
--
Bryan Wu <bryan.wu@canonical.com>
Kernel Developer +86.186-168-78255 Mobile
Canonical Ltd. www.canonical.com
Ubuntu - Linux for human beings | www.ubuntu.com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-08-22 7:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-21 9:03 [PATCH 1/4] leds-lp5523: add channel name in the platform data Kim, Milo
2012-08-22 2:42 ` Bryan Wu
2012-08-22 5:43 ` Kim, Milo
2012-08-22 7:12 ` Bryan Wu
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).