coreboot-gerrit October 2023

coreboot-gerrit@coreboot.org

1 participants
3354 discussions

[XS] Change in coreboot[main]: configurable slp-s0 residency counter frequency in LPIT table
by Kapil Porwal (Code Review) 09 Oct '23

09 Oct '23

Attention is currently required from: Eran Mitrani, Jakub Czapiga, Jérémy Compostella, Lance Zhao, Sukumar Ghorai, Tarun, Tim Wawrzynczak. Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78164?usp=email ) Change subject: configurable slp-s0 residency counter frequency in LPIT table ...................................................................... Patch Set 14: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/78164?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I401dd4a09a67d81a9ea3a56cd22f1a681e2a9349 Gerrit-Change-Number: 78164 Gerrit-PatchSet: 14 Gerrit-Owner: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Mon, 09 Oct 2023 16:58:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/pc80/rtc/option.c: Stop resetting CMOS during s3 resume
by Jonathon Hall (Code Review) 09 Oct '23

09 Oct '23

Attention is currently required from: Bill XIE. Jonathon Hall has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78288?usp=email ) Change subject: drivers/pc80/rtc/option.c: Stop resetting CMOS during s3 resume ...................................................................... Patch Set 5: Code-Review+2 (1 comment) Patchset: PS4: > GM45 platforms with or without CONFIG(STATIC_OPTION_TABLE) always performs memory training and store […] Ah makes sense, so the training cache is only for S3 resume on these platforms. Thanks! -- To view, visit https://review.coreboot.org/c/coreboot/+/78288?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I533e83f3b95f327b0e24f4d750f8812325b7770b Gerrit-Change-Number: 78288 Gerrit-PatchSet: 5 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Comment-Date: Mon, 09 Oct 2023 16:53:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Bill XIE <persmule(a)hardenedlinux.org> Comment-In-Reply-To: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-MessageType: comment

1 0

[XL] Change in coreboot[main]: mb/google/brox: Create new Brox baseboard
by Nick Vaccaro (Code Review) 09 Oct '23

09 Oct '23

Attention is currently required from: Shelley Chen. Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78009?usp=email ) Change subject: mb/google/brox: Create new Brox baseboard ...................................................................... Patch Set 3: (4 comments) Patchset: PS3: > Do you think that we can push this in so that we have a framework for building this board? I can ad […] My concern would be that features or settings get added that are never used and are missed for being removed. That being said, it's up to you how you want to drive the Brox AP fw effort, so if this is the approach you are most comfortable with, I'll follow along. File src/mainboard/google/brox/variants/brox/include/variant/gpio.h: https://review.coreboot.org/c/coreboot/+/78009/comment/450dc47f_eab90417 : PS3, Line 11: define T1_OFF_MS 16 : #define T2_OFF_MS 2 : NIT - alphabetize File src/mainboard/google/brox/variants/brox/memory/dram_id.generated.txt: https://review.coreboot.org/c/coreboot/+/78009/comment/5ac36302_403582fc : PS3, Line 7: MT53E512M32D2NP-046 WT:F 0 (0000) IMO, it's better to check this file (and others in memory folder) in as empty and then add memory options Brox will be using. It will still build even if these files are empty. Do we know yet what memory options Brox plans on? File src/mainboard/google/brox/variants/brox/overridetree.cb: https://review.coreboot.org/c/coreboot/+/78009/comment/f676d95f_9974fd20 : PS3, Line 1: fw_config This is one of those files that you want to be careful with. This information is mirrored in the project config.star and ends up in the OS component. What this means is that you don't want to define an FW_CONFIG bit and then later change definition as it will cause issues if the firmware and OS are not matched such that the definition of FW_CONFIG in the overridetree.cb is the same as that defined in config.star. -- To view, visit https://review.coreboot.org/c/coreboot/+/78009?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I929b465646ac4c69d4bab33ce23848c7b1fa0f98 Gerrit-Change-Number: 78009 Gerrit-PatchSet: 3 Gerrit-Owner: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Comment-Date: Mon, 09 Oct 2023 16:19:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shelley Chen <shchen(a)google.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/rex/var/karis: Correct devicetree touchscreen settings
by Matt DeVillier (Code Review) 09 Oct '23

09 Oct '23

Attention is currently required from: Eran Mitrani, Felix Held, Kapil Porwal, Lucas Chen, Ren Kuo, Tarun, Tyler Wang. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78254?usp=email ) Change subject: mb/google/rex/var/karis: Correct devicetree touchscreen settings ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: > this might be a power sequencing issue that the tochscreen isn't powered on yet when the detection i […] thanks for catching this -- To view, visit https://review.coreboot.org/c/coreboot/+/78254?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6c7815b81eb47fb41e58233fde512ac6b9c000a7 Gerrit-Change-Number: 78254 Gerrit-PatchSet: 5 Gerrit-Owner: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lucas Chen <lucas.chen(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Lucas Chen <lucas.chen(a)quanta.corp-partner.google.com> Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 09 Oct 2023 15:26:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/rex/var/karis: Correct devicetree touchscreen settings
by Matt DeVillier (Code Review) 09 Oct '23

09 Oct '23

Attention is currently required from: Eran Mitrani, Kapil Porwal, Lucas Chen, Ren Kuo, Tarun, Tyler Wang. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78254?usp=email ) Change subject: mb/google/rex/var/karis: Correct devicetree touchscreen settings ...................................................................... Patch Set 5: Code-Review-1 (1 comment) Patchset: PS5: the correct solution IMO is to set the touchscreen power and reset GPIOs correctly in romstage and ramstage so that the detect feature works. romstage needs: /* GPP_C00 : [] ==> EN_TCHSCR_PWR */ PAD_CFG_GPO(GPP_C00, 1, DEEP), /* GPP_C01 : [] ==> SOC_TCHSCR_RST_R_L */ PAD_CFG_GPO(GPP_C01, 0, DEEP), ramstage needs: /* GPP_C00 : [] ==> EN_TCHSCR_PWR */ PAD_CFG_GPO(GPP_C00, 1, DEEP), /* GPP_C01 : [] ==> SOC_TCHSCR_RST_R_L */ PAD_CFG_GPO(GPP_C01, 1, DEEP), -- To view, visit https://review.coreboot.org/c/coreboot/+/78254?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6c7815b81eb47fb41e58233fde512ac6b9c000a7 Gerrit-Change-Number: 78254 Gerrit-PatchSet: 5 Gerrit-Owner: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lucas Chen <lucas.chen(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Lucas Chen <lucas.chen(a)quanta.corp-partner.google.com> Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Mon, 09 Oct 2023 15:25:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[master]: soc/intel/apollolake: Correct the logic for the legacy 8254 timer
by Matt DeVillier (Code Review) 09 Oct '23

09 Oct '23

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77409?usp=email ) Change subject: soc/intel/apollolake: Correct the logic for the legacy 8254 timer ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > This patch let at least at one mainboard (siemens/mc_apl4) run the payload into problems. […] all this patch did was invert the bool value passed to FSP - the setting of that value via the Kconfig or CMOS option was not changed. -- To view, visit https://review.coreboot.org/c/coreboot/+/77409?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2d6c859c0910b796d2ae5874a560ff9974578106 Gerrit-Change-Number: 77409 Gerrit-PatchSet: 2 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Uwe Poeche <uwe.poeche(a)siemens.com> Gerrit-Comment-Date: Mon, 09 Oct 2023 15:16:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Uwe Poeche <uwe.poeche(a)siemens.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/pc80/rtc/option.c: Stop resetting CMOS during s3 resume
by Bill XIE (Code Review) 09 Oct '23

09 Oct '23

Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78288?usp=email ) Change subject: drivers/pc80/rtc/option.c: Stop resetting CMOS during s3 resume ...................................................................... Patch Set 5: (1 comment) Patchset: PS4: > Agree, we shouldn't reset CMOS during S3 resume. […] GM45 platforms with or without CONFIG(STATIC_OPTION_TABLE) always performs memory training and store the result in CMOS above 128 bytes on every boot, and read it during s3 resume, as shown in raminit_read_training() and raminit_write_training() of nb/intel/gm45/raminit_read_write_training.c. -- To view, visit https://review.coreboot.org/c/coreboot/+/78288?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I533e83f3b95f327b0e24f4d750f8812325b7770b Gerrit-Change-Number: 78288 Gerrit-PatchSet: 5 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 09 Oct 2023 15:14:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/pc80/rtc/option.c: Stop resetting CMOS during s3 resume
by Bill XIE (Code Review) 09 Oct '23

09 Oct '23

