coreboot-gerrit January 2024

coreboot-gerrit@coreboot.org

1 participants
3044 discussions

[L] Change in coreboot[main]: soc/intel/skylake: Drop redundant PcieRpEnable
by Nico Huber (Code Review) 15 Jan '24

15 Jan '24

Attention is currently required from: Angel Pons, Erik van den Bogaert, Felix Singer, Frans Hendriks, Jan Samek, Jeremy Soller, Jonathon Hall, Michael Niewöhner, Michał Żygowski, Nicholas Sudsgaard, Piotr Król, Sean Rhodes, Tim Crawford. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79917?usp=email ) Change subject: soc/intel/skylake: Drop redundant PcieRpEnable ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Your patch shouldn't change device state. […] The important part is that we carefully look into the cases where FSP and coreboot settings disagree. So what is you suggestion to do with this patch now? Wait that yours are reviewed and then wait until somebody re-works this one? -- To view, visit https://review.coreboot.org/c/coreboot/+/79917?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: I2f7e3e1dc6b3d8d6159bd4701e6fd90f4b0f67f4 Gerrit-Change-Number: 79917 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Jan Samek <jan.samek(a)siemens.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Jan Samek <jan.samek(a)siemens.com> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Comment-Date: Mon, 15 Jan 2024 16:22:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Jan Samek <jan.samek(a)siemens.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/intel/apollolake: Fix PCI memory resource alloc
by Matt DeVillier (Code Review) 15 Jan '24

15 Jan '24

Attention is currently required from: Paul Menzel, Reto Buerki. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79957?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: soc/intel/apollolake: Fix PCI memory resource alloc ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/79957/comment/70fb5f83_aebdbe57 : PS2, Line 9: Appollo Apollo -- To view, visit https://review.coreboot.org/c/coreboot/+/79957?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: I3a805beb47ab4d19cf8dfce0942485e7982861b1 Gerrit-Change-Number: 79957 Gerrit-PatchSet: 2 Gerrit-Owner: Reto Buerki <reet(a)codelabs.ch> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Reto Buerki <reet(a)codelabs.ch> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Mon, 15 Jan 2024 16:19:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: intel tgl mainboards: Move PcieRpEnable option below dt entries
by Felix Singer (Code Review) 15 Jan '24

