Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32511 )
Change subject: google/kukui: Elaborate panel support for Kukui family boards. ......................................................................
Patch Set 14:
(12 comments)
https://review.coreboot.org/#/c/32511/14//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32511/14//COMMIT_MSG@7 PS14, Line 7: google/kukui: Elaborate panel support for Kukui family boards. Please remove the dot/period at the end.
https://review.coreboot.org/#/c/32511/14//COMMIT_MSG@10 PS14, Line 10: inf information?
https://review.coreboot.org/#/c/32511/14//COMMIT_MSG@14 PS14, Line 14: it's its
https://review.coreboot.org/#/c/32511/14//COMMIT_MSG@14 PS14, Line 14: it's its
https://review.coreboot.org/#/c/32511/14//COMMIT_MSG@14 PS14, Line 14: one space
https://review.coreboot.org/#/c/32511/14/src/mainboard/google/kukui/display.... File src/mainboard/google/kukui/display.h:
https://review.coreboot.org/#/c/32511/14/src/mainboard/google/kukui/display.... PS14, Line 66: /* : * board related functions : */ Are these comments needed?
https://review.coreboot.org/#/c/32511/14/src/mainboard/google/kukui/display.... File src/mainboard/google/kukui/display.c:
PS14: Did you run this through clang-format?
https://review.coreboot.org/#/c/32511/14/src/mainboard/google/kukui/display.... PS14, Line 38: printk(BIOS_ERR, "%s: wrong parameters\n", __func__); Error messages should be more elaborate, so a user can understand it.
https://review.coreboot.org/#/c/32511/14/src/mainboard/google/kukui/display.... PS14, Line 52: printk(BIOS_ERR, "dsi init fail\n"); Maybe?
DSI initialization failed. Continue without.
https://review.coreboot.org/#/c/32511/14/src/mainboard/google/kukui/display.... PS14, Line 93: /* Exported Functions */ Are these comments neeeded?
https://review.coreboot.org/#/c/32511/14/src/mainboard/google/kukui/display.... PS14, Line 113: -1 We have error codes in coreboot. CB_SUCCESS, …
https://review.coreboot.org/#/c/32511/14/src/mainboard/google/kukui/panel_ku... File src/mainboard/google/kukui/panel_kukui.c:
https://review.coreboot.org/#/c/32511/14/src/mainboard/google/kukui/panel_ku... PS14, Line 34: .name = "768x1024@60Hz", Isn’t it landscape?