coreboot-gerrit April 2022

coreboot-gerrit@coreboot.org

1 participants
5879 discussions

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

18 Apr '22

Attention is currently required from: Maulik V Vaghela, Paul Menzel, Sridhar Siricilla, Nick Vaccaro, Eric Lai, Felix Held. Hello build bot (Jenkins), Maulik V Vaghela, Tim Wawrzynczak, Sridhar Siricilla, Nick Vaccaro, Eric Lai, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63552 to look at the new patch set (#5). Change subject: mb/google/brya: Reset XHCI controller while preparing for S5 ...................................................................... mb/google/brya: Reset XHCI controller while preparing for S5 This patch calls `xhci_host_reset()` function to perform XHCI controller reset. Currently, the PMC IPC times out while sending the USB-C (0xA7) command during poweron from S5 (S5->S4->S3->S0). On Brya variants, poweron from S5 state results in PMC error while sending PMC IPC (0xA7) to USB-C active ports, log here: localhost ~ # cbmem -c | grep ERROR [ERROR] PMC IPC timeout after 1000 ms [ERROR] PMC IPC command 0x200a7 failed [ERROR] pmc_send_ipc_cmd failed [ERROR] Failed to setup port:0 to initial state [ERROR] PMC IPC timeout after 1000 ms [ERROR] PMC IPC command 0x200a7 failed [ERROR] pmc_send_ipc_cmd failed [ERROR] Failed to setup port:1 to initial state [ERROR] PMC IPC timeout after 1000 ms [ERROR] PMC IPC command 0x20a0 failed This problem is not seen while powering on from G3 (G3->S5->S4->S3->S0). During poweron the state of USB ports are not the same between S5 and G3 and it appears that the active USB port still is in U3 (suspend) while PMC tries to send the IPC command, which results in a timeout. This patch utilises the S5 SMI handler to reset the XHCI controller using `xhci_host_reset()` prior entering into the S5, it helps to restore the port state to active hence, no PMC timeout is seen with this code change. BUG=b:227289581 TEST=No PMC timeout is observed while sending USB-C PMC command (0xA7) during resume from S5. Total Time: 1,045,855 localhost ~ # cbmem -c | grep ERROR No PMC timeout error is observed with this CL. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: Ibf06a64f055a0cee3659b410652082f31e18e149 --- M src/mainboard/google/brya/smihandler.c 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/63552/5 -- 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: 5 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: 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: 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-MessageType: newpatchset

1 0

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

18 Apr '22

Attention is currently required from: Maulik V Vaghela, Paul Menzel, Sridhar Siricilla, 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 4: (6 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63552/comment/32906929_61a758ac PS4, Line 12: the PMC IPC timeout issue > As you use “the”, is it a common known problem? Is there some errata? Please refer the prior discussion here in this patch Felix Held Patchset 4 Apr 14 is this problem specific to google/brya or will it happen on all boards using the same soc? if the latter is the case, this should probably be done in the corresponding soc code's smi sleep handler Subrata Banik Patchset 4 Apr 14 is this problem specific to google/brya or will it happen on all boards using the same soc? if the latter is the case, this should probably be done in the corresponding soc code's smi sleep handler @Felix, so far we have only seen this issue with ADL-P and ADL-N chrome ref designs and Intel is investigating further to root cause and may be once they provide the TA for this issue, it can re-land into the SOC code. At this moment, don't want to disturb other devices with ADL-SoC unless issue is prove there. https://review.coreboot.org/c/coreboot/+/63552/comment/ee9ff2ca_a2340903 PS4, Line 34: active USB port > Excuse my ignorance, does “active” mean, something needs to be connected to the port? active => the port is enable https://review.coreboot.org/c/coreboot/+/63552/comment/1579a5ca_5470a57e PS4, Line 37: SMI handler > Why does this have to be done in SMM? I guess, below lines are explanatory (isn't it) "This patch utilises the S5 SMI handler to reset the XHCI controller using `xhci_host_reset()` **prior entering into the S5**," we wish to reset the XHCI controller state prior to S5 hence, this is the last function that coreboot has control. https://review.coreboot.org/c/coreboot/+/63552/comment/ff1bc0d2_9ade1e97 PS4, Line 9: This patch calls into `xhci_host_reset()` function to perform XHCI : controller reset. : : Without this patch the PMC IPC timeout issue is seen while sending the : USB-C (0xA7) command during poweron from S5 (S5->S4->S3->S0). : : On Brya variants, poweron from S5 state results in PMC error while : sending PMC IPC (0xA7) to USB-C active ports, log here: : : localhost ~ # cbmem -c | grep ERROR : : [ERROR] PMC IPC timeout after 1000 ms : [ERROR] PMC IPC command 0x200a7 failed : [ERROR] pmc_send_ipc_cmd failed : [ERROR] Failed to setup port:0 to initial state : [ERROR] PMC IPC timeout after 1000 ms : [ERROR] PMC IPC command 0x200a7 failed : [ERROR] pmc_send_ipc_cmd failed : [ERROR] Failed to setup port:1 to initial state : [ERROR] PMC IPC timeout after 1000 ms : [ERROR] PMC IPC command 0x20a0 failed : : This problem is not seen while powering on from G3 (G3->S5->S4->S3->S0). : : During poweron the state of USB ports are not the same between S5 and G3 : and it appears that the active USB port still is in U3 (suspend) while : PMC tries to send the IPC command, which results in a timeout. : : This patch utilises the S5 SMI handler to reset the XHCI controller : using `xhci_host_reset()` prior entering into the S5, it helps to : restore the port state to active hence, no PMC timeout is seen with : this code change. > I’d start by stating the problem, and then describing the fix. Please read the entire commit msg, it does the same IMO. It started with the problem statement and given the serial log where you could witness the PMC IPC error and then it state the solution. Please advice what is missing here ? https://review.coreboot.org/c/coreboot/+/63552/comment/762080e8_afea76d6 PS4, Line 41: > Does resetting add anything to the shutdown time? (Boottime should be unaffected?) I don't think we have KPIs to measure the shutdown time and not sure writing a bit would impact the time. File src/mainboard/google/brya/smihandler.c: https://review.coreboot.org/c/coreboot/+/63552/comment/aa03fd0d_9b6614d6 PS4, Line 15: /* USB sleep preparations */ > Maybe elaborate? Please advice, how you would like to elaborate apart from the function name itself is so clear `xhci_host_reset` and commit msg state the entire story. -- 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: 4 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: 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: 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: Mon, 18 Apr 2022 08:40:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: coreboot_tables: Add PCIe info to coreboot table
by Jianjun Wang (Code Review) 18 Apr '22

18 Apr '22

Attention is currently required from: Shelley Chen, Hung-Te Lin, Nico Huber, Paul Menzel, Rex-BC Chen, Julius Werner, Arthur Heymans. Jianjun Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63251 ) Change subject: coreboot_tables: Add PCIe info to coreboot table ...................................................................... Patch Set 19: (1 comment) Patchset: PS18: > Did you check how the Linux kernel PCIe drivers model this? (No idea, if this is helpful.) Yes, the PCIe driver model to access the configuration space in libpayload [1] will be similar with the linux kernel [2] after these patch serials get merged, and we provides the similar map_bus function [3] like Linux driver did [4]. [1]https://review.coreboot.org/c/coreboot/+/56789/66/payloads/libpayload/dri… [2]https://elixir.bootlin.com/linux/v5.18-rc1/source/drivers/pci/access.c#L80 [3]https://review.coreboot.org/c/coreboot/+/56794/68/payloads/libpayload/dri… [4]https://elixir.bootlin.com/linux/v5.18-rc1/source/drivers/pci/controller/… -- To view, visit https://review.coreboot.org/c/coreboot/+/63251 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6cdce21efc66aa441ec077e6fc1d5d1c6a9aafb0 Gerrit-Change-Number: 63251 Gerrit-PatchSet: 19 Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Rex-BC Chen <rex-bc.chen(a)mediatek.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Rex-BC Chen <rex-bc.chen(a)mediatek.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Mon, 18 Apr 2022 08:09:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/dedede/var/beadrix: Add a Proximity Sensor SX9324 for SAR
by Teddy Shih (Code Review) 18 Apr '22

