[M] Change in coreboot[main]: mb/cwwk: Add CWWK CW-ADLNTB-1C2L-V3.0 board as an adl variants
Attention is currently required from: Angel Pons, Brandon Weeks, Felix Held, Paul Menzel.
Julian Intronati has posted comments on this change by Julian Intronati. ( https://review.coreboot.org/c/coreboot/+/87027?usp=email )
Change subject: mb/cwwk: Add CWWK CW-ADLNTB-1C2L-V3.0 board as an adl variants ......................................................................
Patch Set 4:
(7 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/87027/comment/d48b71e6_a568397d?usp... : PS3, Line 40: - Audio : - S0ix : - Internal USB ports
Can you please specify which of these are not working and which are not tested?
Done
https://review.coreboot.org/c/coreboot/+/87027/comment/6b73e690_dab0d5be?usp... : PS3, Line 10: ports. The variants files structure is inspired by : system76/adl/variants structure, adapted for cw-adl-4l-v1.0 board : and the new cw-adlnth-1c2l-v3.0 (named according to original bios : naming). : : CPU: Intel N100 or N305 : Memory: 1x DDR5-4800 SODIMM (max 16 GB) : NIC: 2x Intel I226-V 2.5 GbE : Expansion: : - M.2 2230 E key : - M.2 2280 M key : - USB 2.0 header : - Fan header : External ports: : - DC power : - 2x Ethernet : - 2x HDMI : - 2x USB 2.0 : : Working: : - Boots Debian 12 with SeaBIOS and EDK II payloads : - Serial port : - External USB ports : - 1x HDMI : - 2x Intel I226 2.5 GbE NICs : - M.2 ports : - ACPI S3 : - Fan (ITE IT8613E) driver IT87 (frankcrawford github fork) : : Not working / not tested: : - Audio : - S0ix : - Internal USB ports : : VBT extracted from vendor UEFI firmware : version F2 (2024-06-26 10:26:38)
Did you mean to indent all of this?
Done, I don't know why it was indented like this probably a misconfiguration of git cola that I used for the first time... My bad sorry and thank you for pointing it.
File src/mainboard/cwwk/adl/Kconfig:
https://review.coreboot.org/c/coreboot/+/87027/comment/cda327b7_146b0d05?usp... : PS3, Line 27: config VARIANT_DIR : default "cw-adl-4l-v1.0" if BOARD_CWWK_CW_ADL_4L_V1 : default "cw-adlntb-1c2l-v3.0" if BOARD_CWWK_CW_ADLNTB_1C2L_V3 : : config MAINBOARD_PART_NUMBER : default "CW-ADL-4L-V1.0" if BOARD_CWWK_CW_ADL_4L_V1 : default "CW-ADLNTB-1C2L-V3.0" if BOARD_CWWK_CW_ADLNTB_1C2L_V3 : : config MAINBOARD_SMBIOS_PRODUCT_NAME : default "CW-ADL-4L-V1.0" if BOARD_CWWK_CW_ADL_4L_V1 : default "CW-X86-P5-V3" if BOARD_CWWK_CW_ADLNTB_1C2L_V3 : : config MAINBOARD_VERSION : default "CW-ADL-4L-V1.0" if BOARD_CWWK_CW_ADL_4L_V1 : default "CW-ADLNTB-1C2L-V3.0" if BOARD_CWWK_CW_ADLNTB_1C2L_V3
nit: If you want, you can align the `if` parts for neatness: […]
Done !
File src/mainboard/cwwk/adl/variants/cw-adlntb-1c2l-v3.0/board_info.txt:
https://review.coreboot.org/c/coreboot/+/87027/comment/c7a55fe0_48f1d28f?usp... : PS3, Line 1: CW-X86-P5-V3
I find it weird that the mainboard name doesn't match the product name
The internal product name of the board is "CW_ADLNTB_1C2L" into the original bios. On the external sticker on the board it's written "CW-X86-P5-V3", that's what I wanted to reflect. I think there is a "commercial name" and a "technical name". Is there a good practice into Coreboot to reflect this namings or should we just keep it "technical" as coreboot target is pretty technical for now ?
File src/mainboard/cwwk/adl/variants/cw-adlntb-1c2l-v3.0/data.vbt:
PS3:
Gerrit says "similarity index 100%" so the VBT is identical. […]
Thank you for this one, much cleaner !
File src/mainboard/cwwk/adl/variants/cw-adlntb-1c2l-v3.0/gpio.c:
https://review.coreboot.org/c/coreboot/+/87027/comment/170e57d8_0db0e113?usp... : PS3, Line 167: /* ------- GPIO Group vGPIO ------- */ : //PAD_CFG_GPO(GPP_VGPIO_0, 0, DEEP), /* GPIO */ : //PAD_CFG_GPI_TRIG_OWN(GPP_VGPIO_4, NONE, DEEP, OFF, ACPI), /* GPIO */ : //PAD_CFG_GPIO_BIDIRECT(GPP_VGPIO_5, 1, NONE, DEEP, LEVEL, ACPI), /* GPIO */ : //PAD_CFG_NF(GPP_VGPIO_6, NONE, DEEP, NF1), /* GPP_VGPIO_6 */ : //PAD_CFG_NF(GPP_VGPIO_7, NONE, DEEP, NF1), /* GPP_VGPIO_7 */ : //PAD_CFG_NF(GPP_VGPIO_8, NONE, DEEP, NF1), /* GPP_VGPIO_8 */ : //PAD_CFG_NF(GPP_VGPIO_9, NONE, DEEP, NF1), /* GPP_VGPIO_9 */ : //PAD_CFG_NF(GPP_VGPIO_10, NONE, DEEP, NF1), /* GPP_VGPIO_10 */ : //PAD_CFG_NF(GPP_VGPIO_11, NONE, DEEP, NF1), /* GPP_VGPIO_11 */ : //PAD_CFG_NF(GPP_VGPIO_12, NONE, DEEP, NF1), /* GPP_VGPIO_12 */ : //PAD_CFG_NF(GPP_VGPIO_13, NONE, DEEP, NF1), /* GPP_VGPIO_13 */ : //PAD_CFG_NF(GPP_VGPIO_18, NONE, DEEP, NF1), /* GPP_VGPIO_18 */ : //PAD_CFG_NF(GPP_VGPIO_19, NONE, DEEP, NF1), /* GPP_VGPIO_19 */ : //PAD_CFG_NF(GPP_VGPIO_20, NONE, DEEP, NF1), /* GPP_VGPIO_20 */ : //PAD_CFG_NF(GPP_VGPIO_21, NONE, DEEP, NF1), /* GPP_VGPIO_21 */ : //PAD_CFG_NF(GPP_VGPIO_22, NONE, DEEP, NF1), /* GPP_VGPIO_22 */ : //PAD_CFG_NF(GPP_VGPIO_23, NONE, DEEP, NF1), /* GPP_VGPIO_23 */ : //PAD_CFG_NF(GPP_VGPIO_24, NONE, DEEP, NF1), /* GPP_VGPIO_24 */ : //PAD_CFG_NF(GPP_VGPIO_25, NONE, DEEP, NF1), /* GPP_VGPIO_25 */ : //PAD_CFG_NF(GPP_VGPIO_30, NONE, DEEP, NF1), /* GPP_VGPIO_30 */ : //PAD_CFG_NF(GPP_VGPIO_31, NONE, DEEP, NF1), /* GPP_VGPIO_31 */ : //PAD_CFG_NF(GPP_VGPIO_32, NONE, DEEP, NF1), /* GPP_VGPIO_32 */ : //PAD_CFG_NF(GPP_VGPIO_33, NONE, DEEP, NF1), /* GPP_VGPIO_33 */ : //PAD_CFG_NF(GPP_VGPIO_34, NONE, DEEP, NF1), /* GPP_VGPIO_34 */ : //PAD_CFG_NF(GPP_VGPIO_35, NONE, DEEP, NF1), /* GPP_VGPIO_35 */ : //PAD_CFG_NF(GPP_VGPIO_36, NONE, DEEP, NF1), /* GPP_VGPIO_36 */ : //PAD_CFG_NF(GPP_VGPIO_37, NONE, DEEP, NF1), /* GPP_VGPIO_37 */
Is this needed? It's all commented out
When i generated the file those gpio has been generated. But the macro doesn't work, as on the other board there are not present. I commented it to go further... I kept them here because of the generation process... My understanding of those "GPIO Group vGPIO" is pretty bad... Better removing them for now even if I don't know why the generation process was generating them ?
File src/mainboard/cwwk/adl/variants/cw-adlntb-1c2l-v3.0/romstage.c:
https://review.coreboot.org/c/coreboot/+/87027/comment/0a8cbe86_22299ec9?usp... : PS3, Line 22: 0x52
Weird that the only DIMM SPD is at SMBus address 0x52, that usually corresponds to the second channe […]
Yeah this is a weird one, that one took me a bunch of debuging to understand, I was stuck on memory initialisation... The board that I have only have one slot, and the address is actualy the second channel... The resistors selecting the address must be mounted badly on the motherboard's PCB. I couldn't manage to localise it for now... I have no board schematics...
-
Julian Intronati (Code Review)