2022-08-03 23:21:39

by Luke Jones

[permalink] [raw]
Subject: [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB

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 <[email protected]>
---
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



2022-08-04 14:34:55

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB

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 <[email protected]>
> ---
> 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


2022-08-13 13:45:18

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB

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/[email protected]/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 <[email protected]>

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