[PATCH v3 2/2] hwmon: Allow to compile dell-smm-hwmon driver without /proc/i8k

[PATCH v3 2/2] hwmon: Allow to compile dell-smm-hwmon driver without /proc/i8k

[Pali Rohár]

This patch splits CONFIG_I8K compile option to SENSORS_DELL_SMM and CONFIG_I8K.
Option SENSORS_DELL_SMM is now used to enable compilation of dell-smm-hwmon
driver and old CONFIG_I8K option to enable /proc/i8k interface in driver.

So this change allows to compile dell-smm-hwmon driver without legacy /proc/i8k
interface which is needed only for old Dell Inspirion models or for userspace
i8kutils package.

For backward compatibility when CONFIG_I8K is enabled then also SENSORS_DELL_SMM
is enabled and so driver dell-smm-hwmon (with /proc/i8k) is compiled.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 arch/x86/Kconfig               |   25 +++----
 drivers/hwmon/Kconfig          |   11 +++
 drivers/hwmon/Makefile         |    2 +-
 drivers/hwmon/dell-smm-hwmon.c |  150 ++++++++++++++++++++++------------------
 4 files changed, 106 insertions(+), 82 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 226d569..7b756b3 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1055,24 +1055,19 @@ config TOSHIBA
 	  Say N otherwise.
 
 config I8K
-	tristate "Dell laptop support"
+	tristate "Dell i8k legacy laptop support"
 	select HWMON
+	select SENSORS_DELL_SMM
 	---help---
-	  This adds a driver to safely access the System Management Mode
-	  of the CPU on the Dell Inspiron 8000. The System Management Mode
-	  is used to read cpu temperature and cooling fan status and to
-	  control the fans on the I8K portables.
+	  This option enables legacy /proc/i8k userspace interface in hwmon
+	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
+	  temperature and allows controlling fan speeds of Dell laptops via
+	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
+	  it reports also power and hotkey status. For fan speed control is
+	  needed userspace package i8kutils.
 
