[PATCH v4 0/1] asus-wmi: add platform_profile support

[PATCH v4 0/1] asus-wmi: add platform_profile support

[Luke D. Jones]

Changelog:
- V2
  + Correctly unregister from platform_profile if
    throttle_thermal_policy fails
  + Do platform_profile_notify() in both throttle_thermal_policy_store()
    and in throttle_thermal_policy_switch_next()
  + Remove unnecessary prep for possible fan-boost modes as this
    doesn't match expected platform_profile behaviour
- V3
  + Add missing declaration for err in
    throttle_thermal_policy_switch_next
- V4
  + Update some return codes to use -ENODATA
  + Switch pr_info() to dev_info()
  + Restructure some if/else blocks
  + Add "select ACPI_PLATFORM_PROFILE" to ASUS_WMI kconfig

Luke D. Jones (1):
  asus-wmi: Add support for platform_profile

 drivers/platform/x86/Kconfig    |   1 +
 drivers/platform/x86/asus-wmi.c | 138 +++++++++++++++++++++++++++++++-
 2 files changed, 135 insertions(+), 4 deletions(-)

-- 
2.31.1

[PATCH v4 1/1] asus-wmi: Add support for platform_profile

[Luke D. Jones]

Add initial support for platform_profile where the support is
based on availability of ASUS_THROTTLE_THERMAL_POLICY.

Because throttle_thermal_policy is used by platform_profile and is
writeable separately to platform_profile any userspace changes to
throttle_thermal_policy need to notify platform_profile.

In future throttle_thermal_policy sysfs should be removed so that
only one method controls the laptop power profile.

Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
 drivers/platform/x86/Kconfig    |   1 +
 drivers/platform/x86/asus-wmi.c | 138 +++++++++++++++++++++++++++++++-
 2 files changed, 135 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index d12db6c316ea..46dec48a36c1 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -281,6 +281,7 @@ config ASUS_WMI
 	select INPUT_SPARSEKMAP
 	select LEDS_CLASS
 	select NEW_LEDS
+	select ACPI_PLATFORM_PROFILE
 	help
 	  Say Y here if you have a WMI aware Asus laptop (like Eee PCs or new
 	  Asus Notebooks).
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 90a6a0d00deb..c11ce06073d1 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -11,6 +11,7 @@
  * Copyright (C) 2005 Dmitry Torokhov <dtor@mail.ru>
  */
 
+#include <asm-generic/errno.h>
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/kernel.h>
@@ -26,6 +27,7 @@
 #include <linux/rfkill.h>
 #include <linux/pci.h>
 #include <linux/pci_hotplug.h>
+#include <linux/platform_profile.h>
 #include <linux/power_supply.h>
 #include <linux/hwmon.h>
 #include <linux/hwmon-sysfs.h>
