Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
payloads/seabios: Add Hardware IRQ Kconfig
Certain Intel SoC platforms require SeaBIOS' HARDWARE_IRQ option to be deselected in order for the platform to boot. Add a Kconfig to properly select the HARDWARE_IRQ enablement based on platform, and write to SeaBIOS' .config file in cases where it needs to be disabled.
Test: build/boot google/clapper (Baytrail) and google/cyan (Braswell), verify boards boot vs hanging at boot menu prompt.
Change-Id: I23e9b30d2d1042c86bd10f134d6fe361edaf8cb2 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M payloads/external/SeaBIOS/Kconfig M payloads/external/SeaBIOS/Makefile 2 files changed, 14 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/39869/1
diff --git a/payloads/external/SeaBIOS/Kconfig b/payloads/external/SeaBIOS/Kconfig index e816775..abe8815 100644 --- a/payloads/external/SeaBIOS/Kconfig +++ b/payloads/external/SeaBIOS/Kconfig @@ -51,6 +51,17 @@ variations during option ROM code execution. It is not known if all option ROMs will behave properly with this option.
+config SEABIOS_HARDWARE_IRQ + prompt "Hardware Interrupts" + default n if SOC_INTEL_BAYTRAIL || SOC_INTEL_BRASWELL || SOC_INTEL_APOLLOLAKE + default y + bool + help + Program and support hardware interrupts using the i8259 + programmable interrupt controller (PIC). This option should + be enabled for all platforms except for those which require + it to be disabled (eg, Baytrail/Braswell and successors) + config SEABIOS_VGA_COREBOOT prompt "Include generated option rom that implements legacy VGA BIOS compatibility" default y if !VENDOR_EMULATION diff --git a/payloads/external/SeaBIOS/Makefile b/payloads/external/SeaBIOS/Makefile index 0086775..cd646d9 100644 --- a/payloads/external/SeaBIOS/Makefile +++ b/payloads/external/SeaBIOS/Makefile @@ -72,6 +72,9 @@ ifneq ($(CONFIG_SEABIOS_DEBUG_LEVEL),-1) echo "CONFIG_DEBUG_LEVEL=$(CONFIG_SEABIOS_DEBUG_LEVEL)" >> seabios/.config endif +ifneq ($(CONFIG_SEABIOS_HARDWARE_IRQ),y) + echo "# CONFIG_HARDWARE_IRQ is not set" >> seabios/.config +endif # This shows how to force a previously set .config option *off* # echo "# CONFIG_SMBIOS is not set" >> seabios/.config $(MAKE) -C seabios olddefconfig OUT=out/
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 1:
I had a working SeaBIOS with hardware IRQs on Braswell and it didn't hang at all (default coreboot config for SeaBIOS + sercon port). Of course I had issues during the porting, but it was due to incorrect irqroutes at the beggining of porting IIRC. Minnowboard (Baytrail) also worked. I will revisit the bug I have encountered for Braswell to check what I did in order to get it working.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 1:
Patch Set 1:
I had a working SeaBIOS with hardware IRQs on Braswell and it didn't hang at all (default coreboot config for SeaBIOS + sercon port). Of course I had issues during the porting, but it was due to incorrect irqroutes at the beggining of porting IIRC. Minnowboard (Baytrail) also worked. I will revisit the bug I have encountered for Braswell to check what I did in order to get it working.
hmm, wonder if it's something specific to the Google boards then. I was testing current upstream master for coreboot and SeaBIOS, and both Baytrail/Braswell hung at the boot menu prompt without HARDWARE_IRQ deselected.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 1:
Do the Google Braswells still run the downstream blob?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/39869/1/payloads/external/SeaBIOS/K... File payloads/external/SeaBIOS/Kconfig:
https://review.coreboot.org/c/coreboot/+/39869/1/payloads/external/SeaBIOS/K... PS1, Line 63: Baytrail Bay Trail
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 1:
Patch Set 1:
Do the Google Braswells still run the downstream blob?
yes, cyan and variants need some UPDs set that don't exist in the upstream version
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 1: Code-Review+2
Hello build bot (Jenkins), Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39869
to look at the new patch set (#2).
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
payloads/seabios: Add Hardware IRQ Kconfig
Certain Intel SoC platforms require SeaBIOS' HARDWARE_IRQ option to be deselected in order for the platform to boot. Add a Kconfig to properly select the HARDWARE_IRQ enablement based on platform, and write to SeaBIOS' .config file in cases where it needs to be disabled.
Test: build/boot google/clapper (Bay Trail) and google/cyan (Braswell), verify boards boot vs hanging at boot menu prompt.
Change-Id: I23e9b30d2d1042c86bd10f134d6fe361edaf8cb2 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M payloads/external/SeaBIOS/Kconfig M payloads/external/SeaBIOS/Makefile 2 files changed, 14 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/39869/2
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39869/1/payloads/external/SeaBIOS/K... File payloads/external/SeaBIOS/Kconfig:
https://review.coreboot.org/c/coreboot/+/39869/1/payloads/external/SeaBIOS/K... PS1, Line 63: Baytrail
Bay Trail
Done
Hello build bot (Jenkins), Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39869
to look at the new patch set (#3).
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
payloads/seabios: Add Hardware IRQ Kconfig
Certain Intel SoC platforms require SeaBIOS' HARDWARE_IRQ option to be deselected in order for the platform to boot. Add a Kconfig to properly select the HARDWARE_IRQ enablement based on platform, and write to SeaBIOS' .config file in cases where it needs to be disabled.
Test: build/boot google/clapper (Bay Trail) and google/cyan (Braswell), verify boards boot vs hanging at boot menu prompt.
Change-Id: I23e9b30d2d1042c86bd10f134d6fe361edaf8cb2 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M payloads/external/SeaBIOS/Kconfig M payloads/external/SeaBIOS/Makefile 2 files changed, 14 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/39869/3
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 3: Code-Review+1
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 3: Code-Review-1
Unfortunately it breaks my Braswell boards. They are running fine with SeaBIOS on current master coreboot. They do not need disabling Hardware IRQ. Please make this selection only for mainboards that affect you.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 3:
Patch Set 3: Code-Review-1
Unfortunately it breaks my Braswell boards. They are running fine with SeaBIOS on current master coreboot. They do not need disabling Hardware IRQ. Please make this selection only for mainboards that affect you.
I know it is not hard select, however default configuration should work for all boards for given microarchitecture (which for Braswell is not true). If you think this is not the right approach then please at least set the HARDWARE_IRQ to default y for Protectli FW2B and FW4B
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 3:
Patch Set 3: Code-Review-1
Unfortunately it breaks my Braswell boards. They are running fine with SeaBIOS on current master coreboot. They do not need disabling Hardware IRQ. Please make this selection only for mainboards that affect you.
thanks for testing Michal, I Was under the impression it was a platform vs board issue. I'll adjust in the next patch set to only select for the google boards I have that need it
Hello build bot (Jenkins), Michał Żygowski, Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39869
to look at the new patch set (#4).
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
payloads/seabios: Add Hardware IRQ Kconfig
Certain boards require SeaBIOS' HARDWARE_IRQ option to be deselected in order for the platform to boot. Add a Kconfig to allow selection of HARDWARE_IRQ enablement, and write to SeaBIOS' .config file in cases where it needs to be disabled. Deselect the option for google/rambi variants so they boot with boards defaults.
Test: build/boot google/clapper, verify board boots vs hanging at boot menu prompt.
Change-Id: I23e9b30d2d1042c86bd10f134d6fe361edaf8cb2 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M payloads/external/SeaBIOS/Kconfig M payloads/external/SeaBIOS/Makefile M src/mainboard/google/rambi/Kconfig 3 files changed, 17 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/39869/4
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 4:
Patch Set 3:
Patch Set 3: Code-Review-1
Unfortunately it breaks my Braswell boards. They are running fine with SeaBIOS on current master coreboot. They do not need disabling Hardware IRQ. Please make this selection only for mainboards that affect you.
thanks for testing Michal, I Was under the impression it was a platform vs board issue. I'll adjust in the next patch set to only select for the google boards I have that need it
Let me know when you finish. I will give +2.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 4:
Patch Set 4:
Let me know when you finish. I will give +2.
I changed it to default to y and will override at the board level. re-testing, seems only baytrail/rambi needs it currently, not braswell.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 4: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39869/4/payloads/external/SeaBIOS/K... File payloads/external/SeaBIOS/Kconfig:
https://review.coreboot.org/c/coreboot/+/39869/4/payloads/external/SeaBIOS/K... PS4, Line 60: This option should : be enabled for all platforms/boards except for those which require : it to be disabled. Is this statement useful?
Maybe extend the description, that disabling this option might fix hanging SeaBIOS menu.
https://review.coreboot.org/c/coreboot/+/39869/4/src/mainboard/google/rambi/... File src/mainboard/google/rambi/Kconfig:
PS4: I’d prefer the board change in a separate commit.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39869/4/src/mainboard/google/rambi/... File src/mainboard/google/rambi/Kconfig:
PS4:
I’d prefer the board change in a separate commit.
What for?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39869/4/payloads/external/SeaBIOS/K... File payloads/external/SeaBIOS/Kconfig:
https://review.coreboot.org/c/coreboot/+/39869/4/payloads/external/SeaBIOS/K... PS4, Line 60: This option should : be enabled for all platforms/boards except for those which require : it to be disabled.
Is this statement useful? […]
"If it needs to be disabled, disable it" feels a bit tautological indeed.
https://review.coreboot.org/c/coreboot/+/39869/4/src/mainboard/google/rambi/... File src/mainboard/google/rambi/Kconfig:
PS4:
What for?
There's two conflicting goals here: 1. make a change well-isolated (so that we could revert rambi individually from the global HARDWARE_IRQ thing, for example), 2. don't add stuff to the tree that isn't used (e.g. having the HARDWARE_IRQ thing floating around without the rambi part merged for whatever reason)
IMNSHO for a change of this size it can go either way.
Hello build bot (Jenkins), Michał Żygowski, Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39869
to look at the new patch set (#5).
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
payloads/seabios: Add Hardware IRQ Kconfig
Certain boards require SeaBIOS' HARDWARE_IRQ option to be deselected in order for the platform to boot. Add a Kconfig to allow selection of HARDWARE_IRQ enablement, and write to SeaBIOS' .config file in cases where it needs to be disabled. Deselect the option for google/rambi variants so they boot with boards defaults.
Test: build/boot google/clapper, verify board boots vs hanging at boot menu prompt.
Change-Id: I23e9b30d2d1042c86bd10f134d6fe361edaf8cb2 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M payloads/external/SeaBIOS/Kconfig M payloads/external/SeaBIOS/Makefile M src/mainboard/google/rambi/Kconfig 3 files changed, 17 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/39869/5
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39869/4/payloads/external/SeaBIOS/K... File payloads/external/SeaBIOS/Kconfig:
https://review.coreboot.org/c/coreboot/+/39869/4/payloads/external/SeaBIOS/K... PS4, Line 60: This option should : be enabled for all platforms/boards except for those which require : it to be disabled.
"If it needs to be disabled, disable it" feels a bit tautological indeed.
better now?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 5: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/39869/4/payloads/external/SeaBIOS/K... File payloads/external/SeaBIOS/Kconfig:
https://review.coreboot.org/c/coreboot/+/39869/4/payloads/external/SeaBIOS/K... PS4, Line 60: This option should : be enabled for all platforms/boards except for those which require : it to be disabled.
better now?
Yes, thanks!
Hello build bot (Jenkins), Patrick Georgi, Michał Żygowski, Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39869
to look at the new patch set (#6).
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
payloads/seabios: Add Hardware IRQ Kconfig
Certain boards require SeaBIOS' HARDWARE_IRQ option to be deselected in order for the platform to boot. Add a Kconfig to allow selection of HARDWARE_IRQ enablement, and write to SeaBIOS' .config file in cases where it needs to be disabled. Deselect the option for google/rambi variants so they boot with boards defaults.
Test: build/boot google/clapper, verify board boots vs hanging at boot menu prompt.
Change-Id: I23e9b30d2d1042c86bd10f134d6fe361edaf8cb2 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M payloads/external/SeaBIOS/Kconfig M payloads/external/SeaBIOS/Makefile M src/mainboard/google/rambi/Kconfig 3 files changed, 18 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/39869/6
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 6: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/39869/4/src/mainboard/google/rambi/... File src/mainboard/google/rambi/Kconfig:
PS4:
There's two conflicting goals here: 1. […]
Angel, mainly because it’s not clear to me from the current commit message.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39869/4/src/mainboard/google/rambi/... File src/mainboard/google/rambi/Kconfig:
PS4:
Angel, mainly because it’s not clear to me from the current commit message.
Sorry, it was there already in the commit message (mixed it up with the TEST line (google/clapper)), and I meant commit messages summary. Anyway, also what Patrick responded.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
Patch Set 6: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39869 )
Change subject: payloads/seabios: Add Hardware IRQ Kconfig ......................................................................
payloads/seabios: Add Hardware IRQ Kconfig
Certain boards require SeaBIOS' HARDWARE_IRQ option to be deselected in order for the platform to boot. Add a Kconfig to allow selection of HARDWARE_IRQ enablement, and write to SeaBIOS' .config file in cases where it needs to be disabled. Deselect the option for google/rambi variants so they boot with boards defaults.
Test: build/boot google/clapper, verify board boots vs hanging at boot menu prompt.
Change-Id: I23e9b30d2d1042c86bd10f134d6fe361edaf8cb2 Signed-off-by: Matt DeVillier matt.devillier@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39869 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Angel Pons th3fanbus@gmail.com --- M payloads/external/SeaBIOS/Kconfig M payloads/external/SeaBIOS/Makefile M src/mainboard/google/rambi/Kconfig 3 files changed, 18 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved
diff --git a/payloads/external/SeaBIOS/Kconfig b/payloads/external/SeaBIOS/Kconfig index e816775..21e4720 100644 --- a/payloads/external/SeaBIOS/Kconfig +++ b/payloads/external/SeaBIOS/Kconfig @@ -51,6 +51,16 @@ variations during option ROM code execution. It is not known if all option ROMs will behave properly with this option.
+config SEABIOS_HARDWARE_IRQ + prompt "Hardware Interrupts" + default y + bool + help + Program and support hardware interrupts using the i8259 + programmable interrupt controller (PIC). Deselected by + boards which would otherwise hang at the boot menu (eg, + google/rambi). + config SEABIOS_VGA_COREBOOT prompt "Include generated option rom that implements legacy VGA BIOS compatibility" default y if !VENDOR_EMULATION diff --git a/payloads/external/SeaBIOS/Makefile b/payloads/external/SeaBIOS/Makefile index 0086775..cd646d9 100644 --- a/payloads/external/SeaBIOS/Makefile +++ b/payloads/external/SeaBIOS/Makefile @@ -72,6 +72,9 @@ ifneq ($(CONFIG_SEABIOS_DEBUG_LEVEL),-1) echo "CONFIG_DEBUG_LEVEL=$(CONFIG_SEABIOS_DEBUG_LEVEL)" >> seabios/.config endif +ifneq ($(CONFIG_SEABIOS_HARDWARE_IRQ),y) + echo "# CONFIG_HARDWARE_IRQ is not set" >> seabios/.config +endif # This shows how to force a previously set .config option *off* # echo "# CONFIG_SMBIOS is not set" >> seabios/.config $(MAKE) -C seabios olddefconfig OUT=out/ diff --git a/src/mainboard/google/rambi/Kconfig b/src/mainboard/google/rambi/Kconfig index bc4aa6e..3cb5e26 100644 --- a/src/mainboard/google/rambi/Kconfig +++ b/src/mainboard/google/rambi/Kconfig @@ -77,6 +77,11 @@ default "GOOGLE"
config CONSOLE_SERIAL + bool + default n + +config SEABIOS_HARDWARE_IRQ + bool default n
endif # BOARD_GOOGLE_BASEBOARD_RAMBI
-
Angel Pons (Code Review) -
Matt DeVillier (Code Review)
-
Michał Żygowski (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)