On Tue, Apr 23, 2013 at 4:52 AM, Linus Walleij
<linus.walleij@xxxxxxxxxxxxxx> wrote:
> From: Linus Walleij <linus.walleij@xxxxxxxxxx>
>
> This patch allows the lp5521 driver to be successfully probed and
> initialised when Device Tree support is enabled.
>
> Based on a patch by Gabriel Fernandez, rewritten in accordance
> with review feedback.
>
> Cc: Gabriel Fernandez <gabriel.fernandez@xxxxxxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> ChangeLog v1->v2:
> - Move DT parsing into the lp55xx-common code.
> - Implement probing for lp5523 as well.
> - Handle errors from all of_property_read* functions.
> - Rename num-chanel to num-channel
> - Prefix all compatible strings with "national,*" like
> "national,lp5521", a bit uncertain about this but seems
> like the right thing to do.
> - Since I have now written more than half the patch I have
> set myself as author so as not to blame Gabriel for all my
> mistakes.
> ---
> .../devicetree/bindings/leds/leds-lp55xx.txt | 21 +++++++++
> drivers/leds/leds-lp5521.c | 20 ++++++--
> drivers/leds/leds-lp5523.c | 19 ++++++--
> drivers/leds/leds-lp55xx-common.c | 54 ++++++++++++++++++++++
> drivers/leds/leds-lp55xx-common.h | 4 ++
> 5 files changed, 108 insertions(+), 10 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-lp55xx.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-lp55xx.txt b/Documentation/devicetree/bindings/leds/leds-lp55xx.txt
> new file mode 100644
> index 0000000..348c88e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-lp55xx.txt
> @@ -0,0 +1,21 @@
> +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
> +- 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:
> +
> +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>;
I guess we missed a space between '/bits/' and '8' here.
And this patch looks fine to me. Milo, can you review this and try it
on your hardware?
Thanks,
-Bryan
> +};
> diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
> index 1001347..f3c15f7 100644
> --- a/drivers/leds/leds-lp5521.c
> +++ b/drivers/leds/leds-lp5521.c
> @@ -31,6 +31,7 @@
> #include <linux/mutex.h>
> #include <linux/platform_data/leds-lp55xx.h>
> #include <linux/slab.h>
> +#include <linux/of.h>
>
> #include "leds-lp55xx-common.h"
>
> @@ -400,12 +401,20 @@ static int lp5521_probe(struct i2c_client *client,
> int ret;
> struct lp55xx_chip *chip;
> struct lp55xx_led *led;
> - struct lp55xx_platform_data *pdata = client->dev.platform_data;
> -
> - if (!pdata) {
> - dev_err(&client->dev, "no platform data\n");
> - return -EINVAL;
> + struct lp55xx_platform_data *pdata;
> + struct device_node *np = client->dev.of_node;
> +
> + if (!client->dev.platform_data) {
> + if (np) {
> + ret = lp55xx_of_populate_pdata(&client->dev, np);
> + if (ret < 0)
> + return ret;
> + } else {
> + dev_err(&client->dev, "no platform data\n");
> + return -EINVAL;
> + }
> }
> + pdata = client->dev.platform_data;
>
> chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
> if (!chip)
> @@ -465,6 +474,7 @@ static int lp5521_remove(struct i2c_client *client)
>
> static const struct i2c_device_id lp5521_id[] = {
> { "lp5521", 0 }, /* Three channel chip */
> + { "national,lp5521", 0 }, /* OF compatible */
> { }
> };
> MODULE_DEVICE_TABLE(i2c, lp5521_id);
> diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
> index 229f734..365e914 100644
> --- a/drivers/leds/leds-lp5523.c
> +++ b/drivers/leds/leds-lp5523.c
> @@ -429,12 +429,20 @@ static int lp5523_probe(struct i2c_client *client,
> int ret;
> struct lp55xx_chip *chip;
> struct lp55xx_led *led;
> - struct lp55xx_platform_data *pdata = client->dev.platform_data;
> -
> - if (!pdata) {
> - dev_err(&client->dev, "no platform data\n");
> - return -EINVAL;
> + struct lp55xx_platform_data *pdata;
> + struct device_node *np = client->dev.of_node;
> +
> + if (!client->dev.platform_data) {
> + if (np) {
> + ret = lp55xx_of_populate_pdata(&client->dev, np);
> + if (ret < 0)
> + return ret;
> + } else {
> + dev_err(&client->dev, "no platform data\n");
> + return -EINVAL;
> + }
> }
> + pdata = client->dev.platform_data;
>
> chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
> if (!chip)
> @@ -495,6 +503,7 @@ static int lp5523_remove(struct i2c_client *client)
> static const struct i2c_device_id lp5523_id[] = {
> { "lp5523", LP5523 },
> { "lp55231", LP55231 },
> + { "national,lp5523", 0 }, /* OF compatible */
> { }
> };
>
> diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c
> index d9eb841..086ce08 100644
> --- a/drivers/leds/leds-lp55xx-common.c
> +++ b/drivers/leds/leds-lp55xx-common.c
> @@ -18,6 +18,7 @@
> #include <linux/leds.h>
> #include <linux/module.h>
> #include <linux/platform_data/leds-lp55xx.h>
> +#include <linux/slab.h>
>
> #include "leds-lp55xx-common.h"
>
> @@ -518,6 +519,59 @@ void lp55xx_unregister_sysfs(struct lp55xx_chip *chip)
> }
> EXPORT_SYMBOL_GPL(lp55xx_unregister_sysfs);
>
> +int lp55xx_of_populate_pdata(struct device *dev, struct device_node *np)
> +{
> + 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;
> +
> + 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;
> +
> + led_config = devm_kzalloc(dev, sizeof(*led_config) * num_channel,
> + GFP_KERNEL);
> + if (!led_config)
> + 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->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;
> + dev->platform_data = pdata;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(lp55xx_of_populate_pdata);
> +
> MODULE_AUTHOR("Milo Kim <milo.kim@xxxxxx>");
> MODULE_DESCRIPTION("LP55xx Common Driver");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/leds/leds-lp55xx-common.h b/drivers/leds/leds-lp55xx-common.h
> index ece4761..0708083 100644
> --- a/drivers/leds/leds-lp55xx-common.h
> +++ b/drivers/leds/leds-lp55xx-common.h
> @@ -131,4 +131,8 @@ extern void lp55xx_unregister_leds(struct lp55xx_led *led,
> extern int lp55xx_register_sysfs(struct lp55xx_chip *chip);
> extern void lp55xx_unregister_sysfs(struct lp55xx_chip *chip);
>
> +/* common device tree population function */
> +extern int lp55xx_of_populate_pdata(struct device *dev,
> + struct device_node *np);
> +
> #endif /* _LEDS_LP55XX_COMMON_H */
> --
> 1.7.11.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
