watchdog: Extend kernel API and add early_timeout_sec feature
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v7 Latest posting 2/21 of 8 patches 0 repliesNew
- v7 posting 1/23 of 8 patches 0 repliesSuperseded
1 of 8 patches is in this archive. The cover letter announces 8.
Patches in v7 posting 2/2 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 8/8 | watchdog: omap_wdt: Convert to use new core extensions | +0 | 1 reply |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v7 yet.
Cover letter [PATCHv7 0/8] watchdog: Extend kernel API and add early_timeout_sec feature
Hello,
Quoted 21 lines · Timo Kokkonen
Quoted 11 lines · Uwe Kleine-K?nig
Hello,
I talked to Marc Kleine-Budde about your approach, thought a bit more about it and want to share a few thoughts with you.
Actually your series addresses three different problems
a) some watchdog hardware isn't stoppable;
b) some watchdog hardware has short maximal timeout;
c) what to do with a watchdog that is already running at probe time?Quoted 8 lines
The common solution is to add a mid-layer between userspace and the driver that bridges the possible hardware limitations when userspace wants to stop the watchdog or set a big timeout value. c) is a bit different but could make use of the infrastructure that is introduced while fixing a+b). The main difference between a+b) and c) is that for
c) you have to introduce some policy. If the series were mine I'd first do three commits that address a), b) and c) each. Then convert drivers to it.
Quoted 5 lines
The infrastructure needed for both a and b is the same, the actual code to implement either a or b is also touching the same functions and quite slim, so I think it may not be feasible to separate those in their own patches. But I'll think about it and see if logical separation makes sense.
Yeah, probably you have to actually try it to see if it's sensible.
Quoted 8 lines
Quoted 7 lines
Guenter and I already said something similar, but I will eventually repeat it here more explicitly: When introducing a midlayer that abstracts between hardware and it's users the IMHO most important thing to get right is to be explicit about which side of a midlayer you're currently working at. That is, be explicit about
watchdog_is_running:
Does it mean the hardware is running, or does userspace believe the watchdog to be active? Same for timeout, stoppable etc.pp.
Quoted 12 lines
Yes, I agree. Thanks for clarifying this. I will keep this in mind and try to be explicit when naming functions and variables. The best way would be to also rename the existing variables too, but that would also touch all the existing drivers. Keeping the current naming scheme in place no doubt is less explicit about which side of the new midlayer they are working on. But luckily the logic of the existing driver doesn't change, they are all exposing things directly to userland and they can be documented as is. The new variables and functions add interfaces only between the new midlayer and HW, so they are explicit as well. So I think it is a fair compromise to leave all existing interface towards the user space as is.
Maybe a good first step before addressing a+b+c) is to cleanup the naming? Not sure how well this works though.
Quoted 5 lines
Quoted 3 lines
When you consider changing the unit the watchdog core is using, why not change to nano seconds and 64 bit variables? You might be able to copy some algorithms and ideas from the timer core that uses these.
Yes, sounds like a good idea. I will look into it.
I wouldn't be surprised if you had to build a parallel watchdog device model for that and assert that both work until all drivers are converted to the new model. Sounds like fun :-)
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |