Change in coreboot[master]: mb/google/nightfury: Override VBT selection for nightfury 2nd sku
Hello SH Kim,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/42904
to review the following change.
Change subject: mb/google/nightfury: Override VBT selection for nightfury 2nd sku ......................................................................
mb/google/nightfury: Override VBT selection for nightfury 2nd sku
Override VBT for nightfury SKU_ID = 2 to support different panel.
BUG=b:159051021 BRANCH=firmware-hatch-12672.B TEST=Built and verified using different VBT by SKU_ID
Change-Id: I9450814aadc43cc7991457c3793f109b889186b9 Signed-off-by: Seunghwan Kim sh_.kim@samsung.corp-partner.google.com --- M src/mainboard/google/hatch/variants/nightfury/Makefile.inc A src/mainboard/google/hatch/variants/nightfury/variant.c 2 files changed, 17 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/42904/1
diff --git a/src/mainboard/google/hatch/variants/nightfury/Makefile.inc b/src/mainboard/google/hatch/variants/nightfury/Makefile.inc index 7799cd2..05f77ad 100644 --- a/src/mainboard/google/hatch/variants/nightfury/Makefile.inc +++ b/src/mainboard/google/hatch/variants/nightfury/Makefile.inc @@ -10,3 +10,4 @@
ramstage-y += gpio.c ramstage-y += ramstage.c +ramstage-y += variant.c diff --git a/src/mainboard/google/hatch/variants/nightfury/variant.c b/src/mainboard/google/hatch/variants/nightfury/variant.c new file mode 100644 index 0000000..8fede2c --- /dev/null +++ b/src/mainboard/google/hatch/variants/nightfury/variant.c @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <baseboard/variants.h> +#include <chip.h> +#include <ec/google/chromeec/ec.h> +#include <drivers/intel/gma/opregion.h> + +const char *mainboard_vbt_filename(void) +{ + uint32_t sku_id = google_chromeec_get_board_sku(); + + if (sku_id == 2) + return "vbt-nightfury-qled.bin"; + else + return "vbt.bin"; +}
Bob Moragues has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42904 )
Change subject: mb/google/nightfury: Override VBT selection for nightfury 2nd sku ......................................................................
Patch Set 1: Code-Review+1
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42904 )
Change subject: mb/google/nightfury: Override VBT selection for nightfury 2nd sku ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/42904/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/nightfury/variant.c:
https://review.coreboot.org/c/coreboot/+/42904/1/src/mainboard/google/hatch/... PS1, Line 10: uint32_t sku_id = google_chromeec_get_board_sku(); nit: const
shkim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42904 )
Change subject: mb/google/nightfury: Override VBT selection for nightfury 2nd sku ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42904/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/nightfury/variant.c:
https://review.coreboot.org/c/coreboot/+/42904/1/src/mainboard/google/hatch/... PS1, Line 10: uint32_t sku_id = google_chromeec_get_board_sku();
nit: const
There is nowhere using "const" for google_chromeec_get_board_sku() function in coreboot project, and the definition of the function doesn't have "const" prefix. Anything I'm missing? https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/src...
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42904 )
Change subject: mb/google/nightfury: Override VBT selection for nightfury 2nd sku ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42904/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/nightfury/variant.c:
https://review.coreboot.org/c/coreboot/+/42904/1/src/mainboard/google/hatch/... PS1, Line 10: uint32_t sku_id = google_chromeec_get_board_sku();
There is nowhere using "const" for google_chromeec_get_board_sku() function in coreboot project, and […]
It doesn't have to be const, it just lets the compiler know the variable cannot be modified.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42904 )
Change subject: mb/google/nightfury: Override VBT selection for nightfury 2nd sku ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42904/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/nightfury/variant.c:
https://review.coreboot.org/c/coreboot/+/42904/1/src/mainboard/google/hatch/... PS1, Line 10: uint32_t sku_id = google_chromeec_get_board_sku();
It doesn't have to be const, it just lets the compiler know the variable cannot be modified.
Sorry I meant `const uint32_t sku_id = google_chromeec_get_board_sku()`
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42904 )
Change subject: mb/google/nightfury: Override VBT selection for nightfury 2nd sku ......................................................................
mb/google/nightfury: Override VBT selection for nightfury 2nd sku
Override VBT for nightfury SKU_ID = 2 to support different panel.
BUG=b:159051021 BRANCH=firmware-hatch-12672.B TEST=Built and verified using different VBT by SKU_ID
Change-Id: I9450814aadc43cc7991457c3793f109b889186b9 Signed-off-by: Seunghwan Kim sh_.kim@samsung.corp-partner.google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42904 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Bob Moragues moragues@google.com Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/variants/nightfury/Makefile.inc A src/mainboard/google/hatch/variants/nightfury/variant.c 2 files changed, 17 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved Bob Moragues: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/hatch/variants/nightfury/Makefile.inc b/src/mainboard/google/hatch/variants/nightfury/Makefile.inc index 7799cd2..05f77ad 100644 --- a/src/mainboard/google/hatch/variants/nightfury/Makefile.inc +++ b/src/mainboard/google/hatch/variants/nightfury/Makefile.inc @@ -10,3 +10,4 @@
ramstage-y += gpio.c ramstage-y += ramstage.c +ramstage-y += variant.c diff --git a/src/mainboard/google/hatch/variants/nightfury/variant.c b/src/mainboard/google/hatch/variants/nightfury/variant.c new file mode 100644 index 0000000..8fede2c --- /dev/null +++ b/src/mainboard/google/hatch/variants/nightfury/variant.c @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <baseboard/variants.h> +#include <chip.h> +#include <ec/google/chromeec/ec.h> +#include <drivers/intel/gma/opregion.h> + +const char *mainboard_vbt_filename(void) +{ + uint32_t sku_id = google_chromeec_get_board_sku(); + + if (sku_id == 2) + return "vbt-nightfury-qled.bin"; + else + return "vbt.bin"; +}
-
Bob Moragues (Code Review) -
shkim (Code Review)
-
Tim Wawrzynczak (Code Review)