Skip to contentSkip to conversation

ADM1030 and Co sensors chips support for 2.6.7-mm1

PATCH New Alexandre d'Alton 4 participants 8 messages May 19, 2005

lore
Share & tools
lore

Alexandre d'Alton

author
#0 lore raw

PATCH 1/1 ADM1030 and Co sensors chips support for 2.6.7-mm1

Hi Greg,

Here is a patch against linux-2.6.7-mm1 kernel that adds support for adm1030 and adm1031 sensors chips.

It has been reviewed several times by Jean Delvare, and he thinks it is ready for inclusion into 2.6 kernel. So here it is.

Thanks,

Alexandre.

Signed-off-by Alexandre d'Alton <[email protected]>
drivers/i2c/chips/Kconfig +10 -0
diff -urN linux-2.6.7-mm1-orig/drivers/i2c/chips/Kconfig linux-2.6.7-mm1/drivers/i2c/chips/Kconfig
@@ -22,6 +22,16 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called adm1021.
 
+config SENSORS_ADM1031
+	tristate "Analog Devices ADM1031 and compatibles"
+	depends on I2C && EXPERIMENTAL
+	select I2C_SENSOR
+	help
+	  If you say yes here you get support for Analog Devices ADM1031 
+	  and ADM1030 sensor chips.
+	  This driver can also be built as a module.  If so, the module
+	  will be called adm1031.
+
 config SENSORS_ASB100
 	tristate "Asus ASB100 Bach"
 	depends on I2C && EXPERIMENTAL
drivers/i2c/chips/Makefile +1 -0
diff -urN linux-2.6.7-mm1-orig/drivers/i2c/chips/Makefile linux-2.6.7-mm1/drivers/i2c/chips/Makefile
@@ -8,6 +8,7 @@
 obj-$(CONFIG_SENSORS_W83781D)	+= w83781d.o
 
 obj-$(CONFIG_SENSORS_ADM1021)	+= adm1021.o
+obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
 obj-$(CONFIG_SENSORS_EEPROM)	+= eeprom.o
 obj-$(CONFIG_SENSORS_FSCHER)	+= fscher.o
drivers/i2c/chips/adm1031.c +965 -0

965 lines not on this page · raw message

author
#1 lore raw

PATCH 1/1 ADM1030 and Co sensors chips support for 2.6.7-mm1

Quoting "Mark M. Hoffman" <[email protected]>:

Quoted 6 lines

Hi Alexandre:

Just a couple nits to pick...

* Alexandre d'Alton <[email protected]> [2004-06-22 19:52:09 +0200]:

Quoted 7 lines

(liberal snips)

drivers/i2c/chips/adm1031.c line 158

Quoted 3 lines
drivers/i2c/chips/adm1031.c +3 -0
+#define AUTO_TEMP_MAX_FROM_REG(reg)		\
+	AUTO_TEMP_RANGE_FROM_REG(reg) +		\
+	AUTO_TEMP_MIN_FROM_REG(reg)

Should add outer parens there.

I agree on that point. I will have to re-send the patch updated.

Quoted 4 lines

Review

Quoted 3 lines
(quoted diff) +2 -0
+#define fan_auto_channel_offset(offset)						\
+static ssize_t show_fan_auto_channel_##offset (struct device *dev, char

*buf) \

Quoted 14 lines

drivers/i2c/chips/adm1031.c line 301

