I2C adm9240 driver, second round

Greetings,

This is my completion of porting adm9240 driver that Michiel Rook started. Being a new driver, it contains some new features as there are no user-space scripts to worry about.

Problems: libsensors can't read temperature, I do not know why.

Removed:
vrm r/w accessor
fan_div r/w accessor
unused macros

Added:
chassis intrusion latch clear write-only accessor: echo 1 > ci_clear automatic fan clock divider control, documented in a separate post analog output
generalised scaling macros for integer scaling with rounding

Changed:
measurement cycle no longer reads entire chip, just the 12 measurement values, controls and limits are read on an as-needed basis -- I've noticed an improvement as the chip reads 12 rather 33 single byte values, previous to this change I could notice measurement updating delay on text terminal.

decrease measurement lockout from 2 to 3/2 seconds

vid report uses external vrm for scaling and does not store copy of vrm.

Documentation, further information:

http://scatter.mine.nu/lmsensors/adm9240/

Debugged on 2.6.11.7, patch against 2.6.12-rc2-mm3 compile tested (only an adjustment to drivers/i2c/chips/Makefile required)

Cheers,
Grant.

diff -X dontdiff-osdl -Nru linux-2.6.12-rc2-mm3/drivers/i2c/chips/Kconfig linux-2.6.12-rc2-mm3c/drivers/i2c/chips/Kconfig
@@ -51,6 +51,16 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called adm1031.
 
+config SENSORS_ADM9240
+	tristate "Analog Devices ADM9240 and compatibles"
+	depends on I2C && EXPERIMENTAL
+	select I2C_SENSOR
+	help
+	  If you say yes here you get support for Analog Devices ADM9240,
+	  Dallas DS1780 and National Semiconductor LM81 sensor chips. 
+	  This driver can also be built as a module.  If so, the module
+	  will be called adm9240.
+
 config SENSORS_ASB100
 	tristate "Asus ASB100 Bach"
 	depends on I2C && EXPERIMENTAL

diff -X dontdiff-osdl -Nru linux-2.6.12-rc2-mm3/drivers/i2c/chips/Makefile linux-2.6.12-rc2-mm3c/drivers/i2c/chips/Makefile
@@ -11,6 +11,7 @@
 obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025.o
 obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
+obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
 obj-$(CONFIG_SENSORS_DS1337)	+= ds1337.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
 obj-$(CONFIG_SENSORS_EEPROM)	+= eeprom.o

Hi Grant,

You named the hysteresis file temp1_hyst, while libsensors expects temp1_max_hyst (and is right at that).

Sounds good to me.

Sounds good too.

Caching register values forever is a bad idea. Sometimes the BIOS or something will access the chip in your back, and if you never refresh the register values you'll never find out.

It was underlined before on this mailing-list that caching belang to the application more than the driver. We do caching for technical reasons (not to stop the chip from monitoring).

If you want to speed up the registers refresh, I would invite you to take a look at the lm85 driver. It has different cache lifetime for measurements and limits. This is still discussable but at least doesn't cache any register value forever.

Are you sure that all three supported chips can refresh the measurement in less than 2 seconds? I think I remember that one of them is slower, wich explaied the 2 seconds.

I am not sure I understand what you mean by "external vrm". The VRM value is guessed from the CPU model by a kernel helper, so it's by definition internal.

Not caching the vrm value is a bad idea, as the helper is not trivial. The overhead of having an additional u8 to store the vrm version is definitely overweighted by the speed up.

More comments on the code itself (possibly) later.

Thanks,
--
Jean Delvare

Updated adm9240 driver
```````````````````````
Fixed
local vrm copy put back in
force default interrupt mode to get temperature alarm status slow update for sensor settings (instead of update on access) automatic fan clock divider now works properly :) report out of range fan_min as zero

sensors now reads temperature (temp1_max_hyst had wrong name)

Driver development is on 2.6.11.7, then patch applied and compile tested on 2.6.12-rc2-mm3.

Documentation, further information:

http://scatter.mine.nu/lmsensors/adm9240/

Comments welcome. Documentation to follow...

Cheers,
Grant.

diff -Nru -X dontdiff-osdl linux-2.6.12-rc2-mm3/drivers/i2c/chips/Kconfig linux-2.6.12-rc2-mm3d/drivers/i2c/chips/Kconfig
@@ -51,6 +51,16 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called adm1031.
 
+config SENSORS_ADM9240
+	tristate "Analog Devices ADM9240 and compatibles"
+	depends on I2C && EXPERIMENTAL
+	select I2C_SENSOR
+	help
+	  If you say yes here you get support for Analog Devices ADM9240,
+	  Dallas DS1780 and National Semiconductor LM81 sensor chips. 
+	  This driver can also be built as a module.  If so, the module
+	  will be called adm9240.
+
 config SENSORS_ASB100
 	tristate "Asus ASB100 Bach"
 	depends on I2C && EXPERIMENTAL

diff -Nru -X dontdiff-osdl linux-2.6.12-rc2-mm3/drivers/i2c/chips/Makefile linux-2.6.12-rc2-mm3d/drivers/i2c/chips/Makefile
@@ -11,6 +11,7 @@
 obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025.o
 obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
+obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
 obj-$(CONFIG_SENSORS_DS1337)	+= ds1337.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
 obj-$(CONFIG_SENSORS_EEPROM)	+= eeprom.o

Hi Khali

There was no temperature alarm status in comparator mode, so I tried 'default interrupt' mode and it worked. Actual interrupt pin drive is disabled, unless I find the chip already setup, in which case I still turn on default interrupt mode as it is the only way to get and alarm state. This also enables temp1_max_hyst, alarm on at temp1_max, off at temp1_max_hyst.

intr_mask does not affect alarm state, I put the read/write accessor in and took it out again as useless, thought it may have been set by BIOS, stopping temp alarm.

You didn't like the idea of not reading chip settings occasionally, I call it slow update, didn't go see what other driver called it.

You asked for it, you got it :)

Two types of overange:

fan is slow, and fan_min is set = alarm state, even when fan is too slow to be measured.

fan_min set too low = fan speed alarm disabled, auto fan clock divider adjustment enabled.

Can report zero for both types as alarm state always valid, that seems correct behaviour to me. I needed the state information before thinking of instrumenting the driver instead. :-/

Since then I've added read-only fan_div accessor to keep sensors 2.9.1 happy, auto fan clock adjust either direction when fan_min alarm disabled. (Forgot I'd patched 2.9.0 to not look at fan_div)

On fresh start (chip measurement not running when driver loaded) I set all chip limits to extremes to disable all alarms. The chip is not initialised by BIOS on my hardware.

Cheers,
Grant.

Hi Grant,

Good.

Might cause problem. Have you checked whether this mode change affects any (physical) output of the chip? If it does, then changing the mode that might have been set up by the BIOS is unwise, as you have no idea what is wired to that output.

What do you mean?

Good.

Where "out of range" means a register value of 0, or am I missing something?

I think that reporting 0 in this case is the correct thing to do, especially if the chip happens to disable fan alarms when the register limit is set to 0. Is it the case of the ADM9240?

OK.

Thanks,
--
Jean Delvare