Re: [PATCH] hw/clock: Expose 'freq-hz' QOM property

2024-05-20 Thread Peter Maydell
On Wed, 8 May 2024 at 22:27, Philippe Mathieu-Daudé  wrote:
>
> On 8/5/24 19:46, Peter Maydell wrote:
> > On Wed, 8 May 2024 at 15:13, Philippe Mathieu-Daudé  
> > wrote:
> >>
> >> Expose the clock frequency via the QOM 'freq-hz' property,
> >> as it might be useful for QTests.
> >>
> >> HMP example:
> >>
> >>$ qemu-system-mips -S -monitor stdio -M mipssim
> >>(qemu) qom-get /machine/cpu-refclk freq-hz
> >>1200
> >>
> >> Inspired-by: Inès Varhol 
> >> Signed-off-by: Philippe Mathieu-Daudé 
> >
> > So I have a couple of thoughts here:
> >
> > (1) if this is intended for qtests, would exposing the period (i.e.
> > QOM equivalent of clock_get() rather than clock_get_hz()) be better?
> > A Hz figure has rounding so it's not as accurate.
>
> Indeed, simpler to compare from QTest perspective.
>
> > (2) We should document this in clocks.rst; I guess we want to say
> > "only intended for use in qtests" (i.e. if you're part of QEMU
> > use the existing function interface, not this).
>
> OK, and we can also only expose this for QTest using:
>
>if (qtest_enabled()) {
>object_property_add(obj, "[qtest-]clock-period", ...);
>}

Yes, that seems reasonable. (I don't know if we have any other
qtest-only properties but I don't see any reason why we
shouldn't have them if we want to expose stuff for tests only.)

thanks
-- PMM



Re: [PATCH] hw/clock: Expose 'freq-hz' QOM property

2024-05-08 Thread Philippe Mathieu-Daudé

On 8/5/24 19:46, Peter Maydell wrote:

On Wed, 8 May 2024 at 15:13, Philippe Mathieu-Daudé  wrote:


Expose the clock frequency via the QOM 'freq-hz' property,
as it might be useful for QTests.

HMP example:

   $ qemu-system-mips -S -monitor stdio -M mipssim
   (qemu) qom-get /machine/cpu-refclk freq-hz
   1200

Inspired-by: Inès Varhol 
Signed-off-by: Philippe Mathieu-Daudé 


So I have a couple of thoughts here:

(1) if this is intended for qtests, would exposing the period (i.e.
QOM equivalent of clock_get() rather than clock_get_hz()) be better?
A Hz figure has rounding so it's not as accurate.


Indeed, simpler to compare from QTest perspective.


(2) We should document this in clocks.rst; I guess we want to say
"only intended for use in qtests" (i.e. if you're part of QEMU
use the existing function interface, not this).


OK, and we can also only expose this for QTest using:

  if (qtest_enabled()) {
  object_property_add(obj, "[qtest-]clock-period", ...);
  }



Re: [PATCH] hw/clock: Expose 'freq-hz' QOM property

2024-05-08 Thread Peter Maydell
On Wed, 8 May 2024 at 15:13, Philippe Mathieu-Daudé  wrote:
>
> Expose the clock frequency via the QOM 'freq-hz' property,
> as it might be useful for QTests.
>
> HMP example:
>
>   $ qemu-system-mips -S -monitor stdio -M mipssim
>   (qemu) qom-get /machine/cpu-refclk freq-hz
>   1200
>
> Inspired-by: Inès Varhol 
> Signed-off-by: Philippe Mathieu-Daudé 

So I have a couple of thoughts here:

(1) if this is intended for qtests, would exposing the period (i.e.
QOM equivalent of clock_get() rather than clock_get_hz()) be better?
A Hz figure has rounding so it's not as accurate.

(2) We should document this in clocks.rst; I guess we want to say
"only intended for use in qtests" (i.e. if you're part of QEMU
use the existing function interface, not this).

thanks
-- PMM



[PATCH] hw/clock: Expose 'freq-hz' QOM property

2024-05-08 Thread Philippe Mathieu-Daudé
Expose the clock frequency via the QOM 'freq-hz' property,
as it might be useful for QTests.

HMP example:

  $ qemu-system-mips -S -monitor stdio -M mipssim
  (qemu) qom-get /machine/cpu-refclk freq-hz
  1200

Inspired-by: Inès Varhol 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/core/clock.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/hw/core/clock.c b/hw/core/clock.c
index e212865307..55f86ef483 100644
--- a/hw/core/clock.c
+++ b/hw/core/clock.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/cutils.h"
+#include "qapi/visitor.h"
 #include "hw/clock.h"
 #include "trace.h"
 
@@ -158,6 +159,14 @@ bool clock_set_mul_div(Clock *clk, uint32_t multiplier, 
uint32_t divider)
 return true;
 }
 
+static void clock_prop_freq_get(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+Clock *clk = CLOCK(obj);
+uint64_t freq_hz = clock_get_hz(clk);
+visit_type_uint64(v, name, _hz, errp);
+}
+
 static void clock_initfn(Object *obj)
 {
 Clock *clk = CLOCK(obj);
@@ -166,6 +175,9 @@ static void clock_initfn(Object *obj)
 clk->divider = 1;
 
 QLIST_INIT(>children);
+
+object_property_add(obj, "freq-hz", "uint64",
+clock_prop_freq_get, NULL, NULL, NULL);
 }
 
 static void clock_finalizefn(Object *obj)
-- 
2.41.0