Adds support for TUF laptop RGB control. This adds a multicolor LED
device, and two sysfs paths for extra feature control.

/sys/devices/platform/asus-nb-wmi/tuf_krgb_mode_index provides
labels for the index fields as "save mode speed"

/sys/devices/platform/asus-nb-wmi/tuf_krgb_mode has the following
as input options via U8 "n n n":
- Save or set, if set, then settings revert on cold boot
- Mode, 0 = Static, 1 = Breathe, 2 = Colour cycle, 3 = Pulse
- Speed, 0 = Slow, 1 = Medium, 2 = Fast

Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
 drivers/platform/x86/asus-wmi.c            | 213 +++++++++++++++++++++
 include/linux/platform_data/x86/asus-wmi.h |   3 +
 2 files changed, 216 insertions(+)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 0e7fbed8a50d..2959f17047a8 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -25,6 +25,7 @@
 #include <linux/input/sparse-keymap.h>
 #include <linux/kernel.h>
 #include <linux/leds.h>
+#include <linux/led-class-multicolor.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/pci_hotplug.h>
@@ -117,6 +118,9 @@ static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
 
 static int throttle_thermal_policy_write(struct asus_wmi *);
 
+static int tuf_rgb_brightness_set(struct led_classdev *cdev,
+							enum led_brightness brightness);
+
 static bool ashs_present(void)
 {
 	int i = 0;
@@ -190,6 +194,14 @@ struct fan_curve_data {
 	u8 percents[FAN_CURVE_POINTS];
 };
 
+struct tuf_rgb_led {
+	struct led_classdev_mc dev;
+	struct mc_subled subled_info[3]; /* r g b */
+	u8 save;
+	u8 mode;
+	u8 speed;
+};
+
 struct asus_wmi {
 	int dsts_id;
 	int spec;
@@ -234,6 +246,9 @@ struct asus_wmi {
 	bool dgpu_disable_available;
 	bool dgpu_disable;
 
+	bool tuf_krgb_mode_available;
+	struct tuf_rgb_led tuf_krgb_mode;
+
 	bool throttle_thermal_policy_available;
 	u8 throttle_thermal_policy_mode;
 
@@ -734,6 +749,116 @@ static ssize_t egpu_enable_store(struct device *dev,
 
 static DEVICE_ATTR_RW(egpu_enable);
 
+/* TUF Laptop Keyboard RGB Modes **********************************************/
+static int tuf_krgb_mode_check_present(struct asus_wmi *asus)
+{
+	u32 result;
+	int err;
+
+	asus->tuf_krgb_mode_available = false;
+
+	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_MODE, &result);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		return err;
+	}
+
+	if (result & ASUS_WMI_DSTS_PRESENCE_BIT) {
+		asus->tuf_krgb_mode_available = true;
+		/* set some sane defaults since we can't read this from WMI */
+		asus->tuf_krgb_mode.save = 1;
+		asus->tuf_krgb_mode.mode = 0;
+		asus->tuf_krgb_mode.speed = 1;
+	}
+
+	return 0;
+}
+
+static ssize_t tuf_krgb_mode_store(struct device *device,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	char *data, *part, *end;
+	u8 res, tmp, arg_num;
+	int err;
+
+	struct asus_wmi *asus = dev_get_drvdata(device);
+	struct led_classdev *cdev = &asus->tuf_krgb_mode.dev.led_cdev;
+
+	data = end = kstrdup(buf, GFP_KERNEL);
+	arg_num = 0;
+
+	while ((part = strsep(&end, " ")) != NULL) {
+		if (part == NULL)
+			return -1;
+
+		res = kstrtou8(part, 10, &tmp);
+		if (res)
+			return -1;
+
+		if (arg_num == 0)
+			asus->tuf_krgb_mode.save = tmp;
+		else if (arg_num == 1)
+			/* These are the known usable modes across all TUF/ROG */
+			asus->tuf_krgb_mode.mode = tmp < 12 && tmp != 9 ? tmp : 0x0a;
+		else if (arg_num == 2) {
+			if (tmp == 0)
+				asus->tuf_krgb_mode.speed = 0xe1;
+			else if (tmp == 1)
+				asus->tuf_krgb_mode.speed = 0xeb;
+			else if (tmp == 2)
+				asus->tuf_krgb_mode.speed = 0xf5;
+			else
+				asus->tuf_krgb_mode.speed = 0xeb;
+		}
+
+		arg_num += 1;
+	}
+
+	err = tuf_rgb_brightness_set(cdev, cdev->brightness);
+	if (err)
+		return err;
+	return 0;
+}
+
+static ssize_t tuf_krgb_mode_show(struct device *device,
+						 struct device_attribute *attr,
+						 char *buf)
+{
+	struct asus_wmi *asus = dev_get_drvdata(device);
+	u8 speed = asus->tuf_krgb_mode.speed;
+	int len;
+
+	if (speed == 0xe1)
+		speed = 0;
+	else if (speed == 0xeb)
+		speed = 1;
+	else if (speed == 0xf5)
+		speed = 2;
+	else
+		speed = 1;
+
+	len = sprintf(buf, "%d %d %d",
+						asus->tuf_krgb_mode.save,
+						asus->tuf_krgb_mode.mode,
+						speed);
+
+	return len;
+}
+
+static DEVICE_ATTR_RW(tuf_krgb_mode);
+
+static ssize_t tuf_krgb_mode_index_show(struct device *device,
+						 struct device_attribute *attr,
+						 char *buf)
+{
+	int len = sprintf(buf, "%s", "save mode speed\n");
+	return len;
+}
+
+static DEVICE_ATTR_RO(tuf_krgb_mode_index);
+
 /* Battery ********************************************************************/
 
 /* The battery maximum charging percentage */
@@ -1028,6 +1153,38 @@ static enum led_brightness lightbar_led_get(struct led_classdev *led_cdev)
 	return result & ASUS_WMI_DSTS_LIGHTBAR_MASK;
 }
 
+static int tuf_rgb_brightness_set(struct led_classdev *cdev,
+	enum led_brightness brightness)
+{
+	u8 r, g, b, mode, speed, save;
+	int err;
+	u32 ret;
+	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev);
+	struct asus_wmi *asus = container_of(mc_cdev, struct asus_wmi, tuf_krgb_mode.dev);
+
+	led_mc_calc_color_components(mc_cdev, brightness);
+	r = mc_cdev->subled_info[0].brightness;
+	g = mc_cdev->subled_info[1].brightness;
+	b = mc_cdev->subled_info[2].brightness;
+	/* 0 still sets the mode/rgb, but does not stick on reboot */
+	save = asus->tuf_krgb_mode.save == 1 ? 0xb5 : 0xb4;
+	mode = asus->tuf_krgb_mode.mode;
+	speed = asus->tuf_krgb_mode.speed;
+
+	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, ASUS_WMI_DEVID_TUF_RGB_MODE,
+			save | (mode << 8) | (r << 16) | (g << 24), (b) | (speed << 8), &ret);
+	if (err) {
+		pr_err("Unable to set TUF RGB data?\n");
+		return err;
+	}
+	return 0;
+}
+
+static enum led_brightness tuf_rgb_brightness_get(struct led_classdev *cdev)
+{
+	return cdev->brightness;
+}
+
 static void asus_wmi_led_exit(struct asus_wmi *asus)
 {
 	led_classdev_unregister(&asus->kbd_led);
@@ -1105,6 +1262,51 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
 					   &asus->lightbar_led);
 	}
 
