Attention is currently required from: Tanu Malhotra, Varshit Pandya, Yuval Peress.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81418?usp=email )
Change subject: mb/google/brox: Configure ISH device based on FW_CONFIG ......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/81418/comment/67d84b0c_b6e4a70c : PS2, Line 10: Similarly dma : property needs to be added only when UFS is enabled through STORAGE_UFS : FW_CONFIG.
the UFS related change is not captured in the title of the patch, may be split into two patches ?
Done. Please refer to my response in your other comment regarding why I am marking this as resolved. Also please refer to the devicetree change which indicates how UFS and ISH are related.
File src/mainboard/google/brox/variants/brox/fw_config.c:
https://review.coreboot.org/c/coreboot/+/81418/comment/575dee27_faa49f87 : PS2, Line 42: if (fw_config_probe(FW_CONFIG(STORAGE, STORAGE_UFS))) { : printk(BIOS_INFO, "Configure GPIOs, device config for UFS.\n"); : config->add_acpi_dma_property = true;
looks like a separate logical change, do you mind splitting this into two patches ?
No, they are one related change. The title of the change is saying that we are configuring the ISH device based on FW_CONFIG. FW_CONFIG has bit masks for both UFS and ISH. Also both UFS and ISH features need the ish pci device to be enabled and configured accordingly. That is what this change is doing.