Hi, On 20 November 2015 at 17:41, Thomas Chou <tho...@wytron.com.tw> wrote: > Hi Simon, > > > On 2015年11月21日 05:10, Simon Glass wrote: >>>>> >>>>> @@ -35,10 +52,10 @@ struct timer_ops { >>>>> * Get the current timer count >>>>> * >>>>> * @dev: The timer device >>>>> - * @count: pointer that returns the current timer count >>>>> + * @count: pointer that returns the current 64-bit timer count >>>>> * @return: 0 if OK, -ve on error >>>>> */ >>>>> - int (*get_count)(struct udevice *dev, unsigned long *count); >>>>> + int (*get_count)(struct udevice *dev, u64 *count); >>>> >>>> >>>> Why do we need to change the API to 64-bit? >>> >>> >>> IMHO, this API should be created to be accept a 64-bit value in the >>> first place. As you see the U-Boot time APIs in lib/time.c like >>> get_ticks() has been using 64-bit value. >> >> >> Right, but the point of this timer is for time delays. Making everyone >> one of those 64-bit on a 32-bit machine does not seem like a win to >> me. >> >>> >>>> >>>> My only concern is that we are pretty happy with the 32-bit timer and >>>> I'm not sure it should change. At present unsigned long can be 32-bit >>>> on 32-bit machines. >>> >>> >>> 32-bit timer has to be converted to 64-bit value as get_ticks() >>> requires 64-bit. As for 'unsigned long can be 32-bit on 32-bit >>> machines', I think we should explicitly declare its width as this is >>> hardware limitation (32-bit timer can only produce 32-bit value, even >>> if it is on a 64-bit machine, where long on 64-bit means 64-bit, which >>> is wrong for this 32-bit timer hardware). >> >> >> This is a bit messy, but for now I think we should follow what >> get_timer() does. It would be worth getting more input on the list I >> think. >> > > I would agree with Bin that the API should have been created as 64 bits. I > considered this option before, because most part of lib/time.c uses 64 bits. > It was only that I didn't have much confidence to change so many global code > (without buildman). I realized this mistake when I wrote the sandbox timer. > It is wasteful to truncate the 64 bits count to 32 bits, and then > reconstruct 32 bits to 64 bits. This is what Bin wanted to resolve. There is > no overhead for 32 bits timer, as the patch only shift the 64 bits > conversion from lib/time.c to driver. Yet for 64 bits timer, this is a good > saving.
OK thank you both for your explanation. This makes sense to me now. Acked-by: Simon Glass <s...@chromium.org> Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot