Re: [PATCH v2 2/2] watchdog: Add new arm_smd_wdt watchdog driver

From: Evan Benn
Date: Fri Apr 03 2020 - 21:07:55 EST


On Sat, Apr 4, 2020 at 9:56 AM Julius Werner <jwerner@xxxxxxxxxxxx> wrote:
>
> > + wdd->info = &smcwd_info;
> > + /* get_timeleft is optional */
> > + if (smcwd_call(SMCWD_GET_TIMELEFT, 0, NULL))
>
> How is this supposed to work? A firmware that implements this call
> would return the time left here which may not be 0 (maybe the watchdog
> was already primed by the bootloader or whatever), so smcwd_call()
> would interpret it as an error.
>
> I think the cleanest solution would be to stick to the same return
> codes in a0 and use a1 to report the time left when a0 is
> PSCI_SUCCESS. This is more consistent with SMCWD_INIT too.

Yes you are right, I have the wrong return code in the get_timeleft
implementation. It should use ->a1 for the actual timeleft, a0 is for
the error code.

Here smcwd_call returns the error code, which is NOT_IMPLEMENTED if
the firmware does not implement timeleft. The timeleft itself cannot
return error codes else we would just return that there I guess.