Re: [Qemu-devel] [PATCH] i6300esb: correctly convert watchdog clock ticks into nanoseconds

[Laurent Vivier <lvivier@redhat.com>]

On 04/08/2015 12:13, Richard W.M. Jones wrote:
> On Mon, Aug 03, 2015 at 03:52:28PM +0200, Laurent Vivier wrote:
>> +    /* A 33 Mhz clock gives a 30 ns tick,
>> +     * convert timeout from ticks to ns
>>       */
>> -    timeout = muldiv64(get_ticks_per_sec(), timeout, 33000000);
>> +    timeout *= 30;
> 
> I see that you've just posted a v2 of this patch.  However here are
> the results of testing the above version.  I used the attached test
> script which automates things, mostly.
> 
> All times are in seconds.
> 
>   Requested timeout       Observed timeout
>          60                     58
>         120                    120
>         250                    249
>         270                    271
>         500                    501  [note 1]
>         520                    522
>        1010                   1016
>        1030                   1035
>        2046                   2058
>        2500  ioctl: WDIOC_SETTIMEOUT: error setting timeout: Invalid argument
>                               [note 2]

Thank you Rich.

> [note 1] I'm not worried about the timeout being off by a few seconds,
> as that could easily be caused by inaccuracies in the test framework.

The driver put 1024 in the counter for 1 second.

So for 520 seconds, we have ((520 * 1024) << 15) * 30 ns = 523 seconds
      2036 seconds          ((2046 * 1024) << 15) * 30 ns = 2059 seconds

So the emulation seems correct.

> [note 2] Maximum setting for i6300esb Linux driver is 2046, see
> linux.git/drivers/watchdog/i6300esb.c but note that the printed
> messages in the driver relating to the range of the timeout are not
> accurate.
> This patch looks good to me.  I will test the v2 patch shortly.

I have no preference of which patch to include in QEMU.

This one is nicer but request the same kind of modification in other
devices, the V2 is a trivial bug fix (currently timer overflow at 256
seconds). I just want to have the bug fixed...

Laurent