[PATCH v2 0/5] platform/x86: uniwill-laptop: Lightbar fixes

[PATCH v2 0/5] platform/x86: uniwill-laptop: Lightbar fixes

[Armin Wolf]

This patch series mainly deals with various issues inside the code
responsible for controlling the lightbar on supported devices.

The first patch utilizes the improved multicolor LED API to report
the correct brightness of the lightbar to userspace. The next two
patches fix issues discovered inside the code for controlling
animations, while the fourth patch fixes an issue with the naming
of the LEDs in general. The last patch also adds support for another
variant of the Intel NUC x15 laptop kit.

All patches have been tested on said Intel NUC x15 laptop and appear
to work.

Changes since v1:
- fix spelling mistake in commit message
- rebase on for-next

Armin Wolf (5):
  platform/x86: uniwill-laptop: Report correct lightbar brightness
  platform/x86: uniwill-laptop: Implement rainbow animation as trigger
  platform/x86: uniwill-laptop: Fix breathing animation on Intel QC
  platform/x86: uniwill-laptop: Label multicolor LEDs correctly
  platform/x86: uniwill-laptop: Extend support for the Intel NUC x15

 .../ABI/removed/sysfs-driver-uniwill-laptop   |  10 +
 .../ABI/testing/sysfs-driver-uniwill-laptop   |  15 +-
 .../admin-guide/laptops/uniwill-laptop.rst    |   9 +-
 MAINTAINERS                                   |   1 +
 drivers/platform/x86/uniwill/Kconfig          |   1 +
 drivers/platform/x86/uniwill/uniwill-acpi.c   | 233 +++++++++++-------
 6 files changed, 165 insertions(+), 104 deletions(-)
 create mode 100644 Documentation/ABI/removed/sysfs-driver-uniwill-laptop

-- 
2.55.0

[PATCH v2 1/5] platform/x86: uniwill-laptop: Report correct lightbar brightness

[Armin Wolf]

The LED sysfs ABI now allows us to specify the maximum values for the
global brightness and the color intensity values independently. At the
same time the maximum brightness value reported for the lightbar (like
200) does not match the real value supported by the hardware (1),
potentially confusing users.

Report the correct maximum brightness value and only use the intensity
values for controlling the lightbar color.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/uniwill/uniwill-acpi.c | 43 ++++++++++-----------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
index 07b69629bb5e..788bf4064d34 100644
--- a/drivers/platform/x86/uniwill/uniwill-acpi.c
+++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
@@ -398,7 +398,7 @@ struct uniwill_data {
 	struct mutex super_key_lock;	/* Protects the toggling of the super key lock state */
 	struct list_head batteries;
 	struct mutex led_lock;		/* Protects writes to the lightbar registers */
-	u8 lightbar_max_brightness;
+	u8 lightbar_max_intensity;
 	struct led_classdev_mc led_mc_cdev;
 	struct mc_subled led_mc_subled_info[LED_CHANNELS];
 	bool kbd_led_single_color;
@@ -429,7 +429,7 @@ struct uniwill_device_descriptor {
 	unsigned int features;
 	bool kbd_led_single_color;
 	u8 kbd_led_max_brightness;
-	u8 lightbar_max_brightness;
+	u8 lightbar_max_intensity;
 	/* Executed during driver probing */
 	int (*probe)(struct uniwill_data *data);
 };
@@ -1511,20 +1511,16 @@ static int uniwill_led_brightness_set(struct led_classdev *led_cdev, enum led_br
 	unsigned int value;
 	int ret;
 
-	ret = led_mc_calc_color_components(led_mc_cdev, brightness);
-	if (ret < 0)
-		return ret;
-
 	guard(mutex)(&data->led_lock);
 
 	for (int i = 0; i < LED_CHANNELS; i++) {
-		/* Prevent the brightness values from overflowing */
-		value = min(data->lightbar_max_brightness, data->led_mc_subled_info[i].brightness);
-		ret = regmap_write(data->regmap, uniwill_led_channel_to_ac_reg[i], value);
+		ret = regmap_write(data->regmap, uniwill_led_channel_to_ac_reg[i],
+				   data->led_mc_subled_info[i].intensity);
 		if (ret < 0)
 			return ret;
 
-		ret = regmap_write(data->regmap, uniwill_led_channel_to_bat_reg[i], value);
+		ret = regmap_write(data->regmap, uniwill_led_channel_to_bat_reg[i],
+				   data->led_mc_subled_info[i].intensity);
 		if (ret < 0)
 			return ret;
 	}
@@ -1588,14 +1584,14 @@ static int uniwill_led_init(struct uniwill_data *data)
 		return ret;
 
 	data->led_mc_cdev.led_cdev.color = LED_COLOR_ID_MULTI;
-	data->led_mc_cdev.led_cdev.max_brightness = data->lightbar_max_brightness;
+	data->led_mc_cdev.led_cdev.max_brightness = 1;
 	data->led_mc_cdev.led_cdev.flags = LED_REJECT_NAME_CONFLICT;
 	data->led_mc_cdev.led_cdev.brightness_set_blocking = uniwill_led_brightness_set;
 
 	if (value & LIGHTBAR_S0_OFF)
 		data->led_mc_cdev.led_cdev.brightness = 0;
 	else
-		data->led_mc_cdev.led_cdev.brightness = data->lightbar_max_brightness;
+		data->led_mc_cdev.led_cdev.brightness = 1;
 
 	for (int i = 0; i < LED_CHANNELS; i++) {
 		data->led_mc_subled_info[i].color_index = color_indices[i];
@@ -1606,9 +1602,9 @@ static int uniwill_led_init(struct uniwill_data *data)
 
 		/*
 		 * Make sure that the initial intensity value is not greater than
-		 * the maximum brightness.
+		 * the maximum intensity.
 		 */
-		value = min(data->lightbar_max_brightness, value);
+		value = min(data->lightbar_max_intensity, value);
 		ret = regmap_write(data->regmap, uniwill_led_channel_to_ac_reg[i], value);
 		if (ret < 0)
 			return ret;
@@ -1618,6 +1614,7 @@ static int uniwill_led_init(struct uniwill_data *data)
 			return ret;
 
 		data->led_mc_subled_info[i].intensity = value;
+		data->led_mc_subled_info[i].max_intensity = data->lightbar_max_intensity;
 		data->led_mc_subled_info[i].channel = i;
 	}
 
@@ -2366,7 +2363,7 @@ static int uniwill_probe(struct platform_device *pdev)
 	data->features = device_descriptor.features;
 	data->kbd_led_single_color = device_descriptor.kbd_led_single_color;
 	data->kbd_led_max_brightness = device_descriptor.kbd_led_max_brightness;
-	data->lightbar_max_brightness = device_descriptor.lightbar_max_brightness;
+	data->lightbar_max_intensity = device_descriptor.lightbar_max_intensity;
 
 	/*
 	 * Some devices might need to perform some device-specific initialization steps
@@ -2713,7 +2710,7 @@ static struct uniwill_device_descriptor lapqc71a_lapqc71b_descriptor __initdata
 		    UNIWILL_FEATURE_GPU_TEMP |
 		    UNIWILL_FEATURE_PRIMARY_FAN |
 		    UNIWILL_FEATURE_SECONDARY_FAN,
-	.lightbar_max_brightness = 36,
+	.lightbar_max_intensity = 36,
 };
 
 static struct uniwill_device_descriptor lapac71h_descriptor __initdata = {
@@ -2737,7 +2734,7 @@ static struct uniwill_device_descriptor lapkc71f_descriptor __initdata = {
 		    UNIWILL_FEATURE_GPU_TEMP |
 		    UNIWILL_FEATURE_PRIMARY_FAN |
 		    UNIWILL_FEATURE_SECONDARY_FAN,
-	.lightbar_max_brightness = 200,
+	.lightbar_max_intensity = 200,
 };
 
 static struct uniwill_device_descriptor pfxnuxx_pfxluxx_phxprxx_descriptor __initdata = {
@@ -2777,7 +2774,7 @@ static struct uniwill_device_descriptor gmxmpxx_descriptor __initdata = {
 		    UNIWILL_FEATURE_PRIMARY_FAN |
 		    UNIWILL_FEATURE_SECONDARY_FAN |
 		    UNIWILL_FEATURE_USB_C_POWER_PRIORITY,
-	.lightbar_max_brightness = 36,
+	.lightbar_max_intensity = 36,
 };
 
 static struct uniwill_device_descriptor phxtqxx_phxagxx_phxaqxx_phxpgxx_descriptor __initdata = {
@@ -2836,7 +2833,7 @@ static struct uniwill_device_descriptor gmxzgxx_gmxtgxx_ste_descriptor __initdat
 		    UNIWILL_FEATURE_SECONDARY_FAN |
 		    UNIWILL_FEATURE_NVIDIA_CTGP_CONTROL |
 		    UNIWILL_FEATURE_USB_C_POWER_PRIORITY,
-	.lightbar_max_brightness = 36,
+	.lightbar_max_intensity = 36,
 };
 
 static int gmxrgxx_probe(struct uniwill_data *data)
@@ -2850,7 +2847,7 @@ static int gmxrgxx_probe(struct uniwill_data *data)
 
 	if (value & LIGHTBAR) {
 		data->features |= UNIWILL_FEATURE_LIGHTBAR;
-		data->lightbar_max_brightness = 36;
+		data->lightbar_max_intensity = 36;
 	}
 
 	ret = regmap_read(data->regmap, EC_ADDR_SUPPORT_2, &value);
@@ -2888,7 +2885,7 @@ static struct uniwill_device_descriptor gmxagxx_descriptor __initdata = {
 		    UNIWILL_FEATURE_PRIMARY_FAN |
 		    UNIWILL_FEATURE_SECONDARY_FAN |
 		    UNIWILL_FEATURE_NVIDIA_CTGP_CONTROL,
-	.lightbar_max_brightness = 36,
+	.lightbar_max_intensity = 36,
 };
 
 static struct uniwill_device_descriptor gmxxgxx_gmxpxxx_descriptor __initdata = {
@@ -3538,8 +3535,8 @@ static int __init uniwill_init(void)
 		device_descriptor.kbd_led_single_color = false;
 		/* Some models only support 3 brightness levels */
 		device_descriptor.kbd_led_max_brightness = 4;
-		/* Some models only support 36 brightness levels per color component */
-		device_descriptor.lightbar_max_brightness = 200;
+		/* Some models only support 36 intensity levels per color component */
+		device_descriptor.lightbar_max_intensity = 200;
 		pr_warn("Enabling potentially unsupported features\n");
 	}
 
-- 
2.55.0

[PATCH v2 2/5] platform/x86: uniwill-laptop: Implement rainbow animation as trigger

[Armin Wolf]

It turns out that some models require the lightbar to be activated
in order to display the rainbow animation, so users have to access
both the "rainbow_animation" sysfs attribute and the LED sysfs
interface when enabling the rainbow animation. This can cause short
glitches when the lightbar was previously disabled.

Fix this by replacing the sysfs attribute with a custom LED trigger
called "uniwill-rainbow". Selecting this trigger will atomically
activate the lightbar and enable the rainbow animation mode. This
can potentially break userspace applications using the old sysfs
attribute, but so far no such application is known to exist (yet).

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 .../ABI/removed/sysfs-driver-uniwill-laptop   |  10 ++
 .../ABI/testing/sysfs-driver-uniwill-laptop   |  11 --
 .../admin-guide/laptops/uniwill-laptop.rst    |   5 +-
 MAINTAINERS                                   |   1 +
 drivers/platform/x86/uniwill/Kconfig          |   1 +
 drivers/platform/x86/uniwill/uniwill-acpi.c   | 133 +++++++++++-------
 6 files changed, 95 insertions(+), 66 deletions(-)
 create mode 100644 Documentation/ABI/removed/sysfs-driver-uniwill-laptop

diff --git a/Documentation/ABI/removed/sysfs-driver-uniwill-laptop b/Documentation/ABI/removed/sysfs-driver-uniwill-laptop
new file mode 100644
index 000000000000..c226efc23b59
--- /dev/null
+++ b/Documentation/ABI/removed/sysfs-driver-uniwill-laptop
@@ -0,0 +1,10 @@
+What:		/sys/bus/platform/devices/INOU0000:XX/rainbow_animation
+Date:		November 2025
+KernelVersion:	6.19
+Contact:	Armin Wolf <W_Armin@gmx.de>
+Description:
+		Forces the integrated lightbar to display a rainbow animation when the machine
+		is not suspended. Writing "1"/"0" into this file enables/disables this
+		functionality.
+
+		Reading this file returns the current status of the rainbow animation functionality.
diff --git a/Documentation/ABI/testing/sysfs-driver-uniwill-laptop b/Documentation/ABI/testing/sysfs-driver-uniwill-laptop
index 943f92c6b561..b9b9feebe2ed 100644
--- a/Documentation/ABI/testing/sysfs-driver-uniwill-laptop
+++ b/Documentation/ABI/testing/sysfs-driver-uniwill-laptop
@@ -29,17 +29,6 @@ Description:
 		Reading this file returns the current enable status of the touchpad toggle
 		functionality.
 
-What:		/sys/bus/platform/devices/INOU0000:XX/rainbow_animation
-Date:		November 2025
-KernelVersion:	6.19
-Contact:	Armin Wolf <W_Armin@gmx.de>
-Description:
-		Forces the integrated lightbar to display a rainbow animation when the machine
-		is not suspended. Writing "1"/"0" into this file enables/disables this
-		functionality.
-
-		Reading this file returns the current status of the rainbow animation functionality.
-
 What:		/sys/bus/platform/devices/INOU0000:XX/breathing_in_suspend
 Date:		November 2025
 KernelVersion:	6.19
diff --git a/Documentation/admin-guide/laptops/uniwill-laptop.rst b/Documentation/admin-guide/laptops/uniwill-laptop.rst
index be50b45b82ef..aaf50e3074c1 100644
--- a/Documentation/admin-guide/laptops/uniwill-laptop.rst
+++ b/Documentation/admin-guide/laptops/uniwill-laptop.rst
@@ -74,8 +74,9 @@ Lightbar
 The ``uniwill-laptop`` driver exposes the lightbar found on some models as a standard multicolor
 LED class device. The default name of this LED class device is ``uniwill:multicolor:status``.
 
-See Documentation/ABI/testing/sysfs-driver-uniwill-laptop for details on how to control the various
-animation modes of the lightbar.
+The lightbar supports a separate rainbow animation mode that can be enabled by selecting the
+`uniwill-rainbow` LED trigger. To enable a breathing animation during suspend, see
+Documentation/ABI/testing/sysfs-driver-uniwill-laptop for details.
 
 Keyboard Backlight
 ------------------
diff --git a/MAINTAINERS b/MAINTAINERS
index fa0571a5e479..f9dd979d0fa7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -28106,6 +28106,7 @@ UNIWILL LAPTOP DRIVER
 M:	Armin Wolf <W_Armin@gmx.de>
 L:	platform-driver-x86@vger.kernel.org
 S:	Maintained
+F:	Documentation/ABI/removed/sysfs-driver-uniwill-laptop
 F:	Documentation/ABI/testing/sysfs-driver-uniwill-laptop
 F:	Documentation/admin-guide/laptops/uniwill-laptop.rst
 F:	Documentation/wmi/devices/uniwill-laptop.rst
diff --git a/drivers/platform/x86/uniwill/Kconfig b/drivers/platform/x86/uniwill/Kconfig
index d07cc8440188..f80794bdfaef 100644
--- a/drivers/platform/x86/uniwill/Kconfig
+++ b/drivers/platform/x86/uniwill/Kconfig
@@ -28,6 +28,7 @@ config UNIWILL_LAPTOP
 	depends on DMI
 	select REGMAP
 	select INPUT_SPARSEKMAP
+	select LEDS_TRIGGERS
 	help
 	  This driver adds support for various extra features found on Uniwill laptops,
 	  like the lightbar, hwmon sensors and hotkeys. It also supports many OEM laptops
diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
index 788bf4064d34..0fb0d267664a 100644
--- a/drivers/platform/x86/uniwill/uniwill-acpi.c
+++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
@@ -900,51 +900,6 @@ static ssize_t touchpad_toggle_enable_show(struct device *dev, struct device_att
 
 static DEVICE_ATTR_RW(touchpad_toggle_enable);
 
-static ssize_t rainbow_animation_store(struct device *dev, struct device_attribute *attr,
-				       const char *buf, size_t count)
-{
-	struct uniwill_data *data = dev_get_drvdata(dev);
-	unsigned int value;
-	bool enable;
-	int ret;
-
-	ret = kstrtobool(buf, &enable);
-	if (ret < 0)
-		return ret;
-
-	if (enable)
-		value = LIGHTBAR_WELCOME;
-	else
-		value = 0;
-
-	guard(mutex)(&data->led_lock);
-
-	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_AC_CTRL, LIGHTBAR_WELCOME, value);
-	if (ret < 0)
-		return ret;
-
-	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_BAT_CTRL, LIGHTBAR_WELCOME, value);
-	if (ret < 0)
-		return ret;
-
-	return count;
-}
-
-static ssize_t rainbow_animation_show(struct device *dev, struct device_attribute *attr, char *buf)
-{
-	struct uniwill_data *data = dev_get_drvdata(dev);
-	unsigned int value;
-	int ret;
-
-	ret = regmap_read(data->regmap, EC_ADDR_LIGHTBAR_AC_CTRL, &value);
-	if (ret < 0)
-		return ret;
-
-	return sysfs_emit(buf, "%d\n", !!(value & LIGHTBAR_WELCOME));
-}
-
-static DEVICE_ATTR_RW(rainbow_animation);
-
 static ssize_t breathing_in_suspend_store(struct device *dev, struct device_attribute *attr,
 					  const char *buf, size_t count)
 {
@@ -1258,7 +1213,6 @@ static struct attribute *uniwill_attrs[] = {
 	&dev_attr_super_key_enable.attr,
 	&dev_attr_touchpad_toggle_enable.attr,
 	/* Lightbar-related */
-	&dev_attr_rainbow_animation.attr,
 	&dev_attr_breathing_in_suspend.attr,
 	/* Power-management-related */
 	&dev_attr_ctgp_offset.attr,
@@ -1288,8 +1242,7 @@ static umode_t uniwill_attr_is_visible(struct kobject *kobj, struct attribute *a
 			return attr->mode;
 	}
 
-	if (attr == &dev_attr_rainbow_animation.attr ||
-	    attr == &dev_attr_breathing_in_suspend.attr) {
+	if (attr == &dev_attr_breathing_in_suspend.attr) {
 		if (uniwill_device_supports(data, UNIWILL_FEATURE_LIGHTBAR))
 			return attr->mode;
 	}
@@ -1537,6 +1490,59 @@ static int uniwill_led_brightness_set(struct led_classdev *led_cdev, enum led_br
 	return regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_BAT_CTRL, LIGHTBAR_S0_OFF, value);
 }
 
+static int uniwill_led_hw_control_set(struct led_classdev *led_cdev, unsigned long flags)
+{
+	struct led_classdev_mc *led_mc_cdev = lcdev_to_mccdev(led_cdev);
+	struct uniwill_data *data = container_of(led_mc_cdev, struct uniwill_data, led_mc_cdev);
+	unsigned int value;
+	int ret;
+
+	guard(mutex)(&data->led_lock);
+
+	if (flags)
+		value = LIGHTBAR_WELCOME;
+	else
+		value = LIGHTBAR_S0_OFF;
+
+	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_AC_CTRL,
+				 LIGHTBAR_S0_OFF | LIGHTBAR_WELCOME, value);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_BAT_CTRL,
+				 LIGHTBAR_S0_OFF | LIGHTBAR_WELCOME, value);
+	if (ret < 0)
+		return ret;
+
+	/* The LED is always on during hw control */
+	led_cdev->brightness = flags;
+
+	return 0;
+}
+
+static int uniwill_lightbar_trigger_activate(struct led_classdev *led_cdev)
+{
+	return led_cdev->hw_control_set(led_cdev, 1);
+}
+
+static void uniwill_lightbar_trigger_deactivate(struct led_classdev *led_cdev)
+{
+	int ret;
+
+	ret = led_cdev->hw_control_set(led_cdev, 0);
+	if (ret < 0)
+		dev_err(led_cdev->dev, "Failed to stop rainbow animation: %d\n", ret);
+}
+
+static struct led_hw_trigger_type uniwill_lightbar_trigger_type;
+
+static struct led_trigger uniwill_lightbar_trigger = {
+	.name = "uniwill-rainbow",
+	.activate = uniwill_lightbar_trigger_activate,
+	.deactivate = uniwill_lightbar_trigger_deactivate,
+	.trigger_type = &uniwill_lightbar_trigger_type,
+};
+
 #define LIGHTBAR_MASK	(LIGHTBAR_APP_EXISTS | LIGHTBAR_S0_OFF | LIGHTBAR_S3_OFF | LIGHTBAR_WELCOME)
 
 static int uniwill_led_init(struct uniwill_data *data)
