2021-05-16 18:25:56

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH 00/17] Add an experimental driver for Intel NUC leds

Hi Greg,

This series add support for the LEDs found at Intel NUCs since
NUC version 6.

On several NUC models, the function of the LEDs are programmable,
which allow them to indicate several different hardware events.

They can even be programmed to represent an userspace-driven event.

Some models come with single colored or dual-colored LEDs, but
high end models have RGB LEDs.

Programming them can ether be done via BIOS or by the OS.

There are 3 different API types, and there is already some OOT
drivers that were written to support them, using procfs, each
one using a different (and IMO confusing) API.

After looking at the existing drivers and not liking the uAPI
interfaces there, I opted to write a new driver from scratch,
unifying support for all different versions and using sysfs
via the leds class.

It should be noticed that those devices use the Windows Management
Interface (WMI). There are actually 3 different implementations for it:

- one for NUC6/NUC7, which has limited support for programming just
two LEDs;
- a complely re-written interface for NUC8, which can program up to
seven LEDs, named version 0.64;
- an extended version of the NUC8 API, added for NUC10, called version
1.0, with has a few differences from version 0.64.

Such WMI APIs are documented at:
- https://www.intel.com/content/www/us/en/support/articles/000023426/intel-nuc/intel-nuc-kits.html
- https://raw.githubusercontent.com/nomego/intel_nuc_led/master/specs/INTEL_WMI_LED_0.64.pdf
- https://www.intel.com/content/dam/support/us/en/documents/intel-nuc/WMI-Spec-Intel-NUC-NUC10ixFNx.pdf

I wrote this driver mainly for my NUC8 (NUC8i7HNK), but I used a NUC6
in order to double-check if NUC6 support was not crashing. Yet, while
the NUC6 model I have accepts the WMI LED API, it doesn't work, as it
seems that the BIOS of my NUC6 doesn't let userspace to program the LEDs.

I don't have any devices using NUC10 API.

Due to the lack of full tests on NUC6 and NUC10, and because I
wrote a new uAPI that's different than the procfs-based ones found
at the OOT drivers, I'm opting to submit this first to staging.

This should allow adjusting the uAPI if needed, and to get feedback from
people using it on NUC6, NUC10 and on other NUC models that would be
compatible with it.

Mauro Carvalho Chehab (17):
staging: add support for NUC WMI LEDs
staging: nuc-wmi: detect WMI API detection
staging: nuc-wmi: add support for changing S0 brightness
staging: nuc-wmi: add all types of brightness
staging: nuc-wmi: allow changing the LED colors
staging: nuc-wmi: add support for WMI API version 1.0
staging: nuc-wmi: add basic support for NUC6 WMI
staging: muc-wmi: add brightness and color for NUC6 API
staging: nuc-wmi: Add support to blink behavior for NUC8/10
staging: nuc-wmi: get rid of an unused variable
staging: nuc-wmi: implement blink control for NUC6
staging: nuc-wmi: better detect NUC6/NUC7 devices
staging: nuc-led: add support for HDD activity default
staging: nuc-wmi: fix software blink behavior logic
staging: nuc-wmi: add support for changing the ethernet type indicator
staging: nuc-wmi: add support for changing the power limit scheme
staging: nuc-led: update the TODOs

MAINTAINERS | 6 +
drivers/staging/Kconfig | 2 +
drivers/staging/Makefile | 1 +
drivers/staging/nuc-led/Kconfig | 11 +
drivers/staging/nuc-led/Makefile | 3 +
drivers/staging/nuc-led/TODO | 8 +
drivers/staging/nuc-led/nuc-wmi.c | 2100 +++++++++++++++++++++++++++++
7 files changed, 2131 insertions(+)
create mode 100644 drivers/staging/nuc-led/Kconfig
create mode 100644 drivers/staging/nuc-led/Makefile
create mode 100644 drivers/staging/nuc-led/TODO
create mode 100644 drivers/staging/nuc-led/nuc-wmi.c

--
2.31.1




2021-05-16 18:32:33

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH 08/17] staging: muc-wmi: add brightness and color for NUC6 API

The NUC6 WMI API is really simple: it has just 2 messages,
that retrieves everything for a LED, and it has just 2 LEDs.

Add support for retrieving and set brightness and color.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/staging/nuc-led/nuc-wmi.c | 198 ++++++++++++++++++++++++++++--
1 file changed, 191 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index db38c40c223a..a365a8603182 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -302,14 +302,13 @@ static int nuc_wmi_query_leds_nuc6(struct device *dev)
{
// FIXME: add a check for the specific models that are known to work
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];
struct nuc_nmi_led *led;
int ret;

