lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <la7mxoxn3rawxij4ybdzpf7xzbiidl66ejv6qxb4sh4n5ugsvu@64royvkee6kv>
Date: Sun, 8 Dec 2024 17:20:52 -0300
From: Kurt Borja <kuurtb@...il.com>
To: Armin Wolf <W_Armin@....de>
Cc: jlee@...e.com, farhan.anwar8@...il.com, rayanmargham4@...il.com, 
	hdegoede@...hat.com, ilpo.jarvinen@...ux.intel.com, 
	platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 4/5] platform/x86: acer-wmi: Implement proper hwmon
 support

On Fri, Nov 29, 2024 at 08:33:58PM +0100, Armin Wolf wrote:
> After looking at the ACPI AML code, it seems that the command 0x0000
> used with ACER_WMID_GET_GAMING_SYS_INFO_METHODID returns a bitmap of
> all supported sensor indices available through the 0x0001 command.
> 
> Those sensor indices seem to include both temperature and fan speed
> sensors, with only the fan speed sensors being currently supported.
> 
> Use the output of this new command to implement reliable sensor
> detection. This fixes detection of fans which do not spin during
> probe, as fans are currently being ignored if their speed is 0.
> 
> Also add support for the new temperature sensor ids.
> 
> Signed-off-by: Armin Wolf <W_Armin@....de>
> ---
>  drivers/platform/x86/acer-wmi.c | 114 ++++++++++++++++++++++----------
>  1 file changed, 80 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index ac4500f33b8c..2c1ea6155bd3 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -30,6 +30,7 @@
>  #include <linux/input/sparse-keymap.h>
>  #include <acpi/video.h>
>  #include <linux/hwmon.h>
> +#include <linux/units.h>
>  #include <linux/bitfield.h>
> 
>  MODULE_AUTHOR("Carlos Corbacho");
> @@ -71,7 +72,10 @@ MODULE_LICENSE("GPL");
>  #define ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET 0x54
> 
>  #define ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK GENMASK_ULL(7, 0)

Hi Armin!

This macro is defined twice.

> -#define ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK GENMASK(20, 8)
> +#define ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK GENMASK_ULL(7, 0)

Here ^.

~ Kurt

