On 20/09/2025 20:41, Mukesh Ojha wrote:
Currently, remoteproc and non-remoteproc subsystems use different
variants of the MDT loader helper API, primarily due to the handling of
the metadata context. Remoteproc subsystems retain this context until
authentication and reset, while non-remoteproc subsystems (e.g., video,
graphics) do not need to retain it and it is freed inside
qcom_scm_pas_init() based on metadata context parameter being NULL.
The above paragraph seems almost 1:1 with the commit log of the previous
patch.
You'd be better off describing in the commit log what you mean by
context here. Not a thread context which is the common meaning of
context. Instead of restating the paragraph from your previous commit,
give the reader of your commit log new information - for example - what
do you mean by context here ?
Add context aware qcom_mdt_pas_load() function which uses context
returned from qcom_scm_pas_ctx_init() and use it till subsystems
is out of set.
"Add a context function qcom_mdt_pas_load() which uses a context
pointer/handle? return from qcom_scm_pas_init() then use that handle
until? a subsystem is out of reset ?
Your commit log doesn't match the code below, which simply provides the
API you will then subsequently build on.
This will also help in unifying the API used by
remoteproc and non-remoteproc subsystems drivers in future and
will also help in early setting context to all the PAS SMC function
to do appropriate things when SoC is running with gunyah hypervisor
or in absence of it.
Your argument for unification should go into the cover letter - this
specific commit is adding a function - not performing that unification
so in my opinion - as a reviewer I should see this argument for
unification @ the cover-letter level and/or at the commit which
implements the unification not the commit that adds the API to
facilitate the unification.
Signed-off-by: Mukesh Ojha <mukesh.o...@oss.qualcomm.com>
---
drivers/soc/qcom/mdt_loader.c | 15 +++++++++++++++
include/linux/soc/qcom/mdt_loader.h | 10 ++++++++++
2 files changed, 25 insertions(+)
diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
index a5c80d4fcc36..2ef079797f05 100644
--- a/drivers/soc/qcom/mdt_loader.c
+++ b/drivers/soc/qcom/mdt_loader.c
@@ -486,5 +486,20 @@ int qcom_mdt_load_no_init(struct device *dev, const struct
firmware *fw,
}
EXPORT_SYMBOL_GPL(qcom_mdt_load_no_init);
+int qcom_mdt_pas_load(struct qcom_scm_pas_ctx *ctx, const struct firmware *fw,
+ const char *firmware, void *mem_region, phys_addr_t
*reloc_base)
+{
+ int ret;
+
+ ret = qcom_mdt_pas_init(ctx->dev, fw, firmware, ctx->pas_id,
+ ctx->mem_phys, ctx->metadata);
+ if (ret)
+ return ret;
+
+ return __qcom_mdt_load(ctx->dev, fw, firmware, mem_region,
ctx->mem_phys,
+ ctx->mem_size, reloc_base);
+}
+EXPORT_SYMBOL_GPL(qcom_mdt_pas_load);
+
MODULE_DESCRIPTION("Firmware parser for Qualcomm MDT format");
MODULE_LICENSE("GPL v2");
diff --git a/include/linux/soc/qcom/mdt_loader.h
b/include/linux/soc/qcom/mdt_loader.h
index 8ea8230579a2..36b8b331ce5f 100644
--- a/include/linux/soc/qcom/mdt_loader.h
+++ b/include/linux/soc/qcom/mdt_loader.h
@@ -11,6 +11,7 @@
struct device;
struct firmware;
struct qcom_scm_pas_metadata;
+struct qcom_scm_pas_ctx;
#if IS_ENABLED(CONFIG_QCOM_MDT_LOADER)
@@ -23,6 +24,9 @@ int qcom_mdt_load(struct device *dev, const struct firmware
*fw,
phys_addr_t mem_phys, size_t mem_size,
phys_addr_t *reloc_base);
+int qcom_mdt_pas_load(struct qcom_scm_pas_ctx *ctx, const struct firmware *fw,
+ const char *firmware, void *mem_region, phys_addr_t
*reloc_base);
+
int qcom_mdt_load_no_init(struct device *dev, const struct firmware *fw,
const char *fw_name, void *mem_region,
phys_addr_t mem_phys, size_t mem_size,
@@ -52,6 +56,12 @@ static inline int qcom_mdt_load(struct device *dev, const
struct firmware *fw,
return -ENODEV;
}
+int qcom_mdt_pas_load(struct qcom_scm_pas_ctx *ctx, const struct firmware *fw,
+ const char *firmware, void *mem_region, phys_addr_t
*reloc_base)
+{
+ return -ENODEV;
+}
+
static inline int qcom_mdt_load_no_init(struct device *dev,
const struct firmware *fw,
const char *fw_name, void *mem_region,
--
2.50.1
