Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36091 )
Change subject: mb/intel/tigerlake_rvp: Do initial mainboard commit ......................................................................
Patch Set 8: Code-Review-1
(4 comments)
There is so much unused stuff on this mainboard port that I had to download this patch to be able to grep if functions are even called...
https://review.coreboot.org/c/coreboot/+/36091/8/src/mainboard/intel/tigerla... File src/mainboard/intel/tigerlake_rvp/spd/spd_util.c:
https://review.coreboot.org/c/coreboot/+/36091/8/src/mainboard/intel/tigerla... PS8, Line 73: void *dqs_map_ptr You know the size it needs from UPD's, so why not use that for the signature of these functions and use it in the sizeof() the memcpy?
https://review.coreboot.org/c/coreboot/+/36091/8/src/mainboard/intel/tigerla... File src/mainboard/intel/tigerlake_rvp/variants/baseboard/include/baseboard/variants.h:
https://review.coreboot.org/c/coreboot/+/36091/8/src/mainboard/intel/tigerla... PS8, Line 31: struct memory_config { : const void *dq_map; : size_t dq_map_size; : const void *dqs_map; : size_t dqs_map_size; : u16 rcomp_resistor; : const void *rcomp_target; : size_t rcomp_target_size; : }; Having an in mainboard API to just for variants to pass on some memory configuration to a different part, presumably common code is bad!
https://review.coreboot.org/c/coreboot/+/36091/8/src/mainboard/intel/tigerla... PS8, Line 40: : size_t variant_memory_sku(void); unused. Just 2 weak implementations...
https://review.coreboot.org/c/coreboot/+/36091/8/src/mainboard/intel/tigerla... PS8, Line 42: void variant_memory_params(struct memory_config *mem_config); unused!
-
Arthur Heymans (Code Review)