On 26/4/24 15:28, Peter Maydell wrote:
On Fri, 26 Apr 2024 at 13:46, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:
Hi Peter,
On 26/4/24 14:29, Peter Maydell wrote:
Currently the sbsa_gdwt watchdog device hardcodes its frequency at
62.5MHz. In real hardware, this watchdog is supposed to be driven
from the system counter, which also drives the CPU generic timers.
Newer CPU types (in particular from Armv8.6) should have a CPU
generic timer frequency of 1GHz, so we can't leave the watchdog
on the old QEMU default of 62.5GHz.
Make the frequency a QOM property so it can be set by the board,
and have our only board that uses this device set that frequency
to the same value it sets the CPU frequency.
Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
---
include/hw/watchdog/sbsa_gwdt.h | 3 +--
hw/arm/sbsa-ref.c | 1 +
hw/watchdog/sbsa_gwdt.c | 15 ++++++++++++++-
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 36f6f717b4b..57c337fd92a 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -543,6 +543,7 @@ static void create_wdt(const SBSAMachineState *sms)
SysBusDevice *s = SYS_BUS_DEVICE(dev);
int irq = sbsa_ref_irqmap[SBSA_GWDT_WS0];
+ qdev_prop_set_uint64(dev, "clock-frequency", SBSA_GTIMER_HZ);
Since we have access to the CPU and its generic timer, what about
just keep the wdg in sync, as smth like:
qdev_prop_set_uint64(dev, "clock-frequency",
object_property_get_uint(OBJECT(some_cpu),
"cntfrq", errp));
That introduces an implicit ordering requirement that
the CPU has been created before the watchdog, which I'm
not super enthusiastic about. "The platform knows the
frequency and sets it on the devices that care" seems
more straightforward to me.
(The really-follow-the-hardware approach here would be to
model the memory mapped system counter and then wire that
up to both the CPUs and the watchdog, but that's a lot
of extra work. I have some half-baked patches in that
direction but for the moment I figure doing the simple
thing is all we need.)
Yeah, since the clock is fixed, the current patch is
good enough.
Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
