From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753099AbbC1OY2 (ORCPT ); Sat, 28 Mar 2015 10:24:28 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:52540 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750855AbbC1OY0 (ORCPT ); Sat, 28 Mar 2015 10:24:26 -0400 Message-ID: <5516B953.2050203@roeck-us.net> Date: Sat, 28 Mar 2015 07:23:15 -0700 From: Guenter Roeck User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: =?UTF-8?B?UGFsaSBSb2jDoXI=?= , Arnd Bergmann , Greg Kroah-Hartman , Jean Delvare CC: Steven Honeyman , Valdis.Kletnieks@vt.edu, Jochen Eisinger , Gabriele Mazzotta , linux-kernel@vger.kernel.org, lm-sensors@lm-sensors.org Subject: Re: [PATCH 2/2] hwmon: Allow to compile dell-smm-hwmon driver without /proc/i8k References: <1427538255-10860-1-git-send-email-pali.rohar@gmail.com> <1427538255-10860-3-git-send-email-pali.rohar@gmail.com> In-Reply-To: <1427538255-10860-3-git-send-email-pali.rohar@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Authenticated_sender: linux@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-CTCH-PVer: 0000001 X-CTCH-Spam: Unknown X-CTCH-VOD: Unknown X-CTCH-Flags: 0 X-CTCH-RefID: str=0001.0A020204.5516B999.0106,ss=1,re=0.001,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0 X-CTCH-Score: 0.001 X-CTCH-ScoreCust: 0.000 X-CTCH-Rules: C_4847, X-CTCH-SenderID: linux@roeck-us.net X-CTCH-SenderID-Flags: 0 X-CTCH-SenderID-TotalMessages: 2 X-CTCH-SenderID-TotalSpam: 0 X-CTCH-SenderID-TotalSuspected: 0 X-CTCH-SenderID-TotalConfirmed: 0 X-CTCH-SenderID-TotalBulk: 0 X-CTCH-SenderID-TotalVirus: 0 X-CTCH-SenderID-TotalRecipients: 0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: mailgid no entry from get_relayhosts_entry X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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: > - > - > - 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@debian.org)"); > MODULE_AUTHOR("Pali Rohár "); > -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