Martin Roth 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. Could you check?
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.h? If it makes sense to move these, does it make sense just go get rid of smbus.h and move everything here?
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. I know that you didn't change it here, but is there a better way? Maybe in a follow-on patch?
OTOH, I don't even know how much these are being used since we're not reading the SPD from the smbus, so maybe it's not worth optimizing further right now.