On Mon, Jul 02, 2018 at 02:33:50PM +0000, Vadim Pasternak wrote: > > > > Also, the usage of "round" is "100 + (r)". A value of 0 is no problem. > > Hi Guenter, > > This is 15000000 / ((val) * (d) / 100 + (r))). > Value is reading from the register ( >=0) , in default case should be: > regval * 1132 / 100 + 500; > > > A value of -100 is problematic. Which makes me wonder - what is the point of > > the offset ? And why "round" ? This looks like a fractional divider to me, where > > d(real) = d / (100 + (r)) > > It might be useful to explain that somewhere, and use a better variable name > > than 'round' to describe the fraction. > > I will change round to fraction and will add a comment before macros > MLXREG_FAN_GET_RPM. Something like below: > /* > * FAN datasheet defines the formula for RPM calculations as RPM = 15/t-high. > * The logic in a programmable device measures the time t-high by sampling the > * tachometer every t-sample (with the default value 11.32 uS) and increment > * a counter (N) as long as the pulse has not change: > * RPM = 15 / (t-sample * (K + Regval)), where: > * Regval: is the value read from the programmable device register; > * - 0xff - represents tachometer fault; > * - 0xfe - represents tachometer minimum value , which is 4444 RPM; > * - 0x00 - represents tachometer maximum value , which is 300000 RPM; > * K: is 44 and it represents the minimum allowed samples per pulse; > * F: is equal K * t-sample (44 * 11.32 = ~500) and it represents a minimum > * fraction in RPM calculation; > * N: is equal K + Regval; > * In order to calculate RPM from the register value the following formula is > * used: RPM = 15 / ((Regval * 11.32 + F) * 10^(-6)), which in the default > * case is modified to: > * RPM = 15000000 / ((Regval * 1132) / 100 + 500); > * - for Regval 0x00, RPM will be 15000000 / 500 = 30000; > * - for Regval 0xfe, RPM will be 15000000 / ((254 * 1132) / 100 + 500) = 4444; > * In common case the formula is modified to: > * RPM = 15000000 / ((Regval * divider) / 100 + fraction). > */ > > Would it be OK? > If F = K * t-sample, and K == 44, F also includes t-sample and is thus partially redundant. If K is a constant, you could write rpm = 15000000 / DIV_ROUND_CLOSEST(((regval) + 44) * (d), 100); If K is not constant, you could provide K as parameter. rpm = 15000000 / DIV_ROUND_CLOSEST(((regval) + (K)) * (d), 100); Using your examples: 15000000 / (((0 + 44) * 1132) / 100) = 30120 15000000 / (((254 + 44) * 1132) / 100) = 4447 You could also use rpm = 15000000 * 100 / (((regval) + (K)) * (d))); or rpm = DIV_ROUND_CLOSEST(15000000 * 100, ((regval) + (K)) * (d)); which would probably generate even more accurate results and at the same time simplify the equation. Again with your examples: rpm = 15000000 * 100 / ((0 + 44) * 1132) = 30115 rpm = 15000000 * 100 / ((254 + 44) * 1132) = 4446 Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
