[PATCH 1/3] remoteproc: qcom_q6v5_pas: Fix unbalanced enable for handover IRQ after attach

[PATCH 1/3] remoteproc: qcom_q6v5_pas: Fix unbalanced enable for handover IRQ after attach

[Mukesh Ojha]

qcom_pas_attach() calls enable_irq() directly for subsystems already
running at kernel probe time, but never sets handover_irq_enabled.
This leaves the IRQ physically enabled with the tracking flag false.

On the first stop+restart cycle, qcom_q6v5_unprepare() consults
handover_irq_enabled, finds it false, and skips the disable_irq_nosync()
call—leaving the IRQ enabled. When qcom_q6v5_prepare() then calls
enable_irq() on the already-enabled IRQ (depth=0), the kernel emits:

  WARNING: Unbalanced enable for IRQ 239

Fix by mirroring the enable_irq()/disable_irq() calls in qcom_pas_attach()
with matching handover_irq_enabled assignments, keeping the flag consistent
with actual hardware state.

Fixes: 16472c99f469 ("remoteproc: qcom: pas: Add late attach support for subsystems")
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index ca8e61254c44..25942200ba03 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -524,6 +524,7 @@ static int qcom_pas_attach(struct rproc *rproc)
 	int ret;
 
 	pas->q6v5.handover_issued = true;
+	pas->q6v5.handover_irq_enabled = true;
 	enable_irq(pas->q6v5.handover_irq);
 
 	pas->q6v5.running = true;
@@ -570,6 +571,7 @@ static int qcom_pas_attach(struct rproc *rproc)
 	pas->rproc->state = RPROC_OFFLINE;
 	ret = -EINVAL;
 disable_running:
+	pas->q6v5.handover_irq_enabled = false;
 	disable_irq(pas->q6v5.handover_irq);
 	pas->q6v5.running = false;
 
-- 
2.55.0

[PATCH 2/3] remoteproc: qcom_q6v5: Skip handover callback if already issued

[Mukesh Ojha]

When qcom_pas_attach() is called (subsystem already running at kernel
probe time), it sets handover_issued = true before enabling the handover
IRQ. The handover line may be high already (from bootloader boot), causing
the IRQ to fire immediately. Since proxy resources were never acquired via
qcom_pas_pds_enable() in the attach path, the resulting handover callback
calls pm_runtime_put() on proxy power domains with usage count = 0:

  genpd genpd:0:d00000.remoteproc: Runtime PM usage count underflow!
  genpd genpd:1:d00000.remoteproc: Runtime PM usage count underflow!

Also, because the old code left handover_irq_enabled = false after attach
(fixed separately), the handover IRQ was never disabled inside the handler,
allowing it to fire repeatedly and generating multiple underflows.

Fix by recording whether handover_issued was already set before the IRQ
fires and skipping the handover callback in that case. The callback
releases proxy resources that are only held when the normal start path
ran qcom_pas_pds_enable(); if handover was already marked as issued,
those resources were never acquired and must not be released.

