coreboot-gerrit January 2024

coreboot-gerrit@coreboot.org

1 participants
3044 discussions

[XS] Change in coreboot[main]: mb/google/brox: Remove CNVi Bluetooth
by Nick Vaccaro (Code Review) 26 Jan '24

26 Jan '24

Attention is currently required from: Shelley Chen. Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80211?usp=email ) Change subject: mb/google/brox: Remove CNVi Bluetooth ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80211?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: Ifcfbc04825d4d4e7f2874a4c52f9c5cf3e657856 Gerrit-Change-Number: 80211 Gerrit-PatchSet: 1 Gerrit-Owner: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Comment-Date: Fri, 26 Jan 2024 00:18:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/brox: Remove CNVi Bluetooth
by Shelley Chen (Code Review) 26 Jan '24

26 Jan '24

Shelley Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80211?usp=email ) Change subject: mb/google/brox: Remove CNVi Bluetooth ...................................................................... mb/google/brox: Remove CNVi Bluetooth This is causing an assertion error on the devices that don't have CNVi enabled because CNVi is hidden behind a FW_CONFIG flag in the overridetree now. BUG=b:319188820 BRANCH=None TEST=emerge-brox coreboot chromeos-bootimage make sure we can boot to kernel on device. Change-Id: Ifcfbc04825d4d4e7f2874a4c52f9c5cf3e657856 Signed-off-by: Shelley Chen <shchen(a)google.com> --- M src/mainboard/google/brox/variants/baseboard/brox/devicetree.cb 1 file changed, 0 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/80211/1 diff --git a/src/mainboard/google/brox/variants/baseboard/brox/devicetree.cb b/src/mainboard/google/brox/variants/baseboard/brox/devicetree.cb index fd2c8ed..be31df6 100644 --- a/src/mainboard/google/brox/variants/baseboard/brox/devicetree.cb +++ b/src/mainboard/google/brox/variants/baseboard/brox/devicetree.cb @@ -23,9 +23,6 @@ register "tcc_offset" = "10" # TCC of 90 - # Enable CNVi BT - register "cnvi_bt_core" = "true" - register "usb2_ports[0]" = "USB2_PORT_TYPE_C(OC_SKIP)" # USB2_C0 register "usb2_ports[1]" = "USB2_PORT_EMPTY" # Disable USB2 Port register "usb2_ports[2]" = "USB2_PORT_TYPE_C(OC_SKIP)" # USB2_C2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80211?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: Ifcfbc04825d4d4e7f2874a4c52f9c5cf3e657856 Gerrit-Change-Number: 80211 Gerrit-PatchSet: 1 Gerrit-Owner: Shelley Chen <shchen(a)google.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: mb/google/brya/var/*: Use name 'LCD0' for internal panel output
by Won Chung (Code Review) 26 Jan '24

26 Jan '24

Attention is currently required from: Eric Lai, Kapil Porwal, Matt DeVillier, Nick Vaccaro, Subrata Banik. Won Chung has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80174?usp=email ) Change subject: mb/google/brya/var/*: Use name 'LCD0' for internal panel output ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Patchset: PS2: > @Won Chung, please check. device name of eDP should not affect TCP (DP), so lgtm -- To view, visit https://review.coreboot.org/c/coreboot/+/80174?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: I389553b2ddc5b09d165229e2d8066cacf852b82c Gerrit-Change-Number: 80174 Gerrit-PatchSet: 3 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Won Chung <wonchung(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Thu, 25 Jan 2024 23:12:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Eric Lai <ericllai(a)google.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: vc/amd/opensil/genoa_poc/mpio: don't add duplicate MPIO descriptors
by Felix Held (Code Review) 25 Jan '24

25 Jan '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80149?usp=email ) Change subject: vc/amd/opensil/genoa_poc/mpio: don't add duplicate MPIO descriptors ...................................................................... vc/amd/opensil/genoa_poc/mpio: don't add duplicate MPIO descriptors When the device right below the MPIO chip driver has downstream devices without another chip in between, those downstream devices will also have their chip_ops entry set to vendorcode_amd_opensil_genoa_poc_mpio_ops. To avoid adding the same MPIO descriptor again for those additional downstream devices, make sure that the chip_info pointer of the device isn't the same as the one of the parent device, since that's only the case for those additional downstream devices. TEST=Onyx still boots to the payload and the MPIO configuration reported from the openSIL code is still the same Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Suggested-by: Nico Huber <nico.h(a)gmx.de> Change-Id: I6ba90fdc83ba089127e6722778bfef29dd480bb4 Reviewed-on: https://review.coreboot.org/c/coreboot/+/80149 Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c 1 file changed, 3 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Marshall Dawson: Looks good to me, but someone else must approve diff --git a/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c b/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c index 28e529d..89314ba 100644 --- a/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c +++ b/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c @@ -186,8 +186,9 @@ mpio_global_config(mpio_data); nbio_config(); - /* Find all devices with this chip */ + /* Find all devices with this chip that are directly below the chip */ for (struct device *dev = &dev_root; dev; dev = dev->next) - if (dev->chip_ops == &vendorcode_amd_opensil_genoa_poc_mpio_ops) + if (dev->chip_ops == &vendorcode_amd_opensil_genoa_poc_mpio_ops && + dev->chip_info != dev->bus->dev->chip_info) per_device_config(mpio_data, dev->bus->dev, dev->chip_info); } -- To view, visit https://review.coreboot.org/c/coreboot/+/80149?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: I6ba90fdc83ba089127e6722778bfef29dd480bb4 Gerrit-Change-Number: 80149 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: soc/amd/genoa_poc: rely less on boot state hooks
by Felix Held (Code Review) 25 Jan '24

