coreboot-gerrit May 2022

coreboot-gerrit@coreboot.org

1 participants
6166 discussions

Change in coreboot[master]: soc/mediatek/mt8186: Prevent early USB wakeup
by Felix Held (Code Review) 02 May '22

02 May '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63858 ) Change subject: soc/mediatek/mt8186: Prevent early USB wakeup ...................................................................... soc/mediatek/mt8186: Prevent early USB wakeup The MT8186 platform fails to suspend due to premature wakeup by USB. In MT8186, we use low level latch to keep USB wakeup signal. However, hardware could latch a wrong signal if it debounces more than one time. As a result, it would enable wakeup function too early. To prevent this issue, we do the following modification: - Delay about 100 us to enable wakeup function in kernel drivers [1]. - To guarantee 100 us is enough, we need to disable the USB debounce by default in coreboot. According to section register 0x404 and 0x420 in "(CODA) MT8169_PERICFG_REG.xls" which is only for MediaTek internal use: The current default value of debounce register for MT8186 USB IP0 and IP1 is incorrect. The reason we add in coreboot is that the default value should be correct when SoC is booting up. This modification is only for MT8186. The subsequent SoCs will adjust the wakeup function to correct register value by default. [1]: 0d8cfeeef3f5 (usb: xhci-mtk: fix random remote wakeup) TEST=after stress test, not found premature wakeup by USB BUG=b:228773975 Signed-off-by: Rex-BC Chen <rex-bc.chen(a)mediatek.com> Change-Id: I296c4491c5959670a39fa8bd6ef987557bbc459f Reviewed-on: https://review.coreboot.org/c/coreboot/+/63858 Reviewed-by: Yu-Ping Wu <yupingso(a)google.com> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> Reviewed-by: Hung-Te Lin <hungte(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/mediatek/mt8186/include/soc/addressmap.h M src/soc/mediatek/mt8186/usb.c 2 files changed, 9 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Hung-Te Lin: Looks good to me, but someone else must approve Yu-Ping Wu: Looks good to me, approved Rex-BC Chen: Looks good to me, but someone else must approve diff --git a/src/soc/mediatek/mt8186/include/soc/addressmap.h b/src/soc/mediatek/mt8186/include/soc/addressmap.h index 069e31d..0aaeb9b 100644 --- a/src/soc/mediatek/mt8186/include/soc/addressmap.h +++ b/src/soc/mediatek/mt8186/include/soc/addressmap.h @@ -21,6 +21,7 @@ IOCFG_BL_BASE = IO_PHYS + 0x00002600, IOCFG_RB_BASE = IO_PHYS + 0x00002A00, IOCFG_RT_BASE = IO_PHYS + 0x00002C00, + PERICFG_BASE = IO_PHYS + 0x00003000, GPIO_BASE = IO_PHYS + 0x00005000, SPM_BASE = IO_PHYS + 0x00006000, RGU_BASE = IO_PHYS + 0x00007000, diff --git a/src/soc/mediatek/mt8186/usb.c b/src/soc/mediatek/mt8186/usb.c index 6d83abb..d133555 100644 --- a/src/soc/mediatek/mt8186/usb.c +++ b/src/soc/mediatek/mt8186/usb.c @@ -10,7 +10,15 @@ #include <soc/gpio.h> #include <soc/usb.h> +#define PERI_USB_WAKEUP_DEC_CON1 0x404 +#define PERI_U3_WAKE_CTRL0 0x420 + void mtk_usb_prepare(void) { gpio_output(GPIO(USB_DRVVBUS_P1), 1); + + /* disable IP0 debounce */ + write32p(PERICFG_BASE + PERI_U3_WAKE_CTRL0, 0); + /* disable IP1 debounce */ + write32p(PERICFG_BASE + PERI_USB_WAKEUP_DEC_CON1, 0); } -- To view, visit https://review.coreboot.org/c/coreboot/+/63858 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I296c4491c5959670a39fa8bd6ef987557bbc459f Gerrit-Change-Number: 63858 Gerrit-PatchSet: 12 Gerrit-Owner: Rex-BC Chen <rex-bc.chen(a)mediatek.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Rex-BC Chen <rex-bc.chen(a)mediatek.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: payloads/tianocore: Allow custom build parameters for custom builds
by Krystian Hebel (Code Review) 02 May '22

