[lm-sensors] [PATCH v2 3/3] hwmon: (lm90) Add support for update_interval sysfs attribute
khali at linux-fr.org
Thu Oct 7 09:06:03 CEST 2010
On Wed, 6 Oct 2010 10:33:06 -0700, Guenter Roeck wrote:
> Hi Jean,
> On Wed, Oct 06, 2010 at 12:07:09PM -0400, Jean Delvare wrote:
> > Note that your algorithm always rounds interval down, even when the
> > closest integer would be up. Proper rounding would need a different
> > algorithm to avoid accumulating rounding errors, I think.
> > As it stands, requesting a 21 ms update interval will lead to a 31.250
> I think you mean 23 ms, not 21 ms. 21 will report as 15 (you almost got me there).
Yes, sorry, I meant 23. Not sure how I managed to mistype it. I didn't
mean to trick you, it's a genuine typo.
> > ms update interval (register value 0x9), reported as 31 ms through
> > sysfs, while it should preferably lead to a 15.625 ms update interval
> > (register value 0xa), currently reported as 15 ms but ideally reported
> > as 16 ms: this is a 7.375 ms error instead of a 8.250 ms error. You may
> > decide this is unimportant though, I leave it up to you.
> I figured this is really a corner case (it really only applies to the 23 ms setting),
> and yields less than 1ms error. Everything else I came up with seemed to be too complicated.
> I could change the update_interval calculation to
> update_interval = (LM90_MAX_CONVRATE_MS + ((1 << i) >> 1)) >> i;
> That would take care of the 15ms vs. 16ms reporting error, and round 62.5 up to 63.
> Not sure if it is worth it, though. Seems to be a bit complicated.
I see you have found a different solution meanwhile, but for
completeness... The above can be simplified to:
update_interval = DIV_ROUND_CLOSEST(LM90_MAX_CONVRATE_MS, 1 << i);
It might be slower, but I'm not even sure, and it doesn't actually
matter, as you do it only once.
Also note that there is no point in using ">> 1" instead of "/ 2" and
"<< 1" instead of "* 2", the compiler is smart and will replace
multiplications and divisions by bit-shift each time this is possible.
More information about the lm-sensors