[PATCH 00/10] i8k patches

[PATCH 00/10] i8k patches

[Pali Rohár]

This patch series contains all i8k patches which were not merged to char-misc tree yet.

Guenter Roeck (3):
  MAINTAINERS: Fix up entry for Dell laptop SMM driver
  i8k: Rework error retries
  i8k: Add support for Dell XPS 13

Pali Rohár (7):
  i8k: Add support for temperature sensor labels
  i8k: Register only temperature sensors which have labels
  i8k: Return -ENODATA for invalid temperature
  i8k: Make fan module parameters an unsigned
  i8k: Autodetect maximal fan speed and fan RPM multiplier
  i8k: Remove DMI config data for Latitude E6x40
  i8k: Add support for fan labels

 MAINTAINERS        |    1 +
 drivers/char/i8k.c |  341 +++++++++++++++++++++++++++++++++++++---------------
 2 files changed, 244 insertions(+), 98 deletions(-)

-- 
1.7.9.5

[PATCH 01/10] i8k: Add support for temperature sensor labels

[Pali Rohár]

This patch adds labels for temperature sensors if SMM function with EAX register
0x11a3 reports it. This information was taken from DOS binary NBSVC.MDM.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Pali Rohár <pali.rohar@gmail.com>
Tested-by: Steven Honeyman <stevenhoneyman@gmail.com>
Tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---
 drivers/char/i8k.c |   74 +++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 61 insertions(+), 13 deletions(-)

diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
index e34a019..663868b 100644
--- a/drivers/char/i8k.c
+++ b/drivers/char/i8k.c
@@ -42,6 +42,7 @@
 #define I8K_SMM_GET_FAN		0x00a3
 #define I8K_SMM_GET_SPEED	0x02a3
 #define I8K_SMM_GET_TEMP	0x10a3
+#define I8K_SMM_GET_TEMP_TYPE	0x11a3
 #define I8K_SMM_GET_DELL_SIG1	0xfea3
 #define I8K_SMM_GET_DELL_SIG2	0xffa3
 
@@ -288,6 +289,14 @@ static int i8k_set_fan(int fan, int speed)
 	return i8k_smm(&regs) ? : i8k_get_fan_status(fan);
 }
 
+static int i8k_get_temp_type(int sensor)
+{
+	struct smm_regs regs = { .eax = I8K_SMM_GET_TEMP_TYPE, };
+
+	regs.ebx = sensor & 0xff;
+	return i8k_smm(&regs) ? : regs.eax & 0xff;
+}
+
 /*
  * Read the cpu temperature.
  */
@@ -493,6 +502,29 @@ static int i8k_open_fs(struct inode *inode, struct file *file)
  * Hwmon interface
  */
 
+static ssize_t i8k_hwmon_show_temp_label(struct device *dev,
+					 struct device_attribute *devattr,
+					 char *buf)
+{
+	static const char * const labels[] = {
+		"CPU",
+		"GPU",
+		"SODIMM",
+		"Other",
+		"Ambient",
+		"Other",
+	};
+	int index = to_sensor_dev_attr(devattr)->index;
+	int type;
+
+	type = i8k_get_temp_type(index);
+	if (type < 0)
+		return type;
+	if (type >= ARRAY_SIZE(labels))
+		type = ARRAY_SIZE(labels) - 1;
+	return sprintf(buf, "%s\n", labels[type]);
+}
+
 static ssize_t i8k_hwmon_show_temp(struct device *dev,
 				   struct device_attribute *devattr,
 				   char *buf)
@@ -555,9 +587,17 @@ static ssize_t i8k_hwmon_set_pwm(struct device *dev,
 }
 
 static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, i8k_hwmon_show_temp, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO, i8k_hwmon_show_temp_label, NULL,
+			  0);
 static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, i8k_hwmon_show_temp, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp2_label, S_IRUGO, i8k_hwmon_show_temp_label, NULL,
+			  1);
 static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, i8k_hwmon_show_temp, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp3_label, S_IRUGO, i8k_hwmon_show_temp_label, NULL,
+			  2);
 static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, i8k_hwmon_show_temp, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp4_label, S_IRUGO, i8k_hwmon_show_temp_label, NULL,
+			  3);
 static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
 			  I8K_FAN_LEFT);
 static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
