coreboot-gerrit April 2022

coreboot-gerrit@coreboot.org

1 participants
5879 discussions

Change in coreboot[master]: soc/qualcomm/common: Add strict flag to clock_configure()
by Julius Werner (Code Review) 01 Apr '22

01 Apr '22

Attention is currently required from: Shelley Chen. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63289 ) Change subject: soc/qualcomm/common: Add strict flag to clock_configure() ...................................................................... Patch Set 4: (3 comments) File src/soc/qualcomm/common/include/soc/clock_common.h: https://review.coreboot.org/c/coreboot/+/63289/comment/38749325_a9be3c54 PS4, Line 151: highest nit: "higher"? File src/soc/qualcomm/sc7180/clock.c: https://review.coreboot.org/c/coreboot/+/63289/comment/d396730f_b52754c5 PS4, Line 218: &mdss_clk_cfg, 0, 0, false); nit: looking at how this works (which is a little haphazard anyway... basically just building a fake array of one element and passing 0 size to force skip the for loop) I think you wouldn't actually need to disable the strict check here, since hz == 0 matches the fake entry. So maybe you don't need the strict parameter after all? File src/soc/qualcomm/sc7280/clock.c: https://review.coreboot.org/c/coreboot/+/63289/comment/69ed7431_ead6ab1e PS4, Line 311: false Why can't these be checked? -- To view, visit https://review.coreboot.org/c/coreboot/+/63289 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9cfad7236241f4d03ff1a56683654649658b68fc Gerrit-Change-Number: 63289 Gerrit-PatchSet: 4 Gerrit-Owner: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Comment-Date: Fri, 01 Apr 2022 19:05:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: device/i2c_bus: Add detect function pointer in i2c_bus_ops
by Matt DeVillier (Code Review) 01 Apr '22

01 Apr '22

Attention is currently required from: Subrata Banik, Tim Wawrzynczak, Julius Werner, Karthik Ramasubramanian. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63209 ) Change subject: device/i2c_bus: Add detect function pointer in i2c_bus_ops ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > For at least the DW i2c driver, a 0-byte transfer just immediately returns -1, https://review. […] copy that. I feel like since this is a function with a very specific purpose, and it simplifies things for the caller, it's worth the small amount of overhead to carry it -- To view, visit https://review.coreboot.org/c/coreboot/+/63209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia8ddfb187eabec966c253b6cc8526491c99151fc Gerrit-Change-Number: 63209 Gerrit-PatchSet: 2 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 01 Apr 2022 18:50:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Makefile.inc: Add the x86 bootblock as a regular cbfs file
by Martin Roth (Code Review) 01 Apr '22

01 Apr '22

Attention is currently required from: Marshall Dawson, Julius Werner, Arthur Heymans, Kyösti Mälkki, Yu-Ping Wu, Felix Held. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56122 ) Change subject: Makefile.inc: Add the x86 bootblock as a regular cbfs file ...................................................................... Patch Set 7: (2 comments) Patchset: PS7: Overall, I think it's looking good if flashrom isn't a problem. Flashrom defaulting to looking for the platform name in the binary is a poor solution anyway. SMBIOS is probably a better, more universal method. File Makefile.inc: https://review.coreboot.org/c/coreboot/+/56122/comment/680a6299_38fa97f5 PS3, Line 840: This top aligns files of type bootblock, assuming that only x86 uses those > > > Instead of making this assumption, maybe add a Kconfig option that's selected by x86? Up to you […] You're right, in the current state, the bootblock area is no longer needed on AMD Ryzen systems because we use the compressed version inside the AMD FW area. I believe that we could actually still use it if we wanted to just by pointing the "BIOS" area in EFS to our uncompressed bootblock region. As an aside, I'd actually really like to get away from the whole amdfwtool being used to create a bin binary. Except for the additional CBFS headers, I don't see any advantage to amdfwtool's method over just adding all of the various binaries separately in CBFS, then telling a new EFS generation tool where they are. I think that would allow for a lot more flexibility. -- To view, visit https://review.coreboot.org/c/coreboot/+/56122 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4de9d7fedf1ae5a37a3310dd42eb07b44c030930 Gerrit-Change-Number: 56122 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 01 Apr 2022 18:41:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Makefile.inc: Update the master header pointer in a separate target
by Martin Roth (Code Review) 01 Apr '22

01 Apr '22

