Attention is currently required from: Michael Niewöhner, Nicholas Chin, Nicholas Sudsgaard.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80695?usp=email )
Change subject: include/device: Merge enums from azalia_device.h and azalia.h
......................................................................
Patch Set 5: Code-Review+1
(1 comment)
File src/include/device/azalia_device.h:
https://review.coreboot.org/c/coreboot/+/80695/comment/94d2e0be_558bb3f7 :
PS5, Line 60: AZALIA_LOCATION_UNKNOWN
Would something like AZALIA_LOCATION_NOT_APPLICABLE make more
sense here? In the mainboard code, an 'unknown' might look like
the developer didn't know what to set.
Or maybe, without looking too close, could we remove the whole value
after the merge locations patch?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80695?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: Ie874b083a18963679981a9cd2b25d123890d628e
Gerrit-Change-Number: 80695
Gerrit-PatchSet: 5
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Sun, 25 Feb 2024 14:21:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, ron minnich.
Hello build bot (Jenkins), ron minnich,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80470?usp=email
to look at the new patch set (#6).
Change subject: util: Add hda-decoder
......................................................................
util: Add hda-decoder
This tool helps take off the burden of manually decoding default
configuration registers. Using decoded values can make code more
self-documenting compared to shrouding it with magic numbers.
This is also written as a module which allows easy integration with
other tools written in Go (e.g. autoport).
Change-Id: Ib4fb652e178517b2b7aceaac8be005c5b2d3b03e
Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
---
A util/hda-decoder/.gitignore
A util/hda-decoder/Makefile
A util/hda-decoder/decoder/lib.go
A util/hda-decoder/decoder/lib_test.go
A util/hda-decoder/description.md
A util/hda-decoder/go.mod
A util/hda-decoder/main.go
7 files changed, 379 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/80470/6
--
To view, visit https://review.coreboot.org/c/coreboot/+/80470?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: Ib4fb652e178517b2b7aceaac8be005c5b2d3b03e
Gerrit-Change-Number: 80470
Gerrit-PatchSet: 6
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: ron minnich <rminnich(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, ron minnich.
Hello build bot (Jenkins), ron minnich,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80470?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: util: Add hda-decoder
......................................................................
util: Add hda-decoder
This tool helps take off the burden of manually decoding default
configuration registers. Using decoded values can make code more
self-documenting compared to shrouding it with magic numbers.
This is also written as a module which allows easy integration with
other tools written in Go (e.g. autoport).
Change-Id: Ib4fb652e178517b2b7aceaac8be005c5b2d3b03e
Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
---
A util/hda-decoder/.gitignore
A util/hda-decoder/Makefile
A util/hda-decoder/decoder/lib.go
A util/hda-decoder/decoder/lib_test.go
A util/hda-decoder/description.md
A util/hda-decoder/go.mod
A util/hda-decoder/main.go
7 files changed, 379 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/80470/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/80470?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: Ib4fb652e178517b2b7aceaac8be005c5b2d3b03e
Gerrit-Change-Number: 80470
Gerrit-PatchSet: 5
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: ron minnich <rminnich(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Michael Niewöhner, Nicholas Chin, Nico Huber.
Nicholas Sudsgaard has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80695?usp=email )
Change subject: include/device: Merge enums from azalia_device.h and azalia.h
......................................................................
Patch Set 5:
(2 comments)
File src/mainboard/siemens/chili/variants/chili/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/80695/comment/368c3f01_b1aa546a :
PS4, Line 17: /* 0x14 Speaker OUT */
> I would like to keep them. It's still much easier to parse than the all-caps […]
Acknowledged
https://review.coreboot.org/c/coreboot/+/80695/comment/2c2f05dd_cd4e7b62 :
PS4, Line 26: 0x0
> There's no reason to make these hex numbers, is there? I was first wondering […]
That makes sense, I should probably change that behavior on my decoder as well.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80695?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: Ie874b083a18963679981a9cd2b25d123890d628e
Gerrit-Change-Number: 80695
Gerrit-PatchSet: 5
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Sun, 25 Feb 2024 13:54:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-MessageType: comment
Attention is currently required from: Michael Niewöhner, Nicholas Chin, Nicholas Sudsgaard.
Hello Michael Niewöhner, Nicholas Chin, Nico Huber, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80695?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: include/device: Merge enums from azalia_device.h and azalia.h
......................................................................
include/device: Merge enums from azalia_device.h and azalia.h
We were keeping 2 copies of the same thing (albeit there were some
slight differences). As azalia_device.h is used much more in the
codebase this was kept as the base and then some of the nice features
of azalia.h were incorporated.
The significant changes are:
- All enum names now use the `AZALIA_` prefix.
This also drops the AzaliaPinConfiguration enum as it was never used
since added in 2013.
Change-Id: Ie874b083a18963679981a9cd2b25d123890d628e
Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
---
D src/include/device/azalia.h
M src/include/device/azalia_device.h
M src/mainboard/clevo/tgl-u/variants/l140mu/hda_verb.c
M src/mainboard/siemens/chili/variants/chili/hda_verb.c
4 files changed, 157 insertions(+), 259 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/80695/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/80695?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: Ie874b083a18963679981a9cd2b25d123890d628e
Gerrit-Change-Number: 80695
Gerrit-PatchSet: 5
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jonathon Hall, Matt DeVillier.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80601?usp=email )
Change subject: mb/purism/librem_cnl/var/librem_mini: Set RTC register defaults
......................................................................
Patch Set 2:
(1 comment)
File src/mainboard/purism/librem_cnl/variants/librem_mini/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/80601/comment/cec058ff_af0d087c :
PS2, Line 154: io 0x60 = 0x070
> alternately, can do CB:80645 instead
Maybe do both? This hardware register is enable by the `2e.10 on` anyway,
and it has an actual value no matter if we set one here. Might be best to
make the value explicit.
Btw. I just realized something. Wouldn't the automatism in soc/intel/lpc
try to forward a 0x70 to LPC? Then setting 0x70 might break access to
the PCH's RTC.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80601?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: I310a834a1fa7b8428879fbc160e4aae0a519e063
Gerrit-Change-Number: 80601
Gerrit-PatchSet: 2
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Martin L Roth <gaumless(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-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Comment-Date: Sun, 25 Feb 2024 13:24:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Matt DeVillier.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80647?usp=email )
Change subject: soc/intel/common/lpc_lib: Demote printk for IO base value 0
......................................................................
Patch Set 1: Code-Review-1
(1 comment)
Patchset:
PS1:
If this function gets called with a 0 value, that's most likely a bug at the
calling site. For instance like the one that is fixed by the parent commit.
It probably wouldn't have been discovered if this wasn't an error message.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80647?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: I593f8e3c7eb9e877f89568fd63eabe12bb777f93
Gerrit-Change-Number: 80647
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Comment-Date: Sun, 25 Feb 2024 13:18:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Martin Roth, Matt DeVillier.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80645?usp=email )
Change subject: device/pnp_device: Demote unassigned resource printk to SPEW
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80645/comment/392e7f41_6f3c32c5 :
PS1, Line 11: the printk but demote it so it doesn't pollute a normal cbmem log.
BIOS_ERR seems too much indeed, but it can be a mistake, so I'd say it
should be at least BIOS_NOTICE:
```
/**
* \brief BIOS_NOTICE - Unexpected but relatively insignificant
*
* Log level for when the system has noticed an issue that is an edge case,
* but is handled and is recoverable. To be used when an end-user would likely
* not notice.
*
* Example - Hardware was misconfigured, but is promptly fixed.
*
* @{
*/
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/80645?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: I3d9f22a06088596e14680190aede2d69880001fa
Gerrit-Change-Number: 80645
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Comment-Date: Sun, 25 Feb 2024 13:15:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Michael Niewöhner, Nicholas Chin, Nicholas Sudsgaard.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80695?usp=email )
Change subject: include/device: Merge enums from azalia_device.h and azalia.h
......................................................................
Patch Set 4:
(3 comments)
File src/include/device/azalia_device.h:
https://review.coreboot.org/c/coreboot/+/80695/comment/3aeeffce_2d29a332 :
PS4, Line 128: conn
> That would open a can of worms once you start using more "complex" things. […]
Ack. Be careful with vendor values, though. If they are hard to express, it's
often because they are wrong.
File src/mainboard/siemens/chili/variants/chili/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/80695/comment/64cd5a23_61d06da1 :
PS4, Line 17: /* 0x14 Speaker OUT */
> Maybe we can drop these comments.
I would like to keep them. It's still much easier to parse than the all-caps
macro list.
https://review.coreboot.org/c/coreboot/+/80695/comment/ecbf556d_d245c33e :
PS4, Line 26: 0x0
There's no reason to make these hex numbers, is there? I was first wondering
why we have magic numbers here at all, then realized that it's kind of a
counter / id. Feels more natural to have decimals.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80695?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: Ie874b083a18963679981a9cd2b25d123890d628e
Gerrit-Change-Number: 80695
Gerrit-PatchSet: 4
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Sun, 25 Feb 2024 13:08:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-MessageType: comment
Attention is currently required from: Michael Niewöhner, Nicholas Sudsgaard.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80740?usp=email )
Change subject: include/device/azalia_device.h: Merge location1 and location2
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/80740?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: I5a61a37ed70027700f07f1532c500f04d7a16ce1
Gerrit-Change-Number: 80740
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Sun, 25 Feb 2024 13:02:08 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment