coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: treewide: Remove unused spinlock functions
by Raul Rangel (Code Review) 08 Nov '21

08 Nov '21

Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59020 ) Change subject: treewide: Remove unused spinlock functions ...................................................................... treewide: Remove unused spinlock functions spin_is_locked and spin_unlock_wait are not used anywhere in the tree. BUG=b:179699789 TEST=Build guybrush Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I3e9a742e6b311d972a260039401bfd8f8766dd36 --- M src/arch/arm/include/armv4/arch/smp/spinlock.h M src/arch/x86/include/arch/smp/spinlock.h M src/include/smp/spinlock.h M src/soc/amd/common/psp_verstage/include/arch/smp/spinlock.h 4 files changed, 0 insertions(+), 19 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/59020/1 diff --git a/src/arch/arm/include/armv4/arch/smp/spinlock.h b/src/arch/arm/include/armv4/arch/smp/spinlock.h index 0a3a4d4..6b680fd 100644 --- a/src/arch/arm/include/armv4/arch/smp/spinlock.h +++ b/src/arch/arm/include/armv4/arch/smp/spinlock.h @@ -4,8 +4,6 @@ #define _ARCH_SMP_SPINLOCK_H #define DECLARE_SPIN_LOCK(x) -#define spin_is_locked(lock) 0 -#define spin_unlock_wait(lock) do {} while (0) #define spin_lock(lock) do {} while (0) #define spin_unlock(lock) do {} while (0) diff --git a/src/arch/x86/include/arch/smp/spinlock.h b/src/arch/x86/include/arch/smp/spinlock.h index 0c06c22..19e9129 100644 --- a/src/arch/x86/include/arch/smp/spinlock.h +++ b/src/arch/x86/include/arch/smp/spinlock.h @@ -22,17 +22,6 @@ #define DECLARE_SPIN_LOCK(x) \ static spinlock_t x = SPIN_LOCK_UNLOCKED; -/* - * Simple spin lock operations. There are two variants, one clears IRQ's - * on the local processor, one does not. - * - * We make no fairness assumptions. They have a cost. - */ -#define barrier() __asm__ __volatile__("" : : : "memory") -#define spin_is_locked(x) (*(volatile char *)(&(x)->lock) <= 0) -#define spin_unlock_wait(x) do { barrier(); } while (spin_is_locked(x)) -#undef barrier - #define spin_lock_string \ "\n1:\t" \ "lock ; decb %0\n\t" \ @@ -74,8 +63,6 @@ #else #define DECLARE_SPIN_LOCK(x) -#define spin_is_locked(lock) 0 -#define spin_unlock_wait(lock) do {} while (0) #define spin_lock(lock) do {} while (0) #define spin_unlock(lock) do {} while (0) diff --git a/src/include/smp/spinlock.h b/src/include/smp/spinlock.h index 8554aa05..15229e5 100644 --- a/src/include/smp/spinlock.h +++ b/src/include/smp/spinlock.h @@ -6,8 +6,6 @@ #else /* !CONFIG_SMP */ #define DECLARE_SPIN_LOCK(x) -#define spin_is_locked(lock) 0 -#define spin_unlock_wait(lock) do {} while (0) #define spin_lock(lock) do {} while (0) #define spin_unlock(lock) do {} while (0) #endif diff --git a/src/soc/amd/common/psp_verstage/include/arch/smp/spinlock.h b/src/soc/amd/common/psp_verstage/include/arch/smp/spinlock.h index 0a3a4d4..6b680fd 100644 --- a/src/soc/amd/common/psp_verstage/include/arch/smp/spinlock.h +++ b/src/soc/amd/common/psp_verstage/include/arch/smp/spinlock.h @@ -4,8 +4,6 @@ #define _ARCH_SMP_SPINLOCK_H #define DECLARE_SPIN_LOCK(x) -#define spin_is_locked(lock) 0 -#define spin_unlock_wait(lock) do {} while (0) #define spin_lock(lock) do {} while (0) #define spin_unlock(lock) do {} while (0) -- To view, visit https://review.coreboot.org/c/coreboot/+/59020 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e9a742e6b311d972a260039401bfd8f8766dd36 Gerrit-Change-Number: 59020 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: [WIP] mb/google,intel: Split chromeos.c files
by build bot (Jenkins) (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Tim Wawrzynczak, Nick Vaccaro. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59018 ) Change subject: [WIP] mb/google,intel: Split chromeos.c files ...................................................................... Patch Set 2: (1 comment) File src/mainboard/intel/kblrvp/bootmode.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-132596): https://review.coreboot.org/c/coreboot/+/59018/comment/fb238ac8_5873983f PS2, Line 21: if (google_chromeec_get_switches() & suspect code indent for conditional statements (16, 16) -- To view, visit https://review.coreboot.org/c/coreboot/+/59018 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib4ccd31edc5ab6c4bc7890a8de1ae270141d18a7 Gerrit-Change-Number: 59018 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Mon, 08 Nov 2021 18:05:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [WIP] mb/google,intel: Split chromeos.c files
by Kyösti Mälkki (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Tim Wawrzynczak, Nick Vaccaro, Kyösti Mälkki. Hello build bot (Jenkins), Tim Wawrzynczak, Nick Vaccaro, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/59018 to look at the new patch set (#2). Change subject: [WIP] mb/google,intel: Split chromeos.c files ...................................................................... [WIP] mb/google,intel: Split chromeos.c files Move all the low-level GPIO support in bootmode.c files and build them for all stages. Keep ChromeOS related ACPI and lbtable support in chromeos.c files and build them only for ramstage. Change-Id: Ib4ccd31edc5ab6c4bc7890a8de1ae270141d18a7 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/mainboard/google/asurada/Makefile.inc A src/mainboard/google/asurada/bootmode.c M src/mainboard/google/asurada/chromeos.c M src/mainboard/google/brya/Makefile.inc A src/mainboard/google/brya/bootmode.c M src/mainboard/google/brya/chromeos.c M src/mainboard/google/cherry/Makefile.inc A src/mainboard/google/cherry/bootmode.c M src/mainboard/google/cherry/chromeos.c M src/mainboard/google/corsola/Makefile.inc R src/mainboard/google/corsola/bootmode.c M src/mainboard/google/daisy/Makefile.inc A src/mainboard/google/daisy/bootmode.c M src/mainboard/google/daisy/chromeos.c M src/mainboard/google/dedede/Makefile.inc A src/mainboard/google/dedede/bootmode.c M src/mainboard/google/dedede/chromeos.c M src/mainboard/google/eve/Makefile.inc A src/mainboard/google/eve/bootmode.c M src/mainboard/google/eve/chromeos.c M src/mainboard/google/fizz/Makefile.inc A src/mainboard/google/fizz/bootmode.c M src/mainboard/google/fizz/chromeos.c M src/mainboard/google/foster/Makefile.inc A src/mainboard/google/foster/bootmode.c M src/mainboard/google/foster/chromeos.c M src/mainboard/google/gale/Makefile.inc A src/mainboard/google/gale/bootmode.c M src/mainboard/google/gale/chromeos.c M src/mainboard/google/glados/Makefile.inc A src/mainboard/google/glados/bootmode.c M src/mainboard/google/glados/chromeos.c M src/mainboard/google/gru/Makefile.inc A src/mainboard/google/gru/bootmode.c M src/mainboard/google/gru/chromeos.c M src/mainboard/google/hatch/Makefile.inc A src/mainboard/google/hatch/bootmode.c M src/mainboard/google/hatch/chromeos.c M src/mainboard/google/kahlee/Makefile.inc A src/mainboard/google/kahlee/bootmode.c M src/mainboard/google/kahlee/chromeos.c M src/mainboard/google/kukui/Makefile.inc A src/mainboard/google/kukui/bootmode.c M src/mainboard/google/kukui/chromeos.c M src/mainboard/google/nyan/Makefile.inc A src/mainboard/google/nyan/bootmode.c M src/mainboard/google/nyan/chromeos.c M src/mainboard/google/nyan_big/Makefile.inc A src/mainboard/google/nyan_big/bootmode.c M src/mainboard/google/nyan_big/chromeos.c M src/mainboard/google/nyan_blaze/Makefile.inc A src/mainboard/google/nyan_blaze/bootmode.c M src/mainboard/google/nyan_blaze/chromeos.c M src/mainboard/google/oak/Makefile.inc A src/mainboard/google/oak/bootmode.c M src/mainboard/google/oak/chromeos.c M src/mainboard/google/octopus/Makefile.inc A src/mainboard/google/octopus/bootmode.c M src/mainboard/google/octopus/chromeos.c M src/mainboard/google/peach_pit/Makefile.inc A src/mainboard/google/peach_pit/bootmode.c M src/mainboard/google/peach_pit/chromeos.c M src/mainboard/google/poppy/Makefile.inc A src/mainboard/google/poppy/bootmode.c M src/mainboard/google/poppy/chromeos.c M src/mainboard/google/rambi/Makefile.inc A src/mainboard/google/rambi/bootmode.c M src/mainboard/google/rambi/chromeos.c M src/mainboard/google/reef/Makefile.inc A src/mainboard/google/reef/bootmode.c M src/mainboard/google/reef/chromeos.c M src/mainboard/google/smaug/Makefile.inc A src/mainboard/google/smaug/bootmode.c M src/mainboard/google/smaug/chromeos.c M src/mainboard/google/storm/Makefile.inc A src/mainboard/google/storm/bootmode.c M src/mainboard/google/storm/chromeos.c M src/mainboard/google/trogdor/Makefile.inc A src/mainboard/google/trogdor/bootmode.c M src/mainboard/google/trogdor/chromeos.c M src/mainboard/google/veyron/Makefile.inc A src/mainboard/google/veyron/bootmode.c M src/mainboard/google/veyron/chromeos.c M src/mainboard/google/veyron_mickey/Makefile.inc A src/mainboard/google/veyron_mickey/bootmode.c M src/mainboard/google/veyron_mickey/chromeos.c M src/mainboard/google/veyron_rialto/Makefile.inc A src/mainboard/google/veyron_rialto/bootmode.c M src/mainboard/google/veyron_rialto/chromeos.c M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/bootmode.c M src/mainboard/google/volteer/chromeos.c M src/mainboard/google/zork/Makefile.inc A src/mainboard/google/zork/bootmode.c M src/mainboard/google/zork/chromeos.c M src/mainboard/intel/adlrvp/Makefile.inc A src/mainboard/intel/adlrvp/bootmode.c M src/mainboard/intel/adlrvp/chromeos.c M src/mainboard/intel/coffeelake_rvp/Makefile.inc A src/mainboard/intel/coffeelake_rvp/bootmode.c M src/mainboard/intel/coffeelake_rvp/chromeos.c M src/mainboard/intel/glkrvp/Makefile.inc A src/mainboard/intel/glkrvp/bootmode.c M src/mainboard/intel/glkrvp/chromeos.c M src/mainboard/intel/icelake_rvp/Makefile.inc A src/mainboard/intel/icelake_rvp/bootmode.c M src/mainboard/intel/icelake_rvp/chromeos.c M src/mainboard/intel/jasperlake_rvp/Makefile.inc A src/mainboard/intel/jasperlake_rvp/bootmode.c M src/mainboard/intel/jasperlake_rvp/chromeos.c M src/mainboard/intel/kblrvp/Makefile.inc A src/mainboard/intel/kblrvp/bootmode.c M src/mainboard/intel/kblrvp/chromeos.c M src/mainboard/intel/kunimitsu/Makefile.inc A src/mainboard/intel/kunimitsu/bootmode.c M src/mainboard/intel/kunimitsu/chromeos.c M src/mainboard/intel/shadowmountain/Makefile.inc A src/mainboard/intel/shadowmountain/bootmode.c M src/mainboard/intel/shadowmountain/chromeos.c M src/mainboard/intel/tglrvp/Makefile.inc A src/mainboard/intel/tglrvp/bootmode.c M src/mainboard/intel/tglrvp/chromeos.c M src/mainboard/intel/wtm2/Makefile.inc A src/mainboard/intel/wtm2/bootmode.c M src/mainboard/intel/wtm2/chromeos.c 125 files changed, 1,015 insertions(+), 849 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/59018/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/59018 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib4ccd31edc5ab6c4bc7890a8de1ae270141d18a7 Gerrit-Change-Number: 59018 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: google/trogdor: Update the power on sequence of ps8640
by Julius Werner (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Douglas Anderson, Xuxin Xiong. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58994 ) Change subject: google/trogdor: Update the power on sequence of ps8640 ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/58994 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia378aafa49ec462c990501ce48721e330d9648b0 Gerrit-Change-Number: 58994 Gerrit-PatchSet: 2 Gerrit-Owner: Xuxin Xiong <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Douglas Anderson <dianders(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Wentao Qin <qinwentao(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Douglas Anderson <dianders(a)chromium.org> Gerrit-Attention: Xuxin Xiong <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Comment-Date: Mon, 08 Nov 2021 18:04:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/psp_verstage: Init TPM on S0i3 resume
by Julius Werner (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Raul Rangel, Andrey Pronin, Kangheui Won, Rob Barnes, Karthik Ramasubramanian. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58870 ) Change subject: soc/amd/psp_verstage: Init TPM on S0i3 resume ...................................................................... Patch Set 7: (2 comments) File src/soc/amd/common/psp_verstage/psp_verstage.c: https://review.coreboot.org/c/coreboot/+/58870/comment/a5e7e129_d039bf53 PS5, Line 230: verstage_mainboard_s0i3_init(); > I think some additional GPIOs(eg. WLAN_AUX_REST_L) are re-initialized in ramstage. […] Sure, but would it hurt to just initialize them the same as the normal boot path? I'd just recommend not creating more special cases than necessary. File src/vendorcode/google/chromeos/Kconfig: https://review.coreboot.org/c/coreboot/+/58870/comment/1a293779_cb481d5e PS3, Line 47: depends on TPM2 && RESUME_PATH_SAME_AS_BOOT > Another option is to broaden RESUME_PATH_SAME_AS_BOOT depends to be ACPI_RESUME || PSP_INIT_TPM_ON_S […] BTW, didn't see this discussion earlier, but as I understand it you're not running through ramstage on resume with this, right? So chromeos/tpm2.c doesn't do anything for you anyway and this question is pretty moot (that's why you had to add another tlcl_disable_platform_hierarchy() in this patch explicitly instead). -- To view, visit https://review.coreboot.org/c/coreboot/+/58870 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie511928da6a8b4be62621fd2c4c31a8d1e724d48 Gerrit-Change-Number: 58870 Gerrit-PatchSet: 7 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Andrey Pronin <apronin(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Andrey Pronin <apronin(a)chromium.org> Gerrit-Attention: Kangheui Won <khwon(a)chromium.org> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Attention: Andrey Pronin <apronin(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Mon, 08 Nov 2021 17:57:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Rob Barnes <robbarnes(a)google.com> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush: Define ACPI Power Resources for FPMCU
by Raul Rangel (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Rob Barnes, Karthik Ramasubramanian, Felix Held. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58705 ) Change subject: mb/google/guybrush: Define ACPI Power Resources for FPMCU ...................................................................... Patch Set 4: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/58705/comment/37e714c0_e8532a48 PS4, Line 18: TEST Only other test I think we need is to update the FPMCU FW. I thought the script unbound the driver, so not sure if the _OFF method will be called in that case. File src/mainboard/google/guybrush/variants/guybrush/overridetree.cb: https://review.coreboot.org/c/coreboot/+/58705/comment/6c3a3e03_9467d2d9 PS4, Line 178: enable_delay_ms > We are actually configuring the enable GPIO to TX state '0' (i.e. disable) in ramstage. […] Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/58705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I52322eaecf6961ff9a196ca9ab2d58b7d4599d4f Gerrit-Change-Number: 58705 Gerrit-PatchSet: 4 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Bhanu Prakash Maiya <bhanumaiya(a)google.com> Gerrit-CC: Craig Hesling <hesling(a)chromium.org> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 08 Nov 2021 17:53:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/herobrine: Initialize USB by calling SOC method
by Julius Werner (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Ravi kumar, Paul Menzel, mturney mturney. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56093 ) Change subject: mb/google/herobrine: Initialize USB by calling SOC method ...................................................................... Patch Set 34: (2 comments) File src/mainboard/google/herobrine/mainboard.c: https://review.coreboot.org/c/coreboot/+/56093/comment/a88b390e_441e6490 PS34, Line 19: void mainboard_blob_fix(void); What is this and what does it have to do with this patch? https://review.coreboot.org/c/coreboot/+/56093/comment/ad57c000_cd6c95fe PS34, Line 23: usb0_board_data If the SNPS doesn't use any board_data, just pass NULL (and don't define a struct for it that is never used). -- To view, visit https://review.coreboot.org/c/coreboot/+/56093 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic378352a97e4f3ed89089f1f7545f8ebb172b1f2 Gerrit-Change-Number: 56093 Gerrit-PatchSet: 34 Gerrit-Owner: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Sandeep Maheswaram <sanm(a)codeaurora.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com> Gerrit-Attention: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: mturney mturney <mturney(a)codeaurora.org> Gerrit-Comment-Date: Mon, 08 Nov 2021 17:50:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: sc7280: Add CPUCP firmware support
by Julius Werner (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Ravi kumar. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49768 ) Change subject: sc7280: Add CPUCP firmware support ...................................................................... Patch Set 86: (1 comment) File src/soc/qualcomm/sc7280/include/soc/cpucp.h: https://review.coreboot.org/c/coreboot/+/49768/comment/1f27b7bb_dd06e960 PS86, Line 6: #define EPSSTOP_EPSS_TOP (void *)0x18598000 These should go into <soc/addressmap.h> (and we don't usually put the (void *) cast in the macro). -- To view, visit https://review.coreboot.org/c/coreboot/+/49768 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idac22c8cb231658616999bc577bdf49f9aa7ae74 Gerrit-Change-Number: 49768 Gerrit-PatchSet: 86 Gerrit-Owner: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com> Gerrit-Attention: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Comment-Date: Mon, 08 Nov 2021 17:48:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush: Define ACPI Power Resources for FPMCU
by Karthik Ramasubramanian (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Raul Rangel, Rob Barnes, Felix Held. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58705 ) Change subject: mb/google/guybrush: Define ACPI Power Resources for FPMCU ...................................................................... Patch Set 4: (1 comment) File src/mainboard/google/guybrush/variants/guybrush/overridetree.cb: https://review.coreboot.org/c/coreboot/+/58705/comment/5d99e02d_6117bf39 PS4, Line 178: enable_delay_ms > Actually, do we also want enable_off_delay_ms = 200? We are actually configuring the enable GPIO to TX state '0' (i.e. disable) in ramstage. The GPIO gets set to enabled state when the control jumps to OS. That will help to meet the enable_off_delay_ms requirement. I did this because _OFF method does not get invoked during shutdown or warm reset. -- To view, visit https://review.coreboot.org/c/coreboot/+/58705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I52322eaecf6961ff9a196ca9ab2d58b7d4599d4f Gerrit-Change-Number: 58705 Gerrit-PatchSet: 4 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Bhanu Prakash Maiya <bhanumaiya(a)google.com> Gerrit-CC: Craig Hesling <hesling(a)chromium.org> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 08 Nov 2021 17:47:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/qualcomm/common/usb: Add support for common USB driver
by Julius Werner (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Ravi kumar, Sandeep Maheswaram, Paul Menzel, mturney mturney. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56091 ) Change subject: soc/qualcomm/common/usb: Add support for common USB driver ...................................................................... Patch Set 34: (8 comments) File src/soc/qualcomm/common/Kconfig: https://review.coreboot.org/c/coreboot/+/56091/comment/71b923ab_32600b06 PS34, Line 15: endif Everything you're adding should be within this if-endif block. https://review.coreboot.org/c/coreboot/+/56091/comment/78e160be_ff7d150d PS34, Line 31: default n If you follow my other comments I don't think you'll need any Kconfigs for this. Just having the SoC Makefile decide which file to compile in should be enough. File src/soc/qualcomm/common/include/soc/usb/snps_usb_phy.h: https://review.coreboot.org/c/coreboot/+/56091/comment/943b16ad_7754384a PS34, Line 41: #define QC_GENMASK(h, l) (BIT(h + 1) - BIT(l)) Use the existing GENMASK() macro from helpers.h rather than rolling your own. (I'm pretty sure I already mentioned this on another CL, please fix this globally across the stack so we don't have to ask for it again on every patch.) https://review.coreboot.org/c/coreboot/+/56091/comment/e51d536a_eb4807a3 PS34, Line 43: #define SLEEPM BIT(0) Please don't define constants with no namespacing like this in global headers. Either move these to a .c file (or a private header that's only included by the specific .c file(s) belonging to this driver), or prefix these with SNPS_USB_ or the like. File src/soc/qualcomm/common/include/soc/usb/usb_common.h: https://review.coreboot.org/c/coreboot/+/56091/comment/f2b680cf_ccad0757 PS34, Line 3: #if CONFIG(QUSB_PHY) Please don't conditionally include things like this, it easily becomes confusing. You should just declare the prototype for hs_usb_phy_init() right in this header, then you shouldn't need to include either of these here. For the board_data, you can either use a union or you can just make it void * and then cast it to the respective type inside the function (although it doesn't look like you're really using the board_data for the snps anyway?). https://review.coreboot.org/c/coreboot/+/56091/comment/5eafc299_92bcb591 PS34, Line 39: * USB BASE ADDRESSES What is this comment supposed to mean? File src/soc/qualcomm/common/usb/qmp_usb_phy.c: https://review.coreboot.org/c/coreboot/+/56091/comment/3b102a24_953db2d9 PS34, Line 7: #if CONFIG(QMPV4_USB_PHY) Please factor these out into separate files (e.g. a qmpv3_usb_phy.c and a qmpv4_usb_phy.c). It's better to duplicate a few lines than make the code this hard to follow through preprocessor guards. File src/soc/qualcomm/common/usb/snps_usb_phy.c: https://review.coreboot.org/c/coreboot/+/56091/comment/9e2f04de_41a3c410 PS34, Line 8: static void qcom_snps_hsphy_write_mask(u32 *offset, u32 mask, u32 val) We already have clrsetbits32() for this. (But generally, you shouldn't read-modify-write every single register on a peripheral that was just reset to power-on default values anyway, that's just a waste of time. Unless you really *need* to preserve the other bits that were set in there already, and you're in a situation where you really can't know at compie-time what those bits would be set to, just use write32().) -- To view, visit https://review.coreboot.org/c/coreboot/+/56091 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1013ded22855286220cfa747cb25418070fe85a7 Gerrit-Change-Number: 56091 Gerrit-PatchSet: 34 Gerrit-Owner: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Sandeep Maheswaram <sanm(a)codeaurora.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com> Gerrit-Attention: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Attention: Sandeep Maheswaram <sanm(a)codeaurora.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: mturney mturney <mturney(a)codeaurora.org> Gerrit-Comment-Date: Mon, 08 Nov 2021 17:45:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2021