Attention is currently required from: Felix Singer, Julius Werner, Philipp M, ron minnich.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74173?usp=email )
Change subject: arch/arm* Fix some linter warnings and errors
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/74173?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: I976cffe971695f591ec42fce46e6743444277759
Gerrit-Change-Number: 74173
Gerrit-PatchSet: 2
Gerrit-Owner: Philipp M
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Philipp M
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: ron minnich <rminnich(a)gmail.com>
Gerrit-Comment-Date: Sun, 04 Feb 2024 16:03:06 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Julius Werner, Philipp M, ron minnich.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74173?usp=email )
Change subject: arch/arm* Fix some linter warnings and errors
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/74173/comment/a88324e6_9b0a8dc5 :
PS2, Line 7: arch/arm*
Please add a colon after the prefix.
https://review.coreboot.org/c/coreboot/+/74173/comment/ecfdf537_c06dedf1 :
PS2, Line 9: Correct some linter issues.
What linter did you run?
--
To view, visit https://review.coreboot.org/c/coreboot/+/74173?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: I976cffe971695f591ec42fce46e6743444277759
Gerrit-Change-Number: 74173
Gerrit-PatchSet: 2
Gerrit-Owner: Philipp M
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
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-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Philipp M
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: ron minnich <rminnich(a)gmail.com>
Gerrit-Comment-Date: Sun, 04 Feb 2024 16:02:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Cliff Huang, David Ruth, Eric Lai, Lance Zhao, Nick Vaccaro, Paul Menzel, Subrata Banik, Tim Wawrzynczak.
Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80170?usp=email )
Change subject: drivers/wifi: Add MTCL function to ACPI SSDT
......................................................................
Patch Set 19: Code-Review+2
--
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: 19
Gerrit-Owner: David Ruth <druth(a)chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
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: 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: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: David Ruth <druth(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Sun, 04 Feb 2024 13:02:56 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Julius Werner.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80251?usp=email )
Change subject: lib: Move IP checksum to commonlib
......................................................................
Patch Set 4:
(1 comment)
File src/commonlib/bsd/include/commonlib/bsd/ipchksum.h:
https://review.coreboot.org/c/coreboot/+/80251/comment/bd904f97_16cecebe :
PS4, Line 9: uint16_t ipchksum(const void *data, size_t size);
: uint16_t ipchksum_add(size_t offset, uint16_t first, uint16_t second);
Before it was:
unsigned long compute_ip_checksum(const void *addr, unsigned long length);
unsigned long add_ip_checksums(unsigned long offset, unsigned long sum,
unsigned long new);
Could you please elaborate, why you changed the signature and used fixed variable types?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80251?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: Ic04c714c00439a17fc04a8a6e730cc2aa19b8e68
Gerrit-Change-Number: 80251
Gerrit-PatchSet: 4
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Sun, 04 Feb 2024 12:53:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80253?usp=email )
Change subject: libpayload: Switch to commonlib ipchksum() algorithm
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
FILO fails to build now, I believe:
```
CC build/main/elfload.o
In file included from /dev/shm/coreboot/payloads/external/FILO/filo/main/elfload.c:32:
include/ipchecksum.h:21:16: error: conflicting types for 'ipchksum'; have 'short unsigned int(const void *, long unsigned int)'
21 | unsigned short ipchksum(const void *data, unsigned long length);
| ^~~~~~~~
In file included from /dev/shm/coreboot/payloads/external/FILO/filo/build/libpayload/bin/../include/libpayload.h:51,
from /dev/shm/coreboot/payloads/external/FILO/filo/main/elfload.c:24:
/dev/shm/coreboot/payloads/external/FILO/filo/build/libpayload/bin/../include/commonlib/bsd/ipchksum.h:9:10: note: previous declaration of 'ipchksum' with type 'uint16_t(const void *, size_t)' {aka 'short unsigned int(const void *, unsigned int)'}
9 | uint16_t ipchksum(const void *data, size_t size);
| ^~~~~~~~
make[2]: *** [Makefile.standalone:35: /dev/shm/coreboot/payloads/external/FILO/filo/build/main/elfload.o] Error 1
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/80253?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: Ie8d323ce9f8d946758619761b4b22d54bce222b6
Gerrit-Change-Number: 80253
Gerrit-PatchSet: 4
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.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-Comment-Date: Sun, 04 Feb 2024 12:36:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Elyes Haouas, Paul Menzel.
Nicholas Sudsgaard has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80332?usp=email )
Change subject: device/azalia: Clean up codec initialization code
......................................................................
Patch Set 5:
(4 comments)
File src/device/azalia_device.c:
https://review.coreboot.org/c/coreboot/+/80332/comment/398e3436_ed399339 :
PS5, Line 9: stdbool.h
> types. […]
Done
https://review.coreboot.org/c/coreboot/+/80332/comment/ff7fa7b7_65d0bd2b :
PS5, Line 230: static bool
> is "enum cb_err" can be used ? […]
Done
https://review.coreboot.org/c/coreboot/+/80332/comment/7a42fc7e_e81eef14 :
PS5, Line 249: u32
> Please, is "size_t' correct here?
Yes, I agree this should be `size_t`.
If I'm going to change this here, I would also like to change all the other `u32`s that should be `size_t` in this file. However that seems like a bit too much and would be better in a separate patch.
File src/include/device/azalia_device.h:
https://review.coreboot.org/c/coreboot/+/80332/comment/1d6ee167_206aa4bf :
PS5, Line 13:
> not sure, but I think those are intentional (same for line #18 & 19)
Hmmm, I think I see the intention here, do you think it would be better keeping this? I would be leaning more on the "not necessary" side.
--
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: 5
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Sun, 04 Feb 2024 12:30:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel.
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 (#7).
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, 113 insertions(+), 103 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/80333/7
--
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: 7
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nicholas Sudsgaard, Paul Menzel.
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 (#6).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: device/azalia: Clean up codec initialization code
......................................................................
device/azalia: Clean up 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/6
--
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: 6
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nicholas Sudsgaard, Paul Menzel.
Elyes Haouas has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80332?usp=email )
Change subject: device/azalia: Clean up codec initialization code
......................................................................
Patch Set 5:
(4 comments)
File src/device/azalia_device.c:
https://review.coreboot.org/c/coreboot/+/80332/comment/bd94dec2_77c032da :
PS5, Line 9: stdbool.h
types.h
https://review.coreboot.org/c/coreboot/+/80332/comment/1caed32f_cbcdaffc :
PS5, Line 230: static bool
is "enum cb_err" can be used ?
(commonlib/bsd/cb_err.h)
https://review.coreboot.org/c/coreboot/+/80332/comment/1cb10d31_b4685469 :
PS5, Line 249: u32
Please, is "size_t' correct here?
File src/include/device/azalia_device.h:
https://review.coreboot.org/c/coreboot/+/80332/comment/00c120c5_083467a4 :
PS5, Line 13:
not sure, but I think those are intentional (same for line #18 & 19)
--
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: 5
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Comment-Date: Sun, 04 Feb 2024 11:00:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel.
Nicholas Sudsgaard has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80333?usp=email )
Change subject: device/azalia: Rework the verb table
......................................................................
Patch Set 6:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80333/comment/27e96f2b_b1c1a28e :
PS5, Line 9: This is a very experimental change and I would appreciate anyone's
: opinion on this. :)
:
> I recommend to bring this up on the mailing list and reference your change-set here.
Done
--
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: 6
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Sat, 03 Feb 2024 23:02:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment