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

[Simon Glass]

Hi,

On 20 November 2015 at 17:41, Thomas Chou <thomas at 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 <sjg at chromium.org>

Regards,
Simon