- cmd = LED_OLD_GET_STATUS;
input[0] = LED_OLD_GET_S0_POWER;
- ret = nuc_nmi_cmd(dev, cmd, input, output);
+ ret = nuc_nmi_cmd(dev, LED_OLD_GET_STATUS, input, output);
if (ret) {
dev_warn(dev, "Get S0 Power: error %d\n", ret);
return ret;
@@ -322,9 +321,8 @@ static int nuc_wmi_query_leds_nuc6(struct device *dev)
led->indicator = fls(led->avail_indicators);
priv->num_leds++;

- cmd = LED_OLD_GET_STATUS;
input[0] = LED_OLD_GET_S0_RING;
- ret = nuc_nmi_cmd(dev, cmd, input, output);
+ ret = nuc_nmi_cmd(dev, LED_OLD_GET_STATUS, input, output);
if (ret) {
dev_warn(dev, "Get S0 Ring: error %d\n", ret);
return ret;
@@ -544,6 +542,167 @@ static ssize_t nuc_wmi_set_brightness_offset(struct device *dev,
} \
static DEVICE_ATTR(_name, 0644, show_##_name, store_##_name)

+/*
+ * NUC6 specific logic
+ */
+
+static int nuc_wmi_nuc6_led_get_set(struct device *dev,
+ struct nuc_nmi_led *led, int *brightness,
+ int *blink_fade, int *color_state)
+{
+ u8 input[NUM_INPUT_ARGS] = { 0 };
+ u8 output[NUM_OUTPUT_ARGS];
+ int ret;
+
+ if (led->id == POWER_LED)
+ input[0] = LED_OLD_GET_S0_POWER;
+ else
+ input[0] = LED_OLD_GET_S0_RING;
+
+ ret = nuc_nmi_cmd(dev, LED_OLD_GET_STATUS, input, output);
+ if (ret) {
+ dev_warn(dev, "Get %s: error %d\n", led_names[led->id], ret);
+ return ret;
+ }
+
+ if (brightness && *brightness >= 0)
+ input[1] = *brightness;
+ else
+ input[1] = output[0];
+
+ if (blink_fade && *blink_fade >= 0)
+ input[2] = *blink_fade;
+ else
+ input[2] = output[1];
+
+ if (color_state && *color_state >= 0)
+ input[3] = *color_state;
+ else
+ input[3] = output[2];
+
+ ret = nuc_nmi_cmd(dev, LED_OLD_SET_LED, input, output);
+ if (ret) {
+ dev_warn(dev, "Get %s: error %d\n", led_names[led->id], ret);
+ return ret;
+ }
+
+ if (brightness)
+ *brightness = output[0];
+ if (blink_fade)
+ *blink_fade = output[1];
+ if (color_state)
+ *color_state = output[2];
+
+ return 0;
+}
+
+static enum led_brightness nuc_wmi_nuc6_get_brightness(struct led_classdev *cdev)
+{
+ struct nuc_nmi_led *led = container_of(cdev, struct nuc_nmi_led, cdev);
+ int ret, brightness = -1;
+
+ ret = nuc_wmi_nuc6_led_get_set(cdev->dev, led, &brightness, NULL, NULL);
+ if (ret)
+ return ret;
+
+ return brightness;
+}
+
+static int nuc_wmi_nuc6_set_brightness(struct led_classdev *cdev,
+ enum led_brightness bright)
+{
+ int brightness = bright;
+ struct nuc_nmi_led *led = container_of(cdev, struct nuc_nmi_led, cdev);
+
+ return nuc_wmi_nuc6_led_get_set(cdev->dev, led, &brightness,
+ NULL, NULL);
+}
+
+static const char * const nuc6_power_colors[] = {
+ "disable",
+ "blue",
+ "amber"
+};
+
+static const char * const nuc6_ring_colors[] = {
+ "disable",
+ "cyan",
+ "pink",
+ "yellow",
+ "blue",
+ "red",
+ "green",
+ "white"
+};
+
+static ssize_t nuc6_show_color(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct led_classdev *cdev = dev_get_drvdata(dev);
+ struct nuc_nmi_led *led = container_of(cdev, struct nuc_nmi_led, cdev);
+ int color = -1, ret, arr_size, i, n;
+ const char * const*color_names;
+ int size = PAGE_SIZE;
+ char *p = buf;
+
+ ret = nuc_wmi_nuc6_led_get_set(dev, led, NULL, NULL, &color);
+ if (ret)
+ return ret;
+
+ if (led->id == POWER_LED) {
+ color_names = nuc6_power_colors;
+ arr_size = ARRAY_SIZE(nuc6_power_colors);
+ } else {
+ color_names = nuc6_ring_colors;
+ arr_size = ARRAY_SIZE(nuc6_ring_colors);
+ }
+
+ for (i = 0; i < arr_size; i++) {
+ if (i == color)
+ n = scnprintf(p, size, "[%s] ", color_names[i]);
+ else
+ n = scnprintf(p, size, "%s ", color_names[i]);
+ p += n;
+ size -= n;
+ }
+ size -= scnprintf(p, size, "\n");
+
+ return PAGE_SIZE - size;
+
+}
+
+static ssize_t nuc6_store_color(struct device *dev,
+ struct device_attribute *attr,
+ 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);
+ const char *tmp;
+ int ret, color;
+
+ tmp = strsep((char **)&buf, ",\n");
+
+ if (led->id == POWER_LED) {
+ for (color = ARRAY_SIZE(nuc6_power_colors)+1; color >= 0; color--)
+ if (!strcasecmp(tmp, nuc6_power_colors[color]))
+ break;
+ } else {
+ for (color = ARRAY_SIZE(nuc6_ring_colors)+1; color >= 0; color--)
+ if (!strcasecmp(tmp, nuc6_ring_colors[color]))
+ break;
+ }
+
+ if (color < 0)
+ return -EINVAL;
+
+ ret = nuc_wmi_nuc6_led_get_set(dev, led, NULL, NULL, &color);
+ if (ret)
+ return ret;
+
+ return len;
+}
+
/* Show/change the LED indicator */

static const char * const led_indicators[] = {
@@ -657,6 +816,9 @@ static ssize_t show_color(struct device *dev,
char *p = buf;
int color, r, g, b;

+ if (led->api_rev == LED_API_NUC6)
+ return nuc6_show_color(dev, attr, buf);
+
if (led->indicator == LED_IND_DISABLE)
return -ENODEV;

@@ -723,6 +885,9 @@ static ssize_t store_color(struct device *dev,
const char *tmp;
u8 r, g, b, val;

+ if (led->api_rev == LED_API_NUC6)
+ return nuc6_store_color(dev, attr, buf, len);
+
if (led->indicator == LED_IND_DISABLE)
return -ENODEV;

@@ -825,6 +990,9 @@ static umode_t nuc_wmi_led_color_is_visible(struct kobject *kobj,
struct nuc_nmi_led *led = container_of(cdev, struct nuc_nmi_led, cdev);
umode_t mode = attr->mode;

+ if (led->api_rev == LED_API_NUC6)
+ return mode;
+
if (led->color_type & LED_SINGLE_COLOR)
return 0;

@@ -978,17 +1146,33 @@ static const struct attribute_group *nuc_wmi_led_attribute_groups[] = {
NULL
};

+static const struct attribute_group *nuc_wmi_nuc6_led_attribute_groups[] = {
+ &nuc_wmi_led_color_attribute_group,
+ NULL
+};
+
static int nuc_wmi_led_register(struct device *dev, struct nuc_nmi_led *led,
enum led_api_rev api_rev)
{
- int brightness;
+ int ret, brightness;

led->cdev.name = led_names[led->id];
led->dev = dev;
led->api_rev = api_rev;

if (led->api_rev == LED_API_NUC6) {
- // FIXME: add NUC6-specific API bits here
+ brightness = -1;
+ ret = nuc_wmi_nuc6_led_get_set(dev, led, &brightness,
+ NULL, NULL);
+ if (ret)
+ return ret;
+
+ led->cdev.groups = nuc_wmi_nuc6_led_attribute_groups;
+ led->cdev.delayed_set_value = brightness;
+ led->cdev.max_brightness = 100;
+ led->cdev.brightness_get = nuc_wmi_nuc6_get_brightness;
+ led->cdev.brightness_set_blocking = nuc_wmi_nuc6_set_brightness;
+
return devm_led_classdev_register(dev, &led->cdev);
}

--
2.31.1


2021-05-16 18:40:45

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH 16/17] staging: nuc-wmi: add support for changing the power limit scheme

The power limit indicator may have 2 behaviors:

1. Its color gradually changes from green to red;
2. It displays a single color

Add support for it.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/staging/nuc-led/nuc-wmi.c | 93 +++++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)

diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index 9e8164cd77ec..2d9c49d72703 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -1764,6 +1764,8 @@ static ssize_t store_ethernet_type(struct device *dev,
if (!nuc_wmi_test_control(dev, led, ctrl))
return -ENODEV;

+ tmp = strsep((char **)&buf, "\n");
+
for (val = 0; val < ARRAY_SIZE(ethernet_type); val++)
if (!strcasecmp(tmp, ethernet_type[val]))
break;
@@ -1783,12 +1785,102 @@ static ssize_t store_ethernet_type(struct device *dev,
return len;
}

+/* Power Limit Indication scheme */
+static const char * const power_limit_scheme[] = {
+ "green to red",
+ "single color"
+};
+
+static ssize_t show_power_limit_scheme(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct led_classdev *cdev = dev_get_drvdata(dev);
+ struct nuc_nmi_led *led = container_of(cdev, struct nuc_nmi_led, cdev);
+ u8 input[NUM_INPUT_ARGS] = { 0 };
+ u8 output[NUM_OUTPUT_ARGS];
+ int ctrl, ret, val, i, n;
+ int size = PAGE_SIZE;
+ char *p = buf;
+
+ if (led->indicator != LED_IND_POWER_LIMIT)
+ return -EINVAL;
+
+ ctrl = led->reg_table[led->indicator][LED_FUNC_POWER_STATE_NUM_CTRLS];
+
+ 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;
+
+ val = output[0];
+
+ for (i = 0; i < ARRAY_SIZE(power_limit_scheme); i++) {
+ if (i == val)
+ n = scnprintf(p, size, "[%s] ", power_limit_scheme[i]);
+ else
+ n = scnprintf(p, size, "%s ", power_limit_scheme[i]);
+ p += n;
+ size -= n;
+ }
+ size -= scnprintf(p, size, "\n");
+
+ return PAGE_SIZE - size;
+}
+
+static ssize_t store_power_limit_scheme(struct device *dev,
+ struct device_attribute *attr,
+ 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 input[NUM_INPUT_ARGS] = { 0 };
+ int ctrl, val, ret;
+ const char *tmp;
+
+ if (led->indicator != LED_IND_POWER_LIMIT)
+ return -EINVAL;
+
+ ctrl = led->reg_table[led->indicator][LED_FUNC_POWER_STATE_NUM_CTRLS];
+
+ if (!nuc_wmi_test_control(dev, led, ctrl))
+ return -ENODEV;
+
+ tmp = strsep((char **)&buf, "\n");
+
+ for (val = 0; val < ARRAY_SIZE(power_limit_scheme); val++)
+ if (!strcasecmp(tmp, power_limit_scheme[val]))
+ break;
+
+ if (val >= ARRAY_SIZE(power_limit_scheme))
+ return -EINVAL;
+
+ input[0] = led->id;
+ input[1] = led->indicator;
+ input[2] = ctrl;
+ input[3] = val;
+
+ ret = nuc_nmi_cmd(dev, LED_SET_VALUE, input, NULL);
+ if (ret)
+ return ret;
+
+ return len;
+}
+
static LED_ATTR_RW(indicator);
static LED_ATTR_RW(color);
static LED_ATTR_RW(blink_behavior);
static LED_ATTR_RW(blink_frequency);
static LED_ATTR_RW(hdd_default);
static LED_ATTR_RW(ethernet_type);
+static LED_ATTR_RW(power_limit_scheme);

LED_ATTR_POWER_STATE_RW(s0_brightness, brightness, 0);
LED_ATTR_POWER_STATE_RW(s0_blink_behavior, blink_behavior, 0);
@@ -1818,6 +1910,7 @@ static struct attribute *nuc_wmi_led_attr[] = {
&dev_attr_indicator.attr,
&dev_attr_hdd_default.attr,
&dev_attr_ethernet_type.attr,
+ &dev_attr_power_limit_scheme.attr,
NULL,
};

--
2.31.1


2021-05-16 18:40:47

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH 09/17] staging: nuc-wmi: Add support to blink behavior for NUC8/10

The hardware blink logic works for both Power State and Software
controlled LEDs.

Just like brightness, there is one different blink behavior
per different power state. Due to that, the logic is somewhat
more complex than what it would be expected otherwise.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/staging/nuc-led/nuc-wmi.c | 347 +++++++++++++++++++++++++++---
1 file changed, 322 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index a365a8603182..8967c8d54dac 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -527,18 +527,30 @@ static ssize_t nuc_wmi_set_brightness_offset(struct device *dev,
#define LED_ATTR_RW(_name) \
DEVICE_ATTR(_name, 0644, show_##_name, store_##_name)

-#define LED_ATTR_POWER_STATE_RW(_name, offset) \
+#define LED_ATTR_POWER_STATE_RW(_name, _offname, _offset) \
static ssize_t show_##_name(struct device *dev, \
struct device_attribute *attr, \
char *buf) \
{ \
- return show_brightness_offset(dev, attr, offset, buf); \
+ struct led_classdev *cdev = dev_get_drvdata(dev); \
+ struct nuc_nmi_led *led; \
+ \
+ led = container_of(cdev, struct nuc_nmi_led, cdev); \
+ if (led->indicator != LED_IND_POWER_STATE) \
+ return -ENODEV; \
+ return offset_show_##_offname(dev, attr, _offset, buf); \
} \
static ssize_t store_##_name(struct device *dev, \
- struct device_attribute *attr, \
- const char *buf, size_t len) \
+ struct device_attribute *attr, \
+ const char *buf, size_t len) \
{ \
- return store_brightness_offset(dev, attr, offset, buf, len); \
+ struct led_classdev *cdev = dev_get_drvdata(dev); \
+ struct nuc_nmi_led *led; \
+ \
+ led = container_of(cdev, struct nuc_nmi_led, cdev); \
+ if (led->indicator != LED_IND_POWER_STATE) \
+ return -ENODEV; \
+ return offset_store_##_offname(dev, attr, _offset, buf, len); \
} \
static DEVICE_ATTR(_name, 0644, show_##_name, store_##_name)

@@ -681,7 +693,7 @@ static ssize_t nuc6_store_color(struct device *dev,
const char *tmp;
int ret, color;

- tmp = strsep((char **)&buf, ",\n");
+ tmp = strsep((char **)&buf, "\n");

if (led->id == POWER_LED) {
for (color = ARRAY_SIZE(nuc6_power_colors)+1; color >= 0; color--)
@@ -1000,7 +1012,7 @@ static umode_t nuc_wmi_led_color_is_visible(struct kobject *kobj,
}

/* Show/store brightness */
-static ssize_t show_brightness_offset(struct device *dev,
+static ssize_t offset_show_brightness(struct device *dev,
struct device_attribute *attr,
u8 offset,
char *buf)
@@ -1009,9 +1021,6 @@ static ssize_t show_brightness_offset(struct device *dev,
struct nuc_nmi_led *led = container_of(cdev, struct nuc_nmi_led, cdev);
int ret;

- if (led->indicator != LED_IND_POWER_STATE)
- return -ENODEV;
-
offset *= led->reg_table[led->indicator][LED_FUNC_POWER_STATE_NUM_CTRLS];

ret = nuc_wmi_get_brightness_offset(dev, led, offset);
@@ -1021,7 +1030,7 @@ static ssize_t show_brightness_offset(struct device *dev,
return scnprintf(buf, PAGE_SIZE, "%d\n", ret);
}

-static ssize_t store_brightness_offset(struct device *dev,
+static ssize_t offset_store_brightness(struct device *dev,
struct device_attribute *attr,
u8 offset,
const char *buf, size_t len)
@@ -1031,9 +1040,6 @@ static ssize_t store_brightness_offset(struct device *dev,
int ret;
u8 val;

- if (led->indicator != LED_IND_POWER_STATE)
- return -ENODEV;
-
if (kstrtou8(buf, 0, &val) || val > 100)
return -EINVAL;

@@ -1067,6 +1073,8 @@ static int nuc_wmi_set_brightness(struct led_classdev *cdev,
return nuc_wmi_set_brightness_offset(cdev->dev, led, 0, brightness);
}

+#define cmp_attr_prefix(a, b) strncmp(a, b, strlen(b))
+
static umode_t nuc_wmi_led_power_state_is_visible(struct kobject *kobj,
struct attribute *attr,
int idx)
@@ -1074,33 +1082,297 @@ static umode_t nuc_wmi_led_power_state_is_visible(struct kobject *kobj,
struct device *dev = kobj_to_dev(kobj);
struct led_classdev *cdev = dev_get_drvdata(dev);
struct nuc_nmi_led *led = container_of(cdev, struct nuc_nmi_led, cdev);
-
umode_t mode = attr->mode;

- if (!strcmp(attr->name, "s0_brightness") ||
- !strcmp(attr->name, "s3_brightness"))
+ if (!cmp_attr_prefix(attr->name, "s0_") ||
+ !cmp_attr_prefix(attr->name, "s3_"))
return mode;

if (led->api_rev == LED_API_REV_0_64) {
- if (!strcmp(attr->name, "s5_brightness") ||
- !strcmp(attr->name, "ready_mode_brightness"))
+ if (!cmp_attr_prefix(attr->name, "s5_") ||
+ !cmp_attr_prefix(attr->name, "ready_mode_"))
return mode;
} else {
- if (!strcmp(attr->name, "standby_brightness"))
+ if (!cmp_attr_prefix(attr->name, "standby_"))
return mode;
}

return 0;
}

+/* Blink */
+static const char * const led_blink_behaviors[] = {
+ "solid",
+ "breathing",
+ "pulsing",
+ "strobing"
+};
+
+static const char * const led_blink_frequencies[] = {
+ "0.1",
+ "0.2",
+ "0.3",
+ "0.4",
+ "0.5",
+ "0.6",
+ "0.7",
+ "0.8",
+ "0.9",
+ "1.0",
+};
+
+static ssize_t offset_show_blink_behavior(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 input[NUM_INPUT_ARGS];
+ u8 output[NUM_OUTPUT_ARGS];
+ int ret, ctrl, val, i, n;
+ int size = PAGE_SIZE;
+ char *p = buf;
+
+ if (led->indicator == LED_IND_DISABLE)
+ return -ENODEV;
+
+ offset *= led->reg_table[led->indicator][LED_FUNC_POWER_STATE_NUM_CTRLS];
+ ctrl = led->reg_table[led->indicator][LED_FUNC_BLINK_BEHAVIOR] + 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;
+
+ val = output[0];
+
+ for (i = 0; i < ARRAY_SIZE(led_blink_behaviors); i++) {
+ if (i == val)
+ n = scnprintf(p, size, "[%s] ", led_blink_behaviors[i]);
+ else
+ n = scnprintf(p, size, "%s ", led_blink_behaviors[i]);
+ p += n;
+ size -= n;
+ }
+ size -= scnprintf(p, size, "\n");
+
+ return PAGE_SIZE - size;
+}
+
+static ssize_t offset_store_blink_behavior(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 input[NUM_INPUT_ARGS] = { 0 };
+ int ctrl, val, ret;
+ const char *tmp;
+
+ if (led->indicator == LED_IND_DISABLE)
+ return -ENODEV;
+
+
+ if (led->id != LED_IND_SOFTWARE && led->id != LED_IND_POWER_STATE)
+ return -ENODEV;
+
+ offset *= led->reg_table[led->indicator][LED_FUNC_POWER_STATE_NUM_CTRLS];
+ ctrl = led->reg_table[led->indicator][LED_FUNC_BLINK_BEHAVIOR] + offset;
+
+ if (!nuc_wmi_test_control(dev, led, ctrl))
+ return -ENODEV;
+
+ tmp = strsep((char **)&buf, "\n");
+
+ for (val = 0; val < ARRAY_SIZE(led_blink_behaviors); val++)
+ if (!strcasecmp(tmp, led_blink_behaviors[val]))
+ break;
+
+ if (val >= ARRAY_SIZE(led_blink_behaviors))
+ return -EINVAL;
+
+ input[0] = led->id;
+ input[1] = led->indicator;
+ input[2] = ctrl;
+ input[3] = val;
+
+ ret = nuc_nmi_cmd(dev, LED_SET_VALUE, input, NULL);
+ if (ret)
+ return ret;
+
+ return len;
+}
+
+static ssize_t show_blink_behavior(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return offset_show_blink_behavior(dev, attr, 0, buf);
+}
+
+static ssize_t store_blink_behavior(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ return offset_store_blink_behavior(dev, attr, 0, buf, len);
+}
+
+static ssize_t offset_show_blink_frequency(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 input[NUM_INPUT_ARGS];
+ u8 output[NUM_OUTPUT_ARGS];
+ int ret, ctrl, val, i, n;
+ int size = PAGE_SIZE;
+ char *p = buf;
+
+ if (led->indicator == LED_IND_DISABLE)
+ return -ENODEV;
+
+ offset *= led->reg_table[led->indicator][LED_FUNC_POWER_STATE_NUM_CTRLS];
+ ctrl = led->reg_table[led->indicator][LED_FUNC_BLINK_BEHAVIOR] + 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;
+
+ val = output[0];
+
+ for (i = 0; i < ARRAY_SIZE(led_blink_frequencies); i++) {
+ if (i == val)
+ n = scnprintf(p, size, "[%s] ", led_blink_frequencies[i]);
+ else
+ n = scnprintf(p, size, "%s ", led_blink_frequencies[i]);
+ p += n;
+ size -= n;
+ }
+ size -= scnprintf(p, size, "\n");
+
+ return PAGE_SIZE - size;
+}
+
+static ssize_t offset_store_blink_frequency(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 input[NUM_INPUT_ARGS] = { 0 };
+ int ctrl, val, ret;
+ const char *tmp;
+
+ if (led->indicator == LED_IND_DISABLE)
+ return -ENODEV;
+
+
+ if (led->id != LED_IND_SOFTWARE && led->id != LED_IND_POWER_STATE)
+ return -ENODEV;
+
+ offset *= led->reg_table[led->indicator][LED_FUNC_POWER_STATE_NUM_CTRLS];
+ ctrl = led->reg_table[led->indicator][LED_FUNC_BLINK_BEHAVIOR] + offset;
+
+ if (!nuc_wmi_test_control(dev, led, ctrl))
+ return -ENODEV;
+
+ tmp = strsep((char **)&buf, "\n");
+
+ for (val = 0; val < ARRAY_SIZE(led_blink_frequencies); val++)
+ if (!strcasecmp(tmp, led_blink_frequencies[val]))
+ break;
+
+ if (val >= ARRAY_SIZE(led_blink_frequencies))
+ return -EINVAL;
+
+ input[0] = led->id;
+ input[1] = led->indicator;
+ input[2] = ctrl;
+ input[3] = val + 1;
+
+ ret = nuc_nmi_cmd(dev, LED_SET_VALUE, input, NULL);
+ if (ret)
+ return ret;
+
+ return len;
+}
+
+static ssize_t show_blink_frequency(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return offset_show_blink_frequency(dev, attr, 0, buf);
+}
+
+static ssize_t store_blink_frequency(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ return offset_store_blink_frequency(dev, attr, 0, buf, len);
+}
+
+static umode_t nuc_wmi_led_blink_is_visible(struct kobject *kobj,
+ struct attribute *attr, int idx)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct led_classdev *cdev = dev_get_drvdata(dev);
+ struct nuc_nmi_led *led = container_of(cdev, struct nuc_nmi_led, cdev);
+ umode_t mode = attr->mode;
+
+ // TODO: implement for NUC6 API
+ if (led->api_rev == LED_API_NUC6)
+ return 0;
+
+ if (led->id == LED_IND_SOFTWARE)
+ return mode;
+
+ return 0;
+}
+
static LED_ATTR_RW(indicator);
static LED_ATTR_RW(color);
+static LED_ATTR_RW(blink_behavior);
+static LED_ATTR_RW(blink_frequency);

-LED_ATTR_POWER_STATE_RW(s0_brightness, 0);
-LED_ATTR_POWER_STATE_RW(s3_brightness, 1);
-LED_ATTR_POWER_STATE_RW(s5_brightness, 2); // Rev 0.64
-LED_ATTR_POWER_STATE_RW(standby_brightness, 2); // Rev 1.0
-LED_ATTR_POWER_STATE_RW(ready_mode_brightness, 3); // Rev 1.0
+LED_ATTR_POWER_STATE_RW(s0_brightness, brightness, 0);
+LED_ATTR_POWER_STATE_RW(s0_blink_behavior, blink_behavior, 0);
+LED_ATTR_POWER_STATE_RW(s0_blink_frequency, blink_frequency, 0);
+LED_ATTR_POWER_STATE_RW(s3_brightness, brightness, 1);
+LED_ATTR_POWER_STATE_RW(s3_blink_behavior, blink_behavior, 1);
+LED_ATTR_POWER_STATE_RW(s3_blink_frequency, blink_frequency, 1);
+
+/* Rev 0.64 */
+LED_ATTR_POWER_STATE_RW(s5_brightness, brightness, 2);
+LED_ATTR_POWER_STATE_RW(s5_blink_behavior, blink_behavior, 2);
+LED_ATTR_POWER_STATE_RW(s5_blink_frequency, blink_frequency, 2);
+LED_ATTR_POWER_STATE_RW(ready_mode_brightness, brightness, 3);
+LED_ATTR_POWER_STATE_RW(ready_mode_blink_behavior, blink_behavior, 3);
+LED_ATTR_POWER_STATE_RW(ready_mode_blink_frequency, blink_frequency, 3);
+
+/* Rev 1.0 */
+LED_ATTR_POWER_STATE_RW(standby_brightness, brightness, 2);
+LED_ATTR_POWER_STATE_RW(standby_blink_behavior, blink_behavior, 2);
+LED_ATTR_POWER_STATE_RW(standby_blink_frequency, blink_frequency, 2);

/*
* Attributes for LEDs
@@ -1121,6 +1393,19 @@ static struct attribute *nuc_wmi_led_power_state_attr[] = {
&dev_attr_standby_brightness.attr,
&dev_attr_s5_brightness.attr,
&dev_attr_ready_mode_brightness.attr,
+
+ &dev_attr_s0_blink_behavior.attr,
+ &dev_attr_s3_blink_behavior.attr,
+ &dev_attr_standby_blink_behavior.attr,
+ &dev_attr_s5_blink_behavior.attr,
+ &dev_attr_ready_mode_blink_behavior.attr,
+
+ &dev_attr_s0_blink_frequency.attr,
+ &dev_attr_s3_blink_frequency.attr,
+ &dev_attr_standby_blink_frequency.attr,
+ &dev_attr_s5_blink_frequency.attr,
+ &dev_attr_ready_mode_blink_frequency.attr,
+
NULL,
};

@@ -1139,10 +1424,22 @@ static const struct attribute_group nuc_wmi_led_color_attribute_group = {
.attrs = nuc_wmi_led_color_attr,
};

+static struct attribute *nuc_wmi_led_blink_behavior_attr[] = {
+ &dev_attr_blink_behavior.attr,
+ &dev_attr_blink_frequency.attr,
+ NULL,
+};
+
+static const struct attribute_group nuc_wmi_led_blink_attribute_group = {
+ .is_visible = nuc_wmi_led_blink_is_visible,
+ .attrs = nuc_wmi_led_blink_behavior_attr,
+};
+
static const struct attribute_group *nuc_wmi_led_attribute_groups[] = {
&nuc_wmi_led_attribute_group,
&nuc_wmi_led_power_state_group,
&nuc_wmi_led_color_attribute_group,
+ &nuc_wmi_led_blink_attribute_group,
NULL
};

--
2.31.1


2021-05-16 18:40:47

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH 01/17] staging: add support for NUC WMI LEDs

Some Intel Next Unit of Computing (NUC) machines have
software-configured LEDs that can be used to display a
variety of events:

- Power State
- HDD Activity
- Ethernet
- WiFi
- Power Limit

They can even be controlled directly via software, without
any hardware-specific indicator connected into them.

Some devices have mono-colored LEDs, but the more advanced
ones have RGB leds that can show any color.

Different color and 4 blink states can be programmed for
thee system states:

- powered on (S0);
- S3;
- Standby.

The NUC BIOSes allow to partially set them for S0, but doesn't
provide any control for the other states, nor does allow
changing the blinking logic.

They all use a WMI interface using GUID:
8C5DA44C-CDC3-46b3-8619-4E26D34390B7

But there are 3 different revisions of the spec, all using
the same GUID, but two different APIs:

- the original one, for NUC6 and to NUCi7:
- https://www.intel.com/content/www/us/en/support/articles/000023426/intel-nuc/intel-nuc-kits.html

- a new one, starting with NUCi8, with two revisions:
- https://raw.githubusercontent.com/nomego/intel_nuc_led/master/specs/INTEL_WMI_LED_0.64.pdf
- https://www.intel.com/content/dam/support/us/en/documents/intel-nuc/WMI-Spec-Intel-NUC-NUC10ixFNx.pdf

There are some OOT drivers for them, but they use procfs
and use a messy interface to setup it. Also, there are
different drivers with the same name, each with support
for each NUC family.

Let's start a new driver from scratch, using the x86 platform
WMI core and the LED class.

This initial version is compatible with NUCi8 and above, and it
was tested with a Hades Canyon NUC (NUC8i7HNK).

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
MAINTAINERS | 6 +
drivers/staging/Kconfig | 2 +
drivers/staging/Makefile | 1 +
drivers/staging/nuc-led/Kconfig | 11 +
drivers/staging/nuc-led/Makefile | 3 +
drivers/staging/nuc-led/TODO | 6 +
drivers/staging/nuc-led/nuc-wmi.c | 489 ++++++++++++++++++++++++++++++
7 files changed, 518 insertions(+)
create mode 100644 drivers/staging/nuc-led/Kconfig
create mode 100644 drivers/staging/nuc-led/Makefile
create mode 100644 drivers/staging/nuc-led/TODO
create mode 100644 drivers/staging/nuc-led/nuc-wmi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index bd7aff0c120f..50d181e1d745 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13063,6 +13063,12 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/aia21/ntfs.git
F: Documentation/filesystems/ntfs.rst
F: fs/ntfs/

+NUC LED DRIVER
+M: Mauro Carvalho Chehab <[email protected]>
+L: [email protected]
+S: Maintained
+F: drivers/staging/nuc-led
+
NUBUS SUBSYSTEM
M: Finn Thain <[email protected]>
L: [email protected]
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index b7ae5bdc4eb5..d1a8e3e08d00 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -84,6 +84,8 @@ source "drivers/staging/greybus/Kconfig"

source "drivers/staging/vc04_services/Kconfig"

+source "drivers/staging/nuc-led/Kconfig"
+
source "drivers/staging/pi433/Kconfig"

source "drivers/staging/mt7621-pci/Kconfig"
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 075c979bfe7c..de937f947edb 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_UNISYSSPAR) += unisys/
obj-$(CONFIG_COMMON_CLK_XLNX_CLKWZRD) += clocking-wizard/
obj-$(CONFIG_FB_TFT) += fbtft/
obj-$(CONFIG_MOST) += most/
+obj-$(CONFIG_LEDS_NUC_WMI) += nuc-led/
obj-$(CONFIG_KS7010) += ks7010/
obj-$(CONFIG_GREYBUS) += greybus/
obj-$(CONFIG_BCM2835_VCHIQ) += vc04_services/
diff --git a/drivers/staging/nuc-led/Kconfig b/drivers/staging/nuc-led/Kconfig
new file mode 100644
index 000000000000..0f870f45bf44
--- /dev/null
+++ b/drivers/staging/nuc-led/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config LEDS_NUC_WMI
+ tristate "Intel NUC WMI support for LEDs"
+ depends on LEDS_CLASS
+ depends on ACPI_WMI
+ help
+ Enable this to support the Intel NUC WMI support for
+ LEDs, starting from NUCi8 and upper devices.
+
+ To compile this driver as a module, choose M here.
diff --git a/drivers/staging/nuc-led/Makefile b/drivers/staging/nuc-led/Makefile
new file mode 100644
index 000000000000..abba9e305fa1
--- /dev/null
+++ b/drivers/staging/nuc-led/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_LEDS_NUC_WMI) += nuc-wmi.o
diff --git a/drivers/staging/nuc-led/TODO b/drivers/staging/nuc-led/TODO
new file mode 100644
index 000000000000..d5296d7186a7
--- /dev/null
+++ b/drivers/staging/nuc-led/TODO
@@ -0,0 +1,6 @@
+- Add support for 6th gen NUCs, like Skull Canyon
+- Improve LED core support to avoid it to try to manage the
+ LED brightness directly;
+- Test it with 8th gen NUCs;
+- Add more functionality to the driver;
+- Stabilize and document its sysfs interface.
diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
new file mode 100644
index 000000000000..15d956ad8556
--- /dev/null
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -0,0 +1,489 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Intel NUC WMI Control WMI Driver
+ *
+ * Currently, it implements only the LED support
+ *
+ * Copyright(c) 2021 Mauro Carvalho Chehab
+ *
+ * Inspired on WMI from https://github.com/nomego/intel_nuc_led
+ *
+ * It follows this spec:
+ * https://www.intel.com/content/dam/support/us/en/documents/intel-nuc/WMI-Spec-Intel-NUC-NUC10ixFNx.pdf
+ */
+
+#include <linux/acpi.h>
+#include <linux/bits.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/wmi.h>
+
+#define NUC_LED_WMI_GUID "8C5DA44C-CDC3-46B3-8619-4E26D34390B7"
+
+#define MAX_LEDS 7
+#define NUM_INPUT_ARGS 4
+#define NUM_OUTPUT_ARGS 3
+
+enum led_cmds {
+ LED_QUERY = 0x03,
+ LED_NEW_GET_STATUS = 0x04,
+ LED_SET_INDICATOR = 0x05,
+ LED_SET_VALUE = 0x06,
+ LED_NOTIFICATION = 0x07,
+ LED_SWITCH_TYPE = 0x08,
+};
+
+enum led_query_subcmd {
+ LED_QUERY_LIST_ALL = 0x00,
+ LED_QUERY_COLOR_TYPE = 0x01,
+ LED_QUERY_INDICATOR_OPTIONS = 0x02,
+ LED_QUERY_CONTROL_ITEMS = 0x03,
+};
+
+enum led_new_get_subcmd {
+ LED_NEW_GET_CURRENT_INDICATOR = 0x00,
+ LED_NEW_GET_CONTROL_ITEM = 0x01,
+};
+
+/* 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",
+ "nuc::skull",
+ "nuc::eyes",
+ "nuc::front1",
+ "nuc::front2",
+ "nuc::front3",
+};
+
+struct nuc_nmi_led {
+ struct led_classdev cdev;
+ struct device *dev;
+ u8 id;
+ u8 indicator;
+ u32 color_type;
+ u32 avail_indicators;
+ u32 control_items;
+};
+
+struct nuc_wmi {
+ struct nuc_nmi_led led[MAX_LEDS * 3]; /* Worse case: RGB LEDs */
+ int num_leds;
+
+ /* Avoid concurrent access to WMI */
+ struct mutex wmi_lock;
+};
+
+static int nuc_nmi_led_error(u8 error_code)
+{
+ switch (error_code) {
+ case 0:
+ return 0;
+ case 0xe1: /* Function not support */
+ return -ENOENT;
+ case 0xe2: /* Undefined device */
+ return -ENODEV;
+ case 0xe3: /* EC no respond */
+ return -EIO;
+ case 0xe4: /* Invalid Parameter */
+ return -EINVAL;
+ case 0xef: /* Unexpected error */
+ return -EFAULT;
+
+ /* Revision 1.0 Errors */
+ case 0xe5: /* Node busy */
+ return -EBUSY;
+ case 0xe6: /* Destination device is disabled or unavailable */
+ return -EACCES;
+ case 0xe7: /* Invalid CEC Opcode */
+ return -ENOENT;
+ case 0xe8: /* Data Buffer size is not enough */
+ return -ENOSPC;
+
+ default: /* Reserved */
+ return -EPROTO;
+ }
+}
+
+static int nuc_nmi_cmd(struct device *dev,
+ u8 cmd,
+ u8 input_args[NUM_INPUT_ARGS],
+ u8 output_args[NUM_OUTPUT_ARGS])
+{
+ struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+ struct nuc_wmi *priv = dev_get_drvdata(dev);
+ struct acpi_buffer input;
+ union acpi_object *obj;
+ acpi_status status;
+ int size, ret;
+ u8 *p;
+
+ input.length = NUM_INPUT_ARGS;
+ input.pointer = input_args;
+
+ mutex_lock(&priv->wmi_lock);
+ status = wmi_evaluate_method(NUC_LED_WMI_GUID, 0, cmd,
+ &input, &output);
+ mutex_unlock(&priv->wmi_lock);
+ if (ACPI_FAILURE(status)) {
+ dev_warn(dev, "cmd %02x (%*ph): ACPI failure: %d\n",
+ cmd, (int)input.length, input_args, ret);
+ return status;
+ }
+
+ obj = output.pointer;
+ if (!obj) {
+ dev_warn(dev, "cmd %02x (%*ph): no output\n",
+ cmd, (int)input.length, input_args);
+ return -EINVAL;
+ }
+
+ if (obj->type == ACPI_TYPE_BUFFER) {
+ if (obj->buffer.length < NUM_OUTPUT_ARGS + 1) {
+ ret = -EINVAL;
+ goto err;
+ }
+ p = (u8 *)obj->buffer.pointer;
+ } else if (obj->type == ACPI_TYPE_INTEGER) {
+ p = (u8 *)&obj->integer.value;
+ } else {
+ return -EINVAL;
+ }
+
+ ret = nuc_nmi_led_error(p[0]);
+ if (ret) {
+ dev_warn(dev, "cmd %02x (%*ph): WMI error code: %02x\n",
+ cmd, (int)input.length, input_args, p[0]);
+ goto err;
+ }
+
+ size = NUM_OUTPUT_ARGS + 1;
+
+ if (output_args) {
+ memcpy(output_args, p + 1, NUM_OUTPUT_ARGS);
+
+ dev_info(dev, "cmd %02x (%*ph), return: %*ph\n",
+ cmd, (int)input.length, input_args, NUM_OUTPUT_ARGS, output_args);
+ } else {
+ dev_info(dev, "cmd %02x (%*ph)\n",
+ cmd, (int)input.length, input_args);
+ }
+
+err:
+ kfree(obj);
+ return ret;
+}
+
+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 output[NUM_OUTPUT_ARGS];
+ int i, id, ret;
+ u8 leds;
+
+ /*
+ * List all LED types support in the platform
+ *
+ * Should work with both NUC8iXXX and NUC10iXXX
+ *
+ * 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);
+ if (ret) {
+ dev_warn(dev, "error %d while listing all LEDs\n", ret);
+ return ret;
+ }
+
+ leds = output[0];
+ if (!leds) {
+ dev_warn(dev, "No LEDs found\n");
+ return -ENODEV;
+ }
+
+ for (id = 0; id < MAX_LEDS; id++) {
+ struct nuc_nmi_led *led = &priv->led[priv->num_leds];
+
+ if (!(leds & BIT(id)))
+ continue;
+
+ led->id = id;
+
+ cmd = LED_QUERY;
+ input[0] = LED_QUERY_COLOR_TYPE;
+ input[1] = id;
+ ret = nuc_nmi_cmd(dev, cmd, input, output);
+ if (ret) {
+ dev_warn(dev, "error %d on led %i\n", ret, i);
+ return ret;
+ }
+
+ led->color_type = output[0] |
+ 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);
+ if (ret) {
+ dev_warn(dev, "error %d on led %i\n", ret, i);
+ return ret;
+ }
+
+ led->avail_indicators = output[0] |
+ 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);
+ if (ret) {
+ dev_warn(dev, "error %d on led %i\n", ret, i);
+ return ret;
+ }
+
+ 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);
+
+ priv->num_leds++;
+ }
+
+ return 0;
+}
+
+/*
+ * LED show/store routines
+ */
+
+#define LED_ATTR_RW(_name) \
+ DEVICE_ATTR(_name, 0644, show_##_name, store_##_name)
+
+static const char * const led_indicators[] = {
+ "Power State",
+ "HDD Activity",
+ "Ethernet",
+ "WiFi",
+ "Software",
+ "Power Limit",
+ "Disable"
+};
+
+static ssize_t show_indicator(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct led_classdev *cdev = dev_get_drvdata(dev);
+ struct nuc_nmi_led *led = container_of(cdev, struct nuc_nmi_led, cdev);
+ int size = PAGE_SIZE;
+ char *p = buf;
+ int i, n;
+
+ for (i = 0; i < fls(led->avail_indicators); i++) {
+ if (!(led->avail_indicators & BIT(i)))
+ continue;
+ if (i == led->indicator)
+ n = scnprintf(p, size, "[%s] ", led_indicators[i]);
+ else
+ n = scnprintf(p, size, "%s ", led_indicators[i]);
+ p += n;
+ size -= n;
+ }
+ size -= scnprintf(p, size, "\n");
+
+ return PAGE_SIZE - size;
+}
+
+static ssize_t store_indicator(struct device *dev,
+ struct device_attribute *attr,
+ 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 };
+ const char *tmp;
+ int ret, i;
+
+ tmp = strsep((char **)&buf, "\n");
+
+ for (i = 0; i < fls(led->avail_indicators); i++) {
+ if (!(led->avail_indicators & BIT(i)))
+ continue;
+
+ if (!strcasecmp(tmp, led_indicators[i])) {
+ cmd = LED_SET_INDICATOR;
+ input[0] = led->id;
+ input[1] = i;
+
+ dev_dbg(dev, "set led %s indicator to %s\n",
+ cdev->name, led_indicators[i]);
+
+ ret = nuc_nmi_cmd(dev, cmd, input, NULL);
+ if (ret)
+ return ret;
+
+ led->indicator = i;
+
+ return len;
+ }
+ }
+
+ return -EINVAL;
+}
+
+static LED_ATTR_RW(indicator);
+
+/*
+ * Attributes for multicolor LEDs
+ */
+
+static struct attribute *nuc_wmi_multicolor_led_attr[] = {
+ &dev_attr_indicator.attr,
+ NULL,
+};
+
+static const struct attribute_group nuc_wmi_led_attribute_group = {
+ .attrs = nuc_wmi_multicolor_led_attr,
+};
+
+static const struct attribute_group *nuc_wmi_led_attribute_groups[] = {
+ &nuc_wmi_led_attribute_group,
+ NULL
+};
+
+static int nuc_wmi_led_register(struct device *dev, struct nuc_nmi_led *led)
+{
+ led->cdev.name = led_names[led->id];
+
+ led->dev = dev;
+ led->cdev.groups = nuc_wmi_led_attribute_groups;
+
+ /*
+ * 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.
+ */
+
+ return devm_led_classdev_register(dev, &led->cdev);
+}
+
+static int nuc_wmi_leds_setup(struct device *dev)
+{
+ struct nuc_wmi *priv = dev_get_drvdata(dev);
+ int ret, i;
+
+ ret = nuc_wmi_query_leds(dev);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < priv->num_leds; i++) {
+ ret = nuc_wmi_led_register(dev, &priv->led[i]);
+ if (ret) {
+ dev_err(dev, "Failed to register led %d: %s\n",
+ i, led_names[priv->led[i].id]);
+ while (--i >= 0)
+ devm_led_classdev_unregister(dev, &priv->led[i].cdev);
+
+ return ret;
+ }
+ }
+ return 0;
+}
+
+static int nuc_wmi_probe(struct wmi_device *wdev, const void *context)
+{
+ struct device *dev = &wdev->dev;
+ struct nuc_wmi *priv;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ mutex_init(&priv->wmi_lock);
+
+ dev_set_drvdata(dev, priv);
+
+ ret = nuc_wmi_leds_setup(dev);
+ if (ret)
+ return ret;
+
+ dev_info(dev, "NUC WMI driver initialized.\n");
+ return 0;
+}
+
+static void nuc_wmi_remove(struct wmi_device *wdev)
+{
+ struct device *dev = &wdev->dev;
+
+ dev_info(dev, "NUC WMI driver removed.\n");
+}
+
+static const struct wmi_device_id nuc_wmi_descriptor_id_table[] = {
+ { .guid_string = NUC_LED_WMI_GUID },
+ { },
+};
+
+static struct wmi_driver nuc_wmi_driver = {
+ .driver = {
+ .name = "nuc-wmi",
+ },
+ .probe = nuc_wmi_probe,
+ .remove = nuc_wmi_remove,
+ .id_table = nuc_wmi_descriptor_id_table,
+};
+
+module_wmi_driver(nuc_wmi_driver);
+
+MODULE_DEVICE_TABLE(wmi, nuc_wmi_descriptor_id_table);
+MODULE_AUTHOR("Mauro Carvalho Chehab <[email protected]>");
+MODULE_DESCRIPTION("Intel NUC WMI driver");
+MODULE_LICENSE("GPL");
--
2.31.1


2021-05-16 18:40:59

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH 13/17] staging: nuc-led: add support for HDD activity default

There are two possible values for HDD activity behavior:

- 0 Normally off, ON when active
- 1 Normally on, OFF when active

Implement a logic to set it.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/staging/nuc-led/nuc-wmi.c | 77 +++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)

diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index 1a6e2b17c888..68143d45c34c 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -1626,10 +1626,86 @@ static umode_t nuc_wmi_led_blink_is_visible(struct kobject *kobj,
return 0;
}

+/* HDD activity behavior */
+static ssize_t show_hdd_default(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct led_classdev *cdev = dev_get_drvdata(dev);
+ struct nuc_nmi_led *led = container_of(cdev, struct nuc_nmi_led, cdev);
+ u8 input[NUM_INPUT_ARGS] = { 0 };
+ u8 output[NUM_OUTPUT_ARGS];
+ int ctrl, ret, val;
+
+ if (led->indicator != LED_IND_HDD_ACTIVITY)
+ return -EINVAL;
+
+ ctrl = led->reg_table[led->indicator][LED_FUNC_HDD_BEHAVIOR];
+
+ 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;
+
+ val = output[0];
+
+ if (val == 0)
+ return scnprintf(buf, PAGE_SIZE, "off\n");
+
+ return scnprintf(buf, PAGE_SIZE, "on\n");
+}
+
+static ssize_t store_hdd_default(struct device *dev,
+ struct device_attribute *attr,
+ 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 input[NUM_INPUT_ARGS] = { 0 };
+ int ctrl, val, ret;
+ const char *tmp;
+
+ if (led->indicator != LED_IND_HDD_ACTIVITY)
+ return -EINVAL;
+
+ ctrl = led->reg_table[led->indicator][LED_FUNC_HDD_BEHAVIOR];
+
+ if (!nuc_wmi_test_control(dev, led, ctrl))
+ return -ENODEV;
+
+ tmp = strsep((char **)&buf, "\n");
+ if (!strcmp(tmp, "on"))
+ val = 1;
+ else if (!strcmp(tmp, "off"))
+ val = 0;
+ else
+ return -EINVAL;
+
+ input[0] = led->id;
+ input[1] = led->indicator;
+ input[2] = ctrl;
+ input[3] = val;
+
+ ret = nuc_nmi_cmd(dev, LED_SET_VALUE, input, NULL);
+ if (ret)
+ return ret;
+
+ return len;
+}
+
+
static LED_ATTR_RW(indicator);
static LED_ATTR_RW(color);
static LED_ATTR_RW(blink_behavior);
static LED_ATTR_RW(blink_frequency);
+static LED_ATTR_RW(hdd_default);

LED_ATTR_POWER_STATE_RW(s0_brightness, brightness, 0);
LED_ATTR_POWER_STATE_RW(s0_blink_behavior, blink_behavior, 0);
@@ -1657,6 +1733,7 @@ LED_ATTR_POWER_STATE_RW(standby_blink_frequency, blink_frequency, 2);

static struct attribute *nuc_wmi_led_attr[] = {
&dev_attr_indicator.attr,
+ &dev_attr_hdd_default.attr,
NULL,
};

--
2.31.1


2021-05-17 09:04:27

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH 00/17] Add an experimental driver for Intel NUC leds

Em Mon, 17 May 2021 10:18:57 +0200
Greg KH <[email protected]> escreveu:

> On Sun, May 16, 2021 at 12:53:28PM +0200, Mauro Carvalho Chehab wrote:
> > Hi Greg,
> >
> > This series add support for the LEDs found at Intel NUCs since
> > NUC version 6.
> >
> > On several NUC models, the function of the LEDs are programmable,
> > which allow them to indicate several different hardware events.
> >
> > They can even be programmed to represent an userspace-driven event.
> >
> > Some models come with single colored or dual-colored LEDs, but
> > high end models have RGB LEDs.
> >
> > Programming them can ether be done via BIOS or by the OS.
> >
> > There are 3 different API types, and there is already some OOT
> > drivers that were written to support them, using procfs, each
> > one using a different (and IMO confusing) API.
> >
> > After looking at the existing drivers and not liking the uAPI
> > interfaces there, I opted to write a new driver from scratch,
> > unifying support for all different versions and using sysfs
> > via the leds class.
>
> Just do this the "right way" and not add it to staging first. Just use
> the existing LED class apis and all should be fine, no need for doing
> anything unusual here.

I'm using the standard LED class already (but not triggers), and the
standard WMI support.

Still, this API is complex, as it controls the LED behavior even when
the machine is suspended. I would feel more comfortable if the ABI
is not set into a stone at the beginning.

But it is your and Pavel's call.

Thanks,
Mauro

2021-05-17 13:10:53

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 00/17] Add an experimental driver for Intel NUC leds

On Sun, May 16, 2021 at 12:53:28PM +0200, Mauro Carvalho Chehab wrote:
> Hi Greg,
>
> This series add support for the LEDs found at Intel NUCs since
> NUC version 6.
>
> On several NUC models, the function of the LEDs are programmable,
> which allow them to indicate several different hardware events.
>
> They can even be programmed to represent an userspace-driven event.
>
> Some models come with single colored or dual-colored LEDs, but
> high end models have RGB LEDs.
>
> Programming them can ether be done via BIOS or by the OS.
>
> There are 3 different API types, and there is already some OOT
> drivers that were written to support them, using procfs, each
> one using a different (and IMO confusing) API.
>
> After looking at the existing drivers and not liking the uAPI
> interfaces there, I opted to write a new driver from scratch,
> unifying support for all different versions and using sysfs
> via the leds class.

Just do this the "right way" and not add it to staging first. Just use
the existing LED class apis and all should be fine, no need for doing
anything unusual here.

thanks,m

greg k-h

2021-05-17 13:10:53

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 01/17] staging: add support for NUC WMI LEDs

On Sun, May 16, 2021 at 12:53:29PM +0200, Mauro Carvalho Chehab wrote:
> Some Intel Next Unit of Computing (NUC) machines have
> software-configured LEDs that can be used to display a
> variety of events:
>
> - Power State
> - HDD Activity
> - Ethernet
> - WiFi
> - Power Limit

<snip>

One nit:

> +static void nuc_wmi_remove(struct wmi_device *wdev)
> +{
> + struct device *dev = &wdev->dev;
> +
> + dev_info(dev, "NUC WMI driver removed.\n");
> +}

Drivers, when working properly, should be quiet. No need for noisy
stuff like this, it just slows down booting/loading for everyone.

thanks,

greg k-h

2021-05-17 13:53:18

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 00/17] Add an experimental driver for Intel NUC leds

On Mon, May 17, 2021 at 11:02:58AM +0200, Mauro Carvalho Chehab wrote:
> Em Mon, 17 May 2021 10:18:57 +0200
> Greg KH <[email protected]> escreveu:
>
> > On Sun, May 16, 2021 at 12:53:28PM +0200, Mauro Carvalho Chehab wrote:
> > > Hi Greg,
> > >
> > > This series add support for the LEDs found at Intel NUCs since
> > > NUC version 6.
> > >
> > > On several NUC models, the function of the LEDs are programmable,
> > > which allow them to indicate several different hardware events.
> > >
> > > They can even be programmed to represent an userspace-driven event.
> > >
> > > Some models come with single colored or dual-colored LEDs, but
> > > high end models have RGB LEDs.
> > >
> > > Programming them can ether be done via BIOS or by the OS.
> > >
> > > There are 3 different API types, and there is already some OOT
> > > drivers that were written to support them, using procfs, each
> > > one using a different (and IMO confusing) API.
> > >
> > > After looking at the existing drivers and not liking the uAPI
> > > interfaces there, I opted to write a new driver from scratch,
> > > unifying support for all different versions and using sysfs
> > > via the leds class.
> >
> > Just do this the "right way" and not add it to staging first. Just use
> > the existing LED class apis and all should be fine, no need for doing
> > anything unusual here.
>
> I'm using the standard LED class already (but not triggers), and the
> standard WMI support.
>
> Still, this API is complex, as it controls the LED behavior even when
> the machine is suspended. I would feel more comfortable if the ABI
> is not set into a stone at the beginning.

code in drivers/staging/ does not mean that you can mess with the
userspace api at will. It still follows the same "rules" of any other
kernel code when it comes to that.

So just work with the LED developers to come to a valid api that works
properly within that framework please.

thanks,

greg k-h