Re: [PATCH v2 02/11] hw/watchdog/wdt_aspeed: Extend MMIO range to cover more registers

[Philippe Mathieu-Daudé]

On 31/12/22 23:52, Dong, Eddie wrote:

> >    #define WDT_RELOAD_VAL_REG          0x0004
> >    #define WDT_RESTART_REG             0x0008
> >    #define WDT_CTRL_REG                0x000C
> >    #define WDT_TIMEOUT_STATUS_REG      0x0010
> >    #define WDT_TIMEOUT_STATUS_CLR_REG  0x0014
> >    #define WDT_RESET_MASK1_REG         0x001C
> >    #define WDT_RESET_MASK2_REG         0x0020
> >    #define WDT_SW_RESET_MASK1_REG      0x0028   <------
> >    #define WDT_SW_RESET_MASK2_REG      0x002C
> >    #define WDT_SW_RESET_CTRL_REG       0x0024
> >
> > Currently QEMU only cover a MMIO region of size 0x20:
> >
> >    #define ASPEED_WDT_REGS_MAX        (0x20 / 4)
> >
> > Change to map the whole 'iosize' which might be bigger, covering the other
>
> The root cause is that ASPEED_WDT_REGS_MAX is too small, right?
> Probably the Qemu is emulating an old version of the hardware.
>
> Given the meaning of ASPEED_WDT_REGS_MAX, it should be larger than iosize, not?
> Probably ASPEED_WDT_REGS_MAX should be per device type (aspeed_2400/2500),
> while iosize is for all devices, and its initial value comes from the per 
> device type REGS_MAX.

You are correct. I don't have access to the specifications neither,
but I suspect you are right, the current watchdog is modelling an
older version that what the AST1030 has.

I started these WDT patches to understand the unimplemented accesses
done by Zephyr, but eventually they resulted irrelevant to the fixes
(see following patches) so I'll simply drop them.

Thanks,

Phil.