@@ -569,31 +609,39 @@ static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
 
 static struct attribute *i8k_attrs[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,	/* 0 */
-	&sensor_dev_attr_temp2_input.dev_attr.attr,	/* 1 */
-	&sensor_dev_attr_temp3_input.dev_attr.attr,	/* 2 */
-	&sensor_dev_attr_temp4_input.dev_attr.attr,	/* 3 */
-	&sensor_dev_attr_fan1_input.dev_attr.attr,	/* 4 */
-	&sensor_dev_attr_pwm1.dev_attr.attr,		/* 5 */
-	&sensor_dev_attr_fan2_input.dev_attr.attr,	/* 6 */
-	&sensor_dev_attr_pwm2.dev_attr.attr,		/* 7 */
+	&sensor_dev_attr_temp1_label.dev_attr.attr,	/* 1 */
+	&sensor_dev_attr_temp2_input.dev_attr.attr,	/* 2 */
+	&sensor_dev_attr_temp2_label.dev_attr.attr,	/* 3 */
+	&sensor_dev_attr_temp3_input.dev_attr.attr,	/* 4 */
+	&sensor_dev_attr_temp3_label.dev_attr.attr,	/* 5 */
+	&sensor_dev_attr_temp4_input.dev_attr.attr,	/* 6 */
+	&sensor_dev_attr_temp4_label.dev_attr.attr,	/* 7 */
+	&sensor_dev_attr_fan1_input.dev_attr.attr,	/* 8 */
+	&sensor_dev_attr_pwm1.dev_attr.attr,		/* 9 */
+	&sensor_dev_attr_fan2_input.dev_attr.attr,	/* 10 */
+	&sensor_dev_attr_pwm2.dev_attr.attr,		/* 11 */
 	NULL
 };
 
 static umode_t i8k_is_visible(struct kobject *kobj, struct attribute *attr,
 			      int index)
 {
-	if (index == 0 && !(i8k_hwmon_flags & I8K_HWMON_HAVE_TEMP1))
+	if (index >= 0 && index <= 1 &&
+	    !(i8k_hwmon_flags & I8K_HWMON_HAVE_TEMP1))
 		return 0;
-	if (index == 1 && !(i8k_hwmon_flags & I8K_HWMON_HAVE_TEMP2))
+	if (index >= 2 && index <= 3 &&
+	    !(i8k_hwmon_flags & I8K_HWMON_HAVE_TEMP2))
 		return 0;
-	if (index == 2 && !(i8k_hwmon_flags & I8K_HWMON_HAVE_TEMP3))
+	if (index >= 4 && index <= 5 &&
+	    !(i8k_hwmon_flags & I8K_HWMON_HAVE_TEMP3))
 		return 0;
-	if (index == 3 && !(i8k_hwmon_flags & I8K_HWMON_HAVE_TEMP4))
+	if (index >= 6 && index <= 7 &&
+	    !(i8k_hwmon_flags & I8K_HWMON_HAVE_TEMP4))
 		return 0;
-	if (index >= 4 && index <= 5 &&
+	if (index >= 8 && index <= 9 &&
 	    !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN1))
 		return 0;
-	if (index >= 6 && index <= 7 &&
+	if (index >= 10 && index <= 11 &&
 	    !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2))
 		return 0;
 
-- 
1.7.9.5

[PATCH 02/10] i8k: Register only temperature sensors which have labels

[Pali Rohár]

Detect presense of sensor by calling type function instead trying to read
temperature value. Type function is working also for sensors which are temporary
turned off (e.g on GPU which is turned off). Dell DOS binary NBSVC.MDM is doing
similar checks, so we should do that too.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/char/i8k.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
index 663868b..afcc9fe 100644
--- a/drivers/char/i8k.c
+++ b/drivers/char/i8k.c
@@ -660,19 +660,19 @@ static int __init i8k_init_hwmon(void)
 
 	i8k_hwmon_flags = 0;
 
-	/* CPU temperature attributes, if temperature reading is OK */
-	err = i8k_get_temp(0);
-	if (err >= 0 || err == -ERANGE)
+	/* CPU temperature attributes, if temperature type is OK */
+	err = i8k_get_temp_type(0);
+	if (err >= 0)
 		i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP1;
 	/* check for additional temperature sensors */
-	err = i8k_get_temp(1);
-	if (err >= 0 || err == -ERANGE)
+	err = i8k_get_temp_type(1);
+	if (err >= 0)
 		i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP2;
-	err = i8k_get_temp(2);
-	if (err >= 0 || err == -ERANGE)
+	err = i8k_get_temp_type(2);
+	if (err >= 0)
 		i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP3;
-	err = i8k_get_temp(3);
-	if (err >= 0 || err == -ERANGE)
+	err = i8k_get_temp_type(3);
+	if (err >= 0)
 		i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP4;
 
 	/* Left fan attributes, if left fan is present */
-- 
1.7.9.5

[PATCH 03/10] i8k: Return -ENODATA for invalid temperature

[Pali Rohár]

Guenter Roeck suggested to return -ENODATA instead -ERANGE or -EINVAL when BIOS
reports invalid temperature value.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/char/i8k.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
index afcc9fe..1854fab 100644
--- a/drivers/char/i8k.c
+++ b/drivers/char/i8k.c
@@ -331,7 +331,7 @@ static int i8k_get_temp(int sensor)
 		prev[sensor] = temp;
 	}
 	if (temp > I8K_MAX_TEMP)
-		return -ERANGE;
+		return -ENODATA;
 #endif
 
 	return temp;
