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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

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
>  
> 
>