+	if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_MODE)) {
+		struct led_classdev_mc *mc_cdev;
+		struct mc_subled *mc_led_info;
+		u8 brightness = 127;
+
+		mc_cdev = &asus->tuf_krgb_mode.dev;
+
+		mc_cdev->led_cdev.name = "asus::multicolour";
+		mc_cdev->led_cdev.flags = LED_CORE_SUSPENDRESUME | LED_RETAIN_AT_SHUTDOWN;
+		mc_cdev->led_cdev.brightness_set_blocking = tuf_rgb_brightness_set;
+		mc_cdev->led_cdev.brightness_get = tuf_rgb_brightness_get;
+
+		/* Let the multicolour LED own the info */
+		mc_led_info = devm_kmalloc_array(
+			&asus->platform_device->dev,
+			3,
+			sizeof(*mc_led_info),
+			GFP_KERNEL | __GFP_ZERO);
+
+		if (!mc_led_info)
+			return -ENOMEM;
+
+		mc_led_info[0].color_index = LED_COLOR_ID_RED;
+		mc_led_info[1].color_index = LED_COLOR_ID_GREEN;
+		mc_led_info[2].color_index = LED_COLOR_ID_BLUE;
+
+		/* It's not possible to get last set data from device so set defaults */
+		asus->tuf_krgb_mode.save = 1;
+		asus->tuf_krgb_mode.mode = 0;
+		asus->tuf_krgb_mode.speed = 1;
+		mc_cdev->led_cdev.brightness = brightness;
+		mc_cdev->led_cdev.max_brightness = brightness;
+		mc_led_info[0].intensity = brightness;
+		mc_led_info[0].brightness = mc_cdev->led_cdev.brightness;
+		mc_led_info[1].brightness = mc_cdev->led_cdev.brightness;
+		mc_led_info[2].brightness = mc_cdev->led_cdev.brightness;
+		led_mc_calc_color_components(mc_cdev, brightness);
+
+		mc_cdev->subled_info = mc_led_info;
+		mc_cdev->num_colors = 3;
+
+		tuf_rgb_brightness_set(&mc_cdev->led_cdev, brightness);
+		rv = led_classdev_multicolor_register(&asus->platform_device->dev, mc_cdev);
+	}
+
 error:
 	if (rv)
 		asus_wmi_led_exit(asus);
