HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39178 )
Change subject: sb/amd: Move smbus to common place ......................................................................
Patch Set 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/39178/7/src/southbridge/amd/agesa/h... File src/southbridge/amd/agesa/hudson/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/39178/7/src/southbridge/amd/agesa/h... PS7, Line 5: romstage-y += smbus_spd.c ../../common/smbus.c
I would avoid using `..` here. […]
why I use an option that will be selected every time?
https://review.coreboot.org/c/coreboot/+/39178/7/src/southbridge/amd/agesa/h... File src/southbridge/amd/agesa/hudson/hudson.c:
https://review.coreboot.org/c/coreboot/+/39178/7/src/southbridge/amd/agesa/h... PS7, Line 26: #include "southbridge/amd/common/smbus.h"
Shouldn't this be using <> ?
Done
https://review.coreboot.org/c/coreboot/+/39178/7/src/southbridge/amd/cimx/sb... File src/southbridge/amd/cimx/sb800/smbus.c:
PS7:
Is this file the exact same thing as sb/amd/common/smbus. […]
yes it is, if we remove the BIOS_DEBUG printk ...
https://review.coreboot.org/c/coreboot/+/39178/7/src/southbridge/amd/common/... File src/southbridge/amd/common/smbus.h:
https://review.coreboot.org/c/coreboot/+/39178/7/src/southbridge/amd/common/... PS7, Line 16: _
I would reorder the elements so that it matches the path. Also, why only one underscore? […]
Done