Re: [PATCH] Replacing (and removing) get_ticks_per_sec() function with NANOSECONDS_PER_SECOND Signed-off-by: Rutuja Shah <rutu.shah.26@gmail.com>

[Christian Borntraeger <borntraeger@de.ibm.com>]

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.