coreboot-gerrit January 2021

coreboot-gerrit@coreboot.org

1 participants
6507 discussions

Change in coreboot[master]: soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP Sili...
by Angel Pons (Code Review) 07 Jan '21

07 Jan '21

Attention is currently required from: Felix Singer, Tim Crawford, Jeremy Soller. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49104 ) Change subject: soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP SiliconInit ...................................................................... Patch Set 5: Code-Review+2 (1 comment) File src/soc/intel/cannonlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/49104/comment/a12fa1fb_f6c5eb5f PS4, Line 528: /* Disable setting subsystem ID, which causes it to lock */ > write-once*. Also added a comment about requiring at least 1 entry. Oops, thanks. -- To view, visit https://review.coreboot.org/c/coreboot/+/49104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieaa45ef7fa8e0da4a25b9174ded1ea0c5d9c4b4e Gerrit-Change-Number: 49104 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Comment-Date: Thu, 07 Jan 2021 20:22:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Tim Crawford <tcrawford(a)system76.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP Sili...
by Tim Crawford (Code Review) 07 Jan '21

07 Jan '21

1 0

Change in coreboot[master]: soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP Sili...
by Tim Crawford (Code Review) 07 Jan '21

07 Jan '21

Attention is currently required from: Felix Singer, Tim Crawford, Jeremy Soller. Hello build bot (Jenkins), Nico Huber, Jeremy Soller, Angel Pons, Michael Niewöhner, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/49104 to look at the new patch set (#5). Change subject: soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP SiliconInit ...................................................................... soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP SiliconInit Prevent the FSP from writing its default SVID SDID values of 8086:7270 for internal devices as this locks most of the registers. Allows the subsystemid values set in devicetree to be used. A description of this behavior, along with example code, is provided in the TigerLake FSP Integration Guide. The xHCI and HDA devices have RW/L registers rather than RW/O registers. They can be written to multiple times but cannot be modified after being locked, which happens during FspSiliconInit. Because coreboot populates subsystem IDs after SiliconInit, these devices specifically must be written beforehand or will otherwise be locked with their default values of 0:0. Tested by checking lspci output on System76 galp3-c, oryp5, oryp6. References: - TigerLake FSP Integration Guide - Intel Document Number 337868-002 Change-Id: Ieaa45ef7fa8e0da4a25b9174ded1ea0c5d9c4b4e Signed-off-by: Jeremy Soller <jeremy(a)system76.com> Signed-off-by: Tim Crawford <tcrawford(a)system76.com> --- M src/soc/intel/cannonlake/fsp_params.c 1 file changed, 43 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/49104/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/49104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieaa45ef7fa8e0da4a25b9174ded1ea0c5d9c4b4e Gerrit-Change-Number: 49104 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/amd/picasso: Add GRXS and GTXS method
by EricR Lai (Code Review) 07 Jan '21

07 Jan '21

EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48944 ) Change subject: soc/amd/picasso: Add GRXS and GTXS method ...................................................................... soc/amd/picasso: Add GRXS and GTXS method Add GRXS and GTXS into gpiolib. We can align with Intel ACPI method for the better usage. This benefit acpi.c to be more clear, too. BUG=b:176270381 BRANCH=zork TEST=Confirm the Goodix touchscreen functional. Signed-off-by: Eric Lai <ericr_lai(a)compal.corp-partner.google.com> Change-Id: I1aa6a8f44f20577e679336889c849dd67cb99f2d --- M src/soc/amd/common/acpi/gpio_bank_lib.asl M src/soc/amd/picasso/acpi.c 2 files changed, 40 insertions(+), 40 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/48944/1 diff --git a/src/soc/amd/common/acpi/gpio_bank_lib.asl b/src/soc/amd/common/acpi/gpio_bank_lib.asl index eb13ab9..b142be5 100644 --- a/src/soc/amd/common/acpi/gpio_bank_lib.asl +++ b/src/soc/amd/common/acpi/gpio_bank_lib.asl @@ -2,6 +2,8 @@ #include <soc/iomap.h> +#define GPIO_INPUT_SHIFT 16 +#define GPIO_INPUT_VALUE (1 << GPIO_INPUT_SHIFT) #define GPIO_OUTPUT_SHIFT 22 #define GPIO_OUTPUT_VALUE (1 << GPIO_OUTPUT_SHIFT) @@ -148,3 +150,35 @@ } VAL0 &= ~GPIO_OUTPUT_VALUE } + +/* + * Get GPIO Rx Value + * Arg0 - GPIO Number + */ +Method (GRXS, 1, Serialized) +{ + OperationRegion (PREG, SystemMemory, GPAD (Arg0), 4) + Field (PREG, AnyAcc, NoLock, Preserve) + { + VAL0, 32 + } + Local0 = (GPIO_INPUT_VALUE & VAL0) >> GPIO_INPUT_SHIFT + + Return (Local0) +} + +/* + * Get GPIO Tx Value + * Arg0 - GPIO Number + */ +Method (GTXS, 1, Serialized) +{ + OperationRegion (PREG, SystemMemory, GPAD (Arg0), 4) + Field (PREG, AnyAcc, NoLock, Preserve) + { + VAL0, 32 + } + Local0 = (GPIO_OUTPUT_VALUE & VAL0) >> GPIO_OUTPUT_SHIFT + + Return (Local0) +} diff --git a/src/soc/amd/picasso/acpi.c b/src/soc/amd/picasso/acpi.c index 63ba26c..ef0d6e2 100644 --- a/src/soc/amd/picasso/acpi.c +++ b/src/soc/amd/picasso/acpi.c @@ -431,57 +431,23 @@ } } -static void acpigen_soc_get_gpio_in_local5(uintptr_t addr) +static int acpigen_soc_get_gpio_state(const char *op, unsigned int gpio_num) { - /* - * Store (\_SB.GPR2 (addr), Local5) - * \_SB.GPR2 is used to read control byte 2 from control register. - * / It is defined in gpio_lib.asl. - */ + /* Store (op (gpio_num), Local0) */ acpigen_write_store(); - acpigen_emit_namestring("\\_SB.GPR2"); - acpigen_write_integer(addr); - acpigen_emit_byte(LOCAL5_OP); -} - -static int acpigen_soc_get_gpio_val(unsigned int gpio_num, uint32_t mask) -{ - if (gpio_num >= SOC_GPIO_TOTAL_PINS) { - printk(BIOS_WARNING, "Warning: Pin %d should be smaller than" - " %d\n", gpio_num, SOC_GPIO_TOTAL_PINS); - return -1; - } - uintptr_t addr = gpio_get_address(gpio_num); - - acpigen_soc_get_gpio_in_local5(addr); - - /* If (And (Local5, mask)) */ - acpigen_write_if_and(LOCAL5_OP, mask); - - /* Store (One, Local0) */ - acpigen_write_store_ops(ONE_OP, LOCAL0_OP); - - acpigen_pop_len(); /* If */ - - /* Else */ - acpigen_write_else(); - - /* Store (Zero, Local0) */ - acpigen_write_store_ops(ZERO_OP, LOCAL0_OP); - - acpigen_pop_len(); /* Else */ - + acpigen_soc_gpio_op(op, gpio_num); + acpigen_emit_byte(LOCAL0_OP); return 0; } int acpigen_soc_read_rx_gpio(unsigned int gpio_num) { - return acpigen_soc_get_gpio_val(gpio_num, GPIO_PIN_IN); + return acpigen_soc_get_gpio_state("\\_SB.GRXS", gpio_num); } int acpigen_soc_get_tx_gpio(unsigned int gpio_num) { - return acpigen_soc_get_gpio_val(gpio_num, GPIO_PIN_OUT); + return acpigen_soc_get_gpio_state("\\_SB.GTXS", gpio_num); } static int acpigen_soc_gpio_op(const char *op, unsigned int gpio_num) -- To view, visit https://review.coreboot.org/c/coreboot/+/48944 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1aa6a8f44f20577e679336889c849dd67cb99f2d Gerrit-Change-Number: 48944 Gerrit-PatchSet: 1 Gerrit-Owner: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-MessageType: newchange

