Re: [PATCH v2 0/2] Add a watchdog driver that uses ARM Secure Monitor Calls.

[Xingyu Chen]

Hi,Julius

On 2020/4/16 6:29, Julius Werner wrote:
> > In addition, It looks more reasonable to use the "msec" as the unit of
> > timeout parameter for the ATF fw interface with SMCWD_SET_TIMEOUT:
> >
> > - The fw interface will compatible with the uboot generic watchdog
> > interface at [0], and there is no need to convert timeout from msec
> > to sec.
>
> I think we're trying hard to keep this compatible to a Trusted
> Firmware counterpart that we have already shipped, so we would prefer
> to keep it at seconds if possible. That's what the Linux watchdog core
> uses as well after all, so it just seems natural. I don't really see
> how what U-Boot does would have anything to do with this.

If the uboot introduces a smcwd driver, and it maybe work like this:

static const struct wdt_ops XXX_wdt_ops = {
	.start = XXX_wdt_start,
	...
}

static int XXX_wdt_start(struct udevice *dev, u64 ms, ulong flags) {
	timeout =  ms / 1000;  //convert timeout from msec to sec.
	smcwd_call(SMCWD_SET_TIMEOUT, timeout, NULL);
	smcwd_call(SMCWD_ENABLE, 0, NULL);
}

Best Regards
> > - Some vendor's watchdog may be not support the "wdt_trigger_reset"
> > reset operation, but they can use the method below to reset the system
> > by the watchdog right now.
> >
> > watchdog_set_time(1);  //1ms
> > watchdog_enable();
>
> They can still do that but they should do that on the Trusted Firmware
> side. Emulating a missing reset functionality should be handled by the
> hardware abstraction layer (in this case Trusted Firmware), not at the
> Linux API level. So Linux would still send a PSCI_SYSTEM_RESET SMC,
> but then Trusted Firmware can choose to implement that by setting the
> watchdog to the smallest possible timeout (which it can because it's
> accessing it directly, not through this SMC interface) and letting it
> expire.
>
> .