@@ -1587,12 +1593,18 @@ static int uniwill_led_init(struct uniwill_data *data)
 	data->led_mc_cdev.led_cdev.max_brightness = 1;
 	data->led_mc_cdev.led_cdev.flags = LED_REJECT_NAME_CONFLICT;
 	data->led_mc_cdev.led_cdev.brightness_set_blocking = uniwill_led_brightness_set;
+	data->led_mc_cdev.led_cdev.trigger_type = &uniwill_lightbar_trigger_type;
+	data->led_mc_cdev.led_cdev.hw_control_trigger = uniwill_lightbar_trigger.name;
+	data->led_mc_cdev.led_cdev.hw_control_set = uniwill_led_hw_control_set;
 
 	if (value & LIGHTBAR_S0_OFF)
 		data->led_mc_cdev.led_cdev.brightness = 0;
 	else
 		data->led_mc_cdev.led_cdev.brightness = 1;
 
+	if (value & LIGHTBAR_WELCOME)
+		data->led_mc_cdev.led_cdev.default_trigger = uniwill_lightbar_trigger.name;
+
 	for (int i = 0; i < LED_CHANNELS; i++) {
 		data->led_mc_subled_info[i].color_index = color_indices[i];
 
@@ -3540,17 +3552,31 @@ static int __init uniwill_init(void)
 		pr_warn("Enabling potentially unsupported features\n");
 	}
 
-	ret = platform_driver_register(&uniwill_driver);
+	/*
+	 * We cannot register the trigger inside the .probe callback of the
+	 * platform driver, because each trigger needs a unique name.
+	 */
+	ret = led_trigger_register(&uniwill_lightbar_trigger);
 	if (ret < 0)
 		return ret;
 
+	ret = platform_driver_register(&uniwill_driver);
+	if (ret < 0)
+		goto err_platform;
+
 	ret = uniwill_wmi_register_driver();
-	if (ret < 0) {
-		platform_driver_unregister(&uniwill_driver);
-		return ret;
-	}
+	if (ret < 0)
+		goto err_wmi;
 
 	return 0;
+
+err_wmi:
+	platform_driver_unregister(&uniwill_driver);
+
+err_platform:
+	led_trigger_unregister(&uniwill_lightbar_trigger);
+
+	return ret;
 }
 module_init(uniwill_init);
 
@@ -3558,6 +3584,7 @@ static void __exit uniwill_exit(void)
 {
 	uniwill_wmi_unregister_driver();
 	platform_driver_unregister(&uniwill_driver);
+	led_trigger_unregister(&uniwill_lightbar_trigger);
 }
 module_exit(uniwill_exit);
 
-- 
2.55.0

[PATCH v2 3/5] platform/x86: uniwill-laptop: Fix breathing animation on Intel QC

[Armin Wolf]

Intel NUC 9 Extreme (Queens County) devices have a separate bit
inside the EC_ADDR_LIGHTBAR_AC_CTRL register for enabling the
breathing animation in s2idle mode.

Add support for this additional setting.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 .../ABI/testing/sysfs-driver-uniwill-laptop   |  4 +--
 drivers/platform/x86/uniwill/uniwill-acpi.c   | 27 ++++++++++++++-----
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-uniwill-laptop b/Documentation/ABI/testing/sysfs-driver-uniwill-laptop
index b9b9feebe2ed..f3060aaf8a76 100644
--- a/Documentation/ABI/testing/sysfs-driver-uniwill-laptop
+++ b/Documentation/ABI/testing/sysfs-driver-uniwill-laptop
@@ -35,8 +35,8 @@ KernelVersion:	6.19
 Contact:	Armin Wolf <W_Armin@gmx.de>
 Description:
 		Causes the integrated lightbar to display a breathing animation when the machine
-		has been suspended and is running on AC power. Writing "1"/"0" into this file
-		enables/disables this functionality.
+		has been suspended, is running on AC power and the lightbar brightness not zero.
+		Writing "1"/"0" into this file enables/disables this functionality.
 
 		Reading this file returns the current status of the breathing animation
 		functionality.
diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
index 0fb0d267664a..29d3578213c6 100644
--- a/drivers/platform/x86/uniwill/uniwill-acpi.c
+++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
@@ -165,9 +165,10 @@
 #define EC_ADDR_LIGHTBAR_AC_CTRL	0x0748
 #define LIGHTBAR_APP_EXISTS		BIT(0)
 #define LIGHTBAR_POWER_SAVE		BIT(1)
-#define LIGHTBAR_S0_OFF			BIT(2)
-#define LIGHTBAR_S3_OFF			BIT(3)	// Breathing animation when suspended
-#define LIGHTBAR_WELCOME		BIT(7)	// Rainbow animation
+#define LIGHTBAR_S0_OFF			BIT(2)	/* Also disables animations */
+#define LIGHTBAR_S3_OFF			BIT(3)	/* No breathing animation when in s2ram */
+#define LIGHTBAR_MODERN_STANDBY_ON	BIT(6)	/* Breathing animation when in s2idle */
+#define LIGHTBAR_WELCOME		BIT(7)	/* Rainbow animation */
 
 #define EC_ADDR_LIGHTBAR_AC_RED		0x0749
 
@@ -306,7 +307,7 @@
 #define EC_ADDR_USB_C_POWER_PRIORITY	0x07CC
 #define USB_C_POWER_PRIORITY		BIT(7)
 
-/* Same bits as EC_ADDR_LIGHTBAR_AC_CTRL except LIGHTBAR_S3_OFF */
+/* Same bits as EC_ADDR_LIGHTBAR_AC_CTRL except LIGHTBAR_S3_OFF and LIGHTBAR_MODERN_STANDBY_ON */
 #define EC_ADDR_LIGHTBAR_BAT_CTRL	0x07E2
 
 #define EC_ADDR_LIGHTBAR_BAT_RED	0x07E3
@@ -913,12 +914,13 @@ static ssize_t breathing_in_suspend_store(struct device *dev, struct device_attr
 		return ret;
 
 	if (enable)
-		value = 0;
+		value = LIGHTBAR_MODERN_STANDBY_ON;
 	else
 		value = LIGHTBAR_S3_OFF;
 
 	/* We only access a single register here, so we do not need to use data->led_lock */
-	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_AC_CTRL, LIGHTBAR_S3_OFF, value);
+	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_AC_CTRL,
+				 LIGHTBAR_S3_OFF | LIGHTBAR_MODERN_STANDBY_ON, value);
 	if (ret < 0)
 		return ret;
 
@@ -936,6 +938,10 @@ static ssize_t breathing_in_suspend_show(struct device *dev, struct device_attri
 	if (ret < 0)
 		return ret;
 
+	/*
+	 * We only test LIGHTBAR_S3_OFF here, because LIGHTBAR_MODERN_STANDBY_ON
+	 * should have the exact opposite value.
+	 */
 	return sysfs_emit(buf, "%d\n", !(value & LIGHTBAR_S3_OFF));
 }
 
@@ -1543,7 +1549,8 @@ static struct led_trigger uniwill_lightbar_trigger = {
 	.trigger_type = &uniwill_lightbar_trigger_type,
 };
 
-#define LIGHTBAR_MASK	(LIGHTBAR_APP_EXISTS | LIGHTBAR_S0_OFF | LIGHTBAR_S3_OFF | LIGHTBAR_WELCOME)
+#define LIGHTBAR_MASK	(LIGHTBAR_APP_EXISTS | LIGHTBAR_S0_OFF | LIGHTBAR_S3_OFF | \
+			 LIGHTBAR_MODERN_STANDBY_ON | LIGHTBAR_WELCOME)
 
 static int uniwill_led_init(struct uniwill_data *data)
 {
@@ -1576,6 +1583,11 @@ static int uniwill_led_init(struct uniwill_data *data)
 		return ret;
 
 	value |= LIGHTBAR_APP_EXISTS;
+	if (value & LIGHTBAR_S3_OFF)
+		value &= ~LIGHTBAR_MODERN_STANDBY_ON;
+	else
+		value |= LIGHTBAR_MODERN_STANDBY_ON;
+
 	ret = regmap_write(data->regmap, EC_ADDR_LIGHTBAR_AC_CTRL, value);
 	if (ret < 0)
 		return ret;
@@ -1585,6 +1597,7 @@ static int uniwill_led_init(struct uniwill_data *data)
 	 * running on battery power.
 	 */
 	value |= LIGHTBAR_S3_OFF;
+	value &= ~LIGHTBAR_MODERN_STANDBY_ON;
 	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_BAT_CTRL, LIGHTBAR_MASK, value);
 	if (ret < 0)
 		return ret;