-	  This driver has been tested only on the Inspiron 8000 but it may
-	  also work with other Dell laptops. You can force loading on other
-	  models by passing the parameter `force=1' to the module. Use at
-	  your own risk.
-
-	  For information on utilities to make use of this driver see the
-	  I8K Linux utilities web site at:
-	  <http://people.debian.org/~dz/i8k/>
-
-	  Say Y if you intend to run this kernel on a Dell Inspiron 8000.
+	  Say Y if you intend to run this kernel on old Dell laptops or want to
+	  use userspace package i8kutils.
 	  Say N otherwise.
 
 config X86_REBOOTFIXUPS
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 25d9e72..714f92a 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -371,6 +371,17 @@ config SENSORS_DS1621
 	  This driver can also be built as a module.  If so, the module
 	  will be called ds1621.
 
+config SENSORS_DELL_SMM
+	tristate "Dell laptop SMM BIOS hwmon driver"
+	depends on X86 && DMI
+	help
+	  This hwmon driver adds support for reporting temperature of different
+	  sensors and controls the fans on Dell laptops via System Management
+	  Mode provided by Dell BIOS.
+
+	  When option I8K is also enabled this driver provides legacy /proc/i8k
+	  userspace interface for i8kutils package.
+
 config SENSORS_DA9052_ADC
 	tristate "Dialog DA9052/DA9053 ADC"
 	depends on PMIC_DA9052
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 51533ac..6cacd0f 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_SENSORS_ATXP1)	+= atxp1.o
 obj-$(CONFIG_SENSORS_CORETEMP)	+= coretemp.o
 obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
 obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o
+obj-$(CONFIG_SENSORS_DELL_SMM)	+= dell-smm-hwmon.o
 obj-$(CONFIG_SENSORS_DME1737)	+= dme1737.o
 obj-$(CONFIG_SENSORS_DS620)	+= ds620.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
@@ -156,7 +157,6 @@ obj-$(CONFIG_SENSORS_W83L785TS)	+= w83l785ts.o
 obj-$(CONFIG_SENSORS_W83L786NG)	+= w83l786ng.o
 obj-$(CONFIG_SENSORS_WM831X)	+= wm831x-hwmon.o
 obj-$(CONFIG_SENSORS_WM8350)	+= wm8350-hwmon.o
-obj-$(CONFIG_I8K)		+= dell-smm-hwmon.o
 
 obj-$(CONFIG_PMBUS)		+= pmbus/
 
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 65d314b..2a80882 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -81,7 +81,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH;
 
 MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)");
 MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>");
-MODULE_DESCRIPTION("Driver for accessing SMM BIOS on Dell laptops");
+MODULE_DESCRIPTION("Dell laptop SMM BIOS hwmon driver");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("i8k");
 
@@ -93,6 +93,7 @@ static bool ignore_dmi;
 module_param(ignore_dmi, bool, 0);
 MODULE_PARM_DESC(ignore_dmi, "Continue probing hardware even if DMI data does not match");
 
+#if IS_ENABLED(CONFIG_I8K)
 static bool restricted;
 module_param(restricted, bool, 0);
 MODULE_PARM_DESC(restricted, "Allow fan control if SYS_ADMIN capability set");
@@ -100,6 +101,7 @@ MODULE_PARM_DESC(restricted, "Allow fan control if SYS_ADMIN capability set");
 static bool power_status;
 module_param(power_status, bool, 0600);
 MODULE_PARM_DESC(power_status, "Report power status in /proc/i8k");
+#endif
 
 static uint fan_mult;
 module_param(fan_mult, uint, 0);
@@ -109,18 +111,6 @@ static uint fan_max;
 module_param(fan_max, uint, 0);
 MODULE_PARM_DESC(fan_max, "Maximum configurable fan speed (default: autodetect)");
 
-static int i8k_open_fs(struct inode *inode, struct file *file);
-static long i8k_ioctl(struct file *, unsigned int, unsigned long);
-
-static const struct file_operations i8k_fops = {
-	.owner		= THIS_MODULE,
-	.open		= i8k_open_fs,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-	.unlocked_ioctl	= i8k_ioctl,
-};
-
 struct smm_regs {
 	unsigned int eax;
 	unsigned int ebx __packed;
@@ -221,45 +211,6 @@ out:
 }
 
 /*
- * Read the Fn key status.
- */
-static int i8k_get_fn_status(void)
-{
-	struct smm_regs regs = { .eax = I8K_SMM_FN_STATUS, };
-	int rc;
-
-	rc = i8k_smm(&regs);
-	if (rc < 0)
-		return rc;
-
-	switch ((regs.eax >> I8K_FN_SHIFT) & I8K_FN_MASK) {
-	case I8K_FN_UP:
-		return I8K_VOL_UP;
-	case I8K_FN_DOWN:
-		return I8K_VOL_DOWN;
-	case I8K_FN_MUTE:
-		return I8K_VOL_MUTE;
-	default:
-		return 0;
-	}
-}
-
-/*
- * Read the power status.
- */
-static int i8k_get_power_status(void)
-{
-	struct smm_regs regs = { .eax = I8K_SMM_POWER_STATUS, };
-	int rc;
-
-	rc = i8k_smm(&regs);
-	if (rc < 0)
-		return rc;
-
-	return (regs.eax & 0xff) == I8K_POWER_AC ? I8K_AC : I8K_BATTERY;
-}
-
-/*
  * Read the fan status.
  */
 static int i8k_get_fan_status(int fan)
@@ -378,6 +329,51 @@ static int i8k_get_dell_signature(int req_fn)
 	return regs.eax == 1145651527 && regs.edx == 1145392204 ? 0 : -1;
 }
 
+#if IS_ENABLED(CONFIG_I8K)
+
+/*
+ * Read the Fn key status.
+ */
+static int i8k_get_fn_status(void)
+{
+	struct smm_regs regs = { .eax = I8K_SMM_FN_STATUS, };
+	int rc;
+
+	rc = i8k_smm(&regs);
+	if (rc < 0)
+		return rc;
+
+	switch ((regs.eax >> I8K_FN_SHIFT) & I8K_FN_MASK) {
+	case I8K_FN_UP:
+		return I8K_VOL_UP;
+	case I8K_FN_DOWN:
+		return I8K_VOL_DOWN;
+	case I8K_FN_MUTE:
+		return I8K_VOL_MUTE;
+	default:
+		return 0;
+	}
+}
+
+/*
+ * Read the power status.
+ */
+static int i8k_get_power_status(void)
+{
+	struct smm_regs regs = { .eax = I8K_SMM_POWER_STATUS, };
+	int rc;
+
+	rc = i8k_smm(&regs);
+	if (rc < 0)
+		return rc;
+
+	return (regs.eax & 0xff) == I8K_POWER_AC ? I8K_AC : I8K_BATTERY;
+}
+
+/*
+ * Procfs interface
+ */
+
 static int
 i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, unsigned long arg)
 {
@@ -528,6 +524,37 @@ static int i8k_open_fs(struct inode *inode, struct file *file)
 	return single_open(file, i8k_proc_show, NULL);
 }
 
+static const struct file_operations i8k_fops = {
+	.owner		= THIS_MODULE,
+	.open		= i8k_open_fs,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+	.unlocked_ioctl	= i8k_ioctl,
+};
+
+static void __init i8k_init_procfs(void)
+{
+	/* Register the proc entry */
+	proc_create("i8k", 0, NULL, &i8k_fops);
+}
+
+static void __exit i8k_exit_procfs(void)
+{
+	remove_proc_entry("i8k", NULL);
+}
+
+#else
+
+static inline void __init i8k_init_procfs(void)
+{
+}
+
+static inline void __exit i8k_exit_procfs(void)
+{
+}
+
+#endif
 
 /*
  * Hwmon interface
@@ -750,8 +777,8 @@ static int __init i8k_init_hwmon(void)
 	if (err >= 0)
 		i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
 
-	i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "i8k", NULL,
-							  i8k_groups);
+	i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "dell-smm",
+							  NULL, i8k_groups);
 	if (IS_ERR(i8k_hwmon_dev)) {
 		err = PTR_ERR(i8k_hwmon_dev);
 		i8k_hwmon_dev = NULL;
@@ -976,33 +1003,24 @@ static int __init i8k_probe(void)
 
 static int __init i8k_init(void)
 {
-	struct proc_dir_entry *proc_i8k;
 	int err;
 
 	/* Are we running on an supported laptop? */
 	if (i8k_probe())
 		return -ENODEV;
 
-	/* Register the proc entry */
-	proc_i8k = proc_create("i8k", 0, NULL, &i8k_fops);
-	if (!proc_i8k)
-		return -ENOENT;
-
 	err = i8k_init_hwmon();
 	if (err)
-		goto exit_remove_proc;
+		return err;
 
+	i8k_init_procfs();
 	return 0;
-
- exit_remove_proc:
-	remove_proc_entry("i8k", NULL);
-	return err;
 }
 
 static void __exit i8k_exit(void)
 {
 	hwmon_device_unregister(i8k_hwmon_dev);
-	remove_proc_entry("i8k", NULL);
+	i8k_exit_procfs();
 }
 
 module_init(i8k_init);
-- 
1.7.10.4

[PATCH v3 1/2] hwmon: Rename i8k driver to dell-smm-hwmon and move it to hwmon tree

[Pali Rohár]

This commit moves i8k driver to hwmon tree under name dell-smm-hwmon which is
better name then abbreviation i8k. For backward compatibility is added macro
MODULE_ALIAS("i8k") so modprobe will load driver also old name i8k. CONFIG_I8K
compile option was not changed.

This commit also adds me as maintainer of this new dell-smm-hwmon driver and
remove Guenter Roeck from list who is implicit maintainer all hwmon drivers.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 MAINTAINERS                                    |    4 ++--
 drivers/char/Makefile                          |    1 -
 drivers/hwmon/Makefile                         |    1 +
 drivers/{char/i8k.c => hwmon/dell-smm-hwmon.c} |    6 ++++--
 4 files changed, 7 insertions(+), 5 deletions(-)
 rename drivers/{char/i8k.c => hwmon/dell-smm-hwmon.c} (99%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2e5bbc0..014c15f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3077,9 +3077,9 @@ S:	Maintained
 F:	drivers/platform/x86/dell-smo8800.c
 
 DELL LAPTOP SMM DRIVER
-M:	Guenter Roeck <linux@roeck-us.net>
+M:	Pali Rohár <pali.rohar@gmail.com>
 S:	Maintained
-F:	drivers/char/i8k.c
+F:	drivers/hwmon/dell-smm-hwmon.c
 F:	include/uapi/linux/i8k.h
 
 DELL SYSTEMS MANAGEMENT BASE DRIVER (dcdbas)
diff --git a/drivers/char/Makefile b/drivers/char/Makefile
index d06cde26..1d9cf00 100644
--- a/drivers/char/Makefile
+++ b/drivers/char/Makefile
@@ -36,7 +36,6 @@ else
   obj-$(CONFIG_NVRAM)	+= nvram.o
 endif
 obj-$(CONFIG_TOSHIBA)		+= toshiba.o
-obj-$(CONFIG_I8K)		+= i8k.o
 obj-$(CONFIG_DS1620)		+= ds1620.o
 obj-$(CONFIG_HW_RANDOM)		+= hw_random/
 obj-$(CONFIG_PPDEV)		+= ppdev.o
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index b4a40f1..51533ac 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -156,6 +156,7 @@ obj-$(CONFIG_SENSORS_W83L785TS)	+= w83l785ts.o
 obj-$(CONFIG_SENSORS_W83L786NG)	+= w83l786ng.o
 obj-$(CONFIG_SENSORS_WM831X)	+= wm831x-hwmon.o
 obj-$(CONFIG_SENSORS_WM8350)	+= wm8350-hwmon.o
+obj-$(CONFIG_I8K)		+= dell-smm-hwmon.o
 
 obj-$(CONFIG_PMBUS)		+= pmbus/
 
diff --git a/drivers/char/i8k.c b/drivers/hwmon/dell-smm-hwmon.c
similarity index 99%
rename from drivers/char/i8k.c
rename to drivers/hwmon/dell-smm-hwmon.c
index a43048b..65d314b 100644
--- a/drivers/char/i8k.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -1,12 +1,12 @@
 /*
- * i8k.c -- Linux driver for accessing the SMM BIOS on Dell laptops.
+ * dell-smm-hwmon.c -- Linux driver for accessing the SMM BIOS on Dell laptops.
  *
  * Copyright (C) 2001  Massimo Dal Zotto <dz@debian.org>
  *
  * Hwmon integration:
  * Copyright (C) 2011  Jean Delvare <jdelvare@suse.de>
  * Copyright (C) 2013, 2014  Guenter Roeck <linux@roeck-us.net>
- * Copyright (C) 2014  Pali Rohár <pali.rohar@gmail.com>
+ * Copyright (C) 2014, 2015  Pali Rohár <pali.rohar@gmail.com>
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the
@@ -80,8 +80,10 @@ static uint i8k_fan_max = I8K_FAN_HIGH;
 #define I8K_HWMON_HAVE_FAN2	(1 << 5)
 
 MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)");
+MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>");
 MODULE_DESCRIPTION("Driver for accessing SMM BIOS on Dell laptops");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("i8k");
 
 static bool force;
 module_param(force, bool, 0);
-- 
1.7.10.4

[PATCH v3 2/2] hwmon: Allow to compile dell-smm-hwmon driver without /proc/i8k

[Pali Rohár]

This patch splits CONFIG_I8K compile option to SENSORS_DELL_SMM and CONFIG_I8K.
Option SENSORS_DELL_SMM is now used to enable compilation of dell-smm-hwmon
driver and old CONFIG_I8K option to enable /proc/i8k interface in driver.

So this change allows to compile dell-smm-hwmon driver without legacy /proc/i8k
interface which is needed only for old Dell Inspirion models or for userspace
i8kutils package.

For backward compatibility when CONFIG_I8K is enabled then also SENSORS_DELL_SMM
is enabled and so driver dell-smm-hwmon (with /proc/i8k) is compiled.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
changes since v2:
 - removed depends on DMI
---
 arch/x86/Kconfig               |   25 +++----
 drivers/hwmon/Kconfig          |   11 +++
 drivers/hwmon/Makefile         |    2 +-
 drivers/hwmon/dell-smm-hwmon.c |  150 ++++++++++++++++++++++------------------
 4 files changed, 106 insertions(+), 82 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 226d569..7b756b3 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1055,24 +1055,19 @@ config TOSHIBA
 	  Say N otherwise.
 
 config I8K
-	tristate "Dell laptop support"
+	tristate "Dell i8k legacy laptop support"
 	select HWMON
+	select SENSORS_DELL_SMM
 	---help---
-	  This adds a driver to safely access the System Management Mode
-	  of the CPU on the Dell Inspiron 8000. The System Management Mode
-	  is used to read cpu temperature and cooling fan status and to
-	  control the fans on the I8K portables.
+	  This option enables legacy /proc/i8k userspace interface in hwmon
+	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
+	  temperature and allows controlling fan speeds of Dell laptops via
+	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
+	  it reports also power and hotkey status. For fan speed control is
+	  needed userspace package i8kutils.
 
-	  This driver has been tested only on the Inspiron 8000 but it may
-	  also work with other Dell laptops. You can force loading on other
-	  models by passing the parameter `force=1' to the module. Use at
-	  your own risk.
-
-	  For information on utilities to make use of this driver see the
-	  I8K Linux utilities web site at:
-	  <http://people.debian.org/~dz/i8k/>
-
-	  Say Y if you intend to run this kernel on a Dell Inspiron 8000.
+	  Say Y if you intend to run this kernel on old Dell laptops or want to
+	  use userspace package i8kutils.
 	  Say N otherwise.
 
 config X86_REBOOTFIXUPS
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 25d9e72..bc9d90b1 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -371,6 +371,17 @@ config SENSORS_DS1621
 	  This driver can also be built as a module.  If so, the module
 	  will be called ds1621.
 
