EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35206 )
Change subject: lib/spd_bin: Extend DDR4 spd information ......................................................................
Patch Set 10:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35206/10/src/lib/spd_bin.c File src/lib/spd_bin.c:
https://review.coreboot.org/c/coreboot/+/35206/10/src/lib/spd_bin.c@71 PS10, Line 71: uint8_t
This is not expected to be modified, const ? Also see other spd_get_* below
ok, make sense to me.
https://review.coreboot.org/c/coreboot/+/35206/10/src/lib/spd_bin.c@73 PS10, Line 73: const
What about making all of these local arrays static? (see others spd_get_*)
I just keep the origin declaration, but static is fine.
https://review.coreboot.org/c/coreboot/+/35206/10/src/lib/spd_bin.c@144 PS10, Line 144: printk(BIOS_INFO, "SPD: module type is "); : printk(BIOS_INFO, "%s\n", spd_get_module_type_string(type));
nit: I don't see a good reason to break this up into two calls to printk, can you leave it as one?
I need warp the line anyway.