18 Apr '22

Attention is currently required from: Henry Sun, Super Ni, Teddy Shih, Aseda Aboagye, Ivan Chen, Karthik Ramasubramanian. Teddy Shih has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63652 ) Change subject: mb/google/dedede/var/beadrix: Add a Proximity Sensor SX9324 for SAR ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Dear All, I have upload a CL of P-Sensor for SAR certification. Please kindly review it. Best Regards Teddy -- To view, visit https://review.coreboot.org/c/coreboot/+/63652 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If172d13aa62503547227adf91f049ea50b948888 Gerrit-Change-Number: 63652 Gerrit-PatchSet: 2 Gerrit-Owner: Teddy Shih <teddyshih(a)ami.corp-partner.google.com> Gerrit-Reviewer: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Reviewer: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Ivan Chen <yulunchen(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Super Ni <super.ni(a)intel.corp-partner.google.com> Gerrit-Reviewer: Teddy Shih <teddyshihau(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eddy Lu <eddylu(a)ami.corp-partner.google.com> Gerrit-CC: Jack Cheng <jack.cheng(a)ecs.corp-partner.google.com> Gerrit-CC: Raymond Chung <raymondchung(a)ami.corp-partner.google.com> Gerrit-Attention: Henry Sun <henrysun(a)google.com> Gerrit-Attention: Super Ni <super.ni(a)intel.corp-partner.google.com> Gerrit-Attention: Teddy Shih <teddyshihau(a)gmail.com> Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Attention: Ivan Chen <yulunchen(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Mon, 18 Apr 2022 08:03:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: docs/coding_style: Clarify use of GCC extensions
by Arthur Heymans (Code Review) 18 Apr '22

