Viresh Kumar viresh.kumar@linaro.org writes:
Hi Preeti/Daniel/others,
There is a requirement to add another mode: CLOCK_EVT_MODE_ONESHOT_STOPPED (lkml.org/lkml/2014/5/9/508)
Why?
I already understand why, and think it's a good goal, but you can't expect reviewers to read the whole background thread and understand. You should summarize the reasons behind needing the new mode (e.g. so timers don't continue to run/overflow when unused.)
To help with understanding, it also might be worth describing a hypothetical simplified solution (like each clockevent driver's ONESHOT mode explicitly checking for KTIME_MAX) but make the case that this should be handled by core code.
to clockevent devices and clockevent-drivers may or maynot support it.
This needs rewording too. I think what you mean is that the new _ONESHOT_STOPPED mode is optional, so clockevent drivers do not have to implement it. But, because of the switch statments used in clockevent drivers, all of them need review/audit/change.
Drivers that don't support it can return failure codes on a call to ->set_mode(), which has a return type of 'void' currently.
Probably a question for the series which actually implments ONESHOT_SHUTDOWN, but wouldn't it be more sane default for drivers that don't support it to just fallback to ONESHOT instead of error out?
Following steps are suggested (by tglx) to get this fixed:
- Add another callback ->set_dev_mode(), with return type 'int'.
- Covert clockevent drivers to use ->set_dev_mode() instead of ->set_mode().
- Once all are converted, remove support for ->set_mode().
Overall, your changelogs need some work.
The part above is duplicated in all the patches in the series, and is not helpful. The cover letter is good for describing the big picture, but each patch should have it's own descriptive changelog.
Kevin