-- 
2.55.0

[PATCH v2 4/5] platform/x86: uniwill-laptop: Label multicolor LEDs correctly

[Armin Wolf]

The LED subsystem specifies that LEDs capable of producing arbitrary
colors (like RGB) should label themselfs as "rgb" instead of
"multicolor".

Use the "rgb" label instead of the "multicolor" label to fix
detection by userspace applications.

Reported-by: Werner Sembach <wse@tuxedocomputers.com>
Closes: https://github.com/Wer-Wolf/uniwill-laptop/issues/18
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 Documentation/admin-guide/laptops/uniwill-laptop.rst | 4 ++--
 drivers/platform/x86/uniwill/uniwill-acpi.c          | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/laptops/uniwill-laptop.rst b/Documentation/admin-guide/laptops/uniwill-laptop.rst
index aaf50e3074c1..d8ce30f10d7c 100644
--- a/Documentation/admin-guide/laptops/uniwill-laptop.rst
+++ b/Documentation/admin-guide/laptops/uniwill-laptop.rst
@@ -72,7 +72,7 @@ Lightbar
 --------
 
 The ``uniwill-laptop`` driver exposes the lightbar found on some models as a standard multicolor
-LED class device. The default name of this LED class device is ``uniwill:multicolor:status``.
+LED class device. The default name of this LED class device is ``uniwill:rgb:status``.
 
 The lightbar supports a separate rainbow animation mode that can be enabled by selecting the
 `uniwill-rainbow` LED trigger. To enable a breathing animation during suspend, see
@@ -83,7 +83,7 @@ Keyboard Backlight
 
 The ``uniwill-laptop`` driver supports controlling the keyboard backlight using the standard
 LED class interface. The default name of this LED class device is ``uniwill:white:kbd_backlight``
-when the keyboard backlight supports only a single color, or ``uniwill:multicolor:kbd_backlight``
+when the keyboard backlight supports only a single color, or ``uniwill:rgb:kbd_backlight``
 when the keyboard backlight supports RGB colors. The maximum intensity for each color channel
 in RGB mode is 50.
 
diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
index 29d3578213c6..ff07bc33c6cc 100644
--- a/drivers/platform/x86/uniwill/uniwill-acpi.c
+++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
@@ -1556,7 +1556,7 @@ static int uniwill_led_init(struct uniwill_data *data)
 {
 	struct led_init_data init_data = {
 		.devicename = DRIVER_NAME,
-		.default_label = "multicolor:" LED_FUNCTION_STATUS,
+		.default_label = "rgb:" LED_FUNCTION_STATUS,
 		.devname_mandatory = true,
 	};
 	unsigned int color_indices[3] = {
@@ -1602,7 +1602,7 @@ static int uniwill_led_init(struct uniwill_data *data)
 	if (ret < 0)
 		return ret;
 
-	data->led_mc_cdev.led_cdev.color = LED_COLOR_ID_MULTI;
+	data->led_mc_cdev.led_cdev.color = LED_COLOR_ID_RGB;
 	data->led_mc_cdev.led_cdev.max_brightness = 1;
 	data->led_mc_cdev.led_cdev.flags = LED_REJECT_NAME_CONFLICT;
 	data->led_mc_cdev.led_cdev.brightness_set_blocking = uniwill_led_brightness_set;
@@ -1810,7 +1810,7 @@ static int uniwill_rgb_kbd_led_init(struct uniwill_data *data)
 		LED_COLOR_ID_BLUE,
 	};
 	struct led_init_data init_data = {
-		.default_label = "multicolor:" LED_FUNCTION_KBD_BACKLIGHT,
+		.default_label = "rgb:" LED_FUNCTION_KBD_BACKLIGHT,
 		.devicename = DRIVER_NAME,
 		.devname_mandatory = true,
 	};
@@ -1871,7 +1871,7 @@ static int uniwill_rgb_kbd_led_init(struct uniwill_data *data)
 		return ret;
 
 	data->kbd_led_mc_cdev.led_cdev.max_brightness = data->kbd_led_max_brightness;
-	data->kbd_led_mc_cdev.led_cdev.color = LED_COLOR_ID_MULTI;
+	data->kbd_led_mc_cdev.led_cdev.color = LED_COLOR_ID_RGB;
 	data->kbd_led_mc_cdev.led_cdev.flags = LED_BRIGHT_HW_CHANGED | LED_REJECT_NAME_CONFLICT;
 	data->kbd_led_mc_cdev.led_cdev.brightness_set_blocking = uniwill_kbd_led_mc_brightness_set;
 	data->kbd_led_mc_cdev.led_cdev.brightness_get = uniwill_kbd_led_mc_brightness_get;
-- 
2.55.0

[PATCH v2 5/5] platform/x86: uniwill-laptop: Extend support for the Intel NUC x15

[Armin Wolf]

A user has reported that the LAPKC71E variant of the Intel NUC x15
also works with the uniwill-laptop driver. The difference between the
LAPKC71E and the LAPKC71F seems to be the dedicated NVidia GPU module.

Add support for this device as well. It is suspected that the LAPKC71F
also supports the new UNIWILL_FEATURE_NVIDIA_CTGP_CONTROL feature, but
this would need to be verified on real hardware.

Link: https://github.com/Wer-Wolf/uniwill-laptop/issues/17
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/uniwill/uniwill-acpi.c | 22 +++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
index ff07bc33c6cc..61e71043e802 100644
--- a/drivers/platform/x86/uniwill/uniwill-acpi.c
+++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
@@ -2749,6 +2749,20 @@ static struct uniwill_device_descriptor lapac71h_descriptor __initdata = {
 		    UNIWILL_FEATURE_SECONDARY_FAN,
 };
 
+static struct uniwill_device_descriptor lapkc71e_descriptor __initdata = {
+	.features = UNIWILL_FEATURE_FN_LOCK |
+		    UNIWILL_FEATURE_SUPER_KEY |
+		    UNIWILL_FEATURE_TOUCHPAD_TOGGLE |
+		    UNIWILL_FEATURE_LIGHTBAR |
+		    UNIWILL_FEATURE_BATTERY_CHARGE_LIMIT |
+		    UNIWILL_FEATURE_CPU_TEMP |
+		    UNIWILL_FEATURE_GPU_TEMP |
+		    UNIWILL_FEATURE_PRIMARY_FAN |
+		    UNIWILL_FEATURE_SECONDARY_FAN |
+		    UNIWILL_FEATURE_NVIDIA_CTGP_CONTROL,
+	.lightbar_max_intensity = 200,
+};
+
 static struct uniwill_device_descriptor lapkc71f_descriptor __initdata = {
 	.features = UNIWILL_FEATURE_FN_LOCK |
 		    UNIWILL_FEATURE_SUPER_KEY |
@@ -3113,6 +3127,14 @@ static const struct dmi_system_id uniwill_dmi_table[] __initconst = {
 		},
 		.driver_data = &lapac71h_descriptor,
 	},
+	{
+		.ident = "Intel NUC x15",
+		.matches = {
+			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Intel(R) Client Systems"),
+			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "LAPKC71E"),
+		},
+		.driver_data = &lapkc71e_descriptor,
+	},
 	{
 		.ident = "Intel NUC x15",
 		.matches = {
-- 
2.55.0

Re: [PATCH v2 0/5] platform/x86: uniwill-laptop: Lightbar fixes

[Ilpo Järvinen]

On Wed, 07 Oct 2026 16:38:53 +0200, Armin Wolf wrote:

> This patch series mainly deals with various issues inside the code
> responsible for controlling the lightbar on supported devices.
> 
> The first patch utilizes the improved multicolor LED API to report
> the correct brightness of the lightbar to userspace. The next two
> patches fix issues discovered inside the code for controlling
> animations, while the fourth patch fixes an issue with the naming
> of the LEDs in general. The last patch also adds support for another
> variant of the Intel NUC x15 laptop kit.
> 
> [...]

Thank you for your contribution, it has been applied to my local
review-ilpo-next branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-next branch only once I've pushed my
local branch there, which might take a while.

FYI [if applicable to your patch], as per Linus' policy change, also
fixes are mostly routed through for-next unless the fix is for a
commit introduced in the most recent cycle or is clearly a regression
fix.

The list of commits applied:
[1/5] platform/x86: uniwill-laptop: Report correct lightbar brightness
      commit: de41d8b4d0dbb81c81cd99c245238d878a3e1951
[2/5] platform/x86: uniwill-laptop: Implement rainbow animation as trigger
      commit: 59e4e406b6c08ad41f3ad86db6d42b82d8539c12
[3/5] platform/x86: uniwill-laptop: Fix breathing animation on Intel QC
      commit: 969a3bfa474e35828e142d0160f393a97d9bf0ee
[4/5] platform/x86: uniwill-laptop: Label multicolor LEDs correctly
      commit: 42e6b269fc1134a3fcce57539034ee496e2c1344
[5/5] platform/x86: uniwill-laptop: Extend support for the Intel NUC x15
      commit: 7a159a14ce0f5d5a3c2f835f39986ec5539ae374

--
 i.

Re: [PATCH v2 0/5] platform/x86: uniwill-laptop: Lightbar fixes

[Armin Wolf]

Am 08.10.26 um 19:38 schrieb Ilpo Järvinen:

> On Wed, 07 Oct 2026 16:38:53 +0200, Armin Wolf wrote:
>
>> This patch series mainly deals with various issues inside the code
>> responsible for controlling the lightbar on supported devices.
>>
>> The first patch utilizes the improved multicolor LED API to report
>> the correct brightness of the lightbar to userspace. The next two
>> patches fix issues discovered inside the code for controlling
>> animations, while the fourth patch fixes an issue with the naming
>> of the LEDs in general. The last patch also adds support for another
>> variant of the Intel NUC x15 laptop kit.
>>
>> [...]
> Thank you for your contribution, it has been applied to my local
> review-ilpo-next branch. Note it will show up in the public
> platform-drivers-x86/review-ilpo-next branch only once I've pushed my
> local branch there, which might take a while.

Thank you :)

I noticed that the LED next branch contains a commit that requires triggers
to implement a new callback and prints a warning if this is not the case.

Should i wait till the next cycle to send a patch implementing said callback
or do you want to rebase for-next onto LED next?

Thanks,
Armin Wolf

> FYI [if applicable to your patch], as per Linus' policy change, also
> fixes are mostly routed through for-next unless the fix is for a
> commit introduced in the most recent cycle or is clearly a regression
> fix.
>
> The list of commits applied:
> [1/5] platform/x86: uniwill-laptop: Report correct lightbar brightness
>        commit: de41d8b4d0dbb81c81cd99c245238d878a3e1951
> [2/5] platform/x86: uniwill-laptop: Implement rainbow animation as trigger
>        commit: 59e4e406b6c08ad41f3ad86db6d42b82d8539c12
> [3/5] platform/x86: uniwill-laptop: Fix breathing animation on Intel QC
>        commit: 969a3bfa474e35828e142d0160f393a97d9bf0ee
> [4/5] platform/x86: uniwill-laptop: Label multicolor LEDs correctly
>        commit: 42e6b269fc1134a3fcce57539034ee496e2c1344
> [5/5] platform/x86: uniwill-laptop: Extend support for the Intel NUC x15
>        commit: 7a159a14ce0f5d5a3c2f835f39986ec5539ae374
>
> --
>   i.
>
>

Re: [PATCH v2 0/5] platform/x86: uniwill-laptop: Lightbar fixes

[Rong Zhang]

Hi Armin, Ilpo,

On Sat, 2026-10-10 at 03:22 +0200, Armin Wolf wrote:
> Am 08.10.26 um 19:38 schrieb Ilpo Järvinen:
> 
> > On Wed, 07 Oct 2026 16:38:53 +0200, Armin Wolf wrote:
> > 
> > > This patch series mainly deals with various issues inside the code
> > > responsible for controlling the lightbar on supported devices.
> > > 
> > > The first patch utilizes the improved multicolor LED API to report
> > > the correct brightness of the lightbar to userspace. The next two
> > > patches fix issues discovered inside the code for controlling
> > > animations, while the fourth patch fixes an issue with the naming
> > > of the LEDs in general. The last patch also adds support for another
> > > variant of the Intel NUC x15 laptop kit.
> > > 
> > > [...]
> > Thank you for your contribution, it has been applied to my local
> > review-ilpo-next branch. Note it will show up in the public
> > platform-drivers-x86/review-ilpo-next branch only once I've pushed my
> > local branch there, which might take a while.
> 
> Thank you :)
> 
> I noticed that the LED next branch contains a commit that requires triggers
> to implement a new callback and prints a warning if this is not the case.

Whoops, I didn't realize there was another private trigger being added
during the same cycle.

I have some additional review comments on the implementation of the
private trigger. See my reply to the relevant patch.

> 
> Should i wait till the next cycle to send a patch implementing said callback
> or do you want to rebase for-next onto LED next?

I would like to ask a similar question, too. The leds part of my series
has been applied[1], leaving the pdx86 part waiting for further review or
apply.

Ilpo, may I ask about your preference here? Would you prefer to merge LED
next and apply the pdx86 part[2] of my series this cycle, or prefer to
wait until the next cycle? If the latter is preferred, should I resubmit
the series then?

[1]: https://lore.kernel.org/all/179025272986.499136.14712876777665893306.b4-ty@b4/
[2]: https://lore.kernel.org/all/20260921-leds-trigger-hw-changed-v7-11-fe3cdb6dec51@rong.moe/

Thanks,
Rong

> 
> Thanks,
> Armin Wolf
> 
> > FYI [if applicable to your patch], as per Linus' policy change, also
> > fixes are mostly routed through for-next unless the fix is for a
> > commit introduced in the most recent cycle or is clearly a regression
> > fix.
> > 
> > The list of commits applied:
> > [1/5] platform/x86: uniwill-laptop: Report correct lightbar brightness
> >        commit: de41d8b4d0dbb81c81cd99c245238d878a3e1951
> > [2/5] platform/x86: uniwill-laptop: Implement rainbow animation as trigger
> >        commit: 59e4e406b6c08ad41f3ad86db6d42b82d8539c12
> > [3/5] platform/x86: uniwill-laptop: Fix breathing animation on Intel QC
> >        commit: 969a3bfa474e35828e142d0160f393a97d9bf0ee
> > [4/5] platform/x86: uniwill-laptop: Label multicolor LEDs correctly
> >        commit: 42e6b269fc1134a3fcce57539034ee496e2c1344
> > [5/5] platform/x86: uniwill-laptop: Extend support for the Intel NUC x15
> >        commit: 7a159a14ce0f5d5a3c2f835f39986ec5539ae374
> > 
> > --
> >   i.
> > 
> > 

