Re: [PATCH v4 2/2] cap11xx: add LED support

[Daniel Mack <daniel@zonque.org>]

On 06/16/2015 04:39 PM, Matt Ranostay wrote:
> On Tue, Jun 16, 2015 at 12:54 AM, Jacek Anaszewski
> <j.anaszewski@samsung.com> wrote:
>> On 06/16/2015 04:46 AM, Matt Ranostay wrote:

>>> +       for_each_child_of_node(node, child) {
>>> +               led->cdev.name =
>>> +                       of_get_property(child, "label", NULL) ? :
>>> child->name;
>>> +               led->cdev.default_trigger =
>>> +                       of_get_property(child, "linux,default-trigger",
>>> NULL);
>>> +               led->cdev.flags = 0;
>>> +               led->cdev.brightness_set = cap11xx_led_set;
>>> +               led->cdev.max_brightness = 1;
>>> +               led->cdev.brightness = LED_OFF;
>>> +
>>> +               error = of_property_read_u32(child, "reg", &reg);
>>> +               if (error != 0 || reg >= priv->num_leds)
>>> +                       continue;
>>> +               led->reg = reg;
>>> +               led->priv = priv;
>>> +
>>> +               INIT_WORK(&led->work, cap11xx_led_work);
>>> +               error = devm_led_classdev_register(dev, &led->cdev);
>>> +               if (error < 0)
>>> +                       return -EINVAL;
>>> +
>>> +               schedule_work(&led->work);
>>
>>
>> Why do you schedule work here? It should be done only in brightness_set op.
>>
> The LED needs to be set to initial turned off state, since there is no
> Power ON/OFF register for the cap11xx devices.. So LEDs state would be
> persistent across warm reboots.
> Thoughts on how to do this better?

What's wrong with this?

  regmap_write(priv->regmap, CAP11XX_REG_LED_OUTPUT_CONTROL, 0);

Outside the loop, of course ...


Daniel