All of lore.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH 4/4] hwmon: (imanager2) Add support for IT8516/18/28
@ 2014-08-07  9:24 Wei-Chun Pan
  2014-08-07 10:06 ` Lee Jones
  0 siblings, 1 reply; 4+ messages in thread
From: Wei-Chun Pan @ 2014-08-07  9:24 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Samuel Ortiz, Lee Jones, Jean Delvare, Wolfram Sang, Louis.Lu,
	Neo.Lo, Hank.Peng, Kevin.Ong, linux-kernel, Wei-Chun Pan

Sorry, I sent wrong patch just now. Please ignore the mail "[PATCH 1/3]
 imanager2: rename io functions and remove no used functions".

This mail include 2 patches:
The first patch is shown I rename some functions.
The second patch is shown I moditfy code according to your comment.

Signed-off-by: Wei-Chun Pan <weichun.pan@advantech.com.tw>

---
diff --git a/drivers/hwmon/imanager2_hwm.c b/drivers/hwmon/imanager2_hwm.c
index 335bffb..ab63296 100644
--- a/drivers/hwmon/imanager2_hwm.c
+++ b/drivers/hwmon/imanager2_hwm.c
@@ -119,17 +119,17 @@ static int imanager2_volt_get_value_by_io(struct imanager2 *ec, int index,
 	u8 portnum;
 	int ret;
 
-	ret = imanager2_mbox_io_read(EC_CMD_ADC_INDEX, pin, &portnum, 1);
+	ret = imanager2_io_read_data(EC_CMD_ADC_INDEX, pin, &portnum, 1);
 	if (ret)
 		return ret;
 	if (portnum == EC_ERROR)
 		return -ENXIO;
 
-	ret = imanager2_mbox_io_simple_read(EC_CMD_ADC_READ_LSB, &buf[1]);
+	ret = imanager2_io_nooffset_readbyte(EC_CMD_ADC_READ_LSB, &buf[1]);
 	if (ret)
 		return ret;
 
-	return imanager2_mbox_io_simple_read(EC_CMD_ADC_READ_MSB, &buf[0]);
+	return imanager2_io_nooffset_readbyte(EC_CMD_ADC_READ_MSB, &buf[0]);
 }
 
 static int imanager2_volt_get_value(struct imanager2 *ec, int index,
@@ -468,7 +468,7 @@ static void imanager2_temp_init(struct imanager2 *ec)
 							      (u8 *)&zone,
 							      &len);
 		} else {
-			ret = imanager2_mbox_io_read(
+			ret = imanager2_io_read_data(
 				EC_CMD_HWRAM_READ,
 				EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_STATUS(thm),
 				&zone.status, 1);
 
@@ -596,7 +593,7 @@ static int imanager2_fan_get_value(struct imanager2 *ec, int index,
 					       EC_CMD_MAILBOX_READ_HW_PIN,
 					       ec_fan_table[index].did, tmp, 2);
 	else
-		ret = imanager2_mbox_io_read(EC_CMD_ACPIRAM_READ,
+		ret = imanager2_io_read_data(EC_CMD_ACPIRAM_READ,
 					     ec_fan_table[index].fspeed_acpireg,
 					     tmp, 2);
 
@@ -621,7 +618,7 @@ static int imanager2_fan_item_init_by_io(struct imanager2 *ec, int fnum)
 	u8 tmp;
 
 	mutex_lock(&ec->lock);
-	ret = imanager2_mbox_io_read(EC_CMD_HWRAM_READ,
+	ret = imanager2_io_read_data(EC_CMD_HWRAM_READ,
 				     EC_HWRAM_ADDR_FAN_CONTROL(fnum), &tmp, 1);
 	mutex_unlock(&ec->lock);

---
> > ---
> >  drivers/hwmon/Kconfig         |   5 +
> >  drivers/hwmon/Makefile        |   1 +
> >  drivers/hwmon/imanager2_hwm.c | 768
> ++++++++++++++++++++++++++++++++++++++++++
> 
> Documentation/hwmon/imanager2_hwm missing.

diff --git a/Documentation/hwmon/imanager2_hwm b/Documentation/hwmon/imanager2_hwm
new file mode 100644
index 0000000..bf3d0b5
--- /dev/null
+++ b/Documentation/hwmon/imanager2_hwm
@@ -0,0 +1,42 @@
+Kernel driver imanager2_hwm
+===========================
+
+Supported chips:
+  * ITE IT8516
+    Prefix: 'it8516'
+    Addresses scanned: I/O chennel 0x029A/0x0299,
+                       IET mailbox chennel 0x029E/0x029F
+    Datasheet: Not publicly available
+  * ITE IT8518
+    Prefix: 'it8518'
+    Addresses scanned: I/O chennel 0x029A/0x0299,
+                       IET mailbox chennel 0x029E/0x029F
+    Datasheet: Not publicly available
+  * ITE IT8528
+    Prefix: 'it8528'
+    Addresses scanned: I/O chennel 0x029A/0x0299
+    Datasheet: Not publicly available
+
+Authors:
+        Richard Vidal-Dorsch <richard.dorsch@advantech.com>
+
+
+Description
+-----------
+
+This driver supports the hardware monitoring features of the IT8516, IT8518, and
+IT8528 chips. These features include 11 voltage sensors, 1 current sensor, 4
+temperature sensors, and 3 fan rotation speed sensors.
+
+ITE IT8516, IT8518, and IT8528 are are 'EC chips'. These chips are like Super I/O control boards. For IT8516 and IT 8518 The control chennel can be I/O or ITE mailbox chennel. I/O chennel is a common way but ITE mailbox chennel performance faster since it does not need to wait IBF (input buffer full) and OBF (output buffer full) to before send or get data.
+
+ITE IT8528 use an I/O chennel way to access mailbox, called I/O mailbox for cost down. Its performance the between pure I/O controller and ITE mailbox controller.
+
+
+sysfs-Interface
+---------------
+
+in[0-11]_input	- adc voltage input
+curr1_input	- adc current input
+temp[1-4]_input	- temperature input
+fan[1-3]_input	- fan speed input

> 
> >  3 files changed, 774 insertions(+)
> >  create mode 100644 drivers/hwmon/imanager2_hwm.c
> >
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index bc196f4..7524fc3 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -39,6 +39,11 @@ config HWMON_DEBUG_CHIP
> >
> >  comment "Native drivers"
> >
> > +config SENSORS_IMANAGER2
> > +	tristate "Support for Advantech iManager2 EC H.W. Monitor"
> > +	select MFD_CORE
> > +	depends on MFD_IMANAGER2
> > +
> Alphabetic order please.
> 
> >  config SENSORS_AB8500
> >  	tristate "AB8500 thermal monitoring"
> >  	depends on AB8500_GPADC && AB8500_BM
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index c48f987..a2c8f07 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -146,6 +146,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_SENSORS_IMANAGER2)	+= imanager2_hwm.o
> >
> Alphabetic order please.
> 
 drivers/hwmon/Kconfig  | 10 +++++-----
 drivers/hwmon/Makefile |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 7524fc3..e39f8e0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -39,11 +39,6 @@ config HWMON_DEBUG_CHIP
 
 comment "Native drivers"
 
-config SENSORS_IMANAGER2
-	tristate "Support for Advantech iManager2 EC H.W. Monitor"
-	select MFD_CORE
-	depends on MFD_IMANAGER2
-
 config SENSORS_AB8500
 	tristate "AB8500 thermal monitoring"
 	depends on AB8500_GPADC && AB8500_BM
@@ -576,6 +571,11 @@ config SENSORS_CORETEMP
 	  sensor inside your CPU. Most of the family 6 CPUs
 	  are supported. Check Documentation/hwmon/coretemp for details.
 
+config SENSORS_IMANAGER2
+	tristate "Support for Advantech iManager2 EC H.W. Monitor"
+	select MFD_CORE
+	depends on MFD_IMANAGER2
+
 config SENSORS_IT87
 	tristate "ITE IT87xx and compatibles"
 	depends on !PPC
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index a2c8f07..564b6fe 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -72,6 +72,7 @@ obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
 obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
 obj-$(CONFIG_SENSORS_IBMPEX)	+= ibmpex.o
 obj-$(CONFIG_SENSORS_IIO_HWMON) += iio_hwmon.o
+obj-$(CONFIG_SENSORS_IMANAGER2)	+= imanager2_hwm.o
 obj-$(CONFIG_SENSORS_INA209)	+= ina209.o
 obj-$(CONFIG_SENSORS_INA2XX)	+= ina2xx.o
 obj-$(CONFIG_SENSORS_IT87)	+= it87.o
@@ -146,7 +147,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_SENSORS_IMANAGER2)	+= imanager2_hwm.o
 
 obj-$(CONFIG_PMBUS)		+= pmbus/

> >  obj-$(CONFIG_PMBUS)		+= pmbus/
> >
> > diff --git a/drivers/hwmon/imanager2_hwm.c

[...]

> b/drivers/hwmon/imanager2_hwm.c
> > new file mode 100644
> > index 0000000..335bffb
> > --- /dev/null
> > +++ b/drivers/hwmon/imanager2_hwm.c
> > @@ -0,0 +1,768 @@
> > +/*
> > + * imanager2_hwm.c - HW Monitoring interface for Advantech EC
> IT8516/18/28
> > + * Copyright (C) 2014  Richard Vidal-Dorsch <richard.dorsch@advantech.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 Free Software Foundation, either version 3 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/hwmon-sysfs.h>
> > +#include <linux/mfd/imanager2_ec.h>
> > +
> > +#define DRV_NAME	"imanager2_hwm"
> > +#define DRV_VERSION	"4.0.1"
> > +
> > +/* ADC */
> > +#define EC_ADC_RESOLUTION_MAX	0x03FF	/* 10-bit */
> > +#define EC_ADC_VALUE_MAX	3000	/* max: 3000 mV or mA */
> > +
> > +/* Thermal */
> > +#define EC_THERMAL_ZONE_MAX	4
> > +
> > +enum thermaltype {
> > +	none,
> > +	sys1,
> > +	cpu,
> > +	sys3,
> > +	sys2,
> > +};
> > +
> > +struct ec_thermalzone {
> > +	u8 channel,
> > +	   addr,
> > +	   cmd,
> > +	   status,
> > +	   fancode,
> > +	   temp;
> > +};
> > +
> > +/* Tacho */
> > +#define EC_MAX_IO_FAN	3
> > +
> > +/* Voltage */
> > +struct volt_item {
> > +	u8 did;
> > +	const char *name;
> > +	int factor;
> > +	bool visible;
> > +};
> > +
> > +static struct volt_item ec_volt_table[] = {
> > +	{
> > +		.did = adcmosbat,
> > +		.name = "BAT CMOS",
> > +	},
> > +	{
> > +		.did = adcbat,
> > +		.name = "BAT",
> > +	},
> > +	{
> > +		.did = adc5vs0,
> > +		.name = "5V S0",
> > +	},
> > +	{
> > +		.did = adv5vs5,
> > +		.name = "5V S5",
> > +	},
> > +	{
> > +		.did = adc33vs0,
> > +		.name = "3V3 S0",
> > +	},
> > +	{
> > +		.did = adc33vs5,
> > +		.name = "3V3 S5",
> > +	},
> > +	{
> > +		.did = adv12vs0,
> > +		.name = "12V S0",
> > +	},
> > +	{
> > +		.did = adcvcorea,
> > +		.name = "Vcore A",
> > +	},
> > +	{
> > +		.did = adcvcoreb,
> > +		.name = "Vcore B",
> > +	},
> > +	{
> > +		.did = adcdc,
> > +		.name = "DC",
> > +	},
> > +	{
> > +		.did = adcdcstby,
> > +		.name = "DC Standby",
> > +	},
> > +	{
> > +		.did = adcdcother,
> > +		.name = "DC Other",
> > +	},
> > +};
> > +
> > +static int imanager2_volt_get_value_by_io(struct imanager2 *ec, int index,
> > +					  u8 *buf)
> > +{
> > +	u8 item = ec->table.devid2itemnum[ec_volt_table[index].did];
> > +	u8 pin = ec->table.pinnum[item];
> > +	u8 portnum;
> > +	int ret;
> > +
> > +	ret = imanager2_mbox_io_read(EC_CMD_ADC_INDEX, pin, &portnum, 1);
> > +	if (ret)
> > +		return ret;
> > +	if (portnum == EC_ERROR)
> > +		return -ENXIO;
> > +
> > +	ret = imanager2_mbox_io_simple_read(EC_CMD_ADC_READ_LSB, &buf[1]);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return imanager2_mbox_io_simple_read(EC_CMD_ADC_READ_MSB,
> &buf[0]);
> > +}
> > +
> > +static int imanager2_volt_get_value(struct imanager2 *ec, int index,
> > +				    u32 *volt_mvolt)
> > +{
> > +	int ret;
> > +	u8 tmp[2];
> > +
> > +	mutex_lock(&ec->lock);
> > +
> > +	if (ec->flag & EC_FLAG_MAILBOX)
> > +		ret = imanager2_mbox_read_data(ec->flag,
> > +					       EC_CMD_MAILBOX_READ_HW_PIN,
> > +					       ec_volt_table[index].did,
> > +					       tmp, 2);
> > +	else
> > +		ret = imanager2_volt_get_value_by_io(ec, index, tmp);
> > +
> > +	mutex_unlock(&ec->lock);
> > +
> > +	if (ret)
> > +		return ret;
> > +
> > +	*volt_mvolt = (((tmp[0] << 8) | tmp[1]) & EC_ADC_RESOLUTION_MAX) *
> > +		      ec_volt_table[index].factor *
> > +		      DIV_ROUND_CLOSEST(EC_ADC_VALUE_MAX,
> > +					EC_ADC_RESOLUTION_MAX);
> > +
> > +	return 0;
> > +}
> > +
> > +static void imanager2_volt_init(struct imanager2 *ec)
> > +{
> > +	u8 did;
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(ec_volt_table); i++) {
> > +		did = ec_volt_table[i].did;
> > +
> > +		if (ec->table.devid2itemnum[did] != EC_TABLE_ITEM_UNUSED) {
> > +			ec_volt_table[i].factor = 1;
> > +			ec_volt_table[i].visible = true;
> > +		} else if (ec->table.devid2itemnum[did + 1] !=
> > +			   EC_TABLE_ITEM_UNUSED) {
> > +			ec_volt_table[i].did += 1;
> > +			ec_volt_table[i].factor = 2;
> > +			ec_volt_table[i].visible = true;
> > +		} else if (ec->table.devid2itemnum[did + 2] !=
> > +			   EC_TABLE_ITEM_UNUSED) {
> > +			ec_volt_table[i].did += 2;
> > +			ec_volt_table[i].factor = 10;
> > +			ec_volt_table[i].visible = true;
> > +		} else {
> > +			ec_volt_table[i].visible = false;
> > +		}
> > +	}
> > +}
> > +
> > +static ssize_t show_in(struct device *dev, struct device_attribute *dev_attr,
> > +		       char *buf)
> > +{
> > +	struct imanager2 *ec = dev_get_drvdata(dev);
> > +	u32 val;
> > +	int ret = imanager2_volt_get_value(ec,
> > +					   to_sensor_dev_attr(dev_attr)->index,
> > +					   &val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return sprintf(buf, "%u\n", val);
> > +}
> > +
> > +static ssize_t show_in_label(struct device *dev,
> > +			     struct device_attribute *dev_attr, char *buf)
> > +{
> > +	return sprintf(buf, "%s\n",
> > +		       ec_volt_table[to_sensor_dev_attr(dev_attr)->index].name);
> > +}
> > +
> > +static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, show_in, NULL, 0);
> > +static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, show_in, NULL, 1);
> > +static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, show_in, NULL, 2);
> > +static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, show_in, NULL, 3);
> > +static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, show_in, NULL, 4);
> > +static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, show_in, NULL, 5);
> > +static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, show_in, NULL, 6);
> > +static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, show_in, NULL, 7);
> > +static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, show_in, NULL, 8);
> > +static SENSOR_DEVICE_ATTR(in9_input, S_IRUGO, show_in, NULL, 9);
> > +static SENSOR_DEVICE_ATTR(in10_input, S_IRUGO, show_in, NULL, 10);
> > +static SENSOR_DEVICE_ATTR(in11_input, S_IRUGO, show_in, NULL, 11);
> > +
> > +static SENSOR_DEVICE_ATTR(in0_label, S_IRUGO, show_in_label, NULL, 0);
> > +static SENSOR_DEVICE_ATTR(in1_label, S_IRUGO, show_in_label, NULL, 1);
> > +static SENSOR_DEVICE_ATTR(in2_label, S_IRUGO, show_in_label, NULL, 2);
> > +static SENSOR_DEVICE_ATTR(in3_label, S_IRUGO, show_in_label, NULL, 3);
> > +static SENSOR_DEVICE_ATTR(in4_label, S_IRUGO, show_in_label, NULL, 4);
> > +static SENSOR_DEVICE_ATTR(in5_label, S_IRUGO, show_in_label, NULL, 5);
> > +static SENSOR_DEVICE_ATTR(in6_label, S_IRUGO, show_in_label, NULL, 6);
> > +static SENSOR_DEVICE_ATTR(in7_label, S_IRUGO, show_in_label, NULL, 7);
> > +static SENSOR_DEVICE_ATTR(in8_label, S_IRUGO, show_in_label, NULL, 8);
> > +static SENSOR_DEVICE_ATTR(in9_label, S_IRUGO, show_in_label, NULL, 9);
> > +static SENSOR_DEVICE_ATTR(in10_label, S_IRUGO, show_in_label, NULL, 10);
> > +static SENSOR_DEVICE_ATTR(in11_label, S_IRUGO, show_in_label, NULL, 11);
> > +
> > +static struct attribute *imanager2_volt_attrs[] = {
> > +	&sensor_dev_attr_in0_label.dev_attr.attr,
> > +	&sensor_dev_attr_in0_input.dev_attr.attr,
> > +
> > +	&sensor_dev_attr_in1_label.dev_attr.attr,
> > +	&sensor_dev_attr_in1_input.dev_attr.attr,
> > +
> > +	&sensor_dev_attr_in2_label.dev_attr.attr,
> > +	&sensor_dev_attr_in2_input.dev_attr.attr,
> > +
> > +	&sensor_dev_attr_in3_label.dev_attr.attr,
> > +	&sensor_dev_attr_in3_input.dev_attr.attr,
> > +
> > +	&sensor_dev_attr_in4_label.dev_attr.attr,
> > +	&sensor_dev_attr_in4_input.dev_attr.attr,
> > +
> > +	&sensor_dev_attr_in5_label.dev_attr.attr,
> > +	&sensor_dev_attr_in5_input.dev_attr.attr,
> > +
> > +	&sensor_dev_attr_in6_label.dev_attr.attr,
> > +	&sensor_dev_attr_in6_input.dev_attr.attr,
> > +
> > +	&sensor_dev_attr_in7_label.dev_attr.attr,
> > +	&sensor_dev_attr_in7_input.dev_attr.attr,
> > +
> > +	&sensor_dev_attr_in8_label.dev_attr.attr,
> > +	&sensor_dev_attr_in8_input.dev_attr.attr,
> > +
> > +	&sensor_dev_attr_in9_label.dev_attr.attr,
> > +	&sensor_dev_attr_in9_input.dev_attr.attr,
> > +
> > +	&sensor_dev_attr_in10_label.dev_attr.attr,
> > +	&sensor_dev_attr_in10_input.dev_attr.attr,
> > +
> > +	&sensor_dev_attr_in11_label.dev_attr.attr,
> > +	&sensor_dev_attr_in11_input.dev_attr.attr,
> > +
> > +	NULL
> > +};
> > +
> > +static umode_t imanager2_volt_mode(struct kobject *kobj, struct attribute
> *attr,
> > +				   int index)
> > +{
> > +	struct device_attribute *dev_attr;
> > +
> > +	dev_attr = container_of(attr, struct device_attribute, attr);
> > +	if (!ec_volt_table[to_sensor_dev_attr(dev_attr)->index].visible)
> > +		return 0;
> > +
> > +	return attr->mode;
> > +}
> > +
> > +static const struct attribute_group imanager2_volt_group = {
> > +	.attrs = imanager2_volt_attrs,
> > +	.is_visible = imanager2_volt_mode,
> > +};
> > +
> > +/* Current */
> > +struct curr_item {
> > +	const u8 did;
> > +	const char *name;
> > +	bool visible;
> > +};
> > +
> > +static struct curr_item ec_curr_table[] = {
> > +	{
> > +		.did = adccurrent,
> > +		.name = "IMON"
> > +	},
> > +};
> > +
> > +static int imanager2_curr_get_value(struct imanager2 *ec, int index,
> > +				    u32 *curr_mamp)
> > +{
> > +	int ret;
> > +	u8 tmp[5];
> > +	u16 value, factor;
> > +	u32 baseunit = 1;
> > +
> > +	mutex_lock(&ec->lock);
> > +	ret = imanager2_mbox_read_data(ec->flag,
> EC_CMD_MAILBOX_READ_HW_PIN,
> > +				       ec_curr_table[index].did, tmp,
> > +				       ARRAY_SIZE(tmp));
> > +	mutex_unlock(&ec->lock);
> > +
> > +	if (ret)
> > +		return ret;
> > +
> > +	value = ((tmp[0] << 8) | tmp[1]) & EC_ADC_RESOLUTION_MAX;
> > +	factor = (tmp[2] << 8) | tmp[3];
> 
> Is it guaranteed that factor is never 0 ? If not, this may result
> in a division by 0 error.
> 

@@ -326,6 +318,11 @@ static int imanager2_curr_get_value(struct imanager2 *ec, int index,
 	value = ((tmp[0] << 8) | tmp[1]) & EC_ADC_RESOLUTION_MAX;
 	factor = (tmp[2] << 8) | tmp[3];
 
+	if (!factor) {
+		*curr_mamp = 0;
+		return 0;
+	}
+
 	while (tmp[4]++)
 		baseunit *= 10;

> > +
> > +	while (tmp[4]++)
> > +		baseunit *= 10;
> > +

[...]

> > +static void imanager2_temp_init(struct imanager2 *ec)
> > +{
> > +	int i, thm, ret;
> > +	u8 tmltype, smbid, fanid;
> > +	struct ec_thermalzone zone;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(ec_temp_table); i++)
> > +		ec_temp_table[i].visible = false;
> > +
> > +	for (thm = 0; thm < EC_THERMAL_ZONE_MAX; thm++) {
> > +		mutex_lock(&ec->lock);
> > +
> > +		if (ec->flag & EC_FLAG_MAILBOX) {
> > +			int len = sizeof(struct ec_thermalzone);
> 
> Checkpatch warning.

I use "./scripts/checkpatch.pl --no-tree --strict -f drivers/hwmon/imanager2_hwm"
but no warning shown out.
Can you help me to find out the warning?

> 
> > +			ret = imanager2_mbox_read_thermalzone(ec->flag, thm,
> > +							      &smbid, &fanid,
> > +							      (u8 *)&zone,
> > +							      &len);

[...]

> > +static struct fan_item ec_fan_table[] = {
> > +	{
> > +		.did = tacho0,
> > +		.name = "Fan CPU",
> > +		.fspeed_acpireg = 0,
> 
> It is not necessary to set such constants to 0.

@@ -566,20 +566,17 @@ static struct fan_item ec_fan_table[] = {
 	{
 		.did = tacho0,
 		.name = "Fan CPU",
-		.fspeed_acpireg = 0,
-		.visible = false
+		.visible = false,
 	},
 	{
 		.did = tacho1,
 		.name = "Fan SYS",
-		.fspeed_acpireg = 0,
-		.visible = false
+		.visible = false,
 	},
 	{
 		.did = tacho2,
 		.name = "Fan SYS2",
-		.fspeed_acpireg = 0,
-		.visible = false
+		.visible = false,
 	},
 };

> 
> > +		.visible = false
> > +	},
> > +	{
> > +		.did = tacho1,
> > +		.name = "Fan SYS",
> > +		.fspeed_acpireg = 0,
> > +		.visible = false
> > +	},
> > +	{
> > +		.did = tacho2,
> > +		.name = "Fan SYS2",
> > +		.fspeed_acpireg = 0,
> > +		.visible = false
> > +	},
> > +};
> > +
> > +static int imanager2_fan_get_value(struct imanager2 *ec, int index,
> > +				   u32 *speed_rpm)
> > +{
> > +	int ret;
> > +	u8 tmp[2];
> > +
> > +	mutex_lock(&ec->lock);
> > +
> > +	if (ec->flag & EC_FLAG_MAILBOX)
> > +		ret = imanager2_mbox_read_data(ec->flag,
> > +					       EC_CMD_MAILBOX_READ_HW_PIN,
> > +					       ec_fan_table[index].did, tmp, 2);
> > +	else
> > +		ret = imanager2_mbox_io_read(EC_CMD_ACPIRAM_READ,
> > +					     ec_fan_table[index].fspeed_acpireg,
> > +					     tmp, 2);
> > +
> > +	mutex_unlock(&ec->lock);
> > +
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (tmp[0] == 0xFF && tmp[1] == 0xFF)
> > +		return -ENODEV;
> 
> That is a bit late for detecting that there is no such device.
> 

@@ -605,11 +602,11 @@ static int imanager2_fan_get_value(struct imanager2 *ec, int index,
 	if (ret)
 		return ret;
 
-	if (tmp[0] == 0xFF && tmp[1] == 0xFF)
-		return -ENODEV;
-
 	*speed_rpm = (tmp[0] << 8) | tmp[1];
 
+	if (*speed_rpm == 0xFFFF)
+		*speed_rpm = 0;
+
 	return 0;
 }

> > +
> > +	*speed_rpm = (tmp[0] << 8) | tmp[1];
> > +
> > +	return 0;
> > +}
> > +
> > +#define EC_FANCTROL_SPEEDSRC_MASK	0x30
> > +
> > +static int imanager2_fan_item_init_by_io(struct imanager2 *ec, int fnum)
> > +{
> > +	int i, ret;
> > +	u8 tmp;
> > +
> > +	mutex_lock(&ec->lock);
> > +	ret = imanager2_mbox_io_read(EC_CMD_HWRAM_READ,
> > +				     EC_HWRAM_ADDR_FAN_CONTROL(fnum), &tmp, 1);
> > +	mutex_unlock(&ec->lock);
> > +
> > +	if (ret)
> > +		return ret;
> > +
> > +	i = ((tmp & EC_FANCTROL_SPEEDSRC_MASK) >> 4) - 1;
> > +	if (i < 0)
> > +		return -ENODEV;
> > +
> 
> This error return is quite pointless. See below.
> 

@@ -627,7 +624,7 @@ static int imanager2_fan_item_init_by_io(struct imanager2 *ec, int fnum)
 
 	i = ((tmp & EC_FANCTROL_SPEEDSRC_MASK) >> 4) - 1;
 	if (i < 0)
-		return -ENODEV;
+		return 0;
 
 	/* Unnecessary set again if it has been set. */
 	if (ec_fan_table[i].visible)

> > +	/* Unnecessary set again if it has been set. */
> > +	if (ec_fan_table[i].visible)
> > +		return 0;
> > +
> > +	ec_fan_table[i].fspeed_acpireg = EC_ACPIRAM_ADDR_FAN_SPEED_BASE(i);
> > +	ec_fan_table[i].visible = true;
> > +
> > +	return 0;
> > +}
> > +
> > +static void imanager2_fan_init(struct imanager2 *ec)
> > +{
> > +	int i;
> > +
> > +	if (ec->flag & EC_FLAG_MAILBOX) {
> > +		for (i = 0; i < ARRAY_SIZE(ec_fan_table); i++)
> > +			ec_fan_table[i].visible =
> > +			    ec->table.devid2itemnum[ec_fan_table[i].did] !=
> > +			    EC_TABLE_ITEM_UNUSED;
> > +	} else {
> > +		int fnum;
> > +
> > +		for (i = 0; i < ARRAY_SIZE(ec_fan_table); i++)
> > +			ec_fan_table[i].visible = false;
> > +
> > +		for (fnum = 0; fnum < EC_MAX_IO_FAN; fnum++) {
> > +			if (imanager2_fan_item_init_by_io(ec, fnum))
> > +				continue;
> 
> This continue is quite pointless.
> 

@@ -654,10 +651,8 @@ static void imanager2_fan_init(struct imanager2 *ec)
 		for (i = 0; i < ARRAY_SIZE(ec_fan_table); i++)
 			ec_fan_table[i].visible = false;
 
-		for (fnum = 0; fnum < EC_MAX_IO_FAN; fnum++) {
-			if (imanager2_fan_item_init_by_io(ec, fnum))
-				continue;
-		}
+		for (fnum = 0; fnum < EC_MAX_IO_FAN; fnum++)
+			imanager2_fan_item_init_by_io(ec, fnum);
 	}
 }

> > +		}
> > +	}
> > +}
> > +
> > +static ssize_t show_fan(struct device *dev, struct device_attribute *dev_attr,
> > +			char *buf)
> > +{
> > +	struct imanager2 *ec = dev_get_drvdata(dev);
> > +	u32 val;
> > +	int ret = imanager2_fan_get_value(ec,
> > +					  to_sensor_dev_attr(dev_attr)->index,
> > +					  &val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return sprintf(buf, "%u\n", val);
> > +}
> > +
> > +static ssize_t show_fan_label(struct device *dev,
> > +			      struct device_attribute *dev_attr, char *buf)
> > +{
> > +	return sprintf(buf, "%s\n",
> > +		       ec_fan_table[to_sensor_dev_attr(dev_attr)->index].name);
> > +}
> > +
> > +static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, show_fan, NULL, 0);
> > +static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, show_fan, NULL, 1);
> > +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, show_fan, NULL, 2);
> > +
> > +static SENSOR_DEVICE_ATTR(fan1_label, S_IRUGO, show_fan_label, NULL, 0);
> > +static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, show_fan_label, NULL, 1);
> > +static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, show_fan_label, NULL, 2);
> > +
> > +static struct attribute *imanager2_fan_attrs[] = {
> > +	&sensor_dev_attr_fan1_label.dev_attr.attr,
> > +	&sensor_dev_attr_fan1_input.dev_attr.attr,
> > +
> > +	&sensor_dev_attr_fan2_label.dev_attr.attr,
> > +	&sensor_dev_attr_fan2_input.dev_attr.attr,
> > +
> > +	&sensor_dev_attr_fan3_label.dev_attr.attr,
> > +	&sensor_dev_attr_fan3_input.dev_attr.attr,
> > +
> > +	NULL
> > +};
> > +
> > +static umode_t imanager2_fan_mode(struct kobject *kobj, struct attribute *attr,
> > +				  int index)
> > +{
> > +	struct device_attribute *dev_attr;
> > +
> > +	dev_attr = container_of(attr, struct device_attribute, attr);
> > +	if (!ec_fan_table[to_sensor_dev_attr(dev_attr)->index].visible)
> > +		return 0;
> > +
> > +	return attr->mode;
> > +}
> > +
> > +static const struct attribute_group imanager2_fan_group = {
> > +	.attrs = imanager2_fan_attrs,
> > +	.is_visible = imanager2_fan_mode,
> > +};
> > +
> > +/* Module */
> > +static const struct attribute_group *imanager2_hwmon_groups[] = {
> > +	&imanager2_volt_group,
> > +	&imanager2_curr_group,
> > +	&imanager2_temp_group,
> > +	&imanager2_fan_group,
> > +	NULL
> > +};
> > +
> > +static int imanager2_hwmon_probe(struct platform_device *pdev)
> > +{
> > +	struct device *hwmon_dev;
> > +	struct imanager2 *hwmon_ec = pdev->dev.parent->platform_data;
> > +
> > +	imanager2_volt_init(hwmon_ec);
> > +	imanager2_curr_init(hwmon_ec);
> > +	imanager2_temp_init(hwmon_ec);
> > +	imanager2_fan_init(hwmon_ec);
> > +
> > +	hwmon_dev = devm_hwmon_device_register_with_groups(
> > +			&pdev->dev, "imanager2", hwmon_ec,
> > +			imanager2_hwmon_groups);
> > +
> > +	if (IS_ERR(hwmon_dev))
> > +		return PTR_ERR(hwmon_dev);
> > +
> > +	return 0;
> 
> 	return PTR_ERR_OR_ZERO(hwmon_dev);
> 

@@ -743,10 +738,7 @@ static int imanager2_hwmon_probe(struct platform_device *pdev)
 			&pdev->dev, "imanager2", hwmon_ec,
 			imanager2_hwmon_groups);
 
-	if (IS_ERR(hwmon_dev))
-		return PTR_ERR(hwmon_dev);
-
-	return 0;
+	return PTR_ERR_OR_ZERO(hwmon_dev);
 }

> > +}
> > +
> > +static struct platform_driver imanager2_hwmon_driver = {
> > +	.probe = imanager2_hwmon_probe,
> > +	.driver = {
> > +			.owner = THIS_MODULE,
> > +			.name = DRV_NAME,
> > +	},
> > +};
> > +
> > +module_platform_driver(imanager2_hwmon_driver);
> > +
> > +MODULE_AUTHOR("Richard Vidal-Dorsch <richard.dorsch at
> advantech.com>");
> > +MODULE_DESCRIPTION("HW Monitoring interface for Advantech EC
> IT8516/18/28");
> > +MODULE_LICENSE("GPL");
> > +MODULE_VERSION(DRV_VERSION);
> > --
> > 1.9.1
> >
> >

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 4/4] hwmon: (imanager2) Add support for IT8516/18/28
  2014-08-07  9:24 [PATCH 4/4] hwmon: (imanager2) Add support for IT8516/18/28 Wei-Chun Pan
@ 2014-08-07 10:06 ` Lee Jones
  0 siblings, 0 replies; 4+ messages in thread
