Attention is currently required from: Patrick Rudolph, Christian Walter.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/66616 )
Change subject: mb/prodrive/hermes: Prevent SGPIO cross-powering 5V rail
......................................................................
Patch Set 5:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/66616/comment/d03095e6_3427dad4
PS2, Line 9: TODO: Write a commit message. This was tested and seems to work.
> I am too tired to write a nice commit message today, I'll do it when I have time and/or get inspired […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/66616
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic872903d5fcdd1c17e02b4c06d5ba29889fbc27d
Gerrit-Change-Number: 66616
Gerrit-PatchSet: 5
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Comment-Date: Thu, 06 Oct 2022 16:59:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph, Christian Walter.
Hello build bot (Jenkins), Patrick Rudolph, Christian Walter,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/66616
to look at the new patch set (#5).
Change subject: mb/prodrive/hermes: Prevent SGPIO cross-powering 5V rail
......................................................................
mb/prodrive/hermes: Prevent SGPIO cross-powering 5V rail
The PCH's SGPIO pads are connected to a buffer chip that is powered from
the always-on +3V3_AUX rail. For some cursed reason, when the SGPIO pads
stay configured as SGPIO when a Poseidon system shuts down, voltage from
the +3V3_AUX-powered buffer chip will leak into the +5V rail through the
SATA backplane. Just pulling the SGPIO pads low before the system powers
off stops the +5V rail from being cross-powered.
This issue has only been observed in S5, but it's very likely other
sleep states are affected as well. Thus, always pull the SGPIO pins
low before entering ACPI S3 or deeper because the power supply will
turn off in these states as well.
TEST=Obtain a Poseidon system, verify that the +5V rail is cross-powered
after going to S5. We measured 0.17V on our system, but voltages as
high as 0.6V were measured on other systems. Verify that unplugging
the SGPIO cable going to the SATA backplane results in the +5V rail
voltage dropping to 0V, which indicates that the voltage leakage is
exclusively coming from the SGPIO and SATA backplane. Finally, make
sure that the +5V rail voltage drops to 0V after going into ACPI S5
with this patch applied and the SGPIO cable connected.
Change-Id: Ic872903d5fcdd1c17e02b4c06d5ba29889fbc27d
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
A src/mainboard/prodrive/hermes/smihandler.c
1 file changed, 55 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/66616/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/66616
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic872903d5fcdd1c17e02b4c06d5ba29889fbc27d
Gerrit-Change-Number: 66616
Gerrit-PatchSet: 5
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-MessageType: newpatchset
Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/68033 )
Change subject: arch/x86/mmap_boot.c: Clean up includes
......................................................................
arch/x86/mmap_boot.c: Clean up includes
Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr>
Change-Id: I85e60c189c1ec1da5cf0e5b864447ef6f7b3f548
Reviewed-on: https://review.coreboot.org/c/coreboot/+/68033
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Martin L Roth <gaumless(a)gmail.com>
---
M src/arch/x86/mmap_boot.c
1 file changed, 14 insertions(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Martin L Roth: Looks good to me, approved
diff --git a/src/arch/x86/mmap_boot.c b/src/arch/x86/mmap_boot.c
index f0ccc86..0e9812f 100644
--- a/src/arch/x86/mmap_boot.c
+++ b/src/arch/x86/mmap_boot.c
@@ -1,8 +1,8 @@
/* SPDX-License-Identifier: GPL-2.0-only */
#include <boot_device.h>
-#include <endian.h>
#include <spi_flash.h>
+#include <stdint.h>
/* The ROM is memory mapped just below 4GiB. Form a pointer for the base. */
#define rom_base ((void *)(uintptr_t)(0x100000000ULL-CONFIG_ROM_SIZE))
--
To view, visit https://review.coreboot.org/c/coreboot/+/68033
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I85e60c189c1ec1da5cf0e5b864447ef6f7b3f548
Gerrit-Change-Number: 68033
Gerrit-PatchSet: 4
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Shelley Chen has submitted this change. ( https://review.coreboot.org/c/coreboot/+/67884 )
Change subject: soc/qualcomm: Update the wait time for checking PCIe link up
......................................................................
soc/qualcomm: Update the wait time for checking PCIe link up
Currently, after the PCIe link is initialized, we wait 100ms every
time the link is not up anymore. However, this causes significant
delay. Assuming the first check is false, we'd like to increase the
frequency of checks for the link to be up. Changing to check every
10ms instead. This seems to save about 90ms in the device
configuration stage of bootup on herobrine.
BUG=b:218406702
BRANCH=None
TEST=reboot from AP console (on herobrine)
prior to fix (from cbmem dump):
40:device configuration 919,391 (202,861)
after fix (from cbmem dump):
40:device configuration 826,294 (112,729)
Change-Id: Ic67e7207c1e9f589b34705dc24f5d1ea423e2d56
Signed-off-by: Shelley Chen <shchen(a)google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/67884
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Tim Wawrzynczak <inforichland(a)gmail.com>
Reviewed-by: mturney mturney <quic_mturney(a)quicinc.com>
Reviewed-by: Douglas Anderson <dianders(a)chromium.org>
---
M src/soc/qualcomm/common/include/soc/pcie.h
1 file changed, 32 insertions(+), 2 deletions(-)
Approvals:
build bot (Jenkins): Verified
Douglas Anderson: Looks good to me, approved
Tim Wawrzynczak: Looks good to me, but someone else must approve
mturney mturney: Looks good to me, but someone else must approve
diff --git a/src/soc/qualcomm/common/include/soc/pcie.h b/src/soc/qualcomm/common/include/soc/pcie.h
index 09ea671..5aa315c 100644
--- a/src/soc/qualcomm/common/include/soc/pcie.h
+++ b/src/soc/qualcomm/common/include/soc/pcie.h
@@ -48,8 +48,8 @@
#define LINK_SPEED_GEN_1 0x1
#define LINK_SPEED_GEN_2 0x2
#define LINK_SPEED_GEN_3 0x3
-#define PCIE_LINK_UP_MS 100
-#define LINK_WAIT_MAX_RETRIES 10
+#define PCIE_LINK_UP_MS 10
+#define LINK_WAIT_MAX_RETRIES 100
#define COMMAND_MASK 0xffff
--
To view, visit https://review.coreboot.org/c/coreboot/+/67884
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic67e7207c1e9f589b34705dc24f5d1ea423e2d56
Gerrit-Change-Number: 67884
Gerrit-PatchSet: 5
Gerrit-Owner: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Douglas Anderson <dianders(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: mturney mturney <quic_mturney(a)quicinc.com>
Gerrit-CC: Sudheer Amrabadi <samrabad(a)codeaurora.org>
Gerrit-CC: Veerabhadrarao Badiganti <vbadigan(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Venkat Thogaru <thogaru(a)qualcomm.corp-partner.google.com>
Gerrit-CC: mturney mturney <mturney(a)codeaurora.org>
Gerrit-MessageType: merged
Attention is currently required from: Patrick Rudolph, Christian Walter.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/66616 )
Change subject: mb/prodrive/hermes: Prevent SGPIO cross-powering 5V rail
......................................................................
Patch Set 4:
(7 comments)
Commit Message:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159867):
https://review.coreboot.org/c/coreboot/+/66616/comment/39adbf00_070aa503
PS4, Line 8:
Possible unwrapped commit description (prefer a maximum 72 chars per line)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159867):
https://review.coreboot.org/c/coreboot/+/66616/comment/4c738d85_2da5b2ce
PS4, Line 9: The PCH's SGPIO pads are connected to a buffer chip which is powered from
Possible unwrapped commit description (prefer a maximum 72 chars per line)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159867):
https://review.coreboot.org/c/coreboot/+/66616/comment/a3a8d734_3f36742d
PS4, Line 10: the always-on +3V3_AUX rail. For some reason, if the SGPIO pads remain in
Possible unwrapped commit description (prefer a maximum 72 chars per line)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159867):
https://review.coreboot.org/c/coreboot/+/66616/comment/9f177cba_58deeb38
PS4, Line 11: their native function when a Poseidon system shuts down, a bit of voltage
Possible unwrapped commit description (prefer a maximum 72 chars per line)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159867):
https://review.coreboot.org/c/coreboot/+/66616/comment/6da7148b_aedbe6b4
PS4, Line 12: from the +3V3_AUX-powered buffer chip will leak into the +5V rail through
Possible unwrapped commit description (prefer a maximum 72 chars per line)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159867):
https://review.coreboot.org/c/coreboot/+/66616/comment/2f98d534_6b93ae1a
PS4, Line 15:
Possible unwrapped commit description (prefer a maximum 72 chars per line)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159867):
https://review.coreboot.org/c/coreboot/+/66616/comment/481bdc1d_1f84509e
PS4, Line 16: This issue has only been observed in S5, but it is very likely other sleep
Possible unwrapped commit description (prefer a maximum 72 chars per line)
--
To view, visit https://review.coreboot.org/c/coreboot/+/66616
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic872903d5fcdd1c17e02b4c06d5ba29889fbc27d
Gerrit-Change-Number: 66616
Gerrit-PatchSet: 4
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Comment-Date: Thu, 06 Oct 2022 16:51:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment