Edward O'Callaghan 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:
(1 comment)
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
I would say the assumptions are breaking down from original implementation: memory down and memory s […]
Yep that's fine as a better reword to clarify that 'unreasonable' means the assumption changed here.
This patch is to exercise out the ideas set out by Furquan in https://review.coreboot.org/c/coreboot/+/36250 where djkurtz and I discussed locally to flesh it out for comparison. Talking with djkurtz, he asked me to take the journey and see which solution looks best in the end.
It is my feeling that variant_memory_sku() doesn't make sense at all for seated dimms nor mainboard_get_dram_part_num() in the same file leaving only mainboard_memory_init_params(). However in the case of mainboard_memory_init_params() the actual implementation is different enough that they may as well be separate functions in separate object files depending on use-case rather than splitting control-flow with the pre-processor involvement. The benefit of doing the split is that it is less invasive as well.
Hope this add some extra context why this got forked off as a off-shot to the side.