From: Lee Jones @ 2014-08-07 10:06 UTC (permalink / raw)
  To: Wei-Chun Pan
  Cc: Guenter Roeck, Samuel Ortiz, Jean Delvare, Wolfram Sang,
	Louis.Lu, Neo.Lo, Hank.Peng, Kevin.Ong, linux-kernel

On Thu, 07 Aug 2014, Wei-Chun Pan wrote:

> Sorry, I sent wrong patch just now. Please ignore the mail "[PATCH 1/3]
>  imanager2: rename io functions and remove no used functions".
> 
> This mail include 2 patches:
> The first patch is shown I rename some functions.
> The second patch is shown I moditfy code according to your comment.
> 
> Signed-off-by: Wei-Chun Pan <weichun.pan@advantech.com.tw>

Please use Git to send patches.  All of the above is in the wrong
order and is missing lots of context.

Please read:
  Documentation/SubmittingPatches
  Documentation/email-clients.txt

> ---
> diff --git a/drivers/hwmon/imanager2_hwm.c b/drivers/hwmon/imanager2_hwm.c
> index 335bffb..ab63296 100644
> --- a/drivers/hwmon/imanager2_hwm.c
> +++ b/drivers/hwmon/imanager2_hwm.c
> @@ -119,17 +119,17 @@ static int imanager2_volt_get_value_by_io(struct imanager2 *ec, int index,
>  	u8 portnum;
>  	int ret;
>  
> -	ret = imanager2_mbox_io_read(EC_CMD_ADC_INDEX, pin, &portnum, 1);
> +	ret = imanager2_io_read_data(EC_CMD_ADC_INDEX, pin, &portnum, 1);
>  	if (ret)
>  		return ret;
>  	if (portnum == EC_ERROR)
>  		return -ENXIO;
>  
> -	ret = imanager2_mbox_io_simple_read(EC_CMD_ADC_READ_LSB, &buf[1]);
> +	ret = imanager2_io_nooffset_readbyte(EC_CMD_ADC_READ_LSB, &buf[1]);
>  	if (ret)
>  		return ret;
>  
> -	return imanager2_mbox_io_simple_read(EC_CMD_ADC_READ_MSB, &buf[0]);
> +	return imanager2_io_nooffset_readbyte(EC_CMD_ADC_READ_MSB, &buf[0]);
>  }

