Change in coreboot[master]: drivers/intel/fsp2_0: Allow coreboot to control FSP serial redirection
Attention is currently required from: Paul Menzel, Tim Wawrzynczak, Angel Pons, Nick Vaccaro, Arthur Heymans, Michael Niewöhner, Lean Sheng Tan, Andrey Petrov. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63166 )
Change subject: drivers/intel/fsp2_0: Allow coreboot to control FSP serial redirection ......................................................................
Patch Set 2:
(16 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/63166/comment/347f1fc3_71ef3fb7 PS1, Line 9: coreboot has implemented native FSP debug handler with commit hash : 3ba6f8cdf (drivers/intel/fsp2_0: Add native implementation for FSP : Debug Handler).
Maybe: […]
Ack
https://review.coreboot.org/c/coreboot/+/63166/comment/b16943c1_63347338 PS1, Line 12:
One space.
Ack
https://review.coreboot.org/c/coreboot/+/63166/comment/b73e8e45_9e6b3f17 PS1, Line 12: However, coreboot still can't control when to redirect FSP debug
Please add a blank line above to separate paragraphs.
Ack
https://review.coreboot.org/c/coreboot/+/63166/comment/8290614e_a8e89e58 PS1, Line 15: wish
wishes
Ack
https://review.coreboot.org/c/coreboot/+/63166/comment/35f4199f_291badd4 PS1, Line 15: to see FSP debug log : with all the coreboot serial image
Please rephrase, as I do not understand it.
Ack
https://review.coreboot.org/c/coreboot/+/63166/comment/9df80cb1_b99486d0 PS1, Line 19: an
a
Ack
https://review.coreboot.org/c/coreboot/+/63166/comment/3deb2de8_0f40be82 PS1, Line 21: incase
in case
Ack
https://review.coreboot.org/c/coreboot/+/63166/comment/4d80850f_a4e77cb9 PS1, Line 26: Non-serial coreboot image
Maybe mention console, or rephrase.
Ack
https://review.coreboot.org/c/coreboot/+/63166/comment/a48b196c_99e6c799 PS1, Line 55: Redrix
google/redrix
Ack
https://review.coreboot.org/c/coreboot/+/63166/comment/77815295_34237d13 PS1, Line 55: #1- #3
#1–#3 or #1--#3
Ack
https://review.coreboot.org/c/coreboot/+/63166/comment/5e744695_da4fae64 PS1, Line 57:
Can FSP messages be put into CBMEM console or is only serial supported?
yes, it can also resides into cbmem console, please refer to https://review.coreboot.org/c/coreboot/+/63009/4//COMMIT_MSG
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/63166/comment/5f891449_e7f3f50f PS1, Line 362: HAVE_FSP_DEBUG
Maybe: `FSP_ENABLE_SERIAL_DEBUG`
Ack
https://review.coreboot.org/c/coreboot/+/63166/comment/94bbd0d3_88dc5c49 PS1, Line 363: Have FSP debug binaries to get the console output
Maybe: Output FSP debug messages on serial console
Ack
https://review.coreboot.org/c/coreboot/+/63166/comment/3ab7624e_7cb5bbf3 PS1, Line 370: Select this option from site-local
I could imagine this is because selecting which FSP image to use is not present in upstream corebo […]
Ack
https://review.coreboot.org/c/coreboot/+/63166/comment/a061df63_fc5c09f2 PS1, Line 371: serial debug
Without “debug”?
Ack
https://review.coreboot.org/c/coreboot/+/63166/comment/4b1a61b6_75459c19 PS1, Line 372: binaries.
What happens, when you have non-FSP debug builds? Is there an error, or just no messages are send to serial console?
there could be two cases: 1. When you have FSP non-serial image integrated into coreboot but wrongly coreboot still selects `FSP_ENABLE_SERIAL_DEBUG`, in that case, FSP won't output any msg over serial due to non-FSP debug .
2. When you have FSP non-serial image integrated into coreboot and coreboot disable serial redirection as mentioned here
https://review.coreboot.org/c/coreboot/+/63167/2/src/soc/intel/alderlake/rom... (the `else` section)
/* Disable Serial debug message */ m_cfg->PcdSerialDebugLevel = 0; /* Disable MRC debug message */ m_cfg->SerialDebugMrcLevel = 0;
so, in short, there won't be any error as you don't know what type of FSP binary is being integrated and only can act based on coreboot FSP_ENABLE_SERIAL_DEBUG kconfig to disable the serial console to ensure no serial msg.
Hope this clarifies your question and sorry for long answer.
Updated the help text to reflect the non-FSP debug scenario as well.
-
Subrata Banik (Code Review)