coreboot-gerrit November 2022

coreboot-gerrit@coreboot.org

1 participants
7286 discussions

[S] Change in coreboot[master]: drivers/intel/fsp2_0/memory_init.c: clean code
by Lean Sheng Tan (Code Review) 05 Nov '22

05 Nov '22

Attention is currently required from: Maximilian Brune, Andrey Petrov. Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68592 ) Change subject: drivers/intel/fsp2_0/memory_init.c: clean code ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/68592/comment/0fd39a45_cab3b80c PS2, Line 8: commit message missing, can you be more specific? -- To view, visit https://review.coreboot.org/c/coreboot/+/68592 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d57c45ede520160ef615725c023b7e92289a995 Gerrit-Change-Number: 68592 Gerrit-PatchSet: 2 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Sat, 05 Nov 2022 03:59:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/block/power_limit: Avoid MSR read if it is not needed
by Felix Held (Code Review) 05 Nov '22

05 Nov '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69185 ) Change subject: soc/intel/block/power_limit: Avoid MSR read if it is not needed ...................................................................... soc/intel/block/power_limit: Avoid MSR read if it is not needed In function 'set_power_limits' there is a path to bail out early if the Kconfig switch SOC_INTEL_DISABLE_POWER_LIMITS is selected. In this case reading the MSR PLATFORM_INFO is useless and can be avoided. So read it right before the value is needed. This was found by the scanbuild. In addition, fix an unnecessary line break to increase code readability. Change-Id: Ibdededdfd56287fb9b9223e78033a3cd6425e1a2 Signed-off-by: Werner Zeh <werner.zeh(a)siemens.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/69185 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Marc Jones <marc(a)marcjonesconsulting.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> --- M src/soc/intel/common/block/power_limit/power_limit.c 1 file changed, 27 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Marc Jones: Looks good to me, approved Angel Pons: Looks good to me, approved Eric Lai: Looks good to me, approved diff --git a/src/soc/intel/common/block/power_limit/power_limit.c b/src/soc/intel/common/block/power_limit/power_limit.c index dce174b..d7c9077 100644 --- a/src/soc/intel/common/block/power_limit/power_limit.c +++ b/src/soc/intel/common/block/power_limit/power_limit.c @@ -72,7 +72,7 @@ void set_power_limits(u8 power_limit_1_time, struct soc_power_limits_config *conf) { - msr_t msr = rdmsr(MSR_PLATFORM_INFO); + msr_t msr; msr_t limit; unsigned int power_unit; unsigned int tdp, min_power, max_power, max_time, tdp_pl2, tdp_pl1; @@ -96,9 +96,9 @@ } if (power_limit_1_time >= ARRAY_SIZE(power_limit_time_sec_to_msr)) - power_limit_1_time = - ARRAY_SIZE(power_limit_time_sec_to_msr) - 1; + power_limit_1_time = ARRAY_SIZE(power_limit_time_sec_to_msr) - 1; + msr = rdmsr(MSR_PLATFORM_INFO); if (!(msr.lo & PLATFORM_INFO_SET_TDP)) return; -- To view, visit https://review.coreboot.org/c/coreboot/+/69185 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibdededdfd56287fb9b9223e78033a3cd6425e1a2 Gerrit-Change-Number: 69185 Gerrit-PatchSet: 3 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: cpu/mp_init.c: Only enable CPUs once they execute code
by Felix Held (Code Review) 05 Nov '22

05 Nov '22

Attention is currently required from: Arthur Heymans. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68892 ) Change subject: cpu/mp_init.c: Only enable CPUs once they execute code ...................................................................... Patch Set 8: (1 comment) Patchset: PS8: > when add_cpu_device is called with the enabled parameter = 0, it'll call find_dev_path instead of al […] the original implementation of the functionality of add_cpu_device was added in https://review.coreboot.org/c/coreboot/+/1186/ and factored out and commonized as add_cpu_device in https://review.coreboot.org/c/coreboot/+/1418 and it looks to me that the enabled there has the meaning the a core isn't fused off, so disabled means here that a core not present or usable -- To view, visit https://review.coreboot.org/c/coreboot/+/68892 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6c8042b4d6127239175924f996f735bf9c83c6e8 Gerrit-Change-Number: 68892 Gerrit-PatchSet: 8 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Sat, 05 Nov 2022 02:36:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: cpu/x86/mp_init.c: Use existing code to create cpu struct device
by Felix Held (Code Review) 05 Nov '22

