Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47972 )
Change subject: mb/google/volteer: Make use of fw_config_is_provisioned() ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/47972/2/src/mainboard/google/voltee... File src/mainboard/google/volteer/fw_config.c:
https://review.coreboot.org/c/coreboot/+/47972/2/src/mainboard/google/voltee... PS2, Line 74: { nit: If you handle the unprovisioned case first, then you don't need to align the rest of the code by one additional tab.
if (!fw_config_is_provisioned()) { ... return; }
https://review.coreboot.org/c/coreboot/+/47972/2/src/mainboard/google/voltee... File src/mainboard/google/volteer/romstage.c:
https://review.coreboot.org/c/coreboot/+/47972/2/src/mainboard/google/voltee... PS2, Line 27: mem_cfg->PchHdaEnable = 0; Not for this change, but I think, we should update baseboard/devicetree.cb so that `device ref hda on end` uses probe statements to enable HDA device. And then SoC code can set PchHdaEnable to 1/0 depending upon the state of the device. This will get rid of the checks here.
https://review.coreboot.org/c/coreboot/+/47972/2/src/mainboard/google/voltee... PS2, Line 35: mem_cfg->TcssDma0En = 0; : mem_cfg->TcssItbtPcie0En = 0; : mem_cfg->TcssItbtPcie1En = 0; I think we already have devices in devicetree that correspond to each of these UPDs. In fact I see TGL SoC code setting these UPDs already. I think we should check if this code is really required. I am suspecting this is just some old code before we had all the fw_config setup done properly. Same for: https://review.coreboot.org/cgit/coreboot.git/tree/src/mainboard/google/volt...