OpenCloudOS-Kernel/drivers/clocksource/sh_cmt.c

1146 lines
28 KiB
C
Raw Normal View History

// SPDX-License-Identifier: GPL-2.0
/*
* SuperH Timer Support - CMT
*
* Copyright (C) 2008 Magnus Damm
*/
#include <linux/clk.h>
#include <linux/clockchips.h>
#include <linux/clocksource.h>
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/ioport.h>
#include <linux/irq.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/pm_domain.h>
#include <linux/pm_runtime.h>
#include <linux/sh_timer.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
#ifdef CONFIG_SUPERH
#include <asm/platform_early.h>
#endif
struct sh_cmt_device;
/*
* The CMT comes in 5 different identified flavours, depending not only on the
* SoC but also on the particular instance. The following table lists the main
* characteristics of those flavours.
*
* 16B 32B 32B-F 48B R-Car Gen2
* -----------------------------------------------------------------------------
* Channels 2 1/4 1 6 2/8
* Control Width 16 16 16 16 32
* Counter Width 16 32 32 32/48 32/48
* Shared Start/Stop Y Y Y Y N
*
* The r8a73a4 / R-Car Gen2 version has a per-channel start/stop register
* located in the channel registers block. All other versions have a shared
* start/stop register located in the global space.
*
* Channels are indexed from 0 to N-1 in the documentation. The channel index
* infers the start/stop bit position in the control register and the channel
* registers block address. Some CMT instances have a subset of channels
* available, in which case the index in the documentation doesn't match the
* "real" index as implemented in hardware. This is for instance the case with
* CMT0 on r8a7740, which is a 32-bit variant with a single channel numbered 0
* in the documentation but using start/stop bit 5 and having its registers
* block at 0x60.
*
* Similarly CMT0 on r8a73a4, r8a7790 and r8a7791, while implementing 32-bit
* channels only, is a 48-bit gen2 CMT with the 48-bit channels unavailable.
*/
enum sh_cmt_model {
SH_CMT_16BIT,
SH_CMT_32BIT,
SH_CMT_48BIT,
SH_CMT0_RCAR_GEN2,
SH_CMT1_RCAR_GEN2,
};
struct sh_cmt_info {
enum sh_cmt_model model;
unsigned int channels_mask;
unsigned long width; /* 16 or 32 bit version of hardware block */
u32 overflow_bit;
u32 clear_bits;
/* callbacks for CMSTR and CMCSR access */
u32 (*read_control)(void __iomem *base, unsigned long offs);
void (*write_control)(void __iomem *base, unsigned long offs,
u32 value);
/* callbacks for CMCNT and CMCOR access */
u32 (*read_count)(void __iomem *base, unsigned long offs);
void (*write_count)(void __iomem *base, unsigned long offs, u32 value);
};
struct sh_cmt_channel {
struct sh_cmt_device *cmt;
unsigned int index; /* Index in the documentation */
unsigned int hwidx; /* Real hardware index */
void __iomem *iostart;
void __iomem *ioctrl;
unsigned int timer_bit;
unsigned long flags;
u32 match_value;
u32 next_match_value;
u32 max_match_value;
raw_spinlock_t lock;
struct clock_event_device ced;
struct clocksource cs;
u64 total_cycles;
bool cs_enabled;
};
struct sh_cmt_device {
struct platform_device *pdev;
const struct sh_cmt_info *info;
void __iomem *mapbase;
struct clk *clk;
clocksource: sh_cmt: Compute rate before registration again With the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration. Currently, sh_cmt violates this requirement in that it registers its clockevent device with a dummy rate and sets its final ->mult and ->shift values from its ->set_state_oneshot() and ->set_state_periodic() functions respectively. This patch moves the setting of the clockevent device's ->mult and ->shift values to before its registration. Note that there has been some back and forth regarding this question with respect to the clocksource also provided by this driver: commit f4d7c3565c16 ("clocksource: sh_cmt: compute mult and shift before registration") moves the rate determination from the clocksource's ->enable() function to before its registration. OTOH, the later commit 3593f5fe40a1 ("clocksource: sh_cmt: __clocksource_updatefreq_hz() update") basically reverts this, saying "Without this patch the old code uses clocksource_register() together with a hack that assumes a never changing clock rate." However, I checked all current sh_cmt users in arch/sh as well as in arch/arm/mach-shmobile carefully and right now, none of them changes any rate in any clock tree relevant to sh_cmt after their respective time_init(). Since all sh_cmt instances are created after time_init(), none of them should ever observe any clock rate changes. What's more, both, a clocksource as well as a clockevent device, can immediately get selected for use at their registration and thus, enabled at this point already. So it's probably safer to assume a "never changing clock rate" here. - Move the struct sh_cmt_channel's ->rate member to struct sh_cmt_device: it's a property of the underlying clock which is in turn specific to the sh_cmt_device. - Determine the ->rate value in sh_cmt_setup() at device probing rather than at first usage. - Set the clockevent device's ->mult and ->shift values right before its registration. - Although not strictly necessary for the upcoming clockevent core changes, set the clocksource's rate at its registration for consistency. Signed-off-by: Nicolai Stange <nicstange@gmail.com> Signed-off-by: John Stultz <john.stultz@linaro.org>
2017-02-07 05:11:59 +08:00
unsigned long rate;
raw_spinlock_t lock; /* Protect the shared start/stop register */
struct sh_cmt_channel *channels;
unsigned int num_channels;
unsigned int hw_channels;
bool has_clockevent;
bool has_clocksource;
};
#define SH_CMT16_CMCSR_CMF (1 << 7)
#define SH_CMT16_CMCSR_CMIE (1 << 6)
#define SH_CMT16_CMCSR_CKS8 (0 << 0)
#define SH_CMT16_CMCSR_CKS32 (1 << 0)
#define SH_CMT16_CMCSR_CKS128 (2 << 0)
#define SH_CMT16_CMCSR_CKS512 (3 << 0)
#define SH_CMT16_CMCSR_CKS_MASK (3 << 0)
#define SH_CMT32_CMCSR_CMF (1 << 15)
#define SH_CMT32_CMCSR_OVF (1 << 14)
#define SH_CMT32_CMCSR_WRFLG (1 << 13)
#define SH_CMT32_CMCSR_STTF (1 << 12)
#define SH_CMT32_CMCSR_STPF (1 << 11)
#define SH_CMT32_CMCSR_SSIE (1 << 10)
#define SH_CMT32_CMCSR_CMS (1 << 9)
#define SH_CMT32_CMCSR_CMM (1 << 8)
#define SH_CMT32_CMCSR_CMTOUT_IE (1 << 7)
#define SH_CMT32_CMCSR_CMR_NONE (0 << 4)
#define SH_CMT32_CMCSR_CMR_DMA (1 << 4)
#define SH_CMT32_CMCSR_CMR_IRQ (2 << 4)
#define SH_CMT32_CMCSR_CMR_MASK (3 << 4)
#define SH_CMT32_CMCSR_DBGIVD (1 << 3)
#define SH_CMT32_CMCSR_CKS_RCLK8 (4 << 0)
#define SH_CMT32_CMCSR_CKS_RCLK32 (5 << 0)
#define SH_CMT32_CMCSR_CKS_RCLK128 (6 << 0)
#define SH_CMT32_CMCSR_CKS_RCLK1 (7 << 0)
#define SH_CMT32_CMCSR_CKS_MASK (7 << 0)
static u32 sh_cmt_read16(void __iomem *base, unsigned long offs)
{
return ioread16(base + (offs << 1));
}
static u32 sh_cmt_read32(void __iomem *base, unsigned long offs)
{
return ioread32(base + (offs << 2));
}
static void sh_cmt_write16(void __iomem *base, unsigned long offs, u32 value)
{
iowrite16(value, base + (offs << 1));
}
static void sh_cmt_write32(void __iomem *base, unsigned long offs, u32 value)
{
iowrite32(value, base + (offs << 2));
}
static const struct sh_cmt_info sh_cmt_info[] = {
[SH_CMT_16BIT] = {
.model = SH_CMT_16BIT,
.width = 16,
.overflow_bit = SH_CMT16_CMCSR_CMF,
.clear_bits = ~SH_CMT16_CMCSR_CMF,
.read_control = sh_cmt_read16,
.write_control = sh_cmt_write16,
.read_count = sh_cmt_read16,
.write_count = sh_cmt_write16,
},
[SH_CMT_32BIT] = {
.model = SH_CMT_32BIT,
.width = 32,
.overflow_bit = SH_CMT32_CMCSR_CMF,
.clear_bits = ~(SH_CMT32_CMCSR_CMF | SH_CMT32_CMCSR_OVF),
.read_control = sh_cmt_read16,
.write_control = sh_cmt_write16,
.read_count = sh_cmt_read32,
.write_count = sh_cmt_write32,
},
[SH_CMT_48BIT] = {
.model = SH_CMT_48BIT,
.channels_mask = 0x3f,
.width = 32,
.overflow_bit = SH_CMT32_CMCSR_CMF,
.clear_bits = ~(SH_CMT32_CMCSR_CMF | SH_CMT32_CMCSR_OVF),
.read_control = sh_cmt_read32,
.write_control = sh_cmt_write32,
.read_count = sh_cmt_read32,
.write_count = sh_cmt_write32,
},
[SH_CMT0_RCAR_GEN2] = {
.model = SH_CMT0_RCAR_GEN2,
.channels_mask = 0x60,
.width = 32,
.overflow_bit = SH_CMT32_CMCSR_CMF,
.clear_bits = ~(SH_CMT32_CMCSR_CMF | SH_CMT32_CMCSR_OVF),
.read_control = sh_cmt_read32,
.write_control = sh_cmt_write32,
.read_count = sh_cmt_read32,
.write_count = sh_cmt_write32,
},
[SH_CMT1_RCAR_GEN2] = {
.model = SH_CMT1_RCAR_GEN2,
.channels_mask = 0xff,
.width = 32,
.overflow_bit = SH_CMT32_CMCSR_CMF,
.clear_bits = ~(SH_CMT32_CMCSR_CMF | SH_CMT32_CMCSR_OVF),
.read_control = sh_cmt_read32,
.write_control = sh_cmt_write32,
.read_count = sh_cmt_read32,
.write_count = sh_cmt_write32,
},
};
#define CMCSR 0 /* channel register */
#define CMCNT 1 /* channel register */
#define CMCOR 2 /* channel register */
#define CMCLKE 0x1000 /* CLK Enable Register (R-Car Gen2) */
static inline u32 sh_cmt_read_cmstr(struct sh_cmt_channel *ch)
{
if (ch->iostart)
return ch->cmt->info->read_control(ch->iostart, 0);
else
return ch->cmt->info->read_control(ch->cmt->mapbase, 0);
}
static inline void sh_cmt_write_cmstr(struct sh_cmt_channel *ch, u32 value)
{
if (ch->iostart)
ch->cmt->info->write_control(ch->iostart, 0, value);
else
ch->cmt->info->write_control(ch->cmt->mapbase, 0, value);
}
static inline u32 sh_cmt_read_cmcsr(struct sh_cmt_channel *ch)
{
return ch->cmt->info->read_control(ch->ioctrl, CMCSR);
}
static inline void sh_cmt_write_cmcsr(struct sh_cmt_channel *ch, u32 value)
{
ch->cmt->info->write_control(ch->ioctrl, CMCSR, value);
}
static inline u32 sh_cmt_read_cmcnt(struct sh_cmt_channel *ch)
{
return ch->cmt->info->read_count(ch->ioctrl, CMCNT);
}
static inline void sh_cmt_write_cmcnt(struct sh_cmt_channel *ch, u32 value)
{
ch->cmt->info->write_count(ch->ioctrl, CMCNT, value);
}
static inline void sh_cmt_write_cmcor(struct sh_cmt_channel *ch, u32 value)
{
ch->cmt->info->write_count(ch->ioctrl, CMCOR, value);
}
static u32 sh_cmt_get_counter(struct sh_cmt_channel *ch, u32 *has_wrapped)
{
u32 v1, v2, v3;
u32 o1, o2;
o1 = sh_cmt_read_cmcsr(ch) & ch->cmt->info->overflow_bit;
/* Make sure the timer value is stable. Stolen from acpi_pm.c */
do {
o2 = o1;
v1 = sh_cmt_read_cmcnt(ch);
v2 = sh_cmt_read_cmcnt(ch);
v3 = sh_cmt_read_cmcnt(ch);
o1 = sh_cmt_read_cmcsr(ch) & ch->cmt->info->overflow_bit;
} while (unlikely((o1 != o2) || (v1 > v2 && v1 < v3)
|| (v2 > v3 && v2 < v1) || (v3 > v1 && v3 < v2)));
*has_wrapped = o1;
return v2;
}
static void sh_cmt_start_stop_ch(struct sh_cmt_channel *ch, int start)
{
unsigned long flags;
u32 value;
/* start stop register shared by multiple timer channels */
raw_spin_lock_irqsave(&ch->cmt->lock, flags);
value = sh_cmt_read_cmstr(ch);
if (start)
value |= 1 << ch->timer_bit;
else
value &= ~(1 << ch->timer_bit);
sh_cmt_write_cmstr(ch, value);
raw_spin_unlock_irqrestore(&ch->cmt->lock, flags);
}
clocksource: sh_cmt: Compute rate before registration again With the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration. Currently, sh_cmt violates this requirement in that it registers its clockevent device with a dummy rate and sets its final ->mult and ->shift values from its ->set_state_oneshot() and ->set_state_periodic() functions respectively. This patch moves the setting of the clockevent device's ->mult and ->shift values to before its registration. Note that there has been some back and forth regarding this question with respect to the clocksource also provided by this driver: commit f4d7c3565c16 ("clocksource: sh_cmt: compute mult and shift before registration") moves the rate determination from the clocksource's ->enable() function to before its registration. OTOH, the later commit 3593f5fe40a1 ("clocksource: sh_cmt: __clocksource_updatefreq_hz() update") basically reverts this, saying "Without this patch the old code uses clocksource_register() together with a hack that assumes a never changing clock rate." However, I checked all current sh_cmt users in arch/sh as well as in arch/arm/mach-shmobile carefully and right now, none of them changes any rate in any clock tree relevant to sh_cmt after their respective time_init(). Since all sh_cmt instances are created after time_init(), none of them should ever observe any clock rate changes. What's more, both, a clocksource as well as a clockevent device, can immediately get selected for use at their registration and thus, enabled at this point already. So it's probably safer to assume a "never changing clock rate" here. - Move the struct sh_cmt_channel's ->rate member to struct sh_cmt_device: it's a property of the underlying clock which is in turn specific to the sh_cmt_device. - Determine the ->rate value in sh_cmt_setup() at device probing rather than at first usage. - Set the clockevent device's ->mult and ->shift values right before its registration. - Although not strictly necessary for the upcoming clockevent core changes, set the clocksource's rate at its registration for consistency. Signed-off-by: Nicolai Stange <nicstange@gmail.com> Signed-off-by: John Stultz <john.stultz@linaro.org>
2017-02-07 05:11:59 +08:00
static int sh_cmt_enable(struct sh_cmt_channel *ch)
{
int k, ret;
dev_pm_syscore_device(&ch->cmt->pdev->dev, true);
/* enable clock */
ret = clk_enable(ch->cmt->clk);
if (ret) {
dev_err(&ch->cmt->pdev->dev, "ch%u: cannot enable clock\n",
ch->index);
goto err0;
}
/* make sure channel is disabled */
sh_cmt_start_stop_ch(ch, 0);
/* configure channel, periodic mode and maximum timeout */
if (ch->cmt->info->width == 16) {
sh_cmt_write_cmcsr(ch, SH_CMT16_CMCSR_CMIE |
SH_CMT16_CMCSR_CKS512);
} else {
sh_cmt_write_cmcsr(ch, SH_CMT32_CMCSR_CMM |
SH_CMT32_CMCSR_CMTOUT_IE |
SH_CMT32_CMCSR_CMR_IRQ |
SH_CMT32_CMCSR_CKS_RCLK8);
}
sh_cmt_write_cmcor(ch, 0xffffffff);
sh_cmt_write_cmcnt(ch, 0);
/*
* According to the sh73a0 user's manual, as CMCNT can be operated
* only by the RCLK (Pseudo 32 kHz), there's one restriction on
* modifying CMCNT register; two RCLK cycles are necessary before
* this register is either read or any modification of the value
* it holds is reflected in the LSI's actual operation.
*
* While at it, we're supposed to clear out the CMCNT as of this
* moment, so make sure it's processed properly here. This will
* take RCLKx2 at maximum.
*/
for (k = 0; k < 100; k++) {
if (!sh_cmt_read_cmcnt(ch))
break;
udelay(1);
}
if (sh_cmt_read_cmcnt(ch)) {
dev_err(&ch->cmt->pdev->dev, "ch%u: cannot clear CMCNT\n",
ch->index);
ret = -ETIMEDOUT;
goto err1;
}
/* enable channel */
sh_cmt_start_stop_ch(ch, 1);
return 0;
err1:
/* stop clock */
clk_disable(ch->cmt->clk);
err0:
return ret;
}
static void sh_cmt_disable(struct sh_cmt_channel *ch)
{
/* disable channel */
sh_cmt_start_stop_ch(ch, 0);
/* disable interrupts in CMT block */
sh_cmt_write_cmcsr(ch, 0);
/* stop clock */
clk_disable(ch->cmt->clk);
dev_pm_syscore_device(&ch->cmt->pdev->dev, false);
}
/* private flags */
#define FLAG_CLOCKEVENT (1 << 0)
#define FLAG_CLOCKSOURCE (1 << 1)
#define FLAG_REPROGRAM (1 << 2)
#define FLAG_SKIPEVENT (1 << 3)
#define FLAG_IRQCONTEXT (1 << 4)
static void sh_cmt_clock_event_program_verify(struct sh_cmt_channel *ch,
int absolute)
{
u32 value = ch->next_match_value;
u32 new_match;
u32 delay = 0;
u32 now = 0;
u32 has_wrapped;
now = sh_cmt_get_counter(ch, &has_wrapped);
ch->flags |= FLAG_REPROGRAM; /* force reprogram */
if (has_wrapped) {
/* we're competing with the interrupt handler.
* -> let the interrupt handler reprogram the timer.
* -> interrupt number two handles the event.
*/
ch->flags |= FLAG_SKIPEVENT;
return;
}
if (absolute)
now = 0;
do {
/* reprogram the timer hardware,
* but don't save the new match value yet.
*/
new_match = now + value + delay;
if (new_match > ch->max_match_value)
new_match = ch->max_match_value;
sh_cmt_write_cmcor(ch, new_match);
now = sh_cmt_get_counter(ch, &has_wrapped);
if (has_wrapped && (new_match > ch->match_value)) {
/* we are changing to a greater match value,
* so this wrap must be caused by the counter
* matching the old value.
* -> first interrupt reprograms the timer.
* -> interrupt number two handles the event.
*/
ch->flags |= FLAG_SKIPEVENT;
break;
}
if (has_wrapped) {
/* we are changing to a smaller match value,
* so the wrap must be caused by the counter
* matching the new value.
* -> save programmed match value.
* -> let isr handle the event.
*/
ch->match_value = new_match;
break;
}
/* be safe: verify hardware settings */
if (now < new_match) {
/* timer value is below match value, all good.
* this makes sure we won't miss any match events.
* -> save programmed match value.
* -> let isr handle the event.
*/
ch->match_value = new_match;
break;
}
/* the counter has reached a value greater
* than our new match value. and since the
* has_wrapped flag isn't set we must have
* programmed a too close event.
* -> increase delay and retry.
*/
if (delay)
delay <<= 1;
else
delay = 1;
if (!delay)
dev_warn(&ch->cmt->pdev->dev, "ch%u: too long delay\n",
ch->index);
} while (delay);
}
static void __sh_cmt_set_next(struct sh_cmt_channel *ch, unsigned long delta)
{
if (delta > ch->max_match_value)
dev_warn(&ch->cmt->pdev->dev, "ch%u: delta out of range\n",
ch->index);
ch->next_match_value = delta;
sh_cmt_clock_event_program_verify(ch, 0);
}
static void sh_cmt_set_next(struct sh_cmt_channel *ch, unsigned long delta)
{
unsigned long flags;
raw_spin_lock_irqsave(&ch->lock, flags);
__sh_cmt_set_next(ch, delta);
raw_spin_unlock_irqrestore(&ch->lock, flags);
}
static irqreturn_t sh_cmt_interrupt(int irq, void *dev_id)
{
struct sh_cmt_channel *ch = dev_id;
/* clear flags */
sh_cmt_write_cmcsr(ch, sh_cmt_read_cmcsr(ch) &
ch->cmt->info->clear_bits);
/* update clock source counter to begin with if enabled
* the wrap flag should be cleared by the timer specific
* isr before we end up here.
*/
if (ch->flags & FLAG_CLOCKSOURCE)
ch->total_cycles += ch->match_value + 1;
if (!(ch->flags & FLAG_REPROGRAM))
ch->next_match_value = ch->max_match_value;
ch->flags |= FLAG_IRQCONTEXT;
if (ch->flags & FLAG_CLOCKEVENT) {
if (!(ch->flags & FLAG_SKIPEVENT)) {
if (clockevent_state_oneshot(&ch->ced)) {
ch->next_match_value = ch->max_match_value;
ch->flags |= FLAG_REPROGRAM;
}
ch->ced.event_handler(&ch->ced);
}
}
ch->flags &= ~FLAG_SKIPEVENT;
if (ch->flags & FLAG_REPROGRAM) {
ch->flags &= ~FLAG_REPROGRAM;
sh_cmt_clock_event_program_verify(ch, 1);
if (ch->flags & FLAG_CLOCKEVENT)
if ((clockevent_state_shutdown(&ch->ced))
|| (ch->match_value == ch->next_match_value))
ch->flags &= ~FLAG_REPROGRAM;
}
ch->flags &= ~FLAG_IRQCONTEXT;
return IRQ_HANDLED;
}
static int sh_cmt_start(struct sh_cmt_channel *ch, unsigned long flag)
{
int ret = 0;
unsigned long flags;
clocksource/drivers/sh_cmt: Fix potential deadlock when calling runtime PM The ch->lock is used to protect the whole enable() and read() of sh_cmt's implementation of struct clocksource. The enable() implementation calls pm_runtime_get_sync() which may result in the clock source to be read() triggering a cyclic lockdep warning for the ch->lock. The sh_cmt driver implement its own balancing of calls to sh_cmt_{enable,disable}() with flags in sh_cmt_{start,stop}(). It does this to deal with that start and stop are shared between the clock source and clock event providers. While this could be improved on verifying corner cases based on any substantial rework on all devices this driver supports might prove hard. As a first step separate the PM handling for clock event and clock source. Always put/get the device when enabling/disabling the clock source but keep the clock event logic unchanged. This allows the sh_cmt implementation of struct clocksource to call PM without holding the ch->lock and avoiding the deadlock. Triggering and log of the deadlock warning, # echo e60f0000.timer > /sys/devices/system/clocksource/clocksource0/current_clocksource [ 46.948370] ====================================================== [ 46.954730] WARNING: possible circular locking dependency detected [ 46.961094] 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 Not tainted [ 46.967985] ------------------------------------------------------ [ 46.974342] migration/0/11 is trying to acquire lock: [ 46.979543] ffff0000403ed220 (&dev->power.lock){-...}-{2:2}, at: __pm_runtime_resume+0x40/0x74 [ 46.988445] [ 46.988445] but task is already holding lock: [ 46.994441] ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.002173] [ 47.002173] which lock already depends on the new lock. [ 47.002173] [ 47.010573] [ 47.010573] the existing dependency chain (in reverse order) is: [ 47.018262] [ 47.018262] -> #3 (&ch->lock){....}-{2:2}: [ 47.024033] lock_acquire.part.0+0x120/0x330 [ 47.028970] lock_acquire+0x64/0x80 [ 47.033105] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.038130] sh_cmt_start+0x28/0x210 [ 47.042352] sh_cmt_clocksource_enable+0x28/0x50 [ 47.047644] change_clocksource+0x9c/0x160 [ 47.052402] multi_cpu_stop+0xa4/0x190 [ 47.056799] cpu_stopper_thread+0x90/0x154 [ 47.061557] smpboot_thread_fn+0x244/0x270 [ 47.066310] kthread+0x154/0x160 [ 47.070175] ret_from_fork+0x10/0x20 [ 47.074390] [ 47.074390] -> #2 (tk_core.seq.seqcount){----}-{0:0}: [ 47.081136] lock_acquire.part.0+0x120/0x330 [ 47.086070] lock_acquire+0x64/0x80 [ 47.090203] seqcount_lockdep_reader_access.constprop.0+0x74/0x100 [ 47.097096] ktime_get+0x28/0xa0 [ 47.100960] hrtimer_start_range_ns+0x210/0x2dc [ 47.106164] generic_sched_clock_init+0x70/0x88 [ 47.111364] sched_clock_init+0x40/0x64 [ 47.115853] start_kernel+0x494/0x524 [ 47.120156] [ 47.120156] -> #1 (hrtimer_bases.lock){-.-.}-{2:2}: [ 47.126721] lock_acquire.part.0+0x120/0x330 [ 47.136042] lock_acquire+0x64/0x80 [ 47.144461] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.153721] hrtimer_start_range_ns+0x68/0x2dc [ 47.163054] rpm_suspend+0x308/0x5dc [ 47.171473] rpm_idle+0xc4/0x2a4 [ 47.179550] pm_runtime_work+0x98/0xc0 [ 47.188209] process_one_work+0x294/0x6f0 [ 47.197142] worker_thread+0x70/0x45c [ 47.205661] kthread+0x154/0x160 [ 47.213673] ret_from_fork+0x10/0x20 [ 47.221957] [ 47.221957] -> #0 (&dev->power.lock){-...}-{2:2}: [ 47.236292] check_noncircular+0x128/0x140 [ 47.244907] __lock_acquire+0x13b0/0x204c [ 47.253332] lock_acquire.part.0+0x120/0x330 [ 47.262033] lock_acquire+0x64/0x80 [ 47.269826] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.278430] __pm_runtime_resume+0x40/0x74 [ 47.286758] sh_cmt_start+0x84/0x210 [ 47.294537] sh_cmt_clocksource_enable+0x28/0x50 [ 47.303449] change_clocksource+0x9c/0x160 [ 47.311783] multi_cpu_stop+0xa4/0x190 [ 47.319720] cpu_stopper_thread+0x90/0x154 [ 47.328022] smpboot_thread_fn+0x244/0x270 [ 47.336298] kthread+0x154/0x160 [ 47.343708] ret_from_fork+0x10/0x20 [ 47.351445] [ 47.351445] other info that might help us debug this: [ 47.351445] [ 47.370225] Chain exists of: [ 47.370225] &dev->power.lock --> tk_core.seq.seqcount --> &ch->lock [ 47.370225] [ 47.392003] Possible unsafe locking scenario: [ 47.392003] [ 47.405314] CPU0 CPU1 [ 47.413569] ---- ---- [ 47.421768] lock(&ch->lock); [ 47.428425] lock(tk_core.seq.seqcount); [ 47.438701] lock(&ch->lock); [ 47.447930] lock(&dev->power.lock); [ 47.455172] [ 47.455172] *** DEADLOCK *** [ 47.455172] [ 47.471433] 3 locks held by migration/0/11: [ 47.479099] #0: ffff8000113c9278 (timekeeper_lock){-.-.}-{2:2}, at: change_clocksource+0x2c/0x160 [ 47.491834] #1: ffff8000113c8f88 (tk_core.seq.seqcount){----}-{0:0}, at: multi_cpu_stop+0xa4/0x190 [ 47.504727] #2: ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.516541] [ 47.516541] stack backtrace: [ 47.528480] CPU: 0 PID: 11 Comm: migration/0 Not tainted 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 [ 47.542147] Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT) [ 47.554241] Call trace: [ 47.560832] dump_backtrace+0x0/0x190 [ 47.568670] show_stack+0x14/0x30 [ 47.576144] dump_stack+0xe8/0x130 [ 47.583670] print_circular_bug+0x1f0/0x200 [ 47.592015] check_noncircular+0x128/0x140 [ 47.600289] __lock_acquire+0x13b0/0x204c [ 47.608486] lock_acquire.part.0+0x120/0x330 [ 47.616953] lock_acquire+0x64/0x80 [ 47.624582] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.633114] __pm_runtime_resume+0x40/0x74 [ 47.641371] sh_cmt_start+0x84/0x210 [ 47.649115] sh_cmt_clocksource_enable+0x28/0x50 [ 47.657916] change_clocksource+0x9c/0x160 [ 47.666165] multi_cpu_stop+0xa4/0x190 [ 47.674056] cpu_stopper_thread+0x90/0x154 [ 47.682308] smpboot_thread_fn+0x244/0x270 [ 47.690560] kthread+0x154/0x160 [ 47.697927] ret_from_fork+0x10/0x20 [ 47.708447] clocksource: Switched to clocksource e60f0000.timer Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> Link: https://lore.kernel.org/r/20201205021921.1456190-2-niklas.soderlund+renesas@ragnatech.se
2020-12-05 10:19:20 +08:00
if (flag & FLAG_CLOCKSOURCE)
pm_runtime_get_sync(&ch->cmt->pdev->dev);
raw_spin_lock_irqsave(&ch->lock, flags);
clocksource/drivers/sh_cmt: Fix potential deadlock when calling runtime PM The ch->lock is used to protect the whole enable() and read() of sh_cmt's implementation of struct clocksource. The enable() implementation calls pm_runtime_get_sync() which may result in the clock source to be read() triggering a cyclic lockdep warning for the ch->lock. The sh_cmt driver implement its own balancing of calls to sh_cmt_{enable,disable}() with flags in sh_cmt_{start,stop}(). It does this to deal with that start and stop are shared between the clock source and clock event providers. While this could be improved on verifying corner cases based on any substantial rework on all devices this driver supports might prove hard. As a first step separate the PM handling for clock event and clock source. Always put/get the device when enabling/disabling the clock source but keep the clock event logic unchanged. This allows the sh_cmt implementation of struct clocksource to call PM without holding the ch->lock and avoiding the deadlock. Triggering and log of the deadlock warning, # echo e60f0000.timer > /sys/devices/system/clocksource/clocksource0/current_clocksource [ 46.948370] ====================================================== [ 46.954730] WARNING: possible circular locking dependency detected [ 46.961094] 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 Not tainted [ 46.967985] ------------------------------------------------------ [ 46.974342] migration/0/11 is trying to acquire lock: [ 46.979543] ffff0000403ed220 (&dev->power.lock){-...}-{2:2}, at: __pm_runtime_resume+0x40/0x74 [ 46.988445] [ 46.988445] but task is already holding lock: [ 46.994441] ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.002173] [ 47.002173] which lock already depends on the new lock. [ 47.002173] [ 47.010573] [ 47.010573] the existing dependency chain (in reverse order) is: [ 47.018262] [ 47.018262] -> #3 (&ch->lock){....}-{2:2}: [ 47.024033] lock_acquire.part.0+0x120/0x330 [ 47.028970] lock_acquire+0x64/0x80 [ 47.033105] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.038130] sh_cmt_start+0x28/0x210 [ 47.042352] sh_cmt_clocksource_enable+0x28/0x50 [ 47.047644] change_clocksource+0x9c/0x160 [ 47.052402] multi_cpu_stop+0xa4/0x190 [ 47.056799] cpu_stopper_thread+0x90/0x154 [ 47.061557] smpboot_thread_fn+0x244/0x270 [ 47.066310] kthread+0x154/0x160 [ 47.070175] ret_from_fork+0x10/0x20 [ 47.074390] [ 47.074390] -> #2 (tk_core.seq.seqcount){----}-{0:0}: [ 47.081136] lock_acquire.part.0+0x120/0x330 [ 47.086070] lock_acquire+0x64/0x80 [ 47.090203] seqcount_lockdep_reader_access.constprop.0+0x74/0x100 [ 47.097096] ktime_get+0x28/0xa0 [ 47.100960] hrtimer_start_range_ns+0x210/0x2dc [ 47.106164] generic_sched_clock_init+0x70/0x88 [ 47.111364] sched_clock_init+0x40/0x64 [ 47.115853] start_kernel+0x494/0x524 [ 47.120156] [ 47.120156] -> #1 (hrtimer_bases.lock){-.-.}-{2:2}: [ 47.126721] lock_acquire.part.0+0x120/0x330 [ 47.136042] lock_acquire+0x64/0x80 [ 47.144461] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.153721] hrtimer_start_range_ns+0x68/0x2dc [ 47.163054] rpm_suspend+0x308/0x5dc [ 47.171473] rpm_idle+0xc4/0x2a4 [ 47.179550] pm_runtime_work+0x98/0xc0 [ 47.188209] process_one_work+0x294/0x6f0 [ 47.197142] worker_thread+0x70/0x45c [ 47.205661] kthread+0x154/0x160 [ 47.213673] ret_from_fork+0x10/0x20 [ 47.221957] [ 47.221957] -> #0 (&dev->power.lock){-...}-{2:2}: [ 47.236292] check_noncircular+0x128/0x140 [ 47.244907] __lock_acquire+0x13b0/0x204c [ 47.253332] lock_acquire.part.0+0x120/0x330 [ 47.262033] lock_acquire+0x64/0x80 [ 47.269826] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.278430] __pm_runtime_resume+0x40/0x74 [ 47.286758] sh_cmt_start+0x84/0x210 [ 47.294537] sh_cmt_clocksource_enable+0x28/0x50 [ 47.303449] change_clocksource+0x9c/0x160 [ 47.311783] multi_cpu_stop+0xa4/0x190 [ 47.319720] cpu_stopper_thread+0x90/0x154 [ 47.328022] smpboot_thread_fn+0x244/0x270 [ 47.336298] kthread+0x154/0x160 [ 47.343708] ret_from_fork+0x10/0x20 [ 47.351445] [ 47.351445] other info that might help us debug this: [ 47.351445] [ 47.370225] Chain exists of: [ 47.370225] &dev->power.lock --> tk_core.seq.seqcount --> &ch->lock [ 47.370225] [ 47.392003] Possible unsafe locking scenario: [ 47.392003] [ 47.405314] CPU0 CPU1 [ 47.413569] ---- ---- [ 47.421768] lock(&ch->lock); [ 47.428425] lock(tk_core.seq.seqcount); [ 47.438701] lock(&ch->lock); [ 47.447930] lock(&dev->power.lock); [ 47.455172] [ 47.455172] *** DEADLOCK *** [ 47.455172] [ 47.471433] 3 locks held by migration/0/11: [ 47.479099] #0: ffff8000113c9278 (timekeeper_lock){-.-.}-{2:2}, at: change_clocksource+0x2c/0x160 [ 47.491834] #1: ffff8000113c8f88 (tk_core.seq.seqcount){----}-{0:0}, at: multi_cpu_stop+0xa4/0x190 [ 47.504727] #2: ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.516541] [ 47.516541] stack backtrace: [ 47.528480] CPU: 0 PID: 11 Comm: migration/0 Not tainted 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 [ 47.542147] Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT) [ 47.554241] Call trace: [ 47.560832] dump_backtrace+0x0/0x190 [ 47.568670] show_stack+0x14/0x30 [ 47.576144] dump_stack+0xe8/0x130 [ 47.583670] print_circular_bug+0x1f0/0x200 [ 47.592015] check_noncircular+0x128/0x140 [ 47.600289] __lock_acquire+0x13b0/0x204c [ 47.608486] lock_acquire.part.0+0x120/0x330 [ 47.616953] lock_acquire+0x64/0x80 [ 47.624582] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.633114] __pm_runtime_resume+0x40/0x74 [ 47.641371] sh_cmt_start+0x84/0x210 [ 47.649115] sh_cmt_clocksource_enable+0x28/0x50 [ 47.657916] change_clocksource+0x9c/0x160 [ 47.666165] multi_cpu_stop+0xa4/0x190 [ 47.674056] cpu_stopper_thread+0x90/0x154 [ 47.682308] smpboot_thread_fn+0x244/0x270 [ 47.690560] kthread+0x154/0x160 [ 47.697927] ret_from_fork+0x10/0x20 [ 47.708447] clocksource: Switched to clocksource e60f0000.timer Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> Link: https://lore.kernel.org/r/20201205021921.1456190-2-niklas.soderlund+renesas@ragnatech.se
2020-12-05 10:19:20 +08:00
if (!(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) {
if (flag & FLAG_CLOCKEVENT)
pm_runtime_get_sync(&ch->cmt->pdev->dev);
clocksource: sh_cmt: Compute rate before registration again With the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration. Currently, sh_cmt violates this requirement in that it registers its clockevent device with a dummy rate and sets its final ->mult and ->shift values from its ->set_state_oneshot() and ->set_state_periodic() functions respectively. This patch moves the setting of the clockevent device's ->mult and ->shift values to before its registration. Note that there has been some back and forth regarding this question with respect to the clocksource also provided by this driver: commit f4d7c3565c16 ("clocksource: sh_cmt: compute mult and shift before registration") moves the rate determination from the clocksource's ->enable() function to before its registration. OTOH, the later commit 3593f5fe40a1 ("clocksource: sh_cmt: __clocksource_updatefreq_hz() update") basically reverts this, saying "Without this patch the old code uses clocksource_register() together with a hack that assumes a never changing clock rate." However, I checked all current sh_cmt users in arch/sh as well as in arch/arm/mach-shmobile carefully and right now, none of them changes any rate in any clock tree relevant to sh_cmt after their respective time_init(). Since all sh_cmt instances are created after time_init(), none of them should ever observe any clock rate changes. What's more, both, a clocksource as well as a clockevent device, can immediately get selected for use at their registration and thus, enabled at this point already. So it's probably safer to assume a "never changing clock rate" here. - Move the struct sh_cmt_channel's ->rate member to struct sh_cmt_device: it's a property of the underlying clock which is in turn specific to the sh_cmt_device. - Determine the ->rate value in sh_cmt_setup() at device probing rather than at first usage. - Set the clockevent device's ->mult and ->shift values right before its registration. - Although not strictly necessary for the upcoming clockevent core changes, set the clocksource's rate at its registration for consistency. Signed-off-by: Nicolai Stange <nicstange@gmail.com> Signed-off-by: John Stultz <john.stultz@linaro.org>
2017-02-07 05:11:59 +08:00
ret = sh_cmt_enable(ch);
clocksource/drivers/sh_cmt: Fix potential deadlock when calling runtime PM The ch->lock is used to protect the whole enable() and read() of sh_cmt's implementation of struct clocksource. The enable() implementation calls pm_runtime_get_sync() which may result in the clock source to be read() triggering a cyclic lockdep warning for the ch->lock. The sh_cmt driver implement its own balancing of calls to sh_cmt_{enable,disable}() with flags in sh_cmt_{start,stop}(). It does this to deal with that start and stop are shared between the clock source and clock event providers. While this could be improved on verifying corner cases based on any substantial rework on all devices this driver supports might prove hard. As a first step separate the PM handling for clock event and clock source. Always put/get the device when enabling/disabling the clock source but keep the clock event logic unchanged. This allows the sh_cmt implementation of struct clocksource to call PM without holding the ch->lock and avoiding the deadlock. Triggering and log of the deadlock warning, # echo e60f0000.timer > /sys/devices/system/clocksource/clocksource0/current_clocksource [ 46.948370] ====================================================== [ 46.954730] WARNING: possible circular locking dependency detected [ 46.961094] 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 Not tainted [ 46.967985] ------------------------------------------------------ [ 46.974342] migration/0/11 is trying to acquire lock: [ 46.979543] ffff0000403ed220 (&dev->power.lock){-...}-{2:2}, at: __pm_runtime_resume+0x40/0x74 [ 46.988445] [ 46.988445] but task is already holding lock: [ 46.994441] ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.002173] [ 47.002173] which lock already depends on the new lock. [ 47.002173] [ 47.010573] [ 47.010573] the existing dependency chain (in reverse order) is: [ 47.018262] [ 47.018262] -> #3 (&ch->lock){....}-{2:2}: [ 47.024033] lock_acquire.part.0+0x120/0x330 [ 47.028970] lock_acquire+0x64/0x80 [ 47.033105] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.038130] sh_cmt_start+0x28/0x210 [ 47.042352] sh_cmt_clocksource_enable+0x28/0x50 [ 47.047644] change_clocksource+0x9c/0x160 [ 47.052402] multi_cpu_stop+0xa4/0x190 [ 47.056799] cpu_stopper_thread+0x90/0x154 [ 47.061557] smpboot_thread_fn+0x244/0x270 [ 47.066310] kthread+0x154/0x160 [ 47.070175] ret_from_fork+0x10/0x20 [ 47.074390] [ 47.074390] -> #2 (tk_core.seq.seqcount){----}-{0:0}: [ 47.081136] lock_acquire.part.0+0x120/0x330 [ 47.086070] lock_acquire+0x64/0x80 [ 47.090203] seqcount_lockdep_reader_access.constprop.0+0x74/0x100 [ 47.097096] ktime_get+0x28/0xa0 [ 47.100960] hrtimer_start_range_ns+0x210/0x2dc [ 47.106164] generic_sched_clock_init+0x70/0x88 [ 47.111364] sched_clock_init+0x40/0x64 [ 47.115853] start_kernel+0x494/0x524 [ 47.120156] [ 47.120156] -> #1 (hrtimer_bases.lock){-.-.}-{2:2}: [ 47.126721] lock_acquire.part.0+0x120/0x330 [ 47.136042] lock_acquire+0x64/0x80 [ 47.144461] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.153721] hrtimer_start_range_ns+0x68/0x2dc [ 47.163054] rpm_suspend+0x308/0x5dc [ 47.171473] rpm_idle+0xc4/0x2a4 [ 47.179550] pm_runtime_work+0x98/0xc0 [ 47.188209] process_one_work+0x294/0x6f0 [ 47.197142] worker_thread+0x70/0x45c [ 47.205661] kthread+0x154/0x160 [ 47.213673] ret_from_fork+0x10/0x20 [ 47.221957] [ 47.221957] -> #0 (&dev->power.lock){-...}-{2:2}: [ 47.236292] check_noncircular+0x128/0x140 [ 47.244907] __lock_acquire+0x13b0/0x204c [ 47.253332] lock_acquire.part.0+0x120/0x330 [ 47.262033] lock_acquire+0x64/0x80 [ 47.269826] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.278430] __pm_runtime_resume+0x40/0x74 [ 47.286758] sh_cmt_start+0x84/0x210 [ 47.294537] sh_cmt_clocksource_enable+0x28/0x50 [ 47.303449] change_clocksource+0x9c/0x160 [ 47.311783] multi_cpu_stop+0xa4/0x190 [ 47.319720] cpu_stopper_thread+0x90/0x154 [ 47.328022] smpboot_thread_fn+0x244/0x270 [ 47.336298] kthread+0x154/0x160 [ 47.343708] ret_from_fork+0x10/0x20 [ 47.351445] [ 47.351445] other info that might help us debug this: [ 47.351445] [ 47.370225] Chain exists of: [ 47.370225] &dev->power.lock --> tk_core.seq.seqcount --> &ch->lock [ 47.370225] [ 47.392003] Possible unsafe locking scenario: [ 47.392003] [ 47.405314] CPU0 CPU1 [ 47.413569] ---- ---- [ 47.421768] lock(&ch->lock); [ 47.428425] lock(tk_core.seq.seqcount); [ 47.438701] lock(&ch->lock); [ 47.447930] lock(&dev->power.lock); [ 47.455172] [ 47.455172] *** DEADLOCK *** [ 47.455172] [ 47.471433] 3 locks held by migration/0/11: [ 47.479099] #0: ffff8000113c9278 (timekeeper_lock){-.-.}-{2:2}, at: change_clocksource+0x2c/0x160 [ 47.491834] #1: ffff8000113c8f88 (tk_core.seq.seqcount){----}-{0:0}, at: multi_cpu_stop+0xa4/0x190 [ 47.504727] #2: ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.516541] [ 47.516541] stack backtrace: [ 47.528480] CPU: 0 PID: 11 Comm: migration/0 Not tainted 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 [ 47.542147] Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT) [ 47.554241] Call trace: [ 47.560832] dump_backtrace+0x0/0x190 [ 47.568670] show_stack+0x14/0x30 [ 47.576144] dump_stack+0xe8/0x130 [ 47.583670] print_circular_bug+0x1f0/0x200 [ 47.592015] check_noncircular+0x128/0x140 [ 47.600289] __lock_acquire+0x13b0/0x204c [ 47.608486] lock_acquire.part.0+0x120/0x330 [ 47.616953] lock_acquire+0x64/0x80 [ 47.624582] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.633114] __pm_runtime_resume+0x40/0x74 [ 47.641371] sh_cmt_start+0x84/0x210 [ 47.649115] sh_cmt_clocksource_enable+0x28/0x50 [ 47.657916] change_clocksource+0x9c/0x160 [ 47.666165] multi_cpu_stop+0xa4/0x190 [ 47.674056] cpu_stopper_thread+0x90/0x154 [ 47.682308] smpboot_thread_fn+0x244/0x270 [ 47.690560] kthread+0x154/0x160 [ 47.697927] ret_from_fork+0x10/0x20 [ 47.708447] clocksource: Switched to clocksource e60f0000.timer Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> Link: https://lore.kernel.org/r/20201205021921.1456190-2-niklas.soderlund+renesas@ragnatech.se
2020-12-05 10:19:20 +08:00
}
if (ret)
goto out;
ch->flags |= flag;
/* setup timeout if no clockevent */
if ((flag == FLAG_CLOCKSOURCE) && (!(ch->flags & FLAG_CLOCKEVENT)))
__sh_cmt_set_next(ch, ch->max_match_value);
out:
raw_spin_unlock_irqrestore(&ch->lock, flags);
return ret;
}
static void sh_cmt_stop(struct sh_cmt_channel *ch, unsigned long flag)
{
unsigned long flags;
unsigned long f;
raw_spin_lock_irqsave(&ch->lock, flags);
f = ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE);
ch->flags &= ~flag;
clocksource/drivers/sh_cmt: Fix potential deadlock when calling runtime PM The ch->lock is used to protect the whole enable() and read() of sh_cmt's implementation of struct clocksource. The enable() implementation calls pm_runtime_get_sync() which may result in the clock source to be read() triggering a cyclic lockdep warning for the ch->lock. The sh_cmt driver implement its own balancing of calls to sh_cmt_{enable,disable}() with flags in sh_cmt_{start,stop}(). It does this to deal with that start and stop are shared between the clock source and clock event providers. While this could be improved on verifying corner cases based on any substantial rework on all devices this driver supports might prove hard. As a first step separate the PM handling for clock event and clock source. Always put/get the device when enabling/disabling the clock source but keep the clock event logic unchanged. This allows the sh_cmt implementation of struct clocksource to call PM without holding the ch->lock and avoiding the deadlock. Triggering and log of the deadlock warning, # echo e60f0000.timer > /sys/devices/system/clocksource/clocksource0/current_clocksource [ 46.948370] ====================================================== [ 46.954730] WARNING: possible circular locking dependency detected [ 46.961094] 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 Not tainted [ 46.967985] ------------------------------------------------------ [ 46.974342] migration/0/11 is trying to acquire lock: [ 46.979543] ffff0000403ed220 (&dev->power.lock){-...}-{2:2}, at: __pm_runtime_resume+0x40/0x74 [ 46.988445] [ 46.988445] but task is already holding lock: [ 46.994441] ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.002173] [ 47.002173] which lock already depends on the new lock. [ 47.002173] [ 47.010573] [ 47.010573] the existing dependency chain (in reverse order) is: [ 47.018262] [ 47.018262] -> #3 (&ch->lock){....}-{2:2}: [ 47.024033] lock_acquire.part.0+0x120/0x330 [ 47.028970] lock_acquire+0x64/0x80 [ 47.033105] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.038130] sh_cmt_start+0x28/0x210 [ 47.042352] sh_cmt_clocksource_enable+0x28/0x50 [ 47.047644] change_clocksource+0x9c/0x160 [ 47.052402] multi_cpu_stop+0xa4/0x190 [ 47.056799] cpu_stopper_thread+0x90/0x154 [ 47.061557] smpboot_thread_fn+0x244/0x270 [ 47.066310] kthread+0x154/0x160 [ 47.070175] ret_from_fork+0x10/0x20 [ 47.074390] [ 47.074390] -> #2 (tk_core.seq.seqcount){----}-{0:0}: [ 47.081136] lock_acquire.part.0+0x120/0x330 [ 47.086070] lock_acquire+0x64/0x80 [ 47.090203] seqcount_lockdep_reader_access.constprop.0+0x74/0x100 [ 47.097096] ktime_get+0x28/0xa0 [ 47.100960] hrtimer_start_range_ns+0x210/0x2dc [ 47.106164] generic_sched_clock_init+0x70/0x88 [ 47.111364] sched_clock_init+0x40/0x64 [ 47.115853] start_kernel+0x494/0x524 [ 47.120156] [ 47.120156] -> #1 (hrtimer_bases.lock){-.-.}-{2:2}: [ 47.126721] lock_acquire.part.0+0x120/0x330 [ 47.136042] lock_acquire+0x64/0x80 [ 47.144461] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.153721] hrtimer_start_range_ns+0x68/0x2dc [ 47.163054] rpm_suspend+0x308/0x5dc [ 47.171473] rpm_idle+0xc4/0x2a4 [ 47.179550] pm_runtime_work+0x98/0xc0 [ 47.188209] process_one_work+0x294/0x6f0 [ 47.197142] worker_thread+0x70/0x45c [ 47.205661] kthread+0x154/0x160 [ 47.213673] ret_from_fork+0x10/0x20 [ 47.221957] [ 47.221957] -> #0 (&dev->power.lock){-...}-{2:2}: [ 47.236292] check_noncircular+0x128/0x140 [ 47.244907] __lock_acquire+0x13b0/0x204c [ 47.253332] lock_acquire.part.0+0x120/0x330 [ 47.262033] lock_acquire+0x64/0x80 [ 47.269826] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.278430] __pm_runtime_resume+0x40/0x74 [ 47.286758] sh_cmt_start+0x84/0x210 [ 47.294537] sh_cmt_clocksource_enable+0x28/0x50 [ 47.303449] change_clocksource+0x9c/0x160 [ 47.311783] multi_cpu_stop+0xa4/0x190 [ 47.319720] cpu_stopper_thread+0x90/0x154 [ 47.328022] smpboot_thread_fn+0x244/0x270 [ 47.336298] kthread+0x154/0x160 [ 47.343708] ret_from_fork+0x10/0x20 [ 47.351445] [ 47.351445] other info that might help us debug this: [ 47.351445] [ 47.370225] Chain exists of: [ 47.370225] &dev->power.lock --> tk_core.seq.seqcount --> &ch->lock [ 47.370225] [ 47.392003] Possible unsafe locking scenario: [ 47.392003] [ 47.405314] CPU0 CPU1 [ 47.413569] ---- ---- [ 47.421768] lock(&ch->lock); [ 47.428425] lock(tk_core.seq.seqcount); [ 47.438701] lock(&ch->lock); [ 47.447930] lock(&dev->power.lock); [ 47.455172] [ 47.455172] *** DEADLOCK *** [ 47.455172] [ 47.471433] 3 locks held by migration/0/11: [ 47.479099] #0: ffff8000113c9278 (timekeeper_lock){-.-.}-{2:2}, at: change_clocksource+0x2c/0x160 [ 47.491834] #1: ffff8000113c8f88 (tk_core.seq.seqcount){----}-{0:0}, at: multi_cpu_stop+0xa4/0x190 [ 47.504727] #2: ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.516541] [ 47.516541] stack backtrace: [ 47.528480] CPU: 0 PID: 11 Comm: migration/0 Not tainted 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 [ 47.542147] Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT) [ 47.554241] Call trace: [ 47.560832] dump_backtrace+0x0/0x190 [ 47.568670] show_stack+0x14/0x30 [ 47.576144] dump_stack+0xe8/0x130 [ 47.583670] print_circular_bug+0x1f0/0x200 [ 47.592015] check_noncircular+0x128/0x140 [ 47.600289] __lock_acquire+0x13b0/0x204c [ 47.608486] lock_acquire.part.0+0x120/0x330 [ 47.616953] lock_acquire+0x64/0x80 [ 47.624582] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.633114] __pm_runtime_resume+0x40/0x74 [ 47.641371] sh_cmt_start+0x84/0x210 [ 47.649115] sh_cmt_clocksource_enable+0x28/0x50 [ 47.657916] change_clocksource+0x9c/0x160 [ 47.666165] multi_cpu_stop+0xa4/0x190 [ 47.674056] cpu_stopper_thread+0x90/0x154 [ 47.682308] smpboot_thread_fn+0x244/0x270 [ 47.690560] kthread+0x154/0x160 [ 47.697927] ret_from_fork+0x10/0x20 [ 47.708447] clocksource: Switched to clocksource e60f0000.timer Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> Link: https://lore.kernel.org/r/20201205021921.1456190-2-niklas.soderlund+renesas@ragnatech.se
2020-12-05 10:19:20 +08:00
if (f && !(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) {
sh_cmt_disable(ch);
clocksource/drivers/sh_cmt: Fix potential deadlock when calling runtime PM The ch->lock is used to protect the whole enable() and read() of sh_cmt's implementation of struct clocksource. The enable() implementation calls pm_runtime_get_sync() which may result in the clock source to be read() triggering a cyclic lockdep warning for the ch->lock. The sh_cmt driver implement its own balancing of calls to sh_cmt_{enable,disable}() with flags in sh_cmt_{start,stop}(). It does this to deal with that start and stop are shared between the clock source and clock event providers. While this could be improved on verifying corner cases based on any substantial rework on all devices this driver supports might prove hard. As a first step separate the PM handling for clock event and clock source. Always put/get the device when enabling/disabling the clock source but keep the clock event logic unchanged. This allows the sh_cmt implementation of struct clocksource to call PM without holding the ch->lock and avoiding the deadlock. Triggering and log of the deadlock warning, # echo e60f0000.timer > /sys/devices/system/clocksource/clocksource0/current_clocksource [ 46.948370] ====================================================== [ 46.954730] WARNING: possible circular locking dependency detected [ 46.961094] 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 Not tainted [ 46.967985] ------------------------------------------------------ [ 46.974342] migration/0/11 is trying to acquire lock: [ 46.979543] ffff0000403ed220 (&dev->power.lock){-...}-{2:2}, at: __pm_runtime_resume+0x40/0x74 [ 46.988445] [ 46.988445] but task is already holding lock: [ 46.994441] ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.002173] [ 47.002173] which lock already depends on the new lock. [ 47.002173] [ 47.010573] [ 47.010573] the existing dependency chain (in reverse order) is: [ 47.018262] [ 47.018262] -> #3 (&ch->lock){....}-{2:2}: [ 47.024033] lock_acquire.part.0+0x120/0x330 [ 47.028970] lock_acquire+0x64/0x80 [ 47.033105] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.038130] sh_cmt_start+0x28/0x210 [ 47.042352] sh_cmt_clocksource_enable+0x28/0x50 [ 47.047644] change_clocksource+0x9c/0x160 [ 47.052402] multi_cpu_stop+0xa4/0x190 [ 47.056799] cpu_stopper_thread+0x90/0x154 [ 47.061557] smpboot_thread_fn+0x244/0x270 [ 47.066310] kthread+0x154/0x160 [ 47.070175] ret_from_fork+0x10/0x20 [ 47.074390] [ 47.074390] -> #2 (tk_core.seq.seqcount){----}-{0:0}: [ 47.081136] lock_acquire.part.0+0x120/0x330 [ 47.086070] lock_acquire+0x64/0x80 [ 47.090203] seqcount_lockdep_reader_access.constprop.0+0x74/0x100 [ 47.097096] ktime_get+0x28/0xa0 [ 47.100960] hrtimer_start_range_ns+0x210/0x2dc [ 47.106164] generic_sched_clock_init+0x70/0x88 [ 47.111364] sched_clock_init+0x40/0x64 [ 47.115853] start_kernel+0x494/0x524 [ 47.120156] [ 47.120156] -> #1 (hrtimer_bases.lock){-.-.}-{2:2}: [ 47.126721] lock_acquire.part.0+0x120/0x330 [ 47.136042] lock_acquire+0x64/0x80 [ 47.144461] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.153721] hrtimer_start_range_ns+0x68/0x2dc [ 47.163054] rpm_suspend+0x308/0x5dc [ 47.171473] rpm_idle+0xc4/0x2a4 [ 47.179550] pm_runtime_work+0x98/0xc0 [ 47.188209] process_one_work+0x294/0x6f0 [ 47.197142] worker_thread+0x70/0x45c [ 47.205661] kthread+0x154/0x160 [ 47.213673] ret_from_fork+0x10/0x20 [ 47.221957] [ 47.221957] -> #0 (&dev->power.lock){-...}-{2:2}: [ 47.236292] check_noncircular+0x128/0x140 [ 47.244907] __lock_acquire+0x13b0/0x204c [ 47.253332] lock_acquire.part.0+0x120/0x330 [ 47.262033] lock_acquire+0x64/0x80 [ 47.269826] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.278430] __pm_runtime_resume+0x40/0x74 [ 47.286758] sh_cmt_start+0x84/0x210 [ 47.294537] sh_cmt_clocksource_enable+0x28/0x50 [ 47.303449] change_clocksource+0x9c/0x160 [ 47.311783] multi_cpu_stop+0xa4/0x190 [ 47.319720] cpu_stopper_thread+0x90/0x154 [ 47.328022] smpboot_thread_fn+0x244/0x270 [ 47.336298] kthread+0x154/0x160 [ 47.343708] ret_from_fork+0x10/0x20 [ 47.351445] [ 47.351445] other info that might help us debug this: [ 47.351445] [ 47.370225] Chain exists of: [ 47.370225] &dev->power.lock --> tk_core.seq.seqcount --> &ch->lock [ 47.370225] [ 47.392003] Possible unsafe locking scenario: [ 47.392003] [ 47.405314] CPU0 CPU1 [ 47.413569] ---- ---- [ 47.421768] lock(&ch->lock); [ 47.428425] lock(tk_core.seq.seqcount); [ 47.438701] lock(&ch->lock); [ 47.447930] lock(&dev->power.lock); [ 47.455172] [ 47.455172] *** DEADLOCK *** [ 47.455172] [ 47.471433] 3 locks held by migration/0/11: [ 47.479099] #0: ffff8000113c9278 (timekeeper_lock){-.-.}-{2:2}, at: change_clocksource+0x2c/0x160 [ 47.491834] #1: ffff8000113c8f88 (tk_core.seq.seqcount){----}-{0:0}, at: multi_cpu_stop+0xa4/0x190 [ 47.504727] #2: ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.516541] [ 47.516541] stack backtrace: [ 47.528480] CPU: 0 PID: 11 Comm: migration/0 Not tainted 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 [ 47.542147] Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT) [ 47.554241] Call trace: [ 47.560832] dump_backtrace+0x0/0x190 [ 47.568670] show_stack+0x14/0x30 [ 47.576144] dump_stack+0xe8/0x130 [ 47.583670] print_circular_bug+0x1f0/0x200 [ 47.592015] check_noncircular+0x128/0x140 [ 47.600289] __lock_acquire+0x13b0/0x204c [ 47.608486] lock_acquire.part.0+0x120/0x330 [ 47.616953] lock_acquire+0x64/0x80 [ 47.624582] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.633114] __pm_runtime_resume+0x40/0x74 [ 47.641371] sh_cmt_start+0x84/0x210 [ 47.649115] sh_cmt_clocksource_enable+0x28/0x50 [ 47.657916] change_clocksource+0x9c/0x160 [ 47.666165] multi_cpu_stop+0xa4/0x190 [ 47.674056] cpu_stopper_thread+0x90/0x154 [ 47.682308] smpboot_thread_fn+0x244/0x270 [ 47.690560] kthread+0x154/0x160 [ 47.697927] ret_from_fork+0x10/0x20 [ 47.708447] clocksource: Switched to clocksource e60f0000.timer Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> Link: https://lore.kernel.org/r/20201205021921.1456190-2-niklas.soderlund+renesas@ragnatech.se
2020-12-05 10:19:20 +08:00
if (flag & FLAG_CLOCKEVENT)
pm_runtime_put(&ch->cmt->pdev->dev);
}
/* adjust the timeout to maximum if only clocksource left */
if ((flag == FLAG_CLOCKEVENT) && (ch->flags & FLAG_CLOCKSOURCE))
__sh_cmt_set_next(ch, ch->max_match_value);
raw_spin_unlock_irqrestore(&ch->lock, flags);
clocksource/drivers/sh_cmt: Fix potential deadlock when calling runtime PM The ch->lock is used to protect the whole enable() and read() of sh_cmt's implementation of struct clocksource. The enable() implementation calls pm_runtime_get_sync() which may result in the clock source to be read() triggering a cyclic lockdep warning for the ch->lock. The sh_cmt driver implement its own balancing of calls to sh_cmt_{enable,disable}() with flags in sh_cmt_{start,stop}(). It does this to deal with that start and stop are shared between the clock source and clock event providers. While this could be improved on verifying corner cases based on any substantial rework on all devices this driver supports might prove hard. As a first step separate the PM handling for clock event and clock source. Always put/get the device when enabling/disabling the clock source but keep the clock event logic unchanged. This allows the sh_cmt implementation of struct clocksource to call PM without holding the ch->lock and avoiding the deadlock. Triggering and log of the deadlock warning, # echo e60f0000.timer > /sys/devices/system/clocksource/clocksource0/current_clocksource [ 46.948370] ====================================================== [ 46.954730] WARNING: possible circular locking dependency detected [ 46.961094] 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 Not tainted [ 46.967985] ------------------------------------------------------ [ 46.974342] migration/0/11 is trying to acquire lock: [ 46.979543] ffff0000403ed220 (&dev->power.lock){-...}-{2:2}, at: __pm_runtime_resume+0x40/0x74 [ 46.988445] [ 46.988445] but task is already holding lock: [ 46.994441] ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.002173] [ 47.002173] which lock already depends on the new lock. [ 47.002173] [ 47.010573] [ 47.010573] the existing dependency chain (in reverse order) is: [ 47.018262] [ 47.018262] -> #3 (&ch->lock){....}-{2:2}: [ 47.024033] lock_acquire.part.0+0x120/0x330 [ 47.028970] lock_acquire+0x64/0x80 [ 47.033105] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.038130] sh_cmt_start+0x28/0x210 [ 47.042352] sh_cmt_clocksource_enable+0x28/0x50 [ 47.047644] change_clocksource+0x9c/0x160 [ 47.052402] multi_cpu_stop+0xa4/0x190 [ 47.056799] cpu_stopper_thread+0x90/0x154 [ 47.061557] smpboot_thread_fn+0x244/0x270 [ 47.066310] kthread+0x154/0x160 [ 47.070175] ret_from_fork+0x10/0x20 [ 47.074390] [ 47.074390] -> #2 (tk_core.seq.seqcount){----}-{0:0}: [ 47.081136] lock_acquire.part.0+0x120/0x330 [ 47.086070] lock_acquire+0x64/0x80 [ 47.090203] seqcount_lockdep_reader_access.constprop.0+0x74/0x100 [ 47.097096] ktime_get+0x28/0xa0 [ 47.100960] hrtimer_start_range_ns+0x210/0x2dc [ 47.106164] generic_sched_clock_init+0x70/0x88 [ 47.111364] sched_clock_init+0x40/0x64 [ 47.115853] start_kernel+0x494/0x524 [ 47.120156] [ 47.120156] -> #1 (hrtimer_bases.lock){-.-.}-{2:2}: [ 47.126721] lock_acquire.part.0+0x120/0x330 [ 47.136042] lock_acquire+0x64/0x80 [ 47.144461] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.153721] hrtimer_start_range_ns+0x68/0x2dc [ 47.163054] rpm_suspend+0x308/0x5dc [ 47.171473] rpm_idle+0xc4/0x2a4 [ 47.179550] pm_runtime_work+0x98/0xc0 [ 47.188209] process_one_work+0x294/0x6f0 [ 47.197142] worker_thread+0x70/0x45c [ 47.205661] kthread+0x154/0x160 [ 47.213673] ret_from_fork+0x10/0x20 [ 47.221957] [ 47.221957] -> #0 (&dev->power.lock){-...}-{2:2}: [ 47.236292] check_noncircular+0x128/0x140 [ 47.244907] __lock_acquire+0x13b0/0x204c [ 47.253332] lock_acquire.part.0+0x120/0x330 [ 47.262033] lock_acquire+0x64/0x80 [ 47.269826] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.278430] __pm_runtime_resume+0x40/0x74 [ 47.286758] sh_cmt_start+0x84/0x210 [ 47.294537] sh_cmt_clocksource_enable+0x28/0x50 [ 47.303449] change_clocksource+0x9c/0x160 [ 47.311783] multi_cpu_stop+0xa4/0x190 [ 47.319720] cpu_stopper_thread+0x90/0x154 [ 47.328022] smpboot_thread_fn+0x244/0x270 [ 47.336298] kthread+0x154/0x160 [ 47.343708] ret_from_fork+0x10/0x20 [ 47.351445] [ 47.351445] other info that might help us debug this: [ 47.351445] [ 47.370225] Chain exists of: [ 47.370225] &dev->power.lock --> tk_core.seq.seqcount --> &ch->lock [ 47.370225] [ 47.392003] Possible unsafe locking scenario: [ 47.392003] [ 47.405314] CPU0 CPU1 [ 47.413569] ---- ---- [ 47.421768] lock(&ch->lock); [ 47.428425] lock(tk_core.seq.seqcount); [ 47.438701] lock(&ch->lock); [ 47.447930] lock(&dev->power.lock); [ 47.455172] [ 47.455172] *** DEADLOCK *** [ 47.455172] [ 47.471433] 3 locks held by migration/0/11: [ 47.479099] #0: ffff8000113c9278 (timekeeper_lock){-.-.}-{2:2}, at: change_clocksource+0x2c/0x160 [ 47.491834] #1: ffff8000113c8f88 (tk_core.seq.seqcount){----}-{0:0}, at: multi_cpu_stop+0xa4/0x190 [ 47.504727] #2: ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.516541] [ 47.516541] stack backtrace: [ 47.528480] CPU: 0 PID: 11 Comm: migration/0 Not tainted 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 [ 47.542147] Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT) [ 47.554241] Call trace: [ 47.560832] dump_backtrace+0x0/0x190 [ 47.568670] show_stack+0x14/0x30 [ 47.576144] dump_stack+0xe8/0x130 [ 47.583670] print_circular_bug+0x1f0/0x200 [ 47.592015] check_noncircular+0x128/0x140 [ 47.600289] __lock_acquire+0x13b0/0x204c [ 47.608486] lock_acquire.part.0+0x120/0x330 [ 47.616953] lock_acquire+0x64/0x80 [ 47.624582] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.633114] __pm_runtime_resume+0x40/0x74 [ 47.641371] sh_cmt_start+0x84/0x210 [ 47.649115] sh_cmt_clocksource_enable+0x28/0x50 [ 47.657916] change_clocksource+0x9c/0x160 [ 47.666165] multi_cpu_stop+0xa4/0x190 [ 47.674056] cpu_stopper_thread+0x90/0x154 [ 47.682308] smpboot_thread_fn+0x244/0x270 [ 47.690560] kthread+0x154/0x160 [ 47.697927] ret_from_fork+0x10/0x20 [ 47.708447] clocksource: Switched to clocksource e60f0000.timer Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> Link: https://lore.kernel.org/r/20201205021921.1456190-2-niklas.soderlund+renesas@ragnatech.se
2020-12-05 10:19:20 +08:00
if (flag & FLAG_CLOCKSOURCE)
pm_runtime_put(&ch->cmt->pdev->dev);
}
static struct sh_cmt_channel *cs_to_sh_cmt(struct clocksource *cs)
{
return container_of(cs, struct sh_cmt_channel, cs);
}
static u64 sh_cmt_clocksource_read(struct clocksource *cs)
{
struct sh_cmt_channel *ch = cs_to_sh_cmt(cs);
unsigned long flags;
u32 has_wrapped;
u64 value;
u32 raw;
raw_spin_lock_irqsave(&ch->lock, flags);
value = ch->total_cycles;
raw = sh_cmt_get_counter(ch, &has_wrapped);
if (unlikely(has_wrapped))
raw += ch->match_value + 1;
raw_spin_unlock_irqrestore(&ch->lock, flags);
return value + raw;
}
static int sh_cmt_clocksource_enable(struct clocksource *cs)
{
int ret;
struct sh_cmt_channel *ch = cs_to_sh_cmt(cs);
WARN_ON(ch->cs_enabled);
ch->total_cycles = 0;
ret = sh_cmt_start(ch, FLAG_CLOCKSOURCE);
clocksource: sh_cmt: Compute rate before registration again With the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration. Currently, sh_cmt violates this requirement in that it registers its clockevent device with a dummy rate and sets its final ->mult and ->shift values from its ->set_state_oneshot() and ->set_state_periodic() functions respectively. This patch moves the setting of the clockevent device's ->mult and ->shift values to before its registration. Note that there has been some back and forth regarding this question with respect to the clocksource also provided by this driver: commit f4d7c3565c16 ("clocksource: sh_cmt: compute mult and shift before registration") moves the rate determination from the clocksource's ->enable() function to before its registration. OTOH, the later commit 3593f5fe40a1 ("clocksource: sh_cmt: __clocksource_updatefreq_hz() update") basically reverts this, saying "Without this patch the old code uses clocksource_register() together with a hack that assumes a never changing clock rate." However, I checked all current sh_cmt users in arch/sh as well as in arch/arm/mach-shmobile carefully and right now, none of them changes any rate in any clock tree relevant to sh_cmt after their respective time_init(). Since all sh_cmt instances are created after time_init(), none of them should ever observe any clock rate changes. What's more, both, a clocksource as well as a clockevent device, can immediately get selected for use at their registration and thus, enabled at this point already. So it's probably safer to assume a "never changing clock rate" here. - Move the struct sh_cmt_channel's ->rate member to struct sh_cmt_device: it's a property of the underlying clock which is in turn specific to the sh_cmt_device. - Determine the ->rate value in sh_cmt_setup() at device probing rather than at first usage. - Set the clockevent device's ->mult and ->shift values right before its registration. - Although not strictly necessary for the upcoming clockevent core changes, set the clocksource's rate at its registration for consistency. Signed-off-by: Nicolai Stange <nicstange@gmail.com> Signed-off-by: John Stultz <john.stultz@linaro.org>
2017-02-07 05:11:59 +08:00
if (!ret)
ch->cs_enabled = true;
clocksource: sh_cmt: Compute rate before registration again With the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration. Currently, sh_cmt violates this requirement in that it registers its clockevent device with a dummy rate and sets its final ->mult and ->shift values from its ->set_state_oneshot() and ->set_state_periodic() functions respectively. This patch moves the setting of the clockevent device's ->mult and ->shift values to before its registration. Note that there has been some back and forth regarding this question with respect to the clocksource also provided by this driver: commit f4d7c3565c16 ("clocksource: sh_cmt: compute mult and shift before registration") moves the rate determination from the clocksource's ->enable() function to before its registration. OTOH, the later commit 3593f5fe40a1 ("clocksource: sh_cmt: __clocksource_updatefreq_hz() update") basically reverts this, saying "Without this patch the old code uses clocksource_register() together with a hack that assumes a never changing clock rate." However, I checked all current sh_cmt users in arch/sh as well as in arch/arm/mach-shmobile carefully and right now, none of them changes any rate in any clock tree relevant to sh_cmt after their respective time_init(). Since all sh_cmt instances are created after time_init(), none of them should ever observe any clock rate changes. What's more, both, a clocksource as well as a clockevent device, can immediately get selected for use at their registration and thus, enabled at this point already. So it's probably safer to assume a "never changing clock rate" here. - Move the struct sh_cmt_channel's ->rate member to struct sh_cmt_device: it's a property of the underlying clock which is in turn specific to the sh_cmt_device. - Determine the ->rate value in sh_cmt_setup() at device probing rather than at first usage. - Set the clockevent device's ->mult and ->shift values right before its registration. - Although not strictly necessary for the upcoming clockevent core changes, set the clocksource's rate at its registration for consistency. Signed-off-by: Nicolai Stange <nicstange@gmail.com> Signed-off-by: John Stultz <john.stultz@linaro.org>
2017-02-07 05:11:59 +08:00
return ret;
}
static void sh_cmt_clocksource_disable(struct clocksource *cs)
{
struct sh_cmt_channel *ch = cs_to_sh_cmt(cs);
WARN_ON(!ch->cs_enabled);
sh_cmt_stop(ch, FLAG_CLOCKSOURCE);
ch->cs_enabled = false;
}
static void sh_cmt_clocksource_suspend(struct clocksource *cs)
{
struct sh_cmt_channel *ch = cs_to_sh_cmt(cs);
clockevents/drivers/sh_cmt: Only perform clocksource suspend/resume if enabled Currently the sh_cmt clocksource timer is disabled or enabled unconditionally on clocksource suspend resp. resume, even if a better clocksource is present (e.g. arch_sys_counter) and the sh_cmt clocksource is not enabled. As sh_cmt is a syscore device when its timer is enabled, this may lead to a genpd.prepared_count imbalance in the presence of PM Domains, which may cause a lock-up during reboot after s2ram. During suspend: - pm_genpd_prepare() is called for all non-syscore devices (incl. sh_cmt), increasing genpd.prepared_count for each device, - clocksource.suspend() is called for all clocksource devices, - sh_cmt_clocksource_suspend() calls sh_cmt_stop(), which is a no-op as the clocksource was not enabled. During resume: - clocksource.resume() is called for all clocksource devices, - sh_cmt_clocksource_resume() calls sh_cmt_start(), which enables the clocksource timer, and turns sh_cmt into a syscore device, - pm_genpd_complete() is called for all non-syscore devices (excl. sh_cmt now!), decreasing genpd.prepared_count for each device but sh_cmt. Now genpd.prepared_count of the PM Domain containing sh_cmt is still 1 instead of zero. On subsequent suspend/resume cycles, sh_cmt is still a syscore device, hence it's skipped for pm_genpd_{prepare,complete}(), keeping the imbalance of genpd.prepared_count at 1. During reboot: - platform_drv_shutdown() is called for any platform device that has a driver with a .shutdown() method (only rcar-dmac on R-Car Gen2), - platform_drv_shutdown() calls dev_pm_domain_detach(), which calls genpd_dev_pm_detach(), - genpd_dev_pm_detach() keeps calling pm_genpd_remove_device() until it doesn't return -EAGAIN[*], - If the device is part of the same PM Domain as sh_cmt, pm_genpd_remove_device() always fails with -EAGAIN due to genpd.prepared_count > 0. - Infinite loop in genpd_dev_pm_detach()[*]. [*] Commit 93af5e9354432828 ("PM / Domains: Avoid infinite loops in attach/detach code") already limited the number of loop iterations, avoiding the lock-up. To fix this, only disable or enable the clocksource timer on clocksource suspend resp. resume if the clocksource was enabled. This was tested on r8a7791/koelsch with the CPG Clock Domain: - using arch_sys_counter as the clocksource, which is the default, and which showed the problem, - using sh_cmt as a clocksource ("echo ffca0000.timer > \ /sys/devices/system/clocksource/clocksource0/current_clocksource"), which behaves the same as before. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/1438875126-12596-2-git-send-email-daniel.lezcano@linaro.org Signed-off-by: Ingo Molnar <mingo@kernel.org>
2015-08-06 23:32:06 +08:00
if (!ch->cs_enabled)
return;
sh_cmt_stop(ch, FLAG_CLOCKSOURCE);
dev_pm_genpd_suspend(&ch->cmt->pdev->dev);
}
static void sh_cmt_clocksource_resume(struct clocksource *cs)
{
struct sh_cmt_channel *ch = cs_to_sh_cmt(cs);
clockevents/drivers/sh_cmt: Only perform clocksource suspend/resume if enabled Currently the sh_cmt clocksource timer is disabled or enabled unconditionally on clocksource suspend resp. resume, even if a better clocksource is present (e.g. arch_sys_counter) and the sh_cmt clocksource is not enabled. As sh_cmt is a syscore device when its timer is enabled, this may lead to a genpd.prepared_count imbalance in the presence of PM Domains, which may cause a lock-up during reboot after s2ram. During suspend: - pm_genpd_prepare() is called for all non-syscore devices (incl. sh_cmt), increasing genpd.prepared_count for each device, - clocksource.suspend() is called for all clocksource devices, - sh_cmt_clocksource_suspend() calls sh_cmt_stop(), which is a no-op as the clocksource was not enabled. During resume: - clocksource.resume() is called for all clocksource devices, - sh_cmt_clocksource_resume() calls sh_cmt_start(), which enables the clocksource timer, and turns sh_cmt into a syscore device, - pm_genpd_complete() is called for all non-syscore devices (excl. sh_cmt now!), decreasing genpd.prepared_count for each device but sh_cmt. Now genpd.prepared_count of the PM Domain containing sh_cmt is still 1 instead of zero. On subsequent suspend/resume cycles, sh_cmt is still a syscore device, hence it's skipped for pm_genpd_{prepare,complete}(), keeping the imbalance of genpd.prepared_count at 1. During reboot: - platform_drv_shutdown() is called for any platform device that has a driver with a .shutdown() method (only rcar-dmac on R-Car Gen2), - platform_drv_shutdown() calls dev_pm_domain_detach(), which calls genpd_dev_pm_detach(), - genpd_dev_pm_detach() keeps calling pm_genpd_remove_device() until it doesn't return -EAGAIN[*], - If the device is part of the same PM Domain as sh_cmt, pm_genpd_remove_device() always fails with -EAGAIN due to genpd.prepared_count > 0. - Infinite loop in genpd_dev_pm_detach()[*]. [*] Commit 93af5e9354432828 ("PM / Domains: Avoid infinite loops in attach/detach code") already limited the number of loop iterations, avoiding the lock-up. To fix this, only disable or enable the clocksource timer on clocksource suspend resp. resume if the clocksource was enabled. This was tested on r8a7791/koelsch with the CPG Clock Domain: - using arch_sys_counter as the clocksource, which is the default, and which showed the problem, - using sh_cmt as a clocksource ("echo ffca0000.timer > \ /sys/devices/system/clocksource/clocksource0/current_clocksource"), which behaves the same as before. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/1438875126-12596-2-git-send-email-daniel.lezcano@linaro.org Signed-off-by: Ingo Molnar <mingo@kernel.org>
2015-08-06 23:32:06 +08:00
if (!ch->cs_enabled)
return;
dev_pm_genpd_resume(&ch->cmt->pdev->dev);
sh_cmt_start(ch, FLAG_CLOCKSOURCE);
}
static int sh_cmt_register_clocksource(struct sh_cmt_channel *ch,
const char *name)
{
struct clocksource *cs = &ch->cs;
cs->name = name;
cs->rating = 125;
cs->read = sh_cmt_clocksource_read;
cs->enable = sh_cmt_clocksource_enable;
cs->disable = sh_cmt_clocksource_disable;
cs->suspend = sh_cmt_clocksource_suspend;
cs->resume = sh_cmt_clocksource_resume;
cs->mask = CLOCKSOURCE_MASK(sizeof(u64) * 8);
cs->flags = CLOCK_SOURCE_IS_CONTINUOUS;
dev_info(&ch->cmt->pdev->dev, "ch%u: used as clock source\n",
ch->index);
clocksource: sh_cmt: Compute rate before registration again With the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration. Currently, sh_cmt violates this requirement in that it registers its clockevent device with a dummy rate and sets its final ->mult and ->shift values from its ->set_state_oneshot() and ->set_state_periodic() functions respectively. This patch moves the setting of the clockevent device's ->mult and ->shift values to before its registration. Note that there has been some back and forth regarding this question with respect to the clocksource also provided by this driver: commit f4d7c3565c16 ("clocksource: sh_cmt: compute mult and shift before registration") moves the rate determination from the clocksource's ->enable() function to before its registration. OTOH, the later commit 3593f5fe40a1 ("clocksource: sh_cmt: __clocksource_updatefreq_hz() update") basically reverts this, saying "Without this patch the old code uses clocksource_register() together with a hack that assumes a never changing clock rate." However, I checked all current sh_cmt users in arch/sh as well as in arch/arm/mach-shmobile carefully and right now, none of them changes any rate in any clock tree relevant to sh_cmt after their respective time_init(). Since all sh_cmt instances are created after time_init(), none of them should ever observe any clock rate changes. What's more, both, a clocksource as well as a clockevent device, can immediately get selected for use at their registration and thus, enabled at this point already. So it's probably safer to assume a "never changing clock rate" here. - Move the struct sh_cmt_channel's ->rate member to struct sh_cmt_device: it's a property of the underlying clock which is in turn specific to the sh_cmt_device. - Determine the ->rate value in sh_cmt_setup() at device probing rather than at first usage. - Set the clockevent device's ->mult and ->shift values right before its registration. - Although not strictly necessary for the upcoming clockevent core changes, set the clocksource's rate at its registration for consistency. Signed-off-by: Nicolai Stange <nicstange@gmail.com> Signed-off-by: John Stultz <john.stultz@linaro.org>
2017-02-07 05:11:59 +08:00
clocksource_register_hz(cs, ch->cmt->rate);
return 0;
}
static struct sh_cmt_channel *ced_to_sh_cmt(struct clock_event_device *ced)
{
return container_of(ced, struct sh_cmt_channel, ced);
}
static void sh_cmt_clock_event_start(struct sh_cmt_channel *ch, int periodic)
{
sh_cmt_start(ch, FLAG_CLOCKEVENT);
if (periodic)
clocksource: sh_cmt: Compute rate before registration again With the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration. Currently, sh_cmt violates this requirement in that it registers its clockevent device with a dummy rate and sets its final ->mult and ->shift values from its ->set_state_oneshot() and ->set_state_periodic() functions respectively. This patch moves the setting of the clockevent device's ->mult and ->shift values to before its registration. Note that there has been some back and forth regarding this question with respect to the clocksource also provided by this driver: commit f4d7c3565c16 ("clocksource: sh_cmt: compute mult and shift before registration") moves the rate determination from the clocksource's ->enable() function to before its registration. OTOH, the later commit 3593f5fe40a1 ("clocksource: sh_cmt: __clocksource_updatefreq_hz() update") basically reverts this, saying "Without this patch the old code uses clocksource_register() together with a hack that assumes a never changing clock rate." However, I checked all current sh_cmt users in arch/sh as well as in arch/arm/mach-shmobile carefully and right now, none of them changes any rate in any clock tree relevant to sh_cmt after their respective time_init(). Since all sh_cmt instances are created after time_init(), none of them should ever observe any clock rate changes. What's more, both, a clocksource as well as a clockevent device, can immediately get selected for use at their registration and thus, enabled at this point already. So it's probably safer to assume a "never changing clock rate" here. - Move the struct sh_cmt_channel's ->rate member to struct sh_cmt_device: it's a property of the underlying clock which is in turn specific to the sh_cmt_device. - Determine the ->rate value in sh_cmt_setup() at device probing rather than at first usage. - Set the clockevent device's ->mult and ->shift values right before its registration. - Although not strictly necessary for the upcoming clockevent core changes, set the clocksource's rate at its registration for consistency. Signed-off-by: Nicolai Stange <nicstange@gmail.com> Signed-off-by: John Stultz <john.stultz@linaro.org>
2017-02-07 05:11:59 +08:00
sh_cmt_set_next(ch, ((ch->cmt->rate + HZ/2) / HZ) - 1);
else
sh_cmt_set_next(ch, ch->max_match_value);
}
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(ch, FLAG_CLOCKEVENT);
return 0;
}
static int sh_cmt_clock_event_set_state(struct clock_event_device *ced,
int periodic)
{
struct sh_cmt_channel *ch = ced_to_sh_cmt(ced);
/* deal with old setting first */
if (clockevent_state_oneshot(ced) || clockevent_state_periodic(ced))
sh_cmt_stop(ch, FLAG_CLOCKEVENT);
dev_info(&ch->cmt->pdev->dev, "ch%u: used for %s clock events\n",
ch->index, periodic ? "periodic" : "oneshot");
sh_cmt_clock_event_start(ch, periodic);
return 0;
}
static int sh_cmt_clock_event_set_oneshot(struct clock_event_device *ced)
{
return sh_cmt_clock_event_set_state(ced, 0);
}
static int sh_cmt_clock_event_set_periodic(struct clock_event_device *ced)
{
return sh_cmt_clock_event_set_state(ced, 1);
}
static int sh_cmt_clock_event_next(unsigned long delta,
struct clock_event_device *ced)
{
struct sh_cmt_channel *ch = ced_to_sh_cmt(ced);
BUG_ON(!clockevent_state_oneshot(ced));
if (likely(ch->flags & FLAG_IRQCONTEXT))
ch->next_match_value = delta - 1;
else
sh_cmt_set_next(ch, delta - 1);
return 0;
}
static void sh_cmt_clock_event_suspend(struct clock_event_device *ced)
{
struct sh_cmt_channel *ch = ced_to_sh_cmt(ced);
dev_pm_genpd_suspend(&ch->cmt->pdev->dev);
clk_unprepare(ch->cmt->clk);
}
static void sh_cmt_clock_event_resume(struct clock_event_device *ced)
{
struct sh_cmt_channel *ch = ced_to_sh_cmt(ced);
clk_prepare(ch->cmt->clk);
dev_pm_genpd_resume(&ch->cmt->pdev->dev);
}
static int sh_cmt_register_clockevent(struct sh_cmt_channel *ch,
const char *name)
{
struct clock_event_device *ced = &ch->ced;
int irq;
int ret;
irq = platform_get_irq(ch->cmt->pdev, ch->index);
if (irq < 0)
return irq;
ret = request_irq(irq, sh_cmt_interrupt,
IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING,
dev_name(&ch->cmt->pdev->dev), ch);
if (ret) {
dev_err(&ch->cmt->pdev->dev, "ch%u: failed to request irq %d\n",
ch->index, irq);
return ret;
}
ced->name = name;
ced->features = CLOCK_EVT_FEAT_PERIODIC;
ced->features |= CLOCK_EVT_FEAT_ONESHOT;
ced->rating = 125;
clocksource: sh_cmt: Set cpumask to cpu_possible_mask 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 as 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>
2014-02-19 23:19:44 +08:00
ced->cpumask = cpu_possible_mask;
ced->set_next_event = sh_cmt_clock_event_next;
ced->set_state_shutdown = sh_cmt_clock_event_shutdown;
ced->set_state_periodic = sh_cmt_clock_event_set_periodic;
ced->set_state_oneshot = sh_cmt_clock_event_set_oneshot;
ced->suspend = sh_cmt_clock_event_suspend;
ced->resume = sh_cmt_clock_event_resume;
clocksource: sh_cmt: Compute rate before registration again With the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration. Currently, sh_cmt violates this requirement in that it registers its clockevent device with a dummy rate and sets its final ->mult and ->shift values from its ->set_state_oneshot() and ->set_state_periodic() functions respectively. This patch moves the setting of the clockevent device's ->mult and ->shift values to before its registration. Note that there has been some back and forth regarding this question with respect to the clocksource also provided by this driver: commit f4d7c3565c16 ("clocksource: sh_cmt: compute mult and shift before registration") moves the rate determination from the clocksource's ->enable() function to before its registration. OTOH, the later commit 3593f5fe40a1 ("clocksource: sh_cmt: __clocksource_updatefreq_hz() update") basically reverts this, saying "Without this patch the old code uses clocksource_register() together with a hack that assumes a never changing clock rate." However, I checked all current sh_cmt users in arch/sh as well as in arch/arm/mach-shmobile carefully and right now, none of them changes any rate in any clock tree relevant to sh_cmt after their respective time_init(). Since all sh_cmt instances are created after time_init(), none of them should ever observe any clock rate changes. What's more, both, a clocksource as well as a clockevent device, can immediately get selected for use at their registration and thus, enabled at this point already. So it's probably safer to assume a "never changing clock rate" here. - Move the struct sh_cmt_channel's ->rate member to struct sh_cmt_device: it's a property of the underlying clock which is in turn specific to the sh_cmt_device. - Determine the ->rate value in sh_cmt_setup() at device probing rather than at first usage. - Set the clockevent device's ->mult and ->shift values right before its registration. - Although not strictly necessary for the upcoming clockevent core changes, set the clocksource's rate at its registration for consistency. Signed-off-by: Nicolai Stange <nicstange@gmail.com> Signed-off-by: John Stultz <john.stultz@linaro.org>
2017-02-07 05:11:59 +08:00
/* TODO: calculate good shift from rate and counter bit width */
ced->shift = 32;
ced->mult = div_sc(ch->cmt->rate, NSEC_PER_SEC, ced->shift);
ced->max_delta_ns = clockevent_delta2ns(ch->max_match_value, ced);
ced->max_delta_ticks = ch->max_match_value;
clocksource: sh_cmt: Compute rate before registration again With the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration. Currently, sh_cmt violates this requirement in that it registers its clockevent device with a dummy rate and sets its final ->mult and ->shift values from its ->set_state_oneshot() and ->set_state_periodic() functions respectively. This patch moves the setting of the clockevent device's ->mult and ->shift values to before its registration. Note that there has been some back and forth regarding this question with respect to the clocksource also provided by this driver: commit f4d7c3565c16 ("clocksource: sh_cmt: compute mult and shift before registration") moves the rate determination from the clocksource's ->enable() function to before its registration. OTOH, the later commit 3593f5fe40a1 ("clocksource: sh_cmt: __clocksource_updatefreq_hz() update") basically reverts this, saying "Without this patch the old code uses clocksource_register() together with a hack that assumes a never changing clock rate." However, I checked all current sh_cmt users in arch/sh as well as in arch/arm/mach-shmobile carefully and right now, none of them changes any rate in any clock tree relevant to sh_cmt after their respective time_init(). Since all sh_cmt instances are created after time_init(), none of them should ever observe any clock rate changes. What's more, both, a clocksource as well as a clockevent device, can immediately get selected for use at their registration and thus, enabled at this point already. So it's probably safer to assume a "never changing clock rate" here. - Move the struct sh_cmt_channel's ->rate member to struct sh_cmt_device: it's a property of the underlying clock which is in turn specific to the sh_cmt_device. - Determine the ->rate value in sh_cmt_setup() at device probing rather than at first usage. - Set the clockevent device's ->mult and ->shift values right before its registration. - Although not strictly necessary for the upcoming clockevent core changes, set the clocksource's rate at its registration for consistency. Signed-off-by: Nicolai Stange <nicstange@gmail.com> Signed-off-by: John Stultz <john.stultz@linaro.org>
2017-02-07 05:11:59 +08:00
ced->min_delta_ns = clockevent_delta2ns(0x1f, ced);
ced->min_delta_ticks = 0x1f;
clocksource: sh_cmt: Compute rate before registration again With the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration. Currently, sh_cmt violates this requirement in that it registers its clockevent device with a dummy rate and sets its final ->mult and ->shift values from its ->set_state_oneshot() and ->set_state_periodic() functions respectively. This patch moves the setting of the clockevent device's ->mult and ->shift values to before its registration. Note that there has been some back and forth regarding this question with respect to the clocksource also provided by this driver: commit f4d7c3565c16 ("clocksource: sh_cmt: compute mult and shift before registration") moves the rate determination from the clocksource's ->enable() function to before its registration. OTOH, the later commit 3593f5fe40a1 ("clocksource: sh_cmt: __clocksource_updatefreq_hz() update") basically reverts this, saying "Without this patch the old code uses clocksource_register() together with a hack that assumes a never changing clock rate." However, I checked all current sh_cmt users in arch/sh as well as in arch/arm/mach-shmobile carefully and right now, none of them changes any rate in any clock tree relevant to sh_cmt after their respective time_init(). Since all sh_cmt instances are created after time_init(), none of them should ever observe any clock rate changes. What's more, both, a clocksource as well as a clockevent device, can immediately get selected for use at their registration and thus, enabled at this point already. So it's probably safer to assume a "never changing clock rate" here. - Move the struct sh_cmt_channel's ->rate member to struct sh_cmt_device: it's a property of the underlying clock which is in turn specific to the sh_cmt_device. - Determine the ->rate value in sh_cmt_setup() at device probing rather than at first usage. - Set the clockevent device's ->mult and ->shift values right before its registration. - Although not strictly necessary for the upcoming clockevent core changes, set the clocksource's rate at its registration for consistency. Signed-off-by: Nicolai Stange <nicstange@gmail.com> Signed-off-by: John Stultz <john.stultz@linaro.org>
2017-02-07 05:11:59 +08:00
dev_info(&ch->cmt->pdev->dev, "ch%u: used for clock events\n",
ch->index);
clockevents_register_device(ced);
return 0;
}
static int sh_cmt_register(struct sh_cmt_channel *ch, const char *name,
bool clockevent, bool clocksource)
{
int ret;
if (clockevent) {
ch->cmt->has_clockevent = true;
ret = sh_cmt_register_clockevent(ch, name);
if (ret < 0)
return ret;
}
if (clocksource) {
ch->cmt->has_clocksource = true;
sh_cmt_register_clocksource(ch, name);
}
return 0;
}
static int sh_cmt_setup_channel(struct sh_cmt_channel *ch, unsigned int index,
unsigned int hwidx, bool clockevent,
bool clocksource, struct sh_cmt_device *cmt)
{
u32 value;
int ret;
/* Skip unused channels. */
if (!clockevent && !clocksource)
return 0;
ch->cmt = cmt;
ch->index = index;
ch->hwidx = hwidx;
ch->timer_bit = hwidx;
/*
* Compute the address of the channel control register block. For the
* timers with a per-channel start/stop register, compute its address
* as well.
*/
switch (cmt->info->model) {
case SH_CMT_16BIT:
ch->ioctrl = cmt->mapbase + 2 + ch->hwidx * 6;
break;
case SH_CMT_32BIT:
case SH_CMT_48BIT:
ch->ioctrl = cmt->mapbase + 0x10 + ch->hwidx * 0x10;
break;
case SH_CMT0_RCAR_GEN2:
case SH_CMT1_RCAR_GEN2:
ch->iostart = cmt->mapbase + ch->hwidx * 0x100;
ch->ioctrl = ch->iostart + 0x10;
ch->timer_bit = 0;
/* Enable the clock supply to the channel */
value = ioread32(cmt->mapbase + CMCLKE);
value |= BIT(hwidx);
iowrite32(value, cmt->mapbase + CMCLKE);
break;
}
if (cmt->info->width == (sizeof(ch->max_match_value) * 8))
ch->max_match_value = ~0;
else
ch->max_match_value = (1 << cmt->info->width) - 1;
ch->match_value = ch->max_match_value;
raw_spin_lock_init(&ch->lock);
ret = sh_cmt_register(ch, dev_name(&cmt->pdev->dev),
clockevent, clocksource);
if (ret) {
dev_err(&cmt->pdev->dev, "ch%u: registration failed\n",
ch->index);
return ret;
}
ch->cs_enabled = false;
return 0;
}
static int sh_cmt_map_memory(struct sh_cmt_device *cmt)
{
struct resource *mem;
mem = platform_get_resource(cmt->pdev, IORESOURCE_MEM, 0);
if (!mem) {
dev_err(&cmt->pdev->dev, "failed to get I/O memory\n");
return -ENXIO;
}
cmt->mapbase = ioremap(mem->start, resource_size(mem));
if (cmt->mapbase == NULL) {
dev_err(&cmt->pdev->dev, "failed to remap I/O memory\n");
return -ENXIO;
}
return 0;
}
static const struct platform_device_id sh_cmt_id_table[] = {
{ "sh-cmt-16", (kernel_ulong_t)&sh_cmt_info[SH_CMT_16BIT] },
{ "sh-cmt-32", (kernel_ulong_t)&sh_cmt_info[SH_CMT_32BIT] },
{ }
};
MODULE_DEVICE_TABLE(platform, sh_cmt_id_table);
static const struct of_device_id sh_cmt_of_table[] __maybe_unused = {
{
/* deprecated, preserved for backward compatibility */
.compatible = "renesas,cmt-48",
.data = &sh_cmt_info[SH_CMT_48BIT]
},
{
/* deprecated, preserved for backward compatibility */
.compatible = "renesas,cmt-48-gen2",
.data = &sh_cmt_info[SH_CMT0_RCAR_GEN2]
},
{
.compatible = "renesas,r8a7740-cmt1",
.data = &sh_cmt_info[SH_CMT_48BIT]
},
{
.compatible = "renesas,sh73a0-cmt1",
.data = &sh_cmt_info[SH_CMT_48BIT]
},
{
.compatible = "renesas,rcar-gen2-cmt0",
.data = &sh_cmt_info[SH_CMT0_RCAR_GEN2]
},
{
.compatible = "renesas,rcar-gen2-cmt1",
.data = &sh_cmt_info[SH_CMT1_RCAR_GEN2]
},
{
.compatible = "renesas,rcar-gen3-cmt0",
.data = &sh_cmt_info[SH_CMT0_RCAR_GEN2]
},
{
.compatible = "renesas,rcar-gen3-cmt1",
.data = &sh_cmt_info[SH_CMT1_RCAR_GEN2]
},
{ }
};
MODULE_DEVICE_TABLE(of, sh_cmt_of_table);
static int sh_cmt_setup(struct sh_cmt_device *cmt, struct platform_device *pdev)
{
unsigned int mask;
unsigned int i;
int ret;
cmt->pdev = pdev;
raw_spin_lock_init(&cmt->lock);
if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
cmt->info = of_device_get_match_data(&pdev->dev);
cmt->hw_channels = cmt->info->channels_mask;
} else if (pdev->dev.platform_data) {
struct sh_timer_config *cfg = pdev->dev.platform_data;
const struct platform_device_id *id = pdev->id_entry;
cmt->info = (const struct sh_cmt_info *)id->driver_data;
cmt->hw_channels = cfg->channels_mask;
} else {
dev_err(&cmt->pdev->dev, "missing platform data\n");
return -ENXIO;
}
/* Get hold of clock. */
cmt->clk = clk_get(&cmt->pdev->dev, "fck");
if (IS_ERR(cmt->clk)) {
dev_err(&cmt->pdev->dev, "cannot get clock\n");
return PTR_ERR(cmt->clk);
}
ret = clk_prepare(cmt->clk);
if (ret < 0)
goto err_clk_put;
clocksource: sh_cmt: Compute rate before registration again With the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration. Currently, sh_cmt violates this requirement in that it registers its clockevent device with a dummy rate and sets its final ->mult and ->shift values from its ->set_state_oneshot() and ->set_state_periodic() functions respectively. This patch moves the setting of the clockevent device's ->mult and ->shift values to before its registration. Note that there has been some back and forth regarding this question with respect to the clocksource also provided by this driver: commit f4d7c3565c16 ("clocksource: sh_cmt: compute mult and shift before registration") moves the rate determination from the clocksource's ->enable() function to before its registration. OTOH, the later commit 3593f5fe40a1 ("clocksource: sh_cmt: __clocksource_updatefreq_hz() update") basically reverts this, saying "Without this patch the old code uses clocksource_register() together with a hack that assumes a never changing clock rate." However, I checked all current sh_cmt users in arch/sh as well as in arch/arm/mach-shmobile carefully and right now, none of them changes any rate in any clock tree relevant to sh_cmt after their respective time_init(). Since all sh_cmt instances are created after time_init(), none of them should ever observe any clock rate changes. What's more, both, a clocksource as well as a clockevent device, can immediately get selected for use at their registration and thus, enabled at this point already. So it's probably safer to assume a "never changing clock rate" here. - Move the struct sh_cmt_channel's ->rate member to struct sh_cmt_device: it's a property of the underlying clock which is in turn specific to the sh_cmt_device. - Determine the ->rate value in sh_cmt_setup() at device probing rather than at first usage. - Set the clockevent device's ->mult and ->shift values right before its registration. - Although not strictly necessary for the upcoming clockevent core changes, set the clocksource's rate at its registration for consistency. Signed-off-by: Nicolai Stange <nicstange@gmail.com> Signed-off-by: John Stultz <john.stultz@linaro.org>
2017-02-07 05:11:59 +08:00
/* Determine clock rate. */
ret = clk_enable(cmt->clk);
if (ret < 0)
goto err_clk_unprepare;
if (cmt->info->width == 16)
cmt->rate = clk_get_rate(cmt->clk) / 512;
else
cmt->rate = clk_get_rate(cmt->clk) / 8;
/* Map the memory resource(s). */
ret = sh_cmt_map_memory(cmt);
if (ret < 0)
goto err_clk_disable;
/* Allocate and setup the channels. */
cmt->num_channels = hweight8(cmt->hw_channels);
treewide: kzalloc() -> kcalloc() The kzalloc() function has a 2-factor argument form, kcalloc(). This patch replaces cases of: kzalloc(a * b, gfp) with: kcalloc(a * b, gfp) as well as handling cases of: kzalloc(a * b * c, gfp) with: kzalloc(array3_size(a, b, c), gfp) as it's slightly less ugly than: kzalloc_array(array_size(a, b), c, gfp) This does, however, attempt to ignore constant size factors like: kzalloc(4 * 1024, gfp) though any constants defined via macros get caught up in the conversion. Any factors with a sizeof() of "unsigned char", "char", and "u8" were dropped, since they're redundant. The Coccinelle script used for this was: // Fix redundant parens around sizeof(). @@ type TYPE; expression THING, E; @@ ( kzalloc( - (sizeof(TYPE)) * E + sizeof(TYPE) * E , ...) | kzalloc( - (sizeof(THING)) * E + sizeof(THING) * E , ...) ) // Drop single-byte sizes and redundant parens. @@ expression COUNT; typedef u8; typedef __u8; @@ ( kzalloc( - sizeof(u8) * (COUNT) + COUNT , ...) | kzalloc( - sizeof(__u8) * (COUNT) + COUNT , ...) | kzalloc( - sizeof(char) * (COUNT) + COUNT , ...) | kzalloc( - sizeof(unsigned char) * (COUNT) + COUNT , ...) | kzalloc( - sizeof(u8) * COUNT + COUNT , ...) | kzalloc( - sizeof(__u8) * COUNT + COUNT , ...) | kzalloc( - sizeof(char) * COUNT + COUNT , ...) | kzalloc( - sizeof(unsigned char) * COUNT + COUNT , ...) ) // 2-factor product with sizeof(type/expression) and identifier or constant. @@ type TYPE; expression THING; identifier COUNT_ID; constant COUNT_CONST; @@ ( - kzalloc + kcalloc ( - sizeof(TYPE) * (COUNT_ID) + COUNT_ID, sizeof(TYPE) , ...) | - kzalloc + kcalloc ( - sizeof(TYPE) * COUNT_ID + COUNT_ID, sizeof(TYPE) , ...) | - kzalloc + kcalloc ( - sizeof(TYPE) * (COUNT_CONST) + COUNT_CONST, sizeof(TYPE) , ...) | - kzalloc + kcalloc ( - sizeof(TYPE) * COUNT_CONST + COUNT_CONST, sizeof(TYPE) , ...) | - kzalloc + kcalloc ( - sizeof(THING) * (COUNT_ID) + COUNT_ID, sizeof(THING) , ...) | - kzalloc + kcalloc ( - sizeof(THING) * COUNT_ID + COUNT_ID, sizeof(THING) , ...) | - kzalloc + kcalloc ( - sizeof(THING) * (COUNT_CONST) + COUNT_CONST, sizeof(THING) , ...) | - kzalloc + kcalloc ( - sizeof(THING) * COUNT_CONST + COUNT_CONST, sizeof(THING) , ...) ) // 2-factor product, only identifiers. @@ identifier SIZE, COUNT; @@ - kzalloc + kcalloc ( - SIZE * COUNT + COUNT, SIZE , ...) // 3-factor product with 1 sizeof(type) or sizeof(expression), with // redundant parens removed. @@ expression THING; identifier STRIDE, COUNT; type TYPE; @@ ( kzalloc( - sizeof(TYPE) * (COUNT) * (STRIDE) + array3_size(COUNT, STRIDE, sizeof(TYPE)) , ...) | kzalloc( - sizeof(TYPE) * (COUNT) * STRIDE + array3_size(COUNT, STRIDE, sizeof(TYPE)) , ...) | kzalloc( - sizeof(TYPE) * COUNT * (STRIDE) + array3_size(COUNT, STRIDE, sizeof(TYPE)) , ...) | kzalloc( - sizeof(TYPE) * COUNT * STRIDE + array3_size(COUNT, STRIDE, sizeof(TYPE)) , ...) | kzalloc( - sizeof(THING) * (COUNT) * (STRIDE) + array3_size(COUNT, STRIDE, sizeof(THING)) , ...) | kzalloc( - sizeof(THING) * (COUNT) * STRIDE + array3_size(COUNT, STRIDE, sizeof(THING)) , ...) | kzalloc( - sizeof(THING) * COUNT * (STRIDE) + array3_size(COUNT, STRIDE, sizeof(THING)) , ...) | kzalloc( - sizeof(THING) * COUNT * STRIDE + array3_size(COUNT, STRIDE, sizeof(THING)) , ...) ) // 3-factor product with 2 sizeof(variable), with redundant parens removed. @@ expression THING1, THING2; identifier COUNT; type TYPE1, TYPE2; @@ ( kzalloc( - sizeof(TYPE1) * sizeof(TYPE2) * COUNT + array3_size(COUNT, sizeof(TYPE1), sizeof(TYPE2)) , ...) | kzalloc( - sizeof(TYPE1) * sizeof(THING2) * (COUNT) + array3_size(COUNT, sizeof(TYPE1), sizeof(TYPE2)) , ...) | kzalloc( - sizeof(THING1) * sizeof(THING2) * COUNT + array3_size(COUNT, sizeof(THING1), sizeof(THING2)) , ...) | kzalloc( - sizeof(THING1) * sizeof(THING2) * (COUNT) + array3_size(COUNT, sizeof(THING1), sizeof(THING2)) , ...) | kzalloc( - sizeof(TYPE1) * sizeof(THING2) * COUNT + array3_size(COUNT, sizeof(TYPE1), sizeof(THING2)) , ...) | kzalloc( - sizeof(TYPE1) * sizeof(THING2) * (COUNT) + array3_size(COUNT, sizeof(TYPE1), sizeof(THING2)) , ...) ) // 3-factor product, only identifiers, with redundant parens removed. @@ identifier STRIDE, SIZE, COUNT; @@ ( kzalloc( - (COUNT) * STRIDE * SIZE + array3_size(COUNT, STRIDE, SIZE) , ...) | kzalloc( - COUNT * (STRIDE) * SIZE + array3_size(COUNT, STRIDE, SIZE) , ...) | kzalloc( - COUNT * STRIDE * (SIZE) + array3_size(COUNT, STRIDE, SIZE) , ...) | kzalloc( - (COUNT) * (STRIDE) * SIZE + array3_size(COUNT, STRIDE, SIZE) , ...) | kzalloc( - COUNT * (STRIDE) * (SIZE) + array3_size(COUNT, STRIDE, SIZE) , ...) | kzalloc( - (COUNT) * STRIDE * (SIZE) + array3_size(COUNT, STRIDE, SIZE) , ...) | kzalloc( - (COUNT) * (STRIDE) * (SIZE) + array3_size(COUNT, STRIDE, SIZE) , ...) | kzalloc( - COUNT * STRIDE * SIZE + array3_size(COUNT, STRIDE, SIZE) , ...) ) // Any remaining multi-factor products, first at least 3-factor products, // when they're not all constants... @@ expression E1, E2, E3; constant C1, C2, C3; @@ ( kzalloc(C1 * C2 * C3, ...) | kzalloc( - (E1) * E2 * E3 + array3_size(E1, E2, E3) , ...) | kzalloc( - (E1) * (E2) * E3 + array3_size(E1, E2, E3) , ...) | kzalloc( - (E1) * (E2) * (E3) + array3_size(E1, E2, E3) , ...) | kzalloc( - E1 * E2 * E3 + array3_size(E1, E2, E3) , ...) ) // And then all remaining 2 factors products when they're not all constants, // keeping sizeof() as the second factor argument. @@ expression THING, E1, E2; type TYPE; constant C1, C2, C3; @@ ( kzalloc(sizeof(THING) * C2, ...) | kzalloc(sizeof(TYPE) * C2, ...) | kzalloc(C1 * C2 * C3, ...) | kzalloc(C1 * C2, ...) | - kzalloc + kcalloc ( - sizeof(TYPE) * (E2) + E2, sizeof(TYPE) , ...) | - kzalloc + kcalloc ( - sizeof(TYPE) * E2 + E2, sizeof(TYPE) , ...) | - kzalloc + kcalloc ( - sizeof(THING) * (E2) + E2, sizeof(THING) , ...) | - kzalloc + kcalloc ( - sizeof(THING) * E2 + E2, sizeof(THING) , ...) | - kzalloc + kcalloc ( - (E1) * E2 + E1, E2 , ...) | - kzalloc + kcalloc ( - (E1) * (E2) + E1, E2 , ...) | - kzalloc + kcalloc ( - E1 * E2 + E1, E2 , ...) ) Signed-off-by: Kees Cook <keescook@chromium.org>
2018-06-13 05:03:40 +08:00
cmt->channels = kcalloc(cmt->num_channels, sizeof(*cmt->channels),
GFP_KERNEL);
if (cmt->channels == NULL) {
ret = -ENOMEM;
goto err_unmap;
}
/*
* Use the first channel as a clock event device and the second channel
* as a clock source. If only one channel is available use it for both.
*/
for (i = 0, mask = cmt->hw_channels; i < cmt->num_channels; ++i) {
unsigned int hwidx = ffs(mask) - 1;
bool clocksource = i == 1 || cmt->num_channels == 1;
bool clockevent = i == 0;
ret = sh_cmt_setup_channel(&cmt->channels[i], i, hwidx,
clockevent, clocksource, cmt);
if (ret < 0)
goto err_unmap;
mask &= ~(1 << hwidx);
}
clk_disable(cmt->clk);
platform_set_drvdata(pdev, cmt);
return 0;
err_unmap:
kfree(cmt->channels);
iounmap(cmt->mapbase);
err_clk_disable:
clk_disable(cmt->clk);
err_clk_unprepare:
clk_unprepare(cmt->clk);
err_clk_put:
clk_put(cmt->clk);
return ret;
}
static int sh_cmt_probe(struct platform_device *pdev)
{
struct sh_cmt_device *cmt = platform_get_drvdata(pdev);
int ret;
if (!is_sh_early_platform_device(pdev)) {
pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);
}
if (cmt) {
dev_info(&pdev->dev, "kept as earlytimer\n");
goto out;
}
cmt = kzalloc(sizeof(*cmt), GFP_KERNEL);
if (cmt == NULL)
return -ENOMEM;
ret = sh_cmt_setup(cmt, pdev);
if (ret) {
kfree(cmt);
pm_runtime_idle(&pdev->dev);
return ret;
}
if (is_sh_early_platform_device(pdev))
return 0;
out:
if (cmt->has_clockevent || cmt->has_clocksource)
pm_runtime_irq_safe(&pdev->dev);
else
pm_runtime_idle(&pdev->dev);
return 0;
}
static int sh_cmt_remove(struct platform_device *pdev)
{
return -EBUSY; /* cannot unregister clockevent and clocksource */
}
static struct platform_driver sh_cmt_device_driver = {
.probe = sh_cmt_probe,
.remove = sh_cmt_remove,
.driver = {
.name = "sh_cmt",
.of_match_table = of_match_ptr(sh_cmt_of_table),
},
.id_table = sh_cmt_id_table,
};
static int __init sh_cmt_init(void)
{
return platform_driver_register(&sh_cmt_device_driver);
}
static void __exit sh_cmt_exit(void)
{
platform_driver_unregister(&sh_cmt_device_driver);
}
#ifdef CONFIG_SUPERH
sh_early_platform_init("earlytimer", &sh_cmt_device_driver);
#endif
subsys_initcall(sh_cmt_init);
module_exit(sh_cmt_exit);
MODULE_AUTHOR("Magnus Damm");
MODULE_DESCRIPTION("SuperH CMT Timer Driver");
MODULE_LICENSE("GPL v2");