The CMT do runtime PM and call clk_enable()/clk_disable() when a new clock event is register and the CMT is not already started. This is not always possible as a spinlock is also needed to sync the internals of the CMT. Running with PROVE_LOCKING uncovers one such issue. ============================= [ BUG: Invalid wait context ] 6.17.0-rc3-arm64-renesas-03071-gb3c4f4122b28-dirty #21 Not tainted ----------------------------- swapper/1/0 is trying to lock: ffff00000898d180 (&dev->power.lock){-...}-{3:3}, at: __pm_runtime_resume+0x38/0x88 ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version 0xAF400001/0xDCC63000, Driver version 5.0 other info that might help us debug this: ccree e6601000.crypto: ARM ccree device initialized context-{5:5} 2 locks held by swapper/1/0: #0: ffff80008173c298 (tick_broadcast_lock){-...}-{2:2}, at: __tick_broadcast_oneshot_control+0xa4/0x3a8 #1: ffff0000089a5858 (&ch->lock){....}-{2:2} usbcore: registered new interface driver usbhid , at: sh_cmt_start+0x30/0x364 stack backtrace: CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Not tainted 6.17.0-rc3-arm64-renesas-03071-gb3c4f4122b28-dirty #21 PREEMPT Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT) Call trace: show_stack+0x14/0x1c (C) dump_stack_lvl+0x6c/0x90 dump_stack+0x14/0x1c __lock_acquire+0x904/0x1584 lock_acquire+0x220/0x34c _raw_spin_lock_irqsave+0x58/0x80 __pm_runtime_resume+0x38/0x88 sh_cmt_start+0x54/0x364 sh_cmt_clock_event_set_oneshot+0x64/0xb8 clockevents_switch_state+0xfc/0x13c tick_broadcast_set_event+0x30/0xa4 __tick_broadcast_oneshot_control+0x1e0/0x3a8 tick_broadcast_oneshot_control+0x30/0x40 cpuidle_enter_state+0x40c/0x680 cpuidle_enter+0x30/0x40 do_idle+0x1f4/0x26c cpu_startup_entry+0x34/0x40 secondary_start_kernel+0x11c/0x13c __secondary_switched+0x74/0x78 Fix this by unconditionally powering on and enabling the needed clocks for all CMT channels which are used for clock events. Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx> --- drivers/clocksource/sh_cmt.c | 87 ++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 49 deletions(-) diff --git a/drivers/clocksource/sh_cmt.c b/drivers/clocksource/sh_cmt.c index 385eb94bbe7c..a57d5523f63b 100644 --- a/drivers/clocksource/sh_cmt.c +++ b/drivers/clocksource/sh_cmt.c @@ -5,6 +5,7 @@ * Copyright (C) 2008 Magnus Damm */ +#include <linux/cleanup.h> #include <linux/clk.h> #include <linux/clockchips.h> #include <linux/clocksource.h> @@ -623,51 +624,6 @@ static void sh_cmt_stop_clocksource(struct sh_cmt_channel *ch) pm_runtime_put(&ch->cmt->pdev->dev); } -static int sh_cmt_start_clockevent(struct sh_cmt_channel *ch) -{ - int ret = 0; - unsigned long flags; - - raw_spin_lock_irqsave(&ch->lock, flags); - - if (!(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) { - pm_runtime_get_sync(&ch->cmt->pdev->dev); - ret = sh_cmt_enable(ch); - } - - if (ret) - goto out; - - ch->flags |= FLAG_CLOCKEVENT; - out: - raw_spin_unlock_irqrestore(&ch->lock, flags); - - return ret; -} - -static void sh_cmt_stop_clockevent(struct sh_cmt_channel *ch) -{ - unsigned long flags; - unsigned long f; - - raw_spin_lock_irqsave(&ch->lock, flags); - - f = ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE); - - ch->flags &= ~FLAG_CLOCKEVENT; - - if (f && !(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) { - sh_cmt_disable(ch); - pm_runtime_put(&ch->cmt->pdev->dev); - } - - /* adjust the timeout to maximum if only clocksource left */ - if (ch->flags & FLAG_CLOCKSOURCE) - __sh_cmt_set_next(ch, ch->max_match_value); - - raw_spin_unlock_irqrestore(&ch->lock, flags); -} - static struct sh_cmt_channel *cs_to_sh_cmt(struct clocksource *cs) { return container_of(cs, struct sh_cmt_channel, cs); @@ -774,19 +730,30 @@ static struct sh_cmt_channel *ced_to_sh_cmt(struct clock_event_device *ced) static void sh_cmt_clock_event_start(struct sh_cmt_channel *ch, int periodic) { - sh_cmt_start_clockevent(ch); - if (periodic) sh_cmt_set_next(ch, ((ch->cmt->rate + HZ/2) / HZ) - 1); else sh_cmt_set_next(ch, ch->max_match_value); } +static void sh_cmt_clock_event_stop(struct sh_cmt_channel *ch) +{ + unsigned long flags; + + raw_spin_lock_irqsave(&ch->lock, flags); + + /* adjust the timeout to maximum if only clocksource left */ + if (ch->flags & FLAG_CLOCKSOURCE) + __sh_cmt_set_next(ch, ch->max_match_value); + + raw_spin_unlock_irqrestore(&ch->lock, flags); +} + static int sh_cmt_clock_event_shutdown(struct clock_event_device *ced) { struct sh_cmt_channel *ch = ced_to_sh_cmt(ced); - sh_cmt_stop_clockevent(ch); + sh_cmt_clock_event_stop(ch); return 0; } @@ -797,7 +764,7 @@ static int sh_cmt_clock_event_set_state(struct clock_event_device *ced, /* deal with old setting first */ if (clockevent_state_oneshot(ced) || clockevent_state_periodic(ced)) - sh_cmt_stop_clockevent(ch); + sh_cmt_clock_event_stop(ch); dev_info(&ch->cmt->pdev->dev, "ch%u: used for %s clock events\n", ch->index, periodic ? "periodic" : "oneshot"); @@ -908,6 +875,28 @@ static int sh_cmt_register(struct sh_cmt_channel *ch, const char *name, ret = sh_cmt_register_clockevent(ch, name); if (ret < 0) return ret; + + /* + * To support clock events the CMT must always be ready to + * accept new events, start it and leave no way for it to be + * turned off. + * + * This is OK as we can't never unregister a CMT device. If this + * is fixed in future an equal unconditional shutdown is needed. + */ + pm_runtime_get_sync(&ch->cmt->pdev->dev); + + scoped_guard(raw_spinlock_irqsave, &ch->lock) { + ret = sh_cmt_enable(ch); + if (ret) + return ret; + + /* + * Flag that the channel is used as a clock event, it's not + * allowed to be powered off! + */ + ch->flags |= FLAG_CLOCKEVENT; + } } if (clocksource) { -- 2.51.0