Improve the logic in order to support not only S0 brightness,
but also the brightness for other indicators and for all
power states.

Signed-off-by: Mauro Carvalho Chehab <mchehab+hua...@kernel.org>
---
 drivers/staging/nuc-led/nuc-wmi.c | 369 ++++++++++++++++++++----------
 1 file changed, 249 insertions(+), 120 deletions(-)

diff --git a/drivers/staging/nuc-led/nuc-wmi.c 
b/drivers/staging/nuc-led/nuc-wmi.c
index 62c2764814dd..711897ba4666 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -55,21 +55,89 @@ enum led_new_get_subcmd {
        LED_NEW_GET_CONTROL_ITEM        = 0x01,
 };
 
+enum led_function {
+       LED_FUNC_BRIGHTNESS,
+       LED_FUNC_COLOR1,
+       LED_FUNC_COLOR_GREEN,
+       LED_FUNC_COLOR_BLUE,
+
+       LED_FUNC_BLINK_BEHAVIOR,
+       LED_FUNC_BLINK_FREQ,
+
+       LED_FUNC_HDD_BEHAVIOR,
+       LED_FUNC_ETH_TYPE,
+       LED_FUNC_POWER_LIMIT_SCHEME,
+
+       MAX_LED_FUNC
+};
+
+enum led_indicators {
+       LED_IND_POWER_STATE,
+       LED_IND_HDD_ACTIVITY,
+       LED_IND_ETHERNET,
+       LED_IND_WIFI,
+       LED_IND_SOFTWARE,
+       LED_IND_POWER_LIMIT,
+       LED_IND_DISABLE,
+
+       MAX_IND = LED_IND_DISABLE
+};
+
+/*
+ * control items ID for each of the valid indicators on spec Rev 0.64.
+ */
+static const u8 led_func_rev_0_64[MAX_IND][MAX_LED_FUNC] = {
+       [LED_IND_POWER_STATE] = {       /* Offsets for each power state */
+               [LED_FUNC_BRIGHTNESS]           = 0x00,
+               [LED_FUNC_BLINK_BEHAVIOR]       = 0x01,
+               [LED_FUNC_BLINK_FREQ]           = 0x02,
+               [LED_FUNC_COLOR1]               = 0x03,
+               [LED_FUNC_COLOR_GREEN]          = 0x04,
+               [LED_FUNC_COLOR_BLUE]           = 0x05
+       },
+       [LED_IND_HDD_ACTIVITY] = {
+               [LED_FUNC_BRIGHTNESS]           = 0x00,
+               [LED_FUNC_COLOR1]               = 0x01,
+               [LED_FUNC_COLOR_GREEN]          = 0x02,
+               [LED_FUNC_COLOR_BLUE]           = 0x03,
+               [LED_FUNC_HDD_BEHAVIOR]         = 0x04
+       },
+       [LED_IND_ETHERNET] = {
+               [LED_FUNC_ETH_TYPE]             = 0x00,
+               [LED_FUNC_BRIGHTNESS]           = 0x01,
+               [LED_FUNC_COLOR1]               = 0x02,
+               [LED_FUNC_COLOR_GREEN]          = 0x03,
+               [LED_FUNC_COLOR_BLUE]           = 0x04
+       },
+       [LED_IND_WIFI] = {
+               [LED_FUNC_BRIGHTNESS]           = 0x00,
+               [LED_FUNC_COLOR1]               = 0x01,
+               [LED_FUNC_COLOR_GREEN]          = 0x02,
+               [LED_FUNC_COLOR_BLUE]           = 0x03
+       },
+       [LED_IND_SOFTWARE] = {
+               [LED_FUNC_BRIGHTNESS]           = 0x00,
+               [LED_FUNC_BLINK_BEHAVIOR]       = 0x01,
+               [LED_FUNC_BLINK_FREQ]           = 0x02,
+               [LED_FUNC_COLOR1]               = 0x03,
+               [LED_FUNC_COLOR_GREEN]          = 0x04,
+               [LED_FUNC_COLOR_BLUE]           = 0x05
+       },
+       [LED_IND_POWER_LIMIT] = {
+               [LED_FUNC_POWER_LIMIT_SCHEME]   = 0x00,
+               [LED_FUNC_BRIGHTNESS]           = 0x01,
+               [LED_FUNC_COLOR1]               = 0x02,
+               [LED_FUNC_COLOR_GREEN]          = 0x03,
+               [LED_FUNC_COLOR_BLUE]           = 0x04
+       },
+};
+
 /* LED color indicator */
 #define LED_BLUE_AMBER         BIT(0)
 #define LED_BLUE_WHITE         BIT(1)
 #define LED_RGB                        BIT(2)
 #define        LED_SINGLE_COLOR        BIT(3)
 
