Hi, This series adds a driver for PAPR hypercall-based watchdog timers, tentatively named "pseries-wdt". I wanted to get some clarification on a few things before submitting the series as a patch, hence the RFC. In particular: - In pseries_wdt_probe() we register the watchdog device with devm_watchdog_register_device(). However, in pseries_wdt_remove(), calling watchdog_unregister_devce() causes a kernel panic later, so I assume this is the wrong thing to do. Do we need to do anything to clean up the watchdog device during pseries_wdt_remove()? Or does devm_watchdog_register_device() ensure the cleanup is handled transparently? - In pseries_wdt_probe(), is it incorrect to devm_kfree() my allocation in the event that devm_watchdog_register_device() fails? - The enormous hypercall input/output comment is mostly for my edification. It seems like the sort of thing that will rot over time. I intend to remove most of it. However, as far as I know the PAPR revision containing these details is not published yet. Should I leave the comment in to ease review for now and remove it later? - Should we print something to the console when probing/removing the watchdog0 device or is that just noise? Most drivers (as distinct from devices) seem to print something during initialization, so that's what I've done in pseries_wdt_module_init() when the capability query succeeds. - The timeout action is currently hardcoded to a hard reset. This could be made configurable through a module parameter. I intend to do this in a later patch unless someone needs it included in the initial patch. - We set EIO if the hypercall fails in pseries_wdt_start() or pseries_wdt_stop(). There is nothing userspace can do if this happens. All hypercall failures in these contexts are unexpected. Given all of that, is there is a more appropriate errno than EIO? - The H_WATCHDOG spec indicates that H_BUSY is possible. Is it probable, though? Should I spin and retry the hypercall in the event that we see it? Or is that pointless?