15 Jan '24

Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79960?usp=email ) Change subject: intel tgl mainboards: Move PcieRpEnable option below dt entries ...................................................................... intel tgl mainboards: Move PcieRpEnable option below dt entries There is work being done on better integrating the root port entries from the devicetree by hooking up the FSP option PcieRpEnable to them, which supersedes the devicetree option. Move the PcieRpEnable option below their related devicetree entries in order to make the review easier when the option is removed. Create devicetree entries in case of they don't exist yet. Change-Id: I65bf27342191129b433d779774e084eecb4e4b3e Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/chronicler/overridetree.cb M src/mainboard/google/volteer/variants/elemi/overridetree.cb M src/mainboard/google/volteer/variants/voema/overridetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb 6 files changed, 51 insertions(+), 31 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/79960/1 diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index 078deb2..e9165c7 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -113,27 +113,23 @@ register "gen3_dec" = "0x00fc0901" # Enable NVMe PCIE 9 using clk 0 - register "PcieRpEnable[8]" = "1" register "PcieRpLtrEnable[8]" = "1" register "PcieClkSrcUsage[0]" = "8" register "PcieClkSrcClkReq[0]" = "0" register "PcieRpSlotImplemented[8]" = "1" # Enable Optane PCIE 11 using clk 0 - register "PcieRpEnable[10]" = "1" register "PcieRpLtrEnable[10]" = "1" register "HybridStorageMode" = "0" register "PcieRpSlotImplemented[10]" = "1" # Enable SD Card PCIE 8 using clk 3 - register "PcieRpEnable[7]" = "1" register "PcieRpLtrEnable[7]" = "1" register "PcieRpHotPlug[7]" = "1" register "PcieClkSrcUsage[3]" = "7" register "PcieClkSrcClkReq[3]" = "3" # Enable WLAN PCIE 7 using clk 1 - register "PcieRpEnable[6]" = "1" register "PcieRpLtrEnable[6]" = "1" register "PcieClkSrcUsage[1]" = "6" register "PcieClkSrcClkReq[1]" = "1" @@ -470,8 +466,11 @@ device ref heci1 on end device ref sata on end device ref pcie_rp1 on end - device ref pcie_rp7 on end + device ref pcie_rp7 on + register "PcieRpEnable[6]" = "1" + end device ref pcie_rp8 on + register "PcieRpEnable[7]" = "1" probe DB_SD SD_GL9755S probe DB_SD SD_RTS5261 probe DB_SD SD_RTS5227S @@ -497,8 +496,12 @@ end end end - device ref pcie_rp9 on end - device ref pcie_rp11 on end + device ref pcie_rp9 on + register "PcieRpEnable[8]" = "1" + end + device ref pcie_rp11 on + register "PcieRpEnable[10]" = "1" + end device ref uart0 on end device ref gspi0 on chip drivers/spi/acpi diff --git a/src/mainboard/google/volteer/variants/chronicler/overridetree.cb b/src/mainboard/google/volteer/variants/chronicler/overridetree.cb index 28f72d7..fe5b0bb 100644 --- a/src/mainboard/google/volteer/variants/chronicler/overridetree.cb +++ b/src/mainboard/google/volteer/variants/chronicler/overridetree.cb @@ -6,7 +6,6 @@ register "typec_aux_bias_pads[0]" = "{.pad_auxp_dc = GPP_E10, .pad_auxn_dc = GPP_E13}" # Enable EMMC PCIE 5 using clk 5 - register "PcieRpEnable[4]" = "1" register "PcieRpLtrEnable[4]" = "1" register "PcieRpHotPlug[4]" = "1" register "PcieClkSrcUsage[5]" = "4" @@ -278,7 +277,9 @@ device generic 0 on end end end - device ref pcie_rp5 on end + device ref pcie_rp5 on + register "PcieRpEnable[4]" = "1" + end device ref pmc hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. diff --git a/src/mainboard/google/volteer/variants/elemi/overridetree.cb b/src/mainboard/google/volteer/variants/elemi/overridetree.cb index 2152ec4..7edd297 100644 --- a/src/mainboard/google/volteer/variants/elemi/overridetree.cb +++ b/src/mainboard/google/volteer/variants/elemi/overridetree.cb @@ -6,7 +6,6 @@ register "typec_aux_bias_pads[0]" = "{.pad_auxp_dc = GPP_E10, .pad_auxn_dc = GPP_E13}" # Enable EMMC PCIE 5 using clk 5 - register "PcieRpEnable[4]" = "1" register "PcieRpLtrEnable[4]" = "1" register "PcieRpHotPlug[4]" = "1" register "PcieClkSrcUsage[5]" = "4" @@ -279,7 +278,9 @@ device generic 0 on end end end - device ref pcie_rp5 on end + device ref pcie_rp5 on + register "PcieRpEnable[4]" = "1" + end device ref pmc hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. diff --git a/src/mainboard/google/volteer/variants/voema/overridetree.cb b/src/mainboard/google/volteer/variants/voema/overridetree.cb index d101b5d..90172fc 100644 --- a/src/mainboard/google/volteer/variants/voema/overridetree.cb +++ b/src/mainboard/google/volteer/variants/voema/overridetree.cb @@ -12,13 +12,11 @@ register "typec_aux_bias_pads[0]" = "{.pad_auxp_dc = GPP_E10, .pad_auxn_dc = GPP_E13}" # Disable WLAN PCIE 7 - register "PcieRpEnable[6]" = "0" register "PcieRpLtrEnable[6]" = "0" register "PcieClkSrcUsage[1]" = "PCIE_CLK_NOTUSED" register "PcieRpSlotImplemented[6]" = "1" # Disable SD Card PCIE 8 - register "PcieRpEnable[7]" = "0" register "PcieRpLtrEnable[7]" = "0" register "PcieRpHotPlug[7]" = "0" register "PcieClkSrcUsage[3]" = "PCIE_CLK_NOTUSED" @@ -102,6 +100,17 @@ probe AUDIO MAX98360_ALC5682I_I2S probe AUDIO RT1011_ALC5682I_I2S end + device ref pcie_rp7 off + register "PcieRpEnable[6]" = "0" + end + device ref pcie_rp8 off + register "PcieRpEnable[7]" = "0" + probe DB_SD SD_GL9755S + probe DB_SD SD_RTS5261 + probe DB_SD SD_RTS5227S + probe DB_SD SD_GL9750 + probe DB_SD SD_OZ711LV2LN + end device ref pcie_rp9 on chip soc/intel/common/block/pcie/rtd3 register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_B2)" diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb index 1af05c4..506825a 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb @@ -37,10 +37,6 @@ # EC memory map range is 0x900-0x9ff register "gen3_dec" = "0x00fc0901" - register "PcieRpEnable[2]" = "1" - register "PcieRpEnable[3]" = "1" - register "PcieRpEnable[8]" = "1" - register "PcieRpEnable[10]" = "1" register "PcieRpSlotImplemented[2]" = "1" register "PcieRpSlotImplemented[3]" = "1" register "PcieRpSlotImplemented[8]" = "1" @@ -279,21 +275,28 @@ device pci 19.2 on end # UART2 0xA0C7 device pci 1c.0 off end # RP1 0xA0B8 device pci 1c.1 off end # RP2 0xA0B9 - device pci 1c.2 on end # RP3 0xA0BA - device pci 1c.3 on + device pci 1c.2 on # RP3 0xA0BA + register "PcieRpEnable[2]" = "1" + end + device pci 1c.3 on # RP4 0xA0BB + register "PcieRpEnable[3]" = "1" chip soc/intel/common/block/pcie/rtd3 register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_B17)" register "srcclk_pin" = "2" device generic 0 on end end - end # RP4 0xA0BB + end device pci 1c.4 off end # RP5 0xA0BC device pci 1c.5 off end # RP6 0xA0BD device pci 1c.6 off end # RP7 0xA0BE device pci 1c.7 off end # RP8 0xA0BF - device pci 1d.0 on end # RP9 0xA0B0 + device pci 1d.0 on # RP9 0xA0B0 + register "PcieRpEnable[8]" = "1" + end device pci 1d.1 off end # RP10 0xA0B1 - device pci 1d.2 on end # RP11 0xA0B2 + device pci 1d.2 on # RP11 0xA0B2 + register "PcieRpEnable[10]" = "1" + end device pci 1d.3 off end # RP12 0xA0B3 device pci 1e.0 off end # UART0 0xA0A8 device pci 1e.1 off end # UART1 0xA0A9 diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb index ad1a45d..9b1177e 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb @@ -38,10 +38,6 @@ # EC memory map range is 0x900-0x9ff register "gen3_dec" = "0x00fc0901" - register "PcieRpEnable[2]" = "1" - register "PcieRpEnable[3]" = "1" - register "PcieRpEnable[8]" = "1" - register "PcieRpEnable[10]" = "1" register "PcieRpSlotImplemented[2]" = "1" register "PcieRpSlotImplemented[3]" = "1" register "PcieRpSlotImplemented[8]" = "1" @@ -283,21 +279,28 @@ device pci 19.2 on end # UART2 0xA0C7 device pci 1c.0 off end # RP1 0xA0B8 device pci 1c.1 off end # RP2 0xA0B9 - device pci 1c.2 on end # RP3 0xA0BA - device pci 1c.3 on + device pci 1c.2 on # RP3 0xA0BA + register "PcieRpEnable[2]" = "1" + end + device pci 1c.3 on # RP4 0xA0BB + register "PcieRpEnable[3]" = "1" chip soc/intel/common/block/pcie/rtd3 register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_B17)" register "srcclk_pin" = "2" device generic 0 on end end - end # RP4 0xA0BB + end device pci 1c.4 off end # RP5 0xA0BC device pci 1c.5 off end # RP6 0xA0BD device pci 1c.6 off end # RP7 0xA0BE device pci 1c.7 off end # RP8 0xA0BF - device pci 1d.0 on end # RP9 0xA0B0 + device pci 1d.0 on # RP9 0xA0B0 + register "PcieRpEnable[8]" = "1" + end device pci 1d.1 off end # RP10 0xA0B1 - device pci 1d.2 on end # RP11 0xA0B2 + device pci 1d.2 on # RP11 0xA0B2 + register "PcieRpEnable[10]" = "1" + end device pci 1d.3 off end # RP12 0xA0B3 device pci 1e.0 off end # UART0 0xA0A8 device pci 1e.1 off end # UART1 0xA0A9 -- To view, visit https://review.coreboot.org/c/coreboot/+/79960?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: I65bf27342191129b433d779774e084eecb4e4b3e Gerrit-Change-Number: 79960 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: include/device/pci_mmio_cfg: assert CONFIG_ECAM_MMCONF_BUS_NUMBER != 0
by Matt DeVillier (Code Review) 15 Jan '24

