RE: [PATCH 4/4] hwmon: (imanager2) Add support for IT8516/18/28
From: Wei-Chun Pan
Date: Thu Aug 07 2014 - 05:25:27 EST
Sorry, I sent wrong patch just now. Please ignore the mail "[PATCH 1/3]
imanager2: rename io functions and remove no used functions".
This mail include 2 patches:
The first patch is shown I rename some functions.
The second patch is shown I moditfy code according to your comment.
Signed-off-by: Wei-Chun Pan <weichun.pan@xxxxxxxxxxxxxxxx>
---
diff --git a/drivers/hwmon/imanager2_hwm.c b/drivers/hwmon/imanager2_hwm.c
index 335bffb..ab63296 100644
--- a/drivers/hwmon/imanager2_hwm.c
+++ b/drivers/hwmon/imanager2_hwm.c
@@ -119,17 +119,17 @@ static int imanager2_volt_get_value_by_io(struct imanager2 *ec, int index,
u8 portnum;
int ret;
- ret = imanager2_mbox_io_read(EC_CMD_ADC_INDEX, pin, &portnum, 1);
+ ret = imanager2_io_read_data(EC_CMD_ADC_INDEX, pin, &portnum, 1);
if (ret)
return ret;
if (portnum == EC_ERROR)
return -ENXIO;
- ret = imanager2_mbox_io_simple_read(EC_CMD_ADC_READ_LSB, &buf[1]);
+ ret = imanager2_io_nooffset_readbyte(EC_CMD_ADC_READ_LSB, &buf[1]);
if (ret)
return ret;
- return imanager2_mbox_io_simple_read(EC_CMD_ADC_READ_MSB, &buf[0]);
+ return imanager2_io_nooffset_readbyte(EC_CMD_ADC_READ_MSB, &buf[0]);
}
static int imanager2_volt_get_value(struct imanager2 *ec, int index,
@@ -468,7 +468,7 @@ static void imanager2_temp_init(struct imanager2 *ec)
(u8 *)&zone,
&len);
} else {
- ret = imanager2_mbox_io_read(
+ ret = imanager2_io_read_data(
EC_CMD_HWRAM_READ,
EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_STATUS(thm),
&zone.status, 1);
@@ -596,7 +593,7 @@ static int imanager2_fan_get_value(struct imanager2 *ec, int index,
EC_CMD_MAILBOX_READ_HW_PIN,
ec_fan_table[index].did, tmp, 2);
else
- ret = imanager2_mbox_io_read(EC_CMD_ACPIRAM_READ,
+ ret = imanager2_io_read_data(EC_CMD_ACPIRAM_READ,
ec_fan_table[index].fspeed_acpireg,
tmp, 2);
@@ -621,7 +618,7 @@ static int imanager2_fan_item_init_by_io(struct imanager2 *ec, int fnum)
u8 tmp;
mutex_lock(&ec->lock);
- ret = imanager2_mbox_io_read(EC_CMD_HWRAM_READ,
+ ret = imanager2_io_read_data(EC_CMD_HWRAM_READ,
EC_HWRAM_ADDR_FAN_CONTROL(fnum), &tmp, 1);
mutex_unlock(&ec->lock);
---
> > ---
> > drivers/hwmon/Kconfig | 5 +
> > drivers/hwmon/Makefile | 1 +
> > drivers/hwmon/imanager2_hwm.c | 768
> ++++++++++++++++++++++++++++++++++++++++++
>
> Documentation/hwmon/imanager2_hwm missing.
diff --git a/Documentation/hwmon/imanager2_hwm b/Documentation/hwmon/imanager2_hwm
new file mode 100644
index 0000000..bf3d0b5
--- /dev/null
+++ b/Documentation/hwmon/imanager2_hwm
@@ -0,0 +1,42 @@
+Kernel driver imanager2_hwm
+===========================
+
+Supported chips:
+ * ITE IT8516
+ Prefix: 'it8516'
+ Addresses scanned: I/O chennel 0x029A/0x0299,
+ IET mailbox chennel 0x029E/0x029F
+ Datasheet: Not publicly available
+ * ITE IT8518
+ Prefix: 'it8518'
+ Addresses scanned: I/O chennel 0x029A/0x0299,
+ IET mailbox chennel 0x029E/0x029F
+ Datasheet: Not publicly available
+ * ITE IT8528
+ Prefix: 'it8528'
+ Addresses scanned: I/O chennel 0x029A/0x0299
+ Datasheet: Not publicly available
+
+Authors:
+ Richard Vidal-Dorsch <richard.dorsch@xxxxxxxxxxxxx>
+
+
+Description
+-----------
+
+This driver supports the hardware monitoring features of the IT8516, IT8518, and
+IT8528 chips. These features include 11 voltage sensors, 1 current sensor, 4
+temperature sensors, and 3 fan rotation speed sensors.
+
+ITE IT8516, IT8518, and IT8528 are are 'EC chips'. These chips are like Super I/O control boards. For IT8516 and IT 8518 The control chennel can be I/O or ITE mailbox chennel. I/O chennel is a common way but ITE mailbox chennel performance faster since it does not need to wait IBF (input buffer full) and OBF (output buffer full) to before send or get data.
+
+ITE IT8528 use an I/O chennel way to access mailbox, called I/O mailbox for cost down. Its performance the between pure I/O controller and ITE mailbox controller.
+
+
+sysfs-Interface
+---------------
+
+in[0-11]_input - adc voltage input
+curr1_input - adc current input
+temp[1-4]_input - temperature input
+fan[1-3]_input - fan speed input
>
> > 3 files changed, 774 insertions(+)
> > create mode 100644 drivers/hwmon/imanager2_hwm.c
> >
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index bc196f4..7524fc3 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -39,6 +39,11 @@ config HWMON_DEBUG_CHIP
> >
> > comment "Native drivers"
> >
> > +config SENSORS_IMANAGER2
> > + tristate "Support for Advantech iManager2 EC H.W. Monitor"
> > + select MFD_CORE
> > + depends on MFD_IMANAGER2
> > +
> Alphabetic order please.
>
> > config SENSORS_AB8500
> > tristate "AB8500 thermal monitoring"
> > depends on AB8500_GPADC && AB8500_BM
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index c48f987..a2c8f07 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -146,6 +146,7 @@ obj-$(CONFIG_SENSORS_W83L785TS) += w83l785ts.o
> > obj-$(CONFIG_SENSORS_W83L786NG) += w83l786ng.o
> > obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
> > obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
> > +obj-$(CONFIG_SENSORS_IMANAGER2) += imanager2_hwm.o
> >
> Alphabetic order please.
>
drivers/hwmon/Kconfig | 10 +++++-----
drivers/hwmon/Makefile | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 7524fc3..e39f8e0 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -39,11 +39,6 @@ config HWMON_DEBUG_CHIP
comment "Native drivers"
-config SENSORS_IMANAGER2
- tristate "Support for Advantech iManager2 EC H.W. Monitor"
- select MFD_CORE
- depends on MFD_IMANAGER2
-
config SENSORS_AB8500
tristate "AB8500 thermal monitoring"
depends on AB8500_GPADC && AB8500_BM
@@ -576,6 +571,11 @@ config SENSORS_CORETEMP
sensor inside your CPU. Most of the family 6 CPUs
are supported. Check Documentation/hwmon/coretemp for details.
+config SENSORS_IMANAGER2
+ tristate "Support for Advantech iManager2 EC H.W. Monitor"
+ select MFD_CORE
+ depends on MFD_IMANAGER2
+
config SENSORS_IT87
tristate "ITE IT87xx and compatibles"
depends on !PPC
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index a2c8f07..564b6fe 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -72,6 +72,7 @@ obj-$(CONFIG_SENSORS_I5K_AMB) += i5k_amb.o
obj-$(CONFIG_SENSORS_IBMAEM) += ibmaem.o
obj-$(CONFIG_SENSORS_IBMPEX) += ibmpex.o
obj-$(CONFIG_SENSORS_IIO_HWMON) += iio_hwmon.o
+obj-$(CONFIG_SENSORS_IMANAGER2) += imanager2_hwm.o
obj-$(CONFIG_SENSORS_INA209) += ina209.o
obj-$(CONFIG_SENSORS_INA2XX) += ina2xx.o
obj-$(CONFIG_SENSORS_IT87) += it87.o
@@ -146,7 +147,6 @@ obj-$(CONFIG_SENSORS_W83L785TS) += w83l785ts.o
obj-$(CONFIG_SENSORS_W83L786NG) += w83l786ng.o
obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
-obj-$(CONFIG_SENSORS_IMANAGER2) += imanager2_hwm.o
obj-$(CONFIG_PMBUS) += pmbus/
> > obj-$(CONFIG_PMBUS) += pmbus/
> >
> > diff --git a/drivers/hwmon/imanager2_hwm.c
[...]
> b/drivers/hwmon/imanager2_hwm.c
> > new file mode 100644
> > index 0000000..335bffb
> > --- /dev/null
> > +++ b/drivers/hwmon/imanager2_hwm.c
> > @@ -0,0 +1,768 @@
> > +/*
> > + * imanager2_hwm.c - HW Monitoring interface for Advantech EC
> IT8516/18/28
> > + * Copyright (C) 2014 Richard Vidal-Dorsch <richard.dorsch@xxxxxxxxxxxxx>
> > + *
> > + * 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/