Attention is currently required from: Felix Singer, Michał Żygowski, Tim Wawrzynczak, Michał Kopeć, Angel Pons, Arthur Heymans.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68791 )
Change subject: ec/clevo/it5570e: add driver for EC used on various Clevo laptops
......................................................................
Patch Set 7:
(9 comments)
File src/ec/clevo/it5570e/commands.c:
https://review.coreboot.org/c/coreboot/+/68791/comment/e3df0c50_b3f10846
PS4, Line 41: while (recv_ec_data() != '$');
> Looks like CB:69077 is fine :O Personally, I still prefer the variant without `do {}`. […]
well, at least I tried... the linter checks regarding if/do/while are pretty complex, so I went your way ;)
https://review.coreboot.org/c/coreboot/+/68791/comment/a457d3df_22949adf
PS4, Line 119: else if (start > 100 || stop > 100) {
> Hmmm, let's find a loophole in the coding style. How about: […]
Done
https://review.coreboot.org/c/coreboot/+/68791/comment/d3c099d0_3fbaea4b
PS4, Line 122: else if (start >= stop) {
> See above
Done
File src/ec/clevo/it5570e/commands.c:
https://review.coreboot.org/c/coreboot/+/68791/comment/0eaffcf6_b2731e92
PS6, Line 120: printk(BIOS_ERR, "EC: invalid flexicharger settings: start/stop > 100%%\n");
> ouch, the plan was to just return bc I'm not a fan of trying to guess what the user intended. […]
Done
File src/ec/clevo/it5570e/ec.c:
https://review.coreboot.org/c/coreboot/+/68791/comment/02a640fe_2c3086fd
PS4, Line 63: version = ec_read_fw_version();
> uhm, the data is read during runtime,
done as discussed; was just a misunderstanding on my side
https://review.coreboot.org/c/coreboot/+/68791/comment/401ea0a4_a1852f23
PS4, Line 94: power_unit = 1 << (msr_read(MSR_PKG_POWER_SKU_UNIT) & 0xf);
> I don't think so. This is being read at runtime. […]
done as discussed
https://review.coreboot.org/c/coreboot/+/68791/comment/8abee0c7_8c3109ac
PS4, Line 124: if (dev->path.type == DEVICE_PATH_GENERIC && dev->path.generic.id == 0) {
> > braces {} are not necessary for any arm of this statement […]
Done
File src/ec/clevo/it5570e/ssdt.c:
https://review.coreboot.org/c/coreboot/+/68791/comment/f835d8c6_437dc2d6
PS4, Line 45: (float)
> Test: […]
-> done without fp
https://review.coreboot.org/c/coreboot/+/68791/comment/06e0a1df_139970c0
PS4, Line 140: for (int i = 0; i < fan_cnt; i++) {
> not necessary but also not forbidden. […]
heh, I just realized that it's already mixed -> done
--
To view, visit https://review.coreboot.org/c/coreboot/+/68791
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic8c0bee9002ad9edcd10c83b775fc723744caaa0
Gerrit-Change-Number: 68791
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Wed, 02 Nov 2022 16:49:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Venkat Thogaru, Sudheer Amrabadi.
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67673 )
Change subject: soc/qualcomm/sc7280: Move AOP load and reset handle to Romstage
......................................................................
Patch Set 18:
(1 comment)
Patchset:
PS18:
Hi Sudheer, just wondering if you figured out what sections in the memlayout are hardcoded to the address (which one was causing the exception before)?
--
To view, visit https://review.coreboot.org/c/coreboot/+/67673
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iabc8ee8f6e7b14d237b0aeaae42da8077f9dafc4
Gerrit-Change-Number: 67673
Gerrit-PatchSet: 18
Gerrit-Owner: Sudheer Amrabadi <samrabad(a)codeaurora.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: T.Michael Turney <tmiketurney(a)gmail.com>
Gerrit-Attention: Venkat Thogaru <thogaru(a)qualcomm.corp-partner.google.com>
Gerrit-Attention: Sudheer Amrabadi <samrabad(a)codeaurora.org>
Gerrit-Comment-Date: Wed, 02 Nov 2022 16:43:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Martin L Roth, Felix Held.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69165 )
Change subject: commonlib: Fix AMD MP2 BUFFER id
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/69165
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iae6213ac99bc5c64fd5dcd681c7922eafa011fc0
Gerrit-Change-Number: 69165
Gerrit-PatchSet: 1
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 02 Nov 2022 16:36:45 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jeremy Soller, Paul Menzel, Angel Pons.
Tim Crawford has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/65301 )
Change subject: mb/system76/adl-p: Add Darter Pro 8
......................................................................
Patch Set 15:
(2 comments)
File src/mainboard/system76/adl-p/Kconfig:
https://review.coreboot.org/c/coreboot/+/65301/comment/f9fd634d_f3935b68
PS14, Line 25: TPM_MEASURED_BOOT
> If it's not strictly required, this would provide more flexibility for people building their own cor […]
Done
File src/mainboard/system76/adl-p/ramstage.c:
https://review.coreboot.org/c/coreboot/+/65301/comment/bc4aa4e4_54a162f3
PS14, Line 25: params->SataPortsSolidStateDrive[1] = 1;
> What does this do?
It might not actually do anything.
The description implies it's to declare that a SATA port is an SSD and not an HDD. The actual code just sets some extra bits in the PCI config related to thermal throttling, but only if `SataThermalSuggestedSetting=1`.
--
To view, visit https://review.coreboot.org/c/coreboot/+/65301
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icc84d6cc3aec7149d9b538305288bbe2b56d53e4
Gerrit-Change-Number: 65301
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Wed, 02 Nov 2022 16:29:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Crawford <tcrawford(a)system76.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Michał Żygowski, Tim Wawrzynczak, Michał Kopeć, Arthur Heymans, Michael Niewöhner.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68791 )
Change subject: ec/clevo/it5570e: add driver for EC used on various Clevo laptops
......................................................................
Patch Set 6:
(2 comments)
File src/ec/clevo/it5570e/ec.c:
https://review.coreboot.org/c/coreboot/+/68791/comment/29782a89_de1dd548
PS4, Line 63: version = ec_read_fw_version();
> uhm, the data is read during runtime,
it still works, because the data returned by the function is effectively read-only, and the pointer address is not reassigned.
File src/ec/clevo/it5570e/ssdt.c:
https://review.coreboot.org/c/coreboot/+/68791/comment/dd4d1c70_bb6b37e8
PS4, Line 61: 0x38c
> well, it's a offset where the region (not just a field) is located. […]
Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/68791
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic8c0bee9002ad9edcd10c83b775fc723744caaa0
Gerrit-Change-Number: 68791
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Wed, 02 Nov 2022 16:29:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Kevin Chiu, Tarun Tuli, Ricky Chang.
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68916 )
Change subject: mb/google/brya/var/lisbon: turn off thunderbolt
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/68916
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iac44315d000c3c0c572efb00e877d039e0308455
Gerrit-Change-Number: 68916
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Chiu <kevin.chiu.17802(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Ricky Chang <rickytlchang(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Attention: Kevin Chiu <kevin.chiu.17802(a)gmail.com>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Ricky Chang <rickytlchang(a)google.com>
Gerrit-Comment-Date: Wed, 02 Nov 2022 16:26:22 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Nick Vaccaro has submitted this change. ( https://review.coreboot.org/c/coreboot/+/68570 )
Change subject: mb/google/brya/var/gladios: use RPL FSP headers
......................................................................
mb/google/brya/var/gladios: use RPL FSP headers
To support an RPL SKU on gladios, gladios must use the FSP for RPL.
Select SOC_INTEL_RAPTORLAKE for gladios so that it will use the RPL
FSP headers for gladios.
BUG=b:239513596
BRANCH=None
TEST=FW_NAME=gladios emerge-brask intel-rplfsp
coreboot-private-files-baseboard-brya coreboot chromeos-bootimage
Signed-off-by: Kevin Chiu <kevin.chiu.17802(a)gmail.com>
Change-Id: Ic30f7fe30eb0a3151cdf46fff609819056b2fbfe
Reviewed-on: https://review.coreboot.org/c/coreboot/+/68570
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Nick Vaccaro <nvaccaro(a)google.com>
---
M src/mainboard/google/brya/Kconfig.name
1 file changed, 24 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Nick Vaccaro: Looks good to me, approved
diff --git a/src/mainboard/google/brya/Kconfig.name b/src/mainboard/google/brya/Kconfig.name
index 8c5af45..7875986 100644
--- a/src/mainboard/google/brya/Kconfig.name
+++ b/src/mainboard/google/brya/Kconfig.name
@@ -286,3 +286,4 @@
config BOARD_GOOGLE_GLADIOS
bool "-> Gladios"
select BOARD_GOOGLE_BASEBOARD_BRASK
+ select SOC_INTEL_RAPTORLAKE
--
To view, visit https://review.coreboot.org/c/coreboot/+/68570
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic30f7fe30eb0a3151cdf46fff609819056b2fbfe
Gerrit-Change-Number: 68570
Gerrit-PatchSet: 3
Gerrit-Owner: Kevin Chiu <kevin.chiu.17802(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Ricky Chang <rickytlchang(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Tim Crawford, Jeremy Soller, Paul Menzel.
Hello build bot (Jenkins), Jeremy Soller, Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/65301
to look at the new patch set (#15).
Change subject: mb/system76/adl-p: Add Darter Pro 8
......................................................................
mb/system76/adl-p: Add Darter Pro 8
The Darter Pro 8 (darp8) is an Alder Lake-P board.
Tested with a custom TianoCore UefiPayloadPkg.
Working:
- PS/2 keyboard, touchpad
- Both DIMM slots (with NMSO480E82-3200EA00)
- M.2 NVMe SSD (with MZVL2500HCJQ)
- M.2 SATA SSD (with WDS100T2B0B)
- All USB ports
- SD card reader
- Webcam
- Ethernet
- WiFi/Bluetooth
- Integrated graphics using Intel GOP driver
- Backlight controls on Windows 10 and Linux 6.1
- HDMI output
- DisplayPort output over USB-C
- Internal microphone
- Internal speakers
- Combined header + mic 3.5mm audio
- S0ix suspend/resume
- Booting Pop!_OS Linux 22.04 with kernel 5.18.5
- Internal flashing with flashrom v1.2-703-g76118a7c10ed
Not working:
- Detection of devices in TBT slot on boot
Change-Id: Icc84d6cc3aec7149d9b538305288bbe2b56d53e4
Signed-off-by: Tim Crawford <tcrawford(a)system76.com>
---
M Documentation/mainboard/index.md
A Documentation/mainboard/system76/darp8.md
A src/mainboard/system76/adl-p/Kconfig
A src/mainboard/system76/adl-p/Kconfig.name
A src/mainboard/system76/adl-p/Makefile.inc
A src/mainboard/system76/adl-p/acpi/backlight.asl
A src/mainboard/system76/adl-p/acpi/mainboard.asl
A src/mainboard/system76/adl-p/acpi/sleep.asl
A src/mainboard/system76/adl-p/board_info.txt
A src/mainboard/system76/adl-p/bootblock.c
A src/mainboard/system76/adl-p/cmos.default
A src/mainboard/system76/adl-p/cmos.layout
A src/mainboard/system76/adl-p/devicetree.cb
A src/mainboard/system76/adl-p/dsdt.asl
A src/mainboard/system76/adl-p/include/mainboard/gpio.h
A src/mainboard/system76/adl-p/ramstage.c
A src/mainboard/system76/adl-p/variants/darp8/board_info.txt
A src/mainboard/system76/adl-p/variants/darp8/data.vbt
A src/mainboard/system76/adl-p/variants/darp8/gpio.c
A src/mainboard/system76/adl-p/variants/darp8/gpio_early.c
A src/mainboard/system76/adl-p/variants/darp8/hda_verb.c
A src/mainboard/system76/adl-p/variants/darp8/overridetree.cb
A src/mainboard/system76/adl-p/variants/darp8/romstage.c
23 files changed, 984 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/65301/15
--
To view, visit https://review.coreboot.org/c/coreboot/+/65301
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icc84d6cc3aec7149d9b538305288bbe2b56d53e4
Gerrit-Change-Number: 65301
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Martin L Roth, Felix Held.
Fred Reitberger has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69165 )
Change subject: commonlib: Fix AMD MP2 BUFFER id
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/69165
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iae6213ac99bc5c64fd5dcd681c7922eafa011fc0
Gerrit-Change-Number: 69165
Gerrit-PatchSet: 1
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 02 Nov 2022 16:04:26 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment