[coreboot-gerrit] Change in coreboot[master]: lib/spd_bin: Extend DDR4 spd information

[EricR Lai (Code Review)]

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 13:

(7 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@64 
PS10, Line 64: 	} else {
> Ack

Done

https://review.coreboot.org/c/coreboot/+/35206/10/src/lib/spd_bin.c@71 
PS10, Line 71: uint8_t
> ok, make sense to me.

Done

https://review.coreboot.org/c/coreboot/+/35206/10/src/lib/spd_bin.c@73 
PS10, Line 73: const
> I would prefer static const

Done

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));
> I need warp the line anyway.

Done

https://review.coreboot.org/c/coreboot/+/35206/10/src/lib/spd_bin.c@146 
PS10, Line 146: /* Module Part Number */
              : 	switch (type) {
              : 	case SPD_DRAM_DDR3:
              : 		memcpy(spd_name, &spd[DDR3_SPD_PART_OFF], DDR3_SPD_PART_LEN);
              : 		spd_name[DDR3_SPD_PART_LEN] = 0;
              : 		break;
              : 	case SPD_DRAM_LPDDR3_INTEL:
              : 	case SPD_DRAM_LPDDR3_JEDEC:
              : 		memcpy(spd_name, &spd[LPDDR3_SPD_PART_OFF],
              : 			LPDDR3_SPD_PART_LEN);
              : 		spd_name[LPDDR3_SPD_PART_LEN] = 0;
              : 		break;
              : 	case SPD_DRAM_DDR4:
              : 		memcpy(spd_name, &spd[DDR4_SPD_PART_OFF], DDR4_SPD_PART_LEN);
              : 		spd_name[DDR4_SPD_PART_LEN] = 0;
              : 		break;
              : 	default:
              : 		break;
              : 	}
> Have a helper for Module Part number as well?

Done

https://review.coreboot.org/c/coreboot/+/35206/11/src/lib/spd_bin.c 
File src/lib/spd_bin.c:

https://review.coreboot.org/c/coreboot/+/35206/11/src/lib/spd_bin.c@171 
PS11, Line 171: 	printk(BIOS_INFO, "SPD: module type is %s\n",
> trailing whitespace

Done

https://review.coreboot.org/c/coreboot/+/35206/12/src/lib/spd_bin.c 
File src/lib/spd_bin.c:

https://review.coreboot.org/c/coreboot/+/35206/12/src/lib/spd_bin.c@171 
PS12, Line 171: 	printk(BIOS_INFO, "SPD: module type is %s\n",
> trailing whitespace

Done