lkml.org 
[lkml]   [2019]   [Mar]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 11/25] leds: lp8860: Use led_compose_name()
From
Date
Dan,

On 3/11/19 1:28 PM, Dan Murphy wrote:
> On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
>> Switch to using generic LED support for composing LED class
>> device name.
>>
>> While at it, avoid iterating through available child of nodes
>> in favor of obtaining single expected child node using single
>> call to of_get_next_available_child().
>>
>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>> Cc: Dan Murphy <dmurphy@ti.com>
>> ---
>> drivers/leds/leds-lp8860.c | 38 +++++++++++++++++++-------------------
>> 1 file changed, 19 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
>> index 39c72a908f3b..7c12ccdc1f2f 100644
>> --- a/drivers/leds/leds-lp8860.c
>> +++ b/drivers/leds/leds-lp8860.c
>> @@ -22,7 +22,6 @@
>> #include <linux/of_gpio.h>
>> #include <linux/gpio/consumer.h>
>> #include <linux/slab.h>
>> -#include <uapi/linux/uleds.h>
>>
>> #define LP8860_DISP_CL1_BRT_MSB 0x00
>> #define LP8860_DISP_CL1_BRT_LSB 0x01
>> @@ -87,6 +86,8 @@
>>
>> #define LP8860_CLEAR_FAULTS 0x01
>>
>> +#define LP8860_NAME "lp8860"
>> +
>> /**
>> * struct lp8860_led -
>> * @lock - Lock for reading/writing the device
>> @@ -96,7 +97,6 @@
>> * @eeprom_regmap - EEPROM register map
>> * @enable_gpio - VDDIO/EN gpio to enable communication interface
>> * @regulator - LED supply regulator pointer
>> - * @label - LED label
>> */
>> struct lp8860_led {
>> struct mutex lock;
>> @@ -106,7 +106,6 @@ struct lp8860_led {
>> struct regmap *eeprom_regmap;
>> struct gpio_desc *enable_gpio;
>> struct regulator *regulator;
>> - char label[LED_MAX_NAME_SIZE];
>> };
>>
>> struct lp8860_eeprom_reg {
>> @@ -387,25 +386,26 @@ static int lp8860_probe(struct i2c_client *client,
>> struct lp8860_led *led;
>> struct device_node *np = client->dev.of_node;
>> struct device_node *child_node;
>> - const char *name;
>> + struct led_init_data init_data;
>>
>> led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL);
>> if (!led)
>> return -ENOMEM;
>>
>> - for_each_available_child_of_node(np, child_node) {
>> - led->led_dev.default_trigger = of_get_property(child_node,
>> - "linux,default-trigger",
>> - NULL);
>> -
>> - ret = of_property_read_string(child_node, "label", &name);
>> - if (!ret)
>> - snprintf(led->label, sizeof(led->label), "%s:%s",
>> - id->name, name);
>> - else
>> - snprintf(led->label, sizeof(led->label),
>> - "%s::display_cluster", id->name);
>> - }
>> + child_node = of_get_next_available_child(np, NULL);
>
> Again this device has multiple outputs that can be banked or separated.
>
> I would prefer to leave the iteration and just change the name generation.

In [0] you agreed on removing that. This code is simply broken since
there is only one instance of struct lp8860_led allocated.

>> + if (!child_node)
>> + return -EINVAL;
>> +
>> + init_data.fwnode = of_fwnode_handle(child_node),
>> +
>> + led->led_dev.default_trigger = of_get_property(child_node,
>> + "linux,default-trigger",
>> + NULL);
>> +
>> + ret = led_compose_name(init_data.fwnode, LP8860_NAME,
>> + ":display_cluster", init_data.name);
>> + if (ret)
>> + return ret;
>>
>> led->enable_gpio = devm_gpiod_get_optional(&client->dev,
>> "enable", GPIOD_OUT_LOW);
>> @@ -420,7 +420,6 @@ static int lp8860_probe(struct i2c_client *client,
>> led->regulator = NULL;
>>
>> led->client = client;
>> - led->led_dev.name = led->label;
>> led->led_dev.brightness_set_blocking = lp8860_brightness_set;
>>
>> mutex_init(&led->lock);
>> @@ -447,7 +446,8 @@ static int lp8860_probe(struct i2c_client *client,
>> if (ret)
>> return ret;
>>
>> - ret = devm_led_classdev_register(&client->dev, &led->led_dev);
>> + ret = devm_led_classdev_register_ext(&client->dev, &led->led_dev,
>> + &init_data);
>> if (ret) {
>> dev_err(&client->dev, "led register err: %d\n", ret);
>> return ret;
>>
>
>

[0] https://lkml.org/lkml/2018/11/12/2231

--
Best regards,
Jacek Anaszewski

\
 
 \ /
  Last update: 2019-03-11 18:25    [W:0.056 / U:3.452 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site