Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33737 )
Change subject: device/oprom/realmode: Add vbe return status support as per VBE spec 3.0 ......................................................................
Patch Set 14:
(3 comments)
Sorry for running off, I thought a little distance would be good for the moment. Just leaving some questions and thoughts. As I don't use this code, I don't care too much.
https://review.coreboot.org/c/coreboot/+/33737/14/src/device/oprom/realmode/... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/33737/14/src/device/oprom/realmode/... PS14, Line 255: default: I'm not sure about the `default`, if VBE ever gets extended or some vendor uses a non-standard code, why would we print the unsupported- mode message?
https://review.coreboot.org/c/coreboot/+/33737/14/src/device/oprom/realmode/... PS14, Line 271: X86_EAX Why use the register file here? wouldn't a local variable be enough? or is this communicating the value elsewhere?
https://review.coreboot.org/c/coreboot/+/33737/14/src/device/oprom/realmode/... PS14, Line 274: die("\nError: In %s function\n", __func__); I don't think we should die(), modesetting errors don't seem generally fatal?