[PATCH] Revert "clocksource/drivers/timer_sun5i: Replace code by clocksource_mmio_init"
From: Chen-Yu Tsai
Date: Tue Oct 18 2016 - 01:50:09 EST
This reverts commit 157dfadef832 ("clocksource/drivers/timer_sun5i:
Replace code by clocksource_mmio_init").
struct clocksource is also used by the clk notifier callback, to
unregister and re-register the clocksource with a different clock rate.
clocksource_mmio_init does not pass back a pointer to the struct used,
and the clk notifier callback fixed assumed the struct clocksource in
struct sun5i_timer_clksrc was valid. This results in a kernel NULL
pointer dereference when the hstimer clock is changed:
Unable to handle kernel NULL pointer dereference at virtual address 00000004
pgd = c0204000
[00000004] *pgd=00000000
Internal error: Oops: 805 [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 44 Comm: kworker/0:1 Not tainted 4.8.0-14234-g346c22e #1
Hardware name: Allwinner sun6i (A31) Family
Workqueue: events dbs_work_handler
task: ee1e0a80 task.stack: ee282000
PC is at clocksource_unbind+0x2c/0x5c
LR is at clocksource_unregister+0x2c/0x44
pc : [<c03a4678>] lr : [<c03a46d4>] psr: 20000013
sp : ee283d38 ip : ee019288 fp : 00124f80
r10: 00000000 r9 : ffffffff r8 : ee007808
r7 : 00000000 r6 : 00000001 r5 : c1442b50 r4 : ee019298
r3 : ee0192cc r2 : 00000000 r1 : 00000000 r0 : 00000000
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 10c5387d Table: 4020406a DAC: 00000051
Process kworker/0:1 (pid: 44, stack limit = 0xee282220)
Stack: (0xee283d38 to 0xee284000)
3d20: ee019298 ee283dac
3d40: 00000001 c03a46d4 ffffffff c0a6f350 ffffffff c035ea50 ee01a0dc ee01a0c4
3d60: 00000000 00000001 ee283dac c035edc0 00000000 c066fb7c ee01a0c0 c1388890
3d80: ee00f480 00000001 fffffffa 00124f80 00124f80 c035edf4 00000000 c13ab624
3da0: 00000001 c0670174 ffffffff ee0188c0 0501bd00 044aa200 ee01a01c ee00f480
3dc0: ee00f480 00000000 00000001 c06702c0 ee00f480 ee00cd00 00000000 00000001
3de0: fffffffa c0670288 ee00cd00 ee00cc80 00000000 00000001 fffffffa c0670288
3e00: ee00cc80 ee00cc00 00000000 00000001 fffffffa c0670288 ee00cc00 ee00c300
3e20: 00000000 00000001 fffffffa c0670288 ee00cc00 337f9800 eefa8050 ee00c300
3e40: fffffffa c0672020 ee686940 337f9800 eefa8050 ee6868c0 fffffffa c0672098
3e60: ee686940 ee663400 eefa8050 c07eea64 000d2f00 c0a216fc 337f9800 3c14dc00
3e80: c1302514 ee57ac00 00000000 00000002 c1462434 00000000 00000000 000d2f00
3ea0: ee22d680 c0a21cb8 00000010 000f6180 000d2f00 00000021 ee68cc00 ee57ac00
3ec0: 0092a180 00a79400 ee68cc80 ee686c40 eefad900 c0a25344 ee68cc40 00000000
3ee0: ee68cc04 ee57ac00 c1408bac 00000000 eefad900 c0a2588c ee68cc40 ee22d680
3f00: eefad900 00000000 eefb0e00 c0357d84 c1302100 eefad918 eefad900 ee22d698
3f20: 00000008 c1302100 eefad918 ee282000 eefad900 c0358a00 ee22bc00 ee22d680
3f40: c03589c8 00000000 ee22bc00 ee22d680 c03589c8 00000000 00000000 00000000
3f60: 00000000 c035d66c 00000000 00000000 ffffffff ee22d680 00000000 00000000
3f80: ee283f80 ee283f80 00000000 00000000 ee283f90 ee283f90 ee22bc00 ee22bc00
3fa0: c035d590 00000000 00000000 c0307d38 00000000 00000000 00000000 00000000
3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
3fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ffffffff ffffffff
[<c03a4678>] (clocksource_unbind) from [<c03a46d4>] (clocksource_unregister+0x2c/0x44)
[<c03a46d4>] (clocksource_unregister) from [<c0a6f350>] (sun5i_rate_cb_clksrc+0x34/0x3c)
[<c0a6f350>] (sun5i_rate_cb_clksrc) from [<c035ea50>] (notifier_call_chain+0x44/0x84)
[<c035ea50>] (notifier_call_chain) from [<c035edc0>] (__srcu_notifier_call_chain+0x44/0x60)
[<c035edc0>] (__srcu_notifier_call_chain) from [<c035edf4>] (srcu_notifier_call_chain+0x18/0x20)
[<c035edf4>] (srcu_notifier_call_chain) from [<c0670174>] (__clk_notify+0x70/0x7c)
[<c0670174>] (__clk_notify) from [<c06702c0>] (clk_propagate_rate_change+0xa4/0xc4)
[<c06702c0>] (clk_propagate_rate_change) from [<c0670288>] (clk_propagate_rate_change+0x6c/0xc4)
Code: e5942038 e2843034 e5941034 e3a00000 (e5812004)
---[ end trace c5149f92effb19a4 ]---
Revert the commit for now. clocksource_mmio_init can be made to pass back
a pointer, but the code churn and usage of an inner struct might not be
worth it.
Reported-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx>
Fixes: 157dfadef832 ("clocksource/drivers/timer_sun5i: Replace code by
clocksource_mmio_init")
Signed-off-by: Chen-Yu Tsai <wens@xxxxxxxx>
---
drivers/clocksource/timer-sun5i.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/clocksource/timer-sun5i.c b/drivers/clocksource/timer-sun5i.c
index c184eb84101e..4f87f3e76d83 100644
--- a/drivers/clocksource/timer-sun5i.c
+++ b/drivers/clocksource/timer-sun5i.c
@@ -152,6 +152,13 @@ static irqreturn_t sun5i_timer_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static cycle_t sun5i_clksrc_read(struct clocksource *clksrc)
+{
+ struct sun5i_timer_clksrc *cs = to_sun5i_timer_clksrc(clksrc);
+
+ return ~readl(cs->timer.base + TIMER_CNTVAL_LO_REG(1));
+}
+
static int sun5i_rate_cb_clksrc(struct notifier_block *nb,
unsigned long event, void *data)
{
@@ -210,8 +217,13 @@ static int __init sun5i_setup_clocksource(struct device_node *node,
writel(TIMER_CTL_ENABLE | TIMER_CTL_RELOAD,
base + TIMER_CTL_REG(1));
- ret = clocksource_mmio_init(base + TIMER_CNTVAL_LO_REG(1), node->name,
- rate, 340, 32, clocksource_mmio_readl_down);
+ cs->clksrc.name = node->name;
+ cs->clksrc.rating = 340;
+ cs->clksrc.read = sun5i_clksrc_read;
+ cs->clksrc.mask = CLOCKSOURCE_MASK(32);
+ cs->clksrc.flags = CLOCK_SOURCE_IS_CONTINUOUS;
+
+ ret = clocksource_register_hz(&cs->clksrc, rate);
if (ret) {
pr_err("Couldn't register clock source.\n");
goto err_remove_notifier;
--
2.9.3