Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36449 )
Change subject: mainboard/google: Allow Hatch variants to read SPD data over SMBus ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
I would still rather see one file with just the memcfg.spd parts in an if/else block.
Tim, the reasoning not using if/else blocks is because its isn't just a field set difference. The implementations are different. variant_memory_sku() in the base assumes variants define GPIO #defines which doesn't make sense for Puff so that ends up being wrapped as well. Further, mainboard_get_dram_part_num() isn't needed so that winds up being wrapped so we are just left with mainboard_memory_init_params() and diff'ing them reveals that although they are structurally similar they are semantically different as the configure FSP's struct is substantiated with totally different values.
The long story shorts is that it degenerates into having two files in one wrapped in large if-else blocks, with a tentative weak symbol in one path.
Is Puff considered a Hatch variant?