Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45405 )
Change subject: drivers/uart: Override uart base address ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45405/2/src/drivers/uart/uart8250io... File src/drivers/uart/uart8250io.c:
https://review.coreboot.org/c/coreboot/+/45405/2/src/drivers/uart/uart8250io... PS2, Line 78: static const unsigned int bases[] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
As said in another patch, no one said that `get_option` should be restricted to CMOS values only. It can be expanded to use VPD settings as well.
As I mentioned in https://review.coreboot.org/c/coreboot/+/45408/3/src/mainboard/ocp/deltalake..., I think we should be careful with using VPD for things it wasn't designed for. VPD was not designed to be efficient in pre-RAM stages. It seems a particularly bad idea to use it to configure something like the UART which is basically the very first thing coreboot needs to initialize on boot. I think(?) the CMOS option table has way less dependencies and is generally a lot "lighter weight", so it seems better suited for this sort of stuff. (That's also why I'm not sure expanding get_option() to VPD is a good idea because it makes it hard to factor in these kinds of differences when it's all merged into one framework. Something that would make get_option() unconditionally try to read VPD on every call just in case that particular option is defined in there would probably be a bad idea.)