DTB co-firmware was previously requested and loaded in
qcom_pas_load(), but its lifetime did not match the actual
start/stop lifecycle of the remoteproc. As a result, the firmware
reference could be retained across restart cycles, leading to a
leak for each successful boot.
Additionally, if qcom_pas_start() failed after loading the DTB
firmware, the remoteproc core would not invoke .stop(), leaving
no opportunity to release the associated firmware reference.
Fix this by moving DTB firmware request and loading into
qcom_pas_start(), so that its lifetime is strictly tied to the
remoteproc start sequence.
Update qcom_pas_start() to ensure proper cleanup on all paths:
- release PAS metadata on failure
- release DTB firmware on both success and failure paths
- unmap DTB carveout where applicable
Remove DTB firmware handling from qcom_pas_load(), as it does not
match the correct ownership and lifecycle model.
With this change, request_firmware() and release_firmware() are
properly paired within the start path, avoiding leaks and ensuring
consistent behavior across restart and failure scenarios.
Fixes: 29814986b82e ("remoteproc: qcom_q6v5_pas: add support for dtb
co-firmware loading")
Signed-off-by: Sailesh Nandanavanam <[email protected]>
---
v2:
- Move DTB firmware request/load from qcom_pas_load() to qcom_pas_start()
- Fix firmware reference leak across restart cycles
- Handle start() failure paths where .stop() is not invoked
- Ensure firmware is released on all success and failure paths
- Remove DTB handling from load() and drop release from stop()
---
drivers/remoteproc/qcom_q6v5_pas.c | 54 ++++++++++++++++--------------
1 file changed, 29 insertions(+), 25 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c
b/drivers/remoteproc/qcom_q6v5_pas.c
index da27d1d3c9da..090f1f09dba3 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -232,28 +232,7 @@ static int qcom_pas_load(struct rproc *rproc, const struct
firmware *fw)
if (pas->lite_dtb_pas_id)
qcom_scm_pas_shutdown(pas->lite_dtb_pas_id);
- if (pas->dtb_pas_id) {
- ret = request_firmware(&pas->dtb_firmware,
pas->dtb_firmware_name, pas->dev);
- if (ret) {
- dev_err(pas->dev, "request_firmware failed for %s:
%d\n",
- pas->dtb_firmware_name, ret);
- return ret;
- }
-
- ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
- pas->dtb_firmware_name,
pas->dtb_mem_region,
- &pas->dtb_mem_reloc);
- if (ret)
- goto release_dtb_metadata;
- }
-
return 0;
-
-release_dtb_metadata:
- qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
- release_firmware(pas->dtb_firmware);
-
- return ret;
}
static void qcom_pas_unmap_carveout(struct rproc *rproc, phys_addr_t mem_phys,
size_t size)
@@ -277,9 +256,24 @@ static int qcom_pas_start(struct rproc *rproc)
struct qcom_pas *pas = rproc->priv;
int ret;
+ if (pas->dtb_pas_id) {
+ ret = request_firmware(&pas->dtb_firmware,
pas->dtb_firmware_name, pas->dev);
+ if (ret) {
+ dev_err(pas->dev, "request_firmware failed for %s:
%d\n",
+ pas->dtb_firmware_name, ret);
+ return ret;
+ }
+
+ ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
+ pas->dtb_firmware_name, pas->dtb_mem_region,
+ &pas->dtb_mem_reloc);
+ if (ret)
+ goto release_dtb_metadata;
+ }
+
ret = qcom_q6v5_prepare(&pas->q6v5);
if (ret)
- return ret;
+ goto release_dtb_metadata;
ret = qcom_pas_pds_enable(pas, pas->proxy_pds, pas->proxy_pd_count);
if (ret < 0)
@@ -350,15 +344,17 @@ static int qcom_pas_start(struct rproc *rproc)
/* firmware is used to pass reference from qcom_pas_start(), drop it
now */
pas->firmware = NULL;
+ if (pas->dtb_firmware) {
+ release_firmware(pas->dtb_firmware);
+ pas->dtb_firmware = NULL;
+ }
+
return 0;
unmap_carveout:
qcom_pas_unmap_carveout(rproc, pas->mem_phys, pas->mem_size);
release_pas_metadata:
qcom_scm_pas_metadata_release(pas->pas_ctx);
- if (pas->dtb_pas_id)
- qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
-
unmap_dtb_carveout:
if (pas->dtb_pas_id)
qcom_pas_unmap_carveout(rproc, pas->dtb_mem_phys,
pas->dtb_mem_size);
@@ -376,6 +372,14 @@ static int qcom_pas_start(struct rproc *rproc)
qcom_pas_pds_disable(pas, pas->proxy_pds, pas->proxy_pd_count);
disable_irqs:
qcom_q6v5_unprepare(&pas->q6v5);
+release_dtb_metadata:
+ if (pas->dtb_pas_id)
+ qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
+release_dtb_firmware:
+ if (pas->dtb_firmware) {
+ release_firmware(pas->dtb_firmware);
+ pas->dtb_firmware = NULL;
+ }
/* firmware is used to pass reference from qcom_pas_start(), drop it
now */
pas->firmware = NULL;
--
2.34.1
