[PATCH 0/2] platform/x86: asus-wmi: support a couple Zenbook 2023 features

[PATCH 0/2] platform/x86: asus-wmi: support a couple Zenbook 2023 features

[Devin Bayer]

This patch series adds support for a couple features on
my Zenbook UX3404VC:

1. the LED on F11 "disable camera" key
2. fan speed control

Devin Bayer (2):
  platform/x86: asus-wmi: support camera disable LED
  platform/x86: asus-wmi: support newer fan_boost_mode dev_id

 drivers/platform/x86/asus-wmi.c            | 121 ++++++++++++++++++++-
 include/linux/platform_data/x86/asus-wmi.h |   3 +
 2 files changed, 119 insertions(+), 5 deletions(-)

-- 
2.45.2

[PATCH 1/2] platform/x86: asus-wmi: support camera disable LED

[Devin Bayer]

Support the LED on F10 above the crossed out camera icon.

Signed-off-by: Devin Bayer <dev@doubly.so>
---
 drivers/platform/x86/asus-wmi.c            | 36 ++++++++++++++++++++++
 include/linux/platform_data/x86/asus-wmi.h |  2 ++
 2 files changed, 38 insertions(+)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 3f9b6285c9a6..5585f15e7920 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -73,6 +73,7 @@ module_param(fnlock_default, bool, 0444);
 #define NOTIFY_LID_FLIP_ROG		0xbd
 
 #define ASUS_WMI_FNLOCK_BIOS_DISABLED	BIT(0)
+#define ASUS_WMI_DEVID_CAMERA_LED		0x00060079
 
 #define ASUS_MID_FAN_DESC		"mid_fan"
 #define ASUS_GPU_FAN_DESC		"gpu_fan"
@@ -238,6 +239,7 @@ struct asus_wmi {
 	struct led_classdev lightbar_led;
 	int lightbar_led_wk;
 	struct led_classdev micmute_led;
+	struct led_classdev camera_led;
 	struct workqueue_struct *led_workqueue;
 	struct work_struct tpd_led_work;
 	struct work_struct wlan_led_work;
@@ -1642,6 +1644,27 @@ static int micmute_led_set(struct led_classdev *led_cdev,
 	return err < 0 ? err : 0;
 }
 
+static enum led_brightness camera_led_get(struct led_classdev *led_cdev)
+{
+	struct asus_wmi *asus;
+	u32 result;
+
+	asus = container_of(led_cdev, struct asus_wmi, camera_led);
+	asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_CAMERA_LED, &result);
+
+	return result & ASUS_WMI_DSTS_BRIGHTNESS_MASK;
+}
+
+static int camera_led_set(struct led_classdev *led_cdev,
+			   enum led_brightness brightness)
+{
+	int state = brightness != LED_OFF;
+	int err;
+
+	err = asus_wmi_set_devstate(ASUS_WMI_DEVID_CAMERA_LED, state, NULL);
+	return err < 0 ? err : 0;
+}
+
 static void asus_wmi_led_exit(struct asus_wmi *asus)
 {
 	led_classdev_unregister(&asus->kbd_led);
@@ -1649,6 +1672,7 @@ static void asus_wmi_led_exit(struct asus_wmi *asus)
 	led_classdev_unregister(&asus->wlan_led);
 	led_classdev_unregister(&asus->lightbar_led);
 	led_classdev_unregister(&asus->micmute_led);
+	led_classdev_unregister(&asus->camera_led);
 
 	if (asus->led_workqueue)
 		destroy_workqueue(asus->led_workqueue);
@@ -1740,6 +1764,18 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
 			goto error;
 	}
 
+	if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_CAMERA_LED)) {
+		asus->camera_led.name = "platform::camera";
+		asus->camera_led.max_brightness = 1;
+		asus->camera_led.brightness_get = camera_led_get;
+		asus->camera_led.brightness_set_blocking = camera_led_set;
+
+		rv = led_classdev_register(&asus->platform_device->dev,
+						&asus->camera_led);
+		if (rv)
+			goto error;
+	}
+
 error:
 	if (rv)
 		asus_wmi_led_exit(asus);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 3eb5cd6773ad..b3c35e33f1e7 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -50,6 +50,8 @@
 #define ASUS_WMI_DEVID_LED5		0x00020015
 #define ASUS_WMI_DEVID_LED6		0x00020016
 #define ASUS_WMI_DEVID_MICMUTE_LED		0x00040017
