Attention is currently required from: Christian Walter.
Hello Christian Walter,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80419?usp=email
to look at the new patch set (#2).
Change subject: util/intelmetool: Add Intel Union Point support
......................................................................
util/intelmetool: Add Intel Union Point support
The device IDs were taken from the 200 series datasheet (page 24).
Change-Id: I34b5cb61dd7b561778cc8506858cd436e6f04f9a
Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
---
M util/intelmetool/intelmetool.h
1 file changed, 22 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/80419/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80419?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: I34b5cb61dd7b561778cc8506858cd436e6f04f9a
Gerrit-Change-Number: 80419
Gerrit-PatchSet: 2
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Christian Walter.
Nicholas Sudsgaard has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80419?usp=email )
Change subject: util/intelmetool: Add Intel Union Point support
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Running the program multiple times there are a few "hiccups". Sometimes it timeouts, sometimes
```
ME: response is not complete
ME: GET FWCAPS message failed
```
and sometimes
```
ME: response is missing data
ME: invalid response, group 255 ?= 132, command 2 ?= 0, is_response 0
ME: not enough room in response buffer: 8 != 6
ME: Firmware Version 0.1023.0.0 (code) 0.0.0.0 (recovery) 0.0.0.0 (fitc)
*** stack smashing detected ***: terminated
Aborted
```
I wanted to make sure this is *expected behaviour* (I currently don't have a supported machine to test this). It does usually work though.
Here is the output from a good run.
```
Bad news, you have a `200 Series PCH LPC Controller (B250)` so you have ME hardware on board and you can't control or disable it, continuing...
MEI found: [8086:a2ba] 200 Series PCH CSME HECI #1
ME Status : 0x90000245
ME Status 2 : 0x86110306
ME: FW Partition Table : OK
ME: Bringup Loader Failure : NO
ME: Firmware Init Complete : YES
ME: Manufacturing Mode : NO
ME: Boot Options Present : NO
ME: Update In Progress : NO
ME: Current Working State : Normal
ME: Current Operation State : M0 with UMA
ME: Current Operation Mode : Normal
ME: Error Code : No Error
ME: Progress Phase : Clean Moff->Mx wake
ME: Power Management Event : Pseudo-global reset
ME: Progress Phase State : Unknown 0x11
ME: Extend Register not valid
ME: Firmware Version 11.8.3425.50 (code) 11.8.3425.50 (recovery) 11.8.3425.50 (fitc)
ME Capability: Full Network manageability : OFF
ME Capability: Regular Network manageability : OFF
ME Capability: Manageability : OFF
ME Capability: Small business technology : OFF
ME Capability: Level III manageability : OFF
ME Capability: IntelR Anti-Theft (AT) : OFF
ME Capability: IntelR Capability Licensing Service (CLS) : ON
ME Capability: IntelR Power Sharing Technology (MPC) : OFF
ME Capability: ICC Over Clocking : OFF
ME Capability: Protected Audio Video Path (PAVP) : ON
ME Capability: IPV6 : OFF
ME Capability: KVM Remote Control (KVM) : OFF
ME Capability: Outbreak Containment Heuristic (OCH) : OFF
ME Capability: Virtual LAN (VLAN) : ON
ME Capability: TLS : OFF
ME Capability: Wireless LAN (WLAN) : OFF
Bad news, you have a `200 Series PCH LPC Controller (B250)` so you have ME hardware on board and you can't control or disable it, continuing...
Boot Guard MSR Output : 0x0
Your system isn't Boot Guard ready.
You can flash other firmware!
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/80419?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: I34b5cb61dd7b561778cc8506858cd436e6f04f9a
Gerrit-Change-Number: 80419
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Comment-Date: Sat, 10 Feb 2024 13:16:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Alexander Couzens, Nicholas Chin, Nicholas Sudsgaard, Paul Menzel.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80343?usp=email )
Change subject: mainboard/lenovo: Add ThinkCentre M710s (Skylake)
......................................................................
Patch Set 5: Code-Review+1
(13 comments)
Patchset:
PS5:
Beside a small nits and places where I wished for a comment,
this looks very good :)
Feel free to ignore any comment that I already marked resolved
and take the comments mentioning "schematics" with a grain of
salt as we might have different versions of them.
File src/mainboard/lenovo/thinkcentre_m710s/Makefile.mk:
https://review.coreboot.org/c/coreboot/+/80343/comment/58113de1_6070710f :
PS5, Line 6: ramstage-y += early_init.c
This is not needed in ramstage.
File src/mainboard/lenovo/thinkcentre_m710s/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/80343/comment/e9071032_7c555607 :
PS5, Line 5: # FSP Configuration
This is a remnant from times where we configured a lot of FSP directly at
the toplevel. The `eist_enable` in particular is a coreboot setting. So
please drop the comment.
https://review.coreboot.org/c/coreboot/+/80343/comment/025ebb94_f26545e1 :
PS5, Line 10: device ref system_agent on end
This one is essential and hence already enabled in the `chipset.cb`. You
can drop this at your convenience.
https://review.coreboot.org/c/coreboot/+/80343/comment/15f498c6_8f17db57 :
PS5, Line 14: register "PcieRpClkSrcNumber[0]" = "0"
Did you test the x16 slot? Please mention that one explicitly in the
commit message.
The `PcieRp*` settings are actually for the PCH root ports. Though, it
seems Intel forgot to add ClkReq settings for the PEG. If this works,
it deserves a comment.
https://review.coreboot.org/c/coreboot/+/80343/comment/8441b25d_810ceae9 :
PS5, Line 51: register "PcieRpClkReqSupport[4]" = "false"
In the schematics it seems connected as CLKREQ_LAN#. If it's not working
please add a comment that it's unknown why.
https://review.coreboot.org/c/coreboot/+/80343/comment/370b9beb_7d40ad9b :
PS5, Line 54: end
I see a PCIe-PCI bridge in the schematics on `pcie_rp6`. Is it not populated?
https://review.coreboot.org/c/coreboot/+/80343/comment/247dcf44_aed16f6c :
PS5, Line 112: # Vendor values dumped using util/superiotool.
Did you push a superiotool patch yet? I'm still a little concerned about
the shift in LDNs we were seeing. But if the current port works, e.g. COM1
in the OS, then the numbers here must be correct.
File src/mainboard/lenovo/thinkcentre_m710s/early_init.c:
PS5:
Please join this with `bootblock.c`. We usually have either one of them.
File src/mainboard/lenovo/thinkcentre_m710s/gpio.h:
https://review.coreboot.org/c/coreboot/+/80343/comment/698bf55c_ed7230f3 :
PS5, Line 14: _PAD_CFG_STRUCT(GPP_A0, PAD_FUNC(NF1) | PAD_RESET(PLTRST) | PAD_TRIG(OFF) | PAD_BUF(TX_DISABLE) | (1 << 1), 0),
Technically the _-prefixed macros are reserved for internal use in
soc/intel/common/. If there is no compatible high-level macro, that's
something to investigate.
I don't mind merging it like this as there are other cases in the tree.
But I'll skip the GPIOs in this review.
File src/mainboard/lenovo/thinkcentre_m710s/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/80343/comment/879e3dcc_05a06d4d :
PS5, Line 20: AZALIA_PIN_CFG(0, 0x1e, 0x411111f0),
We could decode these (there are plenty of definitions and macros
in device/azalia*.h). Though, having this done by hand once, I know
it's not fun. We should write a tool for the decoding, I guess. Or
live with the encoded numbers (nobody seems to care anyway).
Anyway, this is good enough. Just wanted to have it mentioned that
it's not all magic ;)
File src/mainboard/lenovo/thinkcentre_m710s/romstage.c:
https://review.coreboot.org/c/coreboot/+/80343/comment/727a7b72_2f9fbc0c :
PS5, Line 11: 50
First should be 60, for the 4 DIMM configuration.
https://review.coreboot.org/c/coreboot/+/80343/comment/31217355_bbee994e :
PS5, Line 32:
No space after the casts, please.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80343?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: I551753aecfbd2c0ee57d85bb22cb943eb21af3cc
Gerrit-Change-Number: 80343
Gerrit-PatchSet: 5
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
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: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Sat, 10 Feb 2024 12:02:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80411?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: Documentation/mainboard/lenovo: Add ThinkCentre M710s
......................................................................
Documentation/mainboard/lenovo: Add ThinkCentre M710s
Change-Id: I90311257a28bd463712c4d43f8b83baa745509cc
Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
---
M Documentation/mainboard/index.md
A Documentation/mainboard/lenovo/ch341a_pinout.jpg
A Documentation/mainboard/lenovo/thinkcentre_m710s.md
A Documentation/mainboard/lenovo/thinkcentre_m710s_spi_location.jpg
4 files changed, 208 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/80411/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/80411?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: I90311257a28bd463712c4d43f8b83baa745509cc
Gerrit-Change-Number: 80411
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: Alexander Couzens, Nicholas Chin, Paul Menzel.
Hello Alexander Couzens, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80343?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: mainboard/lenovo: Add ThinkCentre M710s (Skylake)
......................................................................
mainboard/lenovo: Add ThinkCentre M710s (Skylake)
Working:
- Can boot Ubuntu 22.04.1 (Linux 6.5.0) using payloads:
- SeaBIOS
- TianoCore EDK 2
- Internal flashing
- PCIe
- SATA
- M.2 SSD
- M.2 WLAN (+ Bluetooth)
- LAN
- USB
- Memory card reader
- CPU fan
- VGA (DP bridge)
- Display ports
- Audio (output)
- COM1
- TPM
Not Working:
- SATA ACPI error
- SuperIO related things
- Power button LED
- PCIe clock related things and AER issues (LiveCD)
- Some drm issue when using EDK 2 and libgfxinit (LiveCD)
- ME cleaner
Untested:
- Audio (input)
Won't Test:
- COM2 header
- LPT header
- PS/2 keyboard and mouse
Thanks to Nico Huber and everyone else on the IRC for helping me write
my first port!
Change-Id: I551753aecfbd2c0ee57d85bb22cb943eb21af3cc
Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
---
A src/mainboard/lenovo/thinkcentre_m710s/Kconfig
A src/mainboard/lenovo/thinkcentre_m710s/Kconfig.name
A src/mainboard/lenovo/thinkcentre_m710s/Makefile.mk
A src/mainboard/lenovo/thinkcentre_m710s/acpi/ec.asl
A src/mainboard/lenovo/thinkcentre_m710s/acpi/superio.asl
A src/mainboard/lenovo/thinkcentre_m710s/board_info.txt
A src/mainboard/lenovo/thinkcentre_m710s/bootblock.c
A src/mainboard/lenovo/thinkcentre_m710s/data.vbt
A src/mainboard/lenovo/thinkcentre_m710s/devicetree.cb
A src/mainboard/lenovo/thinkcentre_m710s/dsdt.asl
A src/mainboard/lenovo/thinkcentre_m710s/early_init.c
A src/mainboard/lenovo/thinkcentre_m710s/gma-mainboard.ads
A src/mainboard/lenovo/thinkcentre_m710s/gpio.h
A src/mainboard/lenovo/thinkcentre_m710s/hda_verb.c
A src/mainboard/lenovo/thinkcentre_m710s/romstage.c
15 files changed, 606 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/80343/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/80343?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: I551753aecfbd2c0ee57d85bb22cb943eb21af3cc
Gerrit-Change-Number: 80343
Gerrit-PatchSet: 5
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-MessageType: newpatchset
Attention is currently required from: Keith Hui.
Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79733?usp=email )
Change subject: mb/asus/p8z77-m/hda_verb.c: Use existing defines for NC pins
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/79733?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: I6da02d7bc4c87cc8477d687b238e6e6c9aec62cd
Gerrit-Change-Number: 79733
Gerrit-PatchSet: 2
Gerrit-Owner: Keith Hui <buurin(a)gmail.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Keith Hui <buurin(a)gmail.com>
Gerrit-Comment-Date: Sat, 10 Feb 2024 06:49:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Martin L Roth.
Hello Martin L Roth, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80399?usp=email
to look at the new patch set (#4).
Change subject: Makefile: Drop unused variable originalobjs
......................................................................
Makefile: Drop unused variable originalobjs
This was added in commit 963bed546f (Make: Use unaltered object list for
dependency inclusion) to fix an issue caused by ramstage-postprocess.
The logic for handling dependency inclusion changed in commit db273065f6
(build system: extend src-to-obj for non-.c/.S files), causing the
variable to become unused.
Change-Id: I011ff2070bc31ab9ddf2536873555d0157f91fce
Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com>
---
M Makefile
1 file changed, 0 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/80399/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/80399?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: I011ff2070bc31ab9ddf2536873555d0157f91fce
Gerrit-Change-Number: 80399
Gerrit-PatchSet: 4
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Martin L Roth.
Hello Martin L Roth, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80399?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: Makefile: Drop unused variable originalobjs
......................................................................
Makefile: Drop unused variable originalobjs
This was added in commit 963bed546f (Make: Use unaltered object list for
dependency inclusion) to fix an issue ramstage-postprocess. The logic
for handling dependency inclusion changed in commit db273065f6 (build
system: extend src-to-obj for non-.c/.S files), causing the variable to
become unused.
Change-Id: I011ff2070bc31ab9ddf2536873555d0157f91fce
Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com>
---
M Makefile
1 file changed, 0 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/80399/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/80399?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: I011ff2070bc31ab9ddf2536873555d0157f91fce
Gerrit-Change-Number: 80399
Gerrit-PatchSet: 3
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-MessageType: newpatchset