coreboot-gerrit February 2024

coreboot-gerrit@coreboot.org

1 participants
3067 discussions

[S] Change in coreboot[main]: brox: Handle GPI_INT pin value lower to GPI_WAKE
by Shelley Chen (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Ashish Kumar Mishra, Bora Guvendik. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80334?usp=email ) Change subject: brox: Handle GPI_INT pin value lower to GPI_WAKE ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Oh also, is this a restriction that we need to adhere to for future devices (INT pin needs to be higher IRQ# than WAKE pin)? -- To view, visit https://review.coreboot.org/c/coreboot/+/80334?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: I8c3d557e888b8d0ceac203f49b702910fba26d6d Gerrit-Change-Number: 80334 Gerrit-PatchSet: 1 Gerrit-Owner: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-CC: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com> Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Comment-Date: Sat, 03 Feb 2024 01:22:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: brox: Handle GPI_INT pin value lower to GPI_WAKE
by Shelley Chen (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Ashish Kumar Mishra, Bora Guvendik. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80334?usp=email ) Change subject: brox: Handle GPI_INT pin value lower to GPI_WAKE ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Thank you Ashish for debugging this issue! I am a bit confused... are you saying that this originally was happening because the EC INT pin was set to GPP_D0 and the EC WAKE pin was set to GPP_D1? But we were also seeing this issue when the pins were swapped originally (a config mistake corrected by https://review.coreboot.org/c/coreboot/+/79886). We were also seeing the same conflict behavior between cros_ec_lpcs and the iadma64 driver at that point too, except it was happening on IRQ #45 instead (as you can see in the dmesg output in https://issuetracker.google.com/319129926#comment1). Was wondering if you know why we were seeing the same issue if the WAKE pin (GPP_D0 at the time) was lower than the INT pin (GPP_D1 at the time) at that point? -- To view, visit https://review.coreboot.org/c/coreboot/+/80334?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: I8c3d557e888b8d0ceac203f49b702910fba26d6d Gerrit-Change-Number: 80334 Gerrit-PatchSet: 1 Gerrit-Owner: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-CC: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com> Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Comment-Date: Sat, 03 Feb 2024 01:20:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: util/cbfstool: Fallback to linear search
by Julius Werner (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Karthik Ramasubramanian, Keith Short, Shelley Chen. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80308?usp=email ) Change subject: util/cbfstool: Fallback to linear search ...................................................................... Patch Set 2: (1 comment) File util/cbfstool/flashmap/fmap.c: https://review.coreboot.org/c/coreboot/+/80308/comment/2e67ba0a_0aea0bc4 : PS1, Line 108: 16 > My EC binary wasn't forcing alignment of FMAP and I ran into this. […] Yeah that's a bit of a weird choice in the `flash_ec` script. `futility load_fmap` would probably make more sense there. This change is definitely way less efficient than just changing the stride to something smaller (because then bsearch will only search the offsets it hasn't already searched at the smaller stride). But either way I'm not sure we should be changing cbfstool for a use case it wasn't meant for anyway. -- To view, visit https://review.coreboot.org/c/coreboot/+/80308?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: Ifa9f6f57d1c59114fd1d4ace2d82b62e953cf6a8 Gerrit-Change-Number: 80308 Gerrit-PatchSet: 2 Gerrit-Owner: Keith Short <keithshort(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Keith Short <keithshort(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Sat, 03 Feb 2024 01:03:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Keith Short <keithshort(a)chromium.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: cpu/x86/Kconfig: Mark 64bit support as stable
by Julius Werner (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Arthur Heymans, Jérémy Compostella. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80338?usp=email ) Change subject: cpu/x86/Kconfig: Mark 64bit support as stable ...................................................................... Patch Set 1: (1 comment) File src/arch/x86/Kconfig: https://review.coreboot.org/c/coreboot/+/80338/comment/80cbdc61_29641201 : PS1, Line 76: EXP nit: If it's no longer experimental, should it still say "EXP" here? -- To view, visit https://review.coreboot.org/c/coreboot/+/80338?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: Ifcf0a1a5097a2d7c064bb709ec0b09ebee13a47d Gerrit-Change-Number: 80338 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Sat, 03 Feb 2024 00:18:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: util/cbfstool: Fallback to linear search
by Keith Short (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Julius Werner, Karthik Ramasubramanian, Shelley Chen. Keith Short has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80308?usp=email ) Change subject: util/cbfstool: Fallback to linear search ...................................................................... Patch Set 2: (1 comment) File util/cbfstool/flashmap/fmap.c: https://review.coreboot.org/c/coreboot/+/80308/comment/b4f81789_0d2efa02 : PS1, Line 108: 16 > I think if you want to allow FMAPs to be found at a smaller alignment it would make more sense to re […] My EC binary wasn't forcing alignment of FMAP and I ran into this. The [flash_ec](http://cs/h/chromium/chromiumos/codesearch/+/main:src/platform/ec/util/flash_ec?l=1909) script uses cbfstool to patch in the CBI information when we store CBI in the EC flash instead of an external EEPROM. This change preserves the speed of the binary search, at the cost of a slower cbfstool on images without an FMAP. Do you know of the distribution of coreboot images with and without an FMAP? -- To view, visit https://review.coreboot.org/c/coreboot/+/80308?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: Ifa9f6f57d1c59114fd1d4ace2d82b62e953cf6a8 Gerrit-Change-Number: 80308 Gerrit-PatchSet: 2 Gerrit-Owner: Keith Short <keithshort(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Sat, 03 Feb 2024 00:05:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: util/cbfstool: Fallback to linear search
by Julius Werner (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Karthik Ramasubramanian, Keith Short, Shelley Chen. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80308?usp=email ) Change subject: util/cbfstool: Fallback to linear search ...................................................................... Patch Set 1: (1 comment) File util/cbfstool/flashmap/fmap.c: https://review.coreboot.org/c/coreboot/+/80308/comment/816d267c_14230a05 : PS1, Line 108: 16 I think if you want to allow FMAPs to be found at a smaller alignment it would make more sense to reduce this number here. However, do we actually want that? This was set to 16 intentionally in CB:317321. I don't think there's a single authoritative "FMAP specification", but I feel like a completely misaligned FMAP would just be not valid. (Usually, we aim to intentionally make the FMAP alignment much larger to reduce lookup times.) Do you have a real situation where an FMAP is aligned to less than 16 bytes? Why can't it be aligned better? (Also, FWIW, cbfstool is only really meant to work on CBFS images, which an EC binary is not. If you're just looking for an FMAP parser, I'd suggest using `dump_fmap -h ec.bin` instead.) -- To view, visit https://review.coreboot.org/c/coreboot/+/80308?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: Ifa9f6f57d1c59114fd1d4ace2d82b62e953cf6a8 Gerrit-Change-Number: 80308 Gerrit-PatchSet: 1 Gerrit-Owner: Keith Short <keithshort(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Keith Short <keithshort(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 02 Feb 2024 23:46:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: device/azalia: Rework the verb table
by Nicholas Sudsgaard (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Nicholas Sudsgaard. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80333?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: device/azalia: Rework the verb table ...................................................................... device/azalia: Rework the verb table This is a very experimental change and I would appreciate anyone's opinion on this. :) My intentions are to make verb tables cleaner on the both the configuration and implementation side. On the configuration side (i.e. hda_verbs.c) this method removes most superfluous comments (e.g. /* Subsystem ID */) and makes it more "structured". On the implementation side I tried to make it easier to read (and hopefully maintain). For example, compare azalia_find_verb() and azalia_find_verb_table(). I also got rid of global variables (but introduced including c source files directly instead...). Change-Id: If8b672e4fd800b34e5ba39fad174fcf1154b0a54 Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> --- M src/device/azalia_device.c M src/include/device/azalia_device.h A src/include/device/azalia_table_exporter.c M src/mainboard/hp/snb_ivb_laptops/variants/2560p/hda_verb.c 4 files changed, 114 insertions(+), 103 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/80333/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/80333?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: If8b672e4fd800b34e5ba39fad174fcf1154b0a54 Gerrit-Change-Number: 80333 Gerrit-PatchSet: 5 Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: device/azalia: Cleanup codec initialization code
by Nicholas Sudsgaard (Code Review) 02 Feb '24

02 Feb '24

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80332?usp=email to look at the new patch set (#4). Change subject: device/azalia: Cleanup codec initialization code ...................................................................... device/azalia: Cleanup codec initialization code azalia_codec_init() was doing too much in one function. This also changes how debug messages will be printed. I focused on reducing clutter on the screen and made the style of the messages consistent. Before: azalia_audio: Initializing codec #5 codec not ready. azalia_audio: Initializing codec #4 codec not valid. azalia_audio: Initializing codec #3 azalia_audio: viddid: ffffffff azalia_audio: verb_size: 4 azalia_audio: verb loaded. After: azalia_audio: codec #5 not ready azalia_audio: codec #4 not valid azalia_audio: initializing codec #3... azalia_audio: - vendor/device id: 0xffffffff azalia_audio: - verb size: 4 azalia_audio: - verb loaded Change-Id: I92b6d184abccdbe0e1bfce98a2c959a97a618a29 Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> --- M src/device/azalia_device.c M src/include/device/azalia_device.h 2 files changed, 61 insertions(+), 40 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/80332/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/80332?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: I92b6d184abccdbe0e1bfce98a2c959a97a618a29 Gerrit-Change-Number: 80332 Gerrit-PatchSet: 4 Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: util/cbfstool: Fallback to linear search
by Keith Short (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Karthik Ramasubramanian, Shelley Chen. Hello Karthik Ramasubramanian, Shelley Chen, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80308?usp=email to look at the new patch set (#2). Change subject: util/cbfstool: Fallback to linear search ...................................................................... util/cbfstool: Fallback to linear search When the image size is a power of 2, cbfstool attempts to find the FMAP region using a binary search. However, if the FMAP location isn't aligned on a 16 byte boundary the binary search fails. Fall back to linear search if binary search fails. TEST=make -C util/cbfstool Change-Id: Ifa9f6f57d1c59114fd1d4ace2d82b62e953cf6a8 Signed-off-by: Keith Short <keithshort(a)chromium.org> --- M util/cbfstool/flashmap/fmap.c 1 file changed, 6 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/80308/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80308?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: Ifa9f6f57d1c59114fd1d4ace2d82b62e953cf6a8 Gerrit-Change-Number: 80308 Gerrit-PatchSet: 2 Gerrit-Owner: Keith Short <keithshort(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: device/azalia: Rework the verb table
by Nicholas Sudsgaard (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Nicholas Sudsgaard. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80333?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: device/azalia: Rework the verb table ...................................................................... device/azalia: Rework the verb table This is a very experimental change and I would appreciate anyone's opinion on this. :) My intentions are to make verb tables cleaner on the both the configuration and implementation side. On the configuration side (i.e. hda_verbs.c) this method removes most superfluous comments (e.g. /* Subsystem ID */) and makes it more "structured". On the implementation side I tried to make it easier to read (and hopefully maintain). For example, compare azalia_find_verb() and azalia_find_verb_table(). I also got rid of global variables (but introduced including c source files directly instead...). Change-Id: If8b672e4fd800b34e5ba39fad174fcf1154b0a54 Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> --- M src/device/azalia_device.c M src/include/device/azalia_device.h A src/include/device/azalia_table_exporter.c M src/mainboard/hp/snb_ivb_laptops/variants/2560p/hda_verb.c 4 files changed, 114 insertions(+), 103 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/80333/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/80333?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: If8b672e4fd800b34e5ba39fad174fcf1154b0a54 Gerrit-Change-Number: 80333 Gerrit-PatchSet: 4 Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2024