Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change board name from nitropad-nv41 -> novacustom_nv4x_adl #1793

Merged
merged 3 commits into from
Nov 15, 2024
Merged

Change board name from nitropad-nv41 -> novacustom_nv4x_adl #1793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9fccfb4
Change board name from nitropad-nv41 -> novacustom_nv4x_adl
tlaurion Sep 11, 2024
15bf330
boards/novacustom_nv4x_adl/novacustom_nv4x_adl.config: 'CONFIG_BOARD_…
tlaurion Nov 4, 2024
6746058
config/coreboot-novacustom_nv4x_adl.config: set CONFIG_MAINBOARD_VERS…
tlaurion Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,8 @@ workflows:
# No need to wait further for other board's cache
# We reuse built modules from x230-hotp-maximized cache only
- build_and_persist:
name: nitropad-nv41
target: nitropad-nv41
name: novacustom_nv4x_adl
target: novacustom_nv4x_adl
subcommand: ""
requires:
- x86-musl-cross-make
Expand Down Expand Up @@ -516,7 +516,7 @@ workflows:
target: nitropad-ns50
subcommand: ""
requires:
- nitropad-nv41
- novacustom_nv4x_adl

# coreboot 4.11
- build:
Expand Down
6 changes: 3 additions & 3 deletions boards/nitropad-nv41/nitropad-nv41.config → ...ustom_nv4x_adl/novacustom_nv4x_adl.config
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# Nitrokey Nitropad NV41 board configuration
# NovaCustom NV4x 12th Gen (nv40pz: Alder Lake) board configuration
# Note: for reference, other GOP enabled FB board is librem_11

export CONFIG_COREBOOT=y
export CONFIG_COREBOOT_VERSION=dasharo
export CONFIG_LINUX_VERSION=6.1.8

CONFIG_COREBOOT_CONFIG=config/coreboot-nitropad-nv41.config
CONFIG_COREBOOT_CONFIG=config/coreboot-novacustom_nv4x_adl.config
CONFIG_LINUX_CONFIG=config/linux-nitropad-x.config

#Enable DEBUG output
Expand Down Expand Up @@ -67,6 +67,6 @@ export CONFIG_BOOT_REQ_HASH=n
export CONFIG_BOOT_REQ_ROLLBACK=n
export CONFIG_BOOT_KERNEL_ADD=""
export CONFIG_BOOT_KERNEL_REMOVE="intel_iommu=on intel_iommu=igfx_off"
export CONFIG_BOARD_NAME="Nitropad NV41"
export CONFIG_BOARD_NAME="NovaCustom NV4x 12th Gen"
export CONFIG_FLASH_OPTIONS="flashprog --progress --programmer internal"
export CONFIG_AUTO_BOOT_TIMEOUT=5
4 changes: 2 additions & 2 deletions config/coreboot-nitropad-nv41.config → config/coreboot-novacustom_nv4x_adl.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ CONFIG_VBOOT_VBNV_OFFSET=0x28
CONFIG_VARIANT_DIR="nv40pz"
CONFIG_OVERRIDE_DEVICETREE="variants/$(CONFIG_VARIANT_DIR)/overridetree.cb"
# CONFIG_VGA_BIOS is not set
CONFIG_MAINBOARD_SMBIOS_MANUFACTURER="Nitrokey"
CONFIG_MAINBOARD_SMBIOS_MANUFACTURER="Notebook"
Copy link
Collaborator Author

@tlaurion tlaurion Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pietrushnic so per reference config in dasharo fork, this is one of the SMBIOS strings dasharo configured.

@macpijan please confirm valid.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

user@localhost:~/heads$ grep BIOS modules/coreboot
CONFIG_COREBOOT_SMBIOS_PRODUCT_NAME ?= $(BOARD)
	sed -i '/^CONFIG_MAINBOARD_SMBIOS_PRODUCT_NAME/d' $(build)/$(coreboot_dir)/.config; \
	echo 'CONFIG_MAINBOARD_SMBIOS_PRODUCT_NAME="$(CONFIG_COREBOOT_SMBIOS_PRODUCT_NAME)"' >> $(build)/$(coreboot_dir)/.config; \
	if [ ! -z "$(CONFIG_COREBOOT_SMBIOS_MANUFACTURER)" ]; then \
		sed -i '/^CONFIG_MAINBOARD_SMBIOS_MANUFACTURER/d' $(build)/$(coreboot_dir)/.config; \
		echo 'CONFIG_MAINBOARD_SMBIOS_MANUFACTURER="$(CONFIG_COREBOOT_SMBIOS_MANUFACTURER)"' >> $(build)/$(coreboot_dir)/.config; \

