Martin Roth 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 6:
(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 33: static inline uint8_t spi_read8(uint8_t reg)
You ack'd the inline and did not address it :(
Yes, I was acknowledging the comment. I would have marked it 'done' if I had done it. As far as I can tell, it has nothing to do with this patch, especially since you wanted me to change the code back to using a simple variable, which I did. :)
https://review.coreboot.org/c/coreboot/+/43307/6/src/soc/amd/common/block/sp... PS6, Line 35: spi_base
My comments were mostly about the asymmetry of MMIO accessors. […]
Personally, I don't think this is the same thing, because the ACPIMMIO addresses aren't set by PCI BARs, where the LPC base is. To me that's a bug difference. I did absolutely consider simply hardcoding it and making the assumption that nothing would ever update it, but that didn't seem like it was the right thing to do.