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.rip...@free-electrons.com>
Fixes: 157dfadef832 ("clocksource/drivers/timer_sun5i: Replace code by
                      clocksource_mmio_init")
Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 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

Reply via email to