Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45177 )
Change subject: soc/amd/picasso: Use lookup table to convert MHz to MT/s ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45177/3/src/soc/amd/picasso/dmi.c File src/soc/amd/picasso/dmi.c:
https://review.coreboot.org/c/coreboot/+/45177/3/src/soc/amd/picasso/dmi.c@2... PS3, Line 21: static uint16_t convert_speed_mhz_to_mts(uint16_t speed_mhz)
I'd also place this function in common code, so that other platforms can use it as well.
In my opinion, it would be simpler to have a table that matches the range of Mhz to standard MT/s values as defined by the DDR spec. It removes any scope for error and inconsistency. Also, easier to ensure that it aligns with the userspace tool expectations since other pieces would be aligning to the DDR spec as well.
https://review.coreboot.org/c/coreboot/+/45177/3/src/soc/amd/picasso/dmi.c@2... PS3, Line 23: speed_mhz I don't think all the speeds and MT/s values are really supported by DDR4 spec. I think we should only put in the supported values and print out an error for rest.