Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29258 )
Change subject: soc/amd/stoneyridge: Access SMBUS through MMIO ......................................................................
Patch Set 12:
(3 comments)
https://review.coreboot.org/#/c/29258/12/src/soc/amd/stoneyridge/include/soc... File src/soc/amd/stoneyridge/include/soc/iomap.h:
https://review.coreboot.org/#/c/29258/12/src/soc/amd/stoneyridge/include/soc... PS12, Line 42: 0xfed80a00
Not sure if there's actually an extra space here or if it's a gerrit artifact. […]
gerrit artifact.
https://review.coreboot.org/#/c/29258/12/src/soc/amd/stoneyridge/include/soc... File src/soc/amd/stoneyridge/include/soc/southbridge.h:
https://review.coreboot.org/#/c/29258/12/src/soc/amd/stoneyridge/include/soc... PS12, Line 116: /* SMBUS MMIO offsets 0xfed80a00 */
Why did these get moved out of smbus. […]
That was Marshall's request. His reasoning was that as it's now MMIO related, it should be placed under the southbridge.h were all MMIO related southbridge.h are. the remaining smbus.h are NOT MMIO related.
https://review.coreboot.org/#/c/29258/12/src/soc/amd/stoneyridge/sm.c File src/soc/amd/stoneyridge/sm.c:
https://review.coreboot.org/#/c/29258/12/src/soc/amd/stoneyridge/sm.c@64 PS12, Line 64: get_sm_mmio(dev)
It seems horribly inefficient to have to do this every time. […]
The code being called is static on the same file. That said, it's 5 lines plus 2 pointers, so I believe it's optimized already. The procedure will return either the SMBUS or the ASF MMIO base address. It's definitely not being used for the SMBUS. I'm not sure if it's being used for ASF, though I don't think so.
-
Richard Spiegel (Code Review)