5 16

Change in coreboot[master]: soc/amd/picasso: Add STXS and CTXS method
by EricR Lai (Code Review) 07 Jan '21

07 Jan '21

EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48943 ) Change subject: soc/amd/picasso: Add STXS and CTXS method ...................................................................... soc/amd/picasso: Add STXS and CTXS method Add STXS and CTXS into gpiolib. We can align with Intel ACPI method for the better usage. This benefit acpi.c to be more clear, too. BUG=b:176270381 BRANCH=zork TEST=Confirm the Goodix touchscreen functional. Signed-off-by: Eric Lai <ericr_lai(a)compal.corp-partner.google.com> Change-Id: If4fcd68496a712fdccf44b91a6192ef58a0a9733 --- M src/soc/amd/common/acpi/gpio_bank_lib.asl M src/soc/amd/picasso/acpi.c 2 files changed, 40 insertions(+), 41 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/48943/1 diff --git a/src/soc/amd/common/acpi/gpio_bank_lib.asl b/src/soc/amd/common/acpi/gpio_bank_lib.asl index 9686ddc..ab76418 100644 --- a/src/soc/amd/common/acpi/gpio_bank_lib.asl +++ b/src/soc/amd/common/acpi/gpio_bank_lib.asl @@ -2,6 +2,8 @@ #include <soc/iomap.h> +#define PAD_CFG0_TX_STATE 0x400000 + /* Get pin control MMIO address */ Method (GPAD, 0x1) { @@ -117,3 +119,31 @@ /* Arg1 - Value for control register */ GPSB (Arg0, 3, Arg1) } + +/* + * Set GPIO Tx Value + * Arg0 - GPIO Number + */ +Method (STXS, 1, Serialized) +{ + OperationRegion (PREG, SystemMemory, GPAD (Arg0), 4) + Field (PREG, AnyAcc, NoLock, Preserve) + { + VAL0, 32 + } + VAL0 |= PAD_CFG0_TX_STATE +} + +/* + * Clear GPIO Tx Value + * Arg0 - GPIO Number + */ +Method (CTXS, 1, Serialized) +{ + OperationRegion (PREG, SystemMemory, GPAD (Arg0), 4) + Field (PREG, AnyAcc, NoLock, Preserve) + { + VAL0, 32 + } + VAL0 &= ~PAD_CFG0_TX_STATE +} diff --git a/src/soc/amd/picasso/acpi.c b/src/soc/amd/picasso/acpi.c index 17940e9..63ba26c 100644 --- a/src/soc/amd/picasso/acpi.c +++ b/src/soc/amd/picasso/acpi.c @@ -474,45 +474,6 @@ return 0; } -static int acpigen_soc_set_gpio_val(unsigned int gpio_num, uint32_t val) -{ - if (gpio_num >= SOC_GPIO_TOTAL_PINS) { - printk(BIOS_WARNING, "Warning: Pin %d should be smaller than" - " %d\n", gpio_num, SOC_GPIO_TOTAL_PINS); - return -1; - } - uintptr_t addr = gpio_get_address(gpio_num); - - /* Store (0x40, Local0) */ - acpigen_write_store(); - acpigen_write_integer(GPIO_PIN_OUT); - acpigen_emit_byte(LOCAL0_OP); - - acpigen_soc_get_gpio_in_local5(addr); - - if (val) { - /* Or (Local5, GPIO_PIN_OUT, Local5) */ - acpigen_write_or(LOCAL5_OP, LOCAL0_OP, LOCAL5_OP); - } else { - /* Not (GPIO_PIN_OUT, Local6) */ - acpigen_write_not(LOCAL0_OP, LOCAL6_OP); - - /* And (Local5, Local6, Local5) */ - acpigen_write_and(LOCAL5_OP, LOCAL6_OP, LOCAL5_OP); - } - - /* - * SB.GPW2 (addr, Local5) - * \_SB.GPW2 is used to write control byte in control register - * / byte 2. It is defined in gpio_lib.asl. - */ - acpigen_emit_namestring("\\_SB.GPW2"); - acpigen_write_integer(addr); - acpigen_emit_byte(LOCAL5_OP); - - return 0; -} - int acpigen_soc_read_rx_gpio(unsigned int gpio_num) { return acpigen_soc_get_gpio_val(gpio_num, GPIO_PIN_IN); @@ -523,12 +484,20 @@ return acpigen_soc_get_gpio_val(gpio_num, GPIO_PIN_OUT); } +static int acpigen_soc_gpio_op(const char *op, unsigned int gpio_num) +{ + /* op (gpio_num) */ + acpigen_emit_namestring(op); + acpigen_write_integer(gpio_num); + return 0; +} + int acpigen_soc_set_tx_gpio(unsigned int gpio_num) { - return acpigen_soc_set_gpio_val(gpio_num, 1); + return acpigen_soc_gpio_op("\\_SB.STXS", gpio_num); } int acpigen_soc_clear_tx_gpio(unsigned int gpio_num) { - return acpigen_soc_set_gpio_val(gpio_num, 0); + return acpigen_soc_gpio_op("\\_SB.CTXS", gpio_num); } -- To view, visit https://review.coreboot.org/c/coreboot/+/48943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If4fcd68496a712fdccf44b91a6192ef58a0a9733 Gerrit-Change-Number: 48943 Gerrit-PatchSet: 1 Gerrit-Owner: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-MessageType: newchange

