Hi Hans, Better late than never... On Mon, 09 Apr 2007 17:20:52 +0200, Hans de Goede wrote: > I've committed everything my students had written + some fixes done by me after > some initial testing to the 3.0.0 sensors branch. > > I claim in no way that this code is perfect, I did things this way to properly > document / atrribute who wrote what, and to give us a starting point as I think > the code in general is ok. It needs testing and cleanups, but its not a bad > starting point perse. > > So please test this: > 1) checkout the 3.0.0 branch: > svn checkout http://lm-sensors.org/svn/lm-sensors/branches/lm-sensors-3.0.0 > 2) comment the entries for your chips int he table at the end of lib/chips.c > and in the table near the end of progs/sensors/main.c > 3) compile and install as usual > 4) run sensors and let me/us know how it works I've tried that with my ADM1032 today, and this uncovered two problems. First problem is the way the new library (and sensors) code handles temperature hysteresis. It handles it as a "standard" temperature limit, which it isn't. An hysteresis value is more like a property of other temperature limit. So having a type named SENSORS_FEATURE_TEMP_HYST doesn't make sense. We need, instead, SENSORS_FEATURE_TEMP_MAX_HYST, and SENSORS_FEATURE_TEMP_CRIT_HYST. Second problem is that the new code doesn't fill the magnitude when creating the dynamic feature tables. It just happens to work in most cases thanks to the general symbol name translation code, which enforces standard magnitudes as a side effect. This doesn't work when we had a dedicated symbol name translation rule for a given chip though (which was the case for the hysteresis value of the lm90/adm1032) because this rule is part of the chip-specific features table which we drop when switching to the generic code. And the general symbol name translation is bound to be deleted at some point too, so we shouldn't rely on it. Here is the patch I have come up with, which fixes both problems and makes the generic code work for my ADM1032. Can you please review it and confirm that it doesn't break anything on your side? Thanks. Index: lib/sensors.h =================================================================== --- lib/sensors.h (r?vision 4405) +++ lib/sensors.h (copie de travail) @@ -166,14 +166,16 @@ SENSORS_FEATURE_FAN_DIV, SENSORS_FEATURE_TEMP = 0x200, - SENSORS_FEATURE_TEMP_HYST, SENSORS_FEATURE_TEMP_OVER, SENSORS_FEATURE_TEMP_MAX, + SENSORS_FEATURE_TEMP_MAX_HYST, SENSORS_FEATURE_TEMP_MIN, + SENSORS_FEATURE_TEMP_MIN_HYST, SENSORS_FEATURE_TEMP_HIGH, SENSORS_FEATURE_TEMP_LOW, SENSORS_FEATURE_TEMP_LIM, SENSORS_FEATURE_TEMP_CRIT, + SENSORS_FEATURE_TEMP_CRIT_HYST, SENSORS_FEATURE_TEMP_ALARM = 0x210, SENSORS_FEATURE_TEMP_FAULT, SENSORS_FEATURE_TEMP_SENS, Index: lib/access.c =================================================================== --- lib/access.c (r?vision 4405) +++ lib/access.c (copie de travail) @@ -515,12 +515,14 @@ }; static struct feature_type_match temp_matches[] = { - { "hyst", SENSORS_FEATURE_TEMP_HYST }, { "over", SENSORS_FEATURE_TEMP_OVER }, { "max", SENSORS_FEATURE_TEMP_MAX }, + { "max_hyst", SENSORS_FEATURE_TEMP_MAX_HYST }, { "min", SENSORS_FEATURE_TEMP_MIN }, + { "min_hyst", SENSORS_FEATURE_TEMP_MIN_HYST }, { "low", SENSORS_FEATURE_TEMP_LOW }, { "crit", SENSORS_FEATURE_TEMP_CRIT }, + { "crit_hyst", SENSORS_FEATURE_TEMP_CRIT_HYST }, { "alarm", SENSORS_FEATURE_TEMP_ALARM }, { "fault", SENSORS_FEATURE_TEMP_FAULT }, { "type", SENSORS_FEATURE_TEMP_SENS }, Index: lib/sysfs.c =================================================================== --- lib/sysfs.c (r?vision 4405) +++ lib/sysfs.c (copie de travail) @@ -37,6 +37,29 @@ #define MAX_SENSORS_PER_TYPE 16 +static +int get_type_scaling(int type) +{ + switch (type & 0xFF10) { + case SENSORS_FEATURE_IN: + case SENSORS_FEATURE_TEMP: + return 3; + case SENSORS_FEATURE_FAN: + return 0; + } + + switch (type) { + case SENSORS_FEATURE_VID: + return 3; + case SENSORS_FEATURE_VRM: + return 1; + default: + return 0; + } + + return 0; +} + static sensors_chip_features sensors_read_dynamic_chip(struct sysfs_device *sysdir) { @@ -149,6 +172,8 @@ SENSORS_MODE_R : (attr->method & SYSFS_METHOD_STORE) ? SENSORS_MODE_W : SENSORS_MODE_NO_RW; + feature.scaling = get_type_scaling(type); + features[i] = feature; fnum++; } Index: prog/sensors/chips_generic.c =================================================================== --- prog/sensors/chips_generic.c (r?vision 4405) +++ prog/sensors/chips_generic.c (copie de travail) @@ -144,27 +144,16 @@ } else { type = MINMAX; } + } else if (TEMP_FEATURE(SENSORS_FEATURE_TEMP_MAX_HYST)) { + min = TEMP_FEATURE_VAL(SENSORS_FEATURE_TEMP_MAX_HYST); + type = HYST; } else if (TEMP_FEATURE(SENSORS_FEATURE_TEMP_CRIT)) { min = TEMP_FEATURE_VAL(SENSORS_FEATURE_TEMP_CRIT); type = CRIT; - } else if (TEMP_FEATURE(SENSORS_FEATURE_TEMP_HYST)) { - min = TEMP_FEATURE_VAL(SENSORS_FEATURE_TEMP_HYST); - type = HYST; } else { min = 0; type = MAXONLY; } - } else if (TEMP_FEATURE(SENSORS_FEATURE_TEMP_HYST)) { - min = TEMP_FEATURE_VAL(SENSORS_FEATURE_TEMP_HYST); - - if (TEMP_FEATURE(SENSORS_FEATURE_TEMP_OVER)) { - max = TEMP_FEATURE_VAL(SENSORS_FEATURE_TEMP_OVER); - type = HYST; - } else { - max = min; - max = 0; - type = HYSTONLY; - } } else if (TEMP_FEATURE(SENSORS_FEATURE_TEMP_LOW)) { min = TEMP_FEATURE_VAL(SENSORS_FEATURE_TEMP_LOW); @@ -214,18 +203,14 @@ } printf("\n"); - if (type == MINMAX && TEMP_FEATURE(SENSORS_FEATURE_TEMP_CRIT)) + if (type != CRIT && TEMP_FEATURE(SENSORS_FEATURE_TEMP_CRIT)) { const sensors_feature_data *subfeature; max = TEMP_FEATURE_VAL(SENSORS_FEATURE_TEMP_CRIT); - if (TEMP_FEATURE(SENSORS_FEATURE_TEMP_HYST)) { - min = TEMP_FEATURE_VAL(SENSORS_FEATURE_TEMP_HYST); + if (TEMP_FEATURE(SENSORS_FEATURE_TEMP_CRIT_HYST)) { + min = TEMP_FEATURE_VAL(SENSORS_FEATURE_TEMP_CRIT_HYST); type = HYSTONLY; - } else if (TEMP_FEATURE(SENSORS_FEATURE_TEMP_MAX) && - !TEMP_FEATURE(SENSORS_FEATURE_TEMP_MIN)) { - min = TEMP_FEATURE_VAL(SENSORS_FEATURE_TEMP_MAX); - type = HYSTONLY; } else { type = SINGLE; min = 0.0; @@ -239,7 +224,7 @@ if (valid) { print_label(label, label_size); free(label); - print_temp_info_real(max, min, 0, 0.0, type, 1, 0); + print_temp_info_real(max, min, 0, 0.0, type, 1, 1); printf("\n"); } } -- Jean Delvare