15 Jan '24

Attention is currently required from: Felix Held. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79933?usp=email ) Change subject: include/device/pci_mmio_cfg: assert CONFIG_ECAM_MMCONF_BUS_NUMBER != 0 ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/79933?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: Ic102b7dca9ffebb2d384a068a1fb1f4b6fb6c5f8 Gerrit-Change-Number: 79933 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 15 Jan 2024 16:09:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: soc/intel/skylake: Drop redundant PcieRpEnable
by Felix Singer (Code Review) 15 Jan '24

15 Jan '24

Attention is currently required from: Angel Pons, Erik van den Bogaert, Frans Hendriks, Jan Samek, Jeremy Soller, Jonathon Hall, Michael Niewöhner, Michał Żygowski, Nicholas Sudsgaard, Nico Huber, Piotr Król, Sean Rhodes, Tim Crawford. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79917?usp=email ) Change subject: soc/intel/skylake: Drop redundant PcieRpEnable ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > This would be a lot easier if you would review. If we do it in multiple patches, […] Your patch shouldn't change device state. It only should do the switch, but both changes in one patch are too much. I also found cases for the other way: A PCIe device is set to enabled in the devicetree but the related FSP option is not set. -- To view, visit https://review.coreboot.org/c/coreboot/+/79917?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: I2f7e3e1dc6b3d8d6159bd4701e6fd90f4b0f67f4 Gerrit-Change-Number: 79917 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Jan Samek <jan.samek(a)siemens.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Jan Samek <jan.samek(a)siemens.com> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Comment-Date: Mon, 15 Jan 2024 16:03:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Jan Samek <jan.samek(a)siemens.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: soc/intel/skylake: Drop redundant PcieRpEnable
by Nico Huber (Code Review) 15 Jan '24

