* [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers
@ 2013-02-18 13:38 Lars-Peter Clausen
2013-02-18 13:38 ` [PATCH v2 2/4] hwmon: (adt7410) Add support for the adt7310/adt7320 Lars-Peter Clausen
` (4 more replies)
0 siblings, 5 replies; 23+ messages in thread
From: Lars-Peter Clausen @ 2013-02-18 13:38 UTC (permalink / raw)
To: Jean Delvare, Guenter Roeck
Cc: Hartmut Knaack, Jonathan Cameron, lm-sensors, linux-iio,
Lars-Peter Clausen
Currently each time the temperature register is read the driver also reads the
threshold and hysteresis registers. This increases the amount of I2C traffic and
time needed to read the temperature by a factor of ~5. Neither the threshold nor
the hysteresis change on their own, so once we've read them, we should be able
to just use the cached value of the registers. This patch modifies the code
accordingly and only reads the threshold and hysteresis registers once during
probe.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
Changes since v1:
* Fix error checking for i2c reads
---
drivers/hwmon/adt7410.c | 91 +++++++++++++++++++++++++++++++------------------
1 file changed, 58 insertions(+), 33 deletions(-)
diff --git a/drivers/hwmon/adt7410.c b/drivers/hwmon/adt7410.c
index 99a7290..b6acfa4 100644
--- a/drivers/hwmon/adt7410.c
+++ b/drivers/hwmon/adt7410.c
@@ -119,45 +119,33 @@ static int adt7410_temp_ready(struct i2c_client *client)
return -ETIMEDOUT;
}
-static struct adt7410_data *adt7410_update_device(struct device *dev)
+static int adt7410_update_temp(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
struct adt7410_data *data = i2c_get_clientdata(client);
- struct adt7410_data *ret = data;
+ int ret = 0;
+
mutex_lock(&data->update_lock);
if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
|| !data->valid) {
- int i, status;
+ int temp;
dev_dbg(&client->dev, "Starting update\n");
- status = adt7410_temp_ready(client); /* check for new value */
- if (unlikely(status)) {
- ret = ERR_PTR(status);
+ ret = adt7410_temp_ready(client); /* check for new value */
+ if (ret)
goto abort;
- }
- for (i = 0; i < ARRAY_SIZE(data->temp); i++) {
- status = i2c_smbus_read_word_swapped(client,
- ADT7410_REG_TEMP[i]);
- if (unlikely(status < 0)) {
- dev_dbg(dev,
- "Failed to read value: reg %d, error %d\n",
- ADT7410_REG_TEMP[i], status);
- ret = ERR_PTR(status);
- goto abort;
- }
- data->temp[i] = status;
- }
- status = i2c_smbus_read_byte_data(client, ADT7410_T_HYST);
- if (unlikely(status < 0)) {
- dev_dbg(dev,
- "Failed to read value: reg %d, error %d\n",
- ADT7410_T_HYST, status);
- ret = ERR_PTR(status);
+
+ temp = i2c_smbus_read_word_swapped(client, ADT7410_REG_TEMP[0]);
+ if (temp < 0) {
+ ret = temp;
+ dev_dbg(dev, "Failed to read value: reg %d, error %d\n",
+ ADT7410_REG_TEMP[0], ret);
goto abort;
}
- data->hyst = status;
+ data->temp[0] = temp;
+
data->last_updated = jiffies;
data->valid = true;
}
@@ -167,6 +155,35 @@ abort:
return ret;
}
+static int adt7410_fill_cache(struct i2c_client *client)
+{
+ struct adt7410_data *data = i2c_get_clientdata(client);
+ int ret;
+ int i;
+
+ for (i = 1; i < ARRAY_SIZE(ADT7410_REG_TEMP); i++) {
+ ret = i2c_smbus_read_word_swapped(client, ADT7410_REG_TEMP[i]);
+ if (ret < 0) {
+ dev_dbg(&client->dev,
+ "Failed to read value: reg %d, error %d\n",
+ ADT7410_REG_TEMP[0], ret);
+ return ret;
+ }
+ data->temp[i] = ret;
+ }
+
+ ret = i2c_smbus_read_byte_data(client, ADT7410_T_HYST);
+ if (ret < 0) {
+ dev_dbg(&client->dev,
+ "Failed to read value: hyst reg, error %d\n",
+ ret);
+ return ret;
+ }
+ data->hyst = ret;
+
+ return 0;
+}
+
static s16 ADT7410_TEMP_TO_REG(long temp)
{
return DIV_ROUND_CLOSEST(clamp_val(temp, ADT7410_TEMP_MIN,
@@ -193,10 +210,16 @@ static ssize_t adt7410_show_temp(struct device *dev,
struct device_attribute *da, char *buf)
{
struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
- struct adt7410_data *data = adt7410_update_device(dev);
+ struct i2c_client *client = to_i2c_client(dev);
+ struct adt7410_data *data = i2c_get_clientdata(client);
- if (IS_ERR(data))
- return PTR_ERR(data);
+ if (attr->index == 0) {
+ int ret;
+
+ ret = adt7410_update_temp(dev);
+ if (ret)
+ return ret;
+ }
return sprintf(buf, "%d\n", ADT7410_REG_TO_TEMP(data,
data->temp[attr->index]));
@@ -232,13 +255,11 @@ static ssize_t adt7410_show_t_hyst(struct device *dev,
char *buf)
{
struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
- struct adt7410_data *data;
+ struct i2c_client *client = to_i2c_client(dev);
+ struct adt7410_data *data = i2c_get_clientdata(client);
int nr = attr->index;
int hyst;
- data = adt7410_update_device(dev);
- if (IS_ERR(data))
- return PTR_ERR(data);
hyst = (data->hyst & ADT7410_T_HYST_MASK) * 1000;
/*
@@ -371,6 +392,10 @@ static int adt7410_probe(struct i2c_client *client,
}
dev_dbg(&client->dev, "Config %02x\n", data->config);
+ ret = adt7410_fill_cache(client);
+ if (ret)
+ goto exit_restore;
+
/* Register sysfs hooks */
ret = sysfs_create_group(&client->dev.kobj, &adt7410_group);
if (ret)
--
1.8.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 2/4] hwmon: (adt7410) Add support for the adt7310/adt7320
2013-02-18 13:38 [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers Lars-Peter Clausen
@ 2013-02-18 13:38 ` Lars-Peter Clausen
2013-02-19 1:30 ` Guenter Roeck
2013-02-18 13:38 ` [PATCH v2 3/4] hwmon: (adt7x10) Add alarm interrupt support Lars-Peter Clausen
` (3 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: Lars-Peter Clausen @ 2013-02-18 13:38 UTC (permalink / raw)
To: Jean Delvare, Guenter Roeck
Cc: Hartmut Knaack, Jonathan Cameron, lm-sensors, linux-iio,
Lars-Peter Clausen
The adt7310/adt7320 is the SPI version of the adt7410/adt7420. The register map
layout is a bit different, i.e. the register addresses differ between the two
variants, but the bit layouts of the individual registers are identical. So both
chip variants can easily be supported by the same driver. The issue of non
matching register address layouts is solved by a simple look-up table which
translates the I2C addresses to the SPI addresses.
The patch moves the bulk of the adt7410 driver to a common module that will be
shared by the adt7410 and adt7310 drivers. This common module implements the
driver logic and uses a set of virtual functions to perform IO access. The
adt7410 and adt7310 driver modules provide proper implementations of these IO
accessor functions for I2C respective SPI.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
Changes since v1:
* Update the driver documentation to include ADT7310/ADT7320/ADT7420
* Pass the result of the read methods via the return value instead of a
pointer argument
* Simplify spi read methods by using spi_w8r8 and spi_w8r16
* Update module description of the shared module
* Fix some typos
---
Documentation/hwmon/adt7410 | 42 ++--
drivers/hwmon/Kconfig | 20 ++
drivers/hwmon/Makefile | 2 +
drivers/hwmon/adt7310.c | 115 +++++++++++
drivers/hwmon/adt7410.c | 464 +++---------------------------------------
drivers/hwmon/adt7x10.c | 476 ++++++++++++++++++++++++++++++++++++++++++++
drivers/hwmon/adt7x10.h | 48 +++++
7 files changed, 720 insertions(+), 447 deletions(-)
create mode 100644 drivers/hwmon/adt7310.c
create mode 100644 drivers/hwmon/adt7x10.c
create mode 100644 drivers/hwmon/adt7x10.h
diff --git a/Documentation/hwmon/adt7410 b/Documentation/hwmon/adt7410
index 9600400..e452ae0 100644
--- a/Documentation/hwmon/adt7410
+++ b/Documentation/hwmon/adt7410
@@ -7,25 +7,41 @@ Supported chips:
Addresses scanned: I2C 0x48 - 0x4B
Datasheet: Publicly available at the Analog Devices website
http://www.analog.com/static/imported-files/data_sheets/ADT7410.pdf
+ * Analog Devices ADT7420
+ Prefix: 'adt7420'
+ Addresses scanned: I2C 0x48 - 0x4B
+ Datasheet: Publicly available at the Analog Devices website
+ http://www.analog.com/static/imported-files/data_sheets/ADT7420.pdf
+ * Analog Devices ADT7310
+ Prefix: 'adt7310'
+ Addresses scanned: I2C 0x48 - 0x4B
+ Datasheet: Publicly available at the Analog Devices website
+ http://www.analog.com/static/imported-files/data_sheets/ADT7310.pdf
+ * Analog Devices ADT7320
+ Prefix: 'adt7320'
+ Addresses scanned: I2C 0x48 - 0x4B
+ Datasheet: Publicly available at the Analog Devices website
+ http://www.analog.com/static/imported-files/data_sheets/ADT7320.pdf
Author: Hartmut Knaack <knaack.h@gmx.de>
Description
-----------
-The ADT7410 is a temperature sensor with rated temperature range of -55°C to
-+150°C. It has a high accuracy of +/-0.5°C and can be operated at a resolution
-of 13 bits (0.0625°C) or 16 bits (0.0078°C). The sensor provides an INT pin to
-indicate that a minimum or maximum temperature set point has been exceeded, as
-well as a critical temperature (CT) pin to indicate that the critical
-temperature set point has been exceeded. Both pins can be set up with a common
-hysteresis of 0°C - 15°C and a fault queue, ranging from 1 to 4 events. Both
-pins can individually set to be active-low or active-high, while the whole
-device can either run in comparator mode or interrupt mode. The ADT7410
-supports continous temperature sampling, as well as sampling one temperature
-value per second or even justget one sample on demand for power saving.
-Besides, it can completely power down its ADC, if power management is
-required.
+The ADT7410 and similar are a temperature sensors with rated temperature range
+of -55°C to +150°C (ADT7310/ADT7410) or -40°C to +150°C (ADT7320/ADT7420). They
+have a high accuracy of +/-0.5°C (ADT7310/ADT7410) or +/-0.2C (ADT7430/ADT7420)
+and can be operated at a resolution of 13 bits (0.0625°C) or 16 bits (0.0078°C).
+The sensor provides an INT pin to indicate that a minimum or maximum temperature
+set point has been exceeded, as well as a critical temperature (CT) pin to
+indicate that the critical temperature set point has been exceeded. Both pins
+can be set up with a common hysteresis of 0°C - 15°C and a fault queue, ranging
+from 1 to 4 events. Both pins can individually set to be active-low or
+active-high, while the whole device can either run in comparator mode or
+interrupt mode. The ADT7410 supports continous temperature sampling, as well as
+sampling one temperature value per second or even justget one sample on demand
+for power saving. Besides, it can completely power down its ADC, if power
+management is required.
Configuration Notes
-------------------
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 89ac1cb..aaa14f4 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -179,9 +179,29 @@ config SENSORS_ADM9240
This driver can also be built as a module. If so, the module
will be called adm9240.
+config SENSORS_ADT7X10
+ tristate
+ help
+ This module contains common code shared by the ADT7310/ADT7320 and
+ ADT7410/ADT7420 temperature monitoring chip drivers.
+
+ If build as a module, the module will be called adt7x10.
+
+config SENSORS_ADT7310
+ tristate "Analog Devices ADT7310/ADT7320"
+ depends on SPI_MASTER
+ select SENSORS_ADT7X10
+ help
+ If you say yes here you get support for the Analog Devices
+ ADT7310 and ADT7320 temperature monitoring chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called adt7310.
+
config SENSORS_ADT7410
tristate "Analog Devices ADT7410/ADT7420"
depends on I2C
+ select SENSORS_ADT7X10
help
If you say yes here you get support for the Analog Devices
ADT7410 and ADT7420 temperature monitoring chips.
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 8d6d97e..5d36a57 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -34,6 +34,8 @@ obj-$(CONFIG_SENSORS_ADM9240) += adm9240.o
obj-$(CONFIG_SENSORS_ADS1015) += ads1015.o
obj-$(CONFIG_SENSORS_ADS7828) += ads7828.o
obj-$(CONFIG_SENSORS_ADS7871) += ads7871.o
+obj-$(CONFIG_SENSORS_ADT7X10) += adt7x10.o
+obj-$(CONFIG_SENSORS_ADT7310) += adt7310.o
obj-$(CONFIG_SENSORS_ADT7410) += adt7410.o
obj-$(CONFIG_SENSORS_ADT7411) += adt7411.o
obj-$(CONFIG_SENSORS_ADT7462) += adt7462.o
diff --git a/drivers/hwmon/adt7310.c b/drivers/hwmon/adt7310.c
new file mode 100644
index 0000000..2196ac3
--- /dev/null
+++ b/drivers/hwmon/adt7310.c
@@ -0,0 +1,115 @@
+/*
+ * ADT7310/ADT7310 digital temperature sensor driver
+ *
+ * Copyright 2010-2013 Analog Devices Inc.
+ * Author: Lars-Peter Clausen <lars@metafoo.de>
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/spi/spi.h>
+#include <asm/unaligned.h>
+
+#include "adt7x10.h"
+
+static const u8 adt7310_reg_table[] = {
+ [ADT7410_TEMPERATURE] = ADT7310_TEMPERATURE,
+ [ADT7410_STATUS] = ADT7310_STATUS,
+ [ADT7410_CONFIG] = ADT7310_CONFIG,
+ [ADT7410_T_ALARM_HIGH] = ADT7310_T_ALARM_HIGH,
+ [ADT7410_T_ALARM_LOW] = ADT7310_T_ALARM_LOW,
+ [ADT7410_T_CRIT] = ADT7310_T_CRIT,
+ [ADT7410_T_HYST] = ADT7310_T_HYST,
+ [ADT7410_ID] = ADT7310_ID,
+};
+
+#define ADT7310_CMD_REG_OFFSET 3
+#define ADT7310_CMD_READ 0x40
+
+#define AD7310_COMMAND(reg) (adt7310_reg_table[(reg)] << ADT7310_CMD_REG_OFFSET)
+
+static int adt7310_spi_read_word(struct device *dev, u8 reg)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ int ret;
+
+ ret = spi_w8r16(spi, AD7310_COMMAND(reg) | ADT7310_CMD_READ);
+ if (ret < 0)
+ return ret;
+
+ return be16_to_cpu(ret);
+}
+
+static int adt7310_spi_write_word(struct device *dev, u8 reg,
+ u16 data)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ u8 buf[3];
+
+ buf[0] = AD7310_COMMAND(reg);
+ put_unaligned_be16(data, &buf[1]);
+
+ return spi_write(spi, buf, sizeof(buf));
+}
+
+static int adt7310_spi_read_byte(struct device *dev, u8 reg)
+{
+ struct spi_device *spi = to_spi_device(dev);
+
+ return spi_w8r8(spi, AD7310_COMMAND(reg) | ADT7310_CMD_READ);
+}
+
+static int adt7310_spi_write_byte(struct device *dev, u8 reg,
+ u8 data)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ u8 buf[2];
+
+ buf[0] = AD7310_COMMAND(reg);
+ buf[1] = data;
+
+ return spi_write(spi, buf, sizeof(buf));
+}
+
+static const struct adt7410_ops adt7310_spi_ops = {
+ .read_word = adt7310_spi_read_word,
+ .write_word = adt7310_spi_write_word,
+ .read_byte = adt7310_spi_read_byte,
+ .write_byte = adt7310_spi_write_byte,
+};
+
+static int adt7310_spi_probe(struct spi_device *spi)
+{
+ return adt7410_probe(&spi->dev, spi_get_device_id(spi)->name,
+ &adt7310_spi_ops);
+}
+
+static int adt7310_spi_remove(struct spi_device *spi)
+{
+ return adt7410_remove(&spi->dev);
+}
+
+static const struct spi_device_id adt7310_id[] = {
+ { "adt7310", 0 },
+ { "adt7320", 0 },
+ {}
+};
+MODULE_DEVICE_TABLE(spi, adt7310_id);
+
+static struct spi_driver adt7310_driver = {
+ .driver = {
+ .name = "adt7310",
+ .owner = THIS_MODULE,
+ .pm = ADT7410_DEV_PM_OPS,
+ },
+ .probe = adt7310_spi_probe,
+ .remove = adt7310_spi_remove,
+ .id_table = adt7310_id,
+};
+module_spi_driver(adt7310_driver);
+
+MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
+MODULE_DESCRIPTION("ADT7310/ADT7320 driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/adt7410.c b/drivers/hwmon/adt7410.c
index b6acfa4..b500ab3 100644
--- a/drivers/hwmon/adt7410.c
+++ b/drivers/hwmon/adt7410.c
@@ -1,485 +1,81 @@
/*
- * adt7410.c - Part of lm_sensors, Linux kernel modules for hardware
- * monitoring
- * This driver handles the ADT7410 and compatible digital temperature sensors.
- * Hartmut Knaack <knaack.h@gmx.de> 2012-07-22
- * based on lm75.c by Frodo Looijaard <frodol@dds.nl>
- * and adt7410.c from iio-staging by Sonic Zhang <sonic.zhang@analog.com>
+ * ADT7410/ADT7420 digital temperature sensor driver
*
- * 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 2 of the License, or
- * (at your option) any later version.
+ * Copyright 2010-2013 Analog Devices Inc.
+ * Author: Lars-Peter Clausen <lars@metafoo.de>
*
- * 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, write to the Free Software
- * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ * Licensed under the GPL-2 or later.
*/
#include <linux/module.h>
#include <linux/init.h>
-#include <linux/slab.h>
-#include <linux/jiffies.h>
#include <linux/i2c.h>
-#include <linux/hwmon.h>
-#include <linux/hwmon-sysfs.h>
-#include <linux/err.h>
-#include <linux/mutex.h>
-#include <linux/delay.h>
-
-/*
- * ADT7410 registers definition
- */
-
-#define ADT7410_TEMPERATURE 0
-#define ADT7410_STATUS 2
-#define ADT7410_CONFIG 3
-#define ADT7410_T_ALARM_HIGH 4
-#define ADT7410_T_ALARM_LOW 6
-#define ADT7410_T_CRIT 8
-#define ADT7410_T_HYST 0xA
-
-/*
- * ADT7410 status
- */
-#define ADT7410_STAT_T_LOW (1 << 4)
-#define ADT7410_STAT_T_HIGH (1 << 5)
-#define ADT7410_STAT_T_CRIT (1 << 6)
-#define ADT7410_STAT_NOT_RDY (1 << 7)
-/*
- * ADT7410 config
- */
-#define ADT7410_FAULT_QUEUE_MASK (1 << 0 | 1 << 1)
-#define ADT7410_CT_POLARITY (1 << 2)
-#define ADT7410_INT_POLARITY (1 << 3)
-#define ADT7410_EVENT_MODE (1 << 4)
-#define ADT7410_MODE_MASK (1 << 5 | 1 << 6)
-#define ADT7410_FULL (0 << 5 | 0 << 6)
-#define ADT7410_PD (1 << 5 | 1 << 6)
-#define ADT7410_RESOLUTION (1 << 7)
+#include "adt7x10.h"
-/*
- * ADT7410 masks
- */
-#define ADT7410_T13_VALUE_MASK 0xFFF8
-#define ADT7410_T_HYST_MASK 0xF
-
-/* straight from the datasheet */
-#define ADT7410_TEMP_MIN (-55000)
-#define ADT7410_TEMP_MAX 150000
-
-enum adt7410_type { /* keep sorted in alphabetical order */
- adt7410,
-};
-
-static const u8 ADT7410_REG_TEMP[4] = {
- ADT7410_TEMPERATURE, /* input */
- ADT7410_T_ALARM_HIGH, /* high */
- ADT7410_T_ALARM_LOW, /* low */
- ADT7410_T_CRIT, /* critical */
-};
-
-/* Each client has this additional data */
-struct adt7410_data {
- struct device *hwmon_dev;
- struct mutex update_lock;
- u8 config;
- u8 oldconfig;
- bool valid; /* true if registers valid */
- unsigned long last_updated; /* In jiffies */
- s16 temp[4]; /* Register values,
- 0 = input
- 1 = high
- 2 = low
- 3 = critical */
- u8 hyst; /* hysteresis offset */
-};
-
-/*
- * adt7410 register access by I2C
- */
-static int adt7410_temp_ready(struct i2c_client *client)
+static int adt7410_i2c_read_word(struct device *dev, u8 reg)
{
- int i, status;
-
- for (i = 0; i < 6; i++) {
- status = i2c_smbus_read_byte_data(client, ADT7410_STATUS);
- if (status < 0)
- return status;
- if (!(status & ADT7410_STAT_NOT_RDY))
- return 0;
- msleep(60);
- }
- return -ETIMEDOUT;
+ return i2c_smbus_read_word_swapped(to_i2c_client(dev), reg);
}
-static int adt7410_update_temp(struct device *dev)
+static int adt7410_i2c_write_word(struct device *dev, u8 reg, u16 data)
{
- struct i2c_client *client = to_i2c_client(dev);
- struct adt7410_data *data = i2c_get_clientdata(client);
- int ret = 0;
-
- mutex_lock(&data->update_lock);
-
- if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
- || !data->valid) {
- int temp;
-
- dev_dbg(&client->dev, "Starting update\n");
-
- ret = adt7410_temp_ready(client); /* check for new value */
- if (ret)
- goto abort;
-
- temp = i2c_smbus_read_word_swapped(client, ADT7410_REG_TEMP[0]);
- if (temp < 0) {
- ret = temp;
- dev_dbg(dev, "Failed to read value: reg %d, error %d\n",
- ADT7410_REG_TEMP[0], ret);
- goto abort;
- }
- data->temp[0] = temp;
-
- data->last_updated = jiffies;
- data->valid = true;
- }
-
-abort:
- mutex_unlock(&data->update_lock);
- return ret;
-}
-
-static int adt7410_fill_cache(struct i2c_client *client)
-{
- struct adt7410_data *data = i2c_get_clientdata(client);
- int ret;
- int i;
-
- for (i = 1; i < ARRAY_SIZE(ADT7410_REG_TEMP); i++) {
- ret = i2c_smbus_read_word_swapped(client, ADT7410_REG_TEMP[i]);
- if (ret < 0) {
- dev_dbg(&client->dev,
- "Failed to read value: reg %d, error %d\n",
- ADT7410_REG_TEMP[0], ret);
- return ret;
- }
- data->temp[i] = ret;
- }
-
- ret = i2c_smbus_read_byte_data(client, ADT7410_T_HYST);
- if (ret < 0) {
- dev_dbg(&client->dev,
- "Failed to read value: hyst reg, error %d\n",
- ret);
- return ret;
- }
- data->hyst = ret;
-
- return 0;
+ return i2c_smbus_write_word_swapped(to_i2c_client(dev), reg, data);
}
-static s16 ADT7410_TEMP_TO_REG(long temp)
+static int adt7410_i2c_read_byte(struct device *dev, u8 reg)
{
- return DIV_ROUND_CLOSEST(clamp_val(temp, ADT7410_TEMP_MIN,
- ADT7410_TEMP_MAX) * 128, 1000);
+ return i2c_smbus_read_byte_data(to_i2c_client(dev), reg);
}
-static int ADT7410_REG_TO_TEMP(struct adt7410_data *data, s16 reg)
+static int adt7410_i2c_write_byte(struct device *dev, u8 reg, u8 data)
{
- /* in 13 bit mode, bits 0-2 are status flags - mask them out */
- if (!(data->config & ADT7410_RESOLUTION))
- reg &= ADT7410_T13_VALUE_MASK;
- /*
- * temperature is stored in twos complement format, in steps of
- * 1/128°C
- */
- return DIV_ROUND_CLOSEST(reg * 1000, 128);
+ return i2c_smbus_write_byte_data(to_i2c_client(dev), reg, data);
}
-/*-----------------------------------------------------------------------*/
-
-/* sysfs attributes for hwmon */
-
-static ssize_t adt7410_show_temp(struct device *dev,
- struct device_attribute *da, char *buf)
-{
- struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
- struct i2c_client *client = to_i2c_client(dev);
- struct adt7410_data *data = i2c_get_clientdata(client);
-
- if (attr->index == 0) {
- int ret;
-
- ret = adt7410_update_temp(dev);
- if (ret)
- return ret;
- }
-
- return sprintf(buf, "%d\n", ADT7410_REG_TO_TEMP(data,
- data->temp[attr->index]));
-}
-
-static ssize_t adt7410_set_temp(struct device *dev,
- struct device_attribute *da,
- const char *buf, size_t count)
-{
- struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
- struct i2c_client *client = to_i2c_client(dev);
- struct adt7410_data *data = i2c_get_clientdata(client);
- int nr = attr->index;
- long temp;
- int ret;
-
- ret = kstrtol(buf, 10, &temp);
- if (ret)
- return ret;
-
- mutex_lock(&data->update_lock);
- data->temp[nr] = ADT7410_TEMP_TO_REG(temp);
- ret = i2c_smbus_write_word_swapped(client, ADT7410_REG_TEMP[nr],
- data->temp[nr]);
- if (ret)
- count = ret;
- mutex_unlock(&data->update_lock);
- return count;
-}
-
-static ssize_t adt7410_show_t_hyst(struct device *dev,
- struct device_attribute *da,
- char *buf)
-{
- struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
- struct i2c_client *client = to_i2c_client(dev);
- struct adt7410_data *data = i2c_get_clientdata(client);
- int nr = attr->index;
- int hyst;
-
- hyst = (data->hyst & ADT7410_T_HYST_MASK) * 1000;
-
- /*
- * hysteresis is stored as a 4 bit offset in the device, convert it
- * to an absolute value
- */
- if (nr == 2) /* min has positive offset, others have negative */
- hyst = -hyst;
- return sprintf(buf, "%d\n",
- ADT7410_REG_TO_TEMP(data, data->temp[nr]) - hyst);
-}
-
-static ssize_t adt7410_set_t_hyst(struct device *dev,
- struct device_attribute *da,
- const char *buf, size_t count)
-{
- struct i2c_client *client = to_i2c_client(dev);
- struct adt7410_data *data = i2c_get_clientdata(client);
- int limit, ret;
- long hyst;
-
- ret = kstrtol(buf, 10, &hyst);
- if (ret)
- return ret;
- /* convert absolute hysteresis value to a 4 bit delta value */
- limit = ADT7410_REG_TO_TEMP(data, data->temp[1]);
- hyst = clamp_val(hyst, ADT7410_TEMP_MIN, ADT7410_TEMP_MAX);
- data->hyst = clamp_val(DIV_ROUND_CLOSEST(limit - hyst, 1000), 0,
- ADT7410_T_HYST_MASK);
- ret = i2c_smbus_write_byte_data(client, ADT7410_T_HYST, data->hyst);
- if (ret)
- return ret;
-
- return count;
-}
-
-static ssize_t adt7410_show_alarm(struct device *dev,
- struct device_attribute *da,
- char *buf)
-{
- struct i2c_client *client = to_i2c_client(dev);
- struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
- int ret;
-
- ret = i2c_smbus_read_byte_data(client, ADT7410_STATUS);
- if (ret < 0)
- return ret;
-
- return sprintf(buf, "%d\n", !!(ret & attr->index));
-}
-
-static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, adt7410_show_temp, NULL, 0);
-static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO,
- adt7410_show_temp, adt7410_set_temp, 1);
-static SENSOR_DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO,
- adt7410_show_temp, adt7410_set_temp, 2);
-static SENSOR_DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO,
- adt7410_show_temp, adt7410_set_temp, 3);
-static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO,
- adt7410_show_t_hyst, adt7410_set_t_hyst, 1);
-static SENSOR_DEVICE_ATTR(temp1_min_hyst, S_IRUGO,
- adt7410_show_t_hyst, NULL, 2);
-static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IRUGO,
- adt7410_show_t_hyst, NULL, 3);
-static SENSOR_DEVICE_ATTR(temp1_min_alarm, S_IRUGO, adt7410_show_alarm,
- NULL, ADT7410_STAT_T_LOW);
-static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO, adt7410_show_alarm,
- NULL, ADT7410_STAT_T_HIGH);
-static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, adt7410_show_alarm,
- NULL, ADT7410_STAT_T_CRIT);
-
-static struct attribute *adt7410_attributes[] = {
- &sensor_dev_attr_temp1_input.dev_attr.attr,
- &sensor_dev_attr_temp1_max.dev_attr.attr,
- &sensor_dev_attr_temp1_min.dev_attr.attr,
- &sensor_dev_attr_temp1_crit.dev_attr.attr,
- &sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
- &sensor_dev_attr_temp1_min_hyst.dev_attr.attr,
- &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
- &sensor_dev_attr_temp1_min_alarm.dev_attr.attr,
- &sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
- &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
- NULL
-};
-
-static const struct attribute_group adt7410_group = {
- .attrs = adt7410_attributes,
+static const struct adt7410_ops adt7410_i2c_ops = {
+ .read_word = adt7410_i2c_read_word,
+ .write_word = adt7410_i2c_write_word,
+ .read_byte = adt7410_i2c_read_byte,
+ .write_byte = adt7410_i2c_write_byte,
};
-/*-----------------------------------------------------------------------*/
-
-/* device probe and removal */
-
-static int adt7410_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int adt7410_i2c_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
{
- struct adt7410_data *data;
- int ret;
if (!i2c_check_functionality(client->adapter,
I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
return -ENODEV;
- data = devm_kzalloc(&client->dev, sizeof(struct adt7410_data),
- GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- i2c_set_clientdata(client, data);
- mutex_init(&data->update_lock);
-
- /* configure as specified */
- ret = i2c_smbus_read_byte_data(client, ADT7410_CONFIG);
- if (ret < 0) {
- dev_dbg(&client->dev, "Can't read config? %d\n", ret);
- return ret;
- }
- data->oldconfig = ret;
- /*
- * Set to 16 bit resolution, continous conversion and comparator mode.
- */
- ret &= ~ADT7410_MODE_MASK;
- data->config = ret | ADT7410_FULL | ADT7410_RESOLUTION |
- ADT7410_EVENT_MODE;
- if (data->config != data->oldconfig) {
- ret = i2c_smbus_write_byte_data(client, ADT7410_CONFIG,
- data->config);
- if (ret)
- return ret;
- }
- dev_dbg(&client->dev, "Config %02x\n", data->config);
-
- ret = adt7410_fill_cache(client);
- if (ret)
- goto exit_restore;
-
- /* Register sysfs hooks */
- ret = sysfs_create_group(&client->dev.kobj, &adt7410_group);
- if (ret)
- goto exit_restore;
-
- data->hwmon_dev = hwmon_device_register(&client->dev);
- if (IS_ERR(data->hwmon_dev)) {
- ret = PTR_ERR(data->hwmon_dev);
- goto exit_remove;
- }
-
- dev_info(&client->dev, "sensor '%s'\n", client->name);
-
- return 0;
-
-exit_remove:
- sysfs_remove_group(&client->dev.kobj, &adt7410_group);
-exit_restore:
- i2c_smbus_write_byte_data(client, ADT7410_CONFIG, data->oldconfig);
- return ret;
+ return adt7410_probe(&client->dev, NULL, &adt7410_i2c_ops);
}
-static int adt7410_remove(struct i2c_client *client)
+static int adt7410_i2c_remove(struct i2c_client *client)
{
- struct adt7410_data *data = i2c_get_clientdata(client);
-
- hwmon_device_unregister(data->hwmon_dev);
- sysfs_remove_group(&client->dev.kobj, &adt7410_group);
- if (data->oldconfig != data->config)
- i2c_smbus_write_byte_data(client, ADT7410_CONFIG,
- data->oldconfig);
- return 0;
+ return adt7410_remove(&client->dev);
}
static const struct i2c_device_id adt7410_ids[] = {
- { "adt7410", adt7410, },
- { "adt7420", adt7410, },
- { /* LIST END */ }
+ { "adt7410", 0 },
+ { "adt7420", 0 },
+ {}
};
MODULE_DEVICE_TABLE(i2c, adt7410_ids);
-#ifdef CONFIG_PM_SLEEP
-static int adt7410_suspend(struct device *dev)
-{
- int ret;
- struct i2c_client *client = to_i2c_client(dev);
- struct adt7410_data *data = i2c_get_clientdata(client);
-
- ret = i2c_smbus_write_byte_data(client, ADT7410_CONFIG,
- data->config | ADT7410_PD);
- return ret;
-}
-
-static int adt7410_resume(struct device *dev)
-{
- int ret;
- struct i2c_client *client = to_i2c_client(dev);
- struct adt7410_data *data = i2c_get_clientdata(client);
-
- ret = i2c_smbus_write_byte_data(client, ADT7410_CONFIG, data->config);
- return ret;
-}
-
-static SIMPLE_DEV_PM_OPS(adt7410_dev_pm_ops, adt7410_suspend, adt7410_resume);
-
-#define ADT7410_DEV_PM_OPS (&adt7410_dev_pm_ops)
-#else
-#define ADT7410_DEV_PM_OPS NULL
-#endif /* CONFIG_PM */
-
static struct i2c_driver adt7410_driver = {
.class = I2C_CLASS_HWMON,
.driver = {
.name = "adt7410",
.pm = ADT7410_DEV_PM_OPS,
},
- .probe = adt7410_probe,
- .remove = adt7410_remove,
+ .probe = adt7410_i2c_probe,
+ .remove = adt7410_i2c_remove,
.id_table = adt7410_ids,
.address_list = I2C_ADDRS(0x48, 0x49, 0x4a, 0x4b),
};
-
module_i2c_driver(adt7410_driver);
-MODULE_AUTHOR("Hartmut Knaack");
-MODULE_DESCRIPTION("ADT7410/ADT7420 driver");
+MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
+MODULE_DESCRIPTION("ADT7410/AD7420 driver");
MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/adt7x10.c b/drivers/hwmon/adt7x10.c
new file mode 100644
index 0000000..eeff198c
--- /dev/null
+++ b/drivers/hwmon/adt7x10.c
@@ -0,0 +1,476 @@
+/*
+ * adt7410.c - Part of lm_sensors, Linux kernel modules for hardware
+ * monitoring
+ * This driver handles the ADT7410 and compatible digital temperature sensors.
+ * Hartmut Knaack <knaack.h@gmx.de> 2012-07-22
+ * based on lm75.c by Frodo Looijaard <frodol@dds.nl>
+ * and adt7410.c from iio-staging by Sonic Zhang <sonic.zhang@analog.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 2 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, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/delay.h>
+
+#include "adt7x10.h"
+
+/*
+ * ADT7410 status
+ */
+#define ADT7410_STAT_T_LOW (1 << 4)
+#define ADT7410_STAT_T_HIGH (1 << 5)
+#define ADT7410_STAT_T_CRIT (1 << 6)
+#define ADT7410_STAT_NOT_RDY (1 << 7)
+
+/*
+ * ADT7410 config
+ */
+#define ADT7410_FAULT_QUEUE_MASK (1 << 0 | 1 << 1)
+#define ADT7410_CT_POLARITY (1 << 2)
+#define ADT7410_INT_POLARITY (1 << 3)
+#define ADT7410_EVENT_MODE (1 << 4)
+#define ADT7410_MODE_MASK (1 << 5 | 1 << 6)
+#define ADT7410_FULL (0 << 5 | 0 << 6)
+#define ADT7410_PD (1 << 5 | 1 << 6)
+#define ADT7410_RESOLUTION (1 << 7)
+
+/*
+ * ADT7410 masks
+ */
+#define ADT7410_T13_VALUE_MASK 0xFFF8
+#define ADT7410_T_HYST_MASK 0xF
+
+/* straight from the datasheet */
+#define ADT7410_TEMP_MIN (-55000)
+#define ADT7410_TEMP_MAX 150000
+
+/* Each client has this additional data */
+struct adt7410_data {
+ const struct adt7410_ops *ops;
+ const char *name;
+ struct device *hwmon_dev;
+ struct mutex update_lock;
+ u8 config;
+ u8 oldconfig;
+ bool valid; /* true if registers valid */
+ unsigned long last_updated; /* In jiffies */
+ s16 temp[4]; /* Register values,
+ 0 = input
+ 1 = high
+ 2 = low
+ 3 = critical */
+ u8 hyst; /* hysteresis offset */
+};
+
+static int adt7410_read_byte(struct device *dev, u8 reg)
+{
+ struct adt7410_data *d = dev_get_drvdata(dev);
+ return d->ops->read_byte(dev, reg);
+}
+
+static int adt7410_write_byte(struct device *dev, u8 reg, u8 data)
+{
+ struct adt7410_data *d = dev_get_drvdata(dev);
+ return d->ops->write_byte(dev, reg, data);
+}
+
+static int adt7410_read_word(struct device *dev, u8 reg)
+{
+ struct adt7410_data *d = dev_get_drvdata(dev);
+ return d->ops->read_word(dev, reg);
+}
+
+static int adt7410_write_word(struct device *dev, u8 reg, u16 data)
+{
+ struct adt7410_data *d = dev_get_drvdata(dev);
+ return d->ops->write_word(dev, reg, data);
+}
+
+static const u8 ADT7410_REG_TEMP[4] = {
+ ADT7410_TEMPERATURE, /* input */
+ ADT7410_T_ALARM_HIGH, /* high */
+ ADT7410_T_ALARM_LOW, /* low */
+ ADT7410_T_CRIT, /* critical */
+};
+
+static int adt7410_temp_ready(struct device *dev)
+{
+ int i, status;
+
+ for (i = 0; i < 6; i++) {
+ status = adt7410_read_byte(dev, ADT7410_STATUS);
+ if (status < 0)
+ return status;
+ if (!(status & ADT7410_STAT_NOT_RDY))
+ return 0;
+ msleep(60);
+ }
+ return -ETIMEDOUT;
+}
+
+static int adt7410_update_temp(struct device *dev)
+{
+ struct adt7410_data *data = dev_get_drvdata(dev);
+ int ret = 0;
+
+ mutex_lock(&data->update_lock);
+
+ if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
+ || !data->valid) {
+ int temp;
+
+ dev_dbg(dev, "Starting update\n");
+
+ ret = adt7410_temp_ready(dev); /* check for new value */
+ if (ret)
+ goto abort;
+
+ temp = adt7410_read_word(dev, ADT7410_REG_TEMP[0]);
+ if (temp < 0) {
+ ret = temp;
+ dev_dbg(dev, "Failed to read value: reg %d, error %d\n",
+ ADT7410_REG_TEMP[0], ret);
+ goto abort;
+ }
+ data->temp[0] = temp;
+ data->last_updated = jiffies;
+ data->valid = true;
+ }
+
+abort:
+ mutex_unlock(&data->update_lock);
+ return ret;
+}
+
+static int adt7410_fill_cache(struct device *dev)
+{
+ struct adt7410_data *data = dev_get_drvdata(dev);
+ int ret;
+ int i;
+
+ for (i = 1; i < ARRAY_SIZE(data->temp); i++) {
+ ret = adt7410_read_word(dev, ADT7410_REG_TEMP[i]);
+ if (ret < 0) {
+ dev_dbg(dev, "Failed to read value: reg %d, error %d\n",
+ ADT7410_REG_TEMP[0], ret);
+ return ret;
+ }
+ data->temp[i] = ret;
+ }
+
+ ret = adt7410_read_byte(dev, ADT7410_T_HYST);
+ if (ret < 0) {
+ dev_dbg(dev, "Failed to read value: reg %d, error %d\n",
+ ADT7410_T_HYST, ret);
+ return ret;
+ }
+ data->hyst = ret;
+
+ return 0;
+}
+
+static s16 ADT7410_TEMP_TO_REG(long temp)
+{
+ return DIV_ROUND_CLOSEST(clamp_val(temp, ADT7410_TEMP_MIN,
+ ADT7410_TEMP_MAX) * 128, 1000);
+}
+
+static int ADT7410_REG_TO_TEMP(struct adt7410_data *data, s16 reg)
+{
+ /* in 13 bit mode, bits 0-2 are status flags - mask them out */
+ if (!(data->config & ADT7410_RESOLUTION))
+ reg &= ADT7410_T13_VALUE_MASK;
+ /*
+ * temperature is stored in twos complement format, in steps of
+ * 1/128°C
+ */
+ return DIV_ROUND_CLOSEST(reg * 1000, 128);
+}
+
+/*-----------------------------------------------------------------------*/
+
+/* sysfs attributes for hwmon */
+
+static ssize_t adt7410_show_temp(struct device *dev,
+ struct device_attribute *da, char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+ struct adt7410_data *data = dev_get_drvdata(dev);
+
+
+ if (attr->index == 0) {
+ int ret;
+
+ ret = adt7410_update_temp(dev);
+ if (ret)
+ return ret;
+ }
+
+ return sprintf(buf, "%d\n", ADT7410_REG_TO_TEMP(data,
+ data->temp[attr->index]));
+}
+
+static ssize_t adt7410_set_temp(struct device *dev,
+ struct device_attribute *da,
+ const char *buf, size_t count)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+ struct adt7410_data *data = dev_get_drvdata(dev);
+ int nr = attr->index;
+ long temp;
+ int ret;
+
+ ret = kstrtol(buf, 10, &temp);
+ if (ret)
+ return ret;
+
+ mutex_lock(&data->update_lock);
+ data->temp[nr] = ADT7410_TEMP_TO_REG(temp);
+ ret = adt7410_write_word(dev, ADT7410_REG_TEMP[nr], data->temp[nr]);
+ if (ret)
+ count = ret;
+ mutex_unlock(&data->update_lock);
+ return count;
+}
+
+static ssize_t adt7410_show_t_hyst(struct device *dev,
+ struct device_attribute *da,
+ char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+ struct adt7410_data *data = dev_get_drvdata(dev);
+ int nr = attr->index;
+ int hyst;
+
+ hyst = (data->hyst & ADT7410_T_HYST_MASK) * 1000;
+
+ /*
+ * hysteresis is stored as a 4 bit offset in the device, convert it
+ * to an absolute value
+ */
+ if (nr == 2) /* min has positive offset, others have negative */
+ hyst = -hyst;
+ return sprintf(buf, "%d\n",
+ ADT7410_REG_TO_TEMP(data, data->temp[nr]) - hyst);
+}
+
+static ssize_t adt7410_set_t_hyst(struct device *dev,
+ struct device_attribute *da,
+ const char *buf, size_t count)
+{
+ struct adt7410_data *data = dev_get_drvdata(dev);
+ int limit, ret;
+ long hyst;
+
+ ret = kstrtol(buf, 10, &hyst);
+ if (ret)
+ return ret;
+ /* convert absolute hysteresis value to a 4 bit delta value */
+ limit = ADT7410_REG_TO_TEMP(data, data->temp[1]);
+ hyst = clamp_val(hyst, ADT7410_TEMP_MIN, ADT7410_TEMP_MAX);
+ data->hyst = clamp_val(DIV_ROUND_CLOSEST(limit - hyst, 1000),
+ 0, ADT7410_T_HYST_MASK);
+ ret = adt7410_write_byte(dev, ADT7410_T_HYST, data->hyst);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t adt7410_show_alarm(struct device *dev,
+ struct device_attribute *da,
+ char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+ int ret;
+
+ ret = adt7410_read_byte(dev, ADT7410_STATUS);
+ if (ret < 0)
+ return ret;
+
+ return sprintf(buf, "%d\n", !!(ret & attr->index));
+}
+
+static ssize_t adt7410_show_name(struct device *dev,
+ struct device_attribute *da, char *buf)
+{
+ struct adt7410_data *data = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%s\n", data->name);
+}
+
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, adt7410_show_temp, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO,
+ adt7410_show_temp, adt7410_set_temp, 1);
+static SENSOR_DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO,
+ adt7410_show_temp, adt7410_set_temp, 2);
+static SENSOR_DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO,
+ adt7410_show_temp, adt7410_set_temp, 3);
+static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO,
+ adt7410_show_t_hyst, adt7410_set_t_hyst, 1);
+static SENSOR_DEVICE_ATTR(temp1_min_hyst, S_IRUGO,
+ adt7410_show_t_hyst, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IRUGO,
+ adt7410_show_t_hyst, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp1_min_alarm, S_IRUGO, adt7410_show_alarm,
+ NULL, ADT7410_STAT_T_LOW);
+static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO, adt7410_show_alarm,
+ NULL, ADT7410_STAT_T_HIGH);
+static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, adt7410_show_alarm,
+ NULL, ADT7410_STAT_T_CRIT);
+static DEVICE_ATTR(name, S_IRUGO, adt7410_show_name, NULL);
+
+static struct attribute *adt7410_attributes[] = {
+ &sensor_dev_attr_temp1_input.dev_attr.attr,
+ &sensor_dev_attr_temp1_max.dev_attr.attr,
+ &sensor_dev_attr_temp1_min.dev_attr.attr,
+ &sensor_dev_attr_temp1_crit.dev_attr.attr,
+ &sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
+ &sensor_dev_attr_temp1_min_hyst.dev_attr.attr,
+ &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
+ &sensor_dev_attr_temp1_min_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group adt7410_group = {
+ .attrs = adt7410_attributes,
+};
+
+int adt7410_probe(struct device *dev, const char *name,
+ const struct adt7410_ops *ops)
+{
+ struct adt7410_data *data;
+ int ret;
+
+ data = devm_kzalloc(dev, sizeof(struct adt7410_data),
+ GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->ops = ops;
+ data->name = name;
+
+ dev_set_drvdata(dev, data);
+ mutex_init(&data->update_lock);
+
+ /* configure as specified */
+ ret = adt7410_read_byte(dev, ADT7410_CONFIG);
+ if (ret < 0) {
+ dev_dbg(dev, "Can't read config? %d\n", ret);
+ return ret;
+ }
+ data->oldconfig = ret;
+
+ /*
+ * Set to 16 bit resolution, continous conversion and comparator mode.
+ */
+ data->config = data->oldconfig;
+ data->config &= ~ADT7410_MODE_MASK;
+ data->config |= ADT7410_FULL | ADT7410_RESOLUTION | ADT7410_EVENT_MODE;
+ if (data->config != data->oldconfig) {
+ ret = adt7410_write_byte(dev, ADT7410_CONFIG, data->config);
+ if (ret)
+ return ret;
+ }
+ dev_dbg(dev, "Config %02x\n", data->config);
+
+ ret = adt7410_fill_cache(dev);
+ if (ret)
+ goto exit_restore;
+
+ /* Register sysfs hooks */
+ ret = sysfs_create_group(&dev->kobj, &adt7410_group);
+ if (ret)
+ goto exit_restore;
+
+ /*
+ * The I2C device will already have it's own 'name' attribute, but for
+ * the SPI device we need to register it. name will only be non NULL if
+ * the device doesn't register the 'name' attribute on its own.
+ */
+ if (name) {
+ ret = device_create_file(dev, &dev_attr_name);
+ if (ret)
+ goto exit_remove;
+ }
+
+ data->hwmon_dev = hwmon_device_register(dev);
+ if (IS_ERR(data->hwmon_dev)) {
+ ret = PTR_ERR(data->hwmon_dev);
+ goto exit_remove_name;
+ }
+
+ return 0;
+
+exit_remove_name:
+ if (name)
+ device_remove_file(dev, &dev_attr_name);
+exit_remove:
+ sysfs_remove_group(&dev->kobj, &adt7410_group);
+exit_restore:
+ adt7410_write_byte(dev, ADT7410_CONFIG, data->oldconfig);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(adt7410_probe);
+
+int adt7410_remove(struct device *dev)
+{
+ struct adt7410_data *data = dev_get_drvdata(dev);
+
+ hwmon_device_unregister(data->hwmon_dev);
+ if (data->name)
+ device_remove_file(dev, &dev_attr_name);
+ sysfs_remove_group(&dev->kobj, &adt7410_group);
+ if (data->oldconfig != data->config)
+ adt7410_write_byte(dev, ADT7410_CONFIG,
+ data->oldconfig);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(adt7410_remove);
+
+#ifdef CONFIG_PM_SLEEP
+
+static int adt7410_suspend(struct device *dev)
+{
+ struct adt7410_data *data = dev_get_drvdata(dev);
+
+ return adt7410_write_byte(dev, ADT7410_CONFIG,
+ data->config | ADT7410_PD);
+}
+
+static int adt7410_resume(struct device *dev)
+{
+ struct adt7410_data *data = dev_get_drvdata(dev);
+
+ return adt7410_write_byte(dev, ADT7410_CONFIG, data->config);
+}
+
+SIMPLE_DEV_PM_OPS(adt7410_dev_pm_ops, adt7410_suspend, adt7410_resume);
+EXPORT_SYMBOL_GPL(adt7410_dev_pm_ops);
+
+#endif /* CONFIG_PM_SLEEP */
+
+MODULE_AUTHOR("Hartmut Knaack");
+MODULE_DESCRIPTION("ADT7410/ADT7420, ADT7310/ADT7320 common code");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/adt7x10.h b/drivers/hwmon/adt7x10.h
new file mode 100644
index 0000000..a7165e6
--- /dev/null
+++ b/drivers/hwmon/adt7x10.h
@@ -0,0 +1,48 @@
+#ifndef __HWMON_ADT7X10_H__
+#define __HWMON_ADT7X10_H__
+
+#include <linux/types.h>
+#include <linux/pm.h>
+
+/* ADT7410 registers definition */
+#define ADT7410_TEMPERATURE 0
+#define ADT7410_STATUS 2
+#define ADT7410_CONFIG 3
+#define ADT7410_T_ALARM_HIGH 4
+#define ADT7410_T_ALARM_LOW 6
+#define ADT7410_T_CRIT 8
+#define ADT7410_T_HYST 0xA
+#define ADT7410_ID 0xB
+
+/* ADT7310 registers definition */
+#define ADT7310_STATUS 0
+#define ADT7310_CONFIG 1
+#define ADT7310_TEMPERATURE 2
+#define ADT7310_ID 3
+#define ADT7310_T_CRIT 4
+#define ADT7310_T_HYST 5
+#define ADT7310_T_ALARM_HIGH 6
+#define ADT7310_T_ALARM_LOW 7
+
+struct device;
+
+struct adt7410_ops {
+ int (*read_byte)(struct device *, u8 reg);
+ int (*write_byte)(struct device *, u8 reg, u8 data);
+ int (*read_word)(struct device *, u8 reg);
+ int (*write_word)(struct device *, u8 reg, u16 data);
+};
+
+int adt7410_probe(struct device *dev, const char *name,
+ const struct adt7410_ops *ops);
+int adt7410_remove(struct device *dev);
+
+
+#ifdef CONFIG_PM_SLEEP
+extern const struct dev_pm_ops adt7410_dev_pm_ops;
+#define ADT7410_DEV_PM_OPS (&adt7410_dev_pm_ops)
+#else
+#define ADT7410_DEV_PM_OPS NULL
+#endif
+
+#endif
--
1.8.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 3/4] hwmon: (adt7x10) Add alarm interrupt support
2013-02-18 13:38 [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers Lars-Peter Clausen
2013-02-18 13:38 ` [PATCH v2 2/4] hwmon: (adt7410) Add support for the adt7310/adt7320 Lars-Peter Clausen
@ 2013-02-18 13:38 ` Lars-Peter Clausen
2013-02-19 1:39 ` Guenter Roeck
2013-02-18 13:38 ` [PATCH v2 4/4] staging:iio: Remove adt7410 driver Lars-Peter Clausen
` (2 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: Lars-Peter Clausen @ 2013-02-18 13:38 UTC (permalink / raw)
To: Jean Delvare, Guenter Roeck
Cc: Hartmut Knaack, Jonathan Cameron, lm-sensors, linux-iio,
Lars-Peter Clausen
This allows a userspace application to poll() on the alarm files to get notified
in case of an temperature threshold event.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
Changes since v1:
* Switch from level triggered and interrupt mode to edge triggered and
comparator mode. In interrupt mode the status register get's cleared after
the first read and so the _alarm files will always read 0, which is not
really what we want.
* Use dev_name(dev) for the interrupt name, instead of 'name' which will be
NULL for I2C devices
---
drivers/hwmon/adt7310.c | 4 ++--
drivers/hwmon/adt7410.c | 4 ++--
drivers/hwmon/adt7x10.c | 43 +++++++++++++++++++++++++++++++++++++++----
drivers/hwmon/adt7x10.h | 4 ++--
4 files changed, 45 insertions(+), 10 deletions(-)
diff --git a/drivers/hwmon/adt7310.c b/drivers/hwmon/adt7310.c
index 2196ac3..e58bb68 100644
--- a/drivers/hwmon/adt7310.c
+++ b/drivers/hwmon/adt7310.c
@@ -82,13 +82,13 @@ static const struct adt7410_ops adt7310_spi_ops = {
static int adt7310_spi_probe(struct spi_device *spi)
{
- return adt7410_probe(&spi->dev, spi_get_device_id(spi)->name,
+ return adt7410_probe(&spi->dev, spi_get_device_id(spi)->name, spi->irq,
&adt7310_spi_ops);
}
static int adt7310_spi_remove(struct spi_device *spi)
{
- return adt7410_remove(&spi->dev);
+ return adt7410_remove(&spi->dev, spi->irq);
}
static const struct spi_device_id adt7310_id[] = {
diff --git a/drivers/hwmon/adt7410.c b/drivers/hwmon/adt7410.c
index b500ab3..3a7d905 100644
--- a/drivers/hwmon/adt7410.c
+++ b/drivers/hwmon/adt7410.c
@@ -48,12 +48,12 @@ static int adt7410_i2c_probe(struct i2c_client *client,
I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
return -ENODEV;
- return adt7410_probe(&client->dev, NULL, &adt7410_i2c_ops);
+ return adt7410_probe(&client->dev, NULL, client->irq, &adt7410_i2c_ops);
}
static int adt7410_i2c_remove(struct i2c_client *client)
{
- return adt7410_remove(&client->dev);
+ return adt7410_remove(&client->dev, client->irq);
}
static const struct i2c_device_id adt7410_ids[] = {
diff --git a/drivers/hwmon/adt7x10.c b/drivers/hwmon/adt7x10.c
index eeff198c..2340a70 100644
--- a/drivers/hwmon/adt7x10.c
+++ b/drivers/hwmon/adt7x10.c
@@ -30,6 +30,7 @@
#include <linux/err.h>
#include <linux/mutex.h>
#include <linux/delay.h>
+#include <linux/interrupt.h>
#include "adt7x10.h"
@@ -112,6 +113,25 @@ static const u8 ADT7410_REG_TEMP[4] = {
ADT7410_T_CRIT, /* critical */
};
+static irqreturn_t adt7410_irq_handler(int irq, void *private)
+{
+ struct device *dev = private;
+ int status;
+
+ status = adt7410_read_byte(dev, ADT7410_STATUS);
+ if (status < 0)
+ return IRQ_HANDLED;
+
+ if (status & ADT7410_STAT_T_HIGH)
+ sysfs_notify(&dev->kobj, NULL, "temp1_max_alarm");
+ if (status & ADT7410_STAT_T_LOW)
+ sysfs_notify(&dev->kobj, NULL, "temp1_min_alarm");
+ if (status & ADT7410_STAT_T_CRIT)
+ sysfs_notify(&dev->kobj, NULL, "temp1_crit_alarm");
+
+ return IRQ_HANDLED;
+}
+
static int adt7410_temp_ready(struct device *dev)
{
int i, status;
@@ -357,7 +377,7 @@ static const struct attribute_group adt7410_group = {
.attrs = adt7410_attributes,
};
-int adt7410_probe(struct device *dev, const char *name,
+int adt7410_probe(struct device *dev, const char *name, int irq,
const struct adt7410_ops *ops)
{
struct adt7410_data *data;
@@ -383,11 +403,13 @@ int adt7410_probe(struct device *dev, const char *name,
data->oldconfig = ret;
/*
- * Set to 16 bit resolution, continous conversion and comparator mode.
+ * Set to 16 bit resolution and continous conversion
*/
data->config = data->oldconfig;
- data->config &= ~ADT7410_MODE_MASK;
+ data->config &= ~(ADT7410_MODE_MASK | ADT7410_CT_POLARITY |
+ ADT7410_INT_POLARITY);
data->config |= ADT7410_FULL | ADT7410_RESOLUTION | ADT7410_EVENT_MODE;
+
if (data->config != data->oldconfig) {
ret = adt7410_write_byte(dev, ADT7410_CONFIG, data->config);
if (ret)
@@ -421,8 +443,18 @@ int adt7410_probe(struct device *dev, const char *name,
goto exit_remove_name;
}
+ if (irq > 0) {
+ ret = request_threaded_irq(irq, NULL, adt7410_irq_handler,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ dev_name(dev), dev);
+ if (ret)
+ goto exit_hwmon_device_unregister;
+ }
+
return 0;
+exit_hwmon_device_unregister:
+ hwmon_device_unregister(data->hwmon_dev);
exit_remove_name:
if (name)
device_remove_file(dev, &dev_attr_name);
@@ -434,10 +466,13 @@ exit_restore:
}
EXPORT_SYMBOL_GPL(adt7410_probe);
-int adt7410_remove(struct device *dev)
+int adt7410_remove(struct device *dev, int irq)
{
struct adt7410_data *data = dev_get_drvdata(dev);
+ if (irq > 0)
+ free_irq(irq, dev);
+
hwmon_device_unregister(data->hwmon_dev);
if (data->name)
device_remove_file(dev, &dev_attr_name);
diff --git a/drivers/hwmon/adt7x10.h b/drivers/hwmon/adt7x10.h
index a7165e6..d67badf 100644
--- a/drivers/hwmon/adt7x10.h
+++ b/drivers/hwmon/adt7x10.h
@@ -33,9 +33,9 @@ struct adt7410_ops {
int (*write_word)(struct device *, u8 reg, u16 data);
};
-int adt7410_probe(struct device *dev, const char *name,
+int adt7410_probe(struct device *dev, const char *name, int irq,
const struct adt7410_ops *ops);
-int adt7410_remove(struct device *dev);
+int adt7410_remove(struct device *dev, int irq);
#ifdef CONFIG_PM_SLEEP
--
1.8.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 4/4] staging:iio: Remove adt7410 driver
2013-02-18 13:38 [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers Lars-Peter Clausen
2013-02-18 13:38 ` [PATCH v2 2/4] hwmon: (adt7410) Add support for the adt7310/adt7320 Lars-Peter Clausen
2013-02-18 13:38 ` [PATCH v2 3/4] hwmon: (adt7x10) Add alarm interrupt support Lars-Peter Clausen
@ 2013-02-18 13:38 ` Lars-Peter Clausen
2013-03-02 16:45 ` Jonathan Cameron
2013-02-18 20:22 ` [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers Hartmut Knaack
2013-02-19 1:32 ` Guenter Roeck
4 siblings, 1 reply; 23+ messages in thread
From: Lars-Peter Clausen @ 2013-02-18 13:38 UTC (permalink / raw)
To: Jean Delvare, Guenter Roeck
Cc: Hartmut Knaack, Jonathan Cameron, lm-sensors, linux-iio,
Lars-Peter Clausen
The adt7410 hwmon driver is feature wise more or less on par with the IIO
driver. So we can finally remove the IIO driver.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/staging/iio/adc/Kconfig | 7 -
drivers/staging/iio/adc/Makefile | 1 -
drivers/staging/iio/adc/adt7410.c | 1102 -------------------------------------
3 files changed, 1110 deletions(-)
delete mode 100644 drivers/staging/iio/adc/adt7410.c
diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
index fb8c239..029ae54 100644
--- a/drivers/staging/iio/adc/Kconfig
+++ b/drivers/staging/iio/adc/Kconfig
@@ -90,13 +90,6 @@ config AD7192
To compile this driver as a module, choose M here: the
module will be called ad7192.
-config ADT7410
- tristate "Analog Devices ADT7310/ADT7410 temperature sensor driver"
- depends on I2C || SPI_MASTER
- help
- Say yes here to build support for Analog Devices ADT7310/ADT7410
- temperature sensors.
-
config AD7280
tristate "Analog Devices AD7280A Lithium Ion Battery Monitoring System"
depends on SPI
diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
index d285596..3e9fb14 100644
--- a/drivers/staging/iio/adc/Makefile
+++ b/drivers/staging/iio/adc/Makefile
@@ -16,7 +16,6 @@ obj-$(CONFIG_AD7291) += ad7291.o
obj-$(CONFIG_AD7780) += ad7780.o
obj-$(CONFIG_AD7816) += ad7816.o
obj-$(CONFIG_AD7192) += ad7192.o
-obj-$(CONFIG_ADT7410) += adt7410.o
obj-$(CONFIG_AD7280) += ad7280a.o
obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o
obj-$(CONFIG_MXS_LRADC) += mxs-lradc.o
diff --git a/drivers/staging/iio/adc/adt7410.c b/drivers/staging/iio/adc/adt7410.c
deleted file mode 100644
index 35455e1..0000000
--- a/drivers/staging/iio/adc/adt7410.c
+++ /dev/null
@@ -1,1102 +0,0 @@
-/*
- * ADT7410 digital temperature sensor driver supporting ADT7310/ADT7410
- *
- * Copyright 2010 Analog Devices Inc.
- *
- * Licensed under the GPL-2 or later.
- */
-
-#include <linux/interrupt.h>
-#include <linux/device.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/sysfs.h>
-#include <linux/list.h>
-#include <linux/i2c.h>
-#include <linux/spi/spi.h>
-#include <linux/module.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
-#include <linux/iio/events.h>
-
-/*
- * ADT7410 registers definition
- */
-
-#define ADT7410_TEMPERATURE 0
-#define ADT7410_STATUS 2
-#define ADT7410_CONFIG 3
-#define ADT7410_T_ALARM_HIGH 4
-#define ADT7410_T_ALARM_LOW 6
-#define ADT7410_T_CRIT 8
-#define ADT7410_T_HYST 0xA
-#define ADT7410_ID 0xB
-#define ADT7410_RESET 0x2F
-
-/*
- * ADT7310 registers definition
- */
-
-#define ADT7310_STATUS 0
-#define ADT7310_CONFIG 1
-#define ADT7310_TEMPERATURE 2
-#define ADT7310_ID 3
-#define ADT7310_T_CRIT 4
-#define ADT7310_T_HYST 5
-#define ADT7310_T_ALARM_HIGH 6
-#define ADT7310_T_ALARM_LOW 7
-
-/*
- * ADT7410 status
- */
-#define ADT7410_STAT_T_LOW 0x10
-#define ADT7410_STAT_T_HIGH 0x20
-#define ADT7410_STAT_T_CRIT 0x40
-#define ADT7410_STAT_NOT_RDY 0x80
-
-/*
- * ADT7410 config
- */
-#define ADT7410_FAULT_QUEUE_MASK 0x3
-#define ADT7410_CT_POLARITY 0x4
-#define ADT7410_INT_POLARITY 0x8
-#define ADT7410_EVENT_MODE 0x10
-#define ADT7410_MODE_MASK 0x60
-#define ADT7410_ONESHOT 0x20
-#define ADT7410_SPS 0x40
-#define ADT7410_PD 0x60
-#define ADT7410_RESOLUTION 0x80
-
-/*
- * ADT7410 masks
- */
-#define ADT7410_T16_VALUE_SIGN 0x8000
-#define ADT7410_T16_VALUE_FLOAT_OFFSET 7
-#define ADT7410_T16_VALUE_FLOAT_MASK 0x7F
-#define ADT7410_T13_VALUE_SIGN 0x1000
-#define ADT7410_T13_VALUE_OFFSET 3
-#define ADT7410_T13_VALUE_FLOAT_OFFSET 4
-#define ADT7410_T13_VALUE_FLOAT_MASK 0xF
-#define ADT7410_T_HYST_MASK 0xF
-#define ADT7410_DEVICE_ID_MASK 0xF
-#define ADT7410_MANUFACTORY_ID_MASK 0xF0
-#define ADT7410_MANUFACTORY_ID_OFFSET 4
-
-
-#define ADT7310_CMD_REG_MASK 0x28
-#define ADT7310_CMD_REG_OFFSET 3
-#define ADT7310_CMD_READ 0x40
-#define ADT7310_CMD_CON_READ 0x4
-
-#define ADT7410_IRQS 2
-
-/*
- * struct adt7410_chip_info - chip specifc information
- */
-
-struct adt7410_chip_info;
-
-struct adt7410_ops {
- int (*read_word)(struct adt7410_chip_info *, u8 reg, u16 *data);
- int (*write_word)(struct adt7410_chip_info *, u8 reg, u16 data);
- int (*read_byte)(struct adt7410_chip_info *, u8 reg, u8 *data);
- int (*write_byte)(struct adt7410_chip_info *, u8 reg, u8 data);
-};
-
-struct adt7410_chip_info {
- struct device *dev;
- u8 config;
-
- const struct adt7410_ops *ops;
-};
-
-static int adt7410_read_word(struct adt7410_chip_info *chip, u8 reg, u16 *data)
-{
- return chip->ops->read_word(chip, reg, data);
-}
-
-static int adt7410_write_word(struct adt7410_chip_info *chip, u8 reg, u16 data)
-{
- return chip->ops->write_word(chip, reg, data);
-}
-
-static int adt7410_read_byte(struct adt7410_chip_info *chip, u8 reg, u8 *data)
-{
- return chip->ops->read_byte(chip, reg, data);
-}
-
-static int adt7410_write_byte(struct adt7410_chip_info *chip, u8 reg, u8 data)
-{
- return chip->ops->write_byte(chip, reg, data);
-}
-
-static ssize_t adt7410_show_mode(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct iio_dev *dev_info = dev_to_iio_dev(dev);
- struct adt7410_chip_info *chip = iio_priv(dev_info);
- u8 config;
-
- config = chip->config & ADT7410_MODE_MASK;
-
- switch (config) {
- case ADT7410_PD:
- return sprintf(buf, "power-down\n");
- case ADT7410_ONESHOT:
- return sprintf(buf, "one-shot\n");
- case ADT7410_SPS:
- return sprintf(buf, "sps\n");
- default:
- return sprintf(buf, "full\n");
- }
-}
-
-static ssize_t adt7410_store_mode(struct device *dev,
- struct device_attribute *attr,
- const char *buf,
- size_t len)
-{
- struct iio_dev *dev_info = dev_to_iio_dev(dev);
- struct adt7410_chip_info *chip = iio_priv(dev_info);
- u16 config;
- int ret;
-
- ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
- if (ret)
- return -EIO;
-
- config = chip->config & (~ADT7410_MODE_MASK);
- if (strcmp(buf, "power-down"))
- config |= ADT7410_PD;
- else if (strcmp(buf, "one-shot"))
- config |= ADT7410_ONESHOT;
- else if (strcmp(buf, "sps"))
- config |= ADT7410_SPS;
-
- ret = adt7410_write_byte(chip, ADT7410_CONFIG, config);
- if (ret)
- return -EIO;
-
- chip->config = config;
-
- return len;
-}
-
-static IIO_DEVICE_ATTR(mode, S_IRUGO | S_IWUSR,
- adt7410_show_mode,
- adt7410_store_mode,
- 0);
-
-static ssize_t adt7410_show_available_modes(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- return sprintf(buf, "full\none-shot\nsps\npower-down\n");
-}
-
-static IIO_DEVICE_ATTR(available_modes, S_IRUGO, adt7410_show_available_modes, NULL, 0);
-
-static ssize_t adt7410_show_resolution(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct iio_dev *dev_info = dev_to_iio_dev(dev);
- struct adt7410_chip_info *chip = iio_priv(dev_info);
- int ret;
- int bits;
-
- ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
- if (ret)
- return -EIO;
-
- if (chip->config & ADT7410_RESOLUTION)
- bits = 16;
- else
- bits = 13;
-
- return sprintf(buf, "%d bits\n", bits);
-}
-
-static ssize_t adt7410_store_resolution(struct device *dev,
- struct device_attribute *attr,
- const char *buf,
- size_t len)
-{
- struct iio_dev *dev_info = dev_to_iio_dev(dev);
- struct adt7410_chip_info *chip = iio_priv(dev_info);
- unsigned long data;
- u16 config;
- int ret;
-
- ret = strict_strtoul(buf, 10, &data);
- if (ret)
- return -EINVAL;
-
- ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
- if (ret)
- return -EIO;
-
- config = chip->config & (~ADT7410_RESOLUTION);
- if (data)
- config |= ADT7410_RESOLUTION;
-
- ret = adt7410_write_byte(chip, ADT7410_CONFIG, config);
- if (ret)
- return -EIO;
-
- chip->config = config;
-
- return len;
-}
-
-static IIO_DEVICE_ATTR(resolution, S_IRUGO | S_IWUSR,
- adt7410_show_resolution,
- adt7410_store_resolution,
- 0);
-
-static ssize_t adt7410_show_id(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct iio_dev *dev_info = dev_to_iio_dev(dev);
- struct adt7410_chip_info *chip = iio_priv(dev_info);
- u8 id;
- int ret;
-
- ret = adt7410_read_byte(chip, ADT7410_ID, &id);
- if (ret)
- return -EIO;
-
- return sprintf(buf, "device id: 0x%x\nmanufactory id: 0x%x\n",
- id & ADT7410_DEVICE_ID_MASK,
- (id & ADT7410_MANUFACTORY_ID_MASK) >> ADT7410_MANUFACTORY_ID_OFFSET);
-}
-
-static IIO_DEVICE_ATTR(id, S_IRUGO | S_IWUSR,
- adt7410_show_id,
- NULL,
- 0);
-
-static ssize_t adt7410_convert_temperature(struct adt7410_chip_info *chip,
- u16 data, char *buf)
-{
- char sign = ' ';
-
- if (!(chip->config & ADT7410_RESOLUTION))
- data &= ~0x7;
-
- if (data & ADT7410_T16_VALUE_SIGN) {
- /* convert supplement to positive value */
- data = (u16)((ADT7410_T16_VALUE_SIGN << 1) - (u32)data);
- sign = '-';
- }
- return sprintf(buf, "%c%d.%.7d\n", sign,
- (data >> ADT7410_T16_VALUE_FLOAT_OFFSET),
- (data & ADT7410_T16_VALUE_FLOAT_MASK) * 78125);
-}
-
-static ssize_t adt7410_show_value(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct iio_dev *dev_info = dev_to_iio_dev(dev);
- struct adt7410_chip_info *chip = iio_priv(dev_info);
- u8 status;
- u16 data;
- int ret, i = 0;
-
- do {
- ret = adt7410_read_byte(chip, ADT7410_STATUS, &status);
- if (ret)
- return -EIO;
- i++;
- if (i == 10000)
- return -EIO;
- } while (status & ADT7410_STAT_NOT_RDY);
-
- ret = adt7410_read_word(chip, ADT7410_TEMPERATURE, &data);
- if (ret)
- return -EIO;
-
- return adt7410_convert_temperature(chip, data, buf);
-}
-
-static IIO_DEVICE_ATTR(value, S_IRUGO, adt7410_show_value, NULL, 0);
-
-static struct attribute *adt7410_attributes[] = {
- &iio_dev_attr_available_modes.dev_attr.attr,
- &iio_dev_attr_mode.dev_attr.attr,
- &iio_dev_attr_resolution.dev_attr.attr,
- &iio_dev_attr_id.dev_attr.attr,
- &iio_dev_attr_value.dev_attr.attr,
- NULL,
-};
-
-static const struct attribute_group adt7410_attribute_group = {
- .attrs = adt7410_attributes,
-};
-
-static irqreturn_t adt7410_event_handler(int irq, void *private)
-{
- struct iio_dev *indio_dev = private;
- struct adt7410_chip_info *chip = iio_priv(indio_dev);
- s64 timestamp = iio_get_time_ns();
- u8 status;
-
- if (adt7410_read_byte(chip, ADT7410_STATUS, &status))
- return IRQ_HANDLED;
-
- if (status & ADT7410_STAT_T_HIGH)
- iio_push_event(indio_dev,
- IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
- IIO_EV_TYPE_THRESH,
- IIO_EV_DIR_RISING),
- timestamp);
- if (status & ADT7410_STAT_T_LOW)
- iio_push_event(indio_dev,
- IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
- IIO_EV_TYPE_THRESH,
- IIO_EV_DIR_FALLING),
- timestamp);
- if (status & ADT7410_STAT_T_CRIT)
- iio_push_event(indio_dev,
- IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
- IIO_EV_TYPE_THRESH,
- IIO_EV_DIR_RISING),
- timestamp);
-
- return IRQ_HANDLED;
-}
-
-static ssize_t adt7410_show_event_mode(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct iio_dev *dev_info = dev_to_iio_dev(dev);
- struct adt7410_chip_info *chip = iio_priv(dev_info);
- int ret;
-
- ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
- if (ret)
- return -EIO;
-
- if (chip->config & ADT7410_EVENT_MODE)
- return sprintf(buf, "interrupt\n");
- else
- return sprintf(buf, "comparator\n");
-}
-
-static ssize_t adt7410_set_event_mode(struct device *dev,
- struct device_attribute *attr,
- const char *buf,
- size_t len)
-{
- struct iio_dev *dev_info = dev_to_iio_dev(dev);
- struct adt7410_chip_info *chip = iio_priv(dev_info);
- u16 config;
- int ret;
-
- ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
- if (ret)
- return -EIO;
-
- config = chip->config &= ~ADT7410_EVENT_MODE;
- if (strcmp(buf, "comparator") != 0)
- config |= ADT7410_EVENT_MODE;
-
- ret = adt7410_write_byte(chip, ADT7410_CONFIG, config);
- if (ret)
- return -EIO;
-
- chip->config = config;
-
- return ret;
-}
-
-static ssize_t adt7410_show_available_event_modes(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- return sprintf(buf, "comparator\ninterrupt\n");
-}
-
-static ssize_t adt7410_show_fault_queue(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct iio_dev *dev_info = dev_to_iio_dev(dev);
- struct adt7410_chip_info *chip = iio_priv(dev_info);
- int ret;
-
- ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
- if (ret)
- return -EIO;
-
- return sprintf(buf, "%d\n", chip->config & ADT7410_FAULT_QUEUE_MASK);
-}
-
-static ssize_t adt7410_set_fault_queue(struct device *dev,
- struct device_attribute *attr,
- const char *buf,
- size_t len)
-{
- struct iio_dev *dev_info = dev_to_iio_dev(dev);
- struct adt7410_chip_info *chip = iio_priv(dev_info);
- unsigned long data;
- int ret;
- u8 config;
-
- ret = strict_strtoul(buf, 10, &data);
- if (ret || data > 3)
- return -EINVAL;
-
- ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
- if (ret)
- return -EIO;
-
- config = chip->config & ~ADT7410_FAULT_QUEUE_MASK;
- config |= data;
- ret = adt7410_write_byte(chip, ADT7410_CONFIG, config);
- if (ret)
- return -EIO;
-
- chip->config = config;
-
- return ret;
-}
-
-static inline ssize_t adt7410_show_t_bound(struct device *dev,
- struct device_attribute *attr,
- u8 bound_reg,
- char *buf)
-{
- struct iio_dev *dev_info = dev_to_iio_dev(dev);
- struct adt7410_chip_info *chip = iio_priv(dev_info);
- u16 data;
- int ret;
-
- ret = adt7410_read_word(chip, bound_reg, &data);
- if (ret)
- return -EIO;
-
- return adt7410_convert_temperature(chip, data, buf);
-}
-
-static inline ssize_t adt7410_set_t_bound(struct device *dev,
- struct device_attribute *attr,
- u8 bound_reg,
- const char *buf,
- size_t len)
-{
- struct iio_dev *dev_info = dev_to_iio_dev(dev);
- struct adt7410_chip_info *chip = iio_priv(dev_info);
- long tmp1, tmp2;
- u16 data;
- char *pos;
- int ret;
-
- pos = strchr(buf, '.');
-
- ret = strict_strtol(buf, 10, &tmp1);
-
- if (ret || tmp1 > 127 || tmp1 < -128)
- return -EINVAL;
-
- if (pos) {
- len = strlen(pos);
-
- if (chip->config & ADT7410_RESOLUTION) {
- if (len > ADT7410_T16_VALUE_FLOAT_OFFSET)
- len = ADT7410_T16_VALUE_FLOAT_OFFSET;
- pos[len] = 0;
- ret = strict_strtol(pos, 10, &tmp2);
-
- if (!ret)
- tmp2 = (tmp2 / 78125) * 78125;
- } else {
- if (len > ADT7410_T13_VALUE_FLOAT_OFFSET)
- len = ADT7410_T13_VALUE_FLOAT_OFFSET;
- pos[len] = 0;
- ret = strict_strtol(pos, 10, &tmp2);
-
- if (!ret)
- tmp2 = (tmp2 / 625) * 625;
- }
- }
-
- if (tmp1 < 0)
- data = (u16)(-tmp1);
- else
- data = (u16)tmp1;
-
- if (chip->config & ADT7410_RESOLUTION) {
- data = (data << ADT7410_T16_VALUE_FLOAT_OFFSET) |
- (tmp2 & ADT7410_T16_VALUE_FLOAT_MASK);
-
- if (tmp1 < 0)
- /* convert positive value to supplyment */
- data = (u16)((ADT7410_T16_VALUE_SIGN << 1) - (u32)data);
- } else {
- data = (data << ADT7410_T13_VALUE_FLOAT_OFFSET) |
- (tmp2 & ADT7410_T13_VALUE_FLOAT_MASK);
-
- if (tmp1 < 0)
- /* convert positive value to supplyment */
- data = (ADT7410_T13_VALUE_SIGN << 1) - data;
- data <<= ADT7410_T13_VALUE_OFFSET;
- }
-
- ret = adt7410_write_word(chip, bound_reg, data);
- if (ret)
- return -EIO;
-
- return ret;
-}
-
-static ssize_t adt7410_show_t_alarm_high(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- return adt7410_show_t_bound(dev, attr,
- ADT7410_T_ALARM_HIGH, buf);
-}
-
-static inline ssize_t adt7410_set_t_alarm_high(struct device *dev,
- struct device_attribute *attr,
- const char *buf,
- size_t len)
-{
- return adt7410_set_t_bound(dev, attr,
- ADT7410_T_ALARM_HIGH, buf, len);
-}
-
-static ssize_t adt7410_show_t_alarm_low(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- return adt7410_show_t_bound(dev, attr,
- ADT7410_T_ALARM_LOW, buf);
-}
-
-static inline ssize_t adt7410_set_t_alarm_low(struct device *dev,
- struct device_attribute *attr,
- const char *buf,
- size_t len)
-{
- return adt7410_set_t_bound(dev, attr,
- ADT7410_T_ALARM_LOW, buf, len);
-}
-
-static ssize_t adt7410_show_t_crit(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- return adt7410_show_t_bound(dev, attr,
- ADT7410_T_CRIT, buf);
-}
-
-static inline ssize_t adt7410_set_t_crit(struct device *dev,
- struct device_attribute *attr,
- const char *buf,
- size_t len)
-{
- return adt7410_set_t_bound(dev, attr,
- ADT7410_T_CRIT, buf, len);
-}
-
-static ssize_t adt7410_show_t_hyst(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct iio_dev *dev_info = dev_to_iio_dev(dev);
- struct adt7410_chip_info *chip = iio_priv(dev_info);
- int ret;
- u8 t_hyst;
-
- ret = adt7410_read_byte(chip, ADT7410_T_HYST, &t_hyst);
- if (ret)
- return -EIO;
-
- return sprintf(buf, "%d\n", t_hyst & ADT7410_T_HYST_MASK);
-}
-
-static inline ssize_t adt7410_set_t_hyst(struct device *dev,
- struct device_attribute *attr,
- const char *buf,
- size_t len)
-{
- struct iio_dev *dev_info = dev_to_iio_dev(dev);
- struct adt7410_chip_info *chip = iio_priv(dev_info);
- int ret;
- unsigned long data;
- u8 t_hyst;
-
- ret = strict_strtol(buf, 10, &data);
-
- if (ret || data > ADT7410_T_HYST_MASK)
- return -EINVAL;
-
- t_hyst = (u8)data;
-
- ret = adt7410_write_byte(chip, ADT7410_T_HYST, t_hyst);
- if (ret)
- return -EIO;
-
- return ret;
-}
-
-static IIO_DEVICE_ATTR(event_mode,
- S_IRUGO | S_IWUSR,
- adt7410_show_event_mode, adt7410_set_event_mode, 0);
-static IIO_DEVICE_ATTR(available_event_modes,
- S_IRUGO,
- adt7410_show_available_event_modes, NULL, 0);
-static IIO_DEVICE_ATTR(fault_queue,
- S_IRUGO | S_IWUSR,
- adt7410_show_fault_queue, adt7410_set_fault_queue, 0);
-static IIO_DEVICE_ATTR(t_alarm_high,
- S_IRUGO | S_IWUSR,
- adt7410_show_t_alarm_high, adt7410_set_t_alarm_high, 0);
-static IIO_DEVICE_ATTR(t_alarm_low,
- S_IRUGO | S_IWUSR,
- adt7410_show_t_alarm_low, adt7410_set_t_alarm_low, 0);
-static IIO_DEVICE_ATTR(t_crit,
- S_IRUGO | S_IWUSR,
- adt7410_show_t_crit, adt7410_set_t_crit, 0);
-static IIO_DEVICE_ATTR(t_hyst,
- S_IRUGO | S_IWUSR,
- adt7410_show_t_hyst, adt7410_set_t_hyst, 0);
-
-static struct attribute *adt7410_event_int_attributes[] = {
- &iio_dev_attr_event_mode.dev_attr.attr,
- &iio_dev_attr_available_event_modes.dev_attr.attr,
- &iio_dev_attr_fault_queue.dev_attr.attr,
- &iio_dev_attr_t_alarm_high.dev_attr.attr,
- &iio_dev_attr_t_alarm_low.dev_attr.attr,
- &iio_dev_attr_t_crit.dev_attr.attr,
- &iio_dev_attr_t_hyst.dev_attr.attr,
- NULL,
-};
-
-static struct attribute_group adt7410_event_attribute_group = {
- .attrs = adt7410_event_int_attributes,
- .name = "events",
-};
-
-static const struct iio_info adt7410_info = {
- .attrs = &adt7410_attribute_group,
- .event_attrs = &adt7410_event_attribute_group,
- .driver_module = THIS_MODULE,
-};
-
-/*
- * device probe and remove
- */
-
-static int adt7410_probe(struct device *dev, int irq,
- const char *name, const struct adt7410_ops *ops)
-{
- unsigned long *adt7410_platform_data = dev->platform_data;
- unsigned long local_pdata[] = {0, 0};
- struct adt7410_chip_info *chip;
- struct iio_dev *indio_dev;
- int ret = 0;
-
- indio_dev = iio_device_alloc(sizeof(*chip));
- if (indio_dev == NULL) {
- ret = -ENOMEM;
- goto error_ret;
- }
- chip = iio_priv(indio_dev);
- /* this is only used for device removal purposes */
- dev_set_drvdata(dev, indio_dev);
-
- chip->dev = dev;
- chip->ops = ops;
-
- indio_dev->name = name;
- indio_dev->dev.parent = dev;
- indio_dev->info = &adt7410_info;
- indio_dev->modes = INDIO_DIRECT_MODE;
-
- if (!adt7410_platform_data)
- adt7410_platform_data = local_pdata;
-
- /* CT critcal temperature event. line 0 */
- if (irq) {
- ret = request_threaded_irq(irq,
- NULL,
- &adt7410_event_handler,
- IRQF_TRIGGER_LOW | IRQF_ONESHOT,
- name,
- indio_dev);
- if (ret)
- goto error_free_dev;
- }
-
- /* INT bound temperature alarm event. line 1 */
- if (adt7410_platform_data[0]) {
- ret = request_threaded_irq(adt7410_platform_data[0],
- NULL,
- &adt7410_event_handler,
- adt7410_platform_data[1] |
- IRQF_ONESHOT,
- name,
- indio_dev);
- if (ret)
- goto error_unreg_ct_irq;
- }
-
- ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
- if (ret) {
- ret = -EIO;
- goto error_unreg_int_irq;
- }
-
- chip->config |= ADT7410_RESOLUTION;
-
- if (irq && adt7410_platform_data[0]) {
-
- /* set irq polarity low level */
- chip->config &= ~ADT7410_CT_POLARITY;
-
- if (adt7410_platform_data[1] & IRQF_TRIGGER_HIGH)
- chip->config |= ADT7410_INT_POLARITY;
- else
- chip->config &= ~ADT7410_INT_POLARITY;
- }
-
- ret = adt7410_write_byte(chip, ADT7410_CONFIG, chip->config);
- if (ret) {
- ret = -EIO;
- goto error_unreg_int_irq;
- }
- ret = iio_device_register(indio_dev);
- if (ret)
- goto error_unreg_int_irq;
-
- dev_info(dev, "%s temperature sensor registered.\n",
- name);
-
- return 0;
-
-error_unreg_int_irq:
- free_irq(adt7410_platform_data[0], indio_dev);
-error_unreg_ct_irq:
- free_irq(irq, indio_dev);
-error_free_dev:
- iio_device_free(indio_dev);
-error_ret:
- return ret;
-}
-
-static int adt7410_remove(struct device *dev, int irq)
-{
- struct iio_dev *indio_dev = dev_get_drvdata(dev);
- unsigned long *adt7410_platform_data = dev->platform_data;
-
- iio_device_unregister(indio_dev);
- if (adt7410_platform_data[0])
- free_irq(adt7410_platform_data[0], indio_dev);
- if (irq)
- free_irq(irq, indio_dev);
- iio_device_free(indio_dev);
-
- return 0;
-}
-
-#if IS_ENABLED(CONFIG_I2C)
-
-static int adt7410_i2c_read_word(struct adt7410_chip_info *chip, u8 reg,
- u16 *data)
-{
- struct i2c_client *client = to_i2c_client(chip->dev);
- int ret = 0;
-
- ret = i2c_smbus_read_word_data(client, reg);
- if (ret < 0) {
- dev_err(&client->dev, "I2C read error\n");
- return ret;
- }
-
- *data = swab16((u16)ret);
-
- return 0;
-}
-
-static int adt7410_i2c_write_word(struct adt7410_chip_info *chip, u8 reg,
- u16 data)
-{
- struct i2c_client *client = to_i2c_client(chip->dev);
- int ret = 0;
-
- ret = i2c_smbus_write_word_data(client, reg, swab16(data));
- if (ret < 0)
- dev_err(&client->dev, "I2C write error\n");
-
- return ret;
-}
-
-static int adt7410_i2c_read_byte(struct adt7410_chip_info *chip, u8 reg,
- u8 *data)
-{
- struct i2c_client *client = to_i2c_client(chip->dev);
- int ret = 0;
-
- ret = i2c_smbus_read_byte_data(client, reg);
- if (ret < 0) {
- dev_err(&client->dev, "I2C read error\n");
- return ret;
- }
-
- *data = (u8)ret;
-
- return 0;
-}
-
-static int adt7410_i2c_write_byte(struct adt7410_chip_info *chip, u8 reg,
- u8 data)
-{
- struct i2c_client *client = to_i2c_client(chip->dev);
- int ret = 0;
-
- ret = i2c_smbus_write_byte_data(client, reg, data);
- if (ret < 0)
- dev_err(&client->dev, "I2C write error\n");
-
- return ret;
-}
-
-static const struct adt7410_ops adt7410_i2c_ops = {
- .read_word = adt7410_i2c_read_word,
- .write_word = adt7410_i2c_write_word,
- .read_byte = adt7410_i2c_read_byte,
- .write_byte = adt7410_i2c_write_byte,
-};
-
-static int adt7410_i2c_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
-{
- return adt7410_probe(&client->dev, client->irq, id->name,
- &adt7410_i2c_ops);
-}
-
-static int adt7410_i2c_remove(struct i2c_client *client)
-{
- return adt7410_remove(&client->dev, client->irq);
-}
-
-static const struct i2c_device_id adt7410_id[] = {
- { "adt7410", 0 },
- {}
-};
-
-MODULE_DEVICE_TABLE(i2c, adt7410_id);
-
-static struct i2c_driver adt7410_driver = {
- .driver = {
- .name = "adt7410",
- },
- .probe = adt7410_i2c_probe,
- .remove = adt7410_i2c_remove,
- .id_table = adt7410_id,
-};
-
-static int __init adt7410_i2c_init(void)
-{
- return i2c_add_driver(&adt7410_driver);
-}
-
-static void __exit adt7410_i2c_exit(void)
-{
- i2c_del_driver(&adt7410_driver);
-}
-
-#else
-
-static int __init adt7410_i2c_init(void) { return 0; };
-static void __exit adt7410_i2c_exit(void) {};
-
-#endif
-
-#if IS_ENABLED(CONFIG_SPI_MASTER)
-
-static const u8 adt7371_reg_table[] = {
- [ADT7410_TEMPERATURE] = ADT7310_TEMPERATURE,
- [ADT7410_STATUS] = ADT7310_STATUS,
- [ADT7410_CONFIG] = ADT7310_CONFIG,
- [ADT7410_T_ALARM_HIGH] = ADT7310_T_ALARM_HIGH,
- [ADT7410_T_ALARM_LOW] = ADT7310_T_ALARM_LOW,
- [ADT7410_T_CRIT] = ADT7310_T_CRIT,
- [ADT7410_T_HYST] = ADT7310_T_HYST,
- [ADT7410_ID] = ADT7310_ID,
-};
-
-#define AD7310_COMMAND(reg) (adt7371_reg_table[(reg)] << ADT7310_CMD_REG_OFFSET)
-
-static int adt7310_spi_read_word(struct adt7410_chip_info *chip,
- u8 reg, u16 *data)
-{
- struct spi_device *spi = to_spi_device(chip->dev);
- u8 command = AD7310_COMMAND(reg);
- int ret = 0;
-
- command |= ADT7310_CMD_READ;
- ret = spi_write(spi, &command, sizeof(command));
- if (ret < 0) {
- dev_err(&spi->dev, "SPI write command error\n");
- return ret;
- }
-
- ret = spi_read(spi, (u8 *)data, sizeof(*data));
- if (ret < 0) {
- dev_err(&spi->dev, "SPI read word error\n");
- return ret;
- }
-
- *data = be16_to_cpu(*data);
-
- return 0;
-}
-
-static int adt7310_spi_write_word(struct adt7410_chip_info *chip, u8 reg,
- u16 data)
-{
- struct spi_device *spi = to_spi_device(chip->dev);
- u8 buf[3];
- int ret = 0;
-
- buf[0] = AD7310_COMMAND(reg);
- buf[1] = (u8)(data >> 8);
- buf[2] = (u8)(data & 0xFF);
-
- ret = spi_write(spi, buf, 3);
- if (ret < 0) {
- dev_err(&spi->dev, "SPI write word error\n");
- return ret;
- }
-
- return ret;
-}
-
-static int adt7310_spi_read_byte(struct adt7410_chip_info *chip, u8 reg,
- u8 *data)
-{
- struct spi_device *spi = to_spi_device(chip->dev);
- u8 command = AD7310_COMMAND(reg);
- int ret = 0;
-
- command |= ADT7310_CMD_READ;
- ret = spi_write(spi, &command, sizeof(command));
- if (ret < 0) {
- dev_err(&spi->dev, "SPI write command error\n");
- return ret;
- }
-
- ret = spi_read(spi, data, sizeof(*data));
- if (ret < 0) {
- dev_err(&spi->dev, "SPI read byte error\n");
- return ret;
- }
-
- return 0;
-}
-
-static int adt7310_spi_write_byte(struct adt7410_chip_info *chip, u8 reg,
- u8 data)
-{
- struct spi_device *spi = to_spi_device(chip->dev);
- u8 buf[2];
- int ret = 0;
-
- buf[0] = AD7310_COMMAND(reg);
- buf[1] = data;
-
- ret = spi_write(spi, buf, 2);
- if (ret < 0) {
- dev_err(&spi->dev, "SPI write byte error\n");
- return ret;
- }
-
- return ret;
-}
-
-static const struct adt7410_ops adt7310_spi_ops = {
- .read_word = adt7310_spi_read_word,
- .write_word = adt7310_spi_write_word,
- .read_byte = adt7310_spi_read_byte,
- .write_byte = adt7310_spi_write_byte,
-};
-
-static int adt7310_spi_probe(struct spi_device *spi)
-{
- return adt7410_probe(&spi->dev, spi->irq,
- spi_get_device_id(spi)->name, &adt7310_spi_ops);
-}
-
-static int adt7310_spi_remove(struct spi_device *spi)
-{
- return adt7410_remove(&spi->dev, spi->irq);
-}
-
-static const struct spi_device_id adt7310_id[] = {
- { "adt7310", 0 },
- {}
-};
-MODULE_DEVICE_TABLE(spi, adt7310_id);
-
-static struct spi_driver adt7310_driver = {
- .driver = {
- .name = "adt7310",
- .owner = THIS_MODULE,
- },
- .probe = adt7310_spi_probe,
- .remove = adt7310_spi_remove,
- .id_table = adt7310_id,
-};
-
-static int __init adt7310_spi_init(void)
-{
- return spi_register_driver(&adt7310_driver);
-}
-
-static void adt7310_spi_exit(void)
-{
- spi_unregister_driver(&adt7310_driver);
-}
-
-#else
-
-static int __init adt7310_spi_init(void) { return 0; };
-static void adt7310_spi_exit(void) {};
-
-#endif
-
-static int __init adt7410_init(void)
-{
- int ret;
-
- ret = adt7310_spi_init();
- if (ret)
- return ret;
-
- ret = adt7410_i2c_init();
- if (ret)
- adt7310_spi_exit();
-
- return ret;
-}
-module_init(adt7410_init);
-
-static void __exit adt7410_exit(void)
-{
- adt7410_i2c_exit();
- adt7310_spi_exit();
-}
-module_exit(adt7410_exit);
-
-MODULE_AUTHOR("Sonic Zhang <sonic.zhang@analog.com>");
-MODULE_DESCRIPTION("Analog Devices ADT7310/ADT7410 digital temperature sensor driver");
-MODULE_LICENSE("GPL v2");
--
1.8.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers
2013-02-18 13:38 [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers Lars-Peter Clausen
` (2 preceding siblings ...)
2013-02-18 13:38 ` [PATCH v2 4/4] staging:iio: Remove adt7410 driver Lars-Peter Clausen
@ 2013-02-18 20:22 ` Hartmut Knaack
2013-02-19 1:02 ` Guenter Roeck
2013-02-19 1:32 ` Guenter Roeck
4 siblings, 1 reply; 23+ messages in thread
From: Hartmut Knaack @ 2013-02-18 20:22 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Jean Delvare, Guenter Roeck, Jonathan Cameron, lm-sensors,
linux-iio
Lars-Peter Clausen schrieb:
> Currently each time the temperature register is read the driver also reads the
> threshold and hysteresis registers. This increases the amount of I2C traffic and
> time needed to read the temperature by a factor of ~5. Neither the threshold nor
> the hysteresis change on their own, so once we've read them, we should be able
> to just use the cached value of the registers. This patch modifies the code
> accordingly and only reads the threshold and hysteresis registers once during
> probe.
I have been thinking about this a lot, and I am concerned about data integrity. From what I know about I2C, there is no data integrity verification specified in the protocol. So, what the master sends is not necessarily what the slave receives (not to mention other devices on the bus, which could potentially mess around with the slaves, or even reset of the slave). Reading back just cached values makes it pretty hard to verify, if there are issues. I think it might be better to call a read-temperature function with a parameter that indicates, which temperature register is required.
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>
> ---
> Changes since v1:
> * Fix error checking for i2c reads
> ---
> drivers/hwmon/adt7410.c | 91 +++++++++++++++++++++++++++++++------------------
> 1 file changed, 58 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/hwmon/adt7410.c b/drivers/hwmon/adt7410.c
> index 99a7290..b6acfa4 100644
> --- a/drivers/hwmon/adt7410.c
> +++ b/drivers/hwmon/adt7410.c
> @@ -119,45 +119,33 @@ static int adt7410_temp_ready(struct i2c_client *client)
> return -ETIMEDOUT;
> }
>
> -static struct adt7410_data *adt7410_update_device(struct device *dev)
> +static int adt7410_update_temp(struct device *dev)
> {
> struct i2c_client *client = to_i2c_client(dev);
> struct adt7410_data *data = i2c_get_clientdata(client);
> - struct adt7410_data *ret = data;
> + int ret = 0;
> +
> mutex_lock(&data->update_lock);
>
> if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
> || !data->valid) {
> - int i, status;
> + int temp;
>
> dev_dbg(&client->dev, "Starting update\n");
>
> - status = adt7410_temp_ready(client); /* check for new value */
> - if (unlikely(status)) {
> - ret = ERR_PTR(status);
> + ret = adt7410_temp_ready(client); /* check for new value */
> + if (ret)
> goto abort;
> - }
> - for (i = 0; i < ARRAY_SIZE(data->temp); i++) {
> - status = i2c_smbus_read_word_swapped(client,
> - ADT7410_REG_TEMP[i]);
> - if (unlikely(status < 0)) {
> - dev_dbg(dev,
> - "Failed to read value: reg %d, error %d\n",
> - ADT7410_REG_TEMP[i], status);
> - ret = ERR_PTR(status);
> - goto abort;
> - }
> - data->temp[i] = status;
> - }
> - status = i2c_smbus_read_byte_data(client, ADT7410_T_HYST);
> - if (unlikely(status < 0)) {
> - dev_dbg(dev,
> - "Failed to read value: reg %d, error %d\n",
> - ADT7410_T_HYST, status);
> - ret = ERR_PTR(status);
> +
> + temp = i2c_smbus_read_word_swapped(client, ADT7410_REG_TEMP[0]);
> + if (temp < 0) {
> + ret = temp;
> + dev_dbg(dev, "Failed to read value: reg %d, error %d\n",
> + ADT7410_REG_TEMP[0], ret);
> goto abort;
> }
> - data->hyst = status;
> + data->temp[0] = temp;
> +
> data->last_updated = jiffies;
> data->valid = true;
> }
> @@ -167,6 +155,35 @@ abort:
> return ret;
> }
>
> +static int adt7410_fill_cache(struct i2c_client *client)
> +{
> + struct adt7410_data *data = i2c_get_clientdata(client);
> + int ret;
> + int i;
> +
> + for (i = 1; i < ARRAY_SIZE(ADT7410_REG_TEMP); i++) {
> + ret = i2c_smbus_read_word_swapped(client, ADT7410_REG_TEMP[i]);
> + if (ret < 0) {
> + dev_dbg(&client->dev,
> + "Failed to read value: reg %d, error %d\n",
> + ADT7410_REG_TEMP[0], ret);
> + return ret;
> + }
> + data->temp[i] = ret;
> + }
> +
> + ret = i2c_smbus_read_byte_data(client, ADT7410_T_HYST);
> + if (ret < 0) {
> + dev_dbg(&client->dev,
> + "Failed to read value: hyst reg, error %d\n",
> + ret);
> + return ret;
> + }
> + data->hyst = ret;
> +
> + return 0;
> +}
> +
> static s16 ADT7410_TEMP_TO_REG(long temp)
> {
> return DIV_ROUND_CLOSEST(clamp_val(temp, ADT7410_TEMP_MIN,
> @@ -193,10 +210,16 @@ static ssize_t adt7410_show_temp(struct device *dev,
> struct device_attribute *da, char *buf)
> {
> struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> - struct adt7410_data *data = adt7410_update_device(dev);
> + struct i2c_client *client = to_i2c_client(dev);
> + struct adt7410_data *data = i2c_get_clientdata(client);
>
> - if (IS_ERR(data))
> - return PTR_ERR(data);
> + if (attr->index == 0) {
> + int ret;
> +
> + ret = adt7410_update_temp(dev);
> + if (ret)
> + return ret;
> + }
>
> return sprintf(buf, "%d\n", ADT7410_REG_TO_TEMP(data,
> data->temp[attr->index]));
> @@ -232,13 +255,11 @@ static ssize_t adt7410_show_t_hyst(struct device *dev,
> char *buf)
> {
> struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> - struct adt7410_data *data;
> + struct i2c_client *client = to_i2c_client(dev);
> + struct adt7410_data *data = i2c_get_clientdata(client);
> int nr = attr->index;
> int hyst;
>
> - data = adt7410_update_device(dev);
> - if (IS_ERR(data))
> - return PTR_ERR(data);
> hyst = (data->hyst & ADT7410_T_HYST_MASK) * 1000;
>
> /*
> @@ -371,6 +392,10 @@ static int adt7410_probe(struct i2c_client *client,
> }
> dev_dbg(&client->dev, "Config %02x\n", data->config);
>
> + ret = adt7410_fill_cache(client);
> + if (ret)
> + goto exit_restore;
> +
> /* Register sysfs hooks */
> ret = sysfs_create_group(&client->dev.kobj, &adt7410_group);
> if (ret)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers
2013-02-18 20:22 ` [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers Hartmut Knaack
@ 2013-02-19 1:02 ` Guenter Roeck
2013-02-19 19:39 ` Hartmut Knaack
0 siblings, 1 reply; 23+ messages in thread
From: Guenter Roeck @ 2013-02-19 1:02 UTC (permalink / raw)
To: Hartmut Knaack
Cc: Lars-Peter Clausen, Jean Delvare, Jonathan Cameron, lm-sensors,
linux-iio
On Mon, Feb 18, 2013 at 09:22:18PM +0100, Hartmut Knaack wrote:
> Lars-Peter Clausen schrieb:
> > Currently each time the temperature register is read the driver also reads the
> > threshold and hysteresis registers. This increases the amount of I2C traffic and
> > time needed to read the temperature by a factor of ~5. Neither the threshold nor
> > the hysteresis change on their own, so once we've read them, we should be able
> > to just use the cached value of the registers. This patch modifies the code
> > accordingly and only reads the threshold and hysteresis registers once during
> > probe.
> I have been thinking about this a lot, and I am concerned about data integrity. From what I know about I2C, there is no data integrity verification specified in the protocol. So, what the master sends is not necessarily what the slave receives (not to mention other devices on the bus, which could potentially mess around with the slaves, or even reset of the slave). Reading back just cached values makes it pretty hard to verify, if there are issues. I think it might be better to call a read-temperature function with a parameter that indicates, which temperature register is required.
I am not concerned about that, unless there is a known issue with the chip
and it is known to return bad data under some circumstances. I am doing the
same in the PMBus drivers, since there are simply too many limit registers
to read on some of the chips (there may literally be more than a hundred).
That works fine most of the time; if it does not work, it is a chip problem,
an i2c bus master problem, a hardware signal problem, or a combination of all.
I actually think it is better if the problem is exposed by cached bad readings.
Then there is a chance to do something about it. This would be much better
than just re-reading the value next time and ignoring the problem.
Something else - the commit window just opened (a week earlier than I guessed),
so this part of the series is going to miss 3.9. Hartmut, if you plan to provide
an Acked-by or Reviewed-by or Tested-by for the first part of the series, you'll
have to do it soon, as I plan to send my push request to Linus around Thursday.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/4] hwmon: (adt7410) Add support for the adt7310/adt7320
2013-02-18 13:38 ` [PATCH v2 2/4] hwmon: (adt7410) Add support for the adt7310/adt7320 Lars-Peter Clausen
@ 2013-02-19 1:30 ` Guenter Roeck
2013-02-19 11:57 ` Lars-Peter Clausen
0 siblings, 1 reply; 23+ messages in thread
From: Guenter Roeck @ 2013-02-19 1:30 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Jean Delvare, Hartmut Knaack, Jonathan Cameron, lm-sensors,
linux-iio
On Mon, Feb 18, 2013 at 02:38:57PM +0100, Lars-Peter Clausen wrote:
> The adt7310/adt7320 is the SPI version of the adt7410/adt7420. The register map
> layout is a bit different, i.e. the register addresses differ between the two
> variants, but the bit layouts of the individual registers are identical. So both
> chip variants can easily be supported by the same driver. The issue of non
> matching register address layouts is solved by a simple look-up table which
> translates the I2C addresses to the SPI addresses.
>
> The patch moves the bulk of the adt7410 driver to a common module that will be
> shared by the adt7410 and adt7310 drivers. This common module implements the
> driver logic and uses a set of virtual functions to perform IO access. The
> adt7410 and adt7310 driver modules provide proper implementations of these IO
> accessor functions for I2C respective SPI.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>
> ---
> Changes since v1:
> * Update the driver documentation to include ADT7310/ADT7320/ADT7420
> * Pass the result of the read methods via the return value instead of a
> pointer argument
> * Simplify spi read methods by using spi_w8r8 and spi_w8r16
> * Update module description of the shared module
> * Fix some typos
> ---
> Documentation/hwmon/adt7410 | 42 ++--
> drivers/hwmon/Kconfig | 20 ++
> drivers/hwmon/Makefile | 2 +
> drivers/hwmon/adt7310.c | 115 +++++++++++
> drivers/hwmon/adt7410.c | 464 +++---------------------------------------
> drivers/hwmon/adt7x10.c | 476 ++++++++++++++++++++++++++++++++++++++++++++
> drivers/hwmon/adt7x10.h | 48 +++++
> 7 files changed, 720 insertions(+), 447 deletions(-)
> create mode 100644 drivers/hwmon/adt7310.c
> create mode 100644 drivers/hwmon/adt7x10.c
> create mode 100644 drivers/hwmon/adt7x10.h
>
> diff --git a/Documentation/hwmon/adt7410 b/Documentation/hwmon/adt7410
> index 9600400..e452ae0 100644
> --- a/Documentation/hwmon/adt7410
> +++ b/Documentation/hwmon/adt7410
> @@ -7,25 +7,41 @@ Supported chips:
> Addresses scanned: I2C 0x48 - 0x4B
> Datasheet: Publicly available at the Analog Devices website
> http://www.analog.com/static/imported-files/data_sheets/ADT7410.pdf
> + * Analog Devices ADT7420
> + Prefix: 'adt7420'
> + Addresses scanned: I2C 0x48 - 0x4B
> + Datasheet: Publicly available at the Analog Devices website
> + http://www.analog.com/static/imported-files/data_sheets/ADT7420.pdf
> + * Analog Devices ADT7310
> + Prefix: 'adt7310'
> + Addresses scanned: I2C 0x48 - 0x4B
> + Datasheet: Publicly available at the Analog Devices website
> + http://www.analog.com/static/imported-files/data_sheets/ADT7310.pdf
> + * Analog Devices ADT7320
> + Prefix: 'adt7320'
> + Addresses scanned: I2C 0x48 - 0x4B
> + Datasheet: Publicly available at the Analog Devices website
> + http://www.analog.com/static/imported-files/data_sheets/ADT7320.pdf
>
> Author: Hartmut Knaack <knaack.h@gmx.de>
>
> Description
> -----------
>
> -The ADT7410 is a temperature sensor with rated temperature range of -55°C to
> -+150°C. It has a high accuracy of +/-0.5°C and can be operated at a resolution
> -of 13 bits (0.0625°C) or 16 bits (0.0078°C). The sensor provides an INT pin to
> -indicate that a minimum or maximum temperature set point has been exceeded, as
> -well as a critical temperature (CT) pin to indicate that the critical
> -temperature set point has been exceeded. Both pins can be set up with a common
> -hysteresis of 0°C - 15°C and a fault queue, ranging from 1 to 4 events. Both
> -pins can individually set to be active-low or active-high, while the whole
> -device can either run in comparator mode or interrupt mode. The ADT7410
> -supports continous temperature sampling, as well as sampling one temperature
> -value per second or even justget one sample on demand for power saving.
> -Besides, it can completely power down its ADC, if power management is
> -required.
> +The ADT7410 and similar are a temperature sensors with rated temperature range
> +of -55°C to +150°C (ADT7310/ADT7410) or -40°C to +150°C (ADT7320/ADT7420). They
> +have a high accuracy of +/-0.5°C (ADT7310/ADT7410) or +/-0.2C (ADT7430/ADT7420)
> +and can be operated at a resolution of 13 bits (0.0625°C) or 16 bits (0.0078°C).
> +The sensor provides an INT pin to indicate that a minimum or maximum temperature
> +set point has been exceeded, as well as a critical temperature (CT) pin to
> +indicate that the critical temperature set point has been exceeded. Both pins
> +can be set up with a common hysteresis of 0°C - 15°C and a fault queue, ranging
> +from 1 to 4 events. Both pins can individually set to be active-low or
> +active-high, while the whole device can either run in comparator mode or
> +interrupt mode. The ADT7410 supports continous temperature sampling, as well as
> +sampling one temperature value per second or even justget one sample on demand
> +for power saving. Besides, it can completely power down its ADC, if power
> +management is required.
>
> Configuration Notes
> -------------------
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 89ac1cb..aaa14f4 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -179,9 +179,29 @@ config SENSORS_ADM9240
> This driver can also be built as a module. If so, the module
> will be called adm9240.
>
> +config SENSORS_ADT7X10
> + tristate
> + help
> + This module contains common code shared by the ADT7310/ADT7320 and
> + ADT7410/ADT7420 temperature monitoring chip drivers.
> +
> + If build as a module, the module will be called adt7x10.
> +
> +config SENSORS_ADT7310
> + tristate "Analog Devices ADT7310/ADT7320"
> + depends on SPI_MASTER
> + select SENSORS_ADT7X10
> + help
> + If you say yes here you get support for the Analog Devices
> + ADT7310 and ADT7320 temperature monitoring chips.
> +
> + This driver can also be built as a module. If so, the module
> + will be called adt7310.
> +
> config SENSORS_ADT7410
> tristate "Analog Devices ADT7410/ADT7420"
> depends on I2C
> + select SENSORS_ADT7X10
> help
> If you say yes here you get support for the Analog Devices
> ADT7410 and ADT7420 temperature monitoring chips.
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 8d6d97e..5d36a57 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -34,6 +34,8 @@ obj-$(CONFIG_SENSORS_ADM9240) += adm9240.o
> obj-$(CONFIG_SENSORS_ADS1015) += ads1015.o
> obj-$(CONFIG_SENSORS_ADS7828) += ads7828.o
> obj-$(CONFIG_SENSORS_ADS7871) += ads7871.o
> +obj-$(CONFIG_SENSORS_ADT7X10) += adt7x10.o
> +obj-$(CONFIG_SENSORS_ADT7310) += adt7310.o
> obj-$(CONFIG_SENSORS_ADT7410) += adt7410.o
> obj-$(CONFIG_SENSORS_ADT7411) += adt7411.o
> obj-$(CONFIG_SENSORS_ADT7462) += adt7462.o
> diff --git a/drivers/hwmon/adt7310.c b/drivers/hwmon/adt7310.c
> new file mode 100644
> index 0000000..2196ac3
> --- /dev/null
> +++ b/drivers/hwmon/adt7310.c
> @@ -0,0 +1,115 @@
> +/*
> + * ADT7310/ADT7310 digital temperature sensor driver
> + *
> + * Copyright 2010-2013 Analog Devices Inc.
Not really; copyright is yours, unless you are signing it off to analog or if
you are working for them and have permission (or the duty) to sign off the
copyright. Even then it should be 2013 only for this file.
> + * Author: Lars-Peter Clausen <lars@metafoo.de>
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/spi/spi.h>
> +#include <asm/unaligned.h>
> +
> +#include "adt7x10.h"
> +
> +static const u8 adt7310_reg_table[] = {
> + [ADT7410_TEMPERATURE] = ADT7310_TEMPERATURE,
> + [ADT7410_STATUS] = ADT7310_STATUS,
> + [ADT7410_CONFIG] = ADT7310_CONFIG,
> + [ADT7410_T_ALARM_HIGH] = ADT7310_T_ALARM_HIGH,
> + [ADT7410_T_ALARM_LOW] = ADT7310_T_ALARM_LOW,
> + [ADT7410_T_CRIT] = ADT7310_T_CRIT,
> + [ADT7410_T_HYST] = ADT7310_T_HYST,
> + [ADT7410_ID] = ADT7310_ID,
> +};
> +
> +#define ADT7310_CMD_REG_OFFSET 3
> +#define ADT7310_CMD_READ 0x40
> +
> +#define AD7310_COMMAND(reg) (adt7310_reg_table[(reg)] << ADT7310_CMD_REG_OFFSET)
> +
> +static int adt7310_spi_read_word(struct device *dev, u8 reg)
> +{
> + struct spi_device *spi = to_spi_device(dev);
> + int ret;
> +
> + ret = spi_w8r16(spi, AD7310_COMMAND(reg) | ADT7310_CMD_READ);
> + if (ret < 0)
> + return ret;
> +
> + return be16_to_cpu(ret);
> +}
> +
> +static int adt7310_spi_write_word(struct device *dev, u8 reg,
> + u16 data)
> +{
> + struct spi_device *spi = to_spi_device(dev);
> + u8 buf[3];
> +
> + buf[0] = AD7310_COMMAND(reg);
> + put_unaligned_be16(data, &buf[1]);
> +
> + return spi_write(spi, buf, sizeof(buf));
> +}
> +
> +static int adt7310_spi_read_byte(struct device *dev, u8 reg)
> +{
> + struct spi_device *spi = to_spi_device(dev);
> +
> + return spi_w8r8(spi, AD7310_COMMAND(reg) | ADT7310_CMD_READ);
> +}
> +
> +static int adt7310_spi_write_byte(struct device *dev, u8 reg,
> + u8 data)
> +{
> + struct spi_device *spi = to_spi_device(dev);
> + u8 buf[2];
> +
> + buf[0] = AD7310_COMMAND(reg);
> + buf[1] = data;
> +
> + return spi_write(spi, buf, sizeof(buf));
> +}
> +
> +static const struct adt7410_ops adt7310_spi_ops = {
> + .read_word = adt7310_spi_read_word,
> + .write_word = adt7310_spi_write_word,
> + .read_byte = adt7310_spi_read_byte,
> + .write_byte = adt7310_spi_write_byte,
> +};
> +
> +static int adt7310_spi_probe(struct spi_device *spi)
> +{
> + return adt7410_probe(&spi->dev, spi_get_device_id(spi)->name,
> + &adt7310_spi_ops);
> +}
> +
> +static int adt7310_spi_remove(struct spi_device *spi)
> +{
> + return adt7410_remove(&spi->dev);
> +}
> +
> +static const struct spi_device_id adt7310_id[] = {
> + { "adt7310", 0 },
> + { "adt7320", 0 },
> + {}
> +};
> +MODULE_DEVICE_TABLE(spi, adt7310_id);
> +
> +static struct spi_driver adt7310_driver = {
> + .driver = {
> + .name = "adt7310",
> + .owner = THIS_MODULE,
> + .pm = ADT7410_DEV_PM_OPS,
Should probably be named ADT7X10_DEV_PM_OPS to avoid confusion.
> + },
> + .probe = adt7310_spi_probe,
> + .remove = adt7310_spi_remove,
> + .id_table = adt7310_id,
> +};
> +module_spi_driver(adt7310_driver);
> +
> +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
> +MODULE_DESCRIPTION("ADT7310/ADT7320 driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hwmon/adt7410.c b/drivers/hwmon/adt7410.c
> index b6acfa4..b500ab3 100644
> --- a/drivers/hwmon/adt7410.c
> +++ b/drivers/hwmon/adt7410.c
> @@ -1,485 +1,81 @@
> /*
> - * adt7410.c - Part of lm_sensors, Linux kernel modules for hardware
> - * monitoring
> - * This driver handles the ADT7410 and compatible digital temperature sensors.
> - * Hartmut Knaack <knaack.h@gmx.de> 2012-07-22
> - * based on lm75.c by Frodo Looijaard <frodol@dds.nl>
> - * and adt7410.c from iio-staging by Sonic Zhang <sonic.zhang@analog.com>
> + * ADT7410/ADT7420 digital temperature sensor driver
> *
> - * 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 2 of the License, or
> - * (at your option) any later version.
> + * Copyright 2010-2013 Analog Devices Inc.
> + * Author: Lars-Peter Clausen <lars@metafoo.de>
Same as before.
> *
> - * 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, write to the Free Software
> - * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + * Licensed under the GPL-2 or later.
> */
>
> #include <linux/module.h>
> #include <linux/init.h>
> -#include <linux/slab.h>
> -#include <linux/jiffies.h>
> #include <linux/i2c.h>
> -#include <linux/hwmon.h>
> -#include <linux/hwmon-sysfs.h>
> -#include <linux/err.h>
> -#include <linux/mutex.h>
> -#include <linux/delay.h>
> -
> -/*
> - * ADT7410 registers definition
> - */
> -
> -#define ADT7410_TEMPERATURE 0
> -#define ADT7410_STATUS 2
> -#define ADT7410_CONFIG 3
> -#define ADT7410_T_ALARM_HIGH 4
> -#define ADT7410_T_ALARM_LOW 6
> -#define ADT7410_T_CRIT 8
> -#define ADT7410_T_HYST 0xA
> -
> -/*
> - * ADT7410 status
> - */
> -#define ADT7410_STAT_T_LOW (1 << 4)
> -#define ADT7410_STAT_T_HIGH (1 << 5)
> -#define ADT7410_STAT_T_CRIT (1 << 6)
> -#define ADT7410_STAT_NOT_RDY (1 << 7)
>
> -/*
> - * ADT7410 config
> - */
> -#define ADT7410_FAULT_QUEUE_MASK (1 << 0 | 1 << 1)
> -#define ADT7410_CT_POLARITY (1 << 2)
> -#define ADT7410_INT_POLARITY (1 << 3)
> -#define ADT7410_EVENT_MODE (1 << 4)
> -#define ADT7410_MODE_MASK (1 << 5 | 1 << 6)
> -#define ADT7410_FULL (0 << 5 | 0 << 6)
> -#define ADT7410_PD (1 << 5 | 1 << 6)
> -#define ADT7410_RESOLUTION (1 << 7)
> +#include "adt7x10.h"
>
> -/*
> - * ADT7410 masks
> - */
> -#define ADT7410_T13_VALUE_MASK 0xFFF8
> -#define ADT7410_T_HYST_MASK 0xF
> -
> -/* straight from the datasheet */
> -#define ADT7410_TEMP_MIN (-55000)
> -#define ADT7410_TEMP_MAX 150000
> -
> -enum adt7410_type { /* keep sorted in alphabetical order */
> - adt7410,
> -};
> -
> -static const u8 ADT7410_REG_TEMP[4] = {
> - ADT7410_TEMPERATURE, /* input */
> - ADT7410_T_ALARM_HIGH, /* high */
> - ADT7410_T_ALARM_LOW, /* low */
> - ADT7410_T_CRIT, /* critical */
> -};
> -
> -/* Each client has this additional data */
> -struct adt7410_data {
> - struct device *hwmon_dev;
> - struct mutex update_lock;
> - u8 config;
> - u8 oldconfig;
> - bool valid; /* true if registers valid */
> - unsigned long last_updated; /* In jiffies */
> - s16 temp[4]; /* Register values,
> - 0 = input
> - 1 = high
> - 2 = low
> - 3 = critical */
> - u8 hyst; /* hysteresis offset */
> -};
> -
> -/*
> - * adt7410 register access by I2C
> - */
> -static int adt7410_temp_ready(struct i2c_client *client)
> +static int adt7410_i2c_read_word(struct device *dev, u8 reg)
> {
> - int i, status;
> -
> - for (i = 0; i < 6; i++) {
> - status = i2c_smbus_read_byte_data(client, ADT7410_STATUS);
> - if (status < 0)
> - return status;
> - if (!(status & ADT7410_STAT_NOT_RDY))
> - return 0;
> - msleep(60);
> - }
> - return -ETIMEDOUT;
> + return i2c_smbus_read_word_swapped(to_i2c_client(dev), reg);
> }
>
> -static int adt7410_update_temp(struct device *dev)
> +static int adt7410_i2c_write_word(struct device *dev, u8 reg, u16 data)
> {
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7410_data *data = i2c_get_clientdata(client);
> - int ret = 0;
> -
> - mutex_lock(&data->update_lock);
> -
> - if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
> - || !data->valid) {
> - int temp;
> -
> - dev_dbg(&client->dev, "Starting update\n");
> -
> - ret = adt7410_temp_ready(client); /* check for new value */
> - if (ret)
> - goto abort;
> -
> - temp = i2c_smbus_read_word_swapped(client, ADT7410_REG_TEMP[0]);
> - if (temp < 0) {
> - ret = temp;
> - dev_dbg(dev, "Failed to read value: reg %d, error %d\n",
> - ADT7410_REG_TEMP[0], ret);
> - goto abort;
> - }
> - data->temp[0] = temp;
> -
> - data->last_updated = jiffies;
> - data->valid = true;
> - }
> -
> -abort:
> - mutex_unlock(&data->update_lock);
> - return ret;
> -}
> -
> -static int adt7410_fill_cache(struct i2c_client *client)
> -{
> - struct adt7410_data *data = i2c_get_clientdata(client);
> - int ret;
> - int i;
> -
> - for (i = 1; i < ARRAY_SIZE(ADT7410_REG_TEMP); i++) {
> - ret = i2c_smbus_read_word_swapped(client, ADT7410_REG_TEMP[i]);
> - if (ret < 0) {
> - dev_dbg(&client->dev,
> - "Failed to read value: reg %d, error %d\n",
> - ADT7410_REG_TEMP[0], ret);
> - return ret;
> - }
> - data->temp[i] = ret;
> - }
> -
> - ret = i2c_smbus_read_byte_data(client, ADT7410_T_HYST);
> - if (ret < 0) {
> - dev_dbg(&client->dev,
> - "Failed to read value: hyst reg, error %d\n",
> - ret);
> - return ret;
> - }
> - data->hyst = ret;
> -
> - return 0;
> + return i2c_smbus_write_word_swapped(to_i2c_client(dev), reg, data);
> }
>
> -static s16 ADT7410_TEMP_TO_REG(long temp)
> +static int adt7410_i2c_read_byte(struct device *dev, u8 reg)
> {
> - return DIV_ROUND_CLOSEST(clamp_val(temp, ADT7410_TEMP_MIN,
> - ADT7410_TEMP_MAX) * 128, 1000);
> + return i2c_smbus_read_byte_data(to_i2c_client(dev), reg);
> }
>
> -static int ADT7410_REG_TO_TEMP(struct adt7410_data *data, s16 reg)
> +static int adt7410_i2c_write_byte(struct device *dev, u8 reg, u8 data)
> {
> - /* in 13 bit mode, bits 0-2 are status flags - mask them out */
> - if (!(data->config & ADT7410_RESOLUTION))
> - reg &= ADT7410_T13_VALUE_MASK;
> - /*
> - * temperature is stored in twos complement format, in steps of
> - * 1/128°C
> - */
> - return DIV_ROUND_CLOSEST(reg * 1000, 128);
> + return i2c_smbus_write_byte_data(to_i2c_client(dev), reg, data);
> }
>
> -/*-----------------------------------------------------------------------*/
> -
> -/* sysfs attributes for hwmon */
> -
> -static ssize_t adt7410_show_temp(struct device *dev,
> - struct device_attribute *da, char *buf)
> -{
> - struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7410_data *data = i2c_get_clientdata(client);
> -
> - if (attr->index == 0) {
> - int ret;
> -
> - ret = adt7410_update_temp(dev);
> - if (ret)
> - return ret;
> - }
> -
> - return sprintf(buf, "%d\n", ADT7410_REG_TO_TEMP(data,
> - data->temp[attr->index]));
> -}
> -
> -static ssize_t adt7410_set_temp(struct device *dev,
> - struct device_attribute *da,
> - const char *buf, size_t count)
> -{
> - struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7410_data *data = i2c_get_clientdata(client);
> - int nr = attr->index;
> - long temp;
> - int ret;
> -
> - ret = kstrtol(buf, 10, &temp);
> - if (ret)
> - return ret;
> -
> - mutex_lock(&data->update_lock);
> - data->temp[nr] = ADT7410_TEMP_TO_REG(temp);
> - ret = i2c_smbus_write_word_swapped(client, ADT7410_REG_TEMP[nr],
> - data->temp[nr]);
> - if (ret)
> - count = ret;
> - mutex_unlock(&data->update_lock);
> - return count;
> -}
> -
> -static ssize_t adt7410_show_t_hyst(struct device *dev,
> - struct device_attribute *da,
> - char *buf)
> -{
> - struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7410_data *data = i2c_get_clientdata(client);
> - int nr = attr->index;
> - int hyst;
> -
> - hyst = (data->hyst & ADT7410_T_HYST_MASK) * 1000;
> -
> - /*
> - * hysteresis is stored as a 4 bit offset in the device, convert it
> - * to an absolute value
> - */
> - if (nr == 2) /* min has positive offset, others have negative */
> - hyst = -hyst;
> - return sprintf(buf, "%d\n",
> - ADT7410_REG_TO_TEMP(data, data->temp[nr]) - hyst);
> -}
> -
> -static ssize_t adt7410_set_t_hyst(struct device *dev,
> - struct device_attribute *da,
> - const char *buf, size_t count)
> -{
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7410_data *data = i2c_get_clientdata(client);
> - int limit, ret;
> - long hyst;
> -
> - ret = kstrtol(buf, 10, &hyst);
> - if (ret)
> - return ret;
> - /* convert absolute hysteresis value to a 4 bit delta value */
> - limit = ADT7410_REG_TO_TEMP(data, data->temp[1]);
> - hyst = clamp_val(hyst, ADT7410_TEMP_MIN, ADT7410_TEMP_MAX);
> - data->hyst = clamp_val(DIV_ROUND_CLOSEST(limit - hyst, 1000), 0,
> - ADT7410_T_HYST_MASK);
> - ret = i2c_smbus_write_byte_data(client, ADT7410_T_HYST, data->hyst);
> - if (ret)
> - return ret;
> -
> - return count;
> -}
> -
> -static ssize_t adt7410_show_alarm(struct device *dev,
> - struct device_attribute *da,
> - char *buf)
> -{
> - struct i2c_client *client = to_i2c_client(dev);
> - struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> - int ret;
> -
> - ret = i2c_smbus_read_byte_data(client, ADT7410_STATUS);
> - if (ret < 0)
> - return ret;
> -
> - return sprintf(buf, "%d\n", !!(ret & attr->index));
> -}
> -
> -static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, adt7410_show_temp, NULL, 0);
> -static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO,
> - adt7410_show_temp, adt7410_set_temp, 1);
> -static SENSOR_DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO,
> - adt7410_show_temp, adt7410_set_temp, 2);
> -static SENSOR_DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO,
> - adt7410_show_temp, adt7410_set_temp, 3);
> -static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO,
> - adt7410_show_t_hyst, adt7410_set_t_hyst, 1);
> -static SENSOR_DEVICE_ATTR(temp1_min_hyst, S_IRUGO,
> - adt7410_show_t_hyst, NULL, 2);
> -static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IRUGO,
> - adt7410_show_t_hyst, NULL, 3);
> -static SENSOR_DEVICE_ATTR(temp1_min_alarm, S_IRUGO, adt7410_show_alarm,
> - NULL, ADT7410_STAT_T_LOW);
> -static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO, adt7410_show_alarm,
> - NULL, ADT7410_STAT_T_HIGH);
> -static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, adt7410_show_alarm,
> - NULL, ADT7410_STAT_T_CRIT);
> -
> -static struct attribute *adt7410_attributes[] = {
> - &sensor_dev_attr_temp1_input.dev_attr.attr,
> - &sensor_dev_attr_temp1_max.dev_attr.attr,
> - &sensor_dev_attr_temp1_min.dev_attr.attr,
> - &sensor_dev_attr_temp1_crit.dev_attr.attr,
> - &sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
> - &sensor_dev_attr_temp1_min_hyst.dev_attr.attr,
> - &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
> - &sensor_dev_attr_temp1_min_alarm.dev_attr.attr,
> - &sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
> - &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
> - NULL
> -};
> -
> -static const struct attribute_group adt7410_group = {
> - .attrs = adt7410_attributes,
> +static const struct adt7410_ops adt7410_i2c_ops = {
> + .read_word = adt7410_i2c_read_word,
> + .write_word = adt7410_i2c_write_word,
> + .read_byte = adt7410_i2c_read_byte,
> + .write_byte = adt7410_i2c_write_byte,
> };
>
> -/*-----------------------------------------------------------------------*/
> -
> -/* device probe and removal */
> -
> -static int adt7410_probe(struct i2c_client *client,
> - const struct i2c_device_id *id)
> +static int adt7410_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> {
> - struct adt7410_data *data;
> - int ret;
>
> if (!i2c_check_functionality(client->adapter,
> I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
> return -ENODEV;
>
> - data = devm_kzalloc(&client->dev, sizeof(struct adt7410_data),
> - GFP_KERNEL);
> - if (!data)
> - return -ENOMEM;
> -
> - i2c_set_clientdata(client, data);
> - mutex_init(&data->update_lock);
> -
> - /* configure as specified */
> - ret = i2c_smbus_read_byte_data(client, ADT7410_CONFIG);
> - if (ret < 0) {
> - dev_dbg(&client->dev, "Can't read config? %d\n", ret);
> - return ret;
> - }
> - data->oldconfig = ret;
> - /*
> - * Set to 16 bit resolution, continous conversion and comparator mode.
> - */
> - ret &= ~ADT7410_MODE_MASK;
> - data->config = ret | ADT7410_FULL | ADT7410_RESOLUTION |
> - ADT7410_EVENT_MODE;
> - if (data->config != data->oldconfig) {
> - ret = i2c_smbus_write_byte_data(client, ADT7410_CONFIG,
> - data->config);
> - if (ret)
> - return ret;
> - }
> - dev_dbg(&client->dev, "Config %02x\n", data->config);
> -
> - ret = adt7410_fill_cache(client);
> - if (ret)
> - goto exit_restore;
> -
> - /* Register sysfs hooks */
> - ret = sysfs_create_group(&client->dev.kobj, &adt7410_group);
> - if (ret)
> - goto exit_restore;
> -
> - data->hwmon_dev = hwmon_device_register(&client->dev);
> - if (IS_ERR(data->hwmon_dev)) {
> - ret = PTR_ERR(data->hwmon_dev);
> - goto exit_remove;
> - }
> -
> - dev_info(&client->dev, "sensor '%s'\n", client->name);
> -
> - return 0;
> -
> -exit_remove:
> - sysfs_remove_group(&client->dev.kobj, &adt7410_group);
> -exit_restore:
> - i2c_smbus_write_byte_data(client, ADT7410_CONFIG, data->oldconfig);
> - return ret;
> + return adt7410_probe(&client->dev, NULL, &adt7410_i2c_ops);
> }
>
> -static int adt7410_remove(struct i2c_client *client)
> +static int adt7410_i2c_remove(struct i2c_client *client)
> {
> - struct adt7410_data *data = i2c_get_clientdata(client);
> -
> - hwmon_device_unregister(data->hwmon_dev);
> - sysfs_remove_group(&client->dev.kobj, &adt7410_group);
> - if (data->oldconfig != data->config)
> - i2c_smbus_write_byte_data(client, ADT7410_CONFIG,
> - data->oldconfig);
> - return 0;
> + return adt7410_remove(&client->dev);
> }
>
> static const struct i2c_device_id adt7410_ids[] = {
> - { "adt7410", adt7410, },
> - { "adt7420", adt7410, },
> - { /* LIST END */ }
> + { "adt7410", 0 },
> + { "adt7420", 0 },
> + {}
> };
> MODULE_DEVICE_TABLE(i2c, adt7410_ids);
>
> -#ifdef CONFIG_PM_SLEEP
> -static int adt7410_suspend(struct device *dev)
> -{
> - int ret;
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7410_data *data = i2c_get_clientdata(client);
> -
> - ret = i2c_smbus_write_byte_data(client, ADT7410_CONFIG,
> - data->config | ADT7410_PD);
> - return ret;
> -}
> -
> -static int adt7410_resume(struct device *dev)
> -{
> - int ret;
> - struct i2c_client *client = to_i2c_client(dev);
> - struct adt7410_data *data = i2c_get_clientdata(client);
> -
> - ret = i2c_smbus_write_byte_data(client, ADT7410_CONFIG, data->config);
> - return ret;
> -}
> -
> -static SIMPLE_DEV_PM_OPS(adt7410_dev_pm_ops, adt7410_suspend, adt7410_resume);
> -
> -#define ADT7410_DEV_PM_OPS (&adt7410_dev_pm_ops)
> -#else
> -#define ADT7410_DEV_PM_OPS NULL
> -#endif /* CONFIG_PM */
> -
> static struct i2c_driver adt7410_driver = {
> .class = I2C_CLASS_HWMON,
> .driver = {
> .name = "adt7410",
> .pm = ADT7410_DEV_PM_OPS,
> },
> - .probe = adt7410_probe,
> - .remove = adt7410_remove,
> + .probe = adt7410_i2c_probe,
> + .remove = adt7410_i2c_remove,
> .id_table = adt7410_ids,
> .address_list = I2C_ADDRS(0x48, 0x49, 0x4a, 0x4b),
> };
> -
> module_i2c_driver(adt7410_driver);
>
> -MODULE_AUTHOR("Hartmut Knaack");
> -MODULE_DESCRIPTION("ADT7410/ADT7420 driver");
> +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
> +MODULE_DESCRIPTION("ADT7410/AD7420 driver");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/hwmon/adt7x10.c b/drivers/hwmon/adt7x10.c
> new file mode 100644
> index 0000000..eeff198c
> --- /dev/null
> +++ b/drivers/hwmon/adt7x10.c
> @@ -0,0 +1,476 @@
> +/*
> + * adt7410.c - Part of lm_sensors, Linux kernel modules for hardware
Needs file name and purpose update
> + * monitoring
> + * This driver handles the ADT7410 and compatible digital temperature sensors.
> + * Hartmut Knaack <knaack.h@gmx.de> 2012-07-22
> + * based on lm75.c by Frodo Looijaard <frodol@dds.nl>
> + * and adt7410.c from iio-staging by Sonic Zhang <sonic.zhang@analog.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 2 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, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/jiffies.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/delay.h>
> +
> +#include "adt7x10.h"
> +
> +/*
> + * ADT7410 status
> + */
> +#define ADT7410_STAT_T_LOW (1 << 4)
> +#define ADT7410_STAT_T_HIGH (1 << 5)
> +#define ADT7410_STAT_T_CRIT (1 << 6)
> +#define ADT7410_STAT_NOT_RDY (1 << 7)
> +
> +/*
> + * ADT7410 config
> + */
> +#define ADT7410_FAULT_QUEUE_MASK (1 << 0 | 1 << 1)
> +#define ADT7410_CT_POLARITY (1 << 2)
> +#define ADT7410_INT_POLARITY (1 << 3)
> +#define ADT7410_EVENT_MODE (1 << 4)
> +#define ADT7410_MODE_MASK (1 << 5 | 1 << 6)
> +#define ADT7410_FULL (0 << 5 | 0 << 6)
> +#define ADT7410_PD (1 << 5 | 1 << 6)
> +#define ADT7410_RESOLUTION (1 << 7)
> +
> +/*
> + * ADT7410 masks
> + */
> +#define ADT7410_T13_VALUE_MASK 0xFFF8
> +#define ADT7410_T_HYST_MASK 0xF
> +
> +/* straight from the datasheet */
> +#define ADT7410_TEMP_MIN (-55000)
> +#define ADT7410_TEMP_MAX 150000
> +
> +/* Each client has this additional data */
> +struct adt7410_data {
> + const struct adt7410_ops *ops;
> + const char *name;
> + struct device *hwmon_dev;
> + struct mutex update_lock;
> + u8 config;
> + u8 oldconfig;
> + bool valid; /* true if registers valid */
> + unsigned long last_updated; /* In jiffies */
> + s16 temp[4]; /* Register values,
> + 0 = input
> + 1 = high
> + 2 = low
> + 3 = critical */
> + u8 hyst; /* hysteresis offset */
> +};
> +
> +static int adt7410_read_byte(struct device *dev, u8 reg)
> +{
> + struct adt7410_data *d = dev_get_drvdata(dev);
> + return d->ops->read_byte(dev, reg);
> +}
> +
> +static int adt7410_write_byte(struct device *dev, u8 reg, u8 data)
> +{
> + struct adt7410_data *d = dev_get_drvdata(dev);
> + return d->ops->write_byte(dev, reg, data);
> +}
> +
> +static int adt7410_read_word(struct device *dev, u8 reg)
> +{
> + struct adt7410_data *d = dev_get_drvdata(dev);
> + return d->ops->read_word(dev, reg);
> +}
> +
> +static int adt7410_write_word(struct device *dev, u8 reg, u16 data)
> +{
> + struct adt7410_data *d = dev_get_drvdata(dev);
> + return d->ops->write_word(dev, reg, data);
> +}
> +
> +static const u8 ADT7410_REG_TEMP[4] = {
> + ADT7410_TEMPERATURE, /* input */
> + ADT7410_T_ALARM_HIGH, /* high */
> + ADT7410_T_ALARM_LOW, /* low */
> + ADT7410_T_CRIT, /* critical */
> +};
> +
> +static int adt7410_temp_ready(struct device *dev)
> +{
> + int i, status;
> +
> + for (i = 0; i < 6; i++) {
> + status = adt7410_read_byte(dev, ADT7410_STATUS);
> + if (status < 0)
> + return status;
> + if (!(status & ADT7410_STAT_NOT_RDY))
> + return 0;
> + msleep(60);
> + }
> + return -ETIMEDOUT;
> +}
> +
> +static int adt7410_update_temp(struct device *dev)
> +{
> + struct adt7410_data *data = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + mutex_lock(&data->update_lock);
> +
> + if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
> + || !data->valid) {
> + int temp;
> +
> + dev_dbg(dev, "Starting update\n");
> +
> + ret = adt7410_temp_ready(dev); /* check for new value */
> + if (ret)
> + goto abort;
> +
> + temp = adt7410_read_word(dev, ADT7410_REG_TEMP[0]);
> + if (temp < 0) {
> + ret = temp;
> + dev_dbg(dev, "Failed to read value: reg %d, error %d\n",
> + ADT7410_REG_TEMP[0], ret);
> + goto abort;
> + }
> + data->temp[0] = temp;
> + data->last_updated = jiffies;
> + data->valid = true;
> + }
> +
> +abort:
> + mutex_unlock(&data->update_lock);
> + return ret;
> +}
> +
> +static int adt7410_fill_cache(struct device *dev)
> +{
> + struct adt7410_data *data = dev_get_drvdata(dev);
> + int ret;
> + int i;
> +
> + for (i = 1; i < ARRAY_SIZE(data->temp); i++) {
> + ret = adt7410_read_word(dev, ADT7410_REG_TEMP[i]);
> + if (ret < 0) {
> + dev_dbg(dev, "Failed to read value: reg %d, error %d\n",
> + ADT7410_REG_TEMP[0], ret);
ADT7410_REG_TEMP[i] as Hartmut pointed out
> + return ret;
> + }
> + data->temp[i] = ret;
> + }
> +
> + ret = adt7410_read_byte(dev, ADT7410_T_HYST);
> + if (ret < 0) {
> + dev_dbg(dev, "Failed to read value: reg %d, error %d\n",
> + ADT7410_T_HYST, ret);
> + return ret;
> + }
> + data->hyst = ret;
> +
> + return 0;
> +}
> +
> +static s16 ADT7410_TEMP_TO_REG(long temp)
> +{
> + return DIV_ROUND_CLOSEST(clamp_val(temp, ADT7410_TEMP_MIN,
> + ADT7410_TEMP_MAX) * 128, 1000);
> +}
> +
> +static int ADT7410_REG_TO_TEMP(struct adt7410_data *data, s16 reg)
> +{
> + /* in 13 bit mode, bits 0-2 are status flags - mask them out */
> + if (!(data->config & ADT7410_RESOLUTION))
> + reg &= ADT7410_T13_VALUE_MASK;
> + /*
> + * temperature is stored in twos complement format, in steps of
> + * 1/128°C
> + */
> + return DIV_ROUND_CLOSEST(reg * 1000, 128);
> +}
> +
> +/*-----------------------------------------------------------------------*/
> +
> +/* sysfs attributes for hwmon */
> +
> +static ssize_t adt7410_show_temp(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct adt7410_data *data = dev_get_drvdata(dev);
> +
> +
> + if (attr->index == 0) {
> + int ret;
> +
> + ret = adt7410_update_temp(dev);
> + if (ret)
> + return ret;
> + }
> +
> + return sprintf(buf, "%d\n", ADT7410_REG_TO_TEMP(data,
> + data->temp[attr->index]));
> +}
> +
> +static ssize_t adt7410_set_temp(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct adt7410_data *data = dev_get_drvdata(dev);
> + int nr = attr->index;
> + long temp;
> + int ret;
> +
> + ret = kstrtol(buf, 10, &temp);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&data->update_lock);
> + data->temp[nr] = ADT7410_TEMP_TO_REG(temp);
> + ret = adt7410_write_word(dev, ADT7410_REG_TEMP[nr], data->temp[nr]);
> + if (ret)
> + count = ret;
> + mutex_unlock(&data->update_lock);
> + return count;
> +}
> +
> +static ssize_t adt7410_show_t_hyst(struct device *dev,
> + struct device_attribute *da,
> + char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct adt7410_data *data = dev_get_drvdata(dev);
> + int nr = attr->index;
> + int hyst;
> +
> + hyst = (data->hyst & ADT7410_T_HYST_MASK) * 1000;
> +
> + /*
> + * hysteresis is stored as a 4 bit offset in the device, convert it
> + * to an absolute value
> + */
> + if (nr == 2) /* min has positive offset, others have negative */
> + hyst = -hyst;
> + return sprintf(buf, "%d\n",
> + ADT7410_REG_TO_TEMP(data, data->temp[nr]) - hyst);
> +}
> +
> +static ssize_t adt7410_set_t_hyst(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct adt7410_data *data = dev_get_drvdata(dev);
> + int limit, ret;
> + long hyst;
> +
> + ret = kstrtol(buf, 10, &hyst);
> + if (ret)
> + return ret;
> + /* convert absolute hysteresis value to a 4 bit delta value */
> + limit = ADT7410_REG_TO_TEMP(data, data->temp[1]);
> + hyst = clamp_val(hyst, ADT7410_TEMP_MIN, ADT7410_TEMP_MAX);
> + data->hyst = clamp_val(DIV_ROUND_CLOSEST(limit - hyst, 1000),
> + 0, ADT7410_T_HYST_MASK);
> + ret = adt7410_write_byte(dev, ADT7410_T_HYST, data->hyst);
> + if (ret)
> + return ret;
> +
> + return count;
> +}
> +
> +static ssize_t adt7410_show_alarm(struct device *dev,
> + struct device_attribute *da,
> + char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + int ret;
> +
> + ret = adt7410_read_byte(dev, ADT7410_STATUS);
> + if (ret < 0)
> + return ret;
> +
> + return sprintf(buf, "%d\n", !!(ret & attr->index));
> +}
> +
> +static ssize_t adt7410_show_name(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct adt7410_data *data = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%s\n", data->name);
> +}
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, adt7410_show_temp, NULL, 0);
> +static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO,
> + adt7410_show_temp, adt7410_set_temp, 1);
> +static SENSOR_DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO,
> + adt7410_show_temp, adt7410_set_temp, 2);
> +static SENSOR_DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO,
> + adt7410_show_temp, adt7410_set_temp, 3);
> +static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO,
> + adt7410_show_t_hyst, adt7410_set_t_hyst, 1);
> +static SENSOR_DEVICE_ATTR(temp1_min_hyst, S_IRUGO,
> + adt7410_show_t_hyst, NULL, 2);
> +static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IRUGO,
> + adt7410_show_t_hyst, NULL, 3);
> +static SENSOR_DEVICE_ATTR(temp1_min_alarm, S_IRUGO, adt7410_show_alarm,
> + NULL, ADT7410_STAT_T_LOW);
> +static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO, adt7410_show_alarm,
> + NULL, ADT7410_STAT_T_HIGH);
> +static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, adt7410_show_alarm,
> + NULL, ADT7410_STAT_T_CRIT);
> +static DEVICE_ATTR(name, S_IRUGO, adt7410_show_name, NULL);
> +
> +static struct attribute *adt7410_attributes[] = {
> + &sensor_dev_attr_temp1_input.dev_attr.attr,
> + &sensor_dev_attr_temp1_max.dev_attr.attr,
> + &sensor_dev_attr_temp1_min.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit.dev_attr.attr,
> + &sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
> + &sensor_dev_attr_temp1_min_hyst.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
> + &sensor_dev_attr_temp1_min_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
> + NULL
> +};
> +
> +static const struct attribute_group adt7410_group = {
> + .attrs = adt7410_attributes,
> +};
> +
> +int adt7410_probe(struct device *dev, const char *name,
> + const struct adt7410_ops *ops)
Preferred alignment is to have the second line start at the (. Usually I don't
complain but since you'll have to do another rev anyway maybe you can fix this
whereever it occurs.
> +{
> + struct adt7410_data *data;
> + int ret;
> +
> + data = devm_kzalloc(dev, sizeof(struct adt7410_data),
> + GFP_KERNEL);
No need to split the line above.
> + if (!data)
> + return -ENOMEM;
> +
> + data->ops = ops;
> + data->name = name;
> +
> + dev_set_drvdata(dev, data);
> + mutex_init(&data->update_lock);
> +
> + /* configure as specified */
> + ret = adt7410_read_byte(dev, ADT7410_CONFIG);
> + if (ret < 0) {
> + dev_dbg(dev, "Can't read config? %d\n", ret);
> + return ret;
> + }
> + data->oldconfig = ret;
> +
> + /*
> + * Set to 16 bit resolution, continous conversion and comparator mode.
> + */
> + data->config = data->oldconfig;
> + data->config &= ~ADT7410_MODE_MASK;
> + data->config |= ADT7410_FULL | ADT7410_RESOLUTION | ADT7410_EVENT_MODE;
> + if (data->config != data->oldconfig) {
> + ret = adt7410_write_byte(dev, ADT7410_CONFIG, data->config);
> + if (ret)
> + return ret;
> + }
> + dev_dbg(dev, "Config %02x\n", data->config);
> +
> + ret = adt7410_fill_cache(dev);
> + if (ret)
> + goto exit_restore;
> +
> + /* Register sysfs hooks */
> + ret = sysfs_create_group(&dev->kobj, &adt7410_group);
> + if (ret)
> + goto exit_restore;
> +
> + /*
> + * The I2C device will already have it's own 'name' attribute, but for
> + * the SPI device we need to register it. name will only be non NULL if
> + * the device doesn't register the 'name' attribute on its own.
> + */
> + if (name) {
> + ret = device_create_file(dev, &dev_attr_name);
> + if (ret)
> + goto exit_remove;
> + }
> +
> + data->hwmon_dev = hwmon_device_register(dev);
> + if (IS_ERR(data->hwmon_dev)) {
> + ret = PTR_ERR(data->hwmon_dev);
> + goto exit_remove_name;
> + }
> +
> + return 0;
> +
> +exit_remove_name:
> + if (name)
> + device_remove_file(dev, &dev_attr_name);
> +exit_remove:
> + sysfs_remove_group(&dev->kobj, &adt7410_group);
> +exit_restore:
> + adt7410_write_byte(dev, ADT7410_CONFIG, data->oldconfig);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(adt7410_probe);
> +
> +int adt7410_remove(struct device *dev)
> +{
> + struct adt7410_data *data = dev_get_drvdata(dev);
> +
> + hwmon_device_unregister(data->hwmon_dev);
> + if (data->name)
> + device_remove_file(dev, &dev_attr_name);
> + sysfs_remove_group(&dev->kobj, &adt7410_group);
> + if (data->oldconfig != data->config)
> + adt7410_write_byte(dev, ADT7410_CONFIG,
> + data->oldconfig);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(adt7410_remove);
> +
> +#ifdef CONFIG_PM_SLEEP
> +
> +static int adt7410_suspend(struct device *dev)
> +{
> + struct adt7410_data *data = dev_get_drvdata(dev);
> +
> + return adt7410_write_byte(dev, ADT7410_CONFIG,
> + data->config | ADT7410_PD);
> +}
> +
> +static int adt7410_resume(struct device *dev)
> +{
> + struct adt7410_data *data = dev_get_drvdata(dev);
> +
> + return adt7410_write_byte(dev, ADT7410_CONFIG, data->config);
> +}
> +
> +SIMPLE_DEV_PM_OPS(adt7410_dev_pm_ops, adt7410_suspend, adt7410_resume);
> +EXPORT_SYMBOL_GPL(adt7410_dev_pm_ops);
> +
> +#endif /* CONFIG_PM_SLEEP */
> +
> +MODULE_AUTHOR("Hartmut Knaack");
> +MODULE_DESCRIPTION("ADT7410/ADT7420, ADT7310/ADT7320 common code");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hwmon/adt7x10.h b/drivers/hwmon/adt7x10.h
> new file mode 100644
> index 0000000..a7165e6
> --- /dev/null
> +++ b/drivers/hwmon/adt7x10.h
> @@ -0,0 +1,48 @@
> +#ifndef __HWMON_ADT7X10_H__
> +#define __HWMON_ADT7X10_H__
> +
> +#include <linux/types.h>
> +#include <linux/pm.h>
> +
> +/* ADT7410 registers definition */
> +#define ADT7410_TEMPERATURE 0
> +#define ADT7410_STATUS 2
> +#define ADT7410_CONFIG 3
> +#define ADT7410_T_ALARM_HIGH 4
> +#define ADT7410_T_ALARM_LOW 6
> +#define ADT7410_T_CRIT 8
> +#define ADT7410_T_HYST 0xA
> +#define ADT7410_ID 0xB
> +
> +/* ADT7310 registers definition */
> +#define ADT7310_STATUS 0
> +#define ADT7310_CONFIG 1
> +#define ADT7310_TEMPERATURE 2
> +#define ADT7310_ID 3
> +#define ADT7310_T_CRIT 4
> +#define ADT7310_T_HYST 5
> +#define ADT7310_T_ALARM_HIGH 6
> +#define ADT7310_T_ALARM_LOW 7
> +
> +struct device;
> +
> +struct adt7410_ops {
> + int (*read_byte)(struct device *, u8 reg);
> + int (*write_byte)(struct device *, u8 reg, u8 data);
> + int (*read_word)(struct device *, u8 reg);
> + int (*write_word)(struct device *, u8 reg, u16 data);
> +};
> +
> +int adt7410_probe(struct device *dev, const char *name,
> + const struct adt7410_ops *ops);
> +int adt7410_remove(struct device *dev);
> +
I think the above should be adt7x10_ops, adt7x10_probe and adt7x10_remove, to
indicate that those are common functions.
I would actually suggest to rename all functions in adt7x10.c (not just the
exported ones).
> +
> +#ifdef CONFIG_PM_SLEEP
> +extern const struct dev_pm_ops adt7410_dev_pm_ops;
> +#define ADT7410_DEV_PM_OPS (&adt7410_dev_pm_ops)
As mentioned above, I think ADT7X10_DEV_PM_OPS would be better here.
> +#else
> +#define ADT7410_DEV_PM_OPS NULL
> +#endif
> +
> +#endif
> --
> 1.8.0
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers
2013-02-18 13:38 [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers Lars-Peter Clausen
` (3 preceding siblings ...)
2013-02-18 20:22 ` [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers Hartmut Knaack
@ 2013-02-19 1:32 ` Guenter Roeck
4 siblings, 0 replies; 23+ messages in thread
From: Guenter Roeck @ 2013-02-19 1:32 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Jean Delvare, Hartmut Knaack, Jonathan Cameron, lm-sensors,
linux-iio
On Mon, Feb 18, 2013 at 02:38:56PM +0100, Lars-Peter Clausen wrote:
> Currently each time the temperature register is read the driver also reads the
> threshold and hysteresis registers. This increases the amount of I2C traffic and
> time needed to read the temperature by a factor of ~5. Neither the threshold nor
> the hysteresis change on their own, so once we've read them, we should be able
> to just use the cached value of the registers. This patch modifies the code
> accordingly and only reads the threshold and hysteresis registers once during
> probe.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>
Just one comment (thanks to Hartmut), otherwise looks good.
> ---
> Changes since v1:
> * Fix error checking for i2c reads
> ---
[ ... ]
> +static int adt7410_fill_cache(struct i2c_client *client)
> +{
> + struct adt7410_data *data = i2c_get_clientdata(client);
> + int ret;
> + int i;
> +
> + for (i = 1; i < ARRAY_SIZE(ADT7410_REG_TEMP); i++) {
> + ret = i2c_smbus_read_word_swapped(client, ADT7410_REG_TEMP[i]);
> + if (ret < 0) {
> + dev_dbg(&client->dev,
> + "Failed to read value: reg %d, error %d\n",
> + ADT7410_REG_TEMP[0], ret);
ADT7410_REG_TEMP[i]
Thanks,
Guenter
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/4] hwmon: (adt7x10) Add alarm interrupt support
2013-02-18 13:38 ` [PATCH v2 3/4] hwmon: (adt7x10) Add alarm interrupt support Lars-Peter Clausen
@ 2013-02-19 1:39 ` Guenter Roeck
2013-02-19 12:05 ` Lars-Peter Clausen
0 siblings, 1 reply; 23+ messages in thread
From: Guenter Roeck @ 2013-02-19 1:39 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Jean Delvare, Hartmut Knaack, Jonathan Cameron, lm-sensors,
linux-iio
On Mon, Feb 18, 2013 at 02:38:58PM +0100, Lars-Peter Clausen wrote:
> This allows a userspace application to poll() on the alarm files to get notified
> in case of an temperature threshold event.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>
> ---
> Changes since v1:
> * Switch from level triggered and interrupt mode to edge triggered and
> comparator mode. In interrupt mode the status register get's cleared after
> the first read and so the _alarm files will always read 0, which is not
> really what we want.
> * Use dev_name(dev) for the interrupt name, instead of 'name' which will be
> NULL for I2C devices
> ---
> drivers/hwmon/adt7310.c | 4 ++--
> drivers/hwmon/adt7410.c | 4 ++--
> drivers/hwmon/adt7x10.c | 43 +++++++++++++++++++++++++++++++++++++++----
> drivers/hwmon/adt7x10.h | 4 ++--
> 4 files changed, 45 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hwmon/adt7310.c b/drivers/hwmon/adt7310.c
> index 2196ac3..e58bb68 100644
> --- a/drivers/hwmon/adt7310.c
> +++ b/drivers/hwmon/adt7310.c
> @@ -82,13 +82,13 @@ static const struct adt7410_ops adt7310_spi_ops = {
>
> static int adt7310_spi_probe(struct spi_device *spi)
> {
> - return adt7410_probe(&spi->dev, spi_get_device_id(spi)->name,
> + return adt7410_probe(&spi->dev, spi_get_device_id(spi)->name, spi->irq,
> &adt7310_spi_ops);
> }
>
> static int adt7310_spi_remove(struct spi_device *spi)
> {
> - return adt7410_remove(&spi->dev);
> + return adt7410_remove(&spi->dev, spi->irq);
> }
>
> static const struct spi_device_id adt7310_id[] = {
> diff --git a/drivers/hwmon/adt7410.c b/drivers/hwmon/adt7410.c
> index b500ab3..3a7d905 100644
> --- a/drivers/hwmon/adt7410.c
> +++ b/drivers/hwmon/adt7410.c
> @@ -48,12 +48,12 @@ static int adt7410_i2c_probe(struct i2c_client *client,
> I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
> return -ENODEV;
>
> - return adt7410_probe(&client->dev, NULL, &adt7410_i2c_ops);
> + return adt7410_probe(&client->dev, NULL, client->irq, &adt7410_i2c_ops);
> }
>
> static int adt7410_i2c_remove(struct i2c_client *client)
> {
> - return adt7410_remove(&client->dev);
> + return adt7410_remove(&client->dev, client->irq);
> }
>
> static const struct i2c_device_id adt7410_ids[] = {
> diff --git a/drivers/hwmon/adt7x10.c b/drivers/hwmon/adt7x10.c
> index eeff198c..2340a70 100644
> --- a/drivers/hwmon/adt7x10.c
> +++ b/drivers/hwmon/adt7x10.c
> @@ -30,6 +30,7 @@
> #include <linux/err.h>
> #include <linux/mutex.h>
> #include <linux/delay.h>
> +#include <linux/interrupt.h>
>
> #include "adt7x10.h"
>
> @@ -112,6 +113,25 @@ static const u8 ADT7410_REG_TEMP[4] = {
> ADT7410_T_CRIT, /* critical */
> };
>
> +static irqreturn_t adt7410_irq_handler(int irq, void *private)
> +{
> + struct device *dev = private;
> + int status;
> +
> + status = adt7410_read_byte(dev, ADT7410_STATUS);
> + if (status < 0)
> + return IRQ_HANDLED;
> +
> + if (status & ADT7410_STAT_T_HIGH)
> + sysfs_notify(&dev->kobj, NULL, "temp1_max_alarm");
> + if (status & ADT7410_STAT_T_LOW)
> + sysfs_notify(&dev->kobj, NULL, "temp1_min_alarm");
> + if (status & ADT7410_STAT_T_CRIT)
> + sysfs_notify(&dev->kobj, NULL, "temp1_crit_alarm");
> +
Question about semantics: Do you want a notification on all attributes each time
one is set during an interrupt, or do you want a notification each time an
attribute changes state ? With the above code, the 1->0 transition does not
result in a notification. Is that on purpose ?
> + return IRQ_HANDLED;
> +}
> +
> static int adt7410_temp_ready(struct device *dev)
> {
> int i, status;
> @@ -357,7 +377,7 @@ static const struct attribute_group adt7410_group = {
> .attrs = adt7410_attributes,
> };
>
> -int adt7410_probe(struct device *dev, const char *name,
> +int adt7410_probe(struct device *dev, const char *name, int irq,
> const struct adt7410_ops *ops)
> {
> struct adt7410_data *data;
> @@ -383,11 +403,13 @@ int adt7410_probe(struct device *dev, const char *name,
> data->oldconfig = ret;
>
> /*
> - * Set to 16 bit resolution, continous conversion and comparator mode.
> + * Set to 16 bit resolution and continous conversion
> */
> data->config = data->oldconfig;
> - data->config &= ~ADT7410_MODE_MASK;
> + data->config &= ~(ADT7410_MODE_MASK | ADT7410_CT_POLARITY |
> + ADT7410_INT_POLARITY);
> data->config |= ADT7410_FULL | ADT7410_RESOLUTION | ADT7410_EVENT_MODE;
> +
> if (data->config != data->oldconfig) {
> ret = adt7410_write_byte(dev, ADT7410_CONFIG, data->config);
> if (ret)
> @@ -421,8 +443,18 @@ int adt7410_probe(struct device *dev, const char *name,
> goto exit_remove_name;
> }
>
> + if (irq > 0) {
> + ret = request_threaded_irq(irq, NULL, adt7410_irq_handler,
> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> + dev_name(dev), dev);
If you use devm_request_threaded_irq, you don't have to release it on remove.
> + if (ret)
> + goto exit_hwmon_device_unregister;
> + }
> +
This code should be before hwmon registration.
> return 0;
>
> +exit_hwmon_device_unregister:
> + hwmon_device_unregister(data->hwmon_dev);
> exit_remove_name:
> if (name)
> device_remove_file(dev, &dev_attr_name);
> @@ -434,10 +466,13 @@ exit_restore:
> }
> EXPORT_SYMBOL_GPL(adt7410_probe);
>
> -int adt7410_remove(struct device *dev)
> +int adt7410_remove(struct device *dev, int irq)
> {
> struct adt7410_data *data = dev_get_drvdata(dev);
>
> + if (irq > 0)
> + free_irq(irq, dev);
> +
> hwmon_device_unregister(data->hwmon_dev);
> if (data->name)
> device_remove_file(dev, &dev_attr_name);
> diff --git a/drivers/hwmon/adt7x10.h b/drivers/hwmon/adt7x10.h
> index a7165e6..d67badf 100644
> --- a/drivers/hwmon/adt7x10.h
> +++ b/drivers/hwmon/adt7x10.h
> @@ -33,9 +33,9 @@ struct adt7410_ops {
> int (*write_word)(struct device *, u8 reg, u16 data);
> };
>
> -int adt7410_probe(struct device *dev, const char *name,
> +int adt7410_probe(struct device *dev, const char *name, int irq,
> const struct adt7410_ops *ops);
> -int adt7410_remove(struct device *dev);
> +int adt7410_remove(struct device *dev, int irq);
>
>
> #ifdef CONFIG_PM_SLEEP
> --
> 1.8.0
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/4] hwmon: (adt7410) Add support for the adt7310/adt7320
2013-02-19 1:30 ` Guenter Roeck
@ 2013-02-19 11:57 ` Lars-Peter Clausen
2013-02-19 16:52 ` Guenter Roeck
0 siblings, 1 reply; 23+ messages in thread
From: Lars-Peter Clausen @ 2013-02-19 11:57 UTC (permalink / raw)
To: Guenter Roeck
Cc: Jean Delvare, Hartmut Knaack, Jonathan Cameron, lm-sensors,
linux-iio
On 02/19/2013 02:30 AM, Guenter Roeck wrote:
> On Mon, Feb 18, 2013 at 02:38:57PM +0100, Lars-Peter Clausen wrote:
[...]
>> diff --git a/drivers/hwmon/adt7310.c b/drivers/hwmon/adt7310.c
>> new file mode 100644
>> index 0000000..2196ac3
>> --- /dev/null
>> +++ b/drivers/hwmon/adt7310.c
>> @@ -0,0 +1,115 @@
>> +/*
>> + * ADT7310/ADT7310 digital temperature sensor driver
>> + *
>> + * Copyright 2010-2013 Analog Devices Inc.
>
> Not really; copyright is yours, unless you are signing it off to analog or if
> you are working for them and have permission (or the duty) to sign off the
> copyright. Even then it should be 2013 only for this file.
I work for them. The copyright notice comes from the IIO adt7410/adt7310
driver, where also this code comes from. Although this portion of the code
was written in 2012, so maybe change it to 2012-2013.
[...]
>> diff --git a/drivers/hwmon/adt7x10.h b/drivers/hwmon/adt7x10.h
>> new file mode 100644
>> index 0000000..a7165e6
>> --- /dev/null
>> +++ b/drivers/hwmon/adt7x10.h
>> @@ -0,0 +1,48 @@
>> +#ifndef __HWMON_ADT7X10_H__
>> +#define __HWMON_ADT7X10_H__
>> +
>> +#include <linux/types.h>
>> +#include <linux/pm.h>
>> +
>> +/* ADT7410 registers definition */
>> +#define ADT7410_TEMPERATURE 0
>> +#define ADT7410_STATUS 2
>> +#define ADT7410_CONFIG 3
>> +#define ADT7410_T_ALARM_HIGH 4
>> +#define ADT7410_T_ALARM_LOW 6
>> +#define ADT7410_T_CRIT 8
>> +#define ADT7410_T_HYST 0xA
>> +#define ADT7410_ID 0xB
>> +
>> +/* ADT7310 registers definition */
>> +#define ADT7310_STATUS 0
>> +#define ADT7310_CONFIG 1
>> +#define ADT7310_TEMPERATURE 2
>> +#define ADT7310_ID 3
>> +#define ADT7310_T_CRIT 4
>> +#define ADT7310_T_HYST 5
>> +#define ADT7310_T_ALARM_HIGH 6
>> +#define ADT7310_T_ALARM_LOW 7
>> +
>> +struct device;
>> +
>> +struct adt7410_ops {
>> + int (*read_byte)(struct device *, u8 reg);
>> + int (*write_byte)(struct device *, u8 reg, u8 data);
>> + int (*read_word)(struct device *, u8 reg);
>> + int (*write_word)(struct device *, u8 reg, u16 data);
>> +};
>> +
>> +int adt7410_probe(struct device *dev, const char *name,
>> + const struct adt7410_ops *ops);
>> +int adt7410_remove(struct device *dev);
>> +
>
> I think the above should be adt7x10_ops, adt7x10_probe and adt7x10_remove, to
> indicate that those are common functions.
>
> I would actually suggest to rename all functions in adt7x10.c (not just the
> exported ones).
I normally don't like to rename all the symbols in one file just because
support for another part is added, since it just cause lots of noise in the
diff. My initial plan was to call the two new modules adt7410-i2c and
adt7410-spi, but then though it is probably going to be confusing to have
the driver for the adt7310 called adt7410-spi. In this case the renaming
probably doesn't hurt since we already have lots of diff noise anyway. It
probably would have made sense to make the review easier to split this up in
two patches, one which renames adt7410 to adt7x10 and a second one which
adds the new functionality.
Thanks,
- Lars
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/4] hwmon: (adt7x10) Add alarm interrupt support
2013-02-19 1:39 ` Guenter Roeck
@ 2013-02-19 12:05 ` Lars-Peter Clausen
2013-02-19 17:10 ` Guenter Roeck
0 siblings, 1 reply; 23+ messages in thread
From: Lars-Peter Clausen @ 2013-02-19 12:05 UTC (permalink / raw)
To: Guenter Roeck
Cc: Jean Delvare, Hartmut Knaack, Jonathan Cameron, lm-sensors,
linux-iio
On 02/19/2013 02:39 AM, Guenter Roeck wrote:
> On Mon, Feb 18, 2013 at 02:38:58PM +0100, Lars-Peter Clausen wrote:
>> This allows a userspace application to poll() on the alarm files to get notified
>> in case of an temperature threshold event.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>>
[...]
>>
>> +static irqreturn_t adt7410_irq_handler(int irq, void *private)
>> +{
>> + struct device *dev = private;
>> + int status;
>> +
>> + status = adt7410_read_byte(dev, ADT7410_STATUS);
>> + if (status < 0)
>> + return IRQ_HANDLED;
>> +
>> + if (status & ADT7410_STAT_T_HIGH)
>> + sysfs_notify(&dev->kobj, NULL, "temp1_max_alarm");
>> + if (status & ADT7410_STAT_T_LOW)
>> + sysfs_notify(&dev->kobj, NULL, "temp1_min_alarm");
>> + if (status & ADT7410_STAT_T_CRIT)
>> + sysfs_notify(&dev->kobj, NULL, "temp1_crit_alarm");
>> +
> Question about semantics: Do you want a notification on all attributes each time
> one is set during an interrupt, or do you want a notification each time an
> attribute changes state ? With the above code, the 1->0 transition does not
> result in a notification. Is that on purpose ?
>
Now that the code uses an edge triggered IRQ and comparator mode it would
actually be possible to also generate an event for a 1->0 transition, but
I'm not sure how much sense that makes. Usually you'd only want to respond
quickly to the case where a threshold event happens.
>> + return IRQ_HANDLED;
>> +}
>> +
>> static int adt7410_temp_ready(struct device *dev)
>> {
>> int i, status;
>> @@ -357,7 +377,7 @@ static const struct attribute_group adt7410_group = {
>> .attrs = adt7410_attributes,
>> };
>>
>> -int adt7410_probe(struct device *dev, const char *name,
>> +int adt7410_probe(struct device *dev, const char *name, int irq,
>> const struct adt7410_ops *ops)
>> {
>> struct adt7410_data *data;
>> @@ -383,11 +403,13 @@ int adt7410_probe(struct device *dev, const char *name,
>> data->oldconfig = ret;
>>
>> /*
>> - * Set to 16 bit resolution, continous conversion and comparator mode.
>> + * Set to 16 bit resolution and continous conversion
>> */
>> data->config = data->oldconfig;
>> - data->config &= ~ADT7410_MODE_MASK;
>> + data->config &= ~(ADT7410_MODE_MASK | ADT7410_CT_POLARITY |
>> + ADT7410_INT_POLARITY);
>> data->config |= ADT7410_FULL | ADT7410_RESOLUTION | ADT7410_EVENT_MODE;
>> +
>> if (data->config != data->oldconfig) {
>> ret = adt7410_write_byte(dev, ADT7410_CONFIG, data->config);
>> if (ret)
>> @@ -421,8 +443,18 @@ int adt7410_probe(struct device *dev, const char *name,
>> goto exit_remove_name;
>> }
>>
>> + if (irq > 0) {
>> + ret = request_threaded_irq(irq, NULL, adt7410_irq_handler,
>> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>> + dev_name(dev), dev);
>
> If you use devm_request_threaded_irq, you don't have to release it on remove.
devm_request_threaded_irq is tricky, since the IRQ will only be freed after
the drivers remove callback has been run. This can cause race conditions,
since the IRQ handler often access resources already freed in the remove
callback. In this case it will work fine since sysfs_notify handles the case
where the files are already gone gracefully and just does nothing in that
case. I'd still prefer to keep the explicit free, so things are cleaned up
in the reverse order to the way they were set up.
>
>> + if (ret)
>> + goto exit_hwmon_device_unregister;
>> + }
>> +
>
> This code should be before hwmon registration.
Why? It doesn't make sense to signal an event before userspace can actually
discover the device.
>
>> return 0;
>>
>> +exit_hwmon_device_unregister:
>> + hwmon_device_unregister(data->hwmon_dev);
>> exit_remove_name:
>> if (name)
>> device_remove_file(dev, &dev_attr_name);
>> @@ -434,10 +466,13 @@ exit_restore:
>> }
>> EXPORT_SYMBOL_GPL(adt7410_probe);
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/4] hwmon: (adt7410) Add support for the adt7310/adt7320
2013-02-19 11:57 ` Lars-Peter Clausen
@ 2013-02-19 16:52 ` Guenter Roeck
0 siblings, 0 replies; 23+ messages in thread
From: Guenter Roeck @ 2013-02-19 16:52 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Jean Delvare, Hartmut Knaack, Jonathan Cameron, lm-sensors,
linux-iio
On Tue, Feb 19, 2013 at 12:57:43PM +0100, Lars-Peter Clausen wrote:
> On 02/19/2013 02:30 AM, Guenter Roeck wrote:
> > On Mon, Feb 18, 2013 at 02:38:57PM +0100, Lars-Peter Clausen wrote:
> [...]
> >> diff --git a/drivers/hwmon/adt7310.c b/drivers/hwmon/adt7310.c
> >> new file mode 100644
> >> index 0000000..2196ac3
> >> --- /dev/null
> >> +++ b/drivers/hwmon/adt7310.c
> >> @@ -0,0 +1,115 @@
> >> +/*
> >> + * ADT7310/ADT7310 digital temperature sensor driver
> >> + *
> >> + * Copyright 2010-2013 Analog Devices Inc.
> >
> > Not really; copyright is yours, unless you are signing it off to analog or if
> > you are working for them and have permission (or the duty) to sign off the
> > copyright. Even then it should be 2013 only for this file.
>
> I work for them. The copyright notice comes from the IIO adt7410/adt7310
> driver, where also this code comes from. Although this portion of the code
> was written in 2012, so maybe change it to 2012-2013.
>
Ok.
> [...]
> >> diff --git a/drivers/hwmon/adt7x10.h b/drivers/hwmon/adt7x10.h
> >> new file mode 100644
> >> index 0000000..a7165e6
> >> --- /dev/null
> >> +++ b/drivers/hwmon/adt7x10.h
> >> @@ -0,0 +1,48 @@
> >> +#ifndef __HWMON_ADT7X10_H__
> >> +#define __HWMON_ADT7X10_H__
> >> +
> >> +#include <linux/types.h>
> >> +#include <linux/pm.h>
> >> +
> >> +/* ADT7410 registers definition */
> >> +#define ADT7410_TEMPERATURE 0
> >> +#define ADT7410_STATUS 2
> >> +#define ADT7410_CONFIG 3
> >> +#define ADT7410_T_ALARM_HIGH 4
> >> +#define ADT7410_T_ALARM_LOW 6
> >> +#define ADT7410_T_CRIT 8
> >> +#define ADT7410_T_HYST 0xA
> >> +#define ADT7410_ID 0xB
> >> +
> >> +/* ADT7310 registers definition */
> >> +#define ADT7310_STATUS 0
> >> +#define ADT7310_CONFIG 1
> >> +#define ADT7310_TEMPERATURE 2
> >> +#define ADT7310_ID 3
> >> +#define ADT7310_T_CRIT 4
> >> +#define ADT7310_T_HYST 5
> >> +#define ADT7310_T_ALARM_HIGH 6
> >> +#define ADT7310_T_ALARM_LOW 7
> >> +
> >> +struct device;
> >> +
> >> +struct adt7410_ops {
> >> + int (*read_byte)(struct device *, u8 reg);
> >> + int (*write_byte)(struct device *, u8 reg, u8 data);
> >> + int (*read_word)(struct device *, u8 reg);
> >> + int (*write_word)(struct device *, u8 reg, u16 data);
> >> +};
> >> +
> >> +int adt7410_probe(struct device *dev, const char *name,
> >> + const struct adt7410_ops *ops);
> >> +int adt7410_remove(struct device *dev);
> >> +
> >
> > I think the above should be adt7x10_ops, adt7x10_probe and adt7x10_remove, to
> > indicate that those are common functions.
> >
> > I would actually suggest to rename all functions in adt7x10.c (not just the
> > exported ones).
>
> I normally don't like to rename all the symbols in one file just because
> support for another part is added, since it just cause lots of noise in the
> diff. My initial plan was to call the two new modules adt7410-i2c and
> adt7410-spi, but then though it is probably going to be confusing to have
> the driver for the adt7310 called adt7410-spi. In this case the renaming
> probably doesn't hurt since we already have lots of diff noise anyway. It
> probably would have made sense to make the review easier to split this up in
> two patches, one which renames adt7410 to adt7x10 and a second one which
> adds the new functionality.
>
In general I agree, but the diff shows the renamed file as all-new code anyway,
so I figured renaming the functions doesn't really create additional noise. I'll
leave it up to you how to handle it in detail, but the exported common functions
and defines should really not be named adt7410. After all, future developers
won't know the history.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/4] hwmon: (adt7x10) Add alarm interrupt support
2013-02-19 12:05 ` Lars-Peter Clausen
@ 2013-02-19 17:10 ` Guenter Roeck
0 siblings, 0 replies; 23+ messages in thread
From: Guenter Roeck @ 2013-02-19 17:10 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Jean Delvare, Hartmut Knaack, Jonathan Cameron, lm-sensors,
linux-iio
On Tue, Feb 19, 2013 at 01:05:42PM +0100, Lars-Peter Clausen wrote:
> On 02/19/2013 02:39 AM, Guenter Roeck wrote:
> > On Mon, Feb 18, 2013 at 02:38:58PM +0100, Lars-Peter Clausen wrote:
> >> This allows a userspace application to poll() on the alarm files to get notified
> >> in case of an temperature threshold event.
> >>
> >> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> >>
> [...]
> >>
> >> +static irqreturn_t adt7410_irq_handler(int irq, void *private)
> >> +{
> >> + struct device *dev = private;
> >> + int status;
> >> +
> >> + status = adt7410_read_byte(dev, ADT7410_STATUS);
> >> + if (status < 0)
> >> + return IRQ_HANDLED;
> >> +
> >> + if (status & ADT7410_STAT_T_HIGH)
> >> + sysfs_notify(&dev->kobj, NULL, "temp1_max_alarm");
> >> + if (status & ADT7410_STAT_T_LOW)
> >> + sysfs_notify(&dev->kobj, NULL, "temp1_min_alarm");
> >> + if (status & ADT7410_STAT_T_CRIT)
> >> + sysfs_notify(&dev->kobj, NULL, "temp1_crit_alarm");
> >> +
> > Question about semantics: Do you want a notification on all attributes each time
> > one is set during an interrupt, or do you want a notification each time an
> > attribute changes state ? With the above code, the 1->0 transition does not
> > result in a notification. Is that on purpose ?
> >
>
> Now that the code uses an edge triggered IRQ and comparator mode it would
> actually be possible to also generate an event for a 1->0 transition, but
> I'm not sure how much sense that makes. Usually you'd only want to respond
> quickly to the case where a threshold event happens.
>
I am not sure if it would make sense either. I am fine either way - this is
quite new functionality for hwmon, and we can add support for two-way
notification later if it turns out to be needed.
> >> + return IRQ_HANDLED;
> >> +}
> >> +
> >> static int adt7410_temp_ready(struct device *dev)
> >> {
> >> int i, status;
> >> @@ -357,7 +377,7 @@ static const struct attribute_group adt7410_group = {
> >> .attrs = adt7410_attributes,
> >> };
> >>
> >> -int adt7410_probe(struct device *dev, const char *name,
> >> +int adt7410_probe(struct device *dev, const char *name, int irq,
> >> const struct adt7410_ops *ops)
> >> {
> >> struct adt7410_data *data;
> >> @@ -383,11 +403,13 @@ int adt7410_probe(struct device *dev, const char *name,
> >> data->oldconfig = ret;
> >>
> >> /*
> >> - * Set to 16 bit resolution, continous conversion and comparator mode.
> >> + * Set to 16 bit resolution and continous conversion
> >> */
> >> data->config = data->oldconfig;
> >> - data->config &= ~ADT7410_MODE_MASK;
> >> + data->config &= ~(ADT7410_MODE_MASK | ADT7410_CT_POLARITY |
> >> + ADT7410_INT_POLARITY);
> >> data->config |= ADT7410_FULL | ADT7410_RESOLUTION | ADT7410_EVENT_MODE;
> >> +
> >> if (data->config != data->oldconfig) {
> >> ret = adt7410_write_byte(dev, ADT7410_CONFIG, data->config);
> >> if (ret)
> >> @@ -421,8 +443,18 @@ int adt7410_probe(struct device *dev, const char *name,
> >> goto exit_remove_name;
> >> }
> >>
> >> + if (irq > 0) {
> >> + ret = request_threaded_irq(irq, NULL, adt7410_irq_handler,
> >> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> >> + dev_name(dev), dev);
> >
> > If you use devm_request_threaded_irq, you don't have to release it on remove.
>
> devm_request_threaded_irq is tricky, since the IRQ will only be freed after
> the drivers remove callback has been run. This can cause race conditions,
> since the IRQ handler often access resources already freed in the remove
> callback. In this case it will work fine since sysfs_notify handles the case
> where the files are already gone gracefully and just does nothing in that
> case. I'd still prefer to keep the explicit free, so things are cleaned up
> in the reverse order to the way they were set up.
>
Ok, makes sense.
> >
> >> + if (ret)
> >> + goto exit_hwmon_device_unregister;
> >> + }
> >> +
> >
> > This code should be before hwmon registration.
>
> Why? It doesn't make sense to signal an event before userspace can actually
> discover the device.
>
You are right. Historically we keep hwmon registration as last operation to
avoid user-space confusion if the attributes are gone by the time user-space
tries to access them. Getting a notification prior to registration is just as
bad, and would be "normal" operation instead of error handling, so your code
makes sense.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers
2013-02-19 1:02 ` Guenter Roeck
@ 2013-02-19 19:39 ` Hartmut Knaack
2013-02-20 1:22 ` Guenter Roeck
0 siblings, 1 reply; 23+ messages in thread
From: Hartmut Knaack @ 2013-02-19 19:39 UTC (permalink / raw)
To: Guenter Roeck
Cc: Lars-Peter Clausen, Jean Delvare, Jonathan Cameron, lm-sensors,
linux-iio
Guenter Roeck schrieb:
> On Mon, Feb 18, 2013 at 09:22:18PM +0100, Hartmut Knaack wrote:
>> Lars-Peter Clausen schrieb:
>>> Currently each time the temperature register is read the driver also reads the
>>> threshold and hysteresis registers. This increases the amount of I2C traffic and
>>> time needed to read the temperature by a factor of ~5. Neither the threshold nor
>>> the hysteresis change on their own, so once we've read them, we should be able
>>> to just use the cached value of the registers. This patch modifies the code
>>> accordingly and only reads the threshold and hysteresis registers once during
>>> probe.
>> I have been thinking about this a lot, and I am concerned about data integrity. From what I know about I2C, there is no data integrity verification specified in the protocol. So, what the master sends is not necessarily what the slave receives (not to mention other devices on the bus, which could potentially mess around with the slaves, or even reset of the slave). Reading back just cached values makes it pretty hard to verify, if there are issues. I think it might be better to call a read-temperature function with a parameter that indicates, which temperature register is required.
> I am not concerned about that, unless there is a known issue with the chip
> and it is known to return bad data under some circumstances. I am doing the
> same in the PMBus drivers, since there are simply too many limit registers
> to read on some of the chips (there may literally be more than a hundred).
> That works fine most of the time; if it does not work, it is a chip problem,
> an i2c bus master problem, a hardware signal problem, or a combination of all.
> I actually think it is better if the problem is exposed by cached bad readings.
Could you please outline the last sentence? I'm having trouble to understand your intention with cached bad readings.
> Then there is a chance to do something about it. This would be much better
> than just re-reading the value next time and ignoring the problem.
>
> Something else - the commit window just opened (a week earlier than I guessed),
> so this part of the series is going to miss 3.9. Hartmut, if you plan to provide
> an Acked-by or Reviewed-by or Tested-by for the first part of the series, you'll
> have to do it soon, as I plan to send my push request to Linus around Thursday.
OK
>
> Thanks,
> Guenter
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers
2013-02-19 19:39 ` Hartmut Knaack
@ 2013-02-20 1:22 ` Guenter Roeck
[not found] ` <5128112C.4080909@gmx.de>
0 siblings, 1 reply; 23+ messages in thread
From: Guenter Roeck @ 2013-02-20 1:22 UTC (permalink / raw)
To: Hartmut Knaack
Cc: Lars-Peter Clausen, Jean Delvare, Jonathan Cameron, lm-sensors,
linux-iio
On Tue, Feb 19, 2013 at 08:39:33PM +0100, Hartmut Knaack wrote:
> Guenter Roeck schrieb:
> > On Mon, Feb 18, 2013 at 09:22:18PM +0100, Hartmut Knaack wrote:
> >> Lars-Peter Clausen schrieb:
> >>> Currently each time the temperature register is read the driver also reads the
> >>> threshold and hysteresis registers. This increases the amount of I2C traffic and
> >>> time needed to read the temperature by a factor of ~5. Neither the threshold nor
> >>> the hysteresis change on their own, so once we've read them, we should be able
> >>> to just use the cached value of the registers. This patch modifies the code
> >>> accordingly and only reads the threshold and hysteresis registers once during
> >>> probe.
> >> I have been thinking about this a lot, and I am concerned about data integrity. From what I know about I2C, there is no data integrity verification specified in the protocol. So, what the master sends is not necessarily what the slave receives (not to mention other devices on the bus, which could potentially mess around with the slaves, or even reset of the slave). Reading back just cached values makes it pretty hard to verify, if there are issues. I think it might be better to call a read-temperature function with a parameter that indicates, which temperature register is required.
> > I am not concerned about that, unless there is a known issue with the chip
> > and it is known to return bad data under some circumstances. I am doing the
> > same in the PMBus drivers, since there are simply too many limit registers
> > to read on some of the chips (there may literally be more than a hundred).
> > That works fine most of the time; if it does not work, it is a chip problem,
> > an i2c bus master problem, a hardware signal problem, or a combination of all.
> > I actually think it is better if the problem is exposed by cached bad readings.
> Could you please outline the last sentence? I'm having trouble to understand your intention with cached bad readings.
Someone will actually notice it (hopefully while testing) and provide feedback.
This gives a chance to fix the problem instead of having it linger around ...
which would likely be the case if the problem is not persistent.
Guenter
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers
[not found] ` <5128112C.4080909@gmx.de>
@ 2013-02-23 20:18 ` Guenter Roeck
2013-02-25 22:03 ` Hartmut Knaack
2013-02-25 9:54 ` Lars-Peter Clausen
1 sibling, 1 reply; 23+ messages in thread
From: Guenter Roeck @ 2013-02-23 20:18 UTC (permalink / raw)
To: Hartmut Knaack
Cc: Lars-Peter Clausen, Jean Delvare, Jonathan Cameron, lm-sensors,
linux-iio
On Sat, Feb 23, 2013 at 01:45:32AM +0100, Hartmut Knaack wrote:
> Guenter Roeck schrieb:
> > On Tue, Feb 19, 2013 at 08:39:33PM +0100, Hartmut Knaack wrote:
> >> Guenter Roeck schrieb:
> >>> On Mon, Feb 18, 2013 at 09:22:18PM +0100, Hartmut Knaack wrote:
> >>>> Lars-Peter Clausen schrieb:
> >>>>> Currently each time the temperature register is read the driver also reads the
> >>>>> threshold and hysteresis registers. This increases the amount of I2C traffic and
> >>>>> time needed to read the temperature by a factor of ~5. Neither the threshold nor
> >>>>> the hysteresis change on their own, so once we've read them, we should be able
> >>>>> to just use the cached value of the registers. This patch modifies the code
> >>>>> accordingly and only reads the threshold and hysteresis registers once during
> >>>>> probe.
> >>>> I have been thinking about this a lot, and I am concerned about data integrity. From what I know about I2C, there is no data integrity verification specified in the protocol. So, what the master sends is not necessarily what the slave receives (not to mention other devices on the bus, which could potentially mess around with the slaves, or even reset of the slave). Reading back just cached values makes it pretty hard to verify, if there are issues. I think it might be better to call a read-temperature function with a parameter that indicates, which temperature register is required.
> >>> I am not concerned about that, unless there is a known issue with the chip
> >>> and it is known to return bad data under some circumstances. I am doing the
> >>> same in the PMBus drivers, since there are simply too many limit registers
> >>> to read on some of the chips (there may literally be more than a hundred).
> >>> That works fine most of the time; if it does not work, it is a chip problem,
> >>> an i2c bus master problem, a hardware signal problem, or a combination of all.
> >>> I actually think it is better if the problem is exposed by cached bad readings.
> >> Could you please outline the last sentence? I'm having trouble to understand your intention with cached bad readings.
> > Someone will actually notice it (hopefully while testing) and provide feedback.
> > This gives a chance to fix the problem instead of having it linger around ...
> > which would likely be the case if the problem is not persistent.
> >
> > Guenter
> Well, think about a use case where you optically decouple your master and slave using PCA9600 (may it be using optical fibers to cover a big distance, or just to operate the slave in a hazardous environment), where the slave is powered from a different source. Now, if the slaves Vcc drops for a certain time, all its registers get reset (especially min, max and crit) - without the master noticing anything.
> Therefor I would prefer something like the following solution, where unnecessary load on the bus gets avoided and the cached values get used, until they expire and get read again from the sensor. This is mainly a draft, but do you see any reason against it?
Yes, it adds a lot of complexity. Also, at least in theory it would apply to all
i2c drivers, so solving the problem for one chip doesn't help much.
A somewhat cleanly designed hardware should inform the software that a device went
offline, for example with an interrupt; otherwise its operation could be severely
affected and system behavior would be unpredictable.
So the question here is if this is a practical problem in your use case, or a
theoretical problem. If it is a theoretical problem, we can address it for
affected drivers as it is seen. If it is a practical problem, I think we'll need
to come up with a simpler solution. The only one I can think of right now would
be a module parameter. If set, it the driver could re-load the cache each time
the temperature sensor is updated.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers
[not found] ` <5128112C.4080909@gmx.de>
2013-02-23 20:18 ` Guenter Roeck
@ 2013-02-25 9:54 ` Lars-Peter Clausen
2013-02-25 21:30 ` Hartmut Knaack
1 sibling, 1 reply; 23+ messages in thread
From: Lars-Peter Clausen @ 2013-02-25 9:54 UTC (permalink / raw)
To: Hartmut Knaack
Cc: Guenter Roeck, Jean Delvare, Jonathan Cameron, lm-sensors,
linux-iio
On 02/23/2013 01:45 AM, Hartmut Knaack wrote:
> Guenter Roeck schrieb:
>> On Tue, Feb 19, 2013 at 08:39:33PM +0100, Hartmut Knaack wrote:
>>> Guenter Roeck schrieb:
>>>> On Mon, Feb 18, 2013 at 09:22:18PM +0100, Hartmut Knaack wrote:
>>>>> Lars-Peter Clausen schrieb:
>>>>>> Currently each time the temperature register is read the driver also reads the
>>>>>> threshold and hysteresis registers. This increases the amount of I2C traffic and
>>>>>> time needed to read the temperature by a factor of ~5. Neither the threshold nor
>>>>>> the hysteresis change on their own, so once we've read them, we should be able
>>>>>> to just use the cached value of the registers. This patch modifies the code
>>>>>> accordingly and only reads the threshold and hysteresis registers once during
>>>>>> probe.
>>>>> I have been thinking about this a lot, and I am concerned about data integrity. From what I know about I2C, there is no data integrity verification specified in the protocol. So, what the master sends is not necessarily what the slave receives (not to mention other devices on the bus, which could potentially mess around with the slaves, or even reset of the slave). Reading back just cached values makes it pretty hard to verify, if there are issues. I think it might be better to call a read-temperature function with a parameter that indicates, which temperature register is required.
>>>> I am not concerned about that, unless there is a known issue with the chip
>>>> and it is known to return bad data under some circumstances. I am doing the
>>>> same in the PMBus drivers, since there are simply too many limit registers
>>>> to read on some of the chips (there may literally be more than a hundred).
>>>> That works fine most of the time; if it does not work, it is a chip problem,
>>>> an i2c bus master problem, a hardware signal problem, or a combination of all.
>>>> I actually think it is better if the problem is exposed by cached bad readings.
>>> Could you please outline the last sentence? I'm having trouble to understand your intention with cached bad readings.
>> Someone will actually notice it (hopefully while testing) and provide feedback.
>> This gives a chance to fix the problem instead of having it linger around ...
>> which would likely be the case if the problem is not persistent.
>>
>> Guenter
> Well, think about a use case where you optically decouple your master and slave using PCA9600 (may it be using optical fibers to cover a big distance, or just to operate the slave in a hazardous environment), where the slave is powered from a different source. Now, if the slaves Vcc drops for a certain time, all its registers get reset (especially min, max and crit) - without the master noticing anything.
> Therefor I would prefer something like the following solution, where unnecessary load on the bus gets avoided and the cached values get used, until they expire and get read again from the sensor. This is mainly a draft, but do you see any reason against it?
> Thanks
>
That's a bit of an artificially constructed situation, isn't it? Anyway,
wouldn't it be better to not cache at all in that case. For the cache to be
useful userspace would have to poll the file with period of less than 1.5
seconds. I don't think anybody is going to do this for the threshold properties.
- Lars
> diff --git a/drivers/hwmon/adt7410.c b/drivers/hwmon/adt7410.c
> old mode 100644
> new mode 100755
> index 99a7290..dc11cac
> --- a/drivers/hwmon/adt7410.c
> +++ b/drivers/hwmon/adt7410.c
> @@ -91,8 +91,13 @@ struct adt7410_data {
> struct mutex update_lock;
> u8 config;
> u8 oldconfig;
> - bool valid; /* true if registers valid */
> - unsigned long last_updated; /* In jiffies */
> + u8 valid; /* true if registers valid */
> + unsigned long last_updated[5]; /* In jiffies
> + 0 = input
> + 1 = high
> + 2 = low
> + 3 = critical
> + 4 = hysteresis */
> s16 temp[4]; /* Register values,
> 0 = input
> 1 = high
> @@ -119,25 +124,27 @@ static int adt7410_temp_ready(struct i2c_client *client)
> return -ETIMEDOUT;
> }
>
> -static struct adt7410_data *adt7410_update_device(struct device *dev)
> +static struct adt7410_data *adt7410_update_device(struct device *dev, int i)
> {
> struct i2c_client *client = to_i2c_client(dev);
> struct adt7410_data *data = i2c_get_clientdata(client);
> struct adt7410_data *ret = data;
> mutex_lock(&data->update_lock);
>
> - if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
> - || !data->valid) {
> - int i, status;
> + if (time_after(jiffies, data->last_updated[i] + HZ + HZ / 2)
> + || !(data->valid & ~(1 << i))) {
> + int status;
>
> dev_dbg(&client->dev, "Starting update\n");
>
> - status = adt7410_temp_ready(client); /* check for new value */
> - if (unlikely(status)) {
> - ret = ERR_PTR(status);
> - goto abort;
> - }
> - for (i = 0; i < ARRAY_SIZE(data->temp); i++) {
> + if (i >= 0 && i <=3) {
> + if (!i) {
> + status = adt7410_temp_ready(client); /* check for new value */
> + if (unlikely(status)) {
> + ret = ERR_PTR(status);
> + goto abort;
> + }
> + }
> status = i2c_smbus_read_word_swapped(client,
> ADT7410_REG_TEMP[i]);
> if (unlikely(status < 0)) {
> @@ -148,18 +155,24 @@ static struct adt7410_data *adt7410_update_device(struct device *dev)
> goto abort;
> }
> data->temp[i] = status;
> + data->last_updated[i] = jiffies;
> + data->valid |= 1 << i;
> }
> - status = i2c_smbus_read_byte_data(client, ADT7410_T_HYST);
> - if (unlikely(status < 0)) {
> - dev_dbg(dev,
> - "Failed to read value: reg %d, error %d\n",
> - ADT7410_T_HYST, status);
> - ret = ERR_PTR(status);
> - goto abort;
> + else if (i == 4) {
> + status = i2c_smbus_read_byte_data(client, ADT7410_T_HYST);
> + if (unlikely(status < 0)) {
> + dev_dbg(dev,
> + "Failed to read value: reg %d, error %d\n",
> + ADT7410_T_HYST, status);
> + ret = ERR_PTR(status);
> + goto abort;
> + }
> + data->hyst = status;
> + data->last_updated[i] = jiffies;
> + data->valid |= 1 << i;
> }
> - data->hyst = status;
> - data->last_updated = jiffies;
> - data->valid = true;
> + else
> + ret = ERR_PTR(-EINVAL);
> }
>
> abort:
> @@ -193,7 +206,7 @@ static ssize_t adt7410_show_temp(struct device *dev,
> struct device_attribute *da, char *buf)
> {
> struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> - struct adt7410_data *data = adt7410_update_device(dev);
> + struct adt7410_data *data = adt7410_update_device(dev, attr->index);
>
> if (IS_ERR(data))
> return PTR_ERR(data);
> @@ -236,7 +249,10 @@ static ssize_t adt7410_show_t_hyst(struct device *dev,
> int nr = attr->index;
> int hyst;
>
> - data = adt7410_update_device(dev);
> + data = adt7410_update_device(dev, nr);
> + if (IS_ERR(data))
> + return PTR_ERR(data);
> + data = adt7410_update_device(dev, 4);
> if (IS_ERR(data))
> return PTR_ERR(data);
> hyst = (data->hyst & ADT7410_T_HYST_MASK) * 1000;
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers
2013-02-25 9:54 ` Lars-Peter Clausen
@ 2013-02-25 21:30 ` Hartmut Knaack
2013-02-26 1:39 ` Guenter Roeck
0 siblings, 1 reply; 23+ messages in thread
From: Hartmut Knaack @ 2013-02-25 21:30 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Guenter Roeck, Jean Delvare, Jonathan Cameron, lm-sensors,
linux-iio
Lars-Peter Clausen schrieb:
> On 02/23/2013 01:45 AM, Hartmut Knaack wrote:
>> Guenter Roeck schrieb:
>>> On Tue, Feb 19, 2013 at 08:39:33PM +0100, Hartmut Knaack wrote:
>>>> Guenter Roeck schrieb:
>>>>> On Mon, Feb 18, 2013 at 09:22:18PM +0100, Hartmut Knaack wrote:
>>>>>> Lars-Peter Clausen schrieb:
>>>>>>> Currently each time the temperature register is read the driver also reads the
>>>>>>> threshold and hysteresis registers. This increases the amount of I2C traffic and
>>>>>>> time needed to read the temperature by a factor of ~5. Neither the threshold nor
>>>>>>> the hysteresis change on their own, so once we've read them, we should be able
>>>>>>> to just use the cached value of the registers. This patch modifies the code
>>>>>>> accordingly and only reads the threshold and hysteresis registers once during
>>>>>>> probe.
>>>>>> I have been thinking about this a lot, and I am concerned about data integrity. From what I know about I2C, there is no data integrity verification specified in the protocol. So, what the master sends is not necessarily what the slave receives (not to mention other devices on the bus, which could potentially mess around with the slaves, or even reset of the slave). Reading back just cached values makes it pretty hard to verify, if there are issues. I think it might be better to call a read-temperature function with a parameter that indicates, which temperature register is required.
>>>>> I am not concerned about that, unless there is a known issue with the chip
>>>>> and it is known to return bad data under some circumstances. I am doing the
>>>>> same in the PMBus drivers, since there are simply too many limit registers
>>>>> to read on some of the chips (there may literally be more than a hundred).
>>>>> That works fine most of the time; if it does not work, it is a chip problem,
>>>>> an i2c bus master problem, a hardware signal problem, or a combination of all.
>>>>> I actually think it is better if the problem is exposed by cached bad readings.
>>>> Could you please outline the last sentence? I'm having trouble to understand your intention with cached bad readings.
>>> Someone will actually notice it (hopefully while testing) and provide feedback.
>>> This gives a chance to fix the problem instead of having it linger around ...
>>> which would likely be the case if the problem is not persistent.
>>>
>>> Guenter
>> Well, think about a use case where you optically decouple your master and slave using PCA9600 (may it be using optical fibers to cover a big distance, or just to operate the slave in a hazardous environment), where the slave is powered from a different source. Now, if the slaves Vcc drops for a certain time, all its registers get reset (especially min, max and crit) - without the master noticing anything.
>> Therefor I would prefer something like the following solution, where unnecessary load on the bus gets avoided and the cached values get used, until they expire and get read again from the sensor. This is mainly a draft, but do you see any reason against it?
>> Thanks
>>
> That's a bit of an artificially constructed situation, isn't it? Anyway,
> wouldn't it be better to not cache at all in that case. For the cache to be
> useful userspace would have to poll the file with period of less than 1.5
> seconds. I don't think anybody is going to do this for the threshold properties.
>
> - Lars
I could agree with not caching those registers - nobody would probably want to read them too often, intentionally (although bugged userspace code might DoS the bus). The other few i2c drivers I know also do uncached reads.
>
>
>> diff --git a/drivers/hwmon/adt7410.c b/drivers/hwmon/adt7410.c
>> old mode 100644
>> new mode 100755
>> index 99a7290..dc11cac
>> --- a/drivers/hwmon/adt7410.c
>> +++ b/drivers/hwmon/adt7410.c
>> @@ -91,8 +91,13 @@ struct adt7410_data {
>> struct mutex update_lock;
>> u8 config;
>> u8 oldconfig;
>> - bool valid; /* true if registers valid */
>> - unsigned long last_updated; /* In jiffies */
>> + u8 valid; /* true if registers valid */
>> + unsigned long last_updated[5]; /* In jiffies
>> + 0 = input
>> + 1 = high
>> + 2 = low
>> + 3 = critical
>> + 4 = hysteresis */
>> s16 temp[4]; /* Register values,
>> 0 = input
>> 1 = high
>> @@ -119,25 +124,27 @@ static int adt7410_temp_ready(struct i2c_client *client)
>> return -ETIMEDOUT;
>> }
>>
>> -static struct adt7410_data *adt7410_update_device(struct device *dev)
>> +static struct adt7410_data *adt7410_update_device(struct device *dev, int i)
>> {
>> struct i2c_client *client = to_i2c_client(dev);
>> struct adt7410_data *data = i2c_get_clientdata(client);
>> struct adt7410_data *ret = data;
>> mutex_lock(&data->update_lock);
>>
>> - if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
>> - || !data->valid) {
>> - int i, status;
>> + if (time_after(jiffies, data->last_updated[i] + HZ + HZ / 2)
>> + || !(data->valid & ~(1 << i))) {
>> + int status;
>>
>> dev_dbg(&client->dev, "Starting update\n");
>>
>> - status = adt7410_temp_ready(client); /* check for new value */
>> - if (unlikely(status)) {
>> - ret = ERR_PTR(status);
>> - goto abort;
>> - }
>> - for (i = 0; i < ARRAY_SIZE(data->temp); i++) {
>> + if (i >= 0 && i <=3) {
>> + if (!i) {
>> + status = adt7410_temp_ready(client); /* check for new value */
>> + if (unlikely(status)) {
>> + ret = ERR_PTR(status);
>> + goto abort;
>> + }
>> + }
>> status = i2c_smbus_read_word_swapped(client,
>> ADT7410_REG_TEMP[i]);
>> if (unlikely(status < 0)) {
>> @@ -148,18 +155,24 @@ static struct adt7410_data *adt7410_update_device(struct device *dev)
>> goto abort;
>> }
>> data->temp[i] = status;
>> + data->last_updated[i] = jiffies;
>> + data->valid |= 1 << i;
>> }
>> - status = i2c_smbus_read_byte_data(client, ADT7410_T_HYST);
>> - if (unlikely(status < 0)) {
>> - dev_dbg(dev,
>> - "Failed to read value: reg %d, error %d\n",
>> - ADT7410_T_HYST, status);
>> - ret = ERR_PTR(status);
>> - goto abort;
>> + else if (i == 4) {
>> + status = i2c_smbus_read_byte_data(client, ADT7410_T_HYST);
>> + if (unlikely(status < 0)) {
>> + dev_dbg(dev,
>> + "Failed to read value: reg %d, error %d\n",
>> + ADT7410_T_HYST, status);
>> + ret = ERR_PTR(status);
>> + goto abort;
>> + }
>> + data->hyst = status;
>> + data->last_updated[i] = jiffies;
>> + data->valid |= 1 << i;
>> }
>> - data->hyst = status;
>> - data->last_updated = jiffies;
>> - data->valid = true;
>> + else
>> + ret = ERR_PTR(-EINVAL);
>> }
>>
>> abort:
>> @@ -193,7 +206,7 @@ static ssize_t adt7410_show_temp(struct device *dev,
>> struct device_attribute *da, char *buf)
>> {
>> struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
>> - struct adt7410_data *data = adt7410_update_device(dev);
>> + struct adt7410_data *data = adt7410_update_device(dev, attr->index);
>>
>> if (IS_ERR(data))
>> return PTR_ERR(data);
>> @@ -236,7 +249,10 @@ static ssize_t adt7410_show_t_hyst(struct device *dev,
>> int nr = attr->index;
>> int hyst;
>>
>> - data = adt7410_update_device(dev);
>> + data = adt7410_update_device(dev, nr);
>> + if (IS_ERR(data))
>> + return PTR_ERR(data);
>> + data = adt7410_update_device(dev, 4);
>> if (IS_ERR(data))
>> return PTR_ERR(data);
>> hyst = (data->hyst & ADT7410_T_HYST_MASK) * 1000;
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers
2013-02-23 20:18 ` Guenter Roeck
@ 2013-02-25 22:03 ` Hartmut Knaack
2013-02-26 1:40 ` Guenter Roeck
0 siblings, 1 reply; 23+ messages in thread
From: Hartmut Knaack @ 2013-02-25 22:03 UTC (permalink / raw)
To: Guenter Roeck
Cc: Lars-Peter Clausen, Jean Delvare, Jonathan Cameron, lm-sensors,
linux-iio
Guenter Roeck schrieb:
> On Sat, Feb 23, 2013 at 01:45:32AM +0100, Hartmut Knaack wrote:
>> Guenter Roeck schrieb:
>>> On Tue, Feb 19, 2013 at 08:39:33PM +0100, Hartmut Knaack wrote:
>>>> Guenter Roeck schrieb:
>>>>> On Mon, Feb 18, 2013 at 09:22:18PM +0100, Hartmut Knaack wrote:
>>>>>> Lars-Peter Clausen schrieb:
>>>>>>> Currently each time the temperature register is read the driver also reads the
>>>>>>> threshold and hysteresis registers. This increases the amount of I2C traffic and
>>>>>>> time needed to read the temperature by a factor of ~5. Neither the threshold nor
>>>>>>> the hysteresis change on their own, so once we've read them, we should be able
>>>>>>> to just use the cached value of the registers. This patch modifies the code
>>>>>>> accordingly and only reads the threshold and hysteresis registers once during
>>>>>>> probe.
>>>>>> I have been thinking about this a lot, and I am concerned about data integrity. From what I know about I2C, there is no data integrity verification specified in the protocol. So, what the master sends is not necessarily what the slave receives (not to mention other devices on the bus, which could potentially mess around with the slaves, or even reset of the slave). Reading back just cached values makes it pretty hard to verify, if there are issues. I think it might be better to call a read-temperature function with a parameter that indicates, which temperature register is required.
>>>>> I am not concerned about that, unless there is a known issue with the chip
>>>>> and it is known to return bad data under some circumstances. I am doing the
>>>>> same in the PMBus drivers, since there are simply too many limit registers
>>>>> to read on some of the chips (there may literally be more than a hundred).
>>>>> That works fine most of the time; if it does not work, it is a chip problem,
>>>>> an i2c bus master problem, a hardware signal problem, or a combination of all.
>>>>> I actually think it is better if the problem is exposed by cached bad readings.
>>>> Could you please outline the last sentence? I'm having trouble to understand your intention with cached bad readings.
>>> Someone will actually notice it (hopefully while testing) and provide feedback.
>>> This gives a chance to fix the problem instead of having it linger around ...
>>> which would likely be the case if the problem is not persistent.
>>>
>>> Guenter
>> Well, think about a use case where you optically decouple your master and slave using PCA9600 (may it be using optical fibers to cover a big distance, or just to operate the slave in a hazardous environment), where the slave is powered from a different source. Now, if the slaves Vcc drops for a certain time, all its registers get reset (especially min, max and crit) - without the master noticing anything.
>> Therefor I would prefer something like the following solution, where unnecessary load on the bus gets avoided and the cached values get used, until they expire and get read again from the sensor. This is mainly a draft, but do you see any reason against it?
> Yes, it adds a lot of complexity. Also, at least in theory it would apply to all
> i2c drivers, so solving the problem for one chip doesn't help much.
>
> A somewhat cleanly designed hardware should inform the software that a device went
> offline, for example with an interrupt; otherwise its operation could be severely
> affected and system behavior would be unpredictable.
>
> So the question here is if this is a practical problem in your use case, or a
> theoretical problem. If it is a theoretical problem, we can address it for
> affected drivers as it is seen. If it is a practical problem, I think we'll need
> to come up with a simpler solution. The only one I can think of right now would
> be a module parameter. If set, it the driver could re-load the cache each time
> the temperature sensor is updated.
>
> Thanks,
> Guenter
>
My intended use for this sensor type is just temperature logging, so it is just a theoretical problem for me. But I don't trust I2C any more than really necessary (and always keeping Murphy's law in mind) and feel somehow responsible for this driver.
I can agree on having a 'paranoid' module parameter to force register reads, just for those folks who need it.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers
2013-02-25 21:30 ` Hartmut Knaack
@ 2013-02-26 1:39 ` Guenter Roeck
0 siblings, 0 replies; 23+ messages in thread
From: Guenter Roeck @ 2013-02-26 1:39 UTC (permalink / raw)
To: Hartmut Knaack
Cc: Lars-Peter Clausen, Jean Delvare, Jonathan Cameron, lm-sensors,
linux-iio
On Mon, Feb 25, 2013 at 10:30:30PM +0100, Hartmut Knaack wrote:
> Lars-Peter Clausen schrieb:
> > On 02/23/2013 01:45 AM, Hartmut Knaack wrote:
> >> Guenter Roeck schrieb:
> >>> On Tue, Feb 19, 2013 at 08:39:33PM +0100, Hartmut Knaack wrote:
> >>>> Guenter Roeck schrieb:
> >>>>> On Mon, Feb 18, 2013 at 09:22:18PM +0100, Hartmut Knaack wrote:
> >>>>>> Lars-Peter Clausen schrieb:
> >>>>>>> Currently each time the temperature register is read the driver also reads the
> >>>>>>> threshold and hysteresis registers. This increases the amount of I2C traffic and
> >>>>>>> time needed to read the temperature by a factor of ~5. Neither the threshold nor
> >>>>>>> the hysteresis change on their own, so once we've read them, we should be able
> >>>>>>> to just use the cached value of the registers. This patch modifies the code
> >>>>>>> accordingly and only reads the threshold and hysteresis registers once during
> >>>>>>> probe.
> >>>>>> I have been thinking about this a lot, and I am concerned about data integrity. From what I know about I2C, there is no data integrity verification specified in the protocol. So, what the master sends is not necessarily what the slave receives (not to mention other devices on the bus, which could potentially mess around with the slaves, or even reset of the slave). Reading back just cached values makes it pretty hard to verify, if there are issues. I think it might be better to call a read-temperature function with a parameter that indicates, which temperature register is required.
> >>>>> I am not concerned about that, unless there is a known issue with the chip
> >>>>> and it is known to return bad data under some circumstances. I am doing the
> >>>>> same in the PMBus drivers, since there are simply too many limit registers
> >>>>> to read on some of the chips (there may literally be more than a hundred).
> >>>>> That works fine most of the time; if it does not work, it is a chip problem,
> >>>>> an i2c bus master problem, a hardware signal problem, or a combination of all.
> >>>>> I actually think it is better if the problem is exposed by cached bad readings.
> >>>> Could you please outline the last sentence? I'm having trouble to understand your intention with cached bad readings.
> >>> Someone will actually notice it (hopefully while testing) and provide feedback.
> >>> This gives a chance to fix the problem instead of having it linger around ...
> >>> which would likely be the case if the problem is not persistent.
> >>>
> >>> Guenter
> >> Well, think about a use case where you optically decouple your master and slave using PCA9600 (may it be using optical fibers to cover a big distance, or just to operate the slave in a hazardous environment), where the slave is powered from a different source. Now, if the slaves Vcc drops for a certain time, all its registers get reset (especially min, max and crit) - without the master noticing anything.
> >> Therefor I would prefer something like the following solution, where unnecessary load on the bus gets avoided and the cached values get used, until they expire and get read again from the sensor. This is mainly a draft, but do you see any reason against it?
> >> Thanks
> >>
> > That's a bit of an artificially constructed situation, isn't it? Anyway,
> > wouldn't it be better to not cache at all in that case. For the cache to be
> > useful userspace would have to poll the file with period of less than 1.5
> > seconds. I don't think anybody is going to do this for the threshold properties.
> >
> > - Lars
> I could agree with not caching those registers - nobody would probably want to read them too often, intentionally (although bugged userspace code might DoS the bus). The other few i2c drivers I know also do uncached reads.
The "sensors" program always reads all attributes.
Guenter
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers
2013-02-25 22:03 ` Hartmut Knaack
@ 2013-02-26 1:40 ` Guenter Roeck
0 siblings, 0 replies; 23+ messages in thread
From: Guenter Roeck @ 2013-02-26 1:40 UTC (permalink / raw)
To: Hartmut Knaack
Cc: Lars-Peter Clausen, Jean Delvare, Jonathan Cameron, lm-sensors,
linux-iio
On Mon, Feb 25, 2013 at 11:03:50PM +0100, Hartmut Knaack wrote:
> Guenter Roeck schrieb:
> > On Sat, Feb 23, 2013 at 01:45:32AM +0100, Hartmut Knaack wrote:
> >> Guenter Roeck schrieb:
> >>> On Tue, Feb 19, 2013 at 08:39:33PM +0100, Hartmut Knaack wrote:
> >>>> Guenter Roeck schrieb:
> >>>>> On Mon, Feb 18, 2013 at 09:22:18PM +0100, Hartmut Knaack wrote:
> >>>>>> Lars-Peter Clausen schrieb:
> >>>>>>> Currently each time the temperature register is read the driver also reads the
> >>>>>>> threshold and hysteresis registers. This increases the amount of I2C traffic and
> >>>>>>> time needed to read the temperature by a factor of ~5. Neither the threshold nor
> >>>>>>> the hysteresis change on their own, so once we've read them, we should be able
> >>>>>>> to just use the cached value of the registers. This patch modifies the code
> >>>>>>> accordingly and only reads the threshold and hysteresis registers once during
> >>>>>>> probe.
> >>>>>> I have been thinking about this a lot, and I am concerned about data integrity. From what I know about I2C, there is no data integrity verification specified in the protocol. So, what the master sends is not necessarily what the slave receives (not to mention other devices on the bus, which could potentially mess around with the slaves, or even reset of the slave). Reading back just cached values makes it pretty hard to verify, if there are issues. I think it might be better to call a read-temperature function with a parameter that indicates, which temperature register is required.
> >>>>> I am not concerned about that, unless there is a known issue with the chip
> >>>>> and it is known to return bad data under some circumstances. I am doing the
> >>>>> same in the PMBus drivers, since there are simply too many limit registers
> >>>>> to read on some of the chips (there may literally be more than a hundred).
> >>>>> That works fine most of the time; if it does not work, it is a chip problem,
> >>>>> an i2c bus master problem, a hardware signal problem, or a combination of all.
> >>>>> I actually think it is better if the problem is exposed by cached bad readings.
> >>>> Could you please outline the last sentence? I'm having trouble to understand your intention with cached bad readings.
> >>> Someone will actually notice it (hopefully while testing) and provide feedback.
> >>> This gives a chance to fix the problem instead of having it linger around ...
> >>> which would likely be the case if the problem is not persistent.
> >>>
> >>> Guenter
> >> Well, think about a use case where you optically decouple your master and slave using PCA9600 (may it be using optical fibers to cover a big distance, or just to operate the slave in a hazardous environment), where the slave is powered from a different source. Now, if the slaves Vcc drops for a certain time, all its registers get reset (especially min, max and crit) - without the master noticing anything.
> >> Therefor I would prefer something like the following solution, where unnecessary load on the bus gets avoided and the cached values get used, until they expire and get read again from the sensor. This is mainly a draft, but do you see any reason against it?
> > Yes, it adds a lot of complexity. Also, at least in theory it would apply to all
> > i2c drivers, so solving the problem for one chip doesn't help much.
> >
> > A somewhat cleanly designed hardware should inform the software that a device went
> > offline, for example with an interrupt; otherwise its operation could be severely
> > affected and system behavior would be unpredictable.
> >
> > So the question here is if this is a practical problem in your use case, or a
> > theoretical problem. If it is a theoretical problem, we can address it for
> > affected drivers as it is seen. If it is a practical problem, I think we'll need
> > to come up with a simpler solution. The only one I can think of right now would
> > be a module parameter. If set, it the driver could re-load the cache each time
> > the temperature sensor is updated.
> >
> > Thanks,
> > Guenter
> >
> My intended use for this sensor type is just temperature logging, so it is just a theoretical problem for me. But I don't trust I2C any more than really necessary (and always keeping Murphy's law in mind) and feel somehow responsible for this driver.
> I can agree on having a 'paranoid' module parameter to force register reads, just for those folks who need it.
>
If it is a theoretic problem, let's address it once/if it ever gets a real one.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 4/4] staging:iio: Remove adt7410 driver
2013-02-18 13:38 ` [PATCH v2 4/4] staging:iio: Remove adt7410 driver Lars-Peter Clausen
@ 2013-03-02 16:45 ` Jonathan Cameron
2013-03-02 17:10 ` Guenter Roeck
0 siblings, 1 reply; 23+ messages in thread
From: Jonathan Cameron @ 2013-03-02 16:45 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Jean Delvare, Guenter Roeck, Hartmut Knaack, Jonathan Cameron,
lm-sensors, linux-iio
On 02/18/2013 01:38 PM, Lars-Peter Clausen wrote:
> The adt7410 hwmon driver is feature wise more or less on par with the IIO
> driver. So we can finally remove the IIO driver.
>
I kind of lost touch on where things were with the last few patches adding support in
the hwmon driver for stuff that was in here. It looked like there were only nitpicks
left so I'll assume it gets sorted this coming cycle and take this patch now
(before I loose it)
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
added to togreg branch of iio.git
> ---
> drivers/staging/iio/adc/Kconfig | 7 -
> drivers/staging/iio/adc/Makefile | 1 -
> drivers/staging/iio/adc/adt7410.c | 1102 -------------------------------------
> 3 files changed, 1110 deletions(-)
> delete mode 100644 drivers/staging/iio/adc/adt7410.c
>
> diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
> index fb8c239..029ae54 100644
> --- a/drivers/staging/iio/adc/Kconfig
> +++ b/drivers/staging/iio/adc/Kconfig
> @@ -90,13 +90,6 @@ config AD7192
> To compile this driver as a module, choose M here: the
> module will be called ad7192.
>
> -config ADT7410
> - tristate "Analog Devices ADT7310/ADT7410 temperature sensor driver"
> - depends on I2C || SPI_MASTER
> - help
> - Say yes here to build support for Analog Devices ADT7310/ADT7410
> - temperature sensors.
> -
> config AD7280
> tristate "Analog Devices AD7280A Lithium Ion Battery Monitoring System"
> depends on SPI
> diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
> index d285596..3e9fb14 100644
> --- a/drivers/staging/iio/adc/Makefile
> +++ b/drivers/staging/iio/adc/Makefile
> @@ -16,7 +16,6 @@ obj-$(CONFIG_AD7291) += ad7291.o
> obj-$(CONFIG_AD7780) += ad7780.o
> obj-$(CONFIG_AD7816) += ad7816.o
> obj-$(CONFIG_AD7192) += ad7192.o
> -obj-$(CONFIG_ADT7410) += adt7410.o
> obj-$(CONFIG_AD7280) += ad7280a.o
> obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o
> obj-$(CONFIG_MXS_LRADC) += mxs-lradc.o
> diff --git a/drivers/staging/iio/adc/adt7410.c b/drivers/staging/iio/adc/adt7410.c
> deleted file mode 100644
> index 35455e1..0000000
> --- a/drivers/staging/iio/adc/adt7410.c
> +++ /dev/null
> @@ -1,1102 +0,0 @@
> -/*
> - * ADT7410 digital temperature sensor driver supporting ADT7310/ADT7410
> - *
> - * Copyright 2010 Analog Devices Inc.
> - *
> - * Licensed under the GPL-2 or later.
> - */
> -
> -#include <linux/interrupt.h>
> -#include <linux/device.h>
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> -#include <linux/sysfs.h>
> -#include <linux/list.h>
> -#include <linux/i2c.h>
> -#include <linux/spi/spi.h>
> -#include <linux/module.h>
> -
> -#include <linux/iio/iio.h>
> -#include <linux/iio/sysfs.h>
> -#include <linux/iio/events.h>
> -
> -/*
> - * ADT7410 registers definition
> - */
> -
> -#define ADT7410_TEMPERATURE 0
> -#define ADT7410_STATUS 2
> -#define ADT7410_CONFIG 3
> -#define ADT7410_T_ALARM_HIGH 4
> -#define ADT7410_T_ALARM_LOW 6
> -#define ADT7410_T_CRIT 8
> -#define ADT7410_T_HYST 0xA
> -#define ADT7410_ID 0xB
> -#define ADT7410_RESET 0x2F
> -
> -/*
> - * ADT7310 registers definition
> - */
> -
> -#define ADT7310_STATUS 0
> -#define ADT7310_CONFIG 1
> -#define ADT7310_TEMPERATURE 2
> -#define ADT7310_ID 3
> -#define ADT7310_T_CRIT 4
> -#define ADT7310_T_HYST 5
> -#define ADT7310_T_ALARM_HIGH 6
> -#define ADT7310_T_ALARM_LOW 7
> -
> -/*
> - * ADT7410 status
> - */
> -#define ADT7410_STAT_T_LOW 0x10
> -#define ADT7410_STAT_T_HIGH 0x20
> -#define ADT7410_STAT_T_CRIT 0x40
> -#define ADT7410_STAT_NOT_RDY 0x80
> -
> -/*
> - * ADT7410 config
> - */
> -#define ADT7410_FAULT_QUEUE_MASK 0x3
> -#define ADT7410_CT_POLARITY 0x4
> -#define ADT7410_INT_POLARITY 0x8
> -#define ADT7410_EVENT_MODE 0x10
> -#define ADT7410_MODE_MASK 0x60
> -#define ADT7410_ONESHOT 0x20
> -#define ADT7410_SPS 0x40
> -#define ADT7410_PD 0x60
> -#define ADT7410_RESOLUTION 0x80
> -
> -/*
> - * ADT7410 masks
> - */
> -#define ADT7410_T16_VALUE_SIGN 0x8000
> -#define ADT7410_T16_VALUE_FLOAT_OFFSET 7
> -#define ADT7410_T16_VALUE_FLOAT_MASK 0x7F
> -#define ADT7410_T13_VALUE_SIGN 0x1000
> -#define ADT7410_T13_VALUE_OFFSET 3
> -#define ADT7410_T13_VALUE_FLOAT_OFFSET 4
> -#define ADT7410_T13_VALUE_FLOAT_MASK 0xF
> -#define ADT7410_T_HYST_MASK 0xF
> -#define ADT7410_DEVICE_ID_MASK 0xF
> -#define ADT7410_MANUFACTORY_ID_MASK 0xF0
> -#define ADT7410_MANUFACTORY_ID_OFFSET 4
> -
> -
> -#define ADT7310_CMD_REG_MASK 0x28
> -#define ADT7310_CMD_REG_OFFSET 3
> -#define ADT7310_CMD_READ 0x40
> -#define ADT7310_CMD_CON_READ 0x4
> -
> -#define ADT7410_IRQS 2
> -
> -/*
> - * struct adt7410_chip_info - chip specifc information
> - */
> -
> -struct adt7410_chip_info;
> -
> -struct adt7410_ops {
> - int (*read_word)(struct adt7410_chip_info *, u8 reg, u16 *data);
> - int (*write_word)(struct adt7410_chip_info *, u8 reg, u16 data);
> - int (*read_byte)(struct adt7410_chip_info *, u8 reg, u8 *data);
> - int (*write_byte)(struct adt7410_chip_info *, u8 reg, u8 data);
> -};
> -
> -struct adt7410_chip_info {
> - struct device *dev;
> - u8 config;
> -
> - const struct adt7410_ops *ops;
> -};
> -
> -static int adt7410_read_word(struct adt7410_chip_info *chip, u8 reg, u16 *data)
> -{
> - return chip->ops->read_word(chip, reg, data);
> -}
> -
> -static int adt7410_write_word(struct adt7410_chip_info *chip, u8 reg, u16 data)
> -{
> - return chip->ops->write_word(chip, reg, data);
> -}
> -
> -static int adt7410_read_byte(struct adt7410_chip_info *chip, u8 reg, u8 *data)
> -{
> - return chip->ops->read_byte(chip, reg, data);
> -}
> -
> -static int adt7410_write_byte(struct adt7410_chip_info *chip, u8 reg, u8 data)
> -{
> - return chip->ops->write_byte(chip, reg, data);
> -}
> -
> -static ssize_t adt7410_show_mode(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - struct iio_dev *dev_info = dev_to_iio_dev(dev);
> - struct adt7410_chip_info *chip = iio_priv(dev_info);
> - u8 config;
> -
> - config = chip->config & ADT7410_MODE_MASK;
> -
> - switch (config) {
> - case ADT7410_PD:
> - return sprintf(buf, "power-down\n");
> - case ADT7410_ONESHOT:
> - return sprintf(buf, "one-shot\n");
> - case ADT7410_SPS:
> - return sprintf(buf, "sps\n");
> - default:
> - return sprintf(buf, "full\n");
> - }
> -}
> -
> -static ssize_t adt7410_store_mode(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf,
> - size_t len)
> -{
> - struct iio_dev *dev_info = dev_to_iio_dev(dev);
> - struct adt7410_chip_info *chip = iio_priv(dev_info);
> - u16 config;
> - int ret;
> -
> - ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
> - if (ret)
> - return -EIO;
> -
> - config = chip->config & (~ADT7410_MODE_MASK);
> - if (strcmp(buf, "power-down"))
> - config |= ADT7410_PD;
> - else if (strcmp(buf, "one-shot"))
> - config |= ADT7410_ONESHOT;
> - else if (strcmp(buf, "sps"))
> - config |= ADT7410_SPS;
> -
> - ret = adt7410_write_byte(chip, ADT7410_CONFIG, config);
> - if (ret)
> - return -EIO;
> -
> - chip->config = config;
> -
> - return len;
> -}
> -
> -static IIO_DEVICE_ATTR(mode, S_IRUGO | S_IWUSR,
> - adt7410_show_mode,
> - adt7410_store_mode,
> - 0);
> -
> -static ssize_t adt7410_show_available_modes(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - return sprintf(buf, "full\none-shot\nsps\npower-down\n");
> -}
> -
> -static IIO_DEVICE_ATTR(available_modes, S_IRUGO, adt7410_show_available_modes, NULL, 0);
> -
> -static ssize_t adt7410_show_resolution(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - struct iio_dev *dev_info = dev_to_iio_dev(dev);
> - struct adt7410_chip_info *chip = iio_priv(dev_info);
> - int ret;
> - int bits;
> -
> - ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
> - if (ret)
> - return -EIO;
> -
> - if (chip->config & ADT7410_RESOLUTION)
> - bits = 16;
> - else
> - bits = 13;
> -
> - return sprintf(buf, "%d bits\n", bits);
> -}
> -
> -static ssize_t adt7410_store_resolution(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf,
> - size_t len)
> -{
> - struct iio_dev *dev_info = dev_to_iio_dev(dev);
> - struct adt7410_chip_info *chip = iio_priv(dev_info);
> - unsigned long data;
> - u16 config;
> - int ret;
> -
> - ret = strict_strtoul(buf, 10, &data);
> - if (ret)
> - return -EINVAL;
> -
> - ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
> - if (ret)
> - return -EIO;
> -
> - config = chip->config & (~ADT7410_RESOLUTION);
> - if (data)
> - config |= ADT7410_RESOLUTION;
> -
> - ret = adt7410_write_byte(chip, ADT7410_CONFIG, config);
> - if (ret)
> - return -EIO;
> -
> - chip->config = config;
> -
> - return len;
> -}
> -
> -static IIO_DEVICE_ATTR(resolution, S_IRUGO | S_IWUSR,
> - adt7410_show_resolution,
> - adt7410_store_resolution,
> - 0);
> -
> -static ssize_t adt7410_show_id(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - struct iio_dev *dev_info = dev_to_iio_dev(dev);
> - struct adt7410_chip_info *chip = iio_priv(dev_info);
> - u8 id;
> - int ret;
> -
> - ret = adt7410_read_byte(chip, ADT7410_ID, &id);
> - if (ret)
> - return -EIO;
> -
> - return sprintf(buf, "device id: 0x%x\nmanufactory id: 0x%x\n",
> - id & ADT7410_DEVICE_ID_MASK,
> - (id & ADT7410_MANUFACTORY_ID_MASK) >> ADT7410_MANUFACTORY_ID_OFFSET);
> -}
> -
> -static IIO_DEVICE_ATTR(id, S_IRUGO | S_IWUSR,
> - adt7410_show_id,
> - NULL,
> - 0);
> -
> -static ssize_t adt7410_convert_temperature(struct adt7410_chip_info *chip,
> - u16 data, char *buf)
> -{
> - char sign = ' ';
> -
> - if (!(chip->config & ADT7410_RESOLUTION))
> - data &= ~0x7;
> -
> - if (data & ADT7410_T16_VALUE_SIGN) {
> - /* convert supplement to positive value */
> - data = (u16)((ADT7410_T16_VALUE_SIGN << 1) - (u32)data);
> - sign = '-';
> - }
> - return sprintf(buf, "%c%d.%.7d\n", sign,
> - (data >> ADT7410_T16_VALUE_FLOAT_OFFSET),
> - (data & ADT7410_T16_VALUE_FLOAT_MASK) * 78125);
> -}
> -
> -static ssize_t adt7410_show_value(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - struct iio_dev *dev_info = dev_to_iio_dev(dev);
> - struct adt7410_chip_info *chip = iio_priv(dev_info);
> - u8 status;
> - u16 data;
> - int ret, i = 0;
> -
> - do {
> - ret = adt7410_read_byte(chip, ADT7410_STATUS, &status);
> - if (ret)
> - return -EIO;
> - i++;
> - if (i == 10000)
> - return -EIO;
> - } while (status & ADT7410_STAT_NOT_RDY);
> -
> - ret = adt7410_read_word(chip, ADT7410_TEMPERATURE, &data);
> - if (ret)
> - return -EIO;
> -
> - return adt7410_convert_temperature(chip, data, buf);
> -}
> -
> -static IIO_DEVICE_ATTR(value, S_IRUGO, adt7410_show_value, NULL, 0);
> -
> -static struct attribute *adt7410_attributes[] = {
> - &iio_dev_attr_available_modes.dev_attr.attr,
> - &iio_dev_attr_mode.dev_attr.attr,
> - &iio_dev_attr_resolution.dev_attr.attr,
> - &iio_dev_attr_id.dev_attr.attr,
> - &iio_dev_attr_value.dev_attr.attr,
> - NULL,
> -};
> -
> -static const struct attribute_group adt7410_attribute_group = {
> - .attrs = adt7410_attributes,
> -};
> -
> -static irqreturn_t adt7410_event_handler(int irq, void *private)
> -{
> - struct iio_dev *indio_dev = private;
> - struct adt7410_chip_info *chip = iio_priv(indio_dev);
> - s64 timestamp = iio_get_time_ns();
> - u8 status;
> -
> - if (adt7410_read_byte(chip, ADT7410_STATUS, &status))
> - return IRQ_HANDLED;
> -
> - if (status & ADT7410_STAT_T_HIGH)
> - iio_push_event(indio_dev,
> - IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
> - IIO_EV_TYPE_THRESH,
> - IIO_EV_DIR_RISING),
> - timestamp);
> - if (status & ADT7410_STAT_T_LOW)
> - iio_push_event(indio_dev,
> - IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
> - IIO_EV_TYPE_THRESH,
> - IIO_EV_DIR_FALLING),
> - timestamp);
> - if (status & ADT7410_STAT_T_CRIT)
> - iio_push_event(indio_dev,
> - IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
> - IIO_EV_TYPE_THRESH,
> - IIO_EV_DIR_RISING),
> - timestamp);
> -
> - return IRQ_HANDLED;
> -}
> -
> -static ssize_t adt7410_show_event_mode(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - struct iio_dev *dev_info = dev_to_iio_dev(dev);
> - struct adt7410_chip_info *chip = iio_priv(dev_info);
> - int ret;
> -
> - ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
> - if (ret)
> - return -EIO;
> -
> - if (chip->config & ADT7410_EVENT_MODE)
> - return sprintf(buf, "interrupt\n");
> - else
> - return sprintf(buf, "comparator\n");
> -}
> -
> -static ssize_t adt7410_set_event_mode(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf,
> - size_t len)
> -{
> - struct iio_dev *dev_info = dev_to_iio_dev(dev);
> - struct adt7410_chip_info *chip = iio_priv(dev_info);
> - u16 config;
> - int ret;
> -
> - ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
> - if (ret)
> - return -EIO;
> -
> - config = chip->config &= ~ADT7410_EVENT_MODE;
> - if (strcmp(buf, "comparator") != 0)
> - config |= ADT7410_EVENT_MODE;
> -
> - ret = adt7410_write_byte(chip, ADT7410_CONFIG, config);
> - if (ret)
> - return -EIO;
> -
> - chip->config = config;
> -
> - return ret;
> -}
> -
> -static ssize_t adt7410_show_available_event_modes(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - return sprintf(buf, "comparator\ninterrupt\n");
> -}
> -
> -static ssize_t adt7410_show_fault_queue(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - struct iio_dev *dev_info = dev_to_iio_dev(dev);
> - struct adt7410_chip_info *chip = iio_priv(dev_info);
> - int ret;
> -
> - ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
> - if (ret)
> - return -EIO;
> -
> - return sprintf(buf, "%d\n", chip->config & ADT7410_FAULT_QUEUE_MASK);
> -}
> -
> -static ssize_t adt7410_set_fault_queue(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf,
> - size_t len)
> -{
> - struct iio_dev *dev_info = dev_to_iio_dev(dev);
> - struct adt7410_chip_info *chip = iio_priv(dev_info);
> - unsigned long data;
> - int ret;
> - u8 config;
> -
> - ret = strict_strtoul(buf, 10, &data);
> - if (ret || data > 3)
> - return -EINVAL;
> -
> - ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
> - if (ret)
> - return -EIO;
> -
> - config = chip->config & ~ADT7410_FAULT_QUEUE_MASK;
> - config |= data;
> - ret = adt7410_write_byte(chip, ADT7410_CONFIG, config);
> - if (ret)
> - return -EIO;
> -
> - chip->config = config;
> -
> - return ret;
> -}
> -
> -static inline ssize_t adt7410_show_t_bound(struct device *dev,
> - struct device_attribute *attr,
> - u8 bound_reg,
> - char *buf)
> -{
> - struct iio_dev *dev_info = dev_to_iio_dev(dev);
> - struct adt7410_chip_info *chip = iio_priv(dev_info);
> - u16 data;
> - int ret;
> -
> - ret = adt7410_read_word(chip, bound_reg, &data);
> - if (ret)
> - return -EIO;
> -
> - return adt7410_convert_temperature(chip, data, buf);
> -}
> -
> -static inline ssize_t adt7410_set_t_bound(struct device *dev,
> - struct device_attribute *attr,
> - u8 bound_reg,
> - const char *buf,
> - size_t len)
> -{
> - struct iio_dev *dev_info = dev_to_iio_dev(dev);
> - struct adt7410_chip_info *chip = iio_priv(dev_info);
> - long tmp1, tmp2;
> - u16 data;
> - char *pos;
> - int ret;
> -
> - pos = strchr(buf, '.');
> -
> - ret = strict_strtol(buf, 10, &tmp1);
> -
> - if (ret || tmp1 > 127 || tmp1 < -128)
> - return -EINVAL;
> -
> - if (pos) {
> - len = strlen(pos);
> -
> - if (chip->config & ADT7410_RESOLUTION) {
> - if (len > ADT7410_T16_VALUE_FLOAT_OFFSET)
> - len = ADT7410_T16_VALUE_FLOAT_OFFSET;
> - pos[len] = 0;
> - ret = strict_strtol(pos, 10, &tmp2);
> -
> - if (!ret)
> - tmp2 = (tmp2 / 78125) * 78125;
> - } else {
> - if (len > ADT7410_T13_VALUE_FLOAT_OFFSET)
> - len = ADT7410_T13_VALUE_FLOAT_OFFSET;
> - pos[len] = 0;
> - ret = strict_strtol(pos, 10, &tmp2);
> -
> - if (!ret)
> - tmp2 = (tmp2 / 625) * 625;
> - }
> - }
> -
> - if (tmp1 < 0)
> - data = (u16)(-tmp1);
> - else
> - data = (u16)tmp1;
> -
> - if (chip->config & ADT7410_RESOLUTION) {
> - data = (data << ADT7410_T16_VALUE_FLOAT_OFFSET) |
> - (tmp2 & ADT7410_T16_VALUE_FLOAT_MASK);
> -
> - if (tmp1 < 0)
> - /* convert positive value to supplyment */
> - data = (u16)((ADT7410_T16_VALUE_SIGN << 1) - (u32)data);
> - } else {
> - data = (data << ADT7410_T13_VALUE_FLOAT_OFFSET) |
> - (tmp2 & ADT7410_T13_VALUE_FLOAT_MASK);
> -
> - if (tmp1 < 0)
> - /* convert positive value to supplyment */
> - data = (ADT7410_T13_VALUE_SIGN << 1) - data;
> - data <<= ADT7410_T13_VALUE_OFFSET;
> - }
> -
> - ret = adt7410_write_word(chip, bound_reg, data);
> - if (ret)
> - return -EIO;
> -
> - return ret;
> -}
> -
> -static ssize_t adt7410_show_t_alarm_high(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - return adt7410_show_t_bound(dev, attr,
> - ADT7410_T_ALARM_HIGH, buf);
> -}
> -
> -static inline ssize_t adt7410_set_t_alarm_high(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf,
> - size_t len)
> -{
> - return adt7410_set_t_bound(dev, attr,
> - ADT7410_T_ALARM_HIGH, buf, len);
> -}
> -
> -static ssize_t adt7410_show_t_alarm_low(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - return adt7410_show_t_bound(dev, attr,
> - ADT7410_T_ALARM_LOW, buf);
> -}
> -
> -static inline ssize_t adt7410_set_t_alarm_low(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf,
> - size_t len)
> -{
> - return adt7410_set_t_bound(dev, attr,
> - ADT7410_T_ALARM_LOW, buf, len);
> -}
> -
> -static ssize_t adt7410_show_t_crit(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - return adt7410_show_t_bound(dev, attr,
> - ADT7410_T_CRIT, buf);
> -}
> -
> -static inline ssize_t adt7410_set_t_crit(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf,
> - size_t len)
> -{
> - return adt7410_set_t_bound(dev, attr,
> - ADT7410_T_CRIT, buf, len);
> -}
> -
> -static ssize_t adt7410_show_t_hyst(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - struct iio_dev *dev_info = dev_to_iio_dev(dev);
> - struct adt7410_chip_info *chip = iio_priv(dev_info);
> - int ret;
> - u8 t_hyst;
> -
> - ret = adt7410_read_byte(chip, ADT7410_T_HYST, &t_hyst);
> - if (ret)
> - return -EIO;
> -
> - return sprintf(buf, "%d\n", t_hyst & ADT7410_T_HYST_MASK);
> -}
> -
> -static inline ssize_t adt7410_set_t_hyst(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf,
> - size_t len)
> -{
> - struct iio_dev *dev_info = dev_to_iio_dev(dev);
> - struct adt7410_chip_info *chip = iio_priv(dev_info);
> - int ret;
> - unsigned long data;
> - u8 t_hyst;
> -
> - ret = strict_strtol(buf, 10, &data);
> -
> - if (ret || data > ADT7410_T_HYST_MASK)
> - return -EINVAL;
> -
> - t_hyst = (u8)data;
> -
> - ret = adt7410_write_byte(chip, ADT7410_T_HYST, t_hyst);
> - if (ret)
> - return -EIO;
> -
> - return ret;
> -}
> -
> -static IIO_DEVICE_ATTR(event_mode,
> - S_IRUGO | S_IWUSR,
> - adt7410_show_event_mode, adt7410_set_event_mode, 0);
> -static IIO_DEVICE_ATTR(available_event_modes,
> - S_IRUGO,
> - adt7410_show_available_event_modes, NULL, 0);
> -static IIO_DEVICE_ATTR(fault_queue,
> - S_IRUGO | S_IWUSR,
> - adt7410_show_fault_queue, adt7410_set_fault_queue, 0);
> -static IIO_DEVICE_ATTR(t_alarm_high,
> - S_IRUGO | S_IWUSR,
> - adt7410_show_t_alarm_high, adt7410_set_t_alarm_high, 0);
> -static IIO_DEVICE_ATTR(t_alarm_low,
> - S_IRUGO | S_IWUSR,
> - adt7410_show_t_alarm_low, adt7410_set_t_alarm_low, 0);
> -static IIO_DEVICE_ATTR(t_crit,
> - S_IRUGO | S_IWUSR,
> - adt7410_show_t_crit, adt7410_set_t_crit, 0);
> -static IIO_DEVICE_ATTR(t_hyst,
> - S_IRUGO | S_IWUSR,
> - adt7410_show_t_hyst, adt7410_set_t_hyst, 0);
> -
> -static struct attribute *adt7410_event_int_attributes[] = {
> - &iio_dev_attr_event_mode.dev_attr.attr,
> - &iio_dev_attr_available_event_modes.dev_attr.attr,
> - &iio_dev_attr_fault_queue.dev_attr.attr,
> - &iio_dev_attr_t_alarm_high.dev_attr.attr,
> - &iio_dev_attr_t_alarm_low.dev_attr.attr,
> - &iio_dev_attr_t_crit.dev_attr.attr,
> - &iio_dev_attr_t_hyst.dev_attr.attr,
> - NULL,
> -};
> -
> -static struct attribute_group adt7410_event_attribute_group = {
> - .attrs = adt7410_event_int_attributes,
> - .name = "events",
> -};
> -
> -static const struct iio_info adt7410_info = {
> - .attrs = &adt7410_attribute_group,
> - .event_attrs = &adt7410_event_attribute_group,
> - .driver_module = THIS_MODULE,
> -};
> -
> -/*
> - * device probe and remove
> - */
> -
> -static int adt7410_probe(struct device *dev, int irq,
> - const char *name, const struct adt7410_ops *ops)
> -{
> - unsigned long *adt7410_platform_data = dev->platform_data;
> - unsigned long local_pdata[] = {0, 0};
> - struct adt7410_chip_info *chip;
> - struct iio_dev *indio_dev;
> - int ret = 0;
> -
> - indio_dev = iio_device_alloc(sizeof(*chip));
> - if (indio_dev == NULL) {
> - ret = -ENOMEM;
> - goto error_ret;
> - }
> - chip = iio_priv(indio_dev);
> - /* this is only used for device removal purposes */
> - dev_set_drvdata(dev, indio_dev);
> -
> - chip->dev = dev;
> - chip->ops = ops;
> -
> - indio_dev->name = name;
> - indio_dev->dev.parent = dev;
> - indio_dev->info = &adt7410_info;
> - indio_dev->modes = INDIO_DIRECT_MODE;
> -
> - if (!adt7410_platform_data)
> - adt7410_platform_data = local_pdata;
> -
> - /* CT critcal temperature event. line 0 */
> - if (irq) {
> - ret = request_threaded_irq(irq,
> - NULL,
> - &adt7410_event_handler,
> - IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> - name,
> - indio_dev);
> - if (ret)
> - goto error_free_dev;
> - }
> -
> - /* INT bound temperature alarm event. line 1 */
> - if (adt7410_platform_data[0]) {
> - ret = request_threaded_irq(adt7410_platform_data[0],
> - NULL,
> - &adt7410_event_handler,
> - adt7410_platform_data[1] |
> - IRQF_ONESHOT,
> - name,
> - indio_dev);
> - if (ret)
> - goto error_unreg_ct_irq;
> - }
> -
> - ret = adt7410_read_byte(chip, ADT7410_CONFIG, &chip->config);
> - if (ret) {
> - ret = -EIO;
> - goto error_unreg_int_irq;
> - }
> -
> - chip->config |= ADT7410_RESOLUTION;
> -
> - if (irq && adt7410_platform_data[0]) {
> -
> - /* set irq polarity low level */
> - chip->config &= ~ADT7410_CT_POLARITY;
> -
> - if (adt7410_platform_data[1] & IRQF_TRIGGER_HIGH)
> - chip->config |= ADT7410_INT_POLARITY;
> - else
> - chip->config &= ~ADT7410_INT_POLARITY;
> - }
> -
> - ret = adt7410_write_byte(chip, ADT7410_CONFIG, chip->config);
> - if (ret) {
> - ret = -EIO;
> - goto error_unreg_int_irq;
> - }
> - ret = iio_device_register(indio_dev);
> - if (ret)
> - goto error_unreg_int_irq;
> -
> - dev_info(dev, "%s temperature sensor registered.\n",
> - name);
> -
> - return 0;
> -
> -error_unreg_int_irq:
> - free_irq(adt7410_platform_data[0], indio_dev);
> -error_unreg_ct_irq:
> - free_irq(irq, indio_dev);
> -error_free_dev:
> - iio_device_free(indio_dev);
> -error_ret:
> - return ret;
> -}
> -
> -static int adt7410_remove(struct device *dev, int irq)
> -{
> - struct iio_dev *indio_dev = dev_get_drvdata(dev);
> - unsigned long *adt7410_platform_data = dev->platform_data;
> -
> - iio_device_unregister(indio_dev);
> - if (adt7410_platform_data[0])
> - free_irq(adt7410_platform_data[0], indio_dev);
> - if (irq)
> - free_irq(irq, indio_dev);
> - iio_device_free(indio_dev);
> -
> - return 0;
> -}
> -
> -#if IS_ENABLED(CONFIG_I2C)
> -
> -static int adt7410_i2c_read_word(struct adt7410_chip_info *chip, u8 reg,
> - u16 *data)
> -{
> - struct i2c_client *client = to_i2c_client(chip->dev);
> - int ret = 0;
> -
> - ret = i2c_smbus_read_word_data(client, reg);
> - if (ret < 0) {
> - dev_err(&client->dev, "I2C read error\n");
> - return ret;
> - }
> -
> - *data = swab16((u16)ret);
> -
> - return 0;
> -}
> -
> -static int adt7410_i2c_write_word(struct adt7410_chip_info *chip, u8 reg,
> - u16 data)
> -{
> - struct i2c_client *client = to_i2c_client(chip->dev);
> - int ret = 0;
> -
> - ret = i2c_smbus_write_word_data(client, reg, swab16(data));
> - if (ret < 0)
> - dev_err(&client->dev, "I2C write error\n");
> -
> - return ret;
> -}
> -
> -static int adt7410_i2c_read_byte(struct adt7410_chip_info *chip, u8 reg,
> - u8 *data)
> -{
> - struct i2c_client *client = to_i2c_client(chip->dev);
> - int ret = 0;
> -
> - ret = i2c_smbus_read_byte_data(client, reg);
> - if (ret < 0) {
> - dev_err(&client->dev, "I2C read error\n");
> - return ret;
> - }
> -
> - *data = (u8)ret;
> -
> - return 0;
> -}
> -
> -static int adt7410_i2c_write_byte(struct adt7410_chip_info *chip, u8 reg,
> - u8 data)
> -{
> - struct i2c_client *client = to_i2c_client(chip->dev);
> - int ret = 0;
> -
> - ret = i2c_smbus_write_byte_data(client, reg, data);
> - if (ret < 0)
> - dev_err(&client->dev, "I2C write error\n");
> -
> - return ret;
> -}
> -
> -static const struct adt7410_ops adt7410_i2c_ops = {
> - .read_word = adt7410_i2c_read_word,
> - .write_word = adt7410_i2c_write_word,
> - .read_byte = adt7410_i2c_read_byte,
> - .write_byte = adt7410_i2c_write_byte,
> -};
> -
> -static int adt7410_i2c_probe(struct i2c_client *client,
> - const struct i2c_device_id *id)
> -{
> - return adt7410_probe(&client->dev, client->irq, id->name,
> - &adt7410_i2c_ops);
> -}
> -
> -static int adt7410_i2c_remove(struct i2c_client *client)
> -{
> - return adt7410_remove(&client->dev, client->irq);
> -}
> -
> -static const struct i2c_device_id adt7410_id[] = {
> - { "adt7410", 0 },
> - {}
> -};
> -
> -MODULE_DEVICE_TABLE(i2c, adt7410_id);
> -
> -static struct i2c_driver adt7410_driver = {
> - .driver = {
> - .name = "adt7410",
> - },
> - .probe = adt7410_i2c_probe,
> - .remove = adt7410_i2c_remove,
> - .id_table = adt7410_id,
> -};
> -
> -static int __init adt7410_i2c_init(void)
> -{
> - return i2c_add_driver(&adt7410_driver);
> -}
> -
> -static void __exit adt7410_i2c_exit(void)
> -{
> - i2c_del_driver(&adt7410_driver);
> -}
> -
> -#else
> -
> -static int __init adt7410_i2c_init(void) { return 0; };
> -static void __exit adt7410_i2c_exit(void) {};
> -
> -#endif
> -
> -#if IS_ENABLED(CONFIG_SPI_MASTER)
> -
> -static const u8 adt7371_reg_table[] = {
> - [ADT7410_TEMPERATURE] = ADT7310_TEMPERATURE,
> - [ADT7410_STATUS] = ADT7310_STATUS,
> - [ADT7410_CONFIG] = ADT7310_CONFIG,
> - [ADT7410_T_ALARM_HIGH] = ADT7310_T_ALARM_HIGH,
> - [ADT7410_T_ALARM_LOW] = ADT7310_T_ALARM_LOW,
> - [ADT7410_T_CRIT] = ADT7310_T_CRIT,
> - [ADT7410_T_HYST] = ADT7310_T_HYST,
> - [ADT7410_ID] = ADT7310_ID,
> -};
> -
> -#define AD7310_COMMAND(reg) (adt7371_reg_table[(reg)] << ADT7310_CMD_REG_OFFSET)
> -
> -static int adt7310_spi_read_word(struct adt7410_chip_info *chip,
> - u8 reg, u16 *data)
> -{
> - struct spi_device *spi = to_spi_device(chip->dev);
> - u8 command = AD7310_COMMAND(reg);
> - int ret = 0;
> -
> - command |= ADT7310_CMD_READ;
> - ret = spi_write(spi, &command, sizeof(command));
> - if (ret < 0) {
> - dev_err(&spi->dev, "SPI write command error\n");
> - return ret;
> - }
> -
> - ret = spi_read(spi, (u8 *)data, sizeof(*data));
> - if (ret < 0) {
> - dev_err(&spi->dev, "SPI read word error\n");
> - return ret;
> - }
> -
> - *data = be16_to_cpu(*data);
> -
> - return 0;
> -}
> -
> -static int adt7310_spi_write_word(struct adt7410_chip_info *chip, u8 reg,
> - u16 data)
> -{
> - struct spi_device *spi = to_spi_device(chip->dev);
> - u8 buf[3];
> - int ret = 0;
> -
> - buf[0] = AD7310_COMMAND(reg);
> - buf[1] = (u8)(data >> 8);
> - buf[2] = (u8)(data & 0xFF);
> -
> - ret = spi_write(spi, buf, 3);
> - if (ret < 0) {
> - dev_err(&spi->dev, "SPI write word error\n");
> - return ret;
> - }
> -
> - return ret;
> -}
> -
> -static int adt7310_spi_read_byte(struct adt7410_chip_info *chip, u8 reg,
> - u8 *data)
> -{
> - struct spi_device *spi = to_spi_device(chip->dev);
> - u8 command = AD7310_COMMAND(reg);
> - int ret = 0;
> -
> - command |= ADT7310_CMD_READ;
> - ret = spi_write(spi, &command, sizeof(command));
> - if (ret < 0) {
> - dev_err(&spi->dev, "SPI write command error\n");
> - return ret;
> - }
> -
> - ret = spi_read(spi, data, sizeof(*data));
> - if (ret < 0) {
> - dev_err(&spi->dev, "SPI read byte error\n");
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> -static int adt7310_spi_write_byte(struct adt7410_chip_info *chip, u8 reg,
> - u8 data)
> -{
> - struct spi_device *spi = to_spi_device(chip->dev);
> - u8 buf[2];
> - int ret = 0;
> -
> - buf[0] = AD7310_COMMAND(reg);
> - buf[1] = data;
> -
> - ret = spi_write(spi, buf, 2);
> - if (ret < 0) {
> - dev_err(&spi->dev, "SPI write byte error\n");
> - return ret;
> - }
> -
> - return ret;
> -}
> -
> -static const struct adt7410_ops adt7310_spi_ops = {
> - .read_word = adt7310_spi_read_word,
> - .write_word = adt7310_spi_write_word,
> - .read_byte = adt7310_spi_read_byte,
> - .write_byte = adt7310_spi_write_byte,
> -};
> -
> -static int adt7310_spi_probe(struct spi_device *spi)
> -{
> - return adt7410_probe(&spi->dev, spi->irq,
> - spi_get_device_id(spi)->name, &adt7310_spi_ops);
> -}
> -
> -static int adt7310_spi_remove(struct spi_device *spi)
> -{
> - return adt7410_remove(&spi->dev, spi->irq);
> -}
> -
> -static const struct spi_device_id adt7310_id[] = {
> - { "adt7310", 0 },
> - {}
> -};
> -MODULE_DEVICE_TABLE(spi, adt7310_id);
> -
> -static struct spi_driver adt7310_driver = {
> - .driver = {
> - .name = "adt7310",
> - .owner = THIS_MODULE,
> - },
> - .probe = adt7310_spi_probe,
> - .remove = adt7310_spi_remove,
> - .id_table = adt7310_id,
> -};
> -
> -static int __init adt7310_spi_init(void)
> -{
> - return spi_register_driver(&adt7310_driver);
> -}
> -
> -static void adt7310_spi_exit(void)
> -{
> - spi_unregister_driver(&adt7310_driver);
> -}
> -
> -#else
> -
> -static int __init adt7310_spi_init(void) { return 0; };
> -static void adt7310_spi_exit(void) {};
> -
> -#endif
> -
> -static int __init adt7410_init(void)
> -{
> - int ret;
> -
> - ret = adt7310_spi_init();
> - if (ret)
> - return ret;
> -
> - ret = adt7410_i2c_init();
> - if (ret)
> - adt7310_spi_exit();
> -
> - return ret;
> -}
> -module_init(adt7410_init);
> -
> -static void __exit adt7410_exit(void)
> -{
> - adt7410_i2c_exit();
> - adt7310_spi_exit();
> -}
> -module_exit(adt7410_exit);
> -
> -MODULE_AUTHOR("Sonic Zhang <sonic.zhang@analog.com>");
> -MODULE_DESCRIPTION("Analog Devices ADT7310/ADT7410 digital temperature sensor driver");
> -MODULE_LICENSE("GPL v2");
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 4/4] staging:iio: Remove adt7410 driver
2013-03-02 16:45 ` Jonathan Cameron
@ 2013-03-02 17:10 ` Guenter Roeck
0 siblings, 0 replies; 23+ messages in thread
From: Guenter Roeck @ 2013-03-02 17:10 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Jean Delvare, Hartmut Knaack,
Jonathan Cameron, lm-sensors, linux-iio
On Sat, Mar 02, 2013 at 04:45:14PM +0000, Jonathan Cameron wrote:
> On 02/18/2013 01:38 PM, Lars-Peter Clausen wrote:
> > The adt7410 hwmon driver is feature wise more or less on par with the IIO
> > driver. So we can finally remove the IIO driver.
> >
> I kind of lost touch on where things were with the last few patches adding support in
> the hwmon driver for stuff that was in here. It looked like there were only nitpicks
> left so I'll assume it gets sorted this coming cycle and take this patch now
> (before I loose it)
I am waiting for a final rev of Lars' patches to add adt7310/adt7320 support and
alarm interrupt support to the hwmon driver, but I don't see a problem getting
it all into 3.10.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2013-03-02 17:09 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-18 13:38 [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers Lars-Peter Clausen
2013-02-18 13:38 ` [PATCH v2 2/4] hwmon: (adt7410) Add support for the adt7310/adt7320 Lars-Peter Clausen
2013-02-19 1:30 ` Guenter Roeck
2013-02-19 11:57 ` Lars-Peter Clausen
2013-02-19 16:52 ` Guenter Roeck
2013-02-18 13:38 ` [PATCH v2 3/4] hwmon: (adt7x10) Add alarm interrupt support Lars-Peter Clausen
2013-02-19 1:39 ` Guenter Roeck
2013-02-19 12:05 ` Lars-Peter Clausen
2013-02-19 17:10 ` Guenter Roeck
2013-02-18 13:38 ` [PATCH v2 4/4] staging:iio: Remove adt7410 driver Lars-Peter Clausen
2013-03-02 16:45 ` Jonathan Cameron
2013-03-02 17:10 ` Guenter Roeck
2013-02-18 20:22 ` [PATCH v2 1/4] hwmon: (adt7410) Don't re-read non-volatile registers Hartmut Knaack
2013-02-19 1:02 ` Guenter Roeck
2013-02-19 19:39 ` Hartmut Knaack
2013-02-20 1:22 ` Guenter Roeck
[not found] ` <5128112C.4080909@gmx.de>
2013-02-23 20:18 ` Guenter Roeck
2013-02-25 22:03 ` Hartmut Knaack
2013-02-26 1:40 ` Guenter Roeck
2013-02-25 9:54 ` Lars-Peter Clausen
2013-02-25 21:30 ` Hartmut Knaack
2013-02-26 1:39 ` Guenter Roeck
2013-02-19 1:32 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox