[PATCH v4 0/2] platform/x86: asus-wmi: TUF keyboard RGB readback

[PATCH v4 0/2] platform/x86: asus-wmi: TUF keyboard RGB readback

[Bartu Alev]

Patches 1-3 of v3 are in review-ilpo-next already, so this reroll
carries only the remaining two patches, rebased on top of it.

DSTS device 0x0010005B evaluates the DSDT method EC0.KBLS(), which
snapshots the EC's host-visible keyboard state into a 16-byte buffer:
lighting mode, red, green and blue levels, animation speed, and the
four power flags.  kbd_rgb_mode and kbd_rgb_state become readable and
report the write format with n/a in the command position, because the
command word belongs to the write verb and has no state equivalent.

Changes in v4:
- report n/a in the command position on read instead of a synthetic 1,
  as discussed in the v2 thread: the command word belongs to the write
  verb, so there is no state to report there
- ABI documentation: describe the n/a command word, state the mode
  clamping exactly (mode 9 and modes 12 and above are sent as 10), and
  note that reads fail with -ENODEV on hardware without readback
  support

Provenance: the KBLS buffer layout and the speed-code semantics were
recovered by static analysis of the EC firmware (ITE IT5570VG), with
the help of an LLM-driven toolchain.  Tested on a TUF Gaming A15
(FA507NV): the v2 readback on kernel 7.2.6, and the S5 behaviour of
the shutdown flag by A/B testing both values.

v3: https://lore.kernel.org/all/20261010101601.888709-1-bartualev@gmail.com/
v2: https://lore.kernel.org/all/20260926005625.171560-1-bartualev@gmail.com/
v1: https://lore.kernel.org/all/20260925200744.129714-1-bartualev@gmail.com/

Signed-off-by: Bartu Alev <bartualev@gmail.com>
---
Bartu Alev (2):
      platform/x86: asus-wmi: add TUF keyboard RGB readback support
      Documentation/ABI: asus-wmi: document TUF keyboard RGB attributes

 Documentation/ABI/testing/sysfs-platform-asus-wmi | 46 +++++++++++++
 drivers/platform/x86/asus-wmi.c                   | 83 ++++++++++++++++++++++-
 include/linux/platform_data/x86/asus-wmi.h        |  3 +
 3 files changed, 130 insertions(+), 2 deletions(-)
---
base-commit: 9d3475065167197742b0882aff21cf8e78393277
change-id: 20261029-asus-tuf-rgb-v4-06ad93a6060b

Best regards,
--  
Bartu Alev <bartualev@gmail.com>

[PATCH v4 1/2] platform/x86: asus-wmi: add TUF keyboard RGB readback support

[Bartu Alev]

The TUF keyboard RGB attributes kbd_rgb_mode and kbd_rgb_state are
write-only, so userspace tools keep their own copy of the lighting
configuration and lose track of it whenever the EC changes state on its
own: after a firmware reset or a boot into another operating system the
tools show a stale configuration until the user rewrites every field.

DSTS device 0x0010005B evaluates the DSDT method EC0.KBLS(), which
snapshots the EC's host-visible keyboard state into a 16-byte buffer:
lighting mode, red, green and blue levels, animation speed, and the
four power-state flags.  The first byte is a constant 1 written by the
AML, which distinguishes a KBLS reply from any other object the method
could return.

Add kbd_rgb_read_status() to evaluate and validate that buffer, parse
it through a packed struct, and convert both attributes to
DEVICE_ATTR_RW.  The read returns the write format with "n/a" in the
command position: the command word belongs to the write verb (apply
now or save to flash) and both verbs update the EC mirror the same
way, so there is no state to report there.  The three EC speed codes
map back to the sysfs indices 0 to 2.

Readback was verified against the EC firmware: the status bytes mirror
EC RAM at 0x0B22 to 0x0B2E, and the speed codes 0xe1, 0xeb and 0xf5
select animation step thresholds of 30, 20 and 10 ticks per phase,
which matches the slow, normal and fast labels.  Tested on a TUF
Gaming A15 (FA507NV).

