All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature sensor
@ 2008-02-12 16:50 Edelhaeuser, Frank
  2008-02-17 20:33 ` Sean MacLennan
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Edelhaeuser, Frank @ 2008-02-12 16:50 UTC (permalink / raw)
  To: lm-sensors

Now posting to lm-sensors (originally sent to i2c list).
Thanks,
Frank

---

Signed-off-by: Frank Edelhaeuser <frank.edelhaeuser@spansion.com>
---
diff -Nur a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
--- a/drivers/hwmon/Kconfig	2007-11-11 21:49:02.000000000 +0100
+++ b/drivers/hwmon/Kconfig	2008-02-01 11:54:48.000000000 +0100
@@ -57,6 +57,16 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called abituguru3.

+config SENSORS_AD7414
+	tristate "Analog Devices AD7414"
+	depends on I2C && EXPERIMENTAL
+	help
+	  If you say yes here you get support for the Analog Devices
+	  AD7414 temperature monitoring chip.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called ad7414.
+
 config SENSORS_AD7418
 	tristate "Analog Devices AD7416, AD7417 and AD7418"
 	depends on I2C && EXPERIMENTAL
diff -Nur a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
--- a/drivers/hwmon/Makefile	2007-11-11 21:49:02.000000000 +0100
+++ b/drivers/hwmon/Makefile	2008-02-01 11:54:48.000000000 +0100
@@ -15,6 +15,7 @@

 obj-$(CONFIG_SENSORS_ABITUGURU)	+= abituguru.o
 obj-$(CONFIG_SENSORS_ABITUGURU3)+= abituguru3.o
+obj-$(CONFIG_SENSORS_AD7414)	+= ad7414.o
 obj-$(CONFIG_SENSORS_AD7418)	+= ad7418.o
 obj-$(CONFIG_SENSORS_ADM1021)	+= adm1021.o
 obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025.o
@@ -66,4 +67,3 @@
 ifeq ($(CONFIG_HWMON_DEBUG_CHIP),y)
 EXTRA_CFLAGS += -DDEBUG
 endif
-
diff -Nur a/drivers/hwmon/ad7414.c b/drivers/hwmon/ad7414.c
--- a/drivers/hwmon/ad7414.c	1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/hwmon/ad7414.c	2008-02-01 14:16:54.000000000 +0100
@@ -0,0 +1,253 @@
+/*
+ * An hwmon driver for the Analog Devices AD7414
+ *
+ * Copyright 2006 Stefan Roese <sr at denx.de>, DENX Software
Engineering
+ *
+ * Copyright (c) 2008 PIKA Technologies
+ *   Sean MacLennan <smaclennan at pikatech.com>
+ *
+ * Copyright (c) 2008 Spansion Inc.
+ *   Frank Edelhaeuser <frank.edelhaeuser at spansion.com>
+ *   (converted to "new style" I2C driver model, removed checkpatch.pl
warnings)
+ *
+ * Based on ad7418.c
+ * Copyright 2006 Tower Technologies, Alessandro Zummo <a.zummo at
towertech.it>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/delay.h>
+
+
+#define DRV_VERSION "0.3"
+
+/* straight from the datasheet */
+#define AD7414_TEMP_MIN (-55000)
+#define AD7414_TEMP_MAX 125000
+
+/* AD7414 registers */
+#define AD7414_REG_TEMP		0x00
+#define AD7414_REG_CONF		0x01
+#define AD7414_REG_T_HIGH	0x02
+#define AD7414_REG_T_LOW	0x03
+
+
+struct ad7414_data {
+	struct i2c_client	client;
+	struct class_device	*dev;
+	struct mutex	lock;	/* atomic read data updates */
+	char			valid;	/* !=0 if following fields are
valid */
+	unsigned long	last_updated;	/* In jiffies */
+	u16			temp_input;	/* Register values */
+	u8			temp_max;
+	u8			temp_min;
+	u8			temp_alert;
+	u8			temp_max_flag;
+	u8			temp_min_flag;
+};
+
+/*
+ * TEMP: 0.001C/bit (-55C to +125C)
+ * REG: (0.5C/bit, two's complement) << 7
+ */
+static inline int AD7414_TEMP_FROM_REG(u16 reg)
+{
+	/* use integer division instead of equivalent right shift to
+	 * guarantee arithmetic shift and preserve the sign
+	 */
+	return ((s16)reg / 128) * 500;
+}
+
+/* All registers are word-sized, except for the configuration
registers.
+ * AD7414 uses a high-byte first convention, which is exactly opposite
to
+ * the usual practice.
+ */
+static int ad7414_read(struct i2c_client *client, u8 reg)
+{
+	if (reg = AD7414_REG_TEMP)
+		return swab16(i2c_smbus_read_word_data(client, reg));
+	else
+		return i2c_smbus_read_byte_data(client, reg);
+}
+
+static int ad7414_write(struct i2c_client *client, u8 reg, u16 value)
+{
+	return i2c_smbus_write_byte_data(client, reg, value);
+}
+
+static struct ad7414_data *ad7414_update_device(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ad7414_data *data = i2c_get_clientdata(client);
+
+	mutex_lock(&data->lock);
+
+	if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
+		|| !data->valid) {
+		dev_dbg(&client->dev, "starting ad7414 update\n");
+
+		data->temp_input = ad7414_read(client, AD7414_REG_TEMP);
+		data->temp_alert = (data->temp_input >> 5) & 0x01;
+		data->temp_max_flag = (data->temp_input >> 4) & 0x01;
+		data->temp_min_flag = (data->temp_input >> 3) & 0x01;
+		data->temp_max = ad7414_read(client, AD7414_REG_T_HIGH);
+		data->temp_min = ad7414_read(client, AD7414_REG_T_LOW);
+
+		data->last_updated = jiffies;
+		data->valid = 1;
+	}
+
+	mutex_unlock(&data->lock);
+
+	return data;
+}
+
+#define show(value) \
+static ssize_t show_##value(struct device *dev, \
+	struct device_attribute *attr, char *buf)		\
+{
\
+	struct ad7414_data *data = ad7414_update_device(dev);
\
+	return sprintf(buf, "%d\n", AD7414_TEMP_FROM_REG(data->value));
\
+}
+show(temp_input);
+
+#define show_8(value)	\
+static ssize_t show_##value(struct device *dev, \
+	struct device_attribute *attr, char *buf)		\
+{								\
+	struct ad7414_data *data = ad7414_update_device(dev);	\
+	return sprintf(buf, "%d\n", data->value);		\
+}
+show_8(temp_max);
+show_8(temp_min);
+show_8(temp_alert);
+show_8(temp_max_flag);
+show_8(temp_min_flag);
+
+#define set(value, reg)	\
+static ssize_t set_##value(struct device *dev, \
+	struct device_attribute *attr, \
+	const char *buf, size_t count)	\
+{								\
+	struct i2c_client *client = to_i2c_client(dev);		\
+	struct ad7414_data *data = i2c_get_clientdata(client);	\
+	int temp = simple_strtoul(buf, NULL, 10);		\
+								\
+	mutex_lock(&data->lock);				\
+	data->value = temp;					\
+	ad7414_write(client, reg, data->value);			\
+	mutex_unlock(&data->lock);				\
+	return count;						\
+}
+set(temp_max, AD7414_REG_T_HIGH);
+set(temp_min, AD7414_REG_T_LOW);
+
+static DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_max,
set_temp_max);
+static DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO, show_temp_min,
set_temp_min);
+static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_input, NULL);
+static DEVICE_ATTR(temp1_alert, S_IRUGO, show_temp_alert, NULL);
+static DEVICE_ATTR(temp1_max_flag, S_IRUGO, show_temp_max_flag, NULL);
+static DEVICE_ATTR(temp1_min_flag, S_IRUGO, show_temp_min_flag, NULL);
+
+
+static struct attribute *ad7414_attributes[] = {
+	&dev_attr_temp1_input.attr,
+	&dev_attr_temp1_max.attr,
+	&dev_attr_temp1_min.attr,
+	&dev_attr_temp1_alert.attr,
+	&dev_attr_temp1_max_flag.attr,
+	&dev_attr_temp1_min_flag.attr,
+	NULL
+};
+
+static const struct attribute_group ad7414_group = {
+	.attrs = ad7414_attributes,
+};
+
+static int ad7414_probe(struct i2c_client *client)
+{
+	struct ad7414_data *data;
+	int err = 0;
+
+	if (!i2c_check_functionality(client->adapter,
I2C_FUNC_SMBUS_BYTE_DATA |
+					I2C_FUNC_SMBUS_WORD_DATA))
+		goto exit;
+
+	data = kzalloc(sizeof(struct ad7414_data), GFP_KERNEL);
+	if (!data) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	i2c_set_clientdata(client, data);
+	mutex_init(&data->lock);
+
+	dev_info(&client->dev, "chip found, driver version " DRV_VERSION
"\n");
+
+	/* Register sysfs hooks */
+	err = sysfs_create_group(&client->dev.kobj, &ad7414_group);
+	if (err)
+		goto exit_free;
+
+	data->dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(data->dev)) {
+		err = PTR_ERR(data->dev);
+		goto exit_remove;
+	}
+
+	return 0;
+
+exit_remove:
+	sysfs_remove_group(&client->dev.kobj, &ad7414_group);
+exit_free:
+	kfree(data);
+exit:
+	return err;
+}
+
+static int ad7414_remove(struct i2c_client *client)
+{
+	struct ad7414_data *data = i2c_get_clientdata(client);
+
+	hwmon_device_unregister(data->dev);
+	sysfs_remove_group(&client->dev.kobj, &ad7414_group);
+	kfree(data);
+	return 0;
+}
+
+static struct i2c_driver ad7414_driver = {
+	.driver = {
+		.name	= "ad7414",
+	},
+	.probe	= ad7414_probe,
+	.remove	= __devexit_p(ad7414_remove),
+};
+
+static int __init ad7414_init(void)
+{
+	return i2c_add_driver(&ad7414_driver);
+}
+
+static void __exit ad7414_exit(void)
+{
+	i2c_del_driver(&ad7414_driver);
+}
+
+MODULE_AUTHOR("Stefan Roese <sr at denx.de>, "
+		"Frank Edelhaeuser <frank.edelhaeuser@spansion.com>");
+
+MODULE_DESCRIPTION("AD7414 driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(DRV_VERSION);
+
+module_init(ad7414_init);
+module_exit(ad7414_exit);

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature sensor
  2008-02-12 16:50 [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature sensor Edelhaeuser, Frank
@ 2008-02-17 20:33 ` Sean MacLennan
  2008-02-18 12:30 ` [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature Jean Delvare
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Sean MacLennan @ 2008-02-17 20:33 UTC (permalink / raw)
  To: lm-sensors

I tried this driver and it didn't detect my ad7414 chip...  the probe 
function is never called

I am using the i2c-ibm_iic.c driver for the i2c bus and running 2.6.25-rc2.

It works using a different version of the driver and the attach and 
detach methods. Could the ibm iic driver be missing something?

Cheers,
  Sean

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* Re: [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature
  2008-02-12 16:50 [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature sensor Edelhaeuser, Frank
  2008-02-17 20:33 ` Sean MacLennan