18 Apr '22

Attention is currently required from: Arthur Heymans, Hung-Te Lin, Nico Huber, Julius Werner. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63660 ) Change subject: docs/coding_style: Clarify use of GCC extensions ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/63660 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0d0eb90d6729fefeb131cdd573ad51f1884afe11 Gerrit-Change-Number: 63660 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Mon, 18 Apr 2022 07:57:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: coreboot_tables: Add PCIe info to coreboot table
by Paul Menzel (Code Review) 18 Apr '22

18 Apr '22

Attention is currently required from: Shelley Chen, Hung-Te Lin, Nico Huber, Rex-BC Chen, Julius Werner, Arthur Heymans, Jianjun Wang. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63251 ) Change subject: coreboot_tables: Add PCIe info to coreboot table ...................................................................... Patch Set 18: (1 comment) Patchset: PS18: Did you check how the Linux kernel PCIe drivers model this? (No idea, if this is helpful.) -- To view, visit https://review.coreboot.org/c/coreboot/+/63251 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6cdce21efc66aa441ec077e6fc1d5d1c6a9aafb0 Gerrit-Change-Number: 63251 Gerrit-PatchSet: 18 Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Rex-BC Chen <rex-bc.chen(a)mediatek.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Rex-BC Chen <rex-bc.chen(a)mediatek.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-Comment-Date: Mon, 18 Apr 2022 07:56:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/dedede/var/beadrix: Add a Proximity Sensor SX9324 for SAR
by Teddy Shih (Code Review) 18 Apr '22

18 Apr '22

