Message ID | 20220613133819.35318-3-angelogioacchino.delregno@collabora.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | MediaTek SoC ARM/ARM64 System Timer | expand |
On 13/06/2022 15:38, AngeloGioacchino Del Regno wrote: > Some MediaTek platforms with a buggy TrustZone ATF firmware will not > initialize the AArch64 System Timer correctly: in these cases, the > System Timer address is correctly programmed, as well as the CNTFRQ_EL0 > register (reading 13MHz, as it should be), but the assigned hardware > timers are never started before (or after) booting Linux. > > In this condition, any call to function get_cycles() will be returning > zero, as CNTVCT_EL0 will always read zero. > > One common critical symptom of that is trying to use the udelay() > function (calling __delay()), which executes the following loop: > > start = get_cycles(); > while ((get_cycles() - start) < cycles) > cpu_relax(); > > which, when CNTVCT_EL0 always reads zero, translates to: > > while((0 - 0) < 0) ==> while(0 < 0) > > ... generating an infinite loop, even though zero is never less > than zero, but always equal to it (this has to be researched, > but it's out of the scope of this commit). > > To fix this issue on the affected MediaTek platforms, the solution > is to simply start the timers that are designed to be System Timer(s). > These timers, downstream, are called "CPUXGPT" and there is one > timer per CPU core; luckily, it is not necessary to set a start bit > on each CPUX General Purpose Timer, but it's conveniently enough to: > - Set the clock divider (input = 26MHz, divider = 2, output = 13MHz); > - Set the ENABLE bit on a global register (starts all CPUX timers). > > The only small hurdle with this setup is that it's all done through > the MCUSYS wrapper, where it is needed, for each read or write, to > select a register address (by writing it to an index register) and > then to perform any R/W on a "CON" register. > > For example, writing "0x1" to the CPUXGPT register offset 0x4: > - Write 0x4 to mcusys INDEX register > - Write 0x1 to mcusys CON register > > Reading from CPUXGPT register offset 0x4: > - Write 0x4 to mcusys INDEX register > - Read mcusys CON register. > > Finally, starting this timer makes platforms affected by this issue > to work correctly. Why not fix the ATF ?
Il 14/06/22 20:35, Daniel Lezcano ha scritto: > On 13/06/2022 15:38, AngeloGioacchino Del Regno wrote: >> Some MediaTek platforms with a buggy TrustZone ATF firmware will not >> initialize the AArch64 System Timer correctly: in these cases, the >> System Timer address is correctly programmed, as well as the CNTFRQ_EL0 >> register (reading 13MHz, as it should be), but the assigned hardware >> timers are never started before (or after) booting Linux. >> >> In this condition, any call to function get_cycles() will be returning >> zero, as CNTVCT_EL0 will always read zero. >> >> One common critical symptom of that is trying to use the udelay() >> function (calling __delay()), which executes the following loop: >> >> start = get_cycles(); >> while ((get_cycles() - start) < cycles) >> cpu_relax(); >> >> which, when CNTVCT_EL0 always reads zero, translates to: >> >> while((0 - 0) < 0) ==> while(0 < 0) >> >> ... generating an infinite loop, even though zero is never less >> than zero, but always equal to it (this has to be researched, >> but it's out of the scope of this commit). >> >> To fix this issue on the affected MediaTek platforms, the solution >> is to simply start the timers that are designed to be System Timer(s). >> These timers, downstream, are called "CPUXGPT" and there is one >> timer per CPU core; luckily, it is not necessary to set a start bit >> on each CPUX General Purpose Timer, but it's conveniently enough to: >> - Set the clock divider (input = 26MHz, divider = 2, output = 13MHz); >> - Set the ENABLE bit on a global register (starts all CPUX timers). >> >> The only small hurdle with this setup is that it's all done through >> the MCUSYS wrapper, where it is needed, for each read or write, to >> select a register address (by writing it to an index register) and >> then to perform any R/W on a "CON" register. >> >> For example, writing "0x1" to the CPUXGPT register offset 0x4: >> - Write 0x4 to mcusys INDEX register >> - Write 0x1 to mcusys CON register >> >> Reading from CPUXGPT register offset 0x4: >> - Write 0x4 to mcusys INDEX register >> - Read mcusys CON register. >> >> Finally, starting this timer makes platforms affected by this issue >> to work correctly. > > Why not fix the ATF ? > > Hello Daniel, that's not the first time this question gets asked, and I've already provided an explanation for that, please look at [1] for the long answer. As for the shorter answer: OEM/ODM signatures, you can't just compile and flash a new ATF on your own (and some ODMs don't even exist anymore). There's no way other than this. [1]: https://patchwork.kernel.org/project/linux-mediatek/patch/20220509210741.12020-3-angelogioacchino.delregno@collabora.com/#24863077 Regards, Angelo
diff --git a/drivers/clocksource/timer-mediatek.c b/drivers/clocksource/timer-mediatek.c index 7bcb4a3f26fb..d5b29fd03ca2 100644 --- a/drivers/clocksource/timer-mediatek.c +++ b/drivers/clocksource/timer-mediatek.c @@ -22,6 +22,19 @@ #define TIMER_SYNC_TICKS (3) +/* cpux mcusys wrapper */ +#define CPUX_CON_REG 0x0 +#define CPUX_IDX_REG 0x4 + +/* cpux */ +#define CPUX_IDX_GLOBAL_CTRL 0x0 + #define CPUX_ENABLE BIT(0) + #define CPUX_CLK_DIV_MASK GENMASK(10, 8) + #define CPUX_CLK_DIV1 BIT(8) + #define CPUX_CLK_DIV2 BIT(9) + #define CPUX_CLK_DIV4 BIT(10) +#define CPUX_IDX_GLOBAL_IRQ 0x30 + /* gpt */ #define GPT_IRQ_EN_REG 0x00 #define GPT_IRQ_ENABLE(val) BIT((val) - 1) @@ -72,6 +85,52 @@ static void __iomem *gpt_sched_reg __read_mostly; +static u32 mtk_cpux_readl(u32 reg_idx, struct timer_of *to) +{ + writel(reg_idx, timer_of_base(to) + CPUX_IDX_REG); + return readl(timer_of_base(to) + CPUX_CON_REG); +} + +static void mtk_cpux_writel(u32 val, u32 reg_idx, struct timer_of *to) +{ + writel(reg_idx, timer_of_base(to) + CPUX_IDX_REG); + writel(val, timer_of_base(to) + CPUX_CON_REG); +} + +static void mtk_cpux_set_irq(struct timer_of *to, bool enable) +{ + const unsigned long *irq_mask = cpumask_bits(cpu_possible_mask); + u32 val; + + val = mtk_cpux_readl(CPUX_IDX_GLOBAL_IRQ, to); + + if (enable) + val |= *irq_mask; + else + val &= ~(*irq_mask); + + mtk_cpux_writel(val, CPUX_IDX_GLOBAL_IRQ, to); +} + +static int mtk_cpux_clkevt_shutdown(struct clock_event_device *clkevt) +{ + /* Clear any irq */ + mtk_cpux_set_irq(to_timer_of(clkevt), false); + + /* + * Disabling CPUXGPT timer will crash the platform, especially + * if Trusted Firmware is using it (usually, for sleep states), + * so we only mask the IRQ and call it a day. + */ + return 0; +} + +static int mtk_cpux_clkevt_resume(struct clock_event_device *clkevt) +{ + mtk_cpux_set_irq(to_timer_of(clkevt), true); + return 0; +} + static void mtk_syst_ack_irq(struct timer_of *to) { /* Clear and disable interrupt */ @@ -281,6 +340,60 @@ static struct timer_of to = { }, }; +static int __init mtk_cpux_init(struct device_node *node) +{ + static struct timer_of to_cpux; + u32 freq, val; + int ret; + + /* + * There are per-cpu interrupts for the CPUX General Purpose Timer + * but since this timer feeds the AArch64 System Timer we can rely + * on the CPU timer PPIs as well, so we don't declare TIMER_OF_IRQ. + */ + to_cpux.flags = TIMER_OF_BASE | TIMER_OF_CLOCK; + to_cpux.clkevt.name = "mtk-cpuxgpt"; + to_cpux.clkevt.rating = 10; + to_cpux.clkevt.cpumask = cpu_possible_mask; + to_cpux.clkevt.set_state_shutdown = mtk_cpux_clkevt_shutdown; + to_cpux.clkevt.tick_resume = mtk_cpux_clkevt_resume; + + /* If this fails, bad things are about to happen... */ + ret = timer_of_init(node, &to_cpux); + if (ret) { + WARN(1, "Cannot start CPUX timers.\n"); + return ret; + } + + /* + * Check if we're given a clock with the right frequency for this + * timer, otherwise warn but keep going with the setup anyway, as + * that makes it possible to still boot the kernel, even though + * it may not work correctly (random lockups, etc). + * The reason behind this is that having an early UART may not be + * possible for everyone and this gives a chance to retrieve kmsg + * for eventual debugging even on consumer devices. + */ + freq = timer_of_rate(&to_cpux); + if (freq > 13000000) + WARN(1, "Requested unsupported timer frequency %u\n", freq); + + /* Clock input is 26MHz, set DIV2 to achieve 13MHz clock */ + val = mtk_cpux_readl(CPUX_IDX_GLOBAL_CTRL, &to_cpux); + val &= ~CPUX_CLK_DIV_MASK; + val |= CPUX_CLK_DIV2; + mtk_cpux_writel(val, CPUX_IDX_GLOBAL_CTRL, &to_cpux); + + /* Enable all CPUXGPT timers */ + val = mtk_cpux_readl(CPUX_IDX_GLOBAL_CTRL, &to_cpux); + mtk_cpux_writel(val | CPUX_ENABLE, CPUX_IDX_GLOBAL_CTRL, &to_cpux); + + clockevents_config_and_register(&to_cpux.clkevt, timer_of_rate(&to_cpux), + TIMER_SYNC_TICKS, 0xffffffff); + + return 0; +} + static int __init mtk_syst_init(struct device_node *node) { int ret; @@ -339,3 +452,4 @@ static int __init mtk_gpt_init(struct device_node *node) } TIMER_OF_DECLARE(mtk_mt6577, "mediatek,mt6577-timer", mtk_gpt_init); TIMER_OF_DECLARE(mtk_mt6765, "mediatek,mt6765-timer", mtk_syst_init); +TIMER_OF_DECLARE(mtk_mt6795, "mediatek,mt6795-systimer", mtk_cpux_init);