Attention is currently required from: Arthur Heymans, Cliff Huang, Lance Zhao, Nico Huber, Paul Menzel, Subrata Banik, Subrata Banik, Tim Wawrzynczak.
David Ruth 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 12:
(5 comments)
Patchset:
PS6:
First of all, don't worry about this comment. I'm merely curious. If you think […]
The CBFS files are specific to board sub types, it's convenient to be able to specify that different subtypes of board have different MTCL data
File src/acpi/acpigen.c:
https://review.coreboot.org/c/coreboot/+/80170/comment/faf3a7c0_6445732f : PS8, Line 1940: * Generate ACPI AML code for MTCL method.
Looking around, it seems there really are only standardized things […]
Thanks! I ended up just moving this functionality to the mtcl.c file, and completely isolating the MTCL specific functionality. I think it cleaned up the implementation quite a bit.
File src/drivers/wifi/generic/mtcl.c:
https://review.coreboot.org/c/coreboot/+/80170/comment/8ef41321_612a4c17 : PS8, Line 52: cbfs_map(WIFI_MTCL_CBFS_DEFAULT_FILENAME, &mtcl_bin_len);
Why not use this pointer instead of having the caller provide a buffer (which may be too small btw). […]
Refactored the file to do this. Put all functionality related to reading the file and writing to the SSDT in the function write_mtcl_function.
File src/include/mtcl.h:
https://review.coreboot.org/c/coreboot/+/80170/comment/215532ce_54bacefb : PS8, Line 21: __packed
Not necessary as all entries are u8?
I think I'll leave it as packed only because it's used to do a cast from an array. I think that helps with clarity, let me know if you think it deserves an explicit comment saying as much.
File src/vendorcode/google/chromeos/mtcl.c:
https://review.coreboot.org/c/coreboot/+/80170/comment/c0f3ce09_041c29d7 : PS6, Line 59: if (mtcl_bin_len != sizeof(struct wifi_mtcl)) {
and keep them localized to this file. […]
Once I moved the acpigen bits to mtcl.c, I was able to completely isolate this implementation detail and remove the sizeof and avoid allocating the memory ahead of time. I think this cleans up the implementation in a worthwhile way.