Hi Rudolf,
> I'm attaching the userspace part this should work applying on the top of the
> latest version of lm-sensors.
My comments:
> + { SENSORS_W83627EHF_IN0_ALARM, "in0_alarm", NOMAP, NOMAP,
> + R, NOSYSCTL, VALUE(1), 0 },
Now that we have individual alarm files, it would make sense to express
their mapping with the corresponding input. That is, the first "NOMAP"
above should be "SENSORS_W83627EHF_IN0".
> +#define SENSORS_W83627EHF_IN0 1 /* R */
> (...)
> +#define SENSORS_W83627EHF_IN9_MAX 30 /* RW */
Please align the values with tabs, like I did for the rest of the
W83627EHF defines.
> + double cur, min, div, max, alarm;
If you want to give function scope to these variables, please do it
properly (i.e. also use them for temperature.)
> + if (!sensors_get_label_and_valid(*name,SENSORS_W83627EHF_IN0+i,&label,&valid) &&
> + !sensors_get_feature(*name,SENSORS_W83627EHF_IN0+i,&cur) &&
> + !sensors_get_feature(*name,SENSORS_W83627EHF_IN0_MIN+i,&min) &&
> + !sensors_get_feature(*name,SENSORS_W83627EHF_IN0_MAX+i,&max) &&
> + !sensors_get_feature(*name,SENSORS_W83627EHF_IN0_ALARM+i,&alarm)) {
> + if (valid) {
> + print_label(label,10);
> + printf("%+6.2f V (min = %+6.2f V, max = %+6.2f V) %s\n",
> + cur,min,max,alarm ? "ALARM" : "");
> + }
> + } else
> + printf("ERROR: Can't get IN%d data!\n",i+1);
> + free(label);
> + }
Coding style, please! I know that sensors' coding style is quite bad,
but let's not make it worse. Just follow the coding style I used for
the rest of the function, so that we have some kind of local
homogeneity.
> - double cur, min, div;
> +
No blank line here please.
> # Winbond W83627EHF configuration originally contributed by Leon Moonen
> # This is for an Asus P5P800.
> chip "w83627ehf-*"
This will need some rewording as you have a different board so the
voltage part may not match the P5P800 at all.
> + label in0 "VCore"
> + label in1 "VIN0"
> + label in2 "AVCC"
> + label in3 "3VCC"
> + label in4 "VIN1"
> + label in5 "VIN2"
> + label in6 "VIN3"
> + label in7 "VSB"
> + label in8 "VBAT"
> + label in9 "VIN4"
The VIN[0-4] labels are not helping much, and may in fact add to
confusion, as they don't match our in0-9 numbering.
> +# +12V is in1 and +5V is in6 as recommended by datasheet
> + compute in1 @*(1+(56/10)), @/(1+(56/10))
> + compute in6 @*(1+(22/10)), @/(1+(22/10))
Just label them that way then.
> - set fan1_min 1200
> - set fan2_min 1700
> + set fan1_min 1200
> + set fan2_min 1700
No whitespace changes please.
> + label temp3 "AUX Temp"
s/AUX/Aux/ for consistency.
> - set temp1_over 45
> - set temp1_hyst 40
> - set temp2_over 45
> - set temp2_hyst 40
> +# set temp1_over 45
> +# set temp1_hyst 40
> +# set temp2_over 45
> +# set temp2_hyst 40
Yes you're right, we should preserve the BIOS settings by default.
Thanks,
--
Jean Delvare