@@ -533,8 +533,6 @@ static ssize_t i8k_hwmon_show_temp(struct device *dev,
 	int temp;
 
 	temp = i8k_get_temp(index);
-	if (temp == -ERANGE)
-		return -EINVAL;
 	if (temp < 0)
 		return temp;
 	return sprintf(buf, "%d\n", temp * 1000);
-- 
1.7.9.5

[PATCH 04/10] MAINTAINERS: Fix up entry for Dell laptop SMM driver

[Pali Rohár]

From: Guenter Roeck <linux@roeck-us.net>

Mark driver as maintained.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 MAINTAINERS |    1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c444907..2c15f7a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2918,6 +2918,7 @@ F:	drivers/platform/x86/dell-laptop.c
 
 DELL LAPTOP SMM DRIVER
 M:	Guenter Roeck <linux@roeck-us.net>
+S:	Maintained
 F:	drivers/char/i8k.c
 F:	include/uapi/linux/i8k.h
 
-- 
1.7.9.5

[PATCH 05/10] i8k: Rework error retries

[Pali Rohár]

From: Guenter Roeck <linux@roeck-us.net>

Instead of returning a previous value if the SMM code returns
an error when trying to read a temperature, retry once.
If that fails again, return -ENODATA. Also return -ENODATA if an
attempt is made to read the GPU temperature but the GPU is
currently turned off.

Drop the I8K_TEMPERATURE_BUG definition and handle the related bug
unconditionally.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/char/i8k.c |   47 ++++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
index 1854fab..0e332fc 100644
--- a/drivers/char/i8k.c
+++ b/drivers/char/i8k.c
@@ -5,7 +5,7 @@
  *
  * Hwmon integration:
  * Copyright (C) 2011  Jean Delvare <jdelvare@suse.de>
- * Copyright (C) 2013  Guenter Roeck <linux@roeck-us.net>
+ * Copyright (C) 2013, 2014  Guenter Roeck <linux@roeck-us.net>
  *
  * 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
@@ -20,6 +20,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/init.h>
@@ -59,8 +60,6 @@
 #define I8K_POWER_AC		0x05
 #define I8K_POWER_BATTERY	0x01
 
-#define I8K_TEMPERATURE_BUG	1
-
 static DEFINE_MUTEX(i8k_mutex);
 static char bios_version[4];
 static struct device *i8k_hwmon_dev;
@@ -300,39 +299,41 @@ static int i8k_get_temp_type(int sensor)
 /*
  * Read the cpu temperature.
  */
-static int i8k_get_temp(int sensor)
+static int _i8k_get_temp(int sensor)
 {
-	struct smm_regs regs = { .eax = I8K_SMM_GET_TEMP, };
-	int rc;
-	int temp;
+	struct smm_regs regs = {
+		.eax = I8K_SMM_GET_TEMP,
+		.ebx = sensor & 0xff,
+	};
 
-#ifdef I8K_TEMPERATURE_BUG
-	static int prev[4] = { I8K_MAX_TEMP+1, I8K_MAX_TEMP+1, I8K_MAX_TEMP+1, I8K_MAX_TEMP+1 };
-#endif
-	regs.ebx = sensor & 0xff;
-	rc = i8k_smm(&regs);
-	if (rc < 0)
-		return rc;
+	return i8k_smm(&regs) ? : regs.eax & 0xff;
+}
 
-	temp = regs.eax & 0xff;
+static int i8k_get_temp(int sensor)
+{
+	int temp = _i8k_get_temp(sensor);
 
-#ifdef I8K_TEMPERATURE_BUG
 	/*
 	 * Sometimes the temperature sensor returns 0x99, which is out of range.
-	 * In this case we return (once) the previous cached value. For example:
+	 * In this case we retry (once) before returning an error.
 	 # 1003655137 00000058 00005a4b
 	 # 1003655138 00000099 00003a80 <--- 0x99 = 153 degrees
 	 # 1003655139 00000054 00005c52
 	 */
-	if (temp > I8K_MAX_TEMP) {
-		temp = prev[sensor];
-		prev[sensor] = I8K_MAX_TEMP+1;
-	} else {
-		prev[sensor] = temp;
+	if (temp == 0x99) {
+		msleep(100);
+		temp = _i8k_get_temp(sensor);
 	}
+	/*
+	 * Return -ENODATA for all invalid temperatures.
+	 *
+	 * Known instances are the 0x99 value as seen above as well as
+	 * 0xc1 (193), which may be returned when trying to read the GPU
+	 * temperature if the system supports a GPU and it is currently
+	 * turned off.
+	 */
 	if (temp > I8K_MAX_TEMP)
 		return -ENODATA;
-#endif
 
 	return temp;
 }
-- 
1.7.9.5

[PATCH 06/10] i8k: Add support for Dell XPS 13

[Pali Rohár]

From: Guenter Roeck <linux@roeck-us.net>

XPS 13 does not support turbo speed, so its initialization data
matches that of XPS M140. Make XPS initialization data generic,
and add support for XPS 13.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/char/i8k.c |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
index 0e332fc..8ec4c37 100644
--- a/drivers/char/i8k.c
+++ b/drivers/char/i8k.c
@@ -705,7 +705,7 @@ enum i8k_configs {
 	DELL_LATITUDE_E6540,
 	DELL_PRECISION_490,
 	DELL_STUDIO,
-	DELL_XPS_M140,
+	DELL_XPS,
 };
 
 static const struct i8k_config_data i8k_config_data[] = {
@@ -725,7 +725,7 @@ static const struct i8k_config_data i8k_config_data[] = {
 		.fan_mult = 1,
 		.fan_max = I8K_FAN_HIGH,
 	},
-	[DELL_XPS_M140] = {
+	[DELL_XPS] = {
 		.fan_mult = 1,
 		.fan_max = I8K_FAN_HIGH,
 	},
@@ -837,12 +837,20 @@ static struct dmi_system_id i8k_dmi_table[] __initdata = {
 		.driver_data = (void *)&i8k_config_data[DELL_STUDIO],
 	},
 	{
+		.ident = "Dell XPS 13",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "XPS13"),
+		},
+		.driver_data = (void *)&i8k_config_data[DELL_XPS],
+	},
+	{
 		.ident = "Dell XPS M140",
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
 			DMI_MATCH(DMI_PRODUCT_NAME, "MXC051"),
 		},
-		.driver_data = (void *)&i8k_config_data[DELL_XPS_M140],
+		.driver_data = (void *)&i8k_config_data[DELL_XPS],
 	},
 	{ }
 };
-- 
1.7.9.5

[PATCH 07/10] i8k: Make fan module parameters an unsigned

[Pali Rohár]

Setting negative fan multiplier or maximal fan speed does make any sense and
can cause problems. So ensure that negative values will not be accepted.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/char/i8k.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
index 8ec4c37..d6e8a26 100644
--- a/drivers/char/i8k.c
+++ b/drivers/char/i8k.c
@@ -64,9 +64,9 @@ static DEFINE_MUTEX(i8k_mutex);
 static char bios_version[4];
 static struct device *i8k_hwmon_dev;
 static u32 i8k_hwmon_flags;
