Attention is currently required from: Angel Pons, Kyösti Mälkki.
Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68264 )
Change subject: drivers/usb/gadget.c: Add support for EHCI debug using the WCH CH347
......................................................................
Patch Set 4:
(1 comment)
File src/drivers/usb/gadget.c:
https://review.coreboot.org/c/coreboot/+/68264/comment/f628f5c8_d70ae139
PS4, Line 215: } __packed;
> Are you conserned of endianess or sizeof() or something else?
I guess what matters here is that those fields are in that order in a contiguous 7 byte area, and that the byte order is the same as what the USB CDC protocol expects. I'm assuming the EHCI debug stack just sends out whatever bytes it receives in the order they are received, and looking at a Wireshark dump of the USB CDC packet it appears that the 32-bit baud rate is sent LSB so I guess endianness could be an issue.
--
To view, visit https://review.coreboot.org/c/coreboot/+/68264
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibd4ad17b7369948003fff7e825b46fe852bc7eb9
Gerrit-Change-Number: 68264
Gerrit-PatchSet: 4
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Comment-Date: Wed, 09 Nov 2022 22:27:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Chen-Tsung Hsieh, Martin L Roth, Macpaul Lin, Julius Werner, Yu-Ping Wu.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/66625 )
Change subject: soc/mediatek/mt8195: replace SPDX identifiers to GPL-2.0-only OR MIT
......................................................................
Patch Set 7: Code-Review+1
(1 comment)
Patchset:
PS7:
Fine with me.
--
To view, visit https://review.coreboot.org/c/coreboot/+/66625
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I79a585c2a611dbfd294c1c94f998d972118b5c52
Gerrit-Change-Number: 66625
Gerrit-PatchSet: 7
Gerrit-Owner: Macpaul Lin <macpaul.lin(a)mediatek.com>
Gerrit-Reviewer: Chen-Tsung Hsieh <chentsung(a)chromium.org>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Rex-BC Chen <rex-bc.chen(a)mediatek.com>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Chen-Tsung Hsieh <chentsung(a)chromium.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Macpaul Lin <macpaul.lin(a)mediatek.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Wed, 09 Nov 2022 22:12:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Nicholas Chin.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68264 )
Change subject: drivers/usb/gadget.c: Add support for EHCI debug using the WCH CH347
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
File src/drivers/usb/gadget.c:
https://review.coreboot.org/c/coreboot/+/68264/comment/064a3346_7a52078a
PS4, Line 215: } __packed;
> This might not be portable.
Are you conserned of endianess or sizeof() or something else?
--
To view, visit https://review.coreboot.org/c/coreboot/+/68264
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibd4ad17b7369948003fff7e825b46fe852bc7eb9
Gerrit-Change-Number: 68264
Gerrit-PatchSet: 4
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Wed, 09 Nov 2022 22:06:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Andrey Petrov.
Hello Andrey Petrov,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/69412
to look at the new patch set (#2).
Change subject: drivers/fsp2: Don't die if the FSP signature doesn't match
......................................................................
drivers/fsp2: Don't die if the FSP signature doesn't match
If the FSP signature doesn't match, there's going to be something else
that kills the system, or not. Either way, killing the system in
coreboot isn't needed. We should absolutely print an error message, but
let the FSP take care of halting the system or better still, returning
to let coreboot print a message and halt at that point.
Signed-off-by: Martin Roth <gaumless(a)gmail.com>
Change-Id: I12aca7cad3298ac36b1fed09efaa190c958cf126
---
M src/drivers/intel/fsp2_0/util.c
1 file changed, 17 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/69412/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/69412
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I12aca7cad3298ac36b1fed09efaa190c958cf126
Gerrit-Change-Number: 69412
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Andrey Petrov.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69412 )
Change subject: drivers/fsp2: Don't die if the FSP signature doesn't match
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-163124):
https://review.coreboot.org/c/coreboot/+/69412/comment/3ce65a79_a17a4fc1
PS1, Line 11: coreboot isn't needed. We should absolutely print an error message, but
Possible unwrapped commit description (prefer a maximum 72 chars per line)
--
To view, visit https://review.coreboot.org/c/coreboot/+/69412
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I12aca7cad3298ac36b1fed09efaa190c958cf126
Gerrit-Change-Number: 69412
Gerrit-PatchSet: 1
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Comment-Date: Wed, 09 Nov 2022 21:43:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Martin L Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/69412 )
Change subject: drivers/fsp2: Don't die if the FSP signature doesn't match
......................................................................
drivers/fsp2: Don't die if the FSP signature doesn't match
If the FSP signature doesn't match, there's going to be something else
that kills the system, or not. Either way, killing the system in
coreboot isn't needed. We should absolutely print an error message, but
let the FSP take care of halting the system or better still, returning to
let coreboot print a message and halt at that point.
Signed-off-by: Martin Roth <gaumless(a)gmail.com>
Change-Id: I12aca7cad3298ac36b1fed09efaa190c958cf126
---
M src/drivers/intel/fsp2_0/util.c
1 file changed, 17 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/69412/1
diff --git a/src/drivers/intel/fsp2_0/util.c b/src/drivers/intel/fsp2_0/util.c
index 1dedb65..1f8e12f 100644
--- a/src/drivers/intel/fsp2_0/util.c
+++ b/src/drivers/intel/fsp2_0/util.c
@@ -182,7 +182,7 @@
if (upd_signature != expected_signature) {
/* The UPD signatures are non-zero-terminated ASCII stored as a little endian
uint64_t, so this needs some casts. */
- die_with_post_code(POST_INVALID_VENDOR_BINARY,
+ printk(BIOS_EMERG,
"Invalid UPD signature! FSP provided \"%8s\", expected was \"%8s\".\n",
(char *)&upd_signature,
(char *)&expected_signature);
--
To view, visit https://review.coreboot.org/c/coreboot/+/69412
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I12aca7cad3298ac36b1fed09efaa190c958cf126
Gerrit-Change-Number: 69412
Gerrit-PatchSet: 1
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-MessageType: newchange