Messages in this thread Patch in this message | | | Date | Fri, 26 Aug 2022 21:56:57 +1200 | From | Luke Jones <> | Subject | Re: [PATCH v4 1/2] asus-wmi: Implement TUF laptop keyboard LED modes |
| |
On Fri, Aug 26 2022 at 11:47:37 +0200, Hans de Goede <hdegoede@redhat.com> wrote: > Hi Luke, > > On 8/26/22 01:22, Luke D. Jones wrote: >> Adds support for changing the laptop keyboard LED mode and colour. >> >> The modes are visible effects such as static, rainbow, pulsing, >> colour cycles. >> >> These sysfs attributes are added to asus::kbd_backlight: >> - kbd_rgb_mode >> - kbd_rgb_mode_index >> >> Signed-off-by: Luke D. Jones <luke@ljones.dev> >> --- >> drivers/platform/x86/asus-wmi.c | 76 >> +++++++++++++++++++++- >> include/linux/platform_data/x86/asus-wmi.h | 3 + >> 2 files changed, 78 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/platform/x86/asus-wmi.c >> b/drivers/platform/x86/asus-wmi.c >> index 0f9f79f249c7..92f16bb9b4ef 100644 >> --- a/drivers/platform/x86/asus-wmi.c >> +++ b/drivers/platform/x86/asus-wmi.c >> @@ -237,6 +237,8 @@ struct asus_wmi { >> bool dgpu_disable_available; >> bool gpu_mux_mode_available; >> >> + bool kbd_rgb_mode_available; >> + >> bool throttle_thermal_policy_available; >> u8 throttle_thermal_policy_mode; >> >> @@ -720,6 +722,69 @@ static ssize_t gpu_mux_mode_store(struct >> device *dev, >> } >> static DEVICE_ATTR_RW(gpu_mux_mode); >> >> +/* TUF Laptop Keyboard RGB Modes >> **********************************************/ >> +static ssize_t kbd_rgb_mode_store(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + u32 cmd, mode, r, g, b, speed; >> + int err; >> + >> + if (sscanf(buf, "%d %d %d %d %d %d", &cmd, &mode, &r, &g, &b, >> &speed) != 6) >> + return -EINVAL; >> + >> + cmd = !!cmd; >> + >> + /* These are the known usable modes across all TUF/ROG */ >> + if (mode >= 12 || mode == 9) >> + mode = 10; >> + >> + switch (speed) { >> + case 0: >> + speed = 0xe1; >> + break; >> + case 1: >> + speed = 0xeb; >> + break; >> + case 2: >> + speed = 0xf5; >> + break; >> + default: >> + speed = 0xeb; >> + } >> + >> + err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, >> ASUS_WMI_DEVID_TUF_RGB_MODE, >> + cmd | (mode << 8) | (r << 16) | (g << 24), b | (speed << 8), >> NULL); >> + if (err) >> + return err; >> + >> + return count; >> +} >> +static DEVICE_ATTR_WO(kbd_rgb_mode); >> + >> +static ssize_t kbd_rgb_mode_index_show(struct device *device, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + return sysfs_emit(buf, "%s\n", "cmd mode red green blue speed"); >> +} >> +static DEVICE_ATTR_RO(kbd_rgb_mode_index); >> + >> +static struct attribute *kbd_rgb_mode_attrs[] = { >> + &dev_attr_kbd_rgb_mode.attr, >> + &dev_attr_kbd_rgb_mode_index.attr, >> + NULL, >> +}; >> + >> +static const struct attribute_group kbd_rgb_mode_group = { >> + .attrs = kbd_rgb_mode_attrs, >> +}; >> + >> +const struct attribute_group *kbd_rgb_mode_groups[] = { >> + NULL, >> + NULL, >> +}; >> + >> /* Battery >> ********************************************************************/ >> >> /* The battery maximum charging percentage */ >> @@ -1038,7 +1103,10 @@ static void asus_wmi_led_exit(struct >> asus_wmi *asus) >> >> static int asus_wmi_led_init(struct asus_wmi *asus) >> { >> - int rv = 0, led_val; >> + int rv = 0, num_rgb_groups = 0, led_val; >> + >> + if (asus->kbd_rgb_mode_available) >> + kbd_rgb_mode_groups[num_rgb_groups++] = &kbd_rgb_mode_group; >> >> asus->led_workqueue = >> create_singlethread_workqueue("led_workqueue"); >> if (!asus->led_workqueue) >> @@ -1066,6 +1134,9 @@ static int asus_wmi_led_init(struct asus_wmi >> *asus) >> asus->kbd_led.brightness_get = kbd_led_get; >> asus->kbd_led.max_brightness = 3; >> >> + if (num_rgb_groups != 0) >> + asus->kbd_led.groups = kbd_rgb_mode_groups; >> + >> rv = led_classdev_register(&asus->platform_device->dev, >> &asus->kbd_led); >> if (rv) >> @@ -3253,6 +3324,8 @@ 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_dgpu_disable.attr) >> + ok = asus->dgpu_disable_available; >> else if (attr == &dev_attr_gpu_mux_mode.attr) >> ok = asus->gpu_mux_mode_available; >> else if (attr == &dev_attr_fan_boost_mode.attr) > > This patch-hunk looks like it is a (mangled) leftover from previous > versions > of the patch.
Do you mean:
+ else if (attr == &dev_attr_dgpu_disable.attr) + ok = asus->dgpu_disable_available;
> > Other then that this latest version of the series looks nice and > clean, > so I've added the series to my review-hans branch minus this > patch-hunk. > > One request can you do a follow up patch documenting the new > attributes in Documentation/ABI/testing/sysfs-platform-asus-wmi > I know the attributes sit under the LED class device, but this > still seems the best place for documenting them.
Yes for sure. I wasn't actually certain where I should put the doc.
> > Regards, > > Hans > > > > >> @@ -3519,6 +3592,7 @@ static int asus_wmi_add(struct >> platform_device *pdev) >> asus->egpu_enable_available = asus_wmi_dev_is_present(asus, >> ASUS_WMI_DEVID_EGPU); >> asus->dgpu_disable_available = asus_wmi_dev_is_present(asus, >> ASUS_WMI_DEVID_DGPU); >> asus->gpu_mux_mode_available = asus_wmi_dev_is_present(asus, >> ASUS_WMI_DEVID_GPU_MUX); >> + asus->kbd_rgb_mode_available = asus_wmi_dev_is_present(asus, >> ASUS_WMI_DEVID_TUF_RGB_MODE); >> asus->panel_overdrive_available = asus_wmi_dev_is_present(asus, >> ASUS_WMI_DEVID_PANEL_OD); >> >> err = fan_boost_mode_check_present(asus); >> diff --git a/include/linux/platform_data/x86/asus-wmi.h >> b/include/linux/platform_data/x86/asus-wmi.h >> index 6e8a95c10d17..3d861477cb20 100644 >> --- a/include/linux/platform_data/x86/asus-wmi.h >> +++ b/include/linux/platform_data/x86/asus-wmi.h >> @@ -103,6 +103,9 @@ >> /* gpu mux switch, 0 = dGPU, 1 = Optimus */ >> #define ASUS_WMI_DEVID_GPU_MUX 0x00090016 >> >> +/* TUF laptop RGB modes/colours */ >> +#define ASUS_WMI_DEVID_TUF_RGB_MODE 0x00100056 >> + >> /* DSTS masks */ >> #define ASUS_WMI_DSTS_STATUS_BIT 0x00000001 >> #define ASUS_WMI_DSTS_UNKNOWN_BIT 0x00000002 >
| |