Re: [PATCH v2 5/7] Watchdog: introduce "pretimeout" into framework

[Guenter Roeck <linux@roeck-us.net>]

On 05/22/2015 05:14 AM, Timo Kokkonen wrote:
[ ... ]

> This is the direction we could go eventually. Of course not all features are in interest for everyone and it may not make sense to try implement everything in the core. But we have a lot of drivers that implement all sorts of timers in them and my patch set could eliminate a lot of code from them. We also have couple of drivers that have pretimeout support and there seems to be a desire make that generic as well. Obviously I think it makes sense to combine this effort in order to avoid conflicts.
>
I would suggest to avoid trying to combine efforts. If anything, I would suggest
to _separate_ patch sets.

> So I am too hoping more guidelines on what is the acceptable direction where to aim at. For example we could make this parameter handling more generic and future proof if we allow an API change that require small change to all drivers. I don't know exactly where the line is drawn.
>
As I just mentioned separately, the more changes you make, the less likely
it will be to get your changes accepted. A patch set requiring changes to
all drivers would have to provide substantial benefits to get accepted.
Talking about parameters (and assuming you mean module parameters), I don't
even see how you could make that work without changing the userspace ABI,
which would be a no-go.

Personally I think it would be easier if you tried to accomplish less
than more. Concentrate on early-timeout (only), get it merged. Provide
a clear separation between is_active (as in "watchdog running") from
is_open (userspace has the driver open) and get it merged. Add the
ability to "ping" a running watchdog while the driver is closed
into the watchdog core and get it merged. Add the ability to soft-ping
a watchdog while the driver is open into the core and get it merged.
Those are all logically separate functions, and could be introduced
separately.

Just my personal opinion, of course.

Thanks,
Guenter