Quoted 11 lines
drivers/i2c/chips/adm1031.c +11 -0
+{										\
+	return show_fan_auto_channel(dev, buf, 0x##offset - 1);			\
+}										\
+static ssize_t set_fan_auto_channel_##offset (struct device *dev,		\
+	const char *buf, size_t count)						\
+{										\
+	return set_fan_auto_channel(dev, buf, count, 0x##offset - 1);		\
+}										\
+static DEVICE_ATTR(auto_fan##offset##_channel, S_IRUGO | S_IWUSR,		\
+		   show_fan_auto_channel_##offset,				\
+		   set_fan_auto_channel_##offset)

A recent driver core patch *necessitates* the trailing semicolon for DEVICE_ATTR().

In fact, I already put the semicolons on the iner calls to DEVICE_ATTR(), but as when I "instanciate"
the macros I put a semicolon, I thought that it was unnecessary on the last one. If you confirm that I have to add them, I'll be glad to do it.

Thanks,

Alex.

Quoted 22 lines

drivers/i2c/chips/adm1031.c line 384

Quoted 7 lines
drivers/i2c/chips/adm1031.c +6 -0
+static DEVICE_ATTR(auto_temp##offset##_off, S_IRUGO,				\
+		   show_auto_temp_##offset##_off, NULL);			\
+static DEVICE_ATTR(auto_temp##offset##_min, S_IRUGO | S_IWUSR,			\
+		   show_auto_temp_##offset##_min, set_auto_temp_##offset##_min);\
+static DEVICE_ATTR(auto_temp##offset##_max, S_IRUGO | S_IWUSR,			\
+		   show_auto_temp_##offset##_max, set_auto_temp_##offset##_max)

Likewise for the last of these three.

drivers/i2c/chips/adm1031.c line 431

Quoted 2 lines
drivers/i2c/chips/adm1031.c +2 -0
+static DEVICE_ATTR(fan##offset##_pwm, S_IRUGO | S_IWUSR,		\
+		   show_pwm_##offset, set_pwm_##offset)

And here... eh, there are more but you get the point.

Regards,

Signature · 2 lines
Mark M. Hoffman
[email protected]
Signature · 3 lines
-------------------------------------------------
Alexandre d'ALTON
                              [email protected]
author
#2 lore raw

PATCH 1/1 ADM1030 and Co sensors chips support for 2.6.7-mm1

In reply to Alexandre d'Alton

Greg, Mark,

Please find (attached because my mailer wraps lines.) an incremental patch that applies on top of the previous one (the one from the first message of the thread) and that adds parenthesis to the macro as pointed out by Mark.

Please apply,

Alex.

Quoting "Mark M. Hoffman" <[email protected]>:

Quoted 7 lines

Hello:

Quoted 3 lines

Quoting "Mark M. Hoffman" <[email protected]>:

Quoted 2 lines

A recent driver core patch *necessitates* the trailing semicolon for DEVICE_ATTR().

* Alexandre d'Alton <[email protected]> [2004-06-30 09:33:01 +0200]:

Quoted 9 lines
Quoted 1 line

In fact, I already put the semicolons on the iner calls to DEVICE_ATTR(),

but as

Quoted 2 lines

when I "instanciate"
the macros I put a semicolon, I thought that it was unnecessary on the last

one.

Quoted 1 line

If you confirm that I have to add them, I'll be glad to do it.

No, you're right. Sorry for the noise.

Mark : No problem :-) better to put only almost perfect stuff on Linux.

Quoted 7 lines

Regards,

Signature · 2 lines
Mark M. Hoffman
[email protected]

-------------- next part --------------
A non-text attachment was scrubbed...
Name: adm1031-mmh-incremental.diff
Type: text/x-patch
Size: 577 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20040701/0a09578a/adm1031-mmh-incremental.bin

#3 lore raw

PATCH 1/1 ADM1030 and Co sensors chips support for 2.6.7-mm1

In reply to Alexandre d'Alton

Hi Alexandre:

Just a couple nits to pick...

* Alexandre d'Alton <[email protected]> [2004-06-22 19:52:09 +0200]:

(liberal snips)

drivers/i2c/chips/adm1031.c line 158

Quoted 3 lines
drivers/i2c/chips/adm1031.c +3 -0
+#define AUTO_TEMP_MAX_FROM_REG(reg)		\
+	AUTO_TEMP_RANGE_FROM_REG(reg) +		\
+	AUTO_TEMP_MIN_FROM_REG(reg)

Should add outer parens there.

drivers/i2c/chips/adm1031.c line 301

Quoted 13 lines
drivers/i2c/chips/adm1031.c +13 -0
+#define fan_auto_channel_offset(offset)						\
+static ssize_t show_fan_auto_channel_##offset (struct device *dev, char *buf)	\
+{										\
+	return show_fan_auto_channel(dev, buf, 0x##offset - 1);			\
+}										\
+static ssize_t set_fan_auto_channel_##offset (struct device *dev,		\
+	const char *buf, size_t count)						\
+{										\
+	return set_fan_auto_channel(dev, buf, count, 0x##offset - 1);		\
+}										\
+static DEVICE_ATTR(auto_fan##offset##_channel, S_IRUGO | S_IWUSR,		\
+		   show_fan_auto_channel_##offset,				\
+		   set_fan_auto_channel_##offset)

A recent driver core patch *necessitates* the trailing semicolon for DEVICE_ATTR().

drivers/i2c/chips/adm1031.c line 384

Quoted 6 lines
drivers/i2c/chips/adm1031.c +6 -0
+static DEVICE_ATTR(auto_temp##offset##_off, S_IRUGO,				\
+		   show_auto_temp_##offset##_off, NULL);			\
+static DEVICE_ATTR(auto_temp##offset##_min, S_IRUGO | S_IWUSR,			\
+		   show_auto_temp_##offset##_min, set_auto_temp_##offset##_min);\
+static DEVICE_ATTR(auto_temp##offset##_max, S_IRUGO | S_IWUSR,			\
+		   show_auto_temp_##offset##_max, set_auto_temp_##offset##_max)

Likewise for the last of these three.

drivers/i2c/chips/adm1031.c line 431

Quoted 2 lines
drivers/i2c/chips/adm1031.c +2 -0
+static DEVICE_ATTR(fan##offset##_pwm, S_IRUGO | S_IWUSR,		\
+		   show_pwm_##offset, set_pwm_##offset)

And here... eh, there are more but you get the point.

Regards,

Signature · 2 lines
Mark M. Hoffman
[email protected]
#4 lore raw

PATCH 1/1 ADM1030 and Co sensors chips support for 2.6.7-mm1

In reply to Alexandre d'Alton
Quoted 2 lines

I agree on that point. I will have to re-send the patch updated.

I think you should rather send an additional patch with just the changes, to be applied after your original patch. It's far easier for the reviewers. If you send a brand new patch they have to review the whole thing again, or do a diff against your patches, which is usually hard to read.

Greg especially cannot "just trust you" and assume that the new patch is really the old one (which MMH and I reviewed) with the few requested changes. You could be an evil guy! ;)

Thanks,
Jean Delvare

#5 lore raw

PATCH 1/1 ADM1030 and Co sensors chips support for 2.6.7-mm1

In reply to Alexandre d'Alton

Hello:

Quoted 3 lines

Quoting "Mark M. Hoffman" <[email protected]>:

Quoted 2 lines

A recent driver core patch *necessitates* the trailing semicolon for DEVICE_ATTR().

Quoted 4 lines · Alexandre d'Alton

In fact, I already put the semicolons on the iner calls to DEVICE_ATTR(), but as when I "instanciate"
the macros I put a semicolon, I thought that it was unnecessary on the last one. If you confirm that I have to add them, I'll be glad to do it.

No, you're right. Sorry for the noise.

Regards,

Signature · 2 lines
Mark M. Hoffman
[email protected]
Applied
#6 lore raw

PATCH 1/1 ADM1030 and Co sensors chips support for 2.6.7-mm1

In reply to Alexandre d'Alton
Quoted 7 lines · Alexandre d'Alton

Hi Greg,

Here is a patch against linux-2.6.7-mm1 kernel that adds support for adm1030 and adm1031 sensors chips.

It has been reviewed several times by Jean Delvare, and he thinks it is ready for inclusion into 2.6 kernel. So here it is.

Applied, thanks.

greg k-h

Applied
#7 lore raw

PATCH 1/1 ADM1030 and Co sensors chips support for 2.6.7-mm1

In reply to Alexandre d'Alton
Quoted 7 lines · Alexandre d'Alton

Greg, Mark,

Please find (attached because my mailer wraps lines.) an incremental patch that applies on top of the previous one (the one from the first message of the thread) and that adds parenthesis to the macro as pointed out by Mark.

Please apply,

Applied, thanks.

greg k-h