Copy link
Collaborator Author

@tlaurion tlaurion Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pietrushnic pretty sure you don't want the manufacturer to be Notebook, but you tell me.
Nitrokey overrided this to Nitrokey which this PR changes and is part of the string I asked numerous tims to review for the sake of DTS/DMIDECODE/FWUPD and other things depending of those strings, those strings changes that would break if we are not careful.

This come from Dasharo fork's reference config, and what coreboot fork's defconfig (vs oldconfig resulting of defconfig) results in Notebook as per that release: https://github.com/Dasharo/coreboot/blob/3a9aa3a4692f3dd49732f5b4e3ec54be385f0969/configs/config.novacustom_nv4x_adl

Copy link
Contributor

@mkopec mkopec Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notebook is correct. Those are the strings in Insyde firmware, and they have to match in Dasharo so that Windows autoinstalls the drivers that Clevo uploaded to Windows Update :(

I suppose it's not a big concern for Heads but I think they should match the UEFI firmware variant. fwupd also includes this SMBIOS string when looking for updates

Copy link

@pietrushnic pietrushnic Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked numerous tims to review

This part was not needed.

@tlaurion, as @mkopec said, Dasharo (coreboot+UEFI) is concerned because of drivers delivered by Windows Update. fwupd uses the same mechanics, but if Novacustom or Nitrokey register in LVFS or Dasharo Team starts its instance, we could leverage that for much simpler updates. We will cross that bridge when we get there. Premature optimization is the root of all evil, so I would treat it as a no-op now.

Only when we can describe to OEMs precisely how to set that up to leverage LVFS can we create rules around that; without these tests and documentation, we should leave it as close to upstream as possible. If OEMs need to adjust that for some reason, we should also give them that freedom unless we can explain the implications for integration, which has not existed so far.

The impact of DTS does not belong to this repository and to this issue, IMHO. @mkopec, can you create an issue for DTS and determine how BOARD will impact DTS by assigning the problem to someone from Zarhus Team? Ideally, deliverables would be guidelines for naming conventions that do not break the way DTS works. Maybe we should mimic NV41 Dasharo (coreboot+Heads) behavior. Please note that the goal is to support both Novacustom and Nitrokey devices in Dasharo (coreboot+Heads).

CONFIG_INTEL_GMA_VBT_FILE="src/mainboard/$(MAINBOARDDIR)/variants/$(VARIANT_DIR)/data.vbt"
# CONFIG_DISABLE_HECI1_AT_PRE_BOOT is not set
CONFIG_PRERAM_CBMEM_CONSOLE_SIZE=0x4000
Expand All @@ -140,7 +140,7 @@ CONFIG_CMOS_LAYOUT_FILE="src/mainboard/$(MAINBOARDDIR)/cmos.layout"
CONFIG_BOOT_DEVICE_SPI_FLASH_BUS=0
CONFIG_BOARD_CLEVO_ADLP_COMMON=y
CONFIG_BOARD_CLEVO_NV40PZ_BASE=y
CONFIG_MAINBOARD_SMBIOS_PRODUCT_NAME="Nitropad NV41"
CONFIG_MAINBOARD_SMBIOS_PRODUCT_NAME="NV4xPZ"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pietrushnic so per reference config in dasharo fork, this is one of the SMBIOS strings dasharo configured.

@macpijan please confirm valid.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

user@localhost:~/heads$ grep BIOS modules/coreboot
CONFIG_COREBOOT_SMBIOS_PRODUCT_NAME ?= $(BOARD)
	sed -i '/^CONFIG_MAINBOARD_SMBIOS_PRODUCT_NAME/d' $(build)/$(coreboot_dir)/.config; \
	echo 'CONFIG_MAINBOARD_SMBIOS_PRODUCT_NAME="$(CONFIG_COREBOOT_SMBIOS_PRODUCT_NAME)"' >> $(build)/$(coreboot_dir)/.config; \
	if [ ! -z "$(CONFIG_COREBOOT_SMBIOS_MANUFACTURER)" ]; then \
		sed -i '/^CONFIG_MAINBOARD_SMBIOS_MANUFACTURER/d' $(build)/$(coreboot_dir)/.config; \
		echo 'CONFIG_MAINBOARD_SMBIOS_MANUFACTURER="$(CONFIG_COREBOOT_SMBIOS_MANUFACTURER)"' >> $(build)/$(coreboot_dir)/.config; \

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dmidecode provides Product Name: novacustom_nv4x_adl as per #1793 (comment)

CONFIG_CONSOLE_POST=y
# CONFIG_USE_PM_ACPI_TIMER is not set
CONFIG_TPM_PIRQ=0x27
Expand Down