| Message ID | 20250305-hwm-v3-7-395e7a1407e2@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | platform/x86: alienware-wmi-wmax: HWMON support + DebugFS + Improvements | expand |
Am 06.03.25 um 01:56 schrieb Kurt Borja: > All models with the "AWCC" WMAX device support monitoring fan speed and > temperature sensors. Expose this feature through the HWMON interface. > > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Jean Delvare <jdelvare@suse.com> > Cc: linux-hwmon@vger.kernel.org > Signed-off-by: Kurt Borja <kuurtb@gmail.com> > --- > drivers/platform/x86/dell/Kconfig | 1 + > drivers/platform/x86/dell/alienware-wmi-wmax.c | 431 +++++++++++++++++++++++++ > 2 files changed, 432 insertions(+) > > diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig > index f8a0dffcaab7c3b423472c5b9093011334a698c8..85a57c01aaada5d899cd8252e77ed6043da5cbdf 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 71fc17e8d103146b8edf53a552ae5ba64414e873..20cf3371ee3c0e1ea038b3ca517e831f3b30dc29 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 > @@ -39,6 +43,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"); > @@ -48,16 +56,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, > }; > @@ -155,9 +166,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, > }; > > @@ -180,6 +200,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, > @@ -216,6 +242,15 @@ struct wmax_u32_args { > u8 arg3; > }; > > +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 { > @@ -231,6 +266,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] = { > @@ -495,6 +535,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) > { > @@ -552,6 +605,32 @@ 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); > } > > +static inline int awcc_op_get_fan_rpm(struct wmi_device *wdev, u8 fan_id, u32 *out) > +{ > + struct wmax_u32_args args = { > + .operation = AWCC_OP_GET_FAN_RPM, > + .arg1 = fan_id, > + .arg2 = 0, > + .arg3 = 0, > + }; > + > + > + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out); > +} > + > +static inline int awcc_op_get_temperature(struct wmi_device *wdev, u8 temp_id, u32 *out) > +{ > + struct wmax_u32_args args = { > + .operation = AWCC_OP_GET_TEMPERATURE, > + .arg1 = temp_id, > + .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 = { > @@ -599,6 +678,345 @@ static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profil > return 0; > } > > +/* > + * 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; > + unsigned int temp_count; > + > + switch (type) { > + case hwmon_temp: > + temp_count = bitmap_weight(priv->temp_sensors, priv->temp_sensors_size); > + > + return channel < temp_count ? 0444 : 0; > + case hwmon_fan: > + return channel < priv->fan_count ? 0444 : 0; > + case hwmon_pwm: > + if (channel >= priv->fan_count) > + return 0; > + > + switch (attr) { > + case hwmon_pwm_enable: Please drop pwm_enable here and only introduce it inside the proper patch. > + return 0644; > + case hwmon_pwm_auto_channels_temp: > + return 0444; > + default: > + return 0; > + } > + default: > + 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, priv->temp_sensors_size, channel); > + > + switch (attr) { > + case hwmon_temp_input: > + ret = awcc_op_get_temperature(priv->wdev, temp, &state); > + if (ret) > + return ret; > + > + *val = state * MILLIDEGREE_PER_DEGREE; > + break; > + default: > + return -EOPNOTSUPP; > + } > + > + break; > + case hwmon_fan: > + fan = priv->fan_data[channel]; > + > + switch (attr) { > + case hwmon_fan_input: > + ret = awcc_op_get_fan_rpm(priv->wdev, fan->id, &state); > + if (ret) > + return ret; > + > + *val = state; > + break; > + case hwmon_fan_min: > + *val = fan->min_rpm; > + break; > + case hwmon_fan_max: > + *val = fan->max_rpm; > + break; > + default: > + return -EOPNOTSUPP; > + } > + > + break; > + case hwmon_pwm: > + fan = priv->fan_data[channel]; > + > + switch (attr) { > + case hwmon_pwm_auto_channels_temp: > + bitmap_copy(val, fan->auto_channels_temp, BITS_PER_LONG); > + break; > + default: > + return -EOPNOTSUPP; > + } > + > + break; > + default: > + return -EOPNOTSUPP; > + } > + > + return 0; > +} > + > +static int awcc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, const char **str) > +{ > + struct awcc_priv *priv = dev_get_drvdata(dev); > + struct awcc_fan_data *fan; > + u8 temp; > + > + switch (type) { > + case hwmon_temp: > + temp = find_nth_bit(priv->temp_sensors, priv->temp_sensors_size, channel); > + > + switch (temp) { > + case AWCC_TEMP_SENSOR_CPU: > + *str = "CPU"; > + break; > + case AWCC_TEMP_SENSOR_GPU: > + *str = "GPU"; > + break; > + default: > + *str = "Unknown"; > + break; > + } > + > + break; > + case hwmon_fan: > + fan = priv->fan_data[channel]; > + > + switch (fan->total_temps) { > + case 0: > + *str = "Independent Fan"; > + break; > + case 1: > + temp = find_first_bit(fan->related_temps, priv->temp_sensors_size); > + > + switch (temp) { > + case AWCC_TEMP_SENSOR_CPU: > + *str = "Processor Fan"; > + break; > + case AWCC_TEMP_SENSOR_GPU: > + *str = "Video Fan"; > + break; > + default: > + *str = "Unknown Fan"; > + break; > + } > + > + break; > + default: > + *str = "Shared Fan"; > + break; > + } > + > + break; > + default: > + return -EOPNOTSUPP; > + } > + > + return 0; > +} > + > +static const struct hwmon_ops awcc_hwmon_ops = { > + .is_visible = awcc_hwmon_is_visible, > + .read = awcc_hwmon_read, > + .read_string = awcc_hwmon_read_string, > +}; > + > +static const struct hwmon_channel_info * const awcc_hwmon_info[] = { > + HWMON_CHANNEL_INFO(temp, > + HWMON_T_LABEL | HWMON_T_INPUT, > + HWMON_T_LABEL | HWMON_T_INPUT, > + HWMON_T_LABEL | HWMON_T_INPUT, > + HWMON_T_LABEL | HWMON_T_INPUT, > + HWMON_T_LABEL | HWMON_T_INPUT, > + HWMON_T_LABEL | HWMON_T_INPUT > + ), > + HWMON_CHANNEL_INFO(fan, > + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, > + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, > + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, > + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, > + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, > + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX > + ), > + HWMON_CHANNEL_INFO(pwm, > + HWMON_PWM_AUTO_CHANNELS_TEMP, > + HWMON_PWM_AUTO_CHANNELS_TEMP, > + HWMON_PWM_AUTO_CHANNELS_TEMP, > + HWMON_PWM_AUTO_CHANNELS_TEMP, > + HWMON_PWM_AUTO_CHANNELS_TEMP, > + HWMON_PWM_AUTO_CHANNELS_TEMP > + ), Since the number of fans and temperature sensors is only known at runtime creating awcc_hwmon_info would make sense. > + NULL > +}; > + > +static const struct hwmon_chip_info awcc_hwmon_chip_info = { > + .ops = &awcc_hwmon_ops, > + .info = awcc_hwmon_info, > +}; > + > +static int awcc_hwmon_temps_init(struct wmi_device *wdev) > +{ > + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev); > + unsigned long temp_sensors[BITS_TO_LONGS(U8_MAX)]; > + unsigned int i, max_sensor_id = 0; > + int ret; > + u32 id; > + > + for (i = 0; i < priv->temp_count; i++) { > + /* > + * Temperature sensors IDs are listed after the fan IDs at > + * offset `fan_count` > + */ > + ret = awcc_op_get_resource_id(wdev, i + priv->fan_count, &id); > + if (ret) > + return ret; > + > + id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id); > + if (id > max_sensor_id) > + max_sensor_id = id; > + > + __set_bit(id, temp_sensors); > + } > + > + /* > + * We prefer to allocate the bitmap dynamically because usually temp IDs > + * are small (< 0x30) and only one UL is needed to store it, but there > + * may be unknown devices that break this rule > + */ Hi, as far as i know the memory allocator inside the kernel at least allocates 32 bytes, so you are not saving any memory with this. I suggest you allocate the bitmaps statically. > + priv->temp_sensors_size = max_sensor_id + 1; > + priv->temp_sensors = devm_bitmap_zalloc(&wdev->dev, priv->temp_sensors_size, > + GFP_KERNEL); > + if (!priv->temp_sensors) > + return -ENOMEM; > + > + bitmap_copy(priv->temp_sensors, temp_sensors, priv->temp_sensors_size); > + > + return 0; > +} > + > +static int awcc_hwmon_fans_init(struct wmi_device *wdev) > +{ > + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev); > + u32 id, min_rpm, max_rpm, temp_count, temp_id; > + unsigned long gather[BITS_TO_LONGS(U8_MAX)]; > + struct awcc_fan_data *fan_data; > + unsigned int i, j; > + int ret; > + > + for (i = 0; i < priv->fan_count; i++) { > + fan_data = devm_kzalloc(&wdev->dev, sizeof(*fan_data), GFP_KERNEL); > + if (!fan_data) > + return -ENOMEM; > + > + fan_data->related_temps = devm_bitmap_zalloc(&wdev->dev, > + priv->temp_sensors_size, > + GFP_KERNEL); Same as above > + if (!priv->temp_sensors) > + return -ENOMEM; > + > + fan_data->auto_channels_temp = devm_bitmap_zalloc(&wdev->dev, > + priv->temp_count, > + GFP_KERNEL); We already know that we only ever use the first sizeof(long) bytes from this bitmap, please do a static allocation here. > + if (!priv->temp_sensors) > + return -ENOMEM; > + > + /* > + * Fan IDs are listed first at offset 0 > + */ > + ret = awcc_op_get_resource_id(wdev, i, &id); > + if (ret) > + return ret; > + id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id); > + > + ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MIN_RPM, id, > + &min_rpm); > + if (ret) > + return ret; > + > + ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MAX_RPM, id, > + &max_rpm); > + if (ret) > + return ret; > + > + ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_TOTAL_FAN_TEMPS, id, > + 0, &temp_count); > + if (ret) > + return ret; > + > + for (j = 0; j < temp_count; j++) { > + ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_FAN_TEMP_ID, > + id, j, &temp_id); > + if (ret) > + break; > + > + temp_id = FIELD_GET(AWCC_RESOURCE_ID_MASK, temp_id); > + if (temp_id < priv->temp_sensors_size) > + __set_bit(temp_id, fan_data->related_temps); > + } > + > + fan_data->id = id; > + fan_data->min_rpm = min_rpm; > + fan_data->max_rpm = max_rpm; > + fan_data->total_temps = bitmap_weight(fan_data->related_temps, > + priv->temp_sensors_size); > + bitmap_gather(gather, fan_data->related_temps, priv->temp_sensors, > + priv->temp_sensors_size); Since fan_data->related_temps is only used for determining the fan label after this it would make sense to determine the fan label here and turn fan_data->related_temps into a local variable on the stack. Thanks, Armin Wolf > + bitmap_copy(fan_data->auto_channels_temp, gather, priv->temp_count); > + priv->fan_data[i] = fan_data; > + } > + > + return 0; > +} > + > +static int awcc_hwmon_init(struct wmi_device *wdev) > +{ > + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev); > + int ret; > + > + priv->fan_data = devm_kcalloc(&wdev->dev, priv->fan_count, > + sizeof(*priv->fan_data), GFP_KERNEL); > + if (!priv->fan_data) > + return -ENOMEM; > + > + ret = awcc_hwmon_temps_init(wdev); > + if (ret) > + return ret; > + > + ret = awcc_hwmon_fans_init(wdev); > + if (ret) > + return ret; > + > + priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi", priv, > + &awcc_hwmon_chip_info, NULL); > + > + return PTR_ERR_OR_ZERO(priv->hwdev); > +} > + > /* > * Thermal Profile control > * - Provides thermal profile control through the Platform Profile API > @@ -753,6 +1171,12 @@ static int alienware_awcc_setup(struct wmi_device *wdev) > priv->wdev = wdev; > dev_set_drvdata(&wdev->dev, priv); > > + if (awcc->hwmon) { > + ret = awcc_hwmon_init(wdev); > + if (ret) > + return ret; > + } > + > if (awcc->pprof) { > ret = awcc_platform_profile_init(wdev); > if (ret) > @@ -833,6 +1257,13 @@ int __init alienware_wmax_wmi_init(void) > if (id) > awcc = id->driver_data; > > + if (force_hwmon) { > + if (!awcc) > + awcc = &empty_quirks; > + > + awcc->hwmon = true; > + } > + > if (force_platform_profile) { > if (!awcc) > awcc = &empty_quirks; >
On Thu Mar 6, 2025 at 5:19 PM -05, Armin Wolf wrote: > Am 06.03.25 um 01:56 schrieb Kurt Borja: > >> All models with the "AWCC" WMAX device support monitoring fan speed and >> temperature sensors. Expose this feature through the HWMON interface. >> >> Cc: Guenter Roeck <linux@roeck-us.net> >> Cc: Jean Delvare <jdelvare@suse.com> >> Cc: linux-hwmon@vger.kernel.org >> Signed-off-by: Kurt Borja <kuurtb@gmail.com> >> --- >> drivers/platform/x86/dell/Kconfig | 1 + >> drivers/platform/x86/dell/alienware-wmi-wmax.c | 431 +++++++++++++++++++++++++ >> 2 files changed, 432 insertions(+) >> >> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig >> index f8a0dffcaab7c3b423472c5b9093011334a698c8..85a57c01aaada5d899cd8252e77ed6043da5cbdf 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 71fc17e8d103146b8edf53a552ae5ba64414e873..20cf3371ee3c0e1ea038b3ca517e831f3b30dc29 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 >> @@ -39,6 +43,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"); >> @@ -48,16 +56,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, >> }; >> @@ -155,9 +166,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, >> }; >> >> @@ -180,6 +200,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, >> @@ -216,6 +242,15 @@ struct wmax_u32_args { >> u8 arg3; >> }; >> >> +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 { >> @@ -231,6 +266,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] = { >> @@ -495,6 +535,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) >> { >> @@ -552,6 +605,32 @@ 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); >> } >> >> +static inline int awcc_op_get_fan_rpm(struct wmi_device *wdev, u8 fan_id, u32 *out) >> +{ >> + struct wmax_u32_args args = { >> + .operation = AWCC_OP_GET_FAN_RPM, >> + .arg1 = fan_id, >> + .arg2 = 0, >> + .arg3 = 0, >> + }; >> + >> + >> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out); >> +} >> + >> +static inline int awcc_op_get_temperature(struct wmi_device *wdev, u8 temp_id, u32 *out) >> +{ >> + struct wmax_u32_args args = { >> + .operation = AWCC_OP_GET_TEMPERATURE, >> + .arg1 = temp_id, >> + .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 = { >> @@ -599,6 +678,345 @@ static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profil >> return 0; >> } >> >> +/* >> + * 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; >> + unsigned int temp_count; >> + >> + switch (type) { >> + case hwmon_temp: >> + temp_count = bitmap_weight(priv->temp_sensors, priv->temp_sensors_size); >> + >> + return channel < temp_count ? 0444 : 0; >> + case hwmon_fan: >> + return channel < priv->fan_count ? 0444 : 0; >> + case hwmon_pwm: >> + if (channel >= priv->fan_count) >> + return 0; >> + >> + switch (attr) { >> + case hwmon_pwm_enable: > > Please drop pwm_enable here and only introduce it inside the proper patch. Thanks, mb. > >> + return 0644; >> + case hwmon_pwm_auto_channels_temp: >> + return 0444; >> + default: >> + return 0; >> + } >> + default: >> + 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, priv->temp_sensors_size, channel); >> + >> + switch (attr) { >> + case hwmon_temp_input: >> + ret = awcc_op_get_temperature(priv->wdev, temp, &state); >> + if (ret) >> + return ret; >> + >> + *val = state * MILLIDEGREE_PER_DEGREE; >> + break; >> + default: >> + return -EOPNOTSUPP; >> + } >> + >> + break; >> + case hwmon_fan: >> + fan = priv->fan_data[channel]; >> + >> + switch (attr) { >> + case hwmon_fan_input: >> + ret = awcc_op_get_fan_rpm(priv->wdev, fan->id, &state); >> + if (ret) >> + return ret; >> + >> + *val = state; >> + break; >> + case hwmon_fan_min: >> + *val = fan->min_rpm; >> + break; >> + case hwmon_fan_max: >> + *val = fan->max_rpm; >> + break; >> + default: >> + return -EOPNOTSUPP; >> + } >> + >> + break; >> + case hwmon_pwm: >> + fan = priv->fan_data[channel]; >> + >> + switch (attr) { >> + case hwmon_pwm_auto_channels_temp: >> + bitmap_copy(val, fan->auto_channels_temp, BITS_PER_LONG); >> + break; >> + default: >> + return -EOPNOTSUPP; >> + } >> + >> + break; >> + default: >> + return -EOPNOTSUPP; >> + } >> + >> + return 0; >> +} >> + >> +static int awcc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type, >> + u32 attr, int channel, const char **str) >> +{ >> + struct awcc_priv *priv = dev_get_drvdata(dev); >> + struct awcc_fan_data *fan; >> + u8 temp; >> + >> + switch (type) { >> + case hwmon_temp: >> + temp = find_nth_bit(priv->temp_sensors, priv->temp_sensors_size, channel); >> + >> + switch (temp) { >> + case AWCC_TEMP_SENSOR_CPU: >> + *str = "CPU"; >> + break; >> + case AWCC_TEMP_SENSOR_GPU: >> + *str = "GPU"; >> + break; >> + default: >> + *str = "Unknown"; >> + break; >> + } >> + >> + break; >> + case hwmon_fan: >> + fan = priv->fan_data[channel]; >> + >> + switch (fan->total_temps) { >> + case 0: >> + *str = "Independent Fan"; >> + break; >> + case 1: >> + temp = find_first_bit(fan->related_temps, priv->temp_sensors_size); >> + >> + switch (temp) { >> + case AWCC_TEMP_SENSOR_CPU: >> + *str = "Processor Fan"; >> + break; >> + case AWCC_TEMP_SENSOR_GPU: >> + *str = "Video Fan"; >> + break; >> + default: >> + *str = "Unknown Fan"; >> + break; >> + } >> + >> + break; >> + default: >> + *str = "Shared Fan"; >> + break; >> + } >> + >> + break; >> + default: >> + return -EOPNOTSUPP; >> + } >> + >> + return 0; >> +} >> + >> +static const struct hwmon_ops awcc_hwmon_ops = { >> + .is_visible = awcc_hwmon_is_visible, >> + .read = awcc_hwmon_read, >> + .read_string = awcc_hwmon_read_string, >> +}; >> + >> +static const struct hwmon_channel_info * const awcc_hwmon_info[] = { >> + HWMON_CHANNEL_INFO(temp, >> + HWMON_T_LABEL | HWMON_T_INPUT, >> + HWMON_T_LABEL | HWMON_T_INPUT, >> + HWMON_T_LABEL | HWMON_T_INPUT, >> + HWMON_T_LABEL | HWMON_T_INPUT, >> + HWMON_T_LABEL | HWMON_T_INPUT, >> + HWMON_T_LABEL | HWMON_T_INPUT >> + ), >> + HWMON_CHANNEL_INFO(fan, >> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX >> + ), >> + HWMON_CHANNEL_INFO(pwm, >> + HWMON_PWM_AUTO_CHANNELS_TEMP, >> + HWMON_PWM_AUTO_CHANNELS_TEMP, >> + HWMON_PWM_AUTO_CHANNELS_TEMP, >> + HWMON_PWM_AUTO_CHANNELS_TEMP, >> + HWMON_PWM_AUTO_CHANNELS_TEMP, >> + HWMON_PWM_AUTO_CHANNELS_TEMP >> + ), > > Since the number of fans and temperature sensors is only known at runtime creating awcc_hwmon_info > would make sense. IIRC Guenter asked another dev to add more CHANNEL_INFO entries instead of doing that? I might be wrong tho. I'm fine either way. > >> + NULL >> +}; >> + >> +static const struct hwmon_chip_info awcc_hwmon_chip_info = { >> + .ops = &awcc_hwmon_ops, >> + .info = awcc_hwmon_info, >> +}; >> + >> +static int awcc_hwmon_temps_init(struct wmi_device *wdev) >> +{ >> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev); >> + unsigned long temp_sensors[BITS_TO_LONGS(U8_MAX)]; >> + unsigned int i, max_sensor_id = 0; >> + int ret; >> + u32 id; >> + >> + for (i = 0; i < priv->temp_count; i++) { >> + /* >> + * Temperature sensors IDs are listed after the fan IDs at >> + * offset `fan_count` >> + */ >> + ret = awcc_op_get_resource_id(wdev, i + priv->fan_count, &id); >> + if (ret) >> + return ret; >> + >> + id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id); >> + if (id > max_sensor_id) >> + max_sensor_id = id; >> + >> + __set_bit(id, temp_sensors); >> + } >> + >> + /* >> + * We prefer to allocate the bitmap dynamically because usually temp IDs >> + * are small (< 0x30) and only one UL is needed to store it, but there >> + * may be unknown devices that break this rule >> + */ > > Hi, > > as far as i know the memory allocator inside the kernel at least allocates 32 bytes, so you are bytes? bits maybe? > not saving any memory with this. I suggest you allocate the bitmaps statically. The thing is - We don't know before hand how big an ID can be. Technically the upper limit is U8_MAX which would require 4 ULs to store, which is a lot. However I haven't seen temp IDs bigger than 0x6, so this way only one UL is allocated for most devices. I would be very grateful if Dell could help us on this one :') > >> + priv->temp_sensors_size = max_sensor_id + 1; >> + priv->temp_sensors = devm_bitmap_zalloc(&wdev->dev, priv->temp_sensors_size, >> + GFP_KERNEL); >> + if (!priv->temp_sensors) >> + return -ENOMEM; >> + >> + bitmap_copy(priv->temp_sensors, temp_sensors, priv->temp_sensors_size); >> + >> + return 0; >> +} >> + >> +static int awcc_hwmon_fans_init(struct wmi_device *wdev) >> +{ >> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev); >> + u32 id, min_rpm, max_rpm, temp_count, temp_id; >> + unsigned long gather[BITS_TO_LONGS(U8_MAX)]; >> + struct awcc_fan_data *fan_data; >> + unsigned int i, j; >> + int ret; >> + >> + for (i = 0; i < priv->fan_count; i++) { >> + fan_data = devm_kzalloc(&wdev->dev, sizeof(*fan_data), GFP_KERNEL); >> + if (!fan_data) >> + return -ENOMEM; >> + >> + fan_data->related_temps = devm_bitmap_zalloc(&wdev->dev, >> + priv->temp_sensors_size, >> + GFP_KERNEL); > > Same as above > >> + if (!priv->temp_sensors) >> + return -ENOMEM; >> + >> + fan_data->auto_channels_temp = devm_bitmap_zalloc(&wdev->dev, >> + priv->temp_count, >> + GFP_KERNEL); > > We already know that we only ever use the first sizeof(long) bytes from this bitmap, > please do a static allocation here. Right, ofc! I forgot about the limit I imposed. > >> + if (!priv->temp_sensors) >> + return -ENOMEM; >> + >> + /* >> + * Fan IDs are listed first at offset 0 >> + */ >> + ret = awcc_op_get_resource_id(wdev, i, &id); >> + if (ret) >> + return ret; >> + id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id); >> + >> + ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MIN_RPM, id, >> + &min_rpm); >> + if (ret) >> + return ret; >> + >> + ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MAX_RPM, id, >> + &max_rpm); >> + if (ret) >> + return ret; >> + >> + ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_TOTAL_FAN_TEMPS, id, >> + 0, &temp_count); >> + if (ret) >> + return ret; >> + >> + for (j = 0; j < temp_count; j++) { >> + ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_FAN_TEMP_ID, >> + id, j, &temp_id); >> + if (ret) >> + break; >> + >> + temp_id = FIELD_GET(AWCC_RESOURCE_ID_MASK, temp_id); >> + if (temp_id < priv->temp_sensors_size) >> + __set_bit(temp_id, fan_data->related_temps); >> + } >> + >> + fan_data->id = id; >> + fan_data->min_rpm = min_rpm; >> + fan_data->max_rpm = max_rpm; >> + fan_data->total_temps = bitmap_weight(fan_data->related_temps, >> + priv->temp_sensors_size); >> + bitmap_gather(gather, fan_data->related_temps, priv->temp_sensors, >> + priv->temp_sensors_size); > > Since fan_data->related_temps is only used for determining the fan label after this it would > make sense to determine the fan label here and turn fan_data->related_temps into a local > variable on the stack. Ack. > > Thanks, > Armin Wolf Thank you very much! Very helpful feedback :) > >> + bitmap_copy(fan_data->auto_channels_temp, gather, priv->temp_count); >> + priv->fan_data[i] = fan_data; >> + } >> + >> + return 0; >> +} >> + >> +static int awcc_hwmon_init(struct wmi_device *wdev) >> +{ >> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev); >> + int ret; >> + >> + priv->fan_data = devm_kcalloc(&wdev->dev, priv->fan_count, >> + sizeof(*priv->fan_data), GFP_KERNEL); >> + if (!priv->fan_data) >> + return -ENOMEM; >> + >> + ret = awcc_hwmon_temps_init(wdev); >> + if (ret) >> + return ret; >> + >> + ret = awcc_hwmon_fans_init(wdev); >> + if (ret) >> + return ret; >> + >> + priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi", priv, >> + &awcc_hwmon_chip_info, NULL); >> + >> + return PTR_ERR_OR_ZERO(priv->hwdev); >> +} >> + >> /* >> * Thermal Profile control >> * - Provides thermal profile control through the Platform Profile API >> @@ -753,6 +1171,12 @@ static int alienware_awcc_setup(struct wmi_device *wdev) >> priv->wdev = wdev; >> dev_set_drvdata(&wdev->dev, priv); >> >> + if (awcc->hwmon) { >> + ret = awcc_hwmon_init(wdev); >> + if (ret) >> + return ret; >> + } >> + >> if (awcc->pprof) { >> ret = awcc_platform_profile_init(wdev); >> if (ret) >> @@ -833,6 +1257,13 @@ int __init alienware_wmax_wmi_init(void) >> if (id) >> awcc = id->driver_data; >> >> + if (force_hwmon) { >> + if (!awcc) >> + awcc = &empty_quirks; >> + >> + awcc->hwmon = true; >> + } >> + >> if (force_platform_profile) { >> if (!awcc) >> awcc = &empty_quirks; >>
Am 07.03.25 um 01:35 schrieb Kurt Borja: > On Thu Mar 6, 2025 at 5:19 PM -05, Armin Wolf wrote: >> Am 06.03.25 um 01:56 schrieb Kurt Borja: >> >>> All models with the "AWCC" WMAX device support monitoring fan speed and >>> temperature sensors. Expose this feature through the HWMON interface. >>> >>> Cc: Guenter Roeck <linux@roeck-us.net> >>> Cc: Jean Delvare <jdelvare@suse.com> >>> Cc: linux-hwmon@vger.kernel.org >>> Signed-off-by: Kurt Borja <kuurtb@gmail.com> >>> --- >>> drivers/platform/x86/dell/Kconfig | 1 + >>> drivers/platform/x86/dell/alienware-wmi-wmax.c | 431 +++++++++++++++++++++++++ >>> 2 files changed, 432 insertions(+) >>> >>> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig >>> index f8a0dffcaab7c3b423472c5b9093011334a698c8..85a57c01aaada5d899cd8252e77ed6043da5cbdf 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 71fc17e8d103146b8edf53a552ae5ba64414e873..20cf3371ee3c0e1ea038b3ca517e831f3b30dc29 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 >>> @@ -39,6 +43,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"); >>> @@ -48,16 +56,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, >>> }; >>> @@ -155,9 +166,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, >>> }; >>> >>> @@ -180,6 +200,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, >>> @@ -216,6 +242,15 @@ struct wmax_u32_args { >>> u8 arg3; >>> }; >>> >>> +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 { >>> @@ -231,6 +266,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] = { >>> @@ -495,6 +535,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) >>> { >>> @@ -552,6 +605,32 @@ 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); >>> } >>> >>> +static inline int awcc_op_get_fan_rpm(struct wmi_device *wdev, u8 fan_id, u32 *out) >>> +{ >>> + struct wmax_u32_args args = { >>> + .operation = AWCC_OP_GET_FAN_RPM, >>> + .arg1 = fan_id, >>> + .arg2 = 0, >>> + .arg3 = 0, >>> + }; >>> + >>> + >>> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out); >>> +} >>> + >>> +static inline int awcc_op_get_temperature(struct wmi_device *wdev, u8 temp_id, u32 *out) >>> +{ >>> + struct wmax_u32_args args = { >>> + .operation = AWCC_OP_GET_TEMPERATURE, >>> + .arg1 = temp_id, >>> + .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 = { >>> @@ -599,6 +678,345 @@ static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profil >>> return 0; >>> } >>> >>> +/* >>> + * 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; >>> + unsigned int temp_count; >>> + >>> + switch (type) { >>> + case hwmon_temp: >>> + temp_count = bitmap_weight(priv->temp_sensors, priv->temp_sensors_size); >>> + >>> + return channel < temp_count ? 0444 : 0; >>> + case hwmon_fan: >>> + return channel < priv->fan_count ? 0444 : 0; >>> + case hwmon_pwm: >>> + if (channel >= priv->fan_count) >>> + return 0; >>> + >>> + switch (attr) { >>> + case hwmon_pwm_enable: >> Please drop pwm_enable here and only introduce it inside the proper patch. > Thanks, mb. > >>> + return 0644; >>> + case hwmon_pwm_auto_channels_temp: >>> + return 0444; >>> + default: >>> + return 0; >>> + } >>> + default: >>> + 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, priv->temp_sensors_size, channel); >>> + >>> + switch (attr) { >>> + case hwmon_temp_input: >>> + ret = awcc_op_get_temperature(priv->wdev, temp, &state); >>> + if (ret) >>> + return ret; >>> + >>> + *val = state * MILLIDEGREE_PER_DEGREE; >>> + break; >>> + default: >>> + return -EOPNOTSUPP; >>> + } >>> + >>> + break; >>> + case hwmon_fan: >>> + fan = priv->fan_data[channel]; >>> + >>> + switch (attr) { >>> + case hwmon_fan_input: >>> + ret = awcc_op_get_fan_rpm(priv->wdev, fan->id, &state); >>> + if (ret) >>> + return ret; >>> + >>> + *val = state; >>> + break; >>> + case hwmon_fan_min: >>> + *val = fan->min_rpm; >>> + break; >>> + case hwmon_fan_max: >>> + *val = fan->max_rpm; >>> + break; >>> + default: >>> + return -EOPNOTSUPP; >>> + } >>> + >>> + break; >>> + case hwmon_pwm: >>> + fan = priv->fan_data[channel]; >>> + >>> + switch (attr) { >>> + case hwmon_pwm_auto_channels_temp: >>> + bitmap_copy(val, fan->auto_channels_temp, BITS_PER_LONG); >>> + break; >>> + default: >>> + return -EOPNOTSUPP; >>> + } >>> + >>> + break; >>> + default: >>> + return -EOPNOTSUPP; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static int awcc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type, >>> + u32 attr, int channel, const char **str) >>> +{ >>> + struct awcc_priv *priv = dev_get_drvdata(dev); >>> + struct awcc_fan_data *fan; >>> + u8 temp; >>> + >>> + switch (type) { >>> + case hwmon_temp: >>> + temp = find_nth_bit(priv->temp_sensors, priv->temp_sensors_size, channel); >>> + >>> + switch (temp) { >>> + case AWCC_TEMP_SENSOR_CPU: >>> + *str = "CPU"; >>> + break; >>> + case AWCC_TEMP_SENSOR_GPU: >>> + *str = "GPU"; >>> + break; >>> + default: >>> + *str = "Unknown"; >>> + break; >>> + } >>> + >>> + break; >>> + case hwmon_fan: >>> + fan = priv->fan_data[channel]; >>> + >>> + switch (fan->total_temps) { >>> + case 0: >>> + *str = "Independent Fan"; >>> + break; >>> + case 1: >>> + temp = find_first_bit(fan->related_temps, priv->temp_sensors_size); >>> + >>> + switch (temp) { >>> + case AWCC_TEMP_SENSOR_CPU: >>> + *str = "Processor Fan"; >>> + break; >>> + case AWCC_TEMP_SENSOR_GPU: >>> + *str = "Video Fan"; >>> + break; >>> + default: >>> + *str = "Unknown Fan"; >>> + break; >>> + } >>> + >>> + break; >>> + default: >>> + *str = "Shared Fan"; >>> + break; >>> + } >>> + >>> + break; >>> + default: >>> + return -EOPNOTSUPP; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static const struct hwmon_ops awcc_hwmon_ops = { >>> + .is_visible = awcc_hwmon_is_visible, >>> + .read = awcc_hwmon_read, >>> + .read_string = awcc_hwmon_read_string, >>> +}; >>> + >>> +static const struct hwmon_channel_info * const awcc_hwmon_info[] = { >>> + HWMON_CHANNEL_INFO(temp, >>> + HWMON_T_LABEL | HWMON_T_INPUT, >>> + HWMON_T_LABEL | HWMON_T_INPUT, >>> + HWMON_T_LABEL | HWMON_T_INPUT, >>> + HWMON_T_LABEL | HWMON_T_INPUT, >>> + HWMON_T_LABEL | HWMON_T_INPUT, >>> + HWMON_T_LABEL | HWMON_T_INPUT >>> + ), >>> + HWMON_CHANNEL_INFO(fan, >>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX >>> + ), >>> + HWMON_CHANNEL_INFO(pwm, >>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>> + HWMON_PWM_AUTO_CHANNELS_TEMP >>> + ), >> Since the number of fans and temperature sensors is only known at runtime creating awcc_hwmon_info >> would make sense. > IIRC Guenter asked another dev to add more CHANNEL_INFO entries instead > of doing that? I might be wrong tho. > > I'm fine either way. > If Guenter is fine with your current approach then you can keep it. >>> + NULL >>> +}; >>> + >>> +static const struct hwmon_chip_info awcc_hwmon_chip_info = { >>> + .ops = &awcc_hwmon_ops, >>> + .info = awcc_hwmon_info, >>> +}; >>> + >>> +static int awcc_hwmon_temps_init(struct wmi_device *wdev) >>> +{ >>> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev); >>> + unsigned long temp_sensors[BITS_TO_LONGS(U8_MAX)]; >>> + unsigned int i, max_sensor_id = 0; >>> + int ret; >>> + u32 id; >>> + >>> + for (i = 0; i < priv->temp_count; i++) { >>> + /* >>> + * Temperature sensors IDs are listed after the fan IDs at >>> + * offset `fan_count` >>> + */ >>> + ret = awcc_op_get_resource_id(wdev, i + priv->fan_count, &id); >>> + if (ret) >>> + return ret; >>> + >>> + id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id); >>> + if (id > max_sensor_id) >>> + max_sensor_id = id; >>> + >>> + __set_bit(id, temp_sensors); >>> + } >>> + >>> + /* >>> + * We prefer to allocate the bitmap dynamically because usually temp IDs >>> + * are small (< 0x30) and only one UL is needed to store it, but there >>> + * may be unknown devices that break this rule >>> + */ >> Hi, >> >> as far as i know the memory allocator inside the kernel at least allocates 32 bytes, so you are > bytes? bits maybe? I am sure it is bytes. >> not saving any memory with this. I suggest you allocate the bitmaps statically. > The thing is - We don't know before hand how big an ID can be. > > Technically the upper limit is U8_MAX which would require 4 ULs to > store, which is a lot. However I haven't seen temp IDs bigger than 0x6, > so this way only one UL is allocated for most devices. > > I would be very grateful if Dell could help us on this one :') > I think wasting 3 * 8 = 24 bytes is OK, especially if you allocate at least 32 bytes for a dynamic bitmap either way. Thanks, Armin Wolf >>> + priv->temp_sensors_size = max_sensor_id + 1; >>> + priv->temp_sensors = devm_bitmap_zalloc(&wdev->dev, priv->temp_sensors_size, >>> + GFP_KERNEL); >>> + if (!priv->temp_sensors) >>> + return -ENOMEM; >>> + >>> + bitmap_copy(priv->temp_sensors, temp_sensors, priv->temp_sensors_size); >>> + >>> + return 0; >>> +} >>> + >>> +static int awcc_hwmon_fans_init(struct wmi_device *wdev) >>> +{ >>> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev); >>> + u32 id, min_rpm, max_rpm, temp_count, temp_id; >>> + unsigned long gather[BITS_TO_LONGS(U8_MAX)]; >>> + struct awcc_fan_data *fan_data; >>> + unsigned int i, j; >>> + int ret; >>> + >>> + for (i = 0; i < priv->fan_count; i++) { >>> + fan_data = devm_kzalloc(&wdev->dev, sizeof(*fan_data), GFP_KERNEL); >>> + if (!fan_data) >>> + return -ENOMEM; >>> + >>> + fan_data->related_temps = devm_bitmap_zalloc(&wdev->dev, >>> + priv->temp_sensors_size, >>> + GFP_KERNEL); >> Same as above >> >>> + if (!priv->temp_sensors) >>> + return -ENOMEM; >>> + >>> + fan_data->auto_channels_temp = devm_bitmap_zalloc(&wdev->dev, >>> + priv->temp_count, >>> + GFP_KERNEL); >> We already know that we only ever use the first sizeof(long) bytes from this bitmap, >> please do a static allocation here. > Right, ofc! I forgot about the limit I imposed. > >>> + if (!priv->temp_sensors) >>> + return -ENOMEM; >>> + >>> + /* >>> + * Fan IDs are listed first at offset 0 >>> + */ >>> + ret = awcc_op_get_resource_id(wdev, i, &id); >>> + if (ret) >>> + return ret; >>> + id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id); >>> + >>> + ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MIN_RPM, id, >>> + &min_rpm); >>> + if (ret) >>> + return ret; >>> + >>> + ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MAX_RPM, id, >>> + &max_rpm); >>> + if (ret) >>> + return ret; >>> + >>> + ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_TOTAL_FAN_TEMPS, id, >>> + 0, &temp_count); >>> + if (ret) >>> + return ret; >>> + >>> + for (j = 0; j < temp_count; j++) { >>> + ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_FAN_TEMP_ID, >>> + id, j, &temp_id); >>> + if (ret) >>> + break; >>> + >>> + temp_id = FIELD_GET(AWCC_RESOURCE_ID_MASK, temp_id); >>> + if (temp_id < priv->temp_sensors_size) >>> + __set_bit(temp_id, fan_data->related_temps); >>> + } >>> + >>> + fan_data->id = id; >>> + fan_data->min_rpm = min_rpm; >>> + fan_data->max_rpm = max_rpm; >>> + fan_data->total_temps = bitmap_weight(fan_data->related_temps, >>> + priv->temp_sensors_size); >>> + bitmap_gather(gather, fan_data->related_temps, priv->temp_sensors, >>> + priv->temp_sensors_size); >> Since fan_data->related_temps is only used for determining the fan label after this it would >> make sense to determine the fan label here and turn fan_data->related_temps into a local >> variable on the stack. > Ack. > >> Thanks, >> Armin Wolf > Thank you very much! Very helpful feedback :) > >>> + bitmap_copy(fan_data->auto_channels_temp, gather, priv->temp_count); >>> + priv->fan_data[i] = fan_data; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static int awcc_hwmon_init(struct wmi_device *wdev) >>> +{ >>> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev); >>> + int ret; >>> + >>> + priv->fan_data = devm_kcalloc(&wdev->dev, priv->fan_count, >>> + sizeof(*priv->fan_data), GFP_KERNEL); >>> + if (!priv->fan_data) >>> + return -ENOMEM; >>> + >>> + ret = awcc_hwmon_temps_init(wdev); >>> + if (ret) >>> + return ret; >>> + >>> + ret = awcc_hwmon_fans_init(wdev); >>> + if (ret) >>> + return ret; >>> + >>> + priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi", priv, >>> + &awcc_hwmon_chip_info, NULL); >>> + >>> + return PTR_ERR_OR_ZERO(priv->hwdev); >>> +} >>> + >>> /* >>> * Thermal Profile control >>> * - Provides thermal profile control through the Platform Profile API >>> @@ -753,6 +1171,12 @@ static int alienware_awcc_setup(struct wmi_device *wdev) >>> priv->wdev = wdev; >>> dev_set_drvdata(&wdev->dev, priv); >>> >>> + if (awcc->hwmon) { >>> + ret = awcc_hwmon_init(wdev); >>> + if (ret) >>> + return ret; >>> + } >>> + >>> if (awcc->pprof) { >>> ret = awcc_platform_profile_init(wdev); >>> if (ret) >>> @@ -833,6 +1257,13 @@ int __init alienware_wmax_wmi_init(void) >>> if (id) >>> awcc = id->driver_data; >>> >>> + if (force_hwmon) { >>> + if (!awcc) >>> + awcc = &empty_quirks; >>> + >>> + awcc->hwmon = true; >>> + } >>> + >>> if (force_platform_profile) { >>> if (!awcc) >>> awcc = &empty_quirks; >>> >
On 3/7/25 13:09, Armin Wolf wrote: > Am 07.03.25 um 01:35 schrieb Kurt Borja: ... >>>> +static const struct hwmon_ops awcc_hwmon_ops = { >>>> + .is_visible = awcc_hwmon_is_visible, >>>> + .read = awcc_hwmon_read, >>>> + .read_string = awcc_hwmon_read_string, >>>> +}; >>>> + >>>> +static const struct hwmon_channel_info * const awcc_hwmon_info[] = { >>>> + HWMON_CHANNEL_INFO(temp, >>>> + HWMON_T_LABEL | HWMON_T_INPUT, >>>> + HWMON_T_LABEL | HWMON_T_INPUT, >>>> + HWMON_T_LABEL | HWMON_T_INPUT, >>>> + HWMON_T_LABEL | HWMON_T_INPUT, >>>> + HWMON_T_LABEL | HWMON_T_INPUT, >>>> + HWMON_T_LABEL | HWMON_T_INPUT >>>> + ), >>>> + HWMON_CHANNEL_INFO(fan, >>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX >>>> + ), >>>> + HWMON_CHANNEL_INFO(pwm, >>>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>>> + HWMON_PWM_AUTO_CHANNELS_TEMP >>>> + ), >>> Since the number of fans and temperature sensors is only known at runtime creating awcc_hwmon_info >>> would make sense. >> IIRC Guenter asked another dev to add more CHANNEL_INFO entries instead >> of doing that? I might be wrong tho. >> >> I'm fine either way. >> > If Guenter is fine with your current approach then you can keep it. > In drivers/hwmon, I prefer static descriptions such as the above and using the is_visible() function to determine if sensor attributes should actually be created. However, as I have mentioned several times, I do not comment on style questions like this (or, for that matter, non-standard sysfs attributes) outside drivers/hwmon, so you can do or request whatever you like. Guenter
On Fri Mar 7, 2025 at 6:59 PM -05, Guenter Roeck wrote: > On 3/7/25 13:09, Armin Wolf wrote: >> Am 07.03.25 um 01:35 schrieb Kurt Borja: > ... >>>>> +static const struct hwmon_ops awcc_hwmon_ops = { >>>>> + .is_visible = awcc_hwmon_is_visible, >>>>> + .read = awcc_hwmon_read, >>>>> + .read_string = awcc_hwmon_read_string, >>>>> +}; >>>>> + >>>>> +static const struct hwmon_channel_info * const awcc_hwmon_info[] = { >>>>> + HWMON_CHANNEL_INFO(temp, >>>>> + HWMON_T_LABEL | HWMON_T_INPUT, >>>>> + HWMON_T_LABEL | HWMON_T_INPUT, >>>>> + HWMON_T_LABEL | HWMON_T_INPUT, >>>>> + HWMON_T_LABEL | HWMON_T_INPUT, >>>>> + HWMON_T_LABEL | HWMON_T_INPUT, >>>>> + HWMON_T_LABEL | HWMON_T_INPUT >>>>> + ), >>>>> + HWMON_CHANNEL_INFO(fan, >>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, >>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX >>>>> + ), >>>>> + HWMON_CHANNEL_INFO(pwm, >>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP, >>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP >>>>> + ), >>>> Since the number of fans and temperature sensors is only known at runtime creating awcc_hwmon_info >>>> would make sense. >>> IIRC Guenter asked another dev to add more CHANNEL_INFO entries instead >>> of doing that? I might be wrong tho. >>> >>> I'm fine either way. >>> >> If Guenter is fine with your current approach then you can keep it. >> > > In drivers/hwmon, I prefer static descriptions such as the above and using > the is_visible() function to determine if sensor attributes should actually > be created. However, as I have mentioned several times, I do not comment on style > questions like this (or, for that matter, non-standard sysfs attributes) outside > drivers/hwmon, so you can do or request whatever you like. > > Guenter Thank you for clarifying! If there are no objections, then I prefer to keep it this way. With the few extra HWMON_CHANNEL_INFO entries.
diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig index f8a0dffcaab7c3b423472c5b9093011334a698c8..85a57c01aaada5d899cd8252e77ed6043da5cbdf 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 71fc17e8d103146b8edf53a552ae5ba64414e873..20cf3371ee3c0e1ea038b3ca517e831f3b30dc29 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 @@ -39,6 +43,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"); @@ -48,16 +56,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, }; @@ -155,9 +166,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, }; @@ -180,6 +200,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, @@ -216,6 +242,15 @@ struct wmax_u32_args { u8 arg3; }; +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 { @@ -231,6 +266,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] = { @@ -495,6 +535,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) { @@ -552,6 +605,32 @@ 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); } +static inline int awcc_op_get_fan_rpm(struct wmi_device *wdev, u8 fan_id, u32 *out) +{ + struct wmax_u32_args args = { + .operation = AWCC_OP_GET_FAN_RPM, + .arg1 = fan_id, + .arg2 = 0, + .arg3 = 0, + }; + + + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out); +} + +static inline int awcc_op_get_temperature(struct wmi_device *wdev, u8 temp_id, u32 *out) +{ + struct wmax_u32_args args = { + .operation = AWCC_OP_GET_TEMPERATURE, + .arg1 = temp_id, + .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 = { @@ -599,6 +678,345 @@ static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profil return 0; } +/* + * 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; + unsigned int temp_count; + + switch (type) { + case hwmon_temp: + temp_count = bitmap_weight(priv->temp_sensors, priv->temp_sensors_size); + + return channel < temp_count ? 0444 : 0; + case hwmon_fan: + return channel < priv->fan_count ? 0444 : 0; + case hwmon_pwm: + if (channel >= priv->fan_count) + return 0; + + switch (attr) { + case hwmon_pwm_enable: + return 0644; + case hwmon_pwm_auto_channels_temp: + return 0444; + default: + return 0; + } + default: + 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, priv->temp_sensors_size, channel); + + switch (attr) { + case hwmon_temp_input: + ret = awcc_op_get_temperature(priv->wdev, temp, &state); + if (ret) + return ret; + + *val = state * MILLIDEGREE_PER_DEGREE; + break; + default: + return -EOPNOTSUPP; + } + + break; + case hwmon_fan: + fan = priv->fan_data[channel]; + + switch (attr) { + case hwmon_fan_input: + ret = awcc_op_get_fan_rpm(priv->wdev, fan->id, &state); + if (ret) + return ret; + + *val = state; + break; + case hwmon_fan_min: + *val = fan->min_rpm; + break; + case hwmon_fan_max: + *val = fan->max_rpm; + break; + default: + return -EOPNOTSUPP; + } + + break; + case hwmon_pwm: + fan = priv->fan_data[channel]; + + switch (attr) { + case hwmon_pwm_auto_channels_temp: + bitmap_copy(val, fan->auto_channels_temp, BITS_PER_LONG); + break; + default: + return -EOPNOTSUPP; + } + + break; + default: + return -EOPNOTSUPP; + } + + return 0; +} + +static int awcc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, const char **str) +{ + struct awcc_priv *priv = dev_get_drvdata(dev); + struct awcc_fan_data *fan; + u8 temp; + + switch (type) { + case hwmon_temp: + temp = find_nth_bit(priv->temp_sensors, priv->temp_sensors_size, channel); + + switch (temp) { + case AWCC_TEMP_SENSOR_CPU: + *str = "CPU"; + break; + case AWCC_TEMP_SENSOR_GPU: + *str = "GPU"; + break; + default: + *str = "Unknown"; + break; + } + + break; + case hwmon_fan: + fan = priv->fan_data[channel]; + + switch (fan->total_temps) { + case 0: + *str = "Independent Fan"; + break; + case 1: + temp = find_first_bit(fan->related_temps, priv->temp_sensors_size); + + switch (temp) { + case AWCC_TEMP_SENSOR_CPU: + *str = "Processor Fan"; + break; + case AWCC_TEMP_SENSOR_GPU: + *str = "Video Fan"; + break; + default: + *str = "Unknown Fan"; + break; + } + + break; + default: + *str = "Shared Fan"; + break; + } + + break; + default: + return -EOPNOTSUPP; + } + + return 0; +} + +static const struct hwmon_ops awcc_hwmon_ops = { + .is_visible = awcc_hwmon_is_visible, + .read = awcc_hwmon_read, + .read_string = awcc_hwmon_read_string, +}; + +static const struct hwmon_channel_info * const awcc_hwmon_info[] = { + HWMON_CHANNEL_INFO(temp, + HWMON_T_LABEL | HWMON_T_INPUT, + HWMON_T_LABEL | HWMON_T_INPUT, + HWMON_T_LABEL | HWMON_T_INPUT, + HWMON_T_LABEL | HWMON_T_INPUT, + HWMON_T_LABEL | HWMON_T_INPUT, + HWMON_T_LABEL | HWMON_T_INPUT + ), + HWMON_CHANNEL_INFO(fan, + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX, + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX + ), + HWMON_CHANNEL_INFO(pwm, + HWMON_PWM_AUTO_CHANNELS_TEMP, + HWMON_PWM_AUTO_CHANNELS_TEMP, + HWMON_PWM_AUTO_CHANNELS_TEMP, + HWMON_PWM_AUTO_CHANNELS_TEMP, + HWMON_PWM_AUTO_CHANNELS_TEMP, + HWMON_PWM_AUTO_CHANNELS_TEMP + ), + NULL +}; + +static const struct hwmon_chip_info awcc_hwmon_chip_info = { + .ops = &awcc_hwmon_ops, + .info = awcc_hwmon_info, +}; + +static int awcc_hwmon_temps_init(struct wmi_device *wdev) +{ + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev); + unsigned long temp_sensors[BITS_TO_LONGS(U8_MAX)]; + unsigned int i, max_sensor_id = 0; + int ret; + u32 id; + + for (i = 0; i < priv->temp_count; i++) { + /* + * Temperature sensors IDs are listed after the fan IDs at + * offset `fan_count` + */ + ret = awcc_op_get_resource_id(wdev, i + priv->fan_count, &id); + if (ret) + return ret; + + id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id); + if (id > max_sensor_id) + max_sensor_id = id; + + __set_bit(id, temp_sensors); + } + + /* + * We prefer to allocate the bitmap dynamically because usually temp IDs + * are small (< 0x30) and only one UL is needed to store it, but there + * may be unknown devices that break this rule + */ + priv->temp_sensors_size = max_sensor_id + 1; + priv->temp_sensors = devm_bitmap_zalloc(&wdev->dev, priv->temp_sensors_size, + GFP_KERNEL); + if (!priv->temp_sensors) + return -ENOMEM; + + bitmap_copy(priv->temp_sensors, temp_sensors, priv->temp_sensors_size); + + return 0; +} + +static int awcc_hwmon_fans_init(struct wmi_device *wdev) +{ + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev); + u32 id, min_rpm, max_rpm, temp_count, temp_id; + unsigned long gather[BITS_TO_LONGS(U8_MAX)]; + struct awcc_fan_data *fan_data; + unsigned int i, j; + int ret; + + for (i = 0; i < priv->fan_count; i++) { + fan_data = devm_kzalloc(&wdev->dev, sizeof(*fan_data), GFP_KERNEL); + if (!fan_data) + return -ENOMEM; + + fan_data->related_temps = devm_bitmap_zalloc(&wdev->dev, + priv->temp_sensors_size, + GFP_KERNEL); + if (!priv->temp_sensors) + return -ENOMEM; + + fan_data->auto_channels_temp = devm_bitmap_zalloc(&wdev->dev, + priv->temp_count, + GFP_KERNEL); + if (!priv->temp_sensors) + return -ENOMEM; + + /* + * Fan IDs are listed first at offset 0 + */ + ret = awcc_op_get_resource_id(wdev, i, &id); + if (ret) + return ret; + id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id); + + ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MIN_RPM, id, + &min_rpm); + if (ret) + return ret; + + ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MAX_RPM, id, + &max_rpm); + if (ret) + return ret; + + ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_TOTAL_FAN_TEMPS, id, + 0, &temp_count); + if (ret) + return ret; + + for (j = 0; j < temp_count; j++) { + ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_FAN_TEMP_ID, + id, j, &temp_id); + if (ret) + break; + + temp_id = FIELD_GET(AWCC_RESOURCE_ID_MASK, temp_id); + if (temp_id < priv->temp_sensors_size) + __set_bit(temp_id, fan_data->related_temps); + } + + fan_data->id = id; + fan_data->min_rpm = min_rpm; + fan_data->max_rpm = max_rpm; + fan_data->total_temps = bitmap_weight(fan_data->related_temps, + priv->temp_sensors_size); + bitmap_gather(gather, fan_data->related_temps, priv->temp_sensors, + priv->temp_sensors_size); + bitmap_copy(fan_data->auto_channels_temp, gather, priv->temp_count); + priv->fan_data[i] = fan_data; + } + + return 0; +} + +static int awcc_hwmon_init(struct wmi_device *wdev) +{ + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev); + int ret; + + priv->fan_data = devm_kcalloc(&wdev->dev, priv->fan_count, + sizeof(*priv->fan_data), GFP_KERNEL); + if (!priv->fan_data) + return -ENOMEM; + + ret = awcc_hwmon_temps_init(wdev); + if (ret) + return ret; + + ret = awcc_hwmon_fans_init(wdev); + if (ret) + return ret; + + priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi", priv, + &awcc_hwmon_chip_info, NULL); + + return PTR_ERR_OR_ZERO(priv->hwdev); +} + /* * Thermal Profile control * - Provides thermal profile control through the Platform Profile API @@ -753,6 +1171,12 @@ static int alienware_awcc_setup(struct wmi_device *wdev) priv->wdev = wdev; dev_set_drvdata(&wdev->dev, priv); + if (awcc->hwmon) { + ret = awcc_hwmon_init(wdev); + if (ret) + return ret; + } + if (awcc->pprof) { ret = awcc_platform_profile_init(wdev); if (ret) @@ -833,6 +1257,13 @@ int __init alienware_wmax_wmi_init(void) if (id) awcc = id->driver_data; + if (force_hwmon) { + if (!awcc) + awcc = &empty_quirks; + + awcc->hwmon = true; + } + if (force_platform_profile) { if (!awcc) awcc = &empty_quirks;
All models with the "AWCC" WMAX device support monitoring fan speed and temperature sensors. Expose this feature through the HWMON interface. Cc: Guenter Roeck <linux@roeck-us.net> Cc: Jean Delvare <jdelvare@suse.com> Cc: linux-hwmon@vger.kernel.org Signed-off-by: Kurt Borja <kuurtb@gmail.com> --- drivers/platform/x86/dell/Kconfig | 1 + drivers/platform/x86/dell/alienware-wmi-wmax.c | 431 +++++++++++++++++++++++++ 2 files changed, 432 insertions(+)