Suggested-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Bartu Alev <bartualev@gmail.com>
---
 drivers/platform/x86/asus-wmi.c            | 83 +++++++++++++++++++++++++++++-
 include/linux/platform_data/x86/asus-wmi.h |  3 ++
 2 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index b33ebdb21aa2..054d1d684bb0 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1062,6 +1062,65 @@ static DEVICE_ATTR_RW(gpu_mux_mode);
 #define TUF_KBD_RGB_SPEED_SLOW		0xe1
 #define TUF_KBD_RGB_SPEED_NORMAL	0xeb
 #define TUF_KBD_RGB_SPEED_FAST		0xf5
+/* TUF keyboard RGB status readback *******************************************/
+struct asus_wmi_kbd_rgb_status {
+	u8 signature;
+	u8 mode;
+	u8 red;
+	u8 green;
+	u8 blue;
+	u8 speed;
+	u8 state;
+	u8 unused[9];
+} __packed;
+
+static int kbd_rgb_read_status(struct asus_wmi_kbd_rgb_status *status)
+{
+	int err;
+
+	err = asus_wmi_evaluate_method_buf(ASUS_WMI_METHODID_DSTS,
+					   ASUS_WMI_DEVID_TUF_RGB_READ_STATUS,
+					   0, (u8 *)status, sizeof(*status));
+	if (err)
+		return err < 0 ? err : -ENODEV;
+
+	/* The AML writes a constant 1 here; anything else is not a KBLS reply */
+	if (status->signature != 1)
+		return -ENODEV;
+
+	return 0;
+}
+
+static ssize_t kbd_rgb_mode_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct asus_wmi_kbd_rgb_status status = {};
+	u32 speed;
+	int err;
+
+	err = kbd_rgb_read_status(&status);
+	if (err)
+		return err;
+
+	switch (status.speed) {
+	case TUF_KBD_RGB_SPEED_SLOW:
+		speed = 0;
+		break;
+	case TUF_KBD_RGB_SPEED_FAST:
+		speed = 2;
+		break;
+	case TUF_KBD_RGB_SPEED_NORMAL:
+	default:
+		speed = 1;
+		break;
+	}
+
+	/* The command word belongs to the write verb, so report n/a. */
+	return sysfs_emit(buf, "n/a %d %d %d %d %d\n",
+			  status.mode, status.red, status.green, status.blue,
+			  speed);
+}
+
 static ssize_t kbd_rgb_mode_store(struct device *dev,
 				 struct device_attribute *attr,
 				 const char *buf, size_t count)
@@ -1114,7 +1173,7 @@ static ssize_t kbd_rgb_mode_store(struct device *dev,
 
 	return count;
 }
-static DEVICE_ATTR_WO(kbd_rgb_mode);
+static DEVICE_ATTR_RW(kbd_rgb_mode);
 
 static DEVICE_STRING_ATTR_RO(kbd_rgb_mode_index, 0444,
 			     "cmd mode red green blue speed");
@@ -1135,6 +1194,26 @@ static const struct attribute_group kbd_rgb_mode_group = {
 #define TUF_KBD_RGB_STATE_AWAKE		BIT(3)
 #define TUF_KBD_RGB_STATE_SLEEP		BIT(5)
 #define TUF_KBD_RGB_STATE_SHUTDOWN	BIT(7)
+static ssize_t kbd_rgb_state_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct asus_wmi_kbd_rgb_status status = {};
+	u8 state;
+	int err;
+
+	err = kbd_rgb_read_status(&status);
+	if (err)
+		return err;
+
+	state = status.state;
+
+	return sysfs_emit(buf, "n/a %d %d %d %d\n",
+			  !!(state & TUF_KBD_RGB_STATE_BOOT),
+			  !!(state & TUF_KBD_RGB_STATE_AWAKE),
+			  !!(state & TUF_KBD_RGB_STATE_SLEEP),
+			  !!(state & TUF_KBD_RGB_STATE_SHUTDOWN));
+}
+
 static ssize_t kbd_rgb_state_store(struct device *dev,
 				 struct device_attribute *attr,
 				 const char *buf, size_t count)