+#define ASUS_WMI_DEVID_CAMERA_LED_NEG		0x00060078
+#define ASUS_WMI_DEVID_CAMERA_LED		0x00060079
 
 /* Backlight and Brightness */
 #define ASUS_WMI_DEVID_ALS_ENABLE	0x00050001 /* Ambient Light Sensor */
-- 
2.45.2

[PATCH 2/2] platform/x86: asus-wmi: support newer fan_boost_mode dev_id

[Devin Bayer]

Support changing the fan mode (silent, performance, standard). I reused
the existing fan_boost_mode sysfs entry.

Signed-off-by: Devin Bayer <dev@doubly.so>
---
 drivers/platform/x86/asus-wmi.c            | 87 ++++++++++++++++++++--
 include/linux/platform_data/x86/asus-wmi.h |  1 +
 2 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 5585f15e7920..e27b8f86d57b 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -73,7 +73,6 @@ module_param(fnlock_default, bool, 0444);
 #define NOTIFY_LID_FLIP_ROG		0xbd
 
 #define ASUS_WMI_FNLOCK_BIOS_DISABLED	BIT(0)
-#define ASUS_WMI_DEVID_CAMERA_LED		0x00060079
 
 #define ASUS_MID_FAN_DESC		"mid_fan"
 #define ASUS_GPU_FAN_DESC		"gpu_fan"
@@ -94,6 +93,10 @@ module_param(fnlock_default, bool, 0444);
 #define ASUS_FAN_BOOST_MODE_SILENT_MASK		0x02
 #define ASUS_FAN_BOOST_MODES_MASK		0x03
 
+#define ASUS_FAN_BOOST_MODE2_NORMAL		0
+#define ASUS_FAN_BOOST_MODE2_SILENT		1
+#define ASUS_FAN_BOOST_MODE2_OVERBOOST		2
+
 #define ASUS_THROTTLE_THERMAL_POLICY_DEFAULT	0
 #define ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST	1
 #define ASUS_THROTTLE_THERMAL_POLICY_SILENT	2
