On 10.06.2013, at 19:20, Scott Wood wrote: > On 06/10/2013 09:26:18 AM, Alexander Graf wrote: >> On 06/10/2013 02:47 PM, Bhushan Bharat-R65777 wrote: >>>> -----Original Message----- >>>> From: Andreas Färber [mailto:afaer...@suse.de] >>>> Sent: Monday, June 10, 2013 5:43 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org; ag...@suse.de; Wood Scott- >>>> B07421; Bhushan Bharat-R65777 >>>> Subject: Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for >>>> target_bit above 61 >>>> So IIUC we can only allow 63 bits due to signedness, thus a maximum of >>>> (1<< 62), thus target_bit<= 61. >>>> Any chance at least the comment can be worded to explain that any better? >>>> Maybe >>>> also use (target-bit + 1>= 63) or period> INT64_MAX as condition? >>> How about this: >>> /* QEMU timer supports a maximum timer of INT64_MAX >>> (0x7fffffff_ffffffff). >>> * Run booke fit/wdog timer when >>> * ((1ULL<< target_bit + 1)< 0x40000000_00000000), i.e target_bit = >>> 61. >>> * Also the time with this maximum target_bit (with current range of >>> * CPU frequency PowerPC supports) will be many many years. So it is >>> * pretty safe to stop the timer above this threshold. */ >> How about >> /* This timeout will take years to trigger. Treat the timer as disabled. */ > > There should be at least a brief mention that it's because the QEMU timer > can't handle larger values,
If it can't handle higher values, maybe it's better to just set the timer value to INT64_MAX when we detect an overflow? That would make the code plainly obvious. Alex > with the detailed explanation in the changelog. A better lower bound on the > number of years would be nice as well (e.g. "hundreds of years"). > > -Scott