5 25

Change in coreboot[master]: soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP Sili...
by Angel Pons (Code Review) 07 Jan '21

07 Jan '21

Attention is currently required from: Felix Singer, Tim Crawford, Jeremy Soller. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49104 ) Change subject: soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP SiliconInit ...................................................................... Patch Set 4: (3 comments) File src/soc/intel/cannonlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/49104/comment/4fa21c7f_8025b522 PS2, Line 550: 1 > If set to 0 (default), then FspSiliconInit will write the default 8086:7270 value. Ack File src/soc/intel/cannonlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/49104/comment/4b333560_92f306f6 PS4, Line 528: /* Disable setting subsystem ID, which causes it to lock */ I couldn't resolve `it` when parsing this sentence. How about: /* Prevent FSP from programming write-only subsystem IDs */ https://review.coreboot.org/c/coreboot/+/49104/comment/ee500075_7b3893fc PS4, Line 548: const struct svid_ssid_init_entry ssid_table[] = {0}; Do not allocate this variable on the stack. After this function returns, any pointers to local variables become dangling references because the underlying storage has been released, so any accesses through said pointers would trigger undefined behavior. Solution: Prefix this with `static`. -- To view, visit https://review.coreboot.org/c/coreboot/+/49104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieaa45ef7fa8e0da4a25b9174ded1ea0c5d9c4b4e Gerrit-Change-Number: 49104 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Comment-Date: Thu, 07 Jan 2021 19:32:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Crawford <tcrawford(a)system76.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/common: Drop ACPIMMIO bank for SMBus device PCI config
by Kyösti Mälkki (Code Review) 07 Jan '21

