All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] mfd: imanager2: Add defines support for IT8516/18/28
@ 2014-07-14 12:54 Wei-Chun Pan
  2014-07-14 12:54 ` [PATCH 2/4] mfd: imanager2: Add Advantech EC APIs " Wei-Chun Pan
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ 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>
---
 include/linux/mfd/imanager2_ec.h | 358 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 358 insertions(+)
 create mode 100644 include/linux/mfd/imanager2_ec.h

diff --git a/include/linux/mfd/imanager2_ec.h b/include/linux/mfd/imanager2_ec.h
new file mode 100644
index 0000000..bf7d70e
--- /dev/null
+++ b/include/linux/mfd/imanager2_ec.h
@@ -0,0 +1,358 @@
+/*
+ * imanager2_ec.h - MFD driver defines of 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/>.
+ */
+
+#ifndef __IMANAGER2_EC_H__
+#define __IMANAGER2_EC_H__
+
+#include <linux/mutex.h>
+
+#define EC_FLAG_IO		0
+#define EC_FLAG_IO_MAILBOX	(1 << 0)
+#define EC_FLAG_MAILBOX		(1 << 1)
+
+#define EC_MAX_DEVICE_ID_NUM	0xFF
+#define EC_MAX_ITEM_NUM		32
+
+struct ec_table {
+	u8 devid2itemnum[EC_MAX_DEVICE_ID_NUM];
+	u8 pinnum[EC_MAX_ITEM_NUM];
+	u8 devid[EC_MAX_ITEM_NUM];
+};
+
+struct ec_version {
+	u16 kernel_ver,
+	    chip_code,
+	    prj_id,
+	    prj_ver;
+};
+
+#define EC_MAX_LEN_PROJECT_NAME	8
+
+struct imanager2 {
+	u16 id;
+	u32 flag;
+	struct mutex lock;				/* protects io */
+	char prj_name[EC_MAX_LEN_PROJECT_NAME + 1];	/* strlen + '\0' */
+	struct ec_version version;
+	struct ec_table table;
+};
+
+/*
+ * Definition
+ */
+#define EC_TABLE_ITEM_UNUSED	0xFF
+#define EC_TABLE_DID_NODEV	0x00
+#define EC_TABLE_HWP_NODEV	0xFF
+#define EC_TABLE_NOITEM		0xFF
+
+#define EC_ERROR		0xFF
+
+#define EC_RAM_BANK_SIZE	32	/* 32 bytes size for each bank. */
+#define EC_RAM_BUFFER_SIZE	256	/* 32 bytes * 8 banks = 256 bytes */
+
+#define EC_SIO_CMD		0x29C
+#define EC_SIO_DATA		0x29D
+
+/* Access Mailbox */
+#define EC_IO_PORT_CMD		0x29A
+#define EC_IO_PORT_DATA		0x299
+
+#define EC_IO_CMD_READ_OFFSET	0xA0
+#define EC_IO_CMD_WRITE_OFFSET	0x50
+
+#define EC_ITE_PORT_OFS		0x29E
+#define EC_ITE_PORT_DATA	0x29F
+
+/*
+ * CMD - IO
+ */
+/* ADC */
+#define EC_CMD_ADC_INDEX				0x15
+#define EC_CMD_ADC_READ_LSB				0x16
+#define EC_CMD_ADC_READ_MSB				0x1F
+/* HW Control Table */
+#define EC_CMD_HWCTRLTABLE_INDEX			0x20
+#define EC_CMD_HWCTRLTABLE_GET_PIN_NUM			0x21
+#define EC_CMD_HWCTRLTABLE_GET_DEVICE_ID		0x22
+#define EC_CMD_HWCTRLTABLE_GET_PIN_ACTIVE_POLARITY	0x23
+/* ACPI RAM */
+#define EC_CMD_ACPIRAM_READ				0x80
+#define EC_CMD_ACPIRAM_WRITE				0x81
+/* Extend RAM */
+#define EC_CMD_EXTRAM_READ				0x86
+#define EC_CMD_EXTRAM_WRITE				0x87
+/* HW RAM */
+#define EC_CMD_HWRAM_READ				0x88
+#define EC_CMD_HWRAM_WRITE				0x89
+
+/*
+ * ACPI RAM Address Table
+ */
+/* n = 1 ~ 2 */
+#define EC_ACPIRAM_ADDR_TEMPERATURE_BASE(n)	(0x60 + 3 * ((n) - 1))
+#define	EC_ACPIRAM_ADDR_LOCAL_TEMPERATURE(n) \
+	EC_ACPIRAM_ADDR_TEMPERATURE_BASE(n)
+#define	EC_ACPIRAM_ADDR_REMOTE_TEMPERATURE(n) \
+	(EC_ACPIRAM_ADDR_TEMPERATURE_BASE(n) + 1)
+#define	EC_ACPIRAM_ADDR_WARNING_TEMPERATURE(n)\
+	(EC_ACPIRAM_ADDR_TEMPERATURE_BASE(n) + 2)
+
+/* N = 0 ~ 2 */
+#define EC_ACPIRAM_ADDR_FAN_SPEED_BASE(N)	(0x70 + 2 * (N))
+
+#define EC_ACPIRAM_ADDR_KERNEL_MAJOR_VERSION	0xF8
+#define EC_ACPIRAM_ADDR_CHIP_VENDOR_CODE	0xFA
+#define EC_ACPIRAM_ADDR_PROJECT_NAME_CODE	0xFC
+#define EC_ACPIRAM_ADDR_FIRMWARE_MAJOR_VERSION	0xFE
+
+/*
+ * HW RAM Address Table
+ */
+/* Thermal Source Control RAM 0xB0-0xC7 (N: 0 ~ 3) */
+#define EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N)	(0xB0 + 6 * (N))
+#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_CHANNEL(N) \
+	EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N)
+#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_ADDR(N) \
+	(EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 1)
+#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_CMD(N)	 \
+	(EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 2)
+#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_STATUS(N) \
+	(EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 3)
+#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_FAN_CODE(N) \
+	(EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 4)
+#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_TEMPERATURE(N) \
+	(EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 5)
+/* Fan Control 0xD0-0xEF (N: 0 ~ 3) */
+#define EC_HWRAM_ADDR_FAN_BASE_ADDR(N)	(0xD0 + 0x10 * (N))
+#define EC_HWRAM_ADDR_FAN_CODE(N)	EC_HWRAM_ADDR_FAN_BASE_ADDR(N)
+#define EC_HWRAM_ADDR_FAN_STATUS(N)	(EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 1)
+#define EC_HWRAM_ADDR_FAN_CONTROL(N)	(EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 2)
+#define EC_HWRAM_ADDR_FAN_TEMP_HI(N)	(EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 3)
+#define EC_HWRAM_ADDR_FAN_TEMP_LO(N)	(EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 4)
+#define EC_HWRAM_ADDR_FAN_TEMP_LOSTOP(N) \
+					(EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 5)
+#define EC_HWRAM_ADDR_FAN_PWM_HI(N)	(EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 6)
+#define EC_HWRAM_ADDR_FAN_PWM_LO(N)	(EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 7)
+
+/*
+ * OFS - Mailbox
+ */
+/* Mailbox Structure */
+#define EC_MAILBOX_OFFSET_CMD		0x00
+#define EC_MAILBOX_OFFSET_STATUS	0x01
+#define EC_MAILBOX_OFFSET_PARA		0x02
+#define EC_MAILBOX_OFFSET_DAT(N)	(0x03 + (N))	/* N = 0x00 ~ 0x2C */
+
+/*
+ * CMD - Mailbox
+ */
+/* GPIO */
+#define EC_CMD_MAILBOX_READ_HW_PIN				0x11
+#define EC_CMD_MAILBOX_WRITE_HW_PIN				0x12
+/* Storage */
+#define EC_CMD_MAILBOX_READ_EC_RAM				0x1E
+#define EC_CMD_MAILBOX_WRITE_EC_RAM				0x1F
+/* OTHERS */
+#define EC_CMD_MAILBOX_READ_DYNAMIC_TABLE			0x20
+/* Thermal Protect */
+#define EC_CMD_MAILBOX_READ_THERMAL_SOURCE			0x42
+#define EC_CMD_MAILBOX_WRITE_THERMAL_SOURCE			0x43
+/* Storage */
+#define EC_CMD_MALLBOX_CLEAR_256_BYTES_BUFFER			0xC0
+#define EC_CMD_MALLBOX_READ_256_BYTES_BUFFER			0xC1
+#define EC_CMD_MALLBOX_WRITE_256_BYTES_BUFFER			0xC2
+#define EC_CMD_MALLBOX_READ_EEPROM_DATA_FROM_256_BYTES_BUFFER	0xC3
+#define EC_CMD_MALLBOX_WRITE_256_BYTES_BUFFER_INTO_EEPROM_DATA	0xC4
+/* General Mailbox Command */
+#define EC_CMD_MAILBOX_GET_FIRMWARE_VERSION_AND_PROJECT_NAME	0xF0
+#define EC_CMD_MAILBOX_CLEAR_ALL				0xFF
+
+/*
+ * Status - Mailbox
+ */
+#define EC_MAILBOX_STATUS_FAIL		0x00
+#define EC_MAILBOX_STATUS_SUCCESS	0x01
+
+/*
+ * PARA - Mailbox
+ */
+/* RAM Type */
+#define EC_RAM_BANK_ACPI	0x01
+#define EC_RAM_BANK_HW		0x02
+#define EC_RAM_BANK_EXT		0x03
+#define EC_RAM_BANK_BUFFER	0x06
+/* Dynamic Type */
+#define EC_DYNAMIC_DEVICE_ID	0x00
+#define EC_DYNAMIC_HW_PIN	0x01
+#define EC_DYNAMIC_POLARITY	0x02
+
+/*
+ * Functions - Mailbox
+ */
+/* command = 0x20 */
+int imanager2_mbox_get_dynamic_table(u32 ecflag, u8 type, u8 *table);
+/* command = 0x42 */
+int imanager2_mbox_read_thermalzone(u32 ecflag, u8 zone, u8 *smbid, u8 *fanid,
+				    u8 *buf, int *len);
+/* command = 0xC0 */
+int imanager2_mbox_clear_ram(u32 ecflag);
+/* command = 0xC1 */
+int imanager2_mbox_read_ram_across_banks(u32 ecflag, u8 *data, int len);
+/* command = 0x1E */
+int imanager2_mbox_read_ram(u32 ecflag, u8 bank, u8 offset, u8 *buf, u8 len);
+/* command = 0x1F */
+int imanager2_mbox_write_ram(u32 ecflag, u8 bank, u8 offset, u8 *buf, u8 len);
+/* command = 0xF0 */
+int imanager2_mbox_get_project_information(u32 ecflag, u8 *prj_name,
+					   u16 *kernel_ver, u16 *chip_code,
+					   u16 *prj_id, u16 *prj_ver);
+
+/*
+ * Functions - basic
+ */
+#define IO_FLAG_OBF	(1 << 0)	/* output buffer full */
+#define IO_FLAG_IBF	(1 << 1)	/* input buffer full  */
+int ec_inb_after_obf1(u8 *data);
+int ec_outb_after_ibc0(u16 port, u8 data);
+/* ITE mailbox access */
+int imanager2_mbox_read_data(u32 ecflag, u8 cmd, u8 para, u8 *data, int len);
+int imanager2_mbox_write_data(u32 ecflag, u8 cmd, u8 para, u8 *data, int len);
+/* only IO access */
+int imanager2_mbox_io_read(u8 command, u8 offset, u8 *buf, u8 len);
+int imanager2_mbox_io_write(u8 command, u8 offset, u8 *buf, u8 len);
+int imanager2_mbox_io_simple_read(u8 command, u8 *value);
+/* ITE Mailbox & IO access */
+int imanager2_mbox_read_ram_support_io(u32 ecflag, u8 bank, u8 addr, u8 *buf,
+				       u8 len);
+
+/*
+ * Device ID
+ */
+enum ec_device_id {
+	/* GPIO */
+	altgpio0 = 0x10,	/* 0x10 */
+	altgpio1,
+	altgpio2,
+	altgpio3,
+	altgpio4,
+	altgpio5,
+	altgpio6,
+	altgpio7,
+	/* GPIO - Button */
+	btn0,
+	btn1,
+	btn2,
+	btn3,
+	btn4,
+	btn5,
+	btn6,
+	btn7,
+	/* PWM - Fan */
+	cpufan_2p,		/* 0x20 */
+	cpufan_4p,
+	sysfan1_2p,
+	sysfan1_4p,
+	sysfan2_2p,
+	sysfan2_4p,
+	/* PWM - Brightness Control */
+	pwmbrightness,
+	/* PWM - System Speaker */
+	pwmbeep,
+	/* SMBus */
+	smboem0,
+	smboem1,
+	smboem2,
+	smbeeprom,
+	smbthermal0,
+	smbthermal1,
+	smbsecurityeep,
+	i2coem,
+	/* DAC - Speaker */
+	dacspeaker,		/* 0x30 */
+	/* SMBus */
+	smbeep2k = 0x38,
+	oemeep,
+	oemeep2k,
+	peci,
+	smboem3,
+	smblink,
+	smbslv,
+	/* GPIO - LED */
+	powerled = 0x40,	/* 0x40 */
+	batledg,
+	oemled0,
+	oemled1,
+	oemled2,
+	batledr,
+	/* SMBus - Smart Battery */
+	smartbat1 = 0x48,
+	smartbat2,
+	/* ADC */
+	adcmosbat = 0x50,	/* 0x50 */
+	adcmosbatx2,
+	adcmosbatx10,
+	adcbat,
+	adcbatx2,
+	adcbatx10,
+	adc5vs0,
+	adc5vs0x2,
+	adc5vs0x10,
+	adv5vs5,
+	adv5vs5x2,
+	adv5vs5x10,
+	adc33vs0,
+	adc33vs0x2,
+	adc33vs0x10,
+	adc33vs5,
+	adc33vs5x2,		/* 0x60 */
+	adc33vs5x10,
+	adv12vs0,
+	adv12vs0x2,
+	adv12vs0x10,
+	adcvcorea,
+	adcvcoreax2,
+	adcvcoreax10,
+	adcvcoreb,
+	adcvcorebx2,
+	adcvcorebx10,
+	adcdc,
+	adcdcx2,
+	adcdcx10,
+	adcdcstby,
+	adcdcstbyx2,
+	adcdcstbyx10,		/* 0x70 */
+	adcdcother,
+	adcdcotherx2,
+	adcdcotherx10,
+	adccurrent,
+	/* IRQ - Watchdog */
+	wdirq = 0x78,
+	/* GPIO - Watchdog */
+	wdnmi,
+	/* Tacho - Fan */
+	tacho0 = 0x80,		/* 0x80 */
+	tacho1,
+	tacho2,
+	/* PWM - Brightness Control */
+	pwmbrightness2 = 0x88,
+	/* GPIO - Backlight Control */
+	brionoff1,
+	brionoff2,
+};
+
+#endif /* __IMANAGER2_EC_H__ */
-- 
1.9.1


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

* [PATCH 2/4] mfd: imanager2: Add Advantech EC APIs support for IT8516/18/28
  2014-07-14 12:54 [PATCH 1/4] mfd: imanager2: Add defines support for IT8516/18/28 Wei-Chun Pan
@ 2014-07-14 12:54 ` Wei-Chun Pan
  2014-07-22  8:36   ` Lee Jones
  2014-07-14 12:54 ` [PATCH 3/4] mfd: imanager2: Add Core supports " Wei-Chun Pan
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ 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/mfd/imanager2_ec.c | 615 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 615 insertions(+)
 create mode 100644 drivers/mfd/imanager2_ec.c