Re: [PATCH v2 2/5] platform/x86: uniwill-laptop: Implement rainbow animation as trigger

[Rong Zhang]

Hi Armin,

On Wed, 2026-10-07 at 16:38 +0200, Armin Wolf wrote:
> It turns out that some models require the lightbar to be activated
> in order to display the rainbow animation, so users have to access
> both the "rainbow_animation" sysfs attribute and the LED sysfs
> interface when enabling the rainbow animation. This can cause short
> glitches when the lightbar was previously disabled.
> 
> Fix this by replacing the sysfs attribute with a custom LED trigger
> called "uniwill-rainbow". Selecting this trigger will atomically
> activate the lightbar and enable the rainbow animation mode. This
> can potentially break userspace applications using the old sysfs
> attribute, but so far no such application is known to exist (yet).
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  .../ABI/removed/sysfs-driver-uniwill-laptop   |  10 ++
>  .../ABI/testing/sysfs-driver-uniwill-laptop   |  11 --
>  .../admin-guide/laptops/uniwill-laptop.rst    |   5 +-
>  MAINTAINERS                                   |   1 +
>  drivers/platform/x86/uniwill/Kconfig          |   1 +
>  drivers/platform/x86/uniwill/uniwill-acpi.c   | 133 +++++++++++-------
>  6 files changed, 95 insertions(+), 66 deletions(-)
>  create mode 100644 Documentation/ABI/removed/sysfs-driver-uniwill-laptop
> 
> diff --git a/Documentation/ABI/removed/sysfs-driver-uniwill-laptop b/Documentation/ABI/removed/sysfs-driver-uniwill-laptop
> new file mode 100644
> index 000000000000..c226efc23b59
> --- /dev/null
> +++ b/Documentation/ABI/removed/sysfs-driver-uniwill-laptop
> @@ -0,0 +1,10 @@
> +What:		/sys/bus/platform/devices/INOU0000:XX/rainbow_animation
> +Date:		November 2025
> +KernelVersion:	6.19
> +Contact:	Armin Wolf <W_Armin@gmx.de>
> +Description:
> +		Forces the integrated lightbar to display a rainbow animation when the machine
> +		is not suspended. Writing "1"/"0" into this file enables/disables this
> +		functionality.
> +
> +		Reading this file returns the current status of the rainbow animation functionality.

It'd be more helpful by documenting the removal date, version and what
has replaced it.

> diff --git a/Documentation/ABI/testing/sysfs-driver-uniwill-laptop b/Documentation/ABI/testing/sysfs-driver-uniwill-laptop
> index 943f92c6b561..b9b9feebe2ed 100644
> --- a/Documentation/ABI/testing/sysfs-driver-uniwill-laptop
> +++ b/Documentation/ABI/testing/sysfs-driver-uniwill-laptop
> @@ -29,17 +29,6 @@ Description:
>  		Reading this file returns the current enable status of the touchpad toggle
>  		functionality.
>  
> -What:		/sys/bus/platform/devices/INOU0000:XX/rainbow_animation
> -Date:		November 2025
> -KernelVersion:	6.19
> -Contact:	Armin Wolf <W_Armin@gmx.de>
> -Description:
> -		Forces the integrated lightbar to display a rainbow animation when the machine
> -		is not suspended. Writing "1"/"0" into this file enables/disables this
> -		functionality.
> -
> -		Reading this file returns the current status of the rainbow animation functionality.
> -
>  What:		/sys/bus/platform/devices/INOU0000:XX/breathing_in_suspend
>  Date:		November 2025
>  KernelVersion:	6.19
> diff --git a/Documentation/admin-guide/laptops/uniwill-laptop.rst b/Documentation/admin-guide/laptops/uniwill-laptop.rst
> index be50b45b82ef..aaf50e3074c1 100644
> --- a/Documentation/admin-guide/laptops/uniwill-laptop.rst
> +++ b/Documentation/admin-guide/laptops/uniwill-laptop.rst
> @@ -74,8 +74,9 @@ Lightbar
>  The ``uniwill-laptop`` driver exposes the lightbar found on some models as a standard multicolor
>  LED class device. The default name of this LED class device is ``uniwill:multicolor:status``.
>  
> -See Documentation/ABI/testing/sysfs-driver-uniwill-laptop for details on how to control the various
> -animation modes of the lightbar.
> +The lightbar supports a separate rainbow animation mode that can be enabled by selecting the
> +`uniwill-rainbow` LED trigger. To enable a breathing animation during suspend, see
> +Documentation/ABI/testing/sysfs-driver-uniwill-laptop for details.
>  
>  Keyboard Backlight
>  ------------------
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fa0571a5e479..f9dd979d0fa7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -28106,6 +28106,7 @@ UNIWILL LAPTOP DRIVER
>  M:	Armin Wolf <W_Armin@gmx.de>
>  L:	platform-driver-x86@vger.kernel.org
>  S:	Maintained
> +F:	Documentation/ABI/removed/sysfs-driver-uniwill-laptop
>  F:	Documentation/ABI/testing/sysfs-driver-uniwill-laptop
>  F:	Documentation/admin-guide/laptops/uniwill-laptop.rst
>  F:	Documentation/wmi/devices/uniwill-laptop.rst
> diff --git a/drivers/platform/x86/uniwill/Kconfig b/drivers/platform/x86/uniwill/Kconfig
> index d07cc8440188..f80794bdfaef 100644
> --- a/drivers/platform/x86/uniwill/Kconfig
> +++ b/drivers/platform/x86/uniwill/Kconfig
> @@ -28,6 +28,7 @@ config UNIWILL_LAPTOP
>  	depends on DMI
>  	select REGMAP
>  	select INPUT_SPARSEKMAP
> +	select LEDS_TRIGGERS
>  	help
>  	  This driver adds support for various extra features found on Uniwill laptops,
>  	  like the lightbar, hwmon sensors and hotkeys. It also supports many OEM laptops
> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
> index 788bf4064d34..0fb0d267664a 100644
> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
> @@ -900,51 +900,6 @@ static ssize_t touchpad_toggle_enable_show(struct device *dev, struct device_att
>  
>  static DEVICE_ATTR_RW(touchpad_toggle_enable);
>  
> -static ssize_t rainbow_animation_store(struct device *dev, struct device_attribute *attr,
> -				       const char *buf, size_t count)
> -{
> -	struct uniwill_data *data = dev_get_drvdata(dev);
> -	unsigned int value;
> -	bool enable;
> -	int ret;
> -
> -	ret = kstrtobool(buf, &enable);
> -	if (ret < 0)
> -		return ret;
> -
> -	if (enable)
> -		value = LIGHTBAR_WELCOME;
> -	else
> -		value = 0;
> -
> -	guard(mutex)(&data->led_lock);
> -
> -	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_AC_CTRL, LIGHTBAR_WELCOME, value);
> -	if (ret < 0)
> -		return ret;
> -
> -	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_BAT_CTRL, LIGHTBAR_WELCOME, value);
> -	if (ret < 0)
> -		return ret;
> -
> -	return count;
> -}
> -
> -static ssize_t rainbow_animation_show(struct device *dev, struct device_attribute *attr, char *buf)
> -{
> -	struct uniwill_data *data = dev_get_drvdata(dev);
> -	unsigned int value;
> -	int ret;
> -
> -	ret = regmap_read(data->regmap, EC_ADDR_LIGHTBAR_AC_CTRL, &value);
> -	if (ret < 0)
> -		return ret;
> -
> -	return sysfs_emit(buf, "%d\n", !!(value & LIGHTBAR_WELCOME));
> -}
> -
> -static DEVICE_ATTR_RW(rainbow_animation);
> -
>  static ssize_t breathing_in_suspend_store(struct device *dev, struct device_attribute *attr,
>  					  const char *buf, size_t count)
>  {
> @@ -1258,7 +1213,6 @@ static struct attribute *uniwill_attrs[] = {
>  	&dev_attr_super_key_enable.attr,
>  	&dev_attr_touchpad_toggle_enable.attr,
>  	/* Lightbar-related */
> -	&dev_attr_rainbow_animation.attr,
>  	&dev_attr_breathing_in_suspend.attr,
>  	/* Power-management-related */
>  	&dev_attr_ctgp_offset.attr,
> @@ -1288,8 +1242,7 @@ static umode_t uniwill_attr_is_visible(struct kobject *kobj, struct attribute *a
>  			return attr->mode;
>  	}
>  
> -	if (attr == &dev_attr_rainbow_animation.attr ||
> -	    attr == &dev_attr_breathing_in_suspend.attr) {
> +	if (attr == &dev_attr_breathing_in_suspend.attr) {
>  		if (uniwill_device_supports(data, UNIWILL_FEATURE_LIGHTBAR))
>  			return attr->mode;

It'd be better to convert breathing_in_suspend into an LED device
attribute. See led_classdev->groups.

>  	}
> @@ -1537,6 +1490,59 @@ static int uniwill_led_brightness_set(struct led_classdev *led_cdev, enum led_br
>  	return regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_BAT_CTRL, LIGHTBAR_S0_OFF, value);
>  }
>  
> +static int uniwill_led_hw_control_set(struct led_classdev *led_cdev, unsigned long flags)
> +{
> +	struct led_classdev_mc *led_mc_cdev = lcdev_to_mccdev(led_cdev);
> +	struct uniwill_data *data = container_of(led_mc_cdev, struct uniwill_data, led_mc_cdev);
> +	unsigned int value;
> +	int ret;
> +
> +	guard(mutex)(&data->led_lock);
> +
> +	if (flags)
> +		value = LIGHTBAR_WELCOME;
> +	else
> +		value = LIGHTBAR_S0_OFF;
> +
> +	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_AC_CTRL,
> +				 LIGHTBAR_S0_OFF | LIGHTBAR_WELCOME, value);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_BAT_CTRL,
> +				 LIGHTBAR_S0_OFF | LIGHTBAR_WELCOME, value);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* The LED is always on during hw control */
> +	led_cdev->brightness = flags;
> +
> +	return 0;
> +}
> +
> +static int uniwill_lightbar_trigger_activate(struct led_classdev *led_cdev)
> +{
> +	return led_cdev->hw_control_set(led_cdev, 1);
> +}
> +
> +static void uniwill_lightbar_trigger_deactivate(struct led_classdev *led_cdev)
> +{
> +	int ret;
> +
> +	ret = led_cdev->hw_control_set(led_cdev, 0);
> +	if (ret < 0)
> +		dev_err(led_cdev->dev, "Failed to stop rainbow animation: %d\n", ret);
> +}
> +
> +static struct led_hw_trigger_type uniwill_lightbar_trigger_type;
> +
> +static struct led_trigger uniwill_lightbar_trigger = {
> +	.name = "uniwill-rainbow",
> +	.activate = uniwill_lightbar_trigger_activate,
> +	.deactivate = uniwill_lightbar_trigger_deactivate,
> +	.trigger_type = &uniwill_lightbar_trigger_type,
> +};
> +
>  #define LIGHTBAR_MASK	(LIGHTBAR_APP_EXISTS | LIGHTBAR_S0_OFF | LIGHTBAR_S3_OFF | LIGHTBAR_WELCOME)
>  
>  static int uniwill_led_init(struct uniwill_data *data)
> @@ -1587,12 +1593,18 @@ static int uniwill_led_init(struct uniwill_data *data)
>  	data->led_mc_cdev.led_cdev.max_brightness = 1;
>  	data->led_mc_cdev.led_cdev.flags = LED_REJECT_NAME_CONFLICT;
>  	data->led_mc_cdev.led_cdev.brightness_set_blocking = uniwill_led_brightness_set;
> +	data->led_mc_cdev.led_cdev.trigger_type = &uniwill_lightbar_trigger_type;
> +	data->led_mc_cdev.led_cdev.hw_control_trigger = uniwill_lightbar_trigger.name;
> +	data->led_mc_cdev.led_cdev.hw_control_set = uniwill_led_hw_control_set;

A private trigger does not need hw_control_set, which is only meant to
coordinate between different drivers, i.e., in the case when the LED
device and the hardware control trigger are located at different drivers.

I've also updated the documentation in commit 8e04e5382aa9 ("leds:
trigger: Add hw_offloaded() callback and provide
trigger_may_offload_to_hw attribute"), quoted below:

   Alternatively, a private trigger can be implemented along with the LED
   driver if the LED's hardware control doesn't fit any generic trigger. To
   associate the private trigger with the LED classdev, their `trigger_type`
   must be the same. To declare that the private trigger provides hardware
   control for the associated LED classdev, set the `hw_control_trigger`
   string to the trigger's name. Since both the LED classdev and the private
   trigger are in the same LED driver, it's not necessary for them to
   coordinate via `hw_control_*` callbacks.

I'd suggest calling uniwill_led_hw_control_set() directly from the
trigger's [de]activate callbacks, just like other drivers.

>  
>  	if (value & LIGHTBAR_S0_OFF)
>  		data->led_mc_cdev.led_cdev.brightness = 0;
>  	else
>  		data->led_mc_cdev.led_cdev.brightness = 1;
>  
> +	if (value & LIGHTBAR_WELCOME)

Check !LIGHTBAR_S0_OFF too?

If the last state is LIGHTBAR_S0_OFF|LIGHTBAR_WELCOME (probably caused by
an outdated driver without this patch), the trigger's activate callback
will be called on probe, lighting up the previously off LED. Is this
intentional?

Thanks,
Rong

> +		data->led_mc_cdev.led_cdev.default_trigger = uniwill_lightbar_trigger.name;
> +
>  	for (int i = 0; i < LED_CHANNELS; i++) {
>  		data->led_mc_subled_info[i].color_index = color_indices[i];
>  
> @@ -3540,17 +3552,31 @@ static int __init uniwill_init(void)
>  		pr_warn("Enabling potentially unsupported features\n");
>  	}
>  
> -	ret = platform_driver_register(&uniwill_driver);
> +	/*
> +	 * We cannot register the trigger inside the .probe callback of the
> +	 * platform driver, because each trigger needs a unique name.
> +	 */
> +	ret = led_trigger_register(&uniwill_lightbar_trigger);
>  	if (ret < 0)
>  		return ret;
>  
> +	ret = platform_driver_register(&uniwill_driver);
> +	if (ret < 0)
> +		goto err_platform;
> +
>  	ret = uniwill_wmi_register_driver();
> -	if (ret < 0) {
> -		platform_driver_unregister(&uniwill_driver);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		goto err_wmi;
>  
>  	return 0;
> +
> +err_wmi:
> +	platform_driver_unregister(&uniwill_driver);
> +
> +err_platform:
> +	led_trigger_unregister(&uniwill_lightbar_trigger);
> +
> +	return ret;
>  }
>  module_init(uniwill_init);
>  
> @@ -3558,6 +3584,7 @@ static void __exit uniwill_exit(void)
>  {
>  	uniwill_wmi_unregister_driver();
>  	platform_driver_unregister(&uniwill_driver);
> +	led_trigger_unregister(&uniwill_lightbar_trigger);
>  }
>  module_exit(uniwill_exit);
>  

