coreboot-gerrit November 2024

coreboot-gerrit@coreboot.org

1 participants
2177 discussions

[S] Change in coreboot[main]: soc/intel/xeon_sp/skx: Fix CPU init
by Shuo Liu (Code Review) 26 Nov '24

26 Nov '24

Attention is currently required from: Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Tim Chu. Shuo Liu has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/85289?usp=email ) Change subject: soc/intel/xeon_sp/skx: Fix CPU init ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: TESTED = tiogapass? -- To view, visit https://review.coreboot.org/c/coreboot/+/85289?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If08ef5150b104b0c2329fcb64a0476ce641c831c Gerrit-Change-Number: 85289 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Tue, 26 Nov 2024 02:55:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[L] Change in coreboot[main]: mainboard: Add MSI MS-7788
by Name of user not set (Code Review) 26 Nov '24

26 Nov '24

Attention is currently required from: Angel Pons, Name of user not set #1005283. Hello Angel Pons, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/79979?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: mainboard: Add MSI MS-7788 ...................................................................... mainboard: Add MSI MS-7788 HACKS: * The SSKPD read hangs the boot, not sure why * I disabled it and everything seems to work... * I changed Sandybridge `chipset.cb` because I was getting warnings in my logs about leftover static devices * This is definitely not the correct way to do things, so I want to know what the right course of action actually _is_ * Do I leave the warning be? * Or is there something else I can do? NOTES: * Been running this BIOS for a while: it's working well for the parts I've tested * MSI h61m-p31/w8 * Winbond 25Q64FVSIG * Fintek F71868A * I haven't hooked this up because there is no code specifically for this chip, I might experiment with the F71869AD code in the future, but right now it's working fine without BIOS superio support Working: * PCIe graphics * Integrated graphics (libgfxinit) * USB (all ports) * Ethernet * All SATA ports * Serial output Yet-to-be-thoroughly-tested: * Suspend states Untested: * PS/2 * PCIe x1 lane Signed-off-by: mrhh69 <122405954+mrhh69(a)users.noreply.github.com> Change-Id: Ia7830fcc7552e9734ec7c57e508a6a77f689e5b0 --- A src/mainboard/msi/ms7788/Kconfig A src/mainboard/msi/ms7788/Kconfig.name A src/mainboard/msi/ms7788/Makefile.mk A src/mainboard/msi/ms7788/acpi/ec.asl A src/mainboard/msi/ms7788/acpi/platform.asl A src/mainboard/msi/ms7788/acpi/superio.asl A src/mainboard/msi/ms7788/board_info.txt A src/mainboard/msi/ms7788/data.vbt A src/mainboard/msi/ms7788/devicetree.cb A src/mainboard/msi/ms7788/dsdt.asl A src/mainboard/msi/ms7788/gma-mainboard.ads A src/mainboard/msi/ms7788/gpio.c A src/mainboard/msi/ms7788/hda_verb.c M src/northbridge/intel/sandybridge/chipset.cb M src/northbridge/intel/sandybridge/romstage.c 15 files changed, 363 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/79979/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/79979?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia7830fcc7552e9734ec7c57e508a6a77f689e5b0 Gerrit-Change-Number: 79979 Gerrit-PatchSet: 6 Gerrit-Owner: Name of user not set #1005283 Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Name of user not set #1005283 Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Name of user not set #1005283 Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>

1 0

[M] Change in coreboot[main]: arch/x86: Enable support for IOAPIC devices
by Shuo Liu (Code Review) 26 Nov '24

26 Nov '24

Attention is currently required from: Cliff Huang, Jérémy Compostella, Lance Zhao, Lean Sheng Tan, Patrick Rudolph, Subrata Banik, Tim Wawrzynczak, yuchi.chen(a)intel.com. Shuo Liu has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/85226?usp=email ) Change subject: arch/x86: Enable support for IOAPIC devices ...................................................................... Patch Set 2: (1 comment) File src/acpi/acpi_apic.c: https://review.coreboot.org/c/coreboot/+/85226/comment/7ddcb09c_994b934c?us… : PS2, Line 244: assert(!CONFIG(ACPI_COMMON_MADT_IOAPIC) || dev->path.ioapic.gsi_base != 0); Should we have a comment here to indicate the IOAPIC gsi0 don't need to have a devicetree object? Or to explicitly continue when ACPI_COMMON_MADT_IOAPIC is selected and dev->path.ioapic.gsi_base == 0? -- To view, visit https://review.coreboot.org/c/coreboot/+/85226?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie13d4f5c4f0704f0935974f90e5b7cf24e94aab3 Gerrit-Change-Number: 85226 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: yuchi.chen(a)intel.com Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: yuchi.chen(a)intel.com Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Tue, 26 Nov 2024 02:46:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in coreboot[main]: mainboard: Add MSI MS-7788
by Name of user not set (Code Review) 26 Nov '24

26 Nov '24

Attention is currently required from: Angel Pons, Name of user not set #1005283. Hello Angel Pons, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/79979?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: mainboard: Add MSI MS-7788 ...................................................................... mainboard: Add MSI MS-7788 HACKS: * The SSKPD read hangs the boot, not sure why * I disabled it and everything seems to work... * I changed Sandybridge `chipset.cb` because I was getting warnings in my logs about leftover static devices * This is definitely not the correct way to do things, so I want to know what the right course of action actually _is_ * Do I leave the warning be? * Or is there something else I can do? NOTES: * Been running this BIOS for a while: it's working well for the parts I've tested * MSI h61m-p31/w8 * Winbond 25Q64FVSIG * Fintek F71868A * I haven't hooked this up because there is no code specifically for this chip, I might experiment with the F71869AD code in the future, but right now it's working fine without BIOS superio support Working: * PCIe graphics * Integrated graphics (libgfxinit) * USB (all ports) * Ethernet * All SATA ports * Serial output Yet-to-be-thoroughly-tested: * Suspend states Untested: * PS/2 * PCIe x1 lane Signed-off-by: mrhh69 <122405954+mrhh69(a)users.noreply.github.com> Change-Id: Ia7830fcc7552e9734ec7c57e508a6a77f689e5b0 --- A src/mainboard/msi/ms7788/Kconfig A src/mainboard/msi/ms7788/Kconfig.name A src/mainboard/msi/ms7788/Makefile.mk A src/mainboard/msi/ms7788/acpi/ec.asl A src/mainboard/msi/ms7788/acpi/platform.asl A src/mainboard/msi/ms7788/acpi/superio.asl A src/mainboard/msi/ms7788/board_info.txt A src/mainboard/msi/ms7788/data.vbt A src/mainboard/msi/ms7788/devicetree.cb A src/mainboard/msi/ms7788/dsdt.asl A src/mainboard/msi/ms7788/gma-mainboard.ads A src/mainboard/msi/ms7788/gpio.c A src/mainboard/msi/ms7788/hda_verb.c M src/northbridge/intel/sandybridge/chipset.cb M src/northbridge/intel/sandybridge/romstage.c 15 files changed, 365 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/79979/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/79979?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia7830fcc7552e9734ec7c57e508a6a77f689e5b0 Gerrit-Change-Number: 79979 Gerrit-PatchSet: 5 Gerrit-Owner: Name of user not set #1005283 Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Name of user not set #1005283 Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Name of user not set #1005283 Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>

1 0

[M] Change in coreboot[main]: arch/x86: Enable support for IOAPIC devices
by Shuo Liu (Code Review) 26 Nov '24

26 Nov '24

Attention is currently required from: Cliff Huang, Jérémy Compostella, Lance Zhao, Lean Sheng Tan, Patrick Rudolph, Subrata Banik, Tim Wawrzynczak, yuchi.chen(a)intel.com. Shuo Liu has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/85226?usp=email ) Change subject: arch/x86: Enable support for IOAPIC devices ...................................................................... Patch Set 2: (1 comment) File src/acpi/acpi_apic.c: https://review.coreboot.org/c/coreboot/+/85226/comment/a28a4ebe_ba9e9f90?us… : PS1, Line 243: while ((dev = dev_find_path(dev, DEVICE_PATH_IOAPIC)) != NULL) { > Not sure if this part could be covered by customer MADT? […] Checked twice, I think I agree with you on this. So I suppose there will be a patch later to remove soc_get_ioapic_info, right? -- To view, visit https://review.coreboot.org/c/coreboot/+/85226?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie13d4f5c4f0704f0935974f90e5b7cf24e94aab3 Gerrit-Change-Number: 85226 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: yuchi.chen(a)intel.com Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: yuchi.chen(a)intel.com Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Tue, 26 Nov 2024 02:42:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>

1 0

[L] Change in coreboot[main]: mainboard: Add MSI MS-7788
by Name of user not set (Code Review) 26 Nov '24

