[PATCH v2 14/40] clocksource: sh_cmt: Set cpumask to cpu_possible_mask

[PATCH v2 14/40] clocksource: sh_cmt: Set cpumask to cpu_possible_mask

[Laurent Pinchart]

The CMT is a global timer not restricted to a single CPU. It has a lower
rating than the TMU or ARM architected timer, but is still useful on
systems where the other timers are stopped during CPU sleep.

When multiple timers are available the timers core selects which timer
to use based on timer ratings.

On SMP systems where timer broadcasting is required, one dummy timer is
instantiated per CPU with a rating of 100. On those systems the CMT
timer has a rating of 80, which makes the dummy timer selected by
default on all CPUs. The CMT is then available, and will be used as a
broadcast timer.

On UP systems no dummy timer is instantiated. The CMT timer has a rating
of 125 on those systems and is used directly a clock event device for
CPU0 without broadcasting.

The CMT rating shouldn't depend on whether we boot a UP or SMP system.
We can't raise the CMT rating to 125 on SMP systems. This would select
CMT as the clock event device for CPU0 as its rating is higher than the
dummy timer rating, and would leave the system without a broadcast
timer. We could instead lower the rating to 80 on all systems, but that
wouldn't reflect reality as ratings between 1 and 99 are documented as
"unfit for real use".

We should raise the rating above 99 and still have the CMT selected as a
broadcast timer. This can be done by changing the cpumask from
cpumask_of(0) to cpu_possible_mask. In that case the timer selection
logic will prefer the previously probed and already selected dummy timer
for all CPUs based on the fact that already selected per-cpu timers are
preferred over new global timers, regardless of their respective
ratings. This also better reflects reality, as the CMT is not tied to
the boot CPU.

Ideally the timer selection logic should realize that the CMT needs to
be used as a broadcast timer on SMP systems as no other broadcast timer
is available, regardless of the cpumask and rating.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/clocksource/sh_cmt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/sh_cmt.c b/drivers/clocksource/sh_cmt.c
index ce00baa..926abe2 100644
--- a/drivers/clocksource/sh_cmt.c
+++ b/drivers/clocksource/sh_cmt.c
@@ -776,7 +776,7 @@ static void sh_cmt_register_clockevent(struct sh_cmt_channel *ch,
 	ced->features = CLOCK_EVT_FEAT_PERIODIC;
 	ced->features |= CLOCK_EVT_FEAT_ONESHOT;
 	ced->rating = rating;
-	ced->cpumask = cpumask_of(0);
+	ced->cpumask = cpu_possible_mask;
 	ced->set_next_event = sh_cmt_clock_event_next;
 	ced->set_mode = sh_cmt_clock_event_mode;
 	ced->suspend = sh_cmt_clock_event_suspend;
-- 
1.8.3.2

Re: [PATCH v2 14/40] clocksource: sh_cmt: Set cpumask to cpu_possible_mask

[Geert Uytterhoeven]

Hi Laurent,

On Fri, Feb 28, 2014 at 4:57 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> On UP systems no dummy timer is instantiated. The CMT timer has a rating
> of 125 on those systems and is used directly a clock event device for

as clock event device.

> CPU0 without broadcasting.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 14/40] clocksource: sh_cmt: Set cpumask to cpu_possible_mask

[Laurent Pinchart]

Hi Geert,

Thank you for the review.

On Saturday 01 March 2014 18:52:00 Geert Uytterhoeven wrote:
> Hi Laurent,
> 
> On Fri, Feb 28, 2014 at 4:57 PM, Laurent Pinchart
> 
> <laurent.pinchart+renesas@ideasonboard.com> wrote:
> > On UP systems no dummy timer is instantiated. The CMT timer has a rating
> > of 125 on those systems and is used directly a clock event device for
> 
> as clock event device.

I'll fix that in v3.

> > CPU0 without broadcasting.

-- 
Regards,

Laurent Pinchart