Re: [PATCH v2 2/5] platform/x86: uniwill-laptop: Implement rainbow animation as trigger

[Armin Wolf]

Am 10.10.26 um 13:20 schrieb Rong Zhang:

> Hi Armin,
>
> On Wed, 2026-10-07 at 16:38 +0200, Armin Wolf wrote:
>> It turns out that some models require the lightbar to be activated
>> in order to display the rainbow animation, so users have to access
>> both the "rainbow_animation" sysfs attribute and the LED sysfs
>> interface when enabling the rainbow animation. This can cause short
>> glitches when the lightbar was previously disabled.
>>
>> Fix this by replacing the sysfs attribute with a custom LED trigger
>> called "uniwill-rainbow". Selecting this trigger will atomically
>> activate the lightbar and enable the rainbow animation mode. This
>> can potentially break userspace applications using the old sysfs
>> attribute, but so far no such application is known to exist (yet).
>>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   .../ABI/removed/sysfs-driver-uniwill-laptop   |  10 ++
>>   .../ABI/testing/sysfs-driver-uniwill-laptop   |  11 --
>>   .../admin-guide/laptops/uniwill-laptop.rst    |   5 +-
>>   MAINTAINERS                                   |   1 +
>>   drivers/platform/x86/uniwill/Kconfig          |   1 +
>>   drivers/platform/x86/uniwill/uniwill-acpi.c   | 133 +++++++++++-------
>>   6 files changed, 95 insertions(+), 66 deletions(-)
>>   create mode 100644 Documentation/ABI/removed/sysfs-driver-uniwill-laptop
>>
>> diff --git a/Documentation/ABI/removed/sysfs-driver-uniwill-laptop b/Documentation/ABI/removed/sysfs-driver-uniwill-laptop
>> new file mode 100644
>> index 000000000000..c226efc23b59
>> --- /dev/null
>> +++ b/Documentation/ABI/removed/sysfs-driver-uniwill-laptop
>> @@ -0,0 +1,10 @@
>> +What:		/sys/bus/platform/devices/INOU0000:XX/rainbow_animation
>> +Date:		November 2025
>> +KernelVersion:	6.19
>> +Contact:	Armin Wolf <W_Armin@gmx.de>
>> +Description:
>> +		Forces the integrated lightbar to display a rainbow animation when the machine
>> +		is not suspended. Writing "1"/"0" into this file enables/disables this
>> +		functionality.
>> +
>> +		Reading this file returns the current status of the rainbow animation functionality.
> It'd be more helpful by documenting the removal date, version and what
> has replaced it.

True, i will write a patch to address this.

>> diff --git a/Documentation/ABI/testing/sysfs-driver-uniwill-laptop b/Documentation/ABI/testing/sysfs-driver-uniwill-laptop
>> index 943f92c6b561..b9b9feebe2ed 100644
>> --- a/Documentation/ABI/testing/sysfs-driver-uniwill-laptop
>> +++ b/Documentation/ABI/testing/sysfs-driver-uniwill-laptop
>> @@ -29,17 +29,6 @@ Description:
>>   		Reading this file returns the current enable status of the touchpad toggle
>>   		functionality.
>>   
>> -What:		/sys/bus/platform/devices/INOU0000:XX/rainbow_animation
>> -Date:		November 2025
>> -KernelVersion:	6.19
>> -Contact:	Armin Wolf <W_Armin@gmx.de>
>> -Description:
>> -		Forces the integrated lightbar to display a rainbow animation when the machine
>> -		is not suspended. Writing "1"/"0" into this file enables/disables this
>> -		functionality.
>> -
>> -		Reading this file returns the current status of the rainbow animation functionality.
>> -
>>   What:		/sys/bus/platform/devices/INOU0000:XX/breathing_in_suspend
>>   Date:		November 2025
>>   KernelVersion:	6.19
>> diff --git a/Documentation/admin-guide/laptops/uniwill-laptop.rst b/Documentation/admin-guide/laptops/uniwill-laptop.rst
>> index be50b45b82ef..aaf50e3074c1 100644
>> --- a/Documentation/admin-guide/laptops/uniwill-laptop.rst
>> +++ b/Documentation/admin-guide/laptops/uniwill-laptop.rst
>> @@ -74,8 +74,9 @@ Lightbar
>>   The ``uniwill-laptop`` driver exposes the lightbar found on some models as a standard multicolor
>>   LED class device. The default name of this LED class device is ``uniwill:multicolor:status``.
>>   
>> -See Documentation/ABI/testing/sysfs-driver-uniwill-laptop for details on how to control the various
>> -animation modes of the lightbar.
>> +The lightbar supports a separate rainbow animation mode that can be enabled by selecting the
>> +`uniwill-rainbow` LED trigger. To enable a breathing animation during suspend, see
>> +Documentation/ABI/testing/sysfs-driver-uniwill-laptop for details.
>>   
>>   Keyboard Backlight
>>   ------------------
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index fa0571a5e479..f9dd979d0fa7 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -28106,6 +28106,7 @@ UNIWILL LAPTOP DRIVER
>>   M:	Armin Wolf <W_Armin@gmx.de>
>>   L:	platform-driver-x86@vger.kernel.org
>>   S:	Maintained
>> +F:	Documentation/ABI/removed/sysfs-driver-uniwill-laptop
>>   F:	Documentation/ABI/testing/sysfs-driver-uniwill-laptop
>>   F:	Documentation/admin-guide/laptops/uniwill-laptop.rst
>>   F:	Documentation/wmi/devices/uniwill-laptop.rst
>> diff --git a/drivers/platform/x86/uniwill/Kconfig b/drivers/platform/x86/uniwill/Kconfig
>> index d07cc8440188..f80794bdfaef 100644
>> --- a/drivers/platform/x86/uniwill/Kconfig
>> +++ b/drivers/platform/x86/uniwill/Kconfig
>> @@ -28,6 +28,7 @@ config UNIWILL_LAPTOP
>>   	depends on DMI
>>   	select REGMAP
>>   	select INPUT_SPARSEKMAP
>> +	select LEDS_TRIGGERS
>>   	help
>>   	  This driver adds support for various extra features found on Uniwill laptops,
>>   	  like the lightbar, hwmon sensors and hotkeys. It also supports many OEM laptops
>> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
>> index 788bf4064d34..0fb0d267664a 100644
>> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
>> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
>> @@ -900,51 +900,6 @@ static ssize_t touchpad_toggle_enable_show(struct device *dev, struct device_att
>>   
>>   static DEVICE_ATTR_RW(touchpad_toggle_enable);
>>   
>> -static ssize_t rainbow_animation_store(struct device *dev, struct device_attribute *attr,
>> -				       const char *buf, size_t count)
>> -{
>> -	struct uniwill_data *data = dev_get_drvdata(dev);
>> -	unsigned int value;
>> -	bool enable;
>> -	int ret;
>> -
>> -	ret = kstrtobool(buf, &enable);
>> -	if (ret < 0)
>> -		return ret;
>> -
>> -	if (enable)
>> -		value = LIGHTBAR_WELCOME;
>> -	else
>> -		value = 0;
>> -
>> -	guard(mutex)(&data->led_lock);
>> -
>> -	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_AC_CTRL, LIGHTBAR_WELCOME, value);
>> -	if (ret < 0)
>> -		return ret;
>> -
>> -	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_BAT_CTRL, LIGHTBAR_WELCOME, value);
>> -	if (ret < 0)
>> -		return ret;
>> -
>> -	return count;
>> -}
>> -
>> -static ssize_t rainbow_animation_show(struct device *dev, struct device_attribute *attr, char *buf)
>> -{
>> -	struct uniwill_data *data = dev_get_drvdata(dev);
>> -	unsigned int value;
>> -	int ret;
>> -
>> -	ret = regmap_read(data->regmap, EC_ADDR_LIGHTBAR_AC_CTRL, &value);
>> -	if (ret < 0)
>> -		return ret;
>> -
>> -	return sysfs_emit(buf, "%d\n", !!(value & LIGHTBAR_WELCOME));
>> -}
>> -
>> -static DEVICE_ATTR_RW(rainbow_animation);
>> -
>>   static ssize_t breathing_in_suspend_store(struct device *dev, struct device_attribute *attr,
>>   					  const char *buf, size_t count)
>>   {
>> @@ -1258,7 +1213,6 @@ static struct attribute *uniwill_attrs[] = {
>>   	&dev_attr_super_key_enable.attr,
>>   	&dev_attr_touchpad_toggle_enable.attr,
>>   	/* Lightbar-related */
>> -	&dev_attr_rainbow_animation.attr,
>>   	&dev_attr_breathing_in_suspend.attr,
>>   	/* Power-management-related */
>>   	&dev_attr_ctgp_offset.attr,
>> @@ -1288,8 +1242,7 @@ static umode_t uniwill_attr_is_visible(struct kobject *kobj, struct attribute *a
>>   			return attr->mode;
>>   	}
>>   
>> -	if (attr == &dev_attr_rainbow_animation.attr ||
>> -	    attr == &dev_attr_breathing_in_suspend.attr) {
>> +	if (attr == &dev_attr_breathing_in_suspend.attr) {
>>   		if (uniwill_device_supports(data, UNIWILL_FEATURE_LIGHTBAR))
>>   			return attr->mode;
> It'd be better to convert breathing_in_suspend into an LED device
> attribute. See led_classdev->groups.

We cannot do this because this field is being used by the multicolor LED class.
I can try to find a workaround for this.

>>   	}
>> @@ -1537,6 +1490,59 @@ static int uniwill_led_brightness_set(struct led_classdev *led_cdev, enum led_br
>>   	return regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_BAT_CTRL, LIGHTBAR_S0_OFF, value);
>>   }
>>   
>> +static int uniwill_led_hw_control_set(struct led_classdev *led_cdev, unsigned long flags)
>> +{
>> +	struct led_classdev_mc *led_mc_cdev = lcdev_to_mccdev(led_cdev);
>> +	struct uniwill_data *data = container_of(led_mc_cdev, struct uniwill_data, led_mc_cdev);
>> +	unsigned int value;
>> +	int ret;
>> +
>> +	guard(mutex)(&data->led_lock);
>> +
>> +	if (flags)
>> +		value = LIGHTBAR_WELCOME;
>> +	else
>> +		value = LIGHTBAR_S0_OFF;
>> +
>> +	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_AC_CTRL,
>> +				 LIGHTBAR_S0_OFF | LIGHTBAR_WELCOME, value);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = regmap_update_bits(data->regmap, EC_ADDR_LIGHTBAR_BAT_CTRL,
>> +				 LIGHTBAR_S0_OFF | LIGHTBAR_WELCOME, value);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	/* The LED is always on during hw control */
>> +	led_cdev->brightness = flags;
>> +
>> +	return 0;
>> +}
>> +
>> +static int uniwill_lightbar_trigger_activate(struct led_classdev *led_cdev)
>> +{
>> +	return led_cdev->hw_control_set(led_cdev, 1);
>> +}
>> +
>> +static void uniwill_lightbar_trigger_deactivate(struct led_classdev *led_cdev)
>> +{
>> +	int ret;
>> +
>> +	ret = led_cdev->hw_control_set(led_cdev, 0);
>> +	if (ret < 0)
>> +		dev_err(led_cdev->dev, "Failed to stop rainbow animation: %d\n", ret);
>> +}
>> +
>> +static struct led_hw_trigger_type uniwill_lightbar_trigger_type;
>> +
>> +static struct led_trigger uniwill_lightbar_trigger = {
>> +	.name = "uniwill-rainbow",
>> +	.activate = uniwill_lightbar_trigger_activate,
>> +	.deactivate = uniwill_lightbar_trigger_deactivate,
>> +	.trigger_type = &uniwill_lightbar_trigger_type,
>> +};
>> +
>>   #define LIGHTBAR_MASK	(LIGHTBAR_APP_EXISTS | LIGHTBAR_S0_OFF | LIGHTBAR_S3_OFF | LIGHTBAR_WELCOME)
>>   
>>   static int uniwill_led_init(struct uniwill_data *data)
>> @@ -1587,12 +1593,18 @@ static int uniwill_led_init(struct uniwill_data *data)
>>   	data->led_mc_cdev.led_cdev.max_brightness = 1;
>>   	data->led_mc_cdev.led_cdev.flags = LED_REJECT_NAME_CONFLICT;
>>   	data->led_mc_cdev.led_cdev.brightness_set_blocking = uniwill_led_brightness_set;
>> +	data->led_mc_cdev.led_cdev.trigger_type = &uniwill_lightbar_trigger_type;
>> +	data->led_mc_cdev.led_cdev.hw_control_trigger = uniwill_lightbar_trigger.name;
>> +	data->led_mc_cdev.led_cdev.hw_control_set = uniwill_led_hw_control_set;
> A private trigger does not need hw_control_set, which is only meant to
> coordinate between different drivers, i.e., in the case when the LED
> device and the hardware control trigger are located at different drivers.
>
> I've also updated the documentation in commit 8e04e5382aa9 ("leds:
> trigger: Add hw_offloaded() callback and provide
> trigger_may_offload_to_hw attribute"), quoted below:
>
>     Alternatively, a private trigger can be implemented along with the LED
>     driver if the LED's hardware control doesn't fit any generic trigger. To
>     associate the private trigger with the LED classdev, their `trigger_type`
>     must be the same. To declare that the private trigger provides hardware
>     control for the associated LED classdev, set the `hw_control_trigger`
>     string to the trigger's name. Since both the LED classdev and the private
>     trigger are in the same LED driver, it's not necessary for them to
>     coordinate via `hw_control_*` callbacks.
>
> I'd suggest calling uniwill_led_hw_control_set() directly from the
> trigger's [de]activate callbacks, just like other drivers.

I decided to use this callbacks in order to be able to add support for the keyboard backlight
animation in the future (if we someday find a device that supports this feature).

>>   
>>   	if (value & LIGHTBAR_S0_OFF)
>>   		data->led_mc_cdev.led_cdev.brightness = 0;
>>   	else
>>   		data->led_mc_cdev.led_cdev.brightness = 1;
>>   
>> +	if (value & LIGHTBAR_WELCOME)
> Check !LIGHTBAR_S0_OFF too?
>
> If the last state is LIGHTBAR_S0_OFF|LIGHTBAR_WELCOME (probably caused by
> an outdated driver without this patch), the trigger's activate callback
> will be called on probe, lighting up the previously off LED. Is this
> intentional?
>
> Thanks,
> Rong

Yes, this is intentional. LIGHTBAR_WELCOME implies !LIGHTBAR_S0_OFF, some devices
even assume this.

Thanks,
Armin Wolf

>> +		data->led_mc_cdev.led_cdev.default_trigger = uniwill_lightbar_trigger.name;
>> +
>>   	for (int i = 0; i < LED_CHANNELS; i++) {
>>   		data->led_mc_subled_info[i].color_index = color_indices[i];
>>   
>> @@ -3540,17 +3552,31 @@ static int __init uniwill_init(void)
>>   		pr_warn("Enabling potentially unsupported features\n");
>>   	}
>>   
>> -	ret = platform_driver_register(&uniwill_driver);
>> +	/*
>> +	 * We cannot register the trigger inside the .probe callback of the
>> +	 * platform driver, because each trigger needs a unique name.
>> +	 */
>> +	ret = led_trigger_register(&uniwill_lightbar_trigger);
>>   	if (ret < 0)
>>   		return ret;
>>   
>> +	ret = platform_driver_register(&uniwill_driver);
>> +	if (ret < 0)
>> +		goto err_platform;
>> +
>>   	ret = uniwill_wmi_register_driver();
>> -	if (ret < 0) {
>> -		platform_driver_unregister(&uniwill_driver);
>> -		return ret;
>> -	}
>> +	if (ret < 0)
>> +		goto err_wmi;
>>   
>>   	return 0;
>> +
>> +err_wmi:
>> +	platform_driver_unregister(&uniwill_driver);
>> +
>> +err_platform:
>> +	led_trigger_unregister(&uniwill_lightbar_trigger);
>> +
>> +	return ret;
>>   }
>>   module_init(uniwill_init);
>>   
>> @@ -3558,6 +3584,7 @@ static void __exit uniwill_exit(void)
>>   {
>>   	uniwill_wmi_unregister_driver();
>>   	platform_driver_unregister(&uniwill_driver);
>> +	led_trigger_unregister(&uniwill_lightbar_trigger);
>>   }
>>   module_exit(uniwill_exit);
>>   