07 Jan '21

Attention is currently required from: Felix Held. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42685 ) Change subject: soc/amd/common: Drop ACPIMMIO bank for SMBus device PCI config ...................................................................... Patch Set 8: (1 comment) File src/soc/amd/picasso/uart.c: https://review.coreboot.org/c/coreboot/+/42685/comment/d77776a5_b0772aea PS8, Line 106: generator divisor programming? 16*115200 = 1.8432M. */ > See previous comments exchanged with Raul; I could not tell what this does, but assumed it would aff […] AFAICS one cannot currently select AMD_SOC_UART_1_8MZ anyways with psp_verstage since the register write cannot be done. And if we added ENV_X86 we have the case of changing baudrate reference clock while UART is already configured. If there is some functionality/legacy support provided here, it's still obscure to me what it is. -- To view, visit https://review.coreboot.org/c/coreboot/+/42685 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5c8ce8de0a6ab0ed41e7e8a5980d0f0510aaa993 Gerrit-Change-Number: 42685 Gerrit-PatchSet: 8 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 07 Jan 2021 19:21:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/emulation/qemu: Copy page tables to DRAM in assembly
by Angel Pons (Code Review) 07 Jan '21

07 Jan '21

Attention is currently required from: Martin Roth, Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49228 ) Change subject: mb/emulation/qemu: Copy page tables to DRAM in assembly ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/49228 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic0bdd2bef7197edd2e7488a8efdeba7eb4ab0dd4 Gerrit-Change-Number: 49228 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Comment-Date: Thu, 07 Jan 2021 19:13:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP Sili...
by Tim Crawford (Code Review) 07 Jan '21

