Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Paul Menzel.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56750 )
Change subject: vc/google/chromeos: Add support for new SAR tables revisions
......................................................................
Patch Set 13:
(1 comment)
File src/vendorcode/google/chromeos/sar.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-126287):
https://review.coreboot.org/c/coreboot/+/56750/comment/ee07d35f_119d9ed8
PS13, Line 33: return (REVISION_SIZE + member_size(struct wifi_sar_delta_table, group_rev0));
line over 96 characters
--
To view, visit https://review.coreboot.org/c/coreboot/+/56750
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I08c3f321938eba04e8bcff4d87cb215422715bb2
Gerrit-Change-Number: 56750
Gerrit-PatchSet: 13
Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com>
Gerrit-CC: Jamie Chen <jamie.chen(a)intel.com>
Gerrit-CC: Janiex Tu <janiex.tu(a)intel.corp-partner.google.com>
Gerrit-CC: Matt Chen <matt.chen(a)intel.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-CC: Simon Yang <simon1.yang(a)intel.com>
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-Comment-Date: Tue, 17 Aug 2021 07:07:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Paul Menzel, Tim Wawrzynczak, Rizwan Qureshi.
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Rizwan Qureshi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56751
to look at the new patch set (#12).
Change subject: drivers/wifi/generic: Enable DSM ACPI entries for Intel WIFI card
......................................................................
drivers/wifi/generic: Enable DSM ACPI entries for Intel WIFI card
Add support for DSM functions as per the document
559910_Intel_Connectivity_Platforms_BIOS_Guidelines_Rev6_4.pdf
BUG=b:191720858
TEST=Check the generated SSDT tables for DSM method
Change-Id: Ie154edf188531fe6c260274edaa694cf3b3605d3
Signed-off-by: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
---
M src/drivers/wifi/generic/acpi.c
M src/include/sar.h
M src/vendorcode/google/chromeos/sar.c
3 files changed, 168 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/56751/12
--
To view, visit https://review.coreboot.org/c/coreboot/+/56751
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie154edf188531fe6c260274edaa694cf3b3605d3
Gerrit-Change-Number: 56751
Gerrit-PatchSet: 12
Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com>
Gerrit-CC: Jamie Chen <jamie.chen(a)intel.com>
Gerrit-CC: Janiex Tu <janiex.tu(a)intel.corp-partner.google.com>
Gerrit-CC: Matt Chen <matt.chen(a)intel.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sherry An <sherry.an(a)intel.com>
Gerrit-CC: Simon Yang <simon1.yang(a)intel.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Paul Menzel.
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56750
to look at the new patch set (#13).
Change subject: vc/google/chromeos: Add support for new SAR tables revisions
......................................................................
vc/google/chromeos: Add support for new SAR tables revisions
Existing SAR infrastructure supports only revision 0 of the SAR tables.
This patch modifies it to extend support intel wifi 6 and wifi 6e
configs as per the connectivity document:
559910_Intel_Connectivity_Platforms_BIOS_Guidelines_Rev6_4.pdf
The SAR table and WGDS configuration blocks size with in the earlier
SAR binary was static with option to enable WGDS block dynamically
using a coreboot config. The new binary format can support to have any
block as dynamic and the size of the configuration block depends on the
revision of the entry. This patch also adds support for PPAG ACPI
entries related to antenna gains.
BUG=b:193665559
TEST=Checked the SSDT entries for WRDS, EWRD, WGDS and PPAG with
different binaries generated by setting different versions in the
config.star
Change-Id: I08c3f321938eba04e8bcff4d87cb215422715bb2
Signed-off-by: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
---
M src/drivers/wifi/generic/Kconfig
M src/drivers/wifi/generic/Makefile.inc
M src/drivers/wifi/generic/acpi.c
M src/include/sar.h
M src/vendorcode/google/chromeos/Makefile.inc
M src/vendorcode/google/chromeos/sar.c
6 files changed, 350 insertions(+), 95 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/56750/13
--
To view, visit https://review.coreboot.org/c/coreboot/+/56750
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I08c3f321938eba04e8bcff4d87cb215422715bb2
Gerrit-Change-Number: 56750
Gerrit-PatchSet: 13
Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com>
Gerrit-CC: Jamie Chen <jamie.chen(a)intel.com>
Gerrit-CC: Janiex Tu <janiex.tu(a)intel.corp-partner.google.com>
Gerrit-CC: Matt Chen <matt.chen(a)intel.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-CC: Simon Yang <simon1.yang(a)intel.com>
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-MessageType: newpatchset
Attention is currently required from: Kangheui Won, Peter Marheine.
Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56990 )
Change subject: mb/google/zork: only enable RTD2141 when present
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56990
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4ada9b492ab221fa98350bf2faf27a23342f3a55
Gerrit-Change-Number: 56990
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Kangheui Won <khwon(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Tue, 17 Aug 2021 06:06:11 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: V Sowmya, Maulik V Vaghela, Tim Wawrzynczak, Subrata Banik, Sridhar Siricilla, Balaji Manigandan, Ronak Kanabar, Kane Chen, Srinidhi N Kaushik, Patrick Rudolph.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56330 )
Change subject: soc/intel/alderlake: Irms UPD enable
......................................................................
Patch Set 6:
(1 comment)
File src/soc/intel/alderlake/fsp_params.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-126283):
https://review.coreboot.org/c/coreboot/+/56330/comment/a7f21ab3_269bd61d
PS6, Line 614: for (size_t i = 0; i < ARRAY_SIZE(config->domain_vr_config); i++){
space required before the open brace '{'
--
To view, visit https://review.coreboot.org/c/coreboot/+/56330
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ice5c775ef9560503109957a1ed994af1d287aafc
Gerrit-Change-Number: 56330
Gerrit-PatchSet: 6
Gerrit-Owner: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Reviewer: Kane Chen <kane.chen(a)intel.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Kane Chen <kane.chen(a)intel.com>
Gerrit-Attention: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 17 Aug 2021 06:03:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Peter Marheine.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56990
to look at the new patch set (#2).
Change subject: mb/google/zork: only enable RTD2141 when present
......................................................................
mb/google/zork: only enable RTD2141 when present
An MST hub is only present on some devices that are configured with a
particular daughterboard indicated by EC fw_config, so add a fw_config
probe to only enable it on devices where present.
BUG=b:185862297
TEST=RTD2141 remains in ACPI tables on a berknip with Dali DB, and is
not present on the same system if probe is changed to enable it
for picasso DB.
BRANCH=zork
Change-Id: I4ada9b492ab221fa98350bf2faf27a23342f3a55
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, 55 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/56990/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56990
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4ada9b492ab221fa98350bf2faf27a23342f3a55
Gerrit-Change-Number: 56990
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: newpatchset
Peter Marheine has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56990 )
Change subject: mb/google/zork: only enable RTD2141 when present
......................................................................
mb/google/zork: only enable RTD2141 when present
An MST hub is only present on some devices that are configured with a
particular daughterboard indicated by EC fw_config, so add a fw_config
probe to only enable it on devices where present.
BUG=b:185862297
TEST=RTD2141 remains in ACPI tables on a berknip with Dali DB, and is
not present on the same system if probe is changed to enable it
for picasso DB.
BRANCH=zork
Change-Id: I4ada9b492ab221fa98350bf2faf27a23342f3a55
---
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, 55 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/56990/1
diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb b/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb
index 8b1c68a..a475202 100644
--- a/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb
+++ b/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb
@@ -1,4 +1,9 @@
# SPDX-License-Identifier: GPL-2.0-or-later
+
+fw_config
+ field USB_DAUGHTERBOARD 0 3 end
+end
+
chip soc/amd/picasso
# Set FADT Configuration
@@ -408,7 +413,8 @@
register "name" = ""MSTH""
register "uid" = "1"
register "desc" = ""Realtek RTD2141B""
- device i2c 4a on end
+ # Device presence is variant-specific
+ device i2c 4a alias db_mst off end
end
end
end
diff --git a/src/mainboard/google/zork/variants/berknip/overridetree.cb b/src/mainboard/google/zork/variants/berknip/overridetree.cb
index fe2eade..793be78 100644
--- a/src/mainboard/google/zork/variants/berknip/overridetree.cb
+++ b/src/mainboard/google/zork/variants/berknip/overridetree.cb
@@ -1,4 +1,12 @@
# SPDX-License-Identifier: GPL-2.0-or-later
+
+fw_config
+ field USB_DAUGHTERBOARD
+ option BERKNIP_DB_PICASSO 0
+ option BERKNIP_DB_DALI 1
+ end
+end
+
chip soc/amd/picasso
# Start : OPN Performance Configuration
@@ -171,4 +179,8 @@
end
end
+ device ref db_mst on
+ probe USB_DAUGHTERBOARD BERKNIP_DB_DALI
+ end
+
end # chip soc/amd/picasso
diff --git a/src/mainboard/google/zork/variants/morphius/overridetree.cb b/src/mainboard/google/zork/variants/morphius/overridetree.cb
index cfc9cb3..bb5e0e7 100644
--- a/src/mainboard/google/zork/variants/morphius/overridetree.cb
+++ b/src/mainboard/google/zork/variants/morphius/overridetree.cb
@@ -1,4 +1,12 @@
# SPDX-License-Identifier: GPL-2.0-or-later
+
+fw_config
+ field USB_DAUGHTERBOARD
+ option MORPHIUS_DB_PICASSO 0
+ option MORPHIUS_DB_DALI 1
+ end
+end
+
chip soc/amd/picasso
# Start : OPN Performance Configuration
@@ -136,4 +144,8 @@
end
end
+ device ref db_mst on
+ probe USB_DAUGHTERBOARD MORPHIUS_DB_DALI
+ end
+
end # chip soc/amd/picasso
diff --git a/src/mainboard/google/zork/variants/trembyle/overridetree.cb b/src/mainboard/google/zork/variants/trembyle/overridetree.cb
index a558aca..394b3bc 100644
--- a/src/mainboard/google/zork/variants/trembyle/overridetree.cb
+++ b/src/mainboard/google/zork/variants/trembyle/overridetree.cb
@@ -1,4 +1,13 @@
# SPDX-License-Identifier: GPL-2.0-or-later
+
+fw_config
+ field USB_DAUGHTERBOARD
+ option TREMBYLE_DB_PICASSO 0
+ option TREMBYLE_DB_DALI 1
+ option TREMBYLE_DB_DALI_HDMI 2
+ end
+end
+
chip soc/amd/picasso
# Start : OPN Performance Configuration
@@ -104,4 +113,8 @@
end
end
+ device ref db_mst on
+ probe USB_DAUGHTERBOARD TREMBYLE_DB_DALI_HDMI
+ end
+
end # chip soc/amd/picasso
diff --git a/src/mainboard/google/zork/variants/woomax/overridetree.cb b/src/mainboard/google/zork/variants/woomax/overridetree.cb
index 20d4783..82a060f 100644
--- a/src/mainboard/google/zork/variants/woomax/overridetree.cb
+++ b/src/mainboard/google/zork/variants/woomax/overridetree.cb
@@ -1,5 +1,12 @@
# SPDX-License-Identifier: GPL-2.0-or-later
+fw_config
+ field USB_DAUGHTERBOARD
+ option WOOMAX_DB_PICASSO 0
+ option WOOMAX_DB_DALI 1
+ end
+end
+
chip soc/amd/picasso
# Start : OPN Performance Configuration
@@ -110,4 +117,8 @@
end
end
+ device ref db_mst on
+ probe USB_DAUGHTERBOARD WOOMAX_DB_DALI
+ end
+
end # chip soc/amd/picasso
--
To view, visit https://review.coreboot.org/c/coreboot/+/56990
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4ada9b492ab221fa98350bf2faf27a23342f3a55
Gerrit-Change-Number: 56990
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Paul Menzel.
Sugnan Prabhu S has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56750 )
Change subject: vc/google/chromeos: Add support for new SAR tables revisions
......................................................................
Patch Set 12:
(1 comment)
File src/vendorcode/google/chromeos/sar_v2.c:
PS12:
> I am not convinced this has to be broken out into a separate file. […]
Tim,
In legacy implementation WGDS uses CONFIG_GEO_SAR_ENABLE to decide whether the block is present or not.
In current implementation WGDS version will always be present which will be set to invalid incase the block is not present.
As SAR revision is not present in the legacy implementation, we need to add extra byte incase USE_SAR
https://review.coreboot.org/c/coreboot/+/56750/12/src/vendorcode/google/chr…
This will lead to too many if(CONFIG(USE_SAR)) checks and affect the code readability, hence I tried to separate it in different file.
I will try to update the code as suggested and have it in single file.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56750
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I08c3f321938eba04e8bcff4d87cb215422715bb2
Gerrit-Change-Number: 56750
Gerrit-PatchSet: 12
Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com>
Gerrit-CC: Jamie Chen <jamie.chen(a)intel.com>
Gerrit-CC: Janiex Tu <janiex.tu(a)intel.corp-partner.google.com>
Gerrit-CC: Matt Chen <matt.chen(a)intel.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-CC: Simon Yang <simon1.yang(a)intel.com>
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-Comment-Date: Tue, 17 Aug 2021 04:46:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Sugnan Prabhu S, Paul Menzel.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56750 )
Change subject: vc/google/chromeos: Add support for new SAR tables revisions
......................................................................
Patch Set 12:
(10 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56750/comment/e89d46cf_d499036a
PS12, Line 14: wdgs
WGDS
https://review.coreboot.org/c/coreboot/+/56750/comment/b30de7c7_e4e8289c
PS12, Line 15: wdgs
WGDS
https://review.coreboot.org/c/coreboot/+/56750/comment/137e78f2_613d8b84
PS12, Line 22: TEST=Checked the SSDT entries for WRDS, EWRD, WGDS and PPAG with
: different binaries generated by setting different versions in the
: config.star
Does this mean you verified revisions 0, 1 and 2 are all generated and decoded correctly?
File src/drivers/wifi/generic/acpi.c:
https://review.coreboot.org/c/coreboot/+/56750/comment/42623c6a_fef7a746
PS12, Line 55: if (sar_table->version > 2) {
: printk(BIOS_ERR, "Invalid SAR table version: %x02\n", sar_table->version);
: return;
: }
suggestion: move this to an `else` clause after line 75, e.g.:
```
} else if (sar_table->version == 2) {
sar_limit_len = BYTES_PER_SAR_LIMIT_REV2;
} else {
printk(BIOS_ERR, "Unsupported SAR table version: %x02\n", sar_table->version);
return;
}
```
https://review.coreboot.org/c/coreboot/+/56750/comment/d1881577_0e752baa
PS12, Line 141: wgds
WGDS
File src/include/sar.h:
https://review.coreboot.org/c/coreboot/+/56750/comment/3c4fa5f4_532ead8e
PS12, Line 7: #define NUM_SAR_LIMITS 4
: #define BYTES_PER_SAR_LIMIT 10
: #define BYTES_PER_SAR_LIMIT_REV1 22
: #define BYTES_PER_SAR_LIMIT_REV2 (BYTES_PER_SAR_LIMIT_REV1 * 2)
: #define ANT_GAINS_TABLE_COUNT 2
: #define REVISION_SIZE 1
: #define PPAG_MODE_SIZE 1
nit: line these all up on the right side
File src/vendorcode/google/chromeos/sar_v2.c:
PS12:
I am not convinced this has to be broken out into a separate file. I think a few `if CONFIG(USE_SAR_V2)` can be used to be paper over the differences.
https://review.coreboot.org/c/coreboot/+/56750/comment/b4838d17_4905bb8c
PS12, Line 14: if (sar_table->version == 0)
: return BYTES_PER_SAR_LIMIT * NUM_SAR_LIMITS + REVISION_SIZE;
: else if (sar_table->version == 1)
: return BYTES_PER_SAR_LIMIT_REV1 * NUM_SAR_LIMITS + REVISION_SIZE;
: else if (sar_table->version == 2)
: return BYTES_PER_SAR_LIMIT_REV2 * NUM_SAR_LIMITS + REVISION_SIZE;
: else
: return REVISION_SIZE;
with above, e.g.
```
if (CONFIG(USE_SAR))
return BYTES_PER_SAR_LIMIT * NUM_SAR_LIMITS;
size_t size = 0;
if (sar_table->version == 0)
size = BYTES_PER_SAR_LIMIT * NUM_SAR_LIMITS;
else if (sar_table->version == 1)
return BYTES_PER_SAR_LIMIT_REV1 * NUM_SAR_LIMITS;
else if (sar_table->version == 2)
return BYTES_PER_SAR_LIMIT_REV2 * NUM_SAR_LIMITS;
if (CONFIG(USE_SAR_V2))
size += REVISION_SIZE;
return size;
```
https://review.coreboot.org/c/coreboot/+/56750/comment/8fe32aae_3ce16450
PS12, Line 24: wgds_table_size
similar here
https://review.coreboot.org/c/coreboot/+/56750/comment/354cc9c8_f4fcad2b
PS12, Line 34: antgains_table_size
would just return `0` for USE_SAR
--
To view, visit https://review.coreboot.org/c/coreboot/+/56750
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I08c3f321938eba04e8bcff4d87cb215422715bb2
Gerrit-Change-Number: 56750
Gerrit-PatchSet: 12
Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com>
Gerrit-CC: Jamie Chen <jamie.chen(a)intel.com>
Gerrit-CC: Janiex Tu <janiex.tu(a)intel.corp-partner.google.com>
Gerrit-CC: Matt Chen <matt.chen(a)intel.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-CC: Simon Yang <simon1.yang(a)intel.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Tue, 17 Aug 2021 04:13:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment