Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31591 )
Change subject: mediaek/mt8183: add dsi driver for mt8183 ......................................................................
Patch Set 1:
(8 comments)
You could try to run all the new files through clang-format.
https://review.coreboot.org/#/c/31591/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31591/1//COMMIT_MSG@7 PS1, Line 7: mediaek mediatek
https://review.coreboot.org/#/c/31591/1//COMMIT_MSG@8 PS1, Line 8: Where is the data sheet (or the name, revision), and is that driver written from scratch or taken from, for example, U-Boot?
https://review.coreboot.org/#/c/31591/1/src/soc/mediatek/mt8183/dsi.c File src/soc/mediatek/mt8183/dsi.c:
https://review.coreboot.org/#/c/31591/1/src/soc/mediatek/mt8183/dsi.c@23 PS1, Line 23: static void mipi_write32(void *a, uint32_t offset, uint32_t v) Are these all common functions, which can be shared?
https://review.coreboot.org/#/c/31591/1/src/soc/mediatek/mt8183/dsi.c@98 PS1, Line 98: BIOS_ERR Why is this an error? BIOS_INFO or NOTICE?
https://review.coreboot.org/#/c/31591/1/src/soc/mediatek/mt8183/dsi.c@139 PS1, Line 139: udelay(30); Is that delay mentioned in the data sheet?
https://review.coreboot.org/#/c/31591/1/src/soc/mediatek/mt8183/dsi.c@415 PS1, Line 415: Timeout time-out
https://review.coreboot.org/#/c/31591/1/src/soc/mediatek/mt8183/dsi.c@415 PS1, Line 415: dsi DONE INT Timeout Please rephrase to be better understandable, and also add the stop watch time.
https://review.coreboot.org/#/c/31591/1/src/soc/mediatek/mt8183/dsi.c@416 PS1, Line 416: Please add a debug message, detailing how long it took by printing the stopwatch value.