From 3c0a4b185f6c82c06025720b00a490c719a6f0ff Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Wed, 21 Oct 2020 09:22:59 +0800 Subject: clocksource/drivers/sp804: Add static for functions such as sp804_clockevents_init() Add static for sp804_clocksource_and_sched_clock_init() and sp804_clockevents_init(), they are only used in timer-sp804.c now. Otherwise, the following warning will be reported: drivers/clocksource/timer-sp804.c:68:12: warning: no previous prototype \ for 'sp804_clocksource_and_sched_clock_init' [-Wmissing-prototypes] drivers/clocksource/timer-sp804.c:162:12: warning: no previous prototype \ for 'sp804_clockevents_init' [-Wmissing-prototypes] Fixes: 975434f8b24a ("clocksource/drivers/sp804: Delete the leading "__" of some functions") Fixes: 65f4d7ddc7b6 ("clocksource/drivers/sp804: Remove unused sp804_timer_disable() and timer-sp804.h") Reported-by: kernel test robot Signed-off-by: Zhen Lei Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201021012259.2067-2-thunder.leizhen@huawei.com --- drivers/clocksource/timer-sp804.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c index 6e8ad4a4ea3c..db5330cc49bc 100644 --- a/drivers/clocksource/timer-sp804.c +++ b/drivers/clocksource/timer-sp804.c @@ -117,10 +117,10 @@ static u64 notrace sp804_read(void) return ~readl_relaxed(sched_clkevt->value); } -int __init sp804_clocksource_and_sched_clock_init(void __iomem *base, - const char *name, - struct clk *clk, - int use_sched_clock) +static int __init sp804_clocksource_and_sched_clock_init(void __iomem *base, + const char *name, + struct clk *clk, + int use_sched_clock) { long rate; struct sp804_clkevt *clkevt; @@ -216,8 +216,8 @@ static struct clock_event_device sp804_clockevent = { .rating = 300, }; -int __init sp804_clockevents_init(void __iomem *base, unsigned int irq, - struct clk *clk, const char *name) +static int __init sp804_clockevents_init(void __iomem *base, unsigned int irq, + struct clk *clk, const char *name) { struct clock_event_device *evt = &sp804_clockevent; long rate; -- cgit v1.2.3 From 3c07bf0fc3558f680374f8ac6d148b0082aa08c6 Mon Sep 17 00:00:00 2001 From: Kefeng Wang Date: Thu, 29 Oct 2020 20:33:14 +0800 Subject: clocksource/drivers/sp804: Make some symbol static drivers/clocksource/timer-sp804.c:38:31: warning: symbol 'arm_sp804_timer' was not declared. Should it be static? drivers/clocksource/timer-sp804.c:47:31: warning: symbol 'hisi_sp804_timer' was not declared. Should it be static? drivers/clocksource/timer-sp804.c:120:12: warning: symbol 'sp804_clocksource_and_sched_clock_init' was not declared. Should it be static? drivers/clocksource/timer-sp804.c:219:12: warning: symbol 'sp804_clockevents_init' was not declared. Should it be static? And move __initdata after the variables. Signed-off-by: Kefeng Wang Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201029123317.90286-2-wangkefeng.wang@huawei.com --- drivers/clocksource/timer-sp804.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c index db5330cc49bc..22a68cb83cf3 100644 --- a/drivers/clocksource/timer-sp804.c +++ b/drivers/clocksource/timer-sp804.c @@ -34,8 +34,7 @@ #define HISI_TIMER_BGLOAD 0x20 #define HISI_TIMER_BGLOAD_H 0x24 - -struct sp804_timer __initdata arm_sp804_timer = { +static struct sp804_timer arm_sp804_timer __initdata = { .load = TIMER_LOAD, .value = TIMER_VALUE, .ctrl = TIMER_CTRL, @@ -44,7 +43,7 @@ struct sp804_timer __initdata arm_sp804_timer = { .width = 32, }; -struct sp804_timer __initdata hisi_sp804_timer = { +static struct sp804_timer hisi_sp804_timer __initdata = { .load = HISI_TIMER_LOAD, .load_h = HISI_TIMER_LOAD_H, .value = HISI_TIMER_VALUE, -- cgit v1.2.3 From 9d4965eb438f0c9f93e91ce6bfec72bbb8def988 Mon Sep 17 00:00:00 2001 From: Kefeng Wang Date: Thu, 29 Oct 2020 20:33:15 +0800 Subject: clocksource/drivers/sp804: Use clk_prepare_enable and clk_disable_unprepare Directly use clk_prepare_enable and clk_disable_unprepare. Signed-off-by: Kefeng Wang Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201029123317.90286-3-wangkefeng.wang@huawei.com --- drivers/clocksource/timer-sp804.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c index 22a68cb83cf3..d74788b47802 100644 --- a/drivers/clocksource/timer-sp804.c +++ b/drivers/clocksource/timer-sp804.c @@ -68,17 +68,9 @@ static long __init sp804_get_clock_rate(struct clk *clk, const char *name) return PTR_ERR(clk); } - err = clk_prepare(clk); - if (err) { - pr_err("sp804: clock failed to prepare: %d\n", err); - clk_put(clk); - return err; - } - - err = clk_enable(clk); + err = clk_prepare_enable(clk); if (err) { pr_err("sp804: clock failed to enable: %d\n", err); - clk_unprepare(clk); clk_put(clk); return err; } @@ -86,8 +78,7 @@ static long __init sp804_get_clock_rate(struct clk *clk, const char *name) rate = clk_get_rate(clk); if (rate < 0) { pr_err("sp804: clock failed to get rate: %ld\n", rate); - clk_disable(clk); - clk_unprepare(clk); + clk_disable_unprepare(clk); clk_put(clk); } -- cgit v1.2.3 From dca54f8ce1c3c979caf06cfdcdf8eab05a00f5ff Mon Sep 17 00:00:00 2001 From: Kefeng Wang Date: Thu, 29 Oct 2020 20:33:16 +0800 Subject: clocksource/drivers/sp804: Correct clk_get_rate handle clk_get_rate won't return negative value, correct clk_get_rate handle. Signed-off-by: Kefeng Wang Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201029123317.90286-4-wangkefeng.wang@huawei.com --- drivers/clocksource/timer-sp804.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c index d74788b47802..fcce839670cb 100644 --- a/drivers/clocksource/timer-sp804.c +++ b/drivers/clocksource/timer-sp804.c @@ -58,7 +58,6 @@ static struct sp804_clkevt sp804_clkevt[NR_TIMERS]; static long __init sp804_get_clock_rate(struct clk *clk, const char *name) { - long rate; int err; if (!clk) @@ -75,14 +74,7 @@ static long __init sp804_get_clock_rate(struct clk *clk, const char *name) return err; } - rate = clk_get_rate(clk); - if (rate < 0) { - pr_err("sp804: clock failed to get rate: %ld\n", rate); - clk_disable_unprepare(clk); - clk_put(clk); - } - - return rate; + return clk_get_rate(clk); } static struct sp804_clkevt * __init sp804_clkevt_get(void __iomem *base) -- cgit v1.2.3 From 19f7ce8e36c09f4a2491b065dabd9162018309b6 Mon Sep 17 00:00:00 2001 From: Kefeng Wang Date: Thu, 29 Oct 2020 20:33:17 +0800 Subject: clocksource/drivers/sp804: Use pr_fmt Add pr_fmt to prefix pr_ output. Signed-off-by: Kefeng Wang Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201029123317.90286-5-wangkefeng.wang@huawei.com --- drivers/clocksource/timer-sp804.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c index fcce839670cb..401d592e85f5 100644 --- a/drivers/clocksource/timer-sp804.c +++ b/drivers/clocksource/timer-sp804.c @@ -5,6 +5,9 @@ * Copyright (C) 1999 - 2003 ARM Limited * Copyright (C) 2000 Deep Blue Solutions Ltd */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -63,13 +66,13 @@ static long __init sp804_get_clock_rate(struct clk *clk, const char *name) if (!clk) clk = clk_get_sys("sp804", name); if (IS_ERR(clk)) { - pr_err("sp804: %s clock not found: %ld\n", name, PTR_ERR(clk)); + pr_err("%s clock not found: %ld\n", name, PTR_ERR(clk)); return PTR_ERR(clk); } err = clk_prepare_enable(clk); if (err) { - pr_err("sp804: clock failed to enable: %d\n", err); + pr_err("clock failed to enable: %d\n", err); clk_put(clk); return err; } @@ -218,7 +221,7 @@ static int __init sp804_clockevents_init(void __iomem *base, unsigned int irq, if (request_irq(irq, sp804_timer_interrupt, IRQF_TIMER | IRQF_IRQPOLL, "timer", &sp804_clockevent)) - pr_err("%s: request_irq() failed\n", "timer"); + pr_err("request_irq() failed\n"); clockevents_config_and_register(evt, rate, 0xf, 0xffffffff); return 0; @@ -280,7 +283,7 @@ static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time if (of_clk_get_parent_count(np) == 3) { clk2 = of_clk_get(np, 1); if (IS_ERR(clk2)) { - pr_err("sp804: %pOFn clock not found: %d\n", np, + pr_err("%pOFn clock not found: %d\n", np, (int)PTR_ERR(clk2)); clk2 = NULL; } -- cgit v1.2.3 From b6ea209ef124dad4045772a759e2aecd191534c0 Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Thu, 5 Nov 2020 13:22:08 -0800 Subject: clocksource/drivers/nps: Remove EZChip NPS clocksource driver NPS platform has been removed from ARC port and there are no in-tree users of it now. So RIP ! Cc: Daniel Lezcano Cc: Thomas Gleixner Cc: linux-kernel@vger.kernel.org Signed-off-by: Vineet Gupta Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201105212210.1891598-2-vgupta@synopsys.com --- drivers/clocksource/Kconfig | 10 -- drivers/clocksource/Makefile | 1 - drivers/clocksource/timer-nps.c | 284 ---------------------------------------- 3 files changed, 295 deletions(-) delete mode 100644 drivers/clocksource/timer-nps.c (limited to 'drivers') diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index 68b087bff59c..390c27cd926d 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -275,16 +275,6 @@ config CLKSRC_TI_32K This option enables support for Texas Instruments 32.768 Hz clocksource available on many OMAP-like platforms. -config CLKSRC_NPS - bool "NPS400 clocksource driver" if COMPILE_TEST - depends on !PHYS_ADDR_T_64BIT - select CLKSRC_MMIO - select TIMER_OF if OF - help - NPS400 clocksource support. - It has a 64-bit counter with update rate up to 1000MHz. - This counter is accessed via couple of 32-bit memory-mapped registers. - config CLKSRC_STM32 bool "Clocksource for STM32 SoCs" if !ARCH_STM32 depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST) diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile index 1c444cc3bb44..3c75cbbf8533 100644 --- a/drivers/clocksource/Makefile +++ b/drivers/clocksource/Makefile @@ -56,7 +56,6 @@ obj-$(CONFIG_CLKSRC_QCOM) += timer-qcom.o obj-$(CONFIG_MTK_TIMER) += timer-mediatek.o obj-$(CONFIG_CLKSRC_PISTACHIO) += timer-pistachio.o obj-$(CONFIG_CLKSRC_TI_32K) += timer-ti-32k.o -obj-$(CONFIG_CLKSRC_NPS) += timer-nps.o obj-$(CONFIG_OXNAS_RPS_TIMER) += timer-oxnas-rps.o obj-$(CONFIG_OWL_TIMER) += timer-owl.o obj-$(CONFIG_MILBEAUT_TIMER) += timer-milbeaut.o diff --git a/drivers/clocksource/timer-nps.c b/drivers/clocksource/timer-nps.c deleted file mode 100644 index 7b6bb0df96ae..000000000000 --- a/drivers/clocksource/timer-nps.c +++ /dev/null @@ -1,284 +0,0 @@ -/* - * Copyright (c) 2016, Mellanox Technologies. All rights reserved. - * - * This software is available to you under a choice of one of two - * licenses. You may choose to be licensed under the terms of the GNU - * General Public License (GPL) Version 2, available from the file - * COPYING in the main directory of this source tree, or the - * OpenIB.org BSD license below: - * - * Redistribution and use in source and binary forms, with or - * without modification, are permitted provided that the following - * conditions are met: - * - * - Redistributions of source code must retain the above - * copyright notice, this list of conditions and the following - * disclaimer. - * - * - Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following - * disclaimer in the documentation and/or other materials - * provided with the distribution. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF - * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND - * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS - * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN - * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN - * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ - -#include -#include -#include -#include -#include -#include -#include -#include - -#define NPS_MSU_TICK_LOW 0xC8 -#define NPS_CLUSTER_OFFSET 8 -#define NPS_CLUSTER_NUM 16 - -/* This array is per cluster of CPUs (Each NPS400 cluster got 256 CPUs) */ -static void *nps_msu_reg_low_addr[NPS_CLUSTER_NUM] __read_mostly; - -static int __init nps_get_timer_clk(struct device_node *node, - unsigned long *timer_freq, - struct clk **clk) -{ - int ret; - - *clk = of_clk_get(node, 0); - ret = PTR_ERR_OR_ZERO(*clk); - if (ret) { - pr_err("timer missing clk\n"); - return ret; - } - - ret = clk_prepare_enable(*clk); - if (ret) { - pr_err("Couldn't enable parent clk\n"); - clk_put(*clk); - return ret; - } - - *timer_freq = clk_get_rate(*clk); - if (!(*timer_freq)) { - pr_err("Couldn't get clk rate\n"); - clk_disable_unprepare(*clk); - clk_put(*clk); - return -EINVAL; - } - - return 0; -} - -static u64 nps_clksrc_read(struct clocksource *clksrc) -{ - int cluster = raw_smp_processor_id() >> NPS_CLUSTER_OFFSET; - - return (u64)ioread32be(nps_msu_reg_low_addr[cluster]); -} - -static int __init nps_setup_clocksource(struct device_node *node) -{ - int ret, cluster; - struct clk *clk; - unsigned long nps_timer1_freq; - - - for (cluster = 0; cluster < NPS_CLUSTER_NUM; cluster++) - nps_msu_reg_low_addr[cluster] = - nps_host_reg((cluster << NPS_CLUSTER_OFFSET), - NPS_MSU_BLKID, NPS_MSU_TICK_LOW); - - ret = nps_get_timer_clk(node, &nps_timer1_freq, &clk); - if (ret) - return ret; - - ret = clocksource_mmio_init(nps_msu_reg_low_addr, "nps-tick", - nps_timer1_freq, 300, 32, nps_clksrc_read); - if (ret) { - pr_err("Couldn't register clock source.\n"); - clk_disable_unprepare(clk); - } - - return ret; -} - -TIMER_OF_DECLARE(ezchip_nps400_clksrc, "ezchip,nps400-timer", - nps_setup_clocksource); -TIMER_OF_DECLARE(ezchip_nps400_clk_src, "ezchip,nps400-timer1", - nps_setup_clocksource); - -#ifdef CONFIG_EZNPS_MTM_EXT -#include - -/* Timer related Aux registers */ -#define NPS_REG_TIMER0_TSI 0xFFFFF850 -#define NPS_REG_TIMER0_LIMIT 0x23 -#define NPS_REG_TIMER0_CTRL 0x22 -#define NPS_REG_TIMER0_CNT 0x21 - -/* - * Interrupt Enabled (IE) - re-arm the timer - * Not Halted (NH) - is cleared when working with JTAG (for debug) - */ -#define TIMER0_CTRL_IE BIT(0) -#define TIMER0_CTRL_NH BIT(1) - -static unsigned long nps_timer0_freq; -static unsigned long nps_timer0_irq; - -static void nps_clkevent_rm_thread(void) -{ - int thread; - unsigned int cflags, enabled_threads; - - hw_schd_save(&cflags); - - enabled_threads = read_aux_reg(NPS_REG_TIMER0_TSI); - - /* remove thread from TSI1 */ - thread = read_aux_reg(CTOP_AUX_THREAD_ID); - enabled_threads &= ~(1 << thread); - write_aux_reg(NPS_REG_TIMER0_TSI, enabled_threads); - - /* Acknowledge and if needed re-arm the timer */ - if (!enabled_threads) - write_aux_reg(NPS_REG_TIMER0_CTRL, TIMER0_CTRL_NH); - else - write_aux_reg(NPS_REG_TIMER0_CTRL, - TIMER0_CTRL_IE | TIMER0_CTRL_NH); - - hw_schd_restore(cflags); -} - -static void nps_clkevent_add_thread(unsigned long delta) -{ - int thread; - unsigned int cflags, enabled_threads; - - hw_schd_save(&cflags); - - /* add thread to TSI1 */ - thread = read_aux_reg(CTOP_AUX_THREAD_ID); - enabled_threads = read_aux_reg(NPS_REG_TIMER0_TSI); - enabled_threads |= (1 << thread); - write_aux_reg(NPS_REG_TIMER0_TSI, enabled_threads); - - /* set next timer event */ - write_aux_reg(NPS_REG_TIMER0_LIMIT, delta); - write_aux_reg(NPS_REG_TIMER0_CNT, 0); - write_aux_reg(NPS_REG_TIMER0_CTRL, - TIMER0_CTRL_IE | TIMER0_CTRL_NH); - - hw_schd_restore(cflags); -} - -/* - * Whenever anyone tries to change modes, we just mask interrupts - * and wait for the next event to get set. - */ -static int nps_clkevent_set_state(struct clock_event_device *dev) -{ - nps_clkevent_rm_thread(); - disable_percpu_irq(nps_timer0_irq); - - return 0; -} - -static int nps_clkevent_set_next_event(unsigned long delta, - struct clock_event_device *dev) -{ - nps_clkevent_add_thread(delta); - enable_percpu_irq(nps_timer0_irq, IRQ_TYPE_NONE); - - return 0; -} - -static DEFINE_PER_CPU(struct clock_event_device, nps_clockevent_device) = { - .name = "NPS Timer0", - .features = CLOCK_EVT_FEAT_ONESHOT, - .rating = 300, - .set_next_event = nps_clkevent_set_next_event, - .set_state_oneshot = nps_clkevent_set_state, - .set_state_oneshot_stopped = nps_clkevent_set_state, - .set_state_shutdown = nps_clkevent_set_state, - .tick_resume = nps_clkevent_set_state, -}; - -static irqreturn_t timer_irq_handler(int irq, void *dev_id) -{ - struct clock_event_device *evt = dev_id; - - nps_clkevent_rm_thread(); - evt->event_handler(evt); - - return IRQ_HANDLED; -} - -static int nps_timer_starting_cpu(unsigned int cpu) -{ - struct clock_event_device *evt = this_cpu_ptr(&nps_clockevent_device); - - evt->cpumask = cpumask_of(smp_processor_id()); - - clockevents_config_and_register(evt, nps_timer0_freq, 0, ULONG_MAX); - enable_percpu_irq(nps_timer0_irq, IRQ_TYPE_NONE); - - return 0; -} - -static int nps_timer_dying_cpu(unsigned int cpu) -{ - disable_percpu_irq(nps_timer0_irq); - return 0; -} - -static int __init nps_setup_clockevent(struct device_node *node) -{ - struct clk *clk; - int ret; - - nps_timer0_irq = irq_of_parse_and_map(node, 0); - if (nps_timer0_irq <= 0) { - pr_err("clockevent: missing irq\n"); - return -EINVAL; - } - - ret = nps_get_timer_clk(node, &nps_timer0_freq, &clk); - if (ret) - return ret; - - /* Needs apriori irq_set_percpu_devid() done in intc map function */ - ret = request_percpu_irq(nps_timer0_irq, timer_irq_handler, - "Timer0 (per-cpu-tick)", - &nps_clockevent_device); - if (ret) { - pr_err("Couldn't request irq\n"); - clk_disable_unprepare(clk); - return ret; - } - - ret = cpuhp_setup_state(CPUHP_AP_ARC_TIMER_STARTING, - "clockevents/nps:starting", - nps_timer_starting_cpu, - nps_timer_dying_cpu); - if (ret) { - pr_err("Failed to setup hotplug state\n"); - clk_disable_unprepare(clk); - free_percpu_irq(nps_timer0_irq, &nps_clockevent_device); - return ret; - } - - return 0; -} - -TIMER_OF_DECLARE(ezchip_nps400_clk_evt, "ezchip,nps400-timer0", - nps_setup_clockevent); -#endif /* CONFIG_EZNPS_MTM_EXT */ -- cgit v1.2.3 From c1e6cad00aa2f17845e7270e38ff3cc82c7b022a Mon Sep 17 00:00:00 2001 From: Yang Yingliang Date: Wed, 11 Nov 2020 14:47:06 +0800 Subject: clocksource/drivers/orion: Add missing clk_disable_unprepare() on error path After calling clk_prepare_enable(), clk_disable_unprepare() need be called on error path. Fixes: fbe4b3566ddc ("clocksource/drivers/orion: Convert init function...") Reported-by: Hulk Robot Signed-off-by: Yang Yingliang Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201111064706.3397156-1-yangyingliang@huawei.com --- drivers/clocksource/timer-orion.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/timer-orion.c b/drivers/clocksource/timer-orion.c index d01ff4181867..5101e834d78f 100644 --- a/drivers/clocksource/timer-orion.c +++ b/drivers/clocksource/timer-orion.c @@ -143,7 +143,8 @@ static int __init orion_timer_init(struct device_node *np) irq = irq_of_parse_and_map(np, 1); if (irq <= 0) { pr_err("%pOFn: unable to parse timer1 irq\n", np); - return -EINVAL; + ret = -EINVAL; + goto out_unprep_clk; } rate = clk_get_rate(clk); @@ -160,7 +161,7 @@ static int __init orion_timer_init(struct device_node *np) clocksource_mmio_readl_down); if (ret) { pr_err("Failed to initialize mmio timer\n"); - return ret; + goto out_unprep_clk; } sched_clock_register(orion_read_sched_clock, 32, rate); @@ -170,7 +171,7 @@ static int __init orion_timer_init(struct device_node *np) "orion_event", NULL); if (ret) { pr_err("%pOFn: unable to setup irq\n", np); - return ret; + goto out_unprep_clk; } ticks_per_jiffy = (clk_get_rate(clk) + HZ/2) / HZ; @@ -183,5 +184,9 @@ static int __init orion_timer_init(struct device_node *np) orion_delay_timer_init(rate); return 0; + +out_unprep_clk: + clk_disable_unprepare(clk); + return ret; } TIMER_OF_DECLARE(orion_timer, "marvell,orion-timer", orion_timer_init); -- cgit v1.2.3 From eee422c46e6840a81c9db18a497b74387a557b29 Mon Sep 17 00:00:00 2001 From: Yu Kuai Date: Mon, 16 Nov 2020 21:51:23 +0800 Subject: clocksource/drivers/cadence_ttc: Fix memory leak in ttc_setup_clockevent() If clk_notifier_register() failed, ttc_setup_clockevent() will return without freeing 'ttcce', which will leak memory. Fixes: 70504f311d4b ("clocksource/drivers/cadence_ttc: Convert init function to return error") Reported-by: Hulk Robot Signed-off-by: Yu Kuai Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201116135123.2164033-1-yukuai3@huawei.com --- drivers/clocksource/timer-cadence-ttc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/timer-cadence-ttc.c b/drivers/clocksource/timer-cadence-ttc.c index 80e960602030..4efd0cf3b602 100644 --- a/drivers/clocksource/timer-cadence-ttc.c +++ b/drivers/clocksource/timer-cadence-ttc.c @@ -413,10 +413,8 @@ static int __init ttc_setup_clockevent(struct clk *clk, ttcce->ttc.clk = clk; err = clk_prepare_enable(ttcce->ttc.clk); - if (err) { - kfree(ttcce); - return err; - } + if (err) + goto out_kfree; ttcce->ttc.clk_rate_change_nb.notifier_call = ttc_rate_change_clockevent_cb; @@ -426,7 +424,7 @@ static int __init ttc_setup_clockevent(struct clk *clk, &ttcce->ttc.clk_rate_change_nb); if (err) { pr_warn("Unable to register clock notifier.\n"); - return err; + goto out_kfree; } ttcce->ttc.freq = clk_get_rate(ttcce->ttc.clk); @@ -455,15 +453,17 @@ static int __init ttc_setup_clockevent(struct clk *clk, err = request_irq(irq, ttc_clock_event_interrupt, IRQF_TIMER, ttcce->ce.name, ttcce); - if (err) { - kfree(ttcce); - return err; - } + if (err) + goto out_kfree; clockevents_config_and_register(&ttcce->ce, ttcce->ttc.freq / PRESCALE, 1, 0xfffe); return 0; + +out_kfree: + kfree(ttcce); + return err; } static int __init ttc_timer_probe(struct platform_device *pdev) -- cgit v1.2.3 From 5bd7cb29eceb52e4b108917786fdbf2a2c2048ef Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Wed, 25 Nov 2020 11:23:45 +0100 Subject: clocksource/drivers/ingenic: Fix section mismatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function ingenic_tcu_get_clock() is annotated for the __init section but it is actually called from the online cpu callback. That will lead to a crash if a CPU is hotplugged after boot time. Remove the __init annotation for the ingenic_tcu_get_clock() function. Fixes: f19d838d08fc (clocksource/drivers/ingenic: Add high resolution timer support for SMP/SMT) Reported-by: kernel test robot Signed-off-by: Daniel Lezcano Reviewed-by: Paul Cercueil Tested-by: 周琰杰 (Zhou Yanjie) Link: https://lore.kernel.org/r/20201125102346.1816310-1-daniel.lezcano@linaro.org --- drivers/clocksource/ingenic-timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/clocksource/ingenic-timer.c b/drivers/clocksource/ingenic-timer.c index 58fd9189fab7..905fd6b163a8 100644 --- a/drivers/clocksource/ingenic-timer.c +++ b/drivers/clocksource/ingenic-timer.c @@ -127,7 +127,7 @@ static irqreturn_t ingenic_tcu_cevt_cb(int irq, void *dev_id) return IRQ_HANDLED; } -static struct clk * __init ingenic_tcu_get_clock(struct device_node *np, int id) +static struct clk *ingenic_tcu_get_clock(struct device_node *np, int id) { struct of_phandle_args args; -- cgit v1.2.3 From ab3105446f1ec4e98fadfc998ee24feec271c16c Mon Sep 17 00:00:00 2001 From: Kefeng Wang Date: Wed, 28 Oct 2020 21:12:30 +0800 Subject: clocksource/drivers/riscv: Make RISCV_TIMER depends on RISCV_SBI The riscv timer is set via SBI timer call, let's make RISCV_TIMER depends on RISCV_SBI, and it also fixes some build issue. Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation for M-mode systems") Signed-off-by: Kefeng Wang Reviewed-by: Palmer Dabbelt Acked-by: Palmer Dabbelt Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201028131230.72907-1-wangkefeng.wang@huawei.com --- drivers/clocksource/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index 390c27cd926d..9f00b8385fd4 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -644,7 +644,7 @@ config ATCPIT100_TIMER config RISCV_TIMER bool "Timer for the RISC-V platform" if COMPILE_TEST - depends on GENERIC_SCHED_CLOCK && RISCV + depends on GENERIC_SCHED_CLOCK && RISCV && RISCV_SBI select TIMER_PROBE select TIMER_OF help -- cgit v1.2.3 From 5d9814df0aec56a638bbf20795abb4cfaf3cd331 Mon Sep 17 00:00:00 2001 From: Dinh Nguyen Date: Sat, 5 Dec 2020 04:52:23 -0600 Subject: clocksource/drivers/dw_apb_timer_of: Add error handling if no clock available commit ("b0fc70ce1f02 arm64: berlin: Select DW_APB_TIMER_OF") added the support for the dw_apb_timer into the arm64 defconfig. However, for some platforms like the Intel Stratix10 and Agilex, the clock manager doesn't get loaded until after the timer driver get loaded. Thus, the driver hits the panic "No clock nor clock-frequency property for" because it cannot properly get the clock. This patch adds the error handling needed for the timer driver so that the kernel can continue booting instead of just hitting the panic. Signed-off-by: Dinh Nguyen Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201205105223.208604-1-dinguyen@kernel.org --- drivers/clocksource/dw_apb_timer_of.c | 57 ++++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 18 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c index ab3ddebe8344..42e7e43b8fcd 100644 --- a/drivers/clocksource/dw_apb_timer_of.c +++ b/drivers/clocksource/dw_apb_timer_of.c @@ -14,12 +14,13 @@ #include #include -static void __init timer_get_base_and_rate(struct device_node *np, +static int __init timer_get_base_and_rate(struct device_node *np, void __iomem **base, u32 *rate) { struct clk *timer_clk; struct clk *pclk; struct reset_control *rstc; + int ret; *base = of_iomap(np, 0); @@ -46,55 +47,67 @@ static void __init timer_get_base_and_rate(struct device_node *np, pr_warn("pclk for %pOFn is present, but could not be activated\n", np); + if (!of_property_read_u32(np, "clock-freq", rate) && + !of_property_read_u32(np, "clock-frequency", rate)) + return 0; + timer_clk = of_clk_get_by_name(np, "timer"); if (IS_ERR(timer_clk)) - goto try_clock_freq; + return PTR_ERR(timer_clk); - if (!clk_prepare_enable(timer_clk)) { - *rate = clk_get_rate(timer_clk); - return; - } + ret = clk_prepare_enable(timer_clk); + if (ret) + return ret; + + *rate = clk_get_rate(timer_clk); + if (!(*rate)) + return -EINVAL; -try_clock_freq: - if (of_property_read_u32(np, "clock-freq", rate) && - of_property_read_u32(np, "clock-frequency", rate)) - panic("No clock nor clock-frequency property for %pOFn", np); + return 0; } -static void __init add_clockevent(struct device_node *event_timer) +static int __init add_clockevent(struct device_node *event_timer) { void __iomem *iobase; struct dw_apb_clock_event_device *ced; u32 irq, rate; + int ret = 0; irq = irq_of_parse_and_map(event_timer, 0); if (irq == 0) panic("No IRQ for clock event timer"); - timer_get_base_and_rate(event_timer, &iobase, &rate); + ret = timer_get_base_and_rate(event_timer, &iobase, &rate); + if (ret) + return ret; ced = dw_apb_clockevent_init(-1, event_timer->name, 300, iobase, irq, rate); if (!ced) - panic("Unable to initialise clockevent device"); + return -EINVAL; dw_apb_clockevent_register(ced); + + return 0; } static void __iomem *sched_io_base; static u32 sched_rate; -static void __init add_clocksource(struct device_node *source_timer) +static int __init add_clocksource(struct device_node *source_timer) { void __iomem *iobase; struct dw_apb_clocksource *cs; u32 rate; + int ret; - timer_get_base_and_rate(source_timer, &iobase, &rate); + ret = timer_get_base_and_rate(source_timer, &iobase, &rate); + if (ret) + return ret; cs = dw_apb_clocksource_init(300, source_timer->name, iobase, rate); if (!cs) - panic("Unable to initialise clocksource device"); + return -EINVAL; dw_apb_clocksource_start(cs); dw_apb_clocksource_register(cs); @@ -106,6 +119,8 @@ static void __init add_clocksource(struct device_node *source_timer) */ sched_io_base = iobase + 0x04; sched_rate = rate; + + return 0; } static u64 notrace read_sched_clock(void) @@ -146,10 +161,14 @@ static struct delay_timer dw_apb_delay_timer = { static int num_called; static int __init dw_apb_timer_init(struct device_node *timer) { + int ret = 0; + switch (num_called) { case 1: pr_debug("%s: found clocksource timer\n", __func__); - add_clocksource(timer); + ret = add_clocksource(timer); + if (ret) + return ret; init_sched_clock(); #ifdef CONFIG_ARM dw_apb_delay_timer.freq = sched_rate; @@ -158,7 +177,9 @@ static int __init dw_apb_timer_init(struct device_node *timer) break; default: pr_debug("%s: found clockevent timer\n", __func__); - add_clockevent(timer); + ret = add_clockevent(timer); + if (ret) + return ret; break; } -- cgit v1.2.3 From d8cc3905b8073c7cfbff94af889fa8dc71f21dd5 Mon Sep 17 00:00:00 2001 From: Keqian Zhu Date: Fri, 4 Dec 2020 15:31:25 +0800 Subject: clocksource/drivers/arm_arch_timer: Use stable count reader in erratum sne In commit 0ea415390cd3 ("clocksource/arm_arch_timer: Use arch_timer_read_counter to access stable counters"), we separate stable and normal count reader to omit unnecessary overhead on systems that have no timer erratum. However, in erratum_set_next_event_tval_generic(), count reader becomes normal reader. This converts it to stable reader. Fixes: 0ea415390cd3 ("clocksource/arm_arch_timer: Use arch_timer_read_counter to access stable counters") Acked-by: Marc Zyngier Signed-off-by: Keqian Zhu Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201204073126.6920-2-zhukeqian1@huawei.com --- drivers/clocksource/arm_arch_timer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 6c3e84180146..777d38cb39b0 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -396,10 +396,10 @@ static void erratum_set_next_event_tval_generic(const int access, unsigned long ctrl &= ~ARCH_TIMER_CTRL_IT_MASK; if (access == ARCH_TIMER_PHYS_ACCESS) { - cval = evt + arch_counter_get_cntpct(); + cval = evt + arch_counter_get_cntpct_stable(); write_sysreg(cval, cntp_cval_el0); } else { - cval = evt + arch_counter_get_cntvct(); + cval = evt + arch_counter_get_cntvct_stable(); write_sysreg(cval, cntv_cval_el0); } -- cgit v1.2.3 From 8b7770b877d187bfdae1eaf587bd2b792479a31c Mon Sep 17 00:00:00 2001 From: Keqian Zhu Date: Fri, 4 Dec 2020 15:31:26 +0800 Subject: clocksource/drivers/arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI ARM virtual counter supports event stream, it can only trigger an event when the trigger bit (the value of CNTKCTL_EL1.EVNTI) of CNTVCT_EL0 changes, so the actual period of event stream is 2^(cntkctl_evnti + 1). For example, when the trigger bit is 0, then virtual counter trigger an event for every two cycles. While we're at it, rework the way we compute the trigger bit position by making it more obvious that when bits [n:n-1] are both set (with n being the most significant bit), we pick bit (n + 1). Fixes: 037f637767a8 ("drivers: clocksource: add support for ARM architected timer event stream") Suggested-by: Marc Zyngier Signed-off-by: Keqian Zhu Acked-by: Marc Zyngier Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201204073126.6920-3-zhukeqian1@huawei.com --- drivers/clocksource/arm_arch_timer.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 777d38cb39b0..d0177824c518 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -822,15 +822,24 @@ static void arch_timer_evtstrm_enable(int divider) static void arch_timer_configure_evtstream(void) { - int evt_stream_div, pos; + int evt_stream_div, lsb; + + /* + * As the event stream can at most be generated at half the frequency + * of the counter, use half the frequency when computing the divider. + */ + evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ / 2; + + /* + * Find the closest power of two to the divisor. If the adjacent bit + * of lsb (last set bit, starts from 0) is set, then we use (lsb + 1). + */ + lsb = fls(evt_stream_div) - 1; + if (lsb > 0 && (evt_stream_div & BIT(lsb - 1))) + lsb++; - /* Find the closest power of two to the divisor */ - evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ; - pos = fls(evt_stream_div); - if (pos > 1 && !(evt_stream_div & (1 << (pos - 2)))) - pos--; /* enable event stream */ - arch_timer_evtstrm_enable(min(pos, 15)); + arch_timer_evtstrm_enable(max(0, min(lsb, 15))); } static void arch_counter_set_user_access(void) -- cgit v1.2.3 From 8ae954caf49ac403c177d117fb8e05cbc866aa3c Mon Sep 17 00:00:00 2001 From: Niklas Söderlund Date: Sat, 5 Dec 2020 03:19:20 +0100 Subject: clocksource/drivers/sh_cmt: Fix potential deadlock when calling runtime PM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ch->lock is used to protect the whole enable() and read() of sh_cmt's implementation of struct clocksource. The enable() implementation calls pm_runtime_get_sync() which may result in the clock source to be read() triggering a cyclic lockdep warning for the ch->lock. The sh_cmt driver implement its own balancing of calls to sh_cmt_{enable,disable}() with flags in sh_cmt_{start,stop}(). It does this to deal with that start and stop are shared between the clock source and clock event providers. While this could be improved on verifying corner cases based on any substantial rework on all devices this driver supports might prove hard. As a first step separate the PM handling for clock event and clock source. Always put/get the device when enabling/disabling the clock source but keep the clock event logic unchanged. This allows the sh_cmt implementation of struct clocksource to call PM without holding the ch->lock and avoiding the deadlock. Triggering and log of the deadlock warning, # echo e60f0000.timer > /sys/devices/system/clocksource/clocksource0/current_clocksource [ 46.948370] ====================================================== [ 46.954730] WARNING: possible circular locking dependency detected [ 46.961094] 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 Not tainted [ 46.967985] ------------------------------------------------------ [ 46.974342] migration/0/11 is trying to acquire lock: [ 46.979543] ffff0000403ed220 (&dev->power.lock){-...}-{2:2}, at: __pm_runtime_resume+0x40/0x74 [ 46.988445] [ 46.988445] but task is already holding lock: [ 46.994441] ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.002173] [ 47.002173] which lock already depends on the new lock. [ 47.002173] [ 47.010573] [ 47.010573] the existing dependency chain (in reverse order) is: [ 47.018262] [ 47.018262] -> #3 (&ch->lock){....}-{2:2}: [ 47.024033] lock_acquire.part.0+0x120/0x330 [ 47.028970] lock_acquire+0x64/0x80 [ 47.033105] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.038130] sh_cmt_start+0x28/0x210 [ 47.042352] sh_cmt_clocksource_enable+0x28/0x50 [ 47.047644] change_clocksource+0x9c/0x160 [ 47.052402] multi_cpu_stop+0xa4/0x190 [ 47.056799] cpu_stopper_thread+0x90/0x154 [ 47.061557] smpboot_thread_fn+0x244/0x270 [ 47.066310] kthread+0x154/0x160 [ 47.070175] ret_from_fork+0x10/0x20 [ 47.074390] [ 47.074390] -> #2 (tk_core.seq.seqcount){----}-{0:0}: [ 47.081136] lock_acquire.part.0+0x120/0x330 [ 47.086070] lock_acquire+0x64/0x80 [ 47.090203] seqcount_lockdep_reader_access.constprop.0+0x74/0x100 [ 47.097096] ktime_get+0x28/0xa0 [ 47.100960] hrtimer_start_range_ns+0x210/0x2dc [ 47.106164] generic_sched_clock_init+0x70/0x88 [ 47.111364] sched_clock_init+0x40/0x64 [ 47.115853] start_kernel+0x494/0x524 [ 47.120156] [ 47.120156] -> #1 (hrtimer_bases.lock){-.-.}-{2:2}: [ 47.126721] lock_acquire.part.0+0x120/0x330 [ 47.136042] lock_acquire+0x64/0x80 [ 47.144461] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.153721] hrtimer_start_range_ns+0x68/0x2dc [ 47.163054] rpm_suspend+0x308/0x5dc [ 47.171473] rpm_idle+0xc4/0x2a4 [ 47.179550] pm_runtime_work+0x98/0xc0 [ 47.188209] process_one_work+0x294/0x6f0 [ 47.197142] worker_thread+0x70/0x45c [ 47.205661] kthread+0x154/0x160 [ 47.213673] ret_from_fork+0x10/0x20 [ 47.221957] [ 47.221957] -> #0 (&dev->power.lock){-...}-{2:2}: [ 47.236292] check_noncircular+0x128/0x140 [ 47.244907] __lock_acquire+0x13b0/0x204c [ 47.253332] lock_acquire.part.0+0x120/0x330 [ 47.262033] lock_acquire+0x64/0x80 [ 47.269826] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.278430] __pm_runtime_resume+0x40/0x74 [ 47.286758] sh_cmt_start+0x84/0x210 [ 47.294537] sh_cmt_clocksource_enable+0x28/0x50 [ 47.303449] change_clocksource+0x9c/0x160 [ 47.311783] multi_cpu_stop+0xa4/0x190 [ 47.319720] cpu_stopper_thread+0x90/0x154 [ 47.328022] smpboot_thread_fn+0x244/0x270 [ 47.336298] kthread+0x154/0x160 [ 47.343708] ret_from_fork+0x10/0x20 [ 47.351445] [ 47.351445] other info that might help us debug this: [ 47.351445] [ 47.370225] Chain exists of: [ 47.370225] &dev->power.lock --> tk_core.seq.seqcount --> &ch->lock [ 47.370225] [ 47.392003] Possible unsafe locking scenario: [ 47.392003] [ 47.405314] CPU0 CPU1 [ 47.413569] ---- ---- [ 47.421768] lock(&ch->lock); [ 47.428425] lock(tk_core.seq.seqcount); [ 47.438701] lock(&ch->lock); [ 47.447930] lock(&dev->power.lock); [ 47.455172] [ 47.455172] *** DEADLOCK *** [ 47.455172] [ 47.471433] 3 locks held by migration/0/11: [ 47.479099] #0: ffff8000113c9278 (timekeeper_lock){-.-.}-{2:2}, at: change_clocksource+0x2c/0x160 [ 47.491834] #1: ffff8000113c8f88 (tk_core.seq.seqcount){----}-{0:0}, at: multi_cpu_stop+0xa4/0x190 [ 47.504727] #2: ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.516541] [ 47.516541] stack backtrace: [ 47.528480] CPU: 0 PID: 11 Comm: migration/0 Not tainted 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 [ 47.542147] Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT) [ 47.554241] Call trace: [ 47.560832] dump_backtrace+0x0/0x190 [ 47.568670] show_stack+0x14/0x30 [ 47.576144] dump_stack+0xe8/0x130 [ 47.583670] print_circular_bug+0x1f0/0x200 [ 47.592015] check_noncircular+0x128/0x140 [ 47.600289] __lock_acquire+0x13b0/0x204c [ 47.608486] lock_acquire.part.0+0x120/0x330 [ 47.616953] lock_acquire+0x64/0x80 [ 47.624582] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.633114] __pm_runtime_resume+0x40/0x74 [ 47.641371] sh_cmt_start+0x84/0x210 [ 47.649115] sh_cmt_clocksource_enable+0x28/0x50 [ 47.657916] change_clocksource+0x9c/0x160 [ 47.666165] multi_cpu_stop+0xa4/0x190 [ 47.674056] cpu_stopper_thread+0x90/0x154 [ 47.682308] smpboot_thread_fn+0x244/0x270 [ 47.690560] kthread+0x154/0x160 [ 47.697927] ret_from_fork+0x10/0x20 [ 47.708447] clocksource: Switched to clocksource e60f0000.timer Signed-off-by: Niklas Söderlund Reviewed-by: Geert Uytterhoeven Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201205021921.1456190-2-niklas.soderlund+renesas@ragnatech.se --- drivers/clocksource/sh_cmt.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/sh_cmt.c b/drivers/clocksource/sh_cmt.c index 760777458a90..19fa3ef75e3b 100644 --- a/drivers/clocksource/sh_cmt.c +++ b/drivers/clocksource/sh_cmt.c @@ -319,7 +319,6 @@ static int sh_cmt_enable(struct sh_cmt_channel *ch) { int k, ret; - pm_runtime_get_sync(&ch->cmt->pdev->dev); dev_pm_syscore_device(&ch->cmt->pdev->dev, true); /* enable clock */ @@ -394,7 +393,6 @@ static void sh_cmt_disable(struct sh_cmt_channel *ch) clk_disable(ch->cmt->clk); dev_pm_syscore_device(&ch->cmt->pdev->dev, false); - pm_runtime_put(&ch->cmt->pdev->dev); } /* private flags */ @@ -562,10 +560,16 @@ static int sh_cmt_start(struct sh_cmt_channel *ch, unsigned long flag) int ret = 0; unsigned long flags; + if (flag & FLAG_CLOCKSOURCE) + pm_runtime_get_sync(&ch->cmt->pdev->dev); + raw_spin_lock_irqsave(&ch->lock, flags); - if (!(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) + if (!(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) { + if (flag & FLAG_CLOCKEVENT) + pm_runtime_get_sync(&ch->cmt->pdev->dev); ret = sh_cmt_enable(ch); + } if (ret) goto out; @@ -590,14 +594,20 @@ static void sh_cmt_stop(struct sh_cmt_channel *ch, unsigned long flag) f = ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE); ch->flags &= ~flag; - if (f && !(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) + if (f && !(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) { sh_cmt_disable(ch); + if (flag & FLAG_CLOCKEVENT) + pm_runtime_put(&ch->cmt->pdev->dev); + } /* adjust the timeout to maximum if only clocksource left */ if ((flag == FLAG_CLOCKEVENT) && (ch->flags & FLAG_CLOCKSOURCE)) __sh_cmt_set_next(ch, ch->max_match_value); raw_spin_unlock_irqrestore(&ch->lock, flags); + + if (flag & FLAG_CLOCKSOURCE) + pm_runtime_put(&ch->cmt->pdev->dev); } static struct sh_cmt_channel *cs_to_sh_cmt(struct clocksource *cs) -- cgit v1.2.3 From 05a0302c35481e9b47fb90ba40922b0a4cae40d8 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 6 Dec 2020 22:46:14 +0100 Subject: rtc: mc146818: Prevent reading garbage The MC146818 driver is prone to read garbage from the RTC. There are several issues all related to the update cycle of the MC146818. The chip increments seconds obviously once per second and indicates that by a bit in a register. The bit goes high 244us before the actual update starts. During the update the readout of the time values is undefined. The code just checks whether the update in progress bit (UIP) is set before reading the clock. If it's set it waits arbitrary 20ms before retrying, which is ample because the maximum update time is ~2ms. But this check does not guarantee that the UIP bit goes high and the actual update happens during the readout. So the following can happen 0.997 UIP = False -> Interrupt/NMI/preemption 0.998 UIP -> True 0.999 Readout <- Undefined To prevent this rework the code so it checks UIP before and after the readout and if set after the readout try again. But that's not enough to cover the following: 0.997 UIP = False Readout seconds -> NMI (or vCPU scheduled out) 0.998 UIP -> True update completes UIP -> False 1.000 Readout minutes,.... UIP check succeeds That can make the readout wrong up to 59 seconds. To prevent this, read the seconds value before the first UIP check, validate it after checking UIP and after reading out the rest. It's amazing that the original i386 code had this actually correct and the generic implementation of the MC146818 driver got it wrong in 2002 and it stayed that way until today. Signed-off-by: Thomas Gleixner Acked-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201206220541.594826678@linutronix.de --- drivers/rtc/rtc-mc146818-lib.c | 64 +++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 25 deletions(-) (limited to 'drivers') diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c index 2ecd8752b088..98048bba1c14 100644 --- a/drivers/rtc/rtc-mc146818-lib.c +++ b/drivers/rtc/rtc-mc146818-lib.c @@ -8,41 +8,41 @@ #include #endif -/* - * Returns true if a clock update is in progress - */ -static inline unsigned char mc146818_is_updating(void) -{ - unsigned char uip; - unsigned long flags; - - spin_lock_irqsave(&rtc_lock, flags); - uip = (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP); - spin_unlock_irqrestore(&rtc_lock, flags); - return uip; -} - unsigned int mc146818_get_time(struct rtc_time *time) { unsigned char ctrl; unsigned long flags; unsigned char century = 0; + bool retry; #ifdef CONFIG_MACH_DECSTATION unsigned int real_year; #endif +again: + spin_lock_irqsave(&rtc_lock, flags); /* - * read RTC once any update in progress is done. The update - * can take just over 2ms. We wait 20ms. There is no need to - * to poll-wait (up to 1s - eeccch) for the falling edge of RTC_UIP. - * If you need to know *exactly* when a second has started, enable - * periodic update complete interrupts, (via ioctl) and then - * immediately read /dev/rtc which will block until you get the IRQ. - * Once the read clears, read the RTC time (again via ioctl). Easy. + * Check whether there is an update in progress during which the + * readout is unspecified. The maximum update time is ~2ms. Poll + * every msec for completion. + * + * Store the second value before checking UIP so a long lasting NMI + * which happens to hit after the UIP check cannot make an update + * cycle invisible. */ - if (mc146818_is_updating()) - mdelay(20); + time->tm_sec = CMOS_READ(RTC_SECONDS); + + if (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) { + spin_unlock_irqrestore(&rtc_lock, flags); + mdelay(1); + goto again; + } + + /* Revalidate the above readout */ + if (time->tm_sec != CMOS_READ(RTC_SECONDS)) { + spin_unlock_irqrestore(&rtc_lock, flags); + goto again; + } /* * Only the values that we read from the RTC are set. We leave @@ -50,8 +50,6 @@ unsigned int mc146818_get_time(struct rtc_time *time) * RTC has RTC_DAY_OF_WEEK, we ignore it, as it is only updated * by the RTC when initially set to a non-zero value. */ - spin_lock_irqsave(&rtc_lock, flags); - time->tm_sec = CMOS_READ(RTC_SECONDS); time->tm_min = CMOS_READ(RTC_MINUTES); time->tm_hour = CMOS_READ(RTC_HOURS); time->tm_mday = CMOS_READ(RTC_DAY_OF_MONTH); @@ -66,8 +64,24 @@ unsigned int mc146818_get_time(struct rtc_time *time) century = CMOS_READ(acpi_gbl_FADT.century); #endif ctrl = CMOS_READ(RTC_CONTROL); + /* + * Check for the UIP bit again. If it is set now then + * the above values may contain garbage. + */ + retry = CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP; + /* + * A NMI might have interrupted the above sequence so check whether + * the seconds value has changed which indicates that the NMI took + * longer than the UIP bit was set. Unlikely, but possible and + * there is also virt... + */ + retry |= time->tm_sec != CMOS_READ(RTC_SECONDS); + spin_unlock_irqrestore(&rtc_lock, flags); + if (retry) + goto again; + if (!(ctrl & RTC_DM_BINARY) || RTC_ALWAYS_BCD) { time->tm_sec = bcd2bin(time->tm_sec); -- cgit v1.2.3 From dcf257e92622ba0e25fdc4b6699683e7ae67e2a1 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 6 Dec 2020 22:46:15 +0100 Subject: rtc: mc146818: Reduce spinlock section in mc146818_set_time() No need to hold the lock and disable interrupts for doing math. Signed-off-by: Thomas Gleixner Acked-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201206220541.709243630@linutronix.de --- drivers/rtc/rtc-mc146818-lib.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c index 98048bba1c14..972a5b9a629d 100644 --- a/drivers/rtc/rtc-mc146818-lib.c +++ b/drivers/rtc/rtc-mc146818-lib.c @@ -135,7 +135,6 @@ int mc146818_set_time(struct rtc_time *time) if (yrs > 255) /* They are unsigned */ return -EINVAL; - spin_lock_irqsave(&rtc_lock, flags); #ifdef CONFIG_MACH_DECSTATION real_yrs = yrs; leap_yr = ((!((yrs + 1900) % 4) && ((yrs + 1900) % 100)) || @@ -164,10 +163,8 @@ int mc146818_set_time(struct rtc_time *time) /* These limits and adjustments are independent of * whether the chip is in binary mode or not. */ - if (yrs > 169) { - spin_unlock_irqrestore(&rtc_lock, flags); + if (yrs > 169) return -EINVAL; - } if (yrs >= 100) yrs -= 100; @@ -183,6 +180,7 @@ int mc146818_set_time(struct rtc_time *time) century = bin2bcd(century); } + spin_lock_irqsave(&rtc_lock, flags); save_control = CMOS_READ(RTC_CONTROL); CMOS_WRITE((save_control|RTC_SET), RTC_CONTROL); save_freq_select = CMOS_READ(RTC_FREQ_SELECT); -- cgit v1.2.3 From b0ecd8e8c5ef376777277c4c2db7de92ac59f23f Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 6 Dec 2020 22:46:16 +0100 Subject: rtc: cmos: Make rtc_cmos sync offset correct The offset for rtc_cmos must be -500ms to work correctly with the current implementation of rtc_set_ntp_time() due to the following: tsched twrite(t2.tv_sec - 1) t2 (seconds increment) twrite - tsched is the transport time for the write to hit the device, which is negligible for this chip because it's accessed directly. t2 - twrite = 500ms according to the datasheet. But rtc_set_ntp_time() calculation of tsched is: tsched = t2 - 1sec - (t2 - twrite) The default for the sync offset is 500ms which means that the write happens at t2 - 1.5 seconds which is obviously off by a second for this device. Make the offset -500ms so it works correct. Signed-off-by: Thomas Gleixner Reviewed-by: Jason Gunthorpe Acked-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201206220541.830517160@linutronix.de --- drivers/rtc/rtc-cmos.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers') diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c index c633319cdb91..7728faca01b4 100644 --- a/drivers/rtc/rtc-cmos.c +++ b/drivers/rtc/rtc-cmos.c @@ -868,6 +868,9 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq) if (retval) goto cleanup2; + /* Set the sync offset for the periodic 11min update correct */ + cmos_rtc.rtc->set_offset_nsec = -(NSEC_PER_SEC / 2); + /* export at least the first block of NVRAM */ nvmem_cfg.size = address_space - NVRAM_OFFSET; if (rtc_nvmem_register(cmos_rtc.rtc, &nvmem_cfg)) -- cgit v1.2.3 From 354c796b9270eb4780e59e3bdb83a3ae4930a832 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 6 Dec 2020 22:46:17 +0100 Subject: rtc: core: Make the sync offset default more realistic The offset which is used to steer the start of an RTC synchronization update via rtc_set_ntp_time() is huge. The math behind this is: tsched twrite(t2.tv_sec - 1) t2 (seconds increment) twrite - tsched is the transport time for the write to hit the device. t2 - twrite depends on the chip and is for most chips one second. The rtc_set_ntp_time() calculation of tsched is: tsched = t2 - 1sec - (t2 - twrite) The default for the sync offset is 500ms which means that twrite - tsched is 500ms assumed that t2 - twrite is one second. This is 0.5 seconds off for RTCs which are directly accessible by IO writes and probably for the majority of i2C/SPI based RTC off by an order of magnitude. Set it to 5ms which should bring it closer to reality. The default can be adjusted by drivers (rtc_cmos does so) and could be adjusted further by a calibration method which is an orthogonal problem. Signed-off-by: Thomas Gleixner Reviewed-by: Jason Gunthorpe Acked-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201206220541.960333166@linutronix.de --- drivers/rtc/class.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c index 7c88d190c51f..d7957376eb96 100644 --- a/drivers/rtc/class.c +++ b/drivers/rtc/class.c @@ -201,7 +201,7 @@ static struct rtc_device *rtc_allocate_device(void) device_initialize(&rtc->dev); /* Drivers can revise this default after allocating the device. */ - rtc->set_offset_nsec = NSEC_PER_SEC / 2; + rtc->set_offset_nsec = 5 * NSEC_PER_MSEC; rtc->irq_freq = 1; rtc->max_user_freq = 64; -- cgit v1.2.3 From 33e62e832384c8cb523044e0e9d99d7133f98e93 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 6 Dec 2020 22:46:19 +0100 Subject: ntp, rtc: Move rtc_set_ntp_time() to ntp code rtc_set_ntp_time() is not really RTC functionality as the code is just a user of RTC. Move it into the NTP code which allows further cleanups. Requested-by: Alexandre Belloni Signed-off-by: Thomas Gleixner Reviewed-by: Jason Gunthorpe Acked-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201206220542.166871172@linutronix.de --- drivers/rtc/Makefile | 1 - drivers/rtc/systohc.c | 61 ----------------------------------- include/linux/rtc.h | 34 -------------------- kernel/time/ntp.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 85 insertions(+), 99 deletions(-) delete mode 100644 drivers/rtc/systohc.c (limited to 'drivers') diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile index bfb57464118d..bb8f319b09fb 100644 --- a/drivers/rtc/Makefile +++ b/drivers/rtc/Makefile @@ -6,7 +6,6 @@ ccflags-$(CONFIG_RTC_DEBUG) := -DDEBUG obj-$(CONFIG_RTC_LIB) += lib.o -obj-$(CONFIG_RTC_SYSTOHC) += systohc.o obj-$(CONFIG_RTC_CLASS) += rtc-core.o obj-$(CONFIG_RTC_MC146818_LIB) += rtc-mc146818-lib.o rtc-core-y := class.o interface.o diff --git a/drivers/rtc/systohc.c b/drivers/rtc/systohc.c deleted file mode 100644 index 8b70f0520e13..000000000000 --- a/drivers/rtc/systohc.c +++ /dev/null @@ -1,61 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -#include -#include - -/** - * rtc_set_ntp_time - Save NTP synchronized time to the RTC - * @now: Current time of day - * @target_nsec: pointer for desired now->tv_nsec value - * - * Replacement for the NTP platform function update_persistent_clock64 - * that stores time for later retrieval by rtc_hctosys. - * - * Returns 0 on successful RTC update, -ENODEV if a RTC update is not - * possible at all, and various other -errno for specific temporary failure - * cases. - * - * -EPROTO is returned if now.tv_nsec is not close enough to *target_nsec. - * - * If temporary failure is indicated the caller should try again 'soon' - */ -int rtc_set_ntp_time(struct timespec64 now, unsigned long *target_nsec) -{ - struct rtc_device *rtc; - struct rtc_time tm; - struct timespec64 to_set; - int err = -ENODEV; - bool ok; - - rtc = rtc_class_open(CONFIG_RTC_SYSTOHC_DEVICE); - if (!rtc) - goto out_err; - - if (!rtc->ops || !rtc->ops->set_time) - goto out_close; - - /* Compute the value of tv_nsec we require the caller to supply in - * now.tv_nsec. This is the value such that (now + - * set_offset_nsec).tv_nsec == 0. - */ - set_normalized_timespec64(&to_set, 0, -rtc->set_offset_nsec); - *target_nsec = to_set.tv_nsec; - - /* The ntp code must call this with the correct value in tv_nsec, if - * it does not we update target_nsec and return EPROTO to make the ntp - * code try again later. - */ - ok = rtc_tv_nsec_ok(rtc->set_offset_nsec, &to_set, &now); - if (!ok) { - err = -EPROTO; - goto out_close; - } - - rtc_time64_to_tm(to_set.tv_sec, &tm); - - err = rtc_set_time(rtc, &tm); - -out_close: - rtc_class_close(rtc); -out_err: - return err; -} diff --git a/include/linux/rtc.h b/include/linux/rtc.h index 22d1575e4991..ff62680b48ca 100644 --- a/include/linux/rtc.h +++ b/include/linux/rtc.h @@ -165,7 +165,6 @@ int __rtc_register_device(struct module *owner, struct rtc_device *rtc); extern int rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm); extern int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm); -extern int rtc_set_ntp_time(struct timespec64 now, unsigned long *target_nsec); int __rtc_read_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm); extern int rtc_read_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alrm); @@ -205,39 +204,6 @@ static inline bool is_leap_year(unsigned int year) return (!(year % 4) && (year % 100)) || !(year % 400); } -/* Determine if we can call to driver to set the time. Drivers can only be - * called to set a second aligned time value, and the field set_offset_nsec - * specifies how far away from the second aligned time to call the driver. - * - * This also computes 'to_set' which is the time we are trying to set, and has - * a zero in tv_nsecs, such that: - * to_set - set_delay_nsec == now +/- FUZZ - * - */ -static inline bool rtc_tv_nsec_ok(s64 set_offset_nsec, - struct timespec64 *to_set, - const struct timespec64 *now) -{ - /* Allowed error in tv_nsec, arbitarily set to 5 jiffies in ns. */ - const unsigned long TIME_SET_NSEC_FUZZ = TICK_NSEC * 5; - struct timespec64 delay = {.tv_sec = 0, - .tv_nsec = set_offset_nsec}; - - *to_set = timespec64_add(*now, delay); - - if (to_set->tv_nsec < TIME_SET_NSEC_FUZZ) { - to_set->tv_nsec = 0; - return true; - } - - if (to_set->tv_nsec > NSEC_PER_SEC - TIME_SET_NSEC_FUZZ) { - to_set->tv_sec++; - to_set->tv_nsec = 0; - return true; - } - return false; -} - #define rtc_register_device(device) \ __rtc_register_device(THIS_MODULE, device) diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c index ff1a7b8ec4ef..84a554622cee 100644 --- a/kernel/time/ntp.c +++ b/kernel/time/ntp.c @@ -519,15 +519,94 @@ static void sched_sync_hw_clock(unsigned long offset_nsec, bool retry) hrtimer_start(&sync_hrtimer, exp, HRTIMER_MODE_ABS); } +/* + * Determine if we can call to driver to set the time. Drivers can only be + * called to set a second aligned time value, and the field set_offset_nsec + * specifies how far away from the second aligned time to call the driver. + * + * This also computes 'to_set' which is the time we are trying to set, and has + * a zero in tv_nsecs, such that: + * to_set - set_delay_nsec == now +/- FUZZ + * + */ +static inline bool rtc_tv_nsec_ok(long set_offset_nsec, + struct timespec64 *to_set, + const struct timespec64 *now) +{ + /* Allowed error in tv_nsec, arbitarily set to 5 jiffies in ns. */ + const unsigned long TIME_SET_NSEC_FUZZ = TICK_NSEC * 5; + struct timespec64 delay = {.tv_sec = 0, + .tv_nsec = set_offset_nsec}; + + *to_set = timespec64_add(*now, delay); + + if (to_set->tv_nsec < TIME_SET_NSEC_FUZZ) { + to_set->tv_nsec = 0; + return true; + } + + if (to_set->tv_nsec > NSEC_PER_SEC - TIME_SET_NSEC_FUZZ) { + to_set->tv_sec++; + to_set->tv_nsec = 0; + return true; + } + return false; +} + +#ifdef CONFIG_RTC_SYSTOHC +/* + * rtc_set_ntp_time - Save NTP synchronized time to the RTC + */ +static int rtc_set_ntp_time(struct timespec64 now, unsigned long *target_nsec) +{ + struct rtc_device *rtc; + struct rtc_time tm; + struct timespec64 to_set; + int err = -ENODEV; + bool ok; + + rtc = rtc_class_open(CONFIG_RTC_SYSTOHC_DEVICE); + if (!rtc) + goto out_err; + + if (!rtc->ops || !rtc->ops->set_time) + goto out_close; + + /* + * Compute the value of tv_nsec we require the caller to supply in + * now.tv_nsec. This is the value such that (now + + * set_offset_nsec).tv_nsec == 0. + */ + set_normalized_timespec64(&to_set, 0, -rtc->set_offset_nsec); + *target_nsec = to_set.tv_nsec; + + /* + * The ntp code must call this with the correct value in tv_nsec, if + * it does not we update target_nsec and return EPROTO to make the ntp + * code try again later. + */ + ok = rtc_tv_nsec_ok(rtc->set_offset_nsec, &to_set, &now); + if (!ok) { + err = -EPROTO; + goto out_close; + } + + rtc_time64_to_tm(to_set.tv_sec, &tm); + + err = rtc_set_time(rtc, &tm); + +out_close: + rtc_class_close(rtc); +out_err: + return err; +} + static void sync_rtc_clock(void) { unsigned long offset_nsec; struct timespec64 adjust; int rc; - if (!IS_ENABLED(CONFIG_RTC_SYSTOHC)) - return; - ktime_get_real_ts64(&adjust); if (persistent_clock_is_local) @@ -544,6 +623,9 @@ static void sync_rtc_clock(void) sched_sync_hw_clock(offset_nsec, rc != 0); } +#else +static inline void sync_rtc_clock(void) { } +#endif #ifdef CONFIG_GENERIC_CMOS_UPDATE int __weak update_persistent_clock64(struct timespec64 now64) -- cgit v1.2.3 From 69eca258c85000564577642ba28335eb4e1df8f0 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 6 Dec 2020 22:46:20 +0100 Subject: ntp: Make the RTC sync offset less obscure The current RTC set_offset_nsec value is not really intuitive to understand. tsched twrite(t2.tv_sec - 1) t2 (seconds increment) The offset is calculated from twrite based on the assumption that t2 - twrite == 1s. That means for the MC146818 RTC the offset needs to be negative so that the write happens 500ms before t2. It's easier to understand when the whole calculation is based on t2. That avoids negative offsets and the meaning is obvious: t2 - twrite: The time defined by the chip when seconds increment after the write. twrite - tsched: The time for the transport to the point where the chip is updated. ==> set_offset_nsec = t2 - tsched ttransport = twrite - tsched tRTCinc = t2 - twrite ==> set_offset_nsec = ttransport + tRTCinc tRTCinc is a chip property and can be obtained from the data sheet. ttransport depends on how the RTC is connected. It is close to 0 for directly accessible RTCs. For RTCs behind a slow bus, e.g. i2c, it's the time required to send the update over the bus. This can be estimated or even calibrated, but that's a different problem. Adjust the implementation and update comments accordingly. Signed-off-by: Thomas Gleixner Acked-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201206220542.263204937@linutronix.de --- drivers/rtc/class.c | 9 +++++++-- drivers/rtc/rtc-cmos.c | 2 +- include/linux/rtc.h | 35 +++++++++++++++++++++++++++++------ kernel/time/ntp.c | 47 ++++++++++++++++++++++++----------------------- 4 files changed, 61 insertions(+), 32 deletions(-) (limited to 'drivers') diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c index d7957376eb96..5855aa2eef62 100644 --- a/drivers/rtc/class.c +++ b/drivers/rtc/class.c @@ -200,8 +200,13 @@ static struct rtc_device *rtc_allocate_device(void) device_initialize(&rtc->dev); - /* Drivers can revise this default after allocating the device. */ - rtc->set_offset_nsec = 5 * NSEC_PER_MSEC; + /* + * Drivers can revise this default after allocating the device. + * The default is what most RTCs do: Increment seconds exactly one + * second after the write happened. This adds a default transport + * time of 5ms which is at least halfways close to reality. + */ + rtc->set_offset_nsec = NSEC_PER_SEC + 5 * NSEC_PER_MSEC; rtc->irq_freq = 1; rtc->max_user_freq = 64; diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c index 7728faca01b4..c5bcd2adc9fe 100644 --- a/drivers/rtc/rtc-cmos.c +++ b/drivers/rtc/rtc-cmos.c @@ -869,7 +869,7 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq) goto cleanup2; /* Set the sync offset for the periodic 11min update correct */ - cmos_rtc.rtc->set_offset_nsec = -(NSEC_PER_SEC / 2); + cmos_rtc.rtc->set_offset_nsec = NSEC_PER_SEC / 2; /* export at least the first block of NVRAM */ nvmem_cfg.size = address_space - NVRAM_OFFSET; diff --git a/include/linux/rtc.h b/include/linux/rtc.h index ff62680b48ca..b829382de6c3 100644 --- a/include/linux/rtc.h +++ b/include/linux/rtc.h @@ -110,13 +110,36 @@ struct rtc_device { /* Some hardware can't support UIE mode */ int uie_unsupported; - /* Number of nsec it takes to set the RTC clock. This influences when - * the set ops are called. An offset: - * - of 0.5 s will call RTC set for wall clock time 10.0 s at 9.5 s - * - of 1.5 s will call RTC set for wall clock time 10.0 s at 8.5 s - * - of -0.5 s will call RTC set for wall clock time 10.0 s at 10.5 s + /* + * This offset specifies the update timing of the RTC. + * + * tsched t1 write(t2.tv_sec - 1sec)) t2 RTC increments seconds + * + * The offset defines how tsched is computed so that the write to + * the RTC (t2.tv_sec - 1sec) is correct versus the time required + * for the transport of the write and the time which the RTC needs + * to increment seconds the first time after the write (t2). + * + * For direct accessible RTCs tsched ~= t1 because the write time + * is negligible. For RTCs behind slow busses the transport time is + * significant and has to be taken into account. + * + * The time between the write (t1) and the first increment after + * the write (t2) is RTC specific. For a MC146818 RTC it's 500ms, + * for many others it's exactly 1 second. Consult the datasheet. + * + * The value of this offset is also used to calculate the to be + * written value (t2.tv_sec - 1sec) at tsched. + * + * The default value for this is NSEC_PER_SEC + 10 msec default + * transport time. The offset can be adjusted by drivers so the + * calculation for the to be written value at tsched becomes + * correct: + * + * newval = tsched + set_offset_nsec - NSEC_PER_SEC + * and (tsched + set_offset_nsec) % NSEC_PER_SEC == 0 */ - long set_offset_nsec; + unsigned long set_offset_nsec; bool registered; diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c index 84a554622cee..a34ac069335f 100644 --- a/kernel/time/ntp.c +++ b/kernel/time/ntp.c @@ -520,22 +520,33 @@ static void sched_sync_hw_clock(unsigned long offset_nsec, bool retry) } /* - * Determine if we can call to driver to set the time. Drivers can only be - * called to set a second aligned time value, and the field set_offset_nsec - * specifies how far away from the second aligned time to call the driver. + * Check whether @now is correct versus the required time to update the RTC + * and calculate the value which needs to be written to the RTC so that the + * next seconds increment of the RTC after the write is aligned with the next + * seconds increment of clock REALTIME. * - * This also computes 'to_set' which is the time we are trying to set, and has - * a zero in tv_nsecs, such that: - * to_set - set_delay_nsec == now +/- FUZZ + * tsched t1 write(t2.tv_sec - 1sec)) t2 RTC increments seconds * + * t2.tv_nsec == 0 + * tsched = t2 - set_offset_nsec + * newval = t2 - NSEC_PER_SEC + * + * ==> neval = tsched + set_offset_nsec - NSEC_PER_SEC + * + * As the execution of this code is not guaranteed to happen exactly at + * tsched this allows it to happen within a fuzzy region: + * + * abs(now - tsched) < FUZZ + * + * If @now is not inside the allowed window the function returns false. */ -static inline bool rtc_tv_nsec_ok(long set_offset_nsec, +static inline bool rtc_tv_nsec_ok(unsigned long set_offset_nsec, struct timespec64 *to_set, const struct timespec64 *now) { /* Allowed error in tv_nsec, arbitarily set to 5 jiffies in ns. */ const unsigned long TIME_SET_NSEC_FUZZ = TICK_NSEC * 5; - struct timespec64 delay = {.tv_sec = 0, + struct timespec64 delay = {.tv_sec = -1, .tv_nsec = set_offset_nsec}; *to_set = timespec64_add(*now, delay); @@ -557,11 +568,11 @@ static inline bool rtc_tv_nsec_ok(long set_offset_nsec, /* * rtc_set_ntp_time - Save NTP synchronized time to the RTC */ -static int rtc_set_ntp_time(struct timespec64 now, unsigned long *target_nsec) +static int rtc_set_ntp_time(struct timespec64 now, unsigned long *offset_nsec) { + struct timespec64 to_set; struct rtc_device *rtc; struct rtc_time tm; - struct timespec64 to_set; int err = -ENODEV; bool ok; @@ -572,19 +583,9 @@ static int rtc_set_ntp_time(struct timespec64 now, unsigned long *target_nsec) if (!rtc->ops || !rtc->ops->set_time) goto out_close; - /* - * Compute the value of tv_nsec we require the caller to supply in - * now.tv_nsec. This is the value such that (now + - * set_offset_nsec).tv_nsec == 0. - */ - set_normalized_timespec64(&to_set, 0, -rtc->set_offset_nsec); - *target_nsec = to_set.tv_nsec; + /* Store the update offset for this RTC */ + *offset_nsec = rtc->set_offset_nsec; - /* - * The ntp code must call this with the correct value in tv_nsec, if - * it does not we update target_nsec and return EPROTO to make the ntp - * code try again later. - */ ok = rtc_tv_nsec_ok(rtc->set_offset_nsec, &to_set, &now); if (!ok) { err = -EPROTO; @@ -657,7 +658,7 @@ static bool sync_cmos_clock(void) * implement this legacy API. */ ktime_get_real_ts64(&now); - if (rtc_tv_nsec_ok(-1 * target_nsec, &adjust, &now)) { + if (rtc_tv_nsec_ok(target_nsec, &adjust, &now)) { if (persistent_clock_is_local) adjust.tv_sec -= (sys_tz.tz_minuteswest * 60); rc = update_persistent_clock64(adjust); -- cgit v1.2.3