Attention is currently required from: Jason Glenesk, Nico Huber, Raul Rangel, Marshall Dawson, Felix Held. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51750 )
Change subject: soc/amd/common: Add ESPI_DEBUG_PRINT macro & Kconfig option ......................................................................
Patch Set 3:
(4 comments)
Patchset:
PS3:
What's the reason to hide these messages by default?
I want to add a bunch more messages with error reporting. This is just the start of that.
This option is the same as the Kconfig options for debugging SMIs, debugging SPI and everything else in the debug section of Kconfig. Yes, we could enable everything by default for the "spew" setting, but it's generally not needed, and would overwhelm the logfile.
File src/mainboard/google/guybrush/Kconfig:
https://review.coreboot.org/c/coreboot/+/51750/comment/82703e91_581ec8fa PS3, Line 29: select DEBUG_ESPI
User-visible options shouldn't be selected. You can also override the default.
Sorry, this wasn't meant to go in - I'll remove it.
File src/soc/amd/common/block/include/amdblocks/espi.h:
https://review.coreboot.org/c/coreboot/+/51750/comment/7a7563b8_8b5e9e80 PS3, Line 13: #endif
I guess we could always have the debug code build tested with something […]
I don't see how this is any different. The printk will still be removed from the build unless ESPI_DEBUG is enabled.
I don't have any objection to the change, but if the goal is build testing, then I don't see how the change helps.
File src/soc/amd/common/block/lpc/Kconfig:
https://review.coreboot.org/c/coreboot/+/51750/comment/fd6652d0_c931bc55 PS3, Line 24: DEBUG_ESPI Maybe this should be changed to ESPI_DEBUG, the same as the macro
-
Martin Roth (Code Review)