@@ -1166,7 +1245,7 @@ static ssize_t kbd_rgb_state_store(struct device *dev,
 
 	return count;
 }
-static DEVICE_ATTR_WO(kbd_rgb_state);
+static DEVICE_ATTR_RW(kbd_rgb_state);
 
 static DEVICE_STRING_ATTR_RO(kbd_rgb_state_index, 0444,
 			     "cmd boot awake sleep shutdown");
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 66c7ab35d6c8..393fd3c5949a 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -166,6 +166,9 @@
 /* TUF laptop RGB power/state */
 #define ASUS_WMI_DEVID_TUF_RGB_CMD	0x00100057
 
+/* TUF laptop RGB keyboard status readback */
+#define ASUS_WMI_DEVID_TUF_RGB_READ_STATUS	0x0010005B
+
 /* Bootup sound control */
 #define ASUS_WMI_DEVID_BOOT_SOUND	0x00130022
 

-- 
2.56.0

[PATCH v4 2/2] Documentation/ABI: asus-wmi: document TUF keyboard RGB attributes

[Bartu Alev]

The kbd_rgb_mode and kbd_rgb_state attributes predate their
documentation: they were introduced as write-only interfaces and never
received ABI entries.  The readback support now being added makes the
omission visible, since the read format is part of the interface.

Describe both attributes, their positional write formats, the write
verbs, and the readback form.  The attributes are gated on WMI device
presence rather than on the laptop model, and userspace tools route
several non-TUF ASUS lines through the same devices, so the
description says ASUS laptops that expose the WMI zonal RGB devices
instead of claiming TUF models only.

Signed-off-by: Bartu Alev <bartualev@gmail.com>
---
 Documentation/ABI/testing/sysfs-platform-asus-wmi | 46 +++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-platform-asus-wmi b/Documentation/ABI/testing/sysfs-platform-asus-wmi
index f9825c6150b5..23e5f265f722 100644
--- a/Documentation/ABI/testing/sysfs-platform-asus-wmi
+++ b/Documentation/ABI/testing/sysfs-platform-asus-wmi
@@ -238,3 +238,49 @@ Description:
 		Set if the MCU can go in to low-power mode on system sleep
 			* 0 - False,
 			* 1 - True
+
+What:		/sys/class/leds/asus::kbd_backlight/kbd_rgb_mode
+Date:		Oct 2026
+KernelVersion:	7.4
+Contact:	Bartu Alev <bartualev@gmail.com>
+Description:
+		Controls the keyboard RGB lighting on ASUS laptops that
+		expose the WMI zonal RGB devices (the kernel names them
+		"TUF" for historical reasons; TUF models have them and so
+		do some other ASUS lines).  Writes take six positional
+		values:
+
+		    <cmd> <mode> <red> <green> <blue> <speed>
+
+		cmd is 1 to save the setting to flash or 0 to apply it for
+		the current boot only.  mode selects the animation, 0 is
+		static; mode 9 and modes 12 and above are sent as 10.  red,
+		green and blue are 0 to 255.  speed is 0 (slow), 1 (normal)
+		or 2 (fast).
+
+		Reads return the current hardware configuration in the same
+		positional format, except the command word, which reads as
+		n/a because it only applies to writes.  Reads fail with
+		-ENODEV on hardware without readback support.
+
+What:		/sys/class/leds/asus::kbd_backlight/kbd_rgb_state
+Date:		Oct 2026
+KernelVersion:	7.4
+Contact:	Bartu Alev <bartualev@gmail.com>
+Description:
+		Controls when the keyboard RGB backlight is lit, on the
+		hardware that exposes kbd_rgb_mode.  Writes take five
+		positional values:
+
+		    <cmd> <boot> <awake> <sleep> <shutdown>
+
+		cmd is 1 to save the setting to flash or 0 to apply it for
+		the current boot only.  Each flag is 1 or 0: boot, awake
+		and sleep enable the backlight during boot, normal
+		operation and suspend, and shutdown keeps the backlight lit
+		while the machine powers off (S5).
+
+		Reads return the current flags in the same positional format,
+		except the command word, which reads as n/a because it only
+		applies to writes.  Reads fail with -ENODEV on hardware
+		without readback support.

