Hi Ingi,
On 11/26/2015 09:02 AM, Ingi Kim wrote:
[...]
+torch_unlock:
+ mutex_unlock(&led->lock);
+ return ret;
+}
+
+static int rt5033_led_flash_brightness_set(struct led_classdev_flash *fled_cdev,
+ u32 brightness)
+{
+ struct rt5033_sub_led *sub_led = flcdev_to_sub_led(fled_cdev);
+ struct rt5033_led *led = sub_led_to_led(sub_led);
+
+ mutex_lock(&led->lock);
+ sub_led->flash_brightness = brightness;
+ mutex_unlock(&led->lock);
Mutex protection is redundant in this case. You would need it if device
state was also changed here.
Okay, I'll remove it.
BTW why flash brightness can't be written to the device here?
Flash brightness is only affected when FLED flashed (strobing).
So, I think it is better to be written in rt5033_led_flash_strobe_set function.
strobe_set op should strobe the flash ASAP, and delegating brightness
setting there extends a delay between calling strobe_set op
and actual flash strobe. If you set the brightness here, then you
wouldn't have to do that in the strobe_set op, of course unless the
the brightness is altered through the API of the other LED, in two
separate LEDs case.
+
+ return 0;
+}
+
+static int rt5033_led_flash_timeout_set(struct led_classdev_flash *fled_cdev,
+ u32 timeout)
+{
+ struct rt5033_sub_led *sub_led = flcdev_to_sub_led(fled_cdev);
+ struct rt5033_led *led = sub_led_to_led(sub_led);
+
+ mutex_lock(&led->lock);
+ sub_led->flash_timeout = timeout;
+ mutex_unlock(&led->lock);
Ditto.
Timeout should be also written here.
If you will add regmap_write in both ops, then mutex protection will
have to be preserved, to assure consistency between registers state
and sub_led->flash_brightness and sub_led->flash_timeout state.
+#define RT5033_FLED_CTRL4_VTRREG_MAX 0x60
Rename DEF to MASK.
Hmm, here it should be: Rename MAX to MASK.
--
Best Regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
