From 73737a5833ace25a8408b0d3b783637cb6bf29d1 Mon Sep 17 00:00:00 2001 From: "Steven Rostedt (Google)" Date: Wed, 23 Nov 2022 21:18:34 +0100 Subject: clocksource/drivers/arm_arch_timer: Do not use timer namespace for timer_shutdown() function A new "shutdown" timer state is being added to the generic timer code. One of the functions to change the timer into the state is called "timer_shutdown()". This means that there can not be other functions called "timer_shutdown()" as the timer code owns the "timer_*" name space. Rename timer_shutdown() to arch_timer_shutdown() to avoid this conflict. Signed-off-by: Steven Rostedt (Google) Signed-off-by: Thomas Gleixner Tested-by: Guenter Roeck Reviewed-by: Guenter Roeck Reviewed-by: Jacob Keller Reviewed-by: Anna-Maria Behnsen Acked-by: Marc Zyngier Link: https://lkml.kernel.org/r/20221106212702.002251651@goodmis.org Link: https://lore.kernel.org/all/20221105060155.409832154@goodmis.org/ Link: https://lore.kernel.org/r/20221110064146.981725531@goodmis.org Link: https://lore.kernel.org/r/20221123201624.574672568@linutronix.de --- drivers/clocksource/arm_arch_timer.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index a7ff77550e17..9c3420a0d19d 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -687,8 +687,8 @@ static irqreturn_t arch_timer_handler_virt_mem(int irq, void *dev_id) return timer_handler(ARCH_TIMER_MEM_VIRT_ACCESS, evt); } -static __always_inline int timer_shutdown(const int access, - struct clock_event_device *clk) +static __always_inline int arch_timer_shutdown(const int access, + struct clock_event_device *clk) { unsigned long ctrl; @@ -701,22 +701,22 @@ static __always_inline int timer_shutdown(const int access, static int arch_timer_shutdown_virt(struct clock_event_device *clk) { - return timer_shutdown(ARCH_TIMER_VIRT_ACCESS, clk); + return arch_timer_shutdown(ARCH_TIMER_VIRT_ACCESS, clk); } static int arch_timer_shutdown_phys(struct clock_event_device *clk) { - return timer_shutdown(ARCH_TIMER_PHYS_ACCESS, clk); + return arch_timer_shutdown(ARCH_TIMER_PHYS_ACCESS, clk); } static int arch_timer_shutdown_virt_mem(struct clock_event_device *clk) { - return timer_shutdown(ARCH_TIMER_MEM_VIRT_ACCESS, clk); + return arch_timer_shutdown(ARCH_TIMER_MEM_VIRT_ACCESS, clk); } static int arch_timer_shutdown_phys_mem(struct clock_event_device *clk) { - return timer_shutdown(ARCH_TIMER_MEM_PHYS_ACCESS, clk); + return arch_timer_shutdown(ARCH_TIMER_MEM_PHYS_ACCESS, clk); } static __always_inline void set_next_event(const int access, unsigned long evt, -- cgit v1.2.3-58-ga151 From 6e1fc2591f116dfb20b65cf27356475461d61bd8 Mon Sep 17 00:00:00 2001 From: "Steven Rostedt (Google)" Date: Wed, 23 Nov 2022 21:18:36 +0100 Subject: clocksource/drivers/sp804: Do not use timer namespace for timer_shutdown() function A new "shutdown" timer state is being added to the generic timer code. One of the functions to change the timer into the state is called "timer_shutdown()". This means that there can not be other functions called "timer_shutdown()" as the timer code owns the "timer_*" name space. Rename timer_shutdown() to evt_timer_shutdown() to avoid this conflict. Signed-off-by: Steven Rostedt (Google) Signed-off-by: Thomas Gleixner Tested-by: Guenter Roeck Reviewed-by: Guenter Roeck Reviewed-by: Jacob Keller Reviewed-by: Anna-Maria Behnsen Link: https://lkml.kernel.org/r/20221106212702.182883323@goodmis.org Link: https://lore.kernel.org/all/20221105060155.592778858@goodmis.org/ Link: https://lore.kernel.org/r/20221110064147.158230501@goodmis.org Link: https://lore.kernel.org/r/20221123201624.634354813@linutronix.de --- drivers/clocksource/timer-sp804.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c index e6a87f4af2b5..cd1916c05325 100644 --- a/drivers/clocksource/timer-sp804.c +++ b/drivers/clocksource/timer-sp804.c @@ -155,14 +155,14 @@ static irqreturn_t sp804_timer_interrupt(int irq, void *dev_id) return IRQ_HANDLED; } -static inline void timer_shutdown(struct clock_event_device *evt) +static inline void evt_timer_shutdown(struct clock_event_device *evt) { writel(0, common_clkevt->ctrl); } static int sp804_shutdown(struct clock_event_device *evt) { - timer_shutdown(evt); + evt_timer_shutdown(evt); return 0; } @@ -171,7 +171,7 @@ static int sp804_set_periodic(struct clock_event_device *evt) unsigned long ctrl = TIMER_CTRL_32BIT | TIMER_CTRL_IE | TIMER_CTRL_PERIODIC | TIMER_CTRL_ENABLE; - timer_shutdown(evt); + evt_timer_shutdown(evt); writel(common_clkevt->reload, common_clkevt->load); writel(ctrl, common_clkevt->ctrl); return 0; -- cgit v1.2.3-58-ga151 From 9a5a305686971f4be10c6d7251c8348d74b3e014 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 23 Nov 2022 21:18:37 +0100 Subject: timers: Get rid of del_singleshot_timer_sync() del_singleshot_timer_sync() used to be an optimization for deleting timers which are not rearmed from the timer callback function. This optimization turned out to be broken and got mapped to del_timer_sync() about 17 years ago. Get rid of the undocumented indirection and use del_timer_sync() directly. No functional change. Signed-off-by: Thomas Gleixner Tested-by: Guenter Roeck Reviewed-by: Jacob Keller Reviewed-by: Anna-Maria Behnsen Link: https://lore.kernel.org/r/20221123201624.706987932@linutronix.de --- drivers/char/tpm/tpm-dev-common.c | 4 ++-- drivers/staging/wlan-ng/hfa384x_usb.c | 4 ++-- drivers/staging/wlan-ng/prism2usb.c | 6 +++--- include/linux/timer.h | 2 -- kernel/time/timer.c | 2 +- net/sunrpc/xprt.c | 2 +- 6 files changed, 9 insertions(+), 11 deletions(-) (limited to 'drivers') diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c index dc4c0a0a5129..30b4c288c1bb 100644 --- a/drivers/char/tpm/tpm-dev-common.c +++ b/drivers/char/tpm/tpm-dev-common.c @@ -155,7 +155,7 @@ ssize_t tpm_common_read(struct file *file, char __user *buf, out: if (!priv->response_length) { *off = 0; - del_singleshot_timer_sync(&priv->user_read_timer); + del_timer_sync(&priv->user_read_timer); flush_work(&priv->timeout_work); } mutex_unlock(&priv->buffer_mutex); @@ -262,7 +262,7 @@ __poll_t tpm_common_poll(struct file *file, poll_table *wait) void tpm_common_release(struct file *file, struct file_priv *priv) { flush_work(&priv->async_work); - del_singleshot_timer_sync(&priv->user_read_timer); + del_timer_sync(&priv->user_read_timer); flush_work(&priv->timeout_work); file->private_data = NULL; priv->response_length = 0; diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c b/drivers/staging/wlan-ng/hfa384x_usb.c index 02fdef7a16c8..c7cd54171d99 100644 --- a/drivers/staging/wlan-ng/hfa384x_usb.c +++ b/drivers/staging/wlan-ng/hfa384x_usb.c @@ -1116,8 +1116,8 @@ cleanup: if (ctlx == get_active_ctlx(hw)) { spin_unlock_irqrestore(&hw->ctlxq.lock, flags); - del_singleshot_timer_sync(&hw->reqtimer); - del_singleshot_timer_sync(&hw->resptimer); + del_timer_sync(&hw->reqtimer); + del_timer_sync(&hw->resptimer); hw->req_timer_done = 1; hw->resp_timer_done = 1; usb_kill_urb(&hw->ctlx_urb); diff --git a/drivers/staging/wlan-ng/prism2usb.c b/drivers/staging/wlan-ng/prism2usb.c index e13da7fadfff..c13f1699e5a2 100644 --- a/drivers/staging/wlan-ng/prism2usb.c +++ b/drivers/staging/wlan-ng/prism2usb.c @@ -170,9 +170,9 @@ static void prism2sta_disconnect_usb(struct usb_interface *interface) */ prism2sta_ifstate(wlandev, P80211ENUM_ifstate_disable); - del_singleshot_timer_sync(&hw->throttle); - del_singleshot_timer_sync(&hw->reqtimer); - del_singleshot_timer_sync(&hw->resptimer); + del_timer_sync(&hw->throttle); + del_timer_sync(&hw->reqtimer); + del_timer_sync(&hw->resptimer); /* Unlink all the URBs. This "removes the wheels" * from the entire CTLX handling mechanism. diff --git a/include/linux/timer.h b/include/linux/timer.h index 648f00105f58..ea35d00123f0 100644 --- a/include/linux/timer.h +++ b/include/linux/timer.h @@ -190,8 +190,6 @@ extern int try_to_del_timer_sync(struct timer_list *timer); # define del_timer_sync(t) del_timer(t) #endif -#define del_singleshot_timer_sync(t) del_timer_sync(t) - extern void init_timers(void); struct hrtimer; extern enum hrtimer_restart it_real_fn(struct hrtimer *); diff --git a/kernel/time/timer.c b/kernel/time/timer.c index f40c88c156fe..190e06980d87 100644 --- a/kernel/time/timer.c +++ b/kernel/time/timer.c @@ -1933,7 +1933,7 @@ signed long __sched schedule_timeout(signed long timeout) timer_setup_on_stack(&timer.timer, process_timeout, 0); __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING); schedule(); - del_singleshot_timer_sync(&timer.timer); + del_timer_sync(&timer.timer); /* Remove the timer from the object tracker */ destroy_timer_on_stack(&timer.timer); diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c index 656cec208371..ab453ede54f0 100644 --- a/net/sunrpc/xprt.c +++ b/net/sunrpc/xprt.c @@ -1164,7 +1164,7 @@ xprt_request_enqueue_receive(struct rpc_task *task) spin_unlock(&xprt->queue_lock); /* Turn off autodisconnect */ - del_singleshot_timer_sync(&xprt->timer); + del_timer_sync(&xprt->timer); return 0; } -- cgit v1.2.3-58-ga151 From e0d3da982c96aeddc1bbf1cf9469dbb9ebdca657 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 23 Nov 2022 21:18:57 +0100 Subject: Bluetooth: hci_qca: Fix the teardown problem for real While discussing solutions for the teardown problem which results from circular dependencies between timers and workqueues, where timers schedule work from their timer callback and workqueues arm the timers from work items, it was discovered that the recent fix to the QCA code is incorrect. That commit fixes the obvious problem of using del_timer() instead of del_timer_sync() and reorders the teardown calls to destroy_workqueue(wq); del_timer_sync(t); This makes it less likely to explode, but it's still broken: destroy_workqueue(wq); /* After this point @wq cannot be touched anymore */ ---> timer expires queue_work(wq) <---- Results in a NULL pointer dereference deep in the work queue core code. del_timer_sync(t); Use the new timer_shutdown_sync() function to ensure that the timers are disarmed, no timer callbacks are running and the timers cannot be armed again. This restores the original teardown sequence: timer_shutdown_sync(t); destroy_workqueue(wq); which is now correct because the timer core silently ignores potential rearming attempts which can happen when destroy_workqueue() drains pending work before mopping up the workqueue. Fixes: 72ef98445aca ("Bluetooth: hci_qca: Use del_timer_sync() before freeing") Signed-off-by: Thomas Gleixner Tested-by: Guenter Roeck Reviewed-by: Jacob Keller Reviewed-by: Anna-Maria Behnsen Acked-by: Luiz Augusto von Dentz Link: https://lore.kernel.org/all/87iljhsftt.ffs@tglx Link: https://lore.kernel.org/r/20221123201625.435907114@linutronix.de --- drivers/bluetooth/hci_qca.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c index 8df11016fd51..ba8be8e1bebd 100644 --- a/drivers/bluetooth/hci_qca.c +++ b/drivers/bluetooth/hci_qca.c @@ -696,9 +696,15 @@ static int qca_close(struct hci_uart *hu) skb_queue_purge(&qca->tx_wait_q); skb_queue_purge(&qca->txq); skb_queue_purge(&qca->rx_memdump_q); + /* + * Shut the timers down so they can't be rearmed when + * destroy_workqueue() drains pending work which in turn might try + * to arm a timer. After shutdown rearm attempts are silently + * ignored by the timer core code. + */ + timer_shutdown_sync(&qca->tx_idle_timer); + timer_shutdown_sync(&qca->wake_retrans_timer); destroy_workqueue(qca->workqueue); - del_timer_sync(&qca->tx_idle_timer); - del_timer_sync(&qca->wake_retrans_timer); qca->hu = NULL; kfree_skb(qca->rx_skb); -- cgit v1.2.3-58-ga151 From 3f44f7156f59cae06e9160eafb5d8b2dfd09e639 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Wed, 30 Nov 2022 22:06:09 +0100 Subject: clocksource/drivers/sh_cmt: Access registers according to spec Documentation for most CMTs say that it takes two input clocks before changes propagate to the timer. This is especially relevant when the timer is stopped to change further settings. Implement the delays according to the spec. To avoid unnecessary delays in atomic mode, also check if the to-be-written value actually differs. CMCNT is a bit special because testing showed that it requires 3 cycles to propagate, which affects all CMTs. Also, the WRFLAG needs to be checked before writing. This fixes "cannot clear CMCNT" messages which occur often on R-Car Gen4 SoCs, but only very rarely on older SoCs for some reason. Fixes: 81b3b2711072 ("clocksource: sh_cmt: Add support for multiple channels per device") Signed-off-by: Wolfram Sang Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20221130210609.7718-1-wsa+renesas@sang-engineering.com --- drivers/clocksource/sh_cmt.c | 88 +++++++++++++++++++++++++++----------------- 1 file changed, 55 insertions(+), 33 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/sh_cmt.c b/drivers/clocksource/sh_cmt.c index 64dcb082d4cf..7b952aa52c0b 100644 --- a/drivers/clocksource/sh_cmt.c +++ b/drivers/clocksource/sh_cmt.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -116,6 +117,7 @@ struct sh_cmt_device { void __iomem *mapbase; struct clk *clk; unsigned long rate; + unsigned int reg_delay; raw_spinlock_t lock; /* Protect the shared start/stop register */ @@ -247,10 +249,17 @@ static inline u32 sh_cmt_read_cmstr(struct sh_cmt_channel *ch) static inline void sh_cmt_write_cmstr(struct sh_cmt_channel *ch, u32 value) { - if (ch->iostart) - ch->cmt->info->write_control(ch->iostart, 0, value); - else - ch->cmt->info->write_control(ch->cmt->mapbase, 0, value); + u32 old_value = sh_cmt_read_cmstr(ch); + + if (value != old_value) { + if (ch->iostart) { + ch->cmt->info->write_control(ch->iostart, 0, value); + udelay(ch->cmt->reg_delay); + } else { + ch->cmt->info->write_control(ch->cmt->mapbase, 0, value); + udelay(ch->cmt->reg_delay); + } + } } static inline u32 sh_cmt_read_cmcsr(struct sh_cmt_channel *ch) @@ -260,7 +269,12 @@ static inline u32 sh_cmt_read_cmcsr(struct sh_cmt_channel *ch) static inline void sh_cmt_write_cmcsr(struct sh_cmt_channel *ch, u32 value) { - ch->cmt->info->write_control(ch->ioctrl, CMCSR, value); + u32 old_value = sh_cmt_read_cmcsr(ch); + + if (value != old_value) { + ch->cmt->info->write_control(ch->ioctrl, CMCSR, value); + udelay(ch->cmt->reg_delay); + } } static inline u32 sh_cmt_read_cmcnt(struct sh_cmt_channel *ch) @@ -268,14 +282,33 @@ static inline u32 sh_cmt_read_cmcnt(struct sh_cmt_channel *ch) return ch->cmt->info->read_count(ch->ioctrl, CMCNT); } -static inline void sh_cmt_write_cmcnt(struct sh_cmt_channel *ch, u32 value) +static inline int sh_cmt_write_cmcnt(struct sh_cmt_channel *ch, u32 value) { + /* Tests showed that we need to wait 3 clocks here */ + unsigned int cmcnt_delay = DIV_ROUND_UP(3 * ch->cmt->reg_delay, 2); + u32 reg; + + if (ch->cmt->info->model > SH_CMT_16BIT) { + int ret = read_poll_timeout_atomic(sh_cmt_read_cmcsr, reg, + !(reg & SH_CMT32_CMCSR_WRFLG), + 1, cmcnt_delay, false, ch); + if (ret < 0) + return ret; + } + ch->cmt->info->write_count(ch->ioctrl, CMCNT, value); + udelay(cmcnt_delay); + return 0; } static inline void sh_cmt_write_cmcor(struct sh_cmt_channel *ch, u32 value) { - ch->cmt->info->write_count(ch->ioctrl, CMCOR, value); + u32 old_value = ch->cmt->info->read_count(ch->ioctrl, CMCOR); + + if (value != old_value) { + ch->cmt->info->write_count(ch->ioctrl, CMCOR, value); + udelay(ch->cmt->reg_delay); + } } static u32 sh_cmt_get_counter(struct sh_cmt_channel *ch, u32 *has_wrapped) @@ -319,7 +352,7 @@ static void sh_cmt_start_stop_ch(struct sh_cmt_channel *ch, int start) static int sh_cmt_enable(struct sh_cmt_channel *ch) { - int k, ret; + int ret; dev_pm_syscore_device(&ch->cmt->pdev->dev, true); @@ -347,26 +380,9 @@ static int sh_cmt_enable(struct sh_cmt_channel *ch) } sh_cmt_write_cmcor(ch, 0xffffffff); - sh_cmt_write_cmcnt(ch, 0); - - /* - * According to the sh73a0 user's manual, as CMCNT can be operated - * only by the RCLK (Pseudo 32 kHz), there's one restriction on - * modifying CMCNT register; two RCLK cycles are necessary before - * this register is either read or any modification of the value - * it holds is reflected in the LSI's actual operation. - * - * While at it, we're supposed to clear out the CMCNT as of this - * moment, so make sure it's processed properly here. This will - * take RCLKx2 at maximum. - */ - for (k = 0; k < 100; k++) { - if (!sh_cmt_read_cmcnt(ch)) - break; - udelay(1); - } + ret = sh_cmt_write_cmcnt(ch, 0); - if (sh_cmt_read_cmcnt(ch)) { + if (ret || sh_cmt_read_cmcnt(ch)) { dev_err(&ch->cmt->pdev->dev, "ch%u: cannot clear CMCNT\n", ch->index); ret = -ETIMEDOUT; @@ -995,8 +1011,8 @@ MODULE_DEVICE_TABLE(of, sh_cmt_of_table); static int sh_cmt_setup(struct sh_cmt_device *cmt, struct platform_device *pdev) { - unsigned int mask; - unsigned int i; + unsigned int mask, i; + unsigned long rate; int ret; cmt->pdev = pdev; @@ -1032,10 +1048,16 @@ static int sh_cmt_setup(struct sh_cmt_device *cmt, struct platform_device *pdev) if (ret < 0) goto err_clk_unprepare; - if (cmt->info->width == 16) - cmt->rate = clk_get_rate(cmt->clk) / 512; - else - cmt->rate = clk_get_rate(cmt->clk) / 8; + rate = clk_get_rate(cmt->clk); + if (!rate) { + ret = -EINVAL; + goto err_clk_disable; + } + + /* We shall wait 2 input clks after register writes */ + if (cmt->info->model >= SH_CMT_48BIT) + cmt->reg_delay = DIV_ROUND_UP(2UL * USEC_PER_SEC, rate); + cmt->rate = rate / (cmt->info->width == 16 ? 512 : 8); /* Map the memory resource(s). */ ret = sh_cmt_map_memory(cmt); -- cgit v1.2.3-58-ga151 From 035629547999d0e9095886f248c2580dc56f36c6 Mon Sep 17 00:00:00 2001 From: Lukas Bulwahn Date: Wed, 23 Nov 2022 09:31:59 +0100 Subject: clocksource/drivers/ingenic-ost: Define pm functions properly in platform_driver struct Commit ca7b72b5a5f2 ("clocksource: Add driver for the Ingenic JZ47xx OST") adds the struct platform_driver ingenic_ost_driver, with the definition of pm functions under the non-existing config PM_SUSPEND, which means the intended pm functions were never actually included in any build. As the only callbacks are .suspend_noirq and .resume_noirq, we can assume that it is intended to be CONFIG_PM_SLEEP. Since commit 1a3c7bb08826 ("PM: core: Add new *_PM_OPS macros, deprecate old ones"), the default pattern for platform_driver definitions conditional for CONFIG_PM_SLEEP is to use pm_sleep_ptr(). As __maybe_unused annotations on the dev_pm_ops structure and its callbacks are not needed anymore, remove these as well. Suggested-by: Paul Cercueil Signed-off-by: Lukas Bulwahn Signed-off-by: Thomas Gleixner Reviewed-by: Paul Cercueil Link: https://lore.kernel.org/r/20221123083159.22821-1-lukas.bulwahn@gmail.com --- drivers/clocksource/ingenic-ost.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/ingenic-ost.c b/drivers/clocksource/ingenic-ost.c index 06d25754e606..9f7c280a1336 100644 --- a/drivers/clocksource/ingenic-ost.c +++ b/drivers/clocksource/ingenic-ost.c @@ -141,7 +141,7 @@ static int __init ingenic_ost_probe(struct platform_device *pdev) return 0; } -static int __maybe_unused ingenic_ost_suspend(struct device *dev) +static int ingenic_ost_suspend(struct device *dev) { struct ingenic_ost *ost = dev_get_drvdata(dev); @@ -150,14 +150,14 @@ static int __maybe_unused ingenic_ost_suspend(struct device *dev) return 0; } -static int __maybe_unused ingenic_ost_resume(struct device *dev) +static int ingenic_ost_resume(struct device *dev) { struct ingenic_ost *ost = dev_get_drvdata(dev); return clk_enable(ost->clk); } -static const struct dev_pm_ops __maybe_unused ingenic_ost_pm_ops = { +static const struct dev_pm_ops ingenic_ost_pm_ops = { /* _noirq: We want the OST clock to be gated last / ungated first */ .suspend_noirq = ingenic_ost_suspend, .resume_noirq = ingenic_ost_resume, @@ -181,9 +181,7 @@ static const struct of_device_id ingenic_ost_of_match[] = { static struct platform_driver ingenic_ost_driver = { .driver = { .name = "ingenic-ost", -#ifdef CONFIG_PM_SUSPEND - .pm = &ingenic_ost_pm_ops, -#endif + .pm = pm_sleep_ptr(&ingenic_ost_pm_ops), .of_match_table = ingenic_ost_of_match, }, }; -- cgit v1.2.3-58-ga151 From db78539fc95cf62b0b8f274368fcd8202eac91f9 Mon Sep 17 00:00:00 2001 From: Jonathan Neuschäfer Date: Fri, 4 Nov 2022 17:18:46 +0100 Subject: clocksource/drivers/timer-npcm7xx: Enable timer 1 clock before use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the WPCM450 SoC, the clocks for each timer can be gated individually. To prevent the timer 1 clock from being gated, enable it explicitly. Signed-off-by: Jonathan Neuschäfer Reviewed-by: Joel Stanley Link: https://lore.kernel.org/r/20221104161850.2889894-3-j.neuschaefer@gmx.net Signed-off-by: Daniel Lezcano --- drivers/clocksource/timer-npcm7xx.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'drivers') diff --git a/drivers/clocksource/timer-npcm7xx.c b/drivers/clocksource/timer-npcm7xx.c index a00520cbb660..9af30af5f989 100644 --- a/drivers/clocksource/timer-npcm7xx.c +++ b/drivers/clocksource/timer-npcm7xx.c @@ -188,6 +188,7 @@ static void __init npcm7xx_clocksource_init(void) static int __init npcm7xx_timer_init(struct device_node *np) { + struct clk *clk; int ret; ret = timer_of_init(np, &npcm7xx_to); @@ -199,6 +200,15 @@ static int __init npcm7xx_timer_init(struct device_node *np) npcm7xx_to.of_clk.rate = npcm7xx_to.of_clk.rate / (NPCM7XX_Tx_MIN_PRESCALE + 1); + /* Enable the clock for timer1, if it exists */ + clk = of_clk_get(np, 1); + if (clk) { + if (!IS_ERR(clk)) + clk_prepare_enable(clk); + else + pr_warn("%pOF: Failed to get clock for timer1: %pe", np, clk); + } + npcm7xx_clocksource_init(); npcm7xx_clockevents_init(); -- cgit v1.2.3-58-ga151 From 45ae272a948a03a7d55748bf52d2f47d3b4e1d5a Mon Sep 17 00:00:00 2001 From: Joe Korty Date: Mon, 21 Nov 2022 14:53:43 +0000 Subject: clocksource/drivers/arm_arch_timer: Fix XGene-1 TVAL register math error The TVAL register is 32 bit signed. Thus only the lower 31 bits are available to specify when an interrupt is to occur at some time in the near future. Attempting to specify a larger interval with TVAL results in a negative time delta which means the timer fires immediately upon being programmed, rather than firing at that expected future time. The solution is for Linux to declare that TVAL is a 31 bit register rather than give its true size of 32 bits. This prevents Linux from programming TVAL with a too-large value. Note that, prior to 5.16, this little trick was the standard way to handle TVAL in Linux, so there is nothing new happening here on that front. The softlockup detector hides the issue, because it keeps generating short timer deadlines that are within the scope of the broken timer. Disable it, and you start using NO_HZ with much longer timer deadlines, which turns into an interrupt flood: 11: 1124855130 949168462 758009394 76417474 104782230 30210281 310890 1734323687 GICv2 29 Level arch_timer And "much longer" isn't that long: it takes less than 43s to underflow TVAL at 50MHz (the frequency of the counter on XGene-1). Some comments on the v1 version of this patch by Marc Zyngier: XGene implements CVAL (a 64bit comparator) in terms of TVAL (a countdown register) instead of the other way around. TVAL being a 32bit register, the width of the counter should equally be 32. However, TVAL is a *signed* value, and keeps counting down in the negative range once the timer fires. It means that any TVAL value with bit 31 set will fire immediately, as it cannot be distinguished from an already expired timer. Reducing the timer range back to a paltry 31 bits papers over the issue. Another problem cannot be fixed though, which is that the timer interrupt *must* be handled within the negative countdown period, or the interrupt will be lost (TVAL will rollover to a positive value, indicative of a new timer deadline). Cc: stable@vger.kernel.org # 5.16+ Fixes: 012f18850452 ("clocksource/drivers/arm_arch_timer: Work around broken CVAL implementations") Signed-off-by: Joe Korty Reviewed-by: Marc Zyngier [maz: revamped the commit message] Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20221024165422.GA51107@zipoli.concurrent-rt.com Link: https://lore.kernel.org/r/20221121145343.896018-1-maz@kernel.org Signed-off-by: Daniel Lezcano --- drivers/clocksource/arm_arch_timer.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 9c3420a0d19d..e2920da18ea1 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -806,6 +806,9 @@ static u64 __arch_timer_check_delta(void) /* * XGene-1 implements CVAL in terms of TVAL, meaning * that the maximum timer range is 32bit. Shame on them. + * + * Note that TVAL is signed, thus has only 31 of its + * 32 bits to express magnitude. */ MIDR_ALL_VERSIONS(MIDR_CPU_MODEL(ARM_CPU_IMP_APM, APM_CPU_PART_POTENZA)), @@ -813,8 +816,8 @@ static u64 __arch_timer_check_delta(void) }; if (is_midr_in_range_list(read_cpuid_id(), broken_cval_midrs)) { - pr_warn_once("Broken CNTx_CVAL_EL1, limiting width to 32bits"); - return CLOCKSOURCE_MASK(32); + pr_warn_once("Broken CNTx_CVAL_EL1, using 32 bit TVAL instead.\n"); + return CLOCKSOURCE_MASK(31); } #endif return CLOCKSOURCE_MASK(arch_counter_get_width()); -- cgit v1.2.3-58-ga151 From 9688498b1648aa98a3ee45d9f07763c099f6fb12 Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Fri, 28 Oct 2022 13:35:26 +0300 Subject: clocksource/drivers/timer-ti-dm: Fix warning for omap_timer_match We can now get a warning for 'omap_timer_match' defined but not used. Let's fix this by dropping of_match_ptr for omap_timer_match. Reported-by: kernel test robot Fixes: ab0bbef3ae0f ("clocksource/drivers/timer-ti-dm: Make timer selectable for ARCH_K3") Signed-off-by: Tony Lindgren Link: https://lore.kernel.org/r/20221028103526.40319-1-tony@atomide.com Signed-off-by: Daniel Lezcano --- drivers/clocksource/timer-ti-dm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/clocksource/timer-ti-dm.c b/drivers/clocksource/timer-ti-dm.c index cad29ded3a48..00af1a8e34fb 100644 --- a/drivers/clocksource/timer-ti-dm.c +++ b/drivers/clocksource/timer-ti-dm.c @@ -1258,7 +1258,7 @@ static struct platform_driver omap_dm_timer_driver = { .remove = omap_dm_timer_remove, .driver = { .name = "omap_timer", - .of_match_table = of_match_ptr(omap_timer_match), + .of_match_table = omap_timer_match, .pm = &omap_dm_timer_pm_ops, }, }; -- cgit v1.2.3-58-ga151 From dedb2aced3e958c6f4811d3e6b392652ff0eea01 Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Fri, 28 Oct 2022 13:36:04 +0300 Subject: clocksource/drivers/timer-ti-dm: Make timer_get_irq static We can make timer_get_irq() static as noted by Janusz. It is only used by omap_rproc_get_timer_irq() via platform data. Reported-by: Janusz Krzysztofik Signed-off-by: Tony Lindgren Link: https://lore.kernel.org/r/20221028103604.40385-1-tony@atomide.com Signed-off-by: Daniel Lezcano --- drivers/clocksource/timer-ti-dm.c | 2 +- include/clocksource/timer-ti-dm.h | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/clocksource/timer-ti-dm.c b/drivers/clocksource/timer-ti-dm.c index 00af1a8e34fb..eeeeb3c08c91 100644 --- a/drivers/clocksource/timer-ti-dm.c +++ b/drivers/clocksource/timer-ti-dm.c @@ -643,7 +643,7 @@ static int omap_dm_timer_free(struct omap_dm_timer *cookie) return 0; } -int omap_dm_timer_get_irq(struct omap_dm_timer *cookie) +static int omap_dm_timer_get_irq(struct omap_dm_timer *cookie) { struct dmtimer *timer = to_dmtimer(cookie); if (timer) diff --git a/include/clocksource/timer-ti-dm.h b/include/clocksource/timer-ti-dm.h index 77eceeae708c..dcc1712f75e7 100644 --- a/include/clocksource/timer-ti-dm.h +++ b/include/clocksource/timer-ti-dm.h @@ -62,8 +62,6 @@ struct omap_dm_timer { }; -int omap_dm_timer_get_irq(struct omap_dm_timer *timer); - u32 omap_dm_timer_modify_idlect_mask(u32 inputmask); /* -- cgit v1.2.3-58-ga151 From 822963b96dfdb72ec4fb1395fbdfa778656b49d1 Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Fri, 28 Oct 2022 13:38:13 +0300 Subject: clocksource/drivers/timer-ti-dm: Clear settings on probe and free Clear the timer control register on driver probe and omap_dm_timer_free(). Otherwise we assume the consumer driver takes care of properly initializing timer interrupts on PWM driver module reload for example. AFAIK this is not currently needed as a fix, I just happened to run into this while cleaning up things. Signed-off-by: Tony Lindgren Link: https://lore.kernel.org/r/20221028103813.40783-1-tony@atomide.com Signed-off-by: Daniel Lezcano --- drivers/clocksource/timer-ti-dm.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'drivers') diff --git a/drivers/clocksource/timer-ti-dm.c b/drivers/clocksource/timer-ti-dm.c index eeeeb3c08c91..b24b903a8822 100644 --- a/drivers/clocksource/timer-ti-dm.c +++ b/drivers/clocksource/timer-ti-dm.c @@ -633,6 +633,8 @@ static struct omap_dm_timer *omap_dm_timer_request_by_node(struct device_node *n static int omap_dm_timer_free(struct omap_dm_timer *cookie) { struct dmtimer *timer; + struct device *dev; + int rc; timer = to_dmtimer(cookie); if (unlikely(!timer)) @@ -640,6 +642,17 @@ static int omap_dm_timer_free(struct omap_dm_timer *cookie) WARN_ON(!timer->reserved); timer->reserved = 0; + + dev = &timer->pdev->dev; + rc = pm_runtime_resume_and_get(dev); + if (rc) + return rc; + + /* Clear timer configuration */ + dmtimer_write(timer, OMAP_TIMER_CTRL_REG, 0); + + pm_runtime_put_sync(dev); + return 0; } @@ -1135,6 +1148,10 @@ static int omap_dm_timer_probe(struct platform_device *pdev) goto err_disable; } __omap_dm_timer_init_regs(timer); + + /* Clear timer configuration */ + dmtimer_write(timer, OMAP_TIMER_CTRL_REG, 0); + pm_runtime_put(dev); } -- cgit v1.2.3-58-ga151 From 180d35a7c05d520314a590c99ad8643d0213f28b Mon Sep 17 00:00:00 2001 From: Yang Yingliang Date: Sat, 29 Oct 2022 19:44:27 +0800 Subject: clocksource/drivers/timer-ti-dm: Fix missing clk_disable_unprepare in dmtimer_systimer_init_clock() If clk_get_rate() fails which is called after clk_prepare_enable(), clk_disable_unprepare() need be called in error path to disable the clock in dmtimer_systimer_init_clock(). Fixes: 52762fbd1c47 ("clocksource/drivers/timer-ti-dm: Add clockevent and clocksource support") Signed-off-by: Yang Yingliang Reviewed-by: Tony Lindgren Link: https://lore.kernel.org/r/20221029114427.946520-1-yangyingliang@huawei.com Signed-off-by: Daniel Lezcano --- drivers/clocksource/timer-ti-dm-systimer.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/clocksource/timer-ti-dm-systimer.c b/drivers/clocksource/timer-ti-dm-systimer.c index 2737407ff069..632523c1232f 100644 --- a/drivers/clocksource/timer-ti-dm-systimer.c +++ b/drivers/clocksource/timer-ti-dm-systimer.c @@ -345,8 +345,10 @@ static int __init dmtimer_systimer_init_clock(struct dmtimer_systimer *t, return error; r = clk_get_rate(clock); - if (!r) + if (!r) { + clk_disable_unprepare(clock); return -ENODEV; + } if (is_ick) t->ick = clock; -- cgit v1.2.3-58-ga151 From 4238568744c0a150d8901e7847092a0f871c938d Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Tue, 1 Nov 2022 22:13:58 +0100 Subject: clocksource/drivers/arm_arch_timer: Use kstrtobool() instead of strtobool() strtobool() is the same as kstrtobool(). However, the latter is more used within the kernel. In order to remove strtobool() and slightly simplify kstrtox.h, switch to the other function name. While at it, include the corresponding header file () Signed-off-by: Christophe JAILLET Acked-by: Mark Rutland Link: https://lore.kernel.org/r/f430bb12e12eb225ab1206db0be64b755ddafbdc.1667336095.git.christophe.jaillet@wanadoo.fr Signed-off-by: Daniel Lezcano --- drivers/clocksource/arm_arch_timer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index e2920da18ea1..1695c56a2aae 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -97,7 +98,7 @@ static bool evtstrm_enable __ro_after_init = IS_ENABLED(CONFIG_ARM_ARCH_TIMER_EV static int __init early_evtstrm_cfg(char *buf) { - return strtobool(buf, &evtstrm_enable); + return kstrtobool(buf, &evtstrm_enable); } early_param("clocksource.arm_arch_timer.evtstrm", early_evtstrm_cfg); -- cgit v1.2.3-58-ga151