Daniel Kurtz has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36492/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36492/1//COMMIT_MSG@9 PS1, Line 9: unreasonably
Yep that's fine as a better reword to clarify that 'unreasonable' means the assumption changed here. […]
The request from me was to follow furquan's advice on CL:36250 on how to implement a single romstage.c that handles both strapping pins and dimms.
This CL is a first attempt at handling the "GPIO straps" problem as suggested by furquan - to move variant_memory_sku() into variants/baseboard/ which provides default GPIO straps".
Can you please follow up with CLs that add SMBUS support on top to see what the result looks like?
https://review.coreboot.org/c/coreboot/+/36492/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/include/variant/gpio.h:
https://review.coreboot.org/c/coreboot/+/36492/1/src/mainboard/google/hatch/... PS1, Line 22: #define GPIO_MEM_CONFIG_0 GPP_H19 This breaks helios b/c it defines its own variant_memory_sku which overrides the weak in baseboard and relies on these #defines.