[PATCH v2 05/17] leds: leds-nuc: add all types of brightness

From: Mauro Carvalho Chehab
Date: Tue May 18 2021 - 11:11:28 EST


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+huawei@xxxxxxxxxx>
---
drivers/leds/leds-nuc.c | 369 +++++++++++++++++++++++++++-------------
1 file changed, 249 insertions(+), 120 deletions(-)

diff --git a/drivers/leds/leds-nuc.c b/drivers/leds/leds-nuc.c
index e12fa2e7a488..df65bf17e0e6 100644
--- a/drivers/leds/leds-nuc.c
+++ b/drivers/leds/leds-nuc.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) {
@@ -255,12 +321,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;
}

@@ -268,23 +333,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;
}

@@ -292,23 +345,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++;
}
@@ -316,6 +364,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
*/
@@ -323,6 +447,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",
@@ -395,98 +534,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[] = {
@@ -496,30 +630,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