* [PATCH 01/17] staging: add support for NUC WMI LEDs
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 16:12 ` Randy Dunlap
2021-05-17 8:20 ` Greg KH
2021-05-16 10:53 ` [PATCH 02/17] staging: nuc-wmi: detect WMI API detection Mauro Carvalho Chehab
` (16 subsequent siblings)
17 siblings, 2 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
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 <mchehab+huawei@kernel.org>
---
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 <mchehab@kernel.org>
+L: devel@driverdev.osuosl.org
+S: Maintained
+F: drivers/staging/nuc-led
+
NUBUS SUBSYSTEM
M: Finn Thain <fthain@telegraphics.com.au>
L: linux-m68k@lists.linux-m68k.org
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 <mchehab+huawei@kernel.org>");
+MODULE_DESCRIPTION("Intel NUC WMI driver");
+MODULE_LICENSE("GPL");
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 01/17] staging: add support for NUC WMI LEDs
2021-05-16 10:53 ` [PATCH 01/17] staging: add support for NUC WMI LEDs Mauro Carvalho Chehab
@ 2021-05-16 16:12 ` Randy Dunlap
2021-05-17 8:20 ` Greg KH
1 sibling, 0 replies; 31+ messages in thread
From: Randy Dunlap @ 2021-05-16 16:12 UTC (permalink / raw)
To: Mauro Carvalho Chehab, gregkh
Cc: linuxarm, mauro.chehab, Pavel Machek, Mauro Carvalho Chehab,
devel, linux-kernel, linux-leds, linux-staging
Hi Mauro,
On 5/16/21 3:53 AM, Mauro Carvalho Chehab wrote:
> 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
Don't use "support" 2 times. Maybe:
Enable this to support the Intel NUC WMI interface for
or
Enable this to build the Intel NUC WMI support for
> + LEDs, starting from NUCi8 and upper devices.
Does "upper" mean "later"? Or more advanced? Not clear.
> +
> + To compile this driver as a module, choose M here.
thanks.
--
~Randy
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 01/17] staging: add support for NUC WMI LEDs
2021-05-16 10:53 ` [PATCH 01/17] staging: add support for NUC WMI LEDs Mauro Carvalho Chehab
2021-05-16 16:12 ` Randy Dunlap
@ 2021-05-17 8:20 ` Greg KH
1 sibling, 0 replies; 31+ messages in thread
From: Greg KH @ 2021-05-17 8:20 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: linuxarm, mauro.chehab, Pavel Machek, Mauro Carvalho Chehab,
devel, linux-kernel, linux-leds, linux-staging
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 02/17] staging: nuc-wmi: detect WMI API detection
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 01/17] staging: add support for NUC WMI LEDs Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-17 9:35 ` Dan Carpenter
2021-05-16 10:53 ` [PATCH 03/17] staging: nuc-wmi: add support for changing S0 brightness Mauro Carvalho Chehab
` (15 subsequent siblings)
17 siblings, 1 reply; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
There are currently 3 known API releases:
- 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
Add a logic to detect between them, preventing the driver
to work with an unsupported version.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/nuc-led/nuc-wmi.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index 15d956ad8556..b75ddd47e443 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -26,6 +26,13 @@
#define NUM_INPUT_ARGS 4
#define NUM_OUTPUT_ARGS 3
+enum led_api_rev {
+ LED_API_UNKNOWN,
+ LED_API_NUC6,
+ LED_API_REV_0_64,
+ LED_API_REV_1_0,
+};
+
enum led_cmds {
LED_QUERY = 0x03,
LED_NEW_GET_STATUS = 0x04,
@@ -33,6 +40,7 @@ enum led_cmds {
LED_SET_VALUE = 0x06,
LED_NOTIFICATION = 0x07,
LED_SWITCH_TYPE = 0x08,
+ LED_VERSION_CONTROL = 0x09,
};
enum led_query_subcmd {
@@ -195,7 +203,7 @@ 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;
+ int i, id, ret, ver = LED_API_UNKNOWN;
u8 leds;
/*
@@ -209,12 +217,28 @@ static int nuc_wmi_query_leds(struct device *dev)
cmd = LED_QUERY;
input[0] = LED_QUERY_LIST_ALL;
ret = nuc_nmi_cmd(dev, cmd, input, output);
- if (ret) {
+ if (ret == -ENOENT) {
+ ver = LED_API_NUC6;
+ } else if (ret) {
dev_warn(dev, "error %d while listing all LEDs\n", ret);
return ret;
+ } else {
+ leds = output[0];
}
- leds = output[0];
+ if (ver != LED_API_NUC6) {
+ ret = nuc_nmi_cmd(dev, LED_VERSION_CONTROL, input, output);
+ ver = output[0] | output[1] << 16;
+ if (!ver)
+ ver = LED_API_REV_0_64;
+ else if (ver == 0x0126)
+ ver = LED_API_REV_1_0;
+ }
+
+ /* Currently, only API Revision 0.64 is supported */
+ if (ver != LED_API_REV_0_64)
+ return -ENODEV;
+
if (!leds) {
dev_warn(dev, "No LEDs found\n");
return -ENODEV;
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 02/17] staging: nuc-wmi: detect WMI API detection
2021-05-16 10:53 ` [PATCH 02/17] staging: nuc-wmi: detect WMI API detection Mauro Carvalho Chehab
@ 2021-05-17 9:35 ` Dan Carpenter
0 siblings, 0 replies; 31+ messages in thread
From: Dan Carpenter @ 2021-05-17 9:35 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: gregkh, linuxarm, mauro.chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
On Sun, May 16, 2021 at 12:53:30PM +0200, Mauro Carvalho Chehab wrote:
> - leds = output[0];
> + if (ver != LED_API_NUC6) {
> + ret = nuc_nmi_cmd(dev, LED_VERSION_CONTROL, input, output);
Does this need to be checked?
if (ret)
return ret;
> + ver = output[0] | output[1] << 16;
> + if (!ver)
> + ver = LED_API_REV_0_64;
> + else if (ver == 0x0126)
> + ver = LED_API_REV_1_0;
> + }
> +
> + /* Currently, only API Revision 0.64 is supported */
> + if (ver != LED_API_REV_0_64)
> + return -ENODEV;
> +
> if (!leds) {
> dev_warn(dev, "No LEDs found\n");
> return -ENODEV;
regards,
dan carpenter
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 03/17] staging: nuc-wmi: add support for changing S0 brightness
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 01/17] staging: add support for NUC WMI LEDs Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 02/17] staging: nuc-wmi: detect WMI API detection Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 04/17] staging: nuc-wmi: add all types of brightness Mauro Carvalho Chehab
` (14 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
Now that the core logic is in place, let's add support to
adjust the S0 brightness level.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/nuc-led/nuc-wmi.c | 79 +++++++++++++++++++++++++++++++
1 file changed, 79 insertions(+)
diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index b75ddd47e443..62c2764814dd 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -392,7 +392,85 @@ 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)
+{
+ 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;
+
+ ret = nuc_nmi_cmd(dev, cmd, input, output);
+ if (ret)
+ 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);
+}
+
+/* Change S0 brightness */
+static ssize_t store_s0_brightness(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 };
+ 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 (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);
+ if (ret)
+ return ret;
+
+ return len;
+}
+
static LED_ATTR_RW(indicator);
+static LED_ATTR_RW(s0_brightness);
/*
* Attributes for multicolor LEDs
@@ -400,6 +478,7 @@ static LED_ATTR_RW(indicator);
static struct attribute *nuc_wmi_multicolor_led_attr[] = {
&dev_attr_indicator.attr,
+ &dev_attr_s0_brightness.attr,
NULL,
};
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 04/17] staging: nuc-wmi: add all types of brightness
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (2 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 03/17] staging: nuc-wmi: add support for changing S0 brightness Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 05/17] staging: nuc-wmi: allow changing the LED colors Mauro Carvalho Chehab
` (13 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
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@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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 05/17] staging: nuc-wmi: allow changing the LED colors
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (3 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 04/17] staging: nuc-wmi: add all types of brightness Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 06/17] staging: nuc-wmi: add support for WMI API version 1.0 Mauro Carvalho Chehab
` (12 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
Add routines to allow seeing and changing the LED colors.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/nuc-led/nuc-wmi.c | 244 ++++++++++++++++++++++++++++--
1 file changed, 228 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index 711897ba4666..07cf18e6f4c4 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -58,8 +58,6 @@ enum led_new_get_subcmd {
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,
@@ -92,43 +90,31 @@ static const u8 led_func_rev_0_64[MAX_IND][MAX_LED_FUNC] = {
[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
},
};
@@ -459,6 +445,8 @@ static ssize_t nuc_wmi_set_brightness_offset(struct device *dev,
} \
static DEVICE_ATTR(_name, 0644, show_##_name, store_##_name)
+/* Show/change the LED indicator */
+
static const char * const led_indicators[] = {
"Power State",
"HDD Activity",
@@ -531,7 +519,220 @@ static ssize_t store_indicator(struct device *dev,
return -EINVAL;
}
-/* Get brightness */
+/* Show/change the LED color */
+
+enum led_colors {
+ LED_COLOR_BLUE,
+ LED_COLOR_AMBER,
+ LED_COLOR_WHITE
+};
+
+struct led_color_name {
+ const char *name;
+ u8 r, g, b;
+};
+
+static const struct led_color_name led_colors[] = {
+ /* The first colors should match the dual-LED colorset */
+ [LED_COLOR_BLUE] = { "blue", 0, 0, 0xff },
+ [LED_COLOR_AMBER] = { "amber", 0xff, 0xbf, 0 },
+ [LED_COLOR_WHITE] = { "white", 0xff, 0xff, 0xff },
+
+ /* Let's add a couple of common color names as well */
+ { "red", 0xff, 0, 0 },
+ { "green", 0, 0xff, 0 },
+ { "yellow", 0xff, 0xff, 0 },
+ { "cyan", 0, 0xff, 0xff },
+ { "magenta", 0xff, 0, 0xff },
+};
+
+static ssize_t 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);
+ u8 input[NUM_INPUT_ARGS];
+ u8 output[NUM_OUTPUT_ARGS];
+ int ret, ctrl;
+ int size = PAGE_SIZE;
+ char *p = buf;
+ int color, r, g, b;
+
+ if (led->indicator == LED_IND_DISABLE)
+ return -ENODEV;
+
+ ctrl = led_func_rev_0_64[led->indicator][LED_FUNC_COLOR1];
+
+ 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;
+
+ if (led->color_type & LED_RGB) {
+ r = output[0];
+
+ input[3]++;
+ ret = nuc_nmi_cmd(dev, LED_NEW_GET_STATUS, input, output);
+ if (ret)
+ return ret;
+
+ g = output[0];
+
+ input[3]++;
+ ret = nuc_nmi_cmd(dev, LED_NEW_GET_STATUS, input, output);
+ if (ret)
+ return ret;
+
+ b = output[0];
+
+ for (color = 0; color < ARRAY_SIZE(led_colors); color++)
+ if (led_colors[color].r == r &&
+ led_colors[color].g == g &&
+ led_colors[color].b == b)
+ return scnprintf(p, size, "%s\n",
+ led_colors[color].name);
+
+ return scnprintf(p, size, "%d,%d,%d\n", r, g, b);
+ }
+
+ if (!output[0])
+ return scnprintf(p, size, "%s\n",
+ led_colors[LED_COLOR_BLUE].name);
+
+ if (led->color_type & LED_BLUE_AMBER)
+ return scnprintf(p, size, "%s\n",
+ led_colors[LED_COLOR_AMBER].name);
+
+ return scnprintf(p, size, "%s\n", led_colors[LED_COLOR_WHITE].name);
+}
+
+static ssize_t 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);
+ u8 input[NUM_INPUT_ARGS] = { 0 };
+ int ret, ctrl, color;
+ const char *tmp;
+ u8 r, g, b, val;
+
+ if (led->indicator == LED_IND_DISABLE)
+ return -ENODEV;
+
+ tmp = strsep((char **)&buf, ",\n");
+
+ for (color = 0; color < ARRAY_SIZE(led_colors); color++)
+ if (!strcasecmp(tmp, led_colors[color].name))
+ break;
+
+ if (color < ARRAY_SIZE(led_colors)) {
+ r = led_colors[color].r;
+ g = led_colors[color].g;
+ b = led_colors[color].b;
+ } else {
+ if (kstrtou8(tmp, 0, &r) || r > 255)
+ return -EINVAL;
+
+ tmp = strsep((char **)&buf, ",\n");
+ if (kstrtou8(tmp, 0, &g) || g > 255)
+ return -EINVAL;
+
+ tmp = strsep((char **)&buf, " \n");
+ if (kstrtou8(tmp, 0, &b) || b > 255)
+ return -EINVAL;
+
+ if (led->color_type & LED_SINGLE_COLOR) {
+ for (color = 0; color <= LED_COLOR_WHITE; color++)
+ if (led_colors[color].r == r &&
+ led_colors[color].g == g &&
+ led_colors[color].b == b)
+ break;
+ }
+ }
+
+ ctrl = led_func_rev_0_64[led->indicator][LED_FUNC_COLOR1];
+
+ /* Dual color LEDs */
+ if (!(led->color_type & LED_RGB)) {
+ if (color == LED_COLOR_BLUE)
+ val = 0;
+ else {
+ if (led->color_type & LED_BLUE_AMBER &&
+ color != LED_COLOR_AMBER)
+ return -EINVAL;
+ else if (color != LED_COLOR_WHITE)
+ return -EINVAL;
+ val =1;
+ }
+
+ 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;
+ }
+
+ /* RGB LEDs */
+ input[0] = led->id;
+ input[1] = led->indicator;
+ input[2] = ctrl;
+ input[3] = r;
+
+ ret = nuc_nmi_cmd(dev, LED_SET_VALUE, input, NULL);
+ if (ret)
+ return ret;
+
+ input[0] = led->id;
+ input[1] = led->indicator;
+ input[2]++;
+ input[3] = g;
+
+ ret = nuc_nmi_cmd(dev, LED_SET_VALUE, input, NULL);
+ if (ret)
+ return ret;
+
+ input[0] = led->id;
+ input[1] = led->indicator;
+ input[2]++;
+ input[3] = b;
+
+ ret = nuc_nmi_cmd(dev, LED_SET_VALUE, input, NULL);
+ if (ret)
+ return ret;
+
+ return len;
+
+ return -EINVAL;
+}
+
+static umode_t nuc_wmi_led_color_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;
+
+ if (led->color_type & LED_SINGLE_COLOR)
+ return 0;
+
+ return mode;
+}
+
+/* Show/store brightness */
static ssize_t show_brightness_offset(struct device *dev,
struct device_attribute *attr,
u8 offset,
@@ -551,7 +752,6 @@ static ssize_t show_brightness_offset(struct device *dev,
return scnprintf(buf, PAGE_SIZE, "%d\n", ret);
}
-/* Change brightness */
static ssize_t store_brightness_offset(struct device *dev,
struct device_attribute *attr,
u8 offset,
@@ -597,6 +797,7 @@ static int nuc_wmi_set_brightness(struct led_classdev *cdev,
}
static LED_ATTR_RW(indicator);
+static LED_ATTR_RW(color);
LED_ATTR_POWER_STATE_RW(s0_brightness, 0x00);
LED_ATTR_POWER_STATE_RW(s3_brightness, 0x06);
@@ -620,8 +821,19 @@ static const struct attribute_group nuc_wmi_led_attribute_group = {
.attrs = nuc_wmi_led_attr,
};
+static struct attribute *nuc_wmi_led_color_attr[] = {
+ &dev_attr_color.attr,
+ NULL,
+};
+
+static const struct attribute_group nuc_wmi_led_color_attribute_group = {
+ .is_visible = nuc_wmi_led_color_is_visible,
+ .attrs = nuc_wmi_led_color_attr,
+};
+
static const struct attribute_group *nuc_wmi_led_attribute_groups[] = {
&nuc_wmi_led_attribute_group,
+ &nuc_wmi_led_color_attribute_group,
NULL
};
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 06/17] staging: nuc-wmi: add support for WMI API version 1.0
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (4 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 05/17] staging: nuc-wmi: allow changing the LED colors Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 07/17] staging: nuc-wmi: add basic support for NUC6 WMI Mauro Carvalho Chehab
` (11 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
The control indicators for WMI version 1.0 (used on NUCi10
and above) are on different locations.
The main difference is on single color LEDs.
Also, the Power State brightness names are defined on a
different way, and there are 3 groups instead of 4.
As the driver was written with some tables to map the
control option values, it is easy to extend it to support
the new definitions: all we need to do is to add the
V1.0 tables and ensure that the right table will be used.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/nuc-led/nuc-wmi.c | 119 +++++++++++++++++++++++++-----
1 file changed, 99 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index 07cf18e6f4c4..e9c59f656283 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -62,6 +62,7 @@ enum led_function {
LED_FUNC_BLINK_BEHAVIOR,
LED_FUNC_BLINK_FREQ,
+ LED_FUNC_POWER_STATE_NUM_CTRLS,
LED_FUNC_HDD_BEHAVIOR,
LED_FUNC_ETH_TYPE,
LED_FUNC_POWER_LIMIT_SCHEME,
@@ -84,8 +85,11 @@ enum led_indicators {
/*
* 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 */
+static const u8 led_func_multicolor[MAX_IND][MAX_LED_FUNC] = {
+ [LED_IND_POWER_STATE] = {
+ [LED_FUNC_POWER_STATE_NUM_CTRLS] = 0x06,
+
+ /* Offsets for each power state */
[LED_FUNC_BRIGHTNESS] = 0x00,
[LED_FUNC_BLINK_BEHAVIOR] = 0x01,
[LED_FUNC_BLINK_FREQ] = 0x02,
@@ -118,6 +122,24 @@ static const u8 led_func_rev_0_64[MAX_IND][MAX_LED_FUNC] = {
},
};
+static const u8 led_func_rev_1_0_singlecolor[MAX_IND][MAX_LED_FUNC] = {
+ [LED_IND_POWER_STATE] = {
+ [LED_FUNC_POWER_STATE_NUM_CTRLS] = 0x02,
+
+ /* Offsets for each power state */
+ [LED_FUNC_BRIGHTNESS] = 0x00,
+ [LED_FUNC_BLINK_BEHAVIOR] = 0x01,
+ },
+ [LED_IND_HDD_ACTIVITY] = {
+ [LED_FUNC_BRIGHTNESS] = 0x00,
+ [LED_FUNC_HDD_BEHAVIOR] = 0x01
+ },
+ [LED_IND_SOFTWARE] = {
+ [LED_FUNC_BRIGHTNESS] = 0x00,
+ [LED_FUNC_BLINK_BEHAVIOR] = 0x01,
+ },
+};
+
/* LED color indicator */
#define LED_BLUE_AMBER BIT(0)
#define LED_BLUE_WHITE BIT(1)
@@ -141,6 +163,9 @@ struct nuc_nmi_led {
u8 indicator;
u32 color_type;
u32 avail_indicators;
+ enum led_api_rev api_rev;
+
+ const u8 (*reg_table)[MAX_LED_FUNC];
};
struct nuc_wmi {
@@ -251,7 +276,7 @@ static int nuc_nmi_cmd(struct device *dev,
return ret;
}
-static int nuc_wmi_query_leds(struct device *dev)
+static int nuc_wmi_query_leds(struct device *dev, enum led_api_rev *api_rev)
{
struct nuc_wmi *priv = dev_get_drvdata(dev);
u8 input[NUM_INPUT_ARGS] = { 0 };
@@ -288,9 +313,11 @@ static int nuc_wmi_query_leds(struct device *dev)
}
/* Currently, only API Revision 0.64 is supported */
- if (ver != LED_API_REV_0_64)
+ if (ver != LED_API_REV_0_64 && ver != LED_API_REV_1_0)
return -ENODEV;
+ *api_rev = ver;
+
if (!leds) {
dev_warn(dev, "No LEDs found\n");
return -ENODEV;
@@ -379,7 +406,7 @@ static int nuc_wmi_get_brightness_offset(struct device *dev,
if (led->indicator == LED_IND_DISABLE)
return -ENODEV;
- ctrl = led_func_rev_0_64[led->indicator][LED_FUNC_BRIGHTNESS] + offset;
+ ctrl = led->reg_table[led->indicator][LED_FUNC_BRIGHTNESS] + offset;
if (!nuc_wmi_test_control(dev, led, ctrl))
return -ENODEV;
@@ -410,7 +437,7 @@ static ssize_t nuc_wmi_set_brightness_offset(struct device *dev,
if (led->indicator == LED_IND_DISABLE)
return -ENODEV;
- ctrl = led_func_rev_0_64[led->indicator][LED_FUNC_BRIGHTNESS] + offset;
+ ctrl = led->reg_table[led->indicator][LED_FUNC_BRIGHTNESS] + offset;
if (!nuc_wmi_test_control(dev, led, ctrl))
return -ENODEV;
@@ -561,7 +588,7 @@ static ssize_t show_color(struct device *dev,
if (led->indicator == LED_IND_DISABLE)
return -ENODEV;
- ctrl = led_func_rev_0_64[led->indicator][LED_FUNC_COLOR1];
+ ctrl = led->reg_table[led->indicator][LED_FUNC_COLOR1];
if (!nuc_wmi_test_control(dev, led, ctrl))
return -ENODEV;
@@ -658,7 +685,7 @@ static ssize_t store_color(struct device *dev,
}
}
- ctrl = led_func_rev_0_64[led->indicator][LED_FUNC_COLOR1];
+ ctrl = led->reg_table[led->indicator][LED_FUNC_COLOR1];
/* Dual color LEDs */
if (!(led->color_type & LED_RGB)) {
@@ -745,6 +772,8 @@ static ssize_t show_brightness_offset(struct device *dev,
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);
if (ret < 0)
return ret;
@@ -768,6 +797,8 @@ static ssize_t store_brightness_offset(struct device *dev,
if (kstrtou8(buf, 0, &val) || val > 100)
return -EINVAL;
+ offset *= led->reg_table[led->indicator][LED_FUNC_POWER_STATE_NUM_CTRLS];
+
ret = nuc_wmi_set_brightness_offset(dev, led, offset, val);
if (ret)
return ret;
@@ -796,13 +827,40 @@ static int nuc_wmi_set_brightness(struct led_classdev *cdev,
return nuc_wmi_set_brightness_offset(cdev->dev, led, 0, brightness);
}
+static umode_t nuc_wmi_led_power_state_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;
+
+ if (!strcmp(attr->name, "s0_brightness") ||
+ !strcmp(attr->name, "s3_brightness"))
+ return mode;
+
+ if (led->api_rev == LED_API_REV_0_64) {
+ if (!strcmp(attr->name, "s5_brightness") ||
+ !strcmp(attr->name, "ready_mode_brightness"))
+ return mode;
+ } else {
+ if (!strcmp(attr->name, "standby_brightness"))
+ return mode;
+ }
+
+ return 0;
+}
+
static LED_ATTR_RW(indicator);
static LED_ATTR_RW(color);
-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);
+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
/*
* Attributes for LEDs
@@ -810,15 +868,25 @@ LED_ATTR_POWER_STATE_RW(ready_mode_brightness, 0x12);
static struct attribute *nuc_wmi_led_attr[] = {
&dev_attr_indicator.attr,
+ NULL,
+};
+
+static const struct attribute_group nuc_wmi_led_attribute_group = {
+ .attrs = nuc_wmi_led_attr,
+};
+
+static struct attribute *nuc_wmi_led_power_state_attr[] = {
&dev_attr_s0_brightness.attr,
&dev_attr_s3_brightness.attr,
+ &dev_attr_standby_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_led_attr,
+static const struct attribute_group nuc_wmi_led_power_state_group = {
+ .is_visible = nuc_wmi_led_power_state_is_visible,
+ .attrs = nuc_wmi_led_power_state_attr,
};
static struct attribute *nuc_wmi_led_color_attr[] = {
@@ -833,26 +901,36 @@ static const struct attribute_group nuc_wmi_led_color_attribute_group = {
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,
NULL
};
-static int nuc_wmi_led_register(struct device *dev, struct nuc_nmi_led *led)
+static int nuc_wmi_led_register(struct device *dev, struct nuc_nmi_led *led,
+ enum led_api_rev api_rev)
{
- int brightness = nuc_wmi_get_brightness_offset(dev, led, 0);
+ int brightness;
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;
+ led->api_rev = api_rev;
- if (led->color_type & LED_SINGLE_COLOR)
+ if (led->color_type & LED_SINGLE_COLOR) {
+ if (led->api_rev == LED_API_REV_1_0)
+ led->reg_table = led_func_rev_1_0_singlecolor;
+ else
+ led->reg_table = led_func_multicolor;
led->cdev.max_brightness = 2;
- else
+ } else {
led->cdev.max_brightness = 100;
+ led->reg_table = led_func_multicolor;
+ }
/* Ensure that the current bright will be preserved */
+ brightness = nuc_wmi_get_brightness_offset(dev, led, 0);
if (brightness >= 0)
led->cdev.delayed_set_value = brightness;
@@ -865,14 +943,15 @@ static int nuc_wmi_led_register(struct device *dev, struct nuc_nmi_led *led)
static int nuc_wmi_leds_setup(struct device *dev)
{
struct nuc_wmi *priv = dev_get_drvdata(dev);
+ enum led_api_rev api_rev;
int ret, i;
- ret = nuc_wmi_query_leds(dev);
+ ret = nuc_wmi_query_leds(dev, &api_rev);
if (ret)
return ret;
for (i = 0; i < priv->num_leds; i++) {
- ret = nuc_wmi_led_register(dev, &priv->led[i]);
+ ret = nuc_wmi_led_register(dev, &priv->led[i], api_rev);
if (ret) {
dev_err(dev, "Failed to register led %d: %s\n",
i, led_names[priv->led[i].id]);
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 07/17] staging: nuc-wmi: add basic support for NUC6 WMI
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (5 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 06/17] staging: nuc-wmi: add support for WMI API version 1.0 Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-17 9:44 ` Dan Carpenter
2021-05-16 10:53 ` [PATCH 08/17] staging: muc-wmi: add brightness and color for NUC6 API Mauro Carvalho Chehab
` (10 subsequent siblings)
17 siblings, 1 reply; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
The NUC6 and NUCi7 supports an earlier version of the LEDs
WMI, as specified at:
https://www.intel.com/content/www/us/en/support/articles/000023426/intel-nuc/intel-nuc-kits.html
Implement the query part of the LED detection for those devices.
Weird enough, at least with Skull Canyon (NUC6i7KYB) using
the latest firmware release (KYSKLi70 0071), the WMI call
return all zeros. It could well be due to a regression at
the Intel's firmware, although this model was not announced
as supporting this WMI. At the manufacturer's site, only
NUC Kits NUC7i[x]BN and NUC6CAY are mentioned.
Yet, it sounds to me that this is due to a firmware bug:
$ sudo fwts wmi -
...
Test 1 of 1: Windows Management Instrumentation test.
...
\_SB_.WMTF._WDG (1 of 1)
GUID: 86CCFD48-205E-4A77-9C48-2021CBEDE341
WMI Method:
Flags : 0x02 (Method)
Object ID : TF
Instance : 0x01
Driver : intel-wmi-thunderbolt (Intel)
FAILED [LOW] WMIMultipleMethod: Test 1, GUID 86CCFD48-205E-4A77-9C48-2021CBEDE341 has multiple associated methods WMTF defined, this is a firmware bug that leads to ambiguous behaviour.
...
\AMW0._WDG (1 of 2)
GUID: 8C5DA44C-CDC3-46B3-8619-4E26D34390B7
WMI Method:
Flags : 0x02 (Method)
Object ID : AA
Instance : 0x01
PASSED: Test 1, 8C5DA44C-CDC3-46B3-8619-4E26D34390B7 has associated method \AMW0.WMAA
...
Low failures: 1
wmi: GUID 86CCFD48-205E-4A77-9C48-2021CBEDE341 has multiple associated methods WMTF defined, this is a firmware bug that leads to ambiguous behaviour.
Anyway, this was good enough to test that this patch will be
producing exactly the WMI query as the NUC6 OOT driver at:
https://github.com/milesp20/intel_nuc_led/
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/nuc-led/nuc-wmi.c | 134 +++++++++++++++++++++++-------
1 file changed, 106 insertions(+), 28 deletions(-)
diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index e9c59f656283..db38c40c223a 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -8,12 +8,15 @@
*
* 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
+ * It follows those specs:
+ * 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
*/
#include <linux/acpi.h>
#include <linux/bits.h>
+#include <linux/dmi.h>
#include <linux/kernel.h>
#include <linux/leds.h>
#include <linux/module.h>
@@ -34,12 +37,21 @@ enum led_api_rev {
};
enum led_cmds {
+ /* NUC6-specific cmds */
+ LED_OLD_GET_STATUS = 0x01,
+ LED_OLD_SET_LED = 0x02,
+
+ /* Rev 0.64 and 1.0 cmds */
+
LED_QUERY = 0x03,
LED_NEW_GET_STATUS = 0x04,
LED_SET_INDICATOR = 0x05,
LED_SET_VALUE = 0x06,
LED_NOTIFICATION = 0x07,
LED_SWITCH_TYPE = 0x08,
+
+ /* Rev 1.0 cmds */
+
LED_VERSION_CONTROL = 0x09,
};
@@ -55,6 +67,11 @@ enum led_new_get_subcmd {
LED_NEW_GET_CONTROL_ITEM = 0x01,
};
+enum led_old_get_subcmd {
+ LED_OLD_GET_S0_POWER = 0x01,
+ LED_OLD_GET_S0_RING = 0x02,
+};
+
enum led_function {
LED_FUNC_BRIGHTNESS,
LED_FUNC_COLOR1,
@@ -146,14 +163,19 @@ static const u8 led_func_rev_1_0_singlecolor[MAX_IND][MAX_LED_FUNC] = {
#define LED_RGB BIT(2)
#define LED_SINGLE_COLOR BIT(3)
+#define POWER_LED 0
+#define RING_LED (MAX_LEDS + 1)
+
static const char * const led_names[] = {
- "nuc::power",
+ [POWER_LED] = "nuc::power",
"nuc::hdd",
"nuc::skull",
"nuc::eyes",
"nuc::front1",
"nuc::front2",
"nuc::front3",
+
+ [RING_LED] = "nuc::ring", // NUC6 models
};
struct nuc_nmi_led {
@@ -276,48 +298,98 @@ static int nuc_nmi_cmd(struct device *dev,
return ret;
}
+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 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);
+ if (ret) {
+ dev_warn(dev, "Get S0 Power: error %d\n", ret);
+ return ret;
+ }
+
+ led = &priv->led[priv->num_leds];
+ led->id = POWER_LED;
+ led->color_type = LED_BLUE_AMBER;
+ led->avail_indicators = LED_IND_POWER_STATE;
+ 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);
+ if (ret) {
+ dev_warn(dev, "Get S0 Ring: error %d\n", ret);
+ return ret;
+ }
+ led = &priv->led[priv->num_leds];
+ led->id = RING_LED;
+ led->color_type = LED_BLUE_AMBER;
+ led->avail_indicators = LED_IND_SOFTWARE;
+ led->indicator = fls(led->avail_indicators);
+ priv->num_leds++;
+
+ return ret;
+}
+
static int nuc_wmi_query_leds(struct device *dev, enum led_api_rev *api_rev)
{
struct nuc_wmi *priv = dev_get_drvdata(dev);
u8 input[NUM_INPUT_ARGS] = { 0 };
u8 output[NUM_OUTPUT_ARGS];
- int id, ret, ver = LED_API_UNKNOWN;
+ int id, ret, ver = LED_API_UNKNOWN, nuc_ver = 0;
u8 leds;
+ const char *dmi_name;
+
+ dmi_name = dmi_get_system_info(DMI_PRODUCT_NAME);
+ if (!dmi_name || !*dmi_name)
+ dmi_name = dmi_get_system_info(DMI_BOARD_NAME);
+
+ if (strncmp(dmi_name, "NUC", 3))
+ return -ENODEV;
+
+ dmi_name +=3;
+ while (*dmi_name) {
+ if (*dmi_name < '0' || *dmi_name > '9')
+ break;
+ nuc_ver = (*dmi_name - '0') + nuc_ver * 10;
+ dmi_name++;
+ }
+
+ if (nuc_ver < 6)
+ return -ENODEV;
+
+ if (nuc_ver < 8) {
+ *api_rev = LED_API_NUC6;
+ return nuc_wmi_query_leds_nuc6(dev);
+ }
- /*
- * 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.
- */
input[0] = LED_QUERY_LIST_ALL;
ret = nuc_nmi_cmd(dev, LED_QUERY, input, output);
- if (ret == -ENOENT) {
- ver = LED_API_NUC6;
- } else if (ret) {
+ if (ret) {
dev_warn(dev, "error %d while listing all LEDs\n", ret);
return ret;
} else {
leds = output[0];
}
- if (ver != LED_API_NUC6) {
- ret = nuc_nmi_cmd(dev, LED_VERSION_CONTROL, input, output);
- ver = output[0] | output[1] << 16;
- if (!ver)
- ver = LED_API_REV_0_64;
- else if (ver == 0x0126)
- ver = LED_API_REV_1_0;
- }
+ ret = nuc_nmi_cmd(dev, LED_VERSION_CONTROL, input, output);
+ ver = output[0] | output[1] << 16;
+ if (!ver)
+ *api_rev = LED_API_REV_0_64;
+ else if (ver == 0x0126)
+ *api_rev = LED_API_REV_1_0;
- /* Currently, only API Revision 0.64 is supported */
- if (ver != LED_API_REV_0_64 && ver != LED_API_REV_1_0)
+ if (*api_rev == LED_API_UNKNOWN)
return -ENODEV;
- *api_rev = ver;
-
if (!leds) {
dev_warn(dev, "No LEDs found\n");
return -ENODEV;
@@ -913,10 +985,16 @@ 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->api_rev = api_rev;
+
+ if (led->api_rev == LED_API_NUC6) {
+ // FIXME: add NUC6-specific API bits here
+ return devm_led_classdev_register(dev, &led->cdev);
+ }
+
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;
- led->api_rev = api_rev;
if (led->color_type & LED_SINGLE_COLOR) {
if (led->api_rev == LED_API_REV_1_0)
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 07/17] staging: nuc-wmi: add basic support for NUC6 WMI
2021-05-16 10:53 ` [PATCH 07/17] staging: nuc-wmi: add basic support for NUC6 WMI Mauro Carvalho Chehab
@ 2021-05-17 9:44 ` Dan Carpenter
0 siblings, 0 replies; 31+ messages in thread
From: Dan Carpenter @ 2021-05-17 9:44 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: gregkh, linuxarm, mauro.chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
On Sun, May 16, 2021 at 12:53:35PM +0200, Mauro Carvalho Chehab wrote:
> +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 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);
> + if (ret) {
> + dev_warn(dev, "Get S0 Power: error %d\n", ret);
> + return ret;
> + }
> +
> + led = &priv->led[priv->num_leds];
> + led->id = POWER_LED;
> + led->color_type = LED_BLUE_AMBER;
> + led->avail_indicators = LED_IND_POWER_STATE;
> + 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);
> + if (ret) {
> + dev_warn(dev, "Get S0 Ring: error %d\n", ret);
> + return ret;
> + }
> + led = &priv->led[priv->num_leds];
> + led->id = RING_LED;
> + led->color_type = LED_BLUE_AMBER;
> + led->avail_indicators = LED_IND_SOFTWARE;
> + led->indicator = fls(led->avail_indicators);
> + priv->num_leds++;
> +
> + return ret;
return 0;
> +}
> +
> static int nuc_wmi_query_leds(struct device *dev, enum led_api_rev *api_rev)
> {
> struct nuc_wmi *priv = dev_get_drvdata(dev);
> u8 input[NUM_INPUT_ARGS] = { 0 };
> u8 output[NUM_OUTPUT_ARGS];
> - int id, ret, ver = LED_API_UNKNOWN;
> + int id, ret, ver = LED_API_UNKNOWN, nuc_ver = 0;
> u8 leds;
> + const char *dmi_name;
> +
> + dmi_name = dmi_get_system_info(DMI_PRODUCT_NAME);
> + if (!dmi_name || !*dmi_name)
> + dmi_name = dmi_get_system_info(DMI_BOARD_NAME);
> +
> + if (strncmp(dmi_name, "NUC", 3))
> + return -ENODEV;
> +
> + dmi_name +=3;
> + while (*dmi_name) {
> + if (*dmi_name < '0' || *dmi_name > '9')
> + break;
> + nuc_ver = (*dmi_name - '0') + nuc_ver * 10;
> + dmi_name++;
> + }
> +
> + if (nuc_ver < 6)
> + return -ENODEV;
> +
> + if (nuc_ver < 8) {
> + *api_rev = LED_API_NUC6;
> + return nuc_wmi_query_leds_nuc6(dev);
> + }
>
> - /*
> - * 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.
> - */
> input[0] = LED_QUERY_LIST_ALL;
> ret = nuc_nmi_cmd(dev, LED_QUERY, input, output);
> - if (ret == -ENOENT) {
> - ver = LED_API_NUC6;
> - } else if (ret) {
> + if (ret) {
> dev_warn(dev, "error %d while listing all LEDs\n", ret);
> return ret;
> } else {
> leds = output[0];
> }
Delete the else and pull the assignment in a tab.
>
> - if (ver != LED_API_NUC6) {
> - ret = nuc_nmi_cmd(dev, LED_VERSION_CONTROL, input, output);
> - ver = output[0] | output[1] << 16;
> - if (!ver)
> - ver = LED_API_REV_0_64;
> - else if (ver == 0x0126)
> - ver = LED_API_REV_1_0;
> - }
> + ret = nuc_nmi_cmd(dev, LED_VERSION_CONTROL, input, output);
> + ver = output[0] | output[1] << 16;
> + if (!ver)
> + *api_rev = LED_API_REV_0_64;
> + else if (ver == 0x0126)
> + *api_rev = LED_API_REV_1_0;
>
regards,
dan carpenter
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 08/17] staging: muc-wmi: add brightness and color for NUC6 API
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (6 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 07/17] staging: nuc-wmi: add basic support for NUC6 WMI Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 09/17] staging: nuc-wmi: Add support to blink behavior for NUC8/10 Mauro Carvalho Chehab
` (9 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
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 <mchehab+huawei@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 09/17] staging: nuc-wmi: Add support to blink behavior for NUC8/10
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (7 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 08/17] staging: muc-wmi: add brightness and color for NUC6 API Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 10/17] staging: nuc-wmi: get rid of an unused variable Mauro Carvalho Chehab
` (8 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
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 <mchehab+huawei@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 10/17] staging: nuc-wmi: get rid of an unused variable
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (8 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 09/17] staging: nuc-wmi: Add support to blink behavior for NUC8/10 Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 11/17] staging: nuc-wmi: implement blink control for NUC6 Mauro Carvalho Chehab
` (7 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
drivers/staging/nuc-led/nuc-wmi.c: In function ‘nuc_nmi_cmd’:
drivers/staging/nuc-led/nuc-wmi.c:242:6: warning: variable ‘size’ set but not used [-Wunused-but-set-variable]
242 | int size, ret;
| ^~~~
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/nuc-led/nuc-wmi.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index 8967c8d54dac..78b0a3279f25 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -239,7 +239,7 @@ static int nuc_nmi_cmd(struct device *dev,
struct acpi_buffer input;
union acpi_object *obj;
acpi_status status;
- int size, ret;
+ int ret;
u8 *p;
input.length = NUM_INPUT_ARGS;
@@ -281,8 +281,6 @@ static int nuc_nmi_cmd(struct device *dev,
goto err;
}
- size = NUM_OUTPUT_ARGS + 1;
-
if (output_args) {
memcpy(output_args, p + 1, NUM_OUTPUT_ARGS);
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 11/17] staging: nuc-wmi: implement blink control for NUC6
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (9 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 10/17] staging: nuc-wmi: get rid of an unused variable Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 12/17] staging: nuc-wmi: better detect NUC6/NUC7 devices Mauro Carvalho Chehab
` (6 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
The blink control logic for NUC6 API is somewhat messy, as it
uses a single register for controlling both the blink type
and the frequency, using a random order.
Let's use the same API as defined for other versions,
splitting this setting on two different properties.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/nuc-led/nuc-wmi.c | 269 +++++++++++++++++++++++++++++-
1 file changed, 267 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index 78b0a3279f25..5bc4dcec3ea8 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -713,6 +713,247 @@ static ssize_t nuc6_store_color(struct device *dev,
return len;
}
+enum nuc6_blink_mode_freq {
+ NUC6_BLINK_MODE_BLINK_1HZ = 0x01,
+ NUC6_BLINK_MODE_BLINK_0_25HZ = 0x02,
+ NUC6_BLINK_MODE_FADE_1HZ = 0x03,
+ NUC6_BLINK_MODE_DONT_BLINK = 0x04,
+
+ /* BIOS equal or upper AY0038 or BN0043 */
+ NUC6_BLINK_MODE_BLINK_0_5HZ = 0x05,
+ NUC6_BLINK_MODE_FADE_0_25HZ = 0x06,
+ NUC6_BLINK_MODE_FADE_0_5HZ = 0x07
+};
+
+enum nuc6_blink_mode {
+ NUC6_BLINK_MODE_SOLID,
+ NUC6_BLINK_MODE_BLINK,
+ NUC6_BLINK_MODE_FADE
+};
+
+static const char * const nuc6_blink_behavior[] = {
+ "solid",
+ "blink",
+ "fade",
+};
+
+enum nuc6_blink_freq {
+ NUC6_BLINK_FREQ_1HZ,
+ NUC6_BLINK_FREQ_0_5HZ,
+ NUC6_BLINK_FREQ_0_25HZ,
+};
+
+static const char * const nuc6_blink_frequency[] = {
+ "1",
+ "0.5",
+ "0.25",
+};
+
+static int nuc_wmi_nuc6_set_blink(struct device *dev,
+ struct nuc_nmi_led *led,
+ int freq, enum nuc6_blink_mode mode)
+{
+ int val;
+
+ switch(mode) {
+ case NUC6_BLINK_MODE_SOLID:
+ val = NUC6_BLINK_MODE_DONT_BLINK;
+ break;
+ case NUC6_BLINK_MODE_BLINK:
+ if (freq == NUC6_BLINK_FREQ_0_25HZ)
+ val = NUC6_BLINK_MODE_BLINK_0_25HZ;
+ else if (freq == NUC6_BLINK_FREQ_0_5HZ)
+ val = NUC6_BLINK_MODE_BLINK_0_5HZ;
+ else
+ val = NUC6_BLINK_MODE_BLINK_1HZ;
+ break;
+ case NUC6_BLINK_MODE_FADE:
+ if (freq == NUC6_BLINK_FREQ_0_25HZ)
+ val = NUC6_BLINK_MODE_FADE_0_25HZ;
+ else if (freq == NUC6_BLINK_FREQ_0_5HZ)
+ val = NUC6_BLINK_MODE_FADE_0_5HZ;
+ else
+ val = NUC6_BLINK_MODE_FADE_1HZ;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return nuc_wmi_nuc6_led_get_set(dev, led, NULL, &val, NULL);
+}
+
+static ssize_t nuc6_show_blink_behavior(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 val = -1, mode = -1, ret, i, n;
+ int size = PAGE_SIZE;
+ char *p = buf;
+
+ ret = nuc_wmi_nuc6_led_get_set(dev, led, NULL, &val, NULL);
+ if (ret)
+ return ret;
+
+ switch (val) {
+ case NUC6_BLINK_MODE_BLINK_1HZ:
+ case NUC6_BLINK_MODE_BLINK_0_25HZ:
+ case NUC6_BLINK_MODE_BLINK_0_5HZ:
+ mode = NUC6_BLINK_MODE_BLINK;
+ break;
+ case NUC6_BLINK_MODE_FADE_1HZ:
+ case NUC6_BLINK_MODE_FADE_0_25HZ:
+ case NUC6_BLINK_MODE_FADE_0_5HZ:
+ mode = NUC6_BLINK_MODE_FADE;
+ break;
+ case NUC6_BLINK_MODE_DONT_BLINK:
+ mode = NUC6_BLINK_MODE_SOLID;
+ break;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(nuc6_blink_behavior); i++) {
+ if (i == mode)
+ n = scnprintf(p, size, "[%s] ", nuc6_blink_behavior[i]);
+ else
+ n = scnprintf(p, size, "%s ", nuc6_blink_behavior[i]);
+ p += n;
+ size -= n;
+ }
+ size -= scnprintf(p, size, "\n");
+
+ return PAGE_SIZE - size;
+
+}
+
+static ssize_t nuc6_store_blink_behavior(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);
+ int ret, val = -1, freq;
+ const char *tmp;
+
+ tmp = strsep((char **)&buf, "\n");
+
+ ret = nuc_wmi_nuc6_led_get_set(dev, led, NULL, &val, NULL);
+ if (ret)
+ return ret;
+
+ /* Preserve the frequency */
+ switch (val) {
+ case NUC6_BLINK_MODE_BLINK_0_25HZ:
+ case NUC6_BLINK_MODE_FADE_0_25HZ:
+ freq = NUC6_BLINK_FREQ_0_25HZ;
+ break;
+ case NUC6_BLINK_MODE_BLINK_0_5HZ:
+ case NUC6_BLINK_MODE_FADE_0_5HZ:
+ freq = NUC6_BLINK_FREQ_0_5HZ;
+ break;
+ default:
+ freq = NUC6_BLINK_FREQ_1HZ;
+ break;
+ }
+
+ for (val = ARRAY_SIZE(nuc6_blink_behavior)+1; val >= 0; val--)
+ if (!strcasecmp(tmp, nuc6_blink_behavior[val]))
+ break;
+ if (val < 0)
+ return -EINVAL;
+
+ ret = nuc_wmi_nuc6_set_blink(dev, led, val, freq);
+ if (ret)
+ return ret;
+
+ return len;
+}
+
+static ssize_t nuc6_show_blink_frequency(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 val = -1, freq = -1, ret, i, n;
+ int size = PAGE_SIZE;
+ char *p = buf;
+
+ ret = nuc_wmi_nuc6_led_get_set(dev, led, NULL, &val, NULL);
+ if (ret)
+ return ret;
+
+ switch (val) {
+ case NUC6_BLINK_MODE_BLINK_0_25HZ:
+ case NUC6_BLINK_MODE_FADE_0_25HZ:
+ freq = NUC6_BLINK_FREQ_0_25HZ;
+ break;
+ case NUC6_BLINK_MODE_BLINK_0_5HZ:
+ case NUC6_BLINK_MODE_FADE_0_5HZ:
+ freq = NUC6_BLINK_FREQ_0_5HZ;
+ break;
+ default:
+ freq = NUC6_BLINK_FREQ_1HZ;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(nuc6_blink_frequency); i++) {
+ if (i == freq)
+ n = scnprintf(p, size, "[%s] ", nuc6_blink_frequency[i]);
+ else
+ n = scnprintf(p, size, "%s ", nuc6_blink_frequency[i]);
+ p += n;
+ size -= n;
+ }
+ size -= scnprintf(p, size, "\n");
+
+ return PAGE_SIZE - size;
+}
+
+static ssize_t nuc6_store_blink_frequency(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);
+ enum nuc6_blink_mode mode;
+ int ret, freq, val = -1;
+ const char *tmp;
+
+ tmp = strsep((char **)&buf, "\n");
+
+ ret = nuc_wmi_nuc6_led_get_set(dev, led, NULL, &val, NULL);
+ if (ret)
+ return ret;
+
+ /* Preserve the blink mode */
+ switch (val) {
+ case NUC6_BLINK_MODE_BLINK_1HZ:
+ case NUC6_BLINK_MODE_BLINK_0_25HZ:
+ case NUC6_BLINK_MODE_BLINK_0_5HZ:
+ mode = NUC6_BLINK_MODE_BLINK;
+ break;
+ case NUC6_BLINK_MODE_FADE_1HZ:
+ case NUC6_BLINK_MODE_FADE_0_25HZ:
+ case NUC6_BLINK_MODE_FADE_0_5HZ:
+ mode = NUC6_BLINK_MODE_FADE;
+ break;
+ default: /* setting frequency NUC6_BLINK_MODE_SOLID won't make sense */
+ return -EINVAL;
+ }
+
+ for (freq = ARRAY_SIZE(nuc6_blink_frequency)+1; freq >= 0; freq--)
+ if (!strcasecmp(tmp, nuc6_blink_frequency[freq]))
+ break;
+ if (freq < 0)
+ return -EINVAL;
+
+ ret = nuc_wmi_nuc6_set_blink(dev, led, mode, freq);
+ if (ret)
+ return ret;
+
+ return len;
+}
+
/* Show/change the LED indicator */
static const char * const led_indicators[] = {
@@ -1214,6 +1455,12 @@ static ssize_t show_blink_behavior(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);
+
+ if (led->api_rev == LED_API_NUC6)
+ return nuc6_show_blink_behavior(dev, attr, buf);
+
return offset_show_blink_behavior(dev, attr, 0, buf);
}
@@ -1221,6 +1468,12 @@ static ssize_t store_blink_behavior(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);
+
+ if (led->api_rev == LED_API_NUC6)
+ return nuc6_store_blink_behavior(dev, attr, buf, len);
+
return offset_store_blink_behavior(dev, attr, 0, buf, len);
}
@@ -1319,6 +1572,12 @@ static ssize_t show_blink_frequency(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);
+
+ if (led->api_rev == LED_API_NUC6)
+ return nuc6_show_blink_frequency(dev, attr, buf);
+
return offset_show_blink_frequency(dev, attr, 0, buf);
}
@@ -1326,6 +1585,12 @@ static ssize_t store_blink_frequency(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);
+
+ if (led->api_rev == LED_API_NUC6)
+ return nuc6_store_blink_frequency(dev, attr, buf, len);
+
return offset_store_blink_frequency(dev, attr, 0, buf, len);
}
@@ -1337,9 +1602,8 @@ static umode_t nuc_wmi_led_blink_is_visible(struct kobject *kobj,
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;
+ return mode;
if (led->id == LED_IND_SOFTWARE)
return mode;
@@ -1443,6 +1707,7 @@ static const struct attribute_group *nuc_wmi_led_attribute_groups[] = {
static const struct attribute_group *nuc_wmi_nuc6_led_attribute_groups[] = {
&nuc_wmi_led_color_attribute_group,
+ &nuc_wmi_led_blink_attribute_group,
NULL
};
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 12/17] staging: nuc-wmi: better detect NUC6/NUC7 devices
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (10 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 11/17] staging: nuc-wmi: implement blink control for NUC6 Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 13/17] staging: nuc-led: add support for HDD activity default Mauro Carvalho Chehab
` (5 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
There's no documented way to detect if the WMI API is valid,
as, when it is not valid, it just returns 4 zeros.
However, as having a value of 0x00 for the blinking state
is not valid, we can check for it at detection time, in
order to disable LEDs control on devices that won't
support it.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/nuc-led/nuc-wmi.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index 5bc4dcec3ea8..1a6e2b17c888 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -312,6 +312,13 @@ static int nuc_wmi_query_leds_nuc6(struct device *dev)
return ret;
}
+ /*
+ * Detect if NUC6/NUC7 supports the WMI API by checking the
+ * returned blink state, as valid values range from 0x01 to 0x07.
+ */
+ if (output[1] == 0x00)
+ return -ENODEV;
+
led = &priv->led[priv->num_leds];
led->id = POWER_LED;
led->color_type = LED_BLUE_AMBER;
@@ -325,6 +332,14 @@ static int nuc_wmi_query_leds_nuc6(struct device *dev)
dev_warn(dev, "Get S0 Ring: error %d\n", ret);
return ret;
}
+
+ /*
+ * Detect if NUC6/NUC7 supports the WMI API by checking the
+ * returned blink state, as valid values range from 0x01 to 0x07.
+ */
+ if (output[1] == 0x00)
+ return -ENODEV;
+
led = &priv->led[priv->num_leds];
led->id = RING_LED;
led->color_type = LED_BLUE_AMBER;
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 13/17] staging: nuc-led: add support for HDD activity default
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (11 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 12/17] staging: nuc-wmi: better detect NUC6/NUC7 devices Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 14/17] staging: nuc-wmi: fix software blink behavior logic Mauro Carvalho Chehab
` (4 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
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 <mchehab+huawei@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 14/17] staging: nuc-wmi: fix software blink behavior logic
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (12 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 13/17] staging: nuc-led: add support for HDD activity default Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 15/17] staging: nuc-wmi: add support for changing the ethernet type indicator Mauro Carvalho Chehab
` (3 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
The is_visible logic for it is plain wrong:
1. it is used only during devnode creation;
2. it was using the wrong field (id, instead of indicator).
Fix it.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/nuc-led/nuc-wmi.c | 30 ++++++++++++------------------
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index 68143d45c34c..fab0094a20e4 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -1476,6 +1476,9 @@ static ssize_t show_blink_behavior(struct device *dev,
if (led->api_rev == LED_API_NUC6)
return nuc6_show_blink_behavior(dev, attr, buf);
+ if (led->indicator != LED_IND_SOFTWARE)
+ return -EINVAL;
+
return offset_show_blink_behavior(dev, attr, 0, buf);
}
@@ -1489,6 +1492,9 @@ static ssize_t store_blink_behavior(struct device *dev,
if (led->api_rev == LED_API_NUC6)
return nuc6_store_blink_behavior(dev, attr, buf, len);
+ if (led->indicator != LED_IND_SOFTWARE)
+ return -EINVAL;
+
return offset_store_blink_behavior(dev, attr, 0, buf, len);
}
@@ -1593,6 +1599,9 @@ static ssize_t show_blink_frequency(struct device *dev,
if (led->api_rev == LED_API_NUC6)
return nuc6_show_blink_frequency(dev, attr, buf);
+ if (led->indicator != LED_IND_SOFTWARE)
+ return -EINVAL;
+
return offset_show_blink_frequency(dev, attr, 0, buf);
}
@@ -1606,26 +1615,12 @@ static ssize_t store_blink_frequency(struct device *dev,
if (led->api_rev == LED_API_NUC6)
return nuc6_store_blink_frequency(dev, attr, buf, len);
+ if (led->indicator != LED_IND_SOFTWARE)
+ return -EINVAL;
+
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;
-
- if (led->api_rev == LED_API_NUC6)
- return mode;
-
- if (led->id == LED_IND_SOFTWARE)
- return mode;
-
- return 0;
-}
-
/* HDD activity behavior */
static ssize_t show_hdd_default(struct device *dev,
struct device_attribute *attr,
@@ -1785,7 +1780,6 @@ static struct attribute *nuc_wmi_led_blink_behavior_attr[] = {
};
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,
};
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 15/17] staging: nuc-wmi: add support for changing the ethernet type indicator
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (13 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 14/17] staging: nuc-wmi: fix software blink behavior logic Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 16/17] staging: nuc-wmi: add support for changing the power limit scheme Mauro Carvalho Chehab
` (2 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
The Ethernet type indicator can be configured to show the status
of LAN1, LAN1 or both. Add support for it.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/nuc-led/nuc-wmi.c | 89 +++++++++++++++++++++++++++++++
1 file changed, 89 insertions(+)
diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index fab0094a20e4..9e8164cd77ec 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -1695,12 +1695,100 @@ static ssize_t store_hdd_default(struct device *dev,
return len;
}
+/* Ethernet type */
+static const char * const ethernet_type[] = {
+ "LAN1",
+ "LAN2",
+ "LAN1+LAN2"
+};
+
+static ssize_t show_ethernet_type(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_ETHERNET)
+ return -EINVAL;
+
+ ctrl = led->reg_table[led->indicator][LED_FUNC_ETH_TYPE];
+
+ 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(ethernet_type); i++) {
+ if (i == val)
+ n = scnprintf(p, size, "[%s] ", ethernet_type[i]);
+ else
+ n = scnprintf(p, size, "%s ", ethernet_type[i]);
+ p += n;
+ size -= n;
+ }
+ size -= scnprintf(p, size, "\n");
+
+ return PAGE_SIZE - size;
+}
+
+static ssize_t store_ethernet_type(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_ETHERNET)
+ return -EINVAL;
+
+ ctrl = led->reg_table[led->indicator][LED_FUNC_ETH_TYPE];
+
+ if (!nuc_wmi_test_control(dev, led, ctrl))
+ return -ENODEV;
+
+ for (val = 0; val < ARRAY_SIZE(ethernet_type); val++)
+ if (!strcasecmp(tmp, ethernet_type[val]))
+ break;
+
+ if (val >= ARRAY_SIZE(ethernet_type))
+ 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);
LED_ATTR_POWER_STATE_RW(s0_brightness, brightness, 0);
LED_ATTR_POWER_STATE_RW(s0_blink_behavior, blink_behavior, 0);
@@ -1729,6 +1817,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,
+ &dev_attr_ethernet_type.attr,
NULL,
};
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 16/17] staging: nuc-wmi: add support for changing the power limit scheme
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (14 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 15/17] staging: nuc-wmi: add support for changing the ethernet type indicator Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 17/17] staging: nuc-led: update the TODOs Mauro Carvalho Chehab
2021-05-17 8:18 ` [PATCH 00/17] Add an experimental driver for Intel NUC leds Greg KH
17 siblings, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
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 <mchehab+huawei@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 17/17] staging: nuc-led: update the TODOs
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (15 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 16/17] staging: nuc-wmi: add support for changing the power limit scheme Mauro Carvalho Chehab
@ 2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 18:21 ` Pavel Machek
2021-05-17 8:18 ` [PATCH 00/17] Add an experimental driver for Intel NUC leds Greg KH
17 siblings, 1 reply; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-16 10:53 UTC (permalink / raw)
To: gregkh
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Pavel Machek,
Mauro Carvalho Chehab, devel, linux-kernel, linux-leds,
linux-staging
Now that most functionality were merged at the driver,
update its TODO list, and add a "TODO" comment for the two
WMI API commands that are currently not implemented.
In summary:
- on Rev 0.64, command 0x07 (LED_NOTIFICATION) is meant to store
all config settings at EEPROM. That doesn't seem to be needed
on Skull Canyon, but tests with different hardware would be
nice in order to double-check it. Also, maybe Rev 1.00 would
make it mandatory;
- Rev 1.00 added command 0x08 to switch the LED type
(LED_SWITCH_TYPE at the driver's nomenclature) between
single color LED and multi color LED). Not sure how this
should be properly implemented (if this is the case);
- The tests for NUC6 version were using a Skull Canyon NUC.
It allowed to check that the driver's logic runs, but
it is not enough to see if everything is really working.
Tests on NUC6 or NUC7 are required;
- On a suspend test, I noticed that some controls were reset
to the default at resume time. It is required to check
what's happening there and address it properly.
- Need to validate the uAPI and document it before moving
this driver out of staging.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/nuc-led/TODO | 12 +++++++-----
drivers/staging/nuc-led/nuc-wmi.c | 6 ++----
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/nuc-led/TODO b/drivers/staging/nuc-led/TODO
index d5296d7186a7..df6f3f653eb0 100644
--- a/drivers/staging/nuc-led/TODO
+++ b/drivers/staging/nuc-led/TODO
@@ -1,6 +1,8 @@
-- 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;
+- Test it with 6th gen and 10th NUCs;
+- Add support for LED_NOTIFICATION;
+- Add support for LED_SWITCH_TYPE;
+- Some LED parameters are changing after returning
+ from suspend. Need to check what's happening there
+ (hardware bug?) and ensure that the parameters will
+ be properly restored after resume.
- Stabilize and document its sysfs interface.
diff --git a/drivers/staging/nuc-led/nuc-wmi.c b/drivers/staging/nuc-led/nuc-wmi.c
index 2d9c49d72703..e87e97d56364 100644
--- a/drivers/staging/nuc-led/nuc-wmi.c
+++ b/drivers/staging/nuc-led/nuc-wmi.c
@@ -42,16 +42,14 @@ enum led_cmds {
LED_OLD_SET_LED = 0x02,
/* Rev 0.64 and 1.0 cmds */
-
LED_QUERY = 0x03,
LED_NEW_GET_STATUS = 0x04,
LED_SET_INDICATOR = 0x05,
LED_SET_VALUE = 0x06,
- LED_NOTIFICATION = 0x07,
- LED_SWITCH_TYPE = 0x08,
+ LED_NOTIFICATION = 0x07, // TODO: add support for it
/* Rev 1.0 cmds */
-
+ LED_SWITCH_TYPE = 0x08, // TODO: add support for it
LED_VERSION_CONTROL = 0x09,
};
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 17/17] staging: nuc-led: update the TODOs
2021-05-16 10:53 ` [PATCH 17/17] staging: nuc-led: update the TODOs Mauro Carvalho Chehab
@ 2021-05-16 18:21 ` Pavel Machek
2021-05-17 6:30 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 31+ messages in thread
From: Pavel Machek @ 2021-05-16 18:21 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: gregkh, linuxarm, mauro.chehab, Mauro Carvalho Chehab, devel,
linux-kernel, linux-leds, linux-staging
Hi!
> - Need to validate the uAPI and document it before moving
> this driver out of staging.
> - Stabilize and document its sysfs interface.
Would you mind starting with this one? We should have existing APIs
for most of functionality described...
We really don't want to merge code with bad API, not even to staging.
Best regards,
Pavel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 17/17] staging: nuc-led: update the TODOs
2021-05-16 18:21 ` Pavel Machek
@ 2021-05-17 6:30 ` Mauro Carvalho Chehab
2021-05-17 8:05 ` Pavel Machek
0 siblings, 1 reply; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-17 6:30 UTC (permalink / raw)
To: Pavel Machek
Cc: gregkh, linuxarm, mauro.chehab, Mauro Carvalho Chehab, devel,
linux-kernel, linux-leds, linux-staging
Hi Pavel,
Em Sun, 16 May 2021 20:21:50 +0200
Pavel Machek <pavel@ucw.cz> escreveu:
> Hi!
>
> > - Need to validate the uAPI and document it before moving
> > this driver out of staging.
>
> > - Stabilize and document its sysfs interface.
>
> Would you mind starting with this one?
Do you mean writing the ABI document for it? Surely I can do that,
but I'm not sure where to put such document while it is on staging.
> We should have existing APIs
> for most of functionality described...
I tried to stay as close as possible to the existing API, but
there are some things that required a different one.
For instance, with WMI rev 0.64 and 1.0, any LED of the device
can be programmed to be a power indicator.
When a LED is programmed this way, there are up to 3 (on rev 1.0) or up
to 4 (on rev 0.64) different brightness level of the LED, and those
are associated with a power status (like S0, S3, S5, "ready mode").
So, the LED API standard "brightness" is meaningless. On the other
hand, when the same LED is programmed to monitor, let's say, the
WiFi or one of the two Ethernets (or both at the same time), the
standard "brightness" level makes sense.
>
> We really don't want to merge code with bad API, not even to staging.
See, this is the API that it is exposed on with a NUC8:
$ tree /sys/class/leds/nuc\:\:front1/
/sys/class/leds/nuc::front1/
├── blink_behavior
├── blink_frequency
├── brightness
├── color
├── device -> ../../../8C5DA44C-CDC3-46B3-8619-4E26D34390B7
├── ethernet_type
├── hdd_default
├── indicator
├── max_brightness
├── power
│ ├── autosuspend_delay_ms
│ ├── control
│ ├── runtime_active_time
│ ├── runtime_status
│ └── runtime_suspended_time
├── power_limit_scheme
├── ready_mode_blink_behavior
├── ready_mode_blink_frequency
├── ready_mode_brightness
├── s0_blink_behavior
├── s0_blink_frequency
├── s0_brightness
├── s3_blink_behavior
├── s3_blink_frequency
├── s3_brightness
├── s5_blink_behavior
├── s5_blink_frequency
├── s5_brightness
├── subsystem -> ../../../../../../../../class/leds
├── trigger
└── uevent
As the behavior of the LEDs can be dynamically changed, each
LED expose parameters for all types of hardware event it can
deal, but only the ones that are applied to its current indicator
type can be seen/changed.
On other words, the "indicator" tells what type of hardware event
the LED is currently measuring:
$ cat /sys/class/leds/nuc\:\:front1/indicator
Power State [HDD Activity] Ethernet WiFi Software Power Limit Disable
In this case, as it is measuring the HDD activity. If one tries to
read/write something to, let's say, the Ethernet type, a -EINVAL
is returned:
$ cat /sys/class/leds/nuc\:\:front1/ethernet_type
cat: '/sys/class/leds/nuc::front1/ethernet_type': Invalid argument
So, before being able to use the ethernet_type, the indicator needs
to be changed:
$ echo Ethernet > /sys/class/leds/nuc\:\:front1/indicator
$ cat /sys/class/leds/nuc\:\:front1/ethernet_type
LAN1 LAN2 [LAN1+LAN2]
Anyway, I suspect that besides a document under ABI, it would
make sense to add a .rst file describing it under admin-guide,
explaining how to use the ABI.
That should likely be easier to discuss if any changes at the
ABI would be needed. Before moving it out of staging, I would
add another one under Documentation/ABI describing the meaning
of each sysfs node.
Would that work for you?
Thanks,
Mauro
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 17/17] staging: nuc-led: update the TODOs
2021-05-17 6:30 ` Mauro Carvalho Chehab
@ 2021-05-17 8:05 ` Pavel Machek
2021-05-17 8:57 ` Mauro Carvalho Chehab
2021-05-17 12:19 ` Mauro Carvalho Chehab
0 siblings, 2 replies; 31+ messages in thread
From: Pavel Machek @ 2021-05-17 8:05 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: gregkh, linuxarm, mauro.chehab, Mauro Carvalho Chehab, devel,
linux-kernel, linux-leds, linux-staging
[-- Attachment #1: Type: text/plain, Size: 2505 bytes --]
Hi!
> > > - Need to validate the uAPI and document it before moving
> > > this driver out of staging.
> >
> > > - Stabilize and document its sysfs interface.
> >
> > Would you mind starting with this one?
>
> Do you mean writing the ABI document for it? Surely I can do that,
> but I'm not sure where to put such document while it is on staging.
No need for formal ABI just yet, but it needs to be clear what the interface
is.
> > We should have existing APIs
> > for most of functionality described...
>
> I tried to stay as close as possible to the existing API, but
> there are some things that required a different one.
I believe it should be possible to move _way_ closer to existing APIs.
> For instance, with WMI rev 0.64 and 1.0, any LED of the device
> can be programmed to be a power indicator.
>
> When a LED is programmed this way, there are up to 3 (on rev 1.0) or up
> to 4 (on rev 0.64) different brightness level of the LED, and those
> are associated with a power status (like S0, S3, S5, "ready mode").
I'll need a description how this works.
> /sys/class/leds/nuc::front1/
> ├── blink_behavior
> ├── blink_frequency
We have timer trigger for these.
> ├── ethernet_type
> ├── hdd_default
> ├── indicator
> ├── ready_mode_blink_behavior
> ├── ready_mode_blink_frequency
> ├── ready_mode_brightness
> ├── s0_blink_behavior
> ├── s0_blink_frequency
> ├── s0_brightness
> ├── s3_blink_behavior
> ├── s3_blink_frequency
> ├── s3_brightness
> ├── s5_blink_behavior
> ├── s5_blink_frequency
> ├── s5_brightness
No. Take a look at triggers; for example hdd monitor should look very
much like existing disk trigger.
> On other words, the "indicator" tells what type of hardware event
> the LED is currently measuring:
>
> $ cat /sys/class/leds/nuc\:\:front1/indicator
> Power State [HDD Activity] Ethernet WiFi Software Power Limit Disable
So this will use existing "trigger" infrastructure.
> That should likely be easier to discuss if any changes at the
> ABI would be needed. Before moving it out of staging, I would
> add another one under Documentation/ABI describing the meaning
> of each sysfs node.
Lets get reasonable ABI before moving it _into_ tree, staging or
otherwise.
Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 17/17] staging: nuc-led: update the TODOs
2021-05-17 8:05 ` Pavel Machek
@ 2021-05-17 8:57 ` Mauro Carvalho Chehab
2021-05-17 9:12 ` Mauro Carvalho Chehab
2021-05-17 12:19 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-17 8:57 UTC (permalink / raw)
To: Pavel Machek
Cc: gregkh, linuxarm, Mauro Carvalho Chehab, devel, linux-kernel,
linux-leds, linux-staging
Em Mon, 17 May 2021 10:05:27 +0200
Pavel Machek <pavel@ucw.cz> escreveu:
> Hi!
>
> > > > - Need to validate the uAPI and document it before moving
> > > > this driver out of staging.
> > >
> > > > - Stabilize and document its sysfs interface.
> > >
> > > Would you mind starting with this one?
> >
> > Do you mean writing the ABI document for it? Surely I can do that,
> > but I'm not sure where to put such document while it is on staging.
>
> No need for formal ABI just yet, but it needs to be clear what the interface
> is.
>
> > > We should have existing APIs
> > > for most of functionality described...
> >
> > I tried to stay as close as possible to the existing API, but
> > there are some things that required a different one.
>
> I believe it should be possible to move _way_ closer to existing APIs.
>
> > For instance, with WMI rev 0.64 and 1.0, any LED of the device
> > can be programmed to be a power indicator.
> >
> > When a LED is programmed this way, there are up to 3 (on rev 1.0) or up
> > to 4 (on rev 0.64) different brightness level of the LED, and those
> > are associated with a power status (like S0, S3, S5, "ready mode").
>
> I'll need a description how this works.
>
> > /sys/class/leds/nuc::front1/
> > ├── blink_behavior
> > ├── blink_frequency
>
> We have timer trigger for these.
Not really. The LED blink behavior is provided by the hardware itself.
The LEDs can blink *even* when the device is suspended or is
hibernating. That's something that a timer trigger can't do ;-)
See below for a draft of the ABI description.
>
> > ├── ethernet_type
> > ├── hdd_default
> > ├── indicator
> > ├── ready_mode_blink_behavior
> > ├── ready_mode_blink_frequency
> > ├── ready_mode_brightness
> > ├── s0_blink_behavior
> > ├── s0_blink_frequency
> > ├── s0_brightness
> > ├── s3_blink_behavior
> > ├── s3_blink_frequency
> > ├── s3_brightness
> > ├── s5_blink_behavior
> > ├── s5_blink_frequency
> > ├── s5_brightness
>
> No. Take a look at triggers; for example hdd monitor should look very
> much like existing disk trigger.
Ok, I'll double-check how this works. Yeah, it would be a way better if
the sysfs nodes could be hidden when changing the indicator type.
For instance, when monitoring disk activity, only those parameters
may be available:
================================= =======================================
Parameter Meaning
================================= =======================================
brightness Brightness in percent (from 0 to 100)
color LED color.
See :ref:`nuc_color`.
hdd_default Default is LED turned ON or OFF.
When set toOFF, the LED will turn on
at disk activity.
When set to ON, the LED will be turned
on by default, turning off at disk
activity.
================================= =======================================
(color is only available for multi-colored or RGB leds).
>
> > On other words, the "indicator" tells what type of hardware event
> > the LED is currently measuring:
> >
> > $ cat /sys/class/leds/nuc\:\:front1/indicator
> > Power State [HDD Activity] Ethernet WiFi Software Power Limit Disable
>
> So this will use existing "trigger" infrastructure.
Ok, I will take a look on that. Are there any driver that I could use
as an example, using it in a configurable way?
> > That should likely be easier to discuss if any changes at the
> > ABI would be needed. Before moving it out of staging, I would
> > add another one under Documentation/ABI describing the meaning
> > of each sysfs node.
>
> Lets get reasonable ABI before moving it _into_ tree, staging or
> otherwise.
I'm enclosing a document that I started to write today, describing the
way the current ABI was designed. The document doesn't describe in
full the NUC6 variant (which is really limited: just two LEDs
with fixed behavior).
Thanks,
Mauro
==================
Intel NUC WMI LEDs
==================
Some models of the Intel Next Unit of Computing (NUC) may have programmable
LEDs on its panel via its BIOS. A subset of those may also be programmed on
user space.
There are currently three different APIs on such devices, depending on the
NUC generation:
* NUC 6/7:
https://www.intel.com/content/www/us/en/support/articles/000023426/intel-nuc/intel-nuc-kits.html
* NUC 8/9:
https://raw.githubusercontent.com/nomego/intel_nuc_led/master/specs/INTEL_WMI_LED_0.64.pdf
* NUC 10 and newer:
https://www.intel.com/content/dam/support/us/en/documents/intel-nuc/WMI-Spec-Intel-NUC-NUC10ixFNx.pdf
This document describes how to use the LEDs API, as supported by the "nuc-wmi"
driver.
Please notice that the LEDs can only be programmed if the BIOS settings
are allowing the Operational System to control them. Instructions about
how to enable it can be found at the manual of each specific NUC, if
the LEDs are userspace programmed for an specific device.
LED devices
===========
When the driver detects NUC LEDs, some sysfs device nodes are created under
the leds class.
On NUC 6, there are (up to) two LEDs available:
============= ==============================
LED name sysfs device node
============= ==============================
Power ``/sys/class/leds/nuc::power``
Ring ``/sys/class/leds/nuc::ring``
============= ==============================
The NUC 6 API is limited, as it allows only to change the LED color, and
its blink state. Its API will be described on :ref:`NUC6 API`.
On NUC 8 and newer generations, up to seven LEDs are supported:
============= ===============================
LED name sysfs device node
============= ===============================
Skull ``/sys/class/leds/nuc::skull``
Skull eyes ``/sys/class/leds/nuc::eyes``
Power ``/sys/class/leds/nuc::power``
HDD ``/sys/class/leds/nuc::hdd``
Front1 ``/sys/class/leds/nuc::front1``
Front2 ``/sys/class/leds/nuc::front2``
Front3 ``/sys/class/leds/nuc::front3``
============= ===============================
The API for NUC 8 and newer allows full control of the LEDs meaning.
NUC 6 API
=========
TODO: describe the limited NUC6 API
NUC 8 and newer generations API
===============================
On NUC8, and newer, several sysfs nodes will allow to control the
functionality of each LED::
/sys/class/leds/nuc::front1
|-- blink_behavior
|-- blink_frequency
|-- brightness
|-- color
|-- ethernet_type
|-- hdd_default
|-- indicator
|-- max_brightness
|-- power_limit_scheme
|-- ready_mode_blink_behavior
|-- ready_mode_blink_frequency
|-- ready_mode_brightness
|-- s0_blink_behavior
|-- s0_blink_frequency
|-- s0_brightness
|-- s3_blink_behavior
|-- s3_blink_frequency
|-- s3_brightness
|-- s5_blink_behavior
|-- s5_blink_frequency
`-- s5_brightness
The sessions below will explain the meaning of each aspect of the API.
.. note::
For the entire NUC8+ API, the following rules apply:
1. any user can read the LEDs parameter;
2. changing a LED parameter is limited to the owner of the sysfs device
nodes (usually, the ``root`` user);
3. changing a LED parameter is case-insensitive;
4. The LED ``indicator`` parameter controls the function of the LED.
All other parameters can be enabled or disabled in runtime, depending
on it. When a certain parameter is disabled, an error code will be
returned.
LED indicator
-------------
Despite the LED's name, the LED API may allow them to indicate different
hardware events.
This is controlled via the ``indicator`` device node. Reading from it displays
all the supported events for a giving LED, and the currently ative one::
$ cat /sys/class/leds/nuc::front1/indicator
Power State [HDD Activity] Ethernet WiFi Software Power Limit Disable
Each LED may support the following indicator types:
============== =======================================================
Indicator type Meaning
============== =======================================================
Power State Shows if the device is powered and what power level
it is (e. g. if the device is suspended or not, and
on which kind of suspended level).
HDD Activity Indicates if the LED is measuring the hard disk (or
SDD) activity.
Ethernet Indicates the activity Ethernet adapter(s)
WiFi Indicates if WiFi is enabled
Software Doesn't indicate any hardware level. Instead, the LED
status is controlled via software.
Power Limit Changes the LED color when the computer is throttling
its power limits.
Disable The LED was disabled.
============== =======================================================
In order to change the type of indicator, you should
just write a new value to the indicator type::
# echo "wifi" > /sys/class/leds/nuc::front1/indicator
$ cat /sys/class/leds/nuc::front1/indicator
Power State HDD Activity Ethernet [WiFi] Software Power Limit Disable
Power State parameters
----------------------
When the LED indicator is measuring *Power State*, the following parameters
may be available:
================================= =======================================
Parameter Meaning
================================= =======================================
<power_state>_brightness Brightness in percent (from 0 to 100)
<power_state>_blink_behavior type of blink.
See :ref:`nuc_blink_behavior`.
<power_state>_blink_frequency Blink frequency.
See :ref:`nuc_blink_behavior`.
<power_state>_color LED color
See :ref:`nuc_color`.
================================= =======================================
Where <power_state> can be:
On NUC8/9 API:
+------------+
| S0 |
+------------+
| S3 |
+------------+
| S5 |
+------------+
| Ready mode |
+------------+
On NUC10 API:
+------------+
| S0 |
+------------+
| S3 |
+------------+
| Standby |
+------------+
HDD Activity parameters
-----------------------
When the LED indicator is measuring *HDD Activity*, the following parameters
may be available:
================================= =======================================
Parameter Meaning
================================= =======================================
brightness Brightness in percent (from 0 to 100)
color LED color.
See :ref:`nuc_color`.
hdd_default Default is LED turned ON or OFF.
When set toOFF, the LED will turn on
at disk activity.
When set to ON, the LED will be turned
on by default, turning off at disk
activity.
================================= =======================================
Ethernet parameters
-------------------
When the LED indicator is measuring *Ethernet*, the following parameters
may be available:
================================= =======================================
Parameter Meaning
================================= =======================================
brightness Brightness in percent (from 0 to 100)
color LED color.
See :ref:`nuc_color`.
ethernet_type What Ethernet interface is monitored.
Can be:
LAN1, LAN2 or LAN1+LAN2.
================================= =======================================
Power limit parameters
----------------------
When the LED indicator is measuring *Power limit*, the following parameters
may be available:
================================= =======================================
Parameter Meaning
================================= =======================================
brightness Brightness in percent (from 0 to 100)
color LED color.
See :ref:`nuc_color`.
power_limit_scheme Indication scheme can be either:
- green to red
- single color
================================= =======================================
.. nuc_color::
NUC LED colors
==============
The NUC LED API may support 3 types of LEDs:
- Mono-colored LEDs;
- Dual-colored LEDs;
- multi-colored LEDs (only on NUC6/7);
- RGB LEDs.
Also, when a let is set to be a *Power limit* indicator, despite the
physical device's LED color, the API may limit it to be a led that
can display only green and red, or just a single color.
The ``color`` and ``<power_state>_color`` parameter supports all those
different settings.
NUC6/7
------
On NUC6 API, the power LED color can be:
+---------+
| disable |
+---------+
| blue |
+---------+
| amber |
+---------+
And the ring LED color can be:
+---------+
| disable |
+---------+
| cyan |
+---------+
| pink |
+---------+
| yellow |
+---------+
| blue |
+---------+
| red |
+---------+
| green |
+---------+
| white |
+---------+
NUC 8 and newer generations
---------------------------
On NUC10 API, the color can be:
============ ====== ===== =====
Color name Red Green Blue
============ ====== ===== =====
blue 0 0 255
amber 255 191 0
white 255 255 255
red 255 0 0
green 0 255 0
yellow 255 255 0
cyan 0 255 255
magenta 255 0 255
<r>,<g>,<b> <r> <g> <b>
============ ====== ===== =====
The color parameter will refuse to set a LED on a color that it is not
supported by the hardware or when the setting is incompatible with the
indicator type. So, when the indicator is set to *Power limit*, and
the ``power_limit_scheme`` is set to ``green to red``, it doesn't
let to set the LED's color.
On the other hand, the behavior is identical if a color is written using
the color's name or its RGB value.
So::
$ cat /sys/class/leds/nuc::front1/color
red
# echo "green" > /sys/class/leds/nuc::front1/color
$ cat /sys/class/leds/nuc::front1/color
green
# echo "255,0,0" > /sys/class/leds/nuc::front1/color
$ cat /sys/class/leds/nuc::front1/color
red
.. nuc_blink_behavior::
NUC Blink behavior
==================
The NUC LEDs hardware supports the following types of blink behavior:
+------------+
| Solid |
+------------+
| Breathing |
+------------+
| Pulsing |
+------------+
| Strobing |
+------------+
Changing the blink behavior will change how the led will be turning
on and off when blinking. Setting it to ``Solid`` disables blinking.
Please notice that not all types of indicator supports blinking.
When blinking, the blink frequency can be changed via ``blink_frequency``
or ``<power_state>_blink_frequency``, depending on the indicator.
Setting it allows to change the blink frequency in Hz, ranging from 0.1 Hz
to 1.0 Hz.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 17/17] staging: nuc-led: update the TODOs
2021-05-17 8:57 ` Mauro Carvalho Chehab
@ 2021-05-17 9:12 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-17 9:12 UTC (permalink / raw)
To: Pavel Machek
Cc: gregkh, linuxarm, Mauro Carvalho Chehab, devel, linux-kernel,
linux-leds, linux-staging
Em Mon, 17 May 2021 10:57:49 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> Em Mon, 17 May 2021 10:05:27 +0200
> Pavel Machek <pavel@ucw.cz> escreveu:
> > No. Take a look at triggers; for example hdd monitor should look very
> > much like existing disk trigger.
Btw, is there a way to trigger brightness?
When a LED is monitoring the power state, brightness should be
hidden, as, instead of a single brightness parameter, the device
will now have one brightness per different power state, e. g.:
When the LED indicator is measuring *Power State*, the following
parameters may be available:
================================= =======================================
Parameter Meaning
================================= =======================================
<power_state>_brightness Brightness in percent (from 0 to 100)
<power_state>_blink_behavior type of blink.
See :ref:`nuc_blink_behavior`.
<power_state>_blink_frequency Blink frequency.
See :ref:`nuc_blink_behavior`.
<power_state>_color LED color
See :ref:`nuc_color`.
================================= =======================================
Where <power_state> is different, depending on the WMI API version:
On version 0.64 (NUC8/9):
+------------+
| s0 |
+------------+
| s3 |
+------------+
| s5 |
+------------+
| ready_mode |
+------------+
Btw, I've no idea what "ready mode" is, as the specs don't explain it.
This particular mode is disabled on my NUC8 device, so I can't test it.
On version 1.0 (NUC10+):
+------------+
| s0 |
+------------+
| s3 |
+------------+
| standby |
+------------+
Note: At the specs, "Standby" is actually "Modern Standby". I ended
simplifying it, as just "standby_brightness" sounds good enough.
Thanks,
Mauro
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 17/17] staging: nuc-led: update the TODOs
2021-05-17 8:05 ` Pavel Machek
2021-05-17 8:57 ` Mauro Carvalho Chehab
@ 2021-05-17 12:19 ` Mauro Carvalho Chehab
1 sibling, 0 replies; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-17 12:19 UTC (permalink / raw)
To: Pavel Machek
Cc: gregkh, linuxarm, mauro.chehab, Mauro Carvalho Chehab, devel,
linux-kernel, linux-leds, linux-staging
Em Mon, 17 May 2021 10:05:27 +0200
Pavel Machek <pavel@ucw.cz> escreveu:
> No. Take a look at triggers; for example hdd monitor should look very
> much like existing disk trigger.
Hmm... after looking at triggers, I'm not sure if this is the right
interface, nor if we're talking about the same thing.
See, at least the way ledtrig-disk.c uses it, two drivers are triggering
the LED based on software events:
drivers/ata/libata-core.c: ledtrig_disk_activity(!!(qc->tf.flags & ATA_TFLAG_WRITE));
drivers/ide/ide-disk.c: ledtrig_disk_activity(rq_data_dir(rq) == WRITE);
This is not how the NUC LEDs are work. On NUC, the hardware itself
triggers the event and/or blinks the LED(*).
(*) By default, blink is enabled only when the device is suspended
or it is hibernating.
There's no need to do any software emulation.
The API is meant to just program the hardware (and/or the firmware)
for it to do the behavior expected by the user.
So, for instance, setting the indicator event that will trigger the
LED is done by sending a WMI message for this GUID object:
8C5DA44C-CDC3-46b3-8619-4E26D34390B7 (somewhat similar to using
the way ACPI works, but WMI is a different firmware interface).
The method at the WMI API which sets the LED indicator is this one:
0x05 message (Set an Indicator option for the LED type)
Such method receives two parameters. The first one is the LED
number, which can be:
0 - Power button LED
1 - HDD LED
2 - Skull LED
3 - Eyes LED
4 - Front LED 1
5 - Front LED 1
6 - Front LED 1
The second one tells which hardware event will trigger the LED:
===== ============== =======================================================
Value Indicator type Meaning
===== ============== =======================================================
0 Power State Shows if the device is powered and what power level
it is (e. g. if the device is suspended or not, and
on which kind of suspended level).
1 HDD Activity Indicates if the LED is measuring the hard disk (or
SDD) activity.
2 Ethernet Indicates the activity Ethernet adapter(s)
3 WiFi Indicates if WiFi is enabled
4 Software Doesn't indicate any hardware level. Instead, the LED
status is controlled via software.
5 Power Limit Changes the LED color when the computer is throttling
its power limits.
6 Disable The LED was disabled (either by BIOS or via WMI).
===== ============== =======================================================
There is an example at page 7 of the datasheet:
https://raw.githubusercontent.com/nomego/intel_nuc_led/master/specs/INTEL_WMI_LED_0.64.pdf
Where it shows what happens if the WMI message is filled with:
<0x05> <0x00> <0x01>
On such example, it changes the behavior of the button named "Power button"
to change it to monitor the disk activity, instead of monitoring if the
device is powered on or not.
Such setting is even persistent after rebooting, and the event is
hardware/firmware triggered.
So, IMO, it would only makes sense to use something else from the LED
class if are there a way for the sysfs nodes to dynamically be shown/hidden
when the indicator changes.
At least on my eyes, that doesn't seem to be what triggers do.
Thanks,
Mauro
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/17] Add an experimental driver for Intel NUC leds
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
` (16 preceding siblings ...)
2021-05-16 10:53 ` [PATCH 17/17] staging: nuc-led: update the TODOs Mauro Carvalho Chehab
@ 2021-05-17 8:18 ` Greg KH
2021-05-17 9:02 ` Mauro Carvalho Chehab
17 siblings, 1 reply; 31+ messages in thread
From: Greg KH @ 2021-05-17 8:18 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: linuxarm, mauro.chehab, Pavel Machek, linux-leds,
Mauro Carvalho Chehab, devel, linux-kernel, linux-staging
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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 00/17] Add an experimental driver for Intel NUC leds
2021-05-17 8:18 ` [PATCH 00/17] Add an experimental driver for Intel NUC leds Greg KH
@ 2021-05-17 9:02 ` Mauro Carvalho Chehab
2021-05-17 9:08 ` Greg KH
0 siblings, 1 reply; 31+ messages in thread
From: Mauro Carvalho Chehab @ 2021-05-17 9:02 UTC (permalink / raw)
To: Greg KH
Cc: linuxarm, mauro.chehab, Pavel Machek, linux-leds,
Mauro Carvalho Chehab, devel, linux-kernel, linux-staging
Em Mon, 17 May 2021 10:18:57 +0200
Greg KH <gregkh@linuxfoundation.org> 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/17] Add an experimental driver for Intel NUC leds
2021-05-17 9:02 ` Mauro Carvalho Chehab
@ 2021-05-17 9:08 ` Greg KH
0 siblings, 0 replies; 31+ messages in thread
From: Greg KH @ 2021-05-17 9:08 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: linuxarm, mauro.chehab, Pavel Machek, linux-leds,
Mauro Carvalho Chehab, devel, linux-kernel, linux-staging
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 <gregkh@linuxfoundation.org> 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
^ permalink raw reply [flat|nested] 31+ messages in thread