+config SENSORS_DELL_SMM
+	tristate "Dell laptop SMM BIOS hwmon driver"
+	depends on X86
+	help
+	  This hwmon driver adds support for reporting temperature of different
+	  sensors and controls the fans on Dell laptops via System Management
+	  Mode provided by Dell BIOS.
+
+	  When option I8K is also enabled this driver provides legacy /proc/i8k
+	  userspace interface for i8kutils package.
+
 config SENSORS_DA9052_ADC
 	tristate "Dialog DA9052/DA9053 ADC"
 	depends on PMIC_DA9052
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 51533ac..6cacd0f 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_SENSORS_ATXP1)	+= atxp1.o
 obj-$(CONFIG_SENSORS_CORETEMP)	+= coretemp.o
 obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
 obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o
+obj-$(CONFIG_SENSORS_DELL_SMM)	+= dell-smm-hwmon.o
 obj-$(CONFIG_SENSORS_DME1737)	+= dme1737.o
 obj-$(CONFIG_SENSORS_DS620)	+= ds620.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
@@ -156,7 +157,6 @@ obj-$(CONFIG_SENSORS_W83L785TS)	+= w83l785ts.o
 obj-$(CONFIG_SENSORS_W83L786NG)	+= w83l786ng.o
 obj-$(CONFIG_SENSORS_WM831X)	+= wm831x-hwmon.o
 obj-$(CONFIG_SENSORS_WM8350)	+= wm8350-hwmon.o