> +#define ACER_PREDATOR_V4_SENSOR_INDEX_BIT_MASK GENMASK_ULL(15, 8)
> +#define ACER_PREDATOR_V4_SENSOR_READING_BIT_MASK GENMASK_ULL(23, 8)
> +#define ACER_PREDATOR_V4_SUPPORTED_SENSORS_BIT_MASK GENMASK_ULL(39, 24)
> 
>  /*
>   * Acer ACPI method GUIDs
> @@ -99,9 +103,17 @@ enum acer_wmi_event_ids {
>  };
> 
>  enum acer_wmi_predator_v4_sys_info_command {
> -	ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS = 0x02,
> -	ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED = 0x0201,
> -	ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED = 0x0601,
> +	ACER_WMID_CMD_GET_PREDATOR_V4_SUPPORTED_SENSORS = 0x0000,
> +	ACER_WMID_CMD_GET_PREDATOR_V4_SENSOR_READING	= 0x0001,
> +	ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS	= 0x0002,
> +};
> +
> +enum acer_wmi_predator_v4_sensor_id {
> +	ACER_WMID_SENSOR_CPU_TEMPERATURE	= 0x01,
> +	ACER_WMID_SENSOR_CPU_FAN_SPEED		= 0x02,
> +	ACER_WMID_SENSOR_EXTERNAL_TEMPERATURE_2 = 0x03,
> +	ACER_WMID_SENSOR_GPU_FAN_SPEED		= 0x06,
> +	ACER_WMID_SENSOR_GPU_TEMPERATURE	= 0x0A,
>  };
> 
>  static const struct key_entry acer_wmi_keymap[] __initconst = {
> @@ -272,6 +284,7 @@ static u16 commun_func_bitmap;
>  static u8 commun_fn_key_number;
>  static bool cycle_gaming_thermal_profile = true;
>  static bool predator_v4;
> +static u64 supported_sensors;
> 
>  module_param(mailled, int, 0444);
>  module_param(brightness, int, 0444);
> @@ -1779,27 +1792,6 @@ static int acer_gsensor_event(void)
>  	return 0;
>  }
> 
> -static int acer_get_fan_speed(int fan)
> -{
> -	u64 fanspeed;
> -	u32 command;
> -	int ret;
> -
> -	if (!quirks->predator_v4)
> -		return -EOPNOTSUPP;
> -
> -	if (fan == 0)
> -		command = ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED;
> -	else
> -		command = ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED;
> -
> -	ret = WMID_gaming_get_sys_info(command, &fanspeed);
> -	if (ret < 0)
> -		return ret;
> -
> -	return FIELD_GET(ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK, fanspeed);
> -}
> -
>  /*
>   *  Predator series turbo button
>   */
> @@ -2688,43 +2680,86 @@ static void __init create_debugfs(void)
>  			   &interface->debug.wmid_devices);
>  }
> 
> +static const enum acer_wmi_predator_v4_sensor_id acer_wmi_temp_channel_to_sensor_id[] = {
> +	[0] = ACER_WMID_SENSOR_CPU_TEMPERATURE,
> +	[1] = ACER_WMID_SENSOR_GPU_TEMPERATURE,
> +	[2] = ACER_WMID_SENSOR_EXTERNAL_TEMPERATURE_2,
> +};
> +
> +static const enum acer_wmi_predator_v4_sensor_id acer_wmi_fan_channel_to_sensor_id[] = {
> +	[0] = ACER_WMID_SENSOR_CPU_FAN_SPEED,
> +	[1] = ACER_WMID_SENSOR_GPU_FAN_SPEED,
> +};
> +
>  static umode_t acer_wmi_hwmon_is_visible(const void *data,
>  					 enum hwmon_sensor_types type, u32 attr,
>  					 int channel)
>  {
> +	enum acer_wmi_predator_v4_sensor_id sensor_id;
> +	const u64 *supported_sensors = data;
> +
>  	switch (type) {
> +	case hwmon_temp:
> +		sensor_id = acer_wmi_temp_channel_to_sensor_id[channel];
> +		break;
>  	case hwmon_fan:
> -		if (acer_get_fan_speed(channel) >= 0)
> -			return 0444;
> +		sensor_id = acer_wmi_fan_channel_to_sensor_id[channel];
>  		break;
>  	default:
>  		return 0;
>  	}
> 
> +	if (*supported_sensors & BIT(sensor_id - 1))
> +		return 0444;
> +
>  	return 0;
>  }
> 
>  static int acer_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>  			       u32 attr, int channel, long *val)
>  {
> +	u64 command = ACER_WMID_CMD_GET_PREDATOR_V4_SENSOR_READING;
> +	u64 result;
>  	int ret;
> 
>  	switch (type) {
> +	case hwmon_temp:
> +		command |= FIELD_PREP(ACER_PREDATOR_V4_SENSOR_INDEX_BIT_MASK,
> +				      acer_wmi_temp_channel_to_sensor_id[channel]);
> +
> +		ret = WMID_gaming_get_sys_info(command, &result);
> +		if (ret < 0)
> +			return ret;
> +
> +		result = FIELD_GET(ACER_PREDATOR_V4_SENSOR_READING_BIT_MASK, result);
> +		*val = result * MILLIDEGREE_PER_DEGREE;
> +		return 0;
>  	case hwmon_fan:
> -		ret = acer_get_fan_speed(channel);
> +		command |= FIELD_PREP(ACER_PREDATOR_V4_SENSOR_INDEX_BIT_MASK,
> +				      acer_wmi_fan_channel_to_sensor_id[channel]);
> +
> +		ret = WMID_gaming_get_sys_info(command, &result);
>  		if (ret < 0)
>  			return ret;
> -		*val = ret;
> -		break;
> +
> +		*val = FIELD_GET(ACER_PREDATOR_V4_SENSOR_READING_BIT_MASK, result);
> +		return 0;
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> -
> -	return 0;
>  }
> 
>  static const struct hwmon_channel_info *const acer_wmi_hwmon_info[] = {
> -	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT, HWMON_F_INPUT), NULL
> +	HWMON_CHANNEL_INFO(temp,
> +			   HWMON_T_INPUT,
> +			   HWMON_T_INPUT,
> +			   HWMON_T_INPUT
> +			   ),
> +	HWMON_CHANNEL_INFO(fan,
> +			   HWMON_F_INPUT,
> +			   HWMON_F_INPUT
> +			   ),
> +	NULL
>  };
> 
>  static const struct hwmon_ops acer_wmi_hwmon_ops = {
> @@ -2741,9 +2776,20 @@ static int acer_wmi_hwmon_init(void)
>  {
>  	struct device *dev = &acer_platform_device->dev;
>  	struct device *hwmon;
> +	u64 result;
> +	int ret;
> +
> +	ret = WMID_gaming_get_sys_info(ACER_WMID_CMD_GET_PREDATOR_V4_SUPPORTED_SENSORS, &result);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Return early if no sensors are available */
> +	supported_sensors = FIELD_GET(ACER_PREDATOR_V4_SUPPORTED_SENSORS_BIT_MASK, result);
> +	if (!supported_sensors)
> +		return 0;
> 
>  	hwmon = devm_hwmon_device_register_with_info(dev, "acer",
> -						     &acer_platform_driver,
> +						     &supported_sensors,
>  						     &acer_wmi_hwmon_chip_info,
>  						     NULL);
> 
> --
> 2.39.5
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