-- 
2.56.0

Re: [PATCH v4 1/2] platform/x86: asus-wmi: add TUF keyboard RGB readback support

[Ilpo Järvinen]

On Thu, 29 Oct 2026, Bartu Alev wrote:

> The TUF keyboard RGB attributes kbd_rgb_mode and kbd_rgb_state are
> write-only, so userspace tools keep their own copy of the lighting
> configuration and lose track of it whenever the EC changes state on its
> own: after a firmware reset or a boot into another operating system the
> tools show a stale configuration until the user rewrites every field.
> 
> DSTS device 0x0010005B evaluates the DSDT method EC0.KBLS(), which
> snapshots the EC's host-visible keyboard state into a 16-byte buffer:
> lighting mode, red, green and blue levels, animation speed, and the
> four power-state flags.  The first byte is a constant 1 written by the
> AML, which distinguishes a KBLS reply from any other object the method
> could return.
> 
> Add kbd_rgb_read_status() to evaluate and validate that buffer, parse
> it through a packed struct, and convert both attributes to
> DEVICE_ATTR_RW.  The read returns the write format with "n/a" in the
> command position: the command word belongs to the write verb (apply
> now or save to flash) and both verbs update the EC mirror the same
> way, so there is no state to report there.  The three EC speed codes
> map back to the sysfs indices 0 to 2.
> 
> Readback was verified against the EC firmware: the status bytes mirror
> EC RAM at 0x0B22 to 0x0B2E, and the speed codes 0xe1, 0xeb and 0xf5
> select animation step thresholds of 30, 20 and 10 ticks per phase,
> which matches the slow, normal and fast labels.  Tested on a TUF
> Gaming A15 (FA507NV).
> 
> Suggested-by: Denis Benato <denis.benato@linux.dev>
> Signed-off-by: Bartu Alev <bartualev@gmail.com>
> ---
>  drivers/platform/x86/asus-wmi.c            | 83 +++++++++++++++++++++++++++++-
>  include/linux/platform_data/x86/asus-wmi.h |  3 ++
>  2 files changed, 84 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index b33ebdb21aa2..054d1d684bb0 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -1062,6 +1062,65 @@ static DEVICE_ATTR_RW(gpu_mux_mode);
>  #define TUF_KBD_RGB_SPEED_SLOW		0xe1
>  #define TUF_KBD_RGB_SPEED_NORMAL	0xeb
>  #define TUF_KBD_RGB_SPEED_FAST		0xf5
> +/* TUF keyboard RGB status readback *******************************************/
> +struct asus_wmi_kbd_rgb_status {
> +	u8 signature;
> +	u8 mode;
> +	u8 red;
> +	u8 green;
> +	u8 blue;
> +	u8 speed;
> +	u8 state;
> +	u8 unused[9];
> +} __packed;
> +
> +static int kbd_rgb_read_status(struct asus_wmi_kbd_rgb_status *status)
> +{
> +	int err;
> +
> +	err = asus_wmi_evaluate_method_buf(ASUS_WMI_METHODID_DSTS,
> +					   ASUS_WMI_DEVID_TUF_RGB_READ_STATUS,
> +					   0, (u8 *)status, sizeof(*status));
> +	if (err)
> +		return err < 0 ? err : -ENODEV;

What do you (or Denis) think on sashiko's mention about 
asus_wmi_evaluate_method_buf() casting integer.value to int which might 
end up aliasing high-bit on return values with errnos?

The other caller seems to also return that value directly out of _store 
function().

-- 
 i.

> +	/* The AML writes a constant 1 here; anything else is not a KBLS reply */
> +	if (status->signature != 1)
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +static ssize_t kbd_rgb_mode_show(struct device *dev,
> +				 struct device_attribute *attr, char *buf)
> +{
> +	struct asus_wmi_kbd_rgb_status status = {};
> +	u32 speed;
> +	int err;
> +
> +	err = kbd_rgb_read_status(&status);
> +	if (err)
> +		return err;
> +
> +	switch (status.speed) {
> +	case TUF_KBD_RGB_SPEED_SLOW:
> +		speed = 0;
> +		break;
> +	case TUF_KBD_RGB_SPEED_FAST:
> +		speed = 2;
> +		break;
> +	case TUF_KBD_RGB_SPEED_NORMAL:
> +	default:
> +		speed = 1;
> +		break;
> +	}
> +
> +	/* The command word belongs to the write verb, so report n/a. */
> +	return sysfs_emit(buf, "n/a %d %d %d %d %d\n",
> +			  status.mode, status.red, status.green, status.blue,
> +			  speed);
> +}
> +
>  static ssize_t kbd_rgb_mode_store(struct device *dev,
>  				 struct device_attribute *attr,
>  				 const char *buf, size_t count)
> @@ -1114,7 +1173,7 @@ static ssize_t kbd_rgb_mode_store(struct device *dev,
>  
>  	return count;
>  }
> -static DEVICE_ATTR_WO(kbd_rgb_mode);
> +static DEVICE_ATTR_RW(kbd_rgb_mode);
>  
>  static DEVICE_STRING_ATTR_RO(kbd_rgb_mode_index, 0444,
>  			     "cmd mode red green blue speed");
> @@ -1135,6 +1194,26 @@ static const struct attribute_group kbd_rgb_mode_group = {
>  #define TUF_KBD_RGB_STATE_AWAKE		BIT(3)
>  #define TUF_KBD_RGB_STATE_SLEEP		BIT(5)
>  #define TUF_KBD_RGB_STATE_SHUTDOWN	BIT(7)
> +static ssize_t kbd_rgb_state_show(struct device *dev,
> +				  struct device_attribute *attr, char *buf)
> +{
> +	struct asus_wmi_kbd_rgb_status status = {};
> +	u8 state;
> +	int err;
> +
> +	err = kbd_rgb_read_status(&status);
> +	if (err)
> +		return err;
> +
> +	state = status.state;
> +
> +	return sysfs_emit(buf, "n/a %d %d %d %d\n",
> +			  !!(state & TUF_KBD_RGB_STATE_BOOT),
> +			  !!(state & TUF_KBD_RGB_STATE_AWAKE),
> +			  !!(state & TUF_KBD_RGB_STATE_SLEEP),
> +			  !!(state & TUF_KBD_RGB_STATE_SHUTDOWN));
> +}
> +
>  static ssize_t kbd_rgb_state_store(struct device *dev,
>  				 struct device_attribute *attr,
>  				 const char *buf, size_t count)
> @@ -1166,7 +1245,7 @@ static ssize_t kbd_rgb_state_store(struct device *dev,
>  
>  	return count;
>  }
> -static DEVICE_ATTR_WO(kbd_rgb_state);
> +static DEVICE_ATTR_RW(kbd_rgb_state);
>  
>  static DEVICE_STRING_ATTR_RO(kbd_rgb_state_index, 0444,
>  			     "cmd boot awake sleep shutdown");
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 66c7ab35d6c8..393fd3c5949a 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -166,6 +166,9 @@
>  /* TUF laptop RGB power/state */
>  #define ASUS_WMI_DEVID_TUF_RGB_CMD	0x00100057
>  
> +/* TUF laptop RGB keyboard status readback */
> +#define ASUS_WMI_DEVID_TUF_RGB_READ_STATUS	0x0010005B
> +
>  /* Bootup sound control */
>  #define ASUS_WMI_DEVID_BOOT_SOUND	0x00130022
>  
> 
> 

Re: [PATCH v4 1/2] platform/x86: asus-wmi: add TUF keyboard RGB readback support

[Denis Benato]

On 11/9/26 19:02, Ilpo Järvinen wrote:
> On Thu, 29 Oct 2026, Bartu Alev wrote:
>
>> The TUF keyboard RGB attributes kbd_rgb_mode and kbd_rgb_state are
>> write-only, so userspace tools keep their own copy of the lighting
>> configuration and lose track of it whenever the EC changes state on its
>> own: after a firmware reset or a boot into another operating system the
>> tools show a stale configuration until the user rewrites every field.
>>
>> DSTS device 0x0010005B evaluates the DSDT method EC0.KBLS(), which
>> snapshots the EC's host-visible keyboard state into a 16-byte buffer:
>> lighting mode, red, green and blue levels, animation speed, and the
>> four power-state flags.  The first byte is a constant 1 written by the
>> AML, which distinguishes a KBLS reply from any other object the method
>> could return.
>>
>> Add kbd_rgb_read_status() to evaluate and validate that buffer, parse
>> it through a packed struct, and convert both attributes to
>> DEVICE_ATTR_RW.  The read returns the write format with "n/a" in the
>> command position: the command word belongs to the write verb (apply
>> now or save to flash) and both verbs update the EC mirror the same
>> way, so there is no state to report there.  The three EC speed codes
>> map back to the sysfs indices 0 to 2.
>>
>> Readback was verified against the EC firmware: the status bytes mirror
>> EC RAM at 0x0B22 to 0x0B2E, and the speed codes 0xe1, 0xeb and 0xf5
>> select animation step thresholds of 30, 20 and 10 ticks per phase,
>> which matches the slow, normal and fast labels.  Tested on a TUF
>> Gaming A15 (FA507NV).
>>
>> Suggested-by: Denis Benato <denis.benato@linux.dev>
>> Signed-off-by: Bartu Alev <bartualev@gmail.com>
>> ---
>>  drivers/platform/x86/asus-wmi.c            | 83 +++++++++++++++++++++++++++++-
>>  include/linux/platform_data/x86/asus-wmi.h |  3 ++
>>  2 files changed, 84 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
>> index b33ebdb21aa2..054d1d684bb0 100644
>> --- a/drivers/platform/x86/asus-wmi.c
>> +++ b/drivers/platform/x86/asus-wmi.c
>> @@ -1062,6 +1062,65 @@ static DEVICE_ATTR_RW(gpu_mux_mode);
>>  #define TUF_KBD_RGB_SPEED_SLOW		0xe1
>>  #define TUF_KBD_RGB_SPEED_NORMAL	0xeb
>>  #define TUF_KBD_RGB_SPEED_FAST		0xf5
>> +/* TUF keyboard RGB status readback *******************************************/
>> +struct asus_wmi_kbd_rgb_status {
>> +	u8 signature;
>> +	u8 mode;
>> +	u8 red;
>> +	u8 green;
>> +	u8 blue;
>> +	u8 speed;
>> +	u8 state;
>> +	u8 unused[9];
>> +} __packed;
>> +
>> +static int kbd_rgb_read_status(struct asus_wmi_kbd_rgb_status *status)
>> +{
>> +	int err;
>> +
>> +	err = asus_wmi_evaluate_method_buf(ASUS_WMI_METHODID_DSTS,
>> +					   ASUS_WMI_DEVID_TUF_RGB_READ_STATUS,
>> +					   0, (u8 *)status, sizeof(*status));
>> +	if (err)
>> +		return err < 0 ? err : -ENODEV;
> What do you (or Denis) think on sashiko's mention about 
> asus_wmi_evaluate_method_buf() casting integer.value to int which might 
> end up aliasing high-bit on return values with errnos?

Hi Ilpo,

I don't see the sashiko response neither in my mail nor in the lore,
but I think I understand the problem.

What I think is that we have been lucky.

ASUS probably chose the presence bit high enough not to cause
troubles when they have to report a status to armoury crate
and they don't use negative numbers, but yes I agree that the
return value is more to be seen as a bitfield (with some-bits
fields as numbers) rather than ints.

I have actually spotted this on a commit from Marco that was not
being sent on the lkml (because he touched that code) and I didn't
like what I saw (I didn't like what was already there).

Those commits were about modernizing the use of mutex
and __free where appropriate and I kinda liked them, will
ask him to revive it...

But proper and coherent error handling will touch basically
a large part of supported hardware and asus-wmi core
parts,

I am a bit hesitant to touch that code because I might break
some old and not widely known device: it's the only reason
I haven't (yet) acted in this regard.

> The other caller seems to also return that value directly out of _store 
> function().

The other caller got lucky exactly as the rest of driver.

setters in wmi don't even report the presence bit so there
is even less "incentive" to actually check it.

Re: [PATCH v4 1/2] platform/x86: asus-wmi: add TUF keyboard RGB readback support

[Ilpo Järvinen]

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

On Tue, 10 Nov 2026, Denis Benato wrote:
> On 11/9/26 19:02, Ilpo Järvinen wrote:
> > On Thu, 29 Oct 2026, Bartu Alev wrote:
> >
> >> The TUF keyboard RGB attributes kbd_rgb_mode and kbd_rgb_state are
> >> write-only, so userspace tools keep their own copy of the lighting
> >> configuration and lose track of it whenever the EC changes state on its
> >> own: after a firmware reset or a boot into another operating system the
> >> tools show a stale configuration until the user rewrites every field.
> >>
> >> DSTS device 0x0010005B evaluates the DSDT method EC0.KBLS(), which
> >> snapshots the EC's host-visible keyboard state into a 16-byte buffer:
> >> lighting mode, red, green and blue levels, animation speed, and the
> >> four power-state flags.  The first byte is a constant 1 written by the
> >> AML, which distinguishes a KBLS reply from any other object the method
> >> could return.
> >>
> >> Add kbd_rgb_read_status() to evaluate and validate that buffer, parse
> >> it through a packed struct, and convert both attributes to
> >> DEVICE_ATTR_RW.  The read returns the write format with "n/a" in the
> >> command position: the command word belongs to the write verb (apply
> >> now or save to flash) and both verbs update the EC mirror the same
> >> way, so there is no state to report there.  The three EC speed codes
> >> map back to the sysfs indices 0 to 2.
> >>
> >> Readback was verified against the EC firmware: the status bytes mirror
> >> EC RAM at 0x0B22 to 0x0B2E, and the speed codes 0xe1, 0xeb and 0xf5
> >> select animation step thresholds of 30, 20 and 10 ticks per phase,
> >> which matches the slow, normal and fast labels.  Tested on a TUF
> >> Gaming A15 (FA507NV).
> >>
> >> Suggested-by: Denis Benato <denis.benato@linux.dev>
> >> Signed-off-by: Bartu Alev <bartualev@gmail.com>
> >> ---
> >>  drivers/platform/x86/asus-wmi.c            | 83 +++++++++++++++++++++++++++++-
> >>  include/linux/platform_data/x86/asus-wmi.h |  3 ++
> >>  2 files changed, 84 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> >> index b33ebdb21aa2..054d1d684bb0 100644
> >> --- a/drivers/platform/x86/asus-wmi.c
> >> +++ b/drivers/platform/x86/asus-wmi.c
> >> @@ -1062,6 +1062,65 @@ static DEVICE_ATTR_RW(gpu_mux_mode);
> >>  #define TUF_KBD_RGB_SPEED_SLOW		0xe1
> >>  #define TUF_KBD_RGB_SPEED_NORMAL	0xeb
> >>  #define TUF_KBD_RGB_SPEED_FAST		0xf5
> >> +/* TUF keyboard RGB status readback *******************************************/
> >> +struct asus_wmi_kbd_rgb_status {
> >> +	u8 signature;
> >> +	u8 mode;
> >> +	u8 red;
> >> +	u8 green;
> >> +	u8 blue;
> >> +	u8 speed;
> >> +	u8 state;
> >> +	u8 unused[9];
> >> +} __packed;
> >> +
> >> +static int kbd_rgb_read_status(struct asus_wmi_kbd_rgb_status *status)
> >> +{
> >> +	int err;
> >> +
> >> +	err = asus_wmi_evaluate_method_buf(ASUS_WMI_METHODID_DSTS,
> >> +					   ASUS_WMI_DEVID_TUF_RGB_READ_STATUS,
> >> +					   0, (u8 *)status, sizeof(*status));
> >> +	if (err)
> >> +		return err < 0 ? err : -ENODEV;
> > What do you (or Denis) think on sashiko's mention about 
> > asus_wmi_evaluate_method_buf() casting integer.value to int which might 
> > end up aliasing high-bit on return values with errnos?
> 
> Hi Ilpo,
> 
> I don't see the sashiko response neither in my mail nor in the lore,
> but I think I understand the problem.

Hi,

https://sashiko.dev is the goto place where you have them for all public 
submissions covered.

It's intentional that we don't encumber (spam) our first time submitters 
with gazillions of pre-existing problems sashiko finds. Many of which are 
largely irrelevant/low value for platform drivers anyway (what user would 
remove a platform driver? or probe() rollback path issues; nice to fix 
those as well but nowhere near urgent or important level).

Sashiko has gotten somewhat better in not spamming the submitters with all 
such pre-existing issues but tries to filter only the ones somewhat 
relevant to the patch so I might reconsider that.

Sashiko's review emails together with using LLM to create and update 
patches also have a tendencity to cause undesirable rapid patch respinning 
cycle no human reviewer wants to see in their inboxes so it's a 
double-edged sword. I don't want to see v10 in a week or two from 
starting, it seems counterproductive as it's discouraging to human 
reviewers.

In this case you need to read between the lines as the report is for the 
Documentation patch, whereas I think it's an indication of a more generic 
problem in the code that isn't a mere documentation issue.

> What I think is that we have been lucky.
> 
> ASUS probably chose the presence bit high enough not to cause
> troubles when they have to report a status to armoury crate
> and they don't use negative numbers, but yes I agree that the
> return value is more to be seen as a bitfield (with some-bits
> fields as numbers) rather than ints.

So if there's only the FF..FE return with high value, it doesn't seem 
problem because it's explicilty checked for but I recall there was also 
FF..FF somewhere (no longer sure if I remember even what driver it was 
but the numbers seems suspiciously close to ones used here), it's not 
being checked in ..._buf() case and casting that to int makes it to alias 
with -EPERM.

> I have actually spotted this on a commit from Marco that was not
> being sent on the lkml (because he touched that code) and I didn't
> like what I saw (I didn't like what was already there).
> 
> Those commits were about modernizing the use of mutex
> and __free where appropriate and I kinda liked them, will
> ask him to revive it...
> 
> But proper and coherent error handling will touch basically
> a large part of supported hardware and asus-wmi core
> parts,
> 
> I am a bit hesitant to touch that code because I might break
> some old and not widely known device: it's the only reason
> I haven't (yet) acted in this regard.
> 
> > The other caller seems to also return that value directly out of _store 
> > function().
> 
> The other caller got lucky exactly as the rest of driver.
> 
> setters in wmi don't even report the presence bit so there
> is even less "incentive" to actually check it.
> 
> 

-- 
 i.