Fixes: 16472c99f469 ("remoteproc: qcom: pas: Add late attach support for subsystems")
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
index fe148b4b3775..12ba0b80ad3e 100644
--- a/drivers/remoteproc/qcom_q6v5.c
+++ b/drivers/remoteproc/qcom_q6v5.c
@@ -198,12 +198,18 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_wait_for_start);
 static irqreturn_t q6v5_handover_interrupt(int irq, void *data)
 {
 	struct qcom_q6v5 *q6v5 = data;
+	bool previously_issued;
 
+	previously_issued = q6v5->handover_issued;
 	q6v5->handover_issued = true;
 
 	q6v5_handover_irq_disable(q6v5, false);
 
-	if (q6v5->handover)
+	/*
+	 * Skip the handover callback if it was already issued (e.g. attach
+	 * path), as the proxy resources were never acquired in that case.
+	 */
+	if (!previously_issued && q6v5->handover)
 		q6v5->handover(q6v5);
 
 	icc_set_bw(q6v5->path, 0, 0);
-- 
2.55.0

Re: [PATCH 2/3] remoteproc: qcom_q6v5: Skip handover callback if already issued

[Stephan Gerhold]

On Fri, Aug 28, 2026 at 11:43:10PM +0530, Mukesh Ojha wrote:
> When qcom_pas_attach() is called (subsystem already running at kernel
> probe time), it sets handover_issued = true before enabling the handover
> IRQ. The handover line may be high already (from bootloader boot), causing
> the IRQ to fire immediately. Since proxy resources were never acquired via
> qcom_pas_pds_enable() in the attach path, the resulting handover callback
> calls pm_runtime_put() on proxy power domains with usage count = 0:
> 
>   genpd genpd:0:d00000.remoteproc: Runtime PM usage count underflow!
>   genpd genpd:1:d00000.remoteproc: Runtime PM usage count underflow!
> 
> Also, because the old code left handover_irq_enabled = false after attach
> (fixed separately), the handover IRQ was never disabled inside the handler,
> allowing it to fire repeatedly and generating multiple underflows.
> 
> Fix by recording whether handover_issued was already set before the IRQ
> fires and skipping the handover callback in that case. The callback
> releases proxy resources that are only held when the normal start path
> ran qcom_pas_pds_enable(); if handover was already marked as issued,
> those resources were never acquired and must not be released.
> 
> Fixes: 16472c99f469 ("remoteproc: qcom: pas: Add late attach support for subsystems")
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

Shawn sent the same patch already and the same comment applies here:
https://lore.kernel.org/linux-arm-msm/amxf7MODINj26a4m@linaro.org/

v2 fixes it properly:
https://lore.kernel.org/linux-arm-msm/20260801011731.1084591-1-shengchao.guo@oss.qualcomm.com/

Thanks,
Stephan

Re: [PATCH 2/3] remoteproc: qcom_q6v5: Skip handover callback if already issued

[Bjorn Andersson]

On Mon, Aug 31, 2026 at 07:52:32AM +0200, Stephan Gerhold wrote:
> On Fri, Aug 28, 2026 at 11:43:10PM +0530, Mukesh Ojha wrote:
> > When qcom_pas_attach() is called (subsystem already running at kernel
> > probe time), it sets handover_issued = true before enabling the handover
> > IRQ. The handover line may be high already (from bootloader boot), causing
> > the IRQ to fire immediately. Since proxy resources were never acquired via
> > qcom_pas_pds_enable() in the attach path, the resulting handover callback
> > calls pm_runtime_put() on proxy power domains with usage count = 0:
> > 
> >   genpd genpd:0:d00000.remoteproc: Runtime PM usage count underflow!
> >   genpd genpd:1:d00000.remoteproc: Runtime PM usage count underflow!
> > 
> > Also, because the old code left handover_irq_enabled = false after attach
> > (fixed separately), the handover IRQ was never disabled inside the handler,
> > allowing it to fire repeatedly and generating multiple underflows.
> > 
> > Fix by recording whether handover_issued was already set before the IRQ
> > fires and skipping the handover callback in that case. The callback
> > releases proxy resources that are only held when the normal start path
> > ran qcom_pas_pds_enable(); if handover was already marked as issued,
> > those resources were never acquired and must not be released.
> > 
> > Fixes: 16472c99f469 ("remoteproc: qcom: pas: Add late attach support for subsystems")
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> 
> Shawn sent the same patch already and the same comment applies here:
> https://lore.kernel.org/linux-arm-msm/amxf7MODINj26a4m@linaro.org/
> 
> v2 fixes it properly:
> https://lore.kernel.org/linux-arm-msm/20260801011731.1084591-1-shengchao.guo@oss.qualcomm.com/
> 

Thanks for the pointer, Stephan. That looks better.

Regards,
Bjorn

> Thanks,
> Stephan

Re: [PATCH 2/3] remoteproc: qcom_q6v5: Skip handover callback if already issued

[Mukesh Ojha]

On Mon, Aug 31, 2026 at 07:52:32AM +0200, Stephan Gerhold wrote:
> On Fri, Aug 28, 2026 at 11:43:10PM +0530, Mukesh Ojha wrote:
> > When qcom_pas_attach() is called (subsystem already running at kernel
> > probe time), it sets handover_issued = true before enabling the handover
> > IRQ. The handover line may be high already (from bootloader boot), causing
> > the IRQ to fire immediately. Since proxy resources were never acquired via
> > qcom_pas_pds_enable() in the attach path, the resulting handover callback
> > calls pm_runtime_put() on proxy power domains with usage count = 0:
> > 
> >   genpd genpd:0:d00000.remoteproc: Runtime PM usage count underflow!
> >   genpd genpd:1:d00000.remoteproc: Runtime PM usage count underflow!
> > 
> > Also, because the old code left handover_irq_enabled = false after attach
> > (fixed separately), the handover IRQ was never disabled inside the handler,
> > allowing it to fire repeatedly and generating multiple underflows.
> > 
> > Fix by recording whether handover_issued was already set before the IRQ
> > fires and skipping the handover callback in that case. The callback
> > releases proxy resources that are only held when the normal start path
> > ran qcom_pas_pds_enable(); if handover was already marked as issued,
> > those resources were never acquired and must not be released.
> > 
> > Fixes: 16472c99f469 ("remoteproc: qcom: pas: Add late attach support for subsystems")
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> 
> Shawn sent the same patch already and the same comment applies here:
> https://lore.kernel.org/linux-arm-msm/amxf7MODINj26a4m@linaro.org/
> 
> v2 fixes it properly:
> https://lore.kernel.org/linux-arm-msm/20260801011731.1084591-1-shengchao.guo@oss.qualcomm.com/

Sure, thanks..


> Thanks,
> Stephan

-- 
-Mukesh Ojha

[PATCH 3/3] remoteproc: qcom_q6v5_pas: Add per-PD proxy performance states for Hawi CDSP

[Mukesh Ojha]

The proxy power domain enable path currently requests INT_MAX performance
state for every proxy PD.  While this serves as a "take highest available"
hint, some SoCs require specific per-domain RPMH levels for correct
operation during firmware load rather than a blanket maximum.

Introduce a proxy_pd_performance_states array in qcom_pas_data to allow
each proxy PD to declare its required RPMH performance level explicitly.
Platforms that do not populate this field retain the existing INT_MAX
behaviour.

Also propagate the return value of dev_pm_genpd_set_performance_state()
and emit a warning on failure rather than silently ignoring it.

Add Hawi CDSP remoteproc support using this infrastructure with the
following proxy PD performance states:

  CX:  RPMH_REGULATOR_LEVEL_TURBO
  MXC: RPMH_REGULATOR_LEVEL_TURBO
  NSP: RPMH_REGULATOR_LEVEL_NOM

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 43 +++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 25942200ba03..c270c81e8bb4 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,7 @@ struct qcom_pas_data {
 	bool decrypt_shutdown;
 
 	char **proxy_pd_names;
+	const unsigned int *proxy_pd_performance_states;
 
 	const char *load_state;
 	const char *ssr_name;
@@ -79,6 +81,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,7 +170,17 @@ 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);
+		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_warn(pas->dev,
+				 "failed to set proxy PD %d state %u: %d\n",
+				 i, state, ret);
+
 		ret = pm_runtime_get_sync(pds[i]);
 		if (ret < 0) {
 			pm_runtime_put_noidle(pds[i]);
@@ -873,6 +886,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;
@@ -1798,6 +1812,32 @@ 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,
+	},
+	.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",
@@ -1827,6 +1867,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

Re: [PATCH 3/3] remoteproc: qcom_q6v5_pas: Add per-PD proxy performance states for Hawi CDSP

[Konrad Dybcio]

On 8/28/26 8:13 PM, Mukesh Ojha wrote:
> The proxy power domain enable path currently requests INT_MAX performance
> state for every proxy PD.  While this serves as a "take highest available"
> hint, some SoCs require specific per-domain RPMH levels for correct
> operation during firmware load rather than a blanket maximum.

I would have assumed having a surplus of power shouldn't be an issue..

Is this specific to Hawi? Is that a firmware bug essentially?

Konrad

Re: [PATCH 3/3] remoteproc: qcom_q6v5_pas: Add per-PD proxy performance states for Hawi CDSP

[Mukesh Ojha]

On Mon, Aug 31, 2026 at 09:08:15AM +0200, Konrad Dybcio wrote:
> On 8/28/26 8:13 PM, Mukesh Ojha wrote:
> > The proxy power domain enable path currently requests INT_MAX performance
> > state for every proxy PD.  While this serves as a "take highest available"
> > hint, some SoCs require specific per-domain RPMH levels for correct
> > operation during firmware load rather than a blanket maximum.
> 
> I would have assumed having a surplus of power shouldn't be an issue..
> 
> Is this specific to Hawi?

Yes, its for both Hawi and Maili as far as I know..

>Is that a firmware bug essentially?

It does not looks to be but not 100% sure and its found recently
on some devices.

-- 
-Mukesh Ojha

Re: [PATCH 3/3] remoteproc: qcom_q6v5_pas: Add per-PD proxy performance states for Hawi CDSP

[Bjorn Andersson]

On Fri, Aug 28, 2026 at 11:43:11PM +0530, Mukesh Ojha wrote:
> The proxy power domain enable path currently requests INT_MAX performance
> state for every proxy PD.  While this serves as a "take highest available"
> hint, some SoCs require specific per-domain RPMH levels for correct
> operation during firmware load rather than a blanket maximum.
> 
> Introduce a proxy_pd_performance_states array in qcom_pas_data to allow
> each proxy PD to declare its required RPMH performance level explicitly.
> Platforms that do not populate this field retain the existing INT_MAX
> behaviour.
> 

Is it possible to encode this using an optional opp-table instead of
filling the driver with such details? (This is a question, not a direct
suggestion)

> Also propagate the return value of dev_pm_genpd_set_performance_state()
> and emit a warning on failure rather than silently ignoring it.

Also remember that whenever you start a paragraph in a commit message
with the word "also"; it's probably a good sign that it would be better
to have a separate commit.

> 
> Add Hawi CDSP remoteproc support using this infrastructure with the
> following proxy PD performance states:

That is quite weird, because you already stated that we added Hawi CDSP
support in
https://lore.kernel.org/r/20260427190614.3679937-2-mukesh.ojha@oss.qualcomm.com


Note that the line:

  compatible = "qcom,hawi-cdsp-pas", "qcom,sm8550-cdsp-pas";

is supposed to tell an OS that "if you have an implementation for
qcom,hawi-cdsp-pas use that, if not you can use the implementation for
qcom,sm8550-cdsp-pas".

This patch tells me that you need to also fix the binding to not say
that - because hawi-cdsp is no longer compatible with sm8550-cdsp.

I'm guessing that this issue might have been a late discovery, state
that in your commit message changing the binding.

> 
>   CX:  RPMH_REGULATOR_LEVEL_TURBO
>   MXC: RPMH_REGULATOR_LEVEL_TURBO
>   NSP: RPMH_REGULATOR_LEVEL_NOM

I'm guessing that what you describe above about INT_MAX being a problem
is only for NSP? Would be nice to not having to guess though.

Regards,
Bjorn

> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  drivers/remoteproc/qcom_q6v5_pas.c | 43 +++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 25942200ba03..c270c81e8bb4 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,7 @@ struct qcom_pas_data {
>  	bool decrypt_shutdown;
>  
>  	char **proxy_pd_names;
> +	const unsigned int *proxy_pd_performance_states;
>  
>  	const char *load_state;
>  	const char *ssr_name;
> @@ -79,6 +81,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,7 +170,17 @@ 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);
> +		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_warn(pas->dev,
> +				 "failed to set proxy PD %d state %u: %d\n",
> +				 i, state, ret);
> +
>  		ret = pm_runtime_get_sync(pds[i]);
>  		if (ret < 0) {
>  			pm_runtime_put_noidle(pds[i]);
> @@ -873,6 +886,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;
> @@ -1798,6 +1812,32 @@ 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,
> +	},
> +	.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",
> @@ -1827,6 +1867,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
> 

Re: [PATCH 3/3] remoteproc: qcom_q6v5_pas: Add per-PD proxy performance states for Hawi CDSP

[Mukesh Ojha]

On Mon, Aug 31, 2026 at 10:56:55AM -0500, Bjorn Andersson wrote:
> On Fri, Aug 28, 2026 at 11:43:11PM +0530, Mukesh Ojha wrote:
> > The proxy power domain enable path currently requests INT_MAX performance
> > state for every proxy PD.  While this serves as a "take highest available"
> > hint, some SoCs require specific per-domain RPMH levels for correct
> > operation during firmware load rather than a blanket maximum.
> > 
> > Introduce a proxy_pd_performance_states array in qcom_pas_data to allow
> > each proxy PD to declare its required RPMH performance level explicitly.
> > Platforms that do not populate this field retain the existing INT_MAX
> > behaviour.
> > 
> 
> Is it possible to encode this using an optional opp-table instead of
> filling the driver with such details? (This is a question, not a direct
> suggestion)

Technically yes, via required-opps, each proxy PD would need its own
opp-table and the remoteproc node would reference specific OPP entries via
phandles. That is heavyweight machinery for three scalar values used only
during the transient firmware-load window. But let me know if you see the
benefit.

> 
> > Also propagate the return value of dev_pm_genpd_set_performance_state()
> > and emit a warning on failure rather than silently ignoring it.
> 
> Also remember that whenever you start a paragraph in a commit message
> with the word "also"; it's probably a good sign that it would be better
> to have a separate commit.

Agreed. The dev_pm_genpd_set_performance_state() error propagation fix should
be a separate commit.
> 
> > 
> > Add Hawi CDSP remoteproc support using this infrastructure with the
> > following proxy PD performance states:
> 
> That is quite weird, because you already stated that we added Hawi CDSP
> support in
> https://lore.kernel.org/r/20260427190614.3679937-2-mukesh.ojha@oss.qualcomm.com
> 
> 
> Note that the line:
> 
>   compatible = "qcom,hawi-cdsp-pas", "qcom,sm8550-cdsp-pas";
> 
> is supposed to tell an OS that "if you have an implementation for
> qcom,hawi-cdsp-pas use that, if not you can use the implementation for
> qcom,sm8550-cdsp-pas".

Yes, the April series added qcom,hawi-cdsp-pas in DT as a fallback to
qcom,sm8550-cdsp-pas. The NSP proxy PD requirement was discovered recently
and I realize you are right. I initially thought nothing was changing
between the HW description but the performance states, but I see from the
point of view of someone who has the latest firmware or DT and is still
running an older OS, it can result in a crash, which is broken.

> 
> This patch tells me that you need to also fix the binding to not say
> that - because hawi-cdsp is no longer compatible with sm8550-cdsp.
> 
> I'm guessing that this issue might have been a late discovery, state
> that in your commit message changing the binding.

I will add a binding patch to v2 that removes qcom,hawi-cdsp-pas from the
sm8550-cdsp-pas fallback list and makes it a standalone compatible, with a
commit message that explains this was a late discovery. The DTS will be
updated to drop the qcom,sm8550-cdsp-pas fallback string.

> 
> > 
> >   CX:  RPMH_REGULATOR_LEVEL_TURBO
> >   MXC: RPMH_REGULATOR_LEVEL_TURBO
> >   NSP: RPMH_REGULATOR_LEVEL_NOM
> 
> I'm guessing that what you describe above about INT_MAX being a problem
> is only for NSP? Would be nice to not having to guess though.

Yes, for NSP only. Will be explicit here.

-Mukesh

Re: [PATCH 3/3] remoteproc: qcom_q6v5_pas: Add per-PD proxy performance states for Hawi CDSP

[Bjorn Andersson]

On Tue, Sep 01, 2026 at 12:46:21AM +0530, Mukesh Ojha wrote:
> On Mon, Aug 31, 2026 at 10:56:55AM -0500, Bjorn Andersson wrote:
> > On Fri, Aug 28, 2026 at 11:43:11PM +0530, Mukesh Ojha wrote:
> > > The proxy power domain enable path currently requests INT_MAX performance
> > > state for every proxy PD.  While this serves as a "take highest available"
> > > hint, some SoCs require specific per-domain RPMH levels for correct
> > > operation during firmware load rather than a blanket maximum.
> > > 
> > > Introduce a proxy_pd_performance_states array in qcom_pas_data to allow
> > > each proxy PD to declare its required RPMH performance level explicitly.
> > > Platforms that do not populate this field retain the existing INT_MAX
> > > behaviour.
> > > 
> > 
> > Is it possible to encode this using an optional opp-table instead of
> > filling the driver with such details? (This is a question, not a direct
> > suggestion)
> 
> Technically yes, via required-opps, each proxy PD would need its own
> opp-table and the remoteproc node would reference specific OPP entries via
> phandles. That is heavyweight machinery for three scalar values used only
> during the transient firmware-load window. But let me know if you see the
> benefit.
> 

I don't like sprinkling these details throughout drivers, but if that's
the alternative I guess your proposed solution is the lesser of two
evils.

Thank you,
Bjorn