Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34599 )
Change subject: lib/coreboot_table: Show splashscreen in lb_table_init ......................................................................
Patch Set 9: Code-Review+1
(6 comments)
https://review.coreboot.org/c/coreboot/+/34599/9//COMMIT_MSG Commit Message:
PS9: Some of the paragraphs might read better if you'd join them (generally, there is no need to give every sentence its own paragraph).
https://review.coreboot.org/c/coreboot/+/34599/9//COMMIT_MSG@19 PS9, Line 19: tested double `tested`
https://review.coreboot.org/c/coreboot/+/34599/9/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/34599/9/src/device/Kconfig@450 PS9, Line 450: It might not be implemented for every framebuffer initialization platform. Can we drop this now?
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/7/src/lib/coreboot_table.c@15... PS7, Line 150: if (CONFIG(BOOTSPLASH)) {
While I fully agree that that would be nice, I also think that it should not be handled now / in the […]
Sure, I was just leaving that thought, if anybody wants to take it, it should be a separate patch (series?), ofc. I might start a discussion on the ML about the API, but don't wait for me if I don't ;)
https://review.coreboot.org/c/coreboot/+/34599/9/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/34599/9/src/lib/coreboot_table.c@15... PS9, Line 152: size_t `size_t` only works if it has the same size as a pointer by coincidence. Better use `uintptr_t`.
Also, please remove the spaces around the parentheses. Then, it should also fit on a single line. (You could also use `uint8_t` to save some extra space.)
https://review.coreboot.org/c/coreboot/+/34599/9/src/lib/coreboot_table.c@15... PS9, Line 153: int unsigned int?