Re: [PATCH v2 3/4] hw/watchdog/sbsa_gwdt: Make watchdog timer frequency a QOM property

[Philippe Mathieu-Daudé]

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.maydell@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));

>       sysbus_realize_and_unref(s, &error_fatal);
>       sysbus_mmio_map(s, 0, rbase);
>       sysbus_mmio_map(s, 1, cbase);