Attention is currently required from: Jason Glenesk, Marshall Dawson, Felix Held.
Fred Reitberger has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64011 )
Change subject: soc/amd/common/block/psp/psp_gen2: move CORE_2_PSP_MSG_38 defines
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/64011
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I67cc2ff63d1c0322b514521975f3ce0f9b1cf5b1
Gerrit-Change-Number: 64011
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 03 May 2022 12:01:39 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Martin L Roth, Maximilian Brune.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64032 )
Change subject: Allow trailing whitespaces in .md files
......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/64032/comment/b7a2cd14_1bbc2eba
PS1, Line 10: mardown
Markdown
https://review.coreboot.org/c/coreboot/+/64032/comment/bb994cd9_a30e8866
PS1, Line 10: a new line
Only two trailing spaces denote a new line, don’t they?
https://review.coreboot.org/c/coreboot/+/64032/comment/731ec45c_5f543584
PS1, Line 9: since trailing whitesspaces have an actual meaning
: in mardown files (a new line), I would allow it
Please start sentences with capital letter, and add with punctuation mark like dot/period.
--
To view, visit https://review.coreboot.org/c/coreboot/+/64032
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibdb92ee857ee4ad32b6afb84ace427b27b41bb7c
Gerrit-Change-Number: 64032
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Martin L Roth <martinroth(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Martin L Roth <martinroth(a)google.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Comment-Date: Tue, 03 May 2022 11:46:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Maximilian Brune has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/64032 )
Change subject: Allow trailing whitespaces in .md files
......................................................................
Allow trailing whitespaces in .md files
since trailing whitesspaces have an actual meaning
in mardown files (a new line), I would allow it
Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com>
Change-Id: Ibdb92ee857ee4ad32b6afb84ace427b27b41bb7c
---
M util/lint/lint-stable-003-whitespace
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/64032/1
diff --git a/util/lint/lint-stable-003-whitespace b/util/lint/lint-stable-003-whitespace
index 6f9d788..9a6db00 100755
--- a/util/lint/lint-stable-003-whitespace
+++ b/util/lint/lint-stable-003-whitespace
@@ -5,7 +5,7 @@
# DESCR: Check for superfluous whitespace in the tree
LC_ALL=C export LC_ALL
-EXCLUDELIST='^src/vendorcode/|^util/kconfig/|^util/nvidia/cbootimage$|COPYING|LICENSE|README|_shipped$|\.patch$|\.bin$|\.hex$|\.jpg$|\.gif$|\.ttf$|\.woff$|\.png$|\.eot$|\.vbt$|\.ico$'
+EXCLUDELIST='^src/vendorcode/|^util/kconfig/|^util/nvidia/cbootimage$|COPYING|LICENSE|README|_shipped$|\.patch$|\.bin$|\.hex$|\.jpg$|\.gif$|\.ttf$|\.woff$|\.png$|\.eot$|\.vbt$|\.ico$|\.md$'
INCLUDELIST="src util payloads Makefile* toolchain.inc tests"
# shellcheck disable=SC2086,SC2046
--
To view, visit https://review.coreboot.org/c/coreboot/+/64032
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibdb92ee857ee4ad32b6afb84ace427b27b41bb7c
Gerrit-Change-Number: 64032
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-MessageType: newchange
Attention is currently required from: Kangheui Won, Paul Menzel, Tim Wawrzynczak, Reka Norman, Nick Vaccaro.
Hello build bot (Jenkins), Kangheui Won, Reka Norman, Tim Wawrzynczak, Nick Vaccaro,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/63893
to look at the new patch set (#8).
Change subject: mb/google/brya/var/craask: Disable LTE-related GPIOs based on fw_config
......................................................................
mb/google/brya/var/craask: Disable LTE-related GPIOs based on fw_config
If the LTE USB DB is not connected, disable the LTE-related GPIOs.
BUG=b:229938024, b:229048361
TEST=emerge-nissa coreboot
Signed-off-by: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Change-Id: I37719cee48370a04534067aa64a3aa77e453948a
---
M src/mainboard/google/brya/variants/craask/Makefile.inc
M src/mainboard/google/brya/variants/craask/gpio.c
2 files changed, 29 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/63893/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/63893
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I37719cee48370a04534067aa64a3aa77e453948a
Gerrit-Change-Number: 63893
Gerrit-PatchSet: 8
Gerrit-Owner: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Cathy Chen <cathy_chen(a)quanta.corp-partner.google.com>
Gerrit-CC: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Attention: Kangheui Won <khwon(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Kangheui Won, Paul Menzel, Tim Wawrzynczak, Reka Norman, Nick Vaccaro.
Tyler Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63893 )
Change subject: mb/google/brya/var/craask: Disable LTE-related GPIOs based on fw_config
......................................................................
Patch Set 7:
(1 comment)
File src/mainboard/google/brya/variants/craask/fw_config.c:
https://review.coreboot.org/c/coreboot/+/63893/comment/02ac1f3e_40269622
PS7, Line 23: if (!fw_config_probe(FW_CONFIG(DB_USB, DB_1C_LTE))) {
: printk(BIOS_DEBUG, "Disable LTE-related GPIO pins.\n");
: gpio_configure_pads(lte_disable_pads, ARRAY_SIZE(lte_disable_pads));
: }
:
> This approach leavess a gpio interrupt line (GPP_H19) floating for a short time. […]
Hi Nick,
I have 2 questions would like to discuss with you:
1. This approach leavess a gpio interrupt line (GPP_H19) floating for a short time.
>> We had set WWAN_RST_L to low and WWAN_EN to high in bootblock stage, but we didn't set interrupt pin. Is it the root cause of GPP_H19 be floating in a short time?
2. Override GPIO setting in gpio.c
>> Reference to Nivviks, seems like we don't need to override GPIO in ramstage for enable LTE module.
I will move disable LTE gpio settings to gpio.c as well.
Thanks!
--
To view, visit https://review.coreboot.org/c/coreboot/+/63893
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I37719cee48370a04534067aa64a3aa77e453948a
Gerrit-Change-Number: 63893
Gerrit-PatchSet: 7
Gerrit-Owner: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Cathy Chen <cathy_chen(a)quanta.corp-partner.google.com>
Gerrit-CC: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Attention: Kangheui Won <khwon(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Comment-Date: Tue, 03 May 2022 11:02:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-MessageType: comment
Terry Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/64027 )
Change subject: mb/google/brya/var/crota: setting for codec reset pin
......................................................................
mb/google/brya/var/crota: setting for codec reset pin
The audio codec of Crota360 used is Cirrus CS42L42, different from Brya (Realtek ALC5682I)
There is no Reset pin on ALC5682 but CS42L42 is needed.
So we use GPP_B15 for the reset pin of CS42L42 controlled that make the power sequence well.
BUG=b:230074351
BRANCH=none
TEST=build coreboot without error
Signed-off-by: Terry Chen <terry_chen(a)wistron.corp-partner.google.com>
Change-Id: Ie942b3c553823510dfa6f6fb70a7b13881fc4c14
---
M src/mainboard/google/brya/variants/crota/gpio.c
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/64027/1
diff --git a/src/mainboard/google/brya/variants/crota/gpio.c b/src/mainboard/google/brya/variants/crota/gpio.c
index ee8ee19..881f78d 100644
--- a/src/mainboard/google/brya/variants/crota/gpio.c
+++ b/src/mainboard/google/brya/variants/crota/gpio.c
@@ -22,8 +22,8 @@
PAD_NC(GPP_B2, NONE),
/* B3 : PROC_GP2 ==> NC */
PAD_NC(GPP_B3, NONE),
- /* B15 : TIME_SYNC0 ==> NC */
- PAD_NC(GPP_B15, NONE),
+ /* B15 : PROC_GP3 ==> AUD_RST_L */
+ PAD_CFG_GPO(GPP_B15, 1, PWROK),
/* C3 : GPP_C3 ==> SML0_SMBCLK */
PAD_CFG_NF(GPP_C3, NONE, DEEP, NF1),
--
To view, visit https://review.coreboot.org/c/coreboot/+/64027
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie942b3c553823510dfa6f6fb70a7b13881fc4c14
Gerrit-Change-Number: 64027
Gerrit-PatchSet: 1
Gerrit-Owner: Terry Chen <terry_chen(a)wistron.corp-partner.google.com>
Gerrit-MessageType: newchange
Attention is currently required from: Tim Wawrzynczak.
Hello build bot (Jenkins), Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/64019
to look at the new patch set (#5).
Change subject: mb/google/brya/variants/crota: Enable Bluetooth offload support
......................................................................
mb/google/brya/variants/crota: Enable Bluetooth offload support
Enable CnviBtAudioOffload UPD from Intel Guideline
BUG=b:230418589
TEST=emerge-byra coreboot and verified pass
Signed-off-by: Terry Chen <terry_chen(a)wistron.corp-partner.google.com>
Change-Id: I7ac54156cc4a8d824ed1c549d66fc369698a352c
---
M src/mainboard/google/brya/variants/crota/Makefile.inc
M src/mainboard/google/brya/variants/crota/gpio.c
A src/mainboard/google/brya/variants/crota/variant.c
3 files changed, 20 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/64019/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/64019
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7ac54156cc4a8d824ed1c549d66fc369698a352c
Gerrit-Change-Number: 64019
Gerrit-PatchSet: 5
Gerrit-Owner: Terry Chen <terry_chen(a)wistron.corp-partner.google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Patrick Rudolph, Paul Menzel, Jingle Hsu, Nill Ge, Marc Jones, Nico Huber, Subrata Banik, Johnny Lin, lichenchen.carl, Tim Chu, Shelly Chang.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63882 )
Change subject: arch/x86/smbios.c: Add SMBIOS type 17 for empty DIMM slots
......................................................................
Patch Set 8:
(2 comments)
Patchset:
PS8:
> But the new 'bool present' hasn't been set by all the platform codes yet, without Kconfig it seems p […]
You could invert `bool present` to `bool empty` or similar, so that the default value doesn't change the original behavior. I guess one could also consider meminfo DIMM entries with a capacity of 0 MB to be empty slots.
If one has access to SPD address info in coreboot, it's possible to know which slots are populated. For example, Haswell mainboards define the `mb_get_spd_map()` function so that northbridge code knows which slots are populated. Haswell supports up to 4 DIMM slots, but mb/asrock/h81m-hds only has 2 DIMM slots, so it only specifies the SPD addresses for 2 slots:
void mb_get_spd_map(struct spd_info *spdi)
{
spdi->addresses[0] = 0x50;
spdi->addresses[2] = 0x52;
}
In contrast, mb/asrock/b85m_pro4 has 4 DIMM slots:
void mb_get_spd_map(struct spd_info *spdi)
{
spdi->addresses[0] = 0x50;
spdi->addresses[1] = 0x51;
spdi->addresses[2] = 0x52;
spdi->addresses[3] = 0x53;
}
So the code that fills in meminfo could use this info to know which slots exist on a mainboard.
However, I don't see something like this on xeon_sp. How does FSP know which DIMM slots are implemented? Or does FSP just probe all possible SPD addresses? If this is the case, I guess one would need to have a mainboard-defined function for xeon_sp that provides a map of implemented DIMM slots, so that xeon_sp code skips generating meminfo entries for inexistent DIMM slots.
File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/63882/comment/d572593e_925aa365
PS2, Line 819: SOC_DIMM_MAX
> For memory-down you might generate type 17 as well. In that case form factor would be "other", "row of chips" or "die".
I understand that type 17 entries should be generated for memory-down as well, but what about half-populated configs? i.e. the board is designed with 2 channels of memory-down, but some SKUs only use one channel.
--
To view, visit https://review.coreboot.org/c/coreboot/+/63882
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia3ff1353667f9d3bd302db42c739b7be9d3f4a32
Gerrit-Change-Number: 63882
Gerrit-PatchSet: 8
Gerrit-Owner: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Reviewer: Jingle Hsu <jingle_hsu(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Reviewer: Nill Ge <geshijian(a)bytedance.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Shelly Chang <Shelly_Chang(a)wiwynn.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: lichenchen.carl <lichenchen.carl(a)bytedance.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Patrick Rudolph
Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jingle Hsu <jingle_hsu(a)wiwynn.com>
Gerrit-Attention: Nill Ge <geshijian(a)bytedance.com>
Gerrit-Attention: Patrick Rudolph
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: lichenchen.carl <lichenchen.carl(a)bytedance.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Attention: Shelly Chang <Shelly_Chang(a)wiwynn.com>
Gerrit-Comment-Date: Tue, 03 May 2022 10:04:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Patrick Rudolph
Comment-In-Reply-To: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: comment