[PATCH v6 02/12] platform/x86: alienware-wmi-wmax: Refactor is_awcc_thermal_mode()

Kurt Borja posted 12 patches 9 months, 1 week ago
There is a newer version of this series
[PATCH v6 02/12] platform/x86: alienware-wmi-wmax: Refactor is_awcc_thermal_mode()
Posted by Kurt Borja 9 months, 1 week ago
Refactor is_awcc_thermal_mode() to use FIELD_GET() instead of bitwise
operations. Drop the check for BIT(8) sensor flag and rename it to
is_awcc_thermal_profile_id().

Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/platform/x86/dell/alienware-wmi-wmax.c | 33 +++++++++++++-------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
index ed70e12d73d7fe5d89f3364c5367820bf47e3c1e..80aefba5b22d6b4ac18aeb2ca356f8c911150abd 100644
--- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
+++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
@@ -34,7 +34,8 @@
 #define AWCC_FAILURE_CODE			0xFFFFFFFF
 #define AWCC_THERMAL_TABLE_MASK			GENMASK(7, 4)
 #define AWCC_THERMAL_MODE_MASK			GENMASK(3, 0)
-#define AWCC_SENSOR_ID_MASK			BIT(8)
+/* Some IDs have a BIT(8) flag that we ignore */
+#define AWCC_RESOURCE_ID_MASK			GENMASK(7, 0)
 
 static bool force_platform_profile;
 module_param_unsafe(force_platform_profile, bool, 0);
