Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46093 )
Change subject: soc/intel/xeon_sp: Move debug macros to console/debug.h ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46093/1/src/include/console/debug.h File src/include/console/debug.h:
https://review.coreboot.org/c/coreboot/+/46093/1/src/include/console/debug.h... PS1, Line 8: #define LOG_MEM_RESOURCE(type, dev, index, base_kb, size_kb) \
Should we use a Kconfig option to enable/disable these macros? Having __LINE__ in them breaks reprod […]
Maybe an ifdef for DEBUG_SPEW level. It wouldn't be included in a build that turned up the debug level after the build. I'd also be ok with removing __LINE__.
Also, should all of this file reside here with the use of dev_path? The FUNC stuff is general enough, but maybe the RESOURCE stuff should go in pci?
https://review.coreboot.org/c/coreboot/+/46093/1/src/soc/intel/xeon_sp/cpx/c... File src/soc/intel/xeon_sp/cpx/chip.c:
https://review.coreboot.org/c/coreboot/+/46093/1/src/soc/intel/xeon_sp/cpx/c... PS1, Line 4: : #include <console/debug.h>
I agree, consistency is important.
I agree with Marshall that we should include all that is required. I also agree that debug.h and console.h should be included in the source files that have printks (which is probably all of them).