Andrey Petrov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34683 )
Change subject: soc/intel/fsp_broadwell_de: Enable early integrated UART ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34683/5/src/soc/intel/fsp_broadwell... File src/soc/intel/fsp_broadwell_de/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/34683/5/src/soc/intel/fsp_broadwell... PS5, Line 71: uint32_t reset_sts = pci_mmio_read_config32(ubox_dev, UBOX_SC_RESET_STATUS);
Please use the wrapper function pci_read_config32 instead.
like discussed on other patch, some of these registers can't be accessed over IO. So wrapper function would silently fail which we do not want. I think we have to explicitly do mmio variant here.
https://review.coreboot.org/c/coreboot/+/34683/5/src/soc/intel/fsp_broadwell... PS5, Line 78: ubox_uart_en &= ~(UBOX_UART_ENABLE_PORT0 | UBOX_UART_ENABLE_PORT1);
only touch the bits for the selected port to allow this function to be called twice, once for each u […]
Ack