Re: [PATCH v2 07/10] platform/x86: alienware-wmi-wmax: Add HWMON support

Ilpo Järvinen
2025-03-04T16:31:12Z

[-- Attachment #1: Type: text/plain, Size: 7816 bytes --]

On Tue, 4 Mar 2025, Kurt Borja wrote:

> On Tue Mar 4, 2025 at 10:53 AM -05, Ilpo Järvinen wrote:
> > On Tue, 25 Feb 2025, Kurt Borja wrote:
> >
> >> All models with the "AWCC" WMAX device support monitoring fan speed and
> >> temperature sensors. Expose this feature through the HWMON interface.
> >>
> >> Sensor readings are cached for 1 second before refreshing them to
> >> mitigate the performance cost of calling WMI methods.
> >>
> >> Cc: Guenter Roeck <linux@roeck-us.net>
> >> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> >> ---
> >> drivers/platform/x86/dell/Kconfig | 1 +
> >> .../platform/x86/dell/alienware-wmi-wmax.c | 403 ++++++++++++++++++
> >> 2 files changed, 404 insertions(+)
> >>
> >> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
> >> index f8a0dffcaab7..85a57c01aaad 100644
> >> --- a/drivers/platform/x86/dell/Kconfig
> >> +++ b/drivers/platform/x86/dell/Kconfig
> >> @@ -43,6 +43,7 @@ config ALIENWARE_WMI_WMAX
> >> bool "Alienware WMAX WMI device driver"
> >> default y
> >> depends on ALIENWARE_WMI
> >> + depends on HWMON
> >> select ACPI_PLATFORM_PROFILE
> >> help
> >> Alienware WMI driver with AlienFX LED, HDMI, amplifier, deep sleep and
> >> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> >> index bbe87f91fcb6..818023a5b205 100644
> >> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
> >> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> >> @@ -9,10 +9,13 @@
> >> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> >>
> >> #include <linux/bitfield.h>
> >> +#include <linux/bitmap.h>
> >> #include <linux/bits.h>
> >> #include <linux/dmi.h>
> >> +#include <linux/hwmon.h>
> >> #include <linux/moduleparam.h>
> >> #include <linux/platform_profile.h>
> >> +#include <linux/units.h>
> >> #include <linux/wmi.h>
> >> #include "alienware-wmi.h"
> >>
> >> @@ -25,6 +28,7 @@
> >> #define WMAX_METHOD_BRIGHTNESS 0x3
> >> #define WMAX_METHOD_ZONE_CONTROL 0x4
> >>
> >> +#define AWCC_METHOD_GET_FAN_SENSORS 0x13
> >> #define AWCC_METHOD_THERMAL_INFORMATION 0x14
> >> #define AWCC_METHOD_THERMAL_CONTROL 0x15
> >> #define AWCC_METHOD_GAME_SHIFT_STATUS 0x25
> >> @@ -38,6 +42,10 @@
> >> /* Arbitrary limit based on supported models */
> >> #define AWCC_MAX_RES_COUNT 16
> >>
> >> +static bool force_hwmon;
> >> +module_param_unsafe(force_hwmon, bool, 0);
> >> +MODULE_PARM_DESC(force_hwmon, "Force probing for HWMON support without checking if the WMI backend is available");
> >> +
> >> static bool force_platform_profile;
> >> module_param_unsafe(force_platform_profile, bool, 0);
> >> MODULE_PARM_DESC(force_platform_profile, "Forces auto-detecting thermal profiles without checking if WMI thermal backend is available");
> >> @@ -47,16 +55,19 @@ module_param_unsafe(force_gmode, bool, 0);
> >> MODULE_PARM_DESC(force_gmode, "Forces G-Mode when performance profile is selected");
> >>
> >> struct awcc_quirks {
> >> + bool hwmon;
> >> bool pprof;
> >> bool gmode;
> >> };
> >>
> >> static struct awcc_quirks g_series_quirks = {
> >> + .hwmon = true,
> >> .pprof = true,
> >> .gmode = true,
> >> };
> >>
> >> static struct awcc_quirks generic_quirks = {
> >> + .hwmon = true,
> >> .pprof = true,
> >> .gmode = false,
> >> };
> >> @@ -154,9 +165,18 @@ static const struct dmi_system_id awcc_dmi_table[] __initconst = {
> >> },
> >> };
> >>
> >> +enum AWCC_GET_FAN_SENSORS_OPERATIONS {
> >> + AWCC_OP_GET_TOTAL_FAN_TEMPS = 0x01,
> >> + AWCC_OP_GET_FAN_TEMP_ID = 0x02,
> >> +};
> >> +
> >> enum AWCC_THERMAL_INFORMATION_OPERATIONS {
> >> AWCC_OP_GET_SYSTEM_DESCRIPTION = 0x02,
> >> AWCC_OP_GET_RESOURCE_ID = 0x03,
> >> + AWCC_OP_GET_TEMPERATURE = 0x04,
> >> + AWCC_OP_GET_FAN_RPM = 0x05,
> >> + AWCC_OP_GET_FAN_MIN_RPM = 0x08,
> >> + AWCC_OP_GET_FAN_MAX_RPM = 0x09,
> >> AWCC_OP_GET_CURRENT_PROFILE = 0x0B,
> >> };
> >>
> >> @@ -179,6 +199,12 @@ enum AWCC_SPECIAL_THERMAL_CODES {
> >> AWCC_SPECIAL_PROFILE_GMODE = 0xAB,
> >> };
> >>
> >> +enum AWCC_TEMP_SENSOR_TYPES {
> >> + AWCC_TEMP_SENSOR_CPU = 0x01,
> >> + AWCC_TEMP_SENSOR_GPU = 0x06,
> >> + AWCC_TEMP_SENSOR_LAST
> >> +};
> >> +
> >> enum awcc_thermal_profile {
> >> AWCC_PROFILE_USTT_BALANCED,
> >> AWCC_PROFILE_USTT_BALANCED_PERFORMANCE,
> >> @@ -215,6 +241,15 @@ struct wmax_u32_args {
> >> u8 arg3;
> >> } __packed;
> >>
> >> +struct awcc_fan_data {
> >> + unsigned long *related_temps;
> >> + unsigned long *auto_channels_temp;
> >> + u32 total_temps;
> >> + u32 min_rpm;
> >> + u32 max_rpm;
> >> + u8 id;
> >> +};
> >> +
> >> struct awcc_priv {
> >> struct wmi_device *wdev;
> >> union {
> >> @@ -230,6 +265,11 @@ struct awcc_priv {
> >>
> >> struct device *ppdev;
> >> u8 supported_profiles[PLATFORM_PROFILE_LAST];
> >> +
> >> + struct device *hwdev;
> >> + struct awcc_fan_data **fan_data;
> >> + unsigned int temp_sensors_size;
> >> + unsigned long *temp_sensors;
> >> };
> >>
> >> static const enum platform_profile_option awcc_mode_to_platform_profile[AWCC_PROFILE_LAST] = {
> >> @@ -494,6 +534,19 @@ static int __awcc_wmi_command(struct wmi_device *wdev, u32 method_id,
> >> return 0;
> >> }
> >>
> >> +static inline int awcc_get_fan_sensors(struct wmi_device *wdev, u8 operation,
> >> + u8 fan_id, u8 index, u32 *out)
> >> +{
> >> + struct wmax_u32_args args = {
> >> + .operation = operation,
> >> + .arg1 = fan_id,
> >> + .arg2 = index,
> >> + .arg3 = 0,
> >> + };
> >> +
> >> + return __awcc_wmi_command(wdev, AWCC_METHOD_GET_FAN_SENSORS, &args, out);
> >> +}
> >> +
> >> static inline int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
> >> u8 arg, u32 *out)
> >> {
> >> @@ -564,6 +617,343 @@ static inline int awcc_op_get_resource_id(struct wmi_device *wdev, u8 index, u32
> >> return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
> >> }
> >>
> >> +/*
> >> + * HWMON
> >> + * - Provides temperature and fan speed monitoring as well as manual fan
> >> + * control
> >> + */
> >> +static umode_t awcc_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> >> + u32 attr, int channel)
> >> +{
> >> + const struct awcc_priv *priv = drvdata;
> >> +
> >> + switch (type) {
> >> + case hwmon_temp:
> >> + if (channel < priv->temp_count)
> >> + return 0444;
> >> +
> >> + break;
> >
> > IMO, these could be written as:
> > return channel < priv->temp_count ? 0444 : 0;
>
> Ack.
>
> >
> >> + case hwmon_fan:
> >> + if (channel < priv->fan_count)
> >> + return 0444;
> >> +
> >> + break;
> >> + case hwmon_pwm:
> >> + if (channel < priv->fan_count)
> >> + return 0444;
> >> +
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> >> + u32 attr, int channel, long *val)
> >> +{
> >> + struct awcc_priv *priv = dev_get_drvdata(dev);
> >> + struct awcc_fan_data *fan;
> >> + u32 state;
> >> + int ret;
> >> + u8 temp;
> >> +
> >> + switch (type) {
> >> + case hwmon_temp:
> >> + temp = find_nth_bit(priv->temp_sensors, U8_MAX, channel);
> >> + if (temp >= U8_MAX)
> >
> > It cannot be larger than as its type is u8??
>
> Thanks, I forgot to change this! U8_MAX should be replaced with
> priv->temp_sensors_size.

I recall seeing another similar line somewhere else in the file so please
take a look at that as well.

> Thank you for reviewing this set! :)
>
>

--
i.