Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43307 )
Change subject: soc/amd/common: Refactor and consolidate code for spi base ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43307/6/src/soc/amd/common/block/sp... File src/soc/amd/common/block/spi/fch_spi_ctrl.c:
https://review.coreboot.org/c/coreboot/+/43307/6/src/soc/amd/common/block/sp... PS6, Line 35: return read8((void *)(spi_base + reg));
Done.
I would not call it slower. For some compilation units, it may double the .text section size.
https://review.coreboot.org/c/coreboot/+/43307/6/src/soc/amd/common/block/sp... PS6, Line 35: spi_base
Personally, I don't think this is the same thing, because the ACPIMMIO addresses aren't set by PCI B […]
It's exactly the same question; Do we guarantee the base variable is initialized prior to calling the MMIO accessors. For ENV_X86 and ACPIMMIO guarantee comes from const variable placed in .text, for ENV_ARM and ACPIMMIO it comes from the program flow.
You are now introducing third approach in which such guarantee does not exist. I just think it was better and cleaner without the function call. Then again, I also feel that access to such hardware (here SPI registers) is better restricted to single compilation unit.