qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [PATCH] Replacing (and removing) get_ticks_per_sec() func


From: Christian Borntraeger
Subject: Re: [Qemu-arm] [PATCH] Replacing (and removing) get_ticks_per_sec() function with NANOSECONDS_PER_SECOND Signed-off-by: Rutuja Shah <address@hidden>
Date: Fri, 11 Mar 2016 14:22:21 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0

On 03/11/2016 01:26 PM, Paolo Bonzini wrote:
> 
> 
> On 11/03/2016 13:12, Christian Borntraeger wrote:
>> On 03/11/2016 01:07 PM, Paolo Bonzini wrote:
>>>
>>>
>>> On 11/03/2016 12:44, Christian Borntraeger wrote:
>>>>> -    s->tick_offset_vmstate = s->tick_offset + delta / 
>>>>> get_ticks_per_sec();
>>>>>> +    s->tick_offset_vmstate = s->tick_offset + delta / 
>>>>>> NANOSECONDS_PER_SECOND;
>>>> [...]
>>>>
>>>> While technically correct, I do not like these changes. The interfaces 
>>>> expect "ticks",
>>>> and the fact that this happens to be a nanosecond does not help regarding 
>>>> readability.
>>>
>>> Actually, I think usage of "tick" in this file is just for historical
>>> reasons.
>>
>> So in essence the patch is ok and we should try to get rid of the "tick" 
>> word in future
>> patches?
> 
> Not necessarily.  The patch stops overloading the word "tick", so that
> "tick" means "whatever the timer device counts".
> 
> In fact, you and I were both confused by the appearance of the word
> "tick" in get_ticks_per_sec(). 
[...]
> the plus meant nanoseconds.  And I got confused *despite being the
> author of that line* (commit b0f2663, "pl031: switch clock base to
> rtc_clock", 2012-03-30).

Given the confusion, the patch description should contain
some of these examples and an explanation why this is the right thing
to do.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]