-static int i8k_fan_mult;
-static int i8k_pwm_mult;
-static int i8k_fan_max = I8K_FAN_HIGH;
+static uint i8k_fan_mult;
+static uint i8k_pwm_mult;
+static uint i8k_fan_max = I8K_FAN_HIGH;
 
 #define I8K_HWMON_HAVE_TEMP1	(1 << 0)
 #define I8K_HWMON_HAVE_TEMP2	(1 << 1)
@@ -95,12 +95,12 @@ static bool power_status;
 module_param(power_status, bool, 0600);
 MODULE_PARM_DESC(power_status, "Report power status in /proc/i8k");
 
-static int fan_mult = I8K_FAN_MULT;
-module_param(fan_mult, int, 0);
+static uint fan_mult = I8K_FAN_MULT;
+module_param(fan_mult, uint, 0);
 MODULE_PARM_DESC(fan_mult, "Factor to multiply fan speed with");
 
-static int fan_max = I8K_FAN_HIGH;
-module_param(fan_max, int, 0);
+static uint fan_max = I8K_FAN_HIGH;
+module_param(fan_max, uint, 0);
 MODULE_PARM_DESC(fan_max, "Maximum configurable fan speed");
 
 static int i8k_open_fs(struct inode *inode, struct file *file);
@@ -696,8 +696,8 @@ static int __init i8k_init_hwmon(void)
 }
 
 struct i8k_config_data {
-	int fan_mult;
-	int fan_max;
+	uint fan_mult;
+	uint fan_max;
 };
 
 enum i8k_configs {
-- 
1.7.9.5

[PATCH 08/10] i8k: Autodetect maximal fan speed and fan RPM multiplier

[Pali Rohár]

This patch adds new function i8k_get_fan_nominal_speed() for doing SMM call
which will return nominal fan RPM for specified fan speed. It returns nominal
RPM value at which fan operate when speed (0, 1, 2, 3) is set. It looks like
RPM value is not accurate, but still provides very useful information.

First it can be used to validate if certain fan speed could be accepted by SMM
for setting fan speed and we can use this routine to detect maximal fan speed.

Second it returns RPM value, so we can check if value looks correct with
multiplier 30 or multiplier 1 (until now only these two multiplier were used).
If RPM value with multiplier 30 is too high, then multiplier 1 is used.

In case when SMM reports that new function is not supported we will fallback
to old hardcoded values. Maximal fan speed would be 2 and RPM multiplier 30.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Tested-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/char/i8k.c |   85 +++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 74 insertions(+), 11 deletions(-)

diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
index d6e8a26..4c66788 100644
--- a/drivers/char/i8k.c
+++ b/drivers/char/i8k.c
@@ -6,6 +6,7 @@
  * 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>
  *
  * 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
@@ -42,12 +43,14 @@
 #define I8K_SMM_SET_FAN		0x01a3
 #define I8K_SMM_GET_FAN		0x00a3
 #define I8K_SMM_GET_SPEED	0x02a3
+#define I8K_SMM_GET_NOM_SPEED	0x04a3
 #define I8K_SMM_GET_TEMP	0x10a3
 #define I8K_SMM_GET_TEMP_TYPE	0x11a3
 #define I8K_SMM_GET_DELL_SIG1	0xfea3
 #define I8K_SMM_GET_DELL_SIG2	0xffa3
 
 #define I8K_FAN_MULT		30
+#define I8K_FAN_MAX_RPM		30000
 #define I8K_MAX_TEMP		127
 
 #define I8K_FN_NONE		0x00
@@ -64,7 +67,7 @@ static DEFINE_MUTEX(i8k_mutex);
 static char bios_version[4];
 static struct device *i8k_hwmon_dev;
 static u32 i8k_hwmon_flags;
-static uint i8k_fan_mult;
+static uint i8k_fan_mult = I8K_FAN_MULT;
 static uint i8k_pwm_mult;
 static uint i8k_fan_max = I8K_FAN_HIGH;
 
@@ -95,13 +98,13 @@ static bool power_status;
 module_param(power_status, bool, 0600);
 MODULE_PARM_DESC(power_status, "Report power status in /proc/i8k");
 
-static uint fan_mult = I8K_FAN_MULT;
+static uint fan_mult;
 module_param(fan_mult, uint, 0);
-MODULE_PARM_DESC(fan_mult, "Factor to multiply fan speed with");
+MODULE_PARM_DESC(fan_mult, "Factor to multiply fan speed with (default: autodetect)");
 
-static uint fan_max = I8K_FAN_HIGH;
+static uint fan_max;
 module_param(fan_max, uint, 0);
-MODULE_PARM_DESC(fan_max, "Maximum configurable fan speed");
+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);
@@ -276,6 +279,17 @@ static int i8k_get_fan_speed(int fan)
 }
 
 /*
+ * Read the fan nominal rpm for specific fan speed.
+ */
+static int i8k_get_fan_nominal_speed(int fan, int speed)
+{
+	struct smm_regs regs = { .eax = I8K_SMM_GET_NOM_SPEED, };
+
+	regs.ebx = (fan & 0xff) | (speed << 8);
+	return i8k_smm(&regs) ? : (regs.eax & 0xffff) * i8k_fan_mult;
+}
+
+/*
  * Set the fan speed (off, low, high). Returns the new fan status.
  */
 static int i8k_set_fan(int fan, int speed)
@@ -863,6 +877,7 @@ MODULE_DEVICE_TABLE(dmi, i8k_dmi_table);
 static int __init i8k_probe(void)
 {
 	const struct dmi_system_id *id;
+	int fan, val, ret;
 
 	/*
 	 * Get DMI information
@@ -891,19 +906,67 @@ static int __init i8k_probe(void)
 			return -ENODEV;
 	}
 
-	i8k_fan_mult = fan_mult;
-	i8k_fan_max = fan_max ? : I8K_FAN_HIGH;	/* Must not be 0 */
+	/*
+	 * Autodetect fan multiplier and maximal fan speed from dmi config
+	 * Values specified in module parameters override values from dmi
+	 */
 	id = dmi_first_match(i8k_dmi_table);
 	if (id && id->driver_data) {
 		const struct i8k_config_data *conf = id->driver_data;
+		if (!fan_mult && conf->fan_mult)
+			fan_mult = conf->fan_mult;
+		if (!fan_max && conf->fan_max)
+			fan_max = conf->fan_max;
+	}
 
-		if (fan_mult == I8K_FAN_MULT && conf->fan_mult)
-			i8k_fan_mult = conf->fan_mult;
-		if (fan_max == I8K_FAN_HIGH && conf->fan_max)
-			i8k_fan_max = conf->fan_max;
+	if (!fan_max) {
+		/*
+		 * Autodetect maximal fan speed value
+		 * Speed value is valid if i8k_get_fan_nominal_speed() not fail
+		 */
+		for (fan = 0; fan < 2; ++fan) {
+			for (val = 1; val < 256; ++val) {
+				if (i8k_get_fan_nominal_speed(fan, val) < 0)
+					break;
+				fan_max = val; /* Must not be 0 */
+			}
+			if (fan_max) {
+				i8k_fan_max = fan_max;
+				break;
+			}
+		}
+	} else {
+		/* Maximal fan speed was specified in module param or in dmi */
+		i8k_fan_max = fan_max;
 	}
+
 	i8k_pwm_mult = DIV_ROUND_UP(255, i8k_fan_max);
 
+	if (!fan_mult) {
+		/*
+		 * Autodetect fan multiplier based on nominal rpm
+		 * If fan reports rpm value too high then set multiplier to 1
+		 *
+		 * Try also setting multiplier from current rpm, but this will
+		 * work only in case when fan is not turned off. It is better
+		 * then nothing for machines which does not support nominal rpm
+		 * SMM function.
+		 */
+		for (fan = 0; fan < 2; ++fan) {
+			ret = i8k_get_fan_nominal_speed(fan, i8k_fan_max);
+			if (ret < 0)
+				ret = i8k_get_fan_speed(fan);
+			if (ret < 0)
+				continue;
+			if (ret > I8K_FAN_MAX_RPM)
+				i8k_fan_mult = 1;
+			break;
+		}
+	} else {
+		/* Fan multiplier was specified in module param or in dmi */
+		i8k_fan_mult = fan_mult;
+	}
+
 	return 0;
 }
 
-- 
1.7.9.5

Re: [PATCH 08/10] i8k: Autodetect maximal fan speed and fan RPM multiplier

[Guenter Roeck]

On 12/21/2014 11:54 AM, Pali Rohár wrote:
> This patch adds new function i8k_get_fan_nominal_speed() for doing SMM call
> which will return nominal fan RPM for specified fan speed. It returns nominal
> RPM value at which fan operate when speed (0, 1, 2, 3) is set. It looks like
> RPM value is not accurate, but still provides very useful information.
>
> First it can be used to validate if certain fan speed could be accepted by SMM
> for setting fan speed and we can use this routine to detect maximal fan speed.
>
> Second it returns RPM value, so we can check if value looks correct with
> multiplier 30 or multiplier 1 (until now only these two multiplier were used).
> If RPM value with multiplier 30 is too high, then multiplier 1 is used.
>
> In case when SMM reports that new function is not supported we will fallback
> to old hardcoded values. Maximal fan speed would be 2 and RPM multiplier 30.
>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> Tested-by: Pali Rohár <pali.rohar@gmail.com>

As mentioned earlier, we can unfortunately not rely on fan speed range
auto-detection. Maybe you can take that part out for now, and we can look
at it again separately.

Thanks,
Guenter

[PATCH 09/10] i8k: Remove DMI config data for Latitude E6440 and E6540

[Pali Rohár]

Both Dell Latitude machines were tested with new fan autodetection code and they
are working fine. We already have DMI_MATCH data for generic Latitude machines
which match also E6440 and E6540 models. So we do not need to maintain DMI data
for those specific machines anymore.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Tested-by: Pali Rohár <pali.rohar@gmail.com>
Tested-by: Steven Honeyman <stevenhoneyman@gmail.com>
---
 drivers/char/i8k.c |   21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
