coreboot-gerrit March 2022

coreboot-gerrit@coreboot.org

1 participants
6700 discussions

Change in coreboot[master]: libpayload/bin/lpgcc: Make lpgcc provide TPM configuration for vboot
by Julius Werner (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Jakub Czapiga, Yu-Ping Wu, Joey Madafferi. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62516 ) Change subject: libpayload/bin/lpgcc: Make lpgcc provide TPM configuration for vboot ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/62516 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I567a9f04d7089699840dc7e0a063cf3030fb934b Gerrit-Change-Number: 62516 Gerrit-PatchSet: 3 Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Joey Madafferi <joeymadafferi(a)gmail.com> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Joey Madafferi <joeymadafferi(a)gmail.com> Gerrit-Comment-Date: Thu, 03 Mar 2022 22:38:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/system76/dgpu: Add driver for System76 switchable graphics
by Jeremy Soller (Code Review) 03 Mar '22

03 Mar '22

Jeremy Soller has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43615 ) Change subject: drivers/system76/dgpu: Add driver for System76 switchable graphics ...................................................................... drivers/system76/dgpu: Add driver for System76 switchable graphics This adds a driver for System76 systems with switchable NVIDIA graphics. The driver provides ACPI support for dynamically powering on and off the GPU, a function for enabling the GPU power in the bootblock, and a hook in ramstage for moving GPU prefetch resources above 4 GiB. Tested on system76/addw2, system76/gaze15, and system76/oryp6, which will be submitted in future patches. Signed-off-by: Jeremy Soller <jeremy(a)system76.com> Change-Id: I05833e88457bfc3612cc9a8cbf1eec68dcfc5566 --- A src/drivers/system76/dgpu/Kconfig A src/drivers/system76/dgpu/Makefile.inc A src/drivers/system76/dgpu/acpi/dgpu.asl A src/drivers/system76/dgpu/bootblock.c A src/drivers/system76/dgpu/ramstage.c 5 files changed, 313 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/43615/1 diff --git a/src/drivers/system76/dgpu/Kconfig b/src/drivers/system76/dgpu/Kconfig new file mode 100644 index 0000000..5096671 --- /dev/null +++ b/src/drivers/system76/dgpu/Kconfig @@ -0,0 +1,5 @@ +config DRIVERS_SYSTEM76_DGPU + bool + default n + help + System76 switchable graphics support diff --git a/src/drivers/system76/dgpu/Makefile.inc b/src/drivers/system76/dgpu/Makefile.inc new file mode 100644 index 0000000..a230f97 --- /dev/null +++ b/src/drivers/system76/dgpu/Makefile.inc @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0-only + +ramstage-$(CONFIG_DRIVERS_SYSTEM76_DGPU) += ramstage.c diff --git a/src/drivers/system76/dgpu/acpi/dgpu.asl b/src/drivers/system76/dgpu/acpi/dgpu.asl new file mode 100644 index 0000000..a721934 --- /dev/null +++ b/src/drivers/system76/dgpu/acpi/dgpu.asl @@ -0,0 +1,201 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Device (\_SB.PCI0.PEGP) { + Name (_ADR, 0x00010000) + + PowerResource (PWRR, 0, 0) { + Name (_STA, 1) + + Method (_ON) { + Debug = "PEGP.PWRR._ON" + If (_STA != 1) { + \_SB.PCI0.PEGP.DEV0._ON () + _STA = 1 + } + } + + Method (_OFF) { + Debug = "PEGP.PWRR._OFF" + If (_STA != 0) { + \_SB.PCI0.PEGP.DEV0._OFF () + _STA = 0 + } + } + } + + Name (_PR0, Package () { \_SB.PCI0.PEGP.PWRR }) + Name (_PR2, Package () { \_SB.PCI0.PEGP.PWRR }) + Name (_PR3, Package () { \_SB.PCI0.PEGP.PWRR }) +} + +Device (\_SB.PCI0.PEGP.DEV0) { + Name(_ADR, 0x00000000) + Name (_STA, 0xF) + Name (LTRE, 0) + + // Memory mapped PCI express registers + // Not sure what this stuff is, but it is used to get into GC6 + OperationRegion (RPCX, SystemMemory, 0xE0008000, 0x1000) + Field (RPCX, ByteAcc, NoLock, Preserve) { + PVID, 16, + PDID, 16, + CMDR, 8, + Offset (0x19), + PRBN, 8, + Offset (0x84), + D0ST, 2, + Offset (0xAA), + CEDR, 1, + Offset (0xAC), + , 4, + CMLW, 6, + Offset (0xB0), + ASPM, 2, + , 2, + P0LD, 1, + RTLK, 1, + Offset (0xC9), + , 2, + LREN, 1, + Offset (0x11A), + , 1, + VCNP, 1, + Offset (0x214), + Offset (0x216), + P0LS, 4, + Offset (0x248), + , 7, + Q0L2, 1, + Q0L0, 1, + Offset (0x504), + Offset (0x506), + PCFG, 2, + Offset (0x508), + TREN, 1, + Offset (0xC20), + , 4, + P0AP, 2, + Offset (0xC38), + , 3, + P0RM, 1, + Offset (0xC74), + P0LT, 4, + Offset (0xD0C), + , 20, + LREV, 1 + } + + Method (_ON) { + Debug = "PEGP.DEV0._ON" + + If (_STA != 0xF) { + Debug = " If DGPU_PWR_EN low" + If (! GTXS (DGPU_PWR_EN)) { + Debug = " DGPU_PWR_EN high" + STXS (DGPU_PWR_EN) + + Debug = " Sleep 16" + Sleep (16) + } + + Debug = " DGPU_RST_N high" + STXS(DGPU_RST_N) + + Debug = " Sleep 10" + Sleep (10) + + Debug = " Q0L0 = 1" + Q0L0 = 1 + + Debug = " Sleep 16" + Sleep (16) + + Debug = " While Q0L0" + Local0 = 0 + While (Q0L0) { + If ((Local0 > 4)) { + Debug = " While Q0L0 timeout" + Break + } + + Sleep (16) + Local0++ + } + + Debug = " P0RM = 0" + P0RM = 0 + + Debug = " P0AP = 0" + P0AP = 0 + + Debug = Concatenate(" LREN = ", ToHexString(LTRE)) + LREN = LTRE + + Debug = " CEDR = 1" + CEDR = 1 + + Debug = " CMDR |= 7" + CMDR |= 7 + + Debug = " _STA = 0xF" + _STA = 0xF + } + } + + Method (_OFF) { + Debug = "PEGP.DEV0._OFF" + + If (_STA != 0x5) { + Debug = Concatenate(" LTRE = ", ToHexString(LREN)) + LTRE = LREN + + Debug = " Q0L2 = 1" + Q0L2 = 1 + + Debug = " Sleep 16" + Sleep (16) + + Debug = " While Q0L2" + Local0 = Zero + While (Q0L2) { + If ((Local0 > 4)) { + Debug = " While Q0L2 timeout" + Break + } + + Sleep (16) + Local0++ + } + + Debug = " P0RM = 1" + P0RM = 1 + + Debug = " P0AP = 3" + P0AP = 3 + + Debug = " Sleep 10" + Sleep (10) + + Debug = " DGPU_RST_N low" + CTXS(DGPU_RST_N) + + Debug = " While DGPU_GC6 low" + Local0 = Zero + While (! GRXS(DGPU_GC6)) { + If ((Local0 > 4)) { + Debug = " While DGPU_GC6 low timeout" + + Debug = " DGPU_PWR_EN low" + CTXS (DGPU_PWR_EN) + Break + } + + Sleep (16) + Local0++ + } + + Debug = " _STA = 0x5" + _STA = 0x5 + } + } +} diff --git a/src/drivers/system76/dgpu/bootblock.c b/src/drivers/system76/dgpu/bootblock.c new file mode 100644 index 0000000..61f7fbf --- /dev/null +++ b/src/drivers/system76/dgpu/bootblock.c @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +//TODO: do not require this to be included in mainboard bootblock.c + +#include <console/console.h> +#include <delay.h> +#include <gpio.h> + +static void dgpu_power_enable(int onoff) { + printk(BIOS_DEBUG, "system76: DGPU power %d\n", onoff); + if (onoff) { + gpio_set(DGPU_RST_N, 0); + mdelay(4); + gpio_set(DGPU_PWR_EN, 1); + mdelay(4); + gpio_set(DGPU_RST_N, 1); + } else { + gpio_set(DGPU_RST_N, 0); + mdelay(4); + gpio_set(DGPU_PWR_EN, 0); + } + mdelay(50); +} diff --git a/src/drivers/system76/dgpu/ramstage.c b/src/drivers/system76/dgpu/ramstage.c new file mode 100644 index 0000000..eeb1607 --- /dev/null +++ b/src/drivers/system76/dgpu/ramstage.c @@ -0,0 +1,81 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <bootstate.h> +#include <console/console.h> +#include <device/pci.h> + +static void dgpu_read_resources(struct device *dev) { + printk(BIOS_INFO, "system76: dgpu_read_resources %s\n", dev_path(dev)); + + pci_dev_read_resources(dev); + + int bar; + // Find all BARs on DGPU, mark them above 4g if prefetchable + for (bar = PCI_BASE_ADDRESS_0; bar <= PCI_BASE_ADDRESS_5; bar += 4) { + printk(BIOS_INFO, " BAR at 0x%02x\n", bar); + + struct resource *res; + res = probe_resource(dev, bar); + if (res) { + if (res->flags & IORESOURCE_PREFETCH) { + printk(BIOS_INFO, " marked above 4g\n"); + res->flags |= IORESOURCE_ABOVE_4G; + } else { + printk(BIOS_INFO, " not prefetch\n"); + } + } else { + printk(BIOS_INFO, " not found\n"); + } + } +} + +static void dgpu_enable_resources(struct device *dev) { + printk(BIOS_INFO, "system76: dgpu_enable_resources %s\n", dev_path(dev)); + + dev->subsystem_vendor = CONFIG_SUBSYSTEM_VENDOR_ID; + dev->subsystem_device = CONFIG_SUBSYSTEM_DEVICE_ID; + printk(BIOS_INFO, " subsystem <- %04x/%04x\n", dev->subsystem_vendor, dev->subsystem_device); + pci_write_config32(dev, 0x40, ((dev->subsystem_device & 0xffff) << 16) | (dev->subsystem_vendor & 0xffff)); + + pci_dev_enable_resources(dev); +} + +static struct device_operations dgpu_pci_ops_dev = { + .read_resources = dgpu_read_resources, + .set_resources = pci_dev_set_resources, + .enable_resources = dgpu_enable_resources, +#if CONFIG(HAVE_ACPI_TABLES) + .write_acpi_tables = pci_rom_write_acpi_tables, + .acpi_fill_ssdt = pci_rom_ssdt, +#endif + .init = pci_dev_init, + .ops_pci = &pci_dev_ops_pci, +}; + +static void dgpu_above_4g(void *unused) { + struct device *pdev; + + // Find PEG0 + pdev = pcidev_on_root(1, 0); + if (!pdev) { + printk(BIOS_ERR, "system76: failed to find PEG0\n"); + return; + } + printk(BIOS_INFO, "system76: PEG0 at %p, %04x:%04x\n", pdev, pdev->vendor, pdev->device); + + int fn; + for (fn = 0; fn < 8; fn++) { + struct device *dev; + + // Find DGPU functions + dev = pcidev_path_behind(pdev->link_list, PCI_DEVFN(0, fn)); + if (dev) { + printk(BIOS_INFO, "system76: DGPU fn %d at %p, %04x:%04x\n", fn, dev, dev->vendor, dev->device); + dev->ops = &dgpu_pci_ops_dev; + } else { + printk(BIOS_ERR, "system76: failed to find DGPU fn %d\n", fn); + } + } +} + +BOOT_STATE_INIT_ENTRY(BS_DEV_RESOURCES, BS_ON_ENTRY, dgpu_above_4g, NULL); -- To view, visit https://review.coreboot.org/c/coreboot/+/43615 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I05833e88457bfc3612cc9a8cbf1eec68dcfc5566 Gerrit-Change-Number: 43615 Gerrit-PatchSet: 1 Gerrit-Owner: Jeremy Soller <jeremy(a)system76.com> Gerrit-MessageType: newchange

6 19

Change in coreboot[master]: mb/google/brya: Move ACPI MPTS method from DSDT to SSDT for Brya and ...
by Cliff Huang (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Bora Guvendik, Selma Bensaid, Paul Menzel. Cliff Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61887 ) Change subject: mb/google/brya: Move ACPI MPTS method from DSDT to SSDT for Brya and Redrix ...................................................................... Patch Set 13: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/61887/comment/c28051b1_36013942 PS12, Line 24: MPST > `MPTS` Done -- To view, visit https://review.coreboot.org/c/coreboot/+/61887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0f0b7638e90a7862173fca99305398bb250373e9 Gerrit-Change-Number: 61887 Gerrit-PatchSet: 13 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Cliff Huang <cliff.huang(a)intel.corp-partner.google.com> Gerrit-CC: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Thu, 03 Mar 2022 22:34:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: Move ACPI MPTS method from DSDT to SSDT for Brya and ...
by Cliff Huang (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Bora Guvendik, Cliff Huang, Selma Bensaid, Paul Menzel. Hello Bora Guvendik, build bot (Jenkins), Selma Bensaid, Tim Wawrzynczak, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/61887 to look at the new patch set (#13). Change subject: mb/google/brya: Move ACPI MPTS method from DSDT to SSDT for Brya and Redrix ...................................................................... mb/google/brya: Move ACPI MPTS method from DSDT to SSDT for Brya and Redrix This change is to move MPTS (Mainboard Prepare To Sleep) method from wwan_power.asl to SSDT. MPTS is mainboard-specific method, while wwan_power.asl is meant for WWAN from its name. Having fixed MPTS method (i.e. DSDT) can not cover the case where device only presents and certain CBI bit(s) is(are) set. In Redrix and Brya, there are SKUs with or without 5G, 4G device. For those with 4G, MPTS method should be different. For those with no WWAN device, no MPTS is needed. Having MPTS generating in SSDT also eliminates the need for introducing Kconfig flags to support different devices in the future. MPTS method is created inside mainboard_fill_ssdt function in which the corresponding variant function is called. This will generate the following for the mainboard: Scope (\_SB) { Method (MPTS, 1, Serialized) { Local0 = \_SB.PCI0.RP01.RTD3._STA () If ((Local0 == One)) { \_SB.PCI0.RP01.PXSX.DPTS (Arg0) } } } Test: Check the SSDT for MPTS method under \_SB after boot to OS Use shutdown command and check the GPIO pins from logical analyzer Signed-off-by: Cliff Huang <cliff.huang(a)intel.com> Change-Id: I0f0b7638e90a7862173fca99305398bb250373e9 --- M src/mainboard/google/brya/Kconfig.name M src/mainboard/google/brya/mainboard.c M src/mainboard/google/brya/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/brya/variants/brya0/overridetree.cb M src/mainboard/google/brya/variants/brya4es/overridetree.cb M src/mainboard/google/brya/variants/redrix/overridetree.cb M src/mainboard/google/brya/variants/redrix4es/overridetree.cb 7 files changed, 66 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/61887/13 -- To view, visit https://review.coreboot.org/c/coreboot/+/61887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0f0b7638e90a7862173fca99305398bb250373e9 Gerrit-Change-Number: 61887 Gerrit-PatchSet: 13 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Cliff Huang <cliff.huang(a)intel.corp-partner.google.com> Gerrit-CC: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: coreboot_tables.c: Expose the ACPI RSDP
by Tim Wawrzynczak (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Arthur Heymans, Lance Zhao, Nico Huber. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62573 ) Change subject: coreboot_tables.c: Expose the ACPI RSDP ...................................................................... Patch Set 2: (1 comment) File src/commonlib/include/commonlib/coreboot_tables.h: https://review.coreboot.org/c/coreboot/+/62573/comment/7229f0b2_bd4fc002 PS1, Line 586: uint64_t > (in C pointers are aligned, right?) Says who? x86 supports misaligned everything by design! The compilers just keep trying to shave off every possible bit everywhere making every assumption they possibly can.... that is so frustrating... > But I think that deserves a broader discussion. Agreed, maybe that would be an interesting topic for upcoming GSoC... -- To view, visit https://review.coreboot.org/c/coreboot/+/62573 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I591312a2c48f0cbbb03b2787e4b365e9c932afff Gerrit-Change-Number: 62573 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Lance Zhao Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 03 Mar 2022 22:30:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: libpayload: Parse the ACPI RSDP table entry
by Nico Huber (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Arthur Heymans. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62576 ) Change subject: libpayload: Parse the ACPI RSDP table entry ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File payloads/libpayload/include/coreboot_tables.h: https://review.coreboot.org/c/coreboot/+/62576/comment/c544dd8f_c8875721 PS3, Line 432: uint64_t `struct cbuint64`? -- To view, visit https://review.coreboot.org/c/coreboot/+/62576 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I583cda63c3f0b58f8d198ed5ecea7c4619c7a897 Gerrit-Change-Number: 62576 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 03 Mar 2022 22:26:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cbfstool/linux_trampoline: Fill the ACPI RSDP entry
by Nico Huber (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Arthur Heymans, ron minnich. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62574 ) Change subject: cbfstool/linux_trampoline: Fill the ACPI RSDP entry ...................................................................... Patch Set 4: Code-Review+1 (1 comment) File util/cbfstool/linux_trampoline.S: https://review.coreboot.org/c/coreboot/+/62574/comment/5d26f2fc_a57a8257 PS4, Line 100: je .endScan Why not a simple `jmp`? We know it's right on this path, isn't it? (and I don't know if `mov` can have an effect on flags ._.) -- To view, visit https://review.coreboot.org/c/coreboot/+/62574 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I688b94608b03b0177c42d2834c7e3beb802ae686 Gerrit-Change-Number: 62574 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Thu, 03 Mar 2022 22:24:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/spd_tools: Encode SDRAM min cycle time (TCKMinPs)
by Reka Norman (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Tim Wawrzynczak, Nick Vaccaro, Karthik Ramasubramanian. Reka Norman has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62387 ) Change subject: util/spd_tools: Encode SDRAM min cycle time (TCKMinPs) ...................................................................... Patch Set 2: Code-Review+1 (3 comments) Patchset: PS2: (+1 only because I don't have +2 permissions yet) File util/spd_tools/src/spd_gen/lp5.go: https://review.coreboot.org/c/coreboot/+/62387/comment/4eedc969_0c02dab0 PS1, Line 587: if ok == false || tCKMinPs == 0 { : return LP5GetDefaultTCKMinPs(memAttribs) : } > I defined the default speed to TCKMinPs mapping inside the earlier table. […] Thanks! This is nicer than what I suggested :) File util/spd_tools/src/spd_gen/lp5.go: https://review.coreboot.org/c/coreboot/+/62387/comment/df9f7606_6f4095e0 PS2, Line 573: Println nit: Println() doesn't support format specifiers. Can you change it to Printf() and add a \n -- To view, visit https://review.coreboot.org/c/coreboot/+/62387 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I99ada7ead3a75befb0f934af871eecc060adcb26 Gerrit-Change-Number: 62387 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Karthikeyan Ramasubramanian <kramasub(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 03 Mar 2022 22:23:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Reka Norman <rekanorman(a)chromium.org> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: coreboot_tables.c: Expose the ACPI RSDP
by Nico Huber (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Arthur Heymans, Lance Zhao, Tim Wawrzynczak. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62573 ) Change subject: coreboot_tables.c: Expose the ACPI RSDP ...................................................................... Patch Set 2: Code-Review+1 (1 comment) File src/commonlib/include/commonlib/coreboot_tables.h: https://review.coreboot.org/c/coreboot/+/62573/comment/8d051498_471addd5 PS1, Line 586: uint64_t > Could start now.... […] Endianess is not the only concern. We had a bug recently that looked (I didn't look close) like GCC generated instructions that didn't care about the LBSs in pointers (in C pointers are aligned, right?). So best option we have are the read/write functions in `commonlib/endian.h`, IMHO. But I think that deserves a broader discussion. -- To view, visit https://review.coreboot.org/c/coreboot/+/62573 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I591312a2c48f0cbbb03b2787e4b365e9c932afff Gerrit-Change-Number: 62573 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Lance Zhao Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 03 Mar 2022 22:21:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cbfstool/linux_trampoline: Fill the ACPI RSDP entry
by build bot (Jenkins) (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Arthur Heymans, Nico Huber, ron minnich. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62574 ) Change subject: cbfstool/linux_trampoline: Fill the ACPI RSDP entry ...................................................................... Patch Set 4: (15 comments) File util/cbfstool/linux_trampoline.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/c4d21478_03568cdc PS4, Line 6: 0xcb, 0x0f, 0x84, 0xc5, 0x00, 0x00, 0x00, 0x8b, 0x59, 0x04, 0x01, 0xcb, 0x8b, 0x49, 0x14, 0x83, line over 96 characters Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/e06d9d34_a2e45451 PS4, Line 6: 0xcb, 0x0f, 0x84, 0xc5, 0x00, 0x00, 0x00, 0x8b, 0x59, 0x04, 0x01, 0xcb, 0x8b, 0x49, 0x14, 0x83, please, no spaces at the start of a line Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/6909c229_ecade045 PS4, Line 11: 0xe8, 0x01, 0x09, 0x00, 0xbf, 0xd0, 0x02, 0x09, 0x00, 0x83, 0xf8, 0x00, 0x74, 0x2f, 0x83, 0x7f, line over 96 characters Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/7397f78d_290b77cc PS4, Line 11: 0xe8, 0x01, 0x09, 0x00, 0xbf, 0xd0, 0x02, 0x09, 0x00, 0x83, 0xf8, 0x00, 0x74, 0x2f, 0x83, 0x7f, please, no spaces at the start of a line Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/84044074_8a6675a6 PS4, Line 13: 0xe8, 0x83, 0x3b, 0x43, 0x75, 0x12, 0x8b, 0x43, 0x08, 0xa3, 0x70, 0x00, 0x09, 0x00, 0x8b, 0x43, line over 96 characters Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/1090d73a_943f96ca PS4, Line 13: 0xe8, 0x83, 0x3b, 0x43, 0x75, 0x12, 0x8b, 0x43, 0x08, 0xa3, 0x70, 0x00, 0x09, 0x00, 0x8b, 0x43, please, no spaces at the start of a line Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/dfbc6b58_a5daf403 PS4, Line 14: 0x0c, 0xa3, 0x74, 0x00, 0x09, 0x00, 0x74, 0x05, 0x83, 0x3b, 0x12, 0x75, 0x00, 0x03, 0x5b, 0x04, line over 96 characters Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/386b01bf_e6605680 PS4, Line 14: 0x0c, 0xa3, 0x74, 0x00, 0x09, 0x00, 0x74, 0x05, 0x83, 0x3b, 0x12, 0x75, 0x00, 0x03, 0x5b, 0x04, please, no spaces at the start of a line Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/1db7bcb4_25fe1315 PS4, Line 15: 0x49, 0x0f, 0x85, 0x78, 0xff, 0xff, 0xff, 0xb8, 0x00, 0x00, 0x04, 0x00, 0xc7, 0x40, 0x10, 0xff, line over 96 characters Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/cae46dbe_f0b8f395 PS4, Line 15: 0x49, 0x0f, 0x85, 0x78, 0xff, 0xff, 0xff, 0xb8, 0x00, 0x00, 0x04, 0x00, 0xc7, 0x40, 0x10, 0xff, please, no spaces at the start of a line Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/642bf848_4806048a PS4, Line 16: 0xff, 0x00, 0x00, 0xc7, 0x40, 0x14, 0x00, 0x9b, 0xcf, 0x00, 0xc7, 0x40, 0x18, 0xff, 0xff, 0x00, line over 96 characters Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/4ef125f4_c3207c8e PS4, Line 16: 0xff, 0x00, 0x00, 0xc7, 0x40, 0x14, 0x00, 0x9b, 0xcf, 0x00, 0xc7, 0x40, 0x18, 0xff, 0xff, 0x00, please, no spaces at the start of a line Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/2bb52ebb_b8c9f873 PS4, Line 17: 0x00, 0xc7, 0x40, 0x1c, 0x00, 0x93, 0xcf, 0x00, 0xc6, 0x00, 0x2b, 0x89, 0x40, 0x02, 0x0f, 0x01, line over 96 characters Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/2b420f64_6304325a PS4, Line 17: 0x00, 0xc7, 0x40, 0x1c, 0x00, 0x93, 0xcf, 0x00, 0xc6, 0x00, 0x2b, 0x89, 0x40, 0x02, 0x0f, 0x01, please, no spaces at the start of a line Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-143065): https://review.coreboot.org/c/coreboot/+/62574/comment/3ccf7061_35630588 PS4, Line 18: 0x10, 0xbe, 0x00, 0x00, 0x09, 0x00, 0xff, 0x25, 0x14, 0x02, 0x09, 0x00, 0xf4, 0xeb, 0xfd please, no spaces at the start of a line -- To view, visit https://review.coreboot.org/c/coreboot/+/62574 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I688b94608b03b0177c42d2834c7e3beb802ae686 Gerrit-Change-Number: 62574 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Thu, 03 Mar 2022 22:17:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2022