@@ -3258,6 +3460,8 @@ static struct attribute *platform_attributes[] = {
 	&dev_attr_touchpad.attr,
 	&dev_attr_egpu_enable.attr,
 	&dev_attr_dgpu_disable.attr,
+	&dev_attr_tuf_krgb_mode.attr,
+	&dev_attr_tuf_krgb_mode_index.attr,
 	&dev_attr_lid_resume.attr,
 	&dev_attr_als_enable.attr,
 	&dev_attr_fan_boost_mode.attr,
@@ -3288,6 +3492,10 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
 		ok = asus->egpu_enable_available;
 	else if (attr == &dev_attr_dgpu_disable.attr)
 		ok = asus->dgpu_disable_available;
+	else if (attr == &dev_attr_tuf_krgb_mode.attr)
+		ok = asus->tuf_krgb_mode_available;
+	else if (attr == &dev_attr_tuf_krgb_mode_index.attr)
+		ok = asus->tuf_krgb_mode_available;
 	else if (attr == &dev_attr_fan_boost_mode.attr)
 		ok = asus->fan_boost_mode_available;
 	else if (attr == &dev_attr_throttle_thermal_policy.attr)
@@ -3557,6 +3765,10 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (err)
 		goto fail_dgpu_disable;
 
+	err = tuf_krgb_mode_check_present(asus);
+	if (err)
+		goto fail_tuf_krgb_mode;
+
 	err = fan_boost_mode_check_present(asus);
 	if (err)
 		goto fail_fan_boost_mode;
@@ -3671,6 +3883,7 @@ static int asus_wmi_add(struct platform_device *pdev)
 fail_fan_boost_mode:
 fail_egpu_enable:
 fail_dgpu_disable:
+fail_tuf_krgb_mode:
 fail_platform:
 fail_panel_od:
 	kfree(asus);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index a571b47ff362..5049c153a3fe 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -98,6 +98,9 @@
 /* dgpu on/off */
 #define ASUS_WMI_DEVID_DGPU		0x00090020
 
+/* TUF laptop RGB modes */
+#define ASUS_WMI_DEVID_TUF_RGB_MODE	0x00100056
+
 /* DSTS masks */
 #define ASUS_WMI_DSTS_STATUS_BIT	0x00000001
 #define ASUS_WMI_DSTS_UNKNOWN_BIT	0x00000002
-- 
2.37.1

Hi "Luke,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.19]
[cannot apply to next-20220812]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Luke-D-Jones/asus-wmi-Add-support-for-TUF-laptop-keyboard-RGB/20220804-071716
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 526942b8134cc34d25d27f95dfff98b8ce2f6fcd
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20220813/202208132140.7Y3f2Xhn-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/e25083de01260bb6c1b3070f7f3e6915de07c44c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Luke-D-Jones/asus-wmi-Add-support-for-TUF-laptop-keyboard-RGB/20220804-071716
        git checkout e25083de01260bb6c1b3070f7f3e6915de07c44c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/platform/x86/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/platform/x86/asus-wmi.c: In function 'tuf_krgb_mode_store':
>> drivers/platform/x86/asus-wmi.c:782:15: warning: variable 'data' set but not used [-Wunused-but-set-variable]
     782 |         char *data, *part, *end;
         |               ^~~~


vim +/data +782 drivers/platform/x86/asus-wmi.c

   777	
   778	static ssize_t tuf_krgb_mode_store(struct device *device,
   779					 struct device_attribute *attr,
   780					 const char *buf, size_t count)
   781	{
 > 782		char *data, *part, *end;
   783		u8 res, tmp, arg_num;
   784		int err;
   785	
   786		struct asus_wmi *asus = dev_get_drvdata(device);
   787		struct led_classdev *cdev = &asus->tuf_krgb_mode.dev.led_cdev;
   788	
   789		data = end = kstrdup(buf, GFP_KERNEL);
   790		arg_num = 0;
   791	
   792		while ((part = strsep(&end, " ")) != NULL) {
   793			if (part == NULL)
   794				return -1;
   795	
   796			res = kstrtou8(part, 10, &tmp);
   797			if (res)
   798				return -1;
   799	
   800			if (arg_num == 0)
   801				asus->tuf_krgb_mode.save = tmp;
   802			else if (arg_num == 1)
   803				/* These are the known usable modes across all TUF/ROG */
   804				asus->tuf_krgb_mode.mode = tmp < 12 && tmp != 9 ? tmp : 0x0a;
   805			else if (arg_num == 2) {
   806				if (tmp == 0)
   807					asus->tuf_krgb_mode.speed = 0xe1;
   808				else if (tmp == 1)
   809					asus->tuf_krgb_mode.speed = 0xeb;
   810				else if (tmp == 2)
   811					asus->tuf_krgb_mode.speed = 0xf5;
   812				else
   813					asus->tuf_krgb_mode.speed = 0xeb;
   814			}
   815	
   816			arg_num += 1;
   817		}
   818	
   819		err = tuf_rgb_brightness_set(cdev, cdev->brightness);
   820		if (err)
   821			return err;
   822		return 0;
   823	}
   824	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Hi,

On 8/4/22 01:16, Luke D. Jones wrote:
> Adds support for TUF laptop RGB control. This adds a multicolor LED
> device, and two sysfs paths for extra feature control.
> 
> /sys/devices/platform/asus-nb-wmi/tuf_krgb_mode_index provides
> labels for the index fields as "save mode speed"

As mentioned in my review of "[PATCH v2 1/1] asus-wmi: Add support
for TUF laptop keyboard states" the new tuf_krgb_mode attribute
should be an extra attribute under the led_class_device, you can do
this by adding this attribute to a separate attribute_group,
lets say e.g. "tuf_rgb_attributes" and then in the code of this
patch add:

	mc_cdev->led_cdev.groups = tuf_rgb_attributes;

and then the "tuf_krgb_mode" file should show up as:
/sys/class/leds/asus::multicolour/tuf_krgb_mode

Also again please drop the tuf_krgb_mode_index file and document
things in Documentation/ABI/testing/sysfs-platform-asus-wmi.

I've not done a detailed review of this yet, but overall this looks
good, definitely moving in the right direction.

My only other remark is that the led_class_device name should be
something like: "asus_wmi::kbd_backlight".

For easier reviewing of the next version, please split this
into 3 patches:

1. Add just the multi color led_class_dev
2. Add tuf_krgb_state attribute under the led_class_dev
3. Add tuf_krgb_mode attribute under the led_class_dev

Also see some further comments inline / below.


> /sys/devices/platform/asus-nb-wmi/tuf_krgb_mode has the following
> as input options via U8 "n n n":
> - Save or set, if set, then settings revert on cold boot
> - Mode, 0 = Static, 1 = Breathe, 2 = Colour cycle, 3 = Pulse
> - Speed, 0 = Slow, 1 = Medium, 2 = Fast
> 
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
>  drivers/platform/x86/asus-wmi.c            | 213 +++++++++++++++++++++
>  include/linux/platform_data/x86/asus-wmi.h |   3 +
>  2 files changed, 216 insertions(+)
> 
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 0e7fbed8a50d..2959f17047a8 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -25,6 +25,7 @@
>  #include <linux/input/sparse-keymap.h>
>  #include <linux/kernel.h>
>  #include <linux/leds.h>
> +#include <linux/led-class-multicolor.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
>  #include <linux/pci_hotplug.h>
> @@ -117,6 +118,9 @@ static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
>  
>  static int throttle_thermal_policy_write(struct asus_wmi *);
>  
> +static int tuf_rgb_brightness_set(struct led_classdev *cdev,
> +							enum led_brightness brightness);
> +
>  static bool ashs_present(void)
>  {
>  	int i = 0;
> @@ -190,6 +194,14 @@ struct fan_curve_data {
>  	u8 percents[FAN_CURVE_POINTS];
>  };
>  
> +struct tuf_rgb_led {
> +	struct led_classdev_mc dev;
> +	struct mc_subled subled_info[3]; /* r g b */
> +	u8 save;
> +	u8 mode;
> +	u8 speed;
> +};
> +
>  struct asus_wmi {
>  	int dsts_id;
>  	int spec;
> @@ -234,6 +246,9 @@ struct asus_wmi {
>  	bool dgpu_disable_available;
>  	bool dgpu_disable;
>  
> +	bool tuf_krgb_mode_available;
> +	struct tuf_rgb_led tuf_krgb_mode;
> +
>  	bool throttle_thermal_policy_available;
>  	u8 throttle_thermal_policy_mode;
>  
> @@ -734,6 +749,116 @@ static ssize_t egpu_enable_store(struct device *dev,
>  
>  static DEVICE_ATTR_RW(egpu_enable);
>  
> +/* TUF Laptop Keyboard RGB Modes **********************************************/
> +static int tuf_krgb_mode_check_present(struct asus_wmi *asus)
> +{
> +	u32 result;
> +	int err;
> +
> +	asus->tuf_krgb_mode_available = false;
> +
> +	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_MODE, &result);
> +	if (err) {
> +		if (err == -ENODEV)
> +			return 0;
> +		return err;
> +	}
> +
> +	if (result & ASUS_WMI_DSTS_PRESENCE_BIT) {
> +		asus->tuf_krgb_mode_available = true;
> +		/* set some sane defaults since we can't read this from WMI */
> +		asus->tuf_krgb_mode.save = 1;
> +		asus->tuf_krgb_mode.mode = 0;
> +		asus->tuf_krgb_mode.speed = 1;

Why not just make tuf_krgb_mode write-only like you have done for tuf_krgb_state ?

> +	}
> +
> +	return 0;
> +}
> +
> +static ssize_t tuf_krgb_mode_store(struct device *device,
> +				 struct device_attribute *attr,
> +				 const char *buf, size_t count)
> +{
> +	char *data, *part, *end;
> +	u8 res, tmp, arg_num;
> +	int err;
> +
> +	struct asus_wmi *asus = dev_get_drvdata(device);
> +	struct led_classdev *cdev = &asus->tuf_krgb_mode.dev.led_cdev;
> +
> +	data = end = kstrdup(buf, GFP_KERNEL);
> +	arg_num = 0;
> +
> +	while ((part = strsep(&end, " ")) != NULL) {
> +		if (part == NULL)
> +			return -1;

return -EINVAL please.

> +
> +		res = kstrtou8(part, 10, &tmp);
> +		if (res)
> +			return -1;

return -EINVAL please.

> +
> +		if (arg_num == 0)
> +			asus->tuf_krgb_mode.save = tmp;
> +		else if (arg_num == 1)
> +			/* These are the known usable modes across all TUF/ROG */
> +			asus->tuf_krgb_mode.mode = tmp < 12 && tmp != 9 ? tmp : 0x0a;
> +		else if (arg_num == 2) {
> +			if (tmp == 0)
> +				asus->tuf_krgb_mode.speed = 0xe1;
> +			else if (tmp == 1)
> +				asus->tuf_krgb_mode.speed = 0xeb;
> +			else if (tmp == 2)
> +				asus->tuf_krgb_mode.speed = 0xf5;
> +			else
> +				asus->tuf_krgb_mode.speed = 0xeb;
> +		}
> +
> +		arg_num += 1;
> +	}

Maybe just replace the kstrdup + the entire while loop with:

	int a, b, c;

	if (sscanf(buf, "%d %d %d", &a, &b, &c) != 3)
		return -EINVAL;

	asus->tuf_krgb_mode.save = a;
	asus->tuf_krgb_mode.mode = b < 12 && b != 9 ? b : 0x0a;

	if (c == 0)
		asus->tuf_krgb_mode.speed = 0xe1;
	else if (c == 1)
		asus->tuf_krgb_mode.speed = 0xeb;
	else if (c == 2)
		asus->tuf_krgb_mode.speed = 0xf5;
	else
		asus->tuf_krgb_mode.speed = 0xeb;
	
That certainly seems a lot cleaner to me ?

And perhaps you can do something similar for
tuf_krgb_state_store  ?



> +
> +	err = tuf_rgb_brightness_set(cdev, cdev->brightness);
> +	if (err)
> +		return err;
> +	return 0;
> +}
> +
> +static ssize_t tuf_krgb_mode_show(struct device *device,
> +						 struct device_attribute *attr,
> +						 char *buf)
> +{
> +	struct asus_wmi *asus = dev_get_drvdata(device);
> +	u8 speed = asus->tuf_krgb_mode.speed;
> +	int len;
> +
> +	if (speed == 0xe1)
> +		speed = 0;
> +	else if (speed == 0xeb)
> +		speed = 1;
> +	else if (speed == 0xf5)
> +		speed = 2;
> +	else
> +		speed = 1;
> +
> +	len = sprintf(buf, "%d %d %d",
> +						asus->tuf_krgb_mode.save,
> +						asus->tuf_krgb_mode.mode,
> +						speed);
> +
> +	return len;
> +}
> +
> +static DEVICE_ATTR_RW(tuf_krgb_mode);

As mentioned above why not just make this write-only
like you have done for tuf_krgb_state ?

> +
> +static ssize_t tuf_krgb_mode_index_show(struct device *device,
> +						 struct device_attribute *attr,
> +						 char *buf)
> +{
> +	int len = sprintf(buf, "%s", "save mode speed\n");
> +	return len;
> +}
> +
> +static DEVICE_ATTR_RO(tuf_krgb_mode_index);
> +
>  /* Battery ********************************************************************/
>  
>  /* The battery maximum charging percentage */
> @@ -1028,6 +1153,38 @@ static enum led_brightness lightbar_led_get(struct led_classdev *led_cdev)
>  	return result & ASUS_WMI_DSTS_LIGHTBAR_MASK;
>  }
>  
> +static int tuf_rgb_brightness_set(struct led_classdev *cdev,
> +	enum led_brightness brightness)
> +{
> +	u8 r, g, b, mode, speed, save;
> +	int err;
> +	u32 ret;
> +	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev);
> +	struct asus_wmi *asus = container_of(mc_cdev, struct asus_wmi, tuf_krgb_mode.dev);
> +
> +	led_mc_calc_color_components(mc_cdev, brightness);
> +	r = mc_cdev->subled_info[0].brightness;
> +	g = mc_cdev->subled_info[1].brightness;
> +	b = mc_cdev->subled_info[2].brightness;
> +	/* 0 still sets the mode/rgb, but does not stick on reboot */
> +	save = asus->tuf_krgb_mode.save == 1 ? 0xb5 : 0xb4;
> +	mode = asus->tuf_krgb_mode.mode;
> +	speed = asus->tuf_krgb_mode.speed;
> +
> +	err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, ASUS_WMI_DEVID_TUF_RGB_MODE,
> +			save | (mode << 8) | (r << 16) | (g << 24), (b) | (speed << 8), &ret);
> +	if (err) {
> +		pr_err("Unable to set TUF RGB data?\n");
> +		return err;
> +	}
> +	return 0;
> +}
> +
> +static enum led_brightness tuf_rgb_brightness_get(struct led_classdev *cdev)
> +{
> +	return cdev->brightness;
> +}
> +
>  static void asus_wmi_led_exit(struct asus_wmi *asus)
>  {
>  	led_classdev_unregister(&asus->kbd_led);
> @@ -1105,6 +1262,51 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
>  					   &asus->lightbar_led);
>  	}
>  
> +	if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_MODE)) {
> +		struct led_classdev_mc *mc_cdev;
> +		struct mc_subled *mc_led_info;
> +		u8 brightness = 127;
> +
> +		mc_cdev = &asus->tuf_krgb_mode.dev;
> +
> +		mc_cdev->led_cdev.name = "asus::multicolour";
> +		mc_cdev->led_cdev.flags = LED_CORE_SUSPENDRESUME | LED_RETAIN_AT_SHUTDOWN;
> +		mc_cdev->led_cdev.brightness_set_blocking = tuf_rgb_brightness_set;
> +		mc_cdev->led_cdev.brightness_get = tuf_rgb_brightness_get;
> +
> +		/* Let the multicolour LED own the info */
> +		mc_led_info = devm_kmalloc_array(
> +			&asus->platform_device->dev,
> +			3,
> +			sizeof(*mc_led_info),
> +			GFP_KERNEL | __GFP_ZERO);
> +
> +		if (!mc_led_info)
> +			return -ENOMEM;
> +
> +		mc_led_info[0].color_index = LED_COLOR_ID_RED;
> +		mc_led_info[1].color_index = LED_COLOR_ID_GREEN;
> +		mc_led_info[2].color_index = LED_COLOR_ID_BLUE;
> +
> +		/* It's not possible to get last set data from device so set defaults */
> +		asus->tuf_krgb_mode.save = 1;
> +		asus->tuf_krgb_mode.mode = 0;
> +		asus->tuf_krgb_mode.speed = 1;
> +		mc_cdev->led_cdev.brightness = brightness;
> +		mc_cdev->led_cdev.max_brightness = brightness;
> +		mc_led_info[0].intensity = brightness;
> +		mc_led_info[0].brightness = mc_cdev->led_cdev.brightness;
> +		mc_led_info[1].brightness = mc_cdev->led_cdev.brightness;
> +		mc_led_info[2].brightness = mc_cdev->led_cdev.brightness;
> +		led_mc_calc_color_components(mc_cdev, brightness);
> +
> +		mc_cdev->subled_info = mc_led_info;
> +		mc_cdev->num_colors = 3;
> +
> +		tuf_rgb_brightness_set(&mc_cdev->led_cdev, brightness);
> +		rv = led_classdev_multicolor_register(&asus->platform_device->dev, mc_cdev);
> +	}
> +
>  error:
>  	if (rv)
>  		asus_wmi_led_exit(asus);
> @@ -3258,6 +3460,8 @@ static struct attribute *platform_attributes[] = {
>  	&dev_attr_touchpad.attr,
>  	&dev_attr_egpu_enable.attr,
>  	&dev_attr_dgpu_disable.attr,
> +	&dev_attr_tuf_krgb_mode.attr,
> +	&dev_attr_tuf_krgb_mode_index.attr,
>  	&dev_attr_lid_resume.attr,
>  	&dev_attr_als_enable.attr,
>  	&dev_attr_fan_boost_mode.attr,
> @@ -3288,6 +3492,10 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
>  		ok = asus->egpu_enable_available;
>  	else if (attr == &dev_attr_dgpu_disable.attr)
>  		ok = asus->dgpu_disable_available;
> +	else if (attr == &dev_attr_tuf_krgb_mode.attr)
> +		ok = asus->tuf_krgb_mode_available;
> +	else if (attr == &dev_attr_tuf_krgb_mode_index.attr)
> +		ok = asus->tuf_krgb_mode_available;
>  	else if (attr == &dev_attr_fan_boost_mode.attr)
>  		ok = asus->fan_boost_mode_available;
>  	else if (attr == &dev_attr_throttle_thermal_policy.attr)
> @@ -3557,6 +3765,10 @@ static int asus_wmi_add(struct platform_device *pdev)
>  	if (err)
>  		goto fail_dgpu_disable;
>  
> +	err = tuf_krgb_mode_check_present(asus);
> +	if (err)
> +		goto fail_tuf_krgb_mode;
> +
>  	err = fan_boost_mode_check_present(asus);
>  	if (err)
>  		goto fail_fan_boost_mode;
> @@ -3671,6 +3883,7 @@ static int asus_wmi_add(struct platform_device *pdev)
>  fail_fan_boost_mode:
>  fail_egpu_enable:
>  fail_dgpu_disable:
> +fail_tuf_krgb_mode:
>  fail_platform:
>  fail_panel_od:
>  	kfree(asus);
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index a571b47ff362..5049c153a3fe 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -98,6 +98,9 @@
>  /* dgpu on/off */
>  #define ASUS_WMI_DEVID_DGPU		0x00090020
>  
> +/* TUF laptop RGB modes */
> +#define ASUS_WMI_DEVID_TUF_RGB_MODE	0x00100056
> +
>  /* DSTS masks */
>  #define ASUS_WMI_DSTS_STATUS_BIT	0x00000001
>  #define ASUS_WMI_DSTS_UNKNOWN_BIT	0x00000002



Regards,

Hans