| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 8 May 2013 10:15:21 -0700
From: Bryan Wu <cooloney@...il.com>
To: "Kim, Milo" <Milo.Kim@...com>
Cc: Linus Walleij <linus.walleij@...aro.org>,
Gabriel Fernandez <gabriel.fernandez@...ricsson.com>,
"linux-leds@...r.kernel.org" <linux-leds@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] leds: lp55xx: support dynamic channel settings in the
device tree structure
On Tue, May 7, 2013 at 12:14 AM, Kim, Milo <Milo.Kim@...com> wrote:
> Currently, the LP55xx DT structure supports max 3 channels.
> However, LP5523 has max 9 channels and LP5562 has 4 channels.
> To enhance this constraint, the DT structure has been changed.
>
> (a) Use the child node for various channel settings instead of fixed array
> (b) Remove 'num_channel' property.
> This value can be retrieved by counting the children node.
> (c) 'chan-name' property supported
> (d) Documentation updates for LP5521 and LP5523
>
> Cc: Linus Walleij <linus.walleij@...aro.org>
> Cc: Gabriel Fernandez <gabriel.fernandez@...ricsson.com>
> Signed-off-by: Milo(Woogyom) Kim <milo.kim@...com>
> ---
> .../devicetree/bindings/leds/leds-lp55xx.txt | 107 ++++++++++++++++++--
> drivers/leds/leds-lp55xx-common.c | 61 +++++------
> 2 files changed, 126 insertions(+), 42 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-lp55xx.txt b/Documentation/devicetree/bindings/leds/leds-lp55xx.txt
> index 348c88e..a454088 100644
> --- a/Documentation/devicetree/bindings/leds/leds-lp55xx.txt
> +++ b/Documentation/devicetree/bindings/leds/leds-lp55xx.txt
> @@ -2,20 +2,113 @@ Binding for National Semiconductor LP55xx Led Drivers
>
> Required properties:
> - compatible: "national,lp5521" or "national,lp5523"
> -- label: Used for naming LEDs
> -- num-channel: Number of LED channels
> +- reg: I2C slave address
> +- clock-mode: Input clock mode, (0: automode, 1: internal, 2: external)
> +
> +Each child has own specific current settings
> - led-cur: Current setting at each led channel (mA x10, 0 if led is not connected)
> - max-cur: Maximun current at each led channel.
> -- clock-mode: Input clock mode, (0: automode, 1: internal, 2: external)
>
> -example:
> +Optional properties:
> +- label: Used for naming LEDs
> +
> +Alternatively, each child can have specific channel name
> +- chan-name: Name of each channel name
> +
> +example 1) LP5521
> +3 LED channels, external clock used. Channel names are 'lp5521_pri:channel0',
> +'lp5521_pri:channel1' and 'lp5521_pri:channel2'
>
> lp5521@32 {
> compatible = "national,lp5521";
> reg = <0x32>;
> label = "lp5521_pri";
> - num-channel = /bits/ 8 <3>;
> - led-cur = /bits/ 8 <0x2f 0x2f 0x2f>;
> - max-cur = /bits/ 8 <0x5f 0x5f 0x5f>;
> clock-mode = /bits/8 <2>;
Let me fix this by adding a space between '/bit/' and '8'.
And this patch looks great to me. I will merge it soon.
-Bryan
> +
> + chan0 {
> + led-cur = /bits/ 8 <0x2f>;
> + max-cur = /bits/ 8 <0x5f>;
> + };
> +
> + chan1 {
> + led-cur = /bits/ 8 <0x2f>;
> + max-cur = /bits/ 8 <0x5f>;
> + };
> +
> + chan2 {
> + led-cur = /bits/ 8 <0x2f>;
> + max-cur = /bits/ 8 <0x5f>;
> + };
> +};
> +
> +example 2) LP5523
> +9 LED channels with specific name. Internal clock used.
> +The I2C slave address is configurable with ASEL1 and ASEL0 pins.
> +Available addresses are 32/33/34/35h.
> +
> +ASEL1 ASEL0 Address
> +-------------------------
> + GND GND 32h
> + GND VEN 33h
> + VEN GND 34h
> + VEN VEN 35h
> +
> +lp5523@32 {
> + compatible = "national,lp5523";
> + reg = <0x32>;
> + clock-mode = /bits/ 8 <1>;
> +
> + chan0 {
> + chan-name = "d1";
> + led-cur = /bits/ 8 <0x14>;
> + max-cur = /bits/ 8 <0x20>;
> + };
> +
> + chan1 {
> + chan-name = "d2";
> + led-cur = /bits/ 8 <0x14>;
> + max-cur = /bits/ 8 <0x20>;
> + };
> +
> + chan2 {
> + chan-name = "d3";
> + led-cur = /bits/ 8 <0x14>;
> + max-cur = /bits/ 8 <0x20>;
> + };
> +
> + chan3 {
> + chan-name = "d4";
> + led-cur = /bits/ 8 <0x14>;
> + max-cur = /bits/ 8 <0x20>;
> + };
> +
> + chan4 {
> + chan-name = "d5";
> + led-cur = /bits/ 8 <0x14>;
> + max-cur = /bits/ 8 <0x20>;
> + };
> +
> + chan5 {
> + chan-name = "d6";
> + led-cur = /bits/ 8 <0x14>;
> + max-cur = /bits/ 8 <0x20>;
> + };
> +
> + chan6 {
> + chan-name = "d7";
> + led-cur = /bits/ 8 <0x14>;
> + max-cur = /bits/ 8 <0x20>;
> + };
> +
> + chan7 {
> + chan-name = "d8";
> + led-cur = /bits/ 8 <0x14>;
> + max-cur = /bits/ 8 <0x20>;
> + };
> +
> + chan8 {
> + chan-name = "d9";
> + led-cur = /bits/ 8 <0x14>;
> + max-cur = /bits/ 8 <0x20>;
> + };
> };
> diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c
> index a0d2bd2..c2fecd4 100644
> --- a/drivers/leds/leds-lp55xx-common.c
> +++ b/drivers/leds/leds-lp55xx-common.c
> @@ -557,51 +557,42 @@ EXPORT_SYMBOL_GPL(lp55xx_unregister_sysfs);
>
> int lp55xx_of_populate_pdata(struct device *dev, struct device_node *np)
> {
> + struct device_node *child;
> struct lp55xx_platform_data *pdata;
> - u8 led_cur[3];
> - u8 max_cur[3];
> - u8 clock_mode;
> - u8 num_channel;
> - const char *label;
> - struct lp55xx_led_config *led_config;
> - int ret;
> - int i;
> + struct lp55xx_led_config *cfg;
> + int num_channels;
> + int i = 0;
>
> pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> if (!pdata)
> return -ENOMEM;
>
> - ret = of_property_read_u8(np, "num-channel", &num_channel);
> - if (ret < 0)
> - return ret;
> - ret = of_property_read_u8_array(np, "led-cur", led_cur, num_channel);
> - if (ret < 0)
> - return ret;
> - ret = of_property_read_u8_array(np, "max-cur", max_cur, num_channel);
> - if (ret < 0)
> - return ret;
> - ret = of_property_read_string(np, "label", &label);
> - if (ret < 0)
> - return ret;
> - ret = of_property_read_u8_array(np, "clock-mode", &clock_mode, 1);
> - if (ret < 0)
> - return ret;
> + num_channels = of_get_child_count(np);
> + if (num_channels == 0) {
> + dev_err(dev, "no LED channels\n");
> + return -EINVAL;
> + }
>
> - led_config = devm_kzalloc(dev, sizeof(*led_config) * num_channel,
> - GFP_KERNEL);
> - if (!led_config)
> + cfg = devm_kzalloc(dev, sizeof(*cfg) * num_channels, GFP_KERNEL);
> + if (!cfg)
> return -ENOMEM;
>
> - for (i = 0; i < num_channel; i++) {
> - led_config[i].chan_nr = i;
> - led_config[i].led_current = led_cur[i];
> - led_config[i].max_current = max_cur[i];
> + pdata->led_config = &cfg[0];
> + pdata->num_channels = num_channels;
> +
> + for_each_child_of_node(np, child) {
> + cfg[i].chan_nr = i;
> +
> + of_property_read_string(child, "chan-name", &cfg[i].name);
> + of_property_read_u8(child, "led-cur", &cfg[i].led_current);
> + of_property_read_u8(child, "max-cur", &cfg[i].max_current);
> +
> + i++;
> }
> - pdata->label = kzalloc(sizeof(char) * 32, GFP_KERNEL);
> - strcpy((char *)pdata->label, (char *) label);
> - pdata->led_config = &led_config[0];
> - pdata->num_channels = num_channel;
> - pdata->clock_mode = clock_mode;
> +
> + of_property_read_string(np, "label", &pdata->label);
> + of_property_read_u8(np, "clock-mode", &pdata->clock_mode);
> +
> dev->platform_data = pdata;
>
> return 0;
> --
> 1.7.9.5
>
> Best Regards,
> Milo
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists