[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <XMV7HR.FTECLY4PXEQX@ljones.dev>
Date: Fri, 26 Aug 2022 21:56:57 +1200
From: Luke Jones <luke@...nes.dev>
To: Hans de Goede <hdegoede@...hat.com>
Cc: markgross@...nel.org, platform-driver-x86@...r.kernel.org,
linux-kernel@...r.kernel.org, andy.shevchenko@...il.com,
pobrn@...tonmail.com, pavel@....cz
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@...hat.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@...nes.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
>
Powered by blists - more mailing lists