Julius Werner has posted comments on this change. ( https://review.coreboot.org/19764 )
Change subject: lib/edid: Split out fill_lb_framebuffer() ......................................................................
Patch Set 5:
(1 comment)
I don't think we want every driver to implement that callback separately. I brought that up hoping for less boilerplate, not more.
https://review.coreboot.org/#/c/19764/5/src/lib/Kconfig File src/lib/Kconfig:
Line 4: Include fill_lb_framebuffer() implementation that caches an EDID.
There already was a separate Kconfig (NATIVE_VGA_INIT_USE_EDID)
Not quite sure I understand the problem. Are you saying that the linked patch removed the global lb_framebuffer symbol without noticing that there were other implementations of it? I mean, that's bad, of course, and obviously people should always check for that when they change a weak function.
But I don't really see how it applies to this case. You have a symbol that's implemented in a dozen different chipsets anyway. If somebody rewrote the code that called fill_lb_framebuffer() to not call that anymore you'd get a problem. Whether the implementation is weak or guarded by a Kconfig doesn't really make a difference... you can still have a board that has the Kconfig set and will compile its fill_lb_framebuffer() implementation even if no code is there to call it anymore... that doesn't result in a linker error.