Re: [PATCH v2 2/5] platform/x86: uniwill-laptop: Implement rainbow animation as trigger

[Rong Zhang]

Hi Armin,

On Sun, 2026-10-11 at 02:52 +0200, Armin Wolf wrote:
> Am 10.10.26 um 13:20 schrieb Rong Zhang:
> 
> > Hi Armin,
> > 
> > On Wed, 2026-10-07 at 16:38 +0200, Armin Wolf wrote:
> > > It turns out that some models require the lightbar to be activated
> > > in order to display the rainbow animation, so users have to access
> > > both the "rainbow_animation" sysfs attribute and the LED sysfs
> > > interface when enabling the rainbow animation. This can cause short
> > > glitches when the lightbar was previously disabled.
> > > 
> > > Fix this by replacing the sysfs attribute with a custom LED trigger
> > > called "uniwill-rainbow". Selecting this trigger will atomically
> > > activate the lightbar and enable the rainbow animation mode. This
> > > can potentially break userspace applications using the old sysfs
> > > attribute, but so far no such application is known to exist (yet).
> > > 
> > > Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> > > ---
> > >   .../ABI/removed/sysfs-driver-uniwill-laptop   |  10 ++
> > >   .../ABI/testing/sysfs-driver-uniwill-laptop   |  11 --
> > >   .../admin-guide/laptops/uniwill-laptop.rst    |   5 +-
> > >   MAINTAINERS                                   |   1 +
> > >   drivers/platform/x86/uniwill/Kconfig          |   1 +
> > >   drivers/platform/x86/uniwill/uniwill-acpi.c   | 133 +++++++++++-------
> > >   6 files changed, 95 insertions(+), 66 deletions(-)
> > >   create mode 100644 Documentation/ABI/removed/sysfs-driver-uniwill-laptop
> > > 
> > > diff --git a/Documentation/ABI/removed/sysfs-driver-uniwill-laptop b/Documentation/ABI/removed/sysfs-driver-uniwill-laptop
> > > new file mode 100644
> > > index 000000000000..c226efc23b59
> > > --- /dev/null
> > > +++ b/Documentation/ABI/removed/sysfs-driver-uniwill-laptop
> > > @@ -0,0 +1,10 @@
> > > +What:		/sys/bus/platform/devices/INOU0000:XX/rainbow_animation
> > > +Date:		November 2025
> > > +KernelVersion:	6.19
> > > +Contact:	Armin Wolf <W_Armin@gmx.de>
> > > +Description:
> > > +		Forces the integrated lightbar to display a rainbow animation when the machine
> > > +		is not suspended. Writing "1"/"0" into this file enables/disables this
> > > +		functionality.
> > > +
> > > +		Reading this file returns the current status of the rainbow animation functionality.
> > It'd be more helpful by documenting the removal date, version and what
> > has replaced it.
> 
> True, i will write a patch to address this.

[..snip..]

