Change in coreboot[master]: mainboard/hatch/Kconfig: Add support for boot with tianocore payload
Meera Ravindranath has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36067 )
Change subject: mainboard/hatch/Kconfig: Add support for boot with tianocore payload ......................................................................
mainboard/hatch/Kconfig: Add support for boot with tianocore payload
Add new config and set the required GBB flags to support boot with tianocore payload.
BUG=none TEST=Allows boot to tianocore on pressing Ctrl+L in depthcharge.
Change-Id: I42fcf23523889d47f0490fbd662ca6b7587ab548 Signed-off-by: Meera Ravindranath meera.ravindranath@intel.com --- M src/mainboard/google/hatch/Kconfig 1 file changed, 8 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/36067/1
diff --git a/src/mainboard/google/hatch/Kconfig b/src/mainboard/google/hatch/Kconfig index 004cc28..246e258 100644 --- a/src/mainboard/google/hatch/Kconfig +++ b/src/mainboard/google/hatch/Kconfig @@ -33,10 +33,18 @@ select GBB_FLAG_FORCE_DEV_BOOT_USB select GBB_FLAG_FORCE_DEV_BOOT_LEGACY select GBB_FLAG_FORCE_MANUAL_RECOVERY + select GBB_FLAG_DISABLE_EC_SOFTWARE_SYNC if BOARD_USES_TIANOCORE + select GBB_FLAG_ENABLE_ALTERNATE_OS if BOARD_USES_TIANOCORE select HAS_RECOVERY_MRC_CACHE select MRC_CLEAR_NORMAL_CACHE_ON_RECOVERY_RETRAIN select VBOOT_LID_SWITCH
+config BOARD_USES_TIANOCORE + bool + default n + select USE_LEGACY_8254_TIMER + select USE_ACPI_PM_TIMER + config CHROMEOS_WIFI_SAR bool "Enable SAR options for Chrome OS build" depends on CHROMEOS
Meera Ravindranath has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/36067 )
Change subject: mainboard/google/hatch/Kconfig: Add support for boot with tianocore payload ......................................................................
mainboard/google/hatch/Kconfig: Add support for boot with tianocore payload
Add new config and set the required GBB flags to support boot with tianocore payload.
BUG=none TEST=Allows boot to tianocore on pressing Ctrl+L in depthcharge.
Change-Id: I42fcf23523889d47f0490fbd662ca6b7587ab548 Signed-off-by: Meera Ravindranath meera.ravindranath@intel.com --- M src/mainboard/google/hatch/Kconfig 1 file changed, 8 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/36067/2
Hello Aamir Bohra, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36067
to look at the new patch set (#3).
Change subject: mainboard/hatch/Kconfig: Add support for boot with tianocore payload ......................................................................
mainboard/hatch/Kconfig: Add support for boot with tianocore payload
Add new config and set the required GBB flags to support boot with tianocore payload.
BUG=none TEST=Allows boot to tianocore on pressing Ctrl L in depthcharge.
Change-Id: I42fcf23523889d47f0490fbd662ca6b7587ab548 Signed-off-by: Meera Ravindranath meera.ravindranath@intel.com --- M src/mainboard/google/hatch/Kconfig 1 file changed, 8 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/36067/3
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36067 )
Change subject: mainboard/hatch/Kconfig: Add support for boot with tianocore payload ......................................................................
Patch Set 3: Code-Review-1
(5 comments)
https://review.coreboot.org/c/coreboot/+/36067/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36067/3//COMMIT_MSG@9 PS3, Line 9: Add new config Why?
https://review.coreboot.org/c/coreboot/+/36067/3//COMMIT_MSG@9 PS3, Line 9: Add new config and set the required GBB flags to support boot with : tianocore payload Please explain, why these GBB flags are required? Why can’t TianoCore be fixed?
https://review.coreboot.org/c/coreboot/+/36067/3//COMMIT_MSG@13 PS3, Line 13: tianocore TianoCore
What version? How can it be loaded from depthcharge?
https://review.coreboot.org/c/coreboot/+/36067/3/src/mainboard/google/hatch/... File src/mainboard/google/hatch/Kconfig:
https://review.coreboot.org/c/coreboot/+/36067/3/src/mainboard/google/hatch/... PS3, Line 37: select GBB_FLAG_ENABLE_ALTERNATE_OS if BOARD_USES_TIANOCORE Why? What does the payload have to do with the OS?
https://review.coreboot.org/c/coreboot/+/36067/3/src/mainboard/google/hatch/... PS3, Line 46: select USE_ACPI_PM_TIMER I do not think, we want such a Kconfig option. The payload is user configurable, and there is already a Kconfig option, if TianoCore is selected as payload.
Meera Ravindranath has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36067 )
Change subject: mainboard/hatch/Kconfig: Add support for boot with tianocore payload ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36067/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36067/3//COMMIT_MSG@9 PS3, Line 9: Add new config
Why?
Boot to TianoCore requires the legacy 8254 clock gating to be disabled and the ACPI PM timer to be enabled.Hence creating this config to be able to easily provide a switch to boot TianoCore.
https://review.coreboot.org/c/coreboot/+/36067/3//COMMIT_MSG@9 PS3, Line 9: Add new config and set the required GBB flags to support boot with : tianocore payload
Please explain, why these GBB flags are required? Why can’t TianoCore be fixed?
Booting to Tianocore also requires GBB flags GBB_FLAG_ENABLE_ALTERNATE_OS and GBB_FLAG_DISABLE_EC_SOFTWARE_SYNC along with the 8254 clock gating and ACPI PM Timer
https://review.coreboot.org/c/coreboot/+/36067/3/src/mainboard/google/hatch/... File src/mainboard/google/hatch/Kconfig:
https://review.coreboot.org/c/coreboot/+/36067/3/src/mainboard/google/hatch/... PS3, Line 37: select GBB_FLAG_ENABLE_ALTERNATE_OS if BOARD_USES_TIANOCORE
Why? What does the payload have to do with the OS?
Accomodated here just to align with the other GBB flags.
https://review.coreboot.org/c/coreboot/+/36067/3/src/mainboard/google/hatch/... PS3, Line 46: select USE_ACPI_PM_TIMER
I do not think, we want such a Kconfig option. […]
Sure. Will explore that option too.
Hello Subrata Banik, Aamir Bohra, Paul Menzel, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36067
to look at the new patch set (#4).
Change subject: mainboard/hatch/Kconfig: Add support for boot with Tianocore payload ......................................................................
mainboard/hatch/Kconfig: Add support for boot with Tianocore payload
Add new config to support boot with Tianocore payload.
BUG=none TEST=Allows boot to tianocore on pressing Ctrl L in depthcharge.
Change-Id: I42fcf23523889d47f0490fbd662ca6b7587ab548 Signed-off-by: Meera Ravindranath meera.ravindranath@intel.com --- M src/mainboard/google/hatch/Kconfig 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/36067/4
Hello Subrata Banik, Aamir Bohra, Paul Menzel, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36067
to look at the new patch set (#5).
Change subject: mainboard/hatch/Kconfig: Add support for boot with Tianocore payload ......................................................................
mainboard/hatch/Kconfig: Add support for boot with Tianocore payload
Add new config to support boot with Tianocore payload.
BUG=none TEST=Allows boot to tianocore on pressing Ctrl L in depthcharge.
Change-Id: I42fcf23523889d47f0490fbd662ca6b7587ab548 Signed-off-by: Meera Ravindranath meera.ravindranath@intel.com --- M src/mainboard/google/hatch/Kconfig 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/36067/5
Meera Ravindranath has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36067 )
Change subject: mainboard/hatch/Kconfig: Add support for boot with Tianocore payload ......................................................................
Patch Set 5:
Hi Paul Menzel, I did explore the Kconfig option of PAYLOAD_TIANOCORE and it would still require the Pm timer and legacy 8254 timer to be enabled externally to support Tianocore boot. It would be difficult to modify the payload config and use it in our SoC code. Hence, we would like to have this BOARD_USES_TIANOCORE Kconfig in our mainboard which would default be set to No and can be selected only when the user needs it. However, I confirmed that the GBB flags are not required and hence modified the CL.
Kindly have a look.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36067 )
Change subject: mainboard/hatch/Kconfig: Add support for boot with Tianocore payload ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36067/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36067/5//COMMIT_MSG@7 PS5, Line 7: mainboard mb
https://review.coreboot.org/c/coreboot/+/36067/5//COMMIT_MSG@7 PS5, Line 7: Tianocore TianoCore is (unfortunately) the official spelling.
https://review.coreboot.org/c/coreboot/+/36067/5//COMMIT_MSG@9 PS5, Line 9: Tianocore Ditto.
https://review.coreboot.org/c/coreboot/+/36067/5/src/mainboard/google/hatch/... File src/mainboard/google/hatch/Kconfig:
https://review.coreboot.org/c/coreboot/+/36067/5/src/mainboard/google/hatch/... PS5, Line 44: select ENABLE_ACPI_PM_TIMER It does not look like the correct place and implementation to me. The payload is selected in the payload menu, and somehow the necessary changes should be done there (I think). At least you need to check somehow, if TianoCore was chosen as payload. I also do not think this is board specific, and the option should be added somewhere else.
Hello Subrata Banik, Aamir Bohra, Paul Menzel, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36067
to look at the new patch set (#6).
Change subject: mb/hatch/Kconfig: Add support for boot with TianoCore payload ......................................................................
mb/hatch/Kconfig: Add support for boot with TianoCore payload
Add new config to support boot with TianoCore payload.
BUG=none TEST=Allows boot to TianoCore on pressing Ctrl L in depthcharge.
Change-Id: I42fcf23523889d47f0490fbd662ca6b7587ab548 Signed-off-by: Meera Ravindranath meera.ravindranath@intel.com --- M src/mainboard/google/hatch/Kconfig 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/36067/6
Meera Ravindranath has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36067 )
Change subject: mb/hatch/Kconfig: Add support for boot with TianoCore payload ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36067/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36067/5//COMMIT_MSG@7 PS5, Line 7: Tianocore
TianoCore is (unfortunately) the official spelling.
Done
https://review.coreboot.org/c/coreboot/+/36067/5//COMMIT_MSG@7 PS5, Line 7: mainboard
mb
Done
https://review.coreboot.org/c/coreboot/+/36067/5//COMMIT_MSG@9 PS5, Line 9: Tianocore
Ditto.
Done
Meera Ravindranath has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/36067 )
Change subject: mb/hatch/Kconfig: Add support for boot with TianoCore payload ......................................................................
Abandoned
-
Meera Ravindranath (Code Review) -
Paul Menzel (Code Review)