02 May '22

Attention is currently required from: Martin L Roth, Michał Kopeć, Stefan Reinauer, Michael Niewöhner. Krystian Hebel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62497 ) Change subject: payloads/tianocore: Allow custom build parameters for custom builds ...................................................................... Patch Set 10: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/62497/comment/ac85144a_d23552d1 PS1, Line 7: for custom builds > I'd just drop that +1 File payloads/external/tianocore/Kconfig: https://review.coreboot.org/c/coreboot/+/62497/comment/c4505263_7cf0879e PS10, Line 192: string "TianoCore custom build parameters" I would put the word 'additional' somewhere in this line. People may skip reading the full description and repeat common parameters here. -- To view, visit https://review.coreboot.org/c/coreboot/+/62497 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I025459ae94592103b4be0c68b422100b7c649d34 Gerrit-Change-Number: 62497 Gerrit-PatchSet: 10 Gerrit-Owner: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin L Roth <martinroth(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin L Roth <martinroth(a)google.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Comment-Date: Mon, 02 May 2022 13:58:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/common/block/psp/psp_gen2: drop unneeded variable initialization
by Felix Held (Code Review) 02 May '22

02 May '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63967 ) Change subject: soc/amd/common/block/psp/psp_gen2: drop unneeded variable initialization ...................................................................... soc/amd/common/block/psp/psp_gen2: drop unneeded variable initialization Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I9a3ec9565e660d5fad61c7e73d56f2f821e152aa Reviewed-on: https://review.coreboot.org/c/coreboot/+/63967 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/common/block/psp/psp_gen2.c 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/common/block/psp/psp_gen2.c b/src/soc/amd/common/block/psp/psp_gen2.c index e8d57fd..12f5fec 100644 --- a/src/soc/amd/common/block/psp/psp_gen2.c +++ b/src/soc/amd/common/block/psp/psp_gen2.c @@ -34,7 +34,7 @@ static u16 rd_mbox_sts(uintptr_t psp_mmio) { - union pspv2_mbox_command tmp = { .val = 0 }; + union pspv2_mbox_command tmp; tmp.val = read32p(psp_mmio + PSP_MAILBOX_COMMAND_OFFSET); return tmp.fields.mbox_status; @@ -51,7 +51,7 @@ static u8 rd_mbox_recovery(uintptr_t psp_mmio) { - union pspv2_mbox_command tmp = { .val = 0 }; + union pspv2_mbox_command tmp; tmp.val = read32p(psp_mmio + PSP_MAILBOX_COMMAND_OFFSET); return !!tmp.fields.recovery; -- To view, visit https://review.coreboot.org/c/coreboot/+/63967 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9a3ec9565e660d5fad61c7e73d56f2f821e152aa Gerrit-Change-Number: 63967 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: 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: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/common/block/psp/psp_gen2: use offsets to access mailbox
by Felix Held (Code Review) 02 May '22

