* [PATCH 1/2] leds/tca6507: Add support for devicetree. @ 2012-10-30 21:45 Marek Belisko 2012-10-30 21:45 ` [PATCH 2/2] Add documentation for tca6507 devicetree bindings Marek Belisko 2012-11-06 18:24 ` [PATCH 1/2] leds/tca6507: Add support for devicetree Bryan Wu 0 siblings, 2 replies; 6+ messages in thread From: Marek Belisko @ 2012-10-30 21:45 UTC (permalink / raw) To: cooloney, rpurdie Cc: linux-leds, linux-kernel, devicetree-discuss, Marek Belisko Support added only for leds (not for gpio's). Signed-off-by: Marek Belisko <marek.belisko@open-nandra.com> --- drivers/leds/leds-tca6507.c | 73 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 3 deletions(-) diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c index dabcf7a..496dd98 100644 --- a/drivers/leds/leds-tca6507.c +++ b/drivers/leds/leds-tca6507.c @@ -667,6 +667,69 @@ static void tca6507_remove_gpio(struct tca6507_chip *tca) } #endif /* CONFIG_GPIOLIB */ +#ifdef CONFIG_OF +static struct tca6507_platform_data * __devinit tca6507_led_dt_init(struct i2c_client *client) +{ + struct device_node *np = client->dev.of_node, *child; + struct tca6507_platform_data *pdata; + struct led_info *tca_leds; + int count = 0; + + for_each_child_of_node(np, child) + count++; + if (!count) + return NULL; + + if (count > NUM_LEDS) + return NULL; + + tca_leds = devm_kzalloc(&client->dev, sizeof(struct led_info) * NUM_LEDS, GFP_KERNEL); + + if (!tca_leds) + return NULL; + + + for_each_child_of_node(np, child) { + struct led_info led; + u32 reg; + int ret; + + led.name = of_get_property(child, "label", NULL) ? : child->name; + led.default_trigger = + of_get_property(child, "linux,default-trigger", NULL); + + ret = of_property_read_u32(child, "reg", ®); + + if (ret != 0) + continue; + tca_leds[reg] = led; + } + pdata = devm_kzalloc(&client->dev, sizeof(struct tca6507_platform_data), GFP_KERNEL); + if (!pdata) { + kfree(tca_leds); + return NULL; + } + + pdata->leds.leds = tca_leds; + pdata->leds.num_leds = NUM_LEDS; + + return pdata; +} + +static const struct of_device_id of_tca6507_leds_match[] = { + { .compatible = "leds-tca6507", }, + {}, +}; + +#else +static int __devinit tca6507_led_dt_init(struct i2c_client *client, struct tca6507_platform_data *data) +{ + return -1; +} + +#define of_tca6507_leds_match NULL +#endif + static int __devinit tca6507_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -683,9 +746,12 @@ static int __devinit tca6507_probe(struct i2c_client *client, return -EIO; if (!pdata || pdata->leds.num_leds != NUM_LEDS) { - dev_err(&client->dev, "Need %d entries in platform-data list\n", - NUM_LEDS); - return -ENODEV; + pdata = tca6507_led_dt_init(client); + if (!pdata) { + dev_err(&client->dev, "Need %d entries in platform-data list\n", + NUM_LEDS); + return -ENODEV; + } } tca = devm_kzalloc(&client->dev, sizeof(*tca), GFP_KERNEL); if (!tca) @@ -750,6 +816,7 @@ static struct i2c_driver tca6507_driver = { .driver = { .name = "leds-tca6507", .owner = THIS_MODULE, + .of_match_table = of_tca6507_leds_match, }, .probe = tca6507_probe, .remove = __devexit_p(tca6507_remove), -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] Add documentation for tca6507 devicetree bindings. 2012-10-30 21:45 [PATCH 1/2] leds/tca6507: Add support for devicetree Marek Belisko @ 2012-10-30 21:45 ` Marek Belisko 2012-11-06 18:24 ` [PATCH 1/2] leds/tca6507: Add support for devicetree Bryan Wu 1 sibling, 0 replies; 6+ messages in thread From: Marek Belisko @ 2012-10-30 21:45 UTC (permalink / raw) To: cooloney, rpurdie Cc: linux-leds, linux-kernel, devicetree-discuss, Marek Belisko Signed-off-by: Marek Belisko <marek.belisko@open-nandra.com> --- Documentation/devicetree/bindings/leds/tca6507.txt | 33 ++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/tca6507.txt diff --git a/Documentation/devicetree/bindings/leds/tca6507.txt b/Documentation/devicetree/bindings/leds/tca6507.txt new file mode 100644 index 0000000..cb0015b9f --- /dev/null +++ b/Documentation/devicetree/bindings/leds/tca6507.txt @@ -0,0 +1,33 @@ +LEDs conected to tca6507 + +Required properties: +- compatible : should be : "leds-tca6507". + +Each led is represented as a sub-node of the leds-tca6507 device. + +LED sub-node properties: +- label : label for this LED +- reg : number of LED line (could be from 0 to 6) +- linux,default-trigger : led default trigger + +Examples: + +tca6507@45 { + compatible = "leds-tca6507"; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x45>; + + led0: red_aux@0 { + label = "red:aux"; + reg = <0x0>; + }; + + led1: green_aux@1 { + label = "green:aux"; + reg = <0x5>; + linux,default-trigger = "default-on"; + }; + +}; + -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] leds/tca6507: Add support for devicetree. 2012-10-30 21:45 [PATCH 1/2] leds/tca6507: Add support for devicetree Marek Belisko 2012-10-30 21:45 ` [PATCH 2/2] Add documentation for tca6507 devicetree bindings Marek Belisko @ 2012-11-06 18:24 ` Bryan Wu [not found] ` <CAK5ve-+fE7T3U9KZ-C5-sZN9PUPq6RybCu+nv9UDb2x0bnarzw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2012-11-07 9:30 ` Jingoo Han 1 sibling, 2 replies; 6+ messages in thread From: Bryan Wu @ 2012-11-06 18:24 UTC (permalink / raw) To: Marek Belisko; +Cc: rpurdie, linux-leds, linux-kernel, devicetree-discuss On Tue, Oct 30, 2012 at 2:45 PM, Marek Belisko <marek.belisko@open-nandra.com> wrote: > Support added only for leds (not for gpio's). > > Signed-off-by: Marek Belisko <marek.belisko@open-nandra.com> > --- > drivers/leds/leds-tca6507.c | 73 +++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 70 insertions(+), 3 deletions(-) > Overall, this looks good to me. Maybe some question as below, -Bryan > diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c > index dabcf7a..496dd98 100644 > --- a/drivers/leds/leds-tca6507.c > +++ b/drivers/leds/leds-tca6507.c > @@ -667,6 +667,69 @@ static void tca6507_remove_gpio(struct tca6507_chip *tca) > } > #endif /* CONFIG_GPIOLIB */ > > +#ifdef CONFIG_OF > +static struct tca6507_platform_data * __devinit tca6507_led_dt_init(struct i2c_client *client) > +{ > + struct device_node *np = client->dev.of_node, *child; > + struct tca6507_platform_data *pdata; > + struct led_info *tca_leds; > + int count = 0; > + > + for_each_child_of_node(np, child) > + count++; > + if (!count) > + return NULL; > + I saw many 'return NULL' here, why not return some error code in ERR_PTR? > + if (count > NUM_LEDS) > + return NULL; > + > + tca_leds = devm_kzalloc(&client->dev, sizeof(struct led_info) * NUM_LEDS, GFP_KERNEL); > + useless empty line. > + if (!tca_leds) > + return NULL; > + > + useless empty line. > + for_each_child_of_node(np, child) { > + struct led_info led; > + u32 reg; > + int ret; > + > + led.name = of_get_property(child, "label", NULL) ? : child->name; > + led.default_trigger = > + of_get_property(child, "linux,default-trigger", NULL); > + > + ret = of_property_read_u32(child, "reg", ®); > + > + if (ret != 0) > + continue; > + tca_leds[reg] = led; > + } > + pdata = devm_kzalloc(&client->dev, sizeof(struct tca6507_platform_data), GFP_KERNEL); > + if (!pdata) { > + kfree(tca_leds); Do we need to kfree here? I think devm_zalloc() will take care of this if the driver failed to register. > + return NULL; > + } > + > + pdata->leds.leds = tca_leds; > + pdata->leds.num_leds = NUM_LEDS; > + > + return pdata; > +} > + > +static const struct of_device_id of_tca6507_leds_match[] = { > + { .compatible = "leds-tca6507", }, > + {}, > +}; > + > +#else > +static int __devinit tca6507_led_dt_init(struct i2c_client *client, struct tca6507_platform_data *data) > +{ > + return -1; > +} > + > +#define of_tca6507_leds_match NULL > +#endif > + > static int __devinit tca6507_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > @@ -683,9 +746,12 @@ static int __devinit tca6507_probe(struct i2c_client *client, > return -EIO; > > if (!pdata || pdata->leds.num_leds != NUM_LEDS) { > - dev_err(&client->dev, "Need %d entries in platform-data list\n", > - NUM_LEDS); > - return -ENODEV; > + pdata = tca6507_led_dt_init(client); > + if (!pdata) { > + dev_err(&client->dev, "Need %d entries in platform-data list\n", > + NUM_LEDS); > + return -ENODEV; > + } > } > tca = devm_kzalloc(&client->dev, sizeof(*tca), GFP_KERNEL); > if (!tca) > @@ -750,6 +816,7 @@ static struct i2c_driver tca6507_driver = { > .driver = { > .name = "leds-tca6507", > .owner = THIS_MODULE, > + .of_match_table = of_tca6507_leds_match, > }, > .probe = tca6507_probe, > .remove = __devexit_p(tca6507_remove), > -- > 1.7.9.5 > ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <CAK5ve-+fE7T3U9KZ-C5-sZN9PUPq6RybCu+nv9UDb2x0bnarzw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 1/2] leds/tca6507: Add support for devicetree. [not found] ` <CAK5ve-+fE7T3U9KZ-C5-sZN9PUPq6RybCu+nv9UDb2x0bnarzw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2012-11-06 20:32 ` Belisko Marek 2012-11-06 23:27 ` Bryan Wu 0 siblings, 1 reply; 6+ messages in thread From: Belisko Marek @ 2012-11-06 20:32 UTC (permalink / raw) To: Bryan Wu Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Richard Purdie, LKML, linux-leds-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1.1: Type: text/plain, Size: 5173 bytes --] Hi, On Tue, Nov 6, 2012 at 7:24 PM, Bryan Wu <cooloney-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > On Tue, Oct 30, 2012 at 2:45 PM, Marek Belisko > <marek.belisko-e6Ot0yJZRx5zTq7rFmK8cA@public.gmane.org> wrote: > > Support added only for leds (not for gpio's). > > > > Signed-off-by: Marek Belisko <marek.belisko-e6Ot0yJZRx5zTq7rFmK8cA@public.gmane.org> > > --- > > drivers/leds/leds-tca6507.c | 73 > +++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 70 insertions(+), 3 deletions(-) > > > > Overall, this looks good to me. Maybe some question as below, > > -Bryan > > > diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c > > index dabcf7a..496dd98 100644 > > --- a/drivers/leds/leds-tca6507.c > > +++ b/drivers/leds/leds-tca6507.c > > @@ -667,6 +667,69 @@ static void tca6507_remove_gpio(struct tca6507_chip > *tca) > > } > > #endif /* CONFIG_GPIOLIB */ > > > > +#ifdef CONFIG_OF > > +static struct tca6507_platform_data * __devinit > tca6507_led_dt_init(struct i2c_client *client) > > +{ > > + struct device_node *np = client->dev.of_node, *child; > > + struct tca6507_platform_data *pdata; > > + struct led_info *tca_leds; > > + int count = 0; > > + > > + for_each_child_of_node(np, child) > > + count++; > > + if (!count) > > + return NULL; > > + > > I saw many 'return NULL' here, why not return some error code in ERR_PTR? > Well this function is only only used to correctly parse DT and set proper platform data which was previously passed from board file. I think we don't need to return (and check) specific error code just return NULL and use some general advice to user that something is wrong. Same way it's done in leds-gpio in gpio_leds_create_of() function. > > > + if (count > NUM_LEDS) > > + return NULL; > > + > > + tca_leds = devm_kzalloc(&client->dev, sizeof(struct led_info) * > NUM_LEDS, GFP_KERNEL); > > + > > useless empty line. > > > + if (!tca_leds) > > + return NULL; > > + > > + > > useless empty line. > > > + for_each_child_of_node(np, child) { > > + struct led_info led; > > + u32 reg; > > + int ret; > > + > > + led.name = of_get_property(child, "label", NULL) ? : > child->name; > > + led.default_trigger = > > + of_get_property(child, "linux,default-trigger", > NULL); > > + > > + ret = of_property_read_u32(child, "reg", ®); > > + > > + if (ret != 0) > > + continue; > > + tca_leds[reg] = led; > > + } > > + pdata = devm_kzalloc(&client->dev, sizeof(struct > tca6507_platform_data), GFP_KERNEL); > > + if (!pdata) { > > + kfree(tca_leds); > > Do we need to kfree here? I think devm_zalloc() will take care of this > if the driver failed to register. > You're right. I'll fix comments are send updated version of patch. Are you OK with binding documentation patch? > > > + return NULL; > > + } > > + > > + pdata->leds.leds = tca_leds; > > + pdata->leds.num_leds = NUM_LEDS; > > + > > + return pdata; > > +} > > + > > +static const struct of_device_id of_tca6507_leds_match[] = { > > + { .compatible = "leds-tca6507", }, > > + {}, > > +}; > > + > > +#else > > +static int __devinit tca6507_led_dt_init(struct i2c_client *client, > struct tca6507_platform_data *data) > > +{ > > + return -1; > > +} > > + > > +#define of_tca6507_leds_match NULL > > +#endif > > + > > static int __devinit tca6507_probe(struct i2c_client *client, > > const struct i2c_device_id *id) > > { > > @@ -683,9 +746,12 @@ static int __devinit tca6507_probe(struct > i2c_client *client, > > return -EIO; > > > > if (!pdata || pdata->leds.num_leds != NUM_LEDS) { > > - dev_err(&client->dev, "Need %d entries in platform-data > list\n", > > - NUM_LEDS); > > - return -ENODEV; > > + pdata = tca6507_led_dt_init(client); > > + if (!pdata) { > > + dev_err(&client->dev, "Need %d entries in > platform-data list\n", > > + NUM_LEDS); > > + return -ENODEV; > > + } > > } > > tca = devm_kzalloc(&client->dev, sizeof(*tca), GFP_KERNEL); > > if (!tca) > > @@ -750,6 +816,7 @@ static struct i2c_driver tca6507_driver = { > > .driver = { > > .name = "leds-tca6507", > > .owner = THIS_MODULE, > > + .of_match_table = of_tca6507_leds_match, > > }, > > .probe = tca6507_probe, > > .remove = __devexit_p(tca6507_remove), > > -- > > 1.7.9.5 > > > mbe -- as simple and primitive as possible ------------------------------------------------- Marek Belisko - OPEN-NANDRA Freelance Developer Ruska Nova Ves 219 | Presov, 08005 Slovak Republic Tel: +421 915 052 184 skype: marekwhite twitter: #opennandra web: http://open-nandra.com [-- Attachment #1.2: Type: text/html, Size: 7519 bytes --] [-- Attachment #2: Type: text/plain, Size: 192 bytes --] _______________________________________________ devicetree-discuss mailing list devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org https://lists.ozlabs.org/listinfo/devicetree-discuss ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] leds/tca6507: Add support for devicetree. 2012-11-06 20:32 ` Belisko Marek @ 2012-11-06 23:27 ` Bryan Wu 0 siblings, 0 replies; 6+ messages in thread From: Bryan Wu @ 2012-11-06 23:27 UTC (permalink / raw) To: Belisko Marek; +Cc: Richard Purdie, linux-leds, LKML, devicetree-discuss On Tue, Nov 6, 2012 at 12:32 PM, Belisko Marek <marek.belisko@gmail.com> wrote: > Hi, > > On Tue, Nov 6, 2012 at 7:24 PM, Bryan Wu <cooloney@gmail.com> wrote: >> >> On Tue, Oct 30, 2012 at 2:45 PM, Marek Belisko >> <marek.belisko@open-nandra.com> wrote: >> > Support added only for leds (not for gpio's). >> > >> > Signed-off-by: Marek Belisko <marek.belisko@open-nandra.com> >> > --- >> > drivers/leds/leds-tca6507.c | 73 >> > +++++++++++++++++++++++++++++++++++++++++-- >> > 1 file changed, 70 insertions(+), 3 deletions(-) >> > >> >> Overall, this looks good to me. Maybe some question as below, >> >> -Bryan >> >> > diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c >> > index dabcf7a..496dd98 100644 >> > --- a/drivers/leds/leds-tca6507.c >> > +++ b/drivers/leds/leds-tca6507.c >> > @@ -667,6 +667,69 @@ static void tca6507_remove_gpio(struct tca6507_chip >> > *tca) >> > } >> > #endif /* CONFIG_GPIOLIB */ >> > >> > +#ifdef CONFIG_OF >> > +static struct tca6507_platform_data * __devinit >> > tca6507_led_dt_init(struct i2c_client *client) >> > +{ >> > + struct device_node *np = client->dev.of_node, *child; >> > + struct tca6507_platform_data *pdata; >> > + struct led_info *tca_leds; >> > + int count = 0; >> > + >> > + for_each_child_of_node(np, child) >> > + count++; >> > + if (!count) >> > + return NULL; >> > + >> >> I saw many 'return NULL' here, why not return some error code in ERR_PTR? > > Well this function is only only used to correctly parse DT and set proper > platform data which was previously passed from board file. I think we don't > need to return (and check) specific error code just return NULL and use some > general advice to user that something is wrong. Same way it's done in > leds-gpio in gpio_leds_create_of() function. Oh, really. In my kernel tree, gpio_leds_create_of() function of drivers/leds/leds-gpio.c always returns something like this ERR_PTR(-ENODEV); I think return some meaningful things is useful than NULL. -Bryan >> >> >> > + if (count > NUM_LEDS) >> > + return NULL; >> > + >> > + tca_leds = devm_kzalloc(&client->dev, sizeof(struct led_info) * >> > NUM_LEDS, GFP_KERNEL); >> > + >> >> useless empty line. >> >> > + if (!tca_leds) >> > + return NULL; >> > + >> > + >> >> useless empty line. >> >> > + for_each_child_of_node(np, child) { >> > + struct led_info led; >> > + u32 reg; >> > + int ret; >> > + >> > + led.name = of_get_property(child, "label", NULL) ? : >> > child->name; >> > + led.default_trigger = >> > + of_get_property(child, "linux,default-trigger", >> > NULL); >> > + >> > + ret = of_property_read_u32(child, "reg", ®); >> > + >> > + if (ret != 0) >> > + continue; >> > + tca_leds[reg] = led; >> > + } >> > + pdata = devm_kzalloc(&client->dev, sizeof(struct >> > tca6507_platform_data), GFP_KERNEL); >> > + if (!pdata) { >> > + kfree(tca_leds); >> >> Do we need to kfree here? I think devm_zalloc() will take care of this >> if the driver failed to register. > > You're right. I'll fix comments are send updated version of patch. Are you > OK with binding documentation patch? >> >> >> > + return NULL; >> > + } >> > + >> > + pdata->leds.leds = tca_leds; >> > + pdata->leds.num_leds = NUM_LEDS; >> > + >> > + return pdata; >> > +} >> > + >> > +static const struct of_device_id of_tca6507_leds_match[] = { >> > + { .compatible = "leds-tca6507", }, >> > + {}, >> > +}; >> > + >> > +#else >> > +static int __devinit tca6507_led_dt_init(struct i2c_client *client, >> > struct tca6507_platform_data *data) >> > +{ >> > + return -1; >> > +} >> > + >> > +#define of_tca6507_leds_match NULL >> > +#endif >> > + >> > static int __devinit tca6507_probe(struct i2c_client *client, >> > const struct i2c_device_id *id) >> > { >> > @@ -683,9 +746,12 @@ static int __devinit tca6507_probe(struct >> > i2c_client *client, >> > return -EIO; >> > >> > if (!pdata || pdata->leds.num_leds != NUM_LEDS) { >> > - dev_err(&client->dev, "Need %d entries in platform-data >> > list\n", >> > - NUM_LEDS); >> > - return -ENODEV; >> > + pdata = tca6507_led_dt_init(client); >> > + if (!pdata) { >> > + dev_err(&client->dev, "Need %d entries in >> > platform-data list\n", >> > + NUM_LEDS); >> > + return -ENODEV; >> > + } >> > } >> > tca = devm_kzalloc(&client->dev, sizeof(*tca), GFP_KERNEL); >> > if (!tca) >> > @@ -750,6 +816,7 @@ static struct i2c_driver tca6507_driver = { >> > .driver = { >> > .name = "leds-tca6507", >> > .owner = THIS_MODULE, >> > + .of_match_table = of_tca6507_leds_match, >> > }, >> > .probe = tca6507_probe, >> > .remove = __devexit_p(tca6507_remove), >> > -- >> > 1.7.9.5 >> > > > > mbe > > -- > as simple and primitive as possible > ------------------------------------------------- > Marek Belisko - OPEN-NANDRA > Freelance Developer > > Ruska Nova Ves 219 | Presov, 08005 Slovak Republic > Tel: +421 915 052 184 > skype: marekwhite > twitter: #opennandra > web: http://open-nandra.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] leds/tca6507: Add support for devicetree. 2012-11-06 18:24 ` [PATCH 1/2] leds/tca6507: Add support for devicetree Bryan Wu [not found] ` <CAK5ve-+fE7T3U9KZ-C5-sZN9PUPq6RybCu+nv9UDb2x0bnarzw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2012-11-07 9:30 ` Jingoo Han 1 sibling, 0 replies; 6+ messages in thread From: Jingoo Han @ 2012-11-07 9:30 UTC (permalink / raw) To: 'Marek Belisko' Cc: 'Bryan Wu', rpurdie, linux-leds, linux-kernel, devicetree-discuss, 'Jingoo Han' On Wednesday, November 07, 2012 3:25 AM Bryan Wu wrote > > On Tue, Oct 30, 2012 at 2:45 PM, Marek Belisko > <marek.belisko@open-nandra.com> wrote: > > Support added only for leds (not for gpio's). > > > > Signed-off-by: Marek Belisko <marek.belisko@open-nandra.com> > > --- > > drivers/leds/leds-tca6507.c | 73 +++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 70 insertions(+), 3 deletions(-) > > > > Overall, this looks good to me. Maybe some question as below, > > -Bryan > > > diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c > > index dabcf7a..496dd98 100644 > > --- a/drivers/leds/leds-tca6507.c > > +++ b/drivers/leds/leds-tca6507.c > > @@ -667,6 +667,69 @@ static void tca6507_remove_gpio(struct tca6507_chip *tca) > > } > > #endif /* CONFIG_GPIOLIB */ > > > > +#ifdef CONFIG_OF > > +static struct tca6507_platform_data * __devinit tca6507_led_dt_init(struct i2c_client *client) > > +{ > > + struct device_node *np = client->dev.of_node, *child; > > + struct tca6507_platform_data *pdata; > > + struct led_info *tca_leds; > > + int count = 0; > > + > > + for_each_child_of_node(np, child) > > + count++; > > + if (!count) > > + return NULL; > > + > > I saw many 'return NULL' here, why not return some error code in ERR_PTR? > > > + if (count > NUM_LEDS) > > + return NULL; > > + > > + tca_leds = devm_kzalloc(&client->dev, sizeof(struct led_info) * NUM_LEDS, GFP_KERNEL); > > + > > useless empty line. > > > + if (!tca_leds) > > + return NULL; > > + > > + > > useless empty line. > > > + for_each_child_of_node(np, child) { > > + struct led_info led; > > + u32 reg; > > + int ret; > > + > > + led.name = of_get_property(child, "label", NULL) ? : child->name; > > + led.default_trigger = > > + of_get_property(child, "linux,default-trigger", NULL); > > + > > + ret = of_property_read_u32(child, "reg", ®); > > + > > + if (ret != 0) > > + continue; > > + tca_leds[reg] = led; > > + } > > + pdata = devm_kzalloc(&client->dev, sizeof(struct tca6507_platform_data), GFP_KERNEL); > > + if (!pdata) { > > + kfree(tca_leds); > > Do we need to kfree here? I think devm_zalloc() will take care of this > if the driver failed to register. If devm_kzalloc() fails, tca6507_probe() will return '-ENODEV'. In this case, kfree() is unnecessary in this case, when devm_kzalloc() is used. Moreover, even if kfree() is necessary, devm_kfree() should be used instead of kfree(). Marek Belisko, please refer to this document. : http://www.kernel.org/doc/htmldocs/device-drivers/API-devm-kzalloc.html Thank you. Best regards, Jingoo Han > > > + return NULL; > > + } > > + > > + pdata->leds.leds = tca_leds; > > + pdata->leds.num_leds = NUM_LEDS; > > + > > + return pdata; > > +} > > + > > +static const struct of_device_id of_tca6507_leds_match[] = { > > + { .compatible = "leds-tca6507", }, > > + {}, > > +}; > > + > > +#else > > +static int __devinit tca6507_led_dt_init(struct i2c_client *client, struct tca6507_platform_data > *data) > > +{ > > + return -1; > > +} > > + > > +#define of_tca6507_leds_match NULL > > +#endif > > + > > static int __devinit tca6507_probe(struct i2c_client *client, > > const struct i2c_device_id *id) > > { > > @@ -683,9 +746,12 @@ static int __devinit tca6507_probe(struct i2c_client *client, > > return -EIO; > > > > if (!pdata || pdata->leds.num_leds != NUM_LEDS) { > > - dev_err(&client->dev, "Need %d entries in platform-data list\n", > > - NUM_LEDS); > > - return -ENODEV; > > + pdata = tca6507_led_dt_init(client); > > + if (!pdata) { > > + dev_err(&client->dev, "Need %d entries in platform-data list\n", > > + NUM_LEDS); > > + return -ENODEV; > > + } > > } > > tca = devm_kzalloc(&client->dev, sizeof(*tca), GFP_KERNEL); > > if (!tca) > > @@ -750,6 +816,7 @@ static struct i2c_driver tca6507_driver = { > > .driver = { > > .name = "leds-tca6507", > > .owner = THIS_MODULE, > > + .of_match_table = of_tca6507_leds_match, > > }, > > .probe = tca6507_probe, > > .remove = __devexit_p(tca6507_remove), > > -- > > 1.7.9.5 > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-leds" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-11-07 9:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-30 21:45 [PATCH 1/2] leds/tca6507: Add support for devicetree Marek Belisko
2012-10-30 21:45 ` [PATCH 2/2] Add documentation for tca6507 devicetree bindings Marek Belisko
2012-11-06 18:24 ` [PATCH 1/2] leds/tca6507: Add support for devicetree Bryan Wu
[not found] ` <CAK5ve-+fE7T3U9KZ-C5-sZN9PUPq6RybCu+nv9UDb2x0bnarzw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-11-06 20:32 ` Belisko Marek
2012-11-06 23:27 ` Bryan Wu
2012-11-07 9:30 ` Jingoo Han
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).