* [PATCH 2/2] clocksource: Make clocksource register functions void
From: Yijing Wang @ 2014-01-23 7:12 UTC (permalink / raw)
To: John Stultz, Thomas Gleixner
Cc: linux-mips, Daniel Lezcano, Kevin Hilman, linux, Sekhar Nori,
Paul Mackerras, H. Peter Anvin, Yijing Wang, Daniel Walker,
Hans-Christian Egtvedt, Jonas Bonn, Kukjin Kim, Russell King,
Richard Weinberger, x86, Tony Lindgren, Ingo Molnar,
linux-arm-msm, David Brown, Haavard Skinnemoen, Mike Frysinger,
user-mode-linux-devel, Nicolas Ferre, Jeff Dike, Barry Song,
linux-samsung-soc, user-mode-linux-user, linux-omap,
linux-arm-kernel, davinci-linux-open-source, Michal Simek,
Jim Cromie, microblaze-uclinux, Hanjun Guo, linux-kernel,
Ralf Baechle, Tony Prisk, Bryan Huntsman, uclinux-dist-devel,
linuxppc-dev
Currently, clocksource_register() and __clocksource_register_scale()
functions always return 0, it's pointless, make functions void.
And remove the dead code that check the clocksource_register_hz()
return value.
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
arch/arm/mach-davinci/time.c | 5 ++---
arch/arm/mach-msm/timer.c | 4 +---
arch/arm/mach-omap2/timer.c | 8 +++-----
arch/avr32/kernel/time.c | 4 +---
arch/blackfin/kernel/time-ts.c | 6 ++----
arch/microblaze/kernel/timer.c | 3 +--
arch/mips/jz4740/time.c | 6 +-----
arch/mips/loongson/common/cs5536/cs5536_mfgpt.c | 3 ++-
arch/openrisc/kernel/time.c | 3 +--
arch/powerpc/kernel/time.c | 6 +-----
arch/um/kernel/time.c | 6 +-----
arch/x86/platform/uv/uv_time.c | 14 ++++++--------
drivers/clocksource/acpi_pm.c | 3 ++-
drivers/clocksource/cadence_ttc_timer.c | 6 +-----
drivers/clocksource/exynos_mct.c | 4 +---
drivers/clocksource/i8253.c | 3 ++-
drivers/clocksource/mmio.c | 3 ++-
drivers/clocksource/samsung_pwm_timer.c | 5 +----
drivers/clocksource/scx200_hrt.c | 3 ++-
drivers/clocksource/tcb_clksrc.c | 8 +-------
drivers/clocksource/timer-marco.c | 2 +-
drivers/clocksource/timer-prima2.c | 2 +-
drivers/clocksource/vt8500_timer.c | 4 +---
include/linux/clocksource.h | 8 ++++----
kernel/time/clocksource.c | 6 ++----
kernel/time/jiffies.c | 3 ++-
26 files changed, 45 insertions(+), 83 deletions(-)
diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c
index 56c6eb5..9536f85 100644
--- a/arch/arm/mach-davinci/time.c
+++ b/arch/arm/mach-davinci/time.c
@@ -387,9 +387,8 @@ void __init davinci_timer_init(void)
/* setup clocksource */
clocksource_davinci.name = id_to_name[clocksource_id];
- if (clocksource_register_hz(&clocksource_davinci,
- davinci_clock_tick_rate))
- printk(err, clocksource_davinci.name);
+ clocksource_register_hz(&clocksource_davinci,
+ davinci_clock_tick_rate);
setup_sched_clock(davinci_read_sched_clock, 32,
davinci_clock_tick_rate);
diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index 1e9c338..c96e034 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -226,9 +226,7 @@ static void __init msm_timer_init(u32 dgt_hz, int sched_bits, int irq,
err:
writel_relaxed(TIMER_ENABLE_EN, source_base + TIMER_ENABLE);
- res = clocksource_register_hz(cs, dgt_hz);
- if (res)
- pr_err("clocksource_register failed\n");
+ clocksource_register_hz(cs, dgt_hz);
setup_sched_clock(msm_sched_clock_read, sched_bits, dgt_hz);
}
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 3ca81e0..beaf7c7 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -473,11 +473,9 @@ static void __init omap2_gptimer_clocksource_init(int gptimer_id,
OMAP_TIMER_NONPOSTED);
setup_sched_clock(dmtimer_read_sched_clock, 32, clksrc.rate);
- if (clocksource_register_hz(&clocksource_gpt, clksrc.rate))
- pr_err("Could not register clocksource %s\n",
- clocksource_gpt.name);
- else
- pr_info("OMAP clocksource: %s at %lu Hz\n",
+ clocksource_register_hz(&clocksource_gpt, clksrc.rate);
+
+ pr_info("OMAP clocksource: %s at %lu Hz\n",
clocksource_gpt.name, clksrc.rate);
}
diff --git a/arch/avr32/kernel/time.c b/arch/avr32/kernel/time.c
index d0f771b..51b4a66 100644
--- a/arch/avr32/kernel/time.c
+++ b/arch/avr32/kernel/time.c
@@ -134,9 +134,7 @@ void __init time_init(void)
/* figure rate for counter */
counter_hz = clk_get_rate(boot_cpu_data.clk);
- ret = clocksource_register_hz(&counter, counter_hz);
- if (ret)
- pr_debug("timer: could not register clocksource: %d\n", ret);
+ clocksource_register_hz(&counter, counter_hz);
/* setup COMPARE clockevent */
comparator.mult = div_sc(counter_hz, NSEC_PER_SEC, comparator.shift);
diff --git a/arch/blackfin/kernel/time-ts.c b/arch/blackfin/kernel/time-ts.c
index cb0a484..df3bb08 100644
--- a/arch/blackfin/kernel/time-ts.c
+++ b/arch/blackfin/kernel/time-ts.c
@@ -51,8 +51,7 @@ static inline unsigned long long bfin_cs_cycles_sched_clock(void)
static int __init bfin_cs_cycles_init(void)
{
- if (clocksource_register_hz(&bfin_cs_cycles, get_cclk()))
- panic("failed to register clocksource");
+ clocksource_register_hz(&bfin_cs_cycles, get_cclk());
return 0;
}
@@ -103,8 +102,7 @@ static int __init bfin_cs_gptimer0_init(void)
{
setup_gptimer0();
- if (clocksource_register_hz(&bfin_cs_gptimer0, get_sclk()))
- panic("failed to register clocksource");
+ clocksource_register_hz(&bfin_cs_gptimer0, get_sclk());
return 0;
}
diff --git a/arch/microblaze/kernel/timer.c b/arch/microblaze/kernel/timer.c
index 3e39b10..6a2417e 100644
--- a/arch/microblaze/kernel/timer.c
+++ b/arch/microblaze/kernel/timer.c
@@ -208,8 +208,7 @@ static struct clocksource clocksource_microblaze = {
static int __init xilinx_clocksource_init(void)
{
- if (clocksource_register_hz(&clocksource_microblaze, timer_clock_freq))
- panic("failed to register clocksource");
+ clocksource_register_hz(&clocksource_microblaze, timer_clock_freq);
/* stop timer1 */
out_be32(timer_baseaddr + TCSR1,
diff --git a/arch/mips/jz4740/time.c b/arch/mips/jz4740/time.c
index 5e430ce..041cdff 100644
--- a/arch/mips/jz4740/time.c
+++ b/arch/mips/jz4740/time.c
@@ -105,7 +105,6 @@ static struct irqaction timer_irqaction = {
void __init plat_time_init(void)
{
- int ret;
uint32_t clk_rate;
uint16_t ctrl;
@@ -121,10 +120,7 @@ void __init plat_time_init(void)
clockevents_register_device(&jz4740_clockevent);
- ret = clocksource_register_hz(&jz4740_clocksource, clk_rate);
-
- if (ret)
- printk(KERN_ERR "Failed to register clocksource: %d\n", ret);
+ clocksource_register_hz(&jz4740_clocksource, clk_rate);
setup_irq(JZ4740_IRQ_TCU0, &timer_irqaction);
diff --git a/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c b/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c
index c639b9d..9fa6d99 100644
--- a/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c
+++ b/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c
@@ -208,7 +208,8 @@ int __init init_mfgpt_clocksource(void)
if (num_possible_cpus() > 1) /* MFGPT does not scale! */
return 0;
- return clocksource_register_hz(&clocksource_mfgpt, MFGPT_TICK_RATE);
+ clocksource_register_hz(&clocksource_mfgpt, MFGPT_TICK_RATE);
+ return 0;
}
arch_initcall(init_mfgpt_clocksource);
diff --git a/arch/openrisc/kernel/time.c b/arch/openrisc/kernel/time.c
index 7c52e94..3f789aa 100644
--- a/arch/openrisc/kernel/time.c
+++ b/arch/openrisc/kernel/time.c
@@ -156,8 +156,7 @@ static struct clocksource openrisc_timer = {
static int __init openrisc_timer_init(void)
{
- if (clocksource_register_hz(&openrisc_timer, cpuinfo.clock_frequency))
- panic("failed to register clocksource");
+ clocksource_register_hz(&openrisc_timer, cpuinfo.clock_frequency);
/* Enable the incrementer: 'continuous' mode with interrupt disabled */
mtspr(SPR_TTMR, SPR_TTMR_CR);
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index b3b1441..27c0627 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -788,11 +788,7 @@ static void __init clocksource_init(void)
else
clock = &clocksource_timebase;
- if (clocksource_register_hz(clock, tb_ticks_per_sec)) {
- printk(KERN_ERR "clocksource: %s is already registered\n",
- clock->name);
- return;
- }
+ clocksource_register_hz(clock, tb_ticks_per_sec);
printk(KERN_INFO "clocksource: %s mult[%x] shift[%d] registered\n",
clock->name, clock->mult, clock->shift);
diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c
index 117568d..2034b58 100644
--- a/arch/um/kernel/time.c
+++ b/arch/um/kernel/time.c
@@ -92,11 +92,7 @@ static void __init setup_itimer(void)
clockevent_delta2ns(60 * HZ, &itimer_clockevent);
itimer_clockevent.min_delta_ns =
clockevent_delta2ns(1, &itimer_clockevent);
- err = clocksource_register_hz(&itimer_clocksource, USEC_PER_SEC);
- if (err) {
- printk(KERN_ERR "clocksource_register_hz returned %d\n", err);
- return;
- }
+ clocksource_register_hz(&itimer_clocksource, USEC_PER_SEC);
clockevents_register_device(&itimer_clockevent);
}
diff --git a/arch/x86/platform/uv/uv_time.c b/arch/x86/platform/uv/uv_time.c
index 5c86786..b963774 100644
--- a/arch/x86/platform/uv/uv_time.c
+++ b/arch/x86/platform/uv/uv_time.c
@@ -379,15 +379,13 @@ static __init int uv_rtc_setup_clock(void)
if (!is_uv_system())
return -ENODEV;
- rc = clocksource_register_hz(&clocksource_uv, sn_rtc_cycles_per_second);
- if (rc)
- printk(KERN_INFO "UV RTC clocksource failed rc %d\n", rc);
- else
- printk(KERN_INFO "UV RTC clocksource registered freq %lu MHz\n",
- sn_rtc_cycles_per_second/(unsigned long)1E6);
+ clocksource_register_hz(&clocksource_uv, sn_rtc_cycles_per_second);
+
+ pr_info("UV RTC clocksource registered freq %lu MHz\n",
+ sn_rtc_cycles_per_second/(unsigned long)1E6);
- if (rc || !uv_rtc_evt_enable || x86_platform_ipi_callback)
- return rc;
+ if (!uv_rtc_evt_enable || x86_platform_ipi_callback)
+ return 0;
/* Setup and register clockevents */
rc = uv_rtc_allocate_timers();
diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c
index 6eab889..ab1dc63 100644
--- a/drivers/clocksource/acpi_pm.c
+++ b/drivers/clocksource/acpi_pm.c
@@ -218,8 +218,9 @@ static int __init init_acpi_pm_clocksource(void)
return -ENODEV;
}
- return clocksource_register_hz(&clocksource_acpi_pm,
+ clocksource_register_hz(&clocksource_acpi_pm,
PMTMR_TICKS_PER_SEC);
+ return 0;
}
/* We use fs_initcall because we want the PCI fixups to have run
diff --git a/drivers/clocksource/cadence_ttc_timer.c b/drivers/clocksource/cadence_ttc_timer.c
index 63f176d..b9b56ed 100644
--- a/drivers/clocksource/cadence_ttc_timer.c
+++ b/drivers/clocksource/cadence_ttc_timer.c
@@ -301,11 +301,7 @@ static void __init ttc_setup_clocksource(struct clk *clk, void __iomem *base)
__raw_writel(CNT_CNTRL_RESET,
ttccs->ttc.base_addr + TTC_CNT_CNTRL_OFFSET);
- err = clocksource_register_hz(&ttccs->cs, ttccs->ttc.freq / PRESCALE);
- if (WARN_ON(err)) {
- kfree(ttccs);
- return;
- }
+ clocksource_register_hz(&ttccs->cs, ttccs->ttc.freq / PRESCALE);
ttc_sched_clock_val_reg = base + TTC_COUNT_VAL_OFFSET;
sched_clock_register(ttc_sched_clock_read, 16, ttccs->ttc.freq / PRESCALE);
diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 62b0de6..98649c7 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -193,9 +193,7 @@ struct clocksource mct_frc = {
static void __init exynos4_clocksource_init(void)
{
exynos4_mct_frc_start(0, 0);
-
- if (clocksource_register_hz(&mct_frc, clk_rate))
- panic("%s: can't register clocksource\n", mct_frc.name);
+ clocksource_register_hz(&mct_frc, clk_rate);
}
static void exynos4_mct_comp0_stop(void)
diff --git a/drivers/clocksource/i8253.c b/drivers/clocksource/i8253.c
index 14ee3ef..9c45f0a 100644
--- a/drivers/clocksource/i8253.c
+++ b/drivers/clocksource/i8253.c
@@ -95,7 +95,8 @@ static struct clocksource i8253_cs = {
int __init clocksource_i8253_init(void)
{
- return clocksource_register_hz(&i8253_cs, PIT_TICK_RATE);
+ clocksource_register_hz(&i8253_cs, PIT_TICK_RATE);
+ return 0;
}
#endif
diff --git a/drivers/clocksource/mmio.c b/drivers/clocksource/mmio.c
index c0e2512..6e0b530 100644
--- a/drivers/clocksource/mmio.c
+++ b/drivers/clocksource/mmio.c
@@ -69,5 +69,6 @@ int __init clocksource_mmio_init(void __iomem *base, const char *name,
cs->clksrc.mask = CLOCKSOURCE_MASK(bits);
cs->clksrc.flags = CLOCK_SOURCE_IS_CONTINUOUS;
- return clocksource_register_hz(&cs->clksrc, hz);
+ clocksource_register_hz(&cs->clksrc, hz);
+ return 0;
}
diff --git a/drivers/clocksource/samsung_pwm_timer.c b/drivers/clocksource/samsung_pwm_timer.c
index 5645cfc..c59292f 100644
--- a/drivers/clocksource/samsung_pwm_timer.c
+++ b/drivers/clocksource/samsung_pwm_timer.c
@@ -340,7 +340,6 @@ static void __init samsung_clocksource_init(void)
{
unsigned long pclk;
unsigned long clock_rate;
- int ret;
pclk = clk_get_rate(pwm.timerclk);
@@ -361,9 +360,7 @@ static void __init samsung_clocksource_init(void)
pwm.variant.bits, clock_rate);
samsung_clocksource.mask = CLOCKSOURCE_MASK(pwm.variant.bits);
- ret = clocksource_register_hz(&samsung_clocksource, clock_rate);
- if (ret)
- panic("samsung_clocksource_timer: can't register clocksource\n");
+ clocksource_register_hz(&samsung_clocksource, clock_rate);
}
static void __init samsung_timer_resources(void)
diff --git a/drivers/clocksource/scx200_hrt.c b/drivers/clocksource/scx200_hrt.c
index 64f9e82..57bdc04 100644
--- a/drivers/clocksource/scx200_hrt.c
+++ b/drivers/clocksource/scx200_hrt.c
@@ -83,7 +83,8 @@ static int __init init_hrt_clocksource(void)
pr_info("enabling scx200 high-res timer (%s MHz +%d ppm)\n", mhz27 ? "27":"1", ppm);
- return clocksource_register_hz(&cs_hrt, freq);
+ clocksource_register_hz(&cs_hrt, freq);
+ return 0;
}
module_init(init_hrt_clocksource);
diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
index 00fdd11..805245d 100644
--- a/drivers/clocksource/tcb_clksrc.c
+++ b/drivers/clocksource/tcb_clksrc.c
@@ -340,9 +340,7 @@ static int __init tcb_clksrc_init(void)
}
/* and away we go! */
- ret = clocksource_register_hz(&clksrc, divided_rate);
- if (ret)
- goto err_disable_t1;
+ clocksource_register_hz(&clksrc, divided_rate);
/* channel 2: periodic and oneshot timer support */
ret = setup_clkevents(tc, clk32k_divisor_idx);
@@ -354,10 +352,6 @@ static int __init tcb_clksrc_init(void)
err_unregister_clksrc:
clocksource_unregister(&clksrc);
-err_disable_t1:
- if (!tc->tcb_config || tc->tcb_config->counter_width != 32)
- clk_disable_unprepare(tc->clk[1]);
-
err_disable_t0:
clk_disable_unprepare(t0_clk);
diff --git a/drivers/clocksource/timer-marco.c b/drivers/clocksource/timer-marco.c
index 09a17d9..ae78ce0 100644
--- a/drivers/clocksource/timer-marco.c
+++ b/drivers/clocksource/timer-marco.c
@@ -283,7 +283,7 @@ static void __init sirfsoc_marco_timer_init(void)
/* Clear all interrupts */
writel_relaxed(0xFFFF, sirfsoc_timer_base + SIRFSOC_TIMER_INTR_STATUS);
- BUG_ON(clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE));
+ clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE);
sirfsoc_clockevent_init();
}
diff --git a/drivers/clocksource/timer-prima2.c b/drivers/clocksource/timer-prima2.c
index 8a492d3..c9cc307 100644
--- a/drivers/clocksource/timer-prima2.c
+++ b/drivers/clocksource/timer-prima2.c
@@ -204,7 +204,7 @@ static void __init sirfsoc_prima2_timer_init(struct device_node *np)
writel_relaxed(0, sirfsoc_timer_base + SIRFSOC_TIMER_COUNTER_HI);
writel_relaxed(BIT(0), sirfsoc_timer_base + SIRFSOC_TIMER_STATUS);
- BUG_ON(clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE));
+ clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE);
sched_clock_register(sirfsoc_read_sched_clock, 64, CLOCK_TICK_RATE);
diff --git a/drivers/clocksource/vt8500_timer.c b/drivers/clocksource/vt8500_timer.c
index 1098ed3..13f5fa4 100644
--- a/drivers/clocksource/vt8500_timer.c
+++ b/drivers/clocksource/vt8500_timer.c
@@ -150,9 +150,7 @@ static void __init vt8500_timer_init(struct device_node *np)
writel(0xf, regbase + TIMER_STATUS_VAL);
writel(~0, regbase + TIMER_MATCH_VAL);
- if (clocksource_register_hz(&clocksource, VT8500_TIMER_HZ))
- pr_err("%s: vt8500_timer_init: clocksource_register failed for %s\n",
- __func__, clocksource.name);
+ clocksource_register_hz(&clocksource, VT8500_TIMER_HZ);
clockevent.cpumask = cpumask_of(0);
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 67301a4..5a17c5e 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -282,7 +282,7 @@ static inline s64 clocksource_cyc2ns(cycle_t cycles, u32 mult, u32 shift)
}
-extern int clocksource_register(struct clocksource*);
+extern void clocksource_register(struct clocksource *);
extern int clocksource_unregister(struct clocksource*);
extern void clocksource_touch_watchdog(void);
extern struct clocksource* clocksource_get_next(void);
@@ -301,17 +301,17 @@ clocks_calc_mult_shift(u32 *mult, u32 *shift, u32 from, u32 to, u32 minsec);
* Don't call __clocksource_register_scale directly, use
* clocksource_register_hz/khz
*/
-extern int
+extern void
__clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq);
extern void
__clocksource_updatefreq_scale(struct clocksource *cs, u32 scale, u32 freq);
-static inline int clocksource_register_hz(struct clocksource *cs, u32 hz)
+static inline void clocksource_register_hz(struct clocksource *cs, u32 hz)
{
return __clocksource_register_scale(cs, 1, hz);
}
-static inline int clocksource_register_khz(struct clocksource *cs, u32 khz)
+static inline void clocksource_register_khz(struct clocksource *cs, u32 khz)
{
return __clocksource_register_scale(cs, 1000, khz);
}
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 9951575..686ff72 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -782,7 +782,7 @@ EXPORT_SYMBOL_GPL(__clocksource_updatefreq_scale);
* This *SHOULD NOT* be called directly! Please use the
* clocksource_register_hz() or clocksource_register_khz helper functions.
*/
-int __clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq)
+void __clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq)
{
/* Initialize mult/shift and max_idle_ns */
@@ -794,7 +794,6 @@ int __clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq)
clocksource_enqueue_watchdog(cs);
clocksource_select();
mutex_unlock(&clocksource_mutex);
- return 0;
}
EXPORT_SYMBOL_GPL(__clocksource_register_scale);
@@ -804,7 +803,7 @@ EXPORT_SYMBOL_GPL(__clocksource_register_scale);
* @cs: clocksource to be registered
*
*/
-int clocksource_register(struct clocksource *cs)
+void clocksource_register(struct clocksource *cs)
{
/* calculate max adjustment for given mult/shift */
cs->maxadj = clocksource_max_adjustment(cs);
@@ -820,7 +819,6 @@ int clocksource_register(struct clocksource *cs)
clocksource_enqueue_watchdog(cs);
clocksource_select();
mutex_unlock(&clocksource_mutex);
- return 0;
}
EXPORT_SYMBOL(clocksource_register);
diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
index 7a925ba..ae4c534 100644
--- a/kernel/time/jiffies.c
+++ b/kernel/time/jiffies.c
@@ -88,7 +88,8 @@ EXPORT_SYMBOL(jiffies);
static int __init init_jiffies_clocksource(void)
{
- return clocksource_register(&clocksource_jiffies);
+ clocksource_register(&clocksource_jiffies);
+ return 0;
}
core_initcall(init_jiffies_clocksource);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 2/2] clocksource: Make clocksource register functions void
From: Tony Prisk @ 2014-01-23 7:45 UTC (permalink / raw)
To: Yijing Wang, John Stultz, Thomas Gleixner
Cc: linux-mips, Daniel Lezcano, Kevin Hilman, linux, Sekhar Nori,
Paul Mackerras, H. Peter Anvin, Daniel Walker,
Hans-Christian Egtvedt, Jonas Bonn, Kukjin Kim, Russell King,
Richard Weinberger, x86, Tony Lindgren, Ingo Molnar,
linux-arm-msm, David Brown, Haavard Skinnemoen, Mike Frysinger,
user-mode-linux-devel, Nicolas Ferre, Jeff Dike, Barry Song,
linux-samsung-soc, user-mode-linux-user, linux-omap,
linux-arm-kernel, davinci-linux-open-source, Michal Simek,
Jim Cromie, microblaze-uclinux, Hanjun Guo, linux-kernel,
Ralf Baechle, Bryan Huntsman, uclinux-dist-devel, linuxppc-dev
In-Reply-To: <1390461166-36440-1-git-send-email-wangyijing@huawei.com>
On 23/01/14 20:12, Yijing Wang wrote:
> Currently, clocksource_register() and __clocksource_register_scale()
> functions always return 0, it's pointless, make functions void.
> And remove the dead code that check the clocksource_register_hz()
> return value.
>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
......
> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> index 67301a4..5a17c5e 100644
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -282,7 +282,7 @@ static inline s64 clocksource_cyc2ns(cycle_t cycles, u32 mult, u32 shift)
> }
>
>
> -extern int clocksource_register(struct clocksource*);
> +extern void clocksource_register(struct clocksource *);
> extern int clocksource_unregister(struct clocksource*);
> extern void clocksource_touch_watchdog(void);
> extern struct clocksource* clocksource_get_next(void);
> @@ -301,17 +301,17 @@ clocks_calc_mult_shift(u32 *mult, u32 *shift, u32 from, u32 to, u32 minsec);
> * Don't call __clocksource_register_scale directly, use
> * clocksource_register_hz/khz
> */
> -extern int
> +extern void
> __clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq);
> extern void
> __clocksource_updatefreq_scale(struct clocksource *cs, u32 scale, u32 freq);
>
> -static inline int clocksource_register_hz(struct clocksource *cs, u32 hz)
> +static inline void clocksource_register_hz(struct clocksource *cs, u32 hz)
> {
> return __clocksource_register_scale(cs, 1, hz);
> }
This doesn't make sense - you are still returning a value on a function
declared void, and the return is now from a function that doesn't return
anything either ?!?!
Doesn't this throw a compile-time warning??
Regards
Tony Prisk
^ permalink raw reply
* Re: [PATCH 2/2] clocksource: Make clocksource register functions void
From: Geert Uytterhoeven @ 2014-01-23 7:58 UTC (permalink / raw)
To: Tony Prisk
Cc: Linux MIPS Mailing List, the arch/x86 maintainers, Kevin Hilman,
linux, Sekhar Nori, Michal Simek, Paul Mackerras, Ralf Baechle,
H. Peter Anvin, Yijing Wang, Daniel Walker,
Hans-Christian Egtvedt, Jonas Bonn, Kukjin Kim, Russell King,
Richard Weinberger, Daniel Lezcano, Tony Lindgren, Ingo Molnar,
microblaze-uclinux, David Brown, Haavard Skinnemoen,
Mike Frysinger, uml-devel, linux-arm-msm, Jeff Dike,
davinci-linux-open-source, linux-samsung-soc, John Stultz,
uml-user, Thomas Gleixner, linux-omap@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Barry Song, Jim Cromie,
Hanjun Guo, linux-kernel@vger.kernel.org, Nicolas Ferre,
Bryan Huntsman, uclinux-dist-devel@blackfin.uclinux.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <52E0C889.6000106@prisktech.co.nz>
On Thu, Jan 23, 2014 at 8:45 AM, Tony Prisk <linux@prisktech.co.nz> wrote:
>> -static inline int clocksource_register_hz(struct clocksource *cs, u32
>> hz)
>> +static inline void clocksource_register_hz(struct clocksource *cs, u32
>> hz)
>> {
>> return __clocksource_register_scale(cs, 1, hz);
>> }
>
>
> This doesn't make sense - you are still returning a value on a function
> declared void, and the return is now from a function that doesn't return
> anything either ?!?!
> Doesn't this throw a compile-time warning??
No, passing on void in functions returning void doesn't cause compiler
warnings.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 2/2] clocksource: Make clocksource register functions void
From: Tony Prisk @ 2014-01-23 8:04 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linux MIPS Mailing List, the arch/x86 maintainers, Kevin Hilman,
linux, Sekhar Nori, Michal Simek, Paul Mackerras, Ralf Baechle,
H. Peter Anvin, Yijing Wang, Daniel Walker,
Hans-Christian Egtvedt, Jonas Bonn, Kukjin Kim, Russell King,
Richard Weinberger, Daniel Lezcano, Tony Lindgren, Ingo Molnar,
microblaze-uclinux, David Brown, Haavard Skinnemoen,
Mike Frysinger, uml-devel, linux-arm-msm, Jeff Dike,
davinci-linux-open-source, linux-samsung-soc, John Stultz,
uml-user, Thomas Gleixner, linux-omap@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Barry Song, Jim Cromie,
Hanjun Guo, linux-kernel@vger.kernel.org, Nicolas Ferre,
Bryan Huntsman, uclinux-dist-devel@blackfin.uclinux.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <CAMuHMdUcKb8m71Z7dUo86MQ_KZgPujxsduUUt3Mz8Oke+DLSVw@mail.gmail.com>
On 23/01/14 20:58, Geert Uytterhoeven wrote:
> On Thu, Jan 23, 2014 at 8:45 AM, Tony Prisk <linux@prisktech.co.nz> wrote:
>>> -static inline int clocksource_register_hz(struct clocksource *cs, u32
>>> hz)
>>> +static inline void clocksource_register_hz(struct clocksource *cs, u32
>>> hz)
>>> {
>>> return __clocksource_register_scale(cs, 1, hz);
>>> }
>>
>> This doesn't make sense - you are still returning a value on a function
>> declared void, and the return is now from a function that doesn't return
>> anything either ?!?!
>> Doesn't this throw a compile-time warning??
> No, passing on void in functions returning void doesn't cause compiler
> warnings.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
Doesn't seem right to me (even if there is no warning) but that's
probably because I used to program in Pascal where functions with no
return were 'procedures' :)
Whether it needs to be changed or not:
For the vt8500 part -
Acked-by: Tony Prisk <linux@prisktech.co.nz>
Regards
Tony Prisk
^ permalink raw reply
* Re: [PATCH 2/2] clocksource: Make clocksource register functions void
From: Hans-Christian Egtvedt @ 2014-01-23 7:40 UTC (permalink / raw)
To: Yijing Wang
Cc: linux-mips, Daniel Lezcano, Kevin Hilman, linux, Sekhar Nori,
Paul Mackerras, H. Peter Anvin, Daniel Walker, Jonas Bonn,
Kukjin Kim, Russell King, Richard Weinberger, x86, Tony Lindgren,
Ingo Molnar, linux-arm-msm, David Brown, Haavard Skinnemoen,
Mike Frysinger, user-mode-linux-devel, Nicolas Ferre, Jeff Dike,
Barry Song, linux-samsung-soc, John Stultz, user-mode-linux-user,
Thomas Gleixner, linux-omap, linux-arm-kernel,
davinci-linux-open-source, Michal Simek, Jim Cromie,
microblaze-uclinux, Hanjun Guo, linux-kernel, Ralf Baechle,
Tony Prisk, Bryan Huntsman, uclinux-dist-devel, linuxppc-dev
In-Reply-To: <1390461166-36440-1-git-send-email-wangyijing@huawei.com>
Around Thu 23 Jan 2014 15:12:46 +0800 or thereabout, Yijing Wang wrote:
> Currently, clocksource_register() and __clocksource_register_scale()
> functions always return 0, it's pointless, make functions void.
> And remove the dead code that check the clocksource_register_hz()
> return value.
>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
For the avr32 related change
Acked-by: Hans-Christian Egtvedt <egtvedt@samfundet.no>
> ---
> arch/arm/mach-davinci/time.c | 5 ++---
> arch/arm/mach-msm/timer.c | 4 +---
> arch/arm/mach-omap2/timer.c | 8 +++-----
> arch/avr32/kernel/time.c | 4 +---
> arch/blackfin/kernel/time-ts.c | 6 ++----
> arch/microblaze/kernel/timer.c | 3 +--
> arch/mips/jz4740/time.c | 6 +-----
> arch/mips/loongson/common/cs5536/cs5536_mfgpt.c | 3 ++-
> arch/openrisc/kernel/time.c | 3 +--
> arch/powerpc/kernel/time.c | 6 +-----
> arch/um/kernel/time.c | 6 +-----
> arch/x86/platform/uv/uv_time.c | 14 ++++++--------
> drivers/clocksource/acpi_pm.c | 3 ++-
> drivers/clocksource/cadence_ttc_timer.c | 6 +-----
> drivers/clocksource/exynos_mct.c | 4 +---
> drivers/clocksource/i8253.c | 3 ++-
> drivers/clocksource/mmio.c | 3 ++-
> drivers/clocksource/samsung_pwm_timer.c | 5 +----
> drivers/clocksource/scx200_hrt.c | 3 ++-
> drivers/clocksource/tcb_clksrc.c | 8 +-------
> drivers/clocksource/timer-marco.c | 2 +-
> drivers/clocksource/timer-prima2.c | 2 +-
> drivers/clocksource/vt8500_timer.c | 4 +---
> include/linux/clocksource.h | 8 ++++----
> kernel/time/clocksource.c | 6 ++----
> kernel/time/jiffies.c | 3 ++-
> 26 files changed, 45 insertions(+), 83 deletions(-)
<snipp diffs>
--
HcE
^ permalink raw reply
* Re: [PATCH 2/2] clocksource: Make clocksource register functions void
From: Yijing Wang @ 2014-01-23 8:17 UTC (permalink / raw)
To: Tony Prisk, Geert Uytterhoeven
Cc: Linux MIPS Mailing List, the arch/x86 maintainers, Kevin Hilman,
linux, Hanjun Guo, Sekhar Nori, Michal Simek, Paul Mackerras,
Ralf Baechle, H. Peter Anvin, Daniel Walker,
Hans-Christian Egtvedt, Jonas Bonn, Kukjin Kim, Russell King,
Richard Weinberger, Daniel Lezcano, Tony Lindgren, Ingo Molnar,
microblaze-uclinux, David Brown, Haavard Skinnemoen,
Mike Frysinger, uml-devel, linux-arm-msm, Jeff Dike,
davinci-linux-open-source, linux-samsung-soc, John Stultz,
uml-user, Thomas Gleixner, linux-omap@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Barry Song, Jim Cromie,
linux-kernel@vger.kernel.org, Nicolas Ferre, Bryan Huntsman,
uclinux-dist-devel@blackfin.uclinux.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <52E0CD18.5080104@prisktech.co.nz>
On 2014/1/23 16:04, Tony Prisk wrote:
> On 23/01/14 20:58, Geert Uytterhoeven wrote:
>> On Thu, Jan 23, 2014 at 8:45 AM, Tony Prisk <linux@prisktech.co.nz> wrote:
>>>> -static inline int clocksource_register_hz(struct clocksource *cs, u32
>>>> hz)
>>>> +static inline void clocksource_register_hz(struct clocksource *cs, u32
>>>> hz)
>>>> {
>>>> return __clocksource_register_scale(cs, 1, hz);
>>>> }
>>>
>>> This doesn't make sense - you are still returning a value on a function
>>> declared void, and the return is now from a function that doesn't return
>>> anything either ?!?!
>>> Doesn't this throw a compile-time warning??
>> No, passing on void in functions returning void doesn't cause compiler
>> warnings.
>>
>> Gr{oetje,eeting}s,
>>
>> Geert
>>
>> --
>> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>>
>> In personal conversations with technical people, I call myself a hacker. But
>> when I'm talking to journalists I just say "programmer" or something like that.
>> -- Linus Torvalds
> Doesn't seem right to me (even if there is no warning) but that's probably because I used to program in Pascal where functions with no return were 'procedures' :)
> Whether it needs to be changed or not:
>
> For the vt8500 part -
> Acked-by: Tony Prisk <linux@prisktech.co.nz>
Thanks!
>
> Regards
> Tony Prisk
>
>
> .
>
--
Thanks!
Yijing
^ permalink raw reply
* Re: [PATCH 2/2] clocksource: Make clocksource register functions void
From: Daniel Lezcano @ 2014-01-23 8:40 UTC (permalink / raw)
To: Yijing Wang, John Stultz, Thomas Gleixner
Cc: linux-mips, Kevin Hilman, linux, Sekhar Nori, Paul Mackerras,
H. Peter Anvin, Daniel Walker, Hans-Christian Egtvedt, Jonas Bonn,
Kukjin Kim, Russell King, Richard Weinberger, x86, Tony Lindgren,
Ingo Molnar, linux-arm-msm, David Brown, Haavard Skinnemoen,
Mike Frysinger, user-mode-linux-devel, Nicolas Ferre, Jeff Dike,
Barry Song, linux-samsung-soc, user-mode-linux-user, linux-omap,
linux-arm-kernel, davinci-linux-open-source, Michal Simek,
Jim Cromie, microblaze-uclinux, Hanjun Guo, linux-kernel,
Ralf Baechle, Tony Prisk, Bryan Huntsman, uclinux-dist-devel,
linuxppc-dev
In-Reply-To: <1390461166-36440-1-git-send-email-wangyijing@huawei.com>
On 01/23/2014 08:12 AM, Yijing Wang wrote:
> Currently, clocksource_register() and __clocksource_register_scale()
> functions always return 0, it's pointless, make functions void.
> And remove the dead code that check the clocksource_register_hz()
> return value.
>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Well, do we really want to change all these files to not take care of a
return value ? What about is we have to check it again later ?
I would recommend to investigate __clocksource_register_scale and the
underneath functions if there is not an error to be returned in the call
stack somewhere which is ignored today.
The same applies for clocksource_register.
Thanks
-- Daniel
> ---
> arch/arm/mach-davinci/time.c | 5 ++---
> arch/arm/mach-msm/timer.c | 4 +---
> arch/arm/mach-omap2/timer.c | 8 +++-----
> arch/avr32/kernel/time.c | 4 +---
> arch/blackfin/kernel/time-ts.c | 6 ++----
> arch/microblaze/kernel/timer.c | 3 +--
> arch/mips/jz4740/time.c | 6 +-----
> arch/mips/loongson/common/cs5536/cs5536_mfgpt.c | 3 ++-
> arch/openrisc/kernel/time.c | 3 +--
> arch/powerpc/kernel/time.c | 6 +-----
> arch/um/kernel/time.c | 6 +-----
> arch/x86/platform/uv/uv_time.c | 14 ++++++--------
> drivers/clocksource/acpi_pm.c | 3 ++-
> drivers/clocksource/cadence_ttc_timer.c | 6 +-----
> drivers/clocksource/exynos_mct.c | 4 +---
> drivers/clocksource/i8253.c | 3 ++-
> drivers/clocksource/mmio.c | 3 ++-
> drivers/clocksource/samsung_pwm_timer.c | 5 +----
> drivers/clocksource/scx200_hrt.c | 3 ++-
> drivers/clocksource/tcb_clksrc.c | 8 +-------
> drivers/clocksource/timer-marco.c | 2 +-
> drivers/clocksource/timer-prima2.c | 2 +-
> drivers/clocksource/vt8500_timer.c | 4 +---
> include/linux/clocksource.h | 8 ++++----
> kernel/time/clocksource.c | 6 ++----
> kernel/time/jiffies.c | 3 ++-
> 26 files changed, 45 insertions(+), 83 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c
> index 56c6eb5..9536f85 100644
> --- a/arch/arm/mach-davinci/time.c
> +++ b/arch/arm/mach-davinci/time.c
> @@ -387,9 +387,8 @@ void __init davinci_timer_init(void)
>
> /* setup clocksource */
> clocksource_davinci.name = id_to_name[clocksource_id];
> - if (clocksource_register_hz(&clocksource_davinci,
> - davinci_clock_tick_rate))
> - printk(err, clocksource_davinci.name);
> + clocksource_register_hz(&clocksource_davinci,
> + davinci_clock_tick_rate);
>
> setup_sched_clock(davinci_read_sched_clock, 32,
> davinci_clock_tick_rate);
> diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
> index 1e9c338..c96e034 100644
> --- a/arch/arm/mach-msm/timer.c
> +++ b/arch/arm/mach-msm/timer.c
> @@ -226,9 +226,7 @@ static void __init msm_timer_init(u32 dgt_hz, int sched_bits, int irq,
>
> err:
> writel_relaxed(TIMER_ENABLE_EN, source_base + TIMER_ENABLE);
> - res = clocksource_register_hz(cs, dgt_hz);
> - if (res)
> - pr_err("clocksource_register failed\n");
> + clocksource_register_hz(cs, dgt_hz);
> setup_sched_clock(msm_sched_clock_read, sched_bits, dgt_hz);
> }
>
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index 3ca81e0..beaf7c7 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -473,11 +473,9 @@ static void __init omap2_gptimer_clocksource_init(int gptimer_id,
> OMAP_TIMER_NONPOSTED);
> setup_sched_clock(dmtimer_read_sched_clock, 32, clksrc.rate);
>
> - if (clocksource_register_hz(&clocksource_gpt, clksrc.rate))
> - pr_err("Could not register clocksource %s\n",
> - clocksource_gpt.name);
> - else
> - pr_info("OMAP clocksource: %s at %lu Hz\n",
> + clocksource_register_hz(&clocksource_gpt, clksrc.rate);
> +
> + pr_info("OMAP clocksource: %s at %lu Hz\n",
> clocksource_gpt.name, clksrc.rate);
> }
>
> diff --git a/arch/avr32/kernel/time.c b/arch/avr32/kernel/time.c
> index d0f771b..51b4a66 100644
> --- a/arch/avr32/kernel/time.c
> +++ b/arch/avr32/kernel/time.c
> @@ -134,9 +134,7 @@ void __init time_init(void)
>
> /* figure rate for counter */
> counter_hz = clk_get_rate(boot_cpu_data.clk);
> - ret = clocksource_register_hz(&counter, counter_hz);
> - if (ret)
> - pr_debug("timer: could not register clocksource: %d\n", ret);
> + clocksource_register_hz(&counter, counter_hz);
>
> /* setup COMPARE clockevent */
> comparator.mult = div_sc(counter_hz, NSEC_PER_SEC, comparator.shift);
> diff --git a/arch/blackfin/kernel/time-ts.c b/arch/blackfin/kernel/time-ts.c
> index cb0a484..df3bb08 100644
> --- a/arch/blackfin/kernel/time-ts.c
> +++ b/arch/blackfin/kernel/time-ts.c
> @@ -51,8 +51,7 @@ static inline unsigned long long bfin_cs_cycles_sched_clock(void)
>
> static int __init bfin_cs_cycles_init(void)
> {
> - if (clocksource_register_hz(&bfin_cs_cycles, get_cclk()))
> - panic("failed to register clocksource");
> + clocksource_register_hz(&bfin_cs_cycles, get_cclk());
>
> return 0;
> }
> @@ -103,8 +102,7 @@ static int __init bfin_cs_gptimer0_init(void)
> {
> setup_gptimer0();
>
> - if (clocksource_register_hz(&bfin_cs_gptimer0, get_sclk()))
> - panic("failed to register clocksource");
> + clocksource_register_hz(&bfin_cs_gptimer0, get_sclk());
>
> return 0;
> }
> diff --git a/arch/microblaze/kernel/timer.c b/arch/microblaze/kernel/timer.c
> index 3e39b10..6a2417e 100644
> --- a/arch/microblaze/kernel/timer.c
> +++ b/arch/microblaze/kernel/timer.c
> @@ -208,8 +208,7 @@ static struct clocksource clocksource_microblaze = {
>
> static int __init xilinx_clocksource_init(void)
> {
> - if (clocksource_register_hz(&clocksource_microblaze, timer_clock_freq))
> - panic("failed to register clocksource");
> + clocksource_register_hz(&clocksource_microblaze, timer_clock_freq);
>
> /* stop timer1 */
> out_be32(timer_baseaddr + TCSR1,
> diff --git a/arch/mips/jz4740/time.c b/arch/mips/jz4740/time.c
> index 5e430ce..041cdff 100644
> --- a/arch/mips/jz4740/time.c
> +++ b/arch/mips/jz4740/time.c
> @@ -105,7 +105,6 @@ static struct irqaction timer_irqaction = {
>
> void __init plat_time_init(void)
> {
> - int ret;
> uint32_t clk_rate;
> uint16_t ctrl;
>
> @@ -121,10 +120,7 @@ void __init plat_time_init(void)
>
> clockevents_register_device(&jz4740_clockevent);
>
> - ret = clocksource_register_hz(&jz4740_clocksource, clk_rate);
> -
> - if (ret)
> - printk(KERN_ERR "Failed to register clocksource: %d\n", ret);
> + clocksource_register_hz(&jz4740_clocksource, clk_rate);
>
> setup_irq(JZ4740_IRQ_TCU0, &timer_irqaction);
>
> diff --git a/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c b/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c
> index c639b9d..9fa6d99 100644
> --- a/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c
> +++ b/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c
> @@ -208,7 +208,8 @@ int __init init_mfgpt_clocksource(void)
> if (num_possible_cpus() > 1) /* MFGPT does not scale! */
> return 0;
>
> - return clocksource_register_hz(&clocksource_mfgpt, MFGPT_TICK_RATE);
> + clocksource_register_hz(&clocksource_mfgpt, MFGPT_TICK_RATE);
> + return 0;
> }
>
> arch_initcall(init_mfgpt_clocksource);
> diff --git a/arch/openrisc/kernel/time.c b/arch/openrisc/kernel/time.c
> index 7c52e94..3f789aa 100644
> --- a/arch/openrisc/kernel/time.c
> +++ b/arch/openrisc/kernel/time.c
> @@ -156,8 +156,7 @@ static struct clocksource openrisc_timer = {
>
> static int __init openrisc_timer_init(void)
> {
> - if (clocksource_register_hz(&openrisc_timer, cpuinfo.clock_frequency))
> - panic("failed to register clocksource");
> + clocksource_register_hz(&openrisc_timer, cpuinfo.clock_frequency);
>
> /* Enable the incrementer: 'continuous' mode with interrupt disabled */
> mtspr(SPR_TTMR, SPR_TTMR_CR);
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index b3b1441..27c0627 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -788,11 +788,7 @@ static void __init clocksource_init(void)
> else
> clock = &clocksource_timebase;
>
> - if (clocksource_register_hz(clock, tb_ticks_per_sec)) {
> - printk(KERN_ERR "clocksource: %s is already registered\n",
> - clock->name);
> - return;
> - }
> + clocksource_register_hz(clock, tb_ticks_per_sec);
>
> printk(KERN_INFO "clocksource: %s mult[%x] shift[%d] registered\n",
> clock->name, clock->mult, clock->shift);
> diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c
> index 117568d..2034b58 100644
> --- a/arch/um/kernel/time.c
> +++ b/arch/um/kernel/time.c
> @@ -92,11 +92,7 @@ static void __init setup_itimer(void)
> clockevent_delta2ns(60 * HZ, &itimer_clockevent);
> itimer_clockevent.min_delta_ns =
> clockevent_delta2ns(1, &itimer_clockevent);
> - err = clocksource_register_hz(&itimer_clocksource, USEC_PER_SEC);
> - if (err) {
> - printk(KERN_ERR "clocksource_register_hz returned %d\n", err);
> - return;
> - }
> + clocksource_register_hz(&itimer_clocksource, USEC_PER_SEC);
> clockevents_register_device(&itimer_clockevent);
> }
>
> diff --git a/arch/x86/platform/uv/uv_time.c b/arch/x86/platform/uv/uv_time.c
> index 5c86786..b963774 100644
> --- a/arch/x86/platform/uv/uv_time.c
> +++ b/arch/x86/platform/uv/uv_time.c
> @@ -379,15 +379,13 @@ static __init int uv_rtc_setup_clock(void)
> if (!is_uv_system())
> return -ENODEV;
>
> - rc = clocksource_register_hz(&clocksource_uv, sn_rtc_cycles_per_second);
> - if (rc)
> - printk(KERN_INFO "UV RTC clocksource failed rc %d\n", rc);
> - else
> - printk(KERN_INFO "UV RTC clocksource registered freq %lu MHz\n",
> - sn_rtc_cycles_per_second/(unsigned long)1E6);
> + clocksource_register_hz(&clocksource_uv, sn_rtc_cycles_per_second);
> +
> + pr_info("UV RTC clocksource registered freq %lu MHz\n",
> + sn_rtc_cycles_per_second/(unsigned long)1E6);
>
> - if (rc || !uv_rtc_evt_enable || x86_platform_ipi_callback)
> - return rc;
> + if (!uv_rtc_evt_enable || x86_platform_ipi_callback)
> + return 0;
>
> /* Setup and register clockevents */
> rc = uv_rtc_allocate_timers();
> diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c
> index 6eab889..ab1dc63 100644
> --- a/drivers/clocksource/acpi_pm.c
> +++ b/drivers/clocksource/acpi_pm.c
> @@ -218,8 +218,9 @@ static int __init init_acpi_pm_clocksource(void)
> return -ENODEV;
> }
>
> - return clocksource_register_hz(&clocksource_acpi_pm,
> + clocksource_register_hz(&clocksource_acpi_pm,
> PMTMR_TICKS_PER_SEC);
> + return 0;
> }
>
> /* We use fs_initcall because we want the PCI fixups to have run
> diff --git a/drivers/clocksource/cadence_ttc_timer.c b/drivers/clocksource/cadence_ttc_timer.c
> index 63f176d..b9b56ed 100644
> --- a/drivers/clocksource/cadence_ttc_timer.c
> +++ b/drivers/clocksource/cadence_ttc_timer.c
> @@ -301,11 +301,7 @@ static void __init ttc_setup_clocksource(struct clk *clk, void __iomem *base)
> __raw_writel(CNT_CNTRL_RESET,
> ttccs->ttc.base_addr + TTC_CNT_CNTRL_OFFSET);
>
> - err = clocksource_register_hz(&ttccs->cs, ttccs->ttc.freq / PRESCALE);
> - if (WARN_ON(err)) {
> - kfree(ttccs);
> - return;
> - }
> + clocksource_register_hz(&ttccs->cs, ttccs->ttc.freq / PRESCALE);
>
> ttc_sched_clock_val_reg = base + TTC_COUNT_VAL_OFFSET;
> sched_clock_register(ttc_sched_clock_read, 16, ttccs->ttc.freq / PRESCALE);
> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
> index 62b0de6..98649c7 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -193,9 +193,7 @@ struct clocksource mct_frc = {
> static void __init exynos4_clocksource_init(void)
> {
> exynos4_mct_frc_start(0, 0);
> -
> - if (clocksource_register_hz(&mct_frc, clk_rate))
> - panic("%s: can't register clocksource\n", mct_frc.name);
> + clocksource_register_hz(&mct_frc, clk_rate);
> }
>
> static void exynos4_mct_comp0_stop(void)
> diff --git a/drivers/clocksource/i8253.c b/drivers/clocksource/i8253.c
> index 14ee3ef..9c45f0a 100644
> --- a/drivers/clocksource/i8253.c
> +++ b/drivers/clocksource/i8253.c
> @@ -95,7 +95,8 @@ static struct clocksource i8253_cs = {
>
> int __init clocksource_i8253_init(void)
> {
> - return clocksource_register_hz(&i8253_cs, PIT_TICK_RATE);
> + clocksource_register_hz(&i8253_cs, PIT_TICK_RATE);
> + return 0;
> }
> #endif
>
> diff --git a/drivers/clocksource/mmio.c b/drivers/clocksource/mmio.c
> index c0e2512..6e0b530 100644
> --- a/drivers/clocksource/mmio.c
> +++ b/drivers/clocksource/mmio.c
> @@ -69,5 +69,6 @@ int __init clocksource_mmio_init(void __iomem *base, const char *name,
> cs->clksrc.mask = CLOCKSOURCE_MASK(bits);
> cs->clksrc.flags = CLOCK_SOURCE_IS_CONTINUOUS;
>
> - return clocksource_register_hz(&cs->clksrc, hz);
> + clocksource_register_hz(&cs->clksrc, hz);
> + return 0;
> }
> diff --git a/drivers/clocksource/samsung_pwm_timer.c b/drivers/clocksource/samsung_pwm_timer.c
> index 5645cfc..c59292f 100644
> --- a/drivers/clocksource/samsung_pwm_timer.c
> +++ b/drivers/clocksource/samsung_pwm_timer.c
> @@ -340,7 +340,6 @@ static void __init samsung_clocksource_init(void)
> {
> unsigned long pclk;
> unsigned long clock_rate;
> - int ret;
>
> pclk = clk_get_rate(pwm.timerclk);
>
> @@ -361,9 +360,7 @@ static void __init samsung_clocksource_init(void)
> pwm.variant.bits, clock_rate);
>
> samsung_clocksource.mask = CLOCKSOURCE_MASK(pwm.variant.bits);
> - ret = clocksource_register_hz(&samsung_clocksource, clock_rate);
> - if (ret)
> - panic("samsung_clocksource_timer: can't register clocksource\n");
> + clocksource_register_hz(&samsung_clocksource, clock_rate);
> }
>
> static void __init samsung_timer_resources(void)
> diff --git a/drivers/clocksource/scx200_hrt.c b/drivers/clocksource/scx200_hrt.c
> index 64f9e82..57bdc04 100644
> --- a/drivers/clocksource/scx200_hrt.c
> +++ b/drivers/clocksource/scx200_hrt.c
> @@ -83,7 +83,8 @@ static int __init init_hrt_clocksource(void)
>
> pr_info("enabling scx200 high-res timer (%s MHz +%d ppm)\n", mhz27 ? "27":"1", ppm);
>
> - return clocksource_register_hz(&cs_hrt, freq);
> + clocksource_register_hz(&cs_hrt, freq);
> + return 0;
> }
>
> module_init(init_hrt_clocksource);
> diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
> index 00fdd11..805245d 100644
> --- a/drivers/clocksource/tcb_clksrc.c
> +++ b/drivers/clocksource/tcb_clksrc.c
> @@ -340,9 +340,7 @@ static int __init tcb_clksrc_init(void)
> }
>
> /* and away we go! */
> - ret = clocksource_register_hz(&clksrc, divided_rate);
> - if (ret)
> - goto err_disable_t1;
> + clocksource_register_hz(&clksrc, divided_rate);
>
> /* channel 2: periodic and oneshot timer support */
> ret = setup_clkevents(tc, clk32k_divisor_idx);
> @@ -354,10 +352,6 @@ static int __init tcb_clksrc_init(void)
> err_unregister_clksrc:
> clocksource_unregister(&clksrc);
>
> -err_disable_t1:
> - if (!tc->tcb_config || tc->tcb_config->counter_width != 32)
> - clk_disable_unprepare(tc->clk[1]);
> -
> err_disable_t0:
> clk_disable_unprepare(t0_clk);
>
> diff --git a/drivers/clocksource/timer-marco.c b/drivers/clocksource/timer-marco.c
> index 09a17d9..ae78ce0 100644
> --- a/drivers/clocksource/timer-marco.c
> +++ b/drivers/clocksource/timer-marco.c
> @@ -283,7 +283,7 @@ static void __init sirfsoc_marco_timer_init(void)
> /* Clear all interrupts */
> writel_relaxed(0xFFFF, sirfsoc_timer_base + SIRFSOC_TIMER_INTR_STATUS);
>
> - BUG_ON(clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE));
> + clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE);
>
> sirfsoc_clockevent_init();
> }
> diff --git a/drivers/clocksource/timer-prima2.c b/drivers/clocksource/timer-prima2.c
> index 8a492d3..c9cc307 100644
> --- a/drivers/clocksource/timer-prima2.c
> +++ b/drivers/clocksource/timer-prima2.c
> @@ -204,7 +204,7 @@ static void __init sirfsoc_prima2_timer_init(struct device_node *np)
> writel_relaxed(0, sirfsoc_timer_base + SIRFSOC_TIMER_COUNTER_HI);
> writel_relaxed(BIT(0), sirfsoc_timer_base + SIRFSOC_TIMER_STATUS);
>
> - BUG_ON(clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE));
> + clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE);
>
> sched_clock_register(sirfsoc_read_sched_clock, 64, CLOCK_TICK_RATE);
>
> diff --git a/drivers/clocksource/vt8500_timer.c b/drivers/clocksource/vt8500_timer.c
> index 1098ed3..13f5fa4 100644
> --- a/drivers/clocksource/vt8500_timer.c
> +++ b/drivers/clocksource/vt8500_timer.c
> @@ -150,9 +150,7 @@ static void __init vt8500_timer_init(struct device_node *np)
> writel(0xf, regbase + TIMER_STATUS_VAL);
> writel(~0, regbase + TIMER_MATCH_VAL);
>
> - if (clocksource_register_hz(&clocksource, VT8500_TIMER_HZ))
> - pr_err("%s: vt8500_timer_init: clocksource_register failed for %s\n",
> - __func__, clocksource.name);
> + clocksource_register_hz(&clocksource, VT8500_TIMER_HZ);
>
> clockevent.cpumask = cpumask_of(0);
>
> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> index 67301a4..5a17c5e 100644
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -282,7 +282,7 @@ static inline s64 clocksource_cyc2ns(cycle_t cycles, u32 mult, u32 shift)
> }
>
>
> -extern int clocksource_register(struct clocksource*);
> +extern void clocksource_register(struct clocksource *);
> extern int clocksource_unregister(struct clocksource*);
> extern void clocksource_touch_watchdog(void);
> extern struct clocksource* clocksource_get_next(void);
> @@ -301,17 +301,17 @@ clocks_calc_mult_shift(u32 *mult, u32 *shift, u32 from, u32 to, u32 minsec);
> * Don't call __clocksource_register_scale directly, use
> * clocksource_register_hz/khz
> */
> -extern int
> +extern void
> __clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq);
> extern void
> __clocksource_updatefreq_scale(struct clocksource *cs, u32 scale, u32 freq);
>
> -static inline int clocksource_register_hz(struct clocksource *cs, u32 hz)
> +static inline void clocksource_register_hz(struct clocksource *cs, u32 hz)
> {
> return __clocksource_register_scale(cs, 1, hz);
> }
>
> -static inline int clocksource_register_khz(struct clocksource *cs, u32 khz)
> +static inline void clocksource_register_khz(struct clocksource *cs, u32 khz)
> {
> return __clocksource_register_scale(cs, 1000, khz);
> }
> diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
> index 9951575..686ff72 100644
> --- a/kernel/time/clocksource.c
> +++ b/kernel/time/clocksource.c
> @@ -782,7 +782,7 @@ EXPORT_SYMBOL_GPL(__clocksource_updatefreq_scale);
> * This *SHOULD NOT* be called directly! Please use the
> * clocksource_register_hz() or clocksource_register_khz helper functions.
> */
> -int __clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq)
> +void __clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq)
> {
>
> /* Initialize mult/shift and max_idle_ns */
> @@ -794,7 +794,6 @@ int __clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq)
> clocksource_enqueue_watchdog(cs);
> clocksource_select();
> mutex_unlock(&clocksource_mutex);
> - return 0;
> }
> EXPORT_SYMBOL_GPL(__clocksource_register_scale);
>
> @@ -804,7 +803,7 @@ EXPORT_SYMBOL_GPL(__clocksource_register_scale);
> * @cs: clocksource to be registered
> *
> */
> -int clocksource_register(struct clocksource *cs)
> +void clocksource_register(struct clocksource *cs)
> {
> /* calculate max adjustment for given mult/shift */
> cs->maxadj = clocksource_max_adjustment(cs);
> @@ -820,7 +819,6 @@ int clocksource_register(struct clocksource *cs)
> clocksource_enqueue_watchdog(cs);
> clocksource_select();
> mutex_unlock(&clocksource_mutex);
> - return 0;
> }
> EXPORT_SYMBOL(clocksource_register);
>
> diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
> index 7a925ba..ae4c534 100644
> --- a/kernel/time/jiffies.c
> +++ b/kernel/time/jiffies.c
> @@ -88,7 +88,8 @@ EXPORT_SYMBOL(jiffies);
>
> static int __init init_jiffies_clocksource(void)
> {
> - return clocksource_register(&clocksource_jiffies);
> + clocksource_register(&clocksource_jiffies);
> + return 0;
> }
>
> core_initcall(init_jiffies_clocksource);
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH 2/2] clocksource: Make clocksource register functions void
From: Yijing Wang @ 2014-01-23 9:01 UTC (permalink / raw)
To: Daniel Lezcano, John Stultz, Thomas Gleixner
Cc: linux-mips, Kevin Hilman, linux, Hanjun Guo, Sekhar Nori,
Paul Mackerras, H. Peter Anvin, Daniel Walker,
Hans-Christian Egtvedt, Jonas Bonn, Kukjin Kim, Russell King,
Richard Weinberger, x86, Tony Lindgren, Ingo Molnar,
linux-arm-msm, David Brown, Haavard Skinnemoen, Mike Frysinger,
user-mode-linux-devel, Nicolas Ferre, Jeff Dike, Barry Song,
linux-samsung-soc, user-mode-linux-user, linux-omap,
linux-arm-kernel, davinci-linux-open-source, Michal Simek,
Jim Cromie, microblaze-uclinux, linux-kernel, Ralf Baechle,
Tony Prisk, Bryan Huntsman, uclinux-dist-devel, linuxppc-dev
In-Reply-To: <52E0D575.5050702@linaro.org>
On 2014/1/23 16:40, Daniel Lezcano wrote:
> On 01/23/2014 08:12 AM, Yijing Wang wrote:
>> Currently, clocksource_register() and __clocksource_register_scale()
>> functions always return 0, it's pointless, make functions void.
>> And remove the dead code that check the clocksource_register_hz()
>> return value.
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>
> Well, do we really want to change all these files to not take care of a return value ? What about is we have to check it again later ?
>
> I would recommend to investigate __clocksource_register_scale and the underneath functions if there is not an error to be returned in the call stack somewhere which is ignored today.
>
> The same applies for clocksource_register.
Hi Daniel, thanks for your comment, all functions type under __clocksource_register_scale() are void.
This just is trivial cleanup patch, I don't know whether we will check these function return value again later.
If there is some possibility to check it again, I agree to keep them. Of course, this is all determined by Thomas.
Thanks!
Yijing.
>
>> ---
>> arch/arm/mach-davinci/time.c | 5 ++---
>> arch/arm/mach-msm/timer.c | 4 +---
>> arch/arm/mach-omap2/timer.c | 8 +++-----
>> arch/avr32/kernel/time.c | 4 +---
>> arch/blackfin/kernel/time-ts.c | 6 ++----
>> arch/microblaze/kernel/timer.c | 3 +--
>> arch/mips/jz4740/time.c | 6 +-----
>> arch/mips/loongson/common/cs5536/cs5536_mfgpt.c | 3 ++-
>> arch/openrisc/kernel/time.c | 3 +--
>> arch/powerpc/kernel/time.c | 6 +-----
>> arch/um/kernel/time.c | 6 +-----
>> arch/x86/platform/uv/uv_time.c | 14 ++++++--------
>> drivers/clocksource/acpi_pm.c | 3 ++-
>> drivers/clocksource/cadence_ttc_timer.c | 6 +-----
>> drivers/clocksource/exynos_mct.c | 4 +---
>> drivers/clocksource/i8253.c | 3 ++-
>> drivers/clocksource/mmio.c | 3 ++-
>> drivers/clocksource/samsung_pwm_timer.c | 5 +----
>> drivers/clocksource/scx200_hrt.c | 3 ++-
>> drivers/clocksource/tcb_clksrc.c | 8 +-------
>> drivers/clocksource/timer-marco.c | 2 +-
>> drivers/clocksource/timer-prima2.c | 2 +-
>> drivers/clocksource/vt8500_timer.c | 4 +---
>> include/linux/clocksource.h | 8 ++++----
>> kernel/time/clocksource.c | 6 ++----
>> kernel/time/jiffies.c | 3 ++-
>> 26 files changed, 45 insertions(+), 83 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c
>> index 56c6eb5..9536f85 100644
>> --- a/arch/arm/mach-davinci/time.c
>> +++ b/arch/arm/mach-davinci/time.c
>> @@ -387,9 +387,8 @@ void __init davinci_timer_init(void)
>>
>> /* setup clocksource */
>> clocksource_davinci.name = id_to_name[clocksource_id];
>> - if (clocksource_register_hz(&clocksource_davinci,
>> - davinci_clock_tick_rate))
>> - printk(err, clocksource_davinci.name);
>> + clocksource_register_hz(&clocksource_davinci,
>> + davinci_clock_tick_rate);
>>
>> setup_sched_clock(davinci_read_sched_clock, 32,
>> davinci_clock_tick_rate);
>> diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
>> index 1e9c338..c96e034 100644
>> --- a/arch/arm/mach-msm/timer.c
>> +++ b/arch/arm/mach-msm/timer.c
>> @@ -226,9 +226,7 @@ static void __init msm_timer_init(u32 dgt_hz, int sched_bits, int irq,
>>
>> err:
>> writel_relaxed(TIMER_ENABLE_EN, source_base + TIMER_ENABLE);
>> - res = clocksource_register_hz(cs, dgt_hz);
>> - if (res)
>> - pr_err("clocksource_register failed\n");
>> + clocksource_register_hz(cs, dgt_hz);
>> setup_sched_clock(msm_sched_clock_read, sched_bits, dgt_hz);
>> }
>>
>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>> index 3ca81e0..beaf7c7 100644
>> --- a/arch/arm/mach-omap2/timer.c
>> +++ b/arch/arm/mach-omap2/timer.c
>> @@ -473,11 +473,9 @@ static void __init omap2_gptimer_clocksource_init(int gptimer_id,
>> OMAP_TIMER_NONPOSTED);
>> setup_sched_clock(dmtimer_read_sched_clock, 32, clksrc.rate);
>>
>> - if (clocksource_register_hz(&clocksource_gpt, clksrc.rate))
>> - pr_err("Could not register clocksource %s\n",
>> - clocksource_gpt.name);
>> - else
>> - pr_info("OMAP clocksource: %s at %lu Hz\n",
>> + clocksource_register_hz(&clocksource_gpt, clksrc.rate);
>> +
>> + pr_info("OMAP clocksource: %s at %lu Hz\n",
>> clocksource_gpt.name, clksrc.rate);
>> }
>>
>> diff --git a/arch/avr32/kernel/time.c b/arch/avr32/kernel/time.c
>> index d0f771b..51b4a66 100644
>> --- a/arch/avr32/kernel/time.c
>> +++ b/arch/avr32/kernel/time.c
>> @@ -134,9 +134,7 @@ void __init time_init(void)
>>
>> /* figure rate for counter */
>> counter_hz = clk_get_rate(boot_cpu_data.clk);
>> - ret = clocksource_register_hz(&counter, counter_hz);
>> - if (ret)
>> - pr_debug("timer: could not register clocksource: %d\n", ret);
>> + clocksource_register_hz(&counter, counter_hz);
>>
>> /* setup COMPARE clockevent */
>> comparator.mult = div_sc(counter_hz, NSEC_PER_SEC, comparator.shift);
>> diff --git a/arch/blackfin/kernel/time-ts.c b/arch/blackfin/kernel/time-ts.c
>> index cb0a484..df3bb08 100644
>> --- a/arch/blackfin/kernel/time-ts.c
>> +++ b/arch/blackfin/kernel/time-ts.c
>> @@ -51,8 +51,7 @@ static inline unsigned long long bfin_cs_cycles_sched_clock(void)
>>
>> static int __init bfin_cs_cycles_init(void)
>> {
>> - if (clocksource_register_hz(&bfin_cs_cycles, get_cclk()))
>> - panic("failed to register clocksource");
>> + clocksource_register_hz(&bfin_cs_cycles, get_cclk());
>>
>> return 0;
>> }
>> @@ -103,8 +102,7 @@ static int __init bfin_cs_gptimer0_init(void)
>> {
>> setup_gptimer0();
>>
>> - if (clocksource_register_hz(&bfin_cs_gptimer0, get_sclk()))
>> - panic("failed to register clocksource");
>> + clocksource_register_hz(&bfin_cs_gptimer0, get_sclk());
>>
>> return 0;
>> }
>> diff --git a/arch/microblaze/kernel/timer.c b/arch/microblaze/kernel/timer.c
>> index 3e39b10..6a2417e 100644
>> --- a/arch/microblaze/kernel/timer.c
>> +++ b/arch/microblaze/kernel/timer.c
>> @@ -208,8 +208,7 @@ static struct clocksource clocksource_microblaze = {
>>
>> static int __init xilinx_clocksource_init(void)
>> {
>> - if (clocksource_register_hz(&clocksource_microblaze, timer_clock_freq))
>> - panic("failed to register clocksource");
>> + clocksource_register_hz(&clocksource_microblaze, timer_clock_freq);
>>
>> /* stop timer1 */
>> out_be32(timer_baseaddr + TCSR1,
>> diff --git a/arch/mips/jz4740/time.c b/arch/mips/jz4740/time.c
>> index 5e430ce..041cdff 100644
>> --- a/arch/mips/jz4740/time.c
>> +++ b/arch/mips/jz4740/time.c
>> @@ -105,7 +105,6 @@ static struct irqaction timer_irqaction = {
>>
>> void __init plat_time_init(void)
>> {
>> - int ret;
>> uint32_t clk_rate;
>> uint16_t ctrl;
>>
>> @@ -121,10 +120,7 @@ void __init plat_time_init(void)
>>
>> clockevents_register_device(&jz4740_clockevent);
>>
>> - ret = clocksource_register_hz(&jz4740_clocksource, clk_rate);
>> -
>> - if (ret)
>> - printk(KERN_ERR "Failed to register clocksource: %d\n", ret);
>> + clocksource_register_hz(&jz4740_clocksource, clk_rate);
>>
>> setup_irq(JZ4740_IRQ_TCU0, &timer_irqaction);
>>
>> diff --git a/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c b/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c
>> index c639b9d..9fa6d99 100644
>> --- a/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c
>> +++ b/arch/mips/loongson/common/cs5536/cs5536_mfgpt.c
>> @@ -208,7 +208,8 @@ int __init init_mfgpt_clocksource(void)
>> if (num_possible_cpus() > 1) /* MFGPT does not scale! */
>> return 0;
>>
>> - return clocksource_register_hz(&clocksource_mfgpt, MFGPT_TICK_RATE);
>> + clocksource_register_hz(&clocksource_mfgpt, MFGPT_TICK_RATE);
>> + return 0;
>> }
>>
>> arch_initcall(init_mfgpt_clocksource);
>> diff --git a/arch/openrisc/kernel/time.c b/arch/openrisc/kernel/time.c
>> index 7c52e94..3f789aa 100644
>> --- a/arch/openrisc/kernel/time.c
>> +++ b/arch/openrisc/kernel/time.c
>> @@ -156,8 +156,7 @@ static struct clocksource openrisc_timer = {
>>
>> static int __init openrisc_timer_init(void)
>> {
>> - if (clocksource_register_hz(&openrisc_timer, cpuinfo.clock_frequency))
>> - panic("failed to register clocksource");
>> + clocksource_register_hz(&openrisc_timer, cpuinfo.clock_frequency);
>>
>> /* Enable the incrementer: 'continuous' mode with interrupt disabled */
>> mtspr(SPR_TTMR, SPR_TTMR_CR);
>> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
>> index b3b1441..27c0627 100644
>> --- a/arch/powerpc/kernel/time.c
>> +++ b/arch/powerpc/kernel/time.c
>> @@ -788,11 +788,7 @@ static void __init clocksource_init(void)
>> else
>> clock = &clocksource_timebase;
>>
>> - if (clocksource_register_hz(clock, tb_ticks_per_sec)) {
>> - printk(KERN_ERR "clocksource: %s is already registered\n",
>> - clock->name);
>> - return;
>> - }
>> + clocksource_register_hz(clock, tb_ticks_per_sec);
>>
>> printk(KERN_INFO "clocksource: %s mult[%x] shift[%d] registered\n",
>> clock->name, clock->mult, clock->shift);
>> diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c
>> index 117568d..2034b58 100644
>> --- a/arch/um/kernel/time.c
>> +++ b/arch/um/kernel/time.c
>> @@ -92,11 +92,7 @@ static void __init setup_itimer(void)
>> clockevent_delta2ns(60 * HZ, &itimer_clockevent);
>> itimer_clockevent.min_delta_ns =
>> clockevent_delta2ns(1, &itimer_clockevent);
>> - err = clocksource_register_hz(&itimer_clocksource, USEC_PER_SEC);
>> - if (err) {
>> - printk(KERN_ERR "clocksource_register_hz returned %d\n", err);
>> - return;
>> - }
>> + clocksource_register_hz(&itimer_clocksource, USEC_PER_SEC);
>> clockevents_register_device(&itimer_clockevent);
>> }
>>
>> diff --git a/arch/x86/platform/uv/uv_time.c b/arch/x86/platform/uv/uv_time.c
>> index 5c86786..b963774 100644
>> --- a/arch/x86/platform/uv/uv_time.c
>> +++ b/arch/x86/platform/uv/uv_time.c
>> @@ -379,15 +379,13 @@ static __init int uv_rtc_setup_clock(void)
>> if (!is_uv_system())
>> return -ENODEV;
>>
>> - rc = clocksource_register_hz(&clocksource_uv, sn_rtc_cycles_per_second);
>> - if (rc)
>> - printk(KERN_INFO "UV RTC clocksource failed rc %d\n", rc);
>> - else
>> - printk(KERN_INFO "UV RTC clocksource registered freq %lu MHz\n",
>> - sn_rtc_cycles_per_second/(unsigned long)1E6);
>> + clocksource_register_hz(&clocksource_uv, sn_rtc_cycles_per_second);
>> +
>> + pr_info("UV RTC clocksource registered freq %lu MHz\n",
>> + sn_rtc_cycles_per_second/(unsigned long)1E6);
>>
>> - if (rc || !uv_rtc_evt_enable || x86_platform_ipi_callback)
>> - return rc;
>> + if (!uv_rtc_evt_enable || x86_platform_ipi_callback)
>> + return 0;
>>
>> /* Setup and register clockevents */
>> rc = uv_rtc_allocate_timers();
>> diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c
>> index 6eab889..ab1dc63 100644
>> --- a/drivers/clocksource/acpi_pm.c
>> +++ b/drivers/clocksource/acpi_pm.c
>> @@ -218,8 +218,9 @@ static int __init init_acpi_pm_clocksource(void)
>> return -ENODEV;
>> }
>>
>> - return clocksource_register_hz(&clocksource_acpi_pm,
>> + clocksource_register_hz(&clocksource_acpi_pm,
>> PMTMR_TICKS_PER_SEC);
>> + return 0;
>> }
>>
>> /* We use fs_initcall because we want the PCI fixups to have run
>> diff --git a/drivers/clocksource/cadence_ttc_timer.c b/drivers/clocksource/cadence_ttc_timer.c
>> index 63f176d..b9b56ed 100644
>> --- a/drivers/clocksource/cadence_ttc_timer.c
>> +++ b/drivers/clocksource/cadence_ttc_timer.c
>> @@ -301,11 +301,7 @@ static void __init ttc_setup_clocksource(struct clk *clk, void __iomem *base)
>> __raw_writel(CNT_CNTRL_RESET,
>> ttccs->ttc.base_addr + TTC_CNT_CNTRL_OFFSET);
>>
>> - err = clocksource_register_hz(&ttccs->cs, ttccs->ttc.freq / PRESCALE);
>> - if (WARN_ON(err)) {
>> - kfree(ttccs);
>> - return;
>> - }
>> + clocksource_register_hz(&ttccs->cs, ttccs->ttc.freq / PRESCALE);
>>
>> ttc_sched_clock_val_reg = base + TTC_COUNT_VAL_OFFSET;
>> sched_clock_register(ttc_sched_clock_read, 16, ttccs->ttc.freq / PRESCALE);
>> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
>> index 62b0de6..98649c7 100644
>> --- a/drivers/clocksource/exynos_mct.c
>> +++ b/drivers/clocksource/exynos_mct.c
>> @@ -193,9 +193,7 @@ struct clocksource mct_frc = {
>> static void __init exynos4_clocksource_init(void)
>> {
>> exynos4_mct_frc_start(0, 0);
>> -
>> - if (clocksource_register_hz(&mct_frc, clk_rate))
>> - panic("%s: can't register clocksource\n", mct_frc.name);
>> + clocksource_register_hz(&mct_frc, clk_rate);
>> }
>>
>> static void exynos4_mct_comp0_stop(void)
>> diff --git a/drivers/clocksource/i8253.c b/drivers/clocksource/i8253.c
>> index 14ee3ef..9c45f0a 100644
>> --- a/drivers/clocksource/i8253.c
>> +++ b/drivers/clocksource/i8253.c
>> @@ -95,7 +95,8 @@ static struct clocksource i8253_cs = {
>>
>> int __init clocksource_i8253_init(void)
>> {
>> - return clocksource_register_hz(&i8253_cs, PIT_TICK_RATE);
>> + clocksource_register_hz(&i8253_cs, PIT_TICK_RATE);
>> + return 0;
>> }
>> #endif
>>
>> diff --git a/drivers/clocksource/mmio.c b/drivers/clocksource/mmio.c
>> index c0e2512..6e0b530 100644
>> --- a/drivers/clocksource/mmio.c
>> +++ b/drivers/clocksource/mmio.c
>> @@ -69,5 +69,6 @@ int __init clocksource_mmio_init(void __iomem *base, const char *name,
>> cs->clksrc.mask = CLOCKSOURCE_MASK(bits);
>> cs->clksrc.flags = CLOCK_SOURCE_IS_CONTINUOUS;
>>
>> - return clocksource_register_hz(&cs->clksrc, hz);
>> + clocksource_register_hz(&cs->clksrc, hz);
>> + return 0;
>> }
>> diff --git a/drivers/clocksource/samsung_pwm_timer.c b/drivers/clocksource/samsung_pwm_timer.c
>> index 5645cfc..c59292f 100644
>> --- a/drivers/clocksource/samsung_pwm_timer.c
>> +++ b/drivers/clocksource/samsung_pwm_timer.c
>> @@ -340,7 +340,6 @@ static void __init samsung_clocksource_init(void)
>> {
>> unsigned long pclk;
>> unsigned long clock_rate;
>> - int ret;
>>
>> pclk = clk_get_rate(pwm.timerclk);
>>
>> @@ -361,9 +360,7 @@ static void __init samsung_clocksource_init(void)
>> pwm.variant.bits, clock_rate);
>>
>> samsung_clocksource.mask = CLOCKSOURCE_MASK(pwm.variant.bits);
>> - ret = clocksource_register_hz(&samsung_clocksource, clock_rate);
>> - if (ret)
>> - panic("samsung_clocksource_timer: can't register clocksource\n");
>> + clocksource_register_hz(&samsung_clocksource, clock_rate);
>> }
>>
>> static void __init samsung_timer_resources(void)
>> diff --git a/drivers/clocksource/scx200_hrt.c b/drivers/clocksource/scx200_hrt.c
>> index 64f9e82..57bdc04 100644
>> --- a/drivers/clocksource/scx200_hrt.c
>> +++ b/drivers/clocksource/scx200_hrt.c
>> @@ -83,7 +83,8 @@ static int __init init_hrt_clocksource(void)
>>
>> pr_info("enabling scx200 high-res timer (%s MHz +%d ppm)\n", mhz27 ? "27":"1", ppm);
>>
>> - return clocksource_register_hz(&cs_hrt, freq);
>> + clocksource_register_hz(&cs_hrt, freq);
>> + return 0;
>> }
>>
>> module_init(init_hrt_clocksource);
>> diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
>> index 00fdd11..805245d 100644
>> --- a/drivers/clocksource/tcb_clksrc.c
>> +++ b/drivers/clocksource/tcb_clksrc.c
>> @@ -340,9 +340,7 @@ static int __init tcb_clksrc_init(void)
>> }
>>
>> /* and away we go! */
>> - ret = clocksource_register_hz(&clksrc, divided_rate);
>> - if (ret)
>> - goto err_disable_t1;
>> + clocksource_register_hz(&clksrc, divided_rate);
>>
>> /* channel 2: periodic and oneshot timer support */
>> ret = setup_clkevents(tc, clk32k_divisor_idx);
>> @@ -354,10 +352,6 @@ static int __init tcb_clksrc_init(void)
>> err_unregister_clksrc:
>> clocksource_unregister(&clksrc);
>>
>> -err_disable_t1:
>> - if (!tc->tcb_config || tc->tcb_config->counter_width != 32)
>> - clk_disable_unprepare(tc->clk[1]);
>> -
>> err_disable_t0:
>> clk_disable_unprepare(t0_clk);
>>
>> diff --git a/drivers/clocksource/timer-marco.c b/drivers/clocksource/timer-marco.c
>> index 09a17d9..ae78ce0 100644
>> --- a/drivers/clocksource/timer-marco.c
>> +++ b/drivers/clocksource/timer-marco.c
>> @@ -283,7 +283,7 @@ static void __init sirfsoc_marco_timer_init(void)
>> /* Clear all interrupts */
>> writel_relaxed(0xFFFF, sirfsoc_timer_base + SIRFSOC_TIMER_INTR_STATUS);
>>
>> - BUG_ON(clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE));
>> + clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE);
>>
>> sirfsoc_clockevent_init();
>> }
>> diff --git a/drivers/clocksource/timer-prima2.c b/drivers/clocksource/timer-prima2.c
>> index 8a492d3..c9cc307 100644
>> --- a/drivers/clocksource/timer-prima2.c
>> +++ b/drivers/clocksource/timer-prima2.c
>> @@ -204,7 +204,7 @@ static void __init sirfsoc_prima2_timer_init(struct device_node *np)
>> writel_relaxed(0, sirfsoc_timer_base + SIRFSOC_TIMER_COUNTER_HI);
>> writel_relaxed(BIT(0), sirfsoc_timer_base + SIRFSOC_TIMER_STATUS);
>>
>> - BUG_ON(clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE));
>> + clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE);
>>
>> sched_clock_register(sirfsoc_read_sched_clock, 64, CLOCK_TICK_RATE);
>>
>> diff --git a/drivers/clocksource/vt8500_timer.c b/drivers/clocksource/vt8500_timer.c
>> index 1098ed3..13f5fa4 100644
>> --- a/drivers/clocksource/vt8500_timer.c
>> +++ b/drivers/clocksource/vt8500_timer.c
>> @@ -150,9 +150,7 @@ static void __init vt8500_timer_init(struct device_node *np)
>> writel(0xf, regbase + TIMER_STATUS_VAL);
>> writel(~0, regbase + TIMER_MATCH_VAL);
>>
>> - if (clocksource_register_hz(&clocksource, VT8500_TIMER_HZ))
>> - pr_err("%s: vt8500_timer_init: clocksource_register failed for %s\n",
>> - __func__, clocksource.name);
>> + clocksource_register_hz(&clocksource, VT8500_TIMER_HZ);
>>
>> clockevent.cpumask = cpumask_of(0);
>>
>> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
>> index 67301a4..5a17c5e 100644
>> --- a/include/linux/clocksource.h
>> +++ b/include/linux/clocksource.h
>> @@ -282,7 +282,7 @@ static inline s64 clocksource_cyc2ns(cycle_t cycles, u32 mult, u32 shift)
>> }
>>
>>
>> -extern int clocksource_register(struct clocksource*);
>> +extern void clocksource_register(struct clocksource *);
>> extern int clocksource_unregister(struct clocksource*);
>> extern void clocksource_touch_watchdog(void);
>> extern struct clocksource* clocksource_get_next(void);
>> @@ -301,17 +301,17 @@ clocks_calc_mult_shift(u32 *mult, u32 *shift, u32 from, u32 to, u32 minsec);
>> * Don't call __clocksource_register_scale directly, use
>> * clocksource_register_hz/khz
>> */
>> -extern int
>> +extern void
>> __clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq);
>> extern void
>> __clocksource_updatefreq_scale(struct clocksource *cs, u32 scale, u32 freq);
>>
>> -static inline int clocksource_register_hz(struct clocksource *cs, u32 hz)
>> +static inline void clocksource_register_hz(struct clocksource *cs, u32 hz)
>> {
>> return __clocksource_register_scale(cs, 1, hz);
>> }
>>
>> -static inline int clocksource_register_khz(struct clocksource *cs, u32 khz)
>> +static inline void clocksource_register_khz(struct clocksource *cs, u32 khz)
>> {
>> return __clocksource_register_scale(cs, 1000, khz);
>> }
>> diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
>> index 9951575..686ff72 100644
>> --- a/kernel/time/clocksource.c
>> +++ b/kernel/time/clocksource.c
>> @@ -782,7 +782,7 @@ EXPORT_SYMBOL_GPL(__clocksource_updatefreq_scale);
>> * This *SHOULD NOT* be called directly! Please use the
>> * clocksource_register_hz() or clocksource_register_khz helper functions.
>> */
>> -int __clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq)
>> +void __clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq)
>> {
>>
>> /* Initialize mult/shift and max_idle_ns */
>> @@ -794,7 +794,6 @@ int __clocksource_register_scale(struct clocksource *cs, u32 scale, u32 freq)
>> clocksource_enqueue_watchdog(cs);
>> clocksource_select();
>> mutex_unlock(&clocksource_mutex);
>> - return 0;
>> }
>> EXPORT_SYMBOL_GPL(__clocksource_register_scale);
>>
>> @@ -804,7 +803,7 @@ EXPORT_SYMBOL_GPL(__clocksource_register_scale);
>> * @cs: clocksource to be registered
>> *
>> */
>> -int clocksource_register(struct clocksource *cs)
>> +void clocksource_register(struct clocksource *cs)
>> {
>> /* calculate max adjustment for given mult/shift */
>> cs->maxadj = clocksource_max_adjustment(cs);
>> @@ -820,7 +819,6 @@ int clocksource_register(struct clocksource *cs)
>> clocksource_enqueue_watchdog(cs);
>> clocksource_select();
>> mutex_unlock(&clocksource_mutex);
>> - return 0;
>> }
>> EXPORT_SYMBOL(clocksource_register);
>>
>> diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
>> index 7a925ba..ae4c534 100644
>> --- a/kernel/time/jiffies.c
>> +++ b/kernel/time/jiffies.c
>> @@ -88,7 +88,8 @@ EXPORT_SYMBOL(jiffies);
>>
>> static int __init init_jiffies_clocksource(void)
>> {
>> - return clocksource_register(&clocksource_jiffies);
>> + clocksource_register(&clocksource_jiffies);
>> + return 0;
>> }
>>
>> core_initcall(init_jiffies_clocksource);
>>
>
>
--
Thanks!
Yijing
^ permalink raw reply
* Re: [PATCH V2] cpuidle/governors: Fix logic in selection of idle states
From: Preeti U Murthy @ 2014-01-23 11:15 UTC (permalink / raw)
To: Daniel Lezcano
Cc: paulmck, linux-pm, rjw, linux-kernel, srivatsa.bhat, linuxppc-dev,
tuukka.tikkanen
In-Reply-To: <52DF815D.50104@linaro.org>
Hi Daniel,
Thank you for the review.
On 01/22/2014 01:59 PM, Daniel Lezcano wrote:
> On 01/17/2014 05:33 AM, Preeti U Murthy wrote:
>>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index a55e68f..831b664 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -131,8 +131,9 @@ int cpuidle_idle_call(void)
>>
>> /* ask the governor for the next state */
>> next_state = cpuidle_curr_governor->select(drv, dev);
>> +
>> + dev->last_residency = 0;
>> if (need_resched()) {
>> - dev->last_residency = 0;
>
> Why do you need to do this change ? ^^^^^
So as to keep the last_residency consistent with the case that this patch
addresses: where no idle state could be selected due to strict latency
requirements or disabled states and hence the cpu exits without entering
idle. Else it would contain the stale value from the previous idle state
entry.
But coming to think of it dev->last_residency is not used when the last
entered idle state index is -1.
So I have reverted this change as well in the revised patch below along
with mentioning the reason in the last paragraph of the changelog.
>
>> /* give the governor an opportunity to reflect on the
>> outcome */
>> if (cpuidle_curr_governor->reflect)
>> cpuidle_curr_governor->reflect(dev, next_state);
>> @@ -140,6 +141,18 @@ int cpuidle_idle_call(void)
>> return 0;
>> }
>>
>> + /* Unlike in the need_resched() case, we return here because the
>> + * governor did not find a suitable idle state. However idle is
>> still
>> + * in progress as we are not asked to reschedule. Hence we return
>> + * without enabling interrupts.
>
> That will lead to a WARN.
>
>> + * NOTE: The return code should still be success, since the
>> verdict of this
>> + * call is "do not enter any idle state" and not a failed call
>> due to
>> + * errors.
>> + */
>> + if (next_state < 0)
>> + return 0;
>> +
>
> Returning from here breaks the symmetry of the trace.
I have addressed the above concerns in the patch found below.
Does the rest of the patch look sound?
Regards
Preeti U Murthy
----------------------------------------------------------------------
cpuidle/governors: Fix logic in selection of idle states
From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
The cpuidle governors today are not handling scenarios where no idle state
can be chosen. Such scenarios coud arise if the user has disabled all the
idle states at runtime or the latency requirement from the cpus is very strict.
The menu governor returns 0th index of the idle state table when no other
idle state is suitable. This is even when the idle state corresponding to this
index is disabled or the latency requirement is strict and the exit_latency
of the lowest idle state is also not acceptable. Hence this patch
fixes this logic in the menu governor by defaulting to an idle state index
of -1 unless any other state is suitable.
The ladder governor needs a few more fixes in addition to that required in the
menu governor. When the ladder governor decides to demote the idle state of a
CPU, it does not check if the lower idle states are enabled. Add this logic
in addition to the logic where it chooses an index of -1 if it can neither
promote or demote the idle state of a cpu nor can it choose the current idle
state.
The cpuidle_idle_call() will return back if the governor decides upon not
entering any idle state. However it cannot return an error code because all
archs have the logic today that if the call to cpuidle_idle_call() fails, it
means that the cpuidle driver failed to *function*; for instance due to
errors during registration. As a result they end up deciding upon a
default idle state on their own, which could very well be a deep idle state.
This is incorrect in cases where no idle state is suitable.
Besides for the scenario that this patch is addressing, the call actually
succeeds. Its just that no idle state is thought to be suitable by the governors.
Under such a circumstance return success code without entering any idle
state.
The consequence of this patch additionally on the menu governor is that as
long as a valid idle state cannot be chosen, the cpuidle statistics that this
governor uses to predict the next idle state remain untouched from the last
valid idle state. This is because an idle state is not even being predicted
in this path, hence there is no point correcting the prediction either.
Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
Changes from V1:https://lkml.org/lkml/2014/1/14/26
1. Change the return code to success from -EINVAL due to the reason mentioned
in the changelog.
2. Add logic that the patch is addressing in the ladder governor as well.
3. Added relevant comments and removed redundant logic as suggested in the
above thread.
---
drivers/cpuidle/cpuidle.c | 15 +++++
drivers/cpuidle/governors/ladder.c | 101 ++++++++++++++++++++++++++----------
| 7 +-
3 files changed, 90 insertions(+), 33 deletions(-)
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index a55e68f..19d17e8 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -131,8 +131,9 @@ int cpuidle_idle_call(void)
/* ask the governor for the next state */
next_state = cpuidle_curr_governor->select(drv, dev);
+
+ dev->last_residency = 0;
if (need_resched()) {
- dev->last_residency = 0;
/* give the governor an opportunity to reflect on the outcome */
if (cpuidle_curr_governor->reflect)
cpuidle_curr_governor->reflect(dev, next_state);
@@ -141,6 +142,16 @@ int cpuidle_idle_call(void)
}
trace_cpu_idle_rcuidle(next_state, dev->cpu);
+ /*
+ * NOTE: The return code should still be success, since the verdict of
+ * this call is "do not enter any idle state". It is not a failed call
+ * due to errors.
+ */
+ if (next_state < 0) {
+ entered_state = next_state;
+ local_irq_enable();
+ goto out;
+ }
broadcast = !!(drv->states[next_state].flags & CPUIDLE_FLAG_TIMER_STOP);
@@ -156,7 +167,7 @@ int cpuidle_idle_call(void)
if (broadcast)
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
- trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);
+out: trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);
/* give the governor an opportunity to reflect on the outcome */
if (cpuidle_curr_governor->reflect)
diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
index 9f08e8c..7e93aaa 100644
--- a/drivers/cpuidle/governors/ladder.c
+++ b/drivers/cpuidle/governors/ladder.c
@@ -58,6 +58,36 @@ static inline void ladder_do_selection(struct ladder_device *ldev,
ldev->last_state_idx = new_idx;
}
+static int can_promote(struct ladder_device *ldev, int last_idx,
+ int last_residency)
+{
+ struct ladder_device_state *last_state;
+
+ last_state = &ldev->states[last_idx];
+ if (last_residency > last_state->threshold.promotion_time) {
+ last_state->stats.promotion_count++;
+ last_state->stats.demotion_count = 0;
+ if (last_state->stats.promotion_count >= last_state->threshold.promotion_count)
+ return 1;
+ }
+ return 0;
+}
+
+static int can_demote(struct ladder_device *ldev, int last_idx,
+ int last_residency)
+{
+ struct ladder_device_state *last_state;
+
+ last_state = &ldev->states[last_idx];
+ if (last_residency < last_state->threshold.demotion_time) {
+ last_state->stats.demotion_count++;
+ last_state->stats.promotion_count = 0;
+ if (last_state->stats.demotion_count >= last_state->threshold.demotion_count)
+ return 1;
+ }
+ return 0;
+}
+
/**
* ladder_select_state - selects the next state to enter
* @drv: cpuidle driver
@@ -73,29 +103,33 @@ static int ladder_select_state(struct cpuidle_driver *drv,
/* Special case when user has set very strict latency requirement */
if (unlikely(latency_req == 0)) {
- ladder_do_selection(ldev, last_idx, 0);
- return 0;
+ if (last_idx >= 0)
+ ladder_do_selection(ldev, last_idx, -1);
+ goto out;
}
- last_state = &ldev->states[last_idx];
-
- if (drv->states[last_idx].flags & CPUIDLE_FLAG_TIME_VALID) {
- last_residency = cpuidle_get_last_residency(dev) - \
- drv->states[last_idx].exit_latency;
+ if (last_idx >= 0) {
+ if (drv->states[last_idx].flags & CPUIDLE_FLAG_TIME_VALID) {
+ last_residency = cpuidle_get_last_residency(dev) - \
+ drv->states[last_idx].exit_latency;
+ } else {
+ last_state = &ldev->states[last_idx];
+ last_residency = last_state->threshold.promotion_time + 1;
+ }
}
- else
- last_residency = last_state->threshold.promotion_time + 1;
/* consider promotion */
if (last_idx < drv->state_count - 1 &&
!drv->states[last_idx + 1].disabled &&
!dev->states_usage[last_idx + 1].disable &&
- last_residency > last_state->threshold.promotion_time &&
drv->states[last_idx + 1].exit_latency <= latency_req) {
- last_state->stats.promotion_count++;
- last_state->stats.demotion_count = 0;
- if (last_state->stats.promotion_count >= last_state->threshold.promotion_count) {
- ladder_do_selection(ldev, last_idx, last_idx + 1);
+ if (last_idx >= 0) {
+ if (can_promote(ldev, last_idx, last_residency)) {
+ ladder_do_selection(ldev, last_idx, last_idx + 1);
+ return last_idx + 1;
+ }
+ } else {
+ ldev->last_state_idx = last_idx + 1;
return last_idx + 1;
}
}
@@ -107,26 +141,36 @@ static int ladder_select_state(struct cpuidle_driver *drv,
drv->states[last_idx].exit_latency > latency_req)) {
int i;
- for (i = last_idx - 1; i > CPUIDLE_DRIVER_STATE_START; i--) {
- if (drv->states[i].exit_latency <= latency_req)
+ for (i = last_idx - 1; i >= CPUIDLE_DRIVER_STATE_START; i--) {
+ if (drv->states[i].exit_latency <= latency_req &&
+ !(drv->states[i].disabled || dev->states_usage[i].disable))
break;
}
- ladder_do_selection(ldev, last_idx, i);
- return i;
+ if (i >= 0) {
+ ladder_do_selection(ldev, last_idx, i);
+ return i;
+ }
+ goto out;
}
- if (last_idx > CPUIDLE_DRIVER_STATE_START &&
- last_residency < last_state->threshold.demotion_time) {
- last_state->stats.demotion_count++;
- last_state->stats.promotion_count = 0;
- if (last_state->stats.demotion_count >= last_state->threshold.demotion_count) {
- ladder_do_selection(ldev, last_idx, last_idx - 1);
- return last_idx - 1;
+ if (last_idx > CPUIDLE_DRIVER_STATE_START) {
+ int i = last_idx - 1;
+
+ if (can_demote(ldev, last_idx, last_residency) &&
+ !(drv->states[i].disabled || dev->states_usage[i].disable)) {
+ ladder_do_selection(ldev, last_idx, i);
+ return i;
}
+ /* We come here when the last_idx is still a suitable idle state,
+ * just that promotion or demotion is not ideal.
+ */
+ ldev->last_state_idx = last_idx;
+ return last_idx;
}
- /* otherwise remain at the current state */
- return last_idx;
+ /* we come here if no idle state is suitable */
+out: ldev->last_state_idx = -1;
+ return ldev->last_state_idx;
}
/**
@@ -166,7 +210,8 @@ static int ladder_enable_device(struct cpuidle_driver *drv,
/**
* ladder_reflect - update the correct last_state_idx
* @dev: the CPU
- * @index: the index of actual state entered
+ * @index: the index of actual state entered or -1 if no idle state is
+ * suitable.
*/
static void ladder_reflect(struct cpuidle_device *dev, int index)
{
--git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index cf7f2f0..e9f17ce 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -297,12 +297,12 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
data->needs_update = 0;
}
- data->last_state_idx = 0;
+ data->last_state_idx = -1;
data->exit_us = 0;
/* Special case when user has set very strict latency requirement */
if (unlikely(latency_req == 0))
- return 0;
+ return data->last_state_idx;
/* determine the expected residency time, round up */
t = ktime_to_timespec(tick_nohz_get_sleep_length());
@@ -368,7 +368,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
/**
* menu_reflect - records that data structures need update
* @dev: the CPU
- * @index: the index of actual entered state
+ * @index: the index of actual entered state or -1 if no idle state is
+ * suitable.
*
* NOTE: it's important to be fast here because this operation will add to
* the overall exit latency.
^ permalink raw reply related
* RE: [PATCH 2/2] clocksource: Make clocksource register functions void
From: David Laight @ 2014-01-23 11:40 UTC (permalink / raw)
To: 'Tony Prisk', Yijing Wang, John Stultz, Thomas Gleixner
Cc: linux-mips@linux-mips.org, x86@kernel.org, Kevin Hilman,
linux@lists.openrisc.net, Sekhar Nori, Michal Simek,
Paul Mackerras, Ralf Baechle, H. Peter Anvin, Daniel Walker,
Hans-Christian Egtvedt, Jonas Bonn, Kukjin Kim, Russell King,
Richard Weinberger, Daniel Lezcano, Tony Lindgren, Ingo Molnar,
microblaze-uclinux@itee.uq.edu.au, David Brown,
Haavard Skinnemoen, Mike Frysinger,
user-mode-linux-devel@lists.sourceforge.net,
linux-arm-msm@vger.kernel.org, Jeff Dike,
davinci-linux-open-source@linux.davincidsp.com,
linux-samsung-soc@vger.kernel.org,
user-mode-linux-user@lists.sourceforge.net,
linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Barry Song, Jim Cromie, Hanjun Guo, linux-kernel@vger.kernel.org,
Nicolas Ferre, Bryan Huntsman,
uclinux-dist-devel@blackfin.uclinux.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <52E0C889.6000106@prisktech.co.nz>
From: Linuxppc-dev Tony Prisk
> On 23/01/14 20:12, Yijing Wang wrote:
> > Currently, clocksource_register() and __clocksource_register_scale()
> > functions always return 0, it's pointless, make functions void.
> > And remove the dead code that check the clocksource_register_hz()
> > return value.
> ......
> > -static inline int clocksource_register_hz(struct clocksource *cs, u32 =
hz)
> > +static inline void clocksource_register_hz(struct clocksource *cs, u32=
hz)
> > {
> > return __clocksource_register_scale(cs, 1, hz);
> > }
>=20
> This doesn't make sense - you are still returning a value on a function
> declared void, and the return is now from a function that doesn't return
> anything either ?!?!
> Doesn't this throw a compile-time warning??
It depends on the compiler.
Recent gcc allow it.
I don't know if it is actually valid C though.
There is no excuse for it on lines like the above though.
David
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers
From: Scott Wood @ 2014-01-23 16:22 UTC (permalink / raw)
To: Wang Dongsheng-B40534
Cc: anton@enomsg.org, linuxppc-dev@lists.ozlabs.org,
Zhao Chenhui-B35336
In-Reply-To: <46dd04cc9b6245fa90d20e2d94ff8523@BN1PR03MB188.namprd03.prod.outlook.com>
On Wed, 2014-01-22 at 20:49 -0600, Wang Dongsheng-B40534 wrote:
> > > >
> > > > The whole point of calling enable_kernel_fp() in C code before
> > > > suspending is to ensure that the FP state gets saved. If FP is used
> > > > after that point it is a bug. If you're worried about such bugs, then
> > > > clear MSR[FP] after calling enable_kernel_fp(), rather than adding
> > > > redundant state saving.
> > > >
> > >
> > > enable_kernel_fp() calling in MEM suspend flow.
> > > Hibernation is different with MEM suspend, and I'm not sure where will call
> > this
> > > interface, so we need to ensure the integrity of the core saving. I don't
> > think
> > > this code is *redundant*. I trust that the kernel can keep the FP related
> > > operations, that's why a judgment is here. :)
> >
> > For hibernation, save_processor_state() is called first, which does
> > flush_fp_to_thread() which has a similar effect (though I wonder if it's
> > being called on the correct task for non-SMP).
> >
> Yes, thanks, I miss this code.:)
>
> But I still think we need to keep this judgment, because i provide an API.
> If you still insist on I can remove *FP*, but I don't want to do this..:)
I insist. Redundant code wastes review and maintenance bandwidth, and
is a potential source of bugs.
-Scott
^ permalink raw reply
* Re: [PATCH] clk: corenet: Update the clock bindings
From: Scott Wood @ 2014-01-23 21:03 UTC (permalink / raw)
To: Tang Yuantian-B29983
Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
Kushwaha Prabhakar-B32579
In-Reply-To: <d61cabaec0af4aada9fadf93e7c0bb61@BL2PR03MB115.namprd03.prod.outlook.com>
On Wed, 2014-01-22 at 20:47 -0600, Tang Yuantian-B29983 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: 2014年1月23日 星期四 8:44
> > To: Tang Yuantian-B29983
> > Cc: Wood Scott-B07421; galak@kernel.crashing.org; linuxppc-
> > dev@lists.ozlabs.org; devicetree@vger.kernel.org; Kushwaha Prabhakar-
> > B32579
> > Subject: Re: [PATCH] clk: corenet: Update the clock bindings
> >
> > On Tue, 2014-01-21 at 10:02 +0800, Tang Yuantian wrote:
> > > From: Tang Yuantian <yuantian.tang@freescale.com>
> > >
> > > Main changs include:
> > > - Clarified the clock nodes' version number
> > > - Fixed a issue in example
> > >
> > > Singed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
> > > ---
> > > Documentation/devicetree/bindings/clock/corenet-clock.txt | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/clock/corenet-clock.txt
> > > b/Documentation/devicetree/bindings/clock/corenet-clock.txt
> > > index 24711af..d6cadef 100644
> > > --- a/Documentation/devicetree/bindings/clock/corenet-clock.txt
> > > +++ b/Documentation/devicetree/bindings/clock/corenet-clock.txt
> > > @@ -54,6 +54,8 @@ Required properties:
> > > It takes parent's clock-frequency as its clock.
> > > * "fsl,qoriq-sysclk-2.0": for input system clock (v2.0).
> > > It takes parent's clock-frequency as its clock.
> > > + Note: v1.0 and v2.0 are clock version which should align to
> > > + clockgen node's they belong to which is chassis version.
> >
> > Instead, how about a note like this near the top of the file:
> >
> > All references to "1.0" and "2.0" refer to the QorIQ chassis version to
> > which the chip complies.
> >
> > Chassis Version Example Chips
> > --------------- -------------
> > 1.0 p4080, p5020, p5040
> > 2.0 t4240, b4860, t1040
> >
> Better, I will update.
>
> >
> > BTW, this binding and the associated driver really should be called
> > "qoriq-clock", not "corenet-clock". This would match the compatible
> > string, and it doesn't really have much to do with corenet (which is part
> > of the QorIQ chassis v1 and v2, but not *this* part). Do you know if the
> > chassis v3 clock interface will be similar enough to share a driver?
> >
> Doesn't QorIQ include some low-end socs, like p1022, p1020?
Yes, but those aren't "QorIQ Chassis 1.0" or "QorIQ Chassis 2.0".
They're mpc85xx-family chips.
In any case, if "qoriq" makes sense for the compatible, I don't see why
it doesn't make sense for the driver.
> This driver has nothing to do with these boards.
> I have no idea about chassis v3. If it has similar clock tree, this driver can be shared.
> Even the driver can't be used by v3, we can easily add v3 support since it has different
> Compatible string.
The reason I mentioned it is that chassis v3 will involve ARM chips that
have their own interconnect rather than corenet.
-Scott
^ permalink raw reply
* RE: [PATCH] clk: corenet: Update the clock bindings
From: Yuantian Tang @ 2014-01-24 2:33 UTC (permalink / raw)
To: Scott Wood
Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
prabhakar@freescale.com
In-Reply-To: <1390511008.24905.581.camel@snotra.buserror.net>
PiA+ID4gSW5zdGVhZCwgaG93IGFib3V0IGEgbm90ZSBsaWtlIHRoaXMgbmVhciB0aGUgdG9wIG9m
IHRoZSBmaWxlOg0KPiA+ID4NCj4gPiA+IEFsbCByZWZlcmVuY2VzIHRvICIxLjAiIGFuZCAiMi4w
IiByZWZlciB0byB0aGUgUW9ySVEgY2hhc3NpcyB2ZXJzaW9uDQo+ID4gPiB0byB3aGljaCB0aGUg
Y2hpcCBjb21wbGllcy4NCj4gPiA+DQo+ID4gPiBDaGFzc2lzIFZlcnNpb24JCUV4YW1wbGUgQ2hp
cHMNCj4gPiA+IC0tLS0tLS0tLS0tLS0tLQkJLS0tLS0tLS0tLS0tLQ0KPiA+ID4gMS4wCQkJcDQw
ODAsIHA1MDIwLCBwNTA0MA0KPiA+ID4gMi4wCQkJdDQyNDAsIGI0ODYwLCB0MTA0MA0KPiA+ID4N
Cj4gPiBCZXR0ZXIsIEkgd2lsbCB1cGRhdGUuDQo+ID4NCj4gPiA+DQo+ID4gPiBCVFcsIHRoaXMg
YmluZGluZyBhbmQgdGhlIGFzc29jaWF0ZWQgZHJpdmVyIHJlYWxseSBzaG91bGQgYmUgY2FsbGVk
DQo+ID4gPiAicW9yaXEtY2xvY2siLCBub3QgImNvcmVuZXQtY2xvY2siLiAgVGhpcyB3b3VsZCBt
YXRjaCB0aGUgY29tcGF0aWJsZQ0KPiA+ID4gc3RyaW5nLCBhbmQgaXQgZG9lc24ndCByZWFsbHkg
aGF2ZSBtdWNoIHRvIGRvIHdpdGggY29yZW5ldCAod2hpY2ggaXMNCj4gPiA+IHBhcnQgb2YgdGhl
IFFvcklRIGNoYXNzaXMgdjEgYW5kIHYyLCBidXQgbm90ICp0aGlzKiBwYXJ0KS4gIERvIHlvdQ0K
PiA+ID4ga25vdyBpZiB0aGUgY2hhc3NpcyB2MyBjbG9jayBpbnRlcmZhY2Ugd2lsbCBiZSBzaW1p
bGFyIGVub3VnaCB0bw0KPiBzaGFyZSBhIGRyaXZlcj8NCj4gPiA+DQo+ID4gRG9lc24ndCBRb3JJ
USBpbmNsdWRlIHNvbWUgbG93LWVuZCBzb2NzLCBsaWtlIHAxMDIyLCBwMTAyMD8NCj4gDQo+IFll
cywgYnV0IHRob3NlIGFyZW4ndCAiUW9ySVEgQ2hhc3NpcyAxLjAiIG9yICJRb3JJUSBDaGFzc2lz
IDIuMCIuDQo+IFRoZXkncmUgbXBjODV4eC1mYW1pbHkgY2hpcHMuDQo+IA0KPiBJbiBhbnkgY2Fz
ZSwgaWYgInFvcmlxIiBtYWtlcyBzZW5zZSBmb3IgdGhlIGNvbXBhdGlibGUsIEkgZG9uJ3Qgc2Vl
IHdoeQ0KPiBpdCBkb2Vzbid0IG1ha2Ugc2Vuc2UgZm9yIHRoZSBkcml2ZXIuDQo+IA0KU28sICJD
b3JlbmV0IiBpcyBhcHByb3ByaWF0ZSBmb3IgZHJpdmVyLg0KSWYgc29tZXRoaW5nIHNob3VsZCBj
aGFuZ2UsIHRoYXQgbXVzdCBiZSBjb21wYXRpYmxlIHN0cmluZy4NCg0KUmVnYXJkcywNCll1YW50
aWFuDQoNCj4gPiBUaGlzIGRyaXZlciBoYXMgbm90aGluZyB0byBkbyB3aXRoIHRoZXNlIGJvYXJk
cy4NCj4gPiBJIGhhdmUgbm8gaWRlYSBhYm91dCBjaGFzc2lzIHYzLiBJZiBpdCBoYXMgc2ltaWxh
ciBjbG9jayB0cmVlLCB0aGlzDQo+IGRyaXZlciBjYW4gYmUgc2hhcmVkLg0KPiA+IEV2ZW4gdGhl
IGRyaXZlciBjYW4ndCBiZSB1c2VkIGJ5IHYzLCB3ZSBjYW4gZWFzaWx5IGFkZCB2MyBzdXBwb3J0
DQo+ID4gc2luY2UgaXQgaGFzIGRpZmZlcmVudCBDb21wYXRpYmxlIHN0cmluZy4NCj4gDQo+IFRo
ZSByZWFzb24gSSBtZW50aW9uZWQgaXQgaXMgdGhhdCBjaGFzc2lzIHYzIHdpbGwgaW52b2x2ZSBB
Uk0gY2hpcHMgdGhhdA0KPiBoYXZlIHRoZWlyIG93biBpbnRlcmNvbm5lY3QgcmF0aGVyIHRoYW4g
Y29yZW5ldC4NCj4gDQo+IC1TY290dA0KPiANCg0K
^ permalink raw reply
* Re: [PATCH] clk: corenet: Update the clock bindings
From: Scott Wood @ 2014-01-24 2:35 UTC (permalink / raw)
To: Tang Yuantian-B29983
Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
Kushwaha Prabhakar-B32579
In-Reply-To: <a0800d606e364b5980fe5c17efca9d9e@BL2PR03MB115.namprd03.prod.outlook.com>
On Thu, 2014-01-23 at 20:33 -0600, Tang Yuantian-B29983 wrote:
> > > > Instead, how about a note like this near the top of the file:
> > > >
> > > > All references to "1.0" and "2.0" refer to the QorIQ chassis version
> > > > to which the chip complies.
> > > >
> > > > Chassis Version Example Chips
> > > > --------------- -------------
> > > > 1.0 p4080, p5020, p5040
> > > > 2.0 t4240, b4860, t1040
> > > >
> > > Better, I will update.
> > >
> > > >
> > > > BTW, this binding and the associated driver really should be called
> > > > "qoriq-clock", not "corenet-clock". This would match the compatible
> > > > string, and it doesn't really have much to do with corenet (which is
> > > > part of the QorIQ chassis v1 and v2, but not *this* part). Do you
> > > > know if the chassis v3 clock interface will be similar enough to
> > share a driver?
> > > >
> > > Doesn't QorIQ include some low-end socs, like p1022, p1020?
> >
> > Yes, but those aren't "QorIQ Chassis 1.0" or "QorIQ Chassis 2.0".
> > They're mpc85xx-family chips.
> >
> > In any case, if "qoriq" makes sense for the compatible, I don't see why
> > it doesn't make sense for the driver.
> >
> So, "Corenet" is appropriate for driver.
> If something should change, that must be compatible string.
No. Corenet is a bus interconnect, not a chip family (despite abuse of
the name in other contexts in Linux/U-Boot). And the binding with qoriq
has already been accepted.
-Scott
^ permalink raw reply
* RE: [PATCH] clk: corenet: Update the clock bindings
From: Yuantian Tang @ 2014-01-24 2:46 UTC (permalink / raw)
To: Scott Wood
Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
prabhakar@freescale.com
In-Reply-To: <1390530934.24905.639.camel@snotra.buserror.net>
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBXb29kIFNjb3R0LUIwNzQyMQ0K
PiBTZW50OiAyMDE05bm0MeaciDI05pelIOaYn+acn+S6lCAxMDozNg0KPiBUbzogVGFuZyBZdWFu
dGlhbi1CMjk5ODMNCj4gQ2M6IGdhbGFrQGtlcm5lbC5jcmFzaGluZy5vcmc7IGxpbnV4cHBjLWRl
dkBsaXN0cy5vemxhYnMub3JnOw0KPiBkZXZpY2V0cmVlQHZnZXIua2VybmVsLm9yZzsgS3VzaHdh
aGEgUHJhYmhha2FyLUIzMjU3OQ0KPiBTdWJqZWN0OiBSZTogW1BBVENIXSBjbGs6IGNvcmVuZXQ6
IFVwZGF0ZSB0aGUgY2xvY2sgYmluZGluZ3MNCj4gDQo+IE9uIFRodSwgMjAxNC0wMS0yMyBhdCAy
MDozMyAtMDYwMCwgVGFuZyBZdWFudGlhbi1CMjk5ODMgd3JvdGU6DQo+ID4gPiA+ID4gSW5zdGVh
ZCwgaG93IGFib3V0IGEgbm90ZSBsaWtlIHRoaXMgbmVhciB0aGUgdG9wIG9mIHRoZSBmaWxlOg0K
PiA+ID4gPiA+DQo+ID4gPiA+ID4gQWxsIHJlZmVyZW5jZXMgdG8gIjEuMCIgYW5kICIyLjAiIHJl
ZmVyIHRvIHRoZSBRb3JJUSBjaGFzc2lzDQo+ID4gPiA+ID4gdmVyc2lvbiB0byB3aGljaCB0aGUg
Y2hpcCBjb21wbGllcy4NCj4gPiA+ID4gPg0KPiA+ID4gPiA+IENoYXNzaXMgVmVyc2lvbgkJRXhh
bXBsZSBDaGlwcw0KPiA+ID4gPiA+IC0tLS0tLS0tLS0tLS0tLQkJLS0tLS0tLS0tLS0tLQ0KPiA+
ID4gPiA+IDEuMAkJCXA0MDgwLCBwNTAyMCwgcDUwNDANCj4gPiA+ID4gPiAyLjAJCQl0NDI0MCwg
YjQ4NjAsIHQxMDQwDQo+ID4gPiA+ID4NCj4gPiA+ID4gQmV0dGVyLCBJIHdpbGwgdXBkYXRlLg0K
PiA+ID4gPg0KPiA+ID4gPiA+DQo+ID4gPiA+ID4gQlRXLCB0aGlzIGJpbmRpbmcgYW5kIHRoZSBh
c3NvY2lhdGVkIGRyaXZlciByZWFsbHkgc2hvdWxkIGJlDQo+ID4gPiA+ID4gY2FsbGVkICJxb3Jp
cS1jbG9jayIsIG5vdCAiY29yZW5ldC1jbG9jayIuICBUaGlzIHdvdWxkIG1hdGNoIHRoZQ0KPiA+
ID4gPiA+IGNvbXBhdGlibGUgc3RyaW5nLCBhbmQgaXQgZG9lc24ndCByZWFsbHkgaGF2ZSBtdWNo
IHRvIGRvIHdpdGgNCj4gPiA+ID4gPiBjb3JlbmV0ICh3aGljaCBpcyBwYXJ0IG9mIHRoZSBRb3JJ
USBjaGFzc2lzIHYxIGFuZCB2MiwgYnV0IG5vdA0KPiA+ID4gPiA+ICp0aGlzKiBwYXJ0KS4gIERv
IHlvdSBrbm93IGlmIHRoZSBjaGFzc2lzIHYzIGNsb2NrIGludGVyZmFjZQ0KPiA+ID4gPiA+IHdp
bGwgYmUgc2ltaWxhciBlbm91Z2ggdG8NCj4gPiA+IHNoYXJlIGEgZHJpdmVyPw0KPiA+ID4gPiA+
DQo+ID4gPiA+IERvZXNuJ3QgUW9ySVEgaW5jbHVkZSBzb21lIGxvdy1lbmQgc29jcywgbGlrZSBw
MTAyMiwgcDEwMjA/DQo+ID4gPg0KPiA+ID4gWWVzLCBidXQgdGhvc2UgYXJlbid0ICJRb3JJUSBD
aGFzc2lzIDEuMCIgb3IgIlFvcklRIENoYXNzaXMgMi4wIi4NCj4gPiA+IFRoZXkncmUgbXBjODV4
eC1mYW1pbHkgY2hpcHMuDQo+ID4gPg0KPiA+ID4gSW4gYW55IGNhc2UsIGlmICJxb3JpcSIgbWFr
ZXMgc2Vuc2UgZm9yIHRoZSBjb21wYXRpYmxlLCBJIGRvbid0IHNlZQ0KPiA+ID4gd2h5IGl0IGRv
ZXNuJ3QgbWFrZSBzZW5zZSBmb3IgdGhlIGRyaXZlci4NCj4gPiA+DQo+ID4gU28sICJDb3JlbmV0
IiBpcyBhcHByb3ByaWF0ZSBmb3IgZHJpdmVyLg0KPiA+IElmIHNvbWV0aGluZyBzaG91bGQgY2hh
bmdlLCB0aGF0IG11c3QgYmUgY29tcGF0aWJsZSBzdHJpbmcuDQo+IA0KPiBOby4gIENvcmVuZXQg
aXMgYSBidXMgaW50ZXJjb25uZWN0LCBub3QgYSBjaGlwIGZhbWlseSAoZGVzcGl0ZSBhYnVzZSBv
Zg0KPiB0aGUgbmFtZSBpbiBvdGhlciBjb250ZXh0cyBpbiBMaW51eC9VLUJvb3QpLiAgQW5kIHRo
ZSBiaW5kaW5nIHdpdGggcW9yaXENCj4gaGFzIGFscmVhZHkgYmVlbiBhY2NlcHRlZC4NCj4gDQpR
b3JJUSBpcyBub3QgdGhlIGJlc3QgbmFtZSBlaXRoZXIgc2luY2UgaXQgaW5jbHVkZSB0aGUgbG93
LWVuZCBzb2NzLg0KV2hhdCB0aGUgbmFtZSBzaG91bGQgYmU/IA0KDQpSZWdhcmRzLA0KWXVhbnRp
YW4NCg0KPiAtU2NvdHQNCj4gDQoNCg==
^ permalink raw reply
* Re: [PATCH] clk: corenet: Update the clock bindings
From: Scott Wood @ 2014-01-24 2:47 UTC (permalink / raw)
To: Tang Yuantian-B29983
Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
Kushwaha Prabhakar-B32579
In-Reply-To: <ac1c6cb1a6e54f6e87b76e2a3006b64a@BL2PR03MB115.namprd03.prod.outlook.com>
On Thu, 2014-01-23 at 20:46 -0600, Tang Yuantian-B29983 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: 2014年1月24日 星期五 10:36
> > To: Tang Yuantian-B29983
> > Cc: galak@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org;
> > devicetree@vger.kernel.org; Kushwaha Prabhakar-B32579
> > Subject: Re: [PATCH] clk: corenet: Update the clock bindings
> >
> > On Thu, 2014-01-23 at 20:33 -0600, Tang Yuantian-B29983 wrote:
> > > > > > Instead, how about a note like this near the top of the file:
> > > > > >
> > > > > > All references to "1.0" and "2.0" refer to the QorIQ chassis
> > > > > > version to which the chip complies.
> > > > > >
> > > > > > Chassis Version Example Chips
> > > > > > --------------- -------------
> > > > > > 1.0 p4080, p5020, p5040
> > > > > > 2.0 t4240, b4860, t1040
> > > > > >
> > > > > Better, I will update.
> > > > >
> > > > > >
> > > > > > BTW, this binding and the associated driver really should be
> > > > > > called "qoriq-clock", not "corenet-clock". This would match the
> > > > > > compatible string, and it doesn't really have much to do with
> > > > > > corenet (which is part of the QorIQ chassis v1 and v2, but not
> > > > > > *this* part). Do you know if the chassis v3 clock interface
> > > > > > will be similar enough to
> > > > share a driver?
> > > > > >
> > > > > Doesn't QorIQ include some low-end socs, like p1022, p1020?
> > > >
> > > > Yes, but those aren't "QorIQ Chassis 1.0" or "QorIQ Chassis 2.0".
> > > > They're mpc85xx-family chips.
> > > >
> > > > In any case, if "qoriq" makes sense for the compatible, I don't see
> > > > why it doesn't make sense for the driver.
> > > >
> > > So, "Corenet" is appropriate for driver.
> > > If something should change, that must be compatible string.
> >
> > No. Corenet is a bus interconnect, not a chip family (despite abuse of
> > the name in other contexts in Linux/U-Boot). And the binding with qoriq
> > has already been accepted.
> >
> QorIQ is not the best name either since it include the low-end socs.
> What the name should be?
Again, those low-end chips do not implement "QorIQ Chassis 1.0" or
"QorIQ Chassis 2.0". That they have "QorIQ" in their name is
irrelevant.
-Scott
^ permalink raw reply
* RE: [PATCH] clk: corenet: Update the clock bindings
From: Yuantian Tang @ 2014-01-24 3:05 UTC (permalink / raw)
To: Scott Wood
Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
prabhakar@freescale.com
In-Reply-To: <1390531664.24905.640.camel@snotra.buserror.net>
PiA+ID4gPiA+IEluIGFueSBjYXNlLCBpZiAicW9yaXEiIG1ha2VzIHNlbnNlIGZvciB0aGUgY29t
cGF0aWJsZSwgSSBkb24ndA0KPiA+ID4gPiA+IHNlZSB3aHkgaXQgZG9lc24ndCBtYWtlIHNlbnNl
IGZvciB0aGUgZHJpdmVyLg0KPiA+ID4gPiA+DQo+ID4gPiA+IFNvLCAiQ29yZW5ldCIgaXMgYXBw
cm9wcmlhdGUgZm9yIGRyaXZlci4NCj4gPiA+ID4gSWYgc29tZXRoaW5nIHNob3VsZCBjaGFuZ2Us
IHRoYXQgbXVzdCBiZSBjb21wYXRpYmxlIHN0cmluZy4NCj4gPiA+DQo+ID4gPiBOby4gIENvcmVu
ZXQgaXMgYSBidXMgaW50ZXJjb25uZWN0LCBub3QgYSBjaGlwIGZhbWlseSAoZGVzcGl0ZSBhYnVz
ZQ0KPiA+ID4gb2YgdGhlIG5hbWUgaW4gb3RoZXIgY29udGV4dHMgaW4gTGludXgvVS1Cb290KS4g
IEFuZCB0aGUgYmluZGluZw0KPiA+ID4gd2l0aCBxb3JpcSBoYXMgYWxyZWFkeSBiZWVuIGFjY2Vw
dGVkLg0KPiA+ID4NCj4gPiBRb3JJUSBpcyBub3QgdGhlIGJlc3QgbmFtZSBlaXRoZXIgc2luY2Ug
aXQgaW5jbHVkZSB0aGUgbG93LWVuZCBzb2NzLg0KPiA+IFdoYXQgdGhlIG5hbWUgc2hvdWxkIGJl
Pw0KPiANCj4gQWdhaW4sIHRob3NlIGxvdy1lbmQgY2hpcHMgZG8gbm90IGltcGxlbWVudCAiUW9y
SVEgQ2hhc3NpcyAxLjAiIG9yICJRb3JJUQ0KPiBDaGFzc2lzIDIuMCIuICBUaGF0IHRoZXkgaGF2
ZSAiUW9ySVEiIGluIHRoZWlyIG5hbWUgaXMgaXJyZWxldmFudC4NCj4gDQpHb3QgaXQuIA0KDQpS
ZWdhcmRzLA0KWXVhbnRpYW4NCg0KPiAtU2NvdHQNCj4gDQoNCg==
^ permalink raw reply
* Re: [PATCH] slub: Don't throw away partial remote slabs if there is no local memory
From: Wanpeng Li @ 2014-01-24 3:09 UTC (permalink / raw)
To: Christoph Lameter
Cc: nacc, David Rientjes, penberg, linux-mm, Han Pingtian, paulus,
Anton Blanchard, mpm, Joonsoo Kim, linuxppc-dev
In-Reply-To: <alpine.DEB.2.10.1401201612340.28048@nuc>
Hi Christoph,
On Mon, Jan 20, 2014 at 04:13:30PM -0600, Christoph Lameter wrote:
>On Mon, 20 Jan 2014, Wanpeng Li wrote:
>
>> >+ enum zone_type high_zoneidx = gfp_zone(flags);
>> >
>> >+ if (!node_present_pages(searchnode)) {
>> >+ zonelist = node_zonelist(searchnode, flags);
>> >+ for_each_zone_zonelist(zone, z, zonelist, high_zoneidx) {
>> >+ searchnode = zone_to_nid(zone);
>> >+ if (node_present_pages(searchnode))
>> >+ break;
>> >+ }
>> >+ }
>> > object = get_partial_node(s, get_node(s, searchnode), c, flags);
>> > if (object || node != NUMA_NO_NODE)
>> > return object;
>> >
>>
>> The patch fix the bug. However, the kernel crashed very quickly after running
>> stress tests for a short while:
>
>This is not a good way of fixing it. How about not asking for memory from
>nodes that are memoryless? Use numa_mem_id() which gives you the next node
>that has memory instead of numa_node_id() (gives you the current node
>regardless if it has memory or not).
diff --git a/mm/slub.c b/mm/slub.c
index 545a170..a1c6040 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1700,6 +1700,9 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
void *object;
int searchnode = (node == NUMA_NO_NODE) ? numa_node_id() : node;
+ if (!node_present_pages(searchnode))
+ searchnode = numa_mem_id();
+
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
return object;
^ permalink raw reply related
* Re: [PATCH] slub: Don't throw away partial remote slabs if there is no local memory
From: Wanpeng Li @ 2014-01-24 3:14 UTC (permalink / raw)
To: Christoph Lameter
Cc: nacc, David Rientjes, penberg, linux-mm, Han Pingtian, paulus,
Anton Blanchard, mpm, Joonsoo Kim, linuxppc-dev
In-Reply-To: <52e1d960.2715420a.3569.1013SMTPIN_ADDED_BROKEN@mx.google.com>
On Fri, Jan 24, 2014 at 11:09:07AM +0800, Wanpeng Li wrote:
>Hi Christoph,
>On Mon, Jan 20, 2014 at 04:13:30PM -0600, Christoph Lameter wrote:
>>On Mon, 20 Jan 2014, Wanpeng Li wrote:
>>
>>> >+ enum zone_type high_zoneidx = gfp_zone(flags);
>>> >
>>> >+ if (!node_present_pages(searchnode)) {
>>> >+ zonelist = node_zonelist(searchnode, flags);
>>> >+ for_each_zone_zonelist(zone, z, zonelist, high_zoneidx) {
>>> >+ searchnode = zone_to_nid(zone);
>>> >+ if (node_present_pages(searchnode))
>>> >+ break;
>>> >+ }
>>> >+ }
>>> > object = get_partial_node(s, get_node(s, searchnode), c, flags);
>>> > if (object || node != NUMA_NO_NODE)
>>> > return object;
>>> >
>>>
>>> The patch fix the bug. However, the kernel crashed very quickly after running
>>> stress tests for a short while:
>>
>>This is not a good way of fixing it. How about not asking for memory from
>>nodes that are memoryless? Use numa_mem_id() which gives you the next node
>>that has memory instead of numa_node_id() (gives you the current node
>>regardless if it has memory or not).
>
>diff --git a/mm/slub.c b/mm/slub.c
>index 545a170..a1c6040 100644
>--- a/mm/slub.c
>+++ b/mm/slub.c
>@@ -1700,6 +1700,9 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
> void *object;
> int searchnode = (node == NUMA_NO_NODE) ? numa_node_id() : node;
>
>+ if (!node_present_pages(searchnode))
>+ searchnode = numa_mem_id();
>+
> object = get_partial_node(s, get_node(s, searchnode), c, flags);
> if (object || node != NUMA_NO_NODE)
> return object;
>
The bug still can't be fixed w/ this patch.
Regards,
Wanpeng Li
>--
>To unsubscribe, send a message with 'unsubscribe linux-mm' in
>the body to majordomo@kvack.org. For more info on Linux MM,
>see: http://www.linux-mm.org/ .
>Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH] powerpc/perf: Add Power8 cache & TLB events
From: Michael Ellerman @ 2014-01-24 4:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: sukadev, Anton Blanchard, mlpesant
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/perf/power8-pmu.c | 144 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 144 insertions(+)
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index a3f7abd..96cee20 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -25,6 +25,37 @@
#define PM_BRU_FIN 0x10068
#define PM_BR_MPRED_CMPL 0x400f6
+/* All L1 D cache load references counted at finish, gated by reject */
+#define PM_LD_REF_L1 0x100ee
+/* Load Missed L1 */
+#define PM_LD_MISS_L1 0x3e054
+/* Store Missed L1 */
+#define PM_ST_MISS_L1 0x300f0
+/* L1 cache data prefetches */
+#define PM_L1_PREF 0x0d8b8
+/* Instruction fetches from L1 */
+#define PM_INST_FROM_L1 0x04080
+/* Demand iCache Miss */
+#define PM_L1_ICACHE_MISS 0x200fd
+/* Instruction Demand sectors wriittent into IL1 */
+#define PM_L1_DEMAND_WRITE 0x0408c
+/* Instruction prefetch written into IL1 */
+#define PM_IC_PREF_WRITE 0x0408e
+/* The data cache was reloaded from local core's L3 due to a demand load */
+#define PM_DATA_FROM_L3 0x4c042
+/* Demand LD - L3 Miss (not L2 hit and not L3 hit) */
+#define PM_DATA_FROM_L3MISS 0x300fe
+/* All successful D-side store dispatches for this thread */
+#define PM_L2_ST 0x17080
+/* All successful D-side store dispatches for this thread that were L2 Miss */
+#define PM_L2_ST_MISS 0x17082
+/* Total HW L3 prefetches(Load+store) */
+#define PM_L3_PREF_ALL 0x4e052
+/* Data PTEG reload */
+#define PM_DTLB_MISS 0x300fc
+/* ITLB Reloaded */
+#define PM_ITLB_MISS 0x400fc
+
/*
* Raw event encoding for POWER8:
@@ -557,6 +588,8 @@ static int power8_generic_events[] = {
[PERF_COUNT_HW_INSTRUCTIONS] = PM_INST_CMPL,
[PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = PM_BRU_FIN,
[PERF_COUNT_HW_BRANCH_MISSES] = PM_BR_MPRED_CMPL,
+ [PERF_COUNT_HW_CACHE_REFERENCES] = PM_LD_REF_L1,
+ [PERF_COUNT_HW_CACHE_MISSES] = PM_LD_MISS_L1,
};
static u64 power8_bhrb_filter_map(u64 branch_sample_type)
@@ -596,6 +629,116 @@ static void power8_config_bhrb(u64 pmu_bhrb_filter)
mtspr(SPRN_MMCRA, (mfspr(SPRN_MMCRA) | pmu_bhrb_filter));
}
+#define C(x) PERF_COUNT_HW_CACHE_##x
+
+/*
+ * Table of generalized cache-related events.
+ * 0 means not supported, -1 means nonsensical, other values
+ * are event codes.
+ */
+static int power8_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
+ [ C(L1D) ] = {
+ [ C(OP_READ) ] = {
+ [ C(RESULT_ACCESS) ] = PM_LD_REF_L1,
+ [ C(RESULT_MISS) ] = PM_LD_MISS_L1,
+ },
+ [ C(OP_WRITE) ] = {
+ [ C(RESULT_ACCESS) ] = 0,
+ [ C(RESULT_MISS) ] = PM_ST_MISS_L1,
+ },
+ [ C(OP_PREFETCH) ] = {
+ [ C(RESULT_ACCESS) ] = PM_L1_PREF,
+ [ C(RESULT_MISS) ] = 0,
+ },
+ },
+ [ C(L1I) ] = {
+ [ C(OP_READ) ] = {
+ [ C(RESULT_ACCESS) ] = PM_INST_FROM_L1,
+ [ C(RESULT_MISS) ] = PM_L1_ICACHE_MISS,
+ },
+ [ C(OP_WRITE) ] = {
+ [ C(RESULT_ACCESS) ] = PM_L1_DEMAND_WRITE,
+ [ C(RESULT_MISS) ] = -1,
+ },
+ [ C(OP_PREFETCH) ] = {
+ [ C(RESULT_ACCESS) ] = PM_IC_PREF_WRITE,
+ [ C(RESULT_MISS) ] = 0,
+ },
+ },
+ [ C(LL) ] = {
+ [ C(OP_READ) ] = {
+ [ C(RESULT_ACCESS) ] = PM_DATA_FROM_L3,
+ [ C(RESULT_MISS) ] = PM_DATA_FROM_L3MISS,
+ },
+ [ C(OP_WRITE) ] = {
+ [ C(RESULT_ACCESS) ] = PM_L2_ST,
+ [ C(RESULT_MISS) ] = PM_L2_ST_MISS,
+ },
+ [ C(OP_PREFETCH) ] = {
+ [ C(RESULT_ACCESS) ] = PM_L3_PREF_ALL,
+ [ C(RESULT_MISS) ] = 0,
+ },
+ },
+ [ C(DTLB) ] = {
+ [ C(OP_READ) ] = {
+ [ C(RESULT_ACCESS) ] = 0,
+ [ C(RESULT_MISS) ] = PM_DTLB_MISS,
+ },
+ [ C(OP_WRITE) ] = {
+ [ C(RESULT_ACCESS) ] = -1,
+ [ C(RESULT_MISS) ] = -1,
+ },
+ [ C(OP_PREFETCH) ] = {
+ [ C(RESULT_ACCESS) ] = -1,
+ [ C(RESULT_MISS) ] = -1,
+ },
+ },
+ [ C(ITLB) ] = {
+ [ C(OP_READ) ] = {
+ [ C(RESULT_ACCESS) ] = 0,
+ [ C(RESULT_MISS) ] = PM_ITLB_MISS,
+ },
+ [ C(OP_WRITE) ] = {
+ [ C(RESULT_ACCESS) ] = -1,
+ [ C(RESULT_MISS) ] = -1,
+ },
+ [ C(OP_PREFETCH) ] = {
+ [ C(RESULT_ACCESS) ] = -1,
+ [ C(RESULT_MISS) ] = -1,
+ },
+ },
+ [ C(BPU) ] = {
+ [ C(OP_READ) ] = {
+ [ C(RESULT_ACCESS) ] = PM_BRU_FIN,
+ [ C(RESULT_MISS) ] = PM_BR_MPRED_CMPL,
+ },
+ [ C(OP_WRITE) ] = {
+ [ C(RESULT_ACCESS) ] = -1,
+ [ C(RESULT_MISS) ] = -1,
+ },
+ [ C(OP_PREFETCH) ] = {
+ [ C(RESULT_ACCESS) ] = -1,
+ [ C(RESULT_MISS) ] = -1,
+ },
+ },
+ [ C(NODE) ] = {
+ [ C(OP_READ) ] = {
+ [ C(RESULT_ACCESS) ] = -1,
+ [ C(RESULT_MISS) ] = -1,
+ },
+ [ C(OP_WRITE) ] = {
+ [ C(RESULT_ACCESS) ] = -1,
+ [ C(RESULT_MISS) ] = -1,
+ },
+ [ C(OP_PREFETCH) ] = {
+ [ C(RESULT_ACCESS) ] = -1,
+ [ C(RESULT_MISS) ] = -1,
+ },
+ },
+};
+
+#undef C
+
static struct power_pmu power8_pmu = {
.name = "POWER8",
.n_counter = 6,
@@ -611,6 +754,7 @@ static struct power_pmu power8_pmu = {
.flags = PPMU_HAS_SSLOT | PPMU_HAS_SIER | PPMU_BHRB | PPMU_EBB,
.n_generic = ARRAY_SIZE(power8_generic_events),
.generic_events = power8_generic_events,
+ .cache_events = &power8_cache_events,
.attr_groups = power8_pmu_attr_groups,
.bhrb_nr = 32,
};
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH V5 6/8] time/cpuidle: Support in tick broadcast framework in the absence of external clock device
From: Preeti U Murthy @ 2014-01-24 6:53 UTC (permalink / raw)
To: Thomas Gleixner
Cc: daniel.lezcano, peterz, fweisbec, paul.gortmaker, paulus, mingo,
mikey, shangw, rafael.j.wysocki, agraf, paulmck, arnd, linux-pm,
rostedt, michael, john.stultz, anton, chenhui.zhao, deepthi,
r58472, geoff, linux-kernel, srivatsa.bhat, schwidefsky,
linuxppc-dev
In-Reply-To: <alpine.DEB.2.02.1401221042130.4260@ionos.tec.linutronix.de>
Hi Thomas,
The below patch works pretty much as is. I tried this out with deep idle
states on our system. Looking through the code and analysing corner
cases also did not bring out any issues to me. I will send out a patch
V2 of this.
Regards
Preeti U Murthy
On 01/22/2014 06:57 PM, Thomas Gleixner wrote:
> On Wed, 15 Jan 2014, Preeti U Murthy wrote:
>> diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
>> index 086ad60..d61404e 100644
>> --- a/kernel/time/clockevents.c
>> +++ b/kernel/time/clockevents.c
>> @@ -524,12 +524,13 @@ void clockevents_resume(void)
>> #ifdef CONFIG_GENERIC_CLOCKEVENTS
>> /**
>> * clockevents_notify - notification about relevant events
>> + * Returns non zero on error.
>> */
>> -void clockevents_notify(unsigned long reason, void *arg)
>> +int clockevents_notify(unsigned long reason, void *arg)
>> {
>
> The interface change of clockevents_notify wants to be a separate
> patch.
>
>> diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
>> index 9532690..1c23912 100644
>> --- a/kernel/time/tick-broadcast.c
>> +++ b/kernel/time/tick-broadcast.c
>> @@ -20,6 +20,7 @@
>> #include <linux/sched.h>
>> #include <linux/smp.h>
>> #include <linux/module.h>
>> +#include <linux/slab.h>
>>
>> #include "tick-internal.h"
>>
>> @@ -35,6 +36,15 @@ static cpumask_var_t tmpmask;
>> static DEFINE_RAW_SPINLOCK(tick_broadcast_lock);
>> static int tick_broadcast_force;
>>
>> +/*
>> + * Helper variables for handling broadcast in the absence of a
>> + * tick_broadcast_device.
>> + * */
>> +static struct hrtimer *bc_hrtimer;
>> +static int bc_cpu = -1;
>> +static ktime_t bc_next_wakeup;
>
> Why do you need another variable to store the expiry time? The
> broadcast code already knows it and the hrtimer expiry value gives you
> the same information for free.
>
>> +static int hrtimer_initialized = 0;
>
> What's the point of this hrtimer_initialized dance? Why not simply
> making the hrtimer static and avoid that all together. Also adding the
> initialization into tick_broadcast_oneshot_available() is
> braindamaged. Why not adding this to tick_broadcast_init() which is
> the proper place to do?
>
> Aside of that you are making this hrtimer mode unconditional, which
> might break existing systems which are not aware of the hrtimer
> implications.
>
> What you really want is a pseudo clock event device which has the
> proper functions for handling the timer and you can register it from
> your architecture code. The broadcast core code needs a few tweaks to
> avoid the shutdown of the cpu local clock event device, but aside of
> that the whole thing just falls into place. So architectures can use
> this if they want and are sure that their low level idle code knows
> about the deep idle preventing return value of
> clockevents_notify(). Once that works you can register the hrtimer
> based broadcast device and a real hardware broadcast device with a
> higher rating. It just works.
>
> Find an incomplete and nonfunctional concept patch below. It should be
> simple to make it work for real.
>
> Thanks,
>
> tglx
> ----
> Index: linux-2.6/include/linux/clockchips.h
> ===================================================================
> --- linux-2.6.orig/include/linux/clockchips.h
> +++ linux-2.6/include/linux/clockchips.h
> @@ -62,6 +62,11 @@ enum clock_event_mode {
> #define CLOCK_EVT_FEAT_DYNIRQ 0x000020
> #define CLOCK_EVT_FEAT_PERCPU 0x000040
>
> +/*
> + * Clockevent device is based on a hrtimer for broadcast
> + */
> +#define CLOCK_EVT_FEAT_HRTIMER 0x000080
> +
> /**
> * struct clock_event_device - clock event device descriptor
> * @event_handler: Assigned by the framework to be called by the low
> @@ -83,6 +88,7 @@ enum clock_event_mode {
> * @name: ptr to clock event name
> * @rating: variable to rate clock event devices
> * @irq: IRQ number (only for non CPU local devices)
> + * @bound_on: Bound on CPU
> * @cpumask: cpumask to indicate for which CPUs this device works
> * @list: list head for the management code
> * @owner: module reference
> @@ -113,6 +119,7 @@ struct clock_event_device {
> const char *name;
> int rating;
> int irq;
> + int bound_on;
> const struct cpumask *cpumask;
> struct list_head list;
> struct module *owner;
> Index: linux-2.6/kernel/time/tick-broadcast-hrtimer.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6/kernel/time/tick-broadcast-hrtimer.c
> @@ -0,0 +1,77 @@
> +
> +static struct hrtimer bctimer;
> +
> +static void bc_set_mode(enum clock_event_mode mode,
> + struct clock_event_device *bc)
> +{
> + switch (mode) {
> + case CLOCK_EVT_MODE_SHUTDOWN:
> + /*
> + * Note, we cannot cancel the timer here as we might
> + * run into the following live lock scenario:
> + *
> + * cpu 0 cpu1
> + * lock(broadcast_lock);
> + * hrtimer_interrupt()
> + * bc_handler()
> + * tick_handle_oneshot_broadcast();
> + * lock(broadcast_lock);
> + * hrtimer_cancel()
> + * wait_for_callback()
> + */
> + hrtimer_try_to_cancel(&bctimer);
> + break;
> + default:
> + break;
> + }
> +}
> +
> +/*
> + * This is called from the guts of the broadcast code when the cpu
> + * which is about to enter idle has the earliest broadcast timer event.
> + */
> +static int bc_set_next(ktime_t expires, struct clock_event_device *bc)
> +{
> + /*
> + * We try to cancel the timer first. If the callback is on
> + * flight on some other cpu then we let it handle it. If we
> + * were able to cancel the timer nothing can rearm it as we
> + * own broadcast_lock.
> + */
> + if (hrtimer_try_to_cancel(&bctimer) >= 0) {
> + hrtimer_start(&bctimer, expires, HRTIMER_MODE_ABS_PINNED);
> + /* Bind the "device" to the cpu */
> + bc->bound_on = smp_processor_id();
> + }
> + return 0;
> +}
> +
> +static struct clock_event_device ce_broadcast_hrtimer = {
> + .set_mode = bc_set_mode,
> + .set_next_ktime = bc_set_next,
> + .features = CLOCK_EVT_FEAT_ONESHOT |
> + CLOCK_EVT_FEAT_KTIME |
> + CLOCK_EVT_FEAT_HRTIMER,
> + .rating = 0,
> + .bound_on = -1,
> + .min_delta_ns = 1,
> + .max_delta_ns = KTIME_MAX,
> + .min_delta_ticks = 1,
> + .max_delta_ticks = KTIME_MAX,
> + .mult = 1,
> + .shift = 0,
> + .cpumask = cpu_all_mask,
> +};
> +
> +static enum hrtimer_restart bc_handler(struct hrtimer *t)
> +{
> + ce_broadcast_hrtimer.event_handler(&ce_broadcast_hrtimer);
> + return HRTIMER_NORESTART;
> +}
> +
> +void tick_setup_hrtimer_broadcast(void)
> +{
> + hrtimer_init(&bctimer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
> + bctimer.function = bc_handler;
> + clockevents_register(&ce_broadcast_hrtimer);
> +}
> Index: linux-2.6/kernel/time/tick-broadcast.c
> ===================================================================
> --- linux-2.6.orig/kernel/time/tick-broadcast.c
> +++ linux-2.6/kernel/time/tick-broadcast.c
> @@ -630,6 +630,42 @@ again:
> raw_spin_unlock(&tick_broadcast_lock);
> }
>
> +static int broadcast_needs_cpu(struct clock_event_device *bc, int cpu)
> +{
> + if (!(bc->features & CLOCK_EVT_FEAT_HRTIMER))
> + return 0;
> + if (bc->next_event.tv64 == KTIME_MAX)
> + return 0;
> + return bc->bound_on == cpu ? -EBUSY : 0;
> +}
> +
> +static void broadcast_shutdown_local(struct clock_event_device *bc,
> + struct clock_event_device *dev)
> +{
> + /*
> + * For hrtimer based broadcasting we cannot shutdown the cpu
> + * local device if our own event is the first one to expire or
> + * if we own the broadcast timer.
> + */
> + if (bc->features & CLOCK_EVT_FEAT_HRTIMER) {
> + if (broadcast_needs_cpu(bc))
> + return;
> + if (dev->next_event.tv64 < bc->next_event.tv64)
> + return;
> + }
> + clockevents_set_mode(dev, CLOCK_EVT_MODE_SHUTDOWN);
> +}
> +
> +static void broadcast_move_bc(int deadcpu)
> +{
> + struct clock_event_device *bc = tick_broadcast_device.evtdev;
> +
> + if (!bc || !broadcast_needs_cpu(bc, deadcpu))
> + return;
> + /* This moves the broadcast assignment to this cpu */
> + clockevents_program_event(bc, bc->next_event, 1);
> +}
> +
> /*
> * Powerstate information: The system enters/leaves a state, where
> * affected devices might stop
> @@ -639,8 +675,8 @@ void tick_broadcast_oneshot_control(unsi
> struct clock_event_device *bc, *dev;
> struct tick_device *td;
> unsigned long flags;
> + int cpu, ret = 0;
> ktime_t now;
> - int cpu;
>
> /*
> * Periodic mode does not care about the enter/exit of power
> @@ -666,7 +702,7 @@ void tick_broadcast_oneshot_control(unsi
> if (reason == CLOCK_EVT_NOTIFY_BROADCAST_ENTER) {
> if (!cpumask_test_and_set_cpu(cpu, tick_broadcast_oneshot_mask)) {
> WARN_ON_ONCE(cpumask_test_cpu(cpu, tick_broadcast_pending_mask));
> - clockevents_set_mode(dev, CLOCK_EVT_MODE_SHUTDOWN);
> + broadcast_shutdown_local(bc, dev);
> /*
> * We only reprogram the broadcast timer if we
> * did not mark ourself in the force mask and
> @@ -679,6 +715,11 @@ void tick_broadcast_oneshot_control(unsi
> dev->next_event.tv64 < bc->next_event.tv64)
> tick_broadcast_set_event(bc, cpu, dev->next_event, 1);
> }
> + /*
> + * If the current CPU owns the hrtimer broadcast
> + * mechanism, it cannot go deep idle.
> + */
> + ret = broadcast_needs_cpu(bc, cpu);
> } else {
> if (cpumask_test_and_clear_cpu(cpu, tick_broadcast_oneshot_mask)) {
> clockevents_set_mode(dev, CLOCK_EVT_MODE_ONESHOT);
> @@ -851,6 +892,8 @@ void tick_shutdown_broadcast_oneshot(uns
> cpumask_clear_cpu(cpu, tick_broadcast_pending_mask);
> cpumask_clear_cpu(cpu, tick_broadcast_force_mask);
>
> + broadcast_move_bc(cpu);
> +
> raw_spin_unlock_irqrestore(&tick_broadcast_lock, flags);
> }
>
>
^ permalink raw reply
* [PATCH V2 0/2] time/cpuidle: Support in tick broadcast framework in absence of external clock device
From: Preeti U Murthy @ 2014-01-24 6:57 UTC (permalink / raw)
To: daniel.lezcano, peterz, fweisbec, galak, paul.gortmaker, paulus,
mingo, mikey, shangw, rafael.j.wysocki, agraf, benh, paulmck,
arnd, linux-pm, rostedt, michael, john.stultz, anton, tglx,
chenhui.zhao, deepthi, r58472, geoff, linux-kernel, srivatsa.bhat,
schwidefsky, svaidy, linuxppc-dev
This earlier version of this patchset can be found here:
https://lkml.org/lkml/2013/12/12/687. This version has been based on the
discussion in http://www.kernelhub.org/?p=2&msg=399516.
This patchset provides the hooks that the architectures without an external
clock device and deep idle states where the local timers stop can make use of.
Presently we are in need of this support on certain implementations of
PowerPC. This patchset has been used on PowerPC for testing with
---
Preeti U Murthy (1):
time: Change the return type of clockevents_notify() to integer
Thomas Gleixner (1):
tick/cpuidle: Initialize hrtimer mode of broadcast
include/linux/clockchips.h | 15 ++++-
kernel/time/Makefile | 2 -
kernel/time/clockevents.c | 8 ++-
kernel/time/tick-broadcast-hrtimer.c | 102 ++++++++++++++++++++++++++++++++++
kernel/time/tick-broadcast.c | 51 ++++++++++++++++-
kernel/time/tick-internal.h | 6 +-
6 files changed, 171 insertions(+), 13 deletions(-)
create mode 100644 kernel/time/tick-broadcast-hrtimer.c
--
^ permalink raw reply
* [PATCH V2 1/2] time: Change the return type of clockevents_notify() to integer
From: Preeti U Murthy @ 2014-01-24 6:57 UTC (permalink / raw)
To: daniel.lezcano, peterz, fweisbec, galak, paul.gortmaker, paulus,
mingo, mikey, shangw, rafael.j.wysocki, agraf, benh, paulmck,
arnd, linux-pm, rostedt, michael, john.stultz, anton, tglx,
chenhui.zhao, deepthi, r58472, geoff, linux-kernel, srivatsa.bhat,
schwidefsky, svaidy, linuxppc-dev
In-Reply-To: <20140124065501.17564.39363.stgit@preeti.in.ibm.com>
The broadcast framework can potentially be made use of by archs which do not have an
external clock device as well. Then, it is required that one of the CPUs need
to handle the broadcasting of wakeup IPIs to the CPUs in deep idle. As a
result its local timers should remain functional all the time. For such
a CPU, the BROADCAST_ENTER notification has to fail indicating that its clock
device cannot be shutdown. To make way for this support, change the return
type of tick_broadcast_oneshot_control() and hence clockevents_notify() to
indicate such scenarios.
Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
---
include/linux/clockchips.h | 6 +++---
kernel/time/clockevents.c | 8 +++++---
kernel/time/tick-broadcast.c | 6 ++++--
kernel/time/tick-internal.h | 6 +++---
4 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index 493aa02..ac81b56 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -186,9 +186,9 @@ static inline int tick_check_broadcast_expired(void) { return 0; }
#endif
#ifdef CONFIG_GENERIC_CLOCKEVENTS
-extern void clockevents_notify(unsigned long reason, void *arg);
+extern int clockevents_notify(unsigned long reason, void *arg);
#else
-static inline void clockevents_notify(unsigned long reason, void *arg) {}
+static inline int clockevents_notify(unsigned long reason, void *arg) {}
#endif
#else /* CONFIG_GENERIC_CLOCKEVENTS_BUILD */
@@ -196,7 +196,7 @@ static inline void clockevents_notify(unsigned long reason, void *arg) {}
static inline void clockevents_suspend(void) {}
static inline void clockevents_resume(void) {}
-static inline void clockevents_notify(unsigned long reason, void *arg) {}
+static inline int clockevents_notify(unsigned long reason, void *arg) {}
static inline int tick_check_broadcast_expired(void) { return 0; }
#endif
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 086ad60..79b8685 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -524,12 +524,13 @@ void clockevents_resume(void)
#ifdef CONFIG_GENERIC_CLOCKEVENTS
/**
* clockevents_notify - notification about relevant events
+ * Returns 0 on success, any other value on error
*/
-void clockevents_notify(unsigned long reason, void *arg)
+int clockevents_notify(unsigned long reason, void *arg)
{
struct clock_event_device *dev, *tmp;
unsigned long flags;
- int cpu;
+ int cpu, ret = 0;
raw_spin_lock_irqsave(&clockevents_lock, flags);
@@ -542,7 +543,7 @@ void clockevents_notify(unsigned long reason, void *arg)
case CLOCK_EVT_NOTIFY_BROADCAST_ENTER:
case CLOCK_EVT_NOTIFY_BROADCAST_EXIT:
- tick_broadcast_oneshot_control(reason);
+ ret = tick_broadcast_oneshot_control(reason);
break;
case CLOCK_EVT_NOTIFY_CPU_DYING:
@@ -585,6 +586,7 @@ void clockevents_notify(unsigned long reason, void *arg)
break;
}
raw_spin_unlock_irqrestore(&clockevents_lock, flags);
+ return ret;
}
EXPORT_SYMBOL_GPL(clockevents_notify);
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 9532690..be00692 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -633,14 +633,15 @@ again:
/*
* Powerstate information: The system enters/leaves a state, where
* affected devices might stop
+ * Returns 0 on success, -EBUSY if the cpu is used to broadcast wakeups.
*/
-void tick_broadcast_oneshot_control(unsigned long reason)
+int tick_broadcast_oneshot_control(unsigned long reason)
{
struct clock_event_device *bc, *dev;
struct tick_device *td;
unsigned long flags;
ktime_t now;
- int cpu;
+ int cpu, ret = 0;
/*
* Periodic mode does not care about the enter/exit of power
@@ -746,6 +747,7 @@ void tick_broadcast_oneshot_control(unsigned long reason)
}
out:
raw_spin_unlock_irqrestore(&tick_broadcast_lock, flags);
+ return ret;
}
/*
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 18e71f7..164465c 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -46,7 +46,7 @@ extern int tick_switch_to_oneshot(void (*handler)(struct clock_event_device *));
extern void tick_resume_oneshot(void);
# ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
extern void tick_broadcast_setup_oneshot(struct clock_event_device *bc);
-extern void tick_broadcast_oneshot_control(unsigned long reason);
+extern int tick_broadcast_oneshot_control(unsigned long reason);
extern void tick_broadcast_switch_to_oneshot(void);
extern void tick_shutdown_broadcast_oneshot(unsigned int *cpup);
extern int tick_resume_broadcast_oneshot(struct clock_event_device *bc);
@@ -58,7 +58,7 @@ static inline void tick_broadcast_setup_oneshot(struct clock_event_device *bc)
{
BUG();
}
-static inline void tick_broadcast_oneshot_control(unsigned long reason) { }
+static inline int tick_broadcast_oneshot_control(unsigned long reason) { }
static inline void tick_broadcast_switch_to_oneshot(void) { }
static inline void tick_shutdown_broadcast_oneshot(unsigned int *cpup) { }
static inline int tick_broadcast_oneshot_active(void) { return 0; }
@@ -87,7 +87,7 @@ static inline void tick_broadcast_setup_oneshot(struct clock_event_device *bc)
{
BUG();
}
-static inline void tick_broadcast_oneshot_control(unsigned long reason) { }
+static inline int tick_broadcast_oneshot_control(unsigned long reason) { }
static inline void tick_shutdown_broadcast_oneshot(unsigned int *cpup) { }
static inline int tick_resume_broadcast_oneshot(struct clock_event_device *bc)
{
^ permalink raw reply related
* [PATCH V2 2/2] tick/cpuidle: Initialize hrtimer mode of broadcast
From: Preeti U Murthy @ 2014-01-24 6:58 UTC (permalink / raw)
To: daniel.lezcano, peterz, fweisbec, galak, paul.gortmaker, paulus,
mingo, mikey, shangw, rafael.j.wysocki, agraf, benh, paulmck,
arnd, linux-pm, rostedt, michael, john.stultz, anton, tglx,
chenhui.zhao, deepthi, r58472, geoff, linux-kernel, srivatsa.bhat,
schwidefsky, svaidy, linuxppc-dev
In-Reply-To: <20140124065501.17564.39363.stgit@preeti.in.ibm.com>
From: Thomas Gleixner <tglx@linutronix.de>
On some architectures, in certain CPU deep idle states the local timers stop.
An external clock device is used to wakeup these CPUs. The kernel support for the
wakeup of these CPUs is provided by the tick broadcast framework by using the
external clock device as the wakeup source.
However not all implementations of architectures provide such an external
clock device. This patch includes support in the broadcast framework to handle
the wakeup of the CPUs in deep idle states on such systems by queuing a hrtimer
on one of the CPUs, which is meant to handle the wakeup of CPUs in deep idle states.
This patchset introduces a pseudo clock device which can be registered by the
archs as tick_broadcast_device in the absence of a real external clock
device. Once registered, the broadcast framework will work as is for these
architectures as long as the archs take care of the BROADCAST_ENTER
notification failing for one of the CPUs. This CPU is made the stand by CPU to
handle wakeup of the CPUs in deep idle and it *must not enter deep idle states*.
The CPU with the earliest wakeup is chosen to be this CPU. Hence this way the
stand by CPU dynamically moves around and so does the hrtimer which is queued
to trigger at the next earliest wakeup time. This is consistent with the case where
an external clock device is present. The smp affinity of this clock device is
set to the CPU with the earliest wakeup. This patchset handles the hotplug of
the stand by CPU as well by moving the hrtimer on to the CPU handling the CPU_DEAD
notification.
Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
[Added Changelog and code to handle reprogramming of hrtimer]
---
include/linux/clockchips.h | 9 +++
kernel/time/Makefile | 2 -
kernel/time/tick-broadcast-hrtimer.c | 102 ++++++++++++++++++++++++++++++++++
kernel/time/tick-broadcast.c | 45 +++++++++++++++
4 files changed, 156 insertions(+), 2 deletions(-)
create mode 100644 kernel/time/tick-broadcast-hrtimer.c
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index ac81b56..2293025 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -62,6 +62,11 @@ enum clock_event_mode {
#define CLOCK_EVT_FEAT_DYNIRQ 0x000020
#define CLOCK_EVT_FEAT_PERCPU 0x000040
+/*
+ * Clockevent device is based on a hrtimer for broadcast
+ */
+#define CLOCK_EVT_FEAT_HRTIMER 0x000080
+
/**
* struct clock_event_device - clock event device descriptor
* @event_handler: Assigned by the framework to be called by the low
@@ -83,6 +88,7 @@ enum clock_event_mode {
* @name: ptr to clock event name
* @rating: variable to rate clock event devices
* @irq: IRQ number (only for non CPU local devices)
+ * @bound_on: Bound on CPU
* @cpumask: cpumask to indicate for which CPUs this device works
* @list: list head for the management code
* @owner: module reference
@@ -113,6 +119,7 @@ struct clock_event_device {
const char *name;
int rating;
int irq;
+ int bound_on;
const struct cpumask *cpumask;
struct list_head list;
struct module *owner;
@@ -180,9 +187,11 @@ extern int tick_receive_broadcast(void);
#endif
#if defined(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) && defined(CONFIG_TICK_ONESHOT)
+extern void tick_setup_hrtimer_broadcast(void);
extern int tick_check_broadcast_expired(void);
#else
static inline int tick_check_broadcast_expired(void) { return 0; }
+static void tick_setup_hrtimer_broadcast(void) {};
#endif
#ifdef CONFIG_GENERIC_CLOCKEVENTS
diff --git a/kernel/time/Makefile b/kernel/time/Makefile
index 9250130..06151ef 100644
--- a/kernel/time/Makefile
+++ b/kernel/time/Makefile
@@ -3,7 +3,7 @@ obj-y += timeconv.o posix-clock.o alarmtimer.o
obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD) += clockevents.o
obj-$(CONFIG_GENERIC_CLOCKEVENTS) += tick-common.o
-obj-$(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) += tick-broadcast.o
+obj-$(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) += tick-broadcast.o tick-broadcast-hrtimer.o
obj-$(CONFIG_GENERIC_SCHED_CLOCK) += sched_clock.o
obj-$(CONFIG_TICK_ONESHOT) += tick-oneshot.o
obj-$(CONFIG_TICK_ONESHOT) += tick-sched.o
diff --git a/kernel/time/tick-broadcast-hrtimer.c b/kernel/time/tick-broadcast-hrtimer.c
new file mode 100644
index 0000000..23f4925
--- /dev/null
+++ b/kernel/time/tick-broadcast-hrtimer.c
@@ -0,0 +1,102 @@
+/*
+ * linux/kernel/time/tick-broadcast-hrtimer.c
+ * This file emulates a local clock event device
+ * via a pseudo clock device.
+ */
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/hrtimer.h>
+#include <linux/interrupt.h>
+#include <linux/percpu.h>
+#include <linux/profile.h>
+#include <linux/clockchips.h>
+#include <linux/sched.h>
+#include <linux/smp.h>
+#include <linux/module.h>
+
+#include "tick-internal.h"
+
+static struct hrtimer bctimer;
+
+static void bc_set_mode(enum clock_event_mode mode,
+ struct clock_event_device *bc)
+{
+ switch (mode) {
+ case CLOCK_EVT_MODE_SHUTDOWN:
+ /*
+ * Note, we cannot cancel the timer here as we might
+ * run into the following live lock scenario:
+ *
+ * cpu 0 cpu1
+ * lock(broadcast_lock);
+ * hrtimer_interrupt()
+ * bc_handler()
+ * tick_handle_oneshot_broadcast();
+ * lock(broadcast_lock);
+ * hrtimer_cancel()
+ * wait_for_callback()
+ */
+ hrtimer_try_to_cancel(&bctimer);
+ break;
+ default:
+ break;
+ }
+}
+
+/*
+ * This is called from the guts of the broadcast code when the cpu
+ * which is about to enter idle has the earliest broadcast timer event.
+ */
+static int bc_set_next(ktime_t expires, struct clock_event_device *bc)
+{
+ ktime_t now, interval;
+ /*
+ * We try to cancel the timer first. If the callback is on
+ * flight on some other cpu then we let it handle it. If we
+ * were able to cancel the timer nothing can rearm it as we
+ * own broadcast_lock.
+ *
+ * However if we are called from the hrtimer interrupt handler
+ * itself, reprogram it.
+ */
+ if (hrtimer_try_to_cancel(&bctimer) >= 0) {
+ hrtimer_start(&bctimer, expires, HRTIMER_MODE_ABS_PINNED);
+ /* Bind the "device" to the cpu */
+ bc->bound_on = smp_processor_id();
+ } else if (bc->bound_on == smp_processor_id()) {
+ now = ktime_get();
+ interval = ktime_sub(expires, now);
+ hrtimer_forward_now(&bctimer, interval);
+ }
+ return 0;
+}
+
+static struct clock_event_device ce_broadcast_hrtimer = {
+ .set_mode = bc_set_mode,
+ .set_next_ktime = bc_set_next,
+ .features = CLOCK_EVT_FEAT_ONESHOT |
+ CLOCK_EVT_FEAT_KTIME |
+ CLOCK_EVT_FEAT_HRTIMER,
+ .rating = 0,
+ .bound_on = -1,
+ .min_delta_ns = 1,
+ .max_delta_ns = KTIME_MAX,
+ .min_delta_ticks = 1,
+ .max_delta_ticks = KTIME_MAX,
+ .mult = 1,
+ .shift = 0,
+ .cpumask = cpu_all_mask,
+};
+
+static enum hrtimer_restart bc_handler(struct hrtimer *t)
+{
+ ce_broadcast_hrtimer.event_handler(&ce_broadcast_hrtimer);
+ return HRTIMER_RESTART;
+}
+
+void tick_setup_hrtimer_broadcast(void)
+{
+ hrtimer_init(&bctimer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
+ bctimer.function = bc_handler;
+ clockevents_register_device(&ce_broadcast_hrtimer);
+}
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index be00692..7ed7e69 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -630,6 +630,42 @@ again:
raw_spin_unlock(&tick_broadcast_lock);
}
+static int broadcast_needs_cpu(struct clock_event_device *bc, int cpu)
+{
+ if (!(bc->features & CLOCK_EVT_FEAT_HRTIMER))
+ return 0;
+ if (bc->next_event.tv64 == KTIME_MAX)
+ return 0;
+ return bc->bound_on == cpu ? -EBUSY : 0;
+}
+
+static void broadcast_shutdown_local(struct clock_event_device *bc,
+ struct clock_event_device *dev)
+{
+ /*
+ * For hrtimer based broadcasting we cannot shutdown the cpu
+ * local device if our own event is the first one to expire or
+ * if we own the broadcast timer.
+ */
+ if (bc->features & CLOCK_EVT_FEAT_HRTIMER) {
+ if (broadcast_needs_cpu(bc, smp_processor_id()))
+ return;
+ if (dev->next_event.tv64 < bc->next_event.tv64)
+ return;
+ }
+ clockevents_set_mode(dev, CLOCK_EVT_MODE_SHUTDOWN);
+}
+
+static void broadcast_move_bc(int deadcpu)
+{
+ struct clock_event_device *bc = tick_broadcast_device.evtdev;
+
+ if (!bc || !broadcast_needs_cpu(bc, deadcpu))
+ return;
+ /* This moves the broadcast assignment to this cpu */
+ clockevents_program_event(bc, bc->next_event, 1);
+}
+
/*
* Powerstate information: The system enters/leaves a state, where
* affected devices might stop
@@ -667,7 +703,7 @@ int tick_broadcast_oneshot_control(unsigned long reason)
if (reason == CLOCK_EVT_NOTIFY_BROADCAST_ENTER) {
if (!cpumask_test_and_set_cpu(cpu, tick_broadcast_oneshot_mask)) {
WARN_ON_ONCE(cpumask_test_cpu(cpu, tick_broadcast_pending_mask));
- clockevents_set_mode(dev, CLOCK_EVT_MODE_SHUTDOWN);
+ broadcast_shutdown_local(bc, dev);
/*
* We only reprogram the broadcast timer if we
* did not mark ourself in the force mask and
@@ -680,6 +716,11 @@ int tick_broadcast_oneshot_control(unsigned long reason)
dev->next_event.tv64 < bc->next_event.tv64)
tick_broadcast_set_event(bc, cpu, dev->next_event, 1);
}
+ /*
+ * If the current CPU owns the hrtimer broadcast
+ * mechanism, it cannot go deep idle.
+ */
+ ret = broadcast_needs_cpu(bc, cpu);
} else {
if (cpumask_test_and_clear_cpu(cpu, tick_broadcast_oneshot_mask)) {
clockevents_set_mode(dev, CLOCK_EVT_MODE_ONESHOT);
@@ -853,6 +894,8 @@ void tick_shutdown_broadcast_oneshot(unsigned int *cpup)
cpumask_clear_cpu(cpu, tick_broadcast_pending_mask);
cpumask_clear_cpu(cpu, tick_broadcast_force_mask);
+ broadcast_move_bc(cpu);
+
raw_spin_unlock_irqrestore(&tick_broadcast_lock, flags);
}
^ permalink raw reply related
* Re: [PATCH V2] cpuidle/governors: Fix logic in selection of idle states
From: Daniel Lezcano @ 2014-01-24 9:08 UTC (permalink / raw)
To: Preeti U Murthy
Cc: paulmck, linux-pm, rjw, linux-kernel, srivatsa.bhat, linuxppc-dev,
tuukka.tikkanen
In-Reply-To: <52E0F9ED.2010506@linux.vnet.ibm.com>
On 01/23/2014 12:15 PM, Preeti U Murthy wrote:
> Hi Daniel,
>
> Thank you for the review.
>
> On 01/22/2014 01:59 PM, Daniel Lezcano wrote:
>> On 01/17/2014 05:33 AM, Preeti U Murthy wrote:
>>>
>>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>>> index a55e68f..831b664 100644
>>> --- a/drivers/cpuidle/cpuidle.c
>>> +++ b/drivers/cpuidle/cpuidle.c
>>> @@ -131,8 +131,9 @@ int cpuidle_idle_call(void)
>>>
>>> /* ask the governor for the next state */
>>> next_state = cpuidle_curr_governor->select(drv, dev);
>>> +
>>> + dev->last_residency = 0;
>>> if (need_resched()) {
>>> - dev->last_residency = 0;
>>
>> Why do you need to do this change ? ^^^^^
>
> So as to keep the last_residency consistent with the case that this patch
> addresses: where no idle state could be selected due to strict latency
> requirements or disabled states and hence the cpu exits without entering
> idle. Else it would contain the stale value from the previous idle state
> entry.
>
> But coming to think of it dev->last_residency is not used when the last
> entered idle state index is -1.
>
> So I have reverted this change as well in the revised patch below along
> with mentioning the reason in the last paragraph of the changelog.
>
>>
>>> /* give the governor an opportunity to reflect on the
>>> outcome */
>>> if (cpuidle_curr_governor->reflect)
>>> cpuidle_curr_governor->reflect(dev, next_state);
>>> @@ -140,6 +141,18 @@ int cpuidle_idle_call(void)
>>> return 0;
>>> }
>>>
>>> + /* Unlike in the need_resched() case, we return here because the
>>> + * governor did not find a suitable idle state. However idle is
>>> still
>>> + * in progress as we are not asked to reschedule. Hence we return
>>> + * without enabling interrupts.
>>
>> That will lead to a WARN.
>>
>>> + * NOTE: The return code should still be success, since the
>>> verdict of this
>>> + * call is "do not enter any idle state" and not a failed call
>>> due to
>>> + * errors.
>>> + */
>>> + if (next_state < 0)
>>> + return 0;
>>> +
>>
>> Returning from here breaks the symmetry of the trace.
>
> I have addressed the above concerns in the patch found below.
> Does the rest of the patch look sound?
>
> Regards
> Preeti U Murthy
>
> ----------------------------------------------------------------------
>
> cpuidle/governors: Fix logic in selection of idle states
>
> From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
>
> The cpuidle governors today are not handling scenarios where no idle state
> can be chosen. Such scenarios coud arise if the user has disabled all the
> idle states at runtime or the latency requirement from the cpus is very strict.
>
> The menu governor returns 0th index of the idle state table when no other
> idle state is suitable. This is even when the idle state corresponding to this
> index is disabled or the latency requirement is strict and the exit_latency
> of the lowest idle state is also not acceptable. Hence this patch
> fixes this logic in the menu governor by defaulting to an idle state index
> of -1 unless any other state is suitable.
>
> The ladder governor needs a few more fixes in addition to that required in the
> menu governor. When the ladder governor decides to demote the idle state of a
> CPU, it does not check if the lower idle states are enabled. Add this logic
> in addition to the logic where it chooses an index of -1 if it can neither
> promote or demote the idle state of a cpu nor can it choose the current idle
> state.
>
> The cpuidle_idle_call() will return back if the governor decides upon not
> entering any idle state. However it cannot return an error code because all
> archs have the logic today that if the call to cpuidle_idle_call() fails, it
> means that the cpuidle driver failed to *function*; for instance due to
> errors during registration. As a result they end up deciding upon a
> default idle state on their own, which could very well be a deep idle state.
> This is incorrect in cases where no idle state is suitable.
>
> Besides for the scenario that this patch is addressing, the call actually
> succeeds. Its just that no idle state is thought to be suitable by the governors.
> Under such a circumstance return success code without entering any idle
> state.
>
> The consequence of this patch additionally on the menu governor is that as
> long as a valid idle state cannot be chosen, the cpuidle statistics that this
> governor uses to predict the next idle state remain untouched from the last
> valid idle state. This is because an idle state is not even being predicted
> in this path, hence there is no point correcting the prediction either.
>
> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
>
> Changes from V1:https://lkml.org/lkml/2014/1/14/26
>
> 1. Change the return code to success from -EINVAL due to the reason mentioned
> in the changelog.
> 2. Add logic that the patch is addressing in the ladder governor as well.
> 3. Added relevant comments and removed redundant logic as suggested in the
> above thread.
> ---
> drivers/cpuidle/cpuidle.c | 15 +++++
> drivers/cpuidle/governors/ladder.c | 101 ++++++++++++++++++++++++++----------
> drivers/cpuidle/governors/menu.c | 7 +-
> 3 files changed, 90 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index a55e68f..19d17e8 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -131,8 +131,9 @@ int cpuidle_idle_call(void)
>
> /* ask the governor for the next state */
> next_state = cpuidle_curr_governor->select(drv, dev);
> +
> + dev->last_residency = 0;
> if (need_resched()) {
What about if (need_resched() || next_state < 0) ?
> - dev->last_residency = 0;
> /* give the governor an opportunity to reflect on the outcome */
> if (cpuidle_curr_governor->reflect)
> cpuidle_curr_governor->reflect(dev, next_state);
> @@ -141,6 +142,16 @@ int cpuidle_idle_call(void)
> }
>
> trace_cpu_idle_rcuidle(next_state, dev->cpu);
> + /*
> + * NOTE: The return code should still be success, since the verdict of
> + * this call is "do not enter any idle state". It is not a failed call
> + * due to errors.
> + */
> + if (next_state < 0) {
> + entered_state = next_state;
> + local_irq_enable();
> + goto out;
> + }
>
> broadcast = !!(drv->states[next_state].flags & CPUIDLE_FLAG_TIMER_STOP);
>
> @@ -156,7 +167,7 @@ int cpuidle_idle_call(void)
> if (broadcast)
> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
>
> - trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);
> +out: trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);
>
> /* give the governor an opportunity to reflect on the outcome */
> if (cpuidle_curr_governor->reflect)
> diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
> index 9f08e8c..7e93aaa 100644
> --- a/drivers/cpuidle/governors/ladder.c
> +++ b/drivers/cpuidle/governors/ladder.c
> @@ -58,6 +58,36 @@ static inline void ladder_do_selection(struct ladder_device *ldev,
> ldev->last_state_idx = new_idx;
> }
>
> +static int can_promote(struct ladder_device *ldev, int last_idx,
> + int last_residency)
> +{
> + struct ladder_device_state *last_state;
> +
> + last_state = &ldev->states[last_idx];
> + if (last_residency > last_state->threshold.promotion_time) {
> + last_state->stats.promotion_count++;
> + last_state->stats.demotion_count = 0;
> + if (last_state->stats.promotion_count >= last_state->threshold.promotion_count)
> + return 1;
> + }
> + return 0;
> +}
> +
> +static int can_demote(struct ladder_device *ldev, int last_idx,
> + int last_residency)
> +{
> + struct ladder_device_state *last_state;
> +
> + last_state = &ldev->states[last_idx];
> + if (last_residency < last_state->threshold.demotion_time) {
> + last_state->stats.demotion_count++;
> + last_state->stats.promotion_count = 0;
> + if (last_state->stats.demotion_count >= last_state->threshold.demotion_count)
> + return 1;
> + }
> + return 0;
> +}
> +
> /**
> * ladder_select_state - selects the next state to enter
> * @drv: cpuidle driver
> @@ -73,29 +103,33 @@ static int ladder_select_state(struct cpuidle_driver *drv,
>
> /* Special case when user has set very strict latency requirement */
> if (unlikely(latency_req == 0)) {
> - ladder_do_selection(ldev, last_idx, 0);
> - return 0;
> + if (last_idx >= 0)
> + ladder_do_selection(ldev, last_idx, -1);
> + goto out;
> }
>
> - last_state = &ldev->states[last_idx];
> -
> - if (drv->states[last_idx].flags & CPUIDLE_FLAG_TIME_VALID) {
> - last_residency = cpuidle_get_last_residency(dev) - \
> - drv->states[last_idx].exit_latency;
> + if (last_idx >= 0) {
> + if (drv->states[last_idx].flags & CPUIDLE_FLAG_TIME_VALID) {
> + last_residency = cpuidle_get_last_residency(dev) - \
> + drv->states[last_idx].exit_latency;
> + } else {
> + last_state = &ldev->states[last_idx];
> + last_residency = last_state->threshold.promotion_time + 1;
> + }
> }
> - else
> - last_residency = last_state->threshold.promotion_time + 1;
>
> /* consider promotion */
> if (last_idx < drv->state_count - 1 &&
> !drv->states[last_idx + 1].disabled &&
> !dev->states_usage[last_idx + 1].disable &&
> - last_residency > last_state->threshold.promotion_time &&
> drv->states[last_idx + 1].exit_latency <= latency_req) {
> - last_state->stats.promotion_count++;
> - last_state->stats.demotion_count = 0;
> - if (last_state->stats.promotion_count >= last_state->threshold.promotion_count) {
> - ladder_do_selection(ldev, last_idx, last_idx + 1);
> + if (last_idx >= 0) {
> + if (can_promote(ldev, last_idx, last_residency)) {
> + ladder_do_selection(ldev, last_idx, last_idx + 1);
> + return last_idx + 1;
> + }
> + } else {
> + ldev->last_state_idx = last_idx + 1;
> return last_idx + 1;
> }
> }
> @@ -107,26 +141,36 @@ static int ladder_select_state(struct cpuidle_driver *drv,
> drv->states[last_idx].exit_latency > latency_req)) {
> int i;
>
> - for (i = last_idx - 1; i > CPUIDLE_DRIVER_STATE_START; i--) {
> - if (drv->states[i].exit_latency <= latency_req)
> + for (i = last_idx - 1; i >= CPUIDLE_DRIVER_STATE_START; i--) {
> + if (drv->states[i].exit_latency <= latency_req &&
> + !(drv->states[i].disabled || dev->states_usage[i].disable))
> break;
> }
> - ladder_do_selection(ldev, last_idx, i);
> - return i;
> + if (i >= 0) {
> + ladder_do_selection(ldev, last_idx, i);
> + return i;
> + }
> + goto out;
> }
>
> - if (last_idx > CPUIDLE_DRIVER_STATE_START &&
> - last_residency < last_state->threshold.demotion_time) {
> - last_state->stats.demotion_count++;
> - last_state->stats.promotion_count = 0;
> - if (last_state->stats.demotion_count >= last_state->threshold.demotion_count) {
> - ladder_do_selection(ldev, last_idx, last_idx - 1);
> - return last_idx - 1;
> + if (last_idx > CPUIDLE_DRIVER_STATE_START) {
> + int i = last_idx - 1;
> +
> + if (can_demote(ldev, last_idx, last_residency) &&
> + !(drv->states[i].disabled || dev->states_usage[i].disable)) {
> + ladder_do_selection(ldev, last_idx, i);
> + return i;
> }
> + /* We come here when the last_idx is still a suitable idle state,
> + * just that promotion or demotion is not ideal.
> + */
> + ldev->last_state_idx = last_idx;
> + return last_idx;
> }
>
> - /* otherwise remain at the current state */
> - return last_idx;
> + /* we come here if no idle state is suitable */
> +out: ldev->last_state_idx = -1;
> + return ldev->last_state_idx;
> }
>
> /**
> @@ -166,7 +210,8 @@ static int ladder_enable_device(struct cpuidle_driver *drv,
> /**
> * ladder_reflect - update the correct last_state_idx
> * @dev: the CPU
> - * @index: the index of actual state entered
> + * @index: the index of actual state entered or -1 if no idle state is
> + * suitable.
> */
> static void ladder_reflect(struct cpuidle_device *dev, int index)
> {
> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
> index cf7f2f0..e9f17ce 100644
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -297,12 +297,12 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
> data->needs_update = 0;
> }
>
> - data->last_state_idx = 0;
> + data->last_state_idx = -1;
> data->exit_us = 0;
>
> /* Special case when user has set very strict latency requirement */
> if (unlikely(latency_req == 0))
> - return 0;
> + return data->last_state_idx;
>
> /* determine the expected residency time, round up */
> t = ktime_to_timespec(tick_nohz_get_sleep_length());
> @@ -368,7 +368,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
> /**
> * menu_reflect - records that data structures need update
> * @dev: the CPU
> - * @index: the index of actual entered state
> + * @index: the index of actual entered state or -1 if no idle state is
> + * suitable.
> *
> * NOTE: it's important to be fast here because this operation will add to
> * the overall exit latency.
>
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox