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