-obj-$(CONFIG_I8K)		+= dell-smm-hwmon.o
 
 obj-$(CONFIG_PMBUS)		+= pmbus/
 
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 65d314b..2a80882 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -81,7 +81,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH;
 
 MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)");
 MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>");
-MODULE_DESCRIPTION("Driver for accessing SMM BIOS on Dell laptops");
+MODULE_DESCRIPTION("Dell laptop SMM BIOS hwmon driver");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("i8k");
 
@@ -93,6 +93,7 @@ static bool ignore_dmi;
 module_param(ignore_dmi, bool, 0);
 MODULE_PARM_DESC(ignore_dmi, "Continue probing hardware even if DMI data does not match");
 
+#if IS_ENABLED(CONFIG_I8K)
 static bool restricted;
 module_param(restricted, bool, 0);
 MODULE_PARM_DESC(restricted, "Allow fan control if SYS_ADMIN capability set");
@@ -100,6 +101,7 @@ MODULE_PARM_DESC(restricted, "Allow fan control if SYS_ADMIN capability set");
 static bool power_status;
 module_param(power_status, bool, 0600);
 MODULE_PARM_DESC(power_status, "Report power status in /proc/i8k");
+#endif
 
 static uint fan_mult;
 module_param(fan_mult, uint, 0);
@@ -109,18 +111,6 @@ static uint fan_max;
 module_param(fan_max, uint, 0);
 MODULE_PARM_DESC(fan_max, "Maximum configurable fan speed (default: autodetect)");
 
-static int i8k_open_fs(struct inode *inode, struct file *file);
-static long i8k_ioctl(struct file *, unsigned int, unsigned long);
-
-static const struct file_operations i8k_fops = {
-	.owner		= THIS_MODULE,
-	.open		= i8k_open_fs,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-	.unlocked_ioctl	= i8k_ioctl,
-};
-
 struct smm_regs {
 	unsigned int eax;
 	unsigned int ebx __packed;
@@ -221,45 +211,6 @@ out:
 }
 
 /*
- * Read the Fn key status.
- */
-static int i8k_get_fn_status(void)
-{
-	struct smm_regs regs = { .eax = I8K_SMM_FN_STATUS, };
-	int rc;
-
-	rc = i8k_smm(&regs);
-	if (rc < 0)
-		return rc;
-
-	switch ((regs.eax >> I8K_FN_SHIFT) & I8K_FN_MASK) {
-	case I8K_FN_UP:
-		return I8K_VOL_UP;
-	case I8K_FN_DOWN:
-		return I8K_VOL_DOWN;
-	case I8K_FN_MUTE:
-		return I8K_VOL_MUTE;
-	default:
-		return 0;
-	}
-}
-
-/*
- * Read the power status.
- */
-static int i8k_get_power_status(void)
-{
-	struct smm_regs regs = { .eax = I8K_SMM_POWER_STATUS, };
-	int rc;
-
-	rc = i8k_smm(&regs);
-	if (rc < 0)
-		return rc;
-
-	return (regs.eax & 0xff) == I8K_POWER_AC ? I8K_AC : I8K_BATTERY;
-}
-
-/*
  * Read the fan status.
  */
 static int i8k_get_fan_status(int fan)
@@ -378,6 +329,51 @@ static int i8k_get_dell_signature(int req_fn)
 	return regs.eax == 1145651527 && regs.edx == 1145392204 ? 0 : -1;
 }
 
