Am 12.02.2016 um 17:10 schrieb Jacek Anaszewski:

On 02/07/2016 01:20 AM, Heiner Kallweit wrote:

quoted

Add helpers for calling brightness_set(_blocking) allowing to
simplify the code and make it better readable.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
  drivers/leds/led-core.c | 40 ++++++++++++++++++++++++++--------------
  1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index ad684b6..c29e4c9 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c

@@ -25,6 +25,26 @@ EXPORT_SYMBOL_GPL(leds_list_lock);
  LIST_HEAD(leds_list);
  EXPORT_SYMBOL_GPL(leds_list);

+static int led_set_output(struct led_classdev *cdev,

s/cdev/led_cdev/

Please adhere to the existing naming convention.

led_set_output isn't too informative name, and can be misleading.
We already have a number of different internal functions for setting LED brightness, so we have to be careful when adding new ones.

At the moment I don't have any good candidate.

__led_set_brightness() and __led_set_brightness_blocking() as a last
resort. We had the former at some point.

Nonetheless it would be great if you came up with some better
names:)

I was expecting such a remark as I wasn't too happy with the names either.
But I had (and still have) no really good idea for a telling name being not
too long (as I'd like to avoid Java-like variable names spanning across more
than one line). Having said that your proposal of using a double underscore
prefix might still be the best compromise.

quoted

+              enum led_brightness value)
+{
+    if (!cdev->brightness_set)
+        return -ENOTSUPP;
+
+    cdev->brightness_set(cdev, value);
+
+    return 0;
+}
+
+static int led_set_output_blocking(struct led_classdev *cdev,
+                   enum led_brightness value)
+{
+    if (!cdev->brightness_set_blocking)
+        return -ENOTSUPP;
+
+    return cdev->brightness_set_blocking(cdev, value);
+}
+
  static void led_timer_function(unsigned long data)
  {
      struct led_classdev *led_cdev = (void *)data;

@@ -91,13 +111,10 @@ static void set_brightness_delayed(struct work_struct *ws)
          led_cdev->flags &= ~LED_BLINK_DISABLE;
      }

-    if (led_cdev->brightness_set)
-        led_cdev->brightness_set(led_cdev, led_cdev->delayed_set_value);
-    else if (led_cdev->brightness_set_blocking)
-        ret = led_cdev->brightness_set_blocking(led_cdev,
-                        led_cdev->delayed_set_value);
-    else
-        ret = -ENOTSUPP;
+    ret = led_set_output(led_cdev, led_cdev->delayed_set_value);
+    if (ret == -ENOTSUPP)
+        ret = led_set_output_blocking(led_cdev,
+                          led_cdev->delayed_set_value);
      if (ret < 0 &&
          /* LED HW might have been unplugged, therefore don't warn */
          !(ret == -ENODEV && (led_cdev->flags & LED_UNREGISTERING) &&

@@ -236,10 +253,8 @@ void led_set_brightness_nopm(struct led_classdev *led_cdev,
                    enum led_brightness value)
  {
      /* Use brightness_set op if available, it is guaranteed not to sleep */
-    if (led_cdev->brightness_set) {
-        led_cdev->brightness_set(led_cdev, value);
+    if (!led_set_output(led_cdev, value))
          return;
-    }

      /* If brightness setting can sleep, delegate it to a work queue task */
      led_cdev->delayed_set_value = value;

@@ -270,10 +285,7 @@ int led_set_brightness_sync(struct led_classdev *led_cdev,
      if (led_cdev->flags & LED_SUSPENDED)
          return 0;

-    if (led_cdev->brightness_set_blocking)
-        return led_cdev->brightness_set_blocking(led_cdev,
-                             led_cdev->brightness);
-    return -ENOTSUPP;
+    return led_set_output_blocking(led_cdev, led_cdev->brightness);
  }
  EXPORT_SYMBOL_GPL(led_set_brightness_sync);