All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Armin Wolf <W_Armin@gmx.de>
Cc: pali@kernel.org, jdelvare@suse.com, linux-hwmon@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/7] hwmon: (dell-smm) Make fan/temp sensor number a u8
Date: Sat, 19 Feb 2022 06:47:41 -0800	[thread overview]
Message-ID: <20220219144741.GA1032777@roeck-us.net> (raw)
In-Reply-To: <20220215191113.16640-4-W_Armin@gmx.de>

On Tue, Feb 15, 2022 at 08:11:09PM +0100, Armin Wolf wrote:
> Right now, we only use bits 0 to 7 of the fan/temp sensor number
> by doing number & 0xff. Passing the value as a u8 makes this
> step unnecessary. Also add checks to the ioctl handler since
> users might get confused when passing 0x00000101 does the same
> as passing 0x00000001.
> 
> Tested on a Dell Inspiron 3505.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> Reviewed-by: Pali Rohár <pali@kernel.org>

Applied to hwmon-next.

Thanks,
Guenter

> ---
>  drivers/hwmon/dell-smm-hwmon.c | 68 ++++++++++++++++++++++------------
>  1 file changed, 45 insertions(+), 23 deletions(-)
> 
> --
> 2.30.2
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 3b49e55d060f..a102034a1d38 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -21,6 +21,7 @@
>  #include <linux/errno.h>
>  #include <linux/hwmon.h>
>  #include <linux/init.h>
> +#include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/platform_device.h>
> @@ -254,46 +255,52 @@ static int i8k_smm(struct smm_regs *regs)
>  /*
>   * Read the fan status.
>   */
> -static int i8k_get_fan_status(const struct dell_smm_data *data, int fan)
> +static int i8k_get_fan_status(const struct dell_smm_data *data, u8 fan)
>  {
> -	struct smm_regs regs = { .eax = I8K_SMM_GET_FAN, };
> +	struct smm_regs regs = {
> +		.eax = I8K_SMM_GET_FAN,
> +		.ebx = fan,
> +	};
> 
>  	if (data->disallow_fan_support)
>  		return -EINVAL;
> 
> -	regs.ebx = fan & 0xff;
>  	return i8k_smm(&regs) ? : regs.eax & 0xff;
>  }
> 
>  /*
>   * Read the fan speed in RPM.
>   */
> -static int i8k_get_fan_speed(const struct dell_smm_data *data, int fan)
> +static int i8k_get_fan_speed(const struct dell_smm_data *data, u8 fan)
>  {
> -	struct smm_regs regs = { .eax = I8K_SMM_GET_SPEED, };
> +	struct smm_regs regs = {
> +		.eax = I8K_SMM_GET_SPEED,
> +		.ebx = fan,
> +	};
> 
>  	if (data->disallow_fan_support)
>  		return -EINVAL;
> 
> -	regs.ebx = fan & 0xff;
>  	return i8k_smm(&regs) ? : (regs.eax & 0xffff) * data->i8k_fan_mult;
>  }
> 
>  /*
>   * Read the fan type.
>   */
> -static int _i8k_get_fan_type(const struct dell_smm_data *data, int fan)
> +static int _i8k_get_fan_type(const struct dell_smm_data *data, u8 fan)
>  {
> -	struct smm_regs regs = { .eax = I8K_SMM_GET_FAN_TYPE, };
> +	struct smm_regs regs = {
> +		.eax = I8K_SMM_GET_FAN_TYPE,
> +		.ebx = fan,
> +	};
> 
>  	if (data->disallow_fan_support || data->disallow_fan_type_call)
>  		return -EINVAL;
> 
> -	regs.ebx = fan & 0xff;
>  	return i8k_smm(&regs) ? : regs.eax & 0xff;
>  }
> 
> -static int i8k_get_fan_type(struct dell_smm_data *data, int fan)
> +static int i8k_get_fan_type(struct dell_smm_data *data, u8 fan)
>  {
>  	/* I8K_SMM_GET_FAN_TYPE SMM call is expensive, so cache values */
>  	if (data->fan_type[fan] == INT_MIN)
> @@ -305,14 +312,16 @@ static int i8k_get_fan_type(struct dell_smm_data *data, int fan)
>  /*
>   * Read the fan nominal rpm for specific fan speed.
>   */
> -static int __init i8k_get_fan_nominal_speed(const struct dell_smm_data *data, int fan, int speed)
> +static int __init i8k_get_fan_nominal_speed(const struct dell_smm_data *data, u8 fan, int speed)
>  {
> -	struct smm_regs regs = { .eax = I8K_SMM_GET_NOM_SPEED, };
> +	struct smm_regs regs = {
> +		.eax = I8K_SMM_GET_NOM_SPEED,
> +		.ebx = fan | (speed << 8),
> +	};
> 
>  	if (data->disallow_fan_support)
>  		return -EINVAL;
> 
> -	regs.ebx = (fan & 0xff) | (speed << 8);
>  	return i8k_smm(&regs) ? : (regs.eax & 0xffff) * data->i8k_fan_mult;
>  }
> 
> @@ -333,7 +342,7 @@ static int i8k_enable_fan_auto_mode(const struct dell_smm_data *data, bool enabl
>  /*
>   * Set the fan speed (off, low, high, ...).
>   */
> -static int i8k_set_fan(const struct dell_smm_data *data, int fan, int speed)
> +static int i8k_set_fan(const struct dell_smm_data *data, u8 fan, int speed)
>  {
>  	struct smm_regs regs = { .eax = I8K_SMM_SET_FAN, };
> 
> @@ -341,33 +350,35 @@ static int i8k_set_fan(const struct dell_smm_data *data, int fan, int speed)
>  		return -EINVAL;
> 
>  	speed = (speed < 0) ? 0 : ((speed > data->i8k_fan_max) ? data->i8k_fan_max : speed);
> -	regs.ebx = (fan & 0xff) | (speed << 8);
> +	regs.ebx = fan | (speed << 8);
> 
>  	return i8k_smm(&regs);
>  }
> 
> -static int __init i8k_get_temp_type(int sensor)
> +static int __init i8k_get_temp_type(u8 sensor)
>  {
> -	struct smm_regs regs = { .eax = I8K_SMM_GET_TEMP_TYPE, };
> +	struct smm_regs regs = {
> +		.eax = I8K_SMM_GET_TEMP_TYPE,
> +		.ebx = sensor,
> +	};
> 
> -	regs.ebx = sensor & 0xff;
>  	return i8k_smm(&regs) ? : regs.eax & 0xff;
>  }
> 
>  /*
>   * Read the cpu temperature.
>   */
> -static int _i8k_get_temp(int sensor)
> +static int _i8k_get_temp(u8 sensor)
>  {
>  	struct smm_regs regs = {
>  		.eax = I8K_SMM_GET_TEMP,
> -		.ebx = sensor & 0xff,
> +		.ebx = sensor,
>  	};
> 
>  	return i8k_smm(&regs) ? : regs.eax & 0xff;
>  }
> 
> -static int i8k_get_temp(int sensor)
> +static int i8k_get_temp(u8 sensor)
>  {
>  	int temp = _i8k_get_temp(sensor);
> 
> @@ -500,6 +511,9 @@ static long i8k_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
>  		if (copy_from_user(&val, argp, sizeof(int)))
>  			return -EFAULT;
> 
> +		if (val > U8_MAX || val < 0)
> +			return -EINVAL;
> +
>  		val = i8k_get_fan_speed(data, val);
>  		break;
> 
> @@ -507,6 +521,9 @@ static long i8k_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
>  		if (copy_from_user(&val, argp, sizeof(int)))
>  			return -EFAULT;
> 
> +		if (val > U8_MAX || val < 0)
> +			return -EINVAL;
> +
>  		val = i8k_get_fan_status(data, val);
>  		break;
> 
> @@ -517,6 +534,9 @@ static long i8k_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
>  		if (copy_from_user(&val, argp, sizeof(int)))
>  			return -EFAULT;
> 
> +		if (val > U8_MAX || val < 0)
> +			return -EINVAL;
> +
>  		if (copy_from_user(&speed, argp + 1, sizeof(int)))
>  			return -EFAULT;
> 
> @@ -924,7 +944,8 @@ static int __init dell_smm_init_hwmon(struct device *dev)
>  {
>  	struct dell_smm_data *data = dev_get_drvdata(dev);
>  	struct device *dell_smm_hwmon_dev;
> -	int i, state, err;
> +	int state, err;
> +	u8 i;
> 
>  	for (i = 0; i < DELL_SMM_NO_TEMP; i++) {
>  		data->temp_type[i] = i8k_get_temp_type(i);
> @@ -1245,7 +1266,8 @@ static int __init dell_smm_probe(struct platform_device *pdev)
>  {
>  	struct dell_smm_data *data;
>  	const struct dmi_system_id *id, *fan_control;
> -	int fan, ret;
> +	int ret;
> +	u8 fan;
> 
>  	data = devm_kzalloc(&pdev->dev, sizeof(struct dell_smm_data), GFP_KERNEL);
>  	if (!data)

  parent reply	other threads:[~2022-02-19 14:47 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-15 19:11 [PATCH 0/7] hwmon: (dell-smm) Miscellaneous improvements Armin Wolf
2022-02-15 19:11 ` [PATCH 1/7] hwmon: (dell-smm) Allow for specifying fan control method as module parameter Armin Wolf
2022-02-15 19:19   ` Pali Rohár
2022-02-15 19:45     ` Armin Wolf
     [not found]     ` <a450a2b6-92d3-d2cd-db63-b578480ff385@gmx.de>
2022-02-15 19:49       ` Pali Rohár
2022-02-15 20:19         ` Armin Wolf
2022-02-15 20:31           ` Pali Rohár
2022-02-15 21:00             ` Armin Wolf
2022-02-15 19:11 ` [PATCH 2/7] hwmon: (dell-smm) Add additional fan mode command combination Armin Wolf
2022-02-15 19:11 ` [PATCH 3/7] hwmon: (dell-smm) Make fan/temp sensor number a u8 Armin Wolf
2022-02-15 19:37   ` Pali Rohár
2022-02-19 14:47   ` Guenter Roeck [this message]
2022-02-15 19:11 ` [PATCH 4/7] hwmon: (dell-smm) Improve temperature sensors detection Armin Wolf
2022-02-19 14:51   ` Guenter Roeck
2022-02-15 19:11 ` [PATCH 5/7] hwmon: (dell-smm) Improve assembly code Armin Wolf
2022-02-16  0:09   ` kernel test robot
2022-02-16  0:09     ` kernel test robot
2022-02-15 19:11 ` [PATCH 6/7] hwmon: (dell-smm) Add SMM interface documentation Armin Wolf
2022-02-15 19:34   ` Pali Rohár
2022-02-19 14:46   ` Guenter Roeck
2022-02-15 19:11 ` [PATCH 7/7] hwmon: (dell-smm) Reword and mark parameter "force" as unsafe Armin Wolf
2022-02-15 19:35   ` Pali Rohár
2022-02-19 14:43   ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220219144741.GA1032777@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=W_Armin@gmx.de \
    --cc=jdelvare@suse.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pali@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.