02 May '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63966 ) Change subject: soc/amd/common/block/psp/psp_gen2: use offsets to access mailbox ...................................................................... soc/amd/common/block/psp/psp_gen2: use offsets to access mailbox Drop struct pspv2_mbox and access the PSP mailbox via their offsets into PSP MMIO region. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ib665d7ae19deae07d6a69c11ba8cf44e45ea4e70 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63966 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Reviewed-by: Fred Reitberger <reitbergerfred(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/common/block/psp/psp_gen2.c 1 file changed, 20 insertions(+), 32 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved Fred Reitberger: Looks good to me, but someone else must approve diff --git a/src/soc/amd/common/block/psp/psp_gen2.c b/src/soc/amd/common/block/psp/psp_gen2.c index ddd9a64..e8d57fd 100644 --- a/src/soc/amd/common/block/psp/psp_gen2.c +++ b/src/soc/amd/common/block/psp/psp_gen2.c @@ -10,12 +10,8 @@ #include <soc/iomap.h> #include "psp_def.h" -#define PSP_MAILBOX_OFFSET 0x10570 - -struct pspv2_mbox { - u32 command; - u64 buffer; -} __packed; +#define PSP_MAILBOX_COMMAND_OFFSET 0x10570 /* 4 bytes */ +#define PSP_MAILBOX_BUFFER_OFFSET 0x10574 /* 8 bytes */ union pspv2_mbox_command { u32 val; @@ -36,45 +32,37 @@ return psp_mmio; } -static void *soc_get_mbox_address(void) -{ - uintptr_t psp_mmio = soc_get_psp_base_address(); - if (!psp_mmio) - return 0; - return (void *)(psp_mmio + PSP_MAILBOX_OFFSET); -} - -static u16 rd_mbox_sts(struct pspv2_mbox *mbox) +static u16 rd_mbox_sts(uintptr_t psp_mmio) { union pspv2_mbox_command tmp = { .val = 0 }; - tmp.val = read32(&mbox->command); + tmp.val = read32p(psp_mmio + PSP_MAILBOX_COMMAND_OFFSET); return tmp.fields.mbox_status; } -static void wr_mbox_cmd(struct pspv2_mbox *mbox, u8 cmd) +static void wr_mbox_cmd(uintptr_t psp_mmio, u8 cmd) { union pspv2_mbox_command tmp = { .val = 0 }; /* Write entire 32-bit area to begin command execution */ tmp.fields.mbox_command = cmd; - write32(&mbox->command, tmp.val); + write32p(psp_mmio + PSP_MAILBOX_COMMAND_OFFSET, tmp.val); } -static u8 rd_mbox_recovery(struct pspv2_mbox *mbox) +static u8 rd_mbox_recovery(uintptr_t psp_mmio) { union pspv2_mbox_command tmp = { .val = 0 }; - tmp.val = read32(&mbox->command); + tmp.val = read32p(psp_mmio + PSP_MAILBOX_COMMAND_OFFSET); return !!tmp.fields.recovery; } -static void wr_mbox_buffer_ptr(struct pspv2_mbox *mbox, void *buffer) +static void wr_mbox_buffer_ptr(uintptr_t psp_mmio, void *buffer) { - write64(&mbox->buffer, (uintptr_t)buffer); + write64p(psp_mmio + PSP_MAILBOX_BUFFER_OFFSET, (uintptr_t)buffer); } -static int wait_command(struct pspv2_mbox *mbox, bool wait_for_ready) +static int wait_command(uintptr_t psp_mmio, bool wait_for_ready) { union pspv2_mbox_command and_mask = { .val = ~0 }; union pspv2_mbox_command expected = { .val = 0 }; @@ -92,7 +80,7 @@ stopwatch_init_msecs_expire(&sw, PSP_CMD_TIMEOUT); do { - tmp = read32(&mbox->command); + tmp = read32p(psp_mmio + PSP_MAILBOX_COMMAND_OFFSET); tmp &= ~and_mask.val; if (tmp == expected.val) return 0; @@ -103,27 +91,27 @@ int send_psp_command(u32 command, void *buffer) { - struct pspv2_mbox *mbox = soc_get_mbox_address(); - if (!mbox) + uintptr_t psp_mmio = soc_get_psp_base_address(); + if (!psp_mmio) return -PSPSTS_NOBASE; - if (rd_mbox_recovery(mbox)) + if (rd_mbox_recovery(psp_mmio)) return -PSPSTS_RECOVERY; - if (wait_command(mbox, true)) + if (wait_command(psp_mmio, true)) return -PSPSTS_CMD_TIMEOUT; /* set address of command-response buffer and write command register */ - wr_mbox_buffer_ptr(mbox, buffer); - wr_mbox_cmd(mbox, command); + wr_mbox_buffer_ptr(psp_mmio, buffer); + wr_mbox_cmd(psp_mmio, command); /* PSP clears command register when complete. All commands except * SxInfo set the Ready bit. */ - if (wait_command(mbox, command != MBOX_BIOS_CMD_SX_INFO)) + if (wait_command(psp_mmio, command != MBOX_BIOS_CMD_SX_INFO)) return -PSPSTS_CMD_TIMEOUT; /* check delivery status */ - if (rd_mbox_sts(mbox)) + if (rd_mbox_sts(psp_mmio)) return -PSPSTS_SEND_ERROR; return 0; -- To view, visit https://review.coreboot.org/c/coreboot/+/63966 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib665d7ae19deae07d6a69c11ba8cf44e45ea4e70 Gerrit-Change-Number: 63966 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: 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: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/common/block/psp/psp_gen2: use read32p instead of typecast
by Felix Held (Code Review) 02 May '22

