Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39872 )
Change subject: mb/google/cyan: Clean up Kconfig ......................................................................
mb/google/cyan: Clean up Kconfig
Cyan has no VGA BIOS available (at least not publicly), so remove related options. Disable SoC serial output by default, since no production devices have this exposed, but leave it as a user option so it can be selected as needed (eg, for use with a Google debug servo).
Change-Id: Ic079a39ca5ad0ac653b52248244b94d4bfbd08a4 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/mainboard/google/cyan/Kconfig 1 file changed, 5 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/39872/1
diff --git a/src/mainboard/google/cyan/Kconfig b/src/mainboard/google/cyan/Kconfig index 94ffbc0..257034d 100644 --- a/src/mainboard/google/cyan/Kconfig +++ b/src/mainboard/google/cyan/Kconfig @@ -6,7 +6,6 @@ select EC_GOOGLE_CHROMEEC_LPC select EC_GOOGLE_CHROMEEC_MEC select EC_GOOGLE_CHROMEEC_ACPI_MEMMAP - select ENABLE_BUILTIN_COM1 select HAVE_ACPI_TABLES select HAVE_OPTION_TABLE select INTEL_GMA_HAVE_VBT @@ -73,23 +72,6 @@ string default "variants/$(CONFIG_VARIANT_DIR)/devicetree.cb"
-config VGA_BIOS_FILE - string - depends on VGA_BIOS - default "3rdparty/blobs/mainboard/intel/strago/vgabios.bin" - help - The C0 version of the video BIOS gets computed from this name - so that they can both be added. Only the correct one for the - system will be run. - -config VGA_BIOS_ID - string - depends on VGA_BIOS - default "8086,22b0" - help - The VGA_BIOS_ID for the C0 version of the video BIOS is hardcoded - in soc/intel/braswell/Makefile.inc as 8086,22b1 - config CBFS_SIZE hex default 0x200000 @@ -98,4 +80,9 @@ string default "GOOGLE"
+config DRIVERS_UART_8250IO + bool + default n + default y if ENABLE_BUILTIN_COM1 + endif # BOARD_GOOGLE_BASEBOARD_CYAN
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39872 )
Change subject: mb/google/cyan: Clean up Kconfig ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39872/1/src/mainboard/google/cyan/K... File src/mainboard/google/cyan/Kconfig:
https://review.coreboot.org/c/coreboot/+/39872/1/src/mainboard/google/cyan/K... PS1, Line 85: config VGA_BIOS_ID : string : depends on VGA_BIOS : default "8086,22b0" : help : The VGA_BIOS_ID for the C0 version of the video BIOS is hardcoded : in soc/intel/braswell/Makefile.inc as 8086,22b1 Why drop this? it's still correct for people that have (extracted) the blob?
https://review.coreboot.org/c/coreboot/+/39872/1/src/mainboard/google/cyan/K... PS1, Line 86: default y if ENABLE_BUILTIN_COM1 If I wanted to enable the serial console, I would probably look in the "Console" menu... so how about inverting this:
config CONSOLE_SERIAL default n
config ENABLE_BUILTIN_COM1 default y if CONSOLE_SERIAL
That would leave us with a spurious DRIVERS_UART_8250IO in the default config, but it shouldn't hurt I guess.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39872
to look at the new patch set (#2).
Change subject: mb/google/cyan: Clean up Kconfig ......................................................................
mb/google/cyan: Clean up Kconfig
Cyan has no VGA BIOS available (at least not publicly), so remove related options. Disable SoC serial output by default, since no production devices have this exposed, but leave it as a user option so it can be selected as needed (eg, for use with a Google debug servo).
Change-Id: Ic079a39ca5ad0ac653b52248244b94d4bfbd08a4 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/mainboard/google/cyan/Kconfig 1 file changed, 6 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/39872/2
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39872 )
Change subject: mb/google/cyan: Clean up Kconfig ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39872/1/src/mainboard/google/cyan/K... File src/mainboard/google/cyan/Kconfig:
https://review.coreboot.org/c/coreboot/+/39872/1/src/mainboard/google/cyan/K... PS1, Line 85: config VGA_BIOS_ID : string : depends on VGA_BIOS : default "8086,22b0" : help : The VGA_BIOS_ID for the C0 version of the video BIOS is hardcoded : in soc/intel/braswell/Makefile.inc as 8086,22b1
Why drop this? it's still correct for people that have (extracted) the blob?
extracted from where? it doesn't exist in the stock firmware images. Only place it might exist are internally at Google/Intel. I guess it doesn't really hurt, just seems like rot when there isn't a VBIOS file to use.
https://review.coreboot.org/c/coreboot/+/39872/1/src/mainboard/google/cyan/K... PS1, Line 86: default y if ENABLE_BUILTIN_COM1
If I wanted to enable the serial console, I would probably look in the "Console" […]
Ack
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39872 )
Change subject: mb/google/cyan: Clean up Kconfig ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/39872/1/src/mainboard/google/cyan/K... File src/mainboard/google/cyan/Kconfig:
https://review.coreboot.org/c/coreboot/+/39872/1/src/mainboard/google/cyan/K... PS1, Line 85: config VGA_BIOS_ID : string : depends on VGA_BIOS : default "8086,22b0" : help : The VGA_BIOS_ID for the C0 version of the video BIOS is hardcoded : in soc/intel/braswell/Makefile.inc as 8086,22b1
extracted from where? it doesn't exist in the stock firmware images. […]
Hmm, judging by this C0 story, I guess the number belongs into braswell/Kconfig anyway.
Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39872 )
Change subject: mb/google/cyan: Clean up Kconfig ......................................................................
mb/google/cyan: Clean up Kconfig
Cyan has no VGA BIOS available (at least not publicly), so remove related options. Disable SoC serial output by default, since no production devices have this exposed, but leave it as a user option so it can be selected as needed (eg, for use with a Google debug servo).
Change-Id: Ic079a39ca5ad0ac653b52248244b94d4bfbd08a4 Signed-off-by: Matt DeVillier matt.devillier@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39872 Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/cyan/Kconfig 1 file changed, 6 insertions(+), 18 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/mainboard/google/cyan/Kconfig b/src/mainboard/google/cyan/Kconfig index 94ffbc0..4561054 100644 --- a/src/mainboard/google/cyan/Kconfig +++ b/src/mainboard/google/cyan/Kconfig @@ -6,7 +6,6 @@ select EC_GOOGLE_CHROMEEC_LPC select EC_GOOGLE_CHROMEEC_MEC select EC_GOOGLE_CHROMEEC_ACPI_MEMMAP - select ENABLE_BUILTIN_COM1 select HAVE_ACPI_TABLES select HAVE_OPTION_TABLE select INTEL_GMA_HAVE_VBT @@ -73,23 +72,6 @@ string default "variants/$(CONFIG_VARIANT_DIR)/devicetree.cb"
-config VGA_BIOS_FILE - string - depends on VGA_BIOS - default "3rdparty/blobs/mainboard/intel/strago/vgabios.bin" - help - The C0 version of the video BIOS gets computed from this name - so that they can both be added. Only the correct one for the - system will be run. - -config VGA_BIOS_ID - string - depends on VGA_BIOS - default "8086,22b0" - help - The VGA_BIOS_ID for the C0 version of the video BIOS is hardcoded - in soc/intel/braswell/Makefile.inc as 8086,22b1 - config CBFS_SIZE hex default 0x200000 @@ -98,4 +80,10 @@ string default "GOOGLE"
+config CONSOLE_SERIAL + default n + +config ENABLE_BUILTIN_COM1 + default y if CONSOLE_SERIAL + endif # BOARD_GOOGLE_BASEBOARD_CYAN
-
Matt DeVillier (Code Review) -
Nico Huber (Code Review)