Re: [PATCH 1/4] leds-lp5523: add channel name in the platform data
On Wed, Aug 22, 2012 at 1:43 PM, Kim, Milo 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 > --- > 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 Kernel Developer+86.186-168-78255 Mobile Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] leds-lp5523: add channel name in the platform data
On Wed, Aug 22, 2012 at 1:43 PM, Kim, Milo milo@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@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...@canonical.com Kernel Developer+86.186-168-78255 Mobile Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/4] leds-lp5523: add channel name in the platform data
> > 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 --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] leds-lp5523: add channel name in the platform data
On Tue, Aug 21, 2012 at 5:03 PM, Kim, Milo 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
> ---
> 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, >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
Kernel Developer+86.186-168-78255 Mobile
Canonical Ltd. www.canonical.com
Ubuntu - Linux for human beings | www.ubuntu.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] leds-lp5523: add channel name in the platform data
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
---
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, >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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] leds-lp5523: add channel name in the platform data
On Tue, Aug 21, 2012 at 5:03 PM, Kim, Milo milo@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@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...@canonical.com
Kernel Developer+86.186-168-78255 Mobile
Canonical Ltd. www.canonical.com
Ubuntu - Linux for human beings | www.ubuntu.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/4] leds-lp5523: add channel name in the platform data
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@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] leds-lp5523: add channel name in the platform data
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@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
