Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41949 )
Change subject: soc/mediatek/mt8183: Support 6GB, 8GB DDR bootup ......................................................................
Patch Set 8:
(9 comments)
https://review.coreboot.org/c/coreboot/+/41949/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41949/8//COMMIT_MSG@10 PS8, Line 10: in future by adding geometry information
… in future, so add geometry information …
https://review.coreboot.org/c/coreboot/+/41949/8//COMMIT_MSG@10 PS8, Line 10: we Nit: Fits on line above.
https://review.coreboot.org/c/coreboot/+/41949/8//COMMIT_MSG@12 PS8, Line 12: Mention that the header version is increased to 3, and that stuff from `emi.h` is moved.
https://review.coreboot.org/c/coreboot/+/41949/8/src/soc/mediatek/mt8183/dra... File src/soc/mediatek/mt8183/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/41949/8/src/soc/mediatek/mt8183/dra... PS8, Line 2694: infor Just *info*?
https://review.coreboot.org/c/coreboot/+/41949/8/src/soc/mediatek/mt8183/dra... PS8, Line 2735: max_density = density; Please print a warning in this case.
https://review.coreboot.org/c/coreboot/+/41949/8/src/soc/mediatek/mt8183/emi... File src/soc/mediatek/mt8183/emi.c:
https://review.coreboot.org/c/coreboot/+/41949/8/src/soc/mediatek/mt8183/emi... PS8, Line 56: u8 trfrc_pb_05t; Renaming the variables, makes this hard to review in Gerrit.
https://review.coreboot.org/c/coreboot/+/41949/8/src/soc/mediatek/mt8183/emi... PS8, Line 327: enum tRFCAB {tRFCAB_130 = 0, tRFCAB_180, tRFCAB_280, tRFCAB_380, tRFCAB_NUM}; Please put each assignment on a separate line.
https://review.coreboot.org/c/coreboot/+/41949/8/src/soc/mediatek/mt8183/emi... PS8, Line 393: dramc_err("density err!\n"); Please make the message more elaborate. Maybe also output the unsupported value.
https://review.coreboot.org/c/coreboot/+/41949/8/src/soc/mediatek/mt8183/emi... PS8, Line 568: u8 density = 0; The initialization is not needed?