Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32097 )
Change subject: WIP: mediaek/mt8183: add panel driver for kukui p2 ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/#/c/32097/1/src/mainboard/google/kukui/mainboard... File src/mainboard/google/kukui/mainboard.c:
https://review.coreboot.org/#/c/32097/1/src/mainboard/google/kukui/mainboard... PS1, Line 60: printk(BIOS_ERR, "board_id() = %d\n", board_id()); This is already printed by common code (lib/coreboot_table.c), doesn't need to be printed again here. Especially not as BIOS_ERR.
https://review.coreboot.org/#/c/32097/1/src/mainboard/google/kukui/mainboard... PS1, Line 80: gpio_set_pull(GPIO(LCM_RST), GPIO_PULL_ENABLE, GPIO_PULL_UP); What are you trying to do here? You can't configure a pin to be an output pin but also have a pull resistor, that makes no sense. That's also why gpio_output() will disable the pull again automatically, so this ends up changing nothing.
https://review.coreboot.org/#/c/32097/1/src/mainboard/google/kukui/mainboard... PS1, Line 226: if (board_id() < 2) { You shouldn't need two if (board_id()...) branches here... either put all of it in variables first so that the code below can be common, or get rid of this and just put these values directly in the code below.
https://review.coreboot.org/#/c/32097/1/src/mainboard/google/kukui/mainboard... PS1, Line 243: sizeof(lcm_init_cmd) / sizeof(struct lcm_init_table) Please use ARRAY_SIZE() for this sort of stuff.