[PATCH 0/5] hwmon: (dell-smm) Miscellaneous Improvments

[PATCH 0/5] hwmon: (dell-smm) Miscellaneous Improvments

[W_Armin]

From: Armin Wolf <W_Armin@gmx.de>

This patch series contains various improvments to the
dell-smm-hwmon driver. The first patch is merely a cosmetic
change. The next two patches improve the ioctl handling
and have been tested with i8kutils (the only program using
this interface). The fourth patch adds some config data
for the Inspiron 3505, which has been verified with the
fan data displayed by the Dell UEFI Diag ROM. The last patch
accelerates the setting of fan speed and has been tested as well
over the legacy interface and the hwmon sysfs interface.

Tested on a Dell Inspiron 3505, the Dell Latitude C600 is
currently out-of-service.

Armin Wolf (5):
  hwmon: (dell-smm) Sort includes in alphabetical order
  hwmon: (dell-smm) Use strscpy_pad()
  hwmon: (dell-smm) Return -ENOIOCTLCMD instead of -EINVAL
  hwmon: (dell-smm) Add Dell Inspiron 3505 config data
  hwmon: (dell-smm) Speed up setting of fan speed

 drivers/hwmon/dell-smm-hwmon.c | 48 +++++++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 15 deletions(-)

--
2.20.1

[PATCH 1/5] hwmon: (dell-smm) Sort includes in alphabetical order

[W_Armin]

From: Armin Wolf <W_Armin@gmx.de>

Sort includes for better overview.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/hwmon/dell-smm-hwmon.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index af0d0d2b6e99..9773d6c0477a 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -12,22 +12,22 @@

 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

+#include <linux/capability.h>
 #include <linux/cpu.h>
+#include <linux/ctype.h>
 #include <linux/delay.h>
+#include <linux/dmi.h>
 #include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/init.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/platform_device.h>
-#include <linux/types.h>
-#include <linux/init.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
-#include <linux/dmi.h>
-#include <linux/capability.h>
-#include <linux/mutex.h>
-#include <linux/hwmon.h>
-#include <linux/uaccess.h>
-#include <linux/ctype.h>
 #include <linux/smp.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>

 #include <linux/i8k.h>

--
2.20.1

[PATCH 2/5] hwmon: (dell-smm) Use strscpy_pad()

[W_Armin]

From: Armin Wolf <W_Armin@gmx.de>

Using strscpy_pad() allows for fewer memory accesses
since memset() will not unconditionally zero-out
the whole buffer.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/hwmon/dell-smm-hwmon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 9773d6c0477a..b0c591bb761a 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -25,6 +25,7 @@
 #include <linux/platform_device.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
+#include <linux/string.h>
 #include <linux/smp.h>
 #include <linux/types.h>
 #include <linux/uaccess.h>
