coreboot-gerrit April 2022

coreboot-gerrit@coreboot.org

1 participants
5879 discussions

Change in coreboot[master]: [WIP]util/cbfstool: Rewrite trampoline in C
by Paul Menzel (Code Review) 19 Apr '22

19 Apr '22

Attention is currently required from: Arthur Heymans. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63716 ) Change subject: [WIP]util/cbfstool: Rewrite trampoline in C ...................................................................... Patch Set 1: (2 comments) File util/cbfstool/x86_linux_trampoline.c: https://review.coreboot.org/c/coreboot/+/63716/comment/d280686c_43abd21b PS1, Line 62: Setup Set up https://review.coreboot.org/c/coreboot/+/63716/comment/73f6f777_9dfad4e6 PS1, Line 63: ** : ** Flat code segment descriptor: : ** selector: 0x10 : ** base : 0x00000000 : ** limit : 0xFFFFFFFF : ** type : code, execute, read : ** : ** Flat data segment descriptor: : ** selector: 0x18 : ** base : 0x00000000 : ** limit : 0xFFFFFFFF : ** type : data, read/write Remove the first *? -- To view, visit https://review.coreboot.org/c/coreboot/+/63716 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibbd7a5ecd225edf87f451a82ff4cbe9fea522a89 Gerrit-Change-Number: 63716 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 19 Apr 2022 15:33:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/jasperlake: Add a workaround for cnvi
by Paul Menzel (Code Review) 19 Apr '22

19 Apr '22

Attention is currently required from: Jamie Chen, Tim Wawrzynczak, Karthik Ramasubramanian. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63675 ) Change subject: soc/intel/jasperlake: Add a workaround for cnvi ...................................................................... Patch Set 3: (8 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63675/comment/53b5a48e_7a90b4e7 PS3, Line 7: soc/intel/jasperlake: Add a workaround for cnvi Maybe: > CNVI: Enable fewer wakeups to decrease high SoC power consumption https://review.coreboot.org/c/coreboot/+/63675/comment/b7d5ced2_7235da20 PS3, Line 9: add a workaround to mitigate : the higher SoC power consumption in S0iX … work around higher SoC power consumption in S0iX, when CNVI has background activity. https://review.coreboot.org/c/coreboot/+/63675/comment/721c5043_dc3e1f8a PS3, Line 15: Those settings are correct. Please add the exact power consumption values. File src/soc/intel/jasperlake/chip.h: https://review.coreboot.org/c/coreboot/+/63675/comment/420fec1d_b7a83e22 PS3, Line 430: AC9560(JfP2) Please add a space before the (. https://review.coreboot.org/c/coreboot/+/63675/comment/15ba647e_34e98e92 PS3, Line 430: chipsets : Please remove the space before the colon. https://review.coreboot.org/c/coreboot/+/63675/comment/74e279ef_103df6ac PS3, Line 431: AX201(HrP2) Ditto. https://review.coreboot.org/c/coreboot/+/63675/comment/8ae36e16_14c7827c PS3, Line 433: 1: Enabled(fewer wakes, lower power); 0: Disabled(more wakes, higher power) Please use true/false, and add a space before the (. https://review.coreboot.org/c/coreboot/+/63675/comment/30458348_31c6ed3c PS3, Line 435: bool cnvi_s0ix_wa; Add “power usage” or something similar to the name? -- To view, visit https://review.coreboot.org/c/coreboot/+/63675 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I56439a406547e2ee1e47d34be14ecc9a8df04693 Gerrit-Change-Number: 63675 Gerrit-PatchSet: 3 Gerrit-Owner: Jamie Chen <jamie.chen(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kane Chen <kane.chen(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Simon Yang <simon1.yang(a)intel.com> Gerrit-Attention: Jamie Chen <jamie.chen(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Tue, 19 Apr 2022 15:32:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: Reset XHCI controller while preparing for S5
by Subrata Banik (Code Review) 19 Apr '22

19 Apr '22

Attention is currently required from: Nico Huber, Maulik V Vaghela, Paul Menzel, Sridhar Siricilla, Arthur Heymans, Nick Vaccaro, Eric Lai, Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63552 ) Change subject: mb/google/brya: Reset XHCI controller while preparing for S5 ...................................................................... Patch Set 6: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/63552/comment/c3138fc7_e9e4fbb7 PS4, Line 37: SMI handler > > > Sorry, thinking error on my part. coreboot of course does not run anymore during shutdown (besides the installed handlers). > > > > > > But, why can’t this be worked around/fixed in the OS driver? > > > > fixing in FW is comparatively easier 😊 and could apply MB specific W/As as well. > How so? - the fixes in FW are effective in timely manner where one can't land those changes easily in kernel. - FW qualification is less costly compared to OS. - we could add device specific W/A as we have mainboard smi handler (originally designed for the same). > You had to add a whole new API function (and should have added > a new file for it, AFAICS; also, you use PCH_DEV_XHCI which contains a > workaround for NULL pointers which should be fixed). In Linux, OTOH, > you probably just need to set a correct quirk flag. > > Doing it in SMM makes OS developers expect that they don't have to > handle it, even provides them an excuse not to do it, and that weakens > the ecosystem. IIRC, SMI handlers were originally added to coreboot > because of a similar story. Now we risk to repeat the 10y+ old mistake. Getting rid of SMI is really a task IMO. -- To view, visit https://review.coreboot.org/c/coreboot/+/63552 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibf06a64f055a0cee3659b410652082f31e18e149 Gerrit-Change-Number: 63552 Gerrit-PatchSet: 6 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Gerrit-CC: Kevin Chang <kevin.chang(a)lcfc.corp-partner.google.com> Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sunshine Chao <sunshine.chao(a)lcfc.corp-partner.google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> 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-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 19 Apr 2022 15:30:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: tests: update CMocka to stable-1.1
by Paul Menzel (Code Review) 19 Apr '22

