Attention is currently required from: Subrata Banik, Maulik V Vaghela, Tim Wawrzynczak, Paul Menzel, Rizwan Qureshi, Sugnan Prabhu S. Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61020 )
Change subject: src/driver/wifi: Add _DSM method for DDRRFIM ......................................................................
Patch Set 18:
(5 comments)
File src/drivers/wifi/generic/acpi.c:
https://review.coreboot.org/c/coreboot/+/61020/comment/df1f7da1_e85aa868 PS16, Line 40: bool is_cnviDdrRfim_enabled = false;
I don't think we need to make this variable a global, see below.
Done
https://review.coreboot.org/c/coreboot/+/61020/comment/80a5410d_e0422175 PS16, Line 160: acpigen_write_return_integer(is_cnviDdrRfim_enabled);
with my suggestions from below, this would now look like […]
Done
https://review.coreboot.org/c/coreboot/+/61020/comment/09724d6c_5b0fbdcb PS16, Line 540: is_cnviDdrRfim_enabled = config->enable_cnvi_ddr_rfim;
with suggestions, this is now a local variable
Done
https://review.coreboot.org/c/coreboot/+/61020/comment/521e0a96_87f9d9f1 PS16, Line 607: NULL
In order to avoid having `is_cnviDdrRfim_enabled` as a global variable, I think you can make it a lo […]
Done
https://review.coreboot.org/c/coreboot/+/61020/comment/14e46007_4858a236 PS16, Line 649: wifi_ssdt_write_properties(dev, path);
I think instead we should pass the `generic` device here or at least grab the CNVi property here, e. […]
I put some prints and saw that wifi_pcie_fill_ssdt is getting called and wifi_cnvi_fill_ssdt is not called. probably this is because wifi_generic_enable is not getting called in generic.c the ops are getting assigned from generic pci driver.
-
Varshit B Pandya (Code Review)