Hello Guenter,

On Tue, Oct 06, 2020 at 02:04:10PM -0700, Guenter Roeck wrote:
> On 10/6/20 11:41 AM, Uwe Kleine-König wrote:
> > Hello Guenter,
> > 
> > On Tue, Oct 06, 2020 at 07:29:11AM -0700, Guenter Roeck wrote:
> >> On 10/6/20 4:56 AM, Guenter Roeck wrote:
> >>> On 10/6/20 3:29 AM, Uwe Kleine-König wrote:
> >>>> Hello,
> >>>>
> >>>> I have an i.MX25 system here with an external watchdog (using the
> >>>> gpio_wdt driver). So the internal watchdog (imx2_wdt) is unused.
> >>>>
> >>>> The problem with the unused imx2_wdt is that this usually provides the
> >>>> restart handler and now a reboot ends with
> >>>>
> >>>> 	reboot: Restarting system
> >>>> 	Reboot failed -- System halted
> >>>>
> >>>> until eventually the watchdog bites and resets the machine.
> >>>>
> >>>> I imagine that this is a common enough issue to warrant a generic
> >>>> solution. My suggestion is to formalize and implement something like:
> >>>>
> >>>> 	watchdog {
> >>>> 		compatible = "linux,wdt-gpio";
> >>>> 		...
> >>>> 		provide-system-reset;
> >>>> 	}
> >>>>
> >>>> with the sematic of: "This is the dedicated mechanism to reset this
> >>>> machine."
> >>>>
> >>>
> >>> Some systems have more than one means to reset it, which is why
> >>> restart handlers have a priority. This in turn suggests that we should
> >>> maybe have a means to set that priority dynamically for the imx2_wdt driver
> >>> (or for watchdog drivers in general) instead of having it fixed at 128.
> >>> That would also solve your problem, assuming there is a different
> >>> (currently lower priority) means to reset the hardware in your system.
> >>>
> >>> Alternatively, can't you just blacklist the imx2-wdt driver ?
> >>
> >> After having another couple hours of sleep and a coffee, I wonder if
> >> this is already done, and the reboot just fails _because_ the imx2_wdt
> >> is _not_ loaded. Is that the case ?
> > 
> > Right, I disabled the imx2_wdt driver.
> >  
> >> If so, it looks like you want the reset functionality of the imx_wdt driver
> >> but not its watchdog functionality.
> > 
> > My thought was to use the gpio-watchdog as reset source, but using the
> > imx-watchdog only for reset but not watchdog is an obvious alternative I
> > didn't think about.
>
> It isn't really something I would have thought to ever be relevant: If
> a watchdog can be used to reset the system, and that method to reset
> the system is known to work and supposed to be used, why not use it as
> system watchdog ? So that use case is quite odd, especially since the
> watchdog on that system can apparently be used to trigger an external
> pin.

The motivation to use the external watchdog is that access to the MRAM
only works if the external watchdog is active. And (if I'm well
informed) this external watchdog was introduced because of some
regulation stuff where it must be guaranteed that there is a watchdog.
With an external watchdog this is much easier to do than with an SoC
internal one.

And then because using two watchdogs is ugly disabling the internal one
was straight forward even though it works just fine (apart from enabling
access to the MRAM).

> If we assume that there was a reason for not using the SoC watchdog,
> we must also assume that using it to reset the system does not really
> work (otherwise, what would be the point of having a separate gpio
> based watchdog in that system ?).
> 
> With that in mind, your other option kind of makes sense. The only
> question would be how to express this in devicetree. I am certainly
> open to accepting a patch introducing such a property/functionality
> into the watchdog core.

OK, will try to come up with a patch.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |