Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34877 )
Change subject: mb/google/kukui: Move panel description to CBFS files ......................................................................
Patch Set 5:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34877/3/src/mainboard/google/kukui/... File src/mainboard/google/kukui/mainboard.c:
https://review.coreboot.org/c/coreboot/+/34877/3/src/mainboard/google/kukui/... PS3, Line 151: edid->y_resolution ? edid->y_resolution : edid->mode.va,
ha is only different from x_resolution if the framebuffer uses aligned rows and the resolution doesn […]
Ack
https://review.coreboot.org/c/coreboot/+/34877/4/src/mainboard/google/kukui/... File src/mainboard/google/kukui/mainboard.c:
https://review.coreboot.org/c/coreboot/+/34877/4/src/mainboard/google/kukui/... PS4, Line 105: static u8 buffer[8 * 1024];
nit: write this as […]
Done
https://review.coreboot.org/c/coreboot/+/34877/4/src/mainboard/google/kukui/... PS4, Line 111: cbfs_name = malloc(name_len);
nit: just use 64 or 128 byte buffer on the stack, no need to precisely measure this out
Done
https://review.coreboot.org/c/coreboot/+/34877/4/src/mainboard/google/kukui/... PS4, Line 116: int r = cbfs_boot_load_file(cbfs_name, buffer, sizeof(buffer),
nit: technically should be size_t
Done
https://review.coreboot.org/c/coreboot/+/34877/4/src/mainboard/google/kukui/... PS4, Line 154: assert(panel->s);
nit: this would make more sense in get_active_panel() before you dereference panel->s for the first […]
The get_active_panel is a per-board implementation so some boards may easily forget returning right things; so here's a double confirm that the returned data is really valid.