Hi Jean, The first version of this message was to large and was rejected, so i had to cut the quote a little bit. > Did pwmconfig work? I get it working now, i just have to set FCTEMPS and FCFANS correct in /etc/fancontrol now it work. The only problem is, that if i stop fancontrol, the program set pwm1_enable to 0, but after this the fan turns at 23rpm. This also happens if i set pwm1_enable to 0 by hand. > sensors output looks like: > mythtv at pc:~$ sensors F75373-* > F75373-i2c-1-2d > Adapter: SMBus nForce2 adapter at 5100 > : > CPU PWM: 31 (enable = 1) > > Did you try setting limits? I try to set limits in sensors.conf but i could see no change. > That's not correct, you should have "in0", "in1"... etc. here, not > "in0_input", "in1_input" etc? The fan and temperature symbols below are > correct. > Also, these compute lines don't make sense. The second formula is > supposed to be the reserve of the first formula, that's not the case > here. > Lastly, you really shouldn't have to divide the values by 1000. This > suggests that the magnitude wasn't set properly in libsensors (it > should be 3, I suggest it is 0). This issue is most probably related to > the incorrect symbol names. I saw values >800V so i thought i have to divide it by 1000 in sensors.conf. Magnitude is 3 and the symbol name is correct now, but the values are still over 800V. What's the reason? > This means that the driver isn't reporting the values in RPM as it > should. Usually no compute statement is needed for fans. That's correct, the value is not in RPM, in the document '2005111153128.pdf' you can download on the fintek page ( http://www.fintek.com.tw/eng/products.asp?BID=4&SID=5 )its written on page 8: "Determine the fan counter according to: Count = (1.5?10^6) / RPM " That means that the driver is reporting the count value, not the rpm. So i have to compute in sensors.conf: RPM = (1.5?10^6)/ Count > > +#define SENSORS_F75373_IN_FEATURES(nr) \ > > + { { SENSORS_F75373_IN(nr), "in" #nr "_input", \ > > + NOMAP, NOMAP, R }, \ > > + NOSYSCTL, VALUE(3), 3 }, \ > > OK, the problem is here: This should be"in" #nr, not "in" #nr > "_input". This will fix your magnitude problem. I changed it to "in" #nr, \ > + { { SENSORS_F75373_IN_MIN(nr), "in" #nr "_min", \ > + SENSORS_F75373_IN(nr), SENSORS_F75373_IN(nr), RW }, \ > + NOSYSCTL, VALUE(2), 3 }, \ > > VALUE(1) (although it won't make a difference for a Linux 2.6 only > driver. VALUE(2) changed to VALUE(1) > + { { SENSORS_F75373_IN_MAX(nr), "in" #nr "_max", \ > + SENSORS_F75373_IN(nr), SENSORS_F75373_IN(nr), RW }, > : > + NOSYSCTL, VALUE(1), 3 }, \ > + { { SENSORS_F75373_TEMP_HYST(nr), "temp" #nr "_hyst", \ > + SENSORS_F75373_TEMP(nr), NOMAP, R }, \ > + NOSYSCTL, VALUE(1), 0 } > SENSORS_F75373_TEMP(nr) instead of NOMAP, VALUE(2) instead of VALUE(1), > and magnitude 3 instead of 0. Is it really read-only? > > (Unless this isn't an hysteresis temperature at all - see below.) I changed it. It is not only read-only, i changed it to RW. > + > +#define SENSORS_F75373_FAN_FEATURES(nr) \ > + { { SENSORS_F75373_FAN(nr), "fan" #nr, \ > + NOMAP, NOMAP, R }, \ > + NOSYSCTL, VALUE(2), 0 }, \ > + { { SENSORS_F75373_FAN_MAX(nr), "fan" #nr "_max", \ > + SENSORS_F75373_FAN(nr), SENSORS_F75373_FAN(nr), RW }, \ > + NOSYSCTL, VALUE(1), 0 }, \ > > Strange, I've never seen a fan speed sensor with a high limit. What > does it correspond to exactly? Why don't you display this value in > "sensors"? I get the information out of the documentation on page 26: "When power on, the PWMOUT1 will output full duty cycle (FFh) to enable system FAN. After 10 seconds when detecting FANIN1 signal, assuming the fan has been fully turned on, the fan speed count detected will be recorded in the register.". Dunno if it should be removed? > + { { SENSORS_F75373_FAN_MIN(nr), "fan" #nr "_min", \ > + SENSORS_F75373_FAN(nr), NOMAP, R }, \ > + NOSYSCTL, VALUE(1), 0 } > > SENSORS_F75373_FAN(nr) instead of NOMAP. > > Is it really read-only? I changed it to SENSORS_F75373_FAN(nr), it is RW. But fanX_min is allways zero, so it lead to an error on calling 'sensors_get_feature(*name, SENSORS_F75373_FAN_MIN(i), &min)', i have to disable it, sensors output is now CPU Fan: 1866 RPM (min = 0 RPM) to CPU Fan: 1866 RPM > + > +#define SENSORS_F75373_PWM_FEATURES(nr) \ > + { { SENSORS_F75373_PWM(nr), "pwm" #nr, \ > + NOMAP, NOMAP, RW }, \ > + NOSYSCTL, VALUE(1), 0 }, \ > + { { SENSORS_F75373_PWM_ENABLE(nr), "pwm" #nr "_enable", \ > + SENSORS_F75373_PWM(nr), SENSORS_F75373_PWM(nr), RW }, \ > + NOSYSCTL, VALUE(3), 0 } > > In fact we don't include PWM stuff in libsensors. They are not sensors, > and are better handled using pwmconfig and fancontrol. I only copied it from SENSORS_DME1737_PWM_FEATURES, should i remove it? > + > +static sensors_chip_feature f75373_features[] = > : > +#define SENSORS_F75373_PREFIX "f75373" > + > +// voltage n from 0 to 3 > > No C++-style comments please. Use /* */. I used now /**/. > +#define SENSORS_F75373_IN(nr) (0x10 + (nr)) > +#define SENSORS_F75373_IN_MAX(nr) (0x20 + (nr) * 2 ) > +#define SENSORS_F75373_IN_MIN(nr) (0x21 + (nr) * 2 ) > > Coding style: no space before closing parenthesis. I changed it. > + > +// temp n from 0 to 1 > +#define SENSORS_F75373_TEMP(nr) (0x14 + (nr)) > : > +#define SENSORS_F75373_PWM_ENABLE(nr) (0x7D + (nr) * 0x10 ) > + > > I hope that you realize that these numbers do NOT have to match the > register numbers in the chip? These numbers are completely arbitrary. > So please make them as simple as possible. No :-), i changed it. > #endif /* def LIB_SENSORS_CHIPS_H */ > diff -urN lm-sensors_org/prog/sensors/chips.c lm-sensors/prog/sensors/chips.c > --- lm-sensors_org/prog/sensors/chips.c 2007-06-17 15:34:33.000000000 +0200 > +++ lm-sensors/prog/sensors/chips.c 2007-06-17 16:29:50.000000000 +0200 > @@ -6131,6 +6131,114 @@ > } > } > > +static void print_f75373_in(const sensors_chip_name *name, int i) > : > + printf("%+6.2f V (min = %+6.2f V, max = %+6.2f V) \n", > + cur, min, max); > > If you don't have any alarm flag to display, you can remove the spaces > before the newline. I changed it. > + } > + } else { > + printf("ERROR: Can't get in%d data!\n", i); > : > + !sensors_get_feature(*name, SENSORS_F75373_TEMP_HYST(i), &alarm) > + ) { > > Coding style: move this at the end of the previous line. I changed it. > + if (valid) { > + print_label(label, 10); > + print_temp_info(cur, max, 0, HYST , 0, 0); > + printf("%s\n", alarm ? "ALARM" : ""); > + } > > This doesn't make sense. What is SENSORS_F75373_TEMP_HYST exactly? Why > do you display it as an alarm? It was a mistake, i deleted the Alarm output, now it is only printf("\n"), there seems not to be a value for an alarm. temp1_max_hyst is always zero, i don't now what it is exactly and when it is not zero. > + } else { > + printf("ERROR: Can't get temp%d data!\n", i); > : > + &valid) && > + !sensors_get_feature(*name, SENSORS_F75373_FAN(i), &cur) && > + !sensors_get_feature(*name, SENSORS_F75373_FAN_MIN(i), &min) > + ){ > > Coding style: move this at the end of the previous line. I changed it. > + if (valid) { > + print_label(label, 10); > + printf("%4.0f RPM (min = %4.0f RPM)\n", > + cur, min); > : > + int i; > + > + for (i = 0; i < 4; i++) { > + print_f75373_in(name, i); > + } > + > + for (i = 1; i < 3; i++) { > > I'd suggest writing it i <= 2, to make it more visible that there are 2 > temperature inputs, not 3. Same below for fans. I changed it. > + print_f75373_temp(name, i); > + } > + > : > @@ -424,6 +424,7 @@ > { "coretemp", print_coretemp }, > { "dme1737", print_dme1737 }, > { "applesmc", print_applesmc }, > + { "F75373", print_f75373 }, > > Should be "f75373", no capital. I'm even surprised it worked with the > capital. If i use f75373 i get following output: F75373-i2c-1-2d Adapter: SMBus nForce2 adapter at 5100 VCC: 1720.00 (in0) in0_min: 0.00 (in0_min) in0_max: 0.00 (in0_max) V1: 1336.00 (in1) in1_min: 0.00 (in1_min) in1_max: 0.00 (in1_max) V2: 1008.00 (in2) in2_min: 0.00 (in2_min) in2_max: 0.00 (in2_max) V3: 792.00 (in3) in3_min: 0.00 (in3_min) in3_max: 0.00 (in3_max) CPU Temp: 49.00 (temp1) temp1_max: 0.00 (temp1_max) temp1_hyst: 0.00 (temp1_hyst) Sys Temp: 46.00 (temp2) temp2_max: 0.00 (temp2_max) temp2_hyst: 0.00 (temp2_hyst) CPU Fan: 1787.84 (fan1) fan1_max: 5681.82 (fan1_max) ERROR: Can't get feature `fan1_min' data! CPU PWM: 31.00 (pwm1) What's the reason? Did I still have to use "F75373"? > Please update your patch and resend it. > > Also, what about the F75375 support? As far as I know the f75375s > driver > supports both chips, so it'd be nice to have user-space support for > both. Also, it might be less confusing to use F75375 for all the > libsensors defines, rather than F75373, so that they match the driver > name. I changed every define/function from F75373 to F75375, i added also: #define SENSORS_F75373_PREFIX "f75373" #define SENSORS_F75375_PREFIX "f75375" and { SENSORS_F75375_PREFIX, f75375_features }, { SENSORS_F75373_PREFIX, f75375_features }, and { "F75375", print_f75375 }, { "F75373", print_f75375 }, i hope that this is ok. > > Thanks, > -- > Jean Delvare Thank you for your review, christian. --------------------------------- Was ist Gl?ck? Schlafen Fische ?berhaupt? Die Antworten gibt?s auf Yahoo! Clever. -------------- next part -------------- An HTML attachment was scrubbed... URL: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20070625/ed421dcc/attachment.html -------------- next part -------------- A non-text attachment was scrubbed... Name: fintek_75373s_support_v2.patch Type: application/octet-stream Size: 9283 bytes Desc: 3093308178-fintek_75373s_support_v2.patch Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20070625/ed421dcc/attachment.obj
