mturney mturney has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37795 )
Change subject: sc7180: add qclib_post_fixup to allow SoC specific code ......................................................................
sc7180: add qclib_post_fixup to allow SoC specific code
* Provide hook for SoC specific code to run after QCLib
Change-Id: I5507187fddfd7d0af0ee0983b06d67077de0dbc8 Signed-off-by: T Michael Turney mturney@codeaurora.org --- M src/soc/qualcomm/common/include/soc/qclib_common.h M src/soc/qualcomm/common/qclib.c 2 files changed, 8 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/37795/1
diff --git a/src/soc/qualcomm/common/include/soc/qclib_common.h b/src/soc/qualcomm/common/include/soc/qclib_common.h index 19ec083..5817c08 100644 --- a/src/soc/qualcomm/common/include/soc/qclib_common.h +++ b/src/soc/qualcomm/common/include/soc/qclib_common.h @@ -72,5 +72,6 @@ uint32_t size, uint32_t attrs); void qclib_load_and_run(void); int qclib_soc_blob_load(void); +int qclib_post_fixup(void);
#endif // _SOC_QUALCOMM_QCLIB_COMMON_H_ diff --git a/src/soc/qualcomm/common/qclib.c b/src/soc/qualcomm/common/qclib.c index ac80a76..ce07078 100644 --- a/src/soc/qualcomm/common/qclib.c +++ b/src/soc/qualcomm/common/qclib.c @@ -119,6 +119,7 @@ }
__weak int qclib_soc_blob_load(void) { return 0; } +__weak int qclib_post_fixup(void) { return 0; }
void qclib_load_and_run(void) { @@ -209,6 +210,12 @@ mmu_restore_context(&pre_qclib_mmu_context); mmu_enable();
+ /* hook for SoC specific fixup after QCLib runs */ + if (qclib_post_fixup()) { + printk(BIOS_ERR, "qclib_post_fixup failed\n"); + goto fail; + } + /* step through I/F table, handling return values */ for (i = 0; i < qclib_cb_if_table.num_entries; i++) if (qclib_cb_if_table.te[i].blob_attributes &
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37795 )
Change subject: sc7180: add qclib_post_fixup to allow SoC specific code ......................................................................
Patch Set 8:
I don't see this used anywhere, why is this needed?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37795 )
Change subject: sc7180: add qclib_post_fixup to allow SoC specific code ......................................................................
Patch Set 8:
Oh, I see where it's used now. I don't think we need to introduce a new interface that's just used in a temporary HACK patch. We should fix the underlying problem instead.
mturney mturney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37795 )
Change subject: sc7180: add qclib_post_fixup to allow SoC specific code ......................................................................
Patch Set 8:
Patch Set 8:
Oh, I see where it's used now. I don't think we need to introduce a new interface that's just used in a temporary HACK patch. We should fix the underlying problem instead.
Agreed that the underlying problem needs to be addressed and it is. I added this callback as a safety-net for future issues that come up, perhaps on one board and not another. There is a similar callback ahead of running qclib so various boards can load non-standard blobs, e.g. While I can't identify a use-case now, this is future-proofing. Happy to leave or remove. Will re-visit when current bug in qclib is addressed and current need is removed.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37795 )
Change subject: sc7180: add qclib_post_fixup to allow SoC specific code ......................................................................
Patch Set 8:
Agreed that the underlying problem needs to be addressed and it is. I added this callback as a safety-net for future issues that come up, perhaps on one board and not another. There is a similar callback ahead of running qclib so various boards can load non-standard blobs, e.g. While I can't identify a use-case now, this is future-proofing. Happy to leave or remove. Will re-visit when current bug in qclib is addressed and current need is removed.
Let's not add interfaces before we need them. It doesn't cost anything to add them later.
mturney mturney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37795 )
Change subject: sc7180: add qclib_post_fixup to allow SoC specific code ......................................................................
Patch Set 9:
Patch Set 8:
Agreed that the underlying problem needs to be addressed and it is. I added this callback as a safety-net for future issues that come up, perhaps on one board and not another. There is a similar callback ahead of running qclib so various boards can load non-standard blobs, e.g. While I can't identify a use-case now, this is future-proofing. Happy to leave or remove. Will re-visit when current bug in qclib is addressed and current need is removed.
Let's not add interfaces before we need them. It doesn't cost anything to add them later.
I have squashed this change in with the HACK patch that used it. No way around introducing this interface to address this temporary problem, otherwise mistral board, that uses same common/qclib.c file is impacted. When trogdor qclib no longer requires workaround, this interface dies with the hack patch.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37795 )
Change subject: sc7180: add qclib_post_fixup to allow SoC specific code ......................................................................
Patch Set 9:
Looks like we didn't end up needing this? Can I abandon the patch?
mturney mturney has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/37795 )
Change subject: sc7180: add qclib_post_fixup to allow SoC specific code ......................................................................
Abandoned
Early patch that addressed transient occurrence, not needed for production.
-
Julius Werner (Code Review) -
mturney mturney (Code Review)