[...]

> ---
> > > ---
> > >  drivers/hwmon/Kconfig         |   5 +
> > >  drivers/hwmon/Makefile        |   1 +
> > >  drivers/hwmon/imanager2_hwm.c | 768
> > ++++++++++++++++++++++++++++++++++++++++++
> > 
> > Documentation/hwmon/imanager2_hwm missing.

WTF?

I'm confused my this and even more so with the stuff below this.
What's the idea of this email?  Are you sending a patch or replying to
a code review?  You can't do both in one submission.

> diff --git a/Documentation/hwmon/imanager2_hwm b/Documentation/hwmon/imanager2_hwm
> new file mode 100644
> index 0000000..bf3d0b5
> --- /dev/null
> +++ b/Documentation/hwmon/imanager2_hwm
> @@ -0,0 +1,42 @@
> +Kernel driver imanager2_hwm
> +===========================
> +
> +Supported chips:
> +  * ITE IT8516
> +    Prefix: 'it8516'
> +    Addresses scanned: I/O chennel 0x029A/0x0299,
> +                       IET mailbox chennel 0x029E/0x029F
> +    Datasheet: Not publicly available
> +  * ITE IT8518
> +    Prefix: 'it8518'
> +    Addresses scanned: I/O chennel 0x029A/0x0299,
> +                       IET mailbox chennel 0x029E/0x029F
> +    Datasheet: Not publicly available
> +  * ITE IT8528
> +    Prefix: 'it8528'
> +    Addresses scanned: I/O chennel 0x029A/0x0299
> +    Datasheet: Not publicly available
> +
> +Authors:
> +        Richard Vidal-Dorsch <richard.dorsch@advantech.com>
> +
> +
> +Description
> +-----------
> +
> +This driver supports the hardware monitoring features of the IT8516, IT8518, and
> +IT8528 chips. These features include 11 voltage sensors, 1 current sensor, 4
> +temperature sensors, and 3 fan rotation speed sensors.
> +
> +ITE IT8516, IT8518, and IT8528 are are 'EC chips'. These chips are like Super I/O control boards. For IT8516 and IT 8518 The control chennel can be I/O or ITE mailbox chennel. I/O chennel is a common way but ITE mailbox chennel performance faster since it does not need to wait IBF (input buffer full) and OBF (output buffer full) to before send or get data.
> +
> +ITE IT8528 use an I/O chennel way to access mailbox, called I/O mailbox for cost down. Its performance the between pure I/O controller and ITE mailbox controller.
> +
> +
> +sysfs-Interface
> +---------------
> +
> +in[0-11]_input	- adc voltage input
> +curr1_input	- adc current input
> +temp[1-4]_input	- temperature input
> +fan[1-3]_input	- fan speed input
> 
> > 
> > >  3 files changed, 774 insertions(+)
> > >  create mode 100644 drivers/hwmon/imanager2_hwm.c
> > >
> > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > > index bc196f4..7524fc3 100644
> > > --- a/drivers/hwmon/Kconfig
> > > +++ b/drivers/hwmon/Kconfig
> > > @@ -39,6 +39,11 @@ config HWMON_DEBUG_CHIP
> > >
> > >  comment "Native drivers"
> > >
> > > +config SENSORS_IMANAGER2
> > > +	tristate "Support for Advantech iManager2 EC H.W. Monitor"
> > > +	select MFD_CORE
> > > +	depends on MFD_IMANAGER2
> > > +
> > Alphabetic order please.
> > 
> > >  config SENSORS_AB8500
> > >  	tristate "AB8500 thermal monitoring"
> > >  	depends on AB8500_GPADC && AB8500_BM
> > > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > > index c48f987..a2c8f07 100644
> > > --- a/drivers/hwmon/Makefile
> > > +++ b/drivers/hwmon/Makefile
> > > @@ -146,6 +146,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_SENSORS_IMANAGER2)	+= imanager2_hwm.o
> > >
> > Alphabetic order please.
> > 
>  drivers/hwmon/Kconfig  | 10 +++++-----
>  drivers/hwmon/Makefile |  2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 7524fc3..e39f8e0 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -39,11 +39,6 @@ config HWMON_DEBUG_CHIP
>  
>  comment "Native drivers"
>  
> -config SENSORS_IMANAGER2
> -	tristate "Support for Advantech iManager2 EC H.W. Monitor"
> -	select MFD_CORE
> -	depends on MFD_IMANAGER2
> -
>  config SENSORS_AB8500
>  	tristate "AB8500 thermal monitoring"
>  	depends on AB8500_GPADC && AB8500_BM
> @@ -576,6 +571,11 @@ config SENSORS_CORETEMP
>  	  sensor inside your CPU. Most of the family 6 CPUs
>  	  are supported. Check Documentation/hwmon/coretemp for details.
>  
> +config SENSORS_IMANAGER2
> +	tristate "Support for Advantech iManager2 EC H.W. Monitor"
> +	select MFD_CORE
> +	depends on MFD_IMANAGER2
> +
>  config SENSORS_IT87
>  	tristate "ITE IT87xx and compatibles"
>  	depends on !PPC
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index a2c8f07..564b6fe 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -72,6 +72,7 @@ obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
>  obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
>  obj-$(CONFIG_SENSORS_IBMPEX)	+= ibmpex.o
>  obj-$(CONFIG_SENSORS_IIO_HWMON) += iio_hwmon.o
> +obj-$(CONFIG_SENSORS_IMANAGER2)	+= imanager2_hwm.o
>  obj-$(CONFIG_SENSORS_INA209)	+= ina209.o
>  obj-$(CONFIG_SENSORS_INA2XX)	+= ina2xx.o
>  obj-$(CONFIG_SENSORS_IT87)	+= it87.o
> @@ -146,7 +147,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_SENSORS_IMANAGER2)	+= imanager2_hwm.o
>  
>  obj-$(CONFIG_PMBUS)		+= pmbus/
> 
> > >  obj-$(CONFIG_PMBUS)		+= pmbus/
> > >
> > > diff --git a/drivers/hwmon/imanager2_hwm.c
> 
> [...]
> 
> > b/drivers/hwmon/imanager2_hwm.c
> > > new file mode 100644
> > > index 0000000..335bffb
> > > --- /dev/null
> > > +++ b/drivers/hwmon/imanager2_hwm.c
> > > @@ -0,0 +1,768 @@
> > > +/*
> > > + * imanager2_hwm.c - HW Monitoring interface for Advantech EC
> > IT8516/18/28
> > > + * Copyright (C) 2014  Richard Vidal-Dorsch <richard.dorsch@advantech.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 Free Software Foundation, either version 3 of the License, or
> > > + * (at your option) any later version.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU General Public License
> > > + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> > > + */
> > > +
> > > +#include <linux/module.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/hwmon.h>
> > > +#include <linux/hwmon-sysfs.h>
> > > +#include <linux/mfd/imanager2_ec.h>
> > > +
> > > +#define DRV_NAME	"imanager2_hwm"
> > > +#define DRV_VERSION	"4.0.1"
> > > +
> > > +/* ADC */
> > > +#define EC_ADC_RESOLUTION_MAX	0x03FF	/* 10-bit */
> > > +#define EC_ADC_VALUE_MAX	3000	/* max: 3000 mV or mA */
> > > +
> > > +/* Thermal */
> > > +#define EC_THERMAL_ZONE_MAX	4
> > > +
> > > +enum thermaltype {
> > > +	none,
> > > +	sys1,
> > > +	cpu,
> > > +	sys3,
> > > +	sys2,
> > > +};
> > > +
> > > +struct ec_thermalzone {
> > > +	u8 channel,
> > > +	   addr,
> > > +	   cmd,
> > > +	   status,
> > > +	   fancode,
> > > +	   temp;
> > > +};
> > > +
> > > +/* Tacho */
> > > +#define EC_MAX_IO_FAN	3
> > > +
> > > +/* Voltage */
> > > +struct volt_item {
> > > +	u8 did;
> > > +	const char *name;
> > > +	int factor;
> > > +	bool visible;
> > > +};
> > > +
> > > +static struct volt_item ec_volt_table[] = {
> > > +	{
> > > +		.did = adcmosbat,
> > > +		.name = "BAT CMOS",
> > > +	},
> > > +	{
> > > +		.did = adcbat,
> > > +		.name = "BAT",
> > > +	},
> > > +	{
> > > +		.did = adc5vs0,
> > > +		.name = "5V S0",
> > > +	},
> > > +	{
> > > +		.did = adv5vs5,
> > > +		.name = "5V S5",
> > > +	},
> > > +	{
> > > +		.did = adc33vs0,
> > > +		.name = "3V3 S0",
> > > +	},
> > > +	{
> > > +		.did = adc33vs5,
> > > +		.name = "3V3 S5",
> > > +	},
> > > +	{
> > > +		.did = adv12vs0,
> > > +		.name = "12V S0",
> > > +	},
> > > +	{
> > > +		.did = adcvcorea,
> > > +		.name = "Vcore A",
> > > +	},
> > > +	{
> > > +		.did = adcvcoreb,
> > > +		.name = "Vcore B",
> > > +	},
> > > +	{
> > > +		.did = adcdc,
> > > +		.name = "DC",
> > > +	},
> > > +	{
> > > +		.did = adcdcstby,
> > > +		.name = "DC Standby",
> > > +	},
> > > +	{
> > > +		.did = adcdcother,
> > > +		.name = "DC Other",
> > > +	},
> > > +};
> > > +
> > > +static int imanager2_volt_get_value_by_io(struct imanager2 *ec, int index,
> > > +					  u8 *buf)
> > > +{
> > > +	u8 item = ec->table.devid2itemnum[ec_volt_table[index].did];
> > > +	u8 pin = ec->table.pinnum[item];
> > > +	u8 portnum;
> > > +	int ret;
> > > +
> > > +	ret = imanager2_mbox_io_read(EC_CMD_ADC_INDEX, pin, &portnum, 1);
> > > +	if (ret)
> > > +		return ret;
> > > +	if (portnum == EC_ERROR)
> > > +		return -ENXIO;
> > > +
> > > +	ret = imanager2_mbox_io_simple_read(EC_CMD_ADC_READ_LSB, &buf[1]);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	return imanager2_mbox_io_simple_read(EC_CMD_ADC_READ_MSB,
> > &buf[0]);
> > > +}
> > > +
> > > +static int imanager2_volt_get_value(struct imanager2 *ec, int index,
> > > +				    u32 *volt_mvolt)
> > > +{
> > > +	int ret;
> > > +	u8 tmp[2];
> > > +
> > > +	mutex_lock(&ec->lock);
> > > +
> > > +	if (ec->flag & EC_FLAG_MAILBOX)
> > > +		ret = imanager2_mbox_read_data(ec->flag,
> > > +					       EC_CMD_MAILBOX_READ_HW_PIN,
> > > +					       ec_volt_table[index].did,
> > > +					       tmp, 2);
> > > +	else
> > > +		ret = imanager2_volt_get_value_by_io(ec, index, tmp);
> > > +
> > > +	mutex_unlock(&ec->lock);
> > > +
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	*volt_mvolt = (((tmp[0] << 8) | tmp[1]) & EC_ADC_RESOLUTION_MAX) *
> > > +		      ec_volt_table[index].factor *
> > > +		      DIV_ROUND_CLOSEST(EC_ADC_VALUE_MAX,
> > > +					EC_ADC_RESOLUTION_MAX);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static void imanager2_volt_init(struct imanager2 *ec)
> > > +{
> > > +	u8 did;
> > > +	int i;
> > > +
> > > +	for (i = 0; i < ARRAY_SIZE(ec_volt_table); i++) {
> > > +		did = ec_volt_table[i].did;
> > > +
> > > +		if (ec->table.devid2itemnum[did] != EC_TABLE_ITEM_UNUSED) {
> > > +			ec_volt_table[i].factor = 1;
> > > +			ec_volt_table[i].visible = true;
> > > +		} else if (ec->table.devid2itemnum[did + 1] !=
> > > +			   EC_TABLE_ITEM_UNUSED) {
> > > +			ec_volt_table[i].did += 1;
> > > +			ec_volt_table[i].factor = 2;
> > > +			ec_volt_table[i].visible = true;
> > > +		} else if (ec->table.devid2itemnum[did + 2] !=
> > > +			   EC_TABLE_ITEM_UNUSED) {
> > > +			ec_volt_table[i].did += 2;
> > > +			ec_volt_table[i].factor = 10;
> > > +			ec_volt_table[i].visible = true;
> > > +		} else {
> > > +			ec_volt_table[i].visible = false;
> > > +		}
> > > +	}
> > > +}
> > > +
> > > +static ssize_t show_in(struct device *dev, struct device_attribute *dev_attr,
> > > +		       char *buf)
> > > +{
> > > +	struct imanager2 *ec = dev_get_drvdata(dev);
> > > +	u32 val;
> > > +	int ret = imanager2_volt_get_value(ec,
> > > +					   to_sensor_dev_attr(dev_attr)->index,
> > > +					   &val);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	return sprintf(buf, "%u\n", val);
> > > +}
> > > +
> > > +static ssize_t show_in_label(struct device *dev,
> > > +			     struct device_attribute *dev_attr, char *buf)
> > > +{
> > > +	return sprintf(buf, "%s\n",
> > > +		       ec_volt_table[to_sensor_dev_attr(dev_attr)->index].name);
> > > +}
> > > +
> > > +static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, show_in, NULL, 0);
> > > +static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, show_in, NULL, 1);
> > > +static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, show_in, NULL, 2);
> > > +static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, show_in, NULL, 3);
> > > +static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, show_in, NULL, 4);
> > > +static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, show_in, NULL, 5);
> > > +static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, show_in, NULL, 6);
> > > +static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, show_in, NULL, 7);
> > > +static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, show_in, NULL, 8);
> > > +static SENSOR_DEVICE_ATTR(in9_input, S_IRUGO, show_in, NULL, 9);
> > > +static SENSOR_DEVICE_ATTR(in10_input, S_IRUGO, show_in, NULL, 10);
> > > +static SENSOR_DEVICE_ATTR(in11_input, S_IRUGO, show_in, NULL, 11);
> > > +
> > > +static SENSOR_DEVICE_ATTR(in0_label, S_IRUGO, show_in_label, NULL, 0);
> > > +static SENSOR_DEVICE_ATTR(in1_label, S_IRUGO, show_in_label, NULL, 1);
> > > +static SENSOR_DEVICE_ATTR(in2_label, S_IRUGO, show_in_label, NULL, 2);
> > > +static SENSOR_DEVICE_ATTR(in3_label, S_IRUGO, show_in_label, NULL, 3);
> > > +static SENSOR_DEVICE_ATTR(in4_label, S_IRUGO, show_in_label, NULL, 4);
> > > +static SENSOR_DEVICE_ATTR(in5_label, S_IRUGO, show_in_label, NULL, 5);
> > > +static SENSOR_DEVICE_ATTR(in6_label, S_IRUGO, show_in_label, NULL, 6);
> > > +static SENSOR_DEVICE_ATTR(in7_label, S_IRUGO, show_in_label, NULL, 7);
> > > +static SENSOR_DEVICE_ATTR(in8_label, S_IRUGO, show_in_label, NULL, 8);
> > > +static SENSOR_DEVICE_ATTR(in9_label, S_IRUGO, show_in_label, NULL, 9);
> > > +static SENSOR_DEVICE_ATTR(in10_label, S_IRUGO, show_in_label, NULL, 10);
> > > +static SENSOR_DEVICE_ATTR(in11_label, S_IRUGO, show_in_label, NULL, 11);
> > > +
> > > +static struct attribute *imanager2_volt_attrs[] = {
> > > +	&sensor_dev_attr_in0_label.dev_attr.attr,
> > > +	&sensor_dev_attr_in0_input.dev_attr.attr,
> > > +
> > > +	&sensor_dev_attr_in1_label.dev_attr.attr,
> > > +	&sensor_dev_attr_in1_input.dev_attr.attr,
> > > +
> > > +	&sensor_dev_attr_in2_label.dev_attr.attr,
> > > +	&sensor_dev_attr_in2_input.dev_attr.attr,
> > > +
> > > +	&sensor_dev_attr_in3_label.dev_attr.attr,
> > > +	&sensor_dev_attr_in3_input.dev_attr.attr,
> > > +
> > > +	&sensor_dev_attr_in4_label.dev_attr.attr,
> > > +	&sensor_dev_attr_in4_input.dev_attr.attr,
> > > +
> > > +	&sensor_dev_attr_in5_label.dev_attr.attr,
> > > +	&sensor_dev_attr_in5_input.dev_attr.attr,
> > > +
> > > +	&sensor_dev_attr_in6_label.dev_attr.attr,
> > > +	&sensor_dev_attr_in6_input.dev_attr.attr,
> > > +
> > > +	&sensor_dev_attr_in7_label.dev_attr.attr,
> > > +	&sensor_dev_attr_in7_input.dev_attr.attr,
> > > +
> > > +	&sensor_dev_attr_in8_label.dev_attr.attr,
> > > +	&sensor_dev_attr_in8_input.dev_attr.attr,
> > > +
> > > +	&sensor_dev_attr_in9_label.dev_attr.attr,
> > > +	&sensor_dev_attr_in9_input.dev_attr.attr,
> > > +
> > > +	&sensor_dev_attr_in10_label.dev_attr.attr,
> > > +	&sensor_dev_attr_in10_input.dev_attr.attr,
> > > +
> > > +	&sensor_dev_attr_in11_label.dev_attr.attr,
> > > +	&sensor_dev_attr_in11_input.dev_attr.attr,
> > > +
> > > +	NULL
> > > +};
> > > +
> > > +static umode_t imanager2_volt_mode(struct kobject *kobj, struct attribute
> > *attr,
> > > +				   int index)
> > > +{
> > > +	struct device_attribute *dev_attr;
> > > +
> > > +	dev_attr = container_of(attr, struct device_attribute, attr);
> > > +	if (!ec_volt_table[to_sensor_dev_attr(dev_attr)->index].visible)
> > > +		return 0;
> > > +
> > > +	return attr->mode;
> > > +}
> > > +
> > > +static const struct attribute_group imanager2_volt_group = {
> > > +	.attrs = imanager2_volt_attrs,
> > > +	.is_visible = imanager2_volt_mode,
> > > +};
> > > +
> > > +/* Current */
> > > +struct curr_item {
> > > +	const u8 did;
> > > +	const char *name;
> > > +	bool visible;
> > > +};
> > > +
> > > +static struct curr_item ec_curr_table[] = {
> > > +	{
> > > +		.did = adccurrent,
> > > +		.name = "IMON"
> > > +	},
> > > +};
> > > +
> > > +static int imanager2_curr_get_value(struct imanager2 *ec, int index,
> > > +				    u32 *curr_mamp)
> > > +{
> > > +	int ret;
> > > +	u8 tmp[5];
> > > +	u16 value, factor;
> > > +	u32 baseunit = 1;
> > > +
> > > +	mutex_lock(&ec->lock);
> > > +	ret = imanager2_mbox_read_data(ec->flag,
> > EC_CMD_MAILBOX_READ_HW_PIN,
> > > +				       ec_curr_table[index].did, tmp,
> > > +				       ARRAY_SIZE(tmp));
> > > +	mutex_unlock(&ec->lock);
> > > +
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	value = ((tmp[0] << 8) | tmp[1]) & EC_ADC_RESOLUTION_MAX;
> > > +	factor = (tmp[2] << 8) | tmp[3];
> > 
> > Is it guaranteed that factor is never 0 ? If not, this may result
> > in a division by 0 error.
> > 
> 
> @@ -326,6 +318,11 @@ static int imanager2_curr_get_value(struct imanager2 *ec, int index,
>  	value = ((tmp[0] << 8) | tmp[1]) & EC_ADC_RESOLUTION_MAX;
>  	factor = (tmp[2] << 8) | tmp[3];
>  
> +	if (!factor) {
> +		*curr_mamp = 0;
> +		return 0;
> +	}
> +
>  	while (tmp[4]++)
>  		baseunit *= 10;
> 
> > > +
> > > +	while (tmp[4]++)
> > > +		baseunit *= 10;
> > > +
> 
> [...]
> 
> > > +static void imanager2_temp_init(struct imanager2 *ec)
> > > +{
> > > +	int i, thm, ret;
> > > +	u8 tmltype, smbid, fanid;
> > > +	struct ec_thermalzone zone;
> > > +
> > > +	for (i = 0; i < ARRAY_SIZE(ec_temp_table); i++)
> > > +		ec_temp_table[i].visible = false;
> > > +
> > > +	for (thm = 0; thm < EC_THERMAL_ZONE_MAX; thm++) {
> > > +		mutex_lock(&ec->lock);
> > > +
> > > +		if (ec->flag & EC_FLAG_MAILBOX) {
> > > +			int len = sizeof(struct ec_thermalzone);
> > 
> > Checkpatch warning.
> 
> I use "./scripts/checkpatch.pl --no-tree --strict -f drivers/hwmon/imanager2_hwm"
> but no warning shown out.
> Can you help me to find out the warning?
> 
> > 
> > > +			ret = imanager2_mbox_read_thermalzone(ec->flag, thm,
> > > +							      &smbid, &fanid,
> > > +							      (u8 *)&zone,
> > > +							      &len);
> 
> [...]
> 
> > > +static struct fan_item ec_fan_table[] = {
> > > +	{
> > > +		.did = tacho0,
> > > +		.name = "Fan CPU",
> > > +		.fspeed_acpireg = 0,
> > 
> > It is not necessary to set such constants to 0.
> 
> @@ -566,20 +566,17 @@ static struct fan_item ec_fan_table[] = {
>  	{
>  		.did = tacho0,
>  		.name = "Fan CPU",
> -		.fspeed_acpireg = 0,
> -		.visible = false
> +		.visible = false,
>  	},
>  	{
>  		.did = tacho1,
>  		.name = "Fan SYS",
> -		.fspeed_acpireg = 0,
> -		.visible = false
> +		.visible = false,
>  	},
>  	{
>  		.did = tacho2,
>  		.name = "Fan SYS2",
> -		.fspeed_acpireg = 0,
> -		.visible = false
> +		.visible = false,
>  	},
>  };
> 
> > 
> > > +		.visible = false
> > > +	},
> > > +	{
> > > +		.did = tacho1,
> > > +		.name = "Fan SYS",
> > > +		.fspeed_acpireg = 0,
> > > +		.visible = false
> > > +	},
> > > +	{
> > > +		.did = tacho2,
> > > +		.name = "Fan SYS2",
> > > +		.fspeed_acpireg = 0,
> > > +		.visible = false
> > > +	},
> > > +};
> > > +
> > > +static int imanager2_fan_get_value(struct imanager2 *ec, int index,
> > > +				   u32 *speed_rpm)
> > > +{
> > > +	int ret;
> > > +	u8 tmp[2];
> > > +
> > > +	mutex_lock(&ec->lock);
> > > +
> > > +	if (ec->flag & EC_FLAG_MAILBOX)
> > > +		ret = imanager2_mbox_read_data(ec->flag,
> > > +					       EC_CMD_MAILBOX_READ_HW_PIN,
> > > +					       ec_fan_table[index].did, tmp, 2);
> > > +	else
> > > +		ret = imanager2_mbox_io_read(EC_CMD_ACPIRAM_READ,
> > > +					     ec_fan_table[index].fspeed_acpireg,
> > > +					     tmp, 2);
> > > +
> > > +	mutex_unlock(&ec->lock);
> > > +
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	if (tmp[0] == 0xFF && tmp[1] == 0xFF)
> > > +		return -ENODEV;
> > 
> > That is a bit late for detecting that there is no such device.
> > 
> 
> @@ -605,11 +602,11 @@ static int imanager2_fan_get_value(struct imanager2 *ec, int index,
>  	if (ret)
>  		return ret;
>  
> -	if (tmp[0] == 0xFF && tmp[1] == 0xFF)
> -		return -ENODEV;
> -
>  	*speed_rpm = (tmp[0] << 8) | tmp[1];
>  
> +	if (*speed_rpm == 0xFFFF)
> +		*speed_rpm = 0;
> +
>  	return 0;
>  }
> 
> > > +
> > > +	*speed_rpm = (tmp[0] << 8) | tmp[1];
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +#define EC_FANCTROL_SPEEDSRC_MASK	0x30
> > > +
> > > +static int imanager2_fan_item_init_by_io(struct imanager2 *ec, int fnum)
> > > +{
> > > +	int i, ret;
> > > +	u8 tmp;
> > > +
> > > +	mutex_lock(&ec->lock);
> > > +	ret = imanager2_mbox_io_read(EC_CMD_HWRAM_READ,
> > > +				     EC_HWRAM_ADDR_FAN_CONTROL(fnum), &tmp, 1);
> > > +	mutex_unlock(&ec->lock);
> > > +
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	i = ((tmp & EC_FANCTROL_SPEEDSRC_MASK) >> 4) - 1;
> > > +	if (i < 0)
> > > +		return -ENODEV;
> > > +
> > 
> > This error return is quite pointless. See below.
> > 
> 
> @@ -627,7 +624,7 @@ static int imanager2_fan_item_init_by_io(struct imanager2 *ec, int fnum)
>  
>  	i = ((tmp & EC_FANCTROL_SPEEDSRC_MASK) >> 4) - 1;
>  	if (i < 0)
> -		return -ENODEV;
> +		return 0;
>  
>  	/* Unnecessary set again if it has been set. */
>  	if (ec_fan_table[i].visible)
> 
> > > +	/* Unnecessary set again if it has been set. */
> > > +	if (ec_fan_table[i].visible)
> > > +		return 0;
> > > +
> > > +	ec_fan_table[i].fspeed_acpireg = EC_ACPIRAM_ADDR_FAN_SPEED_BASE(i);
> > > +	ec_fan_table[i].visible = true;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static void imanager2_fan_init(struct imanager2 *ec)
> > > +{
> > > +	int i;
> > > +
> > > +	if (ec->flag & EC_FLAG_MAILBOX) {
> > > +		for (i = 0; i < ARRAY_SIZE(ec_fan_table); i++)
> > > +			ec_fan_table[i].visible =
> > > +			    ec->table.devid2itemnum[ec_fan_table[i].did] !=
> > > +			    EC_TABLE_ITEM_UNUSED;
> > > +	} else {
> > > +		int fnum;
> > > +
> > > +		for (i = 0; i < ARRAY_SIZE(ec_fan_table); i++)
> > > +			ec_fan_table[i].visible = false;
> > > +
> > > +		for (fnum = 0; fnum < EC_MAX_IO_FAN; fnum++) {
> > > +			if (imanager2_fan_item_init_by_io(ec, fnum))
> > > +				continue;
> > 
> > This continue is quite pointless.
> > 
> 
> @@ -654,10 +651,8 @@ static void imanager2_fan_init(struct imanager2 *ec)
>  		for (i = 0; i < ARRAY_SIZE(ec_fan_table); i++)
>  			ec_fan_table[i].visible = false;
>  
> -		for (fnum = 0; fnum < EC_MAX_IO_FAN; fnum++) {
> -			if (imanager2_fan_item_init_by_io(ec, fnum))
> -				continue;
> -		}
> +		for (fnum = 0; fnum < EC_MAX_IO_FAN; fnum++)
> +			imanager2_fan_item_init_by_io(ec, fnum);
>  	}
>  }
> 
> > > +		}
> > > +	}
> > > +}
> > > +
> > > +static ssize_t show_fan(struct device *dev, struct device_attribute *dev_attr,
> > > +			char *buf)
> > > +{
> > > +	struct imanager2 *ec = dev_get_drvdata(dev);
> > > +	u32 val;
> > > +	int ret = imanager2_fan_get_value(ec,
> > > +					  to_sensor_dev_attr(dev_attr)->index,
> > > +					  &val);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	return sprintf(buf, "%u\n", val);
> > > +}
> > > +
> > > +static ssize_t show_fan_label(struct device *dev,
> > > +			      struct device_attribute *dev_attr, char *buf)
> > > +{
> > > +	return sprintf(buf, "%s\n",
> > > +		       ec_fan_table[to_sensor_dev_attr(dev_attr)->index].name);
> > > +}
> > > +
> > > +static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, show_fan, NULL, 0);
> > > +static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, show_fan, NULL, 1);
> > > +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, show_fan, NULL, 2);
> > > +
> > > +static SENSOR_DEVICE_ATTR(fan1_label, S_IRUGO, show_fan_label, NULL, 0);
> > > +static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, show_fan_label, NULL, 1);
> > > +static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, show_fan_label, NULL, 2);
> > > +
> > > +static struct attribute *imanager2_fan_attrs[] = {
> > > +	&sensor_dev_attr_fan1_label.dev_attr.attr,
> > > +	&sensor_dev_attr_fan1_input.dev_attr.attr,
> > > +
> > > +	&sensor_dev_attr_fan2_label.dev_attr.attr,
> > > +	&sensor_dev_attr_fan2_input.dev_attr.attr,
> > > +
> > > +	&sensor_dev_attr_fan3_label.dev_attr.attr,
> > > +	&sensor_dev_attr_fan3_input.dev_attr.attr,
> > > +
> > > +	NULL
> > > +};
> > > +
> > > +static umode_t imanager2_fan_mode(struct kobject *kobj, struct attribute *attr,
> > > +				  int index)
> > > +{
> > > +	struct device_attribute *dev_attr;
> > > +
> > > +	dev_attr = container_of(attr, struct device_attribute, attr);
> > > +	if (!ec_fan_table[to_sensor_dev_attr(dev_attr)->index].visible)
> > > +		return 0;
> > > +
> > > +	return attr->mode;
> > > +}
> > > +
> > > +static const struct attribute_group imanager2_fan_group = {
> > > +	.attrs = imanager2_fan_attrs,
> > > +	.is_visible = imanager2_fan_mode,
> > > +};
> > > +
> > > +/* Module */
> > > +static const struct attribute_group *imanager2_hwmon_groups[] = {
> > > +	&imanager2_volt_group,
> > > +	&imanager2_curr_group,
> > > +	&imanager2_temp_group,
> > > +	&imanager2_fan_group,
> > > +	NULL
> > > +};
> > > +
> > > +static int imanager2_hwmon_probe(struct platform_device *pdev)
> > > +{
> > > +	struct device *hwmon_dev;
> > > +	struct imanager2 *hwmon_ec = pdev->dev.parent->platform_data;
> > > +
> > > +	imanager2_volt_init(hwmon_ec);
> > > +	imanager2_curr_init(hwmon_ec);
> > > +	imanager2_temp_init(hwmon_ec);
> > > +	imanager2_fan_init(hwmon_ec);
> > > +
> > > +	hwmon_dev = devm_hwmon_device_register_with_groups(
> > > +			&pdev->dev, "imanager2", hwmon_ec,
> > > +			imanager2_hwmon_groups);
> > > +
> > > +	if (IS_ERR(hwmon_dev))
> > > +		return PTR_ERR(hwmon_dev);
> > > +
> > > +	return 0;
> > 
> > 	return PTR_ERR_OR_ZERO(hwmon_dev);
> > 
> 
> @@ -743,10 +738,7 @@ static int imanager2_hwmon_probe(struct platform_device *pdev)
>  			&pdev->dev, "imanager2", hwmon_ec,
>  			imanager2_hwmon_groups);
>  
> -	if (IS_ERR(hwmon_dev))
> -		return PTR_ERR(hwmon_dev);
> -
> -	return 0;
> +	return PTR_ERR_OR_ZERO(hwmon_dev);
>  }
> 
> > > +}
> > > +
> > > +static struct platform_driver imanager2_hwmon_driver = {
> > > +	.probe = imanager2_hwmon_probe,
> > > +	.driver = {
> > > +			.owner = THIS_MODULE,
> > > +			.name = DRV_NAME,
> > > +	},
> > > +};
> > > +
> > > +module_platform_driver(imanager2_hwmon_driver);
> > > +
> > > +MODULE_AUTHOR("Richard Vidal-Dorsch <richard.dorsch at
> > advantech.com>");
> > > +MODULE_DESCRIPTION("HW Monitoring interface for Advantech EC
> > IT8516/18/28");
> > > +MODULE_LICENSE("GPL");
> > > +MODULE_VERSION(DRV_VERSION);
> > >
> > >

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 4/4] hwmon: (imanager2) Add support for IT8516/18/28
  2014-07-14 12:54 ` [PATCH 4/4] hwmon: (imanager2) Add " Wei-Chun Pan
@ 2014-07-14 19:05   ` Guenter Roeck
  0 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2014-07-14 19:05 UTC (permalink / raw)
  To: Wei-Chun Pan
  Cc: Samuel Ortiz, Lee Jones, Jean Delvare, Louis.Lu, Neo.Lo,
	Hank.Peng, Kevin.Ong, linux-kernel

On Mon, Jul 14, 2014 at 05:54:46AM -0700, Wei-Chun Pan wrote:
> Signed-off-by: Wei-Chun Pan <weichun.pan@advantech.com.tw>

No explanation, no patch version, and no change log, meaning we'll
have to go back to your previous submissions to verify what you changed
and why, and if you addressed all comments. This will take a while.

Other comments below.

Guenter

> ---
>  drivers/hwmon/Kconfig         |   5 +
>  drivers/hwmon/Makefile        |   1 +
>  drivers/hwmon/imanager2_hwm.c | 768 ++++++++++++++++++++++++++++++++++++++++++

Documentation/hwmon/imanager2_hwm missing.

>  3 files changed, 774 insertions(+)
>  create mode 100644 drivers/hwmon/imanager2_hwm.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index bc196f4..7524fc3 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -39,6 +39,11 @@ config HWMON_DEBUG_CHIP
>  
>  comment "Native drivers"
>  
> +config SENSORS_IMANAGER2
> +	tristate "Support for Advantech iManager2 EC H.W. Monitor"
> +	select MFD_CORE
> +	depends on MFD_IMANAGER2
> +
Alphabetic order please.

>  config SENSORS_AB8500
>  	tristate "AB8500 thermal monitoring"
>  	depends on AB8500_GPADC && AB8500_BM
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index c48f987..a2c8f07 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -146,6 +146,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_SENSORS_IMANAGER2)	+= imanager2_hwm.o
>  
Alphabetic order please.

