EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Add Wifi SAR table for LTE sku ......................................................................
mb/google/octopus/var/fleex: Add Wifi SAR table for LTE sku
Add wifi sar table for LTE sku.
BUG=b:169115341 TEST=Check new SAR table can be loaded with sku id 4.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I086fa14a9f23e4a0fc0ef8085040219c932dbf17 --- M src/mainboard/google/octopus/variants/fleex/variant.c 1 file changed, 13 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/45640/1
diff --git a/src/mainboard/google/octopus/variants/fleex/variant.c b/src/mainboard/google/octopus/variants/fleex/variant.c index 52f822a..de1e08d 100644 --- a/src/mainboard/google/octopus/variants/fleex/variant.c +++ b/src/mainboard/google/octopus/variants/fleex/variant.c @@ -1,10 +1,11 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <acpi/acpi.h> -#include <ec/google/chromeec/ec.h> #include <baseboard/variants.h> #include <delay.h> +#include <ec/google/chromeec/ec.h> #include <gpio.h> +#include <sar.h>
void variant_smi_sleep(u8 slp_typ) { @@ -15,3 +16,14 @@
power_off_lte_module(); } + +const char *get_wifi_sar_cbfs_filename(void) +{ + const char *filename = NULL; + uint32_t sku_id = google_chromeec_get_board_sku(); + + if (sku_id == 4) + filename = "wifi_sar-fleex-lte.hex"; + + return filename; +}
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Add Wifi SAR table for LTE sku ......................................................................
Patch Set 1:
This change is ready for review.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Add Wifi SAR table for LTE sku ......................................................................
Patch Set 1:
Will upload the private SAR table hex file later. Please help merge this due to the schedule requirement.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Add Wifi SAR table for LTE sku ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45640/1/src/mainboard/google/octopu... File src/mainboard/google/octopus/variants/fleex/variant.c:
https://review.coreboot.org/c/coreboot/+/45640/1/src/mainboard/google/octopu... PS1, Line 25: if (sku_id == 4) From issue's comment, should change to if (sku_id != 4) return "fleex-sar.hex"
Hello build bot (Jenkins), Furquan Shaikh, Henry Sun, Marco Chen, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45640
to look at the new patch set (#2).
Change subject: mb/google/octopus/var/fleex: Add Wifi SAR table for LTE sku ......................................................................
mb/google/octopus/var/fleex: Add Wifi SAR table for LTE sku
Add wifi sar table for LTE sku.
BUG=b:169115341 TEST=Check new SAR table can be loaded with sku id 4.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I086fa14a9f23e4a0fc0ef8085040219c932dbf17 --- M src/mainboard/google/octopus/variants/fleex/variant.c 1 file changed, 13 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/45640/2
Hello build bot (Jenkins), Furquan Shaikh, Henry Sun, Marco Chen, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45640
to look at the new patch set (#3).
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku
Uses Wifi SAR table for non-LTE sku.
BUG=b:169115341 TEST=Check no SAR table can be loaded with sku id 4.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I086fa14a9f23e4a0fc0ef8085040219c932dbf17 --- M src/mainboard/google/octopus/variants/fleex/variant.c 1 file changed, 13 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/45640/3
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45640/1/src/mainboard/google/octopu... File src/mainboard/google/octopus/variants/fleex/variant.c:
https://review.coreboot.org/c/coreboot/+/45640/1/src/mainboard/google/octopu... PS1, Line 25: if (sku_id == 4)
From issue's comment, should change to […]
Done
Ivy Jian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
Patch Set 3: Code-Review+2
Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
Patch Set 3: Code-Review+2
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... File src/mainboard/google/octopus/variants/fleex/variant.c:
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... PS4, Line 25: 4 If you can define a macro instead of magic number, it will be more self-explanatory.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... File src/mainboard/google/octopus/variants/fleex/variant.c:
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... PS4, Line 25: 4
If you can define a macro instead of magic number, it will be more self-explanatory.
sounds good.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... File src/mainboard/google/octopus/variants/fleex/variant.c:
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... PS4, Line 25: 4
sounds good.
will fixed in this CL. https://review.coreboot.org/c/coreboot/+/45687/3
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... File src/mainboard/google/octopus/variants/fleex/variant.c:
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... PS4, Line 25: 4
will fixed in this CL. […]
Sorry for a little back and forth on this. I dont know if there is a different SAR for LTE SKU.
Returning NULL will cause src/vendorcode/google/chromeos/sar.c to pick up wifi_sar_defaults.hex file.
wifi_sar_defaults.hex is built from CONFIG_WIFI_SAR_CBFS_FILEPATH which is wifi_sar-fleex.hex. So the SAR file will end up to be the same for both LTE and non-LTE SKUs.
Can you please double-confirm and correct me if I am wrong.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... File src/mainboard/google/octopus/variants/fleex/variant.c:
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... PS4, Line 25: 4
Sorry for a little back and forth on this. I dont know if there is a different SAR for LTE SKU. […]
I have some CL for remove the default Sar table :) I can add you as reviewer for all relative CL.
Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... File src/mainboard/google/octopus/variants/fleex/variant.c:
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... PS4, Line 25: 4
I have some CL for remove the default Sar table :) I can add you as reviewer for all relative CL.
Yes, please refer to associated issue - b/169115341#comment11 for the relevant discussion and CL:2426124 is corresponding change in config.fleex.
Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/45640/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45640/4//COMMIT_MSG@11 PS4, Line 11: BUG=b:169115341 We also need BRANCH info
Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... File src/mainboard/google/octopus/variants/fleex/variant.c:
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... PS4, Line 25: 4
Yes, please refer to associated issue - b/169115341#comment11 for the relevant discussion and CL:242 […]
By the way, you need to mark it as resolved.
Hello build bot (Jenkins), Furquan Shaikh, Henry Sun, Marco Chen, Marco Chen, Ivy Jian, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45640
to look at the new patch set (#5).
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku
Uses Wifi SAR table for non-LTE sku.
BUG=b:169115341 BRANCH=octopus TEST=Check no SAR table can be loaded with sku id 4.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I086fa14a9f23e4a0fc0ef8085040219c932dbf17 --- M src/mainboard/google/octopus/variants/fleex/variant.c 1 file changed, 13 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/45640/5
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... File src/mainboard/google/octopus/variants/fleex/variant.c:
https://review.coreboot.org/c/coreboot/+/45640/4/src/mainboard/google/octopu... PS4, Line 25: 4
By the way, you need to mark it as resolved.
Thanks for the remind.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45640/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45640/4//COMMIT_MSG@11 PS4, Line 11: BUG=b:169115341
We also need BRANCH info
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Uses Wifi SAR table for non-LTE sku ......................................................................
Patch Set 5: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/45640/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45640/5//COMMIT_MSG@7 PS5, Line 7: Uses nit: Use
https://review.coreboot.org/c/coreboot/+/45640/5//COMMIT_MSG@9 PS5, Line 9: for non-LTE sku. Maybe: "for non-LTE sku only."
https://review.coreboot.org/c/coreboot/+/45640/5//COMMIT_MSG@9 PS5, Line 9: Uses nit: Use
Hello build bot (Jenkins), Furquan Shaikh, Henry Sun, Angel Pons, Marco Chen, Marco Chen, Ivy Jian, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45640
to look at the new patch set (#6).
Change subject: mb/google/octopus/var/fleex: Use Wifi SAR table for non-LTE sku only ......................................................................
mb/google/octopus/var/fleex: Use Wifi SAR table for non-LTE sku only
Use Wifi SAR table for non-LTE sku only.
BUG=b:169115341 BRANCH=octopus TEST=Check no SAR table can be loaded with sku id 4.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I086fa14a9f23e4a0fc0ef8085040219c932dbf17 --- M src/mainboard/google/octopus/variants/fleex/variant.c 1 file changed, 13 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/45640/6
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Use Wifi SAR table for non-LTE sku only ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45640/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45640/5//COMMIT_MSG@7 PS5, Line 7: Uses
nit: Use
Done
https://review.coreboot.org/c/coreboot/+/45640/5//COMMIT_MSG@9 PS5, Line 9: Uses
nit: Use
Done
https://review.coreboot.org/c/coreboot/+/45640/5//COMMIT_MSG@9 PS5, Line 9: for non-LTE sku.
Maybe: "for non-LTE sku only. […]
Done
Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Use Wifi SAR table for non-LTE sku only ......................................................................
Patch Set 6: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45640 )
Change subject: mb/google/octopus/var/fleex: Use Wifi SAR table for non-LTE sku only ......................................................................
mb/google/octopus/var/fleex: Use Wifi SAR table for non-LTE sku only
Use Wifi SAR table for non-LTE sku only.
BUG=b:169115341 BRANCH=octopus TEST=Check no SAR table can be loaded with sku id 4.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I086fa14a9f23e4a0fc0ef8085040219c932dbf17 Reviewed-on: https://review.coreboot.org/c/coreboot/+/45640 Reviewed-by: Marco Chen marcochen@google.com Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Ivy Jian ivy_jian@compal.corp-partner.google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/octopus/variants/fleex/variant.c 1 file changed, 13 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Marco Chen: Looks good to me, approved Ivy Jian: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/octopus/variants/fleex/variant.c b/src/mainboard/google/octopus/variants/fleex/variant.c index 52f822a..1b0784c 100644 --- a/src/mainboard/google/octopus/variants/fleex/variant.c +++ b/src/mainboard/google/octopus/variants/fleex/variant.c @@ -1,10 +1,11 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <acpi/acpi.h> -#include <ec/google/chromeec/ec.h> #include <baseboard/variants.h> #include <delay.h> +#include <ec/google/chromeec/ec.h> #include <gpio.h> +#include <sar.h>
void variant_smi_sleep(u8 slp_typ) { @@ -15,3 +16,14 @@
power_off_lte_module(); } + +const char *get_wifi_sar_cbfs_filename(void) +{ + const char *filename = NULL; + uint32_t sku_id = google_chromeec_get_board_sku(); + + if (sku_id != 4) + filename = "wifi_sar-fleex.hex"; + + return filename; +}
-
Angel Pons (Code Review) -
EricR Lai (Code Review)
-
Ivy Jian (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Marco Chen (Code Review)
-
Patrick Georgi (Code Review)