linux-kernel - Re: [PATCH v2 1/2] platform/x86: asus-wmi: Fix inconsistent use of thermal policies

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f1d86afe-42fb-4883-be24-d6032de26f6e@redhat.com>
Date: Thu, 7 Nov 2024 10:51:40 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Armin Wolf <W_Armin@....de>, corentin.chary@...il.com, luke@...nes.dev,
 mohamed.ghanmi@...com.tn
Cc: srinivas.pandruvada@...ux.intel.com, ilpo.jarvinen@...ux.intel.com,
 Michael@...ronix.com, casey.g.bowman@...el.com,
 platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] platform/x86: asus-wmi: Fix inconsistent use of
 thermal policies

Hi,

On 7-Nov-24 1:38 AM, Armin Wolf wrote:
> When changing the thermal policy using the platform profile API,
> a Vivobook thermal policy is stored in throttle_thermal_policy_mode.
> 
> However everywhere else a normal thermal policy is stored inside this
> variable, potentially confusing the platform profile.
> 
> Fix this by always storing normal thermal policy values inside
> throttle_thermal_policy_mode and only do the conversion when writing
> the thermal policy to hardware. This also fixes the order in which
> throttle_thermal_policy_switch_next() steps through the thermal modes
> on Vivobook machines.
> 
> Tested-by: Casey G Bowman <casey.g.bowman@...el.com>
> Fixes: bcbfcebda2cb ("platform/x86: asus-wmi: add support for vivobook fan profiles")
> Signed-off-by: Armin Wolf <W_Armin@....de>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@...hat.com>

Regards,

Hans


> ---
>  drivers/platform/x86/asus-wmi.c | 64 +++++++++++----------------------
>  1 file changed, 21 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index ab9342a01a48..ce60835d0303 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -3696,10 +3696,28 @@ static int asus_wmi_custom_fan_curve_init(struct asus_wmi *asus)
>  /* Throttle thermal policy ****************************************************/
>  static int throttle_thermal_policy_write(struct asus_wmi *asus)
>  {
> -	u8 value = asus->throttle_thermal_policy_mode;
>  	u32 retval;
> +	u8 value;
>  	int err;
> 
> +	if (asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO) {
> +		switch (asus->throttle_thermal_policy_mode) {
> +		case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT:
> +			value = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO;
> +			break;
> +		case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST:
> +			value = ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO;
> +			break;
> +		case ASUS_THROTTLE_THERMAL_POLICY_SILENT:
> +			value = ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +	} else {
> +		value = asus->throttle_thermal_policy_mode;
> +	}
> +
>  	err = asus_wmi_set_devstate(asus->throttle_thermal_policy_dev,
>  				    value, &retval);
> 
> @@ -3804,46 +3822,6 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
>  static DEVICE_ATTR_RW(throttle_thermal_policy);
> 
>  /* Platform profile ***********************************************************/
> -static int asus_wmi_platform_profile_to_vivo(struct asus_wmi *asus, int mode)
> -{
> -	bool vivo;
> -
> -	vivo = asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO;
> -
> -	if (vivo) {
> -		switch (mode) {
> -		case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT:
> -			return ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO;
> -		case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST:
> -			return ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO;
> -		case ASUS_THROTTLE_THERMAL_POLICY_SILENT:
> -			return ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO;
> -		}
> -	}
> -
> -	return mode;
> -}
> -
> -static int asus_wmi_platform_profile_mode_from_vivo(struct asus_wmi *asus, int mode)
> -{
> -	bool vivo;
> -
> -	vivo = asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO;
> -
> -	if (vivo) {
> -		switch (mode) {
> -		case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO:
> -			return ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
> -		case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO:
> -			return ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST;
> -		case ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO:
> -			return ASUS_THROTTLE_THERMAL_POLICY_SILENT;
> -		}
> -	}
> -
> -	return mode;
> -}
> -
>  static int asus_wmi_platform_profile_get(struct platform_profile_handler *pprof,
>  					enum platform_profile_option *profile)
>  {
> @@ -3853,7 +3831,7 @@ static int asus_wmi_platform_profile_get(struct platform_profile_handler *pprof,
>  	asus = container_of(pprof, struct asus_wmi, platform_profile_handler);
>  	tp = asus->throttle_thermal_policy_mode;
> 
> -	switch (asus_wmi_platform_profile_mode_from_vivo(asus, tp)) {
> +	switch (tp) {
>  	case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT:
>  		*profile = PLATFORM_PROFILE_BALANCED;
>  		break;
> @@ -3892,7 +3870,7 @@ static int asus_wmi_platform_profile_set(struct platform_profile_handler *pprof,
>  		return -EOPNOTSUPP;
>  	}
> 
> -	asus->throttle_thermal_policy_mode = asus_wmi_platform_profile_to_vivo(asus, tp);
> +	asus->throttle_thermal_policy_mode = tp;
>  	return throttle_thermal_policy_write(asus);
>  }
> 
> --
> 2.39.5
>