Skip to content

Commit bd624d7

Browse files
vireshkIngo Molnar
vireshk
authored and
Ingo Molnar
committed
clockevents: Introduce mode specific callbacks
It is not possible for the clockevents core to know which modes (other than those with a corresponding feature flag) are supported by a particular implementation. And drivers are expected to handle transition to all modes elegantly, as ->set_mode() would be issued for them unconditionally. Now, adding support for a new mode complicates things a bit if we want to use the legacy ->set_mode() callback. We need to closely review all clockevents drivers to see if they would break on addition of a new mode. And after such reviews, it is found that we have to do non-trivial changes to most of the drivers [1]. Introduce mode-specific set_mode_*() callbacks, some of which the drivers may or may not implement. A missing callback would clearly convey the message that the corresponding mode isn't supported. A driver may still choose to keep supporting the legacy ->set_mode() callback, but ->set_mode() wouldn't be supporting any new modes beyond RESUME. If a driver wants to benefit from using a new mode, it would be required to migrate to the mode specific callbacks. The legacy ->set_mode() callback and the newly introduced mode-specific callbacks are mutually exclusive. Only one of them should be supported by the driver. Sanity check is done at the time of registration to distinguish between optional and required callbacks and to make error recovery and handling simpler. If the legacy ->set_mode() callback is provided, all mode specific ones would be ignored by the core but a warning is thrown if they are present. Call sites calling ->set_mode() directly are also updated to use __clockevents_set_mode() instead, as ->set_mode() may not be available anymore for few drivers. [1] https://lkml.org/lkml/2014/12/9/605 [2] https://lkml.org/lkml/2015/1/23/255 Suggested-by: Thomas Gleixner <tglx@linutronix.de> [2] Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Daniel Lezcano <daniel.lezcano@linaro.org> Cc: Frederic Weisbecker <fweisbec@gmail.com> Cc: John Stultz <john.stultz@linaro.org> Cc: Kevin Hilman <khilman@linaro.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Preeti U Murthy <preeti@linux.vnet.ibm.com> Cc: linaro-kernel@lists.linaro.org Cc: linaro-networking@linaro.org Link: http://lkml.kernel.org/r/792d59a40423f0acffc9bb0bec9de1341a06fa02.1423788565.git.viresh.kumar@linaro.org Signed-off-by: Ingo Molnar <mingo@kernel.org>
1 parent f40d149 commit bd624d7

File tree

3 files changed

+134
-7
lines changed

3 files changed

+134
-7
lines changed

‎include/linux/clockchips.h

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ enum clock_event_mode {
3939
CLOCK_EVT_MODE_PERIODIC,
4040
CLOCK_EVT_MODE_ONESHOT,
4141
CLOCK_EVT_MODE_RESUME,
42+
43+
/* Legacy ->set_mode() callback doesn't support below modes */
4244
};
4345

4446
/*
@@ -81,7 +83,11 @@ enum clock_event_mode {
8183
* @mode: operating mode assigned by the management code
8284
* @features: features
8385
* @retries: number of forced programming retries
84-
* @set_mode: set mode function
86+
* @set_mode: legacy set mode function, only for modes <= CLOCK_EVT_MODE_RESUME.
87+
* @set_mode_periodic: switch mode to periodic, if !set_mode
88+
* @set_mode_oneshot: switch mode to oneshot, if !set_mode
89+
* @set_mode_shutdown: switch mode to shutdown, if !set_mode
90+
* @set_mode_resume: resume clkevt device, if !set_mode
8591
* @broadcast: function to broadcast events
8692
* @min_delta_ticks: minimum delta value in ticks stored for reconfiguration
8793
* @max_delta_ticks: maximum delta value in ticks stored for reconfiguration
@@ -108,9 +114,20 @@ struct clock_event_device {
108114
unsigned int features;
109115
unsigned long retries;
110116

111-
void (*broadcast)(const struct cpumask *mask);
117+
/*
118+
* Mode transition callback(s): Only one of the two groups should be
119+
* defined:
120+
* - set_mode(), only for modes <= CLOCK_EVT_MODE_RESUME.
121+
* - set_mode_{shutdown|periodic|oneshot|resume}().
122+
*/
112123
void (*set_mode)(enum clock_event_mode mode,
113124
struct clock_event_device *);
125+
int (*set_mode_periodic)(struct clock_event_device *);
126+
int (*set_mode_oneshot)(struct clock_event_device *);
127+
int (*set_mode_shutdown)(struct clock_event_device *);
128+
int (*set_mode_resume)(struct clock_event_device *);
129+
130+
void (*broadcast)(const struct cpumask *mask);
114131
void (*suspend)(struct clock_event_device *);
115132
void (*resume)(struct clock_event_device *);
116133
unsigned long min_delta_ticks;

‎kernel/time/clockevents.c

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,57 @@ u64 clockevent_delta2ns(unsigned long latch, struct clock_event_device *evt)
9494
}
9595
EXPORT_SYMBOL_GPL(clockevent_delta2ns);
9696