>  obj-$(CONFIG_PMBUS)		+= pmbus/
>  
> diff --git a/drivers/hwmon/imanager2_hwm.c b/drivers/hwmon/imanager2_hwm.c
> new file mode 100644
> index 0000000..335bffb
> --- /dev/null
> +++ b/drivers/hwmon/imanager2_hwm.c
> @@ -0,0 +1,768 @@
> +/*
> + * imanager2_hwm.c - HW Monitoring interface for Advantech EC IT8516/18/28
> + * Copyright (C) 2014  Richard Vidal-Dorsch <richard.dorsch@advantech.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 Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/mfd/imanager2_ec.h>
> +
> +#define DRV_NAME	"imanager2_hwm"
> +#define DRV_VERSION	"4.0.1"
> +
> +/* ADC */
> +#define EC_ADC_RESOLUTION_MAX	0x03FF	/* 10-bit */
> +#define EC_ADC_VALUE_MAX	3000	/* max: 3000 mV or mA */
> +
> +/* Thermal */
> +#define EC_THERMAL_ZONE_MAX	4
> +
> +enum thermaltype {
> +	none,
> +	sys1,
> +	cpu,
> +	sys3,
> +	sys2,
> +};
> +
> +struct ec_thermalzone {
> +	u8 channel,
> +	   addr,
> +	   cmd,
> +	   status,
> +	   fancode,
> +	   temp;
> +};
> +
> +/* Tacho */
> +#define EC_MAX_IO_FAN	3
> +
> +/* Voltage */
> +struct volt_item {
> +	u8 did;
> +	const char *name;
> +	int factor;
> +	bool visible;
> +};
> +
> +static struct volt_item ec_volt_table[] = {
> +	{
> +		.did = adcmosbat,
> +		.name = "BAT CMOS",
> +	},
> +	{
> +		.did = adcbat,
> +		.name = "BAT",
> +	},
> +	{
> +		.did = adc5vs0,
> +		.name = "5V S0",
> +	},
> +	{
> +		.did = adv5vs5,
> +		.name = "5V S5",
> +	},
> +	{
> +		.did = adc33vs0,
> +		.name = "3V3 S0",
> +	},
> +	{
> +		.did = adc33vs5,
> +		.name = "3V3 S5",
> +	},
> +	{
> +		.did = adv12vs0,
> +		.name = "12V S0",
> +	},
> +	{
> +		.did = adcvcorea,
> +		.name = "Vcore A",
> +	},
> +	{
> +		.did = adcvcoreb,
> +		.name = "Vcore B",
> +	},
> +	{
> +		.did = adcdc,
> +		.name = "DC",
> +	},
> +	{
> +		.did = adcdcstby,
> +		.name = "DC Standby",
> +	},
> +	{
> +		.did = adcdcother,
> +		.name = "DC Other",
> +	},
> +};
> +
> +static int imanager2_volt_get_value_by_io(struct imanager2 *ec, int index,
> +					  u8 *buf)
> +{
> +	u8 item = ec->table.devid2itemnum[ec_volt_table[index].did];
> +	u8 pin = ec->table.pinnum[item];
> +	u8 portnum;
> +	int ret;
> +
> +	ret = imanager2_mbox_io_read(EC_CMD_ADC_INDEX, pin, &portnum, 1);
> +	if (ret)
> +		return ret;
> +	if (portnum == EC_ERROR)
> +		return -ENXIO;
> +
> +	ret = imanager2_mbox_io_simple_read(EC_CMD_ADC_READ_LSB, &buf[1]);
> +	if (ret)
> +		return ret;
> +
> +	return imanager2_mbox_io_simple_read(EC_CMD_ADC_READ_MSB, &buf[0]);
> +}
> +
> +static int imanager2_volt_get_value(struct imanager2 *ec, int index,
> +				    u32 *volt_mvolt)
> +{
> +	int ret;
> +	u8 tmp[2];
> +
> +	mutex_lock(&ec->lock);
> +
> +	if (ec->flag & EC_FLAG_MAILBOX)
> +		ret = imanager2_mbox_read_data(ec->flag,
> +					       EC_CMD_MAILBOX_READ_HW_PIN,
> +					       ec_volt_table[index].did,
> +					       tmp, 2);
> +	else
> +		ret = imanager2_volt_get_value_by_io(ec, index, tmp);
> +
> +	mutex_unlock(&ec->lock);
> +
> +	if (ret)
> +		return ret;
> +
> +	*volt_mvolt = (((tmp[0] << 8) | tmp[1]) & EC_ADC_RESOLUTION_MAX) *
> +		      ec_volt_table[index].factor *
> +		      DIV_ROUND_CLOSEST(EC_ADC_VALUE_MAX,
> +					EC_ADC_RESOLUTION_MAX);
> +
> +	return 0;
> +}
> +
> +static void imanager2_volt_init(struct imanager2 *ec)
> +{
> +	u8 did;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ec_volt_table); i++) {
> +		did = ec_volt_table[i].did;
> +
> +		if (ec->table.devid2itemnum[did] != EC_TABLE_ITEM_UNUSED) {
> +			ec_volt_table[i].factor = 1;
> +			ec_volt_table[i].visible = true;
> +		} else if (ec->table.devid2itemnum[did + 1] !=
> +			   EC_TABLE_ITEM_UNUSED) {
> +			ec_volt_table[i].did += 1;
> +			ec_volt_table[i].factor = 2;
> +			ec_volt_table[i].visible = true;
> +		} else if (ec->table.devid2itemnum[did + 2] !=
> +			   EC_TABLE_ITEM_UNUSED) {
> +			ec_volt_table[i].did += 2;
> +			ec_volt_table[i].factor = 10;
> +			ec_volt_table[i].visible = true;
> +		} else {
> +			ec_volt_table[i].visible = false;
> +		}
> +	}
> +}
> +
> +static ssize_t show_in(struct device *dev, struct device_attribute *dev_attr,
> +		       char *buf)
> +{
> +	struct imanager2 *ec = dev_get_drvdata(dev);
> +	u32 val;
> +	int ret = imanager2_volt_get_value(ec,
> +					   to_sensor_dev_attr(dev_attr)->index,
> +					   &val);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%u\n", val);
> +}
> +
> +static ssize_t show_in_label(struct device *dev,
> +			     struct device_attribute *dev_attr, char *buf)
> +{
> +	return sprintf(buf, "%s\n",
> +		       ec_volt_table[to_sensor_dev_attr(dev_attr)->index].name);
> +}
> +
> +static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, show_in, NULL, 0);
> +static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, show_in, NULL, 1);
> +static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, show_in, NULL, 2);
> +static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, show_in, NULL, 3);
> +static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, show_in, NULL, 4);
> +static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, show_in, NULL, 5);
> +static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, show_in, NULL, 6);
> +static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, show_in, NULL, 7);
> +static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, show_in, NULL, 8);
> +static SENSOR_DEVICE_ATTR(in9_input, S_IRUGO, show_in, NULL, 9);
> +static SENSOR_DEVICE_ATTR(in10_input, S_IRUGO, show_in, NULL, 10);
> +static SENSOR_DEVICE_ATTR(in11_input, S_IRUGO, show_in, NULL, 11);
> +
> +static SENSOR_DEVICE_ATTR(in0_label, S_IRUGO, show_in_label, NULL, 0);
> +static SENSOR_DEVICE_ATTR(in1_label, S_IRUGO, show_in_label, NULL, 1);
> +static SENSOR_DEVICE_ATTR(in2_label, S_IRUGO, show_in_label, NULL, 2);
> +static SENSOR_DEVICE_ATTR(in3_label, S_IRUGO, show_in_label, NULL, 3);
> +static SENSOR_DEVICE_ATTR(in4_label, S_IRUGO, show_in_label, NULL, 4);
> +static SENSOR_DEVICE_ATTR(in5_label, S_IRUGO, show_in_label, NULL, 5);
> +static SENSOR_DEVICE_ATTR(in6_label, S_IRUGO, show_in_label, NULL, 6);
> +static SENSOR_DEVICE_ATTR(in7_label, S_IRUGO, show_in_label, NULL, 7);
> +static SENSOR_DEVICE_ATTR(in8_label, S_IRUGO, show_in_label, NULL, 8);
> +static SENSOR_DEVICE_ATTR(in9_label, S_IRUGO, show_in_label, NULL, 9);
> +static SENSOR_DEVICE_ATTR(in10_label, S_IRUGO, show_in_label, NULL, 10);
> +static SENSOR_DEVICE_ATTR(in11_label, S_IRUGO, show_in_label, NULL, 11);
> +
> +static struct attribute *imanager2_volt_attrs[] = {
> +	&sensor_dev_attr_in0_label.dev_attr.attr,
> +	&sensor_dev_attr_in0_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_in1_label.dev_attr.attr,
> +	&sensor_dev_attr_in1_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_in2_label.dev_attr.attr,
> +	&sensor_dev_attr_in2_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_in3_label.dev_attr.attr,
> +	&sensor_dev_attr_in3_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_in4_label.dev_attr.attr,
> +	&sensor_dev_attr_in4_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_in5_label.dev_attr.attr,
> +	&sensor_dev_attr_in5_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_in6_label.dev_attr.attr,
> +	&sensor_dev_attr_in6_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_in7_label.dev_attr.attr,
> +	&sensor_dev_attr_in7_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_in8_label.dev_attr.attr,
> +	&sensor_dev_attr_in8_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_in9_label.dev_attr.attr,
> +	&sensor_dev_attr_in9_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_in10_label.dev_attr.attr,
> +	&sensor_dev_attr_in10_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_in11_label.dev_attr.attr,
> +	&sensor_dev_attr_in11_input.dev_attr.attr,
> +
> +	NULL
> +};
> +
> +static umode_t imanager2_volt_mode(struct kobject *kobj, struct attribute *attr,
> +				   int index)
> +{
> +	struct device_attribute *dev_attr;
> +
> +	dev_attr = container_of(attr, struct device_attribute, attr);
> +	if (!ec_volt_table[to_sensor_dev_attr(dev_attr)->index].visible)
> +		return 0;
> +
> +	return attr->mode;
> +}
> +
> +static const struct attribute_group imanager2_volt_group = {
> +	.attrs = imanager2_volt_attrs,
> +	.is_visible = imanager2_volt_mode,
> +};
> +
> +/* Current */
> +struct curr_item {
> +	const u8 did;
> +	const char *name;
> +	bool visible;
> +};
> +
> +static struct curr_item ec_curr_table[] = {
> +	{
> +		.did = adccurrent,
> +		.name = "IMON"
> +	},
> +};
> +
> +static int imanager2_curr_get_value(struct imanager2 *ec, int index,
> +				    u32 *curr_mamp)
> +{
> +	int ret;
> +	u8 tmp[5];
> +	u16 value, factor;
> +	u32 baseunit = 1;
> +
> +	mutex_lock(&ec->lock);
> +	ret = imanager2_mbox_read_data(ec->flag, EC_CMD_MAILBOX_READ_HW_PIN,
> +				       ec_curr_table[index].did, tmp,
> +				       ARRAY_SIZE(tmp));
> +	mutex_unlock(&ec->lock);
> +
> +	if (ret)
> +		return ret;
> +
> +	value = ((tmp[0] << 8) | tmp[1]) & EC_ADC_RESOLUTION_MAX;
> +	factor = (tmp[2] << 8) | tmp[3];

Is it guaranteed that factor is never 0 ? If not, this may result
in a division by 0 error.

> +
> +	while (tmp[4]++)
> +		baseunit *= 10;
> +
> +	*curr_mamp = DIV_ROUND_CLOSEST(value * baseunit * EC_ADC_VALUE_MAX,
> +				       EC_ADC_RESOLUTION_MAX * factor);
> +
> +	return 0;
> +}
> +
> +static void imanager2_curr_init(struct imanager2 *ec)
> +{
> +	u8 did;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ec_curr_table); i++) {
> +		did = ec_curr_table[i].did;
> +		if (ec->table.devid2itemnum[did] != EC_TABLE_ITEM_UNUSED)
> +			ec_curr_table[i].visible = ec->flag & EC_FLAG_MAILBOX;
> +		else
> +			ec_curr_table[i].visible = false;
> +	}
> +}
> +
> +static ssize_t show_curr(struct device *dev, struct device_attribute *dev_attr,
> +			 char *buf)
> +{
> +	struct imanager2 *ec = dev_get_drvdata(dev);
> +	u32 val;
> +	int ret = imanager2_curr_get_value(ec,
> +					   to_sensor_dev_attr(dev_attr)->index,
> +					   &val);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%u\n", val);
> +}
> +
> +static ssize_t show_curr_label(struct device *dev,
> +			       struct device_attribute *dev_attr, char *buf)
> +{
> +	return sprintf(buf, "%s\n",
> +		       ec_curr_table[to_sensor_dev_attr(dev_attr)->index].name);
> +}
> +
> +static SENSOR_DEVICE_ATTR(curr1_input, S_IRUGO, show_curr, NULL, 0);
> +static SENSOR_DEVICE_ATTR(curr1_label, S_IRUGO, show_curr_label, NULL, 0);
> +
> +static struct attribute *imanager2_curr_attrs[] = {
> +	&sensor_dev_attr_curr1_label.dev_attr.attr,
> +	&sensor_dev_attr_curr1_input.dev_attr.attr,
> +
> +	NULL
> +};
> +
> +static umode_t imanager2_curr_mode(struct kobject *kobj, struct attribute *attr,
> +				   int index)
> +{
> +	struct device_attribute *dev_attr;
> +
> +	dev_attr = container_of(attr, struct device_attribute, attr);
> +	if (!ec_curr_table[to_sensor_dev_attr(dev_attr)->index].visible)
> +		return 0;
> +
> +	return attr->mode;
> +}
> +
> +static const struct attribute_group imanager2_curr_group = {
> +	.attrs = imanager2_curr_attrs,
> +	.is_visible = imanager2_curr_mode,
> +};
> +
> +/* Temperature */
> +struct temp_item {
> +	const char *name;
> +	const u8 zonetype_value;
> +	u8 zonetype_acpireg;
> +	bool visible;
> +};
> +
> +static struct temp_item ec_temp_table[] = {
> +	{
> +		.name = "Temp SYS",
> +		.zonetype_value = sys1,
> +		.zonetype_acpireg = EC_ACPIRAM_ADDR_LOCAL_TEMPERATURE(1)
> +	},
> +	{
> +		.name = "Temp CPU",
> +		.zonetype_value = cpu,
> +		.zonetype_acpireg = EC_ACPIRAM_ADDR_REMOTE_TEMPERATURE(1)
> +	},
> +	{
> +		.name = "Temp SYS3",
> +		.zonetype_value = sys3,
> +		.zonetype_acpireg = EC_ACPIRAM_ADDR_LOCAL_TEMPERATURE(2)
> +	},
> +	{
> +		.name = "Temp SYS2",
> +		.zonetype_value = sys2,
> +		.zonetype_acpireg = EC_ACPIRAM_ADDR_REMOTE_TEMPERATURE(2)
> +	},
> +};
> +
> +static int imanager2_temp_get_value(struct imanager2 *ec, int index,
> +				    s32 *temp_mcelsius)
> +{
> +	int ret;
> +	s8 tmp;
> +
> +	mutex_lock(&ec->lock);
> +	ret = imanager2_mbox_read_ram_support_io(
> +		ec->flag, EC_RAM_BANK_ACPI,
> +		ec_temp_table[index].zonetype_acpireg,
> +		(u8 *)&tmp, 1);
> +	mutex_unlock(&ec->lock);
> +
> +	if (ret)
> +		return ret;
> +
> +	*temp_mcelsius = tmp * 1000;
> +
> +	return 0;
> +}
> +
> +static void imanager2_temp_init(struct imanager2 *ec)
> +{
> +	int i, thm, ret;
> +	u8 tmltype, smbid, fanid;
> +	struct ec_thermalzone zone;
> +
> +	for (i = 0; i < ARRAY_SIZE(ec_temp_table); i++)
> +		ec_temp_table[i].visible = false;
> +
> +	for (thm = 0; thm < EC_THERMAL_ZONE_MAX; thm++) {
> +		mutex_lock(&ec->lock);
> +
> +		if (ec->flag & EC_FLAG_MAILBOX) {
> +			int len = sizeof(struct ec_thermalzone);

Checkpatch warning.

> +			ret = imanager2_mbox_read_thermalzone(ec->flag, thm,
> +							      &smbid, &fanid,
> +							      (u8 *)&zone,
> +							      &len);
> +		} else {
> +			ret = imanager2_mbox_io_read(
> +				EC_CMD_HWRAM_READ,
> +				EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_STATUS(thm),
> +				&zone.status, 1);
> +		}
> +
> +		mutex_unlock(&ec->lock);
> +
> +		if (ret)
> +			continue;
> +
> +		tmltype = (zone.status >> 5);
> +
> +		for (i = 0; i < ARRAY_SIZE(ec_temp_table); i++) {
> +			if (tmltype == ec_temp_table[i].zonetype_value) {
> +				ec_temp_table[i].visible = true;
> +				break;
> +			}
> +		}
> +	}
> +}
> +
> +static ssize_t show_temp(struct device *dev, struct device_attribute *dev_attr,
> +			 char *buf)
> +{
> +	struct imanager2 *ec = dev_get_drvdata(dev);
> +	s32 val;
> +	int ret = imanager2_temp_get_value(ec,
> +					   to_sensor_dev_attr(dev_attr)->index,
> +					   &val);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", val);
> +}
> +
> +static ssize_t show_temp_label(struct device *dev,
> +			       struct device_attribute *dev_attr, char *buf)
> +{
> +	return sprintf(buf, "%s\n",
> +		       ec_temp_table[to_sensor_dev_attr(dev_attr)->index].name);
> +}
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp, NULL, 0);
> +static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp, NULL, 1);
> +static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp, NULL, 2);
> +static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp, NULL, 3);
> +
> +static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO, show_temp_label, NULL, 0);
> +static SENSOR_DEVICE_ATTR(temp2_label, S_IRUGO, show_temp_label, NULL, 1);
> +static SENSOR_DEVICE_ATTR(temp3_label, S_IRUGO, show_temp_label, NULL, 2);
> +static SENSOR_DEVICE_ATTR(temp4_label, S_IRUGO, show_temp_label, NULL, 3);
> +
> +static struct attribute *imanager2_temp_attrs[] = {
> +	&sensor_dev_attr_temp1_label.dev_attr.attr,
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_temp2_label.dev_attr.attr,
> +	&sensor_dev_attr_temp2_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_temp3_label.dev_attr.attr,
> +	&sensor_dev_attr_temp3_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_temp4_label.dev_attr.attr,
> +	&sensor_dev_attr_temp4_input.dev_attr.attr,
> +
> +	NULL
> +};
> +
> +static umode_t imanager2_temp_mode(struct kobject *kobj, struct attribute *attr,
> +				   int index)
> +{
> +	struct device_attribute *dev_attr;
> +
> +	dev_attr = container_of(attr, struct device_attribute, attr);
> +	if (!ec_temp_table[to_sensor_dev_attr(dev_attr)->index].visible)
> +		return 0;
> +
> +	return attr->mode;
> +}
> +
> +static const struct attribute_group imanager2_temp_group = {
> +	.attrs = imanager2_temp_attrs,
> +	.is_visible = imanager2_temp_mode,
> +};
> +
> +/* Fan Speed */
> +struct fan_item {
> +	const u8 did;
> +	const char *name;
> +	u8 fspeed_acpireg;
> +	bool visible;
> +};
> +
> +static struct fan_item ec_fan_table[] = {
> +	{
> +		.did = tacho0,
> +		.name = "Fan CPU",
> +		.fspeed_acpireg = 0,

It is not necessary to set such constants to 0.

> +		.visible = false
> +	},
> +	{
> +		.did = tacho1,
> +		.name = "Fan SYS",
> +		.fspeed_acpireg = 0,
> +		.visible = false
> +	},
> +	{
> +		.did = tacho2,
> +		.name = "Fan SYS2",
> +		.fspeed_acpireg = 0,
> +		.visible = false
> +	},
> +};
> +
> +static int imanager2_fan_get_value(struct imanager2 *ec, int index,
> +				   u32 *speed_rpm)
> +{
> +	int ret;
> +	u8 tmp[2];
> +
> +	mutex_lock(&ec->lock);
> +
> +	if (ec->flag & EC_FLAG_MAILBOX)
> +		ret = imanager2_mbox_read_data(ec->flag,
> +					       EC_CMD_MAILBOX_READ_HW_PIN,
> +					       ec_fan_table[index].did, tmp, 2);
> +	else
> +		ret = imanager2_mbox_io_read(EC_CMD_ACPIRAM_READ,
> +					     ec_fan_table[index].fspeed_acpireg,
> +					     tmp, 2);
> +
> +	mutex_unlock(&ec->lock);
> +
> +	if (ret)
> +		return ret;
> +
> +	if (tmp[0] == 0xFF && tmp[1] == 0xFF)
> +		return -ENODEV;

That is a bit late for detecting that there is no such device.

> +
> +	*speed_rpm = (tmp[0] << 8) | tmp[1];
> +
> +	return 0;
> +}
> +
> +#define EC_FANCTROL_SPEEDSRC_MASK	0x30
> +
> +static int imanager2_fan_item_init_by_io(struct imanager2 *ec, int fnum)
> +{
> +	int i, ret;
> +	u8 tmp;
> +
> +	mutex_lock(&ec->lock);
> +	ret = imanager2_mbox_io_read(EC_CMD_HWRAM_READ,
> +				     EC_HWRAM_ADDR_FAN_CONTROL(fnum), &tmp, 1);
> +	mutex_unlock(&ec->lock);
> +
> +	if (ret)
> +		return ret;
> +
> +	i = ((tmp & EC_FANCTROL_SPEEDSRC_MASK) >> 4) - 1;
> +	if (i < 0)
> +		return -ENODEV;
> +

This error return is quite pointless. See below.

> +	/* Unnecessary set again if it has been set. */
> +	if (ec_fan_table[i].visible)
> +		return 0;
> +
> +	ec_fan_table[i].fspeed_acpireg = EC_ACPIRAM_ADDR_FAN_SPEED_BASE(i);
> +	ec_fan_table[i].visible = true;
> +
> +	return 0;
> +}
> +
> +static void imanager2_fan_init(struct imanager2 *ec)
> +{
> +	int i;
> +
> +	if (ec->flag & EC_FLAG_MAILBOX) {
> +		for (i = 0; i < ARRAY_SIZE(ec_fan_table); i++)
> +			ec_fan_table[i].visible =
> +			    ec->table.devid2itemnum[ec_fan_table[i].did] !=
> +			    EC_TABLE_ITEM_UNUSED;
> +	} else {
> +		int fnum;
> +
> +		for (i = 0; i < ARRAY_SIZE(ec_fan_table); i++)
> +			ec_fan_table[i].visible = false;
> +
> +		for (fnum = 0; fnum < EC_MAX_IO_FAN; fnum++) {
> +			if (imanager2_fan_item_init_by_io(ec, fnum))
> +				continue;

This continue is quite pointless.

> +		}
> +	}
> +}
> +
> +static ssize_t show_fan(struct device *dev, struct device_attribute *dev_attr,
> +			char *buf)
> +{
> +	struct imanager2 *ec = dev_get_drvdata(dev);
> +	u32 val;
> +	int ret = imanager2_fan_get_value(ec,
> +					  to_sensor_dev_attr(dev_attr)->index,
> +					  &val);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%u\n", val);
> +}
> +
> +static ssize_t show_fan_label(struct device *dev,
> +			      struct device_attribute *dev_attr, char *buf)
> +{
> +	return sprintf(buf, "%s\n",
> +		       ec_fan_table[to_sensor_dev_attr(dev_attr)->index].name);
> +}
> +
> +static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, show_fan, NULL, 0);
> +static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, show_fan, NULL, 1);
> +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, show_fan, NULL, 2);
> +
> +static SENSOR_DEVICE_ATTR(fan1_label, S_IRUGO, show_fan_label, NULL, 0);
> +static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, show_fan_label, NULL, 1);
> +static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, show_fan_label, NULL, 2);
> +
> +static struct attribute *imanager2_fan_attrs[] = {
> +	&sensor_dev_attr_fan1_label.dev_attr.attr,
> +	&sensor_dev_attr_fan1_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_fan2_label.dev_attr.attr,
> +	&sensor_dev_attr_fan2_input.dev_attr.attr,
> +
> +	&sensor_dev_attr_fan3_label.dev_attr.attr,
> +	&sensor_dev_attr_fan3_input.dev_attr.attr,
> +
> +	NULL
> +};
> +
> +static umode_t imanager2_fan_mode(struct kobject *kobj, struct attribute *attr,
> +				  int index)
> +{
> +	struct device_attribute *dev_attr;
> +
> +	dev_attr = container_of(attr, struct device_attribute, attr);
> +	if (!ec_fan_table[to_sensor_dev_attr(dev_attr)->index].visible)
> +		return 0;
> +
> +	return attr->mode;
> +}
> +
> +static const struct attribute_group imanager2_fan_group = {
> +	.attrs = imanager2_fan_attrs,
> +	.is_visible = imanager2_fan_mode,
> +};
> +
> +/* Module */
> +static const struct attribute_group *imanager2_hwmon_groups[] = {
> +	&imanager2_volt_group,
> +	&imanager2_curr_group,
> +	&imanager2_temp_group,
> +	&imanager2_fan_group,
> +	NULL
> +};
> +
> +static int imanager2_hwmon_probe(struct platform_device *pdev)
> +{
> +	struct device *hwmon_dev;
> +	struct imanager2 *hwmon_ec = pdev->dev.parent->platform_data;
> +
> +	imanager2_volt_init(hwmon_ec);
> +	imanager2_curr_init(hwmon_ec);
> +	imanager2_temp_init(hwmon_ec);
> +	imanager2_fan_init(hwmon_ec);
> +
> +	hwmon_dev = devm_hwmon_device_register_with_groups(
> +			&pdev->dev, "imanager2", hwmon_ec,
> +			imanager2_hwmon_groups);
> +
> +	if (IS_ERR(hwmon_dev))
> +		return PTR_ERR(hwmon_dev);
> +
> +	return 0;

	return PTR_ERR_OR_ZERO(hwmon_dev);

> +}
> +
> +static struct platform_driver imanager2_hwmon_driver = {
> +	.probe = imanager2_hwmon_probe,
> +	.driver = {
> +			.owner = THIS_MODULE,
> +			.name = DRV_NAME,
> +	},
> +};
> +
> +module_platform_driver(imanager2_hwmon_driver);
> +
> +MODULE_AUTHOR("Richard Vidal-Dorsch <richard.dorsch at advantech.com>");
> +MODULE_DESCRIPTION("HW Monitoring interface for Advantech EC IT8516/18/28");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION(DRV_VERSION);
> -- 
> 1.9.1
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 4/4] hwmon: (imanager2) Add support for IT8516/18/28
  2014-07-14 12:54 [PATCH 1/4] mfd: imanager2: Add defines " Wei-Chun Pan
@ 2014-07-14 12:54 ` Wei-Chun Pan
  2014-07-14 19:05   ` Guenter Roeck
  0 siblings, 1 reply; 4+ messages in thread
From: Wei-Chun Pan @ 2014-07-14 12:54 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones, Jean Delvare, Guenter Roeck
  Cc: Louis.Lu, Neo.Lo, Hank.Peng, Kevin.Ong, linux-kernel, Wei-Chun Pan

Signed-off-by: Wei-Chun Pan <weichun.pan@advantech.com.tw>
---
 drivers/hwmon/Kconfig         |   5 +
 drivers/hwmon/Makefile        |   1 +
 drivers/hwmon/imanager2_hwm.c | 768 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 774 insertions(+)
 create mode 100644 drivers/hwmon/imanager2_hwm.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index bc196f4..7524fc3 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -39,6 +39,11 @@ config HWMON_DEBUG_CHIP
 
 comment "Native drivers"
 
+config SENSORS_IMANAGER2
+	tristate "Support for Advantech iManager2 EC H.W. Monitor"
+	select MFD_CORE
+	depends on MFD_IMANAGER2
+
 config SENSORS_AB8500
 	tristate "AB8500 thermal monitoring"
 	depends on AB8500_GPADC && AB8500_BM
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index c48f987..a2c8f07 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -146,6 +146,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_SENSORS_IMANAGER2)	+= imanager2_hwm.o
 
 obj-$(CONFIG_PMBUS)		+= pmbus/
 
