Change in ...coreboot[master]: UPSTREAM: mb/google/octopus: Add custom SAR values for Laser
Peichao Li has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32296
Change subject: UPSTREAM: mb/google/octopus: Add custom SAR values for Laser ......................................................................
UPSTREAM: mb/google/octopus: Add custom SAR values for Laser
Laser would prefer to use different SAR values. Since Laser sku id is 5.
BUG=b:130381493 BRANCH=octopus TEST=build
Change-Id: I5cce38a191edfb235e274db3c788c58b65e0ebe1 --- M src/mainboard/google/octopus/variants/phaser/Makefile.inc A src/mainboard/google/octopus/variants/phaser/mainboard.c 2 files changed, 35 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/32296/1
diff --git a/src/mainboard/google/octopus/variants/phaser/Makefile.inc b/src/mainboard/google/octopus/variants/phaser/Makefile.inc index d54ed40..37270eb 100644 --- a/src/mainboard/google/octopus/variants/phaser/Makefile.inc +++ b/src/mainboard/google/octopus/variants/phaser/Makefile.inc @@ -2,3 +2,4 @@
ramstage-y += variant.c ramstage-y += gpio.c +ramstage-y += mainboard.c diff --git a/src/mainboard/google/octopus/variants/phaser/mainboard.c b/src/mainboard/google/octopus/variants/phaser/mainboard.c new file mode 100644 index 0000000..c0b496f --- /dev/null +++ b/src/mainboard/google/octopus/variants/phaser/mainboard.c @@ -0,0 +1,34 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2018 Google LLC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <boardid.h> +#include <ec/google/chromeec/ec.h> +#include <sar.h> + +const char *get_wifi_sar_cbfs_filename(void) +{ + const char *filename = NULL; + uint32_t sku_id; + if (google_chromeec_cbi_get_sku_id(&sku_id)) + return NULL; + + switch (sku_id) { + case 5: + filename = "wifi_sar-laser.hex"; + break; + } + return filename; +} +
Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: UPSTREAM: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Patch Set 1: Code-Review+1
Hi All,
I have add new SAR just for Laser. Please approve. Thanks a lot!
Hello Xingyu Wu, Jett Rink, Justin TerAvest, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32296
to look at the new patch set (#2).
Change subject: UPSTREAM: mb/google/octopus: Add custom SAR values for Laser ......................................................................
UPSTREAM: mb/google/octopus: Add custom SAR values for Laser
Laser would prefer to use different SAR values. Since Laser sku id is 5.
BUG=b:130381493 BRANCH=octopus TEST=build
Signed-off-by: peichao.wang peichao.wang@bitland.corp-partner.google.com Change-Id: I5cce38a191edfb235e274db3c788c58b65e0ebe1 --- M src/mainboard/google/octopus/variants/phaser/Makefile.inc A src/mainboard/google/octopus/variants/phaser/mainboard.c 2 files changed, 35 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/32296/2
Hello Xingyu Wu, Jett Rink, Justin TerAvest, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32296
to look at the new patch set (#3).
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
mb/google/octopus: Add custom SAR values for Laser
Laser would prefer to use different SAR values. Since Laser sku id is 5.
BUG=b:130381493 BRANCH=octopus TEST=build
Signed-off-by: peichao.wang peichao.wang@bitland.corp-partner.google.com Change-Id: I5cce38a191edfb235e274db3c788c58b65e0ebe1 --- M src/mainboard/google/octopus/variants/phaser/Makefile.inc A src/mainboard/google/octopus/variants/phaser/mainboard.c 2 files changed, 35 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/32296/3
Hello Xingyu Wu, Jett Rink, Justin TerAvest, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32296
to look at the new patch set (#4).
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
mb/google/octopus: Add custom SAR values for Laser
Laser would prefer to use different SAR values. Since Laser sku id is 5.
BUG=b:130381493 BRANCH=octopus TEST=build
Signed-off-by: peichao.wang <peichao.wang@bitland.corp -partner.google.com> Change-Id: I5cce38a191edfb235e274db3c788c58b65e0ebe1 --- M src/mainboard/google/octopus/variants/phaser/Makefile.inc A src/mainboard/google/octopus/variants/phaser/mainboard.c 2 files changed, 35 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/32296/4
Hello Xingyu Wu, Jett Rink, Justin TerAvest, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32296
to look at the new patch set (#5).
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
mb/google/octopus: Add custom SAR values for Laser
Laser would prefer to use different SAR values. Since Laser sku id is 5.
BUG=b:130381493 BRANCH=octopus TEST=build
Change-Id: I5cce38a191edfb235e274db3c788c58b65e0ebe1 Signed-off-by: peichao.wang <peichao.wang@bitland.corp -partner.google.com> --- M src/mainboard/google/octopus/variants/phaser/Makefile.inc A src/mainboard/google/octopus/variants/phaser/mainboard.c 2 files changed, 35 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/32296/5
Hello Xingyu Wu, Jett Rink, Justin TerAvest, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32296
to look at the new patch set (#6).
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
mb/google/octopus: Add custom SAR values for Laser
Laser would prefer to use different SAR values. Since Laser sku id is 5.
BUG=b:130381493 BRANCH=octopus TEST=build
Change-Id: I5cce38a191edfb235e274db3c788c58b65e0ebe1 Signed-off-by: peichao.wang peichao.wang@bitland.corp-partner.google.com --- M src/mainboard/google/octopus/variants/phaser/Makefile.inc A src/mainboard/google/octopus/variants/phaser/mainboard.c 2 files changed, 35 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/32296/6
Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Patch Set 4:
Looks like there is a common way to get sar file already and please refer to https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-ov...
Peichao Li has removed Patrick Georgi from this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Removed reviewer Patrick Georgi.
Peichao Li has removed Martin Roth from this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Removed reviewer Martin Roth.
Peichao Li has removed Furquan Shaikh from this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Removed reviewer Furquan Shaikh.
Peichao Li has removed Justin TerAvest from this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Removed reviewer Justin TerAvest.
Peichao Li has removed Jett Rink from this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Removed reviewer Jett Rink.
Peichao Li has removed Xingyu Wu from this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Removed reviewer Xingyu Wu.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Patch Set 6:
Patch Set 4:
Looks like there is a common way to get sar file already and please refer to https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-ov...
So, laser requires the SAR file, but phaser does not?
Hello Karthik Ramasubramanian, Justin TerAvest, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32296
to look at the new patch set (#7).
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
mb/google/octopus: Add custom SAR values for Laser
Laser would prefer to use different SAR values. Since Laser sku id is 5.
BUG=b:130381493 BRANCH=octopus TEST=build
Change-Id: I5cce38a191edfb235e274db3c788c58b65e0ebe1 Signed-off-by: peichao.wang peichao.wang@bitland.corp-partner.google.com --- M src/mainboard/google/octopus/variants/phaser/Makefile.inc A src/mainboard/google/octopus/variants/phaser/mainboard.c 2 files changed, 35 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/32296/7
Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Patch Set 7:
oh, Furquan is correct and it seems the sar file here is for a specific SKU ID only.
Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Patch Set 7:
Patch Set 6:
Patch Set 4:
Looks like there is a common way to get sar file already and please refer to https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-ov...
So, laser requires the SAR file, but phaser does not?
Hi Furquan,
Yes, just Laser need customized SAR file. Got it let's take a look.
Thanks and best regards
Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Patch Set 7:
Patch Set 7:
Patch Set 6:
Patch Set 4:
Looks like there is a common way to get sar file already and please refer to https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-ov...
So, laser requires the SAR file, but phaser does not?
Hi Furquan,
Yes, just Laser need customized SAR file. Got it let's take a look.
Thanks and best regards
Hi Furquan,
I have check config.phaser, but if modify IFWI, both phaser and Laser project will be impacted. I mean Whether should create a new config file for laser like config.laser. Thanks a lot!
Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Patch Set 7:
Patch Set 7:
Patch Set 7:
Patch Set 6:
Patch Set 4:
Looks like there is a common way to get sar file already and please refer to https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-ov...
So, laser requires the SAR file, but phaser does not?
Hi Furquan,
Yes, just Laser need customized SAR file. Got it let's take a look.
Thanks and best regards
Hi Furquan,
I have check config.phaser, but if modify IFWI, both phaser and Laser project will be impacted. I mean Whether should create a new config file for laser like config.laser. Thanks a lot!
Hi Marco and Furquan,
This is a special case. I think we should apply SAR for Laser according to sku id. sku id 5 is laser, so i think config.phaser would not add this SAR.
Thanks and best regards
Hello Karthik Ramasubramanian, Justin TerAvest, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32296
to look at the new patch set (#8).
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
mb/google/octopus: Add custom SAR values for Laser
Laser would prefer to use different SAR values. Since Laser sku id is 5.
BUG=b:130381493 BRANCH=octopus TEST=build
Change-Id: I5cce38a191edfb235e274db3c788c58b65e0ebe1 Signed-off-by: peichao.wang peichao.wang@bitland.corp-partner.google.com --- M src/mainboard/google/octopus/variants/phaser/Makefile.inc A src/mainboard/google/octopus/variants/phaser/mainboard.c 2 files changed, 35 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/32296/8
Hello Karthik Ramasubramanian, Justin TerAvest, Xingyu Wu, Marco Chen, Justin TerAvest, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32296
to look at the new patch set (#9).
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
mb/google/octopus: Add custom SAR values for Laser
Laser would prefer to use different SAR values. Since Laser sku id is 5.
BUG=b:130381493 BRANCH=octopus TEST=build
Change-Id: I5cce38a191edfb235e274db3c788c58b65e0ebe1 Signed-off-by: peichao.wang peichao.wang@bitland.corp-partner.google.com --- M src/mainboard/google/octopus/variants/phaser/Makefile.inc A src/mainboard/google/octopus/variants/phaser/mainboard.c 2 files changed, 35 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/32296/9
Hello Karthik Ramasubramanian, Justin TerAvest, Xingyu Wu, Marco Chen, Justin TerAvest, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32296
to look at the new patch set (#10).
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
mb/google/octopus: Add custom SAR values for Laser
Laser would prefer to use different SAR values. Since Laser sku id is 5.
BUG=b:130381493 BRANCH=octopus TEST=build
Change-Id: I5cce38a191edfb235e274db3c788c58b65e0ebe1 Signed-off-by: peichao.wang peichao.wang@bitland.corp-partner.google.com --- M src/mainboard/google/octopus/variants/phaser/Makefile.inc A src/mainboard/google/octopus/variants/phaser/mainboard.c 2 files changed, 35 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/32296/10
Hello Karthik Ramasubramanian, Justin TerAvest, Xingyu Wu, Marco Chen, Justin TerAvest, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32296
to look at the new patch set (#11).
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
mb/google/octopus: Add custom SAR values for Laser
Laser would prefer to use different SAR values. Since Laser sku id is 5.
BUG=b:130381493 BRANCH=octopus TEST=build
Change-Id: I5cce38a191edfb235e274db3c788c58b65e0ebe1 Signed-off-by: peichao.wang peichao.wang@bitland.corp-partner.google.com --- M src/mainboard/google/octopus/variants/phaser/Makefile.inc A src/mainboard/google/octopus/variants/phaser/mainboard.c 2 files changed, 33 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/32296/11
Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Patch Set 11: Code-Review+1
Patch Set 7:
Patch Set 7:
Patch Set 7:
Patch Set 6:
Patch Set 4:
Looks like there is a common way to get sar file already and please refer to https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-ov...
So, laser requires the SAR file, but phaser does not?
Hi Furquan,
Yes, just Laser need customized SAR file. Got it let's take a look.
Thanks and best regards
Hi Furquan,
I have check config.phaser, but if modify IFWI, both phaser and Laser project will be impacted. I mean Whether should create a new config file for laser like config.laser. Thanks a lot!
Hi Marco and Furquan,
This is a special case. I think we should apply SAR for Laser according to sku id. sku id 5 is laser, so i think config.phaser would not add this SAR.
Thanks and best regards
Hi Furquan and Marco,
I have modified the code make it adapt Laser machine. Please kindly help take a look.Thanks a lot!
Peichao Li has removed Justin TerAvest from this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Removed reviewer Justin TerAvest.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/#/c/32296/11/src/mainboard/google/octopus/varian... File src/mainboard/google/octopus/variants/phaser/mainboard.c:
https://review.coreboot.org/#/c/32296/11/src/mainboard/google/octopus/varian... PS11, Line 20: get_wifi_sar_cbfs_filename For phaser, this will end up returning NULL which will default to wifi_sar_defaults.hex. But since that file is not present in CBFS, it will end up complaining that the file could not be found and skip adding SAR to ACPI. I believe that is already happening because USE_SAR is selected for all octopus boards.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Patch Set 11: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/#/c/32296/11/src/mainboard/google/octopus/varian... File src/mainboard/google/octopus/variants/phaser/mainboard.c:
https://review.coreboot.org/#/c/32296/11/src/mainboard/google/octopus/varian... PS11, Line 20: get_wifi_sar_cbfs_filename
For phaser, this will end up returning NULL which will default to wifi_sar_defaults.hex. […]
The only difference from before for phaser would be 2 additional print messages. i.e. before it would have been:
Error: Could not locate 'wifi_sar' in VPD. Error: failed from getting SAR limits!
and now it would be: Error: Could not locate 'wifi_sar' in VPD. Checking CBFS for default SAR values wifi_sar_defaults.hex has bad len in CBFS. Error: failed from getting SAR limits!
If we want to really avoid these, we can add a runtime check mb_has_sar() which can be implemented by phaser to return 0 when sku_id != 5. Justin, what do you think?
Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Patch Set 11:
Patch Set 11:
(1 comment)
Hi Furquan,
Could you help merge it into coreboot org?
Thanks and best regards
Furquan Shaikh has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
mb/google/octopus: Add custom SAR values for Laser
Laser would prefer to use different SAR values. Since Laser sku id is 5.
BUG=b:130381493 BRANCH=octopus TEST=build
Change-Id: I5cce38a191edfb235e274db3c788c58b65e0ebe1 Signed-off-by: peichao.wang peichao.wang@bitland.corp-partner.google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32296 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/octopus/variants/phaser/Makefile.inc A src/mainboard/google/octopus/variants/phaser/mainboard.c 2 files changed, 33 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Peichao Li: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/octopus/variants/phaser/Makefile.inc b/src/mainboard/google/octopus/variants/phaser/Makefile.inc index d54ed40..37270eb 100644 --- a/src/mainboard/google/octopus/variants/phaser/Makefile.inc +++ b/src/mainboard/google/octopus/variants/phaser/Makefile.inc @@ -2,3 +2,4 @@
ramstage-y += variant.c ramstage-y += gpio.c +ramstage-y += mainboard.c diff --git a/src/mainboard/google/octopus/variants/phaser/mainboard.c b/src/mainboard/google/octopus/variants/phaser/mainboard.c new file mode 100644 index 0000000..2d44830 --- /dev/null +++ b/src/mainboard/google/octopus/variants/phaser/mainboard.c @@ -0,0 +1,32 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2018 Google LLC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <boardid.h> +#include <ec/google/chromeec/ec.h> +#include <sar.h> + +const char *get_wifi_sar_cbfs_filename(void) +{ + const char *filename = NULL; + uint32_t sku_id; + + if (google_chromeec_cbi_get_sku_id(&sku_id)) + return NULL; + + if (sku_id == 5) + filename = "wifi_sar-laser.hex"; + + return filename; +}
Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32296 )
Change subject: mb/google/octopus: Add custom SAR values for Laser ......................................................................
Patch Set 12:
Hi Furquan,
Please kindly cherry-pick it into chromium master branch. Thanks a lot!
Best regards
-
Furquan Shaikh (Code Review) -
Marco Chen (Code Review)
-
Peichao Li (Code Review)