Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33824 )
Change subject: mainboard/google/hatch: create akemi variant. ......................................................................
Patch Set 17:
(3 comments)
https://review.coreboot.org/c/coreboot/+/33824/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/33824/10//COMMIT_MSG@14 PS10, Line 14: /build/hatch/firmware/coreboot-private/3rdparty/blobs/baseboard/hatch
It seems to me that the path here is for these private blobs like VBT, fitimage or SAR?
Yes, they should be in /build/hatch/firmware/
https://review.coreboot.org/c/coreboot/+/33824/10/src/mainboard/google/hatch... File src/mainboard/google/hatch/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/33824/10/src/mainboard/google/hatch... PS10, Line 5: nit: most use two spaces here
https://review.coreboot.org/c/coreboot/+/33824/17/src/mainboard/google/hatch... File src/mainboard/google/hatch/variants/akemi/gpio.c:
https://review.coreboot.org/c/coreboot/+/33824/17/src/mainboard/google/hatch... PS17, Line 100: early_gpio_table
None of these GPIOs are configured in early_gpio_table[] in baseboard. […]
I see two options: we could either keep adding PAD_NC() in the base_early_gpio_table for variants that need to add new early GPIOs (which for this family of devices, will only be the MEM_STRAP_* pins). The other option is what you suggest, change the baseboard's implementation to weak and allow variants to provide an override function instead. That probably makes more sense, and is less error-prone. Actually, thinking more, I'd rather not have the baseboard provide a weak function and just enforce that each variant provides its early_gpio_table separately. I can work on a patch for that.