Roja Rani Yarubandi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35499 )
Change subject: sc7180: Add QUPv3 FW load & config ......................................................................
Patch Set 37:
(7 comments)
Patch Set 26:
(7 comments)
Done the changes, and shared patches with Mike.
https://review.coreboot.org/c/coreboot/+/35499/26/src/soc/qualcomm/sc7180/in... File src/soc/qualcomm/sc7180/include/soc/addressmap.h:
https://review.coreboot.org/c/coreboot/+/35499/26/src/soc/qualcomm/sc7180/in... PS26, Line 21:
Please standardize on tabs, not spaces for header #defines (and keep it in the same column for all # […]
Done
https://review.coreboot.org/c/coreboot/+/35499/26/src/soc/qualcomm/sc7180/in... File src/soc/qualcomm/sc7180/include/soc/qcom_qup_se.h:
https://review.coreboot.org/c/coreboot/+/35499/26/src/soc/qualcomm/sc7180/in... PS26, Line 233:
This sort of alignment should also be done with tabs if possible.
removed this struct as we are not using it anymore
https://review.coreboot.org/c/coreboot/+/35499/26/src/soc/qualcomm/sc7180/qu... File src/soc/qualcomm/sc7180/qupv3_config.c:
https://review.coreboot.org/c/coreboot/+/35499/26/src/soc/qualcomm/sc7180/qu... PS26, Line 37: struct se_cfg *scfg)
This se_cfg needs to go as well then, obviously (see below). […]
Done the changes
https://review.coreboot.org/c/coreboot/+/35499/26/src/soc/qualcomm/sc7180/qu... PS26, Line 196: fw_list[SE_PROTOCOL_SPI] =
You should check if fw_list[protocol] is NULL here, and avoid reloading the same blob again if it's […]
Done
https://review.coreboot.org/c/coreboot/+/35499/26/src/soc/qualcomm/sc7180/qu... PS26, Line 211: printk(BIOS_INFO, "%s: *ERROR* *INVALID PROTOCOL****\n",
You can use assert() or die() for this.
Done
https://review.coreboot.org/c/coreboot/+/35499/26/src/soc/qualcomm/sc7180/qu... PS26, Line 220: printk(BIOS_INFO, "%s: *ERROR* *PROTOCOL MISMATCH****\n",
I... am very confused what this is supposed to be. […]
Done the changes as per the comments
https://review.coreboot.org/c/coreboot/+/35499/26/src/soc/qualcomm/sc7180/qu... File src/soc/qualcomm/sc7180/qupv3_fw_config.c:
https://review.coreboot.org/c/coreboot/+/35499/26/src/soc/qualcomm/sc7180/qu... PS26, Line 1: /*
I don't understand what makes this file separate from qupv3_config.c. I think you can merge the two.
Done