15 Jan '24

Attention is currently required from: Angel Pons, Erik van den Bogaert, Felix Singer, Frans Hendriks, Jan Samek, Jeremy Soller, Jonathon Hall, Michael Niewöhner, Michał Żygowski, Nicholas Sudsgaard, Piotr Król, Sean Rhodes, Tim Crawford. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79917?usp=email ) Change subject: soc/intel/skylake: Drop redundant PcieRpEnable ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > ok, fair enough This would be a lot easier if you would review. If we do it in multiple patches, we have to review multiple times. -- To view, visit https://review.coreboot.org/c/coreboot/+/79917?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: I2f7e3e1dc6b3d8d6159bd4701e6fd90f4b0f67f4 Gerrit-Change-Number: 79917 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Jan Samek <jan.samek(a)siemens.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Jan Samek <jan.samek(a)siemens.com> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Comment-Date: Mon, 15 Jan 2024 15:54:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Jan Samek <jan.samek(a)siemens.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: device/device_const: add pcidev_path_on_segbus
by Felix Held (Code Review) 15 Jan '24

15 Jan '24

Attention is currently required from: Patrick Rudolph. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79980?usp=email ) Change subject: device/device_const: add pcidev_path_on_segbus ...................................................................... Patch Set 1: (1 comment) File src/device/device_const.c: https://review.coreboot.org/c/coreboot/+/79980/comment/43d61682_bbc2433f : PS1, Line 231: DEVTREE_CONST struct device *pcidev_path_on_segbus(unsigned int segment_group, Nico commented on this code when it was still part of CB:79927 : "TBH, I don't think this is a good idea. There are cases where looking up devices by bus numbers is unavoidable, e.g. external programs device/oprom/ etc. But I wouldn't want to encourage usage by adding more APIs. (Also, the implementation below looks very questionable: starts with pci_root_bus() but still walks the global device list). The code is not wrong, though, so I'll leave this resolved." -- To view, visit https://review.coreboot.org/c/coreboot/+/79980?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: Ibbbee9966abc997ded6389e72912e621c1f0bdbf Gerrit-Change-Number: 79980 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Comment-Date: Mon, 15 Jan 2024 15:53:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: device: Add support for multiple PCI segment groups
by Felix Held (Code Review) 15 Jan '24

