On Fri, 21 Jun 2019 at 09:06, Philippe Mathieu-Daudé <phi...@redhat.com> wrote:
>
> On 6/21/19 10:25 AM, Cédric Le Goater wrote:
> > On 21/06/2019 08:52, Joel Stanley wrote:
> >> The ast2500 uses the watchdog to reset the SDRAM controller. This
> >> operation is usually performed by u-boot's memory training procedure,
> >> and it is enabled by setting a bit in the SCU and then causing the
> >> watchdog to expire. Therefore, we need the watchdog to be able to
> >> access the SCU's register space.
> >>
> >> This causes the watchdog to not perform a system reset when the bit is
> >> set. In the future it could perform a reset of the SDMC model.
> >>
> >> Signed-off-by: Joel Stanley <j...@jms.id.au>
> >
> > I was keeping this patch in my tree (hence the Sob) hoping that
> > someone could find the time to study the reset question. But this
> > patch is useful as it is and I think we should merge it.
> >
> > Reviewed-by: Cédric Le Goater <c...@kaod.org>
> >
> > Thanks,
> >
> > C.
> >
> >> Signed-off-by: Cédric Le Goater <c...@kaod.org>
> >> --- a/hw/watchdog/wdt_aspeed.c
> >> +++ b/hw/watchdog/wdt_aspeed.c
> >> @@ -44,6 +44,9 @@
> >>
> >> #define WDT_RESTART_MAGIC 0x4755
> >>
> >> +#define SCU_RESET_CONTROL1 (0x04 / 4)
> >> +#define SCU_RESET_SDRAM BIT(0)
> >> +
> >> static bool aspeed_wdt_is_enabled(const AspeedWDTState *s)
> >> {
> >> return s->regs[WDT_CTRL] & WDT_CTRL_ENABLE;
> >> @@ -222,6 +225,13 @@ static void aspeed_wdt_timer_expired(void *dev)
> >> {
> >> AspeedWDTState *s = ASPEED_WDT(dev);
> >>
> >> + /* Do not reset on SDRAM controller reset */
> >> + if (s->scu->regs[SCU_RESET_CONTROL1] & SCU_RESET_SDRAM) {
>
> This would be cleaner as an static inlined function in
> "hw/misc/aspeed_scu.h" IMO, maybe 'bool scu_sdram_is_reset()'.
I will take this suggestion on board in the future when I model the
watchdog reset behavior in more detail.
>
> Anyway the patch looks sane:
> Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
Thanks.
Joel