05 Nov '22

Attention is currently required from: Arthur Heymans, Kyösti Mälkki. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64341 ) Change subject: cpu/x86/mp_init.c: Use existing code to create cpu struct device ...................................................................... Patch Set 9: Code-Review+2 (1 comment) Patchset: PS9: verified that this works on amd/mandolin out of scope for this patch, but when looking at the patch i wondered if changing struct device *new = add_cpu_device(cpu_bus, info->cpu->path.apic.apic_id + i, 1); to struct device *new = add_cpu_device(cpu_bus, i, 1); and changing struct device *bsp = add_cpu_device(cpu_bus, lapicid(), 1); to struct device *bsp = add_cpu_device(cpu_bus, 0, 1); would result in the same behavior with the code being a bit clearer. verified that this works on mandolin, but haven't 100% traced the code, so i'm not certain if that would be correct in all cases -- To view, visit https://review.coreboot.org/c/coreboot/+/64341 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I80baadd405b31d6be2fdbb894b0f4b7c775da6f8 Gerrit-Change-Number: 64341 Gerrit-PatchSet: 9 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Sat, 05 Nov 2022 02:20:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: drivers/i2c/generic: Print error when using _CRS and PowerResource
by Felix Held (Code Review) 05 Nov '22

05 Nov '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/60174 ) Change subject: drivers/i2c/generic: Print error when using _CRS and PowerResource ...................................................................... drivers/i2c/generic: Print error when using _CRS and PowerResource Exposing the GPIOs via an ACPI PowerResource and the _CRS results in the OS driver and ACPI thinking they own the GPIO. This can cause timing problems because it's not clear which system should be controlling the GPIO. I'm making this an error because we should really clean these up. BUG=b:210694108 TEST=Boot guybrush and see error: > I2C: 02:5d: ERROR: Exposing GPIOs in Power Resource and _CRS > \_SB.I2C1.H05D: Goodix Touchscreen at I2C: 02:5d Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: Ifcc42ed81fff295fb168a0b343e96b3a650b1c84 Reviewed-on: https://review.coreboot.org/c/coreboot/+/60174 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Tim Van Patten <timvp(a)google.com> --- M src/drivers/i2c/generic/generic.c 1 file changed, 33 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, but someone else must approve Tim Van Patten: Looks good to me, approved diff --git a/src/drivers/i2c/generic/generic.c b/src/drivers/i2c/generic/generic.c index 981c2d9..15fe7ad 100644 --- a/src/drivers/i2c/generic/generic.c +++ b/src/drivers/i2c/generic/generic.c @@ -77,6 +77,16 @@ } } + if (config->has_power_resource && !config->disable_gpio_export_in_crs) { + /* + * This case will most likely cause timing problems. The OS driver might be + * controlling the GPIOs, but the ACPI Power Resource will also be controlling + * them. This will result in the two fighting and stomping on each other. + */ + printk(BIOS_ERR, "%s: Exposing GPIOs in Power Resource and _CRS\n", + dev_path(dev)); + } + /* Device */ acpigen_write_scope(scope); acpigen_write_device(acpi_device_name(dev)); -- To view, visit https://review.coreboot.org/c/coreboot/+/60174 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifcc42ed81fff295fb168a0b343e96b3a650b1c84 Gerrit-Change-Number: 60174 Gerrit-PatchSet: 5 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: cpu/mp_init.c: Only enable CPUs once they execute code
by Felix Held (Code Review) 05 Nov '22

05 Nov '22

Attention is currently required from: Arthur Heymans. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68892 ) Change subject: cpu/mp_init.c: Only enable CPUs once they execute code ...................................................................... Patch Set 8: (1 comment) Patchset: PS8: > this breaks things on amd/mandolin: […] when add_cpu_device is called with the enabled parameter = 0, it'll call find_dev_path instead of alloc_find_dev which won't allocate new cpu devices -- To view, visit https://review.coreboot.org/c/coreboot/+/68892 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6c8042b4d6127239175924f996f735bf9c83c6e8 Gerrit-Change-Number: 68892 Gerrit-PatchSet: 8 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Sat, 05 Nov 2022 02:04:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: cpu/mp_init.c: Only enable CPUs once they execute code
by Felix Held (Code Review) 05 Nov '22