25 Jan '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/79985?usp=email ) Change subject: soc/amd/genoa_poc: rely less on boot state hooks ...................................................................... soc/amd/genoa_poc: rely less on boot state hooks Call setup_opensil, opensil_entry, and fch_init in the right order from the init method of the SoC's chip operations. This brings this SoC both more in line with the other SoCs and avoids using boot state hooks for this which also makes the sequence in which those functions are called easier to understand. Previously the boot states were used so that setup_opensil was run before configure_mpio which was run before opensil_entry(SIL_TP1), but since configure_mpio is called from setup_opensil, this is no longer necessary. TEST=Onyx still boots to the payload and the MPIO configuration reported from the openSIL code is still the same. The FCH init code now runs before the resource allocation like on the AMD SoCs that rely on FSP. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ic752635da5eaa9e333cfb927836f0d260d2ac049 Reviewed-on: https://review.coreboot.org/c/coreboot/+/79985 Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/amd/genoa_poc/chip.c M src/soc/amd/genoa_poc/fch.c M src/soc/amd/genoa_poc/include/soc/southbridge.h M src/vendorcode/amd/opensil/genoa_poc/opensil.h M src/vendorcode/amd/opensil/genoa_poc/ramstage.c 5 files changed, 16 insertions(+), 15 deletions(-) Approvals: Arthur Heymans: Looks good to me, approved build bot (Jenkins): Verified Marshall Dawson: Looks good to me, but someone else must approve diff --git a/src/soc/amd/genoa_poc/chip.c b/src/soc/amd/genoa_poc/chip.c index eb35a25..5f29428 100644 --- a/src/soc/amd/genoa_poc/chip.c +++ b/src/soc/amd/genoa_poc/chip.c @@ -3,10 +3,17 @@ #include <device/device.h> #include <soc/southbridge.h> #include <soc/acpi.h> +#include <soc/southbridge.h> +#include <vendorcode/amd/opensil/genoa_poc/opensil.h> static void soc_init(void *chip_info) { default_dev_ops_root.write_acpi_tables = soc_acpi_write_tables; + + setup_opensil(); + opensil_entry(SIL_TP1); + + fch_init(chip_info); } static void soc_final(void *chip_info) diff --git a/src/soc/amd/genoa_poc/fch.c b/src/soc/amd/genoa_poc/fch.c index c977957..f340d82 100644 --- a/src/soc/amd/genoa_poc/fch.c +++ b/src/soc/amd/genoa_poc/fch.c @@ -81,14 +81,8 @@ configure_smi(SMITYPE_SMI_CMD_PORT, SMI_MODE_SMI); } -static void fch_init(void *unused) +void fch_init(void *chip_info) { set_pci_irqs(); fch_init_acpi_ports(); } - -/* - * Hook this function into the PCI state machine on entry into BS_DEV_ENABLE. - * TODO: can this be done without using BOOT_STATE_INIT_ENTRY? - */ -BOOT_STATE_INIT_ENTRY(BS_DEV_ENABLE, BS_ON_ENTRY, fch_init, NULL); diff --git a/src/soc/amd/genoa_poc/include/soc/southbridge.h b/src/soc/amd/genoa_poc/include/soc/southbridge.h index a761d53..1027a58 100644 --- a/src/soc/amd/genoa_poc/include/soc/southbridge.h +++ b/src/soc/amd/genoa_poc/include/soc/southbridge.h @@ -117,5 +117,6 @@ void fch_pre_init(void); void fch_early_init(void); +void fch_init(void *chip_info); #endif /* AMD_GENOA_POC_SOUTHBRIDGE_H */ diff --git a/src/vendorcode/amd/opensil/genoa_poc/opensil.h b/src/vendorcode/amd/opensil/genoa_poc/opensil.h index fbf46b3..473238d 100644 --- a/src/vendorcode/amd/opensil/genoa_poc/opensil.h +++ b/src/vendorcode/amd/opensil/genoa_poc/opensil.h @@ -4,6 +4,7 @@ #define _OPENSIL_H_ #include <acpi/acpi.h> +#include <xSIM-api.h> void SIL_STATUS_report(const char *function, const int status); // Add the memory map to dev, starting at index idx, returns last use idx @@ -13,4 +14,7 @@ void configure_mpio(void); +void setup_opensil(void); +void opensil_entry(SIL_TIMEPOINT timepoint); + #endif diff --git a/src/vendorcode/amd/opensil/genoa_poc/ramstage.c b/src/vendorcode/amd/opensil/genoa_poc/ramstage.c index 7d03b32..11289bb 100644 --- a/src/vendorcode/amd/opensil/genoa_poc/ramstage.c +++ b/src/vendorcode/amd/opensil/genoa_poc/ramstage.c @@ -112,7 +112,7 @@ } } -static void setup_opensil(void *unused) +void setup_opensil(void) { const SIL_STATUS debug_ret = SilDebugSetup(HostDebugService); SIL_STATUS_report("SilDebugSetup", debug_ret); @@ -129,9 +129,7 @@ configure_mpio(); } -BOOT_STATE_INIT_ENTRY(BS_DEV_INIT_CHIPS, BS_ON_ENTRY, setup_opensil, NULL); - -static void opensil_entry(void *timepoint) +void opensil_entry(SIL_TIMEPOINT timepoint) { SIL_STATUS ret; SIL_TIMEPOINT tp = (uintptr_t)timepoint; @@ -162,7 +160,4 @@ } } -/* TODO: look into calling these functions from some SoC device operations instead of using - * BOOT_STATE_INIT_ENTRY */ -BOOT_STATE_INIT_ENTRY(BS_DEV_INIT_CHIPS, BS_ON_EXIT, opensil_entry, (void *)SIL_TP1); -/* TODO add other timepoints later. Are they NOOP? */ +/* TODO: also call timepoints 2 and 3 from coreboot. Are they NOOP? */ -- To view, visit https://review.coreboot.org/c/coreboot/+/79985?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: Ic752635da5eaa9e333cfb927836f0d260d2ac049 Gerrit-Change-Number: 79985 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: vc/amd/opensil/genoa_poc: move configure_mpio call to setup_opensil
by Felix Held (Code Review) 25 Jan '24