02 May '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63965 ) Change subject: soc/amd/common/block/psp/psp_gen2: use read32p instead of typecast ...................................................................... soc/amd/common/block/psp/psp_gen2: use read32p instead of typecast Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I50b8fc270669f079d4f2ec21aec40388afc1705f Reviewed-on: https://review.coreboot.org/c/coreboot/+/63965 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Reviewed-by: Fred Reitberger <reitbergerfred(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/common/block/psp/psp_gen2.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved Fred Reitberger: Looks good to me, but someone else must approve diff --git a/src/soc/amd/common/block/psp/psp_gen2.c b/src/soc/amd/common/block/psp/psp_gen2.c index 22e19fa..ddd9a64 100644 --- a/src/soc/amd/common/block/psp/psp_gen2.c +++ b/src/soc/amd/common/block/psp/psp_gen2.c @@ -137,7 +137,7 @@ printk(BIOS_WARNING, "PSP: PSP_ADDR_MSR uninitialized\n"); return CB_ERR; } - *msg_38_value = read32((void *)psp_mmio + CORE_2_PSP_MSG_38_OFFSET); + *msg_38_value = read32p(psp_mmio + CORE_2_PSP_MSG_38_OFFSET); return CB_SUCCESS; } -- To view, visit https://review.coreboot.org/c/coreboot/+/63965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I50b8fc270669f079d4f2ec21aec40388afc1705f Gerrit-Change-Number: 63965 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: 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: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/common/block/psp/psp_gen2: use union pspv2_mbox_command
by Felix Held (Code Review) 02 May '22