Attention is currently required from: Bill XIE. Hello Jonathon Hall, Paul Menzel, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78288?usp=email to look at the new patch set (#5). Change subject: drivers/pc80/rtc/option.c: Stop resetting CMOS during s3 resume ...................................................................... drivers/pc80/rtc/option.c: Stop resetting CMOS during s3 resume After commit e12b313844da ("drivers/pc80/rtc/option.c: Allow CMOS defaults to extend to bank 1"), Thinkpad X200 with CONFIG(STATIC_OPTION_TABLE) can no longer resume from s3 (detected via bisect). Further inspection shows that DRAM training result of GM45 is stored in CMOS above 128 bytes in raminit_read_write_training.c, for s3 resume to restore, but it will be erased by sanitize_cmos(), leaving only "untrained" result restored, so s3 resume will fail. However, resetting CMOS seems unnecessary during s3 resume. Now, cmos_need_reset will be negated when acpi_is_wakeup_s3() returns true. Tested: Thinkpad X200 with CONFIG(STATIC_OPTION_TABLE) can resume from s3 again with these changes. Change-Id: I533e83f3b95f327b0e24f4d750f8812325b7770b Signed-off-by: Bill XIE <persmule(a)hardenedlinux.org> --- M src/drivers/pc80/rtc/option.c 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/78288/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/78288?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I533e83f3b95f327b0e24f4d750f8812325b7770b Gerrit-Change-Number: 78288 Gerrit-PatchSet: 5 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: sb/intel/bd82x6x: Use helper for PCIe hotplug
by Felix Held (Code Review) 09 Oct '23

09 Oct '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78227?usp=email ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: sb/intel/bd82x6x: Use helper for PCIe hotplug ...................................................................... sb/intel/bd82x6x: Use helper for PCIe hotplug Introduce pci_is_hotplugable is helper to find hotpluggable PCIe devices. Test: PCI express slot is still marked as the only hotpluggable PCIe root port. Change-Id: I25aae540ff2ffa3ec5b93ed9caa838b4e50048d2 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78227 Reviewed-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Reviewed-by: Jan Samek <jan.samek(a)siemens.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/southbridge/intel/bd82x6x/pcie.c 1 file changed, 9 insertions(+), 5 deletions(-) Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Jan Samek: Looks good to me, approved diff --git a/src/southbridge/intel/bd82x6x/pcie.c b/src/southbridge/intel/bd82x6x/pcie.c index 6f082e8..206894d 100644 --- a/src/southbridge/intel/bd82x6x/pcie.c +++ b/src/southbridge/intel/bd82x6x/pcie.c @@ -32,6 +32,13 @@ return NULL; } +static bool pci_is_hotplugable(struct device *dev) +{ + struct southbridge_intel_bd82x6x_config *config = dev->chip_info; + + return config && config->pcie_hotplug_map[PCI_FUNC(dev->path.pci.devfn)]; +} + static void pch_pcie_pm_early(struct device *dev) { u16 link_width_p0, link_width_p4; @@ -179,7 +186,6 @@ static void pci_init(struct device *dev) { u16 reg16; - struct southbridge_intel_bd82x6x_config *config = dev->chip_info; printk(BIOS_DEBUG, "Initializing PCH PCIe bridge.\n"); @@ -202,7 +208,7 @@ pci_write_config16(dev, 0x1e, reg16); /* Enable expresscard hotplug events. */ - if (config->pcie_hotplug_map[PCI_FUNC(dev->path.pci.devfn)]) { + if (pci_is_hotplugable(dev)) { pci_or_config32(dev, 0xd8, 1 << 30); pci_write_config16(dev, 0x42, 0x142); } @@ -216,9 +222,7 @@ static void pch_pciexp_scan_bridge(struct device *dev) { - struct southbridge_intel_bd82x6x_config *config = dev->chip_info; - - if (CONFIG(PCIEXP_HOTPLUG) && config->pcie_hotplug_map[PCI_FUNC(dev->path.pci.devfn)]) { + if (CONFIG(PCIEXP_HOTPLUG) && pci_is_hotplugable(dev)) { pciexp_hotplug_scan_bridge(dev); } else { /* Normal PCIe Scan */ -- To view, visit https://review.coreboot.org/c/coreboot/+/78227?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I25aae540ff2ffa3ec5b93ed9caa838b4e50048d2 Gerrit-Change-Number: 78227 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jan Samek <jan.samek(a)siemens.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: sb/intel/bd82x6x: Disable unused PCIe root ports
by Felix Held (Code Review) 09 Oct '23

09 Oct '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78226?usp=email ) Change subject: sb/intel/bd82x6x: Disable unused PCIe root ports ...................................................................... sb/intel/bd82x6x: Disable unused PCIe root ports Follow the PCH BIOS spec more closely by porting the broadwell and braswell PCIe downstream device detection. To safe power disable PCIe root ports that have no downstream device connected. By setting the FLAGS_SLOT bit in register PCI_EXP_FLAGS the PCI_EXP_SLTSTA_PDS bit will be updated with in band device detection from the PCIe PHY. While this is primarly used for PCIe hot-plug detection, it is more reliable than probing for downstream devices by reading DID/VID PCI registers. The FLAGS_SLOT bit should stay cleared for integrated devices, as those are known to be present, but to simplify the code all PCIe ports will have the FLAGS_SLOT bit set. There currently used devicetrees might also be lacking integrated devices on the PCH root ports... The SLOTCAP field must be updated by BIOS when the FLAGS_SLOT is set, but it shouldn't be filled for integrated devices. Until now the SLOTCAP field has always been populated and it never was a problem. - Set FLAGS_SLOT "Slot Implemented" bit early. - Read bit PCI_EXP_SLTSTA_PDS to detect connected downstream devices as done on braswell. - Disable unused PCIe slots that are not hotplugable. - Set BIT26 in register 0x338 and wait for bits in register 0x328 to clear as done on broadwell. Test: Tested on Lenovo X220. Unused root ports are disabled and port that are in used or marked hot-plug are kept enabled. Change-Id: I8ccfcab2e0e4faba8322755a4f8c2108d9b007ac Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78226 Reviewed-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/southbridge/intel/bd82x6x/pch.c 1 file changed, 49 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved diff --git a/src/southbridge/intel/bd82x6x/pch.c b/src/southbridge/intel/bd82x6x/pch.c index ae8ed9b..c668f9a6 100644 --- a/src/southbridge/intel/bd82x6x/pch.c +++ b/src/southbridge/intel/bd82x6x/pch.c @@ -6,6 +6,7 @@ #include <device/pci.h> #include <device/pci_ops.h> #include <string.h> +#include <timer.h> #include "chip.h" #include "pch.h" @@ -296,6 +297,52 @@ sizeof(new_hotplug_map)); } +static void check_device_present(struct device *dev) +{ + struct southbridge_intel_bd82x6x_config *config = dev->chip_info; + struct stopwatch timeout; + bool present, hot_plugable; + uint32_t cap; + + /* Set slot implemented. */ + cap = pci_find_capability(dev, PCI_CAP_ID_PCIE); + pci_or_config16(dev, cap + PCI_EXP_FLAGS, PCI_EXP_FLAGS_SLOT); + + /* + * By setting the PCI_EXP_FLAGS_SLOT bit in register PCI_EXP_FLAGS the + * PCI_EXP_SLTSTA_PDS bit will be updated with in band device + * detection from the PCIe PHY. While this is primarly used for PCIe + * hot-plug detection, it is more reliable than probing for downstream + * devices by reading DID/VID PCI registers of such. + * + * Usually the PCI_EXP_FLAGS_SLOT isn't set for integrated devices, + * but to simplify device detection it's set for all ports. + * + * It also allows to detect device before PCI enumeration has run. + */ + hot_plugable = config && config->pcie_hotplug_map[PCI_FUNC(dev->path.pci.devfn)]; + present = !!(pci_read_config16(dev, cap + PCI_EXP_SLTSTA) & PCI_EXP_SLTSTA_PDS); + + printk(BIOS_DEBUG, "%s: %s downstream device\n", + dev_path(dev), present ? "Found a" : "No"); + + if (!present && !hot_plugable) { + /* No device present. */ + stopwatch_init_usecs_expire(&timeout, 50 * 1000); + pci_or_config32(dev, 0x338, 1 << 26); + + while (!stopwatch_expired(&timeout)) { + if ((pci_read_config32(dev, 0x328) & (0x1f << 23)) == 0) + break; + udelay(100); + } + dev->enabled = 0; + } else if (present && !hot_plugable && !dev->enabled) { + /* Port will be disabled, but device present. Disable link. */ + pci_or_config32(dev, cap + PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_LD); + } +} + /* Special handling for PCIe Root Port devices */ static void pch_pcie_enable(struct device *dev) { @@ -304,6 +351,8 @@ if (!config) return; + check_device_present(dev); + /* * Save a copy of the Root Port Function Number map when * starting to walk the list of PCIe Root Ports so it can -- To view, visit https://review.coreboot.org/c/coreboot/+/78226?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8ccfcab2e0e4faba8322755a4f8c2108d9b007ac Gerrit-Change-Number: 78226 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2023