25 Jan '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80148?usp=email ) Change subject: vc/amd/opensil/genoa_poc: move configure_mpio call to setup_opensil ...................................................................... vc/amd/opensil/genoa_poc: move configure_mpio call to setup_opensil Instead of calling configure_mpio from the init function of the MPIO chip struct for the first device that has this struct as chip_ops, call if from setup_opensil. This will allow to do the calls into openSIL from the SoC's chip_ops init function instead of having to rely on boot state hooks. configure_mpio needs to be called after the xSimAssignMemoryTp1 call which sets up the openSIL data structures, but before the opensil_entry(SIL_TP1) call for which the MPIO data structures need to be filled for it to be able to initialize the hardware accordingly. Since the vendorcode_amd_opensil_genoa_poc_mpio_ops struct now no longer assigns configure_mpio to the init function pointer, we have to check if the device's chip_ops pointer points to vendorcode_amd_opensil_genoa_poc_mpio_ops instead of checking if the chip_ops' init function is configure_mpio to match for the devices below the MPIO chips in the devicetree. TEST=Onyx still boots to the payload and the MPIO configuration reported from the openSIL code is still the same Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: If37077c879e266763fd2748a1a8d71c63c94729b Reviewed-on: https://review.coreboot.org/c/coreboot/+/80148 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c M src/vendorcode/amd/opensil/genoa_poc/opensil.h M src/vendorcode/amd/opensil/genoa_poc/ramstage.c 3 files changed, 10 insertions(+), 7 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved diff --git a/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c b/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c index 49a8934..28e529d 100644 --- a/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c +++ b/src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c @@ -7,6 +7,11 @@ #include <device/device.h> #include <device/pci_def.h> #include "chip.h" +#include "../opensil.h" + +struct chip_operations vendorcode_amd_opensil_genoa_poc_mpio_ops = { + CHIP_NAME("AMD GENOA MPIO") +}; static void nbio_config(void) { @@ -175,7 +180,7 @@ mpio_port++; } -static void configure_mpio(void *const config) +void configure_mpio(void) { MPIOCLASS_INPUT_BLK *mpio_data = SilFindStructure(SilId_MpioClass, 0); mpio_global_config(mpio_data); @@ -183,11 +188,6 @@ /* Find all devices with this chip */ for (struct device *dev = &dev_root; dev; dev = dev->next) - if (dev->chip_ops->init == configure_mpio) + if (dev->chip_ops == &vendorcode_amd_opensil_genoa_poc_mpio_ops) per_device_config(mpio_data, dev->bus->dev, dev->chip_info); } - -struct chip_operations vendorcode_amd_opensil_genoa_poc_mpio_ops = { - CHIP_NAME("AMD GENOA MPIO") - .init = configure_mpio, -}; diff --git a/src/vendorcode/amd/opensil/genoa_poc/opensil.h b/src/vendorcode/amd/opensil/genoa_poc/opensil.h index b9caf73..fbf46b3 100644 --- a/src/vendorcode/amd/opensil/genoa_poc/opensil.h +++ b/src/vendorcode/amd/opensil/genoa_poc/opensil.h @@ -11,4 +11,6 @@ // Fill in FADT from openSIL void opensil_fill_fadt_io_ports(acpi_fadt_t *fadt); +void configure_mpio(void); + #endif diff --git a/src/vendorcode/amd/opensil/genoa_poc/ramstage.c b/src/vendorcode/amd/opensil/genoa_poc/ramstage.c index 7c0bc5b..7d03b32 100644 --- a/src/vendorcode/amd/opensil/genoa_poc/ramstage.c +++ b/src/vendorcode/amd/opensil/genoa_poc/ramstage.c @@ -126,6 +126,7 @@ setup_rc_manager_default(); configure_usb(); configure_sata(); + configure_mpio(); } BOOT_STATE_INIT_ENTRY(BS_DEV_INIT_CHIPS, BS_ON_ENTRY, setup_opensil, NULL); -- To view, visit https://review.coreboot.org/c/coreboot/+/80148?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: If37077c879e266763fd2748a1a8d71c63c94729b Gerrit-Change-Number: 80148 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[main]: vc/amd/opensil/genoa_poc/opensil_console: fix host debug print function
by Felix Held (Code Review) 25 Jan '24