diff --git a/drivers/mfd/imanager2_ec.c b/drivers/mfd/imanager2_ec.c
new file mode 100644
index 0000000..f7a0003
--- /dev/null
+++ b/drivers/mfd/imanager2_ec.c
@@ -0,0 +1,615 @@
+/*
+ * imanager2_ec.c - MFD accessing driver of 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/io.h>
+#include <linux/delay.h>
+#include <linux/mfd/imanager2_ec.h>
+
+#define EC_UDELAY_TIME		50
+#define EC_MAX_TIMEOUT_COUNT	1000
+
+static int ec_wait_obf1(void)
+{
+	int i = EC_MAX_TIMEOUT_COUNT;
+	while (i--) {
+		if (inb(EC_IO_PORT_CMD) & IO_FLAG_OBF)
+			return 0;
+
+		udelay(EC_UDELAY_TIME);
+	};
+
+	return -EBUSY;
+}
+
+int ec_inb_after_obf1(u8 *data)
+{
+	int ret = ec_wait_obf1();
+	if (ret)
+		return ret;
+	*data = inb(EC_IO_PORT_DATA);
+	return 0;
+}
+EXPORT_SYMBOL(ec_inb_after_obf1);
+
+static int ec_wait_ibc0(void)
+{
+	int i = EC_MAX_TIMEOUT_COUNT;
+	while (i--) {
+		if (!(inb(EC_IO_PORT_CMD) & IO_FLAG_IBF))
+			return 0;
+
+		udelay(EC_UDELAY_TIME);
+	};
+
+	return -EBUSY;
+}
+
+int ec_outb_after_ibc0(u16 port, u8 data)
+{
+	int ret = ec_wait_ibc0();
+	if (ret)
+		return ret;
+	outb(data, port);
+	return 0;
+}
+EXPORT_SYMBOL(ec_outb_after_ibc0);
+
+static int imanager2_read_mailbox(u32 ecflag, u8 offset, u8 *data)
+{
+	if (ecflag & EC_FLAG_IO_MAILBOX) {
+		int ret = ec_wait_ibc0();
+		if (ret)
+			return ret;
+		inb(EC_IO_PORT_DATA);
+		outb(offset + EC_IO_CMD_READ_OFFSET, EC_IO_PORT_CMD);
+
+		return ec_inb_after_obf1(data);
+	} else {
+		outb(offset, EC_ITE_PORT_OFS);
+		*data = inb(EC_ITE_PORT_DATA);
+	}
+
+	return 0;
+}
+
+static int imanager2_write_mailbox(u32 ecflag, u8 offset, u8 data)
+{
+	if (ecflag & EC_FLAG_IO_MAILBOX) {
+		int ret = ec_outb_after_ibc0(EC_IO_PORT_CMD,
+					     offset + EC_IO_CMD_WRITE_OFFSET);
+		if (ret)
+			return ret;
+
+		return ec_outb_after_ibc0(EC_IO_PORT_DATA, data);
+	} else {
+		outb(offset, EC_ITE_PORT_OFS);
+		outb(data, EC_ITE_PORT_DATA);
+	}
+
+	return 0;
+}
+
+static int imanager2_wait_mailbox_command0(u32 ecflag)
+{
+	u8 cmd;
+	int i, ret;
+
+	for (i = 0; i < EC_MAX_TIMEOUT_COUNT; i++) {
+		ret = imanager2_read_mailbox(ecflag, EC_MAILBOX_OFFSET_CMD,
+					     &cmd);
+		if (ret)
+			return ret;
+		if (!cmd)
+			return 0;
+
+		udelay(EC_UDELAY_TIME);
+	}
+
+	return -EBUSY;
+}
+
+int imanager2_mbox_read_data(u32 ecflag, u8 cmd, u8 para, u8 *data, int len)
+{
+	int ret, i;
+	u8 status;
+
+	ret = imanager2_wait_mailbox_command0(ecflag);
+	if (ret)
+		return ret;
+
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_PARA, para);
+	if (ret)
+		return ret;
+
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_CMD, cmd);
+	if (ret)
+		return ret;
+
+	ret = imanager2_wait_mailbox_command0(ecflag);
+	if (ret)
+		return ret;
+
+	ret = imanager2_read_mailbox(ecflag, EC_MAILBOX_OFFSET_STATUS, &status);
+	if (ret)
+		return ret;
+	if (status != EC_MAILBOX_STATUS_SUCCESS)
+		return -ENXIO;
+
+	for (i = 0; i < len; i++) {
+		ret = imanager2_read_mailbox(ecflag, EC_MAILBOX_OFFSET_DAT(i),
+					     &data[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(imanager2_mbox_read_data);
+
+int imanager2_mbox_write_data(u32 ecflag, u8 cmd, u8 para, u8 *data, int len)
+{
+	int ret, i;
+	u8 status;
+
+	ret = imanager2_wait_mailbox_command0(ecflag);
+	if (ret)
+		return ret;
+
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_PARA, para);
+	for (i = 0; i < len; i++) {
+		ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_DAT(i),
+					      data[i]);
+		if (ret)
+			return ret;
+	}
+
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_CMD, cmd);
+	if (ret)
+		return ret;
+
+	ret = imanager2_wait_mailbox_command0(ecflag);
+	if (ret)
+		return ret;
+
+	ret = imanager2_read_mailbox(ecflag, EC_MAILBOX_OFFSET_STATUS, &status);
+	if (ret)
+		return ret;
+	if (status != EC_MAILBOX_STATUS_SUCCESS)
+		return -ENXIO;
+
+	return 0;
+}
+EXPORT_SYMBOL(imanager2_mbox_write_data);
+
+int imanager2_mbox_read_ram(u32 ecflag, u8 bank, u8 offset, u8 *buf, u8 len)
+{
+	int i, ret;
+	u8 status;
+
+	if (len && !buf)
+		return -EINVAL;
+
+	if (len > 0x2B)
+		return -EINVAL;	/* range: DATA01~DATA2B */
+
+	ret = imanager2_wait_mailbox_command0(ecflag);
+	if (ret)
+		return ret;
+
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_PARA, bank);
+	if (ret)
+		return ret;
+
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_DAT(0x00),
+				      offset);
+	if (ret)
+		return ret;
+
+	if (len > 0x2B)
+		len = 0x2B;	/* range: DATA01~DATA2B */
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_DAT(0x2C), len);
+	if (ret)
+		return ret;
+
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_CMD,
+				      EC_CMD_MAILBOX_READ_EC_RAM);
+	if (ret)
+		return ret;
+
+	ret = imanager2_wait_mailbox_command0(ecflag);
+	if (ret)
+		return ret;
+
+	ret = imanager2_read_mailbox(ecflag, EC_MAILBOX_OFFSET_STATUS, &status);
+	if (ret)
+		return ret;
+	if (status != EC_MAILBOX_STATUS_SUCCESS)
+		return -ENXIO;
+
+	for (i = 0; i < len; i++) {
+		ret = imanager2_read_mailbox(ecflag,
+					     EC_MAILBOX_OFFSET_DAT(1 + i),
+					     &buf[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(imanager2_mbox_read_ram);
+
+int imanager2_mbox_write_ram(u32 ecflag, u8 bank, u8 offset, u8 *buf, u8 len)
+{
+	int i, ret;
+	u8 status;
+
+	if (len && !buf)
+		return -EINVAL;
+
+	if (len > 0x2B)
+		return -EINVAL;	/* range: DATA01~DATA2B */
+
+	ret = imanager2_wait_mailbox_command0(ecflag);
+	if (ret)
+		return ret;
+
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_PARA, bank);
+	if (ret)
+		return ret;
+
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_DAT(0x00),
+				      offset);
+	if (ret)
+		return ret;
+
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_DAT(0x2C), len);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < len; i++) {
+		ret = imanager2_write_mailbox(ecflag,
+					      EC_MAILBOX_OFFSET_DAT(1 + i),
+					      buf[i]);
+		if (ret)
+			return ret;
+	}
+
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_CMD,
+				      EC_CMD_MAILBOX_WRITE_EC_RAM);
+	if (ret)
+		return ret;
+
+	ret = imanager2_wait_mailbox_command0(ecflag);
+	if (ret)
+		return ret;
+
+	ret = imanager2_read_mailbox(ecflag, EC_MAILBOX_OFFSET_STATUS, &status);
+	if (ret)
+		return ret;
+	if (status != EC_MAILBOX_STATUS_SUCCESS)
+		return -ENXIO;
+
+	return 0;
+}
+EXPORT_SYMBOL(imanager2_mbox_write_ram);
+
+int imanager2_mbox_get_dynamic_table(u32 ecflag, u8 type, u8 *table)
+{
+	int i, ret;
+	u8 status;
+
+	if (!table)
+		return -EINVAL;
+
+	ret = imanager2_wait_mailbox_command0(ecflag);
+	if (ret)
+		return ret;
+
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_PARA, type);
+	if (ret)
+		return ret;
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_CMD,
+				      EC_CMD_MAILBOX_READ_DYNAMIC_TABLE);
+	if (ret)
+		return ret;
+
+	ret = imanager2_wait_mailbox_command0(ecflag);
+	if (ret)
+		return ret;
+
+	ret = imanager2_read_mailbox(ecflag, EC_MAILBOX_OFFSET_STATUS, &status);
+	if (ret)
+		return ret;
+	if (status != EC_MAILBOX_STATUS_SUCCESS)
+		return -ENXIO;
+
+	/* table size must be EC_MAX_ITEM_NUM (32 bytes) */
+	for (i = 0; i < EC_MAX_ITEM_NUM; i++) {
+		ret = imanager2_read_mailbox(ecflag, EC_MAILBOX_OFFSET_DAT(i),
+					     &table[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(imanager2_mbox_get_dynamic_table);
+
+int imanager2_mbox_read_thermalzone(u32 ecflag, u8 zone, u8 *smbid, u8 *fanid,
+				    u8 *buf, int *len)
+{
+	int ret, i;
+	u8 status, getlength;
+
+	if (!smbid && !fanid && !len)
+		return -EINVAL;
+
+	if (*len && !buf)
+		return -EINVAL;
+
+	ret = imanager2_wait_mailbox_command0(ecflag);
+	if (ret)
+		return ret;
+
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_PARA, zone);
+	if (ret)
+		return ret;
+	ret = imanager2_write_mailbox(ecflag, EC_MAILBOX_OFFSET_CMD,
+				      EC_CMD_MAILBOX_READ_THERMAL_SOURCE);
+	if (ret)
+		return ret;
+
+	ret = imanager2_wait_mailbox_command0(ecflag);
+	if (ret)
+		return ret;
+
+	ret = imanager2_read_mailbox(ecflag, EC_MAILBOX_OFFSET_STATUS, &status);
+	if (ret)
+		return ret;
+	if (status != EC_MAILBOX_STATUS_SUCCESS)
+		return -ENXIO;
+
+	if (smbid) {
+		ret = imanager2_read_mailbox(ecflag,
+					     EC_MAILBOX_OFFSET_DAT(0x00),
+					     smbid);
+		if (ret)
+			return ret;
+	}
+
+	if (fanid) {
+		ret = imanager2_read_mailbox(ecflag,
+					     EC_MAILBOX_OFFSET_DAT(0x01),
+					     fanid);
+		if (ret)
+			return ret;
+	}
+
+	if (!len)
+		return 0;
+
+	ret = imanager2_read_mailbox(ecflag, EC_MAILBOX_OFFSET_DAT(0x2C),
+				     &getlength);
+	if (ret)
+		return ret;
+
+	if (*len > getlength)
+		*len = getlength;
+
+	for (i = 0; i < *len; i++) {
+		ret = imanager2_read_mailbox(ecflag,
+					     EC_MAILBOX_OFFSET_DAT(0x02 + i),
+					     &buf[i]);
+		if (ret)
+			return ret;
+	}
+
+	if (*len < getlength) {
+		*len = getlength;
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(imanager2_mbox_read_thermalzone);
+
+int imanager2_mbox_get_project_information(u32 ecflag, u8 *prj_name,
+					   u16 *kernel_ver, u16 *chip_code,
+					   u16 *prj_id, u16 *prj_ver)
+{
+	int ret, i;
+
+	if (!prj_name && !kernel_ver && !chip_code && !prj_id && !prj_ver)
+		return -EINVAL;
+
+	ret = imanager2_wait_mailbox_command0(ecflag);
+	if (ret)
+		return ret;
+
+	ret = imanager2_write_mailbox(
+		ecflag, EC_MAILBOX_OFFSET_CMD,
+		EC_CMD_MAILBOX_GET_FIRMWARE_VERSION_AND_PROJECT_NAME);
+
+	ret = imanager2_wait_mailbox_command0(ecflag);
+	if (ret)
+		return ret;
+
+	if (prj_name) {
+		/* projection name length must be 9 bytes */
+		for (i = 0; i < EC_MAX_LEN_PROJECT_NAME; i++) {
+			ret = imanager2_read_mailbox(ecflag,
+						     EC_MAILBOX_OFFSET_DAT(i),
+						     &prj_name[i]);
+			if (ret)
+				return ret;
+		}
+		prj_name[EC_MAX_LEN_PROJECT_NAME] = '\0';
+	}
+
+	if (kernel_ver)
+		for (i = 0; i < sizeof(*kernel_ver); i++) {
+			ret = imanager2_read_mailbox(
+				ecflag, EC_MAILBOX_OFFSET_DAT(0x09 + i),
+				(u8 *)kernel_ver + i);
+			if (ret)
+				return ret;
+		}
+
+	if (chip_code)
+		for (i = 0; i < sizeof(*chip_code); i++) {
+			ret = imanager2_read_mailbox(
+				ecflag, EC_MAILBOX_OFFSET_DAT(0x0B + i),
+				(u8 *)chip_code + i);
+			if (ret)
+				return ret;
+		}
+
+	if (prj_id)
+		for (i = 0; i < sizeof(*prj_id); i++) {
+			ret = imanager2_read_mailbox(
+				ecflag, EC_MAILBOX_OFFSET_DAT(0x0D + i),
+				(u8 *)prj_id + i);
+			if (ret)
+				return ret;
+		}
+
+	if (prj_ver)
+		for (i = 0; i < sizeof(*prj_ver); i++) {
+			ret = imanager2_read_mailbox(
+				ecflag, EC_MAILBOX_OFFSET_DAT(0x0F + i),
+				(u8 *)prj_ver + i);
+			if (ret)
+				return ret;
+		}
+
+	return 0;
+}
+EXPORT_SYMBOL(imanager2_mbox_get_project_information);
+
+/* IO chennel access */
+int imanager2_mbox_io_read(u8 command, u8 offset, u8 *buf, u8 len)
+{
+	int ret, i;
+
+	if (!len)
+		return 0;
+
+	if (!buf)
+		return -EINVAL;
+
+	for (i = 0; i < len; i++) {
+		ret = ec_outb_after_ibc0(EC_IO_PORT_CMD, command);
+		if (ret)
+			return ret;
+
+		ret = ec_outb_after_ibc0(EC_IO_PORT_DATA, offset + i);
+		if (ret)
+			return ret;
+
+		ret = ec_inb_after_obf1(&buf[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(imanager2_mbox_io_read);
+
+int imanager2_mbox_io_write(u8 command, u8 offset, u8 *buf, u8 len)
+{
+	int ret, i;
+
+	if (!buf && len)
+		return -EINVAL;
+
+	for (i = 0; i < len; i++) {
+		ret = ec_outb_after_ibc0(EC_IO_PORT_CMD, command);
+		if (ret)
+			return ret;
+
+		ret = ec_outb_after_ibc0(EC_IO_PORT_DATA, offset + i);
+		if (ret)
+			return ret;
+
+		ret = ec_outb_after_ibc0(EC_IO_PORT_DATA, buf[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(imanager2_mbox_io_write);
+
+int imanager2_mbox_io_simple_read(u8 command, u8 *value)
+{
+	int ret;
+
+	if (!value)
+		return -EINVAL;
+
+	ret = ec_outb_after_ibc0(EC_IO_PORT_CMD, command);
+	if (ret)
+		return ret;
+
+	return ec_inb_after_obf1(value);
+}
+EXPORT_SYMBOL(imanager2_mbox_io_simple_read);
+
+/* ITE Mailbox & IO chennel access*/
+int imanager2_mbox_read_ram_support_io(u32 ecflag, u8 bank, u8 addr, u8 *buf,
+				       u8 len)
+{
+	if (ecflag & EC_FLAG_MAILBOX) {
+		return imanager2_mbox_read_ram(ecflag, bank, addr, buf, len);
+	} else {
+		u8 iocmd;
+
+		if (bank == EC_RAM_BANK_ACPI)
+			iocmd = EC_CMD_ACPIRAM_READ;
+		else if (bank == EC_RAM_BANK_HW)
+			iocmd = EC_CMD_HWRAM_READ;
+		else if (bank == EC_RAM_BANK_EXT)
+			iocmd = EC_CMD_EXTRAM_READ;
+		else
+			return -EINVAL;
+
+		return imanager2_mbox_io_read(iocmd, addr, buf, len);
+	}
+}
+EXPORT_SYMBOL(imanager2_mbox_read_ram_support_io);
+
+int imanager2_mbox_write_ram_support_io(u32 ecflag, u8 bank, u8 addr, u8 *buf,
+					u8 len)
+{
+	u8 iocmd;
+
+	if (ecflag & EC_FLAG_MAILBOX)
+		return imanager2_mbox_write_ram(ecflag, bank, addr, buf, len);
+
+	if (bank == EC_RAM_BANK_ACPI)
+		iocmd = EC_CMD_ACPIRAM_WRITE;
+	else if (bank == EC_RAM_BANK_HW)
+		iocmd = EC_CMD_HWRAM_WRITE;
+	else if (bank == EC_RAM_BANK_EXT)
+		iocmd = EC_CMD_EXTRAM_WRITE;
+	else
+		return -EINVAL;
+
+	return imanager2_mbox_io_write(iocmd, addr, buf, len);
+}
+EXPORT_SYMBOL(imanager2_mbox_write_ram_support_io);
-- 
1.9.1


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

* [PATCH 3/4] mfd: imanager2: Add Core supports for IT8516/18/28
  2014-07-14 12:54 [PATCH 1/4] mfd: imanager2: Add defines support for IT8516/18/28 Wei-Chun Pan
  2014-07-14 12:54 ` [PATCH 2/4] mfd: imanager2: Add Advantech EC APIs " Wei-Chun Pan
@ 2014-07-14 12:54 ` Wei-Chun Pan
  2014-07-22  8:56   ` Lee Jones
  2014-07-14 12:54 ` [PATCH 4/4] hwmon: (imanager2) Add support " Wei-Chun Pan
  2014-07-22  7:58 ` [PATCH 1/4] mfd: imanager2: Add defines " Lee Jones
  3 siblings, 1 reply; 8+ 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/mfd/Kconfig          |   6 +
 drivers/mfd/Makefile         |   2 +
 drivers/mfd/imanager2_core.c | 303 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 311 insertions(+)
 create mode 100644 drivers/mfd/imanager2_core.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 3383412..48b063f 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -10,6 +10,12 @@ config MFD_CORE
 	select IRQ_DOMAIN
 	default n
 
+config MFD_IMANAGER2
+	tristate "Support for Advantech iManager2 EC ICs"
+	select MFD_CORE
+	help
+	  Support for Advantech iManager2 EC ICs
+
 config MFD_CS5535
 	tristate "AMD CS5535 and CS5536 southbridge core functions"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 2851275..10c64ae 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -166,3 +166,5 @@ obj-$(CONFIG_MFD_RETU)		+= retu-mfd.o
 obj-$(CONFIG_MFD_AS3711)	+= as3711.o
 obj-$(CONFIG_MFD_AS3722)	+= as3722.o
 obj-$(CONFIG_MFD_STW481X)	+= stw481x.o
+imanager2-objs			:= imanager2_core.o imanager2_ec.o
+obj-$(CONFIG_MFD_IMANAGER2)	+= imanager2.o
diff --git a/drivers/mfd/imanager2_core.c b/drivers/mfd/imanager2_core.c
new file mode 100644
index 0000000..2264d29
--- /dev/null
+++ b/drivers/mfd/imanager2_core.c
@@ -0,0 +1,303 @@
+/*
+ * imanager2_core.c - MFD core driver of 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/mfd/core.h>
+#include <linux/mfd/imanager2_ec.h>
+
+#define DRV_NAME	"imanager2"
+#define DRV_VERSION	"4.0.1"
+
+static struct mfd_cell imanager2_cells[] = {
+	{
+		.name = "imanager2_hwm",
+	},
+	{
+		.name = "imanager2_i2c",
+	},
+};
+
+enum chips {
+	it8516 = 0x8516,
+	it8518 = 0x8518,
+	it8528 = 0x8528,
+};
+
+#define EC_CMD_AUTHENTICATION	0x30
+
+static int imanager2_authentication(struct imanager2 *ec)
+{
+	u8 tmp;
+	int ret;
+
+	mutex_lock(&ec->lock);
+
+	if (inb(EC_IO_PORT_CMD) == 0xFF && inb(EC_IO_PORT_DATA) == 0xFF) {
+		ret = -ENODEV;
+		goto unlock;
+	}
+
+	if (inb(EC_IO_PORT_CMD) & IO_FLAG_OBF)
+		inb(EC_IO_PORT_DATA);	/* initial OBF */
+
+	if (ec_outb_after_ibc0(EC_IO_PORT_CMD, EC_CMD_AUTHENTICATION)) {
+		ret = -ENODEV;
+		goto unlock;
+	}
+
+	ret = ec_inb_after_obf1(&tmp);
+
+unlock:
+	mutex_unlock(&ec->lock);
+
+	if (ret)
+		return ret;
+
+	if (tmp != 0x95)
+		return -ENODEV;
+
+	return 0;
+}
+
+#define EC_ITE_CHIPID_H8	0x20
+#define EC_ITE_CHIPID_L8	0x21
+
+static int imanager2_get_chip_type(struct imanager2 *ec)
+{
+	mutex_lock(&ec->lock);
+
+	outb(EC_ITE_CHIPID_H8, EC_SIO_CMD);
+	ec->id = inb(EC_SIO_DATA) << 8;
+	outb(EC_ITE_CHIPID_L8, EC_SIO_CMD);
+	ec->id |= inb(EC_SIO_DATA);
+
+	mutex_unlock(&ec->lock);
+
+	switch (ec->id) {
+	case it8516:
+	case it8518:
+		ec->flag = EC_FLAG_IO;
+		break;
+	case it8528:
+		ec->flag |= EC_FLAG_IO_MAILBOX;
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+/*
+ * EC provides IO channel and ITE mailbox ways to access mailbox. IO channel is
+ * a common way to access mailbox, but IET mailbox way is much faster than IO
+ * channel. We prefer ITE mailbox if firmware supports. Source kernel code
+ * X11_05 the first firmware version that supports ITE mailbox.
+ */
+#define EC_CHIPFW_SUPP_ITEMAILBOX	0x1105
+
+static int imanager2_get_info(struct imanager2 *ec)
+{
+	int ret;
+	u8 *tmp = (u8 *)&ec->version.kernel_ver;
+
+	mutex_lock(&ec->lock);
+
+	ret = imanager2_mbox_io_read(EC_CMD_ACPIRAM_READ,
+				     EC_ACPIRAM_ADDR_KERNEL_MAJOR_VERSION,
+				     &tmp[0], 2);
+
+	if (ret)
+		goto unlock;
+
+	if (ec->version.kernel_ver >= EC_CHIPFW_SUPP_ITEMAILBOX) {
+		ec->flag |= EC_FLAG_MAILBOX;
+		ret = imanager2_mbox_get_project_information(
+			ec->flag, ec->prj_name,
+			&ec->version.kernel_ver, &ec->version.chip_code,
+			&ec->version.prj_id, &ec->version.prj_ver);
+	} else {
+		ec->flag &= ~EC_FLAG_MAILBOX;
+		ret = imanager2_mbox_io_read(
+			EC_CMD_ACPIRAM_READ,
+			EC_ACPIRAM_ADDR_KERNEL_MAJOR_VERSION,
+			&tmp[0], sizeof(struct ec_version));
+	}
+unlock:
+	mutex_unlock(&ec->lock);
+
+	return ret;
+}
+
+static int imanager2_device_initial_by_ite(struct imanager2 *ec)
+{
+	int i, ret;
+
+	mutex_lock(&ec->lock);
+
+	ret = imanager2_mbox_get_dynamic_table(ec->flag, EC_DYNAMIC_DEVICE_ID,
+					       ec->table.devid);
+	if (ret)
+		goto unlock;
+
+	ret = imanager2_mbox_get_dynamic_table(ec->flag, EC_DYNAMIC_HW_PIN,
+					       ec->table.pinnum);
+unlock:
+	mutex_unlock(&ec->lock);
+
+	if (ret)
+		return ret;
+
+	for (i = 0; i < EC_MAX_ITEM_NUM; i++) {
+		if (ec->table.devid[i] == EC_TABLE_DID_NODEV)
+			break;
+
+		ec->table.devid2itemnum[ec->table.devid[i]] = i;
+	}
+
+	return 0;
+}
+
+static int imanager2_device_initial_by_io(struct imanager2 *ec)
+{
+	int i, ret;
+	u8 tmp;
+
+	mutex_lock(&ec->lock);
+
+	for (i = 0; i < EC_MAX_ITEM_NUM; i++) {
+		ret = imanager2_mbox_io_read
+		      (EC_CMD_HWCTRLTABLE_INDEX, i, &tmp, 1);
+		if (ret)
+			break;
+		if (tmp == EC_TABLE_NOITEM)
+			break;
+
+		ret = imanager2_mbox_io_simple_read(
+			EC_CMD_HWCTRLTABLE_GET_PIN_NUM, &ec->table.pinnum[i]);
+		if (ret)
+			break;
+
+		ret = imanager2_mbox_io_simple_read(
+			EC_CMD_HWCTRLTABLE_GET_DEVICE_ID, &ec->table.devid[i]);
+		if (ret)
+			break;
+
+		if (ec->table.devid[i] == EC_TABLE_DID_NODEV)
+			continue;
+
+		ec->table.devid2itemnum[ec->table.devid[i]] = i;
+	}
+
+	mutex_unlock(&ec->lock);
+
+	if (ret)
+		return ret;
+
+	if (i < EC_MAX_ITEM_NUM) {
+		memset(&ec->table.devid[i], EC_TABLE_DID_NODEV,
+		       EC_MAX_ITEM_NUM - i);
+		memset(&ec->table.pinnum[i], EC_TABLE_HWP_NODEV,
+		       EC_MAX_ITEM_NUM - i);
+	}
+
+	return ret;
+}
+
+static int imanager2_build_device_table(struct imanager2 *ec)
+{
+	memset(&ec->table.devid2itemnum[0], EC_TABLE_ITEM_UNUSED,
+	       ARRAY_SIZE(ec->table.devid2itemnum));
+
+	if (ec->flag & EC_FLAG_MAILBOX)
+		return imanager2_device_initial_by_ite(ec);
+	else
+		return imanager2_device_initial_by_io(ec);
+}
+
+static struct platform_device *ec_pdev;
+
+static int __init imanager2_mfd_device_init(void)
+{
+	struct imanager2 ec = { 0 };
+	int ret;
+
+	mutex_init(&ec.lock);
+
+	ret = imanager2_authentication(&ec);
+	if (ret)
+		return ret;
+
+	ret = imanager2_get_chip_type(&ec);
+	if (ret)
+		return ret;
+
+	ret = imanager2_get_info(&ec);
+	if (ret)
+		return ret;
+
+	ret = imanager2_build_device_table(&ec);
+	if (ret)
+		return ret;
+
+	ec_pdev = platform_device_alloc(DRV_NAME, PLATFORM_DEVID_AUTO);
+	if (!ec_pdev)
+		return -ENOMEM;
+
+	if (!devm_request_region(&ec_pdev->dev, EC_IO_PORT_DATA, 2, DRV_NAME))
+		return -EIO;
+
+	if (!devm_request_region(&ec_pdev->dev, EC_ITE_PORT_OFS, 2, DRV_NAME))
+		return -EIO;
+
+	ret = platform_device_add_data(ec_pdev, &ec, sizeof(struct imanager2));
+	if (ret)
+		goto exit_device_put;
+
+	ret = platform_device_add(ec_pdev);
+	if (ret)
+		goto exit_device_put;
+
+	ret = mfd_add_devices(&ec_pdev->dev, -1, imanager2_cells,
+			      ARRAY_SIZE(imanager2_cells), NULL, -1, NULL);
+	if (ret)
+		goto exit_device_unregister;
+
+	return 0;
+
+exit_device_unregister:
+	platform_device_unregister(ec_pdev);
+exit_device_put:
+	platform_device_put(ec_pdev);
+
+	return ret;
+}
+
+static void __exit imanager2_mfd_device_exit(void)
+{
+	mfd_remove_devices(&ec_pdev->dev);
+	platform_device_unregister(ec_pdev);
+}
+
+module_init(imanager2_mfd_device_init);
+module_exit(imanager2_mfd_device_exit);
+
+MODULE_AUTHOR("Richard Vidal-Dorsch <richard.dorsch at advantech.com>");
+MODULE_DESCRIPTION("iManager2 platform device definitions v" DRV_VERSION);
+MODULE_LICENSE("GPL");
+MODULE_VERSION(DRV_VERSION);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 8+ 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 support for IT8516/18/28 Wei-Chun Pan
  2014-07-14 12:54 ` [PATCH 2/4] mfd: imanager2: Add Advantech EC APIs " Wei-Chun Pan
  2014-07-14 12:54 ` [PATCH 3/4] mfd: imanager2: Add Core supports " Wei-Chun Pan
@ 2014-07-14 12:54 ` Wei-Chun Pan
  2014-07-14 19:05   ` Guenter Roeck
  2014-07-22  7:58 ` [PATCH 1/4] mfd: imanager2: Add defines " Lee Jones
  3 siblings, 1 reply; 8+ 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] 8+ 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 support " Wei-Chun Pan
@ 2014-07-14 19:05   ` Guenter Roeck
  0 siblings, 0 replies; 8+ 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] 8+ messages in thread

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

This patch needs a commit log.  In fact, it can be squashed into the
first commit which uses these defines.

> Signed-off-by: Wei-Chun Pan <weichun.pan@advantech.com.tw>
> ---
>  include/linux/mfd/imanager2_ec.h | 358 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 358 insertions(+)
>  create mode 100644 include/linux/mfd/imanager2_ec.h
> 
> diff --git a/include/linux/mfd/imanager2_ec.h b/include/linux/mfd/imanager2_ec.h
> new file mode 100644
> index 0000000..bf7d70e
> --- /dev/null
> +++ b/include/linux/mfd/imanager2_ec.h
> @@ -0,0 +1,358 @@
> +/*
> + * imanager2_ec.h - MFD driver defines of 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/>.

This is the looooong version of the licence.  Can you find the shorter one.

> + */
> +
> +#ifndef __IMANAGER2_EC_H__
> +#define __IMANAGER2_EC_H__
> +
> +#include <linux/mutex.h>
> +
> +#define EC_FLAG_IO		0
> +#define EC_FLAG_IO_MAILBOX	(1 << 0)
> +#define EC_FLAG_MAILBOX		(1 << 1)

Use BIT(0), BIT(1) instead.

> +#define EC_MAX_DEVICE_ID_NUM	0xFF
> +#define EC_MAX_ITEM_NUM		32
> +
> +struct ec_table {
> +	u8 devid2itemnum[EC_MAX_DEVICE_ID_NUM];
> +	u8 pinnum[EC_MAX_ITEM_NUM];
> +	u8 devid[EC_MAX_ITEM_NUM];
> +};
> +
> +struct ec_version {
> +	u16 kernel_ver,
> +	    chip_code,
> +	    prj_id,
> +	    prj_ver;

Declare these separately.

> +};
> +
> +#define EC_MAX_LEN_PROJECT_NAME	8

Find a way to do this dynamically, or use 'const char *' instead.

> +struct imanager2 {
> +	u16 id;
> +	u32 flag;
> +	struct mutex lock;				/* protects io */

                                              We know what locks do.

> +	char prj_name[EC_MAX_LEN_PROJECT_NAME + 1];	/* strlen + '\0' */

                                              We know how strings work. 

> +	struct ec_version version;
> +	struct ec_table table;

'table' is awfully generic.

> +};

Use KernelDoc to document the main container.

> +/*
> + * Definition
> + */

This comment is not adding anything to the code here.

> +#define EC_TABLE_ITEM_UNUSED	0xFF
> +#define EC_TABLE_DID_NODEV	0x00
> +#define EC_TABLE_HWP_NODEV	0xFF
> +#define EC_TABLE_NOITEM		0xFF
> +
> +#define EC_ERROR		0xFF
> +
> +#define EC_RAM_BANK_SIZE	32	/* 32 bytes size for each bank. */
> +#define EC_RAM_BUFFER_SIZE	256	/* 32 bytes * 8 banks = 256 bytes */
> +
> +#define EC_SIO_CMD		0x29C
> +#define EC_SIO_DATA		0x29D

12bit commands?  Or are these 16bit and should have a leading 0?

> +/* Access Mailbox */
> +#define EC_IO_PORT_CMD		0x29A
> +#define EC_IO_PORT_DATA		0x299
> +
> +#define EC_IO_CMD_READ_OFFSET	0xA0
> +#define EC_IO_CMD_WRITE_OFFSET	0x50

Should these have leading 0's too?

> +#define EC_ITE_PORT_OFS		0x29E
> +#define EC_ITE_PORT_DATA	0x29F
> +
> +/*
> + * CMD - IO
> + */

Drop this.

> +/* ADC */
> +#define EC_CMD_ADC_INDEX				0x15
> +#define EC_CMD_ADC_READ_LSB				0x16
> +#define EC_CMD_ADC_READ_MSB				0x1F
> +/* HW Control Table */
> +#define EC_CMD_HWCTRLTABLE_INDEX			0x20
> +#define EC_CMD_HWCTRLTABLE_GET_PIN_NUM			0x21
> +#define EC_CMD_HWCTRLTABLE_GET_DEVICE_ID		0x22
> +#define EC_CMD_HWCTRLTABLE_GET_PIN_ACTIVE_POLARITY	0x23
> +/* ACPI RAM */
> +#define EC_CMD_ACPIRAM_READ				0x80
> +#define EC_CMD_ACPIRAM_WRITE				0x81
> +/* Extend RAM */
> +#define EC_CMD_EXTRAM_READ				0x86
> +#define EC_CMD_EXTRAM_WRITE				0x87
> +/* HW RAM */
> +#define EC_CMD_HWRAM_READ				0x88
> +#define EC_CMD_HWRAM_WRITE				0x89
> +
> +/*
> + * ACPI RAM Address Table
> + */
> +/* n = 1 ~ 2 */

Please accompany with words.  Random math is seldom helpful.

I'm guessing that you mean valid values of 'n' can be 1 or 2, but this
is unclear.

> +#define EC_ACPIRAM_ADDR_TEMPERATURE_BASE(n)	(0x60 + 3 * ((n) - 1))

What is 0x60?  Why 3?  Please use #defines for these numbers.

> +#define	EC_ACPIRAM_ADDR_LOCAL_TEMPERATURE(n) \
> +	EC_ACPIRAM_ADDR_TEMPERATURE_BASE(n)
> +#define	EC_ACPIRAM_ADDR_REMOTE_TEMPERATURE(n) \
> +	(EC_ACPIRAM_ADDR_TEMPERATURE_BASE(n) + 1)
> +#define	EC_ACPIRAM_ADDR_WARNING_TEMPERATURE(n)\
> +	(EC_ACPIRAM_ADDR_TEMPERATURE_BASE(n) + 2)
> +
> +/* N = 0 ~ 2 */

Please accompany with words.  Random math is seldom helpful.

> +#define EC_ACPIRAM_ADDR_FAN_SPEED_BASE(N)	(0x70 + 2 * (N))

No magic numbers.  Why are you using 'N' here and 'n' before?

> +#define EC_ACPIRAM_ADDR_KERNEL_MAJOR_VERSION	0xF8
> +#define EC_ACPIRAM_ADDR_CHIP_VENDOR_CODE	0xFA
> +#define EC_ACPIRAM_ADDR_PROJECT_NAME_CODE	0xFC
> +#define EC_ACPIRAM_ADDR_FIRMWARE_MAJOR_VERSION	0xFE
> +
> +/*
> + * HW RAM Address Table
> + */
> +/* Thermal Source Control RAM 0xB0-0xC7 (N: 0 ~ 3) */
> +#define EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N)	(0xB0 + 6 * (N))

Same comments as above.

> +#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_CHANNEL(N) \
> +	EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N)
> +#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_ADDR(N) \
> +	(EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 1)
> +#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_CMD(N)	 \
> +	(EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 2)
> +#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_STATUS(N) \
> +	(EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 3)
> +#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_FAN_CODE(N) \
> +	(EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 4)
> +#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_TEMPERATURE(N) \
> +	(EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 5)

Place more '\n' spacers in - these are difficult to read.

> +/* Fan Control 0xD0-0xEF (N: 0 ~ 3) */
> +#define EC_HWRAM_ADDR_FAN_BASE_ADDR(N)	(0xD0 + 0x10 * (N))
> +#define EC_HWRAM_ADDR_FAN_CODE(N)	EC_HWRAM_ADDR_FAN_BASE_ADDR(N)
> +#define EC_HWRAM_ADDR_FAN_STATUS(N)	(EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 1)
> +#define EC_HWRAM_ADDR_FAN_CONTROL(N)	(EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 2)
> +#define EC_HWRAM_ADDR_FAN_TEMP_HI(N)	(EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 3)
> +#define EC_HWRAM_ADDR_FAN_TEMP_LO(N)	(EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 4)
> +#define EC_HWRAM_ADDR_FAN_TEMP_LOSTOP(N) \
> +					(EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 5)
> +#define EC_HWRAM_ADDR_FAN_PWM_HI(N)	(EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 6)
> +#define EC_HWRAM_ADDR_FAN_PWM_LO(N)	(EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 7)
> +
> +/*
> + * OFS - Mailbox
> + */
> +/* Mailbox Structure */
> +#define EC_MAILBOX_OFFSET_CMD		0x00
> +#define EC_MAILBOX_OFFSET_STATUS	0x01
> +#define EC_MAILBOX_OFFSET_PARA		0x02
> +#define EC_MAILBOX_OFFSET_DAT(N)	(0x03 + (N))	/* N = 0x00 ~ 0x2C */
> +
> +/*
> + * CMD - Mailbox
> + */
> +/* GPIO */
> +#define EC_CMD_MAILBOX_READ_HW_PIN				0x11
> +#define EC_CMD_MAILBOX_WRITE_HW_PIN				0x12
> +/* Storage */
> +#define EC_CMD_MAILBOX_READ_EC_RAM				0x1E
> +#define EC_CMD_MAILBOX_WRITE_EC_RAM				0x1F
> +/* OTHERS */
> +#define EC_CMD_MAILBOX_READ_DYNAMIC_TABLE			0x20
> +/* Thermal Protect */
> +#define EC_CMD_MAILBOX_READ_THERMAL_SOURCE			0x42
> +#define EC_CMD_MAILBOX_WRITE_THERMAL_SOURCE			0x43
> +/* Storage */
> +#define EC_CMD_MALLBOX_CLEAR_256_BYTES_BUFFER			0xC0
> +#define EC_CMD_MALLBOX_READ_256_BYTES_BUFFER			0xC1
> +#define EC_CMD_MALLBOX_WRITE_256_BYTES_BUFFER			0xC2
> +#define EC_CMD_MALLBOX_READ_EEPROM_DATA_FROM_256_BYTES_BUFFER	0xC3
> +#define EC_CMD_MALLBOX_WRITE_256_BYTES_BUFFER_INTO_EEPROM_DATA	0xC4
> +/* General Mailbox Command */
> +#define EC_CMD_MAILBOX_GET_FIRMWARE_VERSION_AND_PROJECT_NAME	0xF0
> +#define EC_CMD_MAILBOX_CLEAR_ALL				0xFF
> +
> +/*
> + * Status - Mailbox
> + */
> +#define EC_MAILBOX_STATUS_FAIL		0x00
> +#define EC_MAILBOX_STATUS_SUCCESS	0x01
> +
> +/*
> + * PARA - Mailbox
> + */
> +/* RAM Type */
> +#define EC_RAM_BANK_ACPI	0x01
> +#define EC_RAM_BANK_HW		0x02
> +#define EC_RAM_BANK_EXT		0x03
> +#define EC_RAM_BANK_BUFFER	0x06
> +/* Dynamic Type */
> +#define EC_DYNAMIC_DEVICE_ID	0x00
> +#define EC_DYNAMIC_HW_PIN	0x01
> +#define EC_DYNAMIC_POLARITY	0x02
> +
> +/*
> + * Functions - Mailbox
> + */
> +/* command = 0x20 */
> +int imanager2_mbox_get_dynamic_table(u32 ecflag, u8 type, u8 *table);
> +/* command = 0x42 */
> +int imanager2_mbox_read_thermalzone(u32 ecflag, u8 zone, u8 *smbid, u8 *fanid,
> +				    u8 *buf, int *len);
> +/* command = 0xC0 */
> +int imanager2_mbox_clear_ram(u32 ecflag);
> +/* command = 0xC1 */
> +int imanager2_mbox_read_ram_across_banks(u32 ecflag, u8 *data, int len);
> +/* command = 0x1E */
> +int imanager2_mbox_read_ram(u32 ecflag, u8 bank, u8 offset, u8 *buf, u8 len);
> +/* command = 0x1F */
> +int imanager2_mbox_write_ram(u32 ecflag, u8 bank, u8 offset, u8 *buf, u8 len);
> +/* command = 0xF0 */
> +int imanager2_mbox_get_project_information(u32 ecflag, u8 *prj_name,
> +					   u16 *kernel_ver, u16 *chip_code,
> +					   u16 *prj_id, u16 *prj_ver);
> +
> +/*
> + * Functions - basic
> + */
> +#define IO_FLAG_OBF	(1 << 0)	/* output buffer full */
> +#define IO_FLAG_IBF	(1 << 1)	/* input buffer full  */

use BIT()

> +int ec_inb_after_obf1(u8 *data);
> +int ec_outb_after_ibc0(u16 port, u8 data);
> +/* ITE mailbox access */
> +int imanager2_mbox_read_data(u32 ecflag, u8 cmd, u8 para, u8 *data, int len);
> +int imanager2_mbox_write_data(u32 ecflag, u8 cmd, u8 para, u8 *data, int len);
> +/* only IO access */
> +int imanager2_mbox_io_read(u8 command, u8 offset, u8 *buf, u8 len);
> +int imanager2_mbox_io_write(u8 command, u8 offset, u8 *buf, u8 len);
> +int imanager2_mbox_io_simple_read(u8 command, u8 *value);
> +/* ITE Mailbox & IO access */
> +int imanager2_mbox_read_ram_support_io(u32 ecflag, u8 bank, u8 addr, u8 *buf,
> +				       u8 len);
> +
> +/*
> + * Device ID
> + */

Comments with the same information is as the type/variable name are
not helpful.  Please remove.

> +enum ec_device_id {
> +	/* GPIO */
> +	altgpio0 = 0x10,	/* 0x10 */

                                You're joking right? 

> +	altgpio1,
> +	altgpio2,
> +	altgpio3,
> +	altgpio4,
> +	altgpio5,
> +	altgpio6,
> +	altgpio7,
> +	/* GPIO - Button */
> +	btn0,
> +	btn1,
> +	btn2,
> +	btn3,
> +	btn4,
> +	btn5,
> +	btn6,
> +	btn7,
> +	/* PWM - Fan */
> +	cpufan_2p,		/* 0x20 */

Remove these comments.

> +	cpufan_4p,
> +	sysfan1_2p,
> +	sysfan1_4p,
> +	sysfan2_2p,
> +	sysfan2_4p,
> +	/* PWM - Brightness Control */
> +	pwmbrightness,
> +	/* PWM - System Speaker */
> +	pwmbeep,
> +	/* SMBus */
> +	smboem0,
> +	smboem1,
> +	smboem2,
> +	smbeeprom,
> +	smbthermal0,
> +	smbthermal1,
> +	smbsecurityeep,
> +	i2coem,
> +	/* DAC - Speaker */
> +	dacspeaker,		/* 0x30 */
> +	/* SMBus */
> +	smbeep2k = 0x38,
> +	oemeep,
> +	oemeep2k,
> +	peci,
> +	smboem3,
> +	smblink,
> +	smbslv,
> +	/* GPIO - LED */
> +	powerled = 0x40,	/* 0x40 */
> +	batledg,
> +	oemled0,
> +	oemled1,
> +	oemled2,
> +	batledr,
> +	/* SMBus - Smart Battery */
> +	smartbat1 = 0x48,
> +	smartbat2,
> +	/* ADC */
> +	adcmosbat = 0x50,	/* 0x50 */
> +	adcmosbatx2,
> +	adcmosbatx10,
> +	adcbat,
> +	adcbatx2,
> +	adcbatx10,
> +	adc5vs0,
> +	adc5vs0x2,
> +	adc5vs0x10,
> +	adv5vs5,
> +	adv5vs5x2,
> +	adv5vs5x10,
> +	adc33vs0,
> +	adc33vs0x2,
> +	adc33vs0x10,
> +	adc33vs5,
> +	adc33vs5x2,		/* 0x60 */
> +	adc33vs5x10,
> +	adv12vs0,
> +	adv12vs0x2,
> +	adv12vs0x10,
> +	adcvcorea,
> +	adcvcoreax2,
> +	adcvcoreax10,
> +	adcvcoreb,
> +	adcvcorebx2,
> +	adcvcorebx10,
> +	adcdc,
> +	adcdcx2,
> +	adcdcx10,
> +	adcdcstby,
> +	adcdcstbyx2,
> +	adcdcstbyx10,		/* 0x70 */
> +	adcdcother,
> +	adcdcotherx2,
> +	adcdcotherx10,
> +	adccurrent,
> +	/* IRQ - Watchdog */
> +	wdirq = 0x78,
> +	/* GPIO - Watchdog */
> +	wdnmi,
> +	/* Tacho - Fan */
> +	tacho0 = 0x80,		/* 0x80 */
> +	tacho1,
> +	tacho2,
> +	/* PWM - Brightness Control */
> +	pwmbrightness2 = 0x88,
> +	/* GPIO - Backlight Control */
> +	brionoff1,
> +	brionoff2,
> +};
> +
> +#endif /* __IMANAGER2_EC_H__ */

-- 
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] 8+ messages in thread

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

On Mon, 14 Jul 2014, Wei-Chun Pan wrote:

You have to write a commit log here.  What is this?  Why is it needed?
What problem does it solve?  What happens if it's not provided?  How
is it implemented?  Etc etc.

> Signed-off-by: Wei-Chun Pan <weichun.pan@advantech.com.tw>
> ---
>  drivers/mfd/imanager2_ec.c | 615 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 615 insertions(+)
>  create mode 100644 drivers/mfd/imanager2_ec.c
> 
> diff --git a/drivers/mfd/imanager2_ec.c b/drivers/mfd/imanager2_ec.c
> new file mode 100644
> index 0000000..f7a0003
> --- /dev/null
> +++ b/drivers/mfd/imanager2_ec.c
> @@ -0,0 +1,615 @@
> +/*
> + * imanager2_ec.c - MFD accessing driver of 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/>.

I'd prefer if you used the short version.

> + */
> +
> +#include <linux/io.h>
> +#include <linux/delay.h>

I'm sure that you're missing a whole bunch of header files here.  You
are to include all files that you make use of in _this_ file.

> +#include <linux/mfd/imanager2_ec.h>

Comment this line out to see what is not defined.  At the very least
you will need export.h and err.h.

[...]

> +static int imanager2_read_mailbox(u32 ecflag, u8 offset, u8 *data)
> +{
> +	if (ecflag & EC_FLAG_IO_MAILBOX) {
> +		int ret = ec_wait_ibc0();
> +		if (ret)
> +			return ret;
> +		inb(EC_IO_PORT_DATA);
> +		outb(offset + EC_IO_CMD_READ_OFFSET, EC_IO_PORT_CMD);
> +
> +		return ec_inb_after_obf1(data);
> +	} else {
> +		outb(offset, EC_ITE_PORT_OFS);
> +		*data = inb(EC_ITE_PORT_DATA);
> +	}
> +
> +	return 0;
> +}

All of the Mailbox controller code in this file should live in
drivers/mailbox.

Also, does your Mailbox controller support IRQs?

-- 
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] 8+ messages in thread

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

There is no way that you are introducing a 300 line driver and have
nothing to say about it.  Please put a nice descriptive commit log
here when you resubmit.

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

I also expect to see a change log here and version information in the
$SUBJECT line of the email.  I think the next one is v3 (or is it
v4?), so the start of the subject should read [PATCH v4 x/y].

>  drivers/mfd/Kconfig          |   6 +
>  drivers/mfd/Makefile         |   2 +
>  drivers/mfd/imanager2_core.c | 303 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 311 insertions(+)
>  create mode 100644 drivers/mfd/imanager2_core.c
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 3383412..48b063f 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -10,6 +10,12 @@ config MFD_CORE
>  	select IRQ_DOMAIN
>  	default n
>  
> +config MFD_IMANAGER2
> +	tristate "Support for Advantech iManager2 EC ICs"
> +	select MFD_CORE
> +	help
> +	  Support for Advantech iManager2 EC ICs
> +
>  config MFD_CS5535
>  	tristate "AMD CS5535 and CS5536 southbridge core functions"
>  	select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 2851275..10c64ae 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -166,3 +166,5 @@ obj-$(CONFIG_MFD_RETU)		+= retu-mfd.o
>  obj-$(CONFIG_MFD_AS3711)	+= as3711.o
>  obj-$(CONFIG_MFD_AS3722)	+= as3722.o
>  obj-$(CONFIG_MFD_STW481X)	+= stw481x.o
> +imanager2-objs			:= imanager2_core.o imanager2_ec.o
> +obj-$(CONFIG_MFD_IMANAGER2)	+= imanager2.o

No need to do this.  Just do:

obj-$(CONFIG_MFD_IMANAGER2)	+= imanager2_core.o imanager2_ec.o

> diff --git a/drivers/mfd/imanager2_core.c b/drivers/mfd/imanager2_core.c
> new file mode 100644
> index 0000000..2264d29
> --- /dev/null
> +++ b/drivers/mfd/imanager2_core.c
> @@ -0,0 +1,303 @@
> +/*
> + * imanager2_core.c - MFD core driver of 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/mfd/core.h>
> +#include <linux/mfd/imanager2_ec.h>

You need more headers than this.

> +#define DRV_NAME	"imanager2"

No need to #define the driver name.

Just use the string, where you need to use it.

> +#define DRV_VERSION	"4.0.1"

Remove this.

> +static struct mfd_cell imanager2_cells[] = {
> +	{
> +		.name = "imanager2_hwm",
> +	},
> +	{
> +		.name = "imanager2_i2c",
> +	},

Put these on a single line.

> +};
> +
> +enum chips {
> +	it8516 = 0x8516,
> +	it8518 = 0x8518,
> +	it8528 = 0x8528,

s/it/CHIP_IT

> +};

Move this to the top.

> +#define EC_CMD_AUTHENTICATION	0x30

Why is this seperate from the rest of the #defines?

> +static int imanager2_authentication(struct imanager2 *ec)
> +{
> +	u8 tmp;
> +	int ret;
> +
> +	mutex_lock(&ec->lock);
> +
> +	if (inb(EC_IO_PORT_CMD) == 0xFF && inb(EC_IO_PORT_DATA) == 0xFF) {
> +		ret = -ENODEV;
> +		goto unlock;
> +	}
> +
> +	if (inb(EC_IO_PORT_CMD) & IO_FLAG_OBF)
> +		inb(EC_IO_PORT_DATA);	/* initial OBF */

What's OBF?

> +	if (ec_outb_after_ibc0(EC_IO_PORT_CMD, EC_CMD_AUTHENTICATION)) {
> +		ret = -ENODEV;
> +		goto unlock;
> +	}
> +
> +	ret = ec_inb_after_obf1(&tmp);
> +
> +unlock:
> +	mutex_unlock(&ec->lock);
> +
> +	if (ret)
> +		return ret;

Remove this.

> +	if (tmp != 0x95)

... and change this to:

	if (!ret && tmp != 0x95)

But 0x95 should be #defined somewhere.

> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +#define EC_ITE_CHIPID_H8	0x20
> +#define EC_ITE_CHIPID_L8	0x21

Group all of the #defines somewhere.

> +static int imanager2_get_chip_type(struct imanager2 *ec)
> +{
> +	mutex_lock(&ec->lock);
> +
> +	outb(EC_ITE_CHIPID_H8, EC_SIO_CMD);
> +	ec->id = inb(EC_SIO_DATA) << 8;
> +	outb(EC_ITE_CHIPID_L8, EC_SIO_CMD);
> +	ec->id |= inb(EC_SIO_DATA);
> +
> +	mutex_unlock(&ec->lock);
> +
> +	switch (ec->id) {
> +	case it8516:
> +	case it8518:
> +		ec->flag = EC_FLAG_IO;
> +		break;
> +	case it8528:
> +		ec->flag |= EC_FLAG_IO_MAILBOX;
> +		break;
> +	default:

No error message to the user?

> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}

[...]

> +static int __init imanager2_mfd_device_init(void)

Drop the '_mfd_device' part of this.

> +{

All of the code below should be in .probe(), not init().

> +	struct imanager2 ec = { 0 };

Allocate memory instead.

> +	int ret;
> +
> +	mutex_init(&ec.lock);
> +
> +	ret = imanager2_authentication(&ec);
> +	if (ret)

How does the user know what broke?  I don't see any err/warn prints?


> +		return ret;
> +
> +	ret = imanager2_get_chip_type(&ec);
> +	if (ret)
> +		return ret;
> +
> +	ret = imanager2_get_info(&ec);
> +	if (ret)
> +		return ret;
> +
> +	ret = imanager2_build_device_table(&ec);
> +	if (ret)
> +		return ret;
> +
> +	ec_pdev = platform_device_alloc(DRV_NAME, PLATFORM_DEVID_AUTO);
> +	if (!ec_pdev)
> +		return -ENOMEM;
> +
> +	if (!devm_request_region(&ec_pdev->dev, EC_IO_PORT_DATA, 2, DRV_NAME))
> +		return -EIO;
> +
> +	if (!devm_request_region(&ec_pdev->dev, EC_ITE_PORT_OFS, 2, DRV_NAME))
> +		return -EIO;
> +
> +	ret = platform_device_add_data(ec_pdev, &ec, sizeof(struct imanager2));
> +	if (ret)
> +		goto exit_device_put;
> +
> +	ret = platform_device_add(ec_pdev);
> +	if (ret)
> +		goto exit_device_put;

If you convert this driver to be a prober platform device, you can get
rid of all of this above.

> +	ret = mfd_add_devices(&ec_pdev->dev, -1, imanager2_cells,
> +			      ARRAY_SIZE(imanager2_cells), NULL, -1, NULL);
> +	if (ret)
> +		goto exit_device_unregister;
> +
> +	return 0;
> +
> +exit_device_unregister:
> +	platform_device_unregister(ec_pdev);
> +exit_device_put:
> +	platform_device_put(ec_pdev);
> +
> +	return ret;
> +}
> +
> +static void __exit imanager2_mfd_device_exit(void)
> +{
> +	mfd_remove_devices(&ec_pdev->dev);
> +	platform_device_unregister(ec_pdev);
> +}
> +
> +module_init(imanager2_mfd_device_init);
> +module_exit(imanager2_mfd_device_exit);

This is a mess.  Use module_platform_driver() instead.

> +MODULE_AUTHOR("Richard Vidal-Dorsch <richard.dorsch at advantech.com>");
> +MODULE_DESCRIPTION("iManager2 platform device definitions v" DRV_VERSION);
> +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] 8+ messages in thread

end of thread, other threads:[~2014-07-22  8:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-14 12:54 [PATCH 1/4] mfd: imanager2: Add defines support for IT8516/18/28 Wei-Chun Pan
2014-07-14 12:54 ` [PATCH 2/4] mfd: imanager2: Add Advantech EC APIs " Wei-Chun Pan
2014-07-22  8:36   ` Lee Jones
2014-07-14 12:54 ` [PATCH 3/4] mfd: imanager2: Add Core supports " Wei-Chun Pan
2014-07-22  8:56   ` Lee Jones
2014-07-14 12:54 ` [PATCH 4/4] hwmon: (imanager2) Add support " Wei-Chun Pan
2014-07-14 19:05   ` Guenter Roeck
2014-07-22  7:58 ` [PATCH 1/4] mfd: imanager2: Add defines " Lee Jones

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.