diff --git a/drivers/hwmon/imanager2_hwm.c b/drivers/hwmon/imanager2_hwm.c
new file mode 100644
index 0000000..335bffb
--- /dev/null
+++ b/drivers/hwmon/imanager2_hwm.c
@@ -0,0 +1,768 @@
+/*
+ * imanager2_hwm.c - HW Monitoring interface for Advantech EC IT8516/18/28
+ * Copyright (C) 2014  Richard Vidal-Dorsch <richard.dorsch@advantech.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 Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/mfd/imanager2_ec.h>
+
+#define DRV_NAME	"imanager2_hwm"
+#define DRV_VERSION	"4.0.1"
+
+/* ADC */
+#define EC_ADC_RESOLUTION_MAX	0x03FF	/* 10-bit */
+#define EC_ADC_VALUE_MAX	3000	/* max: 3000 mV or mA */
+
+/* Thermal */
+#define EC_THERMAL_ZONE_MAX	4
+
+enum thermaltype {
+	none,
+	sys1,
+	cpu,
+	sys3,
+	sys2,
+};
+
+struct ec_thermalzone {
+	u8 channel,
+	   addr,
+	   cmd,
+	   status,
+	   fancode,
+	   temp;
+};
+
+/* Tacho */
+#define EC_MAX_IO_FAN	3
+
+/* Voltage */
+struct volt_item {
+	u8 did;
+	const char *name;
+	int factor;
+	bool visible;
+};
+
+static struct volt_item ec_volt_table[] = {
+	{
+		.did = adcmosbat,
+		.name = "BAT CMOS",
+	},
+	{
+		.did = adcbat,
+		.name = "BAT",
+	},
+	{
+		.did = adc5vs0,
+		.name = "5V S0",
+	},
+	{
+		.did = adv5vs5,
+		.name = "5V S5",
+	},
+	{
+		.did = adc33vs0,
+		.name = "3V3 S0",
+	},
+	{
+		.did = adc33vs5,
+		.name = "3V3 S5",
+	},
+	{
+		.did = adv12vs0,
+		.name = "12V S0",
+	},
+	{
+		.did = adcvcorea,
+		.name = "Vcore A",
+	},
+	{
+		.did = adcvcoreb,
+		.name = "Vcore B",
+	},
+	{
+		.did = adcdc,
+		.name = "DC",
+	},
+	{
+		.did = adcdcstby,
+		.name = "DC Standby",
+	},
+	{
+		.did = adcdcother,
+		.name = "DC Other",
+	},
+};
+
+static int imanager2_volt_get_value_by_io(struct imanager2 *ec, int index,
+					  u8 *buf)
+{
+	u8 item = ec->table.devid2itemnum[ec_volt_table[index].did];
+	u8 pin = ec->table.pinnum[item];
+	u8 portnum;
+	int ret;
+
+	ret = imanager2_mbox_io_read(EC_CMD_ADC_INDEX, pin, &portnum, 1);
+	if (ret)
+		return ret;
+	if (portnum == EC_ERROR)
+		return -ENXIO;
+
+	ret = imanager2_mbox_io_simple_read(EC_CMD_ADC_READ_LSB, &buf[1]);
+	if (ret)
+		return ret;
+
+	return imanager2_mbox_io_simple_read(EC_CMD_ADC_READ_MSB, &buf[0]);
+}
+
+static int imanager2_volt_get_value(struct imanager2 *ec, int index,
+				    u32 *volt_mvolt)
+{
+	int ret;
+	u8 tmp[2];
+
+	mutex_lock(&ec->lock);
+
+	if (ec->flag & EC_FLAG_MAILBOX)
+		ret = imanager2_mbox_read_data(ec->flag,
+					       EC_CMD_MAILBOX_READ_HW_PIN,
+					       ec_volt_table[index].did,
+					       tmp, 2);
+	else
+		ret = imanager2_volt_get_value_by_io(ec, index, tmp);
+
+	mutex_unlock(&ec->lock);
+
+	if (ret)
+		return ret;
+
+	*volt_mvolt = (((tmp[0] << 8) | tmp[1]) & EC_ADC_RESOLUTION_MAX) *
+		      ec_volt_table[index].factor *
+		      DIV_ROUND_CLOSEST(EC_ADC_VALUE_MAX,
+					EC_ADC_RESOLUTION_MAX);
+
+	return 0;
+}
+
+static void imanager2_volt_init(struct imanager2 *ec)
+{
+	u8 did;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ec_volt_table); i++) {
+		did = ec_volt_table[i].did;
+
+		if (ec->table.devid2itemnum[did] != EC_TABLE_ITEM_UNUSED) {
+			ec_volt_table[i].factor = 1;
+			ec_volt_table[i].visible = true;
+		} else if (ec->table.devid2itemnum[did + 1] !=
+			   EC_TABLE_ITEM_UNUSED) {
+			ec_volt_table[i].did += 1;
+			ec_volt_table[i].factor = 2;
+			ec_volt_table[i].visible = true;
+		} else if (ec->table.devid2itemnum[did + 2] !=
+			   EC_TABLE_ITEM_UNUSED) {
+			ec_volt_table[i].did += 2;
+			ec_volt_table[i].factor = 10;
+			ec_volt_table[i].visible = true;
+		} else {
+			ec_volt_table[i].visible = false;
+		}
+	}
+}
+
+static ssize_t show_in(struct device *dev, struct device_attribute *dev_attr,
+		       char *buf)
+{
+	struct imanager2 *ec = dev_get_drvdata(dev);
+	u32 val;
+	int ret = imanager2_volt_get_value(ec,
+					   to_sensor_dev_attr(dev_attr)->index,
+					   &val);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%u\n", val);
+}
+
+static ssize_t show_in_label(struct device *dev,
+			     struct device_attribute *dev_attr, char *buf)
+{
+	return sprintf(buf, "%s\n",
+		       ec_volt_table[to_sensor_dev_attr(dev_attr)->index].name);
+}
+
+static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, show_in, NULL, 0);
+static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, show_in, NULL, 1);
+static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, show_in, NULL, 2);
+static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, show_in, NULL, 3);
+static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, show_in, NULL, 4);
+static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, show_in, NULL, 5);
+static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, show_in, NULL, 6);
+static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, show_in, NULL, 7);
+static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, show_in, NULL, 8);
+static SENSOR_DEVICE_ATTR(in9_input, S_IRUGO, show_in, NULL, 9);
+static SENSOR_DEVICE_ATTR(in10_input, S_IRUGO, show_in, NULL, 10);
+static SENSOR_DEVICE_ATTR(in11_input, S_IRUGO, show_in, NULL, 11);
+
+static SENSOR_DEVICE_ATTR(in0_label, S_IRUGO, show_in_label, NULL, 0);
+static SENSOR_DEVICE_ATTR(in1_label, S_IRUGO, show_in_label, NULL, 1);
+static SENSOR_DEVICE_ATTR(in2_label, S_IRUGO, show_in_label, NULL, 2);
+static SENSOR_DEVICE_ATTR(in3_label, S_IRUGO, show_in_label, NULL, 3);
+static SENSOR_DEVICE_ATTR(in4_label, S_IRUGO, show_in_label, NULL, 4);
+static SENSOR_DEVICE_ATTR(in5_label, S_IRUGO, show_in_label, NULL, 5);
+static SENSOR_DEVICE_ATTR(in6_label, S_IRUGO, show_in_label, NULL, 6);
+static SENSOR_DEVICE_ATTR(in7_label, S_IRUGO, show_in_label, NULL, 7);
+static SENSOR_DEVICE_ATTR(in8_label, S_IRUGO, show_in_label, NULL, 8);
+static SENSOR_DEVICE_ATTR(in9_label, S_IRUGO, show_in_label, NULL, 9);
+static SENSOR_DEVICE_ATTR(in10_label, S_IRUGO, show_in_label, NULL, 10);
+static SENSOR_DEVICE_ATTR(in11_label, S_IRUGO, show_in_label, NULL, 11);
+
+static struct attribute *imanager2_volt_attrs[] = {
+	&sensor_dev_attr_in0_label.dev_attr.attr,
+	&sensor_dev_attr_in0_input.dev_attr.attr,
+
+	&sensor_dev_attr_in1_label.dev_attr.attr,
+	&sensor_dev_attr_in1_input.dev_attr.attr,
+
+	&sensor_dev_attr_in2_label.dev_attr.attr,
+	&sensor_dev_attr_in2_input.dev_attr.attr,
+
+	&sensor_dev_attr_in3_label.dev_attr.attr,
+	&sensor_dev_attr_in3_input.dev_attr.attr,
+
+	&sensor_dev_attr_in4_label.dev_attr.attr,
+	&sensor_dev_attr_in4_input.dev_attr.attr,
+
+	&sensor_dev_attr_in5_label.dev_attr.attr,
+	&sensor_dev_attr_in5_input.dev_attr.attr,
+
+	&sensor_dev_attr_in6_label.dev_attr.attr,
+	&sensor_dev_attr_in6_input.dev_attr.attr,
+
+	&sensor_dev_attr_in7_label.dev_attr.attr,
+	&sensor_dev_attr_in7_input.dev_attr.attr,
+
+	&sensor_dev_attr_in8_label.dev_attr.attr,
+	&sensor_dev_attr_in8_input.dev_attr.attr,
+
+	&sensor_dev_attr_in9_label.dev_attr.attr,
+	&sensor_dev_attr_in9_input.dev_attr.attr,
+
+	&sensor_dev_attr_in10_label.dev_attr.attr,
+	&sensor_dev_attr_in10_input.dev_attr.attr,
+
+	&sensor_dev_attr_in11_label.dev_attr.attr,
+	&sensor_dev_attr_in11_input.dev_attr.attr,
+
+	NULL
+};
+
+static umode_t imanager2_volt_mode(struct kobject *kobj, struct attribute *attr,
+				   int index)
+{
+	struct device_attribute *dev_attr;
+
+	dev_attr = container_of(attr, struct device_attribute, attr);
+	if (!ec_volt_table[to_sensor_dev_attr(dev_attr)->index].visible)
+		return 0;
+
+	return attr->mode;
+}
+
+static const struct attribute_group imanager2_volt_group = {
+	.attrs = imanager2_volt_attrs,
+	.is_visible = imanager2_volt_mode,
+};
+
+/* Current */
+struct curr_item {
+	const u8 did;
+	const char *name;
+	bool visible;
+};
+
+static struct curr_item ec_curr_table[] = {
+	{
+		.did = adccurrent,
+		.name = "IMON"
+	},
+};
+
+static int imanager2_curr_get_value(struct imanager2 *ec, int index,
+				    u32 *curr_mamp)
+{
+	int ret;
+	u8 tmp[5];
+	u16 value, factor;
+	u32 baseunit = 1;
+
+	mutex_lock(&ec->lock);
+	ret = imanager2_mbox_read_data(ec->flag, EC_CMD_MAILBOX_READ_HW_PIN,
+				       ec_curr_table[index].did, tmp,
+				       ARRAY_SIZE(tmp));
+	mutex_unlock(&ec->lock);
+
+	if (ret)
+		return ret;
+
+	value = ((tmp[0] << 8) | tmp[1]) & EC_ADC_RESOLUTION_MAX;
+	factor = (tmp[2] << 8) | tmp[3];
+
+	while (tmp[4]++)
+		baseunit *= 10;
+
+	*curr_mamp = DIV_ROUND_CLOSEST(value * baseunit * EC_ADC_VALUE_MAX,
+				       EC_ADC_RESOLUTION_MAX * factor);
+
+	return 0;
+}
+
+static void imanager2_curr_init(struct imanager2 *ec)
+{
+	u8 did;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ec_curr_table); i++) {
+		did = ec_curr_table[i].did;
+		if (ec->table.devid2itemnum[did] != EC_TABLE_ITEM_UNUSED)
+			ec_curr_table[i].visible = ec->flag & EC_FLAG_MAILBOX;
+		else
+			ec_curr_table[i].visible = false;
+	}
+}
+
+static ssize_t show_curr(struct device *dev, struct device_attribute *dev_attr,
+			 char *buf)
+{
+	struct imanager2 *ec = dev_get_drvdata(dev);
+	u32 val;
+	int ret = imanager2_curr_get_value(ec,
+					   to_sensor_dev_attr(dev_attr)->index,
+					   &val);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%u\n", val);
+}
+
+static ssize_t show_curr_label(struct device *dev,
+			       struct device_attribute *dev_attr, char *buf)
+{
+	return sprintf(buf, "%s\n",
+		       ec_curr_table[to_sensor_dev_attr(dev_attr)->index].name);
+}
+
+static SENSOR_DEVICE_ATTR(curr1_input, S_IRUGO, show_curr, NULL, 0);
+static SENSOR_DEVICE_ATTR(curr1_label, S_IRUGO, show_curr_label, NULL, 0);
+
+static struct attribute *imanager2_curr_attrs[] = {
+	&sensor_dev_attr_curr1_label.dev_attr.attr,
+	&sensor_dev_attr_curr1_input.dev_attr.attr,
+
+	NULL
+};
+
+static umode_t imanager2_curr_mode(struct kobject *kobj, struct attribute *attr,
+				   int index)
+{
+	struct device_attribute *dev_attr;
+
+	dev_attr = container_of(attr, struct device_attribute, attr);
+	if (!ec_curr_table[to_sensor_dev_attr(dev_attr)->index].visible)
+		return 0;
+
+	return attr->mode;
+}
+
+static const struct attribute_group imanager2_curr_group = {
+	.attrs = imanager2_curr_attrs,
+	.is_visible = imanager2_curr_mode,
+};
+
+/* Temperature */
+struct temp_item {
+	const char *name;
+	const u8 zonetype_value;
+	u8 zonetype_acpireg;
+	bool visible;
+};
+
+static struct temp_item ec_temp_table[] = {
+	{
+		.name = "Temp SYS",
+		.zonetype_value = sys1,
+		.zonetype_acpireg = EC_ACPIRAM_ADDR_LOCAL_TEMPERATURE(1)
+	},
+	{
+		.name = "Temp CPU",
+		.zonetype_value = cpu,
+		.zonetype_acpireg = EC_ACPIRAM_ADDR_REMOTE_TEMPERATURE(1)
+	},
+	{
+		.name = "Temp SYS3",
+		.zonetype_value = sys3,
+		.zonetype_acpireg = EC_ACPIRAM_ADDR_LOCAL_TEMPERATURE(2)
+	},
+	{
+		.name = "Temp SYS2",
+		.zonetype_value = sys2,
+		.zonetype_acpireg = EC_ACPIRAM_ADDR_REMOTE_TEMPERATURE(2)
+	},
+};
+
+static int imanager2_temp_get_value(struct imanager2 *ec, int index,
+				    s32 *temp_mcelsius)
+{
+	int ret;
+	s8 tmp;
+
+	mutex_lock(&ec->lock);
+	ret = imanager2_mbox_read_ram_support_io(
+		ec->flag, EC_RAM_BANK_ACPI,
+		ec_temp_table[index].zonetype_acpireg,
+		(u8 *)&tmp, 1);
+	mutex_unlock(&ec->lock);
+
+	if (ret)
+		return ret;
+
+	*temp_mcelsius = tmp * 1000;
+
+	return 0;
+}
+
+static void imanager2_temp_init(struct imanager2 *ec)
+{
+	int i, thm, ret;
+	u8 tmltype, smbid, fanid;
+	struct ec_thermalzone zone;
+
+	for (i = 0; i < ARRAY_SIZE(ec_temp_table); i++)
+		ec_temp_table[i].visible = false;
+
+	for (thm = 0; thm < EC_THERMAL_ZONE_MAX; thm++) {
+		mutex_lock(&ec->lock);
+
+		if (ec->flag & EC_FLAG_MAILBOX) {
+			int len = sizeof(struct ec_thermalzone);
+			ret = imanager2_mbox_read_thermalzone(ec->flag, thm,
+							      &smbid, &fanid,
+							      (u8 *)&zone,
+							      &len);
+		} else {
+			ret = imanager2_mbox_io_read(
+				EC_CMD_HWRAM_READ,
+				EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_STATUS(thm),
+				&zone.status, 1);
+		}
+
+		mutex_unlock(&ec->lock);
+
+		if (ret)
+			continue;
+
+		tmltype = (zone.status >> 5);
+
+		for (i = 0; i < ARRAY_SIZE(ec_temp_table); i++) {
+			if (tmltype == ec_temp_table[i].zonetype_value) {
+				ec_temp_table[i].visible = true;
+				break;
+			}
+		}
+	}
+}
+
+static ssize_t show_temp(struct device *dev, struct device_attribute *dev_attr,
+			 char *buf)
+{
+	struct imanager2 *ec = dev_get_drvdata(dev);
+	s32 val;
+	int ret = imanager2_temp_get_value(ec,
+					   to_sensor_dev_attr(dev_attr)->index,
+					   &val);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", val);
+}
+
+static ssize_t show_temp_label(struct device *dev,
+			       struct device_attribute *dev_attr, char *buf)
+{
+	return sprintf(buf, "%s\n",
+		       ec_temp_table[to_sensor_dev_attr(dev_attr)->index].name);
+}
+
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp, NULL, 3);
+
+static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO, show_temp_label, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp2_label, S_IRUGO, show_temp_label, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp3_label, S_IRUGO, show_temp_label, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp4_label, S_IRUGO, show_temp_label, NULL, 3);
+
+static struct attribute *imanager2_temp_attrs[] = {
+	&sensor_dev_attr_temp1_label.dev_attr.attr,
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+
+	&sensor_dev_attr_temp2_label.dev_attr.attr,
+	&sensor_dev_attr_temp2_input.dev_attr.attr,
+
+	&sensor_dev_attr_temp3_label.dev_attr.attr,
+	&sensor_dev_attr_temp3_input.dev_attr.attr,
+
+	&sensor_dev_attr_temp4_label.dev_attr.attr,
+	&sensor_dev_attr_temp4_input.dev_attr.attr,
+
+	NULL
+};
+
+static umode_t imanager2_temp_mode(struct kobject *kobj, struct attribute *attr,
+				   int index)
+{
+	struct device_attribute *dev_attr;
+
+	dev_attr = container_of(attr, struct device_attribute, attr);
+	if (!ec_temp_table[to_sensor_dev_attr(dev_attr)->index].visible)
+		return 0;
+
+	return attr->mode;
+}
+
+static const struct attribute_group imanager2_temp_group = {
+	.attrs = imanager2_temp_attrs,
+	.is_visible = imanager2_temp_mode,
+};
+
+/* Fan Speed */
+struct fan_item {
+	const u8 did;
+	const char *name;
+	u8 fspeed_acpireg;
+	bool visible;
+};
+
+static struct fan_item ec_fan_table[] = {
+	{
+		.did = tacho0,
+		.name = "Fan CPU",
+		.fspeed_acpireg = 0,
+		.visible = false
+	},
+	{
+		.did = tacho1,
+		.name = "Fan SYS",
+		.fspeed_acpireg = 0,
+		.visible = false
+	},
+	{
+		.did = tacho2,
+		.name = "Fan SYS2",
+		.fspeed_acpireg = 0,
+		.visible = false
+	},
+};
+
+static int imanager2_fan_get_value(struct imanager2 *ec, int index,
+				   u32 *speed_rpm)
+{
+	int ret;
+	u8 tmp[2];
+
+	mutex_lock(&ec->lock);
+
+	if (ec->flag & EC_FLAG_MAILBOX)
+		ret = imanager2_mbox_read_data(ec->flag,
+					       EC_CMD_MAILBOX_READ_HW_PIN,
+					       ec_fan_table[index].did, tmp, 2);
+	else
+		ret = imanager2_mbox_io_read(EC_CMD_ACPIRAM_READ,
+					     ec_fan_table[index].fspeed_acpireg,
+					     tmp, 2);
+
+	mutex_unlock(&ec->lock);
+
+	if (ret)
+		return ret;
+
+	if (tmp[0] == 0xFF && tmp[1] == 0xFF)
+		return -ENODEV;
+
+	*speed_rpm = (tmp[0] << 8) | tmp[1];
+
+	return 0;
+}
+
+#define EC_FANCTROL_SPEEDSRC_MASK	0x30
+
+static int imanager2_fan_item_init_by_io(struct imanager2 *ec, int fnum)
+{
+	int i, ret;
+	u8 tmp;
+
+	mutex_lock(&ec->lock);
+	ret = imanager2_mbox_io_read(EC_CMD_HWRAM_READ,
+				     EC_HWRAM_ADDR_FAN_CONTROL(fnum), &tmp, 1);
+	mutex_unlock(&ec->lock);
+
+	if (ret)
+		return ret;
+
+	i = ((tmp & EC_FANCTROL_SPEEDSRC_MASK) >> 4) - 1;
+	if (i < 0)
+		return -ENODEV;
+
+	/* Unnecessary set again if it has been set. */
+	if (ec_fan_table[i].visible)
+		return 0;
+
+	ec_fan_table[i].fspeed_acpireg = EC_ACPIRAM_ADDR_FAN_SPEED_BASE(i);
+	ec_fan_table[i].visible = true;
+
+	return 0;
+}
+
+static void imanager2_fan_init(struct imanager2 *ec)
+{
+	int i;
+
+	if (ec->flag & EC_FLAG_MAILBOX) {
+		for (i = 0; i < ARRAY_SIZE(ec_fan_table); i++)
+			ec_fan_table[i].visible =
+			    ec->table.devid2itemnum[ec_fan_table[i].did] !=
+			    EC_TABLE_ITEM_UNUSED;
+	} else {
+		int fnum;
+
+		for (i = 0; i < ARRAY_SIZE(ec_fan_table); i++)
+			ec_fan_table[i].visible = false;
+
+		for (fnum = 0; fnum < EC_MAX_IO_FAN; fnum++) {
+			if (imanager2_fan_item_init_by_io(ec, fnum))
+				continue;
+		}
+	}
+}
+
+static ssize_t show_fan(struct device *dev, struct device_attribute *dev_attr,
+			char *buf)
+{
+	struct imanager2 *ec = dev_get_drvdata(dev);
+	u32 val;
+	int ret = imanager2_fan_get_value(ec,
+					  to_sensor_dev_attr(dev_attr)->index,
+					  &val);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%u\n", val);
+}
+
+static ssize_t show_fan_label(struct device *dev,
+			      struct device_attribute *dev_attr, char *buf)
+{
+	return sprintf(buf, "%s\n",
+		       ec_fan_table[to_sensor_dev_attr(dev_attr)->index].name);
+}
+
+static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, show_fan, NULL, 0);
+static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, show_fan, NULL, 1);
+static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, show_fan, NULL, 2);
+
+static SENSOR_DEVICE_ATTR(fan1_label, S_IRUGO, show_fan_label, NULL, 0);
+static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, show_fan_label, NULL, 1);
+static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, show_fan_label, NULL, 2);
+
+static struct attribute *imanager2_fan_attrs[] = {
+	&sensor_dev_attr_fan1_label.dev_attr.attr,
+	&sensor_dev_attr_fan1_input.dev_attr.attr,
+
+	&sensor_dev_attr_fan2_label.dev_attr.attr,
+	&sensor_dev_attr_fan2_input.dev_attr.attr,
+
+	&sensor_dev_attr_fan3_label.dev_attr.attr,
+	&sensor_dev_attr_fan3_input.dev_attr.attr,
+
+	NULL
+};
+
+static umode_t imanager2_fan_mode(struct kobject *kobj, struct attribute *attr,
+				  int index)
+{
+	struct device_attribute *dev_attr;
+
+	dev_attr = container_of(attr, struct device_attribute, attr);
+	if (!ec_fan_table[to_sensor_dev_attr(dev_attr)->index].visible)
+		return 0;
+
+	return attr->mode;
+}
+
+static const struct attribute_group imanager2_fan_group = {
+	.attrs = imanager2_fan_attrs,
+	.is_visible = imanager2_fan_mode,
+};
+
+/* Module */
+static const struct attribute_group *imanager2_hwmon_groups[] = {
+	&imanager2_volt_group,
+	&imanager2_curr_group,
+	&imanager2_temp_group,
+	&imanager2_fan_group,
+	NULL
+};
+
+static int imanager2_hwmon_probe(struct platform_device *pdev)
+{
+	struct device *hwmon_dev;
+	struct imanager2 *hwmon_ec = pdev->dev.parent->platform_data;
+
+	imanager2_volt_init(hwmon_ec);
+	imanager2_curr_init(hwmon_ec);
+	imanager2_temp_init(hwmon_ec);
+	imanager2_fan_init(hwmon_ec);
+
+	hwmon_dev = devm_hwmon_device_register_with_groups(
+			&pdev->dev, "imanager2", hwmon_ec,
+			imanager2_hwmon_groups);
+
+	if (IS_ERR(hwmon_dev))
+		return PTR_ERR(hwmon_dev);
+
+	return 0;
+}
+
+static struct platform_driver imanager2_hwmon_driver = {
+	.probe = imanager2_hwmon_probe,
+	.driver = {
+			.owner = THIS_MODULE,
+			.name = DRV_NAME,
+	},
+};
+
+module_platform_driver(imanager2_hwmon_driver);
+
+MODULE_AUTHOR("Richard Vidal-Dorsch <richard.dorsch at advantech.com>");
+MODULE_DESCRIPTION("HW Monitoring interface for Advantech EC IT8516/18/28");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(DRV_VERSION);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-08-07 10:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-07  9:24 [PATCH 4/4] hwmon: (imanager2) Add support for IT8516/18/28 Wei-Chun Pan
2014-08-07 10:06 ` Lee Jones
  -- strict thread matches above, loose matches on Subject: below --
2014-07-14 12:54 [PATCH 1/4] mfd: imanager2: Add defines " Wei-Chun Pan
2014-07-14 12:54 ` [PATCH 4/4] hwmon: (imanager2) Add " Wei-Chun Pan
2014-07-14 19:05   ` Guenter Roeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.