Attention is currently required from: Bora Guvendik, Anil Kumar K, Cliff Huang, Selma Bensaid, Paul Menzel, Thejaswani Putta. Tim Wawrzynczak 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 2:
(17 comments)
File src/drivers/wwan/fm/acpi_fm350gl.c:
https://review.coreboot.org/c/coreboot/+/61354/comment/e99258ba_47e647c7 PS2, Line 25: initializatioin `initialization`
https://review.coreboot.org/c/coreboot/+/61354/comment/1ca7b7ed_0e1d7eea PS2, Line 26: rtd3 `RTD3`
https://review.coreboot.org/c/coreboot/+/61354/comment/458809d0_3d21cb36 PS2, Line 57: acpigen_pop_len `acpigen_write_if_end()`
https://review.coreboot.org/c/coreboot/+/61354/comment/119dd725_87a848fc PS2, Line 58: acpigen_pop_len We don't really have an `acpigen_write_else_end()` that I see
https://review.coreboot.org/c/coreboot/+/61354/comment/e662f7f4_f9eef9a9 PS2, Line 59: acpigen_pop_len `acpigen_write_method_end()`
https://review.coreboot.org/c/coreboot/+/61354/comment/4af08c51_588fd937 PS2, Line 82: acpigen_pop_len `acpigen_write_method_end()`
https://review.coreboot.org/c/coreboot/+/61354/comment/c7f42249_fd2859dd PS2, Line 98: acpigen_pop_len `acpigen_write_method_end`
https://review.coreboot.org/c/coreboot/+/61354/comment/2be17725_b1b16ae0 PS2, Line 114: acpigen_pop_len `acpigen_write_method_end`
https://review.coreboot.org/c/coreboot/+/61354/comment/5a42a8f1_d29adae1 PS2, Line 155: the 5G driver is looking for MRST._RST for cold reset called during : * firmware update. suggestion: ``` NOTE: the 5G driver will call MRST._RST to trigger a cold reset during firmware update. ```
https://review.coreboot.org/c/coreboot/+/61354/comment/1f081cc8_16e4c056 PS2, Line 161: acpigen_pop_len We added a few new niceities for this like `acpigen_write_device_end()`
https://review.coreboot.org/c/coreboot/+/61354/comment/467b3edc_2b85b975 PS2, Line 162: acpigen_pop_len `acpigen_write_device_end()`
https://review.coreboot.org/c/coreboot/+/61354/comment/9c8af938_edbe4bb3 PS2, Line 163: acpigen_pop_len `acpigen_write_scope_end()`
File src/drivers/wwan/fm/chip.h:
https://review.coreboot.org/c/coreboot/+/61354/comment/c15fd32e_c58b7d5e PS2, Line 10: fcpo nit: `fcpo_gpio`
https://review.coreboot.org/c/coreboot/+/61354/comment/f9f42899_e0cf2c10 PS2, Line 12: fcpo_asserted_delay_ms in other drivers that do power sequencing, we usually call these `X_on_delay_ms` and `X_off_delay_ms` etc.
mind changing all of these here?
https://review.coreboot.org/c/coreboot/+/61354/comment/11a9ea92_2c06f583 PS2, Line 17: reset nit: `reset_gpio`
https://review.coreboot.org/c/coreboot/+/61354/comment/07b11c88_815d79b1 PS2, Line 24: perst nit: `perst_gpio`
https://review.coreboot.org/c/coreboot/+/61354/comment/6b9662fe_f4f83652 PS2, Line 30: wake nit: `wake_gpio`