Attention is currently required from: Jakub Czapiga, Jan Dabros.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56719 )
Change subject: tests/Makefile.inc: Add function wrapping mechanism
......................................................................
Patch Set 2:
(8 comments)
Patchset:
PS2:
Oh, cool, so does this actually work? Nice!
File tests/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/56719/comment/6b36a127_da988b65
PS2, Line 123: $(OBJCOPY) $$@.orig $$(OBJCOPY_FLAGS) $$@.orig2
nit: wouldn't it be easier to generate objcopy_wrap_flags first and then just run
$(OBJCOPY) $$@.orig $$(OBJCOPY_FLAGS) $$$$objcopy_wrap_flags $$@
in one step, rather than calling it twice?
https://review.coreboot.org/c/coreboot/+/56719/comment/35b447d8_fd8a6b00
PS2, Line 126: $$$$sym)
Should put a (properly escaped) $ at the end of the regex to make sure it won't do substring matches (e.g. mocking `fmap_locate_area` shouldn't find the line for `fmap_locate_area_as_rdev`).
https://review.coreboot.org/c/coreboot/+/56719/comment/7f6e4135_e6c6729c
PS2, Line 126: objdump
Let's make a Makefile variable for this, same as we have for $(OBJCOPY)
https://review.coreboot.org/c/coreboot/+/56719/comment/8af49480_abcbc7fa
PS2, Line 128: rev | awk '{ print $$$$2 }' | rev
You can just do
awk '{ print $(NF - 1) }'
and leave out the `rev`s.
https://review.coreboot.org/c/coreboot/+/56719/comment/6942cf57_66afb986
PS2, Line 129: .text.$$$${sym}
This should work in general, but there are rare cases where it may not (e.g. functions from assembly files, or if something uses an explicit __attribute__((section)) for some reason). I think it would be better if you take the section from the objdump output (field $(NF - 2)) to make sure it's the right one.
https://review.coreboot.org/c/coreboot/+/56719/comment/bc2c9ebe_872f1acf
PS2, Line 129: before=$$$${sym}
I don't think you actually need the before=$$$${sym}? Does it not work without that? From my understanding of the man page all that does is affect the order of symbols in the symbol table of the object file, which should make no difference for what we're doing here.
https://review.coreboot.org/c/coreboot/+/56719/comment/d29f76ca_fe2ec6cc
PS2, Line 129: 0
So why are you going through the effort of extracting `addr` if you then hardcode a 0 here? Again, in 99% of cases this offset will be 0, but to preclude problems with rare edge cases it would be better to use the real offset given by objdump.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56719
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7cd0d66a17029955cbf75c8b155a7ebb7f5513aa
Gerrit-Change-Number: 56719
Gerrit-PatchSet: 2
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Comment-Date: Fri, 30 Jul 2021 20:34:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Martin Roth, Rob Barnes.
Hello build bot (Jenkins), Martin Roth, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56667
to look at the new patch set (#4).
Change subject: guybrush: Document USB mapping in devicetree
......................................................................
guybrush: Document USB mapping in devicetree
Add a short documenting comment to each usb entry in devicetree so it is
clear which function each usb port maps to.
BUG=None
TEST=Build
BRANCH=None
Change-Id: I14cbb6af021bb27c89aa82456722f21aa09617be
Signed-off-by: Rob Barnes <robbarnes(a)google.com>
---
M 3rdparty/amd_blobs
M 3rdparty/blobs
M 3rdparty/intel-microcode
M 3rdparty/qc_blobs
M src/mainboard/google/guybrush/variants/baseboard/devicetree.cb
5 files changed, 24 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/56667/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/56667
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I14cbb6af021bb27c89aa82456722f21aa09617be
Gerrit-Change-Number: 56667
Gerrit-PatchSet: 4
Gerrit-Owner: Rob Barnes <robbarnes(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Rob Barnes <robbarnes(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Bora Guvendik, Furquan Shaikh, Chiranjeevi Rapolu, Selma Bensaid, Balaji Manigandan, Thejaswani Putta.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56722 )
Change subject: mb/google/brya: Add RTD3 for WWAN
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
I'm not sure that we wanted to have RTD3 for the WWAN, because of the high latency (~20s) to reconnect to the network after a power cycle
--
To view, visit https://review.coreboot.org/c/coreboot/+/56722
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie9d1ce55cc1297ea0e1069979bbecfaac8f8de05
Gerrit-Change-Number: 56722
Gerrit-PatchSet: 1
Gerrit-Owner: Thejaswani Putta <thejaswani.putta(a)intel.com>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Chiranjeevi Rapolu <chiranjeevi.rapolu(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Chiranjeevi Rapolu <chiranjeevi.rapolu(a)intel.com>
Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Attention: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Attention: Thejaswani Putta <thejaswani.putta(a)intel.com>
Gerrit-Comment-Date: Fri, 30 Jul 2021 20:24:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Thejaswani Putta has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56722 )
Change subject: mb/google/brya: Add RTD3 for WWAN
......................................................................
mb/google/brya: Add RTD3 for WWAN
Enable the PCIe RTD3 driver for WWAN device attached to PCIe Root
Port 6 and provide the reset GPIO / src clk pins.
BUG=None
BRANCH=None
TEST=Build and boot the coreboot image, check if device is enumerated
in the lspci list after warm/cold reboot cycles, run suspend cycles and
check if WWAN is entering L2 LPM.
Signed-off-by: Thejaswani Putta <thejaswani.putta(a)intel.com>
Change-Id: Ie9d1ce55cc1297ea0e1069979bbecfaac8f8de05
---
M src/mainboard/google/brya/variants/baseboard/brya/devicetree.cb
1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/56722/1
diff --git a/src/mainboard/google/brya/variants/baseboard/brya/devicetree.cb b/src/mainboard/google/brya/variants/baseboard/brya/devicetree.cb
index 12bff55..b8d0b5c 100644
--- a/src/mainboard/google/brya/variants/baseboard/brya/devicetree.cb
+++ b/src/mainboard/google/brya/variants/baseboard/brya/devicetree.cb
@@ -140,6 +140,11 @@
device ref sata on end
device ref pcie_rp6 on
# Enable WWAN PCIE 6 using clk 5
+ chip soc/intel/common/block/pcie/rtd3
+ register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_E0)"
+ register "srcclk_pin" = "5"
+ device generic 0 on end
+ end
register "pch_pcie_rp[PCH_RP(6)]" = "{
.clk_src = 5,
.clk_req = 5,
--
To view, visit https://review.coreboot.org/c/coreboot/+/56722
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie9d1ce55cc1297ea0e1069979bbecfaac8f8de05
Gerrit-Change-Number: 56722
Gerrit-PatchSet: 1
Gerrit-Owner: Thejaswani Putta <thejaswani.putta(a)intel.com>
Gerrit-MessageType: newchange
Attention is currently required from: Scott Chao.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56710 )
Change subject: ec/google/chromeec: Add code for KEY_MICMUTE and KEY_KBDILLUMTOGGLE
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Scott, src/ec/google/chromeec/ec_commands.h in coreboot is merely a copy from the platform/ec codebase (platform/ec/include/ec_commands.h). The only difference is I think we change the copyright header slightly. See CB:55911
--
To view, visit https://review.coreboot.org/c/coreboot/+/56710
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie4fa3e627f448265f72279704d258b2d3fe8fc17
Gerrit-Change-Number: 56710
Gerrit-PatchSet: 1
Gerrit-Owner: Scott Chao <scott_chao(a)wistron.corp-partner.google.com>
Gerrit-Reviewer: Rajat Jain <rajatja(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Scott Chao <scott_chao(a)wistron.corp-partner.google.com>
Gerrit-Comment-Date: Fri, 30 Jul 2021 18:47:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak, Scott Chao.
Rajat Jain has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56710 )
Change subject: ec/google/chromeec: Add code for KEY_MICMUTE and KEY_KBDILLUMTOGGLE
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/56710
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie4fa3e627f448265f72279704d258b2d3fe8fc17
Gerrit-Change-Number: 56710
Gerrit-PatchSet: 1
Gerrit-Owner: Scott Chao <scott_chao(a)wistron.corp-partner.google.com>
Gerrit-Reviewer: Rajat Jain <rajatja(a)google.com>
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: Scott Chao <scott_chao(a)wistron.corp-partner.google.com>
Gerrit-Comment-Date: Fri, 30 Jul 2021 18:39:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Ren Kuo, Shou-Chieh Hsu.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56685 )
Change subject: mb/google/dedede/var/magolor: Modify SSFC for camera and touchscreen
......................................................................
Patch Set 7:
(1 comment)
File src/mainboard/google/dedede/variants/magolor/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/56685/comment/ea15dcec_513aa078
PS7, Line 2: field CAMERA_WFC 38 43
: option CAMERA_NONE 0
: option CAMERA_OVTI5675 33
: option CAMERA_OVTI8856 34
: end
: field TS_SOURCE 44 47
: option TS_UNPROVISIONED 0
: option TS_ELAN_6915 1
: option TS_ELAN_6918 2
: option TS_ELAN_0001 3
: option TS_RAYD_0001 4
: option TS_WDHT0002 5
: option TS_GTCH7503 6
: end
This is incorrect after evaluating the SSFC. It should be
```
fw_config
field CAMERA_WFC 38 40
option CAMERA_WFC_NONE 0
option CAMERA_WFC_OVTI5675 33
option CAMERA_WFC_OVTI8856 34
end
field CAMERA_UFC 41 42
option CAMERA_UFC_NONE 0
end
field CAMERA_VCM 43 44
option CAMERA_VCM_NONE 0
option CAMERA_VCM0 1
end
field TS_SOURCE 45 48
option TS_UNPROVISIONED 0
option TS_ELAN_6915 1
option TS_ELAN_6918 2
option TS_ELAN_0001 3
option TS_RAYD_0001 4
option TS_WDHT0002 5
option TS_GTCH7503 6
end
end
```
If none of the projects that are using ELAN6918 has hit the PVT, I would recommend fixing it now.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56685
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I13d76ce8b932f483e20ca5388f1c67eb39ba12a1
Gerrit-Change-Number: 56685
Gerrit-PatchSet: 7
Gerrit-Owner: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Shou-Chieh Hsu <shouchieh(a)google.com>
Gerrit-Reviewer: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Henry Sun <henrysun(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Attention: Shou-Chieh Hsu <shouchieh(a)google.com>
Gerrit-Comment-Date: Fri, 30 Jul 2021 18:30:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56610
to look at the new patch set (#4).
Change subject: mb/google/brya/brya0: Add new DB_USB option
......................................................................
mb/google/brya/brya0: Add new DB_USB option
The KB8001 DB is also to be supported on brya. It requires the SoC's IOM
FW to configure the USB aux signals when alternate modes are entered.
BUG=b:189868217
TEST=compile
Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Change-Id: I21200bf51f219602cd97a36d20a103292f18a6e8
---
M src/mainboard/google/brya/Kconfig
M src/mainboard/google/brya/variants/brya0/overridetree.cb
M src/mainboard/google/brya/variants/brya0/variant.c
3 files changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/56610/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/56610
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I21200bf51f219602cd97a36d20a103292f18a6e8
Gerrit-Change-Number: 56610
Gerrit-PatchSet: 4
Gerrit-Owner: 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