@@ -219,6 +221,9 @@ struct asus_wmi {
 	bool throttle_thermal_policy_available;
 	u8 throttle_thermal_policy_mode;
 
+	struct platform_profile_handler platform_profile_handler;
+	bool platform_profile_support;
+
 	// The RSOC controls the maximum charging percentage.
 	bool battery_rsoc_available;
 
@@ -2103,12 +2108,23 @@ static int throttle_thermal_policy_set_default(struct asus_wmi *asus)
 static int throttle_thermal_policy_switch_next(struct asus_wmi *asus)
 {
 	u8 new_mode = asus->throttle_thermal_policy_mode + 1;
+	int err;
 
 	if (new_mode > ASUS_THROTTLE_THERMAL_POLICY_SILENT)
 		new_mode = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
 
 	asus->throttle_thermal_policy_mode = new_mode;
-	return throttle_thermal_policy_write(asus);
+	err = throttle_thermal_policy_write(asus);
+	if (err)
+		return err;
+
+	/*
+	 * Ensure that platform_profile updates userspace with the change to ensure
+	 * that platform_profile and throttle_thermal_policy_mode are in sync.
+	 */
+	platform_profile_notify();
+
+	return 0;
 }
 
 static ssize_t throttle_thermal_policy_show(struct device *dev,
@@ -2124,9 +2140,10 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
 				    struct device_attribute *attr,
 				    const char *buf, size_t count)
 {
-	int result;
-	u8 new_mode;
 	struct asus_wmi *asus = dev_get_drvdata(dev);
+	u8 new_mode;
+	int result;
+	int err;
 
 	result = kstrtou8(buf, 10, &new_mode);
 	if (result < 0)
@@ -2136,7 +2153,15 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
 		return -EINVAL;
 
 	asus->throttle_thermal_policy_mode = new_mode;
-	throttle_thermal_policy_write(asus);
+	err = throttle_thermal_policy_write(asus);
+	if (err)
+		return err;
+
+	/*
+	 * Ensure that platform_profile updates userspace with the change to ensure
+	 * that platform_profile and throttle_thermal_policy_mode are in sync.
+	 */
+	platform_profile_notify();
 
 	return count;
 }
@@ -2144,6 +2169,101 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
 // Throttle thermal policy: 0 - default, 1 - overboost, 2 - silent
 static DEVICE_ATTR_RW(throttle_thermal_policy);
 
+/* Platform profile ***********************************************************/
+static int platform_profile_get(struct platform_profile_handler *pprof,
+				enum platform_profile_option *profile)
+{
+	struct asus_wmi *asus;
+	int tp;
+
+	asus = container_of(pprof, struct asus_wmi, platform_profile_handler);
+
+	if (!asus->throttle_thermal_policy_available) {
+		return -ENODATA;
+	}
+	tp = asus->throttle_thermal_policy_mode;
+
+	if (tp < 0)
+		return tp;
+
+	switch (tp) {
+	case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT:
+		*profile = PLATFORM_PROFILE_BALANCED;
+		break;
+	case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST:
+		*profile = PLATFORM_PROFILE_PERFORMANCE;
+		break;
+	case ASUS_THROTTLE_THERMAL_POLICY_SILENT:
+		*profile = PLATFORM_PROFILE_QUIET;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int platform_profile_set(struct platform_profile_handler *pprof,
+				enum platform_profile_option profile)
+{
+	struct asus_wmi *asus;
+	int tp;
+
+	asus = container_of(pprof, struct asus_wmi, platform_profile_handler);
+
+	/* All possible toggles like throttle_thermal_policy here */
+	if (!asus->throttle_thermal_policy_available) {
+		return -ENODATA;
+	}
+
+	switch (profile) {
+	case PLATFORM_PROFILE_PERFORMANCE:
+		tp = ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST;
+		break;
+	case PLATFORM_PROFILE_BALANCED:
+		tp = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
+		break;
+	case PLATFORM_PROFILE_QUIET:
+		tp = ASUS_THROTTLE_THERMAL_POLICY_SILENT;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	asus->throttle_thermal_policy_mode = tp;
+	return throttle_thermal_policy_write(asus);
+}
+
+static int platform_profile_setup(struct asus_wmi *asus)
+{
+	int err;
+
+	if (!asus->throttle_thermal_policy_available) {
+		/*
+		 * Not an error if a component platform_profile relies on is unavailable
+		 * so early return, skipping the setup of platform_profile.
+		*/
+		return 0;
+	}
+	dev_info("Using throttle_thermal_policy for platform_profile support\n");
+
+	asus->platform_profile_handler.profile_get = platform_profile_get;
+	asus->platform_profile_handler.profile_set = platform_profile_set;
+
+	set_bit(PLATFORM_PROFILE_QUIET, asus->platform_profile_handler.choices);
+	set_bit(PLATFORM_PROFILE_BALANCED,
+		asus->platform_profile_handler.choices);
+	set_bit(PLATFORM_PROFILE_PERFORMANCE,
+		asus->platform_profile_handler.choices);
+
+	err = platform_profile_register(&asus->platform_profile_handler);
+	if (err)
+		return err;
+
+	asus->platform_profile_support = true;
+	return 0;
+}
+
 /* Backlight ******************************************************************/
 
 static int read_backlight_power(struct asus_wmi *asus)
@@ -2904,6 +3024,10 @@ static int asus_wmi_add(struct platform_device *pdev)
 	else
 		throttle_thermal_policy_set_default(asus);
 
+	err = platform_profile_setup(asus);
+	if (err)
+		goto fail_platform_profile_setup;
+
 	err = panel_od_check_present(asus);
 	if (err)
 		goto fail_panel_od;
@@ -2993,6 +3117,9 @@ static int asus_wmi_add(struct platform_device *pdev)
 	asus_wmi_sysfs_exit(asus->platform_device);
 fail_sysfs:
 fail_throttle_thermal_policy:
+fail_platform_profile_setup:
+	if (asus->platform_profile_support)
+		platform_profile_remove();
 fail_fan_boost_mode:
 fail_egpu_enable:
 fail_dgpu_disable:
@@ -3017,6 +3144,9 @@ static int asus_wmi_remove(struct platform_device *device)
 	asus_fan_set_auto(asus);
 	asus_wmi_battery_exit(asus);
 
+	if (asus->platform_profile_support)
+		platform_profile_remove();
+
 	kfree(asus);
 	return 0;
 }
-- 
2.31.1

Re: [PATCH v4 1/1] asus-wmi: Add support for platform_profile

[Hans de Goede]

Hi Luke,

I have a couple of small remarks once those are resolved in v5
then this should be ready for merging.

On 8/16/21 1:05 AM, Luke D. Jones wrote:
> Add initial support for platform_profile where the support is
> based on availability of ASUS_THROTTLE_THERMAL_POLICY.
> 
> Because throttle_thermal_policy is used by platform_profile and is
> writeable separately to platform_profile any userspace changes to
> throttle_thermal_policy need to notify platform_profile.
> 
> In future throttle_thermal_policy sysfs should be removed so that
> only one method controls the laptop power profile.
> 
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
>  drivers/platform/x86/Kconfig    |   1 +
>  drivers/platform/x86/asus-wmi.c | 138 +++++++++++++++++++++++++++++++-
>  2 files changed, 135 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index d12db6c316ea..46dec48a36c1 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -281,6 +281,7 @@ config ASUS_WMI
>  	select INPUT_SPARSEKMAP
>  	select LEDS_CLASS
>  	select NEW_LEDS
> +	select ACPI_PLATFORM_PROFILE
>  	help
>  	  Say Y here if you have a WMI aware Asus laptop (like Eee PCs or new
>  	  Asus Notebooks).
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 90a6a0d00deb..c11ce06073d1 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -11,6 +11,7 @@
>   * Copyright (C) 2005 Dmitry Torokhov <dtor@mail.ru>
>   */
>  
> +#include <asm-generic/errno.h>

This is a weird place to add an include, also please use
linux/errno.h. But this is not necessary at all, because the
2 if-s using -ENODATA which need this can be dropped
(see below) so please just drop this all together.

>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include <linux/kernel.h>
> @@ -26,6 +27,7 @@
>  #include <linux/rfkill.h>
>  #include <linux/pci.h>
>  #include <linux/pci_hotplug.h>
> +#include <linux/platform_profile.h>
>  #include <linux/power_supply.h>
>  #include <linux/hwmon.h>
>  #include <linux/hwmon-sysfs.h>
> @@ -219,6 +221,9 @@ struct asus_wmi {
>  	bool throttle_thermal_policy_available;
>  	u8 throttle_thermal_policy_mode;
>  
> +	struct platform_profile_handler platform_profile_handler;
> +	bool platform_profile_support;
> +
>  	// The RSOC controls the maximum charging percentage.
>  	bool battery_rsoc_available;
>  
> @@ -2103,12 +2108,23 @@ static int throttle_thermal_policy_set_default(struct asus_wmi *asus)
>  static int throttle_thermal_policy_switch_next(struct asus_wmi *asus)
>  {
>  	u8 new_mode = asus->throttle_thermal_policy_mode + 1;
> +	int err;
>  
>  	if (new_mode > ASUS_THROTTLE_THERMAL_POLICY_SILENT)
>  		new_mode = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
>  
>  	asus->throttle_thermal_policy_mode = new_mode;
> -	return throttle_thermal_policy_write(asus);
> +	err = throttle_thermal_policy_write(asus);
> +	if (err)
> +		return err;
> +
> +	/*
> +	 * Ensure that platform_profile updates userspace with the change to ensure
> +	 * that platform_profile and throttle_thermal_policy_mode are in sync.
> +	 */
> +	platform_profile_notify();
> +
> +	return 0;
>  }
>  
>  static ssize_t throttle_thermal_policy_show(struct device *dev,
> @@ -2124,9 +2140,10 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
>  				    struct device_attribute *attr,
>  				    const char *buf, size_t count)
>  {
> -	int result;
> -	u8 new_mode;
>  	struct asus_wmi *asus = dev_get_drvdata(dev);
> +	u8 new_mode;
> +	int result;
> +	int err;
>  
>  	result = kstrtou8(buf, 10, &new_mode);
>  	if (result < 0)
> @@ -2136,7 +2153,15 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
>  		return -EINVAL;
>  
>  	asus->throttle_thermal_policy_mode = new_mode;
> -	throttle_thermal_policy_write(asus);
> +	err = throttle_thermal_policy_write(asus);
> +	if (err)
> +		return err;
> +
> +	/*
> +	 * Ensure that platform_profile updates userspace with the change to ensure
> +	 * that platform_profile and throttle_thermal_policy_mode are in sync.
> +	 */
> +	platform_profile_notify();
>  
>  	return count;
>  }
> @@ -2144,6 +2169,101 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
>  // Throttle thermal policy: 0 - default, 1 - overboost, 2 - silent
>  static DEVICE_ATTR_RW(throttle_thermal_policy);
>  
> +/* Platform profile ***********************************************************/
> +static int platform_profile_get(struct platform_profile_handler *pprof,
> +				enum platform_profile_option *profile)
> +{
> +	struct asus_wmi *asus;
> +	int tp;
> +
> +	asus = container_of(pprof, struct asus_wmi, platform_profile_handler);
> +
> +	if (!asus->throttle_thermal_policy_available) {
> +		return -ENODATA;
> +	}

Please don't use {} for an if before a single statement.

Also platform_profile_get() only gets used if platform_profile_setup()
proceeds beyond the same check which never happens, so this entire check is
not necessary, please just drop the entire check.

> +	tp = asus->throttle_thermal_policy_mode;
> +
> +	if (tp < 0)
> +		return tp;
> +
> +	switch (tp) {
> +	case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT:
> +		*profile = PLATFORM_PROFILE_BALANCED;
> +		break;
> +	case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST:
> +		*profile = PLATFORM_PROFILE_PERFORMANCE;
> +		break;
> +	case ASUS_THROTTLE_THERMAL_POLICY_SILENT:
> +		*profile = PLATFORM_PROFILE_QUIET;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int platform_profile_set(struct platform_profile_handler *pprof,
> +				enum platform_profile_option profile)
> +{
> +	struct asus_wmi *asus;
> +	int tp;
> +
> +	asus = container_of(pprof, struct asus_wmi, platform_profile_handler);
> +
> +	/* All possible toggles like throttle_thermal_policy here */

This comment seems to be a left-over from when you considered also adding
fan-boost-mode, please drop.

> +	if (!asus->throttle_thermal_policy_available) {
> +		return -ENODATA;
> +	}

As above, please drop.

> +
> +	switch (profile) {
> +	case PLATFORM_PROFILE_PERFORMANCE:
> +		tp = ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST;
> +		break;
> +	case PLATFORM_PROFILE_BALANCED:
> +		tp = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
> +		break;
> +	case PLATFORM_PROFILE_QUIET:
> +		tp = ASUS_THROTTLE_THERMAL_POLICY_SILENT;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	asus->throttle_thermal_policy_mode = tp;
> +	return throttle_thermal_policy_write(asus);
> +}
> +
> +static int platform_profile_setup(struct asus_wmi *asus)
> +{
> +	int err;
> +
> +	if (!asus->throttle_thermal_policy_available) {
> +		/*
> +		 * Not an error if a component platform_profile relies on is unavailable
> +		 * so early return, skipping the setup of platform_profile.
> +		*/
> +		return 0;
> +	}
> +	dev_info("Using throttle_thermal_policy for platform_profile support\n");
> +
> +	asus->platform_profile_handler.profile_get = platform_profile_get;
> +	asus->platform_profile_handler.profile_set = platform_profile_set;
> +
> +	set_bit(PLATFORM_PROFILE_QUIET, asus->platform_profile_handler.choices);
> +	set_bit(PLATFORM_PROFILE_BALANCED,
> +		asus->platform_profile_handler.choices);
> +	set_bit(PLATFORM_PROFILE_PERFORMANCE,
> +		asus->platform_profile_handler.choices);
> +
> +	err = platform_profile_register(&asus->platform_profile_handler);
> +	if (err)
> +		return err;
> +
> +	asus->platform_profile_support = true;
> +	return 0;
> +}
> +
>  /* Backlight ******************************************************************/
>  
>  static int read_backlight_power(struct asus_wmi *asus)
> @@ -2904,6 +3024,10 @@ static int asus_wmi_add(struct platform_device *pdev)
>  	else
>  		throttle_thermal_policy_set_default(asus);
>  
> +	err = platform_profile_setup(asus);
> +	if (err)
> +		goto fail_platform_profile_setup;
> +
>  	err = panel_od_check_present(asus);
>  	if (err)
>  		goto fail_panel_od;



> @@ -2993,6 +3117,9 @@ static int asus_wmi_add(struct platform_device *pdev)
>  	asus_wmi_sysfs_exit(asus->platform_device);
>  fail_sysfs:
>  fail_throttle_thermal_policy:
> +fail_platform_profile_setup:
> +	if (asus->platform_profile_support)
> +		platform_profile_remove();
>  fail_fan_boost_mode:
>  fail_egpu_enable:
>  fail_dgpu_disable:
> @@ -3017,6 +3144,9 @@ static int asus_wmi_remove(struct platform_device *device)
>  	asus_fan_set_auto(asus);
>  	asus_wmi_battery_exit(asus);
>  
> +	if (asus->platform_profile_support)
> +		platform_profile_remove();
> +
>  	kfree(asus);
>  	return 0;
>  }
> 


Regards,

Hans

Re: [PATCH v4 1/1] asus-wmi: Add support for platform_profile

[Hans de Goede]

Hi Luke,

On 8/16/21 10:32 AM, Hans de Goede wrote:
> Hi Luke,
> 
> I have a couple of small remarks once those are resolved in v5
> then this should be ready for merging.

I'm trying to get the pdx86 in order for the upcoming merge-window
and it would be good if we can get this patch included.

Since my suggested changes come down to just dropping 2 unnecessary
"if (!asus->throttle_thermal_policy_available) {...}"
checks, a v5 is not really necessary. I can also just merge v4 and
drop these lines myself when merging, then you don't need to do a v5.

Are you ok with me dropping these extra/unnecessary checks and merging
v4 with that change?

Regards,

Hans




>> +		return -ENODATA;
> 
> On 8/16/21 1:05 AM, Luke D. Jones wrote:
>> Add initial support for platform_profile where the support is
>> based on availability of ASUS_THROTTLE_THERMAL_POLICY.
>>
>> Because throttle_thermal_policy is used by platform_profile and is
>> writeable separately to platform_profile any userspace changes to
>> throttle_thermal_policy need to notify platform_profile.
>>
>> In future throttle_thermal_policy sysfs should be removed so that
>> only one method controls the laptop power profile.
>>
>> Signed-off-by: Luke D. Jones <luke@ljones.dev>
>> ---
>>  drivers/platform/x86/Kconfig    |   1 +
>>  drivers/platform/x86/asus-wmi.c | 138 +++++++++++++++++++++++++++++++-
>>  2 files changed, 135 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index d12db6c316ea..46dec48a36c1 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -281,6 +281,7 @@ config ASUS_WMI
>>  	select INPUT_SPARSEKMAP
>>  	select LEDS_CLASS
>>  	select NEW_LEDS
>> +	select ACPI_PLATFORM_PROFILE
>>  	help
>>  	  Say Y here if you have a WMI aware Asus laptop (like Eee PCs or new
>>  	  Asus Notebooks).
>> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
>> index 90a6a0d00deb..c11ce06073d1 100644
>> --- a/drivers/platform/x86/asus-wmi.c
>> +++ b/drivers/platform/x86/asus-wmi.c
>> @@ -11,6 +11,7 @@
>>   * Copyright (C) 2005 Dmitry Torokhov <dtor@mail.ru>
>>   */
>>  
>> +#include <asm-generic/errno.h>
> 
> This is a weird place to add an include, also please use
> linux/errno.h. But this is not necessary at all, because the
> 2 if-s using -ENODATA which need this can be dropped
> (see below) so please just drop this all together.
> 
>>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>  
>>  #include <linux/kernel.h>
>> @@ -26,6 +27,7 @@
>>  #include <linux/rfkill.h>
>>  #include <linux/pci.h>
>>  #include <linux/pci_hotplug.h>
>> +#include <linux/platform_profile.h>
>>  #include <linux/power_supply.h>
>>  #include <linux/hwmon.h>
>>  #include <linux/hwmon-sysfs.h>
>> @@ -219,6 +221,9 @@ struct asus_wmi {
>>  	bool throttle_thermal_policy_available;
>>  	u8 throttle_thermal_policy_mode;
>>  
>> +	struct platform_profile_handler platform_profile_handler;
>> +	bool platform_profile_support;
>> +
>>  	// The RSOC controls the maximum charging percentage.
>>  	bool battery_rsoc_available;
>>  
>> @@ -2103,12 +2108,23 @@ static int throttle_thermal_policy_set_default(struct asus_wmi *asus)
>>  static int throttle_thermal_policy_switch_next(struct asus_wmi *asus)
>>  {
>>  	u8 new_mode = asus->throttle_thermal_policy_mode + 1;
>> +	int err;
>>  
>>  	if (new_mode > ASUS_THROTTLE_THERMAL_POLICY_SILENT)
>>  		new_mode = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
>>  
>>  	asus->throttle_thermal_policy_mode = new_mode;
>> -	return throttle_thermal_policy_write(asus);
>> +	err = throttle_thermal_policy_write(asus);
>> +	if (err)
>> +		return err;
>> +
>> +	/*
>> +	 * Ensure that platform_profile updates userspace with the change to ensure
>> +	 * that platform_profile and throttle_thermal_policy_mode are in sync.
>> +	 */
>> +	platform_profile_notify();
>> +
>> +	return 0;
>>  }
>>  
>>  static ssize_t throttle_thermal_policy_show(struct device *dev,
>> @@ -2124,9 +2140,10 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
>>  				    struct device_attribute *attr,
>>  				    const char *buf, size_t count)
>>  {
>> -	int result;
>> -	u8 new_mode;
>>  	struct asus_wmi *asus = dev_get_drvdata(dev);
>> +	u8 new_mode;
>> +	int result;
>> +	int err;
>>  
>>  	result = kstrtou8(buf, 10, &new_mode);
>>  	if (result < 0)
>> @@ -2136,7 +2153,15 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
>>  		return -EINVAL;
>>  
>>  	asus->throttle_thermal_policy_mode = new_mode;
>> -	throttle_thermal_policy_write(asus);
>> +	err = throttle_thermal_policy_write(asus);
>> +	if (err)
>> +		return err;
>> +
>> +	/*
>> +	 * Ensure that platform_profile updates userspace with the change to ensure
>> +	 * that platform_profile and throttle_thermal_policy_mode are in sync.
>> +	 */
>> +	platform_profile_notify();
>>  
>>  	return count;
>>  }
>> @@ -2144,6 +2169,101 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
>>  // Throttle thermal policy: 0 - default, 1 - overboost, 2 - silent
>>  static DEVICE_ATTR_RW(throttle_thermal_policy);
>>  
>> +/* Platform profile ***********************************************************/
>> +static int platform_profile_get(struct platform_profile_handler *pprof,
>> +				enum platform_profile_option *profile)
>> +{
>> +	struct asus_wmi *asus;
>> +	int tp;
>> +
>> +	asus = container_of(pprof, struct asus_wmi, platform_profile_handler);
>> +
>> +	if (!asus->throttle_thermal_policy_available) {
>> +		return -ENODATA;
>> +	}
> 
> Please don't use {} for an if before a single statement.
> 
> Also platform_profile_get() only gets used if platform_profile_setup()
> proceeds beyond the same check which never happens, so this entire check is
> not necessary, please just drop the entire check.
> 
>> +	tp = asus->throttle_thermal_policy_mode;
>> +
>> +	if (tp < 0)
>> +		return tp;
>> +
>> +	switch (tp) {
>> +	case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT:
>> +		*profile = PLATFORM_PROFILE_BALANCED;
>> +		break;
>> +	case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST:
>> +		*profile = PLATFORM_PROFILE_PERFORMANCE;
>> +		break;
>> +	case ASUS_THROTTLE_THERMAL_POLICY_SILENT:
>> +		*profile = PLATFORM_PROFILE_QUIET;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int platform_profile_set(struct platform_profile_handler *pprof,
>> +				enum platform_profile_option profile)
>> +{
>> +	struct asus_wmi *asus;
>> +	int tp;
>> +
>> +	asus = container_of(pprof, struct asus_wmi, platform_profile_handler);
>> +
>> +	/* All possible toggles like throttle_thermal_policy here */
> 
> This comment seems to be a left-over from when you considered also adding
> fan-boost-mode, please drop.
> 
>> +	if (!asus->throttle_thermal_policy_available) {
>> +		return -ENODATA;
>> +	}
> 
> As above, please drop.
> 
>> +
>> +	switch (profile) {
>> +	case PLATFORM_PROFILE_PERFORMANCE:
>> +		tp = ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST;
>> +		break;
>> +	case PLATFORM_PROFILE_BALANCED:
>> +		tp = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
>> +		break;
>> +	case PLATFORM_PROFILE_QUIET:
>> +		tp = ASUS_THROTTLE_THERMAL_POLICY_SILENT;
>> +		break;
>> +	default:
>> +		return -EOPNOTSUPP;
>> +	}
>> +
>> +	asus->throttle_thermal_policy_mode = tp;
>> +	return throttle_thermal_policy_write(asus);
>> +}
>> +
>> +static int platform_profile_setup(struct asus_wmi *asus)
>> +{
>> +	int err;
>> +
>> +	if (!asus->throttle_thermal_policy_available) {
>> +		/*
>> +		 * Not an error if a component platform_profile relies on is unavailable
>> +		 * so early return, skipping the setup of platform_profile.
>> +		*/
>> +		return 0;
>> +	}
>> +	dev_info("Using throttle_thermal_policy for platform_profile support\n");
>> +
>> +	asus->platform_profile_handler.profile_get = platform_profile_get;
>> +	asus->platform_profile_handler.profile_set = platform_profile_set;
>> +
>> +	set_bit(PLATFORM_PROFILE_QUIET, asus->platform_profile_handler.choices);
>> +	set_bit(PLATFORM_PROFILE_BALANCED,
>> +		asus->platform_profile_handler.choices);
>> +	set_bit(PLATFORM_PROFILE_PERFORMANCE,
>> +		asus->platform_profile_handler.choices);
>> +
>> +	err = platform_profile_register(&asus->platform_profile_handler);
>> +	if (err)
>> +		return err;
>> +
>> +	asus->platform_profile_support = true;
>> +	return 0;
>> +}
>> +
>>  /* Backlight ******************************************************************/
>>  
>>  static int read_backlight_power(struct asus_wmi *asus)
>> @@ -2904,6 +3024,10 @@ static int asus_wmi_add(struct platform_device *pdev)
>>  	else
>>  		throttle_thermal_policy_set_default(asus);
>>  
>> +	err = platform_profile_setup(asus);
>> +	if (err)
>> +		goto fail_platform_profile_setup;
>> +
>>  	err = panel_od_check_present(asus);
>>  	if (err)
>>  		goto fail_panel_od;
> 
> 
> 
>> @@ -2993,6 +3117,9 @@ static int asus_wmi_add(struct platform_device *pdev)
>>  	asus_wmi_sysfs_exit(asus->platform_device);
>>  fail_sysfs:
>>  fail_throttle_thermal_policy:
>> +fail_platform_profile_setup:
>> +	if (asus->platform_profile_support)
>> +		platform_profile_remove();
>>  fail_fan_boost_mode:
>>  fail_egpu_enable:
>>  fail_dgpu_disable:
>> @@ -3017,6 +3144,9 @@ static int asus_wmi_remove(struct platform_device *device)
>>  	asus_fan_set_auto(asus);
>>  	asus_wmi_battery_exit(asus);
>>  
>> +	if (asus->platform_profile_support)
>> +		platform_profile_remove();
>> +
>>  	kfree(asus);
>>  	return 0;
>>  }
>>
> 
> 
> Regards,
> 
> Hans
>