* [PATCH v2 0/5] hwmon: (dell-smm) Miscellaneous Improvments
@ 2021-10-21 19:05 W_Armin
2021-10-21 19:05 ` [PATCH v2 1/5] hwmon: (dell-smm) Sort includes in alphabetical order W_Armin
` (5 more replies)
0 siblings, 6 replies; 8+ messages in thread
From: W_Armin @ 2021-10-21 19:05 UTC (permalink / raw)
To: pali; +Cc: linux, jdelvare, linux-hwmon
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 clarification
about i8k_config_data[], and 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.
---
Changes in v2:
- drop patch 4 and instead add a comment on when to use
i8k_config_data[]
- reword patch 5
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 comment explaining usage of i8k_config_data[]
hwmon: (dell-smm) Speed up setting of fan speed
drivers/hwmon/dell-smm-hwmon.c | 40 +++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 15 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/5] hwmon: (dell-smm) Sort includes in alphabetical order
2021-10-21 19:05 [PATCH v2 0/5] hwmon: (dell-smm) Miscellaneous Improvments W_Armin
@ 2021-10-21 19:05 ` W_Armin
2021-10-21 19:05 ` [PATCH v2 2/5] hwmon: (dell-smm) Use strscpy_pad() W_Armin
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: W_Armin @ 2021-10-21 19:05 UTC (permalink / raw)
To: pali; +Cc: linux, jdelvare, linux-hwmon
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/5] hwmon: (dell-smm) Use strscpy_pad()
2021-10-21 19:05 [PATCH v2 0/5] hwmon: (dell-smm) Miscellaneous Improvments W_Armin
2021-10-21 19:05 ` [PATCH v2 1/5] hwmon: (dell-smm) Sort includes in alphabetical order W_Armin
@ 2021-10-21 19:05 ` W_Armin
2021-10-21 19:05 ` [PATCH v2 3/5] hwmon: (dell-smm) Return -ENOIOCTLCMD instead of -EINVAL W_Armin
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: W_Armin @ 2021-10-21 19:05 UTC (permalink / raw)
To: pali; +Cc: linux, jdelvare, linux-hwmon
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/5] hwmon: (dell-smm) Return -ENOIOCTLCMD instead of -EINVAL
2021-10-21 19:05 [PATCH v2 0/5] hwmon: (dell-smm) Miscellaneous Improvments W_Armin
2021-10-21 19:05 ` [PATCH v2 1/5] hwmon: (dell-smm) Sort includes in alphabetical order W_Armin
2021-10-21 19:05 ` [PATCH v2 2/5] hwmon: (dell-smm) Use strscpy_pad() W_Armin
@ 2021-10-21 19:05 ` W_Armin
2021-10-21 19:05 ` [PATCH v2 4/5] hwmon: (dell-smm) Add comment explaining usage of i8k_config_data[] W_Armin
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: W_Armin @ 2021-10-21 19:05 UTC (permalink / raw)
To: pali; +Cc: linux, jdelvare, linux-hwmon
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 4/5] hwmon: (dell-smm) Add comment explaining usage of i8k_config_data[]
2021-10-21 19:05 [PATCH v2 0/5] hwmon: (dell-smm) Miscellaneous Improvments W_Armin
` (2 preceding siblings ...)
2021-10-21 19:05 ` [PATCH v2 3/5] hwmon: (dell-smm) Return -ENOIOCTLCMD instead of -EINVAL W_Armin
@ 2021-10-21 19:05 ` W_Armin
2021-10-21 19:10 ` Pali Rohár
2021-10-21 19:05 ` [PATCH v2 5/5] hwmon: (dell-smm) Speed up setting of fan speed W_Armin
2021-10-21 20:14 ` [PATCH v2 0/5] hwmon: (dell-smm) Miscellaneous Improvments Guenter Roeck
5 siblings, 1 reply; 8+ messages in thread
From: W_Armin @ 2021-10-21 19:05 UTC (permalink / raw)
To: pali; +Cc: linux, jdelvare, linux-hwmon
From: Armin Wolf <W_Armin@gmx.de>
i8k_config_data[] should only be used for applying device specific
quirks in case autoconfig does not work properly on certain
devices.
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
drivers/hwmon/dell-smm-hwmon.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 5f0338b4a717..0e1bc3a2dd12 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -996,6 +996,11 @@ enum i8k_configs {
DELL_XPS,
};
+/*
+ * Only use for machines which need some special configuration
+ * in order to work correctly (e.g. if autoconfig fails on this machines).
+ */
+
static const struct i8k_config_data i8k_config_data[] __initconst = {
[DELL_LATITUDE_D520] = {
.fan_mult = 1,
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 5/5] hwmon: (dell-smm) Speed up setting of fan speed
2021-10-21 19:05 [PATCH v2 0/5] hwmon: (dell-smm) Miscellaneous Improvments W_Armin
` (3 preceding siblings ...)
2021-10-21 19:05 ` [PATCH v2 4/5] hwmon: (dell-smm) Add comment explaining usage of i8k_config_data[] W_Armin
@ 2021-10-21 19:05 ` W_Armin
2021-10-21 20:14 ` [PATCH v2 0/5] hwmon: (dell-smm) Miscellaneous Improvments Guenter Roeck
5 siblings, 0 replies; 8+ messages in thread
From: W_Armin @ 2021-10-21 19:05 UTC (permalink / raw)
To: pali; +Cc: linux, jdelvare, linux-hwmon
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 since from the two users of this
function, only i8k_ioctl_unlocked() needs to know the new fan status
while dell_smm_write() ignores the new fan status.
Since SMM calls can be very slow while also making error reporting
difficult for dell_smm_write(), remove the function call from
i8k_set_fan() and call it separately in i8k_ioctl_unlocked().
Tested on a Dell Inspiron 3505.
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
Reviewed-by: Pali Rohár <pali@kernel.org>
---
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 0e1bc3a2dd12..eaace478f508 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(®s) ? : i8k_get_fan_status(data, fan);
+ return i8k_smm(®s);
}
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 4/5] hwmon: (dell-smm) Add comment explaining usage of i8k_config_data[]
2021-10-21 19:05 ` [PATCH v2 4/5] hwmon: (dell-smm) Add comment explaining usage of i8k_config_data[] W_Armin
@ 2021-10-21 19:10 ` Pali Rohár
0 siblings, 0 replies; 8+ messages in thread
From: Pali Rohár @ 2021-10-21 19:10 UTC (permalink / raw)
To: W_Armin; +Cc: linux, jdelvare, linux-hwmon
On Thursday 21 October 2021 21:05:30 W_Armin@gmx.de wrote:
> From: Armin Wolf <W_Armin@gmx.de>
>
> i8k_config_data[] should only be used for applying device specific
> quirks in case autoconfig does not work properly on certain
> devices.
>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
Reviewed-by: Pali Rohár <pali@kernel.org>
> ---
> drivers/hwmon/dell-smm-hwmon.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 5f0338b4a717..0e1bc3a2dd12 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -996,6 +996,11 @@ enum i8k_configs {
> DELL_XPS,
> };
>
> +/*
> + * Only use for machines which need some special configuration
> + * in order to work correctly (e.g. if autoconfig fails on this machines).
> + */
> +
> static const struct i8k_config_data i8k_config_data[] __initconst = {
> [DELL_LATITUDE_D520] = {
> .fan_mult = 1,
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/5] hwmon: (dell-smm) Miscellaneous Improvments
2021-10-21 19:05 [PATCH v2 0/5] hwmon: (dell-smm) Miscellaneous Improvments W_Armin
` (4 preceding siblings ...)
2021-10-21 19:05 ` [PATCH v2 5/5] hwmon: (dell-smm) Speed up setting of fan speed W_Armin
@ 2021-10-21 20:14 ` Guenter Roeck
5 siblings, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2021-10-21 20:14 UTC (permalink / raw)
To: W_Armin; +Cc: pali, jdelvare, linux-hwmon
On Thu, Oct 21, 2021 at 09:05:26PM +0200, W_Armin@gmx.de wrote:
> 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 clarification
> about i8k_config_data[], and 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.
Series applied.
Thanks,
Guenter
> ---
> Changes in v2:
> - drop patch 4 and instead add a comment on when to use
> i8k_config_data[]
> - reword patch 5
>
> 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 comment explaining usage of i8k_config_data[]
> hwmon: (dell-smm) Speed up setting of fan speed
>
> drivers/hwmon/dell-smm-hwmon.c | 40 +++++++++++++++++++++-------------
> 1 file changed, 25 insertions(+), 15 deletions(-)
>
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-10-21 20:14 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21 19:05 [PATCH v2 0/5] hwmon: (dell-smm) Miscellaneous Improvments W_Armin
2021-10-21 19:05 ` [PATCH v2 1/5] hwmon: (dell-smm) Sort includes in alphabetical order W_Armin
2021-10-21 19:05 ` [PATCH v2 2/5] hwmon: (dell-smm) Use strscpy_pad() W_Armin
2021-10-21 19:05 ` [PATCH v2 3/5] hwmon: (dell-smm) Return -ENOIOCTLCMD instead of -EINVAL W_Armin
2021-10-21 19:05 ` [PATCH v2 4/5] hwmon: (dell-smm) Add comment explaining usage of i8k_config_data[] W_Armin
2021-10-21 19:10 ` Pali Rohár
2021-10-21 19:05 ` [PATCH v2 5/5] hwmon: (dell-smm) Speed up setting of fan speed W_Armin
2021-10-21 20:14 ` [PATCH v2 0/5] hwmon: (dell-smm) Miscellaneous Improvments Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).