25 Jan '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80203?usp=email ) Change subject: vc/amd/opensil/genoa_poc/opensil_console: fix host debug print function ...................................................................... vc/amd/opensil/genoa_poc/opensil_console: fix host debug print function Since we pass va_list list to the print function, we need to use vprintk instead of printk. Earlier versions of this code used vsnprintf and a local buffer, but when that code was reworked to not need the temporary buffer, it was replaced by printk instead of the correct vprintk. TEST=Now the console output from openSIL looks as expected: Example line from openSIL's console output when it prints the MPIO configuration from a log some commits before this patch: Host PCI Address - -1352681400:-1353251983:7 Same line with this patch applied looks how it's supposed to: Host PCI Address - 0:0:0 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Tested-by: Varshit Pandya <pandyavarshit(a)gmail.com> Change-Id: Ia931cc80dea5b7eabb75cfb19f8baa9a09cd2dbf Reviewed-on: https://review.coreboot.org/c/coreboot/+/80203 Reviewed-by: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-by: Varshit Pandya <pandyavarshit(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/vendorcode/amd/opensil/genoa_poc/opensil_console.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Matt DeVillier: Looks good to me, approved Arthur Heymans: Looks good to me, approved Varshit Pandya: Looks good to me, approved diff --git a/src/vendorcode/amd/opensil/genoa_poc/opensil_console.c b/src/vendorcode/amd/opensil/genoa_poc/opensil_console.c index 55a3521..35e5eeb 100644 --- a/src/vendorcode/amd/opensil/genoa_poc/opensil_console.c +++ b/src/vendorcode/amd/opensil/genoa_poc/opensil_console.c @@ -39,6 +39,6 @@ /* print formatted message */ va_list args; va_start(args, Line); - printk(loglevel, Message, args); + vprintk(loglevel, Message, args); va_end(args); } -- To view, visit https://review.coreboot.org/c/coreboot/+/80203?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: Ia931cc80dea5b7eabb75cfb19f8baa9a09cd2dbf Gerrit-Change-Number: 80203 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: vc/amd: move verstage on PSP files to new psp_verstage folder
by Matt DeVillier (Code Review) 25 Jan '24