Attention is currently required from: Arthur Heymans, Julius Werner, Yu-Ping Wu. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59132 ) Change subject: Makefile.inc: Update the master header pointer in a separate target ...................................................................... Patch Set 4: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/59132/comment/43f3f75b_7a05c4a5 PS4, Line 9: The makefiles don't like cbfs file names with spaces I'm good with how it's done now, but honestly, this seems odd to me. We should be able to escape it and make it work. maybe '\\ ' so that after the 1st evaluation we get '\ ' presented to the shell? Patchset: PS4: It might be good to give some background on the cbfs master header in the commit message just so that people have a better understanding of what's going on without having to go search gerrit. (This is not meant as a criticism.) Maybe add some of the comments between you and Julius in CB:56120 It probably would have been better in the commit message of that revert, but that's too late now. It's just a little confusing going through commits and seeing the comments: "# the cbfs master header is a deprecated feature only used on x86" and "If FMAP is used anyway there is no need for a cbfs master header.", but still seeing us using it on all platforms. Again, not a big deal, just some documentation of what's going on with the master header might be useful. File Makefile.inc: https://review.coreboot.org/c/coreboot/+/59132/comment/b129c671_ed195b47 PS4, Line 1150: 32 Nit: Maybe put some of these numbers into variables? Some comments as to what's going on here might be good too. # Add a pointer to the cbfs master header in the last 4 bytes of CBFS -- To view, visit https://review.coreboot.org/c/coreboot/+/59132 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3ba01be7da1f09a8cac287751497c18cda97d293 Gerrit-Change-Number: 59132 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Fri, 01 Apr 2022 18:27:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/adl: Disable FSP debug output if !FSP_ENABLE_SERIAL_DEBUG
by Subrata Banik (Code Review) 01 Apr '22

01 Apr '22

