Inline all AWCC WMI helper methods and directly return the newly
introduced __awcc_wmi_command() helper to simplify implementation.
Drop awcc_thermal_control() in favor of awcc_op_activate_profile().
Add awcc_op_get_resource_id(), awcc_op_get_current_profile() and a new
failure code.
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
drivers/platform/x86/dell/alienware-wmi-wmax.c | 150 +++++++++++++++----------
1 file changed, 91 insertions(+), 59 deletions(-)
diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
index 80aefba5b22d6b4ac18aeb2ca356f8c911150abd..b9dbfdc8096c571722b3c7ac3e73989e235e2eb9 100644
--- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
+++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
@@ -32,6 +32,7 @@
#define AWCC_THERMAL_MODE_GMODE 0xAB
#define AWCC_FAILURE_CODE 0xFFFFFFFF
+#define AWCC_FAILURE_CODE_2 0xFFFFFFFE
#define AWCC_THERMAL_TABLE_MASK GENMASK(7, 4)
#define AWCC_THERMAL_MODE_MASK GENMASK(3, 0)
/* Some IDs have a BIT(8) flag that we ignore */
@@ -443,8 +444,7 @@ const struct attribute_group wmax_deepsleep_attribute_group = {
};
/*
- * Thermal Profile control
- * - Provides thermal profile control through the Platform Profile API
+ * AWCC Helpers
*/
static bool is_awcc_thermal_profile_id(u8 code)
{
@@ -463,72 +463,107 @@ static bool is_awcc_thermal_profile_id(u8 code)
return false;
}
-static int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
- u8 arg, u32 *out_data)
+static int __awcc_wmi_command(struct wmi_device *wdev, u32 method_id,
+ struct wmax_u32_args *args, u32 *out)
{
- struct wmax_u32_args in_args = {
+ int ret;
+
+ ret = alienware_wmi_command(wdev, method_id, args, sizeof(*args), out);
+ if (ret)
+ return ret;
+
+ if (*out == AWCC_FAILURE_CODE || *out == AWCC_FAILURE_CODE_2)
+ return -EBADRQC;
+
+ return 0;
+}
+
+static inline int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
+ u8 arg, u32 *out)
+{
+ struct wmax_u32_args args = {
.operation = operation,
.arg1 = arg,
.arg2 = 0,
.arg3 = 0,
};
- int ret;
- ret = alienware_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION,
- &in_args, sizeof(in_args), out_data);
- if (ret < 0)
- return ret;
-
- if (*out_data == AWCC_FAILURE_CODE)
- return -EBADRQC;
-
- return 0;
+ return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
}
-static int awcc_thermal_control(struct wmi_device *wdev, u8 profile)
+static inline int awcc_game_shift_status(struct wmi_device *wdev, u8 operation,
+ u32 *out)
{
- struct wmax_u32_args in_args = {
- .operation = AWCC_OP_ACTIVATE_PROFILE,
- .arg1 = profile,
- .arg2 = 0,
- .arg3 = 0,
- };
- u32 out_data;
- int ret;
-
- ret = alienware_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL,
- &in_args, sizeof(in_args), &out_data);
- if (ret)
- return ret;
-
- if (out_data == AWCC_FAILURE_CODE)
- return -EBADRQC;
-
- return 0;
-}
-
-static int awcc_game_shift_status(struct wmi_device *wdev, u8 operation,
- u32 *out_data)
-{
- struct wmax_u32_args in_args = {
+ struct wmax_u32_args args = {
.operation = operation,
.arg1 = 0,
.arg2 = 0,
.arg3 = 0,
};
- int ret;
- ret = alienware_wmi_command(wdev, AWCC_METHOD_GAME_SHIFT_STATUS,
- &in_args, sizeof(in_args), out_data);
- if (ret < 0)
- return ret;
-
- if (*out_data == AWCC_FAILURE_CODE)
- return -EOPNOTSUPP;
-
- return 0;
+ return __awcc_wmi_command(wdev, AWCC_METHOD_GAME_SHIFT_STATUS, &args, out);
}
+/**
+ * awcc_op_get_resource_id - Get the resource ID at a given index
+ * @wdev: AWCC WMI device
+ * @index: Index
+ * @out: Value returned by the WMI call
+ *
+ * Get the resource ID at a given index. Resource IDs are listed in the
+ * following order:
+ *
+ * - Fan IDs
+ * - Sensor IDs
+ * - Unknown IDs
+ * - Thermal Profile IDs
+ *
+ * The total number of IDs of a given type can be obtained with
+ * AWCC_OP_GET_SYSTEM_DESCRIPTION.
+ *
+ * Return: 0 on success, -errno on failure
+ */
+static inline int awcc_op_get_resource_id(struct wmi_device *wdev, u8 index, u32 *out)
+{
+ struct wmax_u32_args args = {
+ .operation = AWCC_OP_GET_RESOURCE_ID,
+ .arg1 = index,
+ .arg2 = 0,
+ .arg3 = 0,
+ };
+
+ return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
+}
+
+static inline int awcc_op_get_current_profile(struct wmi_device *wdev, u32 *out)
+{
+ struct wmax_u32_args args = {
+ .operation = AWCC_OP_GET_CURRENT_PROFILE,
+ .arg1 = 0,
+ .arg2 = 0,
+ .arg3 = 0,
+ };
+
+ return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
+}
+
+static inline int awcc_op_activate_profile(struct wmi_device *wdev, u8 profile)
+{
+ struct wmax_u32_args args = {
+ .operation = AWCC_OP_ACTIVATE_PROFILE,
+ .arg1 = profile,
+ .arg2 = 0,
+ .arg3 = 0,
+ };
+ u32 out;
+
+ return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL, &args, &out);
+}
+
+/*
+ * Thermal Profile control
+ * - Provides thermal profile control through the Platform Profile API
+ */
static int awcc_platform_profile_get(struct device *dev,
enum platform_profile_option *profile)
{
@@ -536,10 +571,8 @@ static int awcc_platform_profile_get(struct device *dev,
u32 out_data;
int ret;
- ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_CURRENT_PROFILE,
- 0, &out_data);
-
- if (ret < 0)
+ ret = awcc_op_get_current_profile(priv->wdev, &out_data);
+ if (ret)
return ret;
if (out_data == AWCC_THERMAL_MODE_GMODE) {
@@ -550,7 +583,7 @@ static int awcc_platform_profile_get(struct device *dev,
if (!is_awcc_thermal_profile_id(out_data))
return -ENODATA;
- out_data &= AWCC_THERMAL_MODE_MASK;
+ out_data = FIELD_GET(AWCC_THERMAL_MODE_MASK, out_data);
*profile = awcc_mode_to_platform_profile[out_data];
return 0;
@@ -583,8 +616,8 @@ static int awcc_platform_profile_set(struct device *dev,
}
}
- return awcc_thermal_control(priv->wdev,
- priv->supported_thermal_profiles[profile]);
+ return awcc_op_activate_profile(priv->wdev,
+ priv->supported_thermal_profiles[profile]);
}
static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
@@ -606,8 +639,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
first_mode = sys_desc[0] + sys_desc[1];
for (u32 i = 0; i < sys_desc[3]; i++) {
- ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_RESOURCE_ID,
- i + first_mode, &out_data);
+ ret = awcc_op_get_resource_id(priv->wdev, i + first_mode, &out_data);
if (ret == -EIO)
return ret;
--
2.48.1On Thu, 13 Mar 2025, Kurt Borja wrote:
> Inline all AWCC WMI helper methods and directly return the newly
> introduced __awcc_wmi_command() helper to simplify implementation.
>
> Drop awcc_thermal_control() in favor of awcc_op_activate_profile().
>
> Add awcc_op_get_resource_id(), awcc_op_get_current_profile() and a new
> failure code.
>
> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
> drivers/platform/x86/dell/alienware-wmi-wmax.c | 150 +++++++++++++++----------
> 1 file changed, 91 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> index 80aefba5b22d6b4ac18aeb2ca356f8c911150abd..b9dbfdc8096c571722b3c7ac3e73989e235e2eb9 100644
> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> @@ -32,6 +32,7 @@
> #define AWCC_THERMAL_MODE_GMODE 0xAB
>
> #define AWCC_FAILURE_CODE 0xFFFFFFFF
> +#define AWCC_FAILURE_CODE_2 0xFFFFFFFE
> #define AWCC_THERMAL_TABLE_MASK GENMASK(7, 4)
> #define AWCC_THERMAL_MODE_MASK GENMASK(3, 0)
> /* Some IDs have a BIT(8) flag that we ignore */
> @@ -443,8 +444,7 @@ const struct attribute_group wmax_deepsleep_attribute_group = {
> };
>
> /*
> - * Thermal Profile control
> - * - Provides thermal profile control through the Platform Profile API
> + * AWCC Helpers
> */
> static bool is_awcc_thermal_profile_id(u8 code)
> {
> @@ -463,72 +463,107 @@ static bool is_awcc_thermal_profile_id(u8 code)
> return false;
> }
>
> -static int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
> - u8 arg, u32 *out_data)
> +static int __awcc_wmi_command(struct wmi_device *wdev, u32 method_id,
> + struct wmax_u32_args *args, u32 *out)
Why did you put __ into the name?
Some people oppose __ prefix altogether, I don't entirely agree with their
stance but I don't really understand what the underscores here signify.
Normally I see __ being used in three main cases:
- non-__ variant does some locking around the call too the __ func (though
many funcs use _locked postfix these days).
- func is "dangerous" and caller has to really know what he/she is
doing / be careful for some reason.
- non-__ variant exists and somebody cannot come up better name for the
internally called function (not very good use case, IMHO)
I don't see any of those apply here, this looks just an ordinary wrapper
function, thus the question.
> {
> - struct wmax_u32_args in_args = {
> + int ret;
> +
> + ret = alienware_wmi_command(wdev, method_id, args, sizeof(*args), out);
> + if (ret)
> + return ret;
> +
> + if (*out == AWCC_FAILURE_CODE || *out == AWCC_FAILURE_CODE_2)
> + return -EBADRQC;
> +
> + return 0;
> +}
> +
> +static inline int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
> + u8 arg, u32 *out)
> +{
> + struct wmax_u32_args args = {
> .operation = operation,
> .arg1 = arg,
> .arg2 = 0,
> .arg3 = 0,
> };
> - int ret;
>
> - ret = alienware_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION,
> - &in_args, sizeof(in_args), out_data);
> - if (ret < 0)
> - return ret;
> -
> - if (*out_data == AWCC_FAILURE_CODE)
> - return -EBADRQC;
> -
> - return 0;
> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
> }
>
> -static int awcc_thermal_control(struct wmi_device *wdev, u8 profile)
> +static inline int awcc_game_shift_status(struct wmi_device *wdev, u8 operation,
As a general rule, don't add inline to .c files, we leave figuring
inlining out to the compiler.
> + u32 *out)
> {
> - struct wmax_u32_args in_args = {
> - .operation = AWCC_OP_ACTIVATE_PROFILE,
> - .arg1 = profile,
> - .arg2 = 0,
> - .arg3 = 0,
> - };
> - u32 out_data;
> - int ret;
> -
> - ret = alienware_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL,
> - &in_args, sizeof(in_args), &out_data);
> - if (ret)
> - return ret;
> -
> - if (out_data == AWCC_FAILURE_CODE)
> - return -EBADRQC;
> -
> - return 0;
> -}
> -
> -static int awcc_game_shift_status(struct wmi_device *wdev, u8 operation,
> - u32 *out_data)
> -{
> - struct wmax_u32_args in_args = {
> + struct wmax_u32_args args = {
> .operation = operation,
> .arg1 = 0,
> .arg2 = 0,
> .arg3 = 0,
> };
> - int ret;
>
> - ret = alienware_wmi_command(wdev, AWCC_METHOD_GAME_SHIFT_STATUS,
> - &in_args, sizeof(in_args), out_data);
> - if (ret < 0)
> - return ret;
> -
> - if (*out_data == AWCC_FAILURE_CODE)
> - return -EOPNOTSUPP;
> -
> - return 0;
> + return __awcc_wmi_command(wdev, AWCC_METHOD_GAME_SHIFT_STATUS, &args, out);
> }
>
> +/**
> + * awcc_op_get_resource_id - Get the resource ID at a given index
> + * @wdev: AWCC WMI device
> + * @index: Index
> + * @out: Value returned by the WMI call
> + *
> + * Get the resource ID at a given index. Resource IDs are listed in the
Use @index to refer to the argument.
> + * following order:
> + *
> + * - Fan IDs
> + * - Sensor IDs
> + * - Unknown IDs
> + * - Thermal Profile IDs
> + *
> + * The total number of IDs of a given type can be obtained with
> + * AWCC_OP_GET_SYSTEM_DESCRIPTION.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +static inline int awcc_op_get_resource_id(struct wmi_device *wdev, u8 index, u32 *out)
> +{
> + struct wmax_u32_args args = {
> + .operation = AWCC_OP_GET_RESOURCE_ID,
> + .arg1 = index,
> + .arg2 = 0,
> + .arg3 = 0,
> + };
> +
> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
> +}
> +
> +static inline int awcc_op_get_current_profile(struct wmi_device *wdev, u32 *out)
> +{
> + struct wmax_u32_args args = {
> + .operation = AWCC_OP_GET_CURRENT_PROFILE,
> + .arg1 = 0,
> + .arg2 = 0,
> + .arg3 = 0,
> + };
> +
> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
> +}
> +
> +static inline int awcc_op_activate_profile(struct wmi_device *wdev, u8 profile)
> +{
> + struct wmax_u32_args args = {
> + .operation = AWCC_OP_ACTIVATE_PROFILE,
> + .arg1 = profile,
> + .arg2 = 0,
> + .arg3 = 0,
> + };
> + u32 out;
> +
> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL, &args, &out);
> +}
> +
> +/*
> + * Thermal Profile control
> + * - Provides thermal profile control through the Platform Profile API
> + */
> static int awcc_platform_profile_get(struct device *dev,
> enum platform_profile_option *profile)
> {
> @@ -536,10 +571,8 @@ static int awcc_platform_profile_get(struct device *dev,
> u32 out_data;
> int ret;
>
> - ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_CURRENT_PROFILE,
> - 0, &out_data);
> -
> - if (ret < 0)
> + ret = awcc_op_get_current_profile(priv->wdev, &out_data);
> + if (ret)
> return ret;
>
> if (out_data == AWCC_THERMAL_MODE_GMODE) {
> @@ -550,7 +583,7 @@ static int awcc_platform_profile_get(struct device *dev,
> if (!is_awcc_thermal_profile_id(out_data))
> return -ENODATA;
>
> - out_data &= AWCC_THERMAL_MODE_MASK;
> + out_data = FIELD_GET(AWCC_THERMAL_MODE_MASK, out_data);
Should this be part of the earlier patch??
> *profile = awcc_mode_to_platform_profile[out_data];
>
> return 0;
> @@ -583,8 +616,8 @@ static int awcc_platform_profile_set(struct device *dev,
> }
> }
>
> - return awcc_thermal_control(priv->wdev,
> - priv->supported_thermal_profiles[profile]);
> + return awcc_op_activate_profile(priv->wdev,
> + priv->supported_thermal_profiles[profile]);
> }
>
> static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
> @@ -606,8 +639,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
> first_mode = sys_desc[0] + sys_desc[1];
>
> for (u32 i = 0; i < sys_desc[3]; i++) {
> - ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_RESOURCE_ID,
> - i + first_mode, &out_data);
> + ret = awcc_op_get_resource_id(priv->wdev, i + first_mode, &out_data);
>
> if (ret == -EIO)
> return ret;
>
>
--
i.
On Fri Mar 28, 2025 at 11:51 AM -03, Ilpo Järvinen wrote:
> On Thu, 13 Mar 2025, Kurt Borja wrote:
>
>> Inline all AWCC WMI helper methods and directly return the newly
>> introduced __awcc_wmi_command() helper to simplify implementation.
>>
>> Drop awcc_thermal_control() in favor of awcc_op_activate_profile().
>>
>> Add awcc_op_get_resource_id(), awcc_op_get_current_profile() and a new
>> failure code.
>>
>> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>> ---
>> drivers/platform/x86/dell/alienware-wmi-wmax.c | 150 +++++++++++++++----------
>> 1 file changed, 91 insertions(+), 59 deletions(-)
>>
>> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> index 80aefba5b22d6b4ac18aeb2ca356f8c911150abd..b9dbfdc8096c571722b3c7ac3e73989e235e2eb9 100644
>> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> @@ -32,6 +32,7 @@
>> #define AWCC_THERMAL_MODE_GMODE 0xAB
>>
>> #define AWCC_FAILURE_CODE 0xFFFFFFFF
>> +#define AWCC_FAILURE_CODE_2 0xFFFFFFFE
>> #define AWCC_THERMAL_TABLE_MASK GENMASK(7, 4)
>> #define AWCC_THERMAL_MODE_MASK GENMASK(3, 0)
>> /* Some IDs have a BIT(8) flag that we ignore */
>> @@ -443,8 +444,7 @@ const struct attribute_group wmax_deepsleep_attribute_group = {
>> };
>>
>> /*
>> - * Thermal Profile control
>> - * - Provides thermal profile control through the Platform Profile API
>> + * AWCC Helpers
>> */
>> static bool is_awcc_thermal_profile_id(u8 code)
>> {
>> @@ -463,72 +463,107 @@ static bool is_awcc_thermal_profile_id(u8 code)
>> return false;
>> }
>>
>> -static int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
>> - u8 arg, u32 *out_data)
>> +static int __awcc_wmi_command(struct wmi_device *wdev, u32 method_id,
>> + struct wmax_u32_args *args, u32 *out)
>
> Why did you put __ into the name?
>
> Some people oppose __ prefix altogether, I don't entirely agree with their
> stance but I don't really understand what the underscores here signify.
>
> Normally I see __ being used in three main cases:
> - non-__ variant does some locking around the call too the __ func (though
> many funcs use _locked postfix these days).
> - func is "dangerous" and caller has to really know what he/she is
> doing / be careful for some reason.
> - non-__ variant exists and somebody cannot come up better name for the
> internally called function (not very good use case, IMHO)
Oh - I didn't know. Andy also mentioned this prefix is used for
non-atomic versions of some functions like __set_bit().
>
> I don't see any of those apply here, this looks just an ordinary wrapper
> function, thus the question.
I did it to indicate it's a "private" method, so it's not used directly.
But ofc that's not the convention here, so I'll drop it.
>
>> {
>> - struct wmax_u32_args in_args = {
>> + int ret;
>> +
>> + ret = alienware_wmi_command(wdev, method_id, args, sizeof(*args), out);
>> + if (ret)
>> + return ret;
>> +
>> + if (*out == AWCC_FAILURE_CODE || *out == AWCC_FAILURE_CODE_2)
>> + return -EBADRQC;
>> +
>> + return 0;
>> +}
>> +
>> +static inline int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
>> + u8 arg, u32 *out)
>> +{
>> + struct wmax_u32_args args = {
>> .operation = operation,
>> .arg1 = arg,
>> .arg2 = 0,
>> .arg3 = 0,
>> };
>> - int ret;
>>
>> - ret = alienware_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION,
>> - &in_args, sizeof(in_args), out_data);
>> - if (ret < 0)
>> - return ret;
>> -
>> - if (*out_data == AWCC_FAILURE_CODE)
>> - return -EBADRQC;
>> -
>> - return 0;
>> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
>> }
>>
>> -static int awcc_thermal_control(struct wmi_device *wdev, u8 profile)
>> +static inline int awcc_game_shift_status(struct wmi_device *wdev, u8 operation,
>
> As a general rule, don't add inline to .c files, we leave figuring
> inlining out to the compiler.
Good to know, thanks!
>
>> + u32 *out)
>> {
>> - struct wmax_u32_args in_args = {
>> - .operation = AWCC_OP_ACTIVATE_PROFILE,
>> - .arg1 = profile,
>> - .arg2 = 0,
>> - .arg3 = 0,
>> - };
>> - u32 out_data;
>> - int ret;
>> -
>> - ret = alienware_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL,
>> - &in_args, sizeof(in_args), &out_data);
>> - if (ret)
>> - return ret;
>> -
>> - if (out_data == AWCC_FAILURE_CODE)
>> - return -EBADRQC;
>> -
>> - return 0;
>> -}
>> -
>> -static int awcc_game_shift_status(struct wmi_device *wdev, u8 operation,
>> - u32 *out_data)
>> -{
>> - struct wmax_u32_args in_args = {
>> + struct wmax_u32_args args = {
>> .operation = operation,
>> .arg1 = 0,
>> .arg2 = 0,
>> .arg3 = 0,
>> };
>> - int ret;
>>
>> - ret = alienware_wmi_command(wdev, AWCC_METHOD_GAME_SHIFT_STATUS,
>> - &in_args, sizeof(in_args), out_data);
>> - if (ret < 0)
>> - return ret;
>> -
>> - if (*out_data == AWCC_FAILURE_CODE)
>> - return -EOPNOTSUPP;
>> -
>> - return 0;
>> + return __awcc_wmi_command(wdev, AWCC_METHOD_GAME_SHIFT_STATUS, &args, out);
>> }
>>
>> +/**
>> + * awcc_op_get_resource_id - Get the resource ID at a given index
>> + * @wdev: AWCC WMI device
>> + * @index: Index
>> + * @out: Value returned by the WMI call
>> + *
>> + * Get the resource ID at a given index. Resource IDs are listed in the
>
> Use @index to refer to the argument.
Ack
>
>> + * following order:
>> + *
>> + * - Fan IDs
>> + * - Sensor IDs
>> + * - Unknown IDs
>> + * - Thermal Profile IDs
>> + *
>> + * The total number of IDs of a given type can be obtained with
>> + * AWCC_OP_GET_SYSTEM_DESCRIPTION.
>> + *
>> + * Return: 0 on success, -errno on failure
>> + */
>> +static inline int awcc_op_get_resource_id(struct wmi_device *wdev, u8 index, u32 *out)
>> +{
>> + struct wmax_u32_args args = {
>> + .operation = AWCC_OP_GET_RESOURCE_ID,
>> + .arg1 = index,
>> + .arg2 = 0,
>> + .arg3 = 0,
>> + };
>> +
>> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
>> +}
>> +
>> +static inline int awcc_op_get_current_profile(struct wmi_device *wdev, u32 *out)
>> +{
>> + struct wmax_u32_args args = {
>> + .operation = AWCC_OP_GET_CURRENT_PROFILE,
>> + .arg1 = 0,
>> + .arg2 = 0,
>> + .arg3 = 0,
>> + };
>> +
>> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
>> +}
>> +
>> +static inline int awcc_op_activate_profile(struct wmi_device *wdev, u8 profile)
>> +{
>> + struct wmax_u32_args args = {
>> + .operation = AWCC_OP_ACTIVATE_PROFILE,
>> + .arg1 = profile,
>> + .arg2 = 0,
>> + .arg3 = 0,
>> + };
>> + u32 out;
>> +
>> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL, &args, &out);
>> +}
>> +
>> +/*
>> + * Thermal Profile control
>> + * - Provides thermal profile control through the Platform Profile API
>> + */
>> static int awcc_platform_profile_get(struct device *dev,
>> enum platform_profile_option *profile)
>> {
>> @@ -536,10 +571,8 @@ static int awcc_platform_profile_get(struct device *dev,
>> u32 out_data;
>> int ret;
>>
>> - ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_CURRENT_PROFILE,
>> - 0, &out_data);
>> -
>> - if (ret < 0)
>> + ret = awcc_op_get_current_profile(priv->wdev, &out_data);
>> + if (ret)
>> return ret;
>>
>> if (out_data == AWCC_THERMAL_MODE_GMODE) {
>> @@ -550,7 +583,7 @@ static int awcc_platform_profile_get(struct device *dev,
>> if (!is_awcc_thermal_profile_id(out_data))
>> return -ENODATA;
>>
>> - out_data &= AWCC_THERMAL_MODE_MASK;
>> + out_data = FIELD_GET(AWCC_THERMAL_MODE_MASK, out_data);
>
> Should this be part of the earlier patch??
I think I'll leave it in this patch, but I'll move it to
awcc_op_get_resource_id().
--
~ Kurt
>
>> *profile = awcc_mode_to_platform_profile[out_data];
>>
>> return 0;
>> @@ -583,8 +616,8 @@ static int awcc_platform_profile_set(struct device *dev,
>> }
>> }
>>
>> - return awcc_thermal_control(priv->wdev,
>> - priv->supported_thermal_profiles[profile]);
>> + return awcc_op_activate_profile(priv->wdev,
>> + priv->supported_thermal_profiles[profile]);
>> }
>>
>> static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
>> @@ -606,8 +639,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
>> first_mode = sys_desc[0] + sys_desc[1];
>>
>> for (u32 i = 0; i < sys_desc[3]; i++) {
>> - ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_RESOURCE_ID,
>> - i + first_mode, &out_data);
>> + ret = awcc_op_get_resource_id(priv->wdev, i + first_mode, &out_data);
>>
>> if (ret == -EIO)
>> return ret;
>>
>>
On Fri, 28 Mar 2025, Kurt Borja wrote:
> On Fri Mar 28, 2025 at 11:51 AM -03, Ilpo Järvinen wrote:
> > On Thu, 13 Mar 2025, Kurt Borja wrote:
> >
> >> Inline all AWCC WMI helper methods and directly return the newly
> >> introduced __awcc_wmi_command() helper to simplify implementation.
> >>
> >> Drop awcc_thermal_control() in favor of awcc_op_activate_profile().
> >>
> >> Add awcc_op_get_resource_id(), awcc_op_get_current_profile() and a new
> >> failure code.
> >>
> >> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> >> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> >> ---
> >> drivers/platform/x86/dell/alienware-wmi-wmax.c | 150 +++++++++++++++----------
> >> 1 file changed, 91 insertions(+), 59 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> >> index 80aefba5b22d6b4ac18aeb2ca356f8c911150abd..b9dbfdc8096c571722b3c7ac3e73989e235e2eb9 100644
> >> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
> >> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> >> @@ -32,6 +32,7 @@
> >> #define AWCC_THERMAL_MODE_GMODE 0xAB
> >>
> >> #define AWCC_FAILURE_CODE 0xFFFFFFFF
> >> +#define AWCC_FAILURE_CODE_2 0xFFFFFFFE
> >> #define AWCC_THERMAL_TABLE_MASK GENMASK(7, 4)
> >> #define AWCC_THERMAL_MODE_MASK GENMASK(3, 0)
> >> /* Some IDs have a BIT(8) flag that we ignore */
> >> @@ -443,8 +444,7 @@ const struct attribute_group wmax_deepsleep_attribute_group = {
> >> };
> >>
> >> /*
> >> - * Thermal Profile control
> >> - * - Provides thermal profile control through the Platform Profile API
> >> + * AWCC Helpers
> >> */
> >> static bool is_awcc_thermal_profile_id(u8 code)
> >> {
> >> @@ -463,72 +463,107 @@ static bool is_awcc_thermal_profile_id(u8 code)
> >> return false;
> >> }
> >>
> >> -static int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
> >> - u8 arg, u32 *out_data)
> >> +static int __awcc_wmi_command(struct wmi_device *wdev, u32 method_id,
> >> + struct wmax_u32_args *args, u32 *out)
> >
> > Why did you put __ into the name?
> >
> > Some people oppose __ prefix altogether, I don't entirely agree with their
> > stance but I don't really understand what the underscores here signify.
> >
> > Normally I see __ being used in three main cases:
> > - non-__ variant does some locking around the call too the __ func (though
> > many funcs use _locked postfix these days).
> > - func is "dangerous" and caller has to really know what he/she is
> > doing / be careful for some reason.
> > - non-__ variant exists and somebody cannot come up better name for the
> > internally called function (not very good use case, IMHO)
>
> Oh - I didn't know. Andy also mentioned this prefix is used for
> non-atomic versions of some functions like __set_bit().
I mentioned locking which relates to atomicity, however, here you don't
have two variants, one without locking and the other with it so it becomes
very non-obvious what __ means. (I know you posted v7 without it, so
this is just a clarifying comment.)
> > I don't see any of those apply here, this looks just an ordinary wrapper
> > function, thus the question.
>
> I did it to indicate it's a "private" method, so it's not used directly.
> But ofc that's not the convention here, so I'll drop it.
We have "static" for that. And it's called by something anyway or it would
be dead code :-).
--
i.
© 2016 - 2025 Red Hat, Inc.