+#if IS_ENABLED(CONFIG_I8K)
+
+/*
+ * Read the Fn key status.
+ */
+static int i8k_get_fn_status(void)
+{
+	struct smm_regs regs = { .eax = I8K_SMM_FN_STATUS, };
+	int rc;
+
+	rc = i8k_smm(&regs);
+	if (rc < 0)
+		return rc;
+
+	switch ((regs.eax >> I8K_FN_SHIFT) & I8K_FN_MASK) {
+	case I8K_FN_UP:
+		return I8K_VOL_UP;
+	case I8K_FN_DOWN:
+		return I8K_VOL_DOWN;
+	case I8K_FN_MUTE:
+		return I8K_VOL_MUTE;
+	default:
+		return 0;
+	}
+}
+
+/*
+ * Read the power status.
+ */
+static int i8k_get_power_status(void)
+{
+	struct smm_regs regs = { .eax = I8K_SMM_POWER_STATUS, };
+	int rc;
+
+	rc = i8k_smm(&regs);
+	if (rc < 0)
+		return rc;
+
+	return (regs.eax & 0xff) == I8K_POWER_AC ? I8K_AC : I8K_BATTERY;
+}
+
+/*
+ * Procfs interface
+ */
+
 static int
 i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, unsigned long arg)
 {
@@ -528,6 +524,37 @@ static int i8k_open_fs(struct inode *inode, struct file *file)
 	return single_open(file, i8k_proc_show, NULL);
 }
 
+static const struct file_operations i8k_fops = {
+	.owner		= THIS_MODULE,
+	.open		= i8k_open_fs,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+	.unlocked_ioctl	= i8k_ioctl,
+};
+
+static void __init i8k_init_procfs(void)
+{
+	/* Register the proc entry */
+	proc_create("i8k", 0, NULL, &i8k_fops);
+}
+
+static void __exit i8k_exit_procfs(void)
+{
+	remove_proc_entry("i8k", NULL);
+}
+
+#else
+
+static inline void __init i8k_init_procfs(void)
+{
+}
+
+static inline void __exit i8k_exit_procfs(void)
+{
+}
+
+#endif
 
 /*
  * Hwmon interface
@@ -750,8 +777,8 @@ static int __init i8k_init_hwmon(void)
 	if (err >= 0)
 		i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
 
-	i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "i8k", NULL,
-							  i8k_groups);
+	i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "dell-smm",
+							  NULL, i8k_groups);
 	if (IS_ERR(i8k_hwmon_dev)) {
 		err = PTR_ERR(i8k_hwmon_dev);
 		i8k_hwmon_dev = NULL;
@@ -976,33 +1003,24 @@ static int __init i8k_probe(void)
 
 static int __init i8k_init(void)
 {
-	struct proc_dir_entry *proc_i8k;
 	int err;
 
 	/* Are we running on an supported laptop? */
 	if (i8k_probe())
 		return -ENODEV;
 
-	/* Register the proc entry */
-	proc_i8k = proc_create("i8k", 0, NULL, &i8k_fops);
-	if (!proc_i8k)
-		return -ENOENT;
-
 	err = i8k_init_hwmon();
 	if (err)
-		goto exit_remove_proc;
+		return err;
 
+	i8k_init_procfs();
 	return 0;
-
- exit_remove_proc:
-	remove_proc_entry("i8k", NULL);
-	return err;
 }
 
 static void __exit i8k_exit(void)
 {
 	hwmon_device_unregister(i8k_hwmon_dev);
-	remove_proc_entry("i8k", NULL);
+	i8k_exit_procfs();
 }
 
 module_init(i8k_init);
-- 
1.7.10.4

Re: [PATCH v3 2/2] hwmon: Allow to compile dell-smm-hwmon driver without /proc/i8k

[Pali Rohár]

On Thursday 14 May 2015 13:16:37 Pali Rohár wrote:
> This patch splits CONFIG_I8K compile option to SENSORS_DELL_SMM and CONFIG_I8K.
> Option SENSORS_DELL_SMM is now used to enable compilation of dell-smm-hwmon
> driver and old CONFIG_I8K option to enable /proc/i8k interface in driver.
> 
> So this change allows to compile dell-smm-hwmon driver without legacy /proc/i8k
> interface which is needed only for old Dell Inspirion models or for userspace
> i8kutils package.
> 
> For backward compatibility when CONFIG_I8K is enabled then also SENSORS_DELL_SMM
> is enabled and so driver dell-smm-hwmon (with /proc/i8k) is compiled.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
> changes since v2:
>  - removed depends on DMI
> ---
>  arch/x86/Kconfig               |   25 +++----
>  drivers/hwmon/Kconfig          |   11 +++
>  drivers/hwmon/Makefile         |    2 +-
>  drivers/hwmon/dell-smm-hwmon.c |  150 ++++++++++++++++++++++------------------
>  4 files changed, 106 insertions(+), 82 deletions(-)
> 

Greg, can you finally apply this patch series?

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH v3 2/2] hwmon: Allow to compile dell-smm-hwmon driver without /proc/i8k

[Gabriele Mazzotta]

Hi Pali,

I've just noticed an issue with this patch. See the comment here below.

Gabriele

