Re: [PATCH v4 1/2] asus-wmi: Implement TUF laptop keyboard LED modes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Fri, Aug 26 2022 at 11:47:37 +0200, Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxxx>
 ---
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






[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux