[PATCH 2/2] platform: sifive_fu740: fix reset when watchdog is running

[David Abdurachmanov]

On Thu, Jan 6, 2022 at 3:10 PM Nikita Shubin <nikita.shubin at maquefel.me> wrote:
>
> Hello Aurelien!
>
> If it happens during U-Boot SPL that's a bad thing to happen, the ZSBL
> reading from SD card is slow, so we can catch up only in SPL.
>
> First i blamed our non-conforming reset theme, but since Heinrich
> reported that he has the same behavior even for reset button, i should
> assume that:
>
> - It looks like TWDSCALE setting are not being read from OTP
> - We end up in feeding/disabling watchdog early in SPL, and no way to
>   boot if something happened with U-Boot SPL (removing the
>   battery/unplugging the cord ?).
>
> The official manual states following:
>
> | If required, the application supervision by the WATCHDOG timer can be
> |  continued in POWERDOWN mode via WATCHDOG_PD. If the host will not
> |  communicate with the DA9063 during POWERDOWN mode, then the control
> |  interfaces may also be temporarily disabled (see controls
> |  PMIF_DIS/HS2WIRE_DIS)
>
> We don't have WATCHDOG_PD set anywhere, so that's not the reason.
>
> | Once in the ACTIVE state, the DA9063 continues to monitor the system
> | unless it is disabled by setting TWDSCALE to zero. When powering
> | down from ACTIVE mode, the watchdog monitor is stopped unless it
> | enters POWERDOWN mode via WATCHDOG_PD.
>
> | After powering up domain POWER, the DA9063 can initiate a watchdog
> | monitor function. The host processor must write a 1 within a
> | configured tWD_MAX time into control WATCHDOG, thereby indicating
> | that the host is alive. If the host does not write 1 to this watchdog
> | bit within the tWD_MAX time, the DA9063 asserts TWD_ERROR in the
> | FAULT_LOG register and powers down to RESET mode.
>
> Looks like we are experiencing that exact behaviour, through "can
> initiate a watchdog monitor function" bugs me.
>
> On boot:
> da9063_check_watchdog_pd: DA9063_REG_CONTROL_B=0x9
> da9063_check_watchdog_pd: DA9063_REG_CONTROL_C=0xd3
>
> On reset:
> da9063_check_watchdog_pd: DA9063_REG_CONTROL_B=0x9
> da9063_check_watchdog_pd: DA9063_REG_CONTROL_C=0xd3
>
> We can do something (if really can catch up with WDG time window) only
> in U-Boot SPL.
>
> Since this affects only Unmatched and is located in platform code, i
> think that at least we can have a way of "normal" reboot, but it looks
> like da9063 WDG should be avoided.
>
> Anup, what do you think about all this ?

Ping.

I don't think I have seen a comment from Anup, and I don't see the
patch merged in opensbi.

david

>
> Anyway, i've tested you patch and it works as expected.
>
> Reviewed-by: Nikita Shubin <n.shubin at yadro.com>
> Tested-by: Nikita Shubin <n.shubin at yadro.com>
>
> On Wed, 5 Jan 2022 17:51:19 +0100
> Aurelien Jarno <aurelien at aurel32.net> wrote:
>
> > On 2022-01-05 18:16, David Abdurachmanov wrote:
> > > On Wed, Jan 5, 2022 at 3:40 PM Nikita Shubin
> > > <nikita.shubin at maquefel.me> wrote:
> > > >
> > > > Hello Aurelien!
> > > >
> > > > Adding David from SiFive...
> > > >
> > > > On Wed,  5 Jan 2022 08:20:39 +0100
> > > > Aurelien Jarno <aurelien at aurel32.net> wrote:
> > > >
> > > > > When the watchdog is running the HiFive Unmatched board does not
> > > > > reboot properly and shuts down itself a few seconds after
> > > > > reboot, in the early stages of the u-boot loading. On a Linux
> > > > > kernel this happens when the da9063_wdt module is loaded. This
> > > > > does not happen if the module is unloaded before reboot or if
> > > > > the watchdog module is loaded with "stop_on_reboot=1".
> > > >
> > > > | A running application is typically in ACTIVE mode. The DA9063
> > > > | transitions to ACTIVE mode after the host processor performs at
> > > > least | one initial ‘alive’ watchdog write (or alternatively an
> > > > initial | assertion of the KEEP_ACT port) inside the target time
> > > > window. If the | WATCHDOG function is disabled by setting
> > > > TWDSCALE to zero, the DA9063 | transitions to ACTIVE mode when
> > > > all of the sequencer IDs in the POWER | domain are complete.
> >
> > Yes, this seems to match that. The da9063_wdt module is responsible
> > for doing the ‘alive’ watchdog writes.
> >
> > > > Is this that's case mentioned ? What if we press a reset key when
> > > > watchdog is enabled ? Or if it was reseted by thermal sensor ?
> >
> > I am not sure what happens in that case and unfortunately I do not
> > have a board when I can try that.
> >
> > > > Can we disable watchdog on start instead of disabling it before a
> > > > reset ?
> > >
> > > I would suggest doing this too. Disable watchdog on the start.
> >
> > What do you mean by on the start? At the beginning of OpenSBI, as soon
> > as it boots?
> >
> > In that case it might be necessary to do an ‘alive’ watchdog write
> > just before the reset, as we do not know when the OS did the last
> > write, and we have no way of knowing that. Also in that case we might
> > want to increase the timeout, as currently the board gets powered-off
> > in U-Boot SPL, before displaying the OpenSBI banner, so the execution
> > of OpenSBI might not have started yet.
> >
> > All the above looks complex compared to disabling the watchdog before
> > the reset sequence. And leaving the watchdog running does not seem to
> > bring any advantage, as it just powers off the board. I would have
> > expected it resets it again.
> >
> > > IIRC watchdog DA9063 can do 131 seconds (max). Alternative would be
> > > to reconfigure the timer for 131 seconds, kick it in OpenSBI to
> > > give the most possible time and reboot. Hopefully that would be
> > > enough time to reboot and take control of the watchdog again. I
> > > don't know if U-Boot has a DA9063 watchdog driver.
> > >
> > > Disabling it is probably a better option :) System boot with
> > > watchdog disabled and then it can be enabled again.
> >
> > u-boot has support for watchdog, but there is no support for the
> > DA9063 driver, so it's probably a good idea to just disable it.
> >
> > > > Could you please give us a link to actual report ?
> >
> > Unfortunately I do not have a link to share. This was discussed on
> > IRC. I built a Debian kernel with the SRST [1] patch and asked people
> > to test it on an Unmatched board against OpenSBI 1.0 to check if it
> > was working. Tianon reported me quickly that it was not working, the
> > board starts to reboot properly with:
> >
> > | [  372.508832] reboot: Restarting system
> > |
> > | U-Boot SPL 2022.01-rc4+dfsg-1 (Dec 21 2021 - 02:38:20 +0000)
> > | Trying to boot from MMC1
> >
> > But then the board powers off ("all the lights and even the fan are
> > off too").
> >
> > We tracked that the issue was a bad interaction with the da9063 kernel
> > modules and then narrowed it down to the watchdog module.
> >
> > Note that it is using kernel 5.16-rc7 with the DA9063 support enabled
> > in both the config and the DT [2].
> >
> > If you need more details, Tianon (Cc:ed) can probably provide them.
> >
> >
> > [1]
> > https://lists.infradead.org/pipermail/linux-riscv/2021-June/006998.html
> > [2]
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.16-rc8&id=cd29cc8ad2540a4f9a0a3e174394d39e648ef941
> >
>