Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32515
Change subject: purism/librem_skl: remove 13v3 target, clean up KConfig ......................................................................
purism/librem_skl: remove 13v3 target, clean up KConfig
Remove the Librem 13v3 as a separate board; instead build a single firmware image for the 13 v2/v3 boards.
Clean up Kconfig options: - remove entries for 13v3 board - fold entries into a single line where possible - remove reduntant MAINBOARD_VERSION option - specify microcode length separately for SKL and KBL devices
Change-Id: Ic09b8ec5c576f4c4c48ef30ee3f60a4c2c286cd3 Signed-off-by: Matt DeVillier matt.devillier@puri.sm --- M src/mainboard/purism/librem_skl/Kconfig M src/mainboard/purism/librem_skl/Kconfig.name 2 files changed, 10 insertions(+), 31 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/32515/1
diff --git a/src/mainboard/purism/librem_skl/Kconfig b/src/mainboard/purism/librem_skl/Kconfig index 965318f..7edb1b3 100644 --- a/src/mainboard/purism/librem_skl/Kconfig +++ b/src/mainboard/purism/librem_skl/Kconfig @@ -19,11 +19,8 @@
config VARIANT_DIR string - default "librem13v2" if BOARD_PURISM_LIBREM13_V2 - default "librem13v2" if BOARD_PURISM_LIBREM13_V3 - default "librem15v3" if BOARD_PURISM_LIBREM15_V3 - default "librem13v2" if BOARD_PURISM_LIBREM13_V4 - default "librem15v3" if BOARD_PURISM_LIBREM15_V4 + default "librem13v2" if BOARD_PURISM_LIBREM13_V2 || BOARD_PURISM_LIBREM13_V4 + default "librem15v3" if BOARD_PURISM_LIBREM15_V3 || BOARD_PURISM_LIBREM15_V4
config MAINBOARD_VENDOR string @@ -31,28 +28,16 @@
config MAINBOARD_FAMILY string - default "Librem 13" if BOARD_PURISM_LIBREM13_V2 - default "Librem 13" if BOARD_PURISM_LIBREM13_V3 - default "Librem 15" if BOARD_PURISM_LIBREM15_V3 - default "Librem 13" if BOARD_PURISM_LIBREM13_V4 - default "Librem 15" if BOARD_PURISM_LIBREM15_V4 + default "Librem 13" if BOARD_PURISM_LIBREM13_V2 || BOARD_PURISM_LIBREM13_V4 + default "Librem 15" if BOARD_PURISM_LIBREM15_V3 || BOARD_PURISM_LIBREM15_V4
config MAINBOARD_PART_NUMBER string - default "Librem 13 v2" if BOARD_PURISM_LIBREM13_V2 - default "Librem 13 v2" if BOARD_PURISM_LIBREM13_V3 + default "Librem 13 v2/v3" if BOARD_PURISM_LIBREM13_V2 default "Librem 15 v3" if BOARD_PURISM_LIBREM15_V3 default "Librem 13 v4" if BOARD_PURISM_LIBREM13_V4 default "Librem 15 v4" if BOARD_PURISM_LIBREM15_V4
-config MAINBOARD_VERSION - string - default "2.0" if BOARD_PURISM_LIBREM13_V2 - default "3.0" if BOARD_PURISM_LIBREM13_V3 - default "3.0" if BOARD_PURISM_LIBREM15_V3 - default "4.0" if BOARD_PURISM_LIBREM13_V4 - default "4.0" if BOARD_PURISM_LIBREM15_V4 - config MAINBOARD_DIR string default "purism/librem_skl" @@ -67,11 +52,8 @@
config VGA_BIOS_ID string - default "8086,1916" if BOARD_PURISM_LIBREM13_V2 - default "8086,1916" if BOARD_PURISM_LIBREM13_V3 - default "8086,1916" if BOARD_PURISM_LIBREM15_V3 - default "8086,5916" if BOARD_PURISM_LIBREM13_V4 - default "8086,5916" if BOARD_PURISM_LIBREM15_V4 + default "8086,1916" if BOARD_PURISM_LIBREM13_V2 || BOARD_PURISM_LIBREM15_V3 + default "8086,5916" if BOARD_PURISM_LIBREM13_V4 || BOARD_PURISM_LIBREM15_V4
config DIMM_MAX int @@ -83,7 +65,8 @@
config CPU_MICROCODE_CBFS_LEN hex - default 0x18000 + default 0x18800 if BOARD_PURISM_LIBREM13_V2 || BOARD_PURISM_LIBREM15_V3 + default 0x18400 if BOARD_PURISM_LIBREM13_V4 || BOARD_PURISM_LIBREM15_V4
config CPU_MICROCODE_CBFS_LOC hex diff --git a/src/mainboard/purism/librem_skl/Kconfig.name b/src/mainboard/purism/librem_skl/Kconfig.name index 5b82de7..b0dac3e 100644 --- a/src/mainboard/purism/librem_skl/Kconfig.name +++ b/src/mainboard/purism/librem_skl/Kconfig.name @@ -1,9 +1,5 @@ config BOARD_PURISM_LIBREM13_V2 - bool "Librem 13 v2" - select BOARD_PURISM_BASEBOARD_LIBREM_SKL - -config BOARD_PURISM_LIBREM13_V3 - bool "Librem 13 v3" + bool "Librem 13 v2/v3" select BOARD_PURISM_BASEBOARD_LIBREM_SKL
config BOARD_PURISM_LIBREM15_V3
Matt DeVillier has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/32515 )
Change subject: purism/librem_skl: remove 13v3 target, clean up Kconfig ......................................................................
purism/librem_skl: remove 13v3 target, clean up Kconfig
Remove the Librem 13v3 as a separate board; instead build a single firmware image for the 13 v2/v3 boards.
Clean up Kconfig options: - remove entries for 13v3 board - fold entries into a single line where possible - remove redundant MAINBOARD_VERSION option - specify microcode length separately for SKL and KBL devices
Change-Id: Ic09b8ec5c576f4c4c48ef30ee3f60a4c2c286cd3 Signed-off-by: Matt DeVillier matt.devillier@puri.sm --- M src/mainboard/purism/librem_skl/Kconfig M src/mainboard/purism/librem_skl/Kconfig.name 2 files changed, 10 insertions(+), 31 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/32515/2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32515 )
Change subject: purism/librem_skl: remove 13v3 target, clean up Kconfig ......................................................................
Patch Set 2:
(3 comments)
Looks good but I'm confused about the MAINBOARD_VERSION drop.
https://review.coreboot.org/#/c/32515/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32515/2//COMMIT_MSG@15 PS2, Line 15: - remove redundant MAINBOARD_VERSION option How is it redundant? I didn't find another place where it is set.
https://review.coreboot.org/#/c/32515/2//COMMIT_MSG@16 PS2, Line 16: - specify microcode length separately for SKL and KBL devices I'm just wondering. This is only needed for FSP_CAR, right? It's neither the default nor would I expect Purism to use it. So you could probably save yourself some time, maintaining it.
https://review.coreboot.org/#/c/32515/2/src/mainboard/purism/librem_skl/Kcon... File src/mainboard/purism/librem_skl/Kconfig:
https://review.coreboot.org/#/c/32515/2/src/mainboard/purism/librem_skl/Kcon... PS2, Line 36: default "Librem 13 v2/v3" if BOARD_PURISM_LIBREM13_V2 NB. AIUI, this also alters the SMBIOS_PRODUCT_NAME which is sometimes used in OS's to handle quirks.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32515 )
Change subject: purism/librem_skl: remove 13v3 target, clean up Kconfig ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/#/c/32515/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32515/2//COMMIT_MSG@15 PS2, Line 15: - remove redundant MAINBOARD_VERSION option
How is it redundant? I didn't find another place […]
because it just mirrored the board name, it wasn't being used to differentiate between board revisions
https://review.coreboot.org/#/c/32515/2//COMMIT_MSG@16 PS2, Line 16: - specify microcode length separately for SKL and KBL devices
I'm just wondering. This is only needed for FSP_CAR, right? […]
if that's the case, then I'll just drop it. Will test and amend if so.
https://review.coreboot.org/#/c/32515/2/src/mainboard/purism/librem_skl/Kcon... File src/mainboard/purism/librem_skl/Kconfig:
https://review.coreboot.org/#/c/32515/2/src/mainboard/purism/librem_skl/Kcon... PS2, Line 36: default "Librem 13 v2/v3" if BOARD_PURISM_LIBREM13_V2
NB. AIUI, this also alters the SMBIOS_PRODUCT_NAME […]
AFAIK, the only place this might cause an issue is with flashrom, assuming it uses that for the board mismatch check. But I'll check with the PureOS folks to be sure
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32515 )
Change subject: purism/librem_skl: remove 13v3 target, clean up Kconfig ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/32515/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32515/2//COMMIT_MSG@15 PS2, Line 15: - remove redundant MAINBOARD_VERSION option
because it just mirrored the board name, it wasn't being used to differentiate between board revisio […]
It's used to fill the SMBIOS table. Just seems odd to remove that. Or actually, leave it at the default "1.0". But it has a prompt anyway, so it's up to the person configuring the build. And I assume I'm talking to the one who does it for the official builds, so I won't argue ;)
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32515 )
Change subject: purism/librem_skl: remove 13v3 target, clean up Kconfig ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/32515/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32515/2//COMMIT_MSG@15 PS2, Line 15: - remove redundant MAINBOARD_VERSION option
It's used to fill the SMBIOS table. Just seems odd to […]
it should default to 1.0 per src/Kconfig, since no longer being overridden by mainboard
Hello Angel Pons, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32515
to look at the new patch set (#3).
Change subject: purism/librem_skl: remove 13v3 target, clean up Kconfig ......................................................................
purism/librem_skl: remove 13v3 target, clean up Kconfig
Remove the Librem 13v3 as a separate board; instead build a single firmware image for the 13 v2/v3 boards.
Clean up Kconfig options: - remove entries for 13v3 board - fold entries into a single line where possible - remove redundant MAINBOARD_VERSION option (will default to 1.0) - remove unused microcode length/location (only needed for FSP CAR)
Test: build/boot Librem 13 v2/v3 boards with same image
Change-Id: Ic09b8ec5c576f4c4c48ef30ee3f60a4c2c286cd3 Signed-off-by: Matt DeVillier matt.devillier@puri.sm --- M src/mainboard/purism/librem_skl/Kconfig M src/mainboard/purism/librem_skl/Kconfig.name 2 files changed, 7 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/32515/3
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32515 )
Change subject: purism/librem_skl: remove 13v3 target, clean up Kconfig ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32515 )
Change subject: purism/librem_skl: remove 13v3 target, clean up Kconfig ......................................................................
purism/librem_skl: remove 13v3 target, clean up Kconfig
Remove the Librem 13v3 as a separate board; instead build a single firmware image for the 13 v2/v3 boards.
Clean up Kconfig options: - remove entries for 13v3 board - fold entries into a single line where possible - remove redundant MAINBOARD_VERSION option (will default to 1.0) - remove unused microcode length/location (only needed for FSP CAR)
Test: build/boot Librem 13 v2/v3 boards with same image
Change-Id: Ic09b8ec5c576f4c4c48ef30ee3f60a4c2c286cd3 Signed-off-by: Matt DeVillier matt.devillier@puri.sm Reviewed-on: https://review.coreboot.org/c/coreboot/+/32515 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/mainboard/purism/librem_skl/Kconfig M src/mainboard/purism/librem_skl/Kconfig.name 2 files changed, 7 insertions(+), 37 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/mainboard/purism/librem_skl/Kconfig b/src/mainboard/purism/librem_skl/Kconfig index 965318f..2372b9d 100644 --- a/src/mainboard/purism/librem_skl/Kconfig +++ b/src/mainboard/purism/librem_skl/Kconfig @@ -19,11 +19,8 @@
config VARIANT_DIR string - default "librem13v2" if BOARD_PURISM_LIBREM13_V2 - default "librem13v2" if BOARD_PURISM_LIBREM13_V3 - default "librem15v3" if BOARD_PURISM_LIBREM15_V3 - default "librem13v2" if BOARD_PURISM_LIBREM13_V4 - default "librem15v3" if BOARD_PURISM_LIBREM15_V4 + default "librem13v2" if BOARD_PURISM_LIBREM13_V2 || BOARD_PURISM_LIBREM13_V4 + default "librem15v3" if BOARD_PURISM_LIBREM15_V3 || BOARD_PURISM_LIBREM15_V4
config MAINBOARD_VENDOR string @@ -31,28 +28,16 @@
config MAINBOARD_FAMILY string - default "Librem 13" if BOARD_PURISM_LIBREM13_V2 - default "Librem 13" if BOARD_PURISM_LIBREM13_V3 - default "Librem 15" if BOARD_PURISM_LIBREM15_V3 - default "Librem 13" if BOARD_PURISM_LIBREM13_V4 - default "Librem 15" if BOARD_PURISM_LIBREM15_V4 + default "Librem 13" if BOARD_PURISM_LIBREM13_V2 || BOARD_PURISM_LIBREM13_V4 + default "Librem 15" if BOARD_PURISM_LIBREM15_V3 || BOARD_PURISM_LIBREM15_V4
config MAINBOARD_PART_NUMBER string default "Librem 13 v2" if BOARD_PURISM_LIBREM13_V2 - default "Librem 13 v2" if BOARD_PURISM_LIBREM13_V3 default "Librem 15 v3" if BOARD_PURISM_LIBREM15_V3 default "Librem 13 v4" if BOARD_PURISM_LIBREM13_V4 default "Librem 15 v4" if BOARD_PURISM_LIBREM15_V4
-config MAINBOARD_VERSION - string - default "2.0" if BOARD_PURISM_LIBREM13_V2 - default "3.0" if BOARD_PURISM_LIBREM13_V3 - default "3.0" if BOARD_PURISM_LIBREM15_V3 - default "4.0" if BOARD_PURISM_LIBREM13_V4 - default "4.0" if BOARD_PURISM_LIBREM15_V4 - config MAINBOARD_DIR string default "purism/librem_skl" @@ -67,11 +52,8 @@
config VGA_BIOS_ID string - default "8086,1916" if BOARD_PURISM_LIBREM13_V2 - default "8086,1916" if BOARD_PURISM_LIBREM13_V3 - default "8086,1916" if BOARD_PURISM_LIBREM15_V3 - default "8086,5916" if BOARD_PURISM_LIBREM13_V4 - default "8086,5916" if BOARD_PURISM_LIBREM15_V4 + default "8086,1916" if BOARD_PURISM_LIBREM13_V2 || BOARD_PURISM_LIBREM15_V3 + default "8086,5916" if BOARD_PURISM_LIBREM13_V4 || BOARD_PURISM_LIBREM15_V4
config DIMM_MAX int @@ -81,14 +63,6 @@ int default 512
-config CPU_MICROCODE_CBFS_LEN - hex - default 0x18000 - -config CPU_MICROCODE_CBFS_LOC - hex - default 0xFFE115A0 - config CBFS_SIZE hex default 0xe00000 diff --git a/src/mainboard/purism/librem_skl/Kconfig.name b/src/mainboard/purism/librem_skl/Kconfig.name index 5b82de7..b0dac3e 100644 --- a/src/mainboard/purism/librem_skl/Kconfig.name +++ b/src/mainboard/purism/librem_skl/Kconfig.name @@ -1,9 +1,5 @@ config BOARD_PURISM_LIBREM13_V2 - bool "Librem 13 v2" - select BOARD_PURISM_BASEBOARD_LIBREM_SKL - -config BOARD_PURISM_LIBREM13_V3 - bool "Librem 13 v3" + bool "Librem 13 v2/v3" select BOARD_PURISM_BASEBOARD_LIBREM_SKL
config BOARD_PURISM_LIBREM15_V3
-
Matt DeVillier (Code Review) -
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)