index 4c66788..9288edb 100644
--- a/drivers/char/i8k.c
+++ b/drivers/char/i8k.c
@@ -716,7 +716,6 @@ struct i8k_config_data {
 
 enum i8k_configs {
 	DELL_LATITUDE_D520,
-	DELL_LATITUDE_E6540,
 	DELL_PRECISION_490,
 	DELL_STUDIO,
 	DELL_XPS,
@@ -727,10 +726,6 @@ static const struct i8k_config_data i8k_config_data[] = {
 		.fan_mult = 1,
 		.fan_max = I8K_FAN_TURBO,
 	},
-	[DELL_LATITUDE_E6540] = {
-		.fan_mult = 1,
-		.fan_max = I8K_FAN_HIGH,
-	},
 	[DELL_PRECISION_490] = {
 		.fan_mult = 1,
 		.fan_max = I8K_FAN_TURBO,
@@ -776,22 +771,6 @@ static struct dmi_system_id i8k_dmi_table[] __initdata = {
 		.driver_data = (void *)&i8k_config_data[DELL_LATITUDE_D520],
 	},
 	{
-		.ident = "Dell Latitude E6440",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "Latitude E6440"),
-		},
-		.driver_data = (void *)&i8k_config_data[DELL_LATITUDE_E6540],
-	},
-	{
-		.ident = "Dell Latitude E6540",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "Latitude E6540"),
-		},
-		.driver_data = (void *)&i8k_config_data[DELL_LATITUDE_E6540],
-	},
-	{
 		.ident = "Dell Latitude 2",
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
-- 
1.7.9.5

Re: [PATCH 09/10] i8k: Remove DMI config data for Latitude E6440 and E6540

[Guenter Roeck]

On 12/21/2014 11:54 AM, Pali Rohár wrote:
> Both Dell Latitude machines were tested with new fan autodetection code and they
> are working fine. We already have DMI_MATCH data for generic Latitude machines
> which match also E6440 and E6540 models. So we do not need to maintain DMI data
> for those specific machines anymore.
>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> Tested-by: Pali Rohár <pali.rohar@gmail.com>
> Tested-by: Steven Honeyman <stevenhoneyman@gmail.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

[PATCH 10/10] i8k: Add support for fan labels

[Pali Rohár]

This patch adds labels support for fans if SMM function with EAX register
0x03a3 reports it. This information was taken from DOS binary NBSVC.MDM.

Additionally this patch change detection of fan presece. Instead reading fan
status now detection is based on new label SMM function. Dell DOS binary
NBSVC.MDM is doing similar checks, so we should do that too.

This patch also remove I8K_FAN_LEFT and I8K_FAN_RIGHT usage from hwmon driver
part because that names does not make sense anymore. So numeric constants are
used instead. Original /proc/i8k ioctl part was not changed for compatibility
reasons.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/char/i8k.c |   76 ++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 62 insertions(+), 14 deletions(-)

diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
index 9288edb..ca4203c 100644
--- a/drivers/char/i8k.c
+++ b/drivers/char/i8k.c
@@ -43,6 +43,7 @@
 #define I8K_SMM_SET_FAN		0x01a3
 #define I8K_SMM_GET_FAN		0x00a3
 #define I8K_SMM_GET_SPEED	0x02a3
+#define I8K_SMM_GET_FAN_TYPE	0x03a3
 #define I8K_SMM_GET_NOM_SPEED	0x04a3
 #define I8K_SMM_GET_TEMP	0x10a3
 #define I8K_SMM_GET_TEMP_TYPE	0x11a3
@@ -279,6 +280,17 @@ static int i8k_get_fan_speed(int fan)
 }
 
 /*
+ * Read the fan type.
+ */
+static int i8k_get_fan_type(int fan)
+{
+	struct smm_regs regs = { .eax = I8K_SMM_GET_FAN_TYPE, };
+
+	regs.ebx = fan & 0xff;
+	return i8k_smm(&regs) ? : regs.eax & 0xff;
+}
+
+/*
  * Read the fan nominal rpm for specific fan speed.
  */
 static int i8k_get_fan_nominal_speed(int fan, int speed)
@@ -553,6 +565,37 @@ static ssize_t i8k_hwmon_show_temp(struct device *dev,
 	return sprintf(buf, "%d\n", temp * 1000);
 }
 
+static ssize_t i8k_hwmon_show_fan_label(struct device *dev,
+					struct device_attribute *devattr,
+					char *buf)
+{
+	static const char * const labels[] = {
+		"Processor Fan",
+		"Motherboard Fan",
+		"Video Fan",
+		"Power Supply Fan",
+		"Chipset Fan",
+		"Other Fan",
+	};
+	int index = to_sensor_dev_attr(devattr)->index;
+	bool dock = false;
+	int type;
+
+	type = i8k_get_fan_type(index);
+	if (type < 0)
+		return type;
+
+	if (type & 0x10) {
+		dock = true;
+		type &= 0x0F;
+	}
+
+	if (type >= ARRAY_SIZE(labels))
+		type = (ARRAY_SIZE(labels) - 1);
+
+	return sprintf(buf, "%s%s\n", (dock ? "Docking " : ""), labels[type]);
+}
+
 static ssize_t i8k_hwmon_show_fan(struct device *dev,
 				  struct device_attribute *devattr,
 				  char *buf)
@@ -611,14 +654,17 @@ static SENSOR_DEVICE_ATTR(temp3_label, S_IRUGO, i8k_hwmon_show_temp_label, NULL,
 static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, i8k_hwmon_show_temp, NULL, 3);
 static SENSOR_DEVICE_ATTR(temp4_label, S_IRUGO, i8k_hwmon_show_temp_label, NULL,
 			  3);
-static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
-			  I8K_FAN_LEFT);
+static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, i8k_hwmon_show_fan, NULL, 0);
+static SENSOR_DEVICE_ATTR(fan1_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL,
+			  0);
 static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
-			  i8k_hwmon_set_pwm, I8K_FAN_LEFT);
+			  i8k_hwmon_set_pwm, 0);
 static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
-			  I8K_FAN_RIGHT);
+			  1);
+static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL,
+			  1);
 static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
