On Tue, 25 Feb 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().
>
> Signed-off-by: Kurt Borja <kuurtb@xxxxxxxxx>
> Reviewed-by: Armin Wolf <W_Armin@xxxxxx>
> ---
> .../platform/x86/dell/alienware-wmi-wmax.c | 31 ++++++++++---------
> 1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> index ed70e12d73d7..7f0aa88221d6 100644
> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> @@ -34,7 +34,7 @@
> #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)
> +#define AWCC_RESOURCE_ID_MASK GENMASK(7, 0)
>
> static bool force_platform_profile;
> module_param_unsafe(force_platform_profile, bool, 0);
> @@ -168,8 +168,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 +445,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)
> - return false;
> + u8 table = FIELD_GET(AWCC_THERMAL_TABLE_MASK, code);
> + u8 mode = FIELD_GET(AWCC_THERMAL_MODE_MASK, code);
>
> - if ((code & AWCC_THERMAL_MODE_MASK) >= AWCC_PROFILE_LAST)
> + if (mode >= 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 +546,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 +595,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 +614,14 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
> if (ret == -EBADRQC)
> break;
>
> - if (!is_awcc_thermal_mode(out_data))
> + /* Some IDs have a BIT(8) flag that should be ignored */
I find the place of this comment odd. If you want to keep it, it should be
next to the GENMASK() as in this place I'm trying to find the code that
implements what you commented but its nowhere near here.
> + 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);
> }
>
--
i.
