Attention is currently required from: Ricardo Quesada, Jack Rosenthal.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56883 )
Change subject: elogtool: add "clear" command
......................................................................
Patch Set 13: Code-Review+2
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56883/comment/fab7cdb9_3cd09307
PS13, Line 10: using flashrom
flashrom or file write if an input file is provided.
https://review.coreboot.org/c/coreboot/+/56883/comment/fadf1aab_1a4b47ea
PS13, Line 22:
TEST=?
Did you try `elogtool list` after `elogtool clear` to see if it correctly clears the elog region and adds ELOG_TYPE_LOG_CLEAR?
--
To view, visit https://review.coreboot.org/c/coreboot/+/56883
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia28a6eb34c82103ab078a0841b022e2e5e430585
Gerrit-Change-Number: 56883
Gerrit-PatchSet: 13
Gerrit-Owner: Ricardo Quesada <ricardoq(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jack Rosenthal <jrosenth(a)chromium.org>
Gerrit-Reviewer: Ricardo Quesada <ricardoq(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ricardo Quesada <ricardoq(a)google.com>
Gerrit-Attention: Jack Rosenthal <jrosenth(a)chromium.org>
Gerrit-Comment-Date: Wed, 01 Sep 2021 04:56:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak.
Hello Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/57299
to look at the new patch set (#2).
Change subject: mb/google/volteer: Add type-c port info to coreboot table
......................................................................
mb/google/volteer: Add type-c port info to coreboot table
This change adds type-c port information for USB Type-C ports to the
coreboot table. This allows depthcharge to know the usb2 and usb3
port number assignments for each available port, as well as the SBU
and data line orientation for the board.
BUG=b:149830546
TEST='emerge-volteer coreboot chromeos-bootimage', flash and boot
volteer2 to kernel, log in and check cbmem for type-c info exported to
the payload:
localhost ~ # cbmem -c | grep type-c
Passing conn0 type-c info to payload: usb2:9 usb3:1 sbu:0 data:0
Passing conn1 type-c info to payload: usb2:4 usb3:2 sbu:1 data:0
Change-Id: Id5686e5b3dfc6f12aa3f8938f371c14d0b2e490d
Signed-off-by: Nick Vaccaro <nvaccaro(a)google.com>
---
M src/mainboard/google/volteer/mainboard.c
1 file changed, 35 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/57299/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/57299
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id5686e5b3dfc6f12aa3f8938f371c14d0b2e490d
Gerrit-Change-Number: 57299
Gerrit-PatchSet: 2
Gerrit-Owner: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Furquan Shaikh, Tim Wawrzynczak, Paul Menzel, Zhuohao Lee, Patrick Rudolph.
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57069 )
Change subject: coreboot tables: Add type-c port info to coreboot table
......................................................................
Patch Set 9:
(5 comments)
File payloads/libpayload/libc/coreboot.c:
https://review.coreboot.org/c/coreboot/+/57069/comment/17189068_e7d84543
PS8, Line 255: type_c_entry->port_count
> something like `MIN(ARRAY_SIZE(info->type_c_config_info), type_c_entry->port_count)`?
Done
File src/drivers/intel/pmc_mux/conn/conn.c:
PS8:
> looks like stale changes
Ack
File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/57069/comment/73906a02_ddbc84d6
PS8, Line 79: pmc = pcidev_path_on_root(PCH_DEVFN_PMC);
: if (!pmc || !pmc->link_list->children) {
: printk(BIOS_ERR, "%s: unable to find PMC device or its mux\n", __func__);
: return 0;
: }
:
: /*
: * Add each port underneath PMC.MUX; some variants may not have this defined,
: * so it's okay to just silently return here.
: */
: mux = pmc->link_list->children;
:
: while ((child = dev_bus_each_child(mux->link_list, child)) != NULL) {
: if (count >= max_ports)
: break;
> We're now using this logic in several places, maybe there should be a helper function in pmc_mux to […]
Sure, I'll change this to add that.
https://review.coreboot.org/c/coreboot/+/57069/comment/d1312b76_718a2d18
PS8, Line 95: if (fw_config_probe(FW_CONFIG(DB_USB, USB4_GEN2))
: || fw_config_probe(FW_CONFIG(DB_USB, USB3_ACTIVE))
: || fw_config_probe(FW_CONFIG(DB_USB, USB4_GEN3))
: || fw_config_probe(FW_CONFIG(DB_USB, USB3_NO_A)))
> I'd also move this out to the beginning of the function, so if this fails, then the function returns […]
Done
https://review.coreboot.org/c/coreboot/+/57069/comment/7d00602a_29f1a5c1
PS8, Line 109: return count;
> nit: empty line after `}`
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/57069
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ice732be2fa634dbf31ec620552b383c4a5b41451
Gerrit-Change-Number: 57069
Gerrit-PatchSet: 9
Gerrit-Owner: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Zhuohao Lee <zhuohao(a)google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 01 Sep 2021 03:43:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Furquan Shaikh, Paul Menzel, Nick Vaccaro, Zhuohao Lee, Patrick Rudolph.
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Zhuohao Lee, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/57069
to look at the new patch set (#9).
Change subject: coreboot tables: Add type-c port info to coreboot table
......................................................................
coreboot tables: Add type-c port info to coreboot table
This change adds type-c port information for USB Type-C ports to the
coreboot table. This allows depthcharge to know the usb2 and usb3
port number assignments for each available port, as well as the SBU
and data line orientation for the board.
BUG=b:149830546
TEST='emerge-volteer coreboot chromeos-bootimage' and verify it buidls
successfully. Cherry-pick CL to enable this feature for volteer,
flash and boot volteer2 to kernel, log in and check cbmem for type-c
info exported to the payload:
localhost ~ # cbmem -c | grep type-c
Passing conn0 type-c info to payload: usb2:9 usb3:1 sbu:0 data:0
Passing conn1 type-c info to payload: usb2:4 usb3:2 sbu:1 data:0
Signed-off-by: Nick Vaccaro <nvaccaro(a)google.com>
Change-Id: Ice732be2fa634dbf31ec620552b383c4a5b41451
---
M payloads/libpayload/include/coreboot_tables.h
M payloads/libpayload/include/sysinfo.h
M payloads/libpayload/libc/coreboot.c
M src/commonlib/include/commonlib/coreboot_tables.h
M src/drivers/intel/pmc_mux/conn/chip.h
M src/drivers/intel/pmc_mux/conn/conn.c
M src/include/boot/coreboot_tables.h
M src/lib/coreboot_table.c
8 files changed, 166 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/57069/9
--
To view, visit https://review.coreboot.org/c/coreboot/+/57069
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ice732be2fa634dbf31ec620552b383c4a5b41451
Gerrit-Change-Number: 57069
Gerrit-PatchSet: 9
Gerrit-Owner: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Zhuohao Lee <zhuohao(a)google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Sam McNally, Furquan Shaikh, Felix Held.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57298 )
Change subject: mb/google/zork: correct MST probes
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
There might be a case for the behavior this is working around being a bug, but since `device ref` behavior isn't documented and the sconfig compiler is complex enough I can't tell if it's intended or a bug.
--
To view, visit https://review.coreboot.org/c/coreboot/+/57298
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2a8feb544f3fc198fe6313b226ad8995aad31c3e
Gerrit-Change-Number: 57298
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Daniel Kurtz <djkurtz(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 01 Sep 2021 03:38:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Peter Marheine has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/57298 )
Change subject: mb/google/zork: correct MST probes
......................................................................
mb/google/zork: correct MST probes
It turns out that putting a device ref in an overridetree at a different
point in the tree will generate a duplicate device definition, such that
the change introducing this support was ignoring the device presence
specified by overridetree.cb and only using the baseboard configuration.
I believe testing of that change was not redone after the baseboard was
changed to disable the MST, so that conflicting behavior was not
noticed.
The incorrect behavior generated a disabled device for the MST at the
location specified by the baseboard, and one with the probe as a child
of the soc. At runtime this did a fw_config probe of the "I2C 00:4a"
device, and later probed a different "I2C 00:4a" which was already
disabled. As the disabled one came later, it seems to have completely
disabled the MST, discarding the results of the variant-specific probe.
BUG=b:185862297
TEST=10EC2141 device is now present on a Dali berknip
BRANCH=zork
Change-Id: I2a8feb544f3fc198fe6313b226ad8995aad31c3e
Signed-off-by: Peter Marheine <pmarheine(a)chromium.org>
---
M src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb
M src/mainboard/google/zork/variants/berknip/overridetree.cb
M src/mainboard/google/zork/variants/morphius/overridetree.cb
M src/mainboard/google/zork/variants/trembyle/overridetree.cb
M src/mainboard/google/zork/variants/woomax/overridetree.cb
5 files changed, 66 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/57298/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/57298
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2a8feb544f3fc198fe6313b226ad8995aad31c3e
Gerrit-Change-Number: 57298
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: newpatchset