Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38490 )
Change subject: soc/mediatek/mt8183: improve the DRAMC runtime config flow ......................................................................
Patch Set 1:
(5 comments)
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 323: size_t
Couldn't this be u8 (for consistency)?
It should be, the existing code uses u8 in most cases.
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 352: 0x3fffff << 10); Fits on previous line
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 428: size_t Why size_t here?
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 431: /* RX_TRACKING: ON */
Wrong indent.
Also applies to all other comments
https://review.coreboot.org/c/coreboot/+/38490/1/src/soc/mediatek/mt8183/dra... PS1, Line 493: (0x1 << 6) | (0x1 << 4)| (0x1 << 2),
need consistent spacing around '|' (ctx:VxW)
Please correct this
Another suggestion: align the "(0x1 << 12)" blocks:
(0x1 << 21) | (0x7f << 12) | (0xf << 8) | (0x1 << 6) | (0x1 << 4) | (0x1 << 2), (0x1 << 19) | (0x3 << 12) | (0x8 << 8) | (0x1 << 5) | (0x1 << 2) | (0x1 << 0));