Thread (10 messages) 10 messages, 3 authors, 2015-06-17

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

From: Matt Ranostay <hidden>
Date: 2015-06-17 06:34:21
Also in: linux-input, linux-leds

On Tue, Jun 16, 2015 at 2:55 PM, Matt Ranostay [off-list ref] wrote:
On Tue, Jun 16, 2015 at 2:26 PM, Daniel Mack [off-list ref] wrote:
quoted
On 06/16/2015 04:39 PM, Matt Ranostay wrote:
quoted
On Tue, Jun 16, 2015 at 12:54 AM, Jacek Anaszewski
[off-list ref] wrote:
quoted
On 06/16/2015 04:46 AM, Matt Ranostay wrote:
quoted
quoted
quoted
+       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 ...
Works for me, and prevents the workqueues tasks for blocking each
other in regmap_update_bits.

However I just noticed the DLSLEEP state that cap11xx_input_close()
requests via cap11xx_set_sleep(true) could disable the LEDs. Need to
double check this later tonight...
Seems deep sleep only effects the breath function. So never mind :)
quoted
Daniel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help