25 Jan '24

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80209?usp=email ) Change subject: vc/amd: move verstage on PSP files to new psp_verstage folder ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80209?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: Ic47f8b18bc515600add7838f4c7afcb4fff7c004 Gerrit-Change-Number: 80209 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 25 Jan 2024 22:12:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: soc/amd: factor out common acpi_add_ivrs_table implementation
by Matt DeVillier (Code Review) 25 Jan '24

25 Jan '24

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Martin L Roth, Varshit Pandya. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80208?usp=email ) Change subject: soc/amd: factor out common acpi_add_ivrs_table implementation ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80208?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: Idb65c398b747e70ec67107e0a1d4bd6551501347 Gerrit-Change-Number: 80208 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 25 Jan 2024 22:11:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: Add MTCL function to ACPI SSDT tables
by David Ruth (Code Review) 25 Jan '24

25 Jan '24

Attention is currently required from: Cliff Huang, Lance Zhao, Nico Huber, Paul Menzel, Subrata Banik, Subrata Banik, Tim Wawrzynczak. David Ruth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80170?usp=email ) Change subject: Add MTCL function to ACPI SSDT tables ...................................................................... Patch Set 7: (4 comments) Patchset: PS6: > Sorry, is there a misunderstanding? I meant the coreboot firmware (not the firmware […] My answer applied to the data that is being stored in coreboot. I think there must be some misunderstanding. I've tried to rephrase what I meant with more detail. Perhaps I don't understand what aspects of your general question are important to answer. This information is being put here because it should be available to the kernel immediately at system boot. It is specific to WiFi chips made by MediaTek that support 6GHz band operation, but could be used theoretically with any CPU (although I have only tested this with Intel platforms). This is intended only to be enabled on certain Chromebook boards, but there's no reason why it couldn't be used by others. File src/drivers/wifi/generic/acpi.c: https://review.coreboot.org/c/coreboot/+/80170/comment/fdc4d78d_2a13ae7b : PS6, Line 592: uint8_t mtcl_package[sizeof(struct wifi_mtcl)]; > Ok, I think I understand better now why you want this part to be more generic. […] Addressing the organization of the code (and updated the commit message to hopefully make what I'm trying to do clearer): The addition to acpigen generates the ACPI package. I observed similar functions in that file, but if vendor-specific methods and extensions don't belong there, I can move it. If it belongs elsewhere, I could refactor it out, but I think based on the discussion related to chromeos, it would belong best in the src/drivers/wifi tree somewhere, perhaps in mtcl.c. The additions to drivers/wifi are because the goal is to add a function to the SSDT for WiFi chipsets for which it is pertinent. The SSDT for WiFi devices is generated there, so that seems like the correct place to call out to this functionality for certain. The functionality put in chromeos was done so because all other WiFi functionality related to SSDT table generation used in chromeos is there, and I was modeling the structure after existing code. Please let me know if you have strong preferences about reorganization, I'm relatively new to the product, and was attempting to follow the same patterns observed in similar changes that were made previously to implement WiFi SSDT functionality. File src/vendorcode/google/chromeos/mtcl.c: PS6: > > The functionality is being added specifically for Chromebooks and ChromeOS. […] Moved to drivers/wifi/generic/. https://review.coreboot.org/c/coreboot/+/80170/comment/348e4aa0_cd49cf02 : PS6, Line 59: if (mtcl_bin_len != sizeof(struct wifi_mtcl)) { > Do we expect any other callers, any other implementations? I don't expect any other callers (this function must be in the WiFi device's SSDT adn this is only generated in src/drivers/wifi/generic/acpi.c as far as I could tell). What I expect are new revisions to change the structure of the data internally. It'll still be opaque bytes in and out, but I want to allow for structural changes to be abstracted away from the callers, and keep them localized to this file. I also don't see any benefit to passing the structured data to callers, because I expect them to just write bytes through acpigen, and writing a byte array is more straightforward than writing structured data. Is there a benefit to passing the structured data around? I don't see one, but if I'm missing it, it'd help me understand why it should be in the signature. -- To view, visit https://review.coreboot.org/c/coreboot/+/80170?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: I9b5e7312a44e114270e664b983626faa6cfee350 Gerrit-Change-Number: 80170 Gerrit-PatchSet: 7 Gerrit-Owner: David Ruth <druth(a)chromium.org> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.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-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Comment-Date: Thu, 25 Jan 2024 22:06:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: David Ruth <druth(a)chromium.org> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2024