All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors]  [PATCH v4] hwmon: Add support for MAX6642
@ 2011-04-06 18:29 Per Dalén
  2011-04-06 18:58 ` Guenter Roeck
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Per Dalén @ 2011-04-06 18:29 UTC (permalink / raw)
  To: lm-sensors

[-- Attachment #1: Type: text/plain, Size: 296 bytes --]

First a big thanks to Guenter Roeck for helping me (allot) to cleanup
this driver

v3 -> v4:
 * Removed "data->valid = 0;" (missed that in v3 ;)
 * only read the limits once
 * changed the temp_input and temp_high as suggested by Guenter, also
changed the functions/macros related to that change

[-- Attachment #2: max6642_v4.diff --]
[-- Type: text/plain, Size: 12369 bytes --]

MAX6642 is a SMBus-Compatible Remote/Local Temperature Sensor with Overtemperature Alarm from Maxim.
 
Signed-off-by: Per Dalen <per.dalen@appeartv.com>

---
diff --git a/Documentation/hwmon/max6642 b/Documentation/hwmon/max6642
new file mode 100644
index 0000000..afbd3e4
--- /dev/null
+++ b/Documentation/hwmon/max6642
@@ -0,0 +1,21 @@
+Kernel driver max6642
+=====================
+
+Supported chips:
+  * Maxim MAX6642
+    Prefix: 'max6642'
+    Addresses scanned: I2C 0x48-0x4f
+    Datasheet: Publicly available at the Maxim website
+               http://datasheets.maxim-ic.com/en/ds/MAX6642.pdf
+
+Authors:
+        Per Dalen <per.dalen@appeartv.com>
+
+Description
+-----------
+
+The MAX6642 is a digital temperature sensor. It senses its own temperature as
+well as the temperature on one external diode.
+
+All temperature values are given in degrees Celsius. Resolution
+is 0.25 degree for the local temperature and for the remote temperature.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 060ef63..e9a3d7a 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -728,6 +728,17 @@ config SENSORS_MAX6639
 	  This driver can also be built as a module.  If so, the module
 	  will be called max6639.
 
+config SENSORS_MAX6642
+	tristate "Maxim MAX6642 sensor chip"
+	depends on I2C && EXPERIMENTAL
+	help
+	  If you say yes here you get support for MAX6642 sensor chip.
+	  MAX6642 is a SMBus-Compatible Remote/Local Temperature Sensor
+	  with Overtemperature Alarm from Maxim.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called max6642.
+
 config SENSORS_MAX6650
 	tristate "Maxim MAX6650 sensor chip"
 	depends on I2C && EXPERIMENTAL
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 967d0ea..2211752 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_SENSORS_LTC4261)	+= ltc4261.o
 obj-$(CONFIG_SENSORS_MAX1111)	+= max1111.o
 obj-$(CONFIG_SENSORS_MAX1619)	+= max1619.o
 obj-$(CONFIG_SENSORS_MAX6639)	+= max6639.o
+obj-$(CONFIG_SENSORS_MAX6642)	+= max6642.o
 obj-$(CONFIG_SENSORS_MAX6650)	+= max6650.o
 obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
 obj-$(CONFIG_SENSORS_PC87360)	+= pc87360.o
diff --git a/drivers/hwmon/max6642.c b/drivers/hwmon/max6642.c
new file mode 100644
index 0000000..7422351
--- /dev/null
+++ b/drivers/hwmon/max6642.c
@@ -0,0 +1,359 @@
+/*
+ * Driver for +/-1 degree C, SMBus-Compatible Remote/Local Temperature Sensor
+ * with Overtemperature Alarm
+ *
+ * Copyright (C) 2011 AppearTV AS
+ *
+ * Derived from:
+ *
+ *  Based on the max1619 driver.
+ *  Copyright (C) 2003-2004 Alexey Fisher <fishor@mail.ru>
+ *                          Jean Delvare <khali@linux-fr.org>
+ *
+ * The MAX6642 is a sensor chip made by Maxim.
+ * It reports up to two temperatures (its own plus up to
+ * one external one). Complete datasheet can be
+ * obtained from Maxim's website at:
+ *   http://datasheets.maxim-ic.com/en/ds/MAX6642.pdf
+ *
+ * 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/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+
+static const unsigned short normal_i2c[] = {
+	0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f, I2C_CLIENT_END };
+
+/*
+ * The MAX6642 registers
+ */
+
+#define MAX6642_REG_R_MAN_ID		0xFE
+#define MAX6642_REG_R_CONFIG		0x03
+#define MAX6642_REG_W_CONFIG		0x09
+#define MAX6642_REG_R_STATUS		0x02
+#define MAX6642_REG_R_LOCAL_TEMP	0x00
+#define MAX6642_REG_R_LOCAL_TEMPL	0x11
+#define MAX6642_REG_R_LOCAL_HIGH	0x05
+#define MAX6642_REG_W_LOCAL_HIGH	0x0B
+#define MAX6642_REG_R_REMOTE_TEMP	0x01
+#define MAX6642_REG_R_REMOTE_TEMPL	0x10
+#define MAX6642_REG_R_REMOTE_HIGH	0x07
+#define MAX6642_REG_W_REMOTE_HIGH	0x0D
+
+/*
+ * Conversions
+ */
+
+static int temp_from_reg10(int val)
+{
+	return val * 250;
+}
+
+static int temp_from_reg(int val)
+{
+	return val * 1000;
+}
+
+static int temp_to_reg(int val)
+{
+	return val / 1000;
+}
+
+/*
+ * Client data (each client gets its own)
+ */
+
+struct max6642_data {
+	struct device *hwmon_dev;
+	struct mutex update_lock;
+	bool valid; /* zero until following fields are valid */
+	unsigned long last_updated; /* in jiffies */
+
+	/* registers values */
+	u16 temp_input[2]; /* local/remote */
+	u16 temp_high[2]; /* local/remote */
+	u8 alarms;
+};
+
+/*
+ * Real code
+ */
+
+static void max6642_init_client(struct i2c_client *client)
+{
+	u8 config;
+	struct max6642_data *data = i2c_get_clientdata(client);
+
+	/*
+	 * Start the conversions.
+	 */
+	config = i2c_smbus_read_byte_data(client, MAX6642_REG_R_CONFIG);
+	if (config & 0x40)
+		i2c_smbus_write_byte_data(client, MAX6642_REG_W_CONFIG,
+					  config & 0xBF); /* run */
+
+	data->temp_high[0] = i2c_smbus_read_byte_data(client,
+				MAX6642_REG_R_LOCAL_HIGH);
+	data->temp_high[1] = i2c_smbus_read_byte_data(client,
+				MAX6642_REG_R_REMOTE_HIGH);
+}
+
+/* Return 0 if detection is successful, -ENODEV otherwise */
+static int max6642_detect(struct i2c_client *client,
+			  struct i2c_board_info *info)
+{
+	struct i2c_adapter *adapter = client->adapter;
+	u8 reg_config, reg_status, man_id;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		return -ENODEV;
+
+	/* identification */
+	man_id = i2c_smbus_read_byte_data(client, MAX6642_REG_R_MAN_ID);
+	if (man_id != 0x4D)
+		return -ENODEV;
+
+	/*
+	 * We read the config and status register, the 4 lower bits in the
+	 * config register should be zero and bit 5, 3, 1 and 0 should be
+	 * zero in the status register.
+	 */
+	reg_config = i2c_smbus_read_byte_data(client, MAX6642_REG_R_CONFIG);
+	reg_status = i2c_smbus_read_byte_data(client, MAX6642_REG_R_STATUS);
+	if (((reg_config & 0x0f) != 0x00) ||
+	    ((reg_status & 0x2b) != 0x00))
+		return -ENODEV;
+
+	strlcpy(info->type, "max6642", I2C_NAME_SIZE);
+
+	return 0;
+}
+
+static struct max6642_data *max6642_update_device(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct max6642_data *data = i2c_get_clientdata(client);
+	u16 val, tmp;
+
+	mutex_lock(&data->update_lock);
+
+	if (time_after(jiffies, data->last_updated + HZ * 8) || !data->valid) {
+		dev_dbg(&client->dev, "Updating max6642 data.\n");
+		val = i2c_smbus_read_byte_data(client,
+					MAX6642_REG_R_LOCAL_TEMPL);
+		tmp = (val >> 6) & 3;
+		val = i2c_smbus_read_byte_data(client,
+					MAX6642_REG_R_LOCAL_TEMP);
+		val = (val << 2) | tmp;
+		data->temp_input[0] = val;
+		val = i2c_smbus_read_byte_data(client,
+					MAX6642_REG_R_REMOTE_TEMPL);
+		tmp = (val >> 6) & 3;
+		val = i2c_smbus_read_byte_data(client,
+					MAX6642_REG_R_REMOTE_TEMP);
+		val = (val << 2) | tmp;
+		data->temp_input[1] = val;
+		data->alarms = i2c_smbus_read_byte_data(client,
+					MAX6642_REG_R_STATUS);
+
+		data->last_updated = jiffies;
+		data->valid = 1;
+	}
+
+	mutex_unlock(&data->update_lock);
+
+	return data;
+}
+
+/*
+ * Sysfs stuff
+ */
+
+static ssize_t show_temp_max10(struct device *dev,
+			       struct device_attribute *dev_attr, char *buf)
+{
+	struct max6642_data *data = max6642_update_device(dev);
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
+
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+
+	return sprintf(buf, "%d\n",
+		       temp_from_reg10(data->temp_input[attr->index]));
+}
+
+static ssize_t show_temp_max(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	struct max6642_data *data = max6642_update_device(dev);
+	struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(attr);
+
+	return sprintf(buf, "%d\n", temp_from_reg(data->temp_high[attr2->nr]));
+}
+
+static ssize_t set_temp_max(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	unsigned long val;
+	int err;
+	struct i2c_client *client = to_i2c_client(dev);
+	struct max6642_data *data = i2c_get_clientdata(client);
+	struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(attr);
+
+	err = strict_strtoul(buf, 10, &val);
+	if (err < 0)
+		return err;
+
+	mutex_lock(&data->update_lock);
+	data->temp_high[attr2->nr] = SENSORS_LIMIT(temp_to_reg(val), 0, 255);
+	i2c_smbus_write_byte_data(client, attr2->index,
+				  data->temp_high[attr2->nr]);
+	mutex_unlock(&data->update_lock);
+	return count;
+}
+
+static ssize_t show_alarm(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	int bitnr = to_sensor_dev_attr(attr)->index;
+	struct max6642_data *data = max6642_update_device(dev);
+	return sprintf(buf, "%d\n", (data->alarms >> bitnr) & 1);
+}
+
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_max10, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_max10, NULL, 1);
+static SENSOR_DEVICE_ATTR_2(temp1_max, S_IWUSR | S_IRUGO, show_temp_max,
+			    set_temp_max, 0, MAX6642_REG_W_LOCAL_HIGH);
+static SENSOR_DEVICE_ATTR_2(temp2_max, S_IWUSR | S_IRUGO, show_temp_max,
+			    set_temp_max, 1, MAX6642_REG_W_REMOTE_HIGH);
+static SENSOR_DEVICE_ATTR(temp_fault, S_IRUGO, show_alarm, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO, show_alarm, NULL, 6);
+static SENSOR_DEVICE_ATTR(temp2_max_alarm, S_IRUGO, show_alarm, NULL, 4);
+
+static struct attribute *max6642_attributes[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	&sensor_dev_attr_temp2_input.dev_attr.attr,
+	&sensor_dev_attr_temp1_max.dev_attr.attr,
+	&sensor_dev_attr_temp2_max.dev_attr.attr,
+
+	&sensor_dev_attr_temp_fault.dev_attr.attr,
+	&sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
+	&sensor_dev_attr_temp2_max_alarm.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group max6642_group = {
+	.attrs = max6642_attributes,
+};
+
+static int max6642_probe(struct i2c_client *new_client,
+			 const struct i2c_device_id *id)
+{
+	struct max6642_data *data;
+	int err;
+
+	data = kzalloc(sizeof(struct max6642_data), GFP_KERNEL);
+	if (!data) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	i2c_set_clientdata(new_client, data);
+	mutex_init(&data->update_lock);
+
+	/* Initialize the MAX6642 chip */
+	max6642_init_client(new_client);
+
+	/* Register sysfs hooks */
+	err = sysfs_create_group(&new_client->dev.kobj, &max6642_group);
+	if (err)
+		goto exit_free;
+
+	data->hwmon_dev = hwmon_device_register(&new_client->dev);
+	if (IS_ERR(data->hwmon_dev)) {
+		err = PTR_ERR(data->hwmon_dev);
+		goto exit_remove_files;
+	}
+
+	return 0;
+
+exit_remove_files:
+	sysfs_remove_group(&new_client->dev.kobj, &max6642_group);
+exit_free:
+	kfree(data);
+exit:
+	return err;
+}
+
+static int max6642_remove(struct i2c_client *client)
+{
+	struct max6642_data *data = i2c_get_clientdata(client);
+
+	hwmon_device_unregister(data->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &max6642_group);
+
+	kfree(data);
+	return 0;
+}
+
+/*
+ * Driver data (common to all clients)
+ */
+
+static const struct i2c_device_id max6642_id[] = {
+	{ "max6642", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, max6642_id);
+
+static struct i2c_driver max6642_driver = {
+	.class		= I2C_CLASS_HWMON,
+	.driver = {
+		.name	= "max6642",
+	},
+	.probe		= max6642_probe,
+	.remove		= max6642_remove,
+	.id_table	= max6642_id,
+	.detect		= max6642_detect,
+	.address_list	= normal_i2c,
+};
+
+static int __init max6642_init(void)
+{
+	return i2c_add_driver(&max6642_driver);
+}
+
+static void __exit max6642_exit(void)
+{
+	i2c_del_driver(&max6642_driver);
+}
+
+MODULE_AUTHOR("Per Dalen <per.dalen@appeartv.com>");
+MODULE_DESCRIPTION("MAX6642 sensor driver");
+MODULE_LICENSE("GPL");
+
+module_init(max6642_init);
+module_exit(max6642_exit);

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

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

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

* Re: [lm-sensors] [PATCH v4] hwmon: Add support for MAX6642
  2011-04-06 18:29 [lm-sensors] [PATCH v4] hwmon: Add support for MAX6642 Per Dalén
@ 2011-04-06 18:58 ` Guenter Roeck
  2011-04-06 19:06 ` Per Dalén
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2011-04-06 18:58 UTC (permalink / raw)
  To: lm-sensors

Hi Per,

On Wed, Apr 06, 2011 at 02:29:44PM -0400, Per Dalén wrote:
> First a big thanks to Guenter Roeck for helping me (allot) to cleanup
> this driver
> 
> v3 -> v4:
>  * Removed "data->valid = 0;" (missed that in v3 ;)
>  * only read the limits once
>  * changed the temp_input and temp_high as suggested by Guenter, also
> changed the functions/macros related to that change

> MAX6642 is a SMBus-Compatible Remote/Local Temperature Sensor with Overtemperature Alarm from Maxim.
>  
> Signed-off-by: Per Dalen <per.dalen@appeartv.com>
> 
Sorry, one more detail I just noticed. More of a question.

[ ... ]
> +
> +static struct max6642_data *max6642_update_device(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct max6642_data *data = i2c_get_clientdata(client);
> +	u16 val, tmp;
> +
> +	mutex_lock(&data->update_lock);
> +
> +	if (time_after(jiffies, data->last_updated + HZ * 8) || !data->valid) {

Are you sure you only want to update readings every 8 seconds ?

Also, after spending all this time on this driver (and after falling into
the same trap last night with another driver ;), it looks like this is really
just another variant of the lm90, and adding support for max6642 to the lm90
driver might be quite trivial. Have you considered this ?

Thanks,
Guenter


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

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

* Re: [lm-sensors] [PATCH v4] hwmon: Add support for MAX6642
  2011-04-06 18:29 [lm-sensors] [PATCH v4] hwmon: Add support for MAX6642 Per Dalén
  2011-04-06 18:58 ` Guenter Roeck
@ 2011-04-06 19:06 ` Per Dalén
  2011-04-06 19:32 ` Guenter Roeck
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Per Dalén @ 2011-04-06 19:06 UTC (permalink / raw)
  To: lm-sensors

On 04/06/2011 08:58 PM, Guenter Roeck wrote:
> Hi Per,
> 
> On Wed, Apr 06, 2011 at 02:29:44PM -0400, Per Dalén wrote:
>> First a big thanks to Guenter Roeck for helping me (allot) to cleanup
>> this driver
>>
>> v3 -> v4:
>>  * Removed "data->valid = 0;" (missed that in v3 ;)
>>  * only read the limits once
>>  * changed the temp_input and temp_high as suggested by Guenter, also
>> changed the functions/macros related to that change
> 
>> MAX6642 is a SMBus-Compatible Remote/Local Temperature Sensor with Overtemperature Alarm from Maxim.
>>  
>> Signed-off-by: Per Dalen <per.dalen@appeartv.com>
>>
> Sorry, one more detail I just noticed. More of a question.
> 
> [ ... ]
>> +
>> +static struct max6642_data *max6642_update_device(struct device *dev)
>> +{
>> +	struct i2c_client *client = to_i2c_client(dev);
>> +	struct max6642_data *data = i2c_get_clientdata(client);
>> +	u16 val, tmp;
>> +
>> +	mutex_lock(&data->update_lock);
>> +
>> +	if (time_after(jiffies, data->last_updated + HZ * 8) || !data->valid) {
> 
> Are you sure you only want to update readings every 8 seconds ?

No, this is wrong from me. Every second, or what do you think?

> 
> Also, after spending all this time on this driver (and after falling into
> the same trap last night with another driver ;), it looks like this is really
> just another variant of the lm90, and adding support for max6642 to the lm90
> driver might be quite trivial. Have you considered this ?

Hmmm, ;)

"Re: [lm-sensors] [PATCH] hwmon: (lm90) Add support for max6642

On 03/03/2011 08:55 PM, Guenter Roeck wrote:
With all the differences and lacking registers, I wonder if it would be
better to write a new driver, either based on this driver of based on
the max1619 driver.

Guenter"

> 
> Thanks,
> Guenter
> 

br
Per

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

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

* Re: [lm-sensors] [PATCH v4] hwmon: Add support for MAX6642
  2011-04-06 18:29 [lm-sensors] [PATCH v4] hwmon: Add support for MAX6642 Per Dalén
  2011-04-06 18:58 ` Guenter Roeck
  2011-04-06 19:06 ` Per Dalén
@ 2011-04-06 19:32 ` Guenter Roeck
  2011-04-06 20:22 ` Per Dalén
  2011-04-06 21:54 ` Guenter Roeck
  4 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2011-04-06 19:32 UTC (permalink / raw)
  To: lm-sensors

On Wed, Apr 06, 2011 at 03:06:13PM -0400, Per Dalén wrote:
> On 04/06/2011 08:58 PM, Guenter Roeck wrote:
> > Hi Per,
> > 
> > On Wed, Apr 06, 2011 at 02:29:44PM -0400, Per Dalén wrote:
> >> First a big thanks to Guenter Roeck for helping me (allot) to cleanup
> >> this driver
> >>
> >> v3 -> v4:
> >>  * Removed "data->valid = 0;" (missed that in v3 ;)
> >>  * only read the limits once
> >>  * changed the temp_input and temp_high as suggested by Guenter, also
> >> changed the functions/macros related to that change
> > 
> >> MAX6642 is a SMBus-Compatible Remote/Local Temperature Sensor with Overtemperature Alarm from Maxim.
> >>  
> >> Signed-off-by: Per Dalen <per.dalen@appeartv.com>
> >>
> > Sorry, one more detail I just noticed. More of a question.
> > 
> > [ ... ]
> >> +
> >> +static struct max6642_data *max6642_update_device(struct device *dev)
> >> +{
> >> +	struct i2c_client *client = to_i2c_client(dev);
> >> +	struct max6642_data *data = i2c_get_clientdata(client);
> >> +	u16 val, tmp;
> >> +
> >> +	mutex_lock(&data->update_lock);
> >> +
> >> +	if (time_after(jiffies, data->last_updated + HZ * 8) || !data->valid) {
> > 
> > Are you sure you only want to update readings every 8 seconds ?
> 
> No, this is wrong from me. Every second, or what do you think?
> 
Something like that, or HZ + HZ/<fraction> if you want to be fancy.

> > 
> > Also, after spending all this time on this driver (and after falling into
> > the same trap last night with another driver ;), it looks like this is really
> > just another variant of the lm90, and adding support for max6642 to the lm90
> > driver might be quite trivial. Have you considered this ?
> 
> Hmmm, ;)
> 
> "Re: [lm-sensors] [PATCH] hwmon: (lm90) Add support for max6642
> 
> On 03/03/2011 08:55 PM, Guenter Roeck wrote:
> With all the differences and lacking registers, I wonder if it would be
> better to write a new driver, either based on this driver of based on
> the max1619 driver.
> 
> Guenter"
> 
Guess you got me there. But then this was more than a month ago, and it was
my birthday, so who knows how much blood was left in my alcohol ;).

Thanks,
Guenter

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

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

* Re: [lm-sensors] [PATCH v4] hwmon: Add support for MAX6642
  2011-04-06 18:29 [lm-sensors] [PATCH v4] hwmon: Add support for MAX6642 Per Dalén
                   ` (2 preceding siblings ...)
  2011-04-06 19:32 ` Guenter Roeck
@ 2011-04-06 20:22 ` Per Dalén
  2011-04-06 21:54 ` Guenter Roeck
  4 siblings, 0 replies; 6+ messages in thread
From: Per Dalén @ 2011-04-06 20:22 UTC (permalink / raw)
  To: lm-sensors

> On Wed, Apr 06, 2011 at 03:06:13PM -0400, Per Dalén wrote:
>> On 04/06/2011 08:58 PM, Guenter Roeck wrote:
>> > Hi Per,
>> >
>> > On Wed, Apr 06, 2011 at 02:29:44PM -0400, Per Dalén wrote:
>> >> First a big thanks to Guenter Roeck for helping me (allot) to cleanup
>> >> this driver
>> >>
>> >> v3 -> v4:
>> >>  * Removed "data->valid = 0;" (missed that in v3 ;)
>> >>  * only read the limits once
>> >>  * changed the temp_input and temp_high as suggested by Guenter, also
>> >> changed the functions/macros related to that change
>> >
>> >> MAX6642 is a SMBus-Compatible Remote/Local Temperature Sensor with
>> Overtemperature Alarm from Maxim.
>> >>
>> >> Signed-off-by: Per Dalen <per.dalen@appeartv.com>
>> >>
>> > Sorry, one more detail I just noticed. More of a question.
>> >
>> > [ ... ]
>> >> +
>> >> +static struct max6642_data *max6642_update_device(struct device
>> *dev)
>> >> +{
>> >> +	struct i2c_client *client = to_i2c_client(dev);
>> >> +	struct max6642_data *data = i2c_get_clientdata(client);
>> >> +	u16 val, tmp;
>> >> +
>> >> +	mutex_lock(&data->update_lock);
>> >> +
>> >> +	if (time_after(jiffies, data->last_updated + HZ * 8) ||
>> !data->valid) {
>> >
>> > Are you sure you only want to update readings every 8 seconds ?
>>
>> No, this is wrong from me. Every second, or what do you think?
>>
> Something like that, or HZ + HZ/<fraction> if you want to be fancy.
>
>> >
>> > Also, after spending all this time on this driver (and after falling
>> into
>> > the same trap last night with another driver ;), it looks like this is
>> really
>> > just another variant of the lm90, and adding support for max6642 to
>> the lm90
>> > driver might be quite trivial. Have you considered this ?
>>
>> Hmmm, ;)
>>
>> "Re: [lm-sensors] [PATCH] hwmon: (lm90) Add support for max6642
>>
>> On 03/03/2011 08:55 PM, Guenter Roeck wrote:
>> With all the differences and lacking registers, I wonder if it would be
>> better to write a new driver, either based on this driver of based on
>> the max1619 driver.
>>
>> Guenter"
>>
> Guess you got me there. But then this was more than a month ago, and it
> was
> my birthday, so who knows how much blood was left in my alcohol ;).

Ok, that is something I can relate to ;)

Do you think I should do the last fix on the max6642 driver or should I
add it to lm90?

>
> Thanks,
> Guenter
>

/Per


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

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

* Re: [lm-sensors] [PATCH v4] hwmon: Add support for MAX6642
  2011-04-06 18:29 [lm-sensors] [PATCH v4] hwmon: Add support for MAX6642 Per Dalén
                   ` (3 preceding siblings ...)
  2011-04-06 20:22 ` Per Dalén
@ 2011-04-06 21:54 ` Guenter Roeck
  4 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2011-04-06 21:54 UTC (permalink / raw)
  To: lm-sensors

On Wed, Apr 06, 2011 at 03:06:13PM -0400, Per Dalén wrote:
> On 04/06/2011 08:58 PM, Guenter Roeck wrote:
> > Hi Per,
> > 
> > On Wed, Apr 06, 2011 at 02:29:44PM -0400, Per Dalén wrote:
> >> First a big thanks to Guenter Roeck for helping me (allot) to cleanup
> >> this driver
> >>
> >> v3 -> v4:
> >>  * Removed "data->valid = 0;" (missed that in v3 ;)
> >>  * only read the limits once
> >>  * changed the temp_input and temp_high as suggested by Guenter, also
> >> changed the functions/macros related to that change
> > 
> >> MAX6642 is a SMBus-Compatible Remote/Local Temperature Sensor with Overtemperature Alarm from Maxim.
> >>  
> >> Signed-off-by: Per Dalen <per.dalen@appeartv.com>
> >>
> > Sorry, one more detail I just noticed. More of a question.
> > 
> > [ ... ]
> >> +
> >> +static struct max6642_data *max6642_update_device(struct device *dev)
> >> +{
> >> +	struct i2c_client *client = to_i2c_client(dev);
> >> +	struct max6642_data *data = i2c_get_clientdata(client);
> >> +	u16 val, tmp;
> >> +
> >> +	mutex_lock(&data->update_lock);
> >> +
> >> +	if (time_after(jiffies, data->last_updated + HZ * 8) || !data->valid) {
> > 
> > Are you sure you only want to update readings every 8 seconds ?
> 
> No, this is wrong from me. Every second, or what do you think?
> 
No need to re-submit. I applied v4 of your patch, changing the above to HZ.

Thanks,
Guenter

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

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

end of thread, other threads:[~2011-04-06 21:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-06 18:29 [lm-sensors] [PATCH v4] hwmon: Add support for MAX6642 Per Dalén
2011-04-06 18:58 ` Guenter Roeck
2011-04-06 19:06 ` Per Dalén
2011-04-06 19:32 ` Guenter Roeck
2011-04-06 20:22 ` Per Dalén
2011-04-06 21:54 ` Guenter Roeck

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.