On Sun, Dec 19, 2010 at 2:28 PM, Andreas Färber <andreas.faer...@web.de> wrote: > Am 19.12.2010 um 15:16 schrieb Richard W.M. Jones: > >> On Sun, Dec 19, 2010 at 01:51:22PM +0100, Andreas Färber wrote: >>> >>> Am 18.12.2010 um 17:47 schrieb Richard W.M. Jones: >>> >>>> On Sat, Dec 18, 2010 at 05:25:26PM +0100, Andreas Färber wrote: >>>>> >>>>> softfloat.h's int64 type has least-width semantics, >>>>> but this doesn't seem intended here, so use plain int64_t. >>>>> >>>>> v3: >>>>> * Split off. >>>>> >>>>> Cc: Richard W.M. Jones <rjo...@redhat.com> >>>>> Signed-off-by: Andreas Färber <andreas.faer...@web.de> >>>>> --- >>>>> hw/wdt_ib700.c | 2 +- >>>>> 1 files changed, 1 insertions(+), 1 deletions(-) >>>>> >>>>> diff --git a/hw/wdt_ib700.c b/hw/wdt_ib700.c >>>>> index b6235eb..1248464 100644 >>>>> --- a/hw/wdt_ib700.c >>>>> +++ b/hw/wdt_ib700.c >>>>> @@ -53,7 +53,7 @@ static void ib700_write_enable_reg(void *vp, >>>>> uint32_t addr, uint32_t data) >>>>> 30, 28, 26, 24, 22, 20, 18, 16, >>>>> 14, 12, 10, 8, 6, 4, 2, 0 >>>>> }; >>>>> - int64 timeout; >>>>> + int64_t timeout; >>>>> >>>>> ib700_debug("addr = %x, data = %x\n", addr, data); >>>> >>>> The use of int64(_t) was just so that the timeout calculation in the >>>> next two lines would not overflow: >>>> >>>> timeout = (int64_t) time_map[data & 0xF] * get_ticks_per_sec(); >>>> qemu_mod_timer(s->timer, qemu_get_clock (vm_clock) + timeout); >>>> >>>> and from you say it does seem like it was a mistake to use int64 >>>> instead of int64_t. >>> >>> int64_t should be the right choice then. >>> >>>> ACK. >>>> >>>> In more general terms, am I doing the timeout correctly in this code? >>> >>> Being unfamiliar with both the timer code and this device, hard to >>> say for me. >>> You're taking the lower nibble of uint32_t data and indexing >>> time_map[] with it, which contains 16 elements, so okay, upcast it >>> to 64-bit and multiply it to ticks. Assuming that vm_clock works in >>> ticks, adding the calculated timeout for the next expiry technically >>> looks good. Except for the extra space. ;) >>> >>> Care to provide a formal Reviewed-by or Acked-by? I'd respin it for >>> Blue with updated description. >> >> Is it enough just to write this: >> >> Reviewed-by: Richard W.M. Jones <rjo...@redhat.com> >> Acked-by: Richard W.M. Jones <rjo...@redhat.com> >> >> or do you want me to send the updated patch with this added? > > Thanks, that's sufficient! I think of Acked-by as a superset of Reviewed-by > so I'll go with the former.
No, Acked-by tag does not mean much but Reviewed-by carries a lot of weight: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches;h=689e2371095cc5dfea9927120009341f369159aa;hb=HEAD#l423