On Wed, Nov 11, 2020 at 03:15:17PM +0000, Vaittinen, Matti wrote: [ ... ] > > > > + > > > > + priv->wdd.info = &bd957x_wdt_ident; > > > > + priv->wdd.ops = &bd957x_wdt_ops; > > > > + priv->wdd.min_hw_heartbeat_ms = hw_margin_min; > > > > + priv->wdd.max_hw_heartbeat_ms = hw_margin_max; > > > > + priv->wdd.parent = dev; > > > > + priv->wdd.timeout = (hw_margin_max / 2) * 1000; > > > > > > Hmm. Just noticed this value does not make sense, right? > > > Maximum hw_margin is 4416 ms. If I read this correctly timeout > > > should > > > be in seconds - so result is around 2 000 000 seconds here. I > > > think it > > > is useless value... > > > > > > Perhaps > > > priv->wdd.timeout = (hw_margin_max / 2) / 1000; > > > if (!priv->wdd.timeout) > > > priv->wdd.timeout = 1; > > > would be more appropriate. > > > > > > > Yes. Good catch. Actually, since max_hw_heartbeat_ms is specified, > > it can and should be a reasonable constant (like the usual 30 > > seconds). > > It does not and should not be bound by max_hw_heartbeat_ms. > > Thanks for confirming this Guenter. I'd better admit I didn't > understand how the max_hw_heartbeat_ms works. > > If I now read the code correctly, the "watchdog worker" takes care of > feeding for shorter periods than the "timeout" - and only stops > feeding max_hw_heartbeat_ms before timeout expires if userland has not > been feedin wdg. This is really cool approach for short(ish) > max_hw_heartbeat_ms configurations as user-space does not need to meet > "RT requirements". WDG framework is much more advanced that I knew :) Yes, exactly, that is the idea. Various drivers used to implement this within the driver, so it just made sense to pull the functionality into the watchdog core. Thanks, Guenter
