David Guckian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41609 )
Change subject: soc/intel/denverton_ns/bootblock: Fix 32-bit RW PCI_BASE_ADDRESS_0 register ......................................................................
Patch Set 1: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/41609/1/src/soc/intel/denverton_ns/... File src/soc/intel/denverton_ns/bootblock/uart.c:
https://review.coreboot.org/c/coreboot/+/41609/1/src/soc/intel/denverton_ns/... PS1, Line 27: mmio_base
Why is this called `mmio` if we configure I/O ports?
I agree its a poor choice of name used here.
https://review.coreboot.org/c/coreboot/+/41609/1/src/soc/intel/denverton_ns/... PS1, Line 28: pci_write_config16(uart_dev, PCI_BASE_ADDRESS_0, reg16);
Why is this a read-modify-write?
I don't know the reason why this was done, possibly following an example from another SOC.
https://review.coreboot.org/c/coreboot/+/41609/1/src/soc/intel/denverton_ns/... PS1, Line 169: early_config_gpio();
Why do we unconditionally change pad configurations?
I don't know the history for doing this here. I imagine its being done as a precaution in case the GPIOs were set differently in softstraps.