Hello Patrick Georgi,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/45214
to review the following change.
Change subject: trogdor/sc7180: Clarify USE_QC_BLOBS requirements ......................................................................
trogdor/sc7180: Clarify USE_QC_BLOBS requirements
This patch adds some Kconfig hints to make it clearer that the USE_QC_BLOBS option is required for SC7180 boards and guide the user in the right direction through menuconfig. Also add those little arrows to the Trogdor board options that are there on most other boards.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I973cae8026a229408a1a1817c4808b0266387ea7 --- M src/mainboard/google/trogdor/Kconfig.name M src/soc/qualcomm/sc7180/Kconfig 2 files changed, 12 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/45214/1
diff --git a/src/mainboard/google/trogdor/Kconfig.name b/src/mainboard/google/trogdor/Kconfig.name index b7c03f9..4c1d351 100644 --- a/src/mainboard/google/trogdor/Kconfig.name +++ b/src/mainboard/google/trogdor/Kconfig.name @@ -1,17 +1,24 @@ comment "Trogdor"
+if USE_QC_BLOBS + config BOARD_GOOGLE_BUBS - bool "Bubs" + bool "-> Bubs" select BOARD_GOOGLE_TROGDOR_COMMON
config BOARD_GOOGLE_LAZOR - bool "Lazor" + bool "-> Lazor" select BOARD_GOOGLE_TROGDOR_COMMON
config BOARD_GOOGLE_POMPOM - bool "Pompom" + bool "-> Pompom" select BOARD_GOOGLE_TROGDOR_COMMON
config BOARD_GOOGLE_TROGDOR - bool "Trogdor" + bool "-> Trogdor" select BOARD_GOOGLE_TROGDOR_COMMON + +endif + +comment "(Trogdor requires 'Allow QC blobs repository')" + depends on !USE_QC_BLOBS diff --git a/src/soc/qualcomm/sc7180/Kconfig b/src/soc/qualcomm/sc7180/Kconfig index db7350f..488fec6 100644 --- a/src/soc/qualcomm/sc7180/Kconfig +++ b/src/soc/qualcomm/sc7180/Kconfig @@ -2,6 +2,7 @@ config SOC_QUALCOMM_SC7180 bool default n + depends on USE_QC_BLOBS select ARCH_BOOTBLOCK_ARMV8_64 select ARCH_RAMSTAGE_ARMV8_64 select ARCH_ROMSTAGE_ARMV8_64
Hello build bot (Jenkins), Patrick Georgi, Philip Chen,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45214
to look at the new patch set (#2).
Change subject: trogdor/sc7180: Clarify USE_QC_BLOBS requirements ......................................................................
trogdor/sc7180: Clarify USE_QC_BLOBS requirements
This patch adds some Kconfig hints to make it clearer that the USE_QC_BLOBS option is required for SC7180 boards and guide the user in the right direction through menuconfig. Also add those little arrows to the Trogdor board options that are there on most other boards.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I973cae8026a229408a1a1817c4808b0266387ea7 --- M src/mainboard/google/trogdor/Kconfig.name M src/soc/qualcomm/sc7180/Kconfig 2 files changed, 13 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/45214/2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45214 )
Change subject: trogdor/sc7180: Clarify USE_QC_BLOBS requirements ......................................................................
Patch Set 2:
*ping*
Can I get a +2 from anyone?
Philip Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45214 )
Change subject: trogdor/sc7180: Clarify USE_QC_BLOBS requirements ......................................................................
Patch Set 2: Code-Review+2
Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45214 )
Change subject: trogdor/sc7180: Clarify USE_QC_BLOBS requirements ......................................................................
trogdor/sc7180: Clarify USE_QC_BLOBS requirements
This patch adds some Kconfig hints to make it clearer that the USE_QC_BLOBS option is required for SC7180 boards and guide the user in the right direction through menuconfig. Also add those little arrows to the Trogdor board options that are there on most other boards.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I973cae8026a229408a1a1817c4808b0266387ea7 Reviewed-on: https://review.coreboot.org/c/coreboot/+/45214 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Philip Chen philipchen@google.com --- M src/mainboard/google/trogdor/Kconfig.name M src/soc/qualcomm/sc7180/Kconfig 2 files changed, 13 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Philip Chen: Looks good to me, approved
diff --git a/src/mainboard/google/trogdor/Kconfig.name b/src/mainboard/google/trogdor/Kconfig.name index ea12307..174c795 100644 --- a/src/mainboard/google/trogdor/Kconfig.name +++ b/src/mainboard/google/trogdor/Kconfig.name @@ -1,21 +1,28 @@ comment "Trogdor"
+if USE_QC_BLOBS + config BOARD_GOOGLE_BUBS - bool "Bubs" + bool "-> Bubs" select BOARD_GOOGLE_TROGDOR_COMMON
config BOARD_GOOGLE_COACHZ - bool "Coachz" + bool "-> Coachz" select BOARD_GOOGLE_TROGDOR_COMMON
config BOARD_GOOGLE_LAZOR - bool "Lazor" + bool "-> Lazor" select BOARD_GOOGLE_TROGDOR_COMMON
config BOARD_GOOGLE_POMPOM - bool "Pompom" + bool "-> Pompom" select BOARD_GOOGLE_TROGDOR_COMMON
config BOARD_GOOGLE_TROGDOR - bool "Trogdor" + bool "-> Trogdor" select BOARD_GOOGLE_TROGDOR_COMMON + +endif + +comment "(Trogdor requires 'Allow QC blobs repository')" + depends on !USE_QC_BLOBS diff --git a/src/soc/qualcomm/sc7180/Kconfig b/src/soc/qualcomm/sc7180/Kconfig index c66dc92..c37aff9 100644 --- a/src/soc/qualcomm/sc7180/Kconfig +++ b/src/soc/qualcomm/sc7180/Kconfig @@ -2,6 +2,7 @@ config SOC_QUALCOMM_SC7180 bool default n + depends on USE_QC_BLOBS select ARCH_BOOTBLOCK_ARMV8_64 select ARCH_RAMSTAGE_ARMV8_64 select ARCH_ROMSTAGE_ARMV8_64
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45214 )
Change subject: trogdor/sc7180: Clarify USE_QC_BLOBS requirements ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45214/3/src/mainboard/google/trogdo... File src/mainboard/google/trogdor/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/45214/3/src/mainboard/google/trogdo... PS3, Line 3: if USE_QC_BLOBS this is not something that should be in Kconfig.name and also breaks Kconfig:
c0d3@z3r0:[~/src/coreboot.git]# make oldconfig src/mainboard/google/asurada/Kconfig:13:error: recursive dependency detected! src/mainboard/google/asurada/Kconfig:13: symbol BOARD_SPECIFIC_OPTIONS depends on BOARD_GOOGLE_ASURADA_COMMON src/mainboard/google/asurada/Kconfig:4: symbol BOARD_GOOGLE_ASURADA_COMMON is selected by BOARD_GOOGLE_ASURADA src/mainboard/google/asurada/Kconfig.name:3: symbol BOARD_GOOGLE_ASURADA is part of choice <choice> src/mainboard/google/Kconfig:5: choice <choice> contains symbol BOARD_GOOGLE_BUBS src/mainboard/google/trogdor/Kconfig.name:5: symbol BOARD_GOOGLE_BUBS depends on USE_QC_BLOBS src/Kconfig:237: symbol USE_QC_BLOBS depends on USE_BLOBS src/Kconfig:215: symbol USE_BLOBS is selected by BOARD_SPECIFIC_OPTIONS # # configuration written to /home/c0d3/src/coreboot.git/.config # src/mainboard/google/asurada/Kconfig:13:error: recursive dependency detected! src/mainboard/google/asurada/Kconfig:13: symbol BOARD_SPECIFIC_OPTIONS depends on BOARD_GOOGLE_ASURADA_COMMON src/mainboard/google/asurada/Kconfig:4: symbol BOARD_GOOGLE_ASURADA_COMMON is selected by BOARD_GOOGLE_ASURADA src/mainboard/google/asurada/Kconfig.name:3: symbol BOARD_GOOGLE_ASURADA is part of choice <choice> src/mainboard/google/Kconfig:5: choice <choice> contains symbol BOARD_GOOGLE_BUBS src/mainboard/google/trogdor/Kconfig.name:5: symbol BOARD_GOOGLE_BUBS depends on USE_QC_BLOBS src/Kconfig:237: symbol USE_QC_BLOBS depends on USE_BLOBS src/Kconfig:215: symbol USE_BLOBS is selected by BOARD_SPECIFIC_OPTIONS
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45214 )
Change subject: trogdor/sc7180: Clarify USE_QC_BLOBS requirements ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45214/3/src/mainboard/google/trogdo... File src/mainboard/google/trogdor/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/45214/3/src/mainboard/google/trogdo... PS3, Line 3: if USE_QC_BLOBS
this is not something that should be in Kconfig.name and also breaks Kconfig: […]
ugh... nvm. I had a board selecting USE_BLOBS in my local tree, leading to that o.O
-
Julius Werner (Code Review) -
Michael Niewöhner (Code Review)
-
Philip Chen (Code Review)