> > > -
> > >   static ssize_t breathing_in_suspend_store(struct device *dev, struct device_attribute *attr,
> > >   					  const char *buf, size_t count)
> > >   {
> > > @@ -1258,7 +1213,6 @@ static struct attribute *uniwill_attrs[] = {
> > >   	&dev_attr_super_key_enable.attr,
> > >   	&dev_attr_touchpad_toggle_enable.attr,
> > >   	/* Lightbar-related */
> > > -	&dev_attr_rainbow_animation.attr,
> > >   	&dev_attr_breathing_in_suspend.attr,
> > >   	/* Power-management-related */
> > >   	&dev_attr_ctgp_offset.attr,
> > > @@ -1288,8 +1242,7 @@ static umode_t uniwill_attr_is_visible(struct kobject *kobj, struct attribute *a
> > >   			return attr->mode;
> > >   	}
> > >   
> > > -	if (attr == &dev_attr_rainbow_animation.attr ||
> > > -	    attr == &dev_attr_breathing_in_suspend.attr) {
> > > +	if (attr == &dev_attr_breathing_in_suspend.attr) {
> > >   		if (uniwill_device_supports(data, UNIWILL_FEATURE_LIGHTBAR))
> > >   			return attr->mode;
> > It'd be better to convert breathing_in_suspend into an LED device
> > attribute. See led_classdev->groups.
> 
> We cannot do this because this field is being used by the multicolor LED class.

Whoops, I didn't notice that.

> I can try to find a workaround for this.

(+CC Lee, Pavel, linux-leds)

IMO the best way to solve the dilemma is to introduce class_group for
led-class-multicolor and led-class-flash, or else we will have to
introduce (or have already introduced?) more workarounds for other
drivers.

For this cycle let's keep the attribute as is.

[..snip..]

> > > +
> > > +static struct led_trigger uniwill_lightbar_trigger = {
> > > +	.name = "uniwill-rainbow",
> > > +	.activate = uniwill_lightbar_trigger_activate,
> > > +	.deactivate = uniwill_lightbar_trigger_deactivate,
> > > +	.trigger_type = &uniwill_lightbar_trigger_type,
> > > +};
> > > +
> > >   #define LIGHTBAR_MASK	(LIGHTBAR_APP_EXISTS | LIGHTBAR_S0_OFF | LIGHTBAR_S3_OFF | LIGHTBAR_WELCOME)
> > >   
> > >   static int uniwill_led_init(struct uniwill_data *data)
> > > @@ -1587,12 +1593,18 @@ static int uniwill_led_init(struct uniwill_data *data)
> > >   	data->led_mc_cdev.led_cdev.max_brightness = 1;
> > >   	data->led_mc_cdev.led_cdev.flags = LED_REJECT_NAME_CONFLICT;
> > >   	data->led_mc_cdev.led_cdev.brightness_set_blocking = uniwill_led_brightness_set;
> > > +	data->led_mc_cdev.led_cdev.trigger_type = &uniwill_lightbar_trigger_type;
> > > +	data->led_mc_cdev.led_cdev.hw_control_trigger = uniwill_lightbar_trigger.name;
> > > +	data->led_mc_cdev.led_cdev.hw_control_set = uniwill_led_hw_control_set;
> > A private trigger does not need hw_control_set, which is only meant to
> > coordinate between different drivers, i.e., in the case when the LED
> > device and the hardware control trigger are located at different drivers.
> > 
> > I've also updated the documentation in commit 8e04e5382aa9 ("leds:
> > trigger: Add hw_offloaded() callback and provide
> > trigger_may_offload_to_hw attribute"), quoted below:
> > 
> >     Alternatively, a private trigger can be implemented along with the LED
> >     driver if the LED's hardware control doesn't fit any generic trigger. To
> >     associate the private trigger with the LED classdev, their `trigger_type`
> >     must be the same. To declare that the private trigger provides hardware
> >     control for the associated LED classdev, set the `hw_control_trigger`
> >     string to the trigger's name. Since both the LED classdev and the private
> >     trigger are in the same LED driver, it's not necessary for them to
> >     coordinate via `hw_control_*` callbacks.
> > 
> > I'd suggest calling uniwill_led_hw_control_set() directly from the
> > trigger's [de]activate callbacks, just like other drivers.
> 
> I decided to use this callbacks in order to be able to add support for the keyboard backlight
> animation in the future (if we someday find a device that supports this feature).

The name "uniwill-rainbow" sounds irrelevant to keyboard backlight
animation. In this case the latter needs a new private trigger with a new
name, and it can have different [de]activate callbacks. IOW, we don't
need hw_control_set() either.

Otherwise, using hw_control_set() makes sense to me as long as the
current name is relevant.



Since I've CC'ed leds folks:

Per-key lighting is becoming increasingly common for keyboard backlight.

I am actually thinking about introducing LED array/matrix support to the
leds subsystem. An LED array/matrix device is just a special LED device
that controls all constituent LED devices, so that we can:

- implement keyboard backlight animation (or LampArray autonomous mode)
as an array/matrix hardware control trigger [1]
- tune the lighting of the whole array/matrix at once by tuning the LED
array/matrix device [2]
- tune per-key lighting by tuning constituent LED devices
- apply generic LED triggers to the whole array/matrix [1]
- apply different generic LED triggers to different constituent LED
devices
- properly deconflict the above operations

It definitely requires a lot of work to get things (especially locking
and atomicity) right, but it should be very helpful for devices with per-
key lighting once done. I believe missing proper LED array/matrix support
was the main reason why tuxedo_nb04_wmi_ab [3] implemented a virtual
LampArray HID device far before any userspace LampArray tool emerged.

[1]: The constituent LED devices will be inoperable when a trigger is
active on their array/matrix LED device.

[2]: The leds subsystem won't emulate this. The driver must implement
brightness_get() and brightness_set[_blocking]() for the LED array/matrix
device.

[3]: I know Texudo folks are working on adding hid-lamparray to connect
it to the LED subsystem. I've had another independent implementation with
HIDRAW sniff, no hid-generic dependency, autonomous mode as a trigger,
non-RGB LampArray support, and per-key LED device support in my local
tree, but that's another story. I will firstly push it further without
LED array/matrix support. Then I will try to add LED array/matrix
support, with hid-lamparray being the first user.

Thanks,
Rong

> 
> > >   
> > >   	if (value & LIGHTBAR_S0_OFF)
> > >   		data->led_mc_cdev.led_cdev.brightness = 0;
> > >   	else
> > >   		data->led_mc_cdev.led_cdev.brightness = 1;
> > >   
> > > +	if (value & LIGHTBAR_WELCOME)
> > Check !LIGHTBAR_S0_OFF too?
> > 
> > If the last state is LIGHTBAR_S0_OFF|LIGHTBAR_WELCOME (probably caused by
> > an outdated driver without this patch), the trigger's activate callback
> > will be called on probe, lighting up the previously off LED. Is this
> > intentional?
> > 
> > Thanks,
> > Rong
> 
> Yes, this is intentional. LIGHTBAR_WELCOME implies !LIGHTBAR_S0_OFF, some devices
> even assume this.
> 
> Thanks,
> Armin Wolf
> 
> > > +		data->led_mc_cdev.led_cdev.default_trigger = uniwill_lightbar_trigger.name;
> > > +
> > >   	for (int i = 0; i < LED_CHANNELS; i++) {
> > >   		data->led_mc_subled_info[i].color_index = color_indices[i];
> > >   
> > > @@ -3540,17 +3552,31 @@ static int __init uniwill_init(void)
> > >   		pr_warn("Enabling potentially unsupported features\n");
> > >   	}
> > >   
> > > -	ret = platform_driver_register(&uniwill_driver);
> > > +	/*
> > > +	 * We cannot register the trigger inside the .probe callback of the
> > > +	 * platform driver, because each trigger needs a unique name.
> > > +	 */
> > > +	ret = led_trigger_register(&uniwill_lightbar_trigger);
> > >   	if (ret < 0)
> > >   		return ret;
> > >   
> > > +	ret = platform_driver_register(&uniwill_driver);
> > > +	if (ret < 0)
> > > +		goto err_platform;
> > > +
> > >   	ret = uniwill_wmi_register_driver();
> > > -	if (ret < 0) {
> > > -		platform_driver_unregister(&uniwill_driver);
> > > -		return ret;
> > > -	}
> > > +	if (ret < 0)
> > > +		goto err_wmi;
> > >   
> > >   	return 0;
> > > +
> > > +err_wmi:
> > > +	platform_driver_unregister(&uniwill_driver);
> > > +
> > > +err_platform:
> > > +	led_trigger_unregister(&uniwill_lightbar_trigger);
> > > +
> > > +	return ret;
> > >   }
> > >   module_init(uniwill_init);
> > >   
> > > @@ -3558,6 +3584,7 @@ static void __exit uniwill_exit(void)
> > >   {
> > >   	uniwill_wmi_unregister_driver();
> > >   	platform_driver_unregister(&uniwill_driver);
> > > +	led_trigger_unregister(&uniwill_lightbar_trigger);
> > >   }
> > >   module_exit(uniwill_exit);
> > >   

Re: [PATCH v2 2/5] platform/x86: uniwill-laptop: Implement rainbow animation as trigger

[Armin Wolf]

Am 11.10.26 um 21:47 schrieb Rong Zhang:

> Hi Armin,
>
> On Sun, 2026-10-11 at 02:52 +0200, Armin Wolf wrote:
>> Am 10.10.26 um 13:20 schrieb Rong Zhang:
>>
>>> Hi Armin,
>>>
>>> On Wed, 2026-10-07 at 16:38 +0200, Armin Wolf wrote:
>>>> It turns out that some models require the lightbar to be activated
>>>> in order to display the rainbow animation, so users have to access
>>>> both the "rainbow_animation" sysfs attribute and the LED sysfs
>>>> interface when enabling the rainbow animation. This can cause short
>>>> glitches when the lightbar was previously disabled.
>>>>
>>>> Fix this by replacing the sysfs attribute with a custom LED trigger
>>>> called "uniwill-rainbow". Selecting this trigger will atomically
>>>> activate the lightbar and enable the rainbow animation mode. This
>>>> can potentially break userspace applications using the old sysfs
>>>> attribute, but so far no such application is known to exist (yet).
>>>>
>>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>>>> ---
>>>>    .../ABI/removed/sysfs-driver-uniwill-laptop   |  10 ++
>>>>    .../ABI/testing/sysfs-driver-uniwill-laptop   |  11 --
>>>>    .../admin-guide/laptops/uniwill-laptop.rst    |   5 +-
>>>>    MAINTAINERS                                   |   1 +
>>>>    drivers/platform/x86/uniwill/Kconfig          |   1 +
>>>>    drivers/platform/x86/uniwill/uniwill-acpi.c   | 133 +++++++++++-------
>>>>    6 files changed, 95 insertions(+), 66 deletions(-)
>>>>    create mode 100644 Documentation/ABI/removed/sysfs-driver-uniwill-laptop
>>>>
>>>> diff --git a/Documentation/ABI/removed/sysfs-driver-uniwill-laptop b/Documentation/ABI/removed/sysfs-driver-uniwill-laptop
>>>> new file mode 100644
>>>> index 000000000000..c226efc23b59
>>>> --- /dev/null
>>>> +++ b/Documentation/ABI/removed/sysfs-driver-uniwill-laptop
>>>> @@ -0,0 +1,10 @@
>>>> +What:		/sys/bus/platform/devices/INOU0000:XX/rainbow_animation
>>>> +Date:		November 2025
>>>> +KernelVersion:	6.19
>>>> +Contact:	Armin Wolf <W_Armin@gmx.de>
>>>> +Description:
>>>> +		Forces the integrated lightbar to display a rainbow animation when the machine
>>>> +		is not suspended. Writing "1"/"0" into this file enables/disables this
>>>> +		functionality.
>>>> +
>>>> +		Reading this file returns the current status of the rainbow animation functionality.
>>> It'd be more helpful by documenting the removal date, version and what
>>> has replaced it.
>> True, i will write a patch to address this.
> [..snip..]
>
>>>> -
>>>>    static ssize_t breathing_in_suspend_store(struct device *dev, struct device_attribute *attr,
>>>>    					  const char *buf, size_t count)
>>>>    {
>>>> @@ -1258,7 +1213,6 @@ static struct attribute *uniwill_attrs[] = {
>>>>    	&dev_attr_super_key_enable.attr,
>>>>    	&dev_attr_touchpad_toggle_enable.attr,
>>>>    	/* Lightbar-related */
>>>> -	&dev_attr_rainbow_animation.attr,
>>>>    	&dev_attr_breathing_in_suspend.attr,
>>>>    	/* Power-management-related */
>>>>    	&dev_attr_ctgp_offset.attr,
>>>> @@ -1288,8 +1242,7 @@ static umode_t uniwill_attr_is_visible(struct kobject *kobj, struct attribute *a
>>>>    			return attr->mode;
>>>>    	}
>>>>    
>>>> -	if (attr == &dev_attr_rainbow_animation.attr ||
>>>> -	    attr == &dev_attr_breathing_in_suspend.attr) {
>>>> +	if (attr == &dev_attr_breathing_in_suspend.attr) {
>>>>    		if (uniwill_device_supports(data, UNIWILL_FEATURE_LIGHTBAR))
>>>>    			return attr->mode;
>>> It'd be better to convert breathing_in_suspend into an LED device
>>> attribute. See led_classdev->groups.
>> We cannot do this because this field is being used by the multicolor LED class.
> Whoops, I didn't notice that.
>
>> I can try to find a workaround for this.
> (+CC Lee, Pavel, linux-leds)
>
> IMO the best way to solve the dilemma is to introduce class_group for
> led-class-multicolor and led-class-flash, or else we will have to
> introduce (or have already introduced?) more workarounds for other
> drivers.

I think we should rather use a separate device_type for multicolor/flash LED devices,
because this allows us to inject the sysfs groups without having to introduce yet
another field.

Would it be acceptable if multicolor/flash LED support cannot be build as a module anymore?

Thanks,
Armin Wolf

>
> For this cycle let's keep the attribute as is.
>
> [..snip..]
>
>>>> +
>>>> +static struct led_trigger uniwill_lightbar_trigger = {
>>>> +	.name = "uniwill-rainbow",
>>>> +	.activate = uniwill_lightbar_trigger_activate,
>>>> +	.deactivate = uniwill_lightbar_trigger_deactivate,
>>>> +	.trigger_type = &uniwill_lightbar_trigger_type,
>>>> +};
>>>> +
>>>>    #define LIGHTBAR_MASK	(LIGHTBAR_APP_EXISTS | LIGHTBAR_S0_OFF | LIGHTBAR_S3_OFF | LIGHTBAR_WELCOME)
>>>>    
>>>>    static int uniwill_led_init(struct uniwill_data *data)
>>>> @@ -1587,12 +1593,18 @@ static int uniwill_led_init(struct uniwill_data *data)
>>>>    	data->led_mc_cdev.led_cdev.max_brightness = 1;
>>>>    	data->led_mc_cdev.led_cdev.flags = LED_REJECT_NAME_CONFLICT;
>>>>    	data->led_mc_cdev.led_cdev.brightness_set_blocking = uniwill_led_brightness_set;
>>>> +	data->led_mc_cdev.led_cdev.trigger_type = &uniwill_lightbar_trigger_type;
>>>> +	data->led_mc_cdev.led_cdev.hw_control_trigger = uniwill_lightbar_trigger.name;
>>>> +	data->led_mc_cdev.led_cdev.hw_control_set = uniwill_led_hw_control_set;
>>> A private trigger does not need hw_control_set, which is only meant to
>>> coordinate between different drivers, i.e., in the case when the LED
>>> device and the hardware control trigger are located at different drivers.
>>>
>>> I've also updated the documentation in commit 8e04e5382aa9 ("leds:
>>> trigger: Add hw_offloaded() callback and provide
>>> trigger_may_offload_to_hw attribute"), quoted below:
>>>
>>>      Alternatively, a private trigger can be implemented along with the LED
>>>      driver if the LED's hardware control doesn't fit any generic trigger. To
>>>      associate the private trigger with the LED classdev, their `trigger_type`
>>>      must be the same. To declare that the private trigger provides hardware
>>>      control for the associated LED classdev, set the `hw_control_trigger`
>>>      string to the trigger's name. Since both the LED classdev and the private
>>>      trigger are in the same LED driver, it's not necessary for them to
>>>      coordinate via `hw_control_*` callbacks.
>>>
>>> I'd suggest calling uniwill_led_hw_control_set() directly from the
>>> trigger's [de]activate callbacks, just like other drivers.
>> I decided to use this callbacks in order to be able to add support for the keyboard backlight
>> animation in the future (if we someday find a device that supports this feature).
> The name "uniwill-rainbow" sounds irrelevant to keyboard backlight
> animation. In this case the latter needs a new private trigger with a new
> name, and it can have different [de]activate callbacks. IOW, we don't
> need hw_control_set() either.
>
> Otherwise, using hw_control_set() makes sense to me as long as the
> current name is relevant.
>
>
>
> Since I've CC'ed leds folks:
>
> Per-key lighting is becoming increasingly common for keyboard backlight.
>
> I am actually thinking about introducing LED array/matrix support to the
> leds subsystem. An LED array/matrix device is just a special LED device
> that controls all constituent LED devices, so that we can:
>
> - implement keyboard backlight animation (or LampArray autonomous mode)
> as an array/matrix hardware control trigger [1]
> - tune the lighting of the whole array/matrix at once by tuning the LED
> array/matrix device [2]
> - tune per-key lighting by tuning constituent LED devices
> - apply generic LED triggers to the whole array/matrix [1]
> - apply different generic LED triggers to different constituent LED
> devices
> - properly deconflict the above operations
>
> It definitely requires a lot of work to get things (especially locking
> and atomicity) right, but it should be very helpful for devices with per-
> key lighting once done. I believe missing proper LED array/matrix support
> was the main reason why tuxedo_nb04_wmi_ab [3] implemented a virtual
> LampArray HID device far before any userspace LampArray tool emerged.
>
> [1]: The constituent LED devices will be inoperable when a trigger is
> active on their array/matrix LED device.
>
> [2]: The leds subsystem won't emulate this. The driver must implement
> brightness_get() and brightness_set[_blocking]() for the LED array/matrix
> device.
>
> [3]: I know Texudo folks are working on adding hid-lamparray to connect
> it to the LED subsystem. I've had another independent implementation with
> HIDRAW sniff, no hid-generic dependency, autonomous mode as a trigger,
> non-RGB LampArray support, and per-key LED device support in my local
> tree, but that's another story. I will firstly push it further without
> LED array/matrix support. Then I will try to add LED array/matrix
> support, with hid-lamparray being the first user.
>
> Thanks,
> Rong
>
>>>>    
>>>>    	if (value & LIGHTBAR_S0_OFF)
>>>>    		data->led_mc_cdev.led_cdev.brightness = 0;
>>>>    	else
>>>>    		data->led_mc_cdev.led_cdev.brightness = 1;
>>>>    
>>>> +	if (value & LIGHTBAR_WELCOME)
>>> Check !LIGHTBAR_S0_OFF too?
>>>
>>> If the last state is LIGHTBAR_S0_OFF|LIGHTBAR_WELCOME (probably caused by
>>> an outdated driver without this patch), the trigger's activate callback
>>> will be called on probe, lighting up the previously off LED. Is this
>>> intentional?
>>>
>>> Thanks,
>>> Rong
>> Yes, this is intentional. LIGHTBAR_WELCOME implies !LIGHTBAR_S0_OFF, some devices
>> even assume this.
>>
>> Thanks,
>> Armin Wolf
>>
>>>> +		data->led_mc_cdev.led_cdev.default_trigger = uniwill_lightbar_trigger.name;
>>>> +
>>>>    	for (int i = 0; i < LED_CHANNELS; i++) {
>>>>    		data->led_mc_subled_info[i].color_index = color_indices[i];
>>>>    
>>>> @@ -3540,17 +3552,31 @@ static int __init uniwill_init(void)
>>>>    		pr_warn("Enabling potentially unsupported features\n");
>>>>    	}
>>>>    
>>>> -	ret = platform_driver_register(&uniwill_driver);
>>>> +	/*
>>>> +	 * We cannot register the trigger inside the .probe callback of the
>>>> +	 * platform driver, because each trigger needs a unique name.
>>>> +	 */
>>>> +	ret = led_trigger_register(&uniwill_lightbar_trigger);
>>>>    	if (ret < 0)
>>>>    		return ret;
>>>>    
>>>> +	ret = platform_driver_register(&uniwill_driver);
>>>> +	if (ret < 0)
>>>> +		goto err_platform;
>>>> +
>>>>    	ret = uniwill_wmi_register_driver();
>>>> -	if (ret < 0) {
>>>> -		platform_driver_unregister(&uniwill_driver);
>>>> -		return ret;
>>>> -	}
>>>> +	if (ret < 0)
>>>> +		goto err_wmi;
>>>>    
>>>>    	return 0;
>>>> +
>>>> +err_wmi:
>>>> +	platform_driver_unregister(&uniwill_driver);
>>>> +
>>>> +err_platform:
>>>> +	led_trigger_unregister(&uniwill_lightbar_trigger);
>>>> +
>>>> +	return ret;
>>>>    }
>>>>    module_init(uniwill_init);
>>>>    
>>>> @@ -3558,6 +3584,7 @@ static void __exit uniwill_exit(void)
>>>>    {
>>>>    	uniwill_wmi_unregister_driver();
>>>>    	platform_driver_unregister(&uniwill_driver);
>>>> +	led_trigger_unregister(&uniwill_lightbar_trigger);
>>>>    }
>>>>    module_exit(uniwill_exit);
>>>>    

Re: [PATCH v2 2/5] platform/x86: uniwill-laptop: Implement rainbow animation as trigger

[Rong Zhang]

Hi Armin,

On Mon, 2026-10-12 at 00:48 +0200, Armin Wolf wrote:
> Am 11.10.26 um 21:47 schrieb Rong Zhang:
> 
> > Hi Armin,
> > 
> > On Sun, 2026-10-11 at 02:52 +0200, Armin Wolf wrote:
> > > Am 10.10.26 um 13:20 schrieb Rong Zhang:
> > > 
> > > > Hi Armin,
> > > > 
> > > > On Wed, 2026-10-07 at 16:38 +0200, Armin Wolf wrote:
> > > > > It turns out that some models require the lightbar to be activated
> > > > > in order to display the rainbow animation, so users have to access
> > > > > both the "rainbow_animation" sysfs attribute and the LED sysfs
> > > > > interface when enabling the rainbow animation. This can cause short
> > > > > glitches when the lightbar was previously disabled.
> > > > > 
> > > > > Fix this by replacing the sysfs attribute with a custom LED trigger
> > > > > called "uniwill-rainbow". Selecting this trigger will atomically
> > > > > activate the lightbar and enable the rainbow animation mode. This
> > > > > can potentially break userspace applications using the old sysfs
> > > > > attribute, but so far no such application is known to exist (yet).
> > > > > 
> > > > > Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> > > > > ---
> > > > >    .../ABI/removed/sysfs-driver-uniwill-laptop   |  10 ++
> > > > >    .../ABI/testing/sysfs-driver-uniwill-laptop   |  11 --
> > > > >    .../admin-guide/laptops/uniwill-laptop.rst    |   5 +-
> > > > >    MAINTAINERS                                   |   1 +
> > > > >    drivers/platform/x86/uniwill/Kconfig          |   1 +
> > > > >    drivers/platform/x86/uniwill/uniwill-acpi.c   | 133 +++++++++++-------
> > > > >    6 files changed, 95 insertions(+), 66 deletions(-)
> > > > >    create mode 100644 Documentation/ABI/removed/sysfs-driver-uniwill-laptop
> > > > > 
> > > > > diff --git a/Documentation/ABI/removed/sysfs-driver-uniwill-laptop b/Documentation/ABI/removed/sysfs-driver-uniwill-laptop
> > > > > new file mode 100644
> > > > > index 000000000000..c226efc23b59
> > > > > --- /dev/null
> > > > > +++ b/Documentation/ABI/removed/sysfs-driver-uniwill-laptop
> > > > > @@ -0,0 +1,10 @@
> > > > > +What:		/sys/bus/platform/devices/INOU0000:XX/rainbow_animation
> > > > > +Date:		November 2025
> > > > > +KernelVersion:	6.19
> > > > > +Contact:	Armin Wolf <W_Armin@gmx.de>
> > > > > +Description:
> > > > > +		Forces the integrated lightbar to display a rainbow animation when the machine
> > > > > +		is not suspended. Writing "1"/"0" into this file enables/disables this
> > > > > +		functionality.
> > > > > +
> > > > > +		Reading this file returns the current status of the rainbow animation functionality.
> > > > It'd be more helpful by documenting the removal date, version and what
> > > > has replaced it.
> > > True, i will write a patch to address this.
> > [..snip..]
> > 
> > > > > -
> > > > >    static ssize_t breathing_in_suspend_store(struct device *dev, struct device_attribute *attr,
> > > > >    					  const char *buf, size_t count)
> > > > >    {
> > > > > @@ -1258,7 +1213,6 @@ static struct attribute *uniwill_attrs[] = {
> > > > >    	&dev_attr_super_key_enable.attr,
> > > > >    	&dev_attr_touchpad_toggle_enable.attr,
> > > > >    	/* Lightbar-related */
> > > > > -	&dev_attr_rainbow_animation.attr,
> > > > >    	&dev_attr_breathing_in_suspend.attr,
> > > > >    	/* Power-management-related */
> > > > >    	&dev_attr_ctgp_offset.attr,
> > > > > @@ -1288,8 +1242,7 @@ static umode_t uniwill_attr_is_visible(struct kobject *kobj, struct attribute *a
> > > > >    			return attr->mode;
> > > > >    	}
> > > > >    
> > > > > -	if (attr == &dev_attr_rainbow_animation.attr ||
> > > > > -	    attr == &dev_attr_breathing_in_suspend.attr) {
> > > > > +	if (attr == &dev_attr_breathing_in_suspend.attr) {
> > > > >    		if (uniwill_device_supports(data, UNIWILL_FEATURE_LIGHTBAR))
> > > > >    			return attr->mode;
> > > > It'd be better to convert breathing_in_suspend into an LED device
> > > > attribute. See led_classdev->groups.
> > > We cannot do this because this field is being used by the multicolor LED class.
> > Whoops, I didn't notice that.
> > 
> > > I can try to find a workaround for this.
> > (+CC Lee, Pavel, linux-leds)
> > 
> > IMO the best way to solve the dilemma is to introduce class_group for
> > led-class-multicolor and led-class-flash, or else we will have to
> > introduce (or have already introduced?) more workarounds for other
> > drivers.
> 
> I think we should rather use a separate device_type for multicolor/flash LED devices,
> because this allows us to inject the sysfs groups without having to introduce yet
> another field.

Sounds neat. Nice inspiration!

> 
> Would it be acceptable if multicolor/flash LED support cannot be build as a module anymore?

In fact, even LEDS_CLASS is tristate (though rarely built as a module
because LEDS_TRIGGERS promotes it to bool).

From my perspective, forcing LEDS_CLASS_FLASH and LEDS_CLASS_MULTICOLOR
as bool will be too harsh for distros. We don't need to build them into
vmlinux, as long as an internal register function with an extra
device_type parameter is exported. We've already had several glue exports
declared in drivers/leds/leds.h.

Thanks,
Rong

> 
> Thanks,
> Armin Wolf
> 
> > 
> > For this cycle let's keep the attribute as is.
> > 
> > [..snip..]
> > 
> > > > > +
> > > > > +static struct led_trigger uniwill_lightbar_trigger = {
> > > > > +	.name = "uniwill-rainbow",
> > > > > +	.activate = uniwill_lightbar_trigger_activate,
> > > > > +	.deactivate = uniwill_lightbar_trigger_deactivate,
> > > > > +	.trigger_type = &uniwill_lightbar_trigger_type,
> > > > > +};
> > > > > +
> > > > >    #define LIGHTBAR_MASK	(LIGHTBAR_APP_EXISTS | LIGHTBAR_S0_OFF | LIGHTBAR_S3_OFF | LIGHTBAR_WELCOME)
> > > > >    
> > > > >    static int uniwill_led_init(struct uniwill_data *data)
> > > > > @@ -1587,12 +1593,18 @@ static int uniwill_led_init(struct uniwill_data *data)
> > > > >    	data->led_mc_cdev.led_cdev.max_brightness = 1;
> > > > >    	data->led_mc_cdev.led_cdev.flags = LED_REJECT_NAME_CONFLICT;
> > > > >    	data->led_mc_cdev.led_cdev.brightness_set_blocking = uniwill_led_brightness_set;
> > > > > +	data->led_mc_cdev.led_cdev.trigger_type = &uniwill_lightbar_trigger_type;
> > > > > +	data->led_mc_cdev.led_cdev.hw_control_trigger = uniwill_lightbar_trigger.name;
> > > > > +	data->led_mc_cdev.led_cdev.hw_control_set = uniwill_led_hw_control_set;
> > > > A private trigger does not need hw_control_set, which is only meant to
> > > > coordinate between different drivers, i.e., in the case when the LED
> > > > device and the hardware control trigger are located at different drivers.
> > > > 
> > > > I've also updated the documentation in commit 8e04e5382aa9 ("leds:
> > > > trigger: Add hw_offloaded() callback and provide
> > > > trigger_may_offload_to_hw attribute"), quoted below:
> > > > 
> > > >      Alternatively, a private trigger can be implemented along with the LED
> > > >      driver if the LED's hardware control doesn't fit any generic trigger. To
> > > >      associate the private trigger with the LED classdev, their `trigger_type`
> > > >      must be the same. To declare that the private trigger provides hardware
> > > >      control for the associated LED classdev, set the `hw_control_trigger`
> > > >      string to the trigger's name. Since both the LED classdev and the private
> > > >      trigger are in the same LED driver, it's not necessary for them to
> > > >      coordinate via `hw_control_*` callbacks.
> > > > 
> > > > I'd suggest calling uniwill_led_hw_control_set() directly from the
> > > > trigger's [de]activate callbacks, just like other drivers.
> > > I decided to use this callbacks in order to be able to add support for the keyboard backlight
> > > animation in the future (if we someday find a device that supports this feature).
> > The name "uniwill-rainbow" sounds irrelevant to keyboard backlight
> > animation. In this case the latter needs a new private trigger with a new
> > name, and it can have different [de]activate callbacks. IOW, we don't
> > need hw_control_set() either.
> > 
> > Otherwise, using hw_control_set() makes sense to me as long as the
> > current name is relevant.
> > 
> > 
> > 
> > Since I've CC'ed leds folks:
> > 
> > Per-key lighting is becoming increasingly common for keyboard backlight.
> > 
> > I am actually thinking about introducing LED array/matrix support to the
> > leds subsystem. An LED array/matrix device is just a special LED device
> > that controls all constituent LED devices, so that we can:
> > 
> > - implement keyboard backlight animation (or LampArray autonomous mode)
> > as an array/matrix hardware control trigger [1]
> > - tune the lighting of the whole array/matrix at once by tuning the LED
> > array/matrix device [2]
> > - tune per-key lighting by tuning constituent LED devices
> > - apply generic LED triggers to the whole array/matrix [1]
> > - apply different generic LED triggers to different constituent LED
> > devices
> > - properly deconflict the above operations
> > 
> > It definitely requires a lot of work to get things (especially locking
> > and atomicity) right, but it should be very helpful for devices with per-
> > key lighting once done. I believe missing proper LED array/matrix support
> > was the main reason why tuxedo_nb04_wmi_ab [3] implemented a virtual
> > LampArray HID device far before any userspace LampArray tool emerged.
> > 
> > [1]: The constituent LED devices will be inoperable when a trigger is
> > active on their array/matrix LED device.
> > 
> > [2]: The leds subsystem won't emulate this. The driver must implement
> > brightness_get() and brightness_set[_blocking]() for the LED array/matrix
> > device.
> > 
> > [3]: I know Texudo folks are working on adding hid-lamparray to connect
> > it to the LED subsystem. I've had another independent implementation with
> > HIDRAW sniff, no hid-generic dependency, autonomous mode as a trigger,
> > non-RGB LampArray support, and per-key LED device support in my local
> > tree, but that's another story. I will firstly push it further without
> > LED array/matrix support. Then I will try to add LED array/matrix
> > support, with hid-lamparray being the first user.
> > 
> > Thanks,
> > Rong
> > 
> > > > >    
> > > > >    	if (value & LIGHTBAR_S0_OFF)
> > > > >    		data->led_mc_cdev.led_cdev.brightness = 0;
> > > > >    	else
> > > > >    		data->led_mc_cdev.led_cdev.brightness = 1;
> > > > >    
> > > > > +	if (value & LIGHTBAR_WELCOME)
> > > > Check !LIGHTBAR_S0_OFF too?
> > > > 
> > > > If the last state is LIGHTBAR_S0_OFF|LIGHTBAR_WELCOME (probably caused by
> > > > an outdated driver without this patch), the trigger's activate callback
> > > > will be called on probe, lighting up the previously off LED. Is this
> > > > intentional?
> > > > 
> > > > Thanks,
> > > > Rong
> > > Yes, this is intentional. LIGHTBAR_WELCOME implies !LIGHTBAR_S0_OFF, some devices
> > > even assume this.
> > > 
> > > Thanks,
> > > Armin Wolf
> > > 
> > > > > +		data->led_mc_cdev.led_cdev.default_trigger = uniwill_lightbar_trigger.name;
> > > > > +
> > > > >    	for (int i = 0; i < LED_CHANNELS; i++) {
> > > > >    		data->led_mc_subled_info[i].color_index = color_indices[i];
> > > > >    
> > > > > @@ -3540,17 +3552,31 @@ static int __init uniwill_init(void)
> > > > >    		pr_warn("Enabling potentially unsupported features\n");
> > > > >    	}
> > > > >    
> > > > > -	ret = platform_driver_register(&uniwill_driver);
> > > > > +	/*
> > > > > +	 * We cannot register the trigger inside the .probe callback of the
> > > > > +	 * platform driver, because each trigger needs a unique name.
> > > > > +	 */
> > > > > +	ret = led_trigger_register(&uniwill_lightbar_trigger);
> > > > >    	if (ret < 0)
> > > > >    		return ret;
> > > > >    
> > > > > +	ret = platform_driver_register(&uniwill_driver);
> > > > > +	if (ret < 0)
> > > > > +		goto err_platform;
> > > > > +
> > > > >    	ret = uniwill_wmi_register_driver();
> > > > > -	if (ret < 0) {
> > > > > -		platform_driver_unregister(&uniwill_driver);
> > > > > -		return ret;
> > > > > -	}
> > > > > +	if (ret < 0)
> > > > > +		goto err_wmi;
> > > > >    
> > > > >    	return 0;
> > > > > +
> > > > > +err_wmi:
> > > > > +	platform_driver_unregister(&uniwill_driver);
> > > > > +
> > > > > +err_platform:
> > > > > +	led_trigger_unregister(&uniwill_lightbar_trigger);
> > > > > +
> > > > > +	return ret;
> > > > >    }
> > > > >    module_init(uniwill_init);
> > > > >    
> > > > > @@ -3558,6 +3584,7 @@ static void __exit uniwill_exit(void)
> > > > >    {
> > > > >    	uniwill_wmi_unregister_driver();
> > > > >    	platform_driver_unregister(&uniwill_driver);
> > > > > +	led_trigger_unregister(&uniwill_lightbar_trigger);
> > > > >    }
> > > > >    module_exit(uniwill_exit);
> > > > >    

Re: [PATCH v2 0/5] platform/x86: uniwill-laptop: Lightbar fixes

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2692 bytes --]

On Sat, 10 Oct 2026, Armin Wolf wrote:

> Am 08.10.26 um 19:38 schrieb Ilpo Järvinen:
> 
> > On Wed, 07 Oct 2026 16:38:53 +0200, Armin Wolf wrote:
> > 
> > > This patch series mainly deals with various issues inside the code
> > > responsible for controlling the lightbar on supported devices.
> > > 
> > > The first patch utilizes the improved multicolor LED API to report
> > > the correct brightness of the lightbar to userspace. The next two
> > > patches fix issues discovered inside the code for controlling
> > > animations, while the fourth patch fixes an issue with the naming
> > > of the LEDs in general. The last patch also adds support for another
> > > variant of the Intel NUC x15 laptop kit.
> > > 
> > > [...]
> > Thank you for your contribution, it has been applied to my local
> > review-ilpo-next branch. Note it will show up in the public
> > platform-drivers-x86/review-ilpo-next branch only once I've pushed my
> > local branch there, which might take a while.
> 
> Thank you :)
> 
> I noticed that the LED next branch contains a commit that requires triggers
> to implement a new callback and prints a warning if this is not the case.
> 
> Should i wait till the next cycle to send a patch implementing said callback
> or do you want to rebase for-next onto LED next?

...Goes to look at the other repo...

I don't want to base on top of other maintainers next branches (or pull 
them into my for-next).

I suppose it might be best to make that addition after -rc1 through 
fixes, or if time permits, already during the merge window in a separate 
PR after the usual for-next PR.

--
 i.

> Thanks,
> Armin Wolf
> 
> > FYI [if applicable to your patch], as per Linus' policy change, also
> > fixes are mostly routed through for-next unless the fix is for a
> > commit introduced in the most recent cycle or is clearly a regression
> > fix.
> > 
> > The list of commits applied:
> > [1/5] platform/x86: uniwill-laptop: Report correct lightbar brightness
> >        commit: de41d8b4d0dbb81c81cd99c245238d878a3e1951
> > [2/5] platform/x86: uniwill-laptop: Implement rainbow animation as trigger
> >        commit: 59e4e406b6c08ad41f3ad86db6d42b82d8539c12
> > [3/5] platform/x86: uniwill-laptop: Fix breathing animation on Intel QC
> >        commit: 969a3bfa474e35828e142d0160f393a97d9bf0ee
> > [4/5] platform/x86: uniwill-laptop: Label multicolor LEDs correctly
> >        commit: 42e6b269fc1134a3fcce57539034ee496e2c1344
> > [5/5] platform/x86: uniwill-laptop: Extend support for the Intel NUC x15
> >        commit: 7a159a14ce0f5d5a3c2f835f39986ec5539ae374
> > 
> > --
> >   i.
> > 
> > 
> 

-- 
 i.