@ 2008-02-18 12:30 ` Jean Delvare
  2008-02-18 18:15 ` Sean MacLennan
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Jean Delvare @ 2008-02-18 12:30 UTC (permalink / raw)
  To: lm-sensors

Hi Sean,

On Sun, 17 Feb 2008 15:33:14 -0500, Sean MacLennan wrote:
> I tried this driver and it didn't detect my ad7414 chip...  the probe 
> function is never called
> 
> I am using the i2c-ibm_iic.c driver for the i2c bus and running 2.6.25-rc2.
> 
> It works using a different version of the driver and the attach and 
> detach methods. Could the ibm iic driver be missing something?

Frank's driver is a "new-style" i2c driver. These drivers do not probe
the i2c adapters for supported chips automatically. Instead, platform
code should explicitly instantiate the I2C devices that are present on
the system. In the case of embedded designs, this is typically done by
calling i2c_register_board_info() in platform code.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* Re: [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature
  2008-02-12 16:50 [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature sensor Edelhaeuser, Frank
  2008-02-17 20:33 ` Sean MacLennan
  2008-02-18 12:30 ` [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature Jean Delvare
@ 2008-02-18 18:15 ` Sean MacLennan
  2008-02-18 18:55 ` Jean Delvare
  2008-02-19  2:21 ` Sean MacLennan
  4 siblings, 0 replies; 7+ messages in thread
From: Sean MacLennan @ 2008-02-18 18:15 UTC (permalink / raw)
  To: lm-sensors

Jean Delvare wrote:
> Hi Sean,
>
> On Sun, 17 Feb 2008 15:33:14 -0500, Sean MacLennan wrote:
>   
>> I tried this driver and it didn't detect my ad7414 chip...  the probe 
>> function is never called
>>
>> I am using the i2c-ibm_iic.c driver for the i2c bus and running 2.6.25-rc2.
>>
>> It works using a different version of the driver and the attach and 
>> detach methods. Could the ibm iic driver be missing something?
>>     
>
> Frank's driver is a "new-style" i2c driver. These drivers do not probe
> the i2c adapters for supported chips automatically. Instead, platform
> code should explicitly instantiate the I2C devices that are present on
> the system. In the case of embedded designs, this is typically done by
> calling i2c_register_board_info() in platform code.
>   
That works, thanks! So the patch works for me.

Cheers,
   Sean

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* Re: [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature
  2008-02-12 16:50 [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature sensor Edelhaeuser, Frank
                   ` (2 preceding siblings ...)
  2008-02-18 18:15 ` Sean MacLennan
@ 2008-02-18 18:55 ` Jean Delvare
  2008-02-19  2:21 ` Sean MacLennan
  4 siblings, 0 replies; 7+ messages in thread
From: Jean Delvare @ 2008-02-18 18:55 UTC (permalink / raw)
  To: lm-sensors

On Mon, 18 Feb 2008 13:15:14 -0500, Sean MacLennan wrote:
> Jean Delvare wrote:
> > Frank's driver is a "new-style" i2c driver. These drivers do not probe
> > the i2c adapters for supported chips automatically. Instead, platform
> > code should explicitly instantiate the I2C devices that are present on
> > the system. In the case of embedded designs, this is typically done by
> > calling i2c_register_board_info() in platform code.
>
> That works, thanks! So the patch works for me.

Great, thanks for testing. Would you also feel like reviewing the code?

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* Re: [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature
  2008-02-12 16:50 [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature sensor Edelhaeuser, Frank
                   ` (3 preceding siblings ...)
  2008-02-18 18:55 ` Jean Delvare
@ 2008-02-19  2:21 ` Sean MacLennan
  4 siblings, 0 replies; 7+ messages in thread
From: Sean MacLennan @ 2008-02-19  2:21 UTC (permalink / raw)
  To: lm-sensors

Jean Delvare wrote:
> Great, thanks for testing. Would you also feel like reviewing the code?
>
>   
I grabbed the original message from the web interface. Some of the 
longer lines were wrapped,  I am not sure if that is a problem with the 
original patch, or the web interface. An example is:

+ * Copyright 2006 Stefan Roese <sr at denx.de>, DENX Software
Engineering


Some comments:

+struct ad7414_data {
+	struct i2c_client	client;
+	struct class_device	*dev;

I this this should be struct device, not class_device.

+	struct mutex	lock;	/* atomic read data updates */

Line up?

+	char			valid;	/* !=0 if following fields are
valid */
+	unsigned long	last_updated;	/* In jiffies */

Line up?

+	u16			temp_input;	/* Register values */
+	u8			temp_max;
+	u8			temp_min;
+	u8			temp_alert;
+	u8			temp_max_flag;
+	u8			temp_min_flag;
+};



Much codes goes by ;)

+static int ad7414_remove(struct i2c_client *client)
+{
+	struct ad7414_data *data = i2c_get_clientdata(client);
+
+	hwmon_device_unregister(data->dev);
+	sysfs_remove_group(&client->dev.kobj, &ad7414_group);
+	kfree(data);
+	return 0;
+}
+
+static struct i2c_driver ad7414_driver = {
+	.driver = {
+		.name	= "ad7414",
+	},
+	.probe	= ad7414_probe,
+	.remove	= __devexit_p(ad7414_remove),
+};

I believe the ad7414_remove function needs a _devexit to match the 
__devexit_p. Other than that, the code looks clean.

Cheers,
   Sean

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* Re: [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature
@ 2008-06-15  8:01 Jean Delvare
  0 siblings, 0 replies; 7+ messages in thread
From: Jean Delvare @ 2008-06-15  8:01 UTC (permalink / raw)
  To: lm-sensors

Hi Sean,

Sorry for the late reply, your reply got lost in my mailbox.

On Wed, 30 Apr 2008 00:41:01 -0400, Sean MacLennan wrote:
> Here is a rework based on Jean Delvare's suggestions.
> 
> One change that may not be apparent is that the alert bit is active
> low. I feel that it reads wrong to have "cat temp_alert" return 1
> when the alert is off. So I invert this bit. Let me know how you feel
> about this.

I agree with you that 1 should mean "on" in sysfs, regardless of the
physical polarity. However, it seems to me that the ALERT polarity can
be changed (bit 4 of configuration register) so you shouldn't always
invert the bit in sysfs, only when the polarity is actually inverted.

That being said... After reading the datasheet, I am under the
impression that temp1_alert and temp1_max_alarm will trigger on the
same condition (temp1 > temp1_max). So having both files doesn't look
terribly useful to me. Especially when "temp1_alert" is not a standard
feature name, so libsensors will no pick it.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

end of thread, other threads:[~2008-06-15  8:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-12 16:50 [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature sensor Edelhaeuser, Frank
2008-02-17 20:33 ` Sean MacLennan
2008-02-18 12:30 ` [lm-sensors] [PATCH] Add driver for AD7414 i2c temperature Jean Delvare
2008-02-18 18:15 ` Sean MacLennan
2008-02-18 18:55 ` Jean Delvare
2008-02-19  2:21 ` Sean MacLennan
  -- strict thread matches above, loose matches on Subject: below --
2008-06-15  8:01 Jean Delvare

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