[M] Change in coreboot[main]: mb/cwwk: Add CWWK CW-ADLNTB-1C2L-V3.0 board as an adl variants

[Angel Pons (Code Review)]

Attention is currently required from: Brandon Weeks, Felix Held, Julian Intronati, Paul Menzel.

Angel Pons 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: Code-Review+2

(3 comments)

File src/mainboard/cwwk/adl/variants/cw-adlntb-1c2l-v3.0/board_info.txt:

https://review.coreboot.org/c/coreboot/+/87027/comment/f532aef5_f6734a98?usp... :
PS3, Line 1: CW-X86-P5-V3
> The internal product name of the board is "CW_ADLNTB_1C2L" into the original bios. […]

Generally, I would use whatever name is written on the mainboard, as that should identify the hardware. Laptops and prebuilt desktops may be easier to identify by the commercial name (HP 280 G2 Microtower) as opposed to the mainboard codename (Foxconn H-ICEAGE) as the latter is significantly more obscure, but specifying both might be necessary in some cases to fully identify the hardware.

I usually ignore the names in firmware, especially given that some firmwares say that the board name is "To be filled by O.E.M." (i.e. the OEM forgot to put the name in there) 😄

In any case, I marked this comment as resolved because I don't think it's a big deal. The current names seem fine to me.

File src/mainboard/cwwk/adl/variants/cw-adlntb-1c2l-v3.0/gpio.c:

https://review.coreboot.org/c/coreboot/+/87027/comment/9edae1a4_66537b22?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 */
> When i generated the file those gpio has been generated. […]

I've never understood what these VGPIOs do, I'm not sure where they're documented (if they're even documented). If someone finds a reason to configure them in the future, then they can just make another change that adds them.

So yes, I agree with removing the commented-out lines.

File src/mainboard/cwwk/adl/variants/cw-adlntb-1c2l-v3.0/romstage.c:

https://review.coreboot.org/c/coreboot/+/87027/comment/e5176fd0_5afaa830?usp... :
PS3, Line 22: 0x52
> Yeah this is a weird one, that one took me a bunch of debuging to understand, I was stuck on memory  […]

Oof, I can imagine. I know that, at least on older platforms, you can try reading the SPD with userspace tools (e.g. `decode-dimms` from `i2c-tools` on Linux). This saves you from having to guess, but I'm not sure if DDR5 works the same way.