On Wed, Feb 11, 2015 at 11:24:53AM +0800, Viresh Kumar wrote:
On 10 February 2015 at 22:15, Peter Zijlstra peterz@infradead.org wrote:
On Wed, Feb 04, 2015 at 01:06:23PM +0530, Viresh Kumar wrote:
/** Mode transition callback(s): Only one of the two groups should be* defined:* - set_mode(), only for modes <= CLOCK_EVT_MODE_RESUME.* - set_mode_{shutdown|periodic|oneshot|resume}().*/
+static int clockevents_sanity_check(struct clock_event_device *dev) +{ +}
It appears to me you've not actually checked that condition outlined above, a driver could set both the legacy and the new callbacks.
Exactly for this reason I mentioned this in the logs:
If the legacy ->set_mode() callback is provided, all mode specific callbacks would be ignored.
So, either we can mention that in the code as well OR add code to check and WARN about that. Will do whatever looks better to you guys.
I think its better to be strict; esp. with new interfaces. It avoids confusion.
Suppose a driver writer sees these new methods and thinks to use one while still having the set_mode() one -- ie. he didn't actually read the comment. We'd better make sure he fails and goes back to read it.