Attention is currently required from: Arthur Heymans, Gaggery Tsai, Subrata Banik.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77255?usp=email )
Change subject: src/soc/intel/common: Add ACPI BDAT support ......................................................................
Patch Set 10:
(5 comments)
File src/soc/intel/common/block/acpi/acpi_bdat.c:
https://review.coreboot.org/c/coreboot/+/77255/comment/6e6bf95b_a62578ae : PS10, Line 73: Determin Nit: Determine
https://review.coreboot.org/c/coreboot/+/77255/comment/b4de1bbd_1fcfbd6b : PS10, Line 86: if (schema[schema_count] != NULL) { Can the hob_size be 0 or should we check for that too?
https://review.coreboot.org/c/coreboot/+/77255/comment/5daa178b_10b07a35 : PS10, Line 136: sizeof(bdat_header_structure) + : sizeof(bdat_schema_list_structure) + Why not `sizeof(bdat_structure)` itself?
https://review.coreboot.org/c/coreboot/+/77255/comment/0d1a9f17_578721e1 : PS10, Line 160: bdat_header->bios_data_size = sizeof(bdat_structure) + : (schema_count * sizeof(uint32_t)); Nit: Might wrap in the previous line itself
File src/soc/intel/common/block/include/intelblocks/acpi_bdat.h:
https://review.coreboot.org/c/coreboot/+/77255/comment/6a3f1da6_8e69e8ae : PS10, Line 17: typedef struct { I have seen typedefs in utils and vendor specific codes like FSP UPDs. In other places we do not use it much. I am not sure if there is any guideline regarding when/where to use typedefs.