97+
static int __clockevents_set_mode(struct clock_event_device *dev,
98+
enum clock_event_mode mode)
99+
{
100+
/* Transition with legacy set_mode() callback */
101+
if (dev->set_mode) {
102+
/* Legacy callback doesn't support new modes */
103+
if (mode > CLOCK_EVT_MODE_RESUME)
104+
return -ENOSYS;
105+
dev->set_mode(mode, dev);
106+
return 0;
107+
}
108+
109+
if (dev->features & CLOCK_EVT_FEAT_DUMMY)
110+
return 0;
111+
112+
/* Transition with new mode-specific callbacks */
113+
switch (mode) {
114+
case CLOCK_EVT_MODE_UNUSED:
115+
/*
116+
* This is an internal state, which is guaranteed to go from
117+
* SHUTDOWN to UNUSED. No driver interaction required.
118+
*/
119+
return 0;
120+
121+
case CLOCK_EVT_MODE_SHUTDOWN:
122+
return dev->set_mode_shutdown(dev);
123+
124+
case CLOCK_EVT_MODE_PERIODIC:
125+
/* Core internal bug */
126+
if (!(dev->features & CLOCK_EVT_FEAT_PERIODIC))
127+
return -ENOSYS;
128+
return dev->set_mode_periodic(dev);
129+
130+
case CLOCK_EVT_MODE_ONESHOT:
131+
/* Core internal bug */
132+
if (!(dev->features & CLOCK_EVT_FEAT_ONESHOT))
133+
return -ENOSYS;
134+
return dev->set_mode_oneshot(dev);
135+
136+
case CLOCK_EVT_MODE_RESUME:
137+
/* Optional callback */
138+
if (dev->set_mode_resume)
139+
return dev->set_mode_resume(dev);
140+
else
141+
return 0;
142+
143+
default:
144+
return -ENOSYS;
145+
}
146+
}
147+
97148
/**
98149
* clockevents_set_mode - set the operating mode of a clock event device
99150
* @dev: device to modify
@@ -105,7 +156,9 @@ void clockevents_set_mode(struct clock_event_device *dev,
105156
enum clock_event_mode mode)
106157
{
107158
if (dev->mode != mode) {
108-
dev->set_mode(mode, dev);
159+
if (__clockevents_set_mode(dev, mode))
160+
return;
161+
109162
dev->mode = mode;
110163

111164
/*
@@ -373,6 +426,35 @@ int clockevents_unbind_device(struct clock_event_device *ced, int cpu)
373426
}
374427
EXPORT_SYMBOL_GPL(clockevents_unbind);
375428

429+
/* Sanity check of mode transition callbacks */
430+
static int clockevents_sanity_check(struct clock_event_device *dev)
431+
{
432+
/* Legacy set_mode() callback */
433+
if (dev->set_mode) {
434+
/* We shouldn't be supporting new modes now */
435+
WARN_ON(dev->set_mode_periodic || dev->set_mode_oneshot ||
436+
dev->set_mode_shutdown || dev->set_mode_resume);
437+
return 0;
438+
}
439+
440+
if (dev->features & CLOCK_EVT_FEAT_DUMMY)
441+
return 0;
442+
443+
/* New mode-specific callbacks */
444+
if (!dev->set_mode_shutdown)
445+
return -EINVAL;
446+
447+
if ((dev->features & CLOCK_EVT_FEAT_PERIODIC) &&
448+
!dev->set_mode_periodic)
449+
return -EINVAL;
450+
451+
if ((dev->features & CLOCK_EVT_FEAT_ONESHOT) &&
452+
!dev->set_mode_oneshot)
453+
return -EINVAL;
454+
455+
return 0;
456+
}
457+
376458
/**
377459
* clockevents_register_device - register a clock event device
378460
* @dev: device to register
@@ -382,6 +464,8 @@ void clockevents_register_device(struct clock_event_device *dev)
382464
unsigned long flags;
383465

384466
BUG_ON(dev->mode != CLOCK_EVT_MODE_UNUSED);
467+
BUG_ON(clockevents_sanity_check(dev));
468+
385469
if (!dev->cpumask) {
386470
WARN_ON(num_possible_cpus() > 1);
387471
dev->cpumask = cpumask_of(smp_processor_id());
@@ -449,7 +533,7 @@ int __clockevents_update_freq(struct clock_event_device *dev, u32 freq)
449533
return clockevents_program_event(dev, dev->next_event, false);
450534

451535
if (dev->mode == CLOCK_EVT_MODE_PERIODIC)
452-
dev->set_mode(CLOCK_EVT_MODE_PERIODIC, dev);
536+
return __clockevents_set_mode(dev, CLOCK_EVT_MODE_PERIODIC);
453537

454538
return 0;
455539
}

‎kernel/time/timer_list.c

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,9 +228,35 @@ print_tickdevice(struct seq_file *m, struct tick_device *td, int cpu)
228228
print_name_offset(m, dev->set_next_event);
229229
SEQ_printf(m, "\n");
230230

231-
SEQ_printf(m, " set_mode: ");
232-
print_name_offset(m, dev->set_mode);
233-
SEQ_printf(m, "\n");
231+
if (dev->set_mode) {
232+
SEQ_printf(m, " set_mode: ");
233+
print_name_offset(m, dev->set_mode);
234+
SEQ_printf(m, "\n");
235+
} else {
236+
if (dev->set_mode_shutdown) {
237+
SEQ_printf(m, " shutdown: ");
238+
print_name_offset(m, dev->set_mode_shutdown);
239+
SEQ_printf(m, "\n");
240+
}
241+
242+
if (dev->set_mode_periodic) {
243+
SEQ_printf(m, " periodic: ");
244+
print_name_offset(m, dev->set_mode_periodic);
245+
SEQ_printf(m, "\n");
246+
}
247+
248+
if (dev->set_mode_oneshot) {
249+
SEQ_printf(m, " oneshot: ");
250+
print_name_offset(m, dev->set_mode_oneshot);
251+
SEQ_printf(m, "\n");
252+
}
253+
254+
if (dev->set_mode_resume) {
255+
SEQ_printf(m, " resume: ");
256+
print_name_offset(m, dev->set_mode_resume);
257+
SEQ_printf(m, "\n");
258+
}
259+
}
234260

235261
SEQ_printf(m, " event_handler: ");
236262
print_name_offset(m, dev->event_handler);

0 commit comments

Comments
 (0)