From: Daniel Lezcano daniel.lezcano@linaro.org
[ Upstream commit 0b781f527d6f99e68e5b3780ae03cd69a7cb5c0c ]
The driver uses the raw_readl() and raw_writel() functions. Those are not for MMIO devices. Replace them with readl() and writel()
[ dlezcano: Fixed typo in the subject s/reald/readl/ ]
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org Acked-by: Arnd Bergmann arnd@arndb.de Cc: Arnd Bergmann arnd@arndb.de Link: https://lore.kernel.org/r/20250804152344.1109310-2-daniel.lezcano@linaro.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- What changed: The commit replaces all `__raw_readl/__raw_writel` uses with `readl/writel` in the VF610 PIT timer driver, ensuring proper MMIO semantics. Affected spots in `drivers/clocksource/timer-vf-pit.c` include: - `pit_timer_enable`: `__raw_writel(...)` → `writel(...)` (drivers/clocksource/timer-vf-pit.c) - `pit_timer_disable`: `__raw_writel(0, ...)` → `writel(0, ...)` (drivers/clocksource/timer-vf-pit.c) - `pit_irq_acknowledge`: `__raw_writel(PITTFLG_TIF, ...)` → `writel(...)` (drivers/clocksource/timer-vf-pit.c) - `pit_read_sched_clock`: `~__raw_readl(clksrc_base + PITCVAL)` → `~readl(...)` (drivers/clocksource/timer-vf-pit.c) - `pit_clocksource_init`: three writes to `PITTCTRL`/`PITLDVAL` switch to `writel(...)` (drivers/clocksource/timer-vf-pit.c) - `pit_set_next_event`: `__raw_writel(delta - 1, ...)` → `writel(...)` (drivers/clocksource/timer-vf-pit.c) - `pit_clockevent_init`: writes to `PITTCTRL`/`PITTFLG` switch to `writel(...)` (drivers/clocksource/timer-vf-pit.c) - `pit_timer_init`: module enable write `__raw_writel(~PITMCR_MDIS, ...)` → `writel(...)` (drivers/clocksource/timer-vf-pit.c)
- Why this is a bug fix: `__raw_readl/__raw_writel` are explicitly documented as low-level accessors without ordering or byte-order semantics and “not for MMIO registers.” Using them on MMIO can lead to reordering/posting issues on weakly ordered architectures. This can cause: - Timer enable occurring before the new `LDVAL` write reaches hardware in `pit_set_next_event`, producing incorrect next-event timing. - IRQ acknowledge in `pit_timer_interrupt` not being ordered with respect to subsequent code, potentially leading to spurious or missed interrupts. - Sched_clock reads using `__raw_readl` lacking ordering guarantees. The correct `readl/writel` include the architecture-defined ordering required for MMIO and fix these subtle, real-world bugs. See documentation: `Documentation/driver-api/device-io.rst` (section describing `__raw_*` accessors).
- Scope and risk: The change is small, mechanical, and confined to a single driver file. It does not alter logic, APIs, or data structures. It only corrects the MMIO accessor choice. While the timer/clocksource subsystem is critical, this is the minimally invasive, intended API usage and aligns with how other clocksource drivers operate.
- Upstream context: This exact change is upstream as commit 0b781f527d6f9 (“clocksource/drivers/vf-pit: Replace raw_readl/writel to readl/writel”), acknowledged by the timekeeping maintainer. The driver later evolves/renames to `drivers/clocksource/timer-nxp-pit.c`, which consistently uses `readl/writel`, reinforcing that this is the intended, correct pattern.
- Stable backport criteria: - Fixes a real bug that can affect users (ordering on MMIO timer registers). - Minimal and contained patch; no feature additions or architectural changes. - Low regression risk; behavior becomes more robust per documented MMIO rules. - No explicit “Cc: stable”, but it corrects improper API usage present since the driver’s introduction (initial support commit c19672492d233), making it broadly applicable to stable trees that still have `timer-vf-pit.c`.
Given the above, this is a good candidate for stable backport.
drivers/clocksource/timer-vf-pit.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c index 911c92146eca6..8041a8f62d1fa 100644 --- a/drivers/clocksource/timer-vf-pit.c +++ b/drivers/clocksource/timer-vf-pit.c @@ -35,30 +35,30 @@ static unsigned long cycle_per_jiffy;
static inline void pit_timer_enable(void) { - __raw_writel(PITTCTRL_TEN | PITTCTRL_TIE, clkevt_base + PITTCTRL); + writel(PITTCTRL_TEN | PITTCTRL_TIE, clkevt_base + PITTCTRL); }
static inline void pit_timer_disable(void) { - __raw_writel(0, clkevt_base + PITTCTRL); + writel(0, clkevt_base + PITTCTRL); }
static inline void pit_irq_acknowledge(void) { - __raw_writel(PITTFLG_TIF, clkevt_base + PITTFLG); + writel(PITTFLG_TIF, clkevt_base + PITTFLG); }
static u64 notrace pit_read_sched_clock(void) { - return ~__raw_readl(clksrc_base + PITCVAL); + return ~readl(clksrc_base + PITCVAL); }
static int __init pit_clocksource_init(unsigned long rate) { /* set the max load value and start the clock source counter */ - __raw_writel(0, clksrc_base + PITTCTRL); - __raw_writel(~0UL, clksrc_base + PITLDVAL); - __raw_writel(PITTCTRL_TEN, clksrc_base + PITTCTRL); + writel(0, clksrc_base + PITTCTRL); + writel(~0UL, clksrc_base + PITLDVAL); + writel(PITTCTRL_TEN, clksrc_base + PITTCTRL);
sched_clock_register(pit_read_sched_clock, 32, rate); return clocksource_mmio_init(clksrc_base + PITCVAL, "vf-pit", rate, @@ -76,7 +76,7 @@ static int pit_set_next_event(unsigned long delta, * hardware requirement. */ pit_timer_disable(); - __raw_writel(delta - 1, clkevt_base + PITLDVAL); + writel(delta - 1, clkevt_base + PITLDVAL); pit_timer_enable();
return 0; @@ -125,8 +125,8 @@ static struct clock_event_device clockevent_pit = {
static int __init pit_clockevent_init(unsigned long rate, int irq) { - __raw_writel(0, clkevt_base + PITTCTRL); - __raw_writel(PITTFLG_TIF, clkevt_base + PITTFLG); + writel(0, clkevt_base + PITTCTRL); + writel(PITTFLG_TIF, clkevt_base + PITTFLG);
BUG_ON(request_irq(irq, pit_timer_interrupt, IRQF_TIMER | IRQF_IRQPOLL, "VF pit timer", &clockevent_pit)); @@ -183,7 +183,7 @@ static int __init pit_timer_init(struct device_node *np) cycle_per_jiffy = clk_rate / (HZ);
/* enable the pit module */ - __raw_writel(~PITMCR_MDIS, timer_base + PITMCR); + writel(~PITMCR_MDIS, timer_base + PITMCR);
ret = pit_clocksource_init(clk_rate); if (ret)