@@ -268,6 +271,7 @@ struct asus_wmi {
 	int agfn_pwm;
 
 	bool fan_boost_mode_available;
+	u32 fan_boost_mode_dev_id;
 	u8 fan_boost_mode_mask;
 	u8 fan_boost_mode;
 
@@ -3019,14 +3023,14 @@ static int asus_wmi_fan_init(struct asus_wmi *asus)
 
 /* Fan mode *******************************************************************/
 
-static int fan_boost_mode_check_present(struct asus_wmi *asus)
+static int fan_boost_mode1_check_present(struct asus_wmi *asus)
 {
 	u32 result;
 	int err;
 
-	asus->fan_boost_mode_available = false;
+	asus->fan_boost_mode_dev_id = ASUS_WMI_DEVID_FAN_BOOST_MODE;
 
-	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_FAN_BOOST_MODE,
+	err = asus_wmi_get_devstate(asus, asus->fan_boost_mode_dev_id,
 				    &result);
 	if (err) {
 		if (err == -ENODEV)
@@ -3044,16 +3048,87 @@ static int fan_boost_mode_check_present(struct asus_wmi *asus)
 	return 0;
 }
 
+static int fan_boost_mode2_check_present(struct asus_wmi *asus)
+{
+	u32 result;
+	int err;
+
+	asus->fan_boost_mode_mask = ASUS_FAN_BOOST_MODES_MASK;
+	asus->fan_boost_mode_dev_id = ASUS_WMI_DEVID_FAN_BOOST_MODE2;
+
+	err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_FAN_BOOST_MODE2,
+				    &result);
+	if (err) {
+		if (err == -ENODEV)
+			return 0;
+		else
+			return err;
+	}
+
+	if (! (result & ASUS_WMI_DSTS_PRESENCE_BIT))
+		return 0;
+
+	asus->fan_boost_mode_available = true;
+
+	if (result & ASUS_FAN_BOOST_MODE2_SILENT) {
+		asus->fan_boost_mode = ASUS_FAN_BOOST_MODE_SILENT;
+	} else if(result & ASUS_FAN_BOOST_MODE2_OVERBOOST) {
+		asus->fan_boost_mode = ASUS_FAN_BOOST_MODE_OVERBOOST;
+	} else {
+		asus->fan_boost_mode = ASUS_FAN_BOOST_MODE_NORMAL;
+	}
+
+	return 0;
+}
+
+static int fan_boost_mode_check_present(struct asus_wmi *asus)
+{
+	int err;
+
+	asus->fan_boost_mode_available = false;
+
+	err = fan_boost_mode1_check_present(asus);
+	if (err)
+		return err;
+
+	if (!asus->fan_boost_mode_available) {
+		err = fan_boost_mode2_check_present(asus);
+	}
+
+	return err;
+}
+
 static int fan_boost_mode_write(struct asus_wmi *asus)
 {
 	u32 retval;
 	u8 value;
+	u8 hw_value;
 	int err;
 
 	value = asus->fan_boost_mode;
 
-	pr_info("Set fan boost mode: %u\n", value);
-	err = asus_wmi_set_devstate(ASUS_WMI_DEVID_FAN_BOOST_MODE, value,
+	/* transform userspace values into hardware values */
+	if(asus->fan_boost_mode_dev_id == ASUS_WMI_DEVID_FAN_BOOST_MODE2) {
+		switch(value) {
+			case ASUS_FAN_BOOST_MODE_SILENT:
+				hw_value = ASUS_FAN_BOOST_MODE2_SILENT;
+				break;
+			case ASUS_FAN_BOOST_MODE_OVERBOOST:
+				hw_value = ASUS_FAN_BOOST_MODE2_OVERBOOST;
+				break;
+			case ASUS_FAN_BOOST_MODE_NORMAL:
+				hw_value = ASUS_FAN_BOOST_MODE2_NORMAL;
+				break;
+			default:
+				return -EINVAL;
+
+		}
+	} else {
+		hw_value = value;
+	}
+
+	pr_info("Set fan boost mode: user=%u hw=%u\n", value, hw_value);
+	err = asus_wmi_set_devstate(asus->fan_boost_mode_dev_id, hw_value,
 				    &retval);
 
 	sysfs_notify(&asus->platform_device->dev.kobj, NULL,
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index b3c35e33f1e7..62982f67d632 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -65,6 +65,7 @@
 /* Writing a brightness re-enables the screen if disabled */
 #define ASUS_WMI_DEVID_SCREENPAD_LIGHT	0x00050032
 #define ASUS_WMI_DEVID_FAN_BOOST_MODE	0x00110018
+#define ASUS_WMI_DEVID_FAN_BOOST_MODE2	0x00110019
 #define ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY 0x00120075
 
 /* Misc */
-- 
2.45.2

Re: [PATCH 2/2] platform/x86: asus-wmi: support newer fan_boost_mode dev_id

[Luke Jones]

On Thu, 20 Jun 2024, at 8:22 PM, Devin Bayer wrote:
> Support changing the fan mode (silent, performance, standard). I reused
> the existing fan_boost_mode sysfs entry.
> 
> Signed-off-by: Devin Bayer <dev@doubly.so>
> ---
> drivers/platform/x86/asus-wmi.c            | 87 ++++++++++++++++++++--
> include/linux/platform_data/x86/asus-wmi.h |  1 +
> 2 files changed, 82 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 5585f15e7920..e27b8f86d57b 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -73,7 +73,6 @@ module_param(fnlock_default, bool, 0444);
> #define NOTIFY_LID_FLIP_ROG 0xbd
>  
> #define ASUS_WMI_FNLOCK_BIOS_DISABLED BIT(0)
> -#define ASUS_WMI_DEVID_CAMERA_LED 0x00060079

Be careful not to introduce extraneous changes.

>  
> #define ASUS_MID_FAN_DESC "mid_fan"
> #define ASUS_GPU_FAN_DESC "gpu_fan"
> @@ -94,6 +93,10 @@ module_param(fnlock_default, bool, 0444);
> #define ASUS_FAN_BOOST_MODE_SILENT_MASK 0x02
> #define ASUS_FAN_BOOST_MODES_MASK 0x03
>  
> +#define ASUS_FAN_BOOST_MODE2_NORMAL 0
> +#define ASUS_FAN_BOOST_MODE2_SILENT 1
> +#define ASUS_FAN_BOOST_MODE2_OVERBOOST 2
> +
> #define ASUS_THROTTLE_THERMAL_POLICY_DEFAULT 0
> #define ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST 1
> #define ASUS_THROTTLE_THERMAL_POLICY_SILENT 2
> @@ -268,6 +271,7 @@ struct asus_wmi {
> int agfn_pwm;
>  
> bool fan_boost_mode_available;
> + u32 fan_boost_mode_dev_id;
> u8 fan_boost_mode_mask;
> u8 fan_boost_mode;
>  
> @@ -3019,14 +3023,14 @@ static int asus_wmi_fan_init(struct asus_wmi *asus)
>  
> /* Fan mode *******************************************************************/
>  
> -static int fan_boost_mode_check_present(struct asus_wmi *asus)
> +static int fan_boost_mode1_check_present(struct asus_wmi *asus)
> {
> u32 result;
> int err;
>  
> - asus->fan_boost_mode_available = false;
> + asus->fan_boost_mode_dev_id = ASUS_WMI_DEVID_FAN_BOOST_MODE;
>  
> - err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_FAN_BOOST_MODE,
> + err = asus_wmi_get_devstate(asus, asus->fan_boost_mode_dev_id,
>     &result);
> if (err) {
> if (err == -ENODEV)
> @@ -3044,16 +3048,87 @@ static int fan_boost_mode_check_present(struct asus_wmi *asus)
> return 0;
> }
>  
> +static int fan_boost_mode2_check_present(struct asus_wmi *asus)
> +{
> + u32 result;
> + int err;
> +
> + asus->fan_boost_mode_mask = ASUS_FAN_BOOST_MODES_MASK;
> + asus->fan_boost_mode_dev_id = ASUS_WMI_DEVID_FAN_BOOST_MODE2;
> +
> + err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_FAN_BOOST_MODE2,
> +     &result);
> + if (err) {
> + if (err == -ENODEV)
> + return 0;
> + else
> + return err;
> + }
> +
> + if (! (result & ASUS_WMI_DSTS_PRESENCE_BIT))
> + return 0;
> +
> + asus->fan_boost_mode_available = true;
> +
> + if (result & ASUS_FAN_BOOST_MODE2_SILENT) {
> + asus->fan_boost_mode = ASUS_FAN_BOOST_MODE_SILENT;
> + } else if(result & ASUS_FAN_BOOST_MODE2_OVERBOOST) {
> + asus->fan_boost_mode = ASUS_FAN_BOOST_MODE_OVERBOOST;
> + } else {
> + asus->fan_boost_mode = ASUS_FAN_BOOST_MODE_NORMAL;
> + }
> +
> + return 0;
> +}
> +
> +static int fan_boost_mode_check_present(struct asus_wmi *asus)
> +{
> + int err;
> +
> + asus->fan_boost_mode_available = false;
> +
> + err = fan_boost_mode1_check_present(asus);
> + if (err)
> + return err;
> +
> + if (!asus->fan_boost_mode_available) {
> + err = fan_boost_mode2_check_present(asus);
> + }
> +
> + return err;
> +}
> +
> static int fan_boost_mode_write(struct asus_wmi *asus)
> {
> u32 retval;
> u8 value;
> + u8 hw_value;
> int err;
>  
> value = asus->fan_boost_mode;
>  
> - pr_info("Set fan boost mode: %u\n", value);
> - err = asus_wmi_set_devstate(ASUS_WMI_DEVID_FAN_BOOST_MODE, value,
> + /* transform userspace values into hardware values */
> + if(asus->fan_boost_mode_dev_id == ASUS_WMI_DEVID_FAN_BOOST_MODE2) {
> + switch(value) {
> + case ASUS_FAN_BOOST_MODE_SILENT:
> + hw_value = ASUS_FAN_BOOST_MODE2_SILENT;
> + break;
> + case ASUS_FAN_BOOST_MODE_OVERBOOST:
> + hw_value = ASUS_FAN_BOOST_MODE2_OVERBOOST;
> + break;
> + case ASUS_FAN_BOOST_MODE_NORMAL:
> + hw_value = ASUS_FAN_BOOST_MODE2_NORMAL;
> + break;
> + default:
> + return -EINVAL;
> +
> + }
> + } else {
> + hw_value = value;
> + }
> +
> + pr_info("Set fan boost mode: user=%u hw=%u\n", value, hw_value);
> + err = asus_wmi_set_devstate(asus->fan_boost_mode_dev_id, hw_value,
>     &retval);
>  
> sysfs_notify(&asus->platform_device->dev.kobj, NULL,
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index b3c35e33f1e7..62982f67d632 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -65,6 +65,7 @@
> /* Writing a brightness re-enables the screen if disabled */
> #define ASUS_WMI_DEVID_SCREENPAD_LIGHT 0x00050032
> #define ASUS_WMI_DEVID_FAN_BOOST_MODE 0x00110018
> +#define ASUS_WMI_DEVID_FAN_BOOST_MODE2 0x00110019
> #define ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY 0x00120075
>  
> /* Misc */
> -- 
> 2.45.2
> 
> 


Thank you for the work on this. But I must point out that the same 0x00110019 method has already been submitted as a patch to work with the existing "throttle_thermal" functionality, which itself is also tied to platoform_profile class support.

See https://lore.kernel.org/platform-driver-x86/20240609144849.2532-1-mohamed.ghanmi@supcom.tn/T/#mcd18e74676084e21d5c15af84bc08d8c6b375fb9

If you could submit only the first patch instead please?

Re: [PATCH 1/2] platform/x86: asus-wmi: support camera disable LED

[Luke Jones]

On Thu, 20 Jun 2024, at 8:22 PM, Devin Bayer wrote:
> Support the LED on F10 above the crossed out camera icon.
> 
> Signed-off-by: Devin Bayer <dev@doubly.so>
> ---
> drivers/platform/x86/asus-wmi.c            | 36 ++++++++++++++++++++++
> include/linux/platform_data/x86/asus-wmi.h |  2 ++
> 2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 3f9b6285c9a6..5585f15e7920 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -73,6 +73,7 @@ module_param(fnlock_default, bool, 0444);
> #define NOTIFY_LID_FLIP_ROG 0xbd
>  
> #define ASUS_WMI_FNLOCK_BIOS_DISABLED BIT(0)
> +#define ASUS_WMI_DEVID_CAMERA_LED 0x00060079
>  
> #define ASUS_MID_FAN_DESC "mid_fan"
> #define ASUS_GPU_FAN_DESC "gpu_fan"
> @@ -238,6 +239,7 @@ struct asus_wmi {
> struct led_classdev lightbar_led;
> int lightbar_led_wk;
> struct led_classdev micmute_led;
> + struct led_classdev camera_led;
> struct workqueue_struct *led_workqueue;
> struct work_struct tpd_led_work;
> struct work_struct wlan_led_work;
> @@ -1642,6 +1644,27 @@ static int micmute_led_set(struct led_classdev *led_cdev,
> return err < 0 ? err : 0;
> }
>  
> +static enum led_brightness camera_led_get(struct led_classdev *led_cdev)
> +{
> + struct asus_wmi *asus;
> + u32 result;
> +
> + asus = container_of(led_cdev, struct asus_wmi, camera_led);
> + asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_CAMERA_LED, &result);
> +
> + return result & ASUS_WMI_DSTS_BRIGHTNESS_MASK;
> +}
> +
> +static int camera_led_set(struct led_classdev *led_cdev,
> +    enum led_brightness brightness)
> +{
> + int state = brightness != LED_OFF;
> + int err;
> +
> + err = asus_wmi_set_devstate(ASUS_WMI_DEVID_CAMERA_LED, state, NULL);
> + return err < 0 ? err : 0;
> +}
> +
> static void asus_wmi_led_exit(struct asus_wmi *asus)
> {
> led_classdev_unregister(&asus->kbd_led);
> @@ -1649,6 +1672,7 @@ static void asus_wmi_led_exit(struct asus_wmi *asus)
> led_classdev_unregister(&asus->wlan_led);
> led_classdev_unregister(&asus->lightbar_led);
> led_classdev_unregister(&asus->micmute_led);
> + led_classdev_unregister(&asus->camera_led);
>  
> if (asus->led_workqueue)
> destroy_workqueue(asus->led_workqueue);
> @@ -1740,6 +1764,18 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
> goto error;
> }
>  
> + if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_CAMERA_LED)) {
> + asus->camera_led.name = "platform::camera";

What do other devices label their camera LED as? The one I could find appears to use `<vendor>::camera`. So maybe `asus::camera` would be better? This also keeps in line with `asus::kbd_backlight`.

> + asus->camera_led.max_brightness = 1;
> + asus->camera_led.brightness_get = camera_led_get;
> + asus->camera_led.brightness_set_blocking = camera_led_set;
> +
> + rv = led_classdev_register(&asus->platform_device->dev,
> + &asus->camera_led);
> + if (rv)
> + goto error;
> + }
> +
> error:
> if (rv)
> asus_wmi_led_exit(asus);
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 3eb5cd6773ad..b3c35e33f1e7 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -50,6 +50,8 @@
> #define ASUS_WMI_DEVID_LED5 0x00020015
> #define ASUS_WMI_DEVID_LED6 0x00020016
> #define ASUS_WMI_DEVID_MICMUTE_LED 0x00040017
> +#define ASUS_WMI_DEVID_CAMERA_LED_NEG 0x00060078
> +#define ASUS_WMI_DEVID_CAMERA_LED 0x00060079
>  
> /* Backlight and Brightness */
> #define ASUS_WMI_DEVID_ALS_ENABLE 0x00050001 /* Ambient Light Sensor */
> -- 
> 2.45.2
> 

I'll defer final review to Hans and Ilpo to be sure I've not missed anything, otherwise it LGTM pending the one comment above.

Re: [PATCH 1/2] platform/x86: asus-wmi: support camera disable LED

[Devin Bayer]

Thanks for the review, Luke.

On 20/06/2024 23.40, Luke Jones wrote:
>>   
>> + if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_CAMERA_LED)) {
>> + asus->camera_led.name = "platform::camera";
> 
> What do other devices label their camera LED as? The one I could find appears to use `<vendor>::camera`. So maybe `asus::camera` would be better? This also keeps in line with `asus::kbd_backlight`.

I reasoned it would be better to keep the name generic is so out of the 
box desktops could toggle the camera and the LED when KEY_CAMERA is 
pressed, just like with micmute and mute.

But I'll submit a new version with just this patch and the name change.

~ Dev

Re: [PATCH 2/2] platform/x86: asus-wmi: support newer fan_boost_mode dev_id

[Devin Bayer]

On 20/06/2024 23.17, Luke Jones wrote:
>  
>> #define ASUS_WMI_FNLOCK_BIOS_DISABLED BIT(0)
>> -#define ASUS_WMI_DEVID_CAMERA_LED 0x00060079
> 
> Be careful not to introduce extraneous changes.

Yes, will do.

> Thank you for the work on this. But I must point out that the same 0x00110019 method has already been submitted as a patch to work with the existing "throttle_thermal" functionality, which itself is also tied to platoform_profile class support.
> 
> See https://lore.kernel.org/platform-driver-x86/20240609144849.2532-1-mohamed.ghanmi@supcom.tn/T/#mcd18e74676084e21d5c15af84bc08d8c6b375fb9

I see that now.

> If you could submit only the first patch instead please?

I'll followup on this topic in the other thread.

~ Dev

Re: [PATCH 1/2] platform/x86: asus-wmi: support camera disable LED

[Luke Jones]

On Fri, 21 Jun 2024, at 7:50 PM, Devin Bayer wrote:
> 
> Thanks for the review, Luke.
> 
> On 20/06/2024 23.40, Luke Jones wrote:
> >>   
> >> + if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_CAMERA_LED)) {
> >> + asus->camera_led.name = "platform::camera";
> > 
> > What do other devices label their camera LED as? The one I could find appears to use `<vendor>::camera`. So maybe `asus::camera` would be better? This also keeps in line with `asus::kbd_backlight`.
> 
> I reasoned it would be better to keep the name generic is so out of the 
> box desktops could toggle the camera and the LED when KEY_CAMERA is 
> pressed, just like with micmute and mute.

This might be true if one relies solely on the filesystem path, which in any case is a bad move and likely to cause the moon to drift away from earth eventually. Most Linux software will use the udev libraries available to filter devices according to any amount of criteria (and if they are not they *really should* - udev is pretty powerful and freeing.

I've tried finding prior art again and there's just not a lot to go on. ".name = "platform" shows very little except a few micmute labels. "::cam" gets one entry. So my guess is this is still a very new thing or it's not important enough to be used..

In any case looking at the rest of the possible LED entries, mostly those following keyboard, the last part of the name being sensible is what counts the most (e.g "scrolllock", "camera"). This might be setting a precedent, and if so I'd be happy with "::camera" in the LED class conveying expectations well enough.