Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48188 )
Change subject: soc/amd: factor out SMBUS controller registers into common header ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/48188/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/smbus.h:
https://review.coreboot.org/c/coreboot/+/48188/1/src/soc/amd/common/block/in... PS1, Line 14: /* TODO: this one looks odd */ Looking at smbus_wait_until_done(), it looks intentional. I think it's probably a misleading name, however. Rather it should indicate the last transaction is complete.
https://review.coreboot.org/c/coreboot/+/48188/1/src/soc/amd/common/block/in... PS1, Line 30: (0x0 << 2) Yeah, these didn't read well before...
https://review.coreboot.org/c/coreboot/+/48188/1/src/soc/amd/picasso/southbr... File src/soc/amd/picasso/southbridge.c:
https://review.coreboot.org/c/coreboot/+/48188/1/src/soc/amd/picasso/southbr... PS1, Line 116: fch_smbus_init Beyond the scope of this patch, but this might be movable too.