26 Nov '24

Attention is currently required from: Angel Pons, Name of user not set #1005283. Name of user not set #1005283 has posted comments on this change by Name of user not set #1005283. ( https://review.coreboot.org/c/coreboot/+/79979?usp=email ) Change subject: mainboard: Add MSI MS-7788 ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/79979?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia7830fcc7552e9734ec7c57e508a6a77f689e5b0 Gerrit-Change-Number: 79979 Gerrit-PatchSet: 4 Gerrit-Owner: Name of user not set #1005283 Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Name of user not set #1005283 Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Name of user not set #1005283 Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 26 Nov 2024 02:42:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No

1 0

[M] Change in coreboot[main]: drivers/wifi: Support Extended Bluetooth Regulatory Descriptor
by Subrata Banik (Code Review) 26 Nov '24

26 Nov '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Hannah Williams, Jamie Ryu, Jérémy Compostella, Kapil Porwal, YH Lin, Zhixing Ma, dinesh.sharma(a)intel.com. Subrata Banik has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/84947?usp=email ) Change subject: drivers/wifi: Support Extended Bluetooth Regulatory Descriptor ...................................................................... Patch Set 9: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84947?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iebe95815c944d045f4cf686abcd1874a8a45e250 Gerrit-Change-Number: 84947 Gerrit-PatchSet: 9 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: YH Lin <yueherngl(a)google.com> Gerrit-Reviewer: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: dinesh.sharma(a)intel.com Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: YH Lin <yueherngl(a)google.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: dinesh.sharma(a)intel.com Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Tue, 26 Nov 2024 02:22:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in coreboot[main]: arch/x86: Enable support for IOAPIC devices
by Shuo Liu (Code Review) 26 Nov '24

26 Nov '24

Attention is currently required from: Cliff Huang, Jérémy Compostella, Lance Zhao, Lean Sheng Tan, Patrick Rudolph, Subrata Banik, Tim Wawrzynczak, yuchi.chen(a)intel.com. Shuo Liu has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/85226?usp=email ) Change subject: arch/x86: Enable support for IOAPIC devices ...................................................................... Patch Set 2: (1 comment) File src/arch/x86/ioapic.c: https://review.coreboot.org/c/coreboot/+/85226/comment/fe8702ed_fbcd1196?us… : PS1, Line 207: dummy.upstream = bus; > It looks like only the path is used, so we can skip the assignment of upstream here? Done -- To view, visit https://review.coreboot.org/c/coreboot/+/85226?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie13d4f5c4f0704f0935974f90e5b7cf24e94aab3 Gerrit-Change-Number: 85226 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: yuchi.chen(a)intel.com Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: yuchi.chen(a)intel.com Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Tue, 26 Nov 2024 02:19:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>

1 0

[XL] Change in coreboot[main]: soc/intel/snowridge: Add support for Intel Atom Snow Ridge SoC
by Shuo Liu (Code Review) 26 Nov '24

26 Nov '24

1 0

[XL] Change in coreboot[main]: soc/intel/snowridge: Add support for Intel Atom Snow Ridge SoC
by Name of user not set (Code Review) 26 Nov '24

26 Nov '24