02 May '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63964 ) Change subject: soc/amd/common/block/psp/psp_gen2: use union pspv2_mbox_command ...................................................................... soc/amd/common/block/psp/psp_gen2: use union pspv2_mbox_command Don't use unnamed redefinitions of the pspv2_mbox_command union when the union definition can be used instead. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I3757db45272f11bb47e5106ad9054c0a9ca0cd52 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63964 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Reviewed-by: Fred Reitberger <reitbergerfred(a)gmail.com> --- M src/soc/amd/common/block/psp/psp_gen2.c 1 file changed, 3 insertions(+), 12 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved Fred Reitberger: Looks good to me, but someone else must approve diff --git a/src/soc/amd/common/block/psp/psp_gen2.c b/src/soc/amd/common/block/psp/psp_gen2.c index cbae7a5..22e19fa 100644 --- a/src/soc/amd/common/block/psp/psp_gen2.c +++ b/src/soc/amd/common/block/psp/psp_gen2.c @@ -46,10 +46,7 @@ static u16 rd_mbox_sts(struct pspv2_mbox *mbox) { - union { - u32 val; - struct pspv2_mbox_cmd_fields fields; - } tmp = { 0 }; + union pspv2_mbox_command tmp = { .val = 0 }; tmp.val = read32(&mbox->command); return tmp.fields.mbox_status; @@ -57,10 +54,7 @@ static void wr_mbox_cmd(struct pspv2_mbox *mbox, u8 cmd) { - union { - u32 val; - struct pspv2_mbox_cmd_fields fields; - } tmp = { 0 }; + union pspv2_mbox_command tmp = { .val = 0 }; /* Write entire 32-bit area to begin command execution */ tmp.fields.mbox_command = cmd; @@ -69,10 +63,7 @@ static u8 rd_mbox_recovery(struct pspv2_mbox *mbox) { - union { - u32 val; - struct pspv2_mbox_cmd_fields fields; - } tmp = { 0 }; + union pspv2_mbox_command tmp = { .val = 0 }; tmp.val = read32(&mbox->command); return !!tmp.fields.recovery; -- To view, visit https://review.coreboot.org/c/coreboot/+/63964 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3757db45272f11bb47e5106ad9054c0a9ca0cd52 Gerrit-Change-Number: 63964 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: 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: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/common/block/psp/psp_gen2: factor out pspv2_mbox_command union
by Felix Held (Code Review) 02 May '22

02 May '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63963 ) Change subject: soc/amd/common/block/psp/psp_gen2: factor out pspv2_mbox_command union ...................................................................... soc/amd/common/block/psp/psp_gen2: factor out pspv2_mbox_command union The pspv2_mbox struct contained an unnamed union that covered the 32 bits of the command register of the PSP v2 mailbox. Since the pspv2_mbox struct is mainly used for hardware register accesses and the union part is mostly used to access the different bits before/after writing/reading the command register, split this functionality. For the register access a command field is added to the pspv2_mbox struct instead of the unnamed union and for accessing the separate bits of the command register a new named union is added. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: If3f00b6fd73c3f749154b77b940e6d5aa385ec49 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63963 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> --- M src/soc/amd/common/block/psp/psp_gen2.c 1 file changed, 18 insertions(+), 16 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/common/block/psp/psp_gen2.c b/src/soc/amd/common/block/psp/psp_gen2.c index 6f46d82..cbae7a5 100644 --- a/src/soc/amd/common/block/psp/psp_gen2.c +++ b/src/soc/amd/common/block/psp/psp_gen2.c @@ -13,19 +13,21 @@ #define PSP_MAILBOX_OFFSET 0x10570 struct pspv2_mbox { - union { - u32 val; - struct pspv2_mbox_cmd_fields { - u16 mbox_status; - u8 mbox_command; - u32 reserved:6; - u32 recovery:1; - u32 ready:1; - } __packed fields; - }; + u32 command; u64 buffer; } __packed; +union pspv2_mbox_command { + u32 val; + struct pspv2_mbox_cmd_fields { + u16 mbox_status; + u8 mbox_command; + u32 reserved:6; + u32 recovery:1; + u32 ready:1; + } __packed fields; +}; + static uintptr_t soc_get_psp_base_address(void) { uintptr_t psp_mmio = rdmsr(PSP_ADDR_MSR).lo; @@ -49,7 +51,7 @@ struct pspv2_mbox_cmd_fields fields; } tmp = { 0 }; - tmp.val = read32(&mbox->val); + tmp.val = read32(&mbox->command); return tmp.fields.mbox_status; } @@ -62,7 +64,7 @@ /* Write entire 32-bit area to begin command execution */ tmp.fields.mbox_command = cmd; - write32(&mbox->val, tmp.val); + write32(&mbox->command, tmp.val); } static u8 rd_mbox_recovery(struct pspv2_mbox *mbox) @@ -72,7 +74,7 @@ struct pspv2_mbox_cmd_fields fields; } tmp = { 0 }; - tmp.val = read32(&mbox->val); + tmp.val = read32(&mbox->command); return !!tmp.fields.recovery; } @@ -83,8 +85,8 @@ static int wait_command(struct pspv2_mbox *mbox, bool wait_for_ready) { - struct pspv2_mbox and_mask = { .val = ~0 }; - struct pspv2_mbox expected = { .val = 0 }; + union pspv2_mbox_command and_mask = { .val = ~0 }; + union pspv2_mbox_command expected = { .val = 0 }; struct stopwatch sw; u32 tmp; @@ -99,7 +101,7 @@ stopwatch_init_msecs_expire(&sw, PSP_CMD_TIMEOUT); do { - tmp = read32(&mbox->val); + tmp = read32(&mbox->command); tmp &= ~and_mask.val; if (tmp == expected.val) return 0; -- To view, visit https://review.coreboot.org/c/coreboot/+/63963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If3f00b6fd73c3f749154b77b940e6d5aa385ec49 Gerrit-Change-Number: 63963 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: 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: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/common/block/psp/psp_gen2: rename cmd_response to buffer
by Felix Held (Code Review) 02 May '22

02 May '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63962 ) Change subject: soc/amd/common/block/psp/psp_gen2: rename cmd_response to buffer ...................................................................... soc/amd/common/block/psp/psp_gen2: rename cmd_response to buffer The cmd_response field in the pspv2_mbox struct is the buffer used to pass data to the PSP and back to the x86 side, so rename it to buffer. This also aligns the code a bit more with the reference code. Also rename the wr_mbox_cmd_resp function to wr_mbox_buffer_ptr. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I22c8971b07b3dedcc2e6e50e93c98d69ec7379e8 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63962 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Reviewed-by: Fred Reitberger <reitbergerfred(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/common/block/psp/psp_gen2.c 1 file changed, 4 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved Fred Reitberger: Looks good to me, but someone else must approve diff --git a/src/soc/amd/common/block/psp/psp_gen2.c b/src/soc/amd/common/block/psp/psp_gen2.c index 67b9391..6f46d82 100644 --- a/src/soc/amd/common/block/psp/psp_gen2.c +++ b/src/soc/amd/common/block/psp/psp_gen2.c @@ -23,7 +23,7 @@ u32 ready:1; } __packed fields; }; - u64 cmd_response; + u64 buffer; } __packed; static uintptr_t soc_get_psp_base_address(void) @@ -76,9 +76,9 @@ return !!tmp.fields.recovery; } -static void wr_mbox_cmd_resp(struct pspv2_mbox *mbox, void *buffer) +static void wr_mbox_buffer_ptr(struct pspv2_mbox *mbox, void *buffer) { - write64(&mbox->cmd_response, (uintptr_t)buffer); + write64(&mbox->buffer, (uintptr_t)buffer); } static int wait_command(struct pspv2_mbox *mbox, bool wait_for_ready) @@ -121,7 +121,7 @@ return -PSPSTS_CMD_TIMEOUT; /* set address of command-response buffer and write command register */ - wr_mbox_cmd_resp(mbox, buffer); + wr_mbox_buffer_ptr(mbox, buffer); wr_mbox_cmd(mbox, command); /* PSP clears command register when complete. All commands except -- To view, visit https://review.coreboot.org/c/coreboot/+/63962 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22c8971b07b3dedcc2e6e50e93c98d69ec7379e8 Gerrit-Change-Number: 63962 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: 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: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/common/block/psp/psp: remove unneeded line break
by Felix Held (Code Review) 02 May '22

02 May '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63961 ) Change subject: soc/amd/common/block/psp/psp: remove unneeded line break ...................................................................... soc/amd/common/block/psp/psp: remove unneeded line break Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I0f2fa245be6f7fabde53bfc45c1af73fa13fe862 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63961 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Reviewed-by: Fred Reitberger <reitbergerfred(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/common/block/psp/psp.c 1 file changed, 1 insertion(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved Fred Reitberger: Looks good to me, but someone else must approve diff --git a/src/soc/amd/common/block/psp/psp.c b/src/soc/amd/common/block/psp/psp.c index 66f7d59..f989a3b 100644 --- a/src/soc/amd/common/block/psp/psp.c +++ b/src/soc/amd/common/block/psp/psp.c @@ -77,5 +77,4 @@ psp_print_cmd_status(cmd_status, &buffer.header); } -BOOT_STATE_INIT_ENTRY(BS_PAYLOAD_BOOT, BS_ON_ENTRY, - psp_notify_boot_done, NULL); +BOOT_STATE_INIT_ENTRY(BS_PAYLOAD_BOOT, BS_ON_ENTRY, psp_notify_boot_done, NULL); -- To view, visit https://review.coreboot.org/c/coreboot/+/63961 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0f2fa245be6f7fabde53bfc45c1af73fa13fe862 Gerrit-Change-Number: 63961 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: 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: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/common/block/psp: move mbox struct to generation-specific code
by Felix Held (Code Review) 02 May '22

02 May '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63960 ) Change subject: soc/amd/common/block/psp: move mbox struct to generation-specific code ...................................................................... soc/amd/common/block/psp: move mbox struct to generation-specific code The pspv[1,2]_mbox struct is only used in psp_gen[1,2].c, so move those definitions from the common psp_def.h to the specific psp_gen[1,2].c files. Also fix the struct name in the comment about pspv1_mbox. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I0c95e9a6e292b90e0d147c57f59828a9b41e4b82 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63960 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Reviewed-by: Fred Reitberger <reitbergerfred(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/common/block/psp/psp_def.h M src/soc/amd/common/block/psp/psp_gen1.c M src/soc/amd/common/block/psp/psp_gen2.c 3 files changed, 33 insertions(+), 32 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved Fred Reitberger: Looks good to me, but someone else must approve diff --git a/src/soc/amd/common/block/psp/psp_def.h b/src/soc/amd/common/block/psp/psp_def.h index 11686aa..f171efe 100644 --- a/src/soc/amd/common/block/psp/psp_def.h +++ b/src/soc/amd/common/block/psp/psp_def.h @@ -29,38 +29,6 @@ #define CORE_2_PSP_MSG_38_OFFSET 0x10998 #define CORE_2_PSP_MSG_38_FUSE_SPL BIT(12) -/* generic PSP interface status, v1 */ -#define PSPV1_STATUS_INITIALIZED BIT(0) -#define PSPV1_STATUS_ERROR BIT(1) -#define PSPV1_STATUS_TERMINATED BIT(2) -#define PSPV1_STATUS_HALT BIT(3) -#define PSPV1_STATUS_RECOVERY BIT(4) - -/* psp_mbox consists of hardware registers beginning at PSPx000070 - * mbox_command: BIOS->PSP command, cleared by PSP when complete - * mbox_status: BIOS->PSP interface status - * cmd_response: pointer to command/response buffer - */ -struct pspv1_mbox { - u32 mbox_command; - u32 mbox_status; - u64 cmd_response; /* definition conflicts w/BKDG but matches agesa */ -} __packed; - -struct pspv2_mbox { - union { - u32 val; - struct pspv2_mbox_cmd_fields { - u16 mbox_status; - u8 mbox_command; - u32 reserved:6; - u32 recovery:1; - u32 ready:1; - } __packed fields; - }; - u64 cmd_response; -} __packed; - /* command/response format, BIOS builds this in memory * mbox_buffer_header: generic header * mbox_buffer: command-specific buffer format diff --git a/src/soc/amd/common/block/psp/psp_gen1.c b/src/soc/amd/common/block/psp/psp_gen1.c index 37257ba..1227c2a 100644 --- a/src/soc/amd/common/block/psp/psp_gen1.c +++ b/src/soc/amd/common/block/psp/psp_gen1.c @@ -18,6 +18,25 @@ #define PSP_MAILBOX_OFFSET 0x70 +/* generic PSP interface status, v1 */ +#define PSPV1_STATUS_INITIALIZED BIT(0) +#define PSPV1_STATUS_ERROR BIT(1) +#define PSPV1_STATUS_TERMINATED BIT(2) +#define PSPV1_STATUS_HALT BIT(3) +#define PSPV1_STATUS_RECOVERY BIT(4) + +/* + * pspv1_mbox consists of hardware registers beginning at PSPx000070 + * mbox_command: BIOS->PSP command, cleared by PSP when complete + * mbox_status: BIOS->PSP interface status + * cmd_response: pointer to command/response buffer + */ +struct pspv1_mbox { + u32 mbox_command; + u32 mbox_status; + u64 cmd_response; /* definition conflicts w/BKDG but matches agesa */ +} __packed; + static void *soc_get_mbox_address(void) { uintptr_t psp_mmio; diff --git a/src/soc/amd/common/block/psp/psp_gen2.c b/src/soc/amd/common/block/psp/psp_gen2.c index e5f23cd..67b9391 100644 --- a/src/soc/amd/common/block/psp/psp_gen2.c +++ b/src/soc/amd/common/block/psp/psp_gen2.c @@ -12,6 +12,20 @@ #define PSP_MAILBOX_OFFSET 0x10570 +struct pspv2_mbox { + union { + u32 val; + struct pspv2_mbox_cmd_fields { + u16 mbox_status; + u8 mbox_command; + u32 reserved:6; + u32 recovery:1; + u32 ready:1; + } __packed fields; + }; + u64 cmd_response; +} __packed; + static uintptr_t soc_get_psp_base_address(void) { uintptr_t psp_mmio = rdmsr(PSP_ADDR_MSR).lo; -- To view, visit https://review.coreboot.org/c/coreboot/+/63960 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0c95e9a6e292b90e0d147c57f59828a9b41e4b82 Gerrit-Change-Number: 63960 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: 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: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2022