Re: [PATCH 3/4] hw/arm: Add WDT to Allwinner-H3 and Orangepi-PC

[Strahinja Jankovic]

Hi,

On Mon, Mar 13, 2023 at 8:53 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Hi,
>
> On 11/3/23 15:41, Strahinja Jankovic wrote:
> > This patch adds WDT to Allwinner-H3 and Orangepi-PC.
> > WDT is added as an overlay to the Timer module memory area.
> >
> > Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
> > ---
> >   docs/system/arm/orangepi.rst  | 1 +
> >   hw/arm/Kconfig                | 1 +
> >   hw/arm/allwinner-h3.c         | 8 ++++++++
> >   include/hw/arm/allwinner-h3.h | 5 ++++-
> >   4 files changed, 14 insertions(+), 1 deletion(-)
>
>
> > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
> > index 69d0ad6f50..f05afddf7e 100644
> > --- a/hw/arm/allwinner-h3.c
> > +++ b/hw/arm/allwinner-h3.c
> > @@ -49,6 +49,7 @@ const hwaddr allwinner_h3_memmap[] = {
> >       [AW_H3_DEV_OHCI3]      = 0x01c1d400,
> >       [AW_H3_DEV_CCU]        = 0x01c20000,
> >       [AW_H3_DEV_PIT]        = 0x01c20c00,
> > +    [AW_H3_DEV_WDT]        = 0x01c20ca0,
> >       [AW_H3_DEV_UART0]      = 0x01c28000,
> >       [AW_H3_DEV_UART1]      = 0x01c28400,
> >       [AW_H3_DEV_UART2]      = 0x01c28800,
> > @@ -234,6 +235,8 @@ static void allwinner_h3_init(Object *obj)
> >       object_initialize_child(obj, "twi1",  &s->i2c1,  TYPE_AW_I2C_SUN6I);
> >       object_initialize_child(obj, "twi2",  &s->i2c2,  TYPE_AW_I2C_SUN6I);
> >       object_initialize_child(obj, "r_twi", &s->r_twi, TYPE_AW_I2C_SUN6I);
> > +
> > +    object_initialize_child(obj, "wdt", &s->wdt, TYPE_AW_WDT_SUN6I);
> >   }
> >
> >   static void allwinner_h3_realize(DeviceState *dev, Error **errp)
> > @@ -453,6 +456,11 @@ static void allwinner_h3_realize(DeviceState *dev, 
> > Error **errp)
> >       sysbus_connect_irq(SYS_BUS_DEVICE(&s->r_twi), 0,
> >                          qdev_get_gpio_in(DEVICE(&s->gic), 
> > AW_H3_GIC_SPI_R_TWI));
> >
> > +    /* WDT */
> > +    sysbus_realize(SYS_BUS_DEVICE(&s->wdt), &error_fatal);
> > +    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->wdt), 0,
> > +                            s->memmap[AW_H3_DEV_WDT], 1);
>
> Why do you need to overlap?
>

The WDT is part of the Timer component and it lies in the part of
memory already initialized by the AW_H3_DEV_PIT.
I saw the overlay approach used for RTC in Allwinner A10 (same issue,
in A10 both RTC and WDT are part of Timer), so I just reused it.

If there is a better way to handle it, I can update the implementation.

Best regards,
Strahinja