Attention is currently required from: Jonathon Hall, Nico Huber.
Matt DeVillier 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/5f1d22f5_2f34c6a5 :
PS2, Line 154: io 0x60 = 0x070
> Is this forwarded to LPC? If not, would be worth a comment, so nobody […]
it's unused, so I could set it to 0. And maybe rework pnp_set_resource to not error if a given IO register is unused/set to 0?
--
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-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Comment-Date: Sat, 17 Feb 2024 15:51:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80598?usp=email )
Change subject: device/pnp_device: Skip init on disabled functions
......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80598/comment/b15ab7fc_98724f9b :
PS1, Line 11: any resources to that function.
> The code disabled here merely attaches the structures for the […]
I figured kneecapping it earliest was best
https://review.coreboot.org/c/coreboot/+/80598/comment/6100aacf_69eb2936 :
PS1, Line 15: in the log / don't cause any errors.
> Can you share a log? errors are often because of mistakes in the […]
[DEBUG] PNP: 002e.1 60 <- [0x00000000000003f8 - 0x00000000000003ff] size 0x00000008 gran 0x03 io
[DEBUG] PNP: 002e.1 70 <- [0x0000000000000004 - 0x0000000000000004] size 0x00000001 gran 0x00 irq
[DEBUG] PNP: 002e.10 62 <- [0x0000000000000360 - 0x0000000000000361] size 0x00000002 gran 0x01 io
[ERROR] PNP: 002e.10 60 io size: 0x0000000002 not assigned in devicetree
[ERROR] PNP: 002e.10 64 io size: 0x0000000002 not assigned in devicetree
[ERROR] PNP: 002e.10 66 io size: 0x0000000002 not assigned in devicetree
[ERROR] PNP: 002e.10 70 irq size: 0x0000000001 not assigned in devicetree
[DEBUG] LPC: enabling default decode range LPC_IOE_COMA_EN
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
[ERROR] LPC IO decode base 0!
best I can tell, the errors all stem from the IO bases of the various disabled functions being 0. With this patch, all but 3 of the 'decode base 0' errors are removed (those are from the RTC, which has 3 IO windows un-set / defaulting to 0).
File src/device/pnp_device.c:
https://review.coreboot.org/c/coreboot/+/80598/comment/31983cae_5ca7b7a5 :
PS1, Line 403: dev->ops = ops;
> Not setting the ops has certain side effects. For instance one couldn't generate […]
where do we generate SSDT entries for disabled devices?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80598?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: I5c9d5cf34947535bf249b91f78d6fabfb28729ea
Gerrit-Change-Number: 80598
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Martin L Roth <gaumless(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-Comment-Date: Sat, 17 Feb 2024 15:46:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Maciej Pijanowski, Michał Kopeć.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80609?usp=email )
Change subject: mb/lenovo/m920q: add board
......................................................................
Patch Set 3:
(5 comments)
File src/mainboard/lenovo/m920q/devicetree.cb:
PS3:
Move the options into the scope of the related controllers
PS3:
Remove options which are disabled
File src/mainboard/lenovo/m920q/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/80609/comment/b84cd215_7d478806 :
PS3, Line 16: // global NVS and variables
Remove superfluous comment
https://review.coreboot.org/c/coreboot/+/80609/comment/fbc7e68c_6276c850 :
PS3, Line 19: Scope (\_SB) {
: Device (PCI0)
```
Device (\_SB.PCI0) {
```
File src/mainboard/lenovo/m920q/mainboard.c:
https://review.coreboot.org/c/coreboot/+/80609/comment/26d56ad3_a99c5c04 :
PS3, Line 9: static void mainboard_enable(struct device *dev)
: {
: // nothing?
: }
:
: struct chip_operations mainboard_ops = {
: .enable_dev = mainboard_enable,
: };
:
Move this into ramstage.c and do the GPIO configuration in mainboard_enable(). mainboard_silicon_init_params() is not supposed to do that since it runs before the FSP is executed and the FSP reconfigures GPIOs.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80609?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: Iea1dc5745c0ecf687fa18b793f0aab4b0855d6d4
Gerrit-Change-Number: 80609
Gerrit-PatchSet: 3
Gerrit-Owner: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Comment-Date: Sat, 17 Feb 2024 14:18:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Hello build bot (Jenkins),
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 (#4).
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, 375 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/80470/4
--
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: 4
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel.
Reto Buerki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80261?usp=email )
Change subject: mb/up/squared: Make mini PCIe port mode configurable
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80261/comment/ac0511f5_a037b73a :
PS2, Line 9: Add ENABLE_MSATA config knob and pad configuration to put Mini PCIe port
: into mSATA mode.
> I've checked pinouts of mini PCIe and mSATA, they use the same pins for their […]
So is it ok to postpone this to another patch (if required)?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80261?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: Ic2da1dd4252ebb5e373bc65418e321f566d4c10f
Gerrit-Change-Number: 80261
Gerrit-PatchSet: 3
Gerrit-Owner: Reto Buerki <reet(a)codelabs.ch>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Sat, 17 Feb 2024 13:10:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Reto Buerki <reet(a)codelabs.ch>
Gerrit-MessageType: comment
Hello build bot (Jenkins),
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 (#3).
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, 375 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/80470/3
--
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: 3
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel.
Reto Buerki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80261?usp=email )
Change subject: mb/up/squared: Make mini PCIe port mode configurable
......................................................................
Patch Set 3:
(2 comments)
Patchset:
PS3:
Switched the config option to choice menu, and rebased to latest `main`.
File src/mainboard/up/squared/Kconfig:
https://review.coreboot.org/c/coreboot/+/80261/comment/009e666f_6bb52ee5 :
PS2, Line 77: config ENABLE_MSATA
: bool "Use mini-PCIe port for mSATA"
: default n
> Let's wait with that until we figured out what's going on with that GPIO.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/80261?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: Ic2da1dd4252ebb5e373bc65418e321f566d4c10f
Gerrit-Change-Number: 80261
Gerrit-PatchSet: 3
Gerrit-Owner: Reto Buerki <reet(a)codelabs.ch>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Sat, 17 Feb 2024 13:06:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Reto Buerki <reet(a)codelabs.ch>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Reto Buerki.
Hello Felix Singer, Nico Huber, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80261?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Code-Review+1 by Nico Huber, Code-Review+1 by Paul Menzel, Verified+1 by build bot (Jenkins)
Change subject: mb/up/squared: Make mini PCIe port mode configurable
......................................................................
mb/up/squared: Make mini PCIe port mode configurable
Add config choice menu and pad configuration to put Mini PCIe port into
mSATA mode.
The vendor firmware's "Chipset->Mini PCIe / mSATA Switch" option has
been used together with the output of inteltool and intel2pm to deduce
the exact pad configuration.
Note: the vendor firmware does not autodetect the mode, and the default
setting for the port is "Mini PCIe".
Tested with Kingston SUV500MS120G mSATA SSD.
Change-Id: Ic2da1dd4252ebb5e373bc65418e321f566d4c10f
Signed-off-by: Reto Buerki <reet(a)codelabs.ch>
---
M src/mainboard/up/squared/Kconfig
M src/mainboard/up/squared/gpio.h
M src/mainboard/up/squared/romstage.c
3 files changed, 30 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/80261/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/80261?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: Ic2da1dd4252ebb5e373bc65418e321f566d4c10f
Gerrit-Change-Number: 80261
Gerrit-PatchSet: 3
Gerrit-Owner: Reto Buerki <reet(a)codelabs.ch>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Reto Buerki <reet(a)codelabs.ch>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: newpatchset