On Sun, 14 Jul 2013 12:43:56 -0700 Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > On Sun, Jul 14, 2013 at 07:09:59PM +0400, Alexander Shiyan wrote: > > This patch adds a watchdog driver for devices controlled through GPIO, > > (Analog Devices ADM706, for example). Driver written for DT-based systems > > only. No description for Documentation/devicetree yet. > > Comments are welcome. > > > > Signed-off-by: Alexander Shiyan <shc_work@xxxxxxx> > > --- > > drivers/watchdog/Kconfig | 8 +++ > > drivers/watchdog/Makefile | 1 + > > drivers/watchdog/gpio_wdt.c | 121 ++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 130 insertions(+) > > create mode 100644 drivers/watchdog/gpio_wdt.c [...] > Lots of context knowledge here. For a generic driver, how does one know > that toggling the output value triggers the ping ? > > Also, there is no mention that the toggle has to occur within 1.6 seconds. > Linux expects that watchdogs support much larger timeouts. I think it would > make sense to implement a soft-dog which actually triggers the ping, > and handles the higher level ping received from applications. > There are several other watchdog drivers implementing this approach. > > Overall, I think this should be a watchdog driver specifically for > ADM705/706/707/708. [...] > > + nowayout = of_property_read_bool(pdev->dev.of_node, "wdt,nowayout"); > > + if (!nowayout) > > + nowayout = WATCHDOG_NOWAYOUT; > > I don't think it is a good idea to introduce a driver specific > devicetreee property like this one. > > First, it is a configuration parameter and does not describe the hardware. as > such, a module parameter as implemented by other drivers would be more > appropriate. Second, even if a devicetree property was used, it should be > implemented in the watchdog core code and not in drivers. [...] Thanks for the comments. I will make v2 with this recommendation for more generic driver. There are a few ideas. -- Alexander Shiyan <shc_work@xxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
