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/
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/