19 Apr '22

Attention is currently required from: Jakub Czapiga. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63636 ) Change subject: tests: update CMocka to stable-1.1 ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/63636/comment/771943ce_18549bb3 PS4, Line 10: funtion function -- To view, visit https://review.coreboot.org/c/coreboot/+/63636 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I20ebd15324a21c17cccd2976ae9c3f86b040426d Gerrit-Change-Number: 63636 Gerrit-PatchSet: 4 Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jan Dabros <jsd(a)semihalf.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Comment-Date: Tue, 19 Apr 2022 15:25:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/jasperlake: Add a workaround for cnvi
by Subrata Banik (Code Review) 19 Apr '22

19 Apr '22

Attention is currently required from: Jamie Chen, Tim Wawrzynczak, Karthik Ramasubramanian. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63675 ) Change subject: soc/intel/jasperlake: Add a workaround for cnvi ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File src/soc/intel/jasperlake/finalize.c: https://review.coreboot.org/c/coreboot/+/63675/comment/569d1bff_193c4d0e PS3, Line 65: (1 << 0) possible to create a macro? -- To view, visit https://review.coreboot.org/c/coreboot/+/63675 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I56439a406547e2ee1e47d34be14ecc9a8df04693 Gerrit-Change-Number: 63675 Gerrit-PatchSet: 3 Gerrit-Owner: Jamie Chen <jamie.chen(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kane Chen <kane.chen(a)intel.com> Gerrit-CC: Simon Yang <simon1.yang(a)intel.com> Gerrit-Attention: Jamie Chen <jamie.chen(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Tue, 19 Apr 2022 15:11:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/jasperlake: Add a workaround for cnvi
by Jamie Chen (Code Review) 19 Apr '22

19 Apr '22

Attention is currently required from: Subrata Banik, Tim Wawrzynczak, Karthik Ramasubramanian. Jamie Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63675 ) Change subject: soc/intel/jasperlake: Add a workaround for cnvi ...................................................................... Patch Set 3: (3 comments) File src/soc/intel/jasperlake/chip.h: https://review.coreboot.org/c/coreboot/+/63675/comment/8b91f5d5_9a477564 PS1, Line 424: /* : * This is a workaround to mitigate higher SoC power consumption in S0ix : * when the CNVI has background activity. : * : * This setting can only be enabled if xtalsdqdis (Bit 22) in cppmvric1 : * register is 1. : * 1: Enabled ; 0: Disabled : */ : bool cnvi_s0ix_wa; > Can we also please mention […] Done File src/soc/intel/jasperlake/finalize.c: https://review.coreboot.org/c/coreboot/+/63675/comment/5e51a888_f7539b54 PS2, Line 66: write32 > setbits32(pmcbase + CPPMVRIC2, CNVIVNNAONREQQDIS); Done https://review.coreboot.org/c/coreboot/+/63675/comment/cd7c9902_0e4c14e9 PS2, Line 70: write32 > clearbits32 Done -- To view, visit https://review.coreboot.org/c/coreboot/+/63675 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I56439a406547e2ee1e47d34be14ecc9a8df04693 Gerrit-Change-Number: 63675 Gerrit-PatchSet: 3 Gerrit-Owner: Jamie Chen <jamie.chen(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kane Chen <kane.chen(a)intel.com> Gerrit-CC: Simon Yang <simon1.yang(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Tue, 19 Apr 2022 15:08:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/jasperlake: Add a workaround for cnvi
by Jamie Chen (Code Review) 19 Apr '22

19 Apr '22

