[U-Boot] [PATCH v3 05/11] dm: timer: Support 64-bit counter

[Thomas Chou]

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.

Best regards,
Thomas