Attention is currently required from: Cliff Huang, David Ruth, Lance Zhao, Paul Menzel, Subrata Banik, Subrata Banik, Tim Wawrzynczak.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80170?usp=email )
Change subject: Add MTCL function to ACPI SSDT tables ......................................................................
Patch Set 6:
(7 comments)
Patchset:
PS6: Hi, I have a very general question: why is this information put into the firmware? Is it chip or board specific?
File src/acpi/acpigen.c:
https://review.coreboot.org/c/coreboot/+/80170/comment/be805c5e_46e529ce : PS6, Line 1968: acpigen_write_byte(bytes[i]); Is this valid AML? I would expect a Buffer() object around the bytes. What does the decompilation look like?
File src/drivers/wifi/generic/acpi.c:
https://review.coreboot.org/c/coreboot/+/80170/comment/60399bdb_b31b9635 : PS6, Line 45: } Why the weak function? Wouldn't it be an error to select USE_MTCL without a proper implementation?
https://review.coreboot.org/c/coreboot/+/80170/comment/745312d5_db4d21cd : PS6, Line 591: CONFIG(USE_MTCL) Putting this first would allow the compiler/linker to eliminate the other checks, in case it's disabled.
https://review.coreboot.org/c/coreboot/+/80170/comment/19eddb55_327c8d6a : PS6, Line 592: uint8_t mtcl_package[sizeof(struct wifi_mtcl)]; Why not declare a `struct wifi_mtcl`?
File src/vendorcode/google/chromeos/mtcl.c:
PS6: What is chromeos specific about this?
https://review.coreboot.org/c/coreboot/+/80170/comment/c1beb8e2_f6bf911e : PS6, Line 59: if (mtcl_bin_len != sizeof(struct wifi_mtcl)) { If this function knows `struct wifi_mtcl` why isn't it used in the signature?