Re: [PATCH 2/4] hw/arm: Add WDT to Allwinner-A10 and Cubieboard

[Strahinja Jankovic]

Hi Niek,

On Tue, Mar 14, 2023 at 10:21 PM Niek Linnenbank
<nieklinnenbank@gmail.com> wrote:
>
> Hi Strahinja,
>
>
> On Sat, Mar 11, 2023 at 3:42 PM Strahinja Jankovic 
> <strahinjapjankovic@gmail.com> wrote:
>>
>> This patch adds WDT to Allwinner-A10 and Cubieboard.
>> WDT is added as an overlay to the Timer module memory map.
>>
>> Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
>> ---
>>  docs/system/arm/cubieboard.rst | 1 +
>>  hw/arm/Kconfig                 | 1 +
>>  hw/arm/allwinner-a10.c         | 7 +++++++
>>  include/hw/arm/allwinner-a10.h | 2 ++
>>  4 files changed, 11 insertions(+)
>>
>> diff --git a/docs/system/arm/cubieboard.rst b/docs/system/arm/cubieboard.rst
>> index 8d485f5435..58c4a2d3ea 100644
>> --- a/docs/system/arm/cubieboard.rst
>> +++ b/docs/system/arm/cubieboard.rst
>> @@ -15,3 +15,4 @@ Emulated devices:
>>  - USB controller
>>  - SATA controller
>>  - TWI (I2C) controller
>> +- Watchdog timer
>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>> index b5aed4aff5..ec15248536 100644
>> --- a/hw/arm/Kconfig
>> +++ b/hw/arm/Kconfig
>> @@ -325,6 +325,7 @@ config ALLWINNER_A10
>>      select ALLWINNER_A10_PIC
>>      select ALLWINNER_A10_CCM
>>      select ALLWINNER_A10_DRAMC
>> +    select ALLWINNER_WDT
>>      select ALLWINNER_EMAC
>>      select ALLWINNER_I2C
>>      select AXP209_PMU
>> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
>> index b7ca795c71..b0ea3f7f66 100644
>> --- a/hw/arm/allwinner-a10.c
>> +++ b/hw/arm/allwinner-a10.c
>> @@ -38,6 +38,7 @@
>>  #define AW_A10_EHCI_BASE        0x01c14000
>>  #define AW_A10_OHCI_BASE        0x01c14400
>>  #define AW_A10_SATA_BASE        0x01c18000
>> +#define AW_A10_WDT_BASE         0x01c20c90
>
>
> Unfortunately I couldn't find any details about the watchdog in the Allwinner 
> A10 datasheet "A10_Datasheet.pdf", except for a very brief
> summary in chapter 9.1 in the Timer Controller. But I did find that linux is 
> using this same base address and registers with the shared driver code in 
> drivers/watchdog/sunxi_wdt.c.

Thanks for the review. The User Manual for Allwinner A10 is available
at https://linux-sunxi.org/images/1/1e/Allwinner_A10_User_manual_V1.5.pdf,
watchdog registers are on pages 99-100.

Best regards,
Strahinja


>
> Looks good to me.
>
> Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com>
>
>>
>>  #define AW_A10_RTC_BASE         0x01c20d00
>>  #define AW_A10_I2C0_BASE        0x01c2ac00
>>
>> @@ -92,6 +93,8 @@ static void aw_a10_init(Object *obj)
>>      object_initialize_child(obj, "mmc0", &s->mmc0, TYPE_AW_SDHOST_SUN4I);
>>
>>      object_initialize_child(obj, "rtc", &s->rtc, TYPE_AW_RTC_SUN4I);
>> +
>> +    object_initialize_child(obj, "wdt", &s->wdt, TYPE_AW_WDT_SUN4I);
>>  }
>>
>>  static void aw_a10_realize(DeviceState *dev, Error **errp)
>> @@ -203,6 +206,10 @@ static void aw_a10_realize(DeviceState *dev, Error 
>> **errp)
>>      sysbus_realize(SYS_BUS_DEVICE(&s->i2c0), &error_fatal);
>>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c0), 0, AW_A10_I2C0_BASE);
>>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c0), 0, qdev_get_gpio_in(dev, 
>> 7));
>> +
>> +    /* WDT */
>> +    sysbus_realize(SYS_BUS_DEVICE(&s->wdt), &error_fatal);
>> +    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->wdt), 0, AW_A10_WDT_BASE, 1);
>>  }
>>
>>  static void aw_a10_class_init(ObjectClass *oc, void *data)
>> diff --git a/include/hw/arm/allwinner-a10.h b/include/hw/arm/allwinner-a10.h
>> index 095afb225d..cd1465c613 100644
>> --- a/include/hw/arm/allwinner-a10.h
>> +++ b/include/hw/arm/allwinner-a10.h
>> @@ -13,6 +13,7 @@
>>  #include "hw/misc/allwinner-a10-ccm.h"
>>  #include "hw/misc/allwinner-a10-dramc.h"
>>  #include "hw/i2c/allwinner-i2c.h"
>> +#include "hw/watchdog/allwinner-wdt.h"
>>  #include "sysemu/block-backend.h"
>>
>>  #include "target/arm/cpu.h"
>> @@ -41,6 +42,7 @@ struct AwA10State {
>>      AwSdHostState mmc0;
>>      AWI2CState i2c0;
>>      AwRtcState rtc;
>> +    AwWdtState wdt;
>>      MemoryRegion sram_a;
>>      EHCISysBusState ehci[AW_A10_NUM_USB];
>>      OHCISysBusState ohci[AW_A10_NUM_USB];
>> --
>> 2.30.2
>>
>
>
> --
> Niek Linnenbank
>