Attention is currently required from: Teddy Shih. Teddy Shih has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63652 ) Change subject: mb/google/dedede/var/beadrix: Add a Proximity Sensor SX9324 for SAR ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/63652 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If172d13aa62503547227adf91f049ea50b948888 Gerrit-Change-Number: 63652 Gerrit-PatchSet: 2 Gerrit-Owner: Teddy Shih <teddyshih(a)ami.corp-partner.google.com> Gerrit-Reviewer: Teddy Shih <teddyshihau(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Teddy Shih <teddyshihau(a)gmail.com> Gerrit-Attention: Teddy Shih <teddyshih(a)ami.corp-partner.google.com> Gerrit-Comment-Date: Mon, 18 Apr 2022 07:55:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: payloads/external/LinuxBoot: Modify Kconfig file and remove older tags
by Paul Menzel (Code Review) 18 Apr '22

18 Apr '22

Attention is currently required from: Martin L Roth, Christian Walter, Stefan Reinauer, Elias Souza. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63351 ) Change subject: payloads/external/LinuxBoot: Modify Kconfig file and remove older tags ...................................................................... Patch Set 8: (9 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63351/comment/3746e67c_442279a9 PS8, Line 2: RatatouilleDummie Please update. https://review.coreboot.org/c/coreboot/+/63351/comment/3f7a85ce_5189cb35 PS8, Line 7: Modify Kconfig file and remove older tags Please split it into two commits. Something like: 1. Update for master branch to main renaming 2. Use newer tags https://review.coreboot.org/c/coreboot/+/63351/comment/217e73fe_22197868 PS8, Line 9: have modified the name of renamed https://review.coreboot.org/c/coreboot/+/63351/comment/58a1167b_2cbdf47c PS8, Line 10: and Remove? https://review.coreboot.org/c/coreboot/+/63351/comment/c37d77ea_374b069e PS8, Line 10: remove older tag of u-root and use the new tag Why remove the older tags? https://review.coreboot.org/c/coreboot/+/63351/comment/5ac1c28c_2e9a9275 PS8, Line 13: eliassouza Please format correctly. Patchset: PS8: Welcome to coreboot and thank you for your contribution. File payloads/external/LinuxBoot/Kconfig: https://review.coreboot.org/c/coreboot/+/63351/comment/85ba9c07_0246aa27 PS8, Line 13: help Unwanted. File payloads/external/LinuxBoot/targets/u-root.mk: https://review.coreboot.org/c/coreboot/+/63351/comment/8fa74e0a_50f136f9 PS8, Line 8: ARCH-$(CONFIG_LINUXBOOT_X86_64)=amd64 Please make that a separate patch. -- To view, visit https://review.coreboot.org/c/coreboot/+/63351 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I70c4d064b534b28ea0be987b91a30650debc6ffe Gerrit-Change-Number: 63351 Gerrit-PatchSet: 8 Gerrit-Owner: Elias Souza Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Martin L Roth <martinroth(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin L Roth <martinroth(a)google.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Elias Souza Gerrit-Comment-Date: Mon, 18 Apr 2022 07:55:27 +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 Paul Menzel (Code Review) 18 Apr '22

18 Apr '22

Attention is currently required from: Subrata Banik, Maulik V Vaghela, Sridhar Siricilla, Nick Vaccaro, Eric Lai, Felix Held. Paul Menzel 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 4: (8 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63552/comment/bc7f1abc_63c361d6 PS4, Line 9: calls into Just “calls”? https://review.coreboot.org/c/coreboot/+/63552/comment/4307c95e_4d5e2e4d PS4, Line 12: Without this patch the PMC IPC timeout issue is seen Currently, the PMC IPC times out while sending the USB-C (0xA7) command during … https://review.coreboot.org/c/coreboot/+/63552/comment/6d57a7de_0dd91c14 PS4, Line 12: the PMC IPC timeout issue As you use “the”, is it a common known problem? Is there some errata? https://review.coreboot.org/c/coreboot/+/63552/comment/176a13d4_34313430 PS4, Line 34: active USB port Excuse my ignorance, does “active” mean, something needs to be connected to the port? https://review.coreboot.org/c/coreboot/+/63552/comment/6a0ed398_4614430d PS4, Line 37: SMI handler Why does this have to be done in SMM? https://review.coreboot.org/c/coreboot/+/63552/comment/8bdaf478_65e3a727 PS4, Line 9: This patch calls into `xhci_host_reset()` function to perform XHCI : controller reset. : : Without this patch the PMC IPC timeout issue is seen while sending the : USB-C (0xA7) command during poweron from S5 (S5->S4->S3->S0). : : On Brya variants, poweron from S5 state results in PMC error while : sending PMC IPC (0xA7) to USB-C active ports, log here: : : localhost ~ # cbmem -c | grep ERROR : : [ERROR] PMC IPC timeout after 1000 ms : [ERROR] PMC IPC command 0x200a7 failed : [ERROR] pmc_send_ipc_cmd failed : [ERROR] Failed to setup port:0 to initial state : [ERROR] PMC IPC timeout after 1000 ms : [ERROR] PMC IPC command 0x200a7 failed : [ERROR] pmc_send_ipc_cmd failed : [ERROR] Failed to setup port:1 to initial state : [ERROR] PMC IPC timeout after 1000 ms : [ERROR] PMC IPC command 0x20a0 failed : : This problem is not seen while powering on from G3 (G3->S5->S4->S3->S0). : : During poweron the state of USB ports are not the same between S5 and G3 : and it appears that the active USB port still is in U3 (suspend) while : PMC tries to send the IPC command, which results in a timeout. : : This patch utilises the S5 SMI handler to reset the XHCI controller : using `xhci_host_reset()` prior entering into the S5, it helps to : restore the port state to active hence, no PMC timeout is seen with : this code change. I’d start by stating the problem, and then describing the fix. https://review.coreboot.org/c/coreboot/+/63552/comment/4bcba6de_f595e60d PS4, Line 41: Does resetting add anything to the shutdown time? (Boottime should be unaffected?) File src/mainboard/google/brya/smihandler.c: https://review.coreboot.org/c/coreboot/+/63552/comment/9e86deb0_8c2b7e7e PS4, Line 15: /* USB sleep preparations */ Maybe elaborate? -- 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: 4 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: 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> 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: Mon, 18 Apr 2022 07:51:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: coreboot_tables: Add PCIe info to coreboot table
by Yu-Ping Wu (Code Review) 18 Apr '22

18 Apr '22

Attention is currently required from: Shelley Chen, Hung-Te Lin, Nico Huber, Rex-BC Chen, Julius Werner, Arthur Heymans, Jianjun Wang. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63251 ) Change subject: coreboot_tables: Add PCIe info to coreboot table ...................................................................... Patch Set 18: (2 comments) Patchset: PS9: > Can we just pass the ctrl_base to libpayload in this patch? Since the mmio_size is not used for Medi […] I'm fine with that. File src/lib/coreboot_table.c: https://review.coreboot.org/c/coreboot/+/63251/comment/38e99c36_3d736db1 PS12, Line 36: __weak enum cb_err lb_fill_pcie(struct lb_pcie *pcie) : { : return CB_ERR; : } > We do want this function to be general enough to support both x86 and other platforms without ECAM s […] If we only add 'ctrl_base' (which CONFIG_ECAM_MMCONF_BASE_ADDRESS is not) in this patch, then there should be no default value. -- To view, visit https://review.coreboot.org/c/coreboot/+/63251 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6cdce21efc66aa441ec077e6fc1d5d1c6a9aafb0 Gerrit-Change-Number: 63251 Gerrit-PatchSet: 18 Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Rex-BC Chen <rex-bc.chen(a)mediatek.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Rex-BC Chen <rex-bc.chen(a)mediatek.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-Comment-Date: Mon, 18 Apr 2022 07:47:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shelley Chen <shchen(a)google.com> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Comment-In-Reply-To: Jianjun Wang <jianjun.wang(a)mediatek.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