Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39868 )
Change subject: mb/google/deltaur: Return SKU ID info ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39868/4/src/mainboard/google/deltau... File src/mainboard/google/deltaur/sku.c:
https://review.coreboot.org/c/coreboot/+/39868/4/src/mainboard/google/deltau... PS4, Line 17: sku_id Maybe board_sku_id?
https://review.coreboot.org/c/coreboot/+/39868/4/src/mainboard/google/deltau... File src/mainboard/google/deltaur/variants/baseboard/include/baseboard/variants.h:
https://review.coreboot.org/c/coreboot/+/39868/4/src/mainboard/google/deltau... PS4, Line 25: __weak buildbot is right, please no __weak in front of a declaration.
https://review.coreboot.org/c/coreboot/+/39868/4/src/mainboard/google/deltau... File src/mainboard/google/deltaur/variants/deltan/include/variant/variant.h:
https://review.coreboot.org/c/coreboot/+/39868/4/src/mainboard/google/deltau... PS4, Line 18: const static sku_info skus[] = { : /* Deltan 360 - invalid configuration */ : { .id = -1, .name = "sku_invalid" }, : /* Deltan */ : { .id = 1, .name = "sku1" }, : /* Deltan 360 signed - invalid configuration */ : { .id = -1, .name = "sku_invalid" }, : /* Deltan signed */ : { .id = 2, .name = "sku2" }, : }; Variable declarations rarely belong in header files.
Let's make a sku.c under each variant instead (deltaur/variants/deltaur/sku.c, and deltaur/variants/deltan/sku.c). variants.h can then have the prototypes for the functions in sku.c