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 7:
(5 comments)
Patchset:
PS6:
The information is put into firmware so that it's available immediately after power-on, as it handle […]
Sorry, is there a misunderstanding? I meant the coreboot firmware (not the firmware of the wifi controller).
File src/drivers/wifi/generic/acpi.c:
https://review.coreboot.org/c/coreboot/+/80170/comment/e7751c0d_39f9b779 : PS6, Line 45: }
I was following what was previously implemented for a similar feature. […]
Thanks!
https://review.coreboot.org/c/coreboot/+/80170/comment/056b45f4_b777171d : PS6, Line 592: uint8_t mtcl_package[sizeof(struct wifi_mtcl)];
The ACPI function realistically needs to handle a byte array. […]
Ok, I think I understand better now why you want this part to be more generic. But isn't actually everything in this patch Mediatek specific? I don't understand the split (acpigen/, drivers/wifi/, chromeos/) when everything has Mediatek in the name (assuming MTCL means mediatek country list).
File src/vendorcode/google/chromeos/mtcl.c:
PS6:
The functionality is being added specifically for Chromebooks and ChromeOS.
We generally name things by the chip manufacturers not the OEMs or who added it or alike. Also, vendorcode/ is officially for mirrored code that has its upstream somewhere else. I don't expect it to be the case here. The counter- part in Linux is regularly under drivers/net/wireless/, there is no reason to do anything else in coreboot.
`drivers/wifi/generic/` needs this, and if there is no alternative implementation, why not place it there?
https://review.coreboot.org/c/coreboot/+/80170/comment/a4f73970_47e506c5 : PS6, Line 59: if (mtcl_bin_len != sizeof(struct wifi_mtcl)) {
Because that would require callers to use this implementation, and also require all implementations […]
Do we expect any other callers, any other implementations?