Hi
In 2016 'uart_pci_addr' was added to the coreboot table entry for serial devices. (https://review.coreboot.org/c/coreboot/+/14609) It was done for the Intel Quark platform which has its uart on a PCI device like other Intel hardware. Right now only Quark sets this to a non zero value using an awkwardly defined Kconfig parameter: CONFIG_UART_PCI_ADDR. It looks like only tianocore uses this and it's pretty much a NOOP used only to get the VID/DID of the PCI device.
Should we update tianocore and just drop this for the lb_table? Most other payloads don't even have this struct entry updated to contain this entry... Now our codebase has awkward code with "serial.uart_pci_addr = CONFIG_UART_PCI_ADDR;" on a lot of platforms that don't even feature PCI and there is no real use case as far as I can tell.
Do any of your payloads use this in a meaningful way? If not, can we just drop it?
Kind regards
Arthur
yes, and this is a perfect example of how one platform, which is not used, can cause unneeded features to persist and make the codebase more complex than it needs to be.
I support dropping it.
On Sun, Apr 17, 2022 at 2:12 AM Arthur Heymans arthur@aheymans.xyz wrote:
Hi
In 2016 'uart_pci_addr' was added to the coreboot table entry for serial devices. (https://review.coreboot.org/c/coreboot/+/14609) It was done for the Intel Quark platform which has its uart on a PCI device like other Intel hardware. Right now only Quark sets this to a non zero value using an awkwardly defined Kconfig parameter: CONFIG_UART_PCI_ADDR. It looks like only tianocore uses this and it's pretty much a NOOP used only to get the VID/DID of the PCI device.
Should we update tianocore and just drop this for the lb_table? Most other payloads don't even have this struct entry updated to contain this entry... Now our codebase has awkward code with "serial.uart_pci_addr = CONFIG_UART_PCI_ADDR;" on a lot of platforms that don't even feature PCI and there is no real use case as far as I can tell.
Do any of your payloads use this in a meaningful way? If not, can we just drop it?
Kind regards
Arthur _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
Wow, that one chip-specific kludge impacts almost 10 source files.
On Sun, Apr 17, 2022 at 11:15 AM ron minnich rminnich@gmail.com wrote:
yes, and this is a perfect example of how one platform, which is not used, can cause unneeded features to persist and make the codebase more complex than it needs to be.
I support dropping it.
On Sun, Apr 17, 2022 at 2:12 AM Arthur Heymans arthur@aheymans.xyz wrote:
Hi
In 2016 'uart_pci_addr' was added to the coreboot table entry for serial devices. (https://review.coreboot.org/c/coreboot/+/14609) It was done for the Intel Quark platform which has its uart on a PCI device like other Intel hardware. Right now only Quark sets this to a non zero value using an awkwardly defined Kconfig parameter: CONFIG_UART_PCI_ADDR. It looks like only tianocore uses this and it's pretty much a NOOP used only to get the VID/DID of the PCI device.
Should we update tianocore and just drop this for the lb_table? Most other payloads don't even have this struct entry updated to contain this entry... Now our codebase has awkward code with "serial.uart_pci_addr = CONFIG_UART_PCI_ADDR;" on a lot of platforms that don't even feature PCI and there is no real use case as far as I can tell.
Do any of your payloads use this in a meaningful way? If not, can we just drop it?
Kind regards
Arthur _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
-
Arthur Heymans -
ron minnich