Jacek
On 11/08/2018 02:48 PM, Jacek Anaszewski wrote:
> Dan,
>
> On 11/08/2018 07:16 PM, Dan Murphy wrote:
>> On 11/06/2018 04:07 PM, Jacek Anaszewski wrote:
>>> Switch to using generic LED support for composing LED class
>>> device name.
>>>
>>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>
>>> Signed-off-by: Dan Murphy <dmurphy@xxxxxx>
>>> ---
>>> 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 39c72a9..7c12ccd 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);
>>> - }
>>
>> We need to keep a loop here looking for additional nodes as this device has string support and just
>> not cluster support.
>>
>> Just need to get the other LED patches done.
>
> In the current shape only the properties from the last parsed child node
> will be used for initializing the LED class device. So, this should be
> meant also a fix removing this obscurity. Worth mentioning in the commit
> message. Or I'm not getting it right?
Yes I agree as this driver in the current shape only supports cluster mode only not string support.
I have some stashed patches locally that register each string individually for this device.
I have not worked out a good algorithm to set the register appropriately.
If you want to remove the loop then I can add it back once I add string support.
Dan
>
>>> + child_node = of_get_next_available_child(np, NULL);
>>> + 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);
>>
>> Should we use the #defines here for display_cluster?
>
> Right, will fix in the next version.
>
>>> + 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;
>>>
>>
>>
>
--
------------------
Dan Murphy