On Wednesday 29 April 2015 13:41:26 Pali Rohár wrote:
> This patch splits CONFIG_I8K compile option to SENSORS_DELL_SMM and CONFIG_I8K.
> Option SENSORS_DELL_SMM is now used to enable compilation of dell-smm-hwmon
> driver and old CONFIG_I8K option to enable /proc/i8k interface in driver.
> 
> So this change allows to compile dell-smm-hwmon driver without legacy /proc/i8k
> interface which is needed only for old Dell Inspirion models or for userspace
> i8kutils package.
> 
> For backward compatibility when CONFIG_I8K is enabled then also SENSORS_DELL_SMM
> is enabled and so driver dell-smm-hwmon (with /proc/i8k) is compiled.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  arch/x86/Kconfig               |   25 +++----
>  drivers/hwmon/Kconfig          |   11 +++
>  drivers/hwmon/Makefile         |    2 +-
>  drivers/hwmon/dell-smm-hwmon.c |  150 ++++++++++++++++++++++------------------
>  4 files changed, 106 insertions(+), 82 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 226d569..7b756b3 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1055,24 +1055,19 @@ config TOSHIBA
>  	  Say N otherwise.
>  
>  config I8K
> -	tristate "Dell laptop support"
> +	tristate "Dell i8k legacy laptop support"
>  	select HWMON
> +	select SENSORS_DELL_SMM
>  	---help---
> -	  This adds a driver to safely access the System Management Mode
> -	  of the CPU on the Dell Inspiron 8000. The System Management Mode
> -	  is used to read cpu temperature and cooling fan status and to
> -	  control the fans on the I8K portables.
> +	  This option enables legacy /proc/i8k userspace interface in hwmon
> +	  dell-smm-hwmon driver. Character file /proc/i8k reports bios version,
> +	  temperature and allows controlling fan speeds of Dell laptops via
> +	  System Management Mode. For old Dell laptops (like Dell Inspiron 8000)
> +	  it reports also power and hotkey status. For fan speed control is
> +	  needed userspace package i8kutils.
>  
> -	  This driver has been tested only on the Inspiron 8000 but it may
> -	  also work with other Dell laptops. You can force loading on other
> -	  models by passing the parameter `force=1' to the module. Use at
> -	  your own risk.
> -
> -	  For information on utilities to make use of this driver see the
> -	  I8K Linux utilities web site at:
> -	  <http://people.debian.org/~dz/i8k/>
> -
> -	  Say Y if you intend to run this kernel on a Dell Inspiron 8000.
> +	  Say Y if you intend to run this kernel on old Dell laptops or want to
> +	  use userspace package i8kutils.
>  	  Say N otherwise.
>  
>  config X86_REBOOTFIXUPS
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 25d9e72..714f92a 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -371,6 +371,17 @@ config SENSORS_DS1621
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called ds1621.
>  
> +config SENSORS_DELL_SMM
> +	tristate "Dell laptop SMM BIOS hwmon driver"
> +	depends on X86 && DMI
> +	help
> +	  This hwmon driver adds support for reporting temperature of different
> +	  sensors and controls the fans on Dell laptops via System Management
> +	  Mode provided by Dell BIOS.
> +
> +	  When option I8K is also enabled this driver provides legacy /proc/i8k
> +	  userspace interface for i8kutils package.
> +
>  config SENSORS_DA9052_ADC
>  	tristate "Dialog DA9052/DA9053 ADC"
>  	depends on PMIC_DA9052
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 51533ac..6cacd0f 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_SENSORS_ATXP1)	+= atxp1.o
>  obj-$(CONFIG_SENSORS_CORETEMP)	+= coretemp.o
>  obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
>  obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o
> +obj-$(CONFIG_SENSORS_DELL_SMM)	+= dell-smm-hwmon.o
>  obj-$(CONFIG_SENSORS_DME1737)	+= dme1737.o
>  obj-$(CONFIG_SENSORS_DS620)	+= ds620.o
>  obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
> @@ -156,7 +157,6 @@ obj-$(CONFIG_SENSORS_W83L785TS)	+= w83l785ts.o
>  obj-$(CONFIG_SENSORS_W83L786NG)	+= w83l786ng.o
>  obj-$(CONFIG_SENSORS_WM831X)	+= wm831x-hwmon.o
>  obj-$(CONFIG_SENSORS_WM8350)	+= wm8350-hwmon.o
> -obj-$(CONFIG_I8K)		+= dell-smm-hwmon.o
>  
>  obj-$(CONFIG_PMBUS)		+= pmbus/
>  
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 65d314b..2a80882 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -81,7 +81,7 @@ static uint i8k_fan_max = I8K_FAN_HIGH;
>  
>  MODULE_AUTHOR("Massimo Dal Zotto (dz@debian.org)");
>  MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>");
> -MODULE_DESCRIPTION("Driver for accessing SMM BIOS on Dell laptops");
> +MODULE_DESCRIPTION("Dell laptop SMM BIOS hwmon driver");
>  MODULE_LICENSE("GPL");
>  MODULE_ALIAS("i8k");
>  
> @@ -93,6 +93,7 @@ static bool ignore_dmi;
>  module_param(ignore_dmi, bool, 0);
>  MODULE_PARM_DESC(ignore_dmi, "Continue probing hardware even if DMI data does not match");
>  
> +#if IS_ENABLED(CONFIG_I8K)
>  static bool restricted;
>  module_param(restricted, bool, 0);
>  MODULE_PARM_DESC(restricted, "Allow fan control if SYS_ADMIN capability set");
> @@ -100,6 +101,7 @@ MODULE_PARM_DESC(restricted, "Allow fan control if SYS_ADMIN capability set");
>  static bool power_status;
>  module_param(power_status, bool, 0600);
>  MODULE_PARM_DESC(power_status, "Report power status in /proc/i8k");
> +#endif
>  
>  static uint fan_mult;
>  module_param(fan_mult, uint, 0);
> @@ -109,18 +111,6 @@ static uint fan_max;
>  module_param(fan_max, uint, 0);
>  MODULE_PARM_DESC(fan_max, "Maximum configurable fan speed (default: autodetect)");
>  
> -static int i8k_open_fs(struct inode *inode, struct file *file);
> -static long i8k_ioctl(struct file *, unsigned int, unsigned long);
> -
> -static const struct file_operations i8k_fops = {
> -	.owner		= THIS_MODULE,
> -	.open		= i8k_open_fs,
> -	.read		= seq_read,
> -	.llseek		= seq_lseek,
> -	.release	= single_release,
> -	.unlocked_ioctl	= i8k_ioctl,
> -};
> -
>  struct smm_regs {
>  	unsigned int eax;
>  	unsigned int ebx __packed;
> @@ -221,45 +211,6 @@ out:
>  }
>  
>  /*
> - * Read the Fn key status.
> - */
> -static int i8k_get_fn_status(void)
> -{
> -	struct smm_regs regs = { .eax = I8K_SMM_FN_STATUS, };
> -	int rc;
> -
> -	rc = i8k_smm(&regs);
> -	if (rc < 0)
> -		return rc;
> -
> -	switch ((regs.eax >> I8K_FN_SHIFT) & I8K_FN_MASK) {
> -	case I8K_FN_UP:
> -		return I8K_VOL_UP;
> -	case I8K_FN_DOWN:
> -		return I8K_VOL_DOWN;
> -	case I8K_FN_MUTE:
> -		return I8K_VOL_MUTE;
> -	default:
> -		return 0;
> -	}
> -}
> -
> -/*
> - * Read the power status.
> - */
> -static int i8k_get_power_status(void)
> -{
> -	struct smm_regs regs = { .eax = I8K_SMM_POWER_STATUS, };
> -	int rc;
> -
> -	rc = i8k_smm(&regs);
> -	if (rc < 0)
> -		return rc;
> -
> -	return (regs.eax & 0xff) == I8K_POWER_AC ? I8K_AC : I8K_BATTERY;
> -}
> -
> -/*
>   * Read the fan status.
>   */
>  static int i8k_get_fan_status(int fan)
> @@ -378,6 +329,51 @@ static int i8k_get_dell_signature(int req_fn)
>  	return regs.eax == 1145651527 && regs.edx == 1145392204 ? 0 : -1;
>  }
>  
> +#if IS_ENABLED(CONFIG_I8K)
> +
> +/*
> + * Read the Fn key status.
> + */
> +static int i8k_get_fn_status(void)
> +{
> +	struct smm_regs regs = { .eax = I8K_SMM_FN_STATUS, };
> +	int rc;
> +
> +	rc = i8k_smm(&regs);
> +	if (rc < 0)
> +		return rc;
> +
> +	switch ((regs.eax >> I8K_FN_SHIFT) & I8K_FN_MASK) {
> +	case I8K_FN_UP:
> +		return I8K_VOL_UP;
> +	case I8K_FN_DOWN:
> +		return I8K_VOL_DOWN;
> +	case I8K_FN_MUTE:
> +		return I8K_VOL_MUTE;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +/*
> + * Read the power status.
> + */
> +static int i8k_get_power_status(void)
> +{
> +	struct smm_regs regs = { .eax = I8K_SMM_POWER_STATUS, };
> +	int rc;
> +
> +	rc = i8k_smm(&regs);
> +	if (rc < 0)
> +		return rc;
> +
> +	return (regs.eax & 0xff) == I8K_POWER_AC ? I8K_AC : I8K_BATTERY;
> +}
> +
> +/*
> + * Procfs interface
> + */
> +
>  static int
>  i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, unsigned long arg)
>  {
> @@ -528,6 +524,37 @@ static int i8k_open_fs(struct inode *inode, struct file *file)
>  	return single_open(file, i8k_proc_show, NULL);
>  }
>  
> +static const struct file_operations i8k_fops = {
> +	.owner		= THIS_MODULE,
> +	.open		= i8k_open_fs,
> +	.read		= seq_read,
> +	.llseek		= seq_lseek,
> +	.release	= single_release,
> +	.unlocked_ioctl	= i8k_ioctl,
> +};
> +
> +static void __init i8k_init_procfs(void)
> +{
> +	/* Register the proc entry */
> +	proc_create("i8k", 0, NULL, &i8k_fops);
> +}
> +
> +static void __exit i8k_exit_procfs(void)
> +{
> +	remove_proc_entry("i8k", NULL);
> +}
> +
> +#else
> +
> +static inline void __init i8k_init_procfs(void)
> +{
> +}
> +
> +static inline void __exit i8k_exit_procfs(void)
> +{
> +}
> +
> +#endif
>  
>  /*
>   * Hwmon interface
> @@ -750,8 +777,8 @@ static int __init i8k_init_hwmon(void)
>  	if (err >= 0)
>  		i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
>  
> -	i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "i8k", NULL,
> -							  i8k_groups);
> +	i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "dell-smm",
> +							  NULL, i8k_groups);

dell-smm is not a valid name, see hwmon_device_register_with_groups() for
more info (dash not allowed). Because of this, the driver can't be loaded.

>  	if (IS_ERR(i8k_hwmon_dev)) {
>  		err = PTR_ERR(i8k_hwmon_dev);
>  		i8k_hwmon_dev = NULL;
> @@ -976,33 +1003,24 @@ static int __init i8k_probe(void)
>  
>  static int __init i8k_init(void)
>  {
> -	struct proc_dir_entry *proc_i8k;
>  	int err;
>  
>  	/* Are we running on an supported laptop? */
>  	if (i8k_probe())
>  		return -ENODEV;
>  
> -	/* Register the proc entry */
> -	proc_i8k = proc_create("i8k", 0, NULL, &i8k_fops);
> -	if (!proc_i8k)
> -		return -ENOENT;
> -
>  	err = i8k_init_hwmon();
>  	if (err)
> -		goto exit_remove_proc;
> +		return err;
>  
> +	i8k_init_procfs();
>  	return 0;
> -
> - exit_remove_proc:
> -	remove_proc_entry("i8k", NULL);
> -	return err;
>  }
>  
>  static void __exit i8k_exit(void)
>  {
>  	hwmon_device_unregister(i8k_hwmon_dev);
> -	remove_proc_entry("i8k", NULL);
> +	i8k_exit_procfs();
>  }
>  
>  module_init(i8k_init);
> 

Re: [PATCH v3 2/2] hwmon: Allow to compile dell-smm-hwmon driver without /proc/i8k

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1084 bytes --]

On Saturday 27 June 2015 13:34:30 Gabriele Mazzotta wrote:
> Hi Pali,
> 
> I've just noticed an issue with this patch. See the comment here
> below.
> 
> Gabriele
> 
> On Wednesday 29 April 2015 13:41:26 Pali Rohár wrote:
> > @@ -750,8 +777,8 @@ static int __init i8k_init_hwmon(void)
> > 
> >  	if (err >= 0)
> >  	
> >  		i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
> > 
> > -	i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "i8k",
> > NULL, -							  i8k_groups);
> > +	i8k_hwmon_dev = hwmon_device_register_with_groups(NULL,
> > "dell-smm", +							  NULL, i8k_groups);
> 
> dell-smm is not a valid name, see hwmon_device_register_with_groups()
> for more info (dash not allowed). Because of this, the driver can't
> be loaded.
> 

How it is possible? It worked fine on my tested dell machine...

And now I see, I probably tested this change with older kernel version 
(ubuntu/3.13) which did not have that check for invalid characters...

Renaming dell-smm to dell_smm should fix this problem, right?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v3 2/2] hwmon: Allow to compile dell-smm-hwmon driver without /proc/i8k

[Gabriele Mazzotta]

On Saturday 27 June 2015 14:47:16 Pali Rohár wrote:
> On Saturday 27 June 2015 13:34:30 Gabriele Mazzotta wrote:
> > Hi Pali,
> > 
> > I've just noticed an issue with this patch. See the comment here
> > below.
> > 
> > Gabriele
> > 
> > On Wednesday 29 April 2015 13:41:26 Pali Rohár wrote:
> > > @@ -750,8 +777,8 @@ static int __init i8k_init_hwmon(void)
> > > 
> > >  	if (err >= 0)
> > >  	
> > >  		i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
> > > 
> > > -	i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "i8k",
> > > NULL, -							  i8k_groups);
> > > +	i8k_hwmon_dev = hwmon_device_register_with_groups(NULL,
> > > "dell-smm", +							  NULL, i8k_groups);
> > 
> > dell-smm is not a valid name, see hwmon_device_register_with_groups()
> > for more info (dash not allowed). Because of this, the driver can't
> > be loaded.
> > 
> 
> How it is possible? It worked fine on my tested dell machine...
> 
> And now I see, I probably tested this change with older kernel version 
> (ubuntu/3.13) which did not have that check for invalid characters...
> 
> Renaming dell-smm to dell_smm should fix this problem, right?

Yes, everything works just fine with the underscore.

Re: [PATCH v3 2/2] hwmon: Allow to compile dell-smm-hwmon driver without /proc/i8k

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1427 bytes --]

On Saturday 27 June 2015 14:55:40 Gabriele Mazzotta wrote:
> On Saturday 27 June 2015 14:47:16 Pali Rohár wrote:
> > On Saturday 27 June 2015 13:34:30 Gabriele Mazzotta wrote:
> > > Hi Pali,
> > > 
> > > I've just noticed an issue with this patch. See the comment here
> > > below.
> > > 
> > > Gabriele
> > > 
> > > On Wednesday 29 April 2015 13:41:26 Pali Rohár wrote:
> > > > @@ -750,8 +777,8 @@ static int __init i8k_init_hwmon(void)
> > > > 
> > > >  	if (err >= 0)
> > > >  	
> > > >  		i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
> > > > 
> > > > -	i8k_hwmon_dev = hwmon_device_register_with_groups(NULL,
> > > > "i8k", NULL, -							  i8k_groups);
> > > > +	i8k_hwmon_dev = hwmon_device_register_with_groups(NULL,
> > > > "dell-smm", +							  NULL, i8k_groups);
> > > 
> > > dell-smm is not a valid name, see
> > > hwmon_device_register_with_groups() for more info (dash not
> > > allowed). Because of this, the driver can't be loaded.
> > 
> > How it is possible? It worked fine on my tested dell machine...
> > 
> > And now I see, I probably tested this change with older kernel
> > version (ubuntu/3.13) which did not have that check for invalid
> > characters...
> > 
> > Renaming dell-smm to dell_smm should fix this problem, right?
> 
> Yes, everything works just fine with the underscore.

Ok, will you send patch for this small fix?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v3 2/2] hwmon: Allow to compile dell-smm-hwmon driver without /proc/i8k

[Gabriele Mazzotta]

On Saturday 27 June 2015 15:01:34 Pali Rohár wrote:
> On Saturday 27 June 2015 14:55:40 Gabriele Mazzotta wrote:
> > On Saturday 27 June 2015 14:47:16 Pali Rohár wrote:
> > > On Saturday 27 June 2015 13:34:30 Gabriele Mazzotta wrote:
> > > > Hi Pali,
> > > > 
> > > > I've just noticed an issue with this patch. See the comment here
> > > > below.
> > > > 
> > > > Gabriele
> > > > 
> > > > On Wednesday 29 April 2015 13:41:26 Pali Rohár wrote:
> > > > > @@ -750,8 +777,8 @@ static int __init i8k_init_hwmon(void)
> > > > > 
> > > > >  	if (err >= 0)
> > > > >  	
> > > > >  		i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
> > > > > 
> > > > > -	i8k_hwmon_dev = hwmon_device_register_with_groups(NULL,
> > > > > "i8k", NULL, -							  i8k_groups);
> > > > > +	i8k_hwmon_dev = hwmon_device_register_with_groups(NULL,
> > > > > "dell-smm", +							  NULL, i8k_groups);
> > > > 
> > > > dell-smm is not a valid name, see
> > > > hwmon_device_register_with_groups() for more info (dash not
> > > > allowed). Because of this, the driver can't be loaded.
> > > 
> > > How it is possible? It worked fine on my tested dell machine...
> > > 
> > > And now I see, I probably tested this change with older kernel
> > > version (ubuntu/3.13) which did not have that check for invalid
> > > characters...
> > > 
> > > Renaming dell-smm to dell_smm should fix this problem, right?
> > 
> > Yes, everything works just fine with the underscore.
> 
> Ok, will you send patch for this small fix?

OK, I'll do it shortly.