-/* LED indicator options */
-#define LED_IND_POWER_STATE    BIT(0)
-#define LED_IND_HDD_ACTIVITY   BIT(1)
-#define LED_IND_ETHERNET       BIT(2)
-#define LED_IND_WIFI           BIT(3)
-#define LED_IND_SOFTWARE       BIT(4)
-#define LED_IND_POWER_LIMIT    BIT(5)
-#define LED_IND_DISABLE                BIT(6)
-
 static const char * const led_names[] = {
        "nuc::power",
        "nuc::hdd",
@@ -87,7 +155,6 @@ struct nuc_nmi_led {
        u8 indicator;
        u32 color_type;
        u32 avail_indicators;
-       u32 control_items;
 };
 
 struct nuc_wmi {
@@ -201,9 +268,9 @@ static int nuc_nmi_cmd(struct device *dev,
 static int nuc_wmi_query_leds(struct device *dev)
 {
        struct nuc_wmi *priv = dev_get_drvdata(dev);
-       u8 cmd, input[NUM_INPUT_ARGS] = { 0 };
+       u8 input[NUM_INPUT_ARGS] = { 0 };
        u8 output[NUM_OUTPUT_ARGS];
-       int i, id, ret, ver = LED_API_UNKNOWN;
+       int id, ret, ver = LED_API_UNKNOWN;
        u8 leds;
 
        /*
@@ -214,9 +281,8 @@ static int nuc_wmi_query_leds(struct device *dev)
         * FIXME: Should add a fallback code for it to work with older NUCs,
         * as LED_QUERY returns an error on older devices like Skull Canyon.
         */
-       cmd = LED_QUERY;
        input[0] = LED_QUERY_LIST_ALL;
-       ret = nuc_nmi_cmd(dev, cmd, input, output);
+       ret = nuc_nmi_cmd(dev, LED_QUERY, input, output);
        if (ret == -ENOENT) {
                ver = LED_API_NUC6;
        } else if (ret) {
@@ -252,12 +318,11 @@ static int nuc_wmi_query_leds(struct device *dev)
 
                led->id = id;
 
-               cmd = LED_QUERY;
                input[0] = LED_QUERY_COLOR_TYPE;
                input[1] = id;
-               ret = nuc_nmi_cmd(dev, cmd, input, output);
+               ret = nuc_nmi_cmd(dev, LED_QUERY, input, output);
                if (ret) {
-                       dev_warn(dev, "error %d on led %i\n", ret, i);
+                       dev_warn(dev, "error %d on led %i\n", ret, id);
                        return ret;
                }
 
@@ -265,23 +330,11 @@ static int nuc_wmi_query_leds(struct device *dev)
                                  output[1] << 8 |
                                  output[2] << 16;
 
-               cmd = LED_NEW_GET_STATUS;
-               input[0] = LED_NEW_GET_CURRENT_INDICATOR;
-               input[1] = i;
-               ret = nuc_nmi_cmd(dev, cmd, input, output);
-               if (ret) {
-                       dev_warn(dev, "error %d on led %i\n", ret, i);
-                       return ret;
-               }
-
-               led->indicator = output[0];
-
-               cmd = LED_QUERY;
                input[0] = LED_QUERY_INDICATOR_OPTIONS;
-               input[1] = i;
-               ret = nuc_nmi_cmd(dev, cmd, input, output);
+               input[1] = id;
+               ret = nuc_nmi_cmd(dev, LED_QUERY, input, output);
                if (ret) {
-                       dev_warn(dev, "error %d on led %i\n", ret, i);
+                       dev_warn(dev, "error %d on led %i\n", ret, id);
                        return ret;
                }
 
@@ -289,23 +342,18 @@ static int nuc_wmi_query_leds(struct device *dev)
                                        output[1] << 8 |
                                        output[2] << 16;
 
-               cmd = LED_QUERY;
-               input[0] = LED_QUERY_CONTROL_ITEMS;
-               input[1] = i;
-               input[2] = led->indicator;
-               ret = nuc_nmi_cmd(dev, cmd, input, output);
+               input[0] = LED_NEW_GET_CURRENT_INDICATOR;
+               input[1] = id;
+               ret = nuc_nmi_cmd(dev, LED_NEW_GET_STATUS, input, output);
                if (ret) {
-                       dev_warn(dev, "error %d on led %i\n", ret, i);
+                       dev_warn(dev, "error %d on led %i\n", ret, id);
                        return ret;
                }
+               led->indicator = output[0];
 
-               led->control_items = output[0]      |
-                                    output[1] << 8 |
-                                    output[2] << 16;
-
-               dev_dbg(dev, "%s: id: %02x, color type: %06x, indicator: %06x, 
control items: %06x\n",
-                       led_names[led->id], led->id,
-                       led->color_type, led->indicator, led->control_items);
+               dev_dbg(dev, "%s: id: %02x, color type: %06x, indicator: %02x 
(avail %06x)\n",
+                       led_names[led->id], led->id, led->color_type,
+                       led->indicator, led->avail_indicators);
 
                priv->num_leds++;
        }
@@ -313,6 +361,82 @@ static int nuc_wmi_query_leds(struct device *dev)
        return 0;
 }
 
+static bool nuc_wmi_test_control(struct device *dev,
+                                struct nuc_nmi_led *led, u8 ctrl)
+{
+       int ret, avail_ctrls;
+       u8 output[NUM_OUTPUT_ARGS];
+       u8 input[NUM_INPUT_ARGS] = {
+               LED_QUERY_CONTROL_ITEMS,
+               led->id,
+               led->indicator
+       };
+
+       ret = nuc_nmi_cmd(dev, LED_QUERY, input, output);
+       if (ret)
+               return false;
+
+       avail_ctrls = output[0]      |
+                     output[1] << 8 |
+                     output[2] << 16;
+
+       return avail_ctrls & BIT(ctrl);
+}
+
+static int nuc_wmi_get_brightness_offset(struct device *dev,
+                                        struct nuc_nmi_led *led, u8 offset)
+{
+       u8 input[NUM_INPUT_ARGS];
+       u8 output[NUM_OUTPUT_ARGS];
+       int ret, ctrl;
+
+       if (led->indicator == LED_IND_DISABLE)
+               return -ENODEV;
+
+       ctrl = led_func_rev_0_64[led->indicator][LED_FUNC_BRIGHTNESS] + offset;
+
+       if (!nuc_wmi_test_control(dev, led, ctrl))
+               return -ENODEV;
+
+       input[0] = LED_NEW_GET_CONTROL_ITEM;
+       input[1] = led->id;
+       input[2] = led->indicator;
+       input[3] = ctrl;
+
+       ret = nuc_nmi_cmd(dev, LED_NEW_GET_STATUS, input, output);
+       if (ret)
+               return ret;
+
+       dev_dbg(dev, "%s: id: %02x, brightness: %02x\n",
+               led_names[led->id], led->id, output[0]);
+
+       return output[0];
+}
+
+static ssize_t nuc_wmi_set_brightness_offset(struct device *dev,
+                                            struct nuc_nmi_led *led,
+                                            u8 offset,
+                                            u8 val)
+{
+       u8 input[NUM_INPUT_ARGS];
+       int ctrl;
+
+       if (led->indicator == LED_IND_DISABLE)
+               return -ENODEV;
+
+       ctrl = led_func_rev_0_64[led->indicator][LED_FUNC_BRIGHTNESS] + offset;
+
+       if (!nuc_wmi_test_control(dev, led, ctrl))
+               return -ENODEV;
+
+       input[0] = led->id;
+       input[1] = led->indicator;
+       input[2] = ctrl;
+       input[3] = val;
+
+       return nuc_nmi_cmd(dev, LED_SET_VALUE, input, NULL);
+}
+
 /*
  * LED show/store routines
  */
@@ -320,6 +444,21 @@ static int nuc_wmi_query_leds(struct device *dev)
 #define LED_ATTR_RW(_name) \
        DEVICE_ATTR(_name, 0644, show_##_name, store_##_name)
 
+#define LED_ATTR_POWER_STATE_RW(_name, offset)                                \
+       static ssize_t show_##_name(struct device *dev,                        \
+                                   struct device_attribute *attr,             \
+                                   char *buf)                                 \
+       {                                                                      \
+               return show_brightness_offset(dev, attr, offset, buf);         \
+       }                                                                      \
+       static ssize_t store_##_name(struct device *dev,                       \
+                                   struct device_attribute *attr,             \
+                                   const char *buf, size_t len)               \
+       {                                                                      \
+               return store_brightness_offset(dev, attr, offset, buf, len);   \
+       }                                                                      \
+       static DEVICE_ATTR(_name, 0644, show_##_name, store_##_name)
+
 static const char * const led_indicators[] = {
        "Power State",
        "HDD Activity",
@@ -392,98 +531,93 @@ static ssize_t store_indicator(struct device *dev,
        return -EINVAL;
 }
 
-/* Get S0 brightness */
-static ssize_t show_s0_brightness(struct device *dev,
-                                 struct device_attribute *attr,
-                                 char *buf)
+/* Get brightness */
+static ssize_t show_brightness_offset(struct device *dev,
+                                     struct device_attribute *attr,
+                                     u8 offset,
+                                     char *buf)
 {
        struct led_classdev *cdev = dev_get_drvdata(dev);
        struct nuc_nmi_led *led = container_of(cdev, struct nuc_nmi_led, cdev);
-       u8 cmd, input[NUM_INPUT_ARGS] = { 0 };
-       u8 output[NUM_OUTPUT_ARGS];
        int ret;
 
-       cmd = LED_NEW_GET_STATUS;
-       input[0] = LED_NEW_GET_CONTROL_ITEM;
-       input[1] = led->id;
-       input[2] = led->indicator;
-       input[3] = 0;
+       if (led->indicator != LED_IND_POWER_STATE)
+               return -ENODEV;
 
-       ret = nuc_nmi_cmd(dev, cmd, input, output);
-       if (ret)
+       ret = nuc_wmi_get_brightness_offset(dev, led, offset);
+       if (ret < 0)
                return ret;
 
-       /* Multicolor uses a scale from 0 to 100 */
-       if (led->color_type & (LED_BLUE_AMBER | LED_BLUE_WHITE | LED_RGB))
-               return scnprintf(buf, PAGE_SIZE, "%d%%\n", output[0]);
-
-       /* single color uses 0, 50% and 100% */
-       return scnprintf(buf, PAGE_SIZE, "%d%%\n", output[0] * 50);
+       return scnprintf(buf, PAGE_SIZE, "%d\n", ret);
 }
 
-/* Change S0 brightness */
-static ssize_t store_s0_brightness(struct device *dev,
-                                  struct device_attribute *attr,
-                                  const char *buf, size_t len)
+/* Change brightness */
+static ssize_t store_brightness_offset(struct device *dev,
+                                      struct device_attribute *attr,
+                                      u8 offset,
+                                      const char *buf, size_t len)
 {
        struct led_classdev *cdev = dev_get_drvdata(dev);
        struct nuc_nmi_led *led = container_of(cdev, struct nuc_nmi_led, cdev);
-       u8 cmd, input[NUM_INPUT_ARGS] = { 0 };
        int ret;
        u8 val;
 
-       if (led->indicator == LED_IND_DISABLE) {
-               dev_dbg(dev, "Led %s is disabled. ignoring it.\n", cdev->name);
-               return -EACCES;
-       }
+       if (led->indicator != LED_IND_POWER_STATE)
+               return -ENODEV;
 
        if (kstrtou8(buf, 0, &val) || val > 100)
                return -EINVAL;
 
-       /*
-        * For single-color LEDs, the value should be between 0 to 2, but,
-        * in order to have a consistent API, let's always handle it as if
-        * it is a percentage, for both multicolor and single color LEDs.
-        * So:
-        *      value == 0 will disable the LED
-        *      value up to 74% will set it the brightness to 50%
-        *      value equal or above 75% will use the maximum brightness.
-        */
-       if (!(led->color_type & (LED_BLUE_AMBER | LED_BLUE_WHITE | LED_RGB))) {
-               if (val > 0 && val < 75)
-                       val = 1;
-               if (val >= 75)
-                       val = 2;
-       }
-
-       cmd = LED_SET_VALUE;
-       input[0] = led->id;
-       input[1] = led->indicator;
-       input[2] = 0;           /* FIXME: replace by an enum */
-       input[3] = val;
-
-       ret = nuc_nmi_cmd(dev, cmd, input, NULL);
+       ret = nuc_wmi_set_brightness_offset(dev, led, offset, val);
        if (ret)
                return ret;
 
        return len;
 }
 
+static enum led_brightness nuc_wmi_get_brightness(struct led_classdev *cdev)
+{
+       struct nuc_nmi_led *led = container_of(cdev, struct nuc_nmi_led, cdev);
+
+       if (led->indicator == LED_IND_POWER_STATE)
+               return -ENODEV;
+
+       return nuc_wmi_get_brightness_offset(cdev->dev, led, 0);
+}
+
+static int nuc_wmi_set_brightness(struct led_classdev *cdev,
+                                 enum led_brightness brightness)
+{
+       struct nuc_nmi_led *led = container_of(cdev, struct nuc_nmi_led, cdev);
+
+       if (led->indicator == LED_IND_POWER_STATE)
+               return -ENODEV;
+
+       return nuc_wmi_set_brightness_offset(cdev->dev, led, 0, brightness);
+}
+
 static LED_ATTR_RW(indicator);
-static LED_ATTR_RW(s0_brightness);
+
+LED_ATTR_POWER_STATE_RW(s0_brightness, 0x00);
+LED_ATTR_POWER_STATE_RW(s3_brightness, 0x06);
+LED_ATTR_POWER_STATE_RW(s5_brightness, 0x0c);
+LED_ATTR_POWER_STATE_RW(ready_mode_brightness, 0x12);
 
 /*
- * Attributes for multicolor LEDs
+ * Attributes for LEDs
  */
 
-static struct attribute *nuc_wmi_multicolor_led_attr[] = {
+static struct attribute *nuc_wmi_led_attr[] = {
        &dev_attr_indicator.attr,
        &dev_attr_s0_brightness.attr,
+       &dev_attr_s3_brightness.attr,
+       &dev_attr_s5_brightness.attr,
+       &dev_attr_ready_mode_brightness.attr,
        NULL,
 };
 
 static const struct attribute_group nuc_wmi_led_attribute_group = {
-       .attrs          = nuc_wmi_multicolor_led_attr,
+       .attrs = nuc_wmi_led_attr,
 };
 
 static const struct attribute_group *nuc_wmi_led_attribute_groups[] = {
@@ -493,30 +627,25 @@ static const struct attribute_group 
*nuc_wmi_led_attribute_groups[] = {
 
 static int nuc_wmi_led_register(struct device *dev, struct nuc_nmi_led *led)
 {
+       int brightness = nuc_wmi_get_brightness_offset(dev, led, 0);
+
        led->cdev.name = led_names[led->id];
-
        led->dev = dev;
        led->cdev.groups = nuc_wmi_led_attribute_groups;
+       led->cdev.brightness_get = nuc_wmi_get_brightness;
+       led->cdev.brightness_set_blocking = nuc_wmi_set_brightness;
 
-       /*
-        * It can't let the classdev to manage the brightness due to several
-        * reasons:
-        *
-        * 1) classdev has some internal logic to manage the brightness,
-        *    at set_brightness_delayed(), which starts disabling the LEDs;
-        *    While this makes sense on most cases, here, it would appear
-        *    that the NUC was powered off, which is not what happens;
-        * 2) classdev unconditionally tries to set brightness for all
-        *    leds, including the ones that were software-disabled or
-        *    disabled disabled via BIOS menu;
-        * 3) There are 3 types of brightness values for each LED, depending
-        *    on the CPU power state: S0, S3 and S5.
-        *
-        * So, the best seems to export everything via sysfs attributes
-        * directly. This would require some further changes at the
-        * LED class, though, or we would need to create our own LED
-        * class, which seems wrong.
-        */
+       if (led->color_type & LED_SINGLE_COLOR)
+               led->cdev.max_brightness = 2;
+       else
+               led->cdev.max_brightness = 100;
+
+       /* Ensure that the current bright will be preserved */
+       if (brightness >= 0)
+               led->cdev.delayed_set_value = brightness;
+
+       /* Suppress warnings for the LED(s) indicating the power state */
+       led->cdev.flags = LED_HW_PLUGGABLE | LED_UNREGISTERING;
 
        return devm_led_classdev_register(dev, &led->cdev);
 }
-- 
2.31.1

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to