@@ -472,8 +473,7 @@ i8k_ioctl_unlocked(struct file *fp, struct dell_smm_data *data, unsigned int cmd
 		if (restricted && !capable(CAP_SYS_ADMIN))
 			return -EPERM;

-		memset(buff, 0, sizeof(buff));
-		strscpy(buff, data->bios_machineid, sizeof(buff));
+		strscpy_pad(buff, data->bios_machineid, sizeof(buff));
 		break;

 	case I8K_FN_STATUS:
--
2.20.1

[PATCH 3/5] hwmon: (dell-smm) Return -ENOIOCTLCMD instead of -EINVAL

[W_Armin]

From: Armin Wolf <W_Armin@gmx.de>

Returning -ENOIOCTLCMD gives the callers a better
hint of what went wrong and is the recommended
behavior.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/hwmon/dell-smm-hwmon.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index b0c591bb761a..5f0338b4a717 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -18,6 +18,7 @@
 #include <linux/delay.h>
 #include <linux/dmi.h>
 #include <linux/err.h>
+#include <linux/errno.h>
 #include <linux/hwmon.h>
 #include <linux/init.h>
 #include <linux/module.h>
@@ -516,7 +517,7 @@ i8k_ioctl_unlocked(struct file *fp, struct dell_smm_data *data, unsigned int cmd
 		break;

 	default:
-		return -EINVAL;
+		return -ENOIOCTLCMD;
 	}

 	if (val < 0)
--
2.20.1

[PATCH 4/5] hwmon: (dell-smm) Add Dell Inspiron 3505 config data

[W_Armin]

From: Armin Wolf <W_Armin@gmx.de>

After checking the fan speeds reported with the
Dell Diag UEFI ROM, its safe to permanently
set fan_max to I8K_FAN_HIGH and fan_mult
to 1 for the Dell Inspiron 3505.

Tested on a Dell Inspiron 3505.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/hwmon/dell-smm-hwmon.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 5f0338b4a717..2579dd646b20 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -990,6 +990,7 @@ struct i8k_config_data {
 };

 enum i8k_configs {
+	DELL_INSPIRON_3505,
 	DELL_LATITUDE_D520,
 	DELL_PRECISION_490,
 	DELL_STUDIO,
@@ -997,6 +998,10 @@ enum i8k_configs {
 };

 static const struct i8k_config_data i8k_config_data[] __initconst = {
+	[DELL_INSPIRON_3505] = {
+		.fan_mult = 1,
+		.fan_max = I8K_FAN_HIGH,
+	},
 	[DELL_LATITUDE_D520] = {
 		.fan_mult = 1,
 		.fan_max = I8K_FAN_TURBO,
@@ -1030,6 +1035,14 @@ static const struct dmi_system_id i8k_dmi_table[] __initconst = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Latitude"),
 		},
 	},
+	{
+		.ident = "Dell Inspiron 3505",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 3505"),
+		},
+		.driver_data = (void *)&i8k_config_data[DELL_INSPIRON_3505],
+	},
 	{
 		.ident = "Dell Inspiron 2",
 		.matches = {
--
2.20.1

[PATCH 5/5] hwmon: (dell-smm) Speed up setting of fan speed

[W_Armin]

From: Armin Wolf <W_Armin@gmx.de>

When setting the fan speed, i8k_set_fan() calls i8k_get_fan_status(),
causing an unnecessary smm call which can be very slow while also
making error handling difficult.
Fix that by removing the function call from i8k_set_fan() and
call it separately when needed.

Tested on a Dell Inspiron 3505.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/hwmon/dell-smm-hwmon.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 2579dd646b20..62f087f67925 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -327,7 +327,7 @@ static int i8k_enable_fan_auto_mode(const struct dell_smm_data *data, bool enabl
 }

 /*
- * Set the fan speed (off, low, high). Returns the new fan status.
+ * Set the fan speed (off, low, high, ...).
  */
 static int i8k_set_fan(const struct dell_smm_data *data, int fan, int speed)
 {
@@ -339,7 +339,7 @@ static int i8k_set_fan(const struct dell_smm_data *data, int fan, int speed)
 	speed = (speed < 0) ? 0 : ((speed > data->i8k_fan_max) ? data->i8k_fan_max : speed);
 	regs.ebx = (fan & 0xff) | (speed << 8);

-	return i8k_smm(&regs) ? : i8k_get_fan_status(data, fan);
+	return i8k_smm(&regs);
 }

 static int __init i8k_get_temp_type(int sensor)
@@ -453,7 +453,7 @@ static int
 i8k_ioctl_unlocked(struct file *fp, struct dell_smm_data *data, unsigned int cmd, unsigned long arg)
 {
 	int val = 0;
-	int speed;
+	int speed, err;
 	unsigned char buff[16];
 	int __user *argp = (int __user *)arg;

@@ -513,7 +513,11 @@ i8k_ioctl_unlocked(struct file *fp, struct dell_smm_data *data, unsigned int cmd
 		if (copy_from_user(&speed, argp + 1, sizeof(int)))
 			return -EFAULT;

-		val = i8k_set_fan(data, val, speed);
+		err = i8k_set_fan(data, val, speed);
+		if (err < 0)
+			return err;
+
+		val = i8k_get_fan_status(data, val);
 		break;

 	default:
--
2.20.1

Re: [PATCH 4/5] hwmon: (dell-smm) Add Dell Inspiron 3505 config data

[Pali Rohár]

On Thursday 21 October 2021 19:54:46 W_Armin@gmx.de wrote:
> From: Armin Wolf <W_Armin@gmx.de>
> 
> After checking the fan speeds reported with the
> Dell Diag UEFI ROM, its safe to permanently
> set fan_max to I8K_FAN_HIGH and fan_mult
> to 1 for the Dell Inspiron 3505.
> 
> Tested on a Dell Inspiron 3505.

Hello! Are there any issues without this change on Dell Inspiron 3505?
Because i8k_config_data[] array is there for machines which need some
hooks and do not work correctly (e.g. allowing to use I8K_FAN_TURBO).

> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/hwmon/dell-smm-hwmon.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 5f0338b4a717..2579dd646b20 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -990,6 +990,7 @@ struct i8k_config_data {
>  };
> 
>  enum i8k_configs {
> +	DELL_INSPIRON_3505,
>  	DELL_LATITUDE_D520,
>  	DELL_PRECISION_490,
>  	DELL_STUDIO,
> @@ -997,6 +998,10 @@ enum i8k_configs {
>  };
> 
>  static const struct i8k_config_data i8k_config_data[] __initconst = {
> +	[DELL_INSPIRON_3505] = {
> +		.fan_mult = 1,
> +		.fan_max = I8K_FAN_HIGH,
> +	},
>  	[DELL_LATITUDE_D520] = {
>  		.fan_mult = 1,
>  		.fan_max = I8K_FAN_TURBO,
> @@ -1030,6 +1035,14 @@ static const struct dmi_system_id i8k_dmi_table[] __initconst = {
>  			DMI_MATCH(DMI_PRODUCT_NAME, "Latitude"),
>  		},
>  	},
> +	{
> +		.ident = "Dell Inspiron 3505",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 3505"),
> +		},
> +		.driver_data = (void *)&i8k_config_data[DELL_INSPIRON_3505],
> +	},
>  	{
>  		.ident = "Dell Inspiron 2",
>  		.matches = {
> --
> 2.20.1
> 

Re: [PATCH 5/5] hwmon: (dell-smm) Speed up setting of fan speed

[Pali Rohár]

On Thursday 21 October 2021 19:54:47 W_Armin@gmx.de wrote:
> From: Armin Wolf <W_Armin@gmx.de>
> 
> When setting the fan speed, i8k_set_fan() calls i8k_get_fan_status(),
> causing an unnecessary smm call which can be very slow while also
> making error handling difficult.
> Fix that by removing the function call from i8k_set_fan() and
> call it separately when needed.

The important information which is missing in commit message is the fact
that there are only two users of i8k_set_fan() function. First is
dell_smm_write() and second is i8k_ioctl_unlocked().

First user only checks if error occurred during i8k_set_fan() call and
ignores new fan status. Second user needs to know new fan status as it
returns it to userspace.

So this change just speed up first user - dell_smm_write() function.

It would be nice to put this kind of information into commit message as
from code change itself it is not obvious.

Otherwise code change is fine,

Reviewed-by: Pali Rohár <pali@kernel.org>

> Tested on a Dell Inspiron 3505.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/hwmon/dell-smm-hwmon.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 2579dd646b20..62f087f67925 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -327,7 +327,7 @@ static int i8k_enable_fan_auto_mode(const struct dell_smm_data *data, bool enabl
>  }
> 
>  /*
> - * Set the fan speed (off, low, high). Returns the new fan status.
> + * Set the fan speed (off, low, high, ...).
>   */
>  static int i8k_set_fan(const struct dell_smm_data *data, int fan, int speed)
>  {
> @@ -339,7 +339,7 @@ static int i8k_set_fan(const struct dell_smm_data *data, int fan, int speed)
>  	speed = (speed < 0) ? 0 : ((speed > data->i8k_fan_max) ? data->i8k_fan_max : speed);
>  	regs.ebx = (fan & 0xff) | (speed << 8);
> 
> -	return i8k_smm(&regs) ? : i8k_get_fan_status(data, fan);
> +	return i8k_smm(&regs);
>  }
> 
>  static int __init i8k_get_temp_type(int sensor)
> @@ -453,7 +453,7 @@ static int
>  i8k_ioctl_unlocked(struct file *fp, struct dell_smm_data *data, unsigned int cmd, unsigned long arg)
>  {
>  	int val = 0;
> -	int speed;
> +	int speed, err;
>  	unsigned char buff[16];
>  	int __user *argp = (int __user *)arg;
> 
> @@ -513,7 +513,11 @@ i8k_ioctl_unlocked(struct file *fp, struct dell_smm_data *data, unsigned int cmd
>  		if (copy_from_user(&speed, argp + 1, sizeof(int)))
>  			return -EFAULT;
> 
> -		val = i8k_set_fan(data, val, speed);
> +		err = i8k_set_fan(data, val, speed);
> +		if (err < 0)
> +			return err;
> +
> +		val = i8k_get_fan_status(data, val);
>  		break;
> 
>  	default:
> --
> 2.20.1
> 

Re: [PATCH 1/5] hwmon: (dell-smm) Sort includes in alphabetical order

[Pali Rohár]

On Thursday 21 October 2021 19:54:43 W_Armin@gmx.de wrote:
> From: Armin Wolf <W_Armin@gmx.de>
> 
> Sort includes for better overview.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

Acked-by: Pali Rohár <pali@kernel.org>

> ---
>  drivers/hwmon/dell-smm-hwmon.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index af0d0d2b6e99..9773d6c0477a 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -12,22 +12,22 @@
> 
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
> +#include <linux/capability.h>
>  #include <linux/cpu.h>
> +#include <linux/ctype.h>
>  #include <linux/delay.h>
> +#include <linux/dmi.h>
>  #include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/init.h>
>  #include <linux/module.h>
> +#include <linux/mutex.h>
>  #include <linux/platform_device.h>
> -#include <linux/types.h>
> -#include <linux/init.h>
>  #include <linux/proc_fs.h>
>  #include <linux/seq_file.h>
> -#include <linux/dmi.h>
> -#include <linux/capability.h>
> -#include <linux/mutex.h>
> -#include <linux/hwmon.h>
> -#include <linux/uaccess.h>
> -#include <linux/ctype.h>
>  #include <linux/smp.h>
> +#include <linux/types.h>
> +#include <linux/uaccess.h>
> 
>  #include <linux/i8k.h>
> 
> --
> 2.20.1
> 

Re: [PATCH 2/5] hwmon: (dell-smm) Use strscpy_pad()

[Pali Rohár]

On Thursday 21 October 2021 19:54:44 W_Armin@gmx.de wrote:
> From: Armin Wolf <W_Armin@gmx.de>
> 
> Using strscpy_pad() allows for fewer memory accesses
> since memset() will not unconditionally zero-out
> the whole buffer.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

Acked-by: Pali Rohár <pali@kernel.org>

> ---
>  drivers/hwmon/dell-smm-hwmon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 9773d6c0477a..b0c591bb761a 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -25,6 +25,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/proc_fs.h>
>  #include <linux/seq_file.h>
> +#include <linux/string.h>
>  #include <linux/smp.h>
>  #include <linux/types.h>
>  #include <linux/uaccess.h>
> @@ -472,8 +473,7 @@ i8k_ioctl_unlocked(struct file *fp, struct dell_smm_data *data, unsigned int cmd
>  		if (restricted && !capable(CAP_SYS_ADMIN))
>  			return -EPERM;
> 
> -		memset(buff, 0, sizeof(buff));
> -		strscpy(buff, data->bios_machineid, sizeof(buff));
> +		strscpy_pad(buff, data->bios_machineid, sizeof(buff));
>  		break;
> 
>  	case I8K_FN_STATUS:
> --
> 2.20.1
> 

Re: [PATCH 3/5] hwmon: (dell-smm) Return -ENOIOCTLCMD instead of -EINVAL

[Pali Rohár]

On Thursday 21 October 2021 19:54:45 W_Armin@gmx.de wrote:
> From: Armin Wolf <W_Armin@gmx.de>
> 
> Returning -ENOIOCTLCMD gives the callers a better
> hint of what went wrong and is the recommended
> behavior.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

Acked-by: Pali Rohár <pali@kernel.org>

> ---
>  drivers/hwmon/dell-smm-hwmon.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index b0c591bb761a..5f0338b4a717 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -18,6 +18,7 @@
>  #include <linux/delay.h>
>  #include <linux/dmi.h>
>  #include <linux/err.h>
> +#include <linux/errno.h>
>  #include <linux/hwmon.h>
>  #include <linux/init.h>
>  #include <linux/module.h>
> @@ -516,7 +517,7 @@ i8k_ioctl_unlocked(struct file *fp, struct dell_smm_data *data, unsigned int cmd
>  		break;
> 
>  	default:
> -		return -EINVAL;
> +		return -ENOIOCTLCMD;
>  	}
> 
>  	if (val < 0)
> --
> 2.20.1
> 

Re: [PATCH 4/5] hwmon: (dell-smm) Add Dell Inspiron 3505 config data

[Armin Wolf]

Am 21.10.21 um 20:05 schrieb Pali Rohár:
> On Thursday 21 October 2021 19:54:46 W_Armin@gmx.de wrote:
>> From: Armin Wolf <W_Armin@gmx.de>
>>
>> After checking the fan speeds reported with the
>> Dell Diag UEFI ROM, its safe to permanently
>> set fan_max to I8K_FAN_HIGH and fan_mult
>> to 1 for the Dell Inspiron 3505.
>>
>> Tested on a Dell Inspiron 3505.
> Hello! Are there any issues without this change on Dell Inspiron 3505?
> Because i8k_config_data[] array is there for machines which need some
> hooks and do not work correctly (e.g. allowing to use I8K_FAN_TURBO).
The Inspiron 3505 works fine without this change. If i8k_config_data[]
is only there for applying
device specific quirks, then this change can be removed.
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   drivers/hwmon/dell-smm-hwmon.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
>> index 5f0338b4a717..2579dd646b20 100644
>> --- a/drivers/hwmon/dell-smm-hwmon.c
>> +++ b/drivers/hwmon/dell-smm-hwmon.c
>> @@ -990,6 +990,7 @@ struct i8k_config_data {
>>   };
>>
>>   enum i8k_configs {
>> +	DELL_INSPIRON_3505,
>>   	DELL_LATITUDE_D520,
>>   	DELL_PRECISION_490,
>>   	DELL_STUDIO,
>> @@ -997,6 +998,10 @@ enum i8k_configs {
>>   };
>>
>>   static const struct i8k_config_data i8k_config_data[] __initconst = {
>> +	[DELL_INSPIRON_3505] = {
>> +		.fan_mult = 1,
>> +		.fan_max = I8K_FAN_HIGH,
>> +	},
>>   	[DELL_LATITUDE_D520] = {
>>   		.fan_mult = 1,
>>   		.fan_max = I8K_FAN_TURBO,
>> @@ -1030,6 +1035,14 @@ static const struct dmi_system_id i8k_dmi_table[] __initconst = {
>>   			DMI_MATCH(DMI_PRODUCT_NAME, "Latitude"),
>>   		},
>>   	},
>> +	{
>> +		.ident = "Dell Inspiron 3505",
>> +		.matches = {
>> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>> +			DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 3505"),
>> +		},
>> +		.driver_data = (void *)&i8k_config_data[DELL_INSPIRON_3505],
>> +	},
>>   	{
>>   		.ident = "Dell Inspiron 2",
>>   		.matches = {
>> --
>> 2.20.1
>>

Re: [PATCH 4/5] hwmon: (dell-smm) Add Dell Inspiron 3505 config data

[Pali Rohár]

On Thursday 21 October 2021 20:36:16 Armin Wolf wrote:
> Am 21.10.21 um 20:05 schrieb Pali Rohár:
> > On Thursday 21 October 2021 19:54:46 W_Armin@gmx.de wrote:
> > > From: Armin Wolf <W_Armin@gmx.de>
> > > 
> > > After checking the fan speeds reported with the
> > > Dell Diag UEFI ROM, its safe to permanently
> > > set fan_max to I8K_FAN_HIGH and fan_mult
> > > to 1 for the Dell Inspiron 3505.
> > > 
> > > Tested on a Dell Inspiron 3505.
> > Hello! Are there any issues without this change on Dell Inspiron 3505?
> > Because i8k_config_data[] array is there for machines which need some
> > hooks and do not work correctly (e.g. allowing to use I8K_FAN_TURBO).
> The Inspiron 3505 works fine without this change. If i8k_config_data[]
> is only there for applying
> device specific quirks, then this change can be removed.

In past when autodetection of fan_mult started working, I removed
machines which was tested that do not need hook entry anymore.

We really do not need special entry for every possible Dell machine on
the world in this table.

Hm... Maybe some comment for this array could be add to express why it
is there or why entries here are needed...

> > > Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> > > ---
> > >   drivers/hwmon/dell-smm-hwmon.c | 13 +++++++++++++
> > >   1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> > > index 5f0338b4a717..2579dd646b20 100644
> > > --- a/drivers/hwmon/dell-smm-hwmon.c
> > > +++ b/drivers/hwmon/dell-smm-hwmon.c
> > > @@ -990,6 +990,7 @@ struct i8k_config_data {
> > >   };
> > > 
> > >   enum i8k_configs {
> > > +	DELL_INSPIRON_3505,
> > >   	DELL_LATITUDE_D520,
> > >   	DELL_PRECISION_490,
> > >   	DELL_STUDIO,
> > > @@ -997,6 +998,10 @@ enum i8k_configs {
> > >   };
> > > 
> > >   static const struct i8k_config_data i8k_config_data[] __initconst = {
> > > +	[DELL_INSPIRON_3505] = {
> > > +		.fan_mult = 1,
> > > +		.fan_max = I8K_FAN_HIGH,
> > > +	},
> > >   	[DELL_LATITUDE_D520] = {
> > >   		.fan_mult = 1,
> > >   		.fan_max = I8K_FAN_TURBO,
> > > @@ -1030,6 +1035,14 @@ static const struct dmi_system_id i8k_dmi_table[] __initconst = {
> > >   			DMI_MATCH(DMI_PRODUCT_NAME, "Latitude"),
> > >   		},
> > >   	},
> > > +	{
> > > +		.ident = "Dell Inspiron 3505",
> > > +		.matches = {
> > > +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> > > +			DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 3505"),
> > > +		},
> > > +		.driver_data = (void *)&i8k_config_data[DELL_INSPIRON_3505],
> > > +	},
> > >   	{
> > >   		.ident = "Dell Inspiron 2",
> > >   		.matches = {
> > > --
> > > 2.20.1
> > > 
>