Attention is currently required from: Angel Pons, Elyes Haouas, Felix Held, Felix Singer, Frans Hendriks, Jérémy Compostella, Maximilian Brune, Patrick Rudolph, Paul Menzel, Shuo Liu. yuchi.chen(a)intel.com has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/83321?usp=email ) Change subject: soc/intel/snowridge: Add support for Intel Atom Snow Ridge SoC ...................................................................... Patch Set 63: (12 comments) File src/soc/intel/snowridge/Kconfig: https://review.coreboot.org/c/coreboot/+/83321/comment/7600e5a9_2a17ef32?us… : PS28, Line 39: select FSP_CAR > Does native CAR work? As Vasiliy replied to you in another comment, native CAR doesn't works on Snow Ridge. File src/soc/intel/snowridge/acpi.c: https://review.coreboot.org/c/coreboot/+/83321/comment/3e267b04_eeeb7607?us… : PS60, Line 115: intel_write_pci_PRT(scope, pin_irq_map, map_count, &pirq_map); > The scope could be obtained from domain's device path, e.g. […] Done File src/soc/intel/snowridge/acpi/pcie.asl: https://review.coreboot.org/c/coreboot/+/83321/comment/441356a6_519f544b?us… : PS33, Line 235: > Are RP04, RP05, RP06, RP07 not implemented in Snow Ridge? Yes in Snow Ridge there are 2 Root Port clusters, cluster 0 and cluster 2. Cluster 0 contains Root Port 0~4 and cluster 2 contains Root Port 8~11. In the latest patch the ACPI PRT method for Root Port is generated dynamically. File src/soc/intel/snowridge/acpi/southcluster.asl: https://review.coreboot.org/c/coreboot/+/83321/comment/ca9aee79_cfdec085?us… : PS28, Line 13: // SATA Controller 0 : Device (SAT0) : { : Name (_ADR, 0x00070000) : Name (_DDN, "SATA Controller 0") : } : : // SATA Controller 2 : Device (SAT2) : { : Name (_ADR, 0x000e0000) : Name (_DDN, "SATA Controller 2") : } : > No SATA Controller 1? Yes there are only SATA controller 0 and SATA controller 2 in Snow Ridge. They are only place holders thus I deleted them in the latest patch. File src/soc/intel/snowridge/chip.c: https://review.coreboot.org/c/coreboot/+/83321/comment/997b32a4_b4fb5643?us… : PS28, Line 229: die("Please add domain 0 to device tree!\n"); > Should be able to guarantee this using a chipset devicetree, btw. In the latest patch the device tree is moved to `src/soc/intel/snowridge/chipset.cb`, mainboard only contains an empty device tree to pass compilation. File src/soc/intel/snowridge/common/fsp_hob.h: https://review.coreboot.org/c/coreboot/+/83321/comment/dfddb3d1_90d0f101?us… : PS28, Line 10: extern const uint8_t fsp_hob_fia_override_status_guid[16]; > Could use `guid_t` I believe: https://github. […] Done File src/soc/intel/snowridge/common/hob_display.c: https://review.coreboot.org/c/coreboot/+/83321/comment/97c02057_13647006?us… : PS28, Line 12: const void *guid; > Would be nice to use `guid_t` (well, `const guid_t *guid`): https://github. […] Done File src/soc/intel/snowridge/common/kti_cache.c: PS28: > Does KTI refer to UPI (high-speed CPU interconnect links on server platforms), or is it something el […] yes, in FSP output its call KTI. I think KTI is code name, and UPI is official name. File src/soc/intel/snowridge/common/reset.c: https://review.coreboot.org/c/coreboot/+/83321/comment/be381012_780f9544?us… : PS28, Line 26: do_full_reset(); > Does the CSE or SPS need to be notified about the global reset? This feels odd Actually I'm not very familiar with this. I checked other `do_global_reset()` implementations, they called `cse_request_global_reset()` in `src/soc/intel/common/block/cse/cse.c`, but on Snow Ridge, we didn't use that common code. File src/soc/intel/snowridge/cpu.c: https://review.coreboot.org/c/coreboot/+/83321/comment/79a760d7_c7ad3111?us… : PS28, Line 72: /** : * Skip Pre MP init MTRR programming as MTRRs are mirrored from BSP that are set prior to : * ramstage. Real MTRRs programming are being done after resource allocation. : */ > nit: reflow comment to not exceed 96 characters per line […] Done File src/soc/intel/snowridge/heci.c: https://review.coreboot.org/c/coreboot/+/83321/comment/5dd22fc8_795d46c5?us… : PS28, Line 25: res->flags |= IORESOURCE_MEM; > Wouldn't `IORESOURCE_MEM` be set already? Or is it mutuallyy exclusive with `IORESOURCE_PCI64`? `IORESOURCE_MEM` is removed. File src/soc/intel/snowridge/include/soc/iomap.h: https://review.coreboot.org/c/coreboot/+/83321/comment/e40688ed_3bfe5a0e?us… : PS28, Line 13: @sa > Does this mean "see also"? Not sure yes that's a conventional usage of Doxygen. -- To view, visit https://review.coreboot.org/c/coreboot/+/83321?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I32ad836dfaaff0d1816eac41e5a7d19ece11080f Gerrit-Change-Number: 83321 Gerrit-PatchSet: 63 Gerrit-Owner: yuchi.chen(a)intel.com Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Vasiliy Khoruzhick <vasilykh(a)arista.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Tue, 26 Nov 2024 02:09:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>

1 0

← Newer
1
...
9
10
11
12
13
14
15
...
218
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2024