Hi Khali,
On Sat, 16 Apr 2005 23:00:13 +0200, Jean Delvare <khali at linux-fr.org> wrote:
>
>Good catch. I thought that the "if" clause was handling that case, but
>there is a small window (42 to 54 RPM or so) which will go through and
>actually result in an invalid divider (256), you're correct.
It is a very small window at low speed end, took me ages to work it
out and find the special case when I lifted your code :)
>I also noticed that the "if" clause would fail to catch 41 RPM as it
>should, so I fixed it by using a less error-prone test.
>
>I now have:
>
> if (!val) {
> /* No min limit, alarm disabled */
> data->fan_min[nr] = 255;
> new_div = data->fan_div[nr]; /* No change */
> } else if ((reg = 1350000U / val) >= 128 * 255) {
> /* Speed below this value cannot possibly be represented,
> even with the highest divider (128) */
> data->fan_min[nr] = 255;
> new_div = 7; /* 128 == (1 << 7) */
> } else {
> /* Automatically pick the best divider, i.e. the one such
> that the min limit will correspond to a register value
> in the 96..192 range */
> new_div = 0;
> while (reg > 192 && new_div < 7) {
> reg >>= 1;
> new_div++;
> }
> data->fan_min[nr] = reg;
> }
>
>Looks OK now?
Yes, but is the (!val) test needed? I'd merge it with next test
like so:
if (val < 1350000U / (128 * 254)) {
as same end result is to store 255 to fan_min.
Cheers,
Grant.