Attention is currently required from: Tim Wawrzynczak, Arthur Heymans, Nick Vaccaro, Eric Lai. Hello build bot (Jenkins), Tim Wawrzynczak, Nick Vaccaro, Arthur Heymans, Eric Lai, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63167 to look at the new patch set (#4). Change subject: soc/intel/adl: Disable FSP debug output if !FSP_ENABLE_SERIAL_DEBUG ...................................................................... soc/intel/adl: Disable FSP debug output if !FSP_ENABLE_SERIAL_DEBUG This patch binds all FSP-M and FSP-S UPDs required for serial redirection with `FSP_ENABLE_SERIAL_DEBUG` config to allow coreboot to choose when to enable FSP debug output redirection to serial port. For example: PcdSerialDebugLevel => For controlling FSP debug level between FSP-M/S SerialDebugMrcLevel => For controllig MRC debug level. With this change FSP debug output will only be enabled when the user enables `FSP_ENABLE_SERIAL_DEBUG` from site-local config with coreboot serial image. BUG=b:225544587 TEST=Able to build and boot brya. Also, the FSP debug log is exactly the same before and with this code change. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I779c56b8b0fdebf45ea85b3b456a2d8066e26489 --- M src/soc/intel/alderlake/fsp_params.c M src/soc/intel/alderlake/romstage/fsp_params.c 2 files changed, 21 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/63167/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/63167 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I779c56b8b0fdebf45ea85b3b456a2d8066e26489 Gerrit-Change-Number: 63167 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/intel/fsp2_0: Allow coreboot to control FSP serial redirection
by Subrata Banik (Code Review) 01 Apr '22

01 Apr '22

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/ro… (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. -- To view, visit https://review.coreboot.org/c/coreboot/+/63166 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0b008ca9d4f40bfa6a989a6fd655c234f91fde65 Gerrit-Change-Number: 63166 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Fri, 01 Apr 2022 18:10:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Disable FSP debug output if !FSP_ENABLE_SERIAL_D...
by Subrata Banik (Code Review) 01 Apr '22

01 Apr '22

Attention is currently required from: Tim Wawrzynczak, Arthur Heymans, Nick Vaccaro, Eric Lai. Hello build bot (Jenkins), Tim Wawrzynczak, Nick Vaccaro, Arthur Heymans, Eric Lai, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63167 to look at the new patch set (#3). Change subject: soc/intel/alderlake: Disable FSP debug output if !FSP_ENABLE_SERIAL_DEBUG ...................................................................... soc/intel/alderlake: Disable FSP debug output if !FSP_ENABLE_SERIAL_DEBUG This patch binds all FSP-M and FSP-S UPDs required for serial redirection with `FSP_ENABLE_SERIAL_DEBUG` config to allow coreboot to choose when to enable FSP debug output redirection to serial port. For example: PcdSerialDebugLevel => For controlling FSP debug level between FSP-M/S SerialDebugMrcLevel => For controllig MRC debug level. With this change FSP debug output will only be enabled when the user enables `FSP_ENABLE_SERIAL_DEBUG` from site-local config with coreboot serial image. BUG=b:225544587 TEST=Able to build and boot brya. Also, the FSP debug log is exactly the same before and with this code change. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I779c56b8b0fdebf45ea85b3b456a2d8066e26489 --- M src/soc/intel/alderlake/fsp_params.c M src/soc/intel/alderlake/romstage/fsp_params.c 2 files changed, 21 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/63167/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/63167 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I779c56b8b0fdebf45ea85b3b456a2d8066e26489 Gerrit-Change-Number: 63167 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/intel/fsp2_0: Allow coreboot to control FSP serial redirection
by Subrata Banik (Code Review) 01 Apr '22

01 Apr '22

Attention is currently required from: Paul Menzel, Tim Wawrzynczak, Angel Pons, Nick Vaccaro, Arthur Heymans, Michael Niewöhner, Lean Sheng Tan, Andrey Petrov. Hello build bot (Jenkins), Tim Wawrzynczak, Angel Pons, Nick Vaccaro, Arthur Heymans, Eric Lai, Lean Sheng Tan, Andrey Petrov, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63166 to look at the new patch set (#2). Change subject: drivers/intel/fsp2_0: Allow coreboot to control FSP serial redirection ...................................................................... drivers/intel/fsp2_0: Allow coreboot to control FSP serial redirection Commit 3ba6f8cdf (drivers/intel/fsp2_0: Add native implementation for FSP Debug Handler) implements a native FSP debug handler. However, coreboot still can't control when to redirect FSP debug output to the serial console, i.e., at present, integrating a FSP debug binary is enough to output FSP serial messages irrespective of whether user is intended to see FSP debug log. coreboot needs additional mechanism to control FSP debug binary to redirect debug messages over serial port. This patch introduces a config `FSP_ENABLE_SERIAL_DEBUG` to control the FSP debug output, user to enable this config from site-local config file in case like to override the default FSP serial redirection behaviour in more controlled way from coreboot. There could be scenarios as below: Scenario 1: coreboot release image integrated with the FSP debug binaries, is capable of redirecting to the serial console, but coreboot decides to override the config as below to skip FSP debug output redirection to the serial port. `#`FSP Serial console disabled by default (do not remove) `#`CONFIG_FSP_ENABLE_SERIAL_DEBUG is not set Scenario 2: For coreboot serial image with FSP debug binaries integrated but coreboot decides to skip FSP debug output redirection to the serial port. `#`FSP Serial console disabled by default (do not remove) `#`CONFIG_FSP_ENABLE_SERIAL_DEBUG is not set CONFIG_CONSOLE_SERIAL=y CONFIG_CONSOLE_SERIAL_115200=y CONFIG_UART_DEBUG=y CONFIG_UART_FOR_CONSOLE=0 Scenario 3: The final image could be a coreboot serial image with FSP serial redirection enabled to output to the serial port. CONFIG_FSP_ENABLE_SERIAL_DEBUG=y CONFIG_CONSOLE_SERIAL=y CONFIG_CONSOLE_SERIAL_115200=y CONFIG_UART_DEBUG=y CONFIG_UART_FOR_CONSOLE=0 BUG=b:227151510 TEST=Able to build and boot google/redrix with all scenarios between #1--#3 and able to meet the expectation as mentioned above. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I0b008ca9d4f40bfa6a989a6fd655c234f91fde65 --- M src/drivers/intel/fsp2_0/Kconfig 1 file changed, 12 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/63166/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/63166 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0b008ca9d4f40bfa6a989a6fd655c234f91fde65 Gerrit-Change-Number: 63166 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: [NEEDTEST]soc/amd/non-car: Don't add bootblock cbfs file
by build bot (Jenkins) (Code Review) 01 Apr '22

01 Apr '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Fred Reitberger, Felix Held. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63226 ) Change subject: [NEEDTEST]soc/amd/non-car: Don't add bootblock cbfs file ...................................................................... Patch Set 3: Verified+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/63226 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I72f0092e0e3628b388f6da6a417c2857a510b187 Gerrit-Change-Number: 63226 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 01 Apr 2022 17:19:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/apl: Drop cbfs bootblock
by build bot (Jenkins) (Code Review) 01 Apr '22

01 Apr '22

Attention is currently required from: Andrey Petrov. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63225 ) Change subject: soc/intel/apl: Drop cbfs bootblock ...................................................................... Patch Set 3: Verified+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/63225 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e70e080765dd7306074a8cf71c8795b8fbbb8a2 Gerrit-Change-Number: 63225 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Fri, 01 Apr 2022 17:10:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2022