Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36764 )
Change subject: src/mainboard/ocp/monolake: Create SMBIOS type 16 for Monolake platform ......................................................................
Patch Set 6:
(7 comments)
https://review.coreboot.org/c/coreboot/+/36764/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36764/6//COMMIT_MSG@7 PS6, Line 7: src/mainboard/ocp/monolake: Create SMBIOS type 16 for Monolake platform Please remove *src* from the prefix, and you can abbreviate *mainboard*.
mb/ocp/monolake: …
https://review.coreboot.org/c/coreboot/+/36764/6//COMMIT_MSG@9 PS6, Line 9: can`t Wrong apostrophe: can’t or can't
https://review.coreboot.org/c/coreboot/+/36764/6//COMMIT_MSG@10 PS6, Line 10: it`s Ditto.
https://review.coreboot.org/c/coreboot/+/36764/6/src/mainboard/ocp/monolake/... File src/mainboard/ocp/monolake/mainboard.c:
https://review.coreboot.org/c/coreboot/+/36764/6/src/mainboard/ocp/monolake/... PS6, Line 33: 1 One space?
https://review.coreboot.org/c/coreboot/+/36764/6/src/mainboard/ocp/monolake/... PS6, Line 34: #define MAX_DIMM_SIZE_GB (32UL << 20) We have a macro MiB: 32 * MiB.
https://review.coreboot.org/c/coreboot/+/36764/6/src/mainboard/ocp/monolake/... PS6, Line 72: printk(BIOS_INFO, "Create SMBIOS type 16\n"); Is it used like this in the other parts of the code?
Creating SMBIOS tables type 16 (note, ECC information is hard-coded) …
without line break and in the end
printk(BIOS_INFO, "done\n");
https://review.coreboot.org/c/coreboot/+/36764/6/src/mainboard/ocp/monolake/... PS6, Line 117: #if CONFIG(GENERATE_SMBIOS_TABLES) Can’t you use the C if statement and include the new function without getting an unused function error?
if (CONFIG(GENERATE_SMBIOS_TABLES)) dev->ops->get_smbios_data = write_smbios_type16;