This patchseries makes some changes to the clock API:
 * Remove clock_get_ns()
 * Add clock_ticks_to_ns() to return number of nanoseconds
   it will take the clock to tick N times
 * clock_display_freq() to return prettily-formatted string
   for showing humans the approximate clock frequency

This is based on discussions we had about these APIs a little while
back.  The core driver here is that the clock objects internally
store the period in units of 2^-32 ns, so both clock_get_ns() and
clock_get_hz() are inherently returning a rounded-off result, which
can be badly inaccurate for fast clocks or if you want to multiply it
by a large tick count.

Ideally I'd like to get rid of clock_get_hz() as well, but
that looks trickier than handling clock_get_ns().

Patch 4 borrows a lot of the concept from one of Philippe's that he
sent out previously.

NB: tested with 'make check' and 'make check-acceptance' only.

Changes v1->v2:
 * In patch 1, saturate return value to INT64_MAX (and update
   docs to explain this and why)

Patches 2-4 have been reviewed, only patch 1 needs review.

thanks
-- PMM

Peter Maydell (4):
  clock: Introduce clock_ticks_to_ns()
  target/mips: Don't use clock_get_ns() in clock period calculation
  clock: Remove clock_get_ns()
  clock: Define and use new clock_display_freq()

 docs/devel/clocks.rst  | 51 ++++++++++++++++++++++++++++++++++++----
 include/hw/clock.h     | 53 +++++++++++++++++++++++++++++++++++++++---
 hw/core/clock.c        |  6 +++++
 softmmu/qdev-monitor.c |  6 ++---
 target/mips/cpu.c      |  4 ++--
 5 files changed, 108 insertions(+), 12 deletions(-)

-- 
2.20.1

On 12/15/20 4:09 PM, Peter Maydell wrote:
> This patchseries makes some changes to the clock API:
>  * Remove clock_get_ns()
>  * Add clock_ticks_to_ns() to return number of nanoseconds
>    it will take the clock to tick N times
>  * clock_display_freq() to return prettily-formatted string
>    for showing humans the approximate clock frequency
> 
> This is based on discussions we had about these APIs a little while
> back.  The core driver here is that the clock objects internally
> store the period in units of 2^-32 ns, so both clock_get_ns() and
> clock_get_hz() are inherently returning a rounded-off result, which
> can be badly inaccurate for fast clocks or if you want to multiply it
> by a large tick count.
> 
> Ideally I'd like to get rid of clock_get_hz() as well, but
> that looks trickier than handling clock_get_ns().
> 
> Patch 4 borrows a lot of the concept from one of Philippe's that he
> sent out previously.
> 
> NB: tested with 'make check' and 'make check-acceptance' only.

Tested using loongson3-virt @2GHz
https://lists.gnu.org/archive/html/qemu-devel/2020-12/msg04036.html

Without your series:
qemu-system-mips64el: target/mips/cpu.c:385: mips_cp0_period_set:
Assertion `env->cp0_count_ns' failed.
Aborted (core dumped)

With: OK (cpu are not displayed in 'info qtree).

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Thanks!

Phil.

On 12/15/20 4:29 PM, Philippe Mathieu-Daudé wrote:
> On 12/15/20 4:09 PM, Peter Maydell wrote:
>> This patchseries makes some changes to the clock API:
>>  * Remove clock_get_ns()
>>  * Add clock_ticks_to_ns() to return number of nanoseconds
>>    it will take the clock to tick N times
>>  * clock_display_freq() to return prettily-formatted string
>>    for showing humans the approximate clock frequency
>>
...
> 
> Tested using loongson3-virt @2GHz
> https://lists.gnu.org/archive/html/qemu-devel/2020-12/msg04036.html
> 
> Without your series:
> qemu-system-mips64el: target/mips/cpu.c:385: mips_cp0_period_set:
> Assertion `env->cp0_count_ns' failed.
> Aborted (core dumped)
> 
> With: OK (cpu are not displayed in 'info qtree').
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

I meant:
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

But now I reviewed, so confirming:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Thanks!

Phil.

On 12/15/20 4:09 PM, Peter Maydell wrote:
> This patchseries makes some changes to the clock API:
>  * Remove clock_get_ns()
>  * Add clock_ticks_to_ns() to return number of nanoseconds
>    it will take the clock to tick N times
>  * clock_display_freq() to return prettily-formatted string
>    for showing humans the approximate clock frequency
> 
> This is based on discussions we had about these APIs a little while
> back.  The core driver here is that the clock objects internally
> store the period in units of 2^-32 ns, so both clock_get_ns() and
> clock_get_hz() are inherently returning a rounded-off result, which
> can be badly inaccurate for fast clocks or if you want to multiply it
> by a large tick count.
...
> Peter Maydell (4):
>   clock: Introduce clock_ticks_to_ns()
>   target/mips: Don't use clock_get_ns() in clock period calculation
>   clock: Remove clock_get_ns()
>   clock: Define and use new clock_display_freq()
> 
>  docs/devel/clocks.rst  | 51 ++++++++++++++++++++++++++++++++++++----
>  include/hw/clock.h     | 53 +++++++++++++++++++++++++++++++++++++++---
>  hw/core/clock.c        |  6 +++++
>  softmmu/qdev-monitor.c |  6 ++---
>  target/mips/cpu.c      |  4 ++--
>  5 files changed, 108 insertions(+), 12 deletions(-)

Until someone else prefers otherwise, I plan to queue this series
via the MIPS tree, as I can add the patch which increases the
Loongson3v frequency to 2GHz on top (the unique machine using
a frequency over 1GHz).

Thanks,

Phil.

On 1/1/21 9:35 PM, Philippe Mathieu-Daudé wrote:
> On 12/15/20 4:09 PM, Peter Maydell wrote:
>> This patchseries makes some changes to the clock API:
>>  * Remove clock_get_ns()
>>  * Add clock_ticks_to_ns() to return number of nanoseconds
>>    it will take the clock to tick N times
>>  * clock_display_freq() to return prettily-formatted string
>>    for showing humans the approximate clock frequency
>>
>> This is based on discussions we had about these APIs a little while
>> back.  The core driver here is that the clock objects internally
>> store the period in units of 2^-32 ns, so both clock_get_ns() and
>> clock_get_hz() are inherently returning a rounded-off result, which
>> can be badly inaccurate for fast clocks or if you want to multiply it
>> by a large tick count.
> ...
>> Peter Maydell (4):
>>   clock: Introduce clock_ticks_to_ns()
>>   target/mips: Don't use clock_get_ns() in clock period calculation
>>   clock: Remove clock_get_ns()
>>   clock: Define and use new clock_display_freq()
>>
>>  docs/devel/clocks.rst  | 51 ++++++++++++++++++++++++++++++++++++----
>>  include/hw/clock.h     | 53 +++++++++++++++++++++++++++++++++++++++---
>>  hw/core/clock.c        |  6 +++++
>>  softmmu/qdev-monitor.c |  6 ++---
>>  target/mips/cpu.c      |  4 ++--
>>  5 files changed, 108 insertions(+), 12 deletions(-)
> 
> Until someone else prefers otherwise, I plan to queue this series
> via the MIPS tree, as I can add the patch which increases the
> Loongson3v frequency to 2GHz on top (the unique machine using
> a frequency over 1GHz).

Thanks, series applied to mips-next.