[PATCH 4/4] clock: Define and use new clock_display_freq()

Peter Maydell posted 4 patches 5 years, 2 months ago
Maintainers: "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Eduardo Habkost <ehabkost@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>
There is a newer version of this series
[PATCH 4/4] clock: Define and use new clock_display_freq()
Posted by Peter Maydell 5 years, 2 months ago
It's common to want to print a human-readable indication of a clock's
frequency. Provide a utility function in the clock API to return a
string which is a displayable representation of the frequency,
and use it in qdev-monitor.c.

Before:

  (qemu) info qtree
  [...]
  dev: xilinx,zynq_slcr, id ""
    clock-in "ps_clk" freq_hz=3.333333e+07
    mmio 00000000f8000000/0000000000001000

After:

  dev: xilinx,zynq_slcr, id ""
    clock-in "ps_clk" freq_hz=33.3 MHz
    mmio 00000000f8000000/0000000000001000


Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This is based on Philippe's patch
"qdev-monitor: Display frequencies scaled to SI unit"
but I have abstracted out the "prettified string" into the clock API.
---
 docs/devel/clocks.rst  |  5 +++++
 include/hw/clock.h     | 12 ++++++++++++
 hw/core/clock.c        |  6 ++++++
 softmmu/qdev-monitor.c |  6 +++---
 4 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/docs/devel/clocks.rst b/docs/devel/clocks.rst
index 9a93d1361b4..cf8067542a1 100644
--- a/docs/devel/clocks.rst
+++ b/docs/devel/clocks.rst
@@ -267,6 +267,11 @@ Here is an example:
                         clock_get(dev->my_clk_input));
     }
 
+If you are only interested in the frequency for displaying it to
+humans (for instance in debugging), use ``clock_display_freq()``,
+which returns a prettified string-representation, e.g. "33.3 MHz".
+The caller must free the string with g_free() after use.
+
 Calculating expiry deadlines
 ----------------------------
 
diff --git a/include/hw/clock.h b/include/hw/clock.h
index 9c0b1eb4c3f..7bc9afb0800 100644
--- a/include/hw/clock.h
+++ b/include/hw/clock.h
@@ -252,4 +252,16 @@ static inline bool clock_is_enabled(const Clock *clk)
     return clock_get(clk) != 0;
 }
 
+/**
+ * clock_display_freq: return human-readable representation of clock frequency
+ * @clk: clock
+ *
+ * Return a string which has a human-readable representation of the
+ * clock's frequency, e.g. "33.3 MHz". This is intended for debug
+ * and display purposes.
+ *
+ * The caller is responsible for freeing the string with g_free().
+ */
+char *clock_display_freq(Clock *clk);
+
 #endif /* QEMU_HW_CLOCK_H */
diff --git a/hw/core/clock.c b/hw/core/clock.c
index 8c6af223e7c..76b5f468b6e 100644
--- a/hw/core/clock.c
+++ b/hw/core/clock.c
@@ -12,6 +12,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "hw/clock.h"
 #include "trace.h"
 
@@ -111,6 +112,11 @@ static void clock_disconnect(Clock *clk)
     QLIST_REMOVE(clk, sibling);
 }
 
+char *clock_display_freq(Clock *clk)
+{
+    return freq_to_str(clock_get_hz(clk));
+}
+
 static void clock_initfn(Object *obj)
 {
     Clock *clk = CLOCK(obj);
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index bf79d0bbcd9..6263d600026 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -747,11 +747,11 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
         }
     }
     QLIST_FOREACH(ncl, &dev->clocks, node) {
-        qdev_printf("clock-%s%s \"%s\" freq_hz=%e\n",
+        g_autofree char *freq_str = clock_display_freq(ncl->clock);
+        qdev_printf("clock-%s%s \"%s\" freq_hz=%s\n",
                     ncl->output ? "out" : "in",
                     ncl->alias ? " (alias)" : "",
-                    ncl->name,
-                    CLOCK_PERIOD_TO_HZ(1.0 * clock_get(ncl->clock)));
+                    ncl->name, freq_str);
     }
     class = object_get_class(OBJECT(dev));
     do {
-- 
2.20.1


Re: [PATCH 4/4] clock: Define and use new clock_display_freq()
Posted by Richard Henderson 5 years, 2 months ago
On 12/8/20 12:15 PM, Peter Maydell wrote:
> It's common to want to print a human-readable indication of a clock's
> frequency. Provide a utility function in the clock API to return a
> string which is a displayable representation of the frequency,
> and use it in qdev-monitor.c.
> 
> Before:
> 
>   (qemu) info qtree
>   [...]
>   dev: xilinx,zynq_slcr, id ""
>     clock-in "ps_clk" freq_hz=3.333333e+07
>     mmio 00000000f8000000/0000000000001000
> 
> After:
> 
>   dev: xilinx,zynq_slcr, id ""
>     clock-in "ps_clk" freq_hz=33.3 MHz
>     mmio 00000000f8000000/0000000000001000
> 
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is based on Philippe's patch
> "qdev-monitor: Display frequencies scaled to SI unit"
> but I have abstracted out the "prettified string" into the clock API.
> ---

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 4/4] clock: Define and use new clock_display_freq()
Posted by Luc Michel 5 years, 2 months ago
On 12/8/20 7:15 PM, Peter Maydell wrote:
> It's common to want to print a human-readable indication of a clock's
> frequency. Provide a utility function in the clock API to return a
> string which is a displayable representation of the frequency,
> and use it in qdev-monitor.c.
> 
> Before:
> 
>    (qemu) info qtree
>    [...]
>    dev: xilinx,zynq_slcr, id ""
>      clock-in "ps_clk" freq_hz=3.333333e+07
>      mmio 00000000f8000000/0000000000001000
> 
> After:
> 
>    dev: xilinx,zynq_slcr, id ""
>      clock-in "ps_clk" freq_hz=33.3 MHz
>      mmio 00000000f8000000/0000000000001000
> 
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Luc Michel <luc@lmichel.fr>

