Attention is currently required from: Martin Roth, Rex-BC Chen, Yu-Ping Wu, Yidi Lin. Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52541 )
Change subject: soc/mediatek/mt8195: Add timer support ......................................................................
Patch Set 1:
(5 comments)
Patchset:
PS1: I think we can simply merge this to common/timer.c, and have the timer_prepare in mt8195/timer.c?
File src/soc/mediatek/mt8195/include/soc/timer_v2.h:
https://review.coreboot.org/c/coreboot/+/52541/comment/39c1bd4c_7e25efa6 PS1, Line 11: struct mtk_gpt_regs { : u32 reserved1[40]; : u32 gpt6_con; : u32 reserved2; : u32 gpt6_cnt_l; : u32 reserved3; : u32 gpt6_cnt_h; : }; I think we can make the different regs defined in
common/timer_v1.h common/timer_v2.h
And then in soc folders, create
mt8183/timer_soc.h # include "timer_v1.h" mt8173/timer_soc.h # include "timer_v1.h" mt8195/timer_soc.h # include "timer_v2.h"
And in common/timer.h do
#include "timer_soc.h"
https://review.coreboot.org/c/coreboot/+/52541/comment/8ff18134_1339d30e PS1, Line 25: 1 << 0, BIT(0)
https://review.coreboot.org/c/coreboot/+/52541/comment/5924c84e_f7a4708e PS1, Line 27: GPT_CLK_MASK Do you want to use the SET32_BITFIELDS API? that will change these to
DEFINE_BIT(GPT_CON_EN, 0) DEFINE_BIT(GPT_CON_CLR, 1) DEFINE_BITFIELD(GPT_CLK, 3, 2)
...
and then you can have the values for fields as
enum { GPT_CLK_SYS = 0 ... }
File src/soc/mediatek/mt8195/timer.c:
https://review.coreboot.org/c/coreboot/+/52541/comment/390e8c6e_75ba3836 PS1, Line 29: static void enable_systimer_compensation(void) : { : clrbits32((void *)SYSTIMER_BASE, COMP_FEATURE_MASK); : setbits32((void *)SYSTIMER_BASE, COMP_25_MASK); : } this can be implemented as timer_prepare function.
-
Hung-Te Lin (Code Review)