15 Jan '24

Attention is currently required from: Arthur Heymans, Christian Walter, Cliff Huang, Fred Reitberger, Jason Glenesk, Johnny Lin, Jérémy Compostella, Lance Zhao, Matt DeVillier, Nico Huber, Patrick Rudolph, Tim Chu, Tim Wawrzynczak. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79927?usp=email ) Change subject: device: Add support for multiple PCI segment groups ...................................................................... Patch Set 11: (2 comments) Patchset: PS8: > the last few things where i'm not 100% sure how to handle those are the yabel and realmode oprom exe […] done File src/device/device_const.c: https://review.coreboot.org/c/coreboot/+/79927/comment/b24665c8_2d4f4f07 : PS7, Line 233: pci_devfn_t devfn) > TBH, I don't think this is a good idea. There are cases where looking up […] factored out that one into another patch; also talked to Patrick and he'll have a look if the xeon sp patch that uses that function could be modified to not need it -- To view, visit https://review.coreboot.org/c/coreboot/+/79927?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: Ied3313c41896362dd989ee2ab1b1bcdced840aa8 Gerrit-Change-Number: 79927 Gerrit-PatchSet: 11 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Mon, 15 Jan 2024 15:47:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: device/device_const: add pcidev_path_on_segbus
by Felix Held (Code Review) 15 Jan '24

15 Jan '24

Attention is currently required from: Patrick Rudolph. Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/79980?usp=email to review the following change. Change subject: device/device_const: add pcidev_path_on_segbus ...................................................................... device/device_const: add pcidev_path_on_segbus Add pcidev_path_on_segbus which is pcidev_path_on_bus that takes the PCI segment group number as argument instead of only using segment group 0. Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Change-Id: Ibbbee9966abc997ded6389e72912e621c1f0bdbf --- M src/device/device_const.c M src/include/device/device.h 2 files changed, 12 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/79980/1 diff --git a/src/device/device_const.c b/src/device/device_const.c index c91c009..3a6233b 100644 --- a/src/device/device_const.c +++ b/src/device/device_const.c @@ -225,8 +225,16 @@ DEVTREE_CONST struct device *pcidev_path_on_bus(unsigned int bus, pci_devfn_t devfn) { + return pcidev_path_on_segbus(0, bus, devfn); +} + +DEVTREE_CONST struct device *pcidev_path_on_segbus(unsigned int segment_group, + unsigned int bus, + pci_devfn_t devfn) +{ DEVTREE_CONST struct bus *parent = pci_root_bus(); DEVTREE_CONST struct device *dev = parent->children; + assert(segment_group < (CONFIG(ECAM_MMCONF_SUPPORT) ? PCI_SEGMENT_GROUP_COUNT : 1)); /* FIXME: Write the loop with topology links. */ while (dev) { @@ -234,7 +242,7 @@ dev = dev->next; continue; } - if (dev->bus->secondary == bus && dev->bus->segment_group == 0) + if (dev->bus->secondary == bus && dev->bus->segment_group == segment_group) return pcidev_path_behind(dev->bus, devfn); dev = dev->next; } diff --git a/src/include/device/device.h b/src/include/device/device.h index 50307e6..c8e45ef 100644 --- a/src/include/device/device.h +++ b/src/include/device/device.h @@ -426,6 +426,9 @@ pci_devfn_t devfn); DEVTREE_CONST struct device *pcidev_path_on_root(pci_devfn_t devfn); DEVTREE_CONST struct device *pcidev_path_on_bus(unsigned int bus, pci_devfn_t devfn); +DEVTREE_CONST struct device *pcidev_path_on_segbus(unsigned int segment, + unsigned int bus, + pci_devfn_t devfn); DEVTREE_CONST struct device *pcidev_on_root(uint8_t dev, uint8_t fn); DEVTREE_CONST struct bus *pci_root_bus(void); /* Find PCI device with given D#:F# sitting behind the given PCI-to-PCI bridge device. */ -- To view, visit https://review.coreboot.org/c/coreboot/+/79980?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: Ibbbee9966abc997ded6389e72912e621c1f0bdbf Gerrit-Change-Number: 79980 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: device: Add support for multiple PCI segment groups
by Felix Held (Code Review) 15 Jan '24

15 Jan '24

Attention is currently required from: Arthur Heymans, Christian Walter, Cliff Huang, Felix Held, Fred Reitberger, Jason Glenesk, Johnny Lin, Jérémy Compostella, Lance Zhao, Matt DeVillier, Nico Huber, Patrick Rudolph, Tim Chu, Tim Wawrzynczak. Hello Arthur Heymans, Christian Walter, Cliff Huang, Fred Reitberger, Jason Glenesk, Johnny Lin, Jérémy Compostella, Lance Zhao, Matt DeVillier, Nico Huber, Patrick Rudolph, Tim Chu, Tim Wawrzynczak, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/79927?usp=email to look at the new patch set (#11). The following approvals got outdated and were removed: Code-Review+1 by Nico Huber, Code-Review+2 by Patrick Rudolph, Verified+1 by build bot (Jenkins) The change is no longer submittable: Code-Review and Verified are unsatisfied now. Change subject: device: Add support for multiple PCI segment groups ...................................................................... device: Add support for multiple PCI segment groups Add initial support for multiple PCI segment groups. Instead of modifying secondary in the bus struct introduce a new segment_group struct element and keep existing common code. Since all platforms currently only use 1 segment this is not a functional change. On platforms that support more than 1 segment the segment has to be set when creating the PCI domain. Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ied3313c41896362dd989ee2ab1b1bcdced840aa8 --- M src/acpi/acpi.c M src/arch/x86/mpspec.c M src/device/Kconfig M src/device/device.c M src/device/device_const.c M src/device/device_util.c M src/device/pci_device.c M src/device/pci_rom.c M src/device/pcix_device.c M src/include/device/device.h M src/include/device/pci_def.h M src/include/device/pci_ops.h M src/lib/smbios.c M src/northbridge/amd/pi/00730F01/northbridge.c M src/soc/amd/common/block/acpi/ivrs.c M src/soc/amd/common/block/data_fabric/domain.c M src/soc/amd/genoa_poc/domain.c M src/soc/intel/xeon_sp/cpx/soc_acpi.c M src/soc/intel/xeon_sp/uncore.c 19 files changed, 91 insertions(+), 42 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/79927/11 -- To view, visit https://review.coreboot.org/c/coreboot/+/79927?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: Ied3313c41896362dd989ee2ab1b1bcdced840aa8 Gerrit-Change-Number: 79927 Gerrit-PatchSet: 11 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2024