-			  i8k_hwmon_set_pwm, I8K_FAN_RIGHT);
+			  i8k_hwmon_set_pwm, 1);
 
 static struct attribute *i8k_attrs[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,	/* 0 */
@@ -630,9 +676,11 @@ static struct attribute *i8k_attrs[] = {
 	&sensor_dev_attr_temp4_input.dev_attr.attr,	/* 6 */
 	&sensor_dev_attr_temp4_label.dev_attr.attr,	/* 7 */
 	&sensor_dev_attr_fan1_input.dev_attr.attr,	/* 8 */
-	&sensor_dev_attr_pwm1.dev_attr.attr,		/* 9 */
-	&sensor_dev_attr_fan2_input.dev_attr.attr,	/* 10 */
-	&sensor_dev_attr_pwm2.dev_attr.attr,		/* 11 */
+	&sensor_dev_attr_fan1_label.dev_attr.attr,	/* 9 */
+	&sensor_dev_attr_pwm1.dev_attr.attr,		/* 10 */
+	&sensor_dev_attr_fan2_input.dev_attr.attr,	/* 11 */
+	&sensor_dev_attr_fan2_label.dev_attr.attr,	/* 12 */
+	&sensor_dev_attr_pwm2.dev_attr.attr,		/* 13 */
 	NULL
 };
 
@@ -651,10 +699,10 @@ static umode_t i8k_is_visible(struct kobject *kobj, struct attribute *attr,
 	if (index >= 6 && index <= 7 &&
 	    !(i8k_hwmon_flags & I8K_HWMON_HAVE_TEMP4))
 		return 0;
-	if (index >= 8 && index <= 9 &&
+	if (index >= 8 && index <= 10 &&
 	    !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN1))
 		return 0;
-	if (index >= 10 && index <= 11 &&
+	if (index >= 11 && index <= 13 &&
 	    !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2))
 		return 0;
 
@@ -688,13 +736,13 @@ static int __init i8k_init_hwmon(void)
 	if (err >= 0)
 		i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP4;
 
-	/* Left fan attributes, if left fan is present */
-	err = i8k_get_fan_status(I8K_FAN_LEFT);
+	/* First fan attributes, if fan type is OK */
+	err = i8k_get_fan_type(0);
 	if (err >= 0)
 		i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN1;
 
-	/* Right fan attributes, if right fan is present */
-	err = i8k_get_fan_status(I8K_FAN_RIGHT);
+	/* Second fan attributes, if fan type is OK */
+	err = i8k_get_fan_type(1);
 	if (err >= 0)
 		i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
 
-- 
1.7.9.5

Re: [PATCH 10/10] i8k: Add support for fan labels

[Guenter Roeck]

On 12/21/2014 11:54 AM, Pali Rohár wrote:
> This patch adds labels support for fans if SMM function with EAX register
> 0x03a3 reports it. This information was taken from DOS binary NBSVC.MDM.
>
> Additionally this patch change detection of fan presece. Instead reading fan
> status now detection is based on new label SMM function. Dell DOS binary
> NBSVC.MDM is doing similar checks, so we should do that too.
>
> This patch also remove I8K_FAN_LEFT and I8K_FAN_RIGHT usage from hwmon driver
> part because that names does not make sense anymore. So numeric constants are
> used instead. Original /proc/i8k ioctl part was not changed for compatibility
> reasons.
>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>   drivers/char/i8k.c |   76 ++++++++++++++++++++++++++++++++++++++++++----------
>   1 file changed, 62 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
> index 9288edb..ca4203c 100644
> --- a/drivers/char/i8k.c
> +++ b/drivers/char/i8k.c
> @@ -43,6 +43,7 @@
>   #define I8K_SMM_SET_FAN		0x01a3
>   #define I8K_SMM_GET_FAN		0x00a3
>   #define I8K_SMM_GET_SPEED	0x02a3
> +#define I8K_SMM_GET_FAN_TYPE	0x03a3
>   #define I8K_SMM_GET_NOM_SPEED	0x04a3
>   #define I8K_SMM_GET_TEMP	0x10a3
>   #define I8K_SMM_GET_TEMP_TYPE	0x11a3
> @@ -279,6 +280,17 @@ static int i8k_get_fan_speed(int fan)
>   }
>
>   /*
> + * Read the fan type.
> + */
> +static int i8k_get_fan_type(int fan)
> +{
> +	struct smm_regs regs = { .eax = I8K_SMM_GET_FAN_TYPE, };
> +
> +	regs.ebx = fan & 0xff;
> +	return i8k_smm(&regs) ? : regs.eax & 0xff;
> +}
> +
> +/*
>    * Read the fan nominal rpm for specific fan speed.
>    */
>   static int i8k_get_fan_nominal_speed(int fan, int speed)
> @@ -553,6 +565,37 @@ static ssize_t i8k_hwmon_show_temp(struct device *dev,
>   	return sprintf(buf, "%d\n", temp * 1000);
>   }
>
> +static ssize_t i8k_hwmon_show_fan_label(struct device *dev,
> +					struct device_attribute *devattr,
> +					char *buf)
> +{
> +	static const char * const labels[] = {
> +		"Processor Fan",
> +		"Motherboard Fan",
> +		"Video Fan",
> +		"Power Supply Fan",
> +		"Chipset Fan",
> +		"Other Fan",
> +	};
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	bool dock = false;
> +	int type;
> +
> +	type = i8k_get_fan_type(index);
> +	if (type < 0)
> +		return type;
> +
> +	if (type & 0x10) {
> +		dock = true;
> +		type &= 0x0F;
> +	}
> +
What if bit 5..7 is set ? This would result in a result of "other fan",
which given the above does not seem to be correct. Given that, I wonder
why the type mask is only applied if bit 4 is set and not unconditionally.

Thanks,
Guenter

> +	if (type >= ARRAY_SIZE(labels))
> +		type = (ARRAY_SIZE(labels) - 1);
> +
> +	return sprintf(buf, "%s%s\n", (dock ? "Docking " : ""), labels[type]);
> +}
> +
>   static ssize_t i8k_hwmon_show_fan(struct device *dev,
>   				  struct device_attribute *devattr,
>   				  char *buf)
> @@ -611,14 +654,17 @@ static SENSOR_DEVICE_ATTR(temp3_label, S_IRUGO, i8k_hwmon_show_temp_label, NULL,
>   static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, i8k_hwmon_show_temp, NULL, 3);
>   static SENSOR_DEVICE_ATTR(temp4_label, S_IRUGO, i8k_hwmon_show_temp_label, NULL,
>   			  3);
> -static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
> -			  I8K_FAN_LEFT);
> +static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, i8k_hwmon_show_fan, NULL, 0);
> +static SENSOR_DEVICE_ATTR(fan1_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL,
> +			  0);
>   static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
> -			  i8k_hwmon_set_pwm, I8K_FAN_LEFT);
> +			  i8k_hwmon_set_pwm, 0);
>   static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
> -			  I8K_FAN_RIGHT);
> +			  1);
> +static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL,
> +			  1);
>   static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
> -			  i8k_hwmon_set_pwm, I8K_FAN_RIGHT);
> +			  i8k_hwmon_set_pwm, 1);
>
>   static struct attribute *i8k_attrs[] = {
>   	&sensor_dev_attr_temp1_input.dev_attr.attr,	/* 0 */
> @@ -630,9 +676,11 @@ static struct attribute *i8k_attrs[] = {
>   	&sensor_dev_attr_temp4_input.dev_attr.attr,	/* 6 */
>   	&sensor_dev_attr_temp4_label.dev_attr.attr,	/* 7 */
>   	&sensor_dev_attr_fan1_input.dev_attr.attr,	/* 8 */
> -	&sensor_dev_attr_pwm1.dev_attr.attr,		/* 9 */
> -	&sensor_dev_attr_fan2_input.dev_attr.attr,	/* 10 */
> -	&sensor_dev_attr_pwm2.dev_attr.attr,		/* 11 */
> +	&sensor_dev_attr_fan1_label.dev_attr.attr,	/* 9 */
> +	&sensor_dev_attr_pwm1.dev_attr.attr,		/* 10 */
> +	&sensor_dev_attr_fan2_input.dev_attr.attr,	/* 11 */
> +	&sensor_dev_attr_fan2_label.dev_attr.attr,	/* 12 */
> +	&sensor_dev_attr_pwm2.dev_attr.attr,		/* 13 */
>   	NULL
>   };
>
> @@ -651,10 +699,10 @@ static umode_t i8k_is_visible(struct kobject *kobj, struct attribute *attr,
>   	if (index >= 6 && index <= 7 &&
>   	    !(i8k_hwmon_flags & I8K_HWMON_HAVE_TEMP4))
>   		return 0;
> -	if (index >= 8 && index <= 9 &&
> +	if (index >= 8 && index <= 10 &&
>   	    !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN1))
>   		return 0;
> -	if (index >= 10 && index <= 11 &&
> +	if (index >= 11 && index <= 13 &&
>   	    !(i8k_hwmon_flags & I8K_HWMON_HAVE_FAN2))
>   		return 0;
>
> @@ -688,13 +736,13 @@ static int __init i8k_init_hwmon(void)
>   	if (err >= 0)
>   		i8k_hwmon_flags |= I8K_HWMON_HAVE_TEMP4;
>
> -	/* Left fan attributes, if left fan is present */
> -	err = i8k_get_fan_status(I8K_FAN_LEFT);
> +	/* First fan attributes, if fan type is OK */
> +	err = i8k_get_fan_type(0);
>   	if (err >= 0)
>   		i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN1;
>
> -	/* Right fan attributes, if right fan is present */
> -	err = i8k_get_fan_status(I8K_FAN_RIGHT);
> +	/* Second fan attributes, if fan type is OK */
> +	err = i8k_get_fan_type(1);
>   	if (err >= 0)
>   		i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN2;
>
>

Re: [PATCH 10/10] i8k: Add support for fan labels

[Pali Rohár]

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

On Monday 22 December 2014 00:01:49 Guenter Roeck wrote:
> > +static ssize_t i8k_hwmon_show_fan_label(struct device *dev,
> > +					struct device_attribute *devattr,
> > +					char *buf)
> > +{
> > +	static const char * const labels[] = {
> > +		"Processor Fan",
> > +		"Motherboard Fan",
> > +		"Video Fan",
> > +		"Power Supply Fan",
> > +		"Chipset Fan",
> > +		"Other Fan",
> > +	};
> > +	int index = to_sensor_dev_attr(devattr)->index;
> > +	bool dock = false;
> > +	int type;
> > +
> > +	type = i8k_get_fan_type(index);
> > +	if (type < 0)
> > +		return type;
> > +
> > +	if (type & 0x10) {
> > +		dock = true;
> > +		type &= 0x0F;
> > +	}
> > +
> 
> What if bit 5..7 is set ? This would result in a result of
> "other fan", which given the above does not seem to be
> correct. Given that, I wonder why the type mask is only
> applied if bit 4 is set and not unconditionally.
> 
> Thanks,
> Guenter
> 

NBSVC.MDM maps values 0x0-0x5 to exactly to strings in labels[]. 
Next it maps 0x10-0x15 to values same values in strings labels[] 
but with prefix "Docking". I do not see nothing more in NBSVC.MDM 
so for other values is "Other Fan" better than nothing.

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

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