> ---
> This is based on Philippe's patch
> "qdev-monitor: Display frequencies scaled to SI unit"
> but I have abstracted out the "prettified string" into the clock API.
> ---
>   docs/devel/clocks.rst  |  5 +++++
>   include/hw/clock.h     | 12 ++++++++++++
>   hw/core/clock.c        |  6 ++++++
>   softmmu/qdev-monitor.c |  6 +++---
>   4 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/devel/clocks.rst b/docs/devel/clocks.rst
> index 9a93d1361b4..cf8067542a1 100644
> --- a/docs/devel/clocks.rst
> +++ b/docs/devel/clocks.rst
> @@ -267,6 +267,11 @@ Here is an example:
>                           clock_get(dev->my_clk_input));
>       }
>   
> +If you are only interested in the frequency for displaying it to
> +humans (for instance in debugging), use ``clock_display_freq()``,
> +which returns a prettified string-representation, e.g. "33.3 MHz".
> +The caller must free the string with g_free() after use.
> +
>   Calculating expiry deadlines
>   ----------------------------
>   
> diff --git a/include/hw/clock.h b/include/hw/clock.h
> index 9c0b1eb4c3f..7bc9afb0800 100644
> --- a/include/hw/clock.h
> +++ b/include/hw/clock.h
> @@ -252,4 +252,16 @@ static inline bool clock_is_enabled(const Clock *clk)
>       return clock_get(clk) != 0;
>   }
>   
> +/**
> + * clock_display_freq: return human-readable representation of clock frequency
> + * @clk: clock
> + *
> + * Return a string which has a human-readable representation of the
> + * clock's frequency, e.g. "33.3 MHz". This is intended for debug
> + * and display purposes.
> + *
> + * The caller is responsible for freeing the string with g_free().
> + */
> +char *clock_display_freq(Clock *clk);
> +
>   #endif /* QEMU_HW_CLOCK_H */
> diff --git a/hw/core/clock.c b/hw/core/clock.c
> index 8c6af223e7c..76b5f468b6e 100644
> --- a/hw/core/clock.c
> +++ b/hw/core/clock.c
> @@ -12,6 +12,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>   #include "hw/clock.h"
>   #include "trace.h"
>   
> @@ -111,6 +112,11 @@ static void clock_disconnect(Clock *clk)
>       QLIST_REMOVE(clk, sibling);
>   }
>   
> +char *clock_display_freq(Clock *clk)
> +{
> +    return freq_to_str(clock_get_hz(clk));
> +}
> +
>   static void clock_initfn(Object *obj)
>   {
>       Clock *clk = CLOCK(obj);
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index bf79d0bbcd9..6263d600026 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -747,11 +747,11 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
>           }
>       }
>       QLIST_FOREACH(ncl, &dev->clocks, node) {
> -        qdev_printf("clock-%s%s \"%s\" freq_hz=%e\n",
> +        g_autofree char *freq_str = clock_display_freq(ncl->clock);
> +        qdev_printf("clock-%s%s \"%s\" freq_hz=%s\n",
>                       ncl->output ? "out" : "in",
>                       ncl->alias ? " (alias)" : "",
> -                    ncl->name,
> -                    CLOCK_PERIOD_TO_HZ(1.0 * clock_get(ncl->clock)));
> +                    ncl->name, freq_str);
>       }
>       class = object_get_class(OBJECT(dev));
>       do {
>