[coreboot-gerrit] Change in coreboot[master]: lib/edid: Split out fill_lb_framebuffer()

[Julius Werner (Code Review)]

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.


-- 
To view, visit https://review.coreboot.org/19764
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I90634b835bd8e2d150b1c714328a5b2774d891bd
Gerrit-PatchSet: 5
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Nico Huber <nico.h at gmx.de>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner at google.com>
Gerrit-Reviewer: Nico Huber <nico.h at gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-HasComments: Yes