Re: [PATCH] clockevents: Introduce mode specific callbacks

[Peter Zijlstra <peterz@infradead.org>]

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.