07 Jan '21

Attention is currently required from: Felix Singer, Jeremy Soller, Angel Pons. Tim Crawford has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49104 ) Change subject: soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP SiliconInit ...................................................................... Patch Set 4: (2 comments) File src/soc/intel/cannonlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/49104/comment/1ef3e3dd_3277009b PS2, Line 552: /* Program XHCI SSID/SVID before FSP silicon init */ : dev = pcidev_path_on_root(PCH_DEVFN_XHCI); : if (!dev->subsystem_vendor || !dev->subsystem_device) { : pci_write_config32(dev, PCI_SUBSYSTEM_VENDOR_ID, : pci_read_config32(dev, PCI_VENDOR_ID)); : } else { : pci_write_config32(dev, PCI_SUBSYSTEM_VENDOR_ID, : ((dev->subsystem_device & 0xffff) << 16) | : (dev->subsystem_vendor & 0xffff)); : } : : /* Program HDAudio SSID/SVID before FSP silicon init */ : dev = pcidev_path_on_root(PCH_DEVFN_HDA); : if (!dev->subsystem_vendor || !dev->subsystem_device) { : pci_write_config32(dev, PCI_SUBSYSTEM_VENDOR_ID, : pci_read_config32(dev, PCI_VENDOR_ID)); : } else { : pci_write_config32(dev, PCI_SUBSYSTEM_VENDOR_ID, : ((dev->subsystem_device & 0xffff) << 16) | : (dev->subsystem_vendor & 0xffff)); : } > Ah, not all are RW/O... […] Done File src/soc/intel/cannonlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/49104/comment/67b6f179_989370f5 PS3, Line 555: if (dev->subsystem_vendor && dev->subsystem_device) { : pci_write_config32(dev, PCI_SUBSYSTEM_VENDOR_ID, : ((dev->subsystem_device & 0xffff) << 16) | : (dev->subsystem_vendor & 0xffff)); : } else { : pci_write_config32(dev, PCI_SUBSYSTEM_VENDOR_ID, : pci_read_config32(dev, PCI_VENDOR_ID)); : } > This logic is already implemented in `pci_dev_set_subsystem` Done -- To view, visit https://review.coreboot.org/c/coreboot/+/49104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieaa45ef7fa8e0da4a25b9174ded1ea0c5d9c4b4e Gerrit-Change-Number: 49104 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 07 Jan 2021 18:38:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Tim Crawford <tcrawford(a)system76.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/emulation/qemu: Copy page tables to DRAM in assembly
by build bot (Jenkins) (Code Review) 07 Jan '21

07 Jan '21

Attention is currently required from: Martin Roth. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49228 ) Change subject: mb/emulation/qemu: Copy page tables to DRAM in assembly ...................................................................... Patch Set 1: (2 comments) File src/mainboard/emulation/qemu-i440fx/mainboard.c: https://review.coreboot.org/c/coreboot/+/49228/comment/63c4f851_c9e1958c PS1, Line 36: /* Reserve page tables in DRAM. FIXME: Remove once x86_64 page tables reside in CBMEM */ line over 96 characters File src/mainboard/emulation/qemu-q35/mainboard.c: https://review.coreboot.org/c/coreboot/+/49228/comment/bd8fc0b7_f92687be PS1, Line 49: /* Reserve page tables in DRAM. FIXME: Remove once x86_64 page tables reside in CBMEM */ line over 96 characters -- To view, visit https://review.coreboot.org/c/coreboot/+/49228 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic0bdd2bef7197edd2e7488a8efdeba7eb4ab0dd4 Gerrit-Change-Number: 49228 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Comment-Date: Thu, 07 Jan 2021 18:37:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2021