Attention is currently required from: David Wu, Ren Kuo, Subrata Banik.
Karthik Ramasubramanian has posted comments on this change by Ren Kuo. ( https://review.coreboot.org/c/coreboot/+/85107?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: mb/google/brox/var/jubilant: Add fw_config for WWAN Sar Sensor ......................................................................
Patch Set 2:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/85107/comment/4d08a01d_e171cb79?usp... : PS2, Line 10: remvoed Nit: removed
https://review.coreboot.org/c/coreboot/+/85107/comment/7b18d4cb_484f83c3?usp... : PS2, Line 18: to Nit: remove 'to'
File src/mainboard/google/brox/variants/jubilant/Makefile.mk:
https://review.coreboot.org/c/coreboot/+/85107/comment/b96de3e9_3f78119f?usp... : PS2, Line 14: smm-y-$(CONFIG_FW_CONFIG) smm-$(CONFIG_FW_CONFIG)
File src/mainboard/google/brox/variants/jubilant/smihandler.c:
https://review.coreboot.org/c/coreboot/+/85107/comment/93ecac24_e1d76350?usp... : PS2, Line 16: if (slp_typ == ACPI_S5) { : if (!fw_config_probe(FW_CONFIG(DB_USB, DB_1A))) { I would recommend to check specifically for LTE and LTE SAR config. This will help in the future when you have another Daughter board without LTE config. That way you don't have to come and change here.
``` if (slp_typ == ACPI_S5 && (fw_config_probe(FW_CONFIG(DB_USB, DB_1A_LTE)) || (fw_config_probe(FW_CONFIG(DB_USB, DB_1A_LTE_SAR)) { ... } ```
File src/mainboard/google/brox/variants/jubilant/variant.c:
https://review.coreboot.org/c/coreboot/+/85107/comment/06802900_a142a802?usp... : PS2, Line 41: if (!fw_config_probe(FW_CONFIG(DB_USB, DB_1A))) { Same comment as in the other file:
``` if (fw_config_probe(FW_CONFIG(DB_USB, DB_1A_LTE) || fw_config_probe(FW_CONFIG(DB_USB, DB_1A_LTE_SAR))) { ... } ```