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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/28/2015 03:24 AM, 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@xxxxxxxxx>
---
  arch/x86/Kconfig               |   25 +++++++++----------------
  drivers/hwmon/Kconfig          |   11 +++++++++++
  drivers/hwmon/Makefile         |    2 +-
  drivers/hwmon/dell-smm-hwmon.c |   20 ++++++++++++++++----
  4 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b7d31ca..8d0266b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1063,24 +1063,17 @@ config TOSHIBA
  	  Say N otherwise.

  config I8K
-	tristate "Dell laptop support"
-	select HWMON
+	bool "Dell i8k legacy laptop support"

tristate and still "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 options enables legacy /proc/i8k userspace interface in
+	  dell-smm-hwmon driver. Character file /proc/i8k reports power and
+	  hotkey status on old Dell laptops (like Dell Inspiron 8000) via
+	  System Management Mode provided by Dell BIOS. This /proc/i8k interface
+	  is also used by userspace package i8kutils to control laptop fans.

-	  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 110fade..86eeb7d 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1703,6 +1703,17 @@ config SENSORS_ULTRA45
  	  This driver provides support for the Ultra45 workstation environmental
  	  sensors.

+config SENSORS_DELL_SMM
+	tristate "Dell laptop SMM BIOS hwmon driver"
+	depends ON X86

Wondering as well .. .did you test this ?

+	---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.
+
Please add this in alphabetic order.

  if ACPI

  comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 1c3e458..9eec614 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -155,7 +155,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_SENSORS_DELL_SMM)	+= dell-smm-hwmon.o

Same here.


  obj-$(CONFIG_PMBUS)		+= pmbus/

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 2b04e4f..e9661dc 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@xxxxxxxxxx)");
  MODULE_AUTHOR("Pali Rohár <pali.rohar@xxxxxxxxx>");
-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");

+#ifdef 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,6 +111,7 @@ static uint fan_max;
  module_param(fan_max, uint, 0);
  MODULE_PARM_DESC(fan_max, "Maximum configurable fan speed (default: autodetect)");

+#ifdef CONFIG_I8K
  static int i8k_open_fs(struct inode *inode, struct file *file);
  static long i8k_ioctl(struct file *, unsigned int, unsigned long);

@@ -120,6 +123,7 @@ static const struct file_operations i8k_fops = {
  	.release	= single_release,
  	.unlocked_ioctl	= i8k_ioctl,
  };
+#endif

  struct smm_regs {
  	unsigned int eax;
@@ -220,6 +224,7 @@ out:
  	return rc;
  }

+#ifdef CONFIG_I8K
  /*
   * Read the Fn key status.
   */
@@ -258,6 +263,7 @@ static int i8k_get_power_status(void)

  	return (regs.eax & 0xff) == I8K_POWER_AC ? I8K_AC : I8K_BATTERY;
  }
+#endif

  /*
   * Read the fan status.
@@ -378,6 +384,7 @@ static int i8k_get_dell_signature(int req_fn)
  	return regs.eax == 1145651527 && regs.edx == 1145392204 ? 0 : -1;
  }

+#ifdef CONFIG_I8K
  static int
  i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, unsigned long arg)
  {
@@ -525,6 +532,7 @@ static int i8k_open_fs(struct inode *inode, struct file *file)
  {
  	return single_open(file, i8k_proc_show, NULL);
  }
+#endif


  /*
@@ -974,17 +982,17 @@ 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;

+#ifdef CONFIG_I8K
  	/* Register the proc entry */
-	proc_i8k = proc_create("i8k", 0, NULL, &i8k_fops);
-	if (!proc_i8k)
+	if (!proc_create("i8k", 0, NULL, &i8k_fops))
  		return -ENOENT;

I would prefer not to fail here but report a warning.
This is no longer a fatal condition.

+#endif

  	err = i8k_init_hwmon();
  	if (err)
@@ -993,14 +1001,18 @@ static int __init i8k_init(void)
  	return 0;

   exit_remove_proc:
+#ifdef CONFIG_I8K
  	remove_proc_entry("i8k", NULL);
+#endif
  	return err;
  }

  static void __exit i8k_exit(void)
  {
  	hwmon_device_unregister(i8k_hwmon_dev);
+#ifdef CONFIG_I8K
  	remove_proc_entry("i8k", NULL);
+#endif
  }

  module_init(i8k_init);


Can you move all the conditional functions and global variables together
under a single #ifdef ? That should include functions to create the proc
entries, and shim functions for the same if I8K is not configured.

Also, the #ifdef would not cover the case where I8K is configured as module
(there is no reason to force it to bool). You should use "#if IS_ENABLED(CONFIG_I8K)"
instead.

Thanks,
Guenter


_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors





[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux