* [PATCH v3 0/3] remoteproc: Hawi CDSP support with per-PD
@ 2026-09-09 10:50 Mukesh Ojha
2026-09-09 10:50 ` [PATCH v3 1/3] remoteproc: qcom_q6v5_pas: propagate dev_pm_genpd_set_performance_state() errors Mukesh Ojha
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Mukesh Ojha @ 2026-09-09 10:50 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manivannan Sadhasivam
Cc: linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
Mukesh Ojha
qcom,hawi-cdsp-pas was initially grouped as a fallback to
qcom,sm8550-cdsp-pas on the hardware-level compatibility. Bringup revealed
that the NSP proxy power domain on Hawi requires a specific RPMH
performance level below what sm8550 already supports, breaking the
compatibility contract the fallback string implies. Remove
qcom,hawi-cdsp-pas from the sm8550-cdsp-pas fallback items block, add it to
the standalone compatible enum, and extend the existing cx/mxc/nsp
power-domain constraint to cover it explicitly.
The fix is two-part: first, propagate the dev_pm_genpd_set_performance_state()
return value so the failure becomes visible instead of silently proceeding
with firmware load. Second, introduce a per-PD performance state table in
qcom_pas_data so platforms can declare explicit RPMH levels for each proxy
domain. Platforms that omit the table retain the existing INT_MAX behaviour.
Hawi CDSP declares CX/MXC at TURBO and NSP at NOM, matching the hardware
requirement.
The binding and DTS are updated in tandem: qcom,hawi-cdsp-pas is made a
standalone compatible (dropped from the sm8550-cdsp-pas fallback group)
because the proxy PD behaviour diverges from sm8550, breaking the
compatibility contract the fallback string implies.
---
Changes in v3:
- Converted warn into failure in 1/3 as per review.
- Added R-b tag.
- Link to v2:
https://lore.kernel.org/lkml/20260902204334.2061226-1-mukesh.ojha@oss.qualcomm.com/
Changes in v2:
- Propagating the err dev_pm_genpd_set_performance_state() as a
separate commit.
- Added a binding correction for hawi as it should be
standalone one instead of falling back to sm8550.
- Add num_proxy_pd_performance_states count field with a
WARN_ON in
probe to catch mismatch between the array size and the actual
proxy
PD count at boot time, returning -EINVAL if they diverge.
- [v1]
https://lore.kernel.org/lkml/20260828181311.4038346-3-mukesh.ojha@oss.qualcomm.com/
Mukesh Ojha (3):
remoteproc: qcom_q6v5_pas: propagate
dev_pm_genpd_set_performance_state() errors
dt-bindings: remoteproc: qcom,sm8550-pas: make qcom,hawi-cdsp-pas
standalone
remoteproc: qcom_q6v5_pas: add per-PD proxy performance states for
Hawi CDSP
.../bindings/remoteproc/qcom,sm8550-pas.yaml | 3 +-
drivers/remoteproc/qcom_q6v5_pas.c | 54 ++++++++++++++++++-
2 files changed, 55 insertions(+), 2 deletions(-)
--
2.55.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/3] remoteproc: qcom_q6v5_pas: propagate dev_pm_genpd_set_performance_state() errors
2026-09-09 10:50 [PATCH v3 0/3] remoteproc: Hawi CDSP support with per-PD Mukesh Ojha
@ 2026-09-09 10:50 ` Mukesh Ojha
2026-09-09 11:08 ` sashiko-bot
2026-09-09 10:50 ` [PATCH v3 2/3] dt-bindings: remoteproc: qcom,sm8550-pas: make qcom,hawi-cdsp-pas standalone Mukesh Ojha
2026-09-09 10:50 ` [PATCH v3 3/3] remoteproc: qcom_q6v5_pas: add per-PD proxy performance states for Hawi CDSP Mukesh Ojha
2 siblings, 1 reply; 10+ messages in thread
From: Mukesh Ojha @ 2026-09-09 10:50 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manivannan Sadhasivam
Cc: linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
Mukesh Ojha, Abel Vesa
The proxy power domain enable path discards the return value of
dev_pm_genpd_set_performance_state(), masking failures silently.
When the call fails the performance state is not applied, yet
firmware load proceeds without any indication of the problem.
Capture the return value and bail out on failure, unwinding any
previously voted proxy PDs via the existing unroll path. A proxy
performance state failure indicates that the requested OPP is not
supported by the underlying power domain; continuing would leave
the remoteproc running with an unconfigured voltage/frequency
vote, so treat it as fatal.
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
drivers/remoteproc/qcom_q6v5_pas.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index a005546c265d..c163e66347f8 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -167,7 +167,13 @@ static int qcom_pas_pds_enable(struct qcom_pas *pas, struct device **pds,
int i;
for (i = 0; i < pd_count; i++) {
- dev_pm_genpd_set_performance_state(pds[i], INT_MAX);
+ ret = dev_pm_genpd_set_performance_state(pds[i], INT_MAX);
+ if (ret) {
+ dev_err(pas->dev,
+ "failed to set proxy PD %d state %u: %d\n",
+ i, INT_MAX, ret);
+ goto unroll_pd_votes;
+ }
ret = pm_runtime_get_sync(pds[i]);
if (ret < 0) {
pm_runtime_put_noidle(pds[i]);
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/3] dt-bindings: remoteproc: qcom,sm8550-pas: make qcom,hawi-cdsp-pas standalone
2026-09-09 10:50 [PATCH v3 0/3] remoteproc: Hawi CDSP support with per-PD Mukesh Ojha
2026-09-09 10:50 ` [PATCH v3 1/3] remoteproc: qcom_q6v5_pas: propagate dev_pm_genpd_set_performance_state() errors Mukesh Ojha
@ 2026-09-09 10:50 ` Mukesh Ojha
2026-09-10 4:26 ` Yijie Yang
2026-09-09 10:50 ` [PATCH v3 3/3] remoteproc: qcom_q6v5_pas: add per-PD proxy performance states for Hawi CDSP Mukesh Ojha
2 siblings, 1 reply; 10+ messages in thread
From: Mukesh Ojha @ 2026-09-09 10:50 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manivannan Sadhasivam
Cc: linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
Mukesh Ojha, Krzysztof Kozlowski
qcom,hawi-cdsp-pas was initially grouped as a fallback to
qcom,sm8550-cdsp-pas on the assumption of hardware-level compatibility.
Bringup revealed that the NSP proxy power domain on Hawi requires a
specific RPMH performance level that the sm8550 driver data does not
provide, breaking the compatibility contract the fallback string implies.
Remove qcom,hawi-cdsp-pas from the sm8550-cdsp-pas fallback items block,
add it to the standalone compatible enum, and extend the existing
cx/mxc/nsp power-domain constraint to cover it explicitly.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
.../devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml
index c58e8a6c7fe1..cf57095cd49d 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml
@@ -18,6 +18,7 @@ properties:
oneOf:
- enum:
- qcom,eliza-cdsp-pas
+ - qcom,hawi-cdsp-pas
- qcom,sdx75-mpss-pas
- qcom,sm8550-adsp-pas
- qcom,sm8550-cdsp-pas
@@ -40,7 +41,6 @@ properties:
- items:
- enum:
- qcom,glymur-cdsp-pas
- - qcom,hawi-cdsp-pas
- qcom,kaanapali-cdsp-pas
- qcom,maili-cdsp-pas
- const: qcom,sm8550-cdsp-pas
@@ -273,6 +273,7 @@ allOf:
compatible:
contains:
enum:
+ - qcom,hawi-cdsp-pas
- qcom,sm8550-cdsp-pas
- qcom,sm8650-cdsp-pas
- qcom,x1e80100-cdsp-pas
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 3/3] remoteproc: qcom_q6v5_pas: add per-PD proxy performance states for Hawi CDSP
2026-09-09 10:50 [PATCH v3 0/3] remoteproc: Hawi CDSP support with per-PD Mukesh Ojha
2026-09-09 10:50 ` [PATCH v3 1/3] remoteproc: qcom_q6v5_pas: propagate dev_pm_genpd_set_performance_state() errors Mukesh Ojha
2026-09-09 10:50 ` [PATCH v3 2/3] dt-bindings: remoteproc: qcom,sm8550-pas: make qcom,hawi-cdsp-pas standalone Mukesh Ojha
@ 2026-09-09 10:50 ` Mukesh Ojha
2026-09-09 11:30 ` sashiko-bot
2026-09-10 4:29 ` Yijie Yang
2 siblings, 2 replies; 10+ messages in thread
From: Mukesh Ojha @ 2026-09-09 10:50 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manivannan Sadhasivam
Cc: linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
Mukesh Ojha, Abel Vesa
The proxy power domain enable path requests INT_MAX performance
state for every proxy PD. On Hawi, the NSP proxy power domain's
RPMH power domain has no OPP at INT_MAX, while CX and MXC accept
INT_MAX, mapping to their maximum supported level.
Introduce a proxy_pd_performance_states array in qcom_pas_data
to allow per-PD RPMH levels to be declared explicitly. Platforms
that omit this field retain the existing INT_MAX behaviour.
Add Hawi CDSP remoteproc support with the following proxy PD
performance states:
CX: RPMH_REGULATOR_LEVEL_TURBO
MXC: RPMH_REGULATOR_LEVEL_TURBO
NSP: RPMH_REGULATOR_LEVEL_NOM
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
drivers/remoteproc/qcom_q6v5_pas.c | 50 ++++++++++++++++++++++++++++--
1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index c163e66347f8..8ac66db80193 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -28,6 +28,7 @@
#include <linux/soc/qcom/mdt_loader.h>
#include <linux/soc/qcom/smem.h>
#include <linux/soc/qcom/smem_state.h>
+#include <dt-bindings/power/qcom,rpmhpd.h>
#include "qcom_common.h"
#include "qcom_pil_info.h"
@@ -51,6 +52,8 @@ struct qcom_pas_data {
bool decrypt_shutdown;
char **proxy_pd_names;
+ const unsigned int *proxy_pd_performance_states;
+ unsigned int num_proxy_pd_performance_states;
const char *load_state;
const char *ssr_name;
@@ -79,6 +82,7 @@ struct qcom_pas {
struct regulator *px_supply;
struct device *proxy_pds[3];
+ const unsigned int *proxy_pd_performance_states;
int proxy_pd_count;
@@ -167,11 +171,16 @@ static int qcom_pas_pds_enable(struct qcom_pas *pas, struct device **pds,
int i;
for (i = 0; i < pd_count; i++) {
- ret = dev_pm_genpd_set_performance_state(pds[i], INT_MAX);
+ unsigned int state = INT_MAX;
+
+ if (pas->proxy_pd_performance_states)
+ state = pas->proxy_pd_performance_states[i];
+
+ ret = dev_pm_genpd_set_performance_state(pds[i], state);
if (ret) {
dev_err(pas->dev,
"failed to set proxy PD %d state %u: %d\n",
- i, INT_MAX, ret);
+ i, state, ret);
goto unroll_pd_votes;
}
ret = pm_runtime_get_sync(pds[i]);
@@ -879,6 +888,7 @@ static int qcom_pas_probe(struct platform_device *pdev)
pas->info_name = desc->sysmon_name;
pas->smem_host_id = desc->smem_host_id;
pas->decrypt_shutdown = desc->decrypt_shutdown;
+ pas->proxy_pd_performance_states = desc->proxy_pd_performance_states;
pas->region_assign_idx = desc->region_assign_idx;
pas->region_assign_count = min_t(int, MAX_ASSIGN_COUNT, desc->region_assign_count);
pas->region_assign_vmid = desc->region_assign_vmid;
@@ -914,6 +924,14 @@ static int qcom_pas_probe(struct platform_device *pdev)
goto unassign_mem;
pas->proxy_pd_count = ret;
+ if (WARN(desc->proxy_pd_performance_states &&
+ desc->num_proxy_pd_performance_states != pas->proxy_pd_count,
+ "proxy_pd_performance_states count %u != pd count %d\n",
+ desc->num_proxy_pd_performance_states, pas->proxy_pd_count)) {
+ ret = -EINVAL;
+ goto detach_proxy_pds;
+ }
+
ret = qcom_q6v5_init(&pas->q6v5, pdev, rproc, desc->crash_reason_smem,
desc->load_state, qcom_pas_handover);
if (ret)
@@ -1804,6 +1822,33 @@ static const struct qcom_pas_data glymur_soccp_resource = {
.needs_tzmem = true,
};
+static const struct qcom_pas_data hawi_cdsp_resource = {
+ .crash_reason_smem = 601,
+ .firmware_name = "cdsp.mdt",
+ .dtb_firmware_name = "cdsp_dtb.mdt",
+ .pas_id = 18,
+ .dtb_pas_id = 0x25,
+ .minidump_id = 7,
+ .auto_boot = true,
+ .proxy_pd_names = (char*[]){
+ "cx",
+ "mxc",
+ "nsp",
+ NULL
+ },
+ .proxy_pd_performance_states = (const unsigned int[]){
+ RPMH_REGULATOR_LEVEL_TURBO,
+ RPMH_REGULATOR_LEVEL_TURBO,
+ RPMH_REGULATOR_LEVEL_NOM,
+ },
+ .num_proxy_pd_performance_states = 3,
+ .load_state = "cdsp",
+ .ssr_name = "cdsp",
+ .sysmon_name = "cdsp",
+ .ssctl_id = 0x17,
+ .smem_host_id = 5,
+};
+
static const struct qcom_pas_data eliza_cdsp_resource = {
.crash_reason_smem = 601,
.firmware_name = "cdsp.mbn",
@@ -1833,6 +1878,7 @@ static const struct of_device_id qcom_pas_of_match[] = {
{ .compatible = "qcom,eliza-adsp-pas", .data = &sm8550_adsp_resource },
{ .compatible = "qcom,eliza-cdsp-pas", .data = &eliza_cdsp_resource },
{ .compatible = "qcom,glymur-soccp-pas", .data = &glymur_soccp_resource },
+ { .compatible = "qcom,hawi-cdsp-pas", .data = &hawi_cdsp_resource },
{ .compatible = "qcom,kaanapali-soccp-pas", .data = &kaanapali_soccp_resource },
{ .compatible = "qcom,milos-adsp-pas", .data = &sm8550_adsp_resource },
{ .compatible = "qcom,milos-cdsp-pas", .data = &milos_cdsp_resource },
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] remoteproc: qcom_q6v5_pas: propagate dev_pm_genpd_set_performance_state() errors
2026-09-09 10:50 ` [PATCH v3 1/3] remoteproc: qcom_q6v5_pas: propagate dev_pm_genpd_set_performance_state() errors Mukesh Ojha
@ 2026-09-09 11:08 ` sashiko-bot
2026-09-10 15:01 ` Mukesh Ojha
0 siblings, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-09-09 11:08 UTC (permalink / raw)
To: Mukesh Ojha; +Cc: robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The added error check for dev_pm_genpd_set_performance_state() is dead code and completely fails to fix the silent failure masking.
Pre-existing issues:
- [High] Pre-existing identical sequence flaw in sister remoteproc drivers silently masks performance state errors.
--
commit 79e5a0b96d1b1d35117390df652884f8c2c16c71
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
remoteproc: qcom_q6v5_pas: propagate dev_pm_genpd_set_performance_state() errors
This commit attempts to capture and handle errors from
dev_pm_genpd_set_performance_state() in qcom_pas_pds_enable() to avoid
silently masking performance state failures.
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index ca8e61254c449..42bcbe72f95bd 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -167,7 +167,13 @@ static int qcom_pas_pds_enable(struct qcom_pas *pas, struct device **pds,
> int i;
>
> for (i = 0; i < pd_count; i++) {
> - dev_pm_genpd_set_performance_state(pds[i], INT_MAX);
> + ret = dev_pm_genpd_set_performance_state(pds[i], INT_MAX);
> + if (ret) {
> + dev_err(pas->dev,
> + "failed to set proxy PD %d state %u: %d\n",
> + i, INT_MAX, ret);
> + goto unroll_pd_votes;
> + }
> ret = pm_runtime_get_sync(pds[i]);
[Severity: High]
Is this newly added error check able to catch performance state errors?
Because dev_pm_genpd_set_performance_state() is called before
pm_runtime_get_sync(), the proxy PD device is still in the RPM_SUSPENDED
state when this executes.
When called on an RPM_SUSPENDED device, genpd_dev_pm_set_performance_state()
merely caches the request in rpm_pstate and unconditionally returns 0.
As a result, this check will never evaluate to true. The actual application
of the state is deferred to pm_runtime_get_sync(), which calls
genpd_runtime_resume() and genpd_restore_performance_state().
Because genpd_restore_performance_state() returns void, it drops any errors
from the underlying provider.
Does this mean the failure remains silently masked despite this patch?
[Severity: High]
This isn't a bug introduced by this patch, but this identical sequence flaw
exists in several sister remoteproc drivers.
In qcom_q6v5_mss.c:q6v5_pds_enable(),
qcom_q6v5_adsp.c:qcom_rproc_pds_enable(), and qcom_wcnss.c:wcnss_start(),
the drivers also call dev_pm_genpd_set_performance_state() while the proxy
PDs are in the RPM_SUSPENDED state (prior to calling pm_runtime_get_sync()).
Since they are suspended, the genpd framework defers execution to the resume
path where errors are discarded, silently masking performance state failures.
Should the initialization sequence be adjusted across all these drivers so
the devices are resumed before setting the performance state?
> if (ret < 0) {
> pm_runtime_put_noidle(pds[i]);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909105049.1317985-1-mukesh.ojha@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/3] remoteproc: qcom_q6v5_pas: add per-PD proxy performance states for Hawi CDSP
2026-09-09 10:50 ` [PATCH v3 3/3] remoteproc: qcom_q6v5_pas: add per-PD proxy performance states for Hawi CDSP Mukesh Ojha
@ 2026-09-09 11:30 ` sashiko-bot
2026-09-10 4:29 ` Yijie Yang
1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-09-09 11:30 UTC (permalink / raw)
To: Mukesh Ojha; +Cc: robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Medium] Asynchronous Runtime PM put during device removal violates PM guidelines by racing with device detachment.
--
commit d6d74f48a42727f6a3e8aba532064a1c4577b574
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
remoteproc: qcom_q6v5_pas: add per-PD proxy performance states for Hawi CDSP
The proxy power domain enable path requests INT_MAX performance state
for every proxy PD. This commit introduces a proxy_pd_performance_states
array in qcom_pas_data to allow per-PD RPMH levels to be declared
explicitly, and adds Hawi CDSP remoteproc support.
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 42bcbe72f95bd..6b23e711f03dc 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
[ ... ]
> @@ -912,6 +922,14 @@ static int qcom_pas_probe(struct platform_device *pdev)
> goto unassign_mem;
> pas->proxy_pd_count = ret;
>
> + if (WARN(desc->proxy_pd_performance_states &&
> + desc->num_proxy_pd_performance_states != pas->proxy_pd_count,
> + "proxy_pd_performance_states count %u != pd count %d\n",
> + desc->num_proxy_pd_performance_states, pas->proxy_pd_count)) {
> + ret = -EINVAL;
> + goto detach_proxy_pds;
> + }
> +
> ret = qcom_q6v5_init(&pas->q6v5, pdev, rproc, desc->crash_reason_smem,
> desc->load_state, qcom_pas_handover);
[Severity: Medium]
This isn't a bug introduced by this patch, but looking at the driver removal
path:
drivers/remoteproc/qcom_q6v5_pas.c:qcom_pas_pds_disable() {
...
pm_runtime_put(pds[i]);
...
}
This asynchronous put queues suspend work that will be cancelled by the
subsequent detach during driver removal:
drivers/remoteproc/qcom_q6v5_pas.c:qcom_pas_remove() {
...
rproc_del(...);
qcom_pas_pds_detach(pas, pas->proxy_pds, pas->proxy_pd_count);
...
}
This detaches and deletes the virtual devices, triggering pm_runtime_disable()
before the asynchronous suspend is guaranteed to complete.
Does this violate the PM guideline which mandates using pm_runtime_put_sync()
instead of pm_runtime_put() when device removal follows immediately?
While GenPD happens to gracefully handle the domain cleanup during detach by
decrementing the device count and queueing power-off work, relying on this
internal fallback appears to violate the API contract and leaves the device's
Runtime PM state machine in an incorrect intermediate state.
Can this be triggered locally by unbinding the driver?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909105049.1317985-1-mukesh.ojha@oss.qualcomm.com?part=3
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] dt-bindings: remoteproc: qcom,sm8550-pas: make qcom,hawi-cdsp-pas standalone
2026-09-09 10:50 ` [PATCH v3 2/3] dt-bindings: remoteproc: qcom,sm8550-pas: make qcom,hawi-cdsp-pas standalone Mukesh Ojha
@ 2026-09-10 4:26 ` Yijie Yang
2026-09-10 15:06 ` Mukesh Ojha
0 siblings, 1 reply; 10+ messages in thread
From: Yijie Yang @ 2026-09-10 4:26 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manivannan Sadhasivam
Cc: linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
Krzysztof Kozlowski
Maili faces the same issue, and I have verified that they share the same
fix. Please bundle the Maili binding changes into the next version of
this patch series.
On 9/9/2026 6:50 PM, Mukesh Ojha wrote:
> qcom,hawi-cdsp-pas was initially grouped as a fallback to
> qcom,sm8550-cdsp-pas on the assumption of hardware-level compatibility.
> Bringup revealed that the NSP proxy power domain on Hawi requires a
> specific RPMH performance level that the sm8550 driver data does not
> provide, breaking the compatibility contract the fallback string implies.
>
> Remove qcom,hawi-cdsp-pas from the sm8550-cdsp-pas fallback items block,
> add it to the standalone compatible enum, and extend the existing
> cx/mxc/nsp power-domain constraint to cover it explicitly.
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
> .../devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml
> index c58e8a6c7fe1..cf57095cd49d 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,sm8550-pas.yaml
> @@ -18,6 +18,7 @@ properties:
> oneOf:
> - enum:
> - qcom,eliza-cdsp-pas
> + - qcom,hawi-cdsp-pas
> - qcom,sdx75-mpss-pas
> - qcom,sm8550-adsp-pas
> - qcom,sm8550-cdsp-pas
> @@ -40,7 +41,6 @@ properties:
> - items:
> - enum:
> - qcom,glymur-cdsp-pas
> - - qcom,hawi-cdsp-pas
> - qcom,kaanapali-cdsp-pas
> - qcom,maili-cdsp-pas
> - const: qcom,sm8550-cdsp-pas
> @@ -273,6 +273,7 @@ allOf:
> compatible:
> contains:
> enum:
> + - qcom,hawi-cdsp-pas
> - qcom,sm8550-cdsp-pas
> - qcom,sm8650-cdsp-pas
> - qcom,x1e80100-cdsp-pas
--
Best Regards,
Yijie
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/3] remoteproc: qcom_q6v5_pas: add per-PD proxy performance states for Hawi CDSP
2026-09-09 10:50 ` [PATCH v3 3/3] remoteproc: qcom_q6v5_pas: add per-PD proxy performance states for Hawi CDSP Mukesh Ojha
2026-09-09 11:30 ` sashiko-bot
@ 2026-09-10 4:29 ` Yijie Yang
1 sibling, 0 replies; 10+ messages in thread
From: Yijie Yang @ 2026-09-10 4:29 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manivannan Sadhasivam
Cc: linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
Abel Vesa
Tested-by: Yijie Yang <yijie.yang@oss.qualcomm.com>
On 9/9/2026 6:50 PM, Mukesh Ojha wrote:
> The proxy power domain enable path requests INT_MAX performance
> state for every proxy PD. On Hawi, the NSP proxy power domain's
> RPMH power domain has no OPP at INT_MAX, while CX and MXC accept
> INT_MAX, mapping to their maximum supported level.
>
> Introduce a proxy_pd_performance_states array in qcom_pas_data
> to allow per-PD RPMH levels to be declared explicitly. Platforms
> that omit this field retain the existing INT_MAX behaviour.
>
> Add Hawi CDSP remoteproc support with the following proxy PD
> performance states:
>
> CX: RPMH_REGULATOR_LEVEL_TURBO
> MXC: RPMH_REGULATOR_LEVEL_TURBO
> NSP: RPMH_REGULATOR_LEVEL_NOM
>
> Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
> drivers/remoteproc/qcom_q6v5_pas.c | 50 ++++++++++++++++++++++++++++--
> 1 file changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index c163e66347f8..8ac66db80193 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -28,6 +28,7 @@
> #include <linux/soc/qcom/mdt_loader.h>
> #include <linux/soc/qcom/smem.h>
> #include <linux/soc/qcom/smem_state.h>
> +#include <dt-bindings/power/qcom,rpmhpd.h>
>
> #include "qcom_common.h"
> #include "qcom_pil_info.h"
> @@ -51,6 +52,8 @@ struct qcom_pas_data {
> bool decrypt_shutdown;
>
> char **proxy_pd_names;
> + const unsigned int *proxy_pd_performance_states;
> + unsigned int num_proxy_pd_performance_states;
>
> const char *load_state;
> const char *ssr_name;
> @@ -79,6 +82,7 @@ struct qcom_pas {
> struct regulator *px_supply;
>
> struct device *proxy_pds[3];
> + const unsigned int *proxy_pd_performance_states;
>
> int proxy_pd_count;
>
> @@ -167,11 +171,16 @@ static int qcom_pas_pds_enable(struct qcom_pas *pas, struct device **pds,
> int i;
>
> for (i = 0; i < pd_count; i++) {
> - ret = dev_pm_genpd_set_performance_state(pds[i], INT_MAX);
> + unsigned int state = INT_MAX;
> +
> + if (pas->proxy_pd_performance_states)
> + state = pas->proxy_pd_performance_states[i];
> +
> + ret = dev_pm_genpd_set_performance_state(pds[i], state);
> if (ret) {
> dev_err(pas->dev,
> "failed to set proxy PD %d state %u: %d\n",
> - i, INT_MAX, ret);
> + i, state, ret);
> goto unroll_pd_votes;
> }
> ret = pm_runtime_get_sync(pds[i]);
> @@ -879,6 +888,7 @@ static int qcom_pas_probe(struct platform_device *pdev)
> pas->info_name = desc->sysmon_name;
> pas->smem_host_id = desc->smem_host_id;
> pas->decrypt_shutdown = desc->decrypt_shutdown;
> + pas->proxy_pd_performance_states = desc->proxy_pd_performance_states;
> pas->region_assign_idx = desc->region_assign_idx;
> pas->region_assign_count = min_t(int, MAX_ASSIGN_COUNT, desc->region_assign_count);
> pas->region_assign_vmid = desc->region_assign_vmid;
> @@ -914,6 +924,14 @@ static int qcom_pas_probe(struct platform_device *pdev)
> goto unassign_mem;
> pas->proxy_pd_count = ret;
>
> + if (WARN(desc->proxy_pd_performance_states &&
> + desc->num_proxy_pd_performance_states != pas->proxy_pd_count,
> + "proxy_pd_performance_states count %u != pd count %d\n",
> + desc->num_proxy_pd_performance_states, pas->proxy_pd_count)) {
> + ret = -EINVAL;
> + goto detach_proxy_pds;
> + }
> +
> ret = qcom_q6v5_init(&pas->q6v5, pdev, rproc, desc->crash_reason_smem,
> desc->load_state, qcom_pas_handover);
> if (ret)
> @@ -1804,6 +1822,33 @@ static const struct qcom_pas_data glymur_soccp_resource = {
> .needs_tzmem = true,
> };
>
> +static const struct qcom_pas_data hawi_cdsp_resource = {
> + .crash_reason_smem = 601,
> + .firmware_name = "cdsp.mdt",
> + .dtb_firmware_name = "cdsp_dtb.mdt",
> + .pas_id = 18,
> + .dtb_pas_id = 0x25,
> + .minidump_id = 7,
> + .auto_boot = true,
> + .proxy_pd_names = (char*[]){
> + "cx",
> + "mxc",
> + "nsp",
> + NULL
> + },
> + .proxy_pd_performance_states = (const unsigned int[]){
> + RPMH_REGULATOR_LEVEL_TURBO,
> + RPMH_REGULATOR_LEVEL_TURBO,
> + RPMH_REGULATOR_LEVEL_NOM,
> + },
> + .num_proxy_pd_performance_states = 3,
> + .load_state = "cdsp",
> + .ssr_name = "cdsp",
> + .sysmon_name = "cdsp",
> + .ssctl_id = 0x17,
> + .smem_host_id = 5,
> +};
> +
> static const struct qcom_pas_data eliza_cdsp_resource = {
> .crash_reason_smem = 601,
> .firmware_name = "cdsp.mbn",
> @@ -1833,6 +1878,7 @@ static const struct of_device_id qcom_pas_of_match[] = {
> { .compatible = "qcom,eliza-adsp-pas", .data = &sm8550_adsp_resource },
> { .compatible = "qcom,eliza-cdsp-pas", .data = &eliza_cdsp_resource },
> { .compatible = "qcom,glymur-soccp-pas", .data = &glymur_soccp_resource },
> + { .compatible = "qcom,hawi-cdsp-pas", .data = &hawi_cdsp_resource },
> { .compatible = "qcom,kaanapali-soccp-pas", .data = &kaanapali_soccp_resource },
> { .compatible = "qcom,milos-adsp-pas", .data = &sm8550_adsp_resource },
> { .compatible = "qcom,milos-cdsp-pas", .data = &milos_cdsp_resource },
--
Best Regards,
Yijie
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] remoteproc: qcom_q6v5_pas: propagate dev_pm_genpd_set_performance_state() errors
2026-09-09 11:08 ` sashiko-bot
@ 2026-09-10 15:01 ` Mukesh Ojha
0 siblings, 0 replies; 10+ messages in thread
From: Mukesh Ojha @ 2026-09-10 15:01 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, conor+dt, devicetree
On Wed, Sep 09, 2026 at 11:08:39AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [High] The added error check for dev_pm_genpd_set_performance_state() is dead code and completely fails to fix the silent failure masking.
>
> Pre-existing issues:
> - [High] Pre-existing identical sequence flaw in sister remoteproc drivers silently masks performance state errors.
> --
>
> commit 79e5a0b96d1b1d35117390df652884f8c2c16c71
> Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
>
> remoteproc: qcom_q6v5_pas: propagate dev_pm_genpd_set_performance_state() errors
>
> This commit attempts to capture and handle errors from
> dev_pm_genpd_set_performance_state() in qcom_pas_pds_enable() to avoid
> silently masking performance state failures.
>
> > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> > index ca8e61254c449..42bcbe72f95bd 100644
> > --- a/drivers/remoteproc/qcom_q6v5_pas.c
> > +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> > @@ -167,7 +167,13 @@ static int qcom_pas_pds_enable(struct qcom_pas *pas, struct device **pds,
> > int i;
> >
> > for (i = 0; i < pd_count; i++) {
> > - dev_pm_genpd_set_performance_state(pds[i], INT_MAX);
> > + ret = dev_pm_genpd_set_performance_state(pds[i], INT_MAX);
> > + if (ret) {
> > + dev_err(pas->dev,
> > + "failed to set proxy PD %d state %u: %d\n",
> > + i, INT_MAX, ret);
> > + goto unroll_pd_votes;
> > + }
> > ret = pm_runtime_get_sync(pds[i]);
>
> [Severity: High]
> Is this newly added error check able to catch performance state errors?
>
> Because dev_pm_genpd_set_performance_state() is called before
> pm_runtime_get_sync(), the proxy PD device is still in the RPM_SUSPENDED
> state when this executes.
>
> When called on an RPM_SUSPENDED device, genpd_dev_pm_set_performance_state()
> merely caches the request in rpm_pstate and unconditionally returns 0.
>
> As a result, this check will never evaluate to true. The actual application
> of the state is deferred to pm_runtime_get_sync(), which calls
> genpd_runtime_resume() and genpd_restore_performance_state().
>
> Because genpd_restore_performance_state() returns void, it drops any errors
> from the underlying provider.
>
> Does this mean the failure remains silently masked despite this patch?
I should drop this patch as it seems not checking error value was
intentional.
>
> [Severity: High]
> This isn't a bug introduced by this patch, but this identical sequence flaw
> exists in several sister remoteproc drivers.
>
> In qcom_q6v5_mss.c:q6v5_pds_enable(),
> qcom_q6v5_adsp.c:qcom_rproc_pds_enable(), and qcom_wcnss.c:wcnss_start(),
> the drivers also call dev_pm_genpd_set_performance_state() while the proxy
> PDs are in the RPM_SUSPENDED state (prior to calling pm_runtime_get_sync()).
>
> Since they are suspended, the genpd framework defers execution to the resume
> path where errors are discarded, silently masking performance state failures.
>
> Should the initialization sequence be adjusted across all these drivers so
> the devices are resumed before setting the performance state?
No need to fix this; this seems intentional across remoteproc
driver.
>
> > if (ret < 0) {
> > pm_runtime_put_noidle(pds[i]);
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260909105049.1317985-1-mukesh.ojha@oss.qualcomm.com?part=1
--
-Mukesh Ojha
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] dt-bindings: remoteproc: qcom,sm8550-pas: make qcom,hawi-cdsp-pas standalone
2026-09-10 4:26 ` Yijie Yang
@ 2026-09-10 15:06 ` Mukesh Ojha
0 siblings, 0 replies; 10+ messages in thread
From: Mukesh Ojha @ 2026-09-10 15:06 UTC (permalink / raw)
To: Yijie Yang
Cc: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Manivannan Sadhasivam,
linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
Krzysztof Kozlowski
On Thu, Sep 10, 2026 at 12:26:21PM +0800, Yijie Yang wrote:
> Maili faces the same issue, and I have verified that they share the same
> fix. Please bundle the Maili binding changes into the next version of this
> patch series.
>
Sure.
-
-Mukesh Ojha
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-09-10 15:06 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-09 10:50 [PATCH v3 0/3] remoteproc: Hawi CDSP support with per-PD Mukesh Ojha
2026-09-09 10:50 ` [PATCH v3 1/3] remoteproc: qcom_q6v5_pas: propagate dev_pm_genpd_set_performance_state() errors Mukesh Ojha
2026-09-09 11:08 ` sashiko-bot
2026-09-10 15:01 ` Mukesh Ojha
2026-09-09 10:50 ` [PATCH v3 2/3] dt-bindings: remoteproc: qcom,sm8550-pas: make qcom,hawi-cdsp-pas standalone Mukesh Ojha
2026-09-10 4:26 ` Yijie Yang
2026-09-10 15:06 ` Mukesh Ojha
2026-09-09 10:50 ` [PATCH v3 3/3] remoteproc: qcom_q6v5_pas: add per-PD proxy performance states for Hawi CDSP Mukesh Ojha
2026-09-09 11:30 ` sashiko-bot
2026-09-10 4:29 ` Yijie Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox