On 18/07/2019 14:24, Jacek Anaszewski wrote:
Hi Jean,
Thank you for the updated patch set.
I have some more comments below.
On 7/17/19 3:59 PM, Jean-Jacques Hiblot wrote:
+static bool __led_need_regulator_update(struct led_classdev *led_cdev,
+ int brightness)
+{
+ bool new_state = (brightness != LED_OFF);
How about:
bool new_state = !!brightness;
Throughout the code LED_OFF is used when the LED is turned off. I think
it would be more consistent to use it there too.
+
+ return led_cdev->regulator && led_cdev->regulator_state != new_state;
+}
+static int __led_handle_regulator(struct led_classdev *led_cdev,
+ int brightness)
+{
+ int rc;
+
+ if (__led_need_regulator_update(led_cdev, brightness)) {
+
+ if (brightness != LED_OFF)
+ rc = regulator_enable(led_cdev->regulator);
+ else
+ rc = regulator_disable(led_cdev->regulator);
+ if (rc)
+ return rc;
+
+ led_cdev->regulator_state = (brightness != LED_OFF);
+ }
+ return 0;
+}
Let's have these function names without leading underscores.
OK.
static int __led_set_brightness(struct led_classdev *led_cdev,
enum led_brightness value)
{
@@ -115,6 +142,8 @@ static void set_brightness_delayed(struct work_struct *ws)
if (ret == -ENOTSUPP)
ret = __led_set_brightness_blocking(led_cdev,
led_cdev->delayed_set_value);
+ __led_handle_regulator(led_cdev, led_cdev->delayed_set_value)
If you called it from __led_set_brightness() and
We cannot call it from __led_set_brightness() because it is supposed not
to block.
JJ
