* [PATCH v14 1/9] firmware: qcom_scm: ipq5332: add support to pass metadata size
2026-08-03 10:46 [PATCH v14 0/9] Add new driver for WCSS secure PIL loading Varadarajan Narayanan
@ 2026-08-03 10:46 ` Varadarajan Narayanan
2026-08-03 10:55 ` sashiko-bot
2026-08-03 12:35 ` Mukesh Ojha
2026-08-03 10:46 ` [PATCH v14 2/9] dt-bindings: remoteproc: qcom: document hexagon based WCSS secure PIL Varadarajan Narayanan
` (7 subsequent siblings)
8 siblings, 2 replies; 20+ messages in thread
From: Varadarajan Narayanan @ 2026-08-03 10:46 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu
Cc: linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
Konrad Dybcio, Gokul Sriram Palanisamy, Dmitry Baryshkov,
Vignesh Viswanathan, Varadarajan Narayanan
From: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
IPQ5332 security software running under trustzone requires metadata size.
With new command support added in TrustZone that includes a size parameter,
pass metadata size as well.
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
Signed-off-by: Gokul Sriram Palanisamy <gokul.sriram.p@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Tested-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
drivers/firmware/qcom/qcom_scm.c | 16 ++++++++++++----
drivers/firmware/qcom/qcom_scm.h | 1 +
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 3eaa4c9ccf3c..8902c445cf19 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -598,7 +598,7 @@ EXPORT_SYMBOL_GPL(devm_qcom_scm_pas_context_alloc);
static int __qcom_scm_pas_init_image(struct device *dev, u32 pas_id,
dma_addr_t mdata_phys,
- struct qcom_scm_res *res)
+ struct qcom_scm_res *res, size_t size)
{
struct qcom_scm_desc desc = {
.svc = QCOM_SCM_SVC_PIL,
@@ -619,6 +619,14 @@ static int __qcom_scm_pas_init_image(struct device *dev, u32 pas_id,
desc.args[1] = mdata_phys;
+ if (__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_PIL,
+ QCOM_SCM_PIL_PAS_INIT_IMAGE_V2)) {
+ desc.cmd = QCOM_SCM_PIL_PAS_INIT_IMAGE_V2;
+ desc.arginfo = QCOM_SCM_ARGS(3, QCOM_SCM_VAL, QCOM_SCM_RW,
+ QCOM_SCM_VAL);
+ desc.args[2] = size;
+ }
+
ret = qcom_scm_call(dev, &desc, res);
qcom_scm_bw_disable();
@@ -644,7 +652,7 @@ static int qcom_scm_pas_prep_and_init_image(struct device *dev,
memcpy(mdata_buf, metadata, size);
mdata_phys = qcom_tzmem_to_phys(mdata_buf);
- ret = __qcom_scm_pas_init_image(dev, ctx->pas_id, mdata_phys, &res);
+ ret = __qcom_scm_pas_init_image(dev, ctx->pas_id, mdata_phys, &res, size);
if (ret < 0)
qcom_tzmem_free(mdata_buf);
else
@@ -683,7 +691,7 @@ static int __qcom_scm_pas_init_image2(struct device *dev, u32 pas_id,
memcpy(mdata_buf, metadata, size);
- ret = __qcom_scm_pas_init_image(dev, pas_id, mdata_phys, &res);
+ ret = __qcom_scm_pas_init_image(dev, pas_id, mdata_phys, &res, size);
if (ret < 0 || !ctx) {
dma_free_coherent(dev, size, mdata_buf, mdata_phys);
} else if (ctx) {
@@ -2528,7 +2536,7 @@ static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
gunyah_wdt_dev = platform_device_register_simple("gunyah-wdt", -1,
NULL, 0);
if (IS_ERR(gunyah_wdt_dev)) {
- dev_err(scm->dev, "Failed to register Gunyah watchdog device: %ld\n",
+ dev_err(scm->dev, "Failed to register Gunyah watchdog device: %pe\n",
PTR_ERR(gunyah_wdt_dev));
return;
}
diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
index cf90a565fdfb..e58c76aba45c 100644
--- a/drivers/firmware/qcom/qcom_scm.h
+++ b/drivers/firmware/qcom/qcom_scm.h
@@ -105,6 +105,7 @@ int qcom_scm_shm_bridge_enable(struct device *scm_dev);
#define QCOM_SCM_PIL_PAS_SHUTDOWN 0x06
#define QCOM_SCM_PIL_PAS_IS_SUPPORTED 0x07
#define QCOM_SCM_PIL_PAS_MSS_RESET 0x0a
+#define QCOM_SCM_PIL_PAS_INIT_IMAGE_V2 0x1a
#define QCOM_SCM_PIL_PAS_GET_RSCTABLE 0x21
#define QCOM_SCM_SVC_IO 0x05
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v14 1/9] firmware: qcom_scm: ipq5332: add support to pass metadata size
2026-08-03 10:46 ` [PATCH v14 1/9] firmware: qcom_scm: ipq5332: add support to pass metadata size Varadarajan Narayanan
@ 2026-08-03 10:55 ` sashiko-bot
2026-08-03 12:35 ` Mukesh Ojha
1 sibling, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-08-03 10:55 UTC (permalink / raw)
To: Varadarajan Narayanan; +Cc: devicetree, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The format specifier `%pe` is used with `PTR_ERR(gunyah_wdt_dev)`, causing a variadic type mismatch since `%pe` expects a pointer while `PTR_ERR()` returns a `long`.
--
--- Patch [1]: [PATCH v14 1/9] firmware: qcom_scm: ipq5332: add support to pass metadata size ---
commit fc14da7f3ebc47ea633f966df3048e0592f524ee
Author: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
firmware: qcom_scm: ipq5332: add support to pass metadata size
IPQ5332 security software running under trustzone requires metadata size.
With new command support added in TrustZone that includes a size parameter,
this passes the metadata size as well.
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 3eaa4c9ccf3cc..8902c445cf19f 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
[ ... ]
> @@ -2528,7 +2536,7 @@ static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
> gunyah_wdt_dev = platform_device_register_simple("gunyah-wdt", -1,
> NULL, 0);
> if (IS_ERR(gunyah_wdt_dev)) {
> - dev_err(scm->dev, "Failed to register Gunyah watchdog device: %ld\n",
> + dev_err(scm->dev, "Failed to register Gunyah watchdog device: %pe\n",
> PTR_ERR(gunyah_wdt_dev));
[Severity: Medium]
Will this type mismatch cause formatting or build issues?
The %pe format specifier strictly expects a pointer, but PTR_ERR() returns a
long. Passing a long to %pe violates C variadic argument typing rules and
triggers a compiler warning that could break the build in configurations
using -Werror. At runtime, accessing a long as a pointer from va_list can
lead to undefined behavior depending on the architecture's calling
convention.
Could PTR_ERR() be removed so that gunyah_wdt_dev is passed directly to %pe?
> return;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-rproc-v14-0-a1f87b0b0ad2@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v14 1/9] firmware: qcom_scm: ipq5332: add support to pass metadata size
2026-08-03 10:46 ` [PATCH v14 1/9] firmware: qcom_scm: ipq5332: add support to pass metadata size Varadarajan Narayanan
2026-08-03 10:55 ` sashiko-bot
@ 2026-08-03 12:35 ` Mukesh Ojha
2026-08-04 11:23 ` Varadarajan Narayanan
1 sibling, 1 reply; 20+ messages in thread
From: Mukesh Ojha @ 2026-08-03 12:35 UTC (permalink / raw)
To: Varadarajan Narayanan
Cc: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu,
linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
Konrad Dybcio, Gokul Sriram Palanisamy, Dmitry Baryshkov,
Vignesh Viswanathan
On Mon, Aug 03, 2026 at 04:16:07PM +0530, Varadarajan Narayanan wrote:
> From: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
>
> IPQ5332 security software running under trustzone requires metadata size.
> With new command support added in TrustZone that includes a size parameter,
> pass metadata size as well.
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
> Signed-off-by: Gokul Sriram Palanisamy <gokul.sriram.p@oss.qualcomm.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Tested-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
> Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
> ---
> drivers/firmware/qcom/qcom_scm.c | 16 ++++++++++++----
> drivers/firmware/qcom/qcom_scm.h | 1 +
> 2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 3eaa4c9ccf3c..8902c445cf19 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -598,7 +598,7 @@ EXPORT_SYMBOL_GPL(devm_qcom_scm_pas_context_alloc);
>
> static int __qcom_scm_pas_init_image(struct device *dev, u32 pas_id,
> dma_addr_t mdata_phys,
> - struct qcom_scm_res *res)
> + struct qcom_scm_res *res, size_t size)
> {
> struct qcom_scm_desc desc = {
> .svc = QCOM_SCM_SVC_PIL,
> @@ -619,6 +619,14 @@ static int __qcom_scm_pas_init_image(struct device *dev, u32 pas_id,
>
> desc.args[1] = mdata_phys;
>
> + if (__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_PIL,
> + QCOM_SCM_PIL_PAS_INIT_IMAGE_V2)) {
> + desc.cmd = QCOM_SCM_PIL_PAS_INIT_IMAGE_V2;
> + desc.arginfo = QCOM_SCM_ARGS(3, QCOM_SCM_VAL, QCOM_SCM_RW,
> + QCOM_SCM_VAL);
> + desc.args[2] = size;
> + }
> +
> ret = qcom_scm_call(dev, &desc, res);
> qcom_scm_bw_disable();
>
> @@ -644,7 +652,7 @@ static int qcom_scm_pas_prep_and_init_image(struct device *dev,
> memcpy(mdata_buf, metadata, size);
> mdata_phys = qcom_tzmem_to_phys(mdata_buf);
>
> - ret = __qcom_scm_pas_init_image(dev, ctx->pas_id, mdata_phys, &res);
> + ret = __qcom_scm_pas_init_image(dev, ctx->pas_id, mdata_phys, &res, size);
> if (ret < 0)
> qcom_tzmem_free(mdata_buf);
> else
> @@ -683,7 +691,7 @@ static int __qcom_scm_pas_init_image2(struct device *dev, u32 pas_id,
>
> memcpy(mdata_buf, metadata, size);
>
> - ret = __qcom_scm_pas_init_image(dev, pas_id, mdata_phys, &res);
> + ret = __qcom_scm_pas_init_image(dev, pas_id, mdata_phys, &res, size);
> if (ret < 0 || !ctx) {
> dma_free_coherent(dev, size, mdata_buf, mdata_phys);
> } else if (ctx) {
> @@ -2528,7 +2536,7 @@ static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
> gunyah_wdt_dev = platform_device_register_simple("gunyah-wdt", -1,
> NULL, 0);
> if (IS_ERR(gunyah_wdt_dev)) {
> - dev_err(scm->dev, "Failed to register Gunyah watchdog device: %ld\n",
> + dev_err(scm->dev, "Failed to register Gunyah watchdog device: %pe\n",
stray change..
> PTR_ERR(gunyah_wdt_dev));
> return;
> }
> diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
> index cf90a565fdfb..e58c76aba45c 100644
> --- a/drivers/firmware/qcom/qcom_scm.h
> +++ b/drivers/firmware/qcom/qcom_scm.h
> @@ -105,6 +105,7 @@ int qcom_scm_shm_bridge_enable(struct device *scm_dev);
> #define QCOM_SCM_PIL_PAS_SHUTDOWN 0x06
> #define QCOM_SCM_PIL_PAS_IS_SUPPORTED 0x07
> #define QCOM_SCM_PIL_PAS_MSS_RESET 0x0a
> +#define QCOM_SCM_PIL_PAS_INIT_IMAGE_V2 0x1a
> #define QCOM_SCM_PIL_PAS_GET_RSCTABLE 0x21
>
> #define QCOM_SCM_SVC_IO 0x05
>
> --
> 2.34.1
>
Otherwise, looks fine.
Reviewed-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
--
-Mukesh Ojha
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v14 1/9] firmware: qcom_scm: ipq5332: add support to pass metadata size
2026-08-03 12:35 ` Mukesh Ojha
@ 2026-08-04 11:23 ` Varadarajan Narayanan
2026-08-04 13:17 ` Mukesh Ojha
2026-08-04 17:38 ` Jeff Johnson
0 siblings, 2 replies; 20+ messages in thread
From: Varadarajan Narayanan @ 2026-08-04 11:23 UTC (permalink / raw)
To: Mukesh Ojha
Cc: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu,
linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
Konrad Dybcio, Gokul Sriram Palanisamy, Dmitry Baryshkov,
Vignesh Viswanathan
Mukesh,
[ . . . ]
> > if (IS_ERR(gunyah_wdt_dev)) {
> > - dev_err(scm->dev, "Failed to register Gunyah watchdog device: %ld\n",
> > + dev_err(scm->dev, "Failed to register Gunyah watchdog device: %pe\n",
>
> stray change..
Based on AI feedhack changed %ld to %pe to print PTR_ERR.
Thanks
Varada
> > PTR_ERR(gunyah_wdt_dev));
> > return;
> > }
> > diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
> > index cf90a565fdfb..e58c76aba45c 100644
> > --- a/drivers/firmware/qcom/qcom_scm.h
> > +++ b/drivers/firmware/qcom/qcom_scm.h
> > @@ -105,6 +105,7 @@ int qcom_scm_shm_bridge_enable(struct device *scm_dev);
> > #define QCOM_SCM_PIL_PAS_SHUTDOWN 0x06
> > #define QCOM_SCM_PIL_PAS_IS_SUPPORTED 0x07
> > #define QCOM_SCM_PIL_PAS_MSS_RESET 0x0a
> > +#define QCOM_SCM_PIL_PAS_INIT_IMAGE_V2 0x1a
> > #define QCOM_SCM_PIL_PAS_GET_RSCTABLE 0x21
> >
> > #define QCOM_SCM_SVC_IO 0x05
> >
> > --
> > 2.34.1
> >
>
> Otherwise, looks fine.
>
> Reviewed-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
>
> --
> -Mukesh Ojha
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v14 1/9] firmware: qcom_scm: ipq5332: add support to pass metadata size
2026-08-04 11:23 ` Varadarajan Narayanan
@ 2026-08-04 13:17 ` Mukesh Ojha
2026-08-04 17:38 ` Jeff Johnson
1 sibling, 0 replies; 20+ messages in thread
From: Mukesh Ojha @ 2026-08-04 13:17 UTC (permalink / raw)
To: Varadarajan Narayanan
Cc: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu,
linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
Konrad Dybcio, Gokul Sriram Palanisamy, Dmitry Baryshkov,
Vignesh Viswanathan
On Tue, Aug 04, 2026 at 04:53:48PM +0530, Varadarajan Narayanan wrote:
> Mukesh,
>
> [ . . . ]
>
> > > if (IS_ERR(gunyah_wdt_dev)) {
> > > - dev_err(scm->dev, "Failed to register Gunyah watchdog device: %ld\n",
> > > + dev_err(scm->dev, "Failed to register Gunyah watchdog device: %pe\n",
> >
> > stray change..
>
> Based on AI feedhack changed %ld to %pe to print PTR_ERR.
Should be separate change.
-Mukesh
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v14 1/9] firmware: qcom_scm: ipq5332: add support to pass metadata size
2026-08-04 11:23 ` Varadarajan Narayanan
2026-08-04 13:17 ` Mukesh Ojha
@ 2026-08-04 17:38 ` Jeff Johnson
1 sibling, 0 replies; 20+ messages in thread
From: Jeff Johnson @ 2026-08-04 17:38 UTC (permalink / raw)
To: Varadarajan Narayanan, Mukesh Ojha
Cc: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu,
linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
Konrad Dybcio, Gokul Sriram Palanisamy, Dmitry Baryshkov,
Vignesh Viswanathan
On 8/4/2026 4:23 AM, Varadarajan Narayanan wrote:
> Mukesh,
>
> [ . . . ]
>
>>> if (IS_ERR(gunyah_wdt_dev)) {
>>> - dev_err(scm->dev, "Failed to register Gunyah watchdog device: %ld\n",
>>> + dev_err(scm->dev, "Failed to register Gunyah watchdog device: %pe\n",
>>
>> stray change..
>
> Based on AI feedhack changed %ld to %pe to print PTR_ERR.
That was bad feedback. You just want the number, not the human readable string
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v14 2/9] dt-bindings: remoteproc: qcom: document hexagon based WCSS secure PIL
2026-08-03 10:46 [PATCH v14 0/9] Add new driver for WCSS secure PIL loading Varadarajan Narayanan
2026-08-03 10:46 ` [PATCH v14 1/9] firmware: qcom_scm: ipq5332: add support to pass metadata size Varadarajan Narayanan
@ 2026-08-03 10:46 ` Varadarajan Narayanan
2026-08-03 11:01 ` sashiko-bot
2026-08-03 10:46 ` [PATCH v14 3/9] remoteproc: qcom: add hexagon based WCSS secure PIL driver Varadarajan Narayanan
` (6 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Varadarajan Narayanan @ 2026-08-03 10:46 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu
Cc: linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
Gokul Sriram Palanisamy, George Moussalem, Krzysztof Kozlowski,
Varadarajan Narayanan
From: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
Add new binding document for hexagon based WCSS secure PIL remoteproc.
IPQ5018, IPQ5332 and IPQ9574 follow secure PIL remoteproc.
Signed-off-by: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
Signed-off-by: Gokul Sriram Palanisamy <gokul.sriram.p@oss.qualcomm.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
[ Dropped ipq5424 support ]
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
.../remoteproc/qcom,ipq5018-wcss-sec-pil.yaml | 178 +++++++++++++++++++++
1 file changed, 178 insertions(+)
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,ipq5018-wcss-sec-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,ipq5018-wcss-sec-pil.yaml
new file mode 100644
index 000000000000..1ec8df60d963
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,ipq5018-wcss-sec-pil.yaml
@@ -0,0 +1,178 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/qcom,ipq5018-wcss-sec-pil.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm WCSS Secure Peripheral Image Loader
+
+maintainers:
+ - Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
+
+description:
+ Wireless Connectivity Subsystem (WCSS) Secure Peripheral Image Loader loads
+ firmware and power up QDSP6 remoteproc on the Qualcomm IPQ series SoC.
+
+properties:
+ compatible:
+ enum:
+ - qcom,ipq5018-wcss-sec-pil
+ - qcom,ipq5332-wcss-sec-pil
+ - qcom,ipq9574-wcss-sec-pil
+
+ reg:
+ maxItems: 1
+
+ firmware-name:
+ maxItems: 1
+ description: Firmware name for the Hexagon core
+
+ interrupts:
+ items:
+ - description: Watchdog interrupt
+ - description: Fatal interrupt
+ - description: Ready interrupt
+ - description: Handover interrupt
+ - description: Stop acknowledge interrupt
+
+ interrupt-names:
+ items:
+ - const: wdog
+ - const: fatal
+ - const: ready
+ - const: handover
+ - const: stop-ack
+
+ clocks:
+ minItems: 1
+ items:
+ - description: sleep clock
+ - description: AHB interconnect clock
+
+ clock-names:
+ minItems: 1
+ items:
+ - const: sleep
+ - const: interconnect
+
+ mboxes:
+ items:
+ - description: TMECom mailbox
+
+ qcom,smem-states:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: States used by the AP to signal the remote processor
+ items:
+ - minItems: 2
+ - maxItems: 2
+
+ qcom,smem-state-names:
+ description:
+ Names of the states used by the AP to signal the remote processor
+ items:
+ - const: stop
+ - const: shutdown
+
+ memory-region:
+ items:
+ - description: Q6 reserved region
+
+ glink-edge:
+ $ref: /schemas/remoteproc/qcom,glink-edge.yaml#
+ description:
+ Qualcomm G-Link subnode which represents communication edge, channels
+ and devices related to the Modem.
+ unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - firmware-name
+ - interrupts
+ - interrupt-names
+ - qcom,smem-states
+ - qcom,smem-state-names
+ - memory-region
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,ipq5018-wcss-sec-pil
+ then:
+ properties:
+ clocks:
+ items:
+ - description: sleep clock
+ - description: AHB interconnect clock
+ clock-names:
+ items:
+ - const: sleep
+ - const: interconnect
+ required:
+ - clocks
+ - clock-names
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,ipq5332-wcss-sec-pil
+ then:
+ properties:
+ clocks:
+ items:
+ - description: sleep clock
+ clock-names:
+ items:
+ - const: sleep
+ required:
+ - clocks
+ - clock-names
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,ipq9574-wcss-sec-pil
+ then:
+ properties:
+ clocks: false
+ clock-names: false
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ q6v5_wcss: remoteproc@cd00000 {
+ compatible = "qcom,ipq9574-wcss-sec-pil";
+ reg = <0x0cd00000 0x10000>;
+ firmware-name = "ath11k/IPQ9574/hw1.0/q6_fw.mbn";
+ interrupts-extended = <&intc GIC_SPI 325 IRQ_TYPE_EDGE_RISING>,
+ <&smp2p_wcss_in 0 IRQ_TYPE_NONE>,
+ <&smp2p_wcss_in 1 IRQ_TYPE_NONE>,
+ <&smp2p_wcss_in 2 IRQ_TYPE_NONE>,
+ <&smp2p_wcss_in 3 IRQ_TYPE_NONE>;
+ interrupt-names = "wdog",
+ "fatal",
+ "ready",
+ "handover",
+ "stop-ack";
+
+ qcom,smem-states = <&smp2p_wcss_out 1>,
+ <&smp2p_wcss_out 0>;
+ qcom,smem-state-names = "stop",
+ "shutdown";
+ memory-region = <&q6_region>;
+
+ glink-edge {
+ interrupts = <GIC_SPI 321 IRQ_TYPE_EDGE_RISING>;
+ label = "rtr";
+ qcom,remote-pid = <1>;
+ mboxes = <&apcs_glb 8>;
+ };
+ };
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v14 2/9] dt-bindings: remoteproc: qcom: document hexagon based WCSS secure PIL
2026-08-03 10:46 ` [PATCH v14 2/9] dt-bindings: remoteproc: qcom: document hexagon based WCSS secure PIL Varadarajan Narayanan
@ 2026-08-03 11:01 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-08-03 11:01 UTC (permalink / raw)
To: Varadarajan Narayanan; +Cc: conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incorrect placement of `minItems` and `maxItems` inside the `items` list for `qcom,smem-states`.
- [Low] Copy-paste error in the description for `glink-edge` referencing the 'Modem'.
--
--- Patch [2]: [PATCH v14 2/9] dt-bindings: remoteproc: qcom: document hexagon based WCSS secure PIL ---
commit 8f051c37adcc2888cdf46122f432edbd580d36be
Author: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
dt-bindings: remoteproc: qcom: document hexagon based WCSS secure PIL
Add new binding document for hexagon based WCSS secure PIL remoteproc.
IPQ5018, IPQ5332 and IPQ9574 follow secure PIL remoteproc.
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,ipq5018-wcss-sec-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,ipq5018-wcss-sec-pil.yaml
> new file mode 100644
> index 0000000000000..1ec8df60d9633
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,ipq5018-wcss-sec-pil.yaml
[ ... ]
> + mboxes:
> + items:
> + - description: TMECom mailbox
> +
> + qcom,smem-states:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: States used by the AP to signal the remote processor
> + items:
> + - minItems: 2
> + - maxItems: 2
[Severity: Medium]
Is it intentional to place minItems and maxItems as elements inside the items
array?
By placing them inside the items array, the dt-schema validator treats them as
schemas for the individual elements of the phandle array (i.e. the first tuple
is validated against {minItems: 2} and the second against {maxItems: 2}).
If the intent is to restrict the length of the qcom,smem-states array itself
to 2 items, should these be sibling properties to items rather than inside it?
> +
> + qcom,smem-state-names:
> + description:
> + Names of the states used by the AP to signal the remote processor
[ ... ]
> + memory-region:
> + items:
> + - description: Q6 reserved region
> +
> + glink-edge:
> + $ref: /schemas/remoteproc/qcom,glink-edge.yaml#
> + description:
> + Qualcomm G-Link subnode which represents communication edge, channels
> + and devices related to the Modem.
[Severity: Low]
This isn't a bug, but should the description reference "WCSS" instead of
"Modem"?
Since this binding is for the Wireless Connectivity Subsystem, mentioning the
Modem appears to be a leftover from a copy-paste of a modem remoteproc binding.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-rproc-v14-0-a1f87b0b0ad2@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v14 3/9] remoteproc: qcom: add hexagon based WCSS secure PIL driver
2026-08-03 10:46 [PATCH v14 0/9] Add new driver for WCSS secure PIL loading Varadarajan Narayanan
2026-08-03 10:46 ` [PATCH v14 1/9] firmware: qcom_scm: ipq5332: add support to pass metadata size Varadarajan Narayanan
2026-08-03 10:46 ` [PATCH v14 2/9] dt-bindings: remoteproc: qcom: document hexagon based WCSS secure PIL Varadarajan Narayanan
@ 2026-08-03 10:46 ` Varadarajan Narayanan
2026-08-03 11:06 ` sashiko-bot
2026-08-03 12:44 ` Mukesh Ojha
2026-08-03 10:46 ` [PATCH v14 4/9] arm64: dts: qcom: ipq5018: add nodes to bring up q6 Varadarajan Narayanan
` (5 subsequent siblings)
8 siblings, 2 replies; 20+ messages in thread
From: Varadarajan Narayanan @ 2026-08-03 10:46 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu
Cc: linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
Vignesh Viswanathan, Gokul Sriram Palanisamy, George Moussalem,
Dmitry Baryshkov, Varadarajan Narayanan
From: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
Add support to bring up hexagon based WCSS using secure PIL. All IPQxxxx
SoCs support secure Peripheral Image Loading (PIL).
Secure PIL image is signed firmware image which only trusted software such
as TrustZone (TZ) can authenticate and load. Linux kernel will send a
Peripheral Authentication Service (PAS) request to TZ to authenticate and
load the PIL images.
In order to avoid overloading the existing WCSS driver or PAS driver, we
came up with this new PAS based IPQ WCSS driver.
Signed-off-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
Signed-off-by: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
Signed-off-by: Gokul Sriram Palanisamy <gokul.sriram.p@oss.qualcomm.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
[ Dropped ipq5424 support ]
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Tested-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
drivers/remoteproc/Kconfig | 20 ++
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/qcom_q6v5_wcss_sec.c | 337 ++++++++++++++++++++++++++++++++
include/linux/remoteproc.h | 2 +
4 files changed, 360 insertions(+)
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 5b56b2dcc725..3feea5bda129 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -267,6 +267,26 @@ config QCOM_Q6V5_WCSS
Hexagon V5 based WCSS remote processors on e.g. IPQ8074. This is
a non-TrustZone wireless subsystem.
+config QCOM_Q6V5_WCSS_SEC
+ tristate "Qualcomm Hexagon based WCSS Secure Peripheral Image Loader"
+ depends on OF && ARCH_QCOM
+ depends on QCOM_SMEM
+ depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
+ depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
+ depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n
+ select QCOM_MDT_LOADER
+ select QCOM_PIL_INFO
+ select QCOM_Q6V5_COMMON
+ select QCOM_RPROC_COMMON
+ select QCOM_SCM
+ help
+ Say y here to support the Qualcomm Secure Peripheral Image Loader
+ for the Hexagon based remote processors on e.g. IPQ5332.
+
+ This is TrustZone wireless subsystem. The firmware is
+ verified and booted with the help of the Peripheral Authentication
+ System (PAS) in TrustZone.
+
config QCOM_SYSMON
tristate "Qualcomm sysmon driver"
depends on RPMSG
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 689686de0d41..f74519ea6502 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_QCOM_Q6V5_ADSP) += qcom_q6v5_adsp.o
obj-$(CONFIG_QCOM_Q6V5_MSS) += qcom_q6v5_mss.o
obj-$(CONFIG_QCOM_Q6V5_PAS) += qcom_q6v5_pas.o
obj-$(CONFIG_QCOM_Q6V5_WCSS) += qcom_q6v5_wcss.o
+obj-$(CONFIG_QCOM_Q6V5_WCSS_SEC) += qcom_q6v5_wcss_sec.o
obj-$(CONFIG_QCOM_SYSMON) += qcom_sysmon.o
obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o
qcom_wcnss_pil-y += qcom_wcnss.o
diff --git a/drivers/remoteproc/qcom_q6v5_wcss_sec.c b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
new file mode 100644
index 000000000000..4837825d1717
--- /dev/null
+++ b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
@@ -0,0 +1,337 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+#include <linux/clk.h>
+#include <linux/firmware/qcom/qcom_pas.h>
+#include <linux/io.h>
+#include <linux/mailbox_client.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/soc/qcom/mdt_loader.h>
+
+#include "qcom_common.h"
+#include "qcom_q6v5.h"
+#include "qcom_pil_info.h"
+
+#define WCSS_CRASH_REASON 421
+
+#define WCSS_PAS_ID 0x6
+#define MPD_WCSS_PAS_ID 0xd
+
+#define Q6_WAIT_TIMEOUT (5 * HZ)
+
+struct wcss_sec {
+ struct device *dev;
+ struct qcom_rproc_glink glink_subdev;
+ struct qcom_rproc_ssr ssr_subdev;
+ struct qcom_q6v5 q6;
+ phys_addr_t mem_phys;
+ phys_addr_t mem_reloc;
+ void *mem_region;
+ size_t mem_size;
+ const struct wcss_data *desc;
+};
+
+struct wcss_data {
+ u32 pasid;
+ const char *ss_name;
+ bool auto_boot;
+};
+
+static int wcss_sec_start(struct rproc *rproc)
+{
+ struct wcss_sec *wcss = rproc->priv;
+ struct device *dev = wcss->dev;
+ int ret;
+
+ ret = qcom_q6v5_prepare(&wcss->q6);
+ if (ret)
+ return ret;
+
+ ret = qcom_pas_auth_and_reset(wcss->desc->pasid);
+ if (ret) {
+ dev_err(dev, "wcss_reset failed\n");
+ goto unprepare;
+ }
+
+ ret = qcom_q6v5_wait_for_start(&wcss->q6, msecs_to_jiffies(Q6_WAIT_TIMEOUT));
+
+ if (ret == -ETIMEDOUT) {
+ dev_err(dev, "start timed out\n");
+ qcom_pas_shutdown(wcss->desc->pasid);
+ goto unprepare;
+ }
+
+ return 0;
+
+unprepare:
+ qcom_q6v5_unprepare(&wcss->q6);
+
+ return ret;
+}
+
+static int wcss_sec_stop(struct rproc *rproc)
+{
+ struct wcss_sec *wcss = rproc->priv;
+ struct device *dev = wcss->dev;
+ int ret;
+
+ ret = qcom_q6v5_request_stop(&wcss->q6, NULL);
+ if (ret == -ETIMEDOUT)
+ dev_err(dev, "timed out on wait\n");
+
+ ret = qcom_pas_shutdown(wcss->desc->pasid);
+ if (ret)
+ dev_err(dev, "Failed to shutdown %d\n", ret);
+
+ qcom_q6v5_unprepare(&wcss->q6);
+
+ return ret;
+}
+
+static void *wcss_sec_da_to_va(struct rproc *rproc, u64 da, size_t len,
+ bool *is_iomem)
+{
+ struct wcss_sec *wcss = rproc->priv;
+ int offset;
+
+ offset = da - wcss->mem_reloc;
+ if (offset < 0 || offset + len > wcss->mem_size)
+ return NULL;
+
+ if (is_iomem)
+ *is_iomem = true;
+
+ return wcss->mem_region + offset;
+}
+
+static int wcss_sec_load(struct rproc *rproc, const struct firmware *fw)
+{
+ struct wcss_sec *wcss = rproc->priv;
+ struct device *dev = wcss->dev;
+ int ret;
+
+ ret = qcom_mdt_load(dev, fw, rproc->firmware, wcss->desc->pasid, wcss->mem_region,
+ wcss->mem_phys, wcss->mem_size, &wcss->mem_reloc);
+ if (ret)
+ return ret;
+
+ qcom_pil_info_store("wcss", wcss->mem_phys, wcss->mem_size);
+
+ return 0;
+}
+
+static unsigned long wcss_sec_panic(struct rproc *rproc)
+{
+ struct wcss_sec *wcss = rproc->priv;
+
+ return qcom_q6v5_panic(&wcss->q6);
+}
+
+static void wcss_sec_copy_segment(struct rproc *rproc,
+ struct rproc_dump_segment *segment,
+ void *dest, size_t offset, size_t size)
+{
+ struct wcss_sec *wcss = rproc->priv;
+ struct device *dev = wcss->dev;
+
+ if (!segment->io_ptr)
+ segment->io_ptr = ioremap_wc(segment->da, segment->size);
+
+ if (!segment->io_ptr) {
+ dev_err(dev, "Failed to ioremap segment %pad size 0x%zx\n",
+ &segment->da, segment->size);
+ return;
+ }
+
+ if (offset + size <= segment->size) {
+ memcpy_fromio(dest, segment->io_ptr + offset, size);
+ } else {
+ iounmap(segment->io_ptr);
+ segment->io_ptr = NULL;
+ }
+}
+
+static int wcss_sec_dump_segments(struct rproc *rproc,
+ const struct firmware *fw)
+{
+ struct device *dev = rproc->dev.parent;
+ struct reserved_mem *rmem = NULL;
+ struct device_node *node;
+ int num_segs, index;
+ int ret;
+
+ /*
+ * Parse through additional reserved memory regions for the rproc
+ * and add them to the coredump segments
+ */
+ num_segs = of_count_phandle_with_args(dev->of_node,
+ "memory-region", NULL);
+ for (index = 0; index < num_segs; index++) {
+ node = of_parse_phandle(dev->of_node,
+ "memory-region", index);
+ if (!node)
+ return -EINVAL;
+
+ rmem = of_reserved_mem_lookup(node);
+ of_node_put(node);
+ if (!rmem) {
+ dev_err(dev, "unable to acquire memory-region index %d num_segs %d\n",
+ index, num_segs);
+ return -EINVAL;
+ }
+
+ dev_dbg(dev, "Adding segment 0x%pa size 0x%pa",
+ &rmem->base, &rmem->size);
+ ret = rproc_coredump_add_custom_segment(rproc,
+ rmem->base,
+ rmem->size,
+ wcss_sec_copy_segment,
+ NULL);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct rproc_ops wcss_sec_ops = {
+ .start = wcss_sec_start,
+ .stop = wcss_sec_stop,
+ .da_to_va = wcss_sec_da_to_va,
+ .load = wcss_sec_load,
+ .get_boot_addr = rproc_elf_get_boot_addr,
+ .panic = wcss_sec_panic,
+ .parse_fw = wcss_sec_dump_segments,
+};
+
+static int wcss_sec_alloc_memory_region(struct wcss_sec *wcss)
+{
+ struct device *dev = wcss->dev;
+ struct resource res;
+ int ret;
+
+ ret = of_reserved_mem_region_to_resource(dev->of_node, 0, &res);
+ if (ret) {
+ dev_err(dev, "unable to acquire memory-region resource\n");
+ return ret;
+ }
+
+ wcss->mem_phys = res.start;
+ wcss->mem_reloc = res.start;
+ wcss->mem_size = resource_size(&res);
+ wcss->mem_region = devm_ioremap_resource_wc(dev, &res);
+ if (IS_ERR(wcss->mem_region)) {
+ dev_err(dev, "unable to map memory region: %pR\n", &res);
+ return PTR_ERR(wcss->mem_region);
+ }
+
+ return 0;
+}
+
+static int wcss_sec_probe(struct platform_device *pdev)
+{
+ const struct wcss_data *desc = of_device_get_match_data(&pdev->dev);
+ const char *fw_name = NULL;
+ struct wcss_sec *wcss;
+ struct clk *sleep_clk;
+ struct clk *int_clk;
+ struct rproc *rproc;
+ int ret;
+
+ ret = of_property_read_string(pdev->dev.of_node, "firmware-name",
+ &fw_name);
+ if (ret < 0)
+ return ret;
+
+ rproc = devm_rproc_alloc(&pdev->dev, desc->ss_name, &wcss_sec_ops,
+ fw_name, sizeof(*wcss));
+ if (!rproc) {
+ dev_err(&pdev->dev, "failed to allocate rproc\n");
+ return -ENOMEM;
+ }
+
+ wcss = rproc->priv;
+ wcss->dev = &pdev->dev;
+ wcss->desc = desc;
+
+ ret = wcss_sec_alloc_memory_region(wcss);
+ if (ret)
+ return ret;
+
+ sleep_clk = devm_clk_get_optional_enabled(&pdev->dev, "sleep");
+ if (IS_ERR(sleep_clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(sleep_clk),
+ "Failed to get sleep clock\n");
+
+ int_clk = devm_clk_get_optional_enabled(&pdev->dev, "interconnect");
+ if (IS_ERR(int_clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(int_clk),
+ "Failed to get interconnect clock\n");
+
+ ret = qcom_q6v5_init(&wcss->q6, pdev, rproc,
+ WCSS_CRASH_REASON, NULL, NULL);
+ if (ret)
+ return ret;
+
+ qcom_add_glink_subdev(rproc, &wcss->glink_subdev, desc->ss_name);
+ qcom_add_ssr_subdev(rproc, &wcss->ssr_subdev, desc->ss_name);
+
+ rproc->auto_boot = desc->auto_boot;
+ rproc->dump_conf = RPROC_COREDUMP_INLINE;
+ rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
+
+ ret = devm_rproc_add(&pdev->dev, rproc);
+ if (ret) {
+ qcom_remove_ssr_subdev(rproc, &wcss->ssr_subdev);
+ qcom_remove_glink_subdev(rproc, &wcss->glink_subdev);
+ qcom_q6v5_deinit(&wcss->q6);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, rproc);
+
+ return 0;
+}
+
+static void wcss_sec_remove(struct platform_device *pdev)
+{
+ struct rproc *rproc = platform_get_drvdata(pdev);
+ struct wcss_sec *wcss = rproc->priv;
+
+ qcom_remove_ssr_subdev(rproc, &wcss->ssr_subdev);
+ qcom_remove_glink_subdev(rproc, &wcss->glink_subdev);
+ qcom_q6v5_deinit(&wcss->q6);
+}
+
+static const struct wcss_data wcss_sec_ipq5332_res_init = {
+ .pasid = MPD_WCSS_PAS_ID,
+ .ss_name = "q6wcss",
+};
+
+static const struct wcss_data wcss_sec_ipq9574_res_init = {
+ .pasid = WCSS_PAS_ID,
+ .ss_name = "q6wcss",
+};
+
+static const struct of_device_id wcss_sec_of_match[] = {
+ { .compatible = "qcom,ipq5018-wcss-sec-pil", .data = &wcss_sec_ipq5332_res_init },
+ { .compatible = "qcom,ipq5332-wcss-sec-pil", .data = &wcss_sec_ipq5332_res_init },
+ { .compatible = "qcom,ipq9574-wcss-sec-pil", .data = &wcss_sec_ipq9574_res_init },
+ { },
+};
+MODULE_DEVICE_TABLE(of, wcss_sec_of_match);
+
+static struct platform_driver wcss_sec_driver = {
+ .probe = wcss_sec_probe,
+ .remove = wcss_sec_remove,
+ .driver = {
+ .name = "qcom-wcss-secure-pil",
+ .of_match_table = wcss_sec_of_match,
+ },
+};
+module_platform_driver(wcss_sec_driver);
+
+MODULE_DESCRIPTION("Hexagon WCSS Secure Peripheral Image Loader");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index ef711a5b1a7f..cf06b9846309 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -204,6 +204,7 @@ enum rproc_dump_mechanism {
* @node: list node related to the rproc segment list
* @da: device address of the segment
* @size: size of the segment
+ * @io_ptr: ptr to store the ioremapped dump segment
* @priv: private data associated with the dump_segment
* @dump: custom dump function to fill device memory segment associated
* with coredump
@@ -215,6 +216,7 @@ struct rproc_dump_segment {
dma_addr_t da;
size_t size;
+ void __iomem *io_ptr;
void *priv;
void (*dump)(struct rproc *rproc, struct rproc_dump_segment *segment,
void *dest, size_t offset, size_t size);
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v14 3/9] remoteproc: qcom: add hexagon based WCSS secure PIL driver
2026-08-03 10:46 ` [PATCH v14 3/9] remoteproc: qcom: add hexagon based WCSS secure PIL driver Varadarajan Narayanan
@ 2026-08-03 11:06 ` sashiko-bot
2026-08-03 12:44 ` Mukesh Ojha
1 sibling, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-08-03 11:06 UTC (permalink / raw)
To: Varadarajan Narayanan; +Cc: conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] Teardown asymmetry between `devm_rproc_add` and manual `remove` cleanup causes subdevices to skip shutdown routines and causes lockless list corruption.
- [High] `wcss_sec_copy_segment` silently returns on `ioremap_wc` failure, corrupting the devcoredump with uninitialized memory or zeroes.
- [Medium] Permanent `ioremap_wc` mapping leak during coredump segment reading due to unreachable unmap condition.
- [Medium] Error paths in `wcss_sec_dump_segments` permanently leak previously added custom coredump segments.
- [Medium] Missing NULL check on `of_device_get_match_data` during driver probe.
Pre-existing issues:
- [High] Unconditionally calling `qcom_remove_ssr_subdev()` on uninitialized subdev causes a kernel panic if `qcom_add_ssr_subdev()` failed silently.
--
--- Patch [3]: [PATCH v14 3/9] remoteproc: qcom: add hexagon based WCSS secure PIL driver ---
commit f96fb4a46b22ed73d132da71366a2ebbfe51729c
Author: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
Subject: remoteproc: qcom: add hexagon based WCSS secure PIL driver
Adds support for a Hexagon-based WCSS secure PIL driver for IPQxxxx SoCs
using TrustZone (TZ) for authentication and loading.
> diff --git a/drivers/remoteproc/qcom_q6v5_wcss_sec.c b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
[ ... ]
> +static void wcss_sec_copy_segment(struct rproc *rproc,
> + struct rproc_dump_segment *segment,
> + void *dest, size_t offset, size_t size)
> +{
> + struct wcss_sec *wcss = rproc->priv;
> + struct device *dev = wcss->dev;
> +
> + if (!segment->io_ptr)
> + segment->io_ptr = ioremap_wc(segment->da, segment->size);
> +
> + if (!segment->io_ptr) {
> + dev_err(dev, "Failed to ioremap segment %pad size 0x%zx\n",
> + &segment->da, segment->size);
> + return;
[Severity: High]
If ioremap_wc() fails, this returns early without populating the userspace
buffer. Because it returns void, the devcoredump core assumes the copy
succeeded.
Does this cause the framework to leak uninitialized kernel memory
(if not zeroed) or zeroes to userspace, silently corrupting the coredump
instead of gracefully failing?
> + }
> +
> + if (offset + size <= segment->size) {
> + memcpy_fromio(dest, segment->io_ptr + offset, size);
> + } else {
> + iounmap(segment->io_ptr);
> + segment->io_ptr = NULL;
> + }
[Severity: Medium]
Because the remoteproc framework bounds the read size such that
offset + size <= segment->size is always true, is this else branch
mathematically dead code?
During cleanup, rproc_coredump_cleanup() frees the segment structure but
lacks a hook to unmap the custom io_ptr. Could this permanently leak vmalloc
space on every crash dump?
> +}
[ ... ]
> +static int wcss_sec_dump_segments(struct rproc *rproc,
> + const struct firmware *fw)
> +{
> + struct device *dev = rproc->dev.parent;
> + struct reserved_mem *rmem = NULL;
> + struct device_node *node;
> + int num_segs, index;
> + int ret;
> +
> + /*
> + * Parse through additional reserved memory regions for the rproc
> + * and add them to the coredump segments
> + */
> + num_segs = of_count_phandle_with_args(dev->of_node,
> + "memory-region", NULL);
> + for (index = 0; index < num_segs; index++) {
> + node = of_parse_phandle(dev->of_node,
> + "memory-region", index);
> + if (!node)
> + return -EINVAL;
> +
> + rmem = of_reserved_mem_lookup(node);
> + of_node_put(node);
> + if (!rmem) {
> + dev_err(dev, "unable to acquire memory-region index %d num_segs %d\n",
> + index, num_segs);
> + return -EINVAL;
[Severity: Medium]
If an error occurs midway through the segment parsing loop, does returning
early here leave previously added segments on the rproc->dump_segments list?
The remoteproc core's rproc_fw_boot() handles a parse_fw() failure by jumping
directly to unprepare_rproc, bypassing rproc_resource_cleanup(). Will any
segments successfully added in earlier iterations be permanently leaked?
> + }
[ ... ]
> +static int wcss_sec_probe(struct platform_device *pdev)
> +{
> + const struct wcss_data *desc = of_device_get_match_data(&pdev->dev);
> + const char *fw_name = NULL;
> + struct wcss_sec *wcss;
> + struct clk *sleep_clk;
> + struct clk *int_clk;
> + struct rproc *rproc;
> + int ret;
> +
> + ret = of_property_read_string(pdev->dev.of_node, "firmware-name",
> + &fw_name);
> + if (ret < 0)
> + return ret;
> +
> + rproc = devm_rproc_alloc(&pdev->dev, desc->ss_name, &wcss_sec_ops,
> + fw_name, sizeof(*wcss));
[Severity: Medium]
Is a NULL check needed for the return value of of_device_get_match_data()?
If the driver is bound manually via sysfs to a platform device with a
compatible of_node that lacks match data in the driver's table, it could
return NULL and cause a NULL pointer dereference on desc->ss_name.
> + if (!rproc) {
[ ... ]
> + qcom_add_glink_subdev(rproc, &wcss->glink_subdev, desc->ss_name);
> + qcom_add_ssr_subdev(rproc, &wcss->ssr_subdev, desc->ss_name);
> +
> + rproc->auto_boot = desc->auto_boot;
> + rproc->dump_conf = RPROC_COREDUMP_INLINE;
> + rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
> +
> + ret = devm_rproc_add(&pdev->dev, rproc);
> + if (ret) {
> + qcom_remove_ssr_subdev(rproc, &wcss->ssr_subdev);
[Severity: High]
This is a pre-existing issue, but if qcom_add_ssr_subdev() fails (e.g., due
to -ENOMEM), it returns early and leaves ssr->subdev.node zero-initialized.
Later, unconditionally calling qcom_remove_ssr_subdev() here during probe
error handling (or in wcss_sec_remove() below) invokes list_del() on the
zero-initialized list head. Can this dereference a NULL prev pointer and
cause a kernel panic?
> + qcom_remove_glink_subdev(rproc, &wcss->glink_subdev);
> + qcom_q6v5_deinit(&wcss->q6);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, rproc);
> +
> + return 0;
> +}
> +
> +static void wcss_sec_remove(struct platform_device *pdev)
> +{
> + struct rproc *rproc = platform_get_drvdata(pdev);
> + struct wcss_sec *wcss = rproc->priv;
> +
> + qcom_remove_ssr_subdev(rproc, &wcss->ssr_subdev);
> + qcom_remove_glink_subdev(rproc, &wcss->glink_subdev);
[Severity: High]
Because rproc_del() is deferred to devres cleanup (after remove()),
does manually removing subdevices from rproc->subdevs here while the rproc
is still active skip their .stop callbacks?
Since they are no longer in the list when rproc_del() executes, it seems
this leaves GLINK edges active and dangling for a powered-off remoteproc,
potentially corrupting the GLINK state machine and freeing structures still
in use. Should the subdevices be left for rproc_del() to clean up?
> + qcom_q6v5_deinit(&wcss->q6);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-rproc-v14-0-a1f87b0b0ad2@oss.qualcomm.com?part=3
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v14 3/9] remoteproc: qcom: add hexagon based WCSS secure PIL driver
2026-08-03 10:46 ` [PATCH v14 3/9] remoteproc: qcom: add hexagon based WCSS secure PIL driver Varadarajan Narayanan
2026-08-03 11:06 ` sashiko-bot
@ 2026-08-03 12:44 ` Mukesh Ojha
2026-08-04 11:19 ` Vignesh Viswanathan
1 sibling, 1 reply; 20+ messages in thread
From: Mukesh Ojha @ 2026-08-03 12:44 UTC (permalink / raw)
To: Varadarajan Narayanan
Cc: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu,
linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
Vignesh Viswanathan, Gokul Sriram Palanisamy, George Moussalem,
Dmitry Baryshkov
On Mon, Aug 03, 2026 at 04:16:09PM +0530, Varadarajan Narayanan wrote:
> From: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
>
> Add support to bring up hexagon based WCSS using secure PIL. All IPQxxxx
> SoCs support secure Peripheral Image Loading (PIL).
>
> Secure PIL image is signed firmware image which only trusted software such
> as TrustZone (TZ) can authenticate and load. Linux kernel will send a
> Peripheral Authentication Service (PAS) request to TZ to authenticate and
> load the PIL images.
>
> In order to avoid overloading the existing WCSS driver or PAS driver, we
> came up with this new PAS based IPQ WCSS driver.
Can you tell why you need another driver if you are using the same PAS
method and it is using more of everything what PAS already
supports ?
>
> Signed-off-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
> Signed-off-by: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
> Signed-off-by: Gokul Sriram Palanisamy <gokul.sriram.p@oss.qualcomm.com>
> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> [ Dropped ipq5424 support ]
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Tested-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
Author and tested looks weird..here..
> Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
> ---
> drivers/remoteproc/Kconfig | 20 ++
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/qcom_q6v5_wcss_sec.c | 337 ++++++++++++++++++++++++++++++++
> include/linux/remoteproc.h | 2 +
> 4 files changed, 360 insertions(+)
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 5b56b2dcc725..3feea5bda129 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -267,6 +267,26 @@ config QCOM_Q6V5_WCSS
> Hexagon V5 based WCSS remote processors on e.g. IPQ8074. This is
> a non-TrustZone wireless subsystem.
>
> +config QCOM_Q6V5_WCSS_SEC
> + tristate "Qualcomm Hexagon based WCSS Secure Peripheral Image Loader"
> + depends on OF && ARCH_QCOM
> + depends on QCOM_SMEM
> + depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
> + depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
> + depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n
> + select QCOM_MDT_LOADER
> + select QCOM_PIL_INFO
> + select QCOM_Q6V5_COMMON
> + select QCOM_RPROC_COMMON
> + select QCOM_SCM
> + help
> + Say y here to support the Qualcomm Secure Peripheral Image Loader
> + for the Hexagon based remote processors on e.g. IPQ5332.
> +
> + This is TrustZone wireless subsystem. The firmware is
> + verified and booted with the help of the Peripheral Authentication
> + System (PAS) in TrustZone.
> +
> config QCOM_SYSMON
> tristate "Qualcomm sysmon driver"
> depends on RPMSG
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 689686de0d41..f74519ea6502 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_QCOM_Q6V5_ADSP) += qcom_q6v5_adsp.o
> obj-$(CONFIG_QCOM_Q6V5_MSS) += qcom_q6v5_mss.o
> obj-$(CONFIG_QCOM_Q6V5_PAS) += qcom_q6v5_pas.o
> obj-$(CONFIG_QCOM_Q6V5_WCSS) += qcom_q6v5_wcss.o
> +obj-$(CONFIG_QCOM_Q6V5_WCSS_SEC) += qcom_q6v5_wcss_sec.o
> obj-$(CONFIG_QCOM_SYSMON) += qcom_sysmon.o
> obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o
> qcom_wcnss_pil-y += qcom_wcnss.o
> diff --git a/drivers/remoteproc/qcom_q6v5_wcss_sec.c b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
> new file mode 100644
> index 000000000000..4837825d1717
> --- /dev/null
> +++ b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
> @@ -0,0 +1,337 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +#include <linux/clk.h>
> +#include <linux/firmware/qcom/qcom_pas.h>
> +#include <linux/io.h>
> +#include <linux/mailbox_client.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/platform_device.h>
> +#include <linux/soc/qcom/mdt_loader.h>
> +
> +#include "qcom_common.h"
> +#include "qcom_q6v5.h"
> +#include "qcom_pil_info.h"
> +
> +#define WCSS_CRASH_REASON 421
> +
> +#define WCSS_PAS_ID 0x6
> +#define MPD_WCSS_PAS_ID 0xd
> +
> +#define Q6_WAIT_TIMEOUT (5 * HZ)
> +
> +struct wcss_sec {
> + struct device *dev;
> + struct qcom_rproc_glink glink_subdev;
> + struct qcom_rproc_ssr ssr_subdev;
> + struct qcom_q6v5 q6;
> + phys_addr_t mem_phys;
> + phys_addr_t mem_reloc;
> + void *mem_region;
> + size_t mem_size;
> + const struct wcss_data *desc;
> +};
> +
> +struct wcss_data {
> + u32 pasid;
> + const char *ss_name;
> + bool auto_boot;
> +};
> +
> +static int wcss_sec_start(struct rproc *rproc)
> +{
> + struct wcss_sec *wcss = rproc->priv;
> + struct device *dev = wcss->dev;
> + int ret;
> +
> + ret = qcom_q6v5_prepare(&wcss->q6);
> + if (ret)
> + return ret;
> +
> + ret = qcom_pas_auth_and_reset(wcss->desc->pasid);
> + if (ret) {
> + dev_err(dev, "wcss_reset failed\n");
> + goto unprepare;
> + }
> +
> + ret = qcom_q6v5_wait_for_start(&wcss->q6, msecs_to_jiffies(Q6_WAIT_TIMEOUT));
> +
> + if (ret == -ETIMEDOUT) {
> + dev_err(dev, "start timed out\n");
> + qcom_pas_shutdown(wcss->desc->pasid);
> + goto unprepare;
> + }
> +
> + return 0;
> +
> +unprepare:
> + qcom_q6v5_unprepare(&wcss->q6);
> +
> + return ret;
> +}
> +
> +static int wcss_sec_stop(struct rproc *rproc)
> +{
> + struct wcss_sec *wcss = rproc->priv;
> + struct device *dev = wcss->dev;
> + int ret;
> +
> + ret = qcom_q6v5_request_stop(&wcss->q6, NULL);
> + if (ret == -ETIMEDOUT)
> + dev_err(dev, "timed out on wait\n");
> +
> + ret = qcom_pas_shutdown(wcss->desc->pasid);
> + if (ret)
> + dev_err(dev, "Failed to shutdown %d\n", ret);
> +
> + qcom_q6v5_unprepare(&wcss->q6);
> +
> + return ret;
> +}
> +
> +static void *wcss_sec_da_to_va(struct rproc *rproc, u64 da, size_t len,
> + bool *is_iomem)
> +{
> + struct wcss_sec *wcss = rproc->priv;
> + int offset;
> +
> + offset = da - wcss->mem_reloc;
> + if (offset < 0 || offset + len > wcss->mem_size)
> + return NULL;
> +
> + if (is_iomem)
> + *is_iomem = true;
> +
> + return wcss->mem_region + offset;
> +}
> +
> +static int wcss_sec_load(struct rproc *rproc, const struct firmware *fw)
> +{
> + struct wcss_sec *wcss = rproc->priv;
> + struct device *dev = wcss->dev;
> + int ret;
> +
> + ret = qcom_mdt_load(dev, fw, rproc->firmware, wcss->desc->pasid, wcss->mem_region,
> + wcss->mem_phys, wcss->mem_size, &wcss->mem_reloc);
> + if (ret)
> + return ret;
> +
> + qcom_pil_info_store("wcss", wcss->mem_phys, wcss->mem_size);
> +
> + return 0;
> +}
> +
> +static unsigned long wcss_sec_panic(struct rproc *rproc)
> +{
> + struct wcss_sec *wcss = rproc->priv;
> +
> + return qcom_q6v5_panic(&wcss->q6);
> +}
> +
> +static void wcss_sec_copy_segment(struct rproc *rproc,
> + struct rproc_dump_segment *segment,
> + void *dest, size_t offset, size_t size)
> +{
> + struct wcss_sec *wcss = rproc->priv;
> + struct device *dev = wcss->dev;
> +
> + if (!segment->io_ptr)
> + segment->io_ptr = ioremap_wc(segment->da, segment->size);
> +
> + if (!segment->io_ptr) {
> + dev_err(dev, "Failed to ioremap segment %pad size 0x%zx\n",
> + &segment->da, segment->size);
> + return;
> + }
> +
> + if (offset + size <= segment->size) {
> + memcpy_fromio(dest, segment->io_ptr + offset, size);
> + } else {
> + iounmap(segment->io_ptr);
> + segment->io_ptr = NULL;
> + }
> +}
> +
> +static int wcss_sec_dump_segments(struct rproc *rproc,
> + const struct firmware *fw)
> +{
> + struct device *dev = rproc->dev.parent;
> + struct reserved_mem *rmem = NULL;
> + struct device_node *node;
> + int num_segs, index;
> + int ret;
> +
> + /*
> + * Parse through additional reserved memory regions for the rproc
> + * and add them to the coredump segments
> + */
> + num_segs = of_count_phandle_with_args(dev->of_node,
> + "memory-region", NULL);
> + for (index = 0; index < num_segs; index++) {
> + node = of_parse_phandle(dev->of_node,
> + "memory-region", index);
> + if (!node)
> + return -EINVAL;
> +
> + rmem = of_reserved_mem_lookup(node);
> + of_node_put(node);
> + if (!rmem) {
> + dev_err(dev, "unable to acquire memory-region index %d num_segs %d\n",
> + index, num_segs);
> + return -EINVAL;
> + }
> +
> + dev_dbg(dev, "Adding segment 0x%pa size 0x%pa",
> + &rmem->base, &rmem->size);
> + ret = rproc_coredump_add_custom_segment(rproc,
> + rmem->base,
> + rmem->size,
> + wcss_sec_copy_segment,
> + NULL);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct rproc_ops wcss_sec_ops = {
> + .start = wcss_sec_start,
> + .stop = wcss_sec_stop,
> + .da_to_va = wcss_sec_da_to_va,
> + .load = wcss_sec_load,
> + .get_boot_addr = rproc_elf_get_boot_addr,
> + .panic = wcss_sec_panic,
> + .parse_fw = wcss_sec_dump_segments,
> +};
> +
> +static int wcss_sec_alloc_memory_region(struct wcss_sec *wcss)
> +{
> + struct device *dev = wcss->dev;
> + struct resource res;
> + int ret;
> +
> + ret = of_reserved_mem_region_to_resource(dev->of_node, 0, &res);
> + if (ret) {
> + dev_err(dev, "unable to acquire memory-region resource\n");
> + return ret;
> + }
> +
> + wcss->mem_phys = res.start;
> + wcss->mem_reloc = res.start;
> + wcss->mem_size = resource_size(&res);
> + wcss->mem_region = devm_ioremap_resource_wc(dev, &res);
> + if (IS_ERR(wcss->mem_region)) {
> + dev_err(dev, "unable to map memory region: %pR\n", &res);
> + return PTR_ERR(wcss->mem_region);
> + }
> +
> + return 0;
> +}
> +
> +static int wcss_sec_probe(struct platform_device *pdev)
> +{
> + const struct wcss_data *desc = of_device_get_match_data(&pdev->dev);
> + const char *fw_name = NULL;
> + struct wcss_sec *wcss;
> + struct clk *sleep_clk;
> + struct clk *int_clk;
> + struct rproc *rproc;
> + int ret;
> +
> + ret = of_property_read_string(pdev->dev.of_node, "firmware-name",
> + &fw_name);
> + if (ret < 0)
> + return ret;
> +
> + rproc = devm_rproc_alloc(&pdev->dev, desc->ss_name, &wcss_sec_ops,
> + fw_name, sizeof(*wcss));
> + if (!rproc) {
> + dev_err(&pdev->dev, "failed to allocate rproc\n");
> + return -ENOMEM;
> + }
> +
> + wcss = rproc->priv;
> + wcss->dev = &pdev->dev;
> + wcss->desc = desc;
> +
> + ret = wcss_sec_alloc_memory_region(wcss);
> + if (ret)
> + return ret;
> +
> + sleep_clk = devm_clk_get_optional_enabled(&pdev->dev, "sleep");
> + if (IS_ERR(sleep_clk))
> + return dev_err_probe(&pdev->dev, PTR_ERR(sleep_clk),
> + "Failed to get sleep clock\n");
> +
> + int_clk = devm_clk_get_optional_enabled(&pdev->dev, "interconnect");
> + if (IS_ERR(int_clk))
> + return dev_err_probe(&pdev->dev, PTR_ERR(int_clk),
> + "Failed to get interconnect clock\n");
> +
> + ret = qcom_q6v5_init(&wcss->q6, pdev, rproc,
> + WCSS_CRASH_REASON, NULL, NULL);
> + if (ret)
> + return ret;
> +
> + qcom_add_glink_subdev(rproc, &wcss->glink_subdev, desc->ss_name);
> + qcom_add_ssr_subdev(rproc, &wcss->ssr_subdev, desc->ss_name);
> +
> + rproc->auto_boot = desc->auto_boot;
> + rproc->dump_conf = RPROC_COREDUMP_INLINE;
> + rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
> +
> + ret = devm_rproc_add(&pdev->dev, rproc);
> + if (ret) {
> + qcom_remove_ssr_subdev(rproc, &wcss->ssr_subdev);
> + qcom_remove_glink_subdev(rproc, &wcss->glink_subdev);
> + qcom_q6v5_deinit(&wcss->q6);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, rproc);
> +
> + return 0;
> +}
> +
> +static void wcss_sec_remove(struct platform_device *pdev)
> +{
> + struct rproc *rproc = platform_get_drvdata(pdev);
> + struct wcss_sec *wcss = rproc->priv;
> +
> + qcom_remove_ssr_subdev(rproc, &wcss->ssr_subdev);
> + qcom_remove_glink_subdev(rproc, &wcss->glink_subdev);
> + qcom_q6v5_deinit(&wcss->q6);
> +}
> +
> +static const struct wcss_data wcss_sec_ipq5332_res_init = {
> + .pasid = MPD_WCSS_PAS_ID,
> + .ss_name = "q6wcss",
> +};
> +
> +static const struct wcss_data wcss_sec_ipq9574_res_init = {
> + .pasid = WCSS_PAS_ID,
> + .ss_name = "q6wcss",
> +};
> +
> +static const struct of_device_id wcss_sec_of_match[] = {
> + { .compatible = "qcom,ipq5018-wcss-sec-pil", .data = &wcss_sec_ipq5332_res_init },
> + { .compatible = "qcom,ipq5332-wcss-sec-pil", .data = &wcss_sec_ipq5332_res_init },
> + { .compatible = "qcom,ipq9574-wcss-sec-pil", .data = &wcss_sec_ipq9574_res_init },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, wcss_sec_of_match);
> +
> +static struct platform_driver wcss_sec_driver = {
> + .probe = wcss_sec_probe,
> + .remove = wcss_sec_remove,
> + .driver = {
> + .name = "qcom-wcss-secure-pil",
> + .of_match_table = wcss_sec_of_match,
> + },
> +};
> +module_platform_driver(wcss_sec_driver);
> +
> +MODULE_DESCRIPTION("Hexagon WCSS Secure Peripheral Image Loader");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index ef711a5b1a7f..cf06b9846309 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -204,6 +204,7 @@ enum rproc_dump_mechanism {
> * @node: list node related to the rproc segment list
> * @da: device address of the segment
> * @size: size of the segment
> + * @io_ptr: ptr to store the ioremapped dump segment
> * @priv: private data associated with the dump_segment
> * @dump: custom dump function to fill device memory segment associated
> * with coredump
> @@ -215,6 +216,7 @@ struct rproc_dump_segment {
> dma_addr_t da;
> size_t size;
>
> + void __iomem *io_ptr;
> void *priv;
> void (*dump)(struct rproc *rproc, struct rproc_dump_segment *segment,
> void *dest, size_t offset, size_t size);
>
> --
> 2.34.1
>
--
-Mukesh Ojha
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v14 3/9] remoteproc: qcom: add hexagon based WCSS secure PIL driver
2026-08-03 12:44 ` Mukesh Ojha
@ 2026-08-04 11:19 ` Vignesh Viswanathan
2026-08-04 13:10 ` Mukesh Ojha
0 siblings, 1 reply; 20+ messages in thread
From: Vignesh Viswanathan @ 2026-08-04 11:19 UTC (permalink / raw)
To: Mukesh Ojha, Varadarajan Narayanan
Cc: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu,
linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
Gokul Sriram Palanisamy, George Moussalem, Dmitry Baryshkov
On 8/3/2026 6:14 PM, Mukesh Ojha wrote:
> On Mon, Aug 03, 2026 at 04:16:09PM +0530, Varadarajan Narayanan wrote:
>> From: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
>>
>> Add support to bring up hexagon based WCSS using secure PIL. All IPQxxxx
>> SoCs support secure Peripheral Image Loading (PIL).
>>
>> Secure PIL image is signed firmware image which only trusted software such
>> as TrustZone (TZ) can authenticate and load. Linux kernel will send a
>> Peripheral Authentication Service (PAS) request to TZ to authenticate and
>> load the PIL images.
>>
>> In order to avoid overloading the existing WCSS driver or PAS driver, we
>> came up with this new PAS based IPQ WCSS driver.
>
> Can you tell why you need another driver if you are using the same PAS
> method and it is using more of everything what PAS already
> supports ?
>
Hi Mukesh,
We just use pas_auth_and_reset and pas_shutdown APIs and do not need
the entire framework provided by the PAS driver for IPQ WCSS.
Also, feedback from Bjorn in [1] was to have separate driver
instead of overloading existing WCSS driver.
[1] https://lore.kernel.org/all/YFK1VpP74rQH2Z72@builder.lan/
>>
>> Signed-off-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
>> Signed-off-by: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
>> Signed-off-by: Gokul Sriram Palanisamy <gokul.sriram.p@oss.qualcomm.com>
>> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
>> [ Dropped ipq5424 support ]
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>> Tested-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
>
>
> Author and tested looks weird..here..
Will drop Tested-by here.
Thanks,
Vignesh
>
>
>> Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
>> ---
>> drivers/remoteproc/Kconfig | 20 ++
>> drivers/remoteproc/Makefile | 1 +
>> drivers/remoteproc/qcom_q6v5_wcss_sec.c | 337 ++++++++++++++++++++++++++++++++
>> include/linux/remoteproc.h | 2 +
>> 4 files changed, 360 insertions(+)
>>
>> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
>> index 5b56b2dcc725..3feea5bda129 100644
>> --- a/drivers/remoteproc/Kconfig
>> +++ b/drivers/remoteproc/Kconfig
>> @@ -267,6 +267,26 @@ config QCOM_Q6V5_WCSS
>> Hexagon V5 based WCSS remote processors on e.g. IPQ8074. This is
>> a non-TrustZone wireless subsystem.
>>
>> +config QCOM_Q6V5_WCSS_SEC
>> + tristate "Qualcomm Hexagon based WCSS Secure Peripheral Image Loader"
>> + depends on OF && ARCH_QCOM
>> + depends on QCOM_SMEM
>> + depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
>> + depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
>> + depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n
>> + select QCOM_MDT_LOADER
>> + select QCOM_PIL_INFO
>> + select QCOM_Q6V5_COMMON
>> + select QCOM_RPROC_COMMON
>> + select QCOM_SCM
>> + help
>> + Say y here to support the Qualcomm Secure Peripheral Image Loader
>> + for the Hexagon based remote processors on e.g. IPQ5332.
>> +
>> + This is TrustZone wireless subsystem. The firmware is
>> + verified and booted with the help of the Peripheral Authentication
>> + System (PAS) in TrustZone.
>> +
>> config QCOM_SYSMON
>> tristate "Qualcomm sysmon driver"
>> depends on RPMSG
>> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
>> index 689686de0d41..f74519ea6502 100644
>> --- a/drivers/remoteproc/Makefile
>> +++ b/drivers/remoteproc/Makefile
>> @@ -29,6 +29,7 @@ obj-$(CONFIG_QCOM_Q6V5_ADSP) += qcom_q6v5_adsp.o
>> obj-$(CONFIG_QCOM_Q6V5_MSS) += qcom_q6v5_mss.o
>> obj-$(CONFIG_QCOM_Q6V5_PAS) += qcom_q6v5_pas.o
>> obj-$(CONFIG_QCOM_Q6V5_WCSS) += qcom_q6v5_wcss.o
>> +obj-$(CONFIG_QCOM_Q6V5_WCSS_SEC) += qcom_q6v5_wcss_sec.o
>> obj-$(CONFIG_QCOM_SYSMON) += qcom_sysmon.o
>> obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o
>> qcom_wcnss_pil-y += qcom_wcnss.o
>> diff --git a/drivers/remoteproc/qcom_q6v5_wcss_sec.c b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
>> new file mode 100644
>> index 000000000000..4837825d1717
>> --- /dev/null
>> +++ b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
>> @@ -0,0 +1,337 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +#include <linux/clk.h>
>> +#include <linux/firmware/qcom/qcom_pas.h>
>> +#include <linux/io.h>
>> +#include <linux/mailbox_client.h>
>> +#include <linux/of_reserved_mem.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/soc/qcom/mdt_loader.h>
>> +
>> +#include "qcom_common.h"
>> +#include "qcom_q6v5.h"
>> +#include "qcom_pil_info.h"
>> +
>> +#define WCSS_CRASH_REASON 421
>> +
>> +#define WCSS_PAS_ID 0x6
>> +#define MPD_WCSS_PAS_ID 0xd
>> +
>> +#define Q6_WAIT_TIMEOUT (5 * HZ)
>> +
>> +struct wcss_sec {
>> + struct device *dev;
>> + struct qcom_rproc_glink glink_subdev;
>> + struct qcom_rproc_ssr ssr_subdev;
>> + struct qcom_q6v5 q6;
>> + phys_addr_t mem_phys;
>> + phys_addr_t mem_reloc;
>> + void *mem_region;
>> + size_t mem_size;
>> + const struct wcss_data *desc;
>> +};
>> +
>> +struct wcss_data {
>> + u32 pasid;
>> + const char *ss_name;
>> + bool auto_boot;
>> +};
>> +
>> +static int wcss_sec_start(struct rproc *rproc)
>> +{
>> + struct wcss_sec *wcss = rproc->priv;
>> + struct device *dev = wcss->dev;
>> + int ret;
>> +
>> + ret = qcom_q6v5_prepare(&wcss->q6);
>> + if (ret)
>> + return ret;
>> +
>> + ret = qcom_pas_auth_and_reset(wcss->desc->pasid);
>> + if (ret) {
>> + dev_err(dev, "wcss_reset failed\n");
>> + goto unprepare;
>> + }
>> +
>> + ret = qcom_q6v5_wait_for_start(&wcss->q6, msecs_to_jiffies(Q6_WAIT_TIMEOUT));
>> +
>> + if (ret == -ETIMEDOUT) {
>> + dev_err(dev, "start timed out\n");
>> + qcom_pas_shutdown(wcss->desc->pasid);
>> + goto unprepare;
>> + }
>> +
>> + return 0;
>> +
>> +unprepare:
>> + qcom_q6v5_unprepare(&wcss->q6);
>> +
>> + return ret;
>> +}
>> +
>> +static int wcss_sec_stop(struct rproc *rproc)
>> +{
>> + struct wcss_sec *wcss = rproc->priv;
>> + struct device *dev = wcss->dev;
>> + int ret;
>> +
>> + ret = qcom_q6v5_request_stop(&wcss->q6, NULL);
>> + if (ret == -ETIMEDOUT)
>> + dev_err(dev, "timed out on wait\n");
>> +
>> + ret = qcom_pas_shutdown(wcss->desc->pasid);
>> + if (ret)
>> + dev_err(dev, "Failed to shutdown %d\n", ret);
>> +
>> + qcom_q6v5_unprepare(&wcss->q6);
>> +
>> + return ret;
>> +}
>> +
>> +static void *wcss_sec_da_to_va(struct rproc *rproc, u64 da, size_t len,
>> + bool *is_iomem)
>> +{
>> + struct wcss_sec *wcss = rproc->priv;
>> + int offset;
>> +
>> + offset = da - wcss->mem_reloc;
>> + if (offset < 0 || offset + len > wcss->mem_size)
>> + return NULL;
>> +
>> + if (is_iomem)
>> + *is_iomem = true;
>> +
>> + return wcss->mem_region + offset;
>> +}
>> +
>> +static int wcss_sec_load(struct rproc *rproc, const struct firmware *fw)
>> +{
>> + struct wcss_sec *wcss = rproc->priv;
>> + struct device *dev = wcss->dev;
>> + int ret;
>> +
>> + ret = qcom_mdt_load(dev, fw, rproc->firmware, wcss->desc->pasid, wcss->mem_region,
>> + wcss->mem_phys, wcss->mem_size, &wcss->mem_reloc);
>> + if (ret)
>> + return ret;
>> +
>> + qcom_pil_info_store("wcss", wcss->mem_phys, wcss->mem_size);
>> +
>> + return 0;
>> +}
>> +
>> +static unsigned long wcss_sec_panic(struct rproc *rproc)
>> +{
>> + struct wcss_sec *wcss = rproc->priv;
>> +
>> + return qcom_q6v5_panic(&wcss->q6);
>> +}
>> +
>> +static void wcss_sec_copy_segment(struct rproc *rproc,
>> + struct rproc_dump_segment *segment,
>> + void *dest, size_t offset, size_t size)
>> +{
>> + struct wcss_sec *wcss = rproc->priv;
>> + struct device *dev = wcss->dev;
>> +
>> + if (!segment->io_ptr)
>> + segment->io_ptr = ioremap_wc(segment->da, segment->size);
>> +
>> + if (!segment->io_ptr) {
>> + dev_err(dev, "Failed to ioremap segment %pad size 0x%zx\n",
>> + &segment->da, segment->size);
>> + return;
>> + }
>> +
>> + if (offset + size <= segment->size) {
>> + memcpy_fromio(dest, segment->io_ptr + offset, size);
>> + } else {
>> + iounmap(segment->io_ptr);
>> + segment->io_ptr = NULL;
>> + }
>> +}
>> +
>> +static int wcss_sec_dump_segments(struct rproc *rproc,
>> + const struct firmware *fw)
>> +{
>> + struct device *dev = rproc->dev.parent;
>> + struct reserved_mem *rmem = NULL;
>> + struct device_node *node;
>> + int num_segs, index;
>> + int ret;
>> +
>> + /*
>> + * Parse through additional reserved memory regions for the rproc
>> + * and add them to the coredump segments
>> + */
>> + num_segs = of_count_phandle_with_args(dev->of_node,
>> + "memory-region", NULL);
>> + for (index = 0; index < num_segs; index++) {
>> + node = of_parse_phandle(dev->of_node,
>> + "memory-region", index);
>> + if (!node)
>> + return -EINVAL;
>> +
>> + rmem = of_reserved_mem_lookup(node);
>> + of_node_put(node);
>> + if (!rmem) {
>> + dev_err(dev, "unable to acquire memory-region index %d num_segs %d\n",
>> + index, num_segs);
>> + return -EINVAL;
>> + }
>> +
>> + dev_dbg(dev, "Adding segment 0x%pa size 0x%pa",
>> + &rmem->base, &rmem->size);
>> + ret = rproc_coredump_add_custom_segment(rproc,
>> + rmem->base,
>> + rmem->size,
>> + wcss_sec_copy_segment,
>> + NULL);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct rproc_ops wcss_sec_ops = {
>> + .start = wcss_sec_start,
>> + .stop = wcss_sec_stop,
>> + .da_to_va = wcss_sec_da_to_va,
>> + .load = wcss_sec_load,
>> + .get_boot_addr = rproc_elf_get_boot_addr,
>> + .panic = wcss_sec_panic,
>> + .parse_fw = wcss_sec_dump_segments,
>> +};
>> +
>> +static int wcss_sec_alloc_memory_region(struct wcss_sec *wcss)
>> +{
>> + struct device *dev = wcss->dev;
>> + struct resource res;
>> + int ret;
>> +
>> + ret = of_reserved_mem_region_to_resource(dev->of_node, 0, &res);
>> + if (ret) {
>> + dev_err(dev, "unable to acquire memory-region resource\n");
>> + return ret;
>> + }
>> +
>> + wcss->mem_phys = res.start;
>> + wcss->mem_reloc = res.start;
>> + wcss->mem_size = resource_size(&res);
>> + wcss->mem_region = devm_ioremap_resource_wc(dev, &res);
>> + if (IS_ERR(wcss->mem_region)) {
>> + dev_err(dev, "unable to map memory region: %pR\n", &res);
>> + return PTR_ERR(wcss->mem_region);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int wcss_sec_probe(struct platform_device *pdev)
>> +{
>> + const struct wcss_data *desc = of_device_get_match_data(&pdev->dev);
>> + const char *fw_name = NULL;
>> + struct wcss_sec *wcss;
>> + struct clk *sleep_clk;
>> + struct clk *int_clk;
>> + struct rproc *rproc;
>> + int ret;
>> +
>> + ret = of_property_read_string(pdev->dev.of_node, "firmware-name",
>> + &fw_name);
>> + if (ret < 0)
>> + return ret;
>> +
>> + rproc = devm_rproc_alloc(&pdev->dev, desc->ss_name, &wcss_sec_ops,
>> + fw_name, sizeof(*wcss));
>> + if (!rproc) {
>> + dev_err(&pdev->dev, "failed to allocate rproc\n");
>> + return -ENOMEM;
>> + }
>> +
>> + wcss = rproc->priv;
>> + wcss->dev = &pdev->dev;
>> + wcss->desc = desc;
>> +
>> + ret = wcss_sec_alloc_memory_region(wcss);
>> + if (ret)
>> + return ret;
>> +
>> + sleep_clk = devm_clk_get_optional_enabled(&pdev->dev, "sleep");
>> + if (IS_ERR(sleep_clk))
>> + return dev_err_probe(&pdev->dev, PTR_ERR(sleep_clk),
>> + "Failed to get sleep clock\n");
>> +
>> + int_clk = devm_clk_get_optional_enabled(&pdev->dev, "interconnect");
>> + if (IS_ERR(int_clk))
>> + return dev_err_probe(&pdev->dev, PTR_ERR(int_clk),
>> + "Failed to get interconnect clock\n");
>> +
>> + ret = qcom_q6v5_init(&wcss->q6, pdev, rproc,
>> + WCSS_CRASH_REASON, NULL, NULL);
>> + if (ret)
>> + return ret;
>> +
>> + qcom_add_glink_subdev(rproc, &wcss->glink_subdev, desc->ss_name);
>> + qcom_add_ssr_subdev(rproc, &wcss->ssr_subdev, desc->ss_name);
>> +
>> + rproc->auto_boot = desc->auto_boot;
>> + rproc->dump_conf = RPROC_COREDUMP_INLINE;
>> + rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
>> +
>> + ret = devm_rproc_add(&pdev->dev, rproc);
>> + if (ret) {
>> + qcom_remove_ssr_subdev(rproc, &wcss->ssr_subdev);
>> + qcom_remove_glink_subdev(rproc, &wcss->glink_subdev);
>> + qcom_q6v5_deinit(&wcss->q6);
>> + return ret;
>> + }
>> +
>> + platform_set_drvdata(pdev, rproc);
>> +
>> + return 0;
>> +}
>> +
>> +static void wcss_sec_remove(struct platform_device *pdev)
>> +{
>> + struct rproc *rproc = platform_get_drvdata(pdev);
>> + struct wcss_sec *wcss = rproc->priv;
>> +
>> + qcom_remove_ssr_subdev(rproc, &wcss->ssr_subdev);
>> + qcom_remove_glink_subdev(rproc, &wcss->glink_subdev);
>> + qcom_q6v5_deinit(&wcss->q6);
>> +}
>> +
>> +static const struct wcss_data wcss_sec_ipq5332_res_init = {
>> + .pasid = MPD_WCSS_PAS_ID,
>> + .ss_name = "q6wcss",
>> +};
>> +
>> +static const struct wcss_data wcss_sec_ipq9574_res_init = {
>> + .pasid = WCSS_PAS_ID,
>> + .ss_name = "q6wcss",
>> +};
>> +
>> +static const struct of_device_id wcss_sec_of_match[] = {
>> + { .compatible = "qcom,ipq5018-wcss-sec-pil", .data = &wcss_sec_ipq5332_res_init },
>> + { .compatible = "qcom,ipq5332-wcss-sec-pil", .data = &wcss_sec_ipq5332_res_init },
>> + { .compatible = "qcom,ipq9574-wcss-sec-pil", .data = &wcss_sec_ipq9574_res_init },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, wcss_sec_of_match);
>> +
>> +static struct platform_driver wcss_sec_driver = {
>> + .probe = wcss_sec_probe,
>> + .remove = wcss_sec_remove,
>> + .driver = {
>> + .name = "qcom-wcss-secure-pil",
>> + .of_match_table = wcss_sec_of_match,
>> + },
>> +};
>> +module_platform_driver(wcss_sec_driver);
>> +
>> +MODULE_DESCRIPTION("Hexagon WCSS Secure Peripheral Image Loader");
>> +MODULE_LICENSE("GPL");
>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>> index ef711a5b1a7f..cf06b9846309 100644
>> --- a/include/linux/remoteproc.h
>> +++ b/include/linux/remoteproc.h
>> @@ -204,6 +204,7 @@ enum rproc_dump_mechanism {
>> * @node: list node related to the rproc segment list
>> * @da: device address of the segment
>> * @size: size of the segment
>> + * @io_ptr: ptr to store the ioremapped dump segment
>> * @priv: private data associated with the dump_segment
>> * @dump: custom dump function to fill device memory segment associated
>> * with coredump
>> @@ -215,6 +216,7 @@ struct rproc_dump_segment {
>> dma_addr_t da;
>> size_t size;
>>
>> + void __iomem *io_ptr;
>> void *priv;
>> void (*dump)(struct rproc *rproc, struct rproc_dump_segment *segment,
>> void *dest, size_t offset, size_t size);
>>
>> --
>> 2.34.1
>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v14 3/9] remoteproc: qcom: add hexagon based WCSS secure PIL driver
2026-08-04 11:19 ` Vignesh Viswanathan
@ 2026-08-04 13:10 ` Mukesh Ojha
0 siblings, 0 replies; 20+ messages in thread
From: Mukesh Ojha @ 2026-08-04 13:10 UTC (permalink / raw)
To: Vignesh Viswanathan
Cc: Varadarajan Narayanan, Bjorn Andersson, Konrad Dybcio,
Mathieu Poirier, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Manikanta Mylavarapu, linux-arm-msm, linux-kernel,
linux-remoteproc, devicetree, Gokul Sriram Palanisamy,
George Moussalem, Dmitry Baryshkov
On Tue, Aug 04, 2026 at 04:49:17PM +0530, Vignesh Viswanathan wrote:
>
>
> On 8/3/2026 6:14 PM, Mukesh Ojha wrote:
> > On Mon, Aug 03, 2026 at 04:16:09PM +0530, Varadarajan Narayanan wrote:
> >> From: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
> >>
> >> Add support to bring up hexagon based WCSS using secure PIL. All IPQxxxx
> >> SoCs support secure Peripheral Image Loading (PIL).
> >>
> >> Secure PIL image is signed firmware image which only trusted software such
> >> as TrustZone (TZ) can authenticate and load. Linux kernel will send a
> >> Peripheral Authentication Service (PAS) request to TZ to authenticate and
> >> load the PIL images.
> >>
> >> In order to avoid overloading the existing WCSS driver or PAS driver, we
> >> came up with this new PAS based IPQ WCSS driver.
> >
> > Can you tell why you need another driver if you are using the same PAS
> > method and it is using more of everything what PAS already
> > supports ?
> >
>
> Hi Mukesh,
>
> We just use pas_auth_and_reset and pas_shutdown APIs and do not need
> the entire framework provided by the PAS driver for IPQ WCSS.
But you are using qcom_mdt_load() in wcss_sec_load(), which internally will
call pas_init and memset SMCs.
>
> Also, feedback from Bjorn in [1] was to have separate driver
> instead of overloading existing WCSS driver.
That's a very old thread, 5 years is a long time. We can still revisit why
you need a separate driver. If you already have all the stuff required to
be a PAS driver, why not use that?
>
> [1] https://lore.kernel.org/all/YFK1VpP74rQH2Z72@builder.lan/
>
-Mukesh
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v14 4/9] arm64: dts: qcom: ipq5018: add nodes to bring up q6
2026-08-03 10:46 [PATCH v14 0/9] Add new driver for WCSS secure PIL loading Varadarajan Narayanan
` (2 preceding siblings ...)
2026-08-03 10:46 ` [PATCH v14 3/9] remoteproc: qcom: add hexagon based WCSS secure PIL driver Varadarajan Narayanan
@ 2026-08-03 10:46 ` Varadarajan Narayanan
2026-08-03 10:46 ` [PATCH v14 5/9] arm64: dts: qcom: ipq5332: " Varadarajan Narayanan
` (4 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Varadarajan Narayanan @ 2026-08-03 10:46 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu
Cc: linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
George Moussalem, Dmitry Baryshkov, Konrad Dybcio,
Vignesh Viswanathan, Varadarajan Narayanan
From: George Moussalem <george.moussalem@outlook.com>
Enable nodes required for q6 remoteproc bring up.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
[ Change 'qcom,smem-state-names' order to resolve dt-bindings-check error ]
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Tested-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/ipq5018.dtsi | 66 +++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
index 4fc627b47fe7..82af532bff38 100644
--- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
@@ -158,6 +158,35 @@ tz_region: tz@4ac00000 {
reg = <0x0 0x4ac00000 0x0 0x200000>;
no-map;
};
+
+ q6_region: wcss@4b000000 {
+ reg = <0x0 0x4b000000 0x0 0x1b00000>;
+ no-map;
+ };
+ };
+
+ wcss: smp2p-wcss {
+ compatible = "qcom,smp2p";
+ qcom,smem = <435>, <428>;
+
+ interrupt-parent = <&intc>;
+ interrupts = <GIC_SPI 177 IRQ_TYPE_EDGE_RISING>;
+
+ mboxes = <&apcs_glb 9>;
+
+ qcom,local-pid = <0>;
+ qcom,remote-pid = <1>;
+
+ wcss_smp2p_out: master-kernel {
+ qcom,entry-name = "master-kernel";
+ #qcom,smem-state-cells = <1>;
+ };
+
+ wcss_smp2p_in: slave-kernel {
+ qcom,entry-name = "slave-kernel";
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
};
soc: soc@0 {
@@ -734,6 +763,43 @@ frame@b128000 {
};
};
+ q6v5_wcss: remoteproc@cd00000 {
+ compatible = "qcom,ipq5018-wcss-sec-pil";
+ reg = <0x0cd00000 0x10000>;
+ firmware-name = "ath11k/IPQ5018/hw1.0/q6_fw.mbn";
+ interrupts-extended = <&intc GIC_SPI 291 IRQ_TYPE_EDGE_RISING>,
+ <&wcss_smp2p_in 0 IRQ_TYPE_NONE>,
+ <&wcss_smp2p_in 1 IRQ_TYPE_NONE>,
+ <&wcss_smp2p_in 2 IRQ_TYPE_NONE>,
+ <&wcss_smp2p_in 3 IRQ_TYPE_NONE>;
+ interrupt-names = "wdog",
+ "fatal",
+ "ready",
+ "handover",
+ "stop-ack";
+
+ clocks = <&gcc GCC_SLEEP_CLK_SRC>,
+ <&gcc GCC_SYS_NOC_WCSS_AHB_CLK>;
+ clock-names = "sleep",
+ "interconnect";
+
+ qcom,smem-states = <&wcss_smp2p_out 1>,
+ <&wcss_smp2p_out 0>;
+ qcom,smem-state-names = "stop",
+ "shutdown";
+
+ memory-region = <&q6_region>;
+
+ status = "disabled";
+
+ glink-edge {
+ interrupts = <GIC_SPI 179 IRQ_TYPE_EDGE_RISING>;
+ label = "rtr";
+ qcom,remote-pid = <1>;
+ mboxes = <&apcs_glb 8>;
+ };
+ };
+
pcie1: pcie@80000000 {
compatible = "qcom,pcie-ipq5018";
reg = <0x80000000 0xf1d>,
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v14 5/9] arm64: dts: qcom: ipq5332: add nodes to bring up q6
2026-08-03 10:46 [PATCH v14 0/9] Add new driver for WCSS secure PIL loading Varadarajan Narayanan
` (3 preceding siblings ...)
2026-08-03 10:46 ` [PATCH v14 4/9] arm64: dts: qcom: ipq5018: add nodes to bring up q6 Varadarajan Narayanan
@ 2026-08-03 10:46 ` Varadarajan Narayanan
2026-08-03 10:46 ` [PATCH v14 6/9] arm64: dts: qcom: ipq9574: " Varadarajan Narayanan
` (3 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Varadarajan Narayanan @ 2026-08-03 10:46 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu
Cc: linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
Gokul Sriram Palanisamy, George Moussalem, Konrad Dybcio,
Dmitry Baryshkov, Vignesh Viswanathan, Varadarajan Narayanan
From: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
Enable nodes required for q6 remoteproc bring up.
Signed-off-by: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
Signed-off-by: Gokul Sriram Palanisamy <gokul.sriram.p@oss.qualcomm.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Tested-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/ipq5332.dtsi | 64 +++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index 74d9de8d7641..ff4187fb7fba 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
@@ -159,6 +159,35 @@ smem@4a800000 {
hwlocks = <&tcsr_mutex 3>;
};
+
+ q6_region: wcss@4a900000 {
+ reg = <0x0 0x4a900000 0x0 0x2b00000>;
+ no-map;
+ };
+ };
+
+ wcss: smp2p-wcss {
+ compatible = "qcom,smp2p";
+ qcom,smem = <435>, <428>;
+
+ interrupt-parent = <&intc>;
+ interrupts = <GIC_SPI 418 IRQ_TYPE_EDGE_RISING>;
+
+ mboxes = <&apcs_glb 9>;
+
+ qcom,local-pid = <0>;
+ qcom,remote-pid = <1>;
+
+ wcss_smp2p_out: master-kernel {
+ qcom,entry-name = "master-kernel";
+ #qcom,smem-state-cells = <1>;
+ };
+
+ wcss_smp2p_in: slave-kernel {
+ qcom,entry-name = "slave-kernel";
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
};
soc@0 {
@@ -656,6 +685,41 @@ frame@b128000 {
};
};
+ q6v5_wcss: remoteproc@d100000 {
+ compatible = "qcom,ipq5332-wcss-sec-pil";
+ reg = <0x0d100000 0x10000>;
+ firmware-name = "ath12k/IPQ5332/hw1.0/q6_fw0.mbn";
+ interrupts-extended = <&intc GIC_SPI 421 IRQ_TYPE_EDGE_RISING>,
+ <&wcss_smp2p_in 0 IRQ_TYPE_NONE>,
+ <&wcss_smp2p_in 1 IRQ_TYPE_NONE>,
+ <&wcss_smp2p_in 2 IRQ_TYPE_NONE>,
+ <&wcss_smp2p_in 3 IRQ_TYPE_NONE>;
+ interrupt-names = "wdog",
+ "fatal",
+ "ready",
+ "handover",
+ "stop-ack";
+
+ clocks = <&gcc GCC_IM_SLEEP_CLK>;
+ clock-names = "sleep";
+
+ qcom,smem-states = <&wcss_smp2p_out 1>,
+ <&wcss_smp2p_out 0>;
+ qcom,smem-state-names = "stop",
+ "shutdown";
+
+ memory-region = <&q6_region>;
+
+ status = "disabled";
+
+ glink-edge {
+ interrupts = <GIC_SPI 417 IRQ_TYPE_EDGE_RISING>;
+ label = "rtr";
+ qcom,remote-pid = <1>;
+ mboxes = <&apcs_glb 8>;
+ };
+ };
+
pcie1: pcie@18000000 {
compatible = "qcom,pcie-ipq5332", "qcom,pcie-ipq9574";
reg = <0x18000000 0xf1c>,
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v14 6/9] arm64: dts: qcom: ipq9574: add nodes to bring up q6
2026-08-03 10:46 [PATCH v14 0/9] Add new driver for WCSS secure PIL loading Varadarajan Narayanan
` (4 preceding siblings ...)
2026-08-03 10:46 ` [PATCH v14 5/9] arm64: dts: qcom: ipq5332: " Varadarajan Narayanan
@ 2026-08-03 10:46 ` Varadarajan Narayanan
2026-08-03 10:46 ` [PATCH v14 7/9] arm64: dts: qcom: ipq5018: Enable q6v5_wcss Varadarajan Narayanan
` (2 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Varadarajan Narayanan @ 2026-08-03 10:46 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu
Cc: linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
Gokul Sriram Palanisamy, George Moussalem, Konrad Dybcio,
Dmitry Baryshkov, Vignesh Viswanathan, Varadarajan Narayanan
From: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
Enable nodes required for q6 remoteproc bring up.
Signed-off-by: Manikanta Mylavarapu <manikanta.mylavarapu@oss.qualcomm.com>
Signed-off-by: Gokul Sriram Palanisamy <gokul.sriram.p@oss.qualcomm.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Tested-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/ipq9574.dtsi | 62 ++++++++++++++++++++++++++++++++++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 5a5bda5f21a1..194bba02f2ea 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -3,7 +3,7 @@
* IPQ9574 SoC device tree source
*
* Copyright (c) 2020-2021 The Linux Foundation. All rights reserved.
- * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2023-2025, Qualcomm Innovation Center, Inc. All rights reserved.
*/
#include <dt-bindings/clock/qcom,apss-ipq.h>
@@ -222,6 +222,35 @@ smem@4aa00000 {
hwlocks = <&tcsr_mutex 3>;
no-map;
};
+
+ q6_region: wcss@4ab00000 {
+ reg = <0x0 0x4ab00000 0x0 0x2b00000>;
+ no-map;
+ };
+ };
+
+ wcss: smp2p-wcss {
+ compatible = "qcom,smp2p";
+ qcom,smem = <435>, <428>;
+
+ interrupt-parent = <&intc>;
+ interrupts = <GIC_SPI 322 IRQ_TYPE_EDGE_RISING>;
+
+ mboxes = <&apcs_glb 9>;
+
+ qcom,local-pid = <0>;
+ qcom,remote-pid = <1>;
+
+ smp2p_wcss_out: master-kernel {
+ qcom,entry-name = "master-kernel";
+ #qcom,smem-state-cells = <1>;
+ };
+
+ smp2p_wcss_in: slave-kernel {
+ qcom,entry-name = "slave-kernel";
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
};
soc: soc@0 {
@@ -924,6 +953,37 @@ frame@b128000 {
};
};
+ q6v5_wcss: remoteproc@cd00000 {
+ compatible = "qcom,ipq9574-wcss-sec-pil";
+ reg = <0x0cd00000 0x10000>;
+ firmware-name = "ath11k/IPQ9574/hw1.0/q6_fw.mbn";
+ interrupts-extended = <&intc GIC_SPI 325 IRQ_TYPE_EDGE_RISING>,
+ <&smp2p_wcss_in 0 IRQ_TYPE_NONE>,
+ <&smp2p_wcss_in 1 IRQ_TYPE_NONE>,
+ <&smp2p_wcss_in 2 IRQ_TYPE_NONE>,
+ <&smp2p_wcss_in 3 IRQ_TYPE_NONE>;
+ interrupt-names = "wdog",
+ "fatal",
+ "ready",
+ "handover",
+ "stop-ack";
+
+ qcom,smem-states = <&smp2p_wcss_out 1>,
+ <&smp2p_wcss_out 0>;
+ qcom,smem-state-names = "stop",
+ "shutdown";
+ memory-region = <&q6_region>;
+
+ status = "disabled";
+
+ glink-edge {
+ interrupts = <GIC_SPI 321 IRQ_TYPE_EDGE_RISING>;
+ label = "rtr";
+ qcom,remote-pid = <1>;
+ mboxes = <&apcs_glb 8>;
+ };
+ };
+
pcie1: pcie@10000000 {
compatible = "qcom,pcie-ipq9574";
reg = <0x10000000 0xf1d>,
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v14 7/9] arm64: dts: qcom: ipq5018: Enable q6v5_wcss
2026-08-03 10:46 [PATCH v14 0/9] Add new driver for WCSS secure PIL loading Varadarajan Narayanan
` (5 preceding siblings ...)
2026-08-03 10:46 ` [PATCH v14 6/9] arm64: dts: qcom: ipq9574: " Varadarajan Narayanan
@ 2026-08-03 10:46 ` Varadarajan Narayanan
2026-08-03 10:46 ` [PATCH v14 8/9] arm64: dts: qcom: ipq5332: " Varadarajan Narayanan
2026-08-03 10:46 ` [PATCH v14 9/9] arm64: dts: qcom: ipq9574: " Varadarajan Narayanan
8 siblings, 0 replies; 20+ messages in thread
From: Varadarajan Narayanan @ 2026-08-03 10:46 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu
Cc: linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
Varadarajan Narayanan
Turn on the q6v5_wcss node.
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts b/arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts
index 33eef92b19b1..ce3970358056 100644
--- a/arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts
@@ -46,6 +46,10 @@ &pcie0_port0 {
wake-gpios = <&tlmm 16 GPIO_ACTIVE_LOW>;
};
+&q6v5_wcss {
+ status = "okay";
+};
+
&sdhc_1 {
pinctrl-0 = <&sdc_default_state>;
pinctrl-names = "default";
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v14 8/9] arm64: dts: qcom: ipq5332: Enable q6v5_wcss
2026-08-03 10:46 [PATCH v14 0/9] Add new driver for WCSS secure PIL loading Varadarajan Narayanan
` (6 preceding siblings ...)
2026-08-03 10:46 ` [PATCH v14 7/9] arm64: dts: qcom: ipq5018: Enable q6v5_wcss Varadarajan Narayanan
@ 2026-08-03 10:46 ` Varadarajan Narayanan
2026-08-03 10:46 ` [PATCH v14 9/9] arm64: dts: qcom: ipq9574: " Varadarajan Narayanan
8 siblings, 0 replies; 20+ messages in thread
From: Varadarajan Narayanan @ 2026-08-03 10:46 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu
Cc: linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
Varadarajan Narayanan
Turn on the q6v5_wcss node.
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/ipq5332-rdp-common.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp-common.dtsi b/arch/arm64/boot/dts/qcom/ipq5332-rdp-common.dtsi
index 3bbf748f2e5c..9e82afb354d4 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332-rdp-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp-common.dtsi
@@ -55,6 +55,10 @@ &blsp1_uart0 {
status = "okay";
};
+&q6v5_wcss {
+ status = "okay";
+};
+
/*
* The bootstrap pins for the board select the XO clock frequency that
* supports 48 MHZ or 96 MHZ. This setting automatically enables the
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v14 9/9] arm64: dts: qcom: ipq9574: Enable q6v5_wcss
2026-08-03 10:46 [PATCH v14 0/9] Add new driver for WCSS secure PIL loading Varadarajan Narayanan
` (7 preceding siblings ...)
2026-08-03 10:46 ` [PATCH v14 8/9] arm64: dts: qcom: ipq5332: " Varadarajan Narayanan
@ 2026-08-03 10:46 ` Varadarajan Narayanan
8 siblings, 0 replies; 20+ messages in thread
From: Varadarajan Narayanan @ 2026-08-03 10:46 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manikanta Mylavarapu
Cc: linux-arm-msm, linux-kernel, linux-remoteproc, devicetree,
Varadarajan Narayanan
Turn on the q6v5_wcss node.
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi b/arch/arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi
index 62877b46f9b3..494a150535cf 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp-common.dtsi
@@ -113,6 +113,10 @@ &cpu3 {
cpu-supply = <&mp5496_s1>;
};
+&q6v5_wcss {
+ status = "okay";
+};
+
&rpm_requests {
regulators {
compatible = "qcom,rpm-mp5496-regulators";
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread