Re: [PATCH v2 3/3] hw/arm: Add watchdog timer to Allwinner H40 and Bananapi board

[Philippe Mathieu-Daudé]

Hi,

(Cc'ing Li, Strahinja and Niek)

On 15/1/24 19:27, Guenter Roeck wrote:
> Add watchdog timer support to Allwinner-H40 and Bananapi.
> The watchdog timer is added as an overlay to the Timer
> module memory map.

I'm confused by these registers from TYPE_AW_A10_PIT
and the TYPE_AW_WDT implementation you are using:

  #define AW_A10_PIT_WDOG_CONTROL    0x90
  #define AW_A10_PIT_WDOG_MODE       0x94

Do we have 2 implementations of the same peripheral?

Should we instanciate AW_WDT within AW_A10_PIT?

> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>   docs/system/arm/bananapi_m2u.rst | 2 +-
>   hw/arm/Kconfig                   | 1 +
>   hw/arm/allwinner-r40.c           | 8 ++++++++
>   include/hw/arm/allwinner-r40.h   | 3 +++
>   4 files changed, 13 insertions(+), 1 deletion(-)


> diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
> index 534be4a735..a28e5b3886 100644
> --- a/hw/arm/allwinner-r40.c
> +++ b/hw/arm/allwinner-r40.c
> @@ -53,6 +53,7 @@ const hwaddr allwinner_r40_memmap[] = {
>       [AW_R40_DEV_OHCI2]      = 0x01c1c400,
>       [AW_R40_DEV_CCU]        = 0x01c20000,
>       [AW_R40_DEV_PIT]        = 0x01c20c00,
> +    [AW_R40_DEV_WDT]        = 0x01c20c90,
>       [AW_R40_DEV_UART0]      = 0x01c28000,
>       [AW_R40_DEV_UART1]      = 0x01c28400,
>       [AW_R40_DEV_UART2]      = 0x01c28800,
> @@ -279,6 +280,8 @@ static void allwinner_r40_init(Object *obj)
>       object_property_add_alias(obj, "clk1-freq", OBJECT(&s->timer),
>                                 "clk1-freq");
>   
> +    object_initialize_child(obj, "wdt", &s->wdt, TYPE_AW_WDT_SUN4I);
> +
>       object_initialize_child(obj, "ccu", &s->ccu, TYPE_AW_R40_CCU);
>   
>       for (int i = 0; i < AW_R40_NUM_MMCS; i++) {
> @@ -545,6 +548,11 @@ static void allwinner_r40_realize(DeviceState *dev, Error 
> **errp)
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->emac), 0,
>                          qdev_get_gpio_in(DEVICE(&s->gic), 
> AW_R40_GIC_SPI_EMAC));
>   
> +    /* WDT */
> +    sysbus_realize(SYS_BUS_DEVICE(&s->wdt), &error_fatal);
> +    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->wdt), 0,
> +                            allwinner_r40_memmap[AW_R40_DEV_WDT], 1);
> +