Attention is currently required from: Bora Guvendik, Anil Kumar K, Selma Bensaid, Tim Wawrzynczak, Paul Menzel, Thejaswani Putta. Cliff Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61354 )
Change subject: drivers/wwan/fm: Add Fibocom 5G WWAN ACPI support ......................................................................
Patch Set 5:
(11 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/61354/comment/c80b4f02_f26975f8 PS2, Line 10: when
thanks
Done
https://review.coreboot.org/c/coreboot/+/61354/comment/c44ca835_800e7a95 PS2, Line 9: Support PXSX._RST and PXSX.MRST._RST for warm and cold reset. : PXSX._RST is invoked when driver removal. : : Test: : Add chip entry to the corresponding root port and check PXSX Device : is generated in ssdt.
sure
Done
File src/drivers/wwan/fm/acpi_fm350gl.c:
https://review.coreboot.org/c/coreboot/+/61354/comment/bb09c53b_6603707d PS2, Line 29:
FHRF: first half reset flow and arg0. I will add the missing header.
Done
https://review.coreboot.org/c/coreboot/+/61354/comment/526b41d8_826f814c PS2, Line 44: warm reset
Arg0 = 0: warm reset. […]
Done
https://review.coreboot.org/c/coreboot/+/61354/comment/467a2582_14a4cc75 PS2, Line 61:
Can you add a comment here about what SHRF does?
Done
https://review.coreboot.org/c/coreboot/+/61354/comment/2cdea57b_ca4c1f85 PS2, Line 90: acpi_device_path_join(parent_dev, "RTD3._OFS")
nit: this is used twice, extract to a local variable
Done
https://review.coreboot.org/c/coreboot/+/61354/comment/b408742d_ab5ed671 PS2, Line 90: acpigen_write_store_int_to_namestr(1, acpi_device_path_join(parent_dev, "RTD3._OFS"));
To ensure that these _ON and _OFF calls are properly paired, I would rather see these incremented an […]
Done
https://review.coreboot.org/c/coreboot/+/61354/comment/75e11632_5f53489e PS2, Line 106: acpigen_write_store_int_to_namestr(1, acpi_device_path_join(parent_dev, "RTD3._OFS"));
ok. Let me change that.
Done
https://review.coreboot.org/c/coreboot/+/61354/comment/551c5f72_bde1409f PS2, Line 106: acpi_device_path_join(parent_dev, "RTD3._OFS")
I will keep the last one.
Done
https://review.coreboot.org/c/coreboot/+/61354/comment/563ed210_3533ad6b PS2, Line 167: .read_resources = noop_read_resources, : .set_resources = noop_set_resources, : .acpi_fill_ssdt = wwan_fm350gl_acpi_fill_ssdt, : .acpi_name = wwan_fm350gl_acpi_name, : };
ok
Done
File src/drivers/wwan/fm/chip.h:
https://review.coreboot.org/c/coreboot/+/61354/comment/7e595d08_33dea0de PS2, Line 12: fcpo_asserted_delay_ms
in other drivers that do power sequencing, we usually call these […]
Done
-
Cliff Huang (Code Review)