Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42551 )
Change subject: hatch: Create wyvern variant ......................................................................
Patch Set 1:
(4 comments)
Hey Paul, thanks for the patch. Please see inline comments. I think there is a few not too bad bugs here however could you raise them from review and assign them to me please.
https://review.coreboot.org/c/coreboot/+/42551/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/wyvern/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/42551/1/src/mainboard/google/hatch/... PS1, Line 4: SPD_SOURCES = ``` ramstage-y += gpio.c ramstage-y += mainboard.c bootblock-y += gpio.c ```
No SPD_SOURCES to be defined here as dimm's are not downed.
https://review.coreboot.org/c/coreboot/+/42551/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/wyvern/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/42551/1/src/mainboard/google/hatch/... PS1, Line 2: : #include <baseboard/acpi/dptf.asl> I will make a common puff dptf.asl as well since we diverged from baseboard vanilla.
https://review.coreboot.org/c/coreboot/+/42551/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/wyvern/include/variant/ec.h:
https://review.coreboot.org/c/coreboot/+/42551/1/src/mainboard/google/hatch/... PS1, Line 5: : #include <baseboard/ec.h> our ec.h is more specialised than the baseboard however I think we can make it common so I will craft a patch for that now.
https://review.coreboot.org/c/coreboot/+/42551/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/wyvern/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42551/1/src/mainboard/google/hatch/... PS1, Line 1: chip soc/intel/cannonlake : : device domain 0 on : end : : end We should somehow work out how to improve making puff overrides more copy-able if we can't do multi-layer dt yet.