@@ -168,8 +169,8 @@ enum AWCC_GAME_SHIFT_STATUS_OPERATIONS {
 };
 
 enum AWCC_THERMAL_TABLES {
-	AWCC_THERMAL_TABLE_LEGACY		= 0x90,
-	AWCC_THERMAL_TABLE_USTT			= 0xA0,
+	AWCC_THERMAL_TABLE_LEGACY		= 0x9,
+	AWCC_THERMAL_TABLE_USTT			= 0xA,
 };
 
 enum awcc_thermal_profile {
@@ -445,20 +446,18 @@ const struct attribute_group wmax_deepsleep_attribute_group = {
  * Thermal Profile control
  *  - Provides thermal profile control through the Platform Profile API
  */
-static bool is_awcc_thermal_mode(u32 code)
+static bool is_awcc_thermal_profile_id(u8 code)
 {
-	if (code & AWCC_SENSOR_ID_MASK)
+	u8 table = FIELD_GET(AWCC_THERMAL_TABLE_MASK, code);
+	u8 mode = FIELD_GET(AWCC_THERMAL_MODE_MASK, code);
+
+	if (mode >= AWCC_PROFILE_LAST)
 		return false;
 
-	if ((code & AWCC_THERMAL_MODE_MASK) >= AWCC_PROFILE_LAST)
-		return false;
-
-	if ((code & AWCC_THERMAL_TABLE_MASK) == AWCC_THERMAL_TABLE_LEGACY &&
-	    (code & AWCC_THERMAL_MODE_MASK) >= AWCC_PROFILE_LEGACY_QUIET)
+	if (table == AWCC_THERMAL_TABLE_LEGACY && mode >= AWCC_PROFILE_LEGACY_QUIET)
 		return true;
 
-	if ((code & AWCC_THERMAL_TABLE_MASK) == AWCC_THERMAL_TABLE_USTT &&
-	    (code & AWCC_THERMAL_MODE_MASK) <= AWCC_PROFILE_USTT_LOW_POWER)
+	if (table == AWCC_THERMAL_TABLE_USTT && mode <= AWCC_PROFILE_USTT_LOW_POWER)
 		return true;
 
 	return false;
@@ -548,7 +547,7 @@ static int awcc_platform_profile_get(struct device *dev,
 		return 0;
 	}
 
-	if (!is_awcc_thermal_mode(out_data))
+	if (!is_awcc_thermal_profile_id(out_data))
 		return -ENODATA;
 
 	out_data &= AWCC_THERMAL_MODE_MASK;
@@ -597,6 +596,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
 	u32 first_mode;
 	u32 out_data;
 	int ret;
+	u8 id;
 
 	ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_SYSTEM_DESCRIPTION,
 				       0, (u32 *) &sys_desc);
@@ -615,12 +615,13 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
 		if (ret == -EBADRQC)
 			break;
 
-		if (!is_awcc_thermal_mode(out_data))
+		id = FIELD_GET(AWCC_RESOURCE_ID_MASK, out_data);
+		if (!is_awcc_thermal_profile_id(id))
 			continue;
 
-		mode = out_data & AWCC_THERMAL_MODE_MASK;
+		mode = FIELD_GET(AWCC_THERMAL_MODE_MASK, id);
 		profile = awcc_mode_to_platform_profile[mode];
-		priv->supported_thermal_profiles[profile] = out_data;
+		priv->supported_thermal_profiles[profile] = id;
 
 		set_bit(profile, choices);
 	}

-- 
2.48.1
Re: [PATCH v6 02/12] platform/x86: alienware-wmi-wmax: Refactor is_awcc_thermal_mode()
Posted by Ilpo Järvinen 8 months, 3 weeks ago
On Thu, 13 Mar 2025, Kurt Borja wrote:

> Refactor is_awcc_thermal_mode() to use FIELD_GET() instead of bitwise
> operations. Drop the check for BIT(8) sensor flag and rename it to
> is_awcc_thermal_profile_id().
> 
> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
>  drivers/platform/x86/dell/alienware-wmi-wmax.c | 33 +++++++++++++-------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> index ed70e12d73d7fe5d89f3364c5367820bf47e3c1e..80aefba5b22d6b4ac18aeb2ca356f8c911150abd 100644
> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> @@ -34,7 +34,8 @@
>  #define AWCC_FAILURE_CODE			0xFFFFFFFF
>  #define AWCC_THERMAL_TABLE_MASK			GENMASK(7, 4)
>  #define AWCC_THERMAL_MODE_MASK			GENMASK(3, 0)
> -#define AWCC_SENSOR_ID_MASK			BIT(8)

I'd prefer leaving this in place even if it now becomes unused, or 
actually I suggest you move it before the GENMASKs to keep the values in 
order, and leave an empty row between failure code and these id related 
masks.

> +/* Some IDs have a BIT(8) flag that we ignore */
> +#define AWCC_RESOURCE_ID_MASK			GENMASK(7, 0)

(AWCC_THERMAL_TABLE_MASK | AWCC_THERMAL_MODE_MASK) ?
  
>  static bool force_platform_profile;
>  module_param_unsafe(force_platform_profile, bool, 0);
> @@ -168,8 +169,8 @@ enum AWCC_GAME_SHIFT_STATUS_OPERATIONS {
>  };
>  
>  enum AWCC_THERMAL_TABLES {
> -	AWCC_THERMAL_TABLE_LEGACY		= 0x90,
> -	AWCC_THERMAL_TABLE_USTT			= 0xA0,
> +	AWCC_THERMAL_TABLE_LEGACY		= 0x9,
> +	AWCC_THERMAL_TABLE_USTT			= 0xA,
>  };
>  
>  enum awcc_thermal_profile {
> @@ -445,20 +446,18 @@ const struct attribute_group wmax_deepsleep_attribute_group = {
>   * Thermal Profile control
>   *  - Provides thermal profile control through the Platform Profile API
>   */
> -static bool is_awcc_thermal_mode(u32 code)
> +static bool is_awcc_thermal_profile_id(u8 code)
>  {
> -	if (code & AWCC_SENSOR_ID_MASK)
> +	u8 table = FIELD_GET(AWCC_THERMAL_TABLE_MASK, code);
> +	u8 mode = FIELD_GET(AWCC_THERMAL_MODE_MASK, code);
> +
> +	if (mode >= AWCC_PROFILE_LAST)
>  		return false;
>  
> -	if ((code & AWCC_THERMAL_MODE_MASK) >= AWCC_PROFILE_LAST)
> -		return false;
> -
> -	if ((code & AWCC_THERMAL_TABLE_MASK) == AWCC_THERMAL_TABLE_LEGACY &&
> -	    (code & AWCC_THERMAL_MODE_MASK) >= AWCC_PROFILE_LEGACY_QUIET)
> +	if (table == AWCC_THERMAL_TABLE_LEGACY && mode >= AWCC_PROFILE_LEGACY_QUIET)
>  		return true;
>  
> -	if ((code & AWCC_THERMAL_TABLE_MASK) == AWCC_THERMAL_TABLE_USTT &&
> -	    (code & AWCC_THERMAL_MODE_MASK) <= AWCC_PROFILE_USTT_LOW_POWER)
> +	if (table == AWCC_THERMAL_TABLE_USTT && mode <= AWCC_PROFILE_USTT_LOW_POWER)
>  		return true;
>  
>  	return false;
> @@ -548,7 +547,7 @@ static int awcc_platform_profile_get(struct device *dev,
>  		return 0;
>  	}
>  
> -	if (!is_awcc_thermal_mode(out_data))
> +	if (!is_awcc_thermal_profile_id(out_data))
>  		return -ENODATA;
>  
>  	out_data &= AWCC_THERMAL_MODE_MASK;
> @@ -597,6 +596,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
>  	u32 first_mode;
>  	u32 out_data;
>  	int ret;
> +	u8 id;
>  
>  	ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_SYSTEM_DESCRIPTION,
>  				       0, (u32 *) &sys_desc);
> @@ -615,12 +615,13 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
>  		if (ret == -EBADRQC)
>  			break;
>  
> -		if (!is_awcc_thermal_mode(out_data))
> +		id = FIELD_GET(AWCC_RESOURCE_ID_MASK, out_data);
> +		if (!is_awcc_thermal_profile_id(id))
>  			continue;
>  
> -		mode = out_data & AWCC_THERMAL_MODE_MASK;
> +		mode = FIELD_GET(AWCC_THERMAL_MODE_MASK, id);
>  		profile = awcc_mode_to_platform_profile[mode];
> -		priv->supported_thermal_profiles[profile] = out_data;
> +		priv->supported_thermal_profiles[profile] = id;
>  
>  		set_bit(profile, choices);
>  	}

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.
Re: [PATCH v6 02/12] platform/x86: alienware-wmi-wmax: Refactor is_awcc_thermal_mode()
Posted by Kurt Borja 8 months, 3 weeks ago
On Fri Mar 28, 2025 at 11:17 AM -03, Ilpo Järvinen wrote:
> On Thu, 13 Mar 2025, Kurt Borja wrote:
>
>> Refactor is_awcc_thermal_mode() to use FIELD_GET() instead of bitwise
>> operations. Drop the check for BIT(8) sensor flag and rename it to
>> is_awcc_thermal_profile_id().
>> 
>> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>> ---
>>  drivers/platform/x86/dell/alienware-wmi-wmax.c | 33 +++++++++++++-------------
>>  1 file changed, 17 insertions(+), 16 deletions(-)
>> 
>> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> index ed70e12d73d7fe5d89f3364c5367820bf47e3c1e..80aefba5b22d6b4ac18aeb2ca356f8c911150abd 100644
>> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> @@ -34,7 +34,8 @@
>>  #define AWCC_FAILURE_CODE			0xFFFFFFFF
>>  #define AWCC_THERMAL_TABLE_MASK			GENMASK(7, 4)
>>  #define AWCC_THERMAL_MODE_MASK			GENMASK(3, 0)
>> -#define AWCC_SENSOR_ID_MASK			BIT(8)
>
> I'd prefer leaving this in place even if it now becomes unused, or 
> actually I suggest you move it before the GENMASKs to keep the values in 
> order, and leave an empty row between failure code and these id related 
> masks.

Sure, I'll move it to the top.

>
>> +/* Some IDs have a BIT(8) flag that we ignore */
>> +#define AWCC_RESOURCE_ID_MASK			GENMASK(7, 0)
>
> (AWCC_THERMAL_TABLE_MASK | AWCC_THERMAL_MODE_MASK) ?

I prefer not to go for this because AWCC_RESOURCE_ID_MASK will be used
for all IDs in the following patches, not just thermal ones.

I'm doing this because I found that Alienware Aurora desktops also used
the BIT(8) flag for thermal profile IDs, not just sensors.

>   
>>  static bool force_platform_profile;
>>  module_param_unsafe(force_platform_profile, bool, 0);
>> @@ -168,8 +169,8 @@ enum AWCC_GAME_SHIFT_STATUS_OPERATIONS {
>>  };
>>  
>>  enum AWCC_THERMAL_TABLES {
>> -	AWCC_THERMAL_TABLE_LEGACY		= 0x90,
>> -	AWCC_THERMAL_TABLE_USTT			= 0xA0,
>> +	AWCC_THERMAL_TABLE_LEGACY		= 0x9,
>> +	AWCC_THERMAL_TABLE_USTT			= 0xA,
>>  };
>>  
>>  enum awcc_thermal_profile {
>> @@ -445,20 +446,18 @@ const struct attribute_group wmax_deepsleep_attribute_group = {
>>   * Thermal Profile control
>>   *  - Provides thermal profile control through the Platform Profile API
>>   */
>> -static bool is_awcc_thermal_mode(u32 code)
>> +static bool is_awcc_thermal_profile_id(u8 code)
>>  {
>> -	if (code & AWCC_SENSOR_ID_MASK)
>> +	u8 table = FIELD_GET(AWCC_THERMAL_TABLE_MASK, code);
>> +	u8 mode = FIELD_GET(AWCC_THERMAL_MODE_MASK, code);
>> +
>> +	if (mode >= AWCC_PROFILE_LAST)
>>  		return false;
>>  
>> -	if ((code & AWCC_THERMAL_MODE_MASK) >= AWCC_PROFILE_LAST)
>> -		return false;
>> -
>> -	if ((code & AWCC_THERMAL_TABLE_MASK) == AWCC_THERMAL_TABLE_LEGACY &&
>> -	    (code & AWCC_THERMAL_MODE_MASK) >= AWCC_PROFILE_LEGACY_QUIET)
>> +	if (table == AWCC_THERMAL_TABLE_LEGACY && mode >= AWCC_PROFILE_LEGACY_QUIET)
>>  		return true;
>>  
>> -	if ((code & AWCC_THERMAL_TABLE_MASK) == AWCC_THERMAL_TABLE_USTT &&
>> -	    (code & AWCC_THERMAL_MODE_MASK) <= AWCC_PROFILE_USTT_LOW_POWER)
>> +	if (table == AWCC_THERMAL_TABLE_USTT && mode <= AWCC_PROFILE_USTT_LOW_POWER)
>>  		return true;
>>  
>>  	return false;
>> @@ -548,7 +547,7 @@ static int awcc_platform_profile_get(struct device *dev,
>>  		return 0;
>>  	}
>>  
>> -	if (!is_awcc_thermal_mode(out_data))
>> +	if (!is_awcc_thermal_profile_id(out_data))
>>  		return -ENODATA;
>>  
>>  	out_data &= AWCC_THERMAL_MODE_MASK;
>> @@ -597,6 +596,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
>>  	u32 first_mode;
>>  	u32 out_data;
>>  	int ret;
>> +	u8 id;
>>  
>>  	ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_SYSTEM_DESCRIPTION,
>>  				       0, (u32 *) &sys_desc);
>> @@ -615,12 +615,13 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
>>  		if (ret == -EBADRQC)
>>  			break;
>>  
>> -		if (!is_awcc_thermal_mode(out_data))
>> +		id = FIELD_GET(AWCC_RESOURCE_ID_MASK, out_data);
>> +		if (!is_awcc_thermal_profile_id(id))
>>  			continue;
>>  
>> -		mode = out_data & AWCC_THERMAL_MODE_MASK;
>> +		mode = FIELD_GET(AWCC_THERMAL_MODE_MASK, id);
>>  		profile = awcc_mode_to_platform_profile[mode];
>> -		priv->supported_thermal_profiles[profile] = out_data;
>> +		priv->supported_thermal_profiles[profile] = id;
>>  
>>  		set_bit(profile, choices);
>>  	}
>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Thanks!

-- 
 ~ Kurt