05 Nov '22

Attention is currently required from: Arthur Heymans. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68892 ) Change subject: cpu/mp_init.c: Only enable CPUs once they execute code ...................................................................... Patch Set 8: (1 comment) Patchset: PS8: this breaks things on amd/mandolin: [INFO ] CPU: AMD Ryzen 3 PRO 3300U w/ Radeon Vega Mobile Gfx. [INFO ] LAPIC 0x0 in XAPIC mode. [DEBUG] CPU: APIC: 00 enabled [CRIT ] Could not allocate CPU device [CRIT ] Could not allocate CPU device [CRIT ] ERROR: More cpus requested (4) than supported (2). [ERROR] MP initialization failure. [EMERG] mp_init_with_smm failed. Halting. -- To view, visit https://review.coreboot.org/c/coreboot/+/68892 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6c8042b4d6127239175924f996f735bf9c83c6e8 Gerrit-Change-Number: 68892 Gerrit-PatchSet: 8 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Sat, 05 Nov 2022 01:42:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/brask: Disable PCH USB2 phy power gating for moli
by Casper Chang (Code Review) 05 Nov '22

05 Nov '22

Attention is currently required from: YH Lin, Tarun Tuli, Nick Vaccaro. Casper Chang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69193 ) Change subject: mb/google/brask: Disable PCH USB2 phy power gating for moli ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: Hi Nick, Tarun and YH, Would you please help to add one more +2 to land this CL to chormium. Thank you very much. -- To view, visit https://review.coreboot.org/c/coreboot/+/69193 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I457d410501be996f0f29ec622e1829f1581c4970 Gerrit-Change-Number: 69193 Gerrit-PatchSet: 5 Gerrit-Owner: Casper Chang <casper_chang(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: YH Lin <yueherngl(a)google.com> Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Curtis Chen <curtis.chen(a)intel.corp-partner.google.com> Gerrit-CC: Malik Hsu <malik_hsu(a)wistron.corp-partner.google.com> Gerrit-CC: Raihow Shi <raihow_shi(a)wistron.corp-partner.google.com> Gerrit-Attention: YH Lin <yueherngl(a)google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Comment-Date: Sat, 05 Nov 2022 01:03:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: ec/starlabs/merlin: Rename the Cezanne EC code
by Angel Pons (Code Review) 05 Nov '22

05 Nov '22

Attention is currently required from: Sean Rhodes. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68319 ) Change subject: ec/starlabs/merlin: Rename the Cezanne EC code ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/68319 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I25f812cb1c6cefca1ebbe3bee5d20cf521dd60af Gerrit-Change-Number: 68319 Gerrit-PatchSet: 6 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Comment-Date: Fri, 04 Nov 2022 23:04:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/sifive/ux00ddr.h: Remove set but unused variables
by build bot (Jenkins) (Code Review) 05 Nov '22

05 Nov '22

Attention is currently required from: Arthur Heymans, Philipp Hug, ron minnich, Elyes Haouas. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63074 ) Change subject: soc/sifive/ux00ddr.h: Remove set but unused variables ...................................................................... Patch Set 20: (4 comments) File src/soc/sifive/fu540/ux00ddr.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-162537): https://review.coreboot.org/c/coreboot/+/63074/comment/d65b36e8_883dd4d6 PS20, Line 179: /* char slicelsc = '0'; */ code indent should use tabs where possible Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-162537): https://review.coreboot.org/c/coreboot/+/63074/comment/2a974450_ce5768a9 PS20, Line 180: /* char slicemsc = '0'; */ code indent should use tabs where possible Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-162537): https://review.coreboot.org/c/coreboot/+/63074/comment/165d4272_55207653 PS20, Line 181: /* slicelsc += (dq % 10); */ code indent should use tabs where possible Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-162537): https://review.coreboot.org/c/coreboot/+/63074/comment/643df221_ae30df2c PS20, Line 182: /* slicemsc += (dq / 10); */ code indent should use tabs where possible -- To view, visit https://review.coreboot.org/c/coreboot/+/63074 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I442a42e297f2968dd2c824a93a9a1e2bc74ea2f4 Gerrit-Change-Number: 63074 Gerrit-PatchSet: 20 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Fri, 04 Nov 2022 23:04:16 +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 November 2022