Attention is currently required from: Jamie Chen, Tim Wawrzynczak, Karthik Ramasubramanian. Hello build bot (Jenkins), Subrata Banik, Tim Wawrzynczak, Tim Wawrzynczak, Karthik Ramasubramanian, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63675 to look at the new patch set (#3). Change subject: soc/intel/jasperlake: Add a workaround for cnvi ...................................................................... soc/intel/jasperlake: Add a workaround for cnvi According to Intel TA#724456, add a workaround to mitigate the higher SoC power consumption in S0iX when CNVI has background activity. BUG=b:201263040 TEST=Turn on this setting and build and verify on Drawcia. Those settings are correct. Change-Id: I56439a406547e2ee1e47d34be14ecc9a8df04693 Signed-off-by: Jamie Chen <jamie.chen(a)intel.com> --- M src/soc/intel/jasperlake/chip.h M src/soc/intel/jasperlake/finalize.c M src/soc/intel/jasperlake/include/soc/pmc.h 3 files changed, 22 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/63675/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/63675 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I56439a406547e2ee1e47d34be14ecc9a8df04693 Gerrit-Change-Number: 63675 Gerrit-PatchSet: 3 Gerrit-Owner: Jamie Chen <jamie.chen(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kane Chen <kane.chen(a)intel.com> Gerrit-CC: Simon Yang <simon1.yang(a)intel.com> Gerrit-Attention: Jamie Chen <jamie.chen(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/brya: Reset XHCI controller while preparing for S5
by Nico Huber (Code Review) 19 Apr '22

19 Apr '22

Attention is currently required from: Maulik V Vaghela, Paul Menzel, Sridhar Siricilla, Arthur Heymans, Nick Vaccaro, Eric Lai, Felix Held. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63552 ) Change subject: mb/google/brya: Reset XHCI controller while preparing for S5 ...................................................................... Patch Set 6: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/63552/comment/5ad9a7ad_b4b9c9bb PS4, Line 37: SMI handler > > Sorry, thinking error on my part. coreboot of course does not run anymore during shutdown (besides the installed handlers). > > > > But, why can’t this be worked around/fixed in the OS driver? > > fixing in FW is comparatively easier 😊 How so? You had to add a whole new API function (and should have added a new file for it, AFAICS; also, you use PCH_DEV_XHCI which contains a workaround for NULL pointers which should be fixed). In Linux, OTOH, you probably just need to set a correct quirk flag. Doing it in SMM makes OS developers expect that they don't have to handle it, even provides them an excuse not to do it, and that weakens the ecosystem. IIRC, SMI handlers were originally added to coreboot because of a similar story. Now we risk to repeat the 10y+ old mistake. -- To view, visit https://review.coreboot.org/c/coreboot/+/63552 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibf06a64f055a0cee3659b410652082f31e18e149 Gerrit-Change-Number: 63552 Gerrit-PatchSet: 6 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Gerrit-CC: Kevin Chang <kevin.chang(a)lcfc.corp-partner.google.com> Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sunshine Chao <sunshine.chao(a)lcfc.corp-partner.google.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> 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-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 19 Apr 2022 15:06:35 +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> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: Reset XHCI controller while preparing for S5
by Subrata Banik (Code Review) 19 Apr '22

19 Apr '22

Attention is currently required from: Maulik V Vaghela, Paul Menzel, Sridhar Siricilla, Arthur Heymans, Nick Vaccaro, Eric Lai, Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63552 ) Change subject: mb/google/brya: Reset XHCI controller while preparing for S5 ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: > Can the reset be just called on the resume from S5 path instead of in the smihandler before entering S5? Could be done, but I prefer resetting the controller during entry path instead resume. 1. At resume we have to reset the XHCI controller early, don't know if there is any complication of XHCI being pursued it's bad state while it entered into S5 and upon resume also, the state is remain the same, unless reset explicitly. 2. Doing early XHCI reset, meaning, keeping this code into the bootblock (which is typically, RO for Chrome FW), hence, we wish to keep it outside RO. 3. If Intel could figure out any better solution to fix this problem then we can revert the SMI handler W/A easily. -- To view, visit https://review.coreboot.org/c/coreboot/+/63552 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibf06a64f055a0cee3659b410652082f31e18e149 Gerrit-Change-Number: 63552 Gerrit-PatchSet: 6 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Gerrit-CC: Kevin Chang <kevin.chang(a)lcfc.corp-partner.google.com> Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sunshine Chao <sunshine.chao(a)lcfc.corp-partner.google.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> 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-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 19 Apr 2022 15:01:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/smbus: Add `finalize` operation for smbus
by Subrata Banik (Code Review) 19 Apr '22

19 Apr '22

Attention is currently required from: Felix Singer, Tim Wawrzynczak, Angel Pons, Arthur Heymans, Nick Vaccaro, Eric Lai, Werner Zeh. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63640 ) Change subject: soc/intel/common/smbus: Add `finalize` operation for smbus ...................................................................... Patch Set 5: (1 comment) File src/soc/intel/common/block/smbus/smbus.c: https://review.coreboot.org/c/coreboot/+/63640/comment/9ecd4337_a7d8d2b2 PS4, Line 88: smbus_final > `finalize_smbus` is not consistent with the naming of the other functions, though. The idea is that the `smbus_` prefix works as a namespace. yeah, I had the same concern as well. But didn't wish to continue this discussion as programming the register is more important. -- To view, visit https://review.coreboot.org/c/coreboot/+/63640 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie945680049514e6c5d797790a381a6946e836926 Gerrit-Change-Number: 63640 Gerrit-PatchSet: 5 Gerrit-Owner: Subrata Banik <subratabanik(a)google.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: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> 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-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Tue, 19 Apr 2022 14:58:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2022