Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34914 )
Change subject: soc/amd/common: Add AcpiMmio access for SMBus PCI device ......................................................................
Patch Set 6:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34914/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34914/4//COMMIT_MSG@9 PS4, Line 9: 0xfed80000
0xfed80a00
+a00 is MMIO for controlling the bus.
https://review.coreboot.org/c/coreboot/+/34914/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34914/5//COMMIT_MSG@7 PS5, Line 7: SMBus
SMBus access through MMIO
That's a fixed MMIO option to the controller instead of relying on the variable I/O locations. This is the PCI device itself, not the registers for talking to the bus.
And I believe you mentioned UART can also be accessed.
No, there's a register for accessing a handful of settings that apply to the UART at D14F0xFC. It's a register in the PCI device config space, not accessing a UART itself.
https://review.coreboot.org/c/coreboot/+/34914/5//COMMIT_MSG@9 PS5, Line 9: PCI
Which device/function?
Done
https://review.coreboot.org/c/coreboot/+/34914/4/src/soc/amd/common/block/ac... File src/soc/amd/common/block/acpimmio/mmio_util.c:
https://review.coreboot.org/c/coreboot/+/34914/4/src/soc/amd/common/block/ac... PS4, Line 69: 0xfed80000
0xfed80a00
fed80000 is correct
https://review.coreboot.org/c/coreboot/+/34914/4/src/soc/amd/common/block/ac... PS4, Line 73: ACPIMMIO_SM_PCI_BASE
Why not ACPIMMIO_SMB_PCI_BASE? "SM" could be confused with system management (SMM).
I don't think it's very confusing. The very next group is smi_read/write/n using ACPI_SMI_BASE. I can't think that there's a PCI-based SMM bank of registers we'd be accessing.