[PATCH 0/2] remoteproc: qcom: q6v5: Fix handover IRQ regressions on attach

[PATCH 0/2] remoteproc: qcom: q6v5: Fix handover IRQ regressions on attach

[Shawn Guo]

This series fixes a couple of regressions introduced by commit
bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ one-shot"),
observed on Nord SoC where the ADSP is booted by the bootloader and the
remoteproc driver attaches to it.

Shawn Guo (2):
  remoteproc: qcom: q6v5_pas: Fix unbalanced handover IRQ enable on
    attach
  remoteproc: qcom: q6v5: Ignore stale handover IRQ delivered on attach

 drivers/remoteproc/qcom_q6v5.c     | 36 +++++++++++++++++++++++++-----
 drivers/remoteproc/qcom_q6v5.h     |  2 ++
 drivers/remoteproc/qcom_q6v5_pas.c |  4 ++--
 3 files changed, 34 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH 1/2] remoteproc: qcom: q6v5_pas: Fix unbalanced handover IRQ enable on attach

[Shawn Guo]

commit bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ
one-shot") introduced tracked, idempotent enable/disable helpers for
the handover IRQ, gated on a new q6v5->handover_irq_enabled flag. It
converted qcom_q6v5_prepare()/unprepare() and the handover ISR to use
these helpers, but missed qcom_pas_attach(), which still toggles the
IRQ directly via enable_irq()/disable_irq() without updating the
tracked state.

This desyncs the real IRQ enable depth from handover_irq_enabled for
any remoteproc that boots up already attached (e.g. ADSP started by
the bootloader). qcom_pas_attach()'s raw enable_irq() leaves the IRQ
enabled while handover_irq_enabled stays false, so a subsequent
stop's qcom_q6v5_unprepare() sees the flag as false and skips
disabling the IRQ. The next start's qcom_q6v5_prepare() then calls
enable_irq() on an IRQ that was never disabled, producing:

  WARNING: Unbalanced enable for IRQ ...

Additionally, any handover interrupt latched by hardware before probe
and delivered once qcom_pas_attach() unmasks it is no longer
suppressed (the ISR's early-return-if-already-issued guard was
replaced by irq disable-after-first-fire), so it runs the handover
callback and disables proxy power-domains/clocks that attach() never
enabled, causing genpd runtime PM usage-count underflow warnings.

Route qcom_pas_attach()'s IRQ handling through the same
qcom_q6v5_handover_irq_enable()/qcom_q6v5_handover_irq_disable()
helpers (now exported) used elsewhere, keeping handover_irq_enabled
in sync with the real IRQ state.

Fixes: bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ one-shot")
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5.c     | 21 ++++++++++++++++-----
 drivers/remoteproc/qcom_q6v5.h     |  2 ++
 drivers/remoteproc/qcom_q6v5_pas.c |  4 ++--
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
index fe148b4b3775..10fa38f264c8 100644
--- a/drivers/remoteproc/qcom_q6v5.c
+++ b/drivers/remoteproc/qcom_q6v5.c
@@ -36,7 +36,11 @@ static int q6v5_load_state_toggle(struct qcom_q6v5 *q6v5, bool enable)
 	return ret;
 }
 
-static void q6v5_handover_irq_enable(struct qcom_q6v5 *q6v5)
+/**
+ * qcom_q6v5_handover_irq_enable() - idempotently enable the handover IRQ
+ * @q6v5:	reference to qcom_q6v5 context
+ */
+void qcom_q6v5_handover_irq_enable(struct qcom_q6v5 *q6v5)
 {
 	unsigned long flags;
 	bool enable = false;
@@ -51,8 +55,14 @@ static void q6v5_handover_irq_enable(struct qcom_q6v5 *q6v5)
 	if (enable)
 		enable_irq(q6v5->handover_irq);
 }
+EXPORT_SYMBOL_GPL(qcom_q6v5_handover_irq_enable);
 
-static void q6v5_handover_irq_disable(struct qcom_q6v5 *q6v5, bool sync)
+/**
+ * qcom_q6v5_handover_irq_disable() - idempotently disable the handover IRQ
+ * @q6v5:	reference to qcom_q6v5 context
+ * @sync:	whether to synchronize against an in-flight handler
+ */
+void qcom_q6v5_handover_irq_disable(struct qcom_q6v5 *q6v5, bool sync)
 {
 	unsigned long flags;
 	bool disable = false;
@@ -69,6 +79,7 @@ static void q6v5_handover_irq_disable(struct qcom_q6v5 *q6v5, bool sync)
 	if (sync)
 		synchronize_irq(q6v5->handover_irq);
 }
+EXPORT_SYMBOL_GPL(qcom_q6v5_handover_irq_disable);
 
 /**
  * qcom_q6v5_prepare() - reinitialize the qcom_q6v5 context before start
@@ -98,7 +109,7 @@ int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5)
 	q6v5->running = true;
 	q6v5->handover_issued = false;
 
-	q6v5_handover_irq_enable(q6v5);
+	qcom_q6v5_handover_irq_enable(q6v5);
 
 	return 0;
 }
@@ -112,7 +123,7 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_prepare);
  */
 int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5)
 {
-	q6v5_handover_irq_disable(q6v5, true);
+	qcom_q6v5_handover_irq_disable(q6v5, true);
 
 	q6v5_load_state_toggle(q6v5, false);
 
@@ -201,7 +212,7 @@ static irqreturn_t q6v5_handover_interrupt(int irq, void *data)
 
 	q6v5->handover_issued = true;
 
-	q6v5_handover_irq_disable(q6v5, false);
+	qcom_q6v5_handover_irq_disable(q6v5, false);
 
 	if (q6v5->handover)
 		q6v5->handover(q6v5);
diff --git a/drivers/remoteproc/qcom_q6v5.h b/drivers/remoteproc/qcom_q6v5.h
index 8991ff090579..24263cb514ab 100644
--- a/drivers/remoteproc/qcom_q6v5.h
+++ b/drivers/remoteproc/qcom_q6v5.h
@@ -53,6 +53,8 @@ void qcom_q6v5_deinit(struct qcom_q6v5 *q6v5);
 
 int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5);
 int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5);
+void qcom_q6v5_handover_irq_enable(struct qcom_q6v5 *q6v5);
+void qcom_q6v5_handover_irq_disable(struct qcom_q6v5 *q6v5, bool sync);
 int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5, struct qcom_sysmon *sysmon);
 int qcom_q6v5_wait_for_start(struct qcom_q6v5 *q6v5, int timeout);
 unsigned long qcom_q6v5_panic(struct qcom_q6v5 *q6v5);
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index a4b233d92efb..0f1ffd4b6182 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -521,7 +521,7 @@ static int qcom_pas_attach(struct rproc *rproc)
 	int ret;
 
 	pas->q6v5.handover_issued = true;
-	enable_irq(pas->q6v5.handover_irq);
+	qcom_q6v5_handover_irq_enable(&pas->q6v5);
 
 	pas->q6v5.running = true;
 	ret = irq_get_irqchip_state(pas->q6v5.fatal_irq,
@@ -567,7 +567,7 @@ static int qcom_pas_attach(struct rproc *rproc)
 	pas->rproc->state = RPROC_OFFLINE;
 	ret = -EINVAL;
 disable_running:
-	disable_irq(pas->q6v5.handover_irq);
+	qcom_q6v5_handover_irq_disable(&pas->q6v5, true);
 	pas->q6v5.running = false;
 
 	return ret;
-- 
2.43.0

Re: [PATCH 1/2] remoteproc: qcom: q6v5_pas: Fix unbalanced handover IRQ enable on attach

[Abel Vesa]

On 26-07-31 10:56:53, Shawn Guo wrote:
> commit bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ
> one-shot") introduced tracked, idempotent enable/disable helpers for
> the handover IRQ, gated on a new q6v5->handover_irq_enabled flag. It
> converted qcom_q6v5_prepare()/unprepare() and the handover ISR to use
> these helpers, but missed qcom_pas_attach(), which still toggles the
> IRQ directly via enable_irq()/disable_irq() without updating the
> tracked state.
> 
> This desyncs the real IRQ enable depth from handover_irq_enabled for
> any remoteproc that boots up already attached (e.g. ADSP started by
> the bootloader). qcom_pas_attach()'s raw enable_irq() leaves the IRQ
> enabled while handover_irq_enabled stays false, so a subsequent
> stop's qcom_q6v5_unprepare() sees the flag as false and skips
> disabling the IRQ. The next start's qcom_q6v5_prepare() then calls
> enable_irq() on an IRQ that was never disabled, producing:
> 
>   WARNING: Unbalanced enable for IRQ ...
> 
> Additionally, any handover interrupt latched by hardware before probe
> and delivered once qcom_pas_attach() unmasks it is no longer
> suppressed (the ISR's early-return-if-already-issued guard was
> replaced by irq disable-after-first-fire), so it runs the handover
> callback and disables proxy power-domains/clocks that attach() never
> enabled, causing genpd runtime PM usage-count underflow warnings.
> 
> Route qcom_pas_attach()'s IRQ handling through the same
> qcom_q6v5_handover_irq_enable()/qcom_q6v5_handover_irq_disable()
> helpers (now exported) used elsewhere, keeping handover_irq_enabled
> in sync with the real IRQ state.
> 
> Fixes: bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ one-shot")
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>

Thanks for fixing this. I should've caught this on Glymur.

The fix looks sane to me, so:

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>

[PATCH 2/2] remoteproc: qcom: q6v5: Ignore stale handover IRQ delivered on attach

[Shawn Guo]

Commit bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ one-shot")
dropped the check that ignored a handover interrupt after handover
had already been issued, relying instead on disabling the IRQ after
its first delivery to make it one-shot.

That is not sufficient for qcom_pas_attach(): when attaching to a
remote processor that was already booted by the bootloader, handover
has already happened out-of-band, before this driver ever probed.
qcom_pas_attach() sets handover_issued = true and unmasks the
handover IRQ to keep the enable/disable tracking consistent, but the
transition that signals handover is latched at the interrupt
controller while masked, so unmasking still delivers that one IRQ.

Since this driver instance never ran qcom_pas_start() for that boot,
it never took the proxy power-domain/clock/regulator votes that
q6v5->handover() releases. Running the handover callback for this
stale, already-accounted-for signal disables those votes without a
matching enable, producing:

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

Restore the early-return when handover_issued is already set, so a
stale IRQ delivered on attach is dropped after being disabled instead
of re-running the handover callback.

Fixes: bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ one-shot")
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
index 10fa38f264c8..e715083846aa 100644
--- a/drivers/remoteproc/qcom_q6v5.c
+++ b/drivers/remoteproc/qcom_q6v5.c
@@ -210,10 +210,23 @@ static irqreturn_t q6v5_handover_interrupt(int irq, void *data)
 {
 	struct qcom_q6v5 *q6v5 = data;
 
-	q6v5->handover_issued = true;
-
 	qcom_q6v5_handover_irq_disable(q6v5, false);
 
+	/*
+	 * When attaching to an already-running remote processor,
+	 * handover_issued is set before the IRQ is unmasked, since the
+	 * handover already happened out-of-band, before this driver probed.
+	 * The transition that signals it is latched at the interrupt
+	 * controller while masked, so unmasking still delivers this one
+	 * IRQ. Ignore it: this driver instance never enabled the resources
+	 * (proxy PDs, clocks, etc.) that the handover callback would tear
+	 * down.
+	 */
+	if (q6v5->handover_issued)
+		return IRQ_HANDLED;
+
+	q6v5->handover_issued = true;
+
 	if (q6v5->handover)
 		q6v5->handover(q6v5);
 
-- 
2.43.0

Re: [PATCH 2/2] remoteproc: qcom: q6v5: Ignore stale handover IRQ delivered on attach

[Abel Vesa]

On 26-07-31 10:56:54, Shawn Guo wrote:
> Commit bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ one-shot")
> dropped the check that ignored a handover interrupt after handover
> had already been issued, relying instead on disabling the IRQ after
> its first delivery to make it one-shot.
> 
> That is not sufficient for qcom_pas_attach(): when attaching to a
> remote processor that was already booted by the bootloader, handover
> has already happened out-of-band, before this driver ever probed.
> qcom_pas_attach() sets handover_issued = true and unmasks the
> handover IRQ to keep the enable/disable tracking consistent, but the
> transition that signals handover is latched at the interrupt
> controller while masked, so unmasking still delivers that one IRQ.
> 
> Since this driver instance never ran qcom_pas_start() for that boot,
> it never took the proxy power-domain/clock/regulator votes that
> q6v5->handover() releases. Running the handover callback for this
> stale, already-accounted-for signal disables those votes without a
> matching enable, producing:
> 
>   genpd genpd:0:4c00000.remoteproc: Runtime PM usage count underflow!
>   genpd genpd:1:4c00000.remoteproc: Runtime PM usage count underflow!
> 
> Restore the early-return when handover_issued is already set, so a
> stale IRQ delivered on attach is dropped after being disabled instead
> of re-running the handover callback.
> 
> Fixes: bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ one-shot")
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>

Re: [PATCH 2/2] remoteproc: qcom: q6v5: Ignore stale handover IRQ delivered on attach

[Stephan Gerhold]

On Fri, Jul 31, 2026 at 10:56:54AM +0800, Shawn Guo wrote:
> Commit bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ one-shot")
> dropped the check that ignored a handover interrupt after handover
> had already been issued, relying instead on disabling the IRQ after
> its first delivery to make it one-shot.
> 
> That is not sufficient for qcom_pas_attach(): when attaching to a
> remote processor that was already booted by the bootloader, handover
> has already happened out-of-band, before this driver ever probed.
> qcom_pas_attach() sets handover_issued = true and unmasks the
> handover IRQ to keep the enable/disable tracking consistent, but the
> transition that signals handover is latched at the interrupt
> controller while masked, so unmasking still delivers that one IRQ.
> 
> Since this driver instance never ran qcom_pas_start() for that boot,
> it never took the proxy power-domain/clock/regulator votes that
> q6v5->handover() releases. Running the handover callback for this
> stale, already-accounted-for signal disables those votes without a
> matching enable, producing:
> 
>   genpd genpd:0:4c00000.remoteproc: Runtime PM usage count underflow!
>   genpd genpd:1:4c00000.remoteproc: Runtime PM usage count underflow!
> 
> Restore the early-return when handover_issued is already set, so a
> stale IRQ delivered on attach is dropped after being disabled instead
> of re-running the handover callback.
> 
> Fixes: bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ one-shot")
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> ---
>  drivers/remoteproc/qcom_q6v5.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
> index 10fa38f264c8..e715083846aa 100644
> --- a/drivers/remoteproc/qcom_q6v5.c
> +++ b/drivers/remoteproc/qcom_q6v5.c
> @@ -210,10 +210,23 @@ static irqreturn_t q6v5_handover_interrupt(int irq, void *data)
>  {
>  	struct qcom_q6v5 *q6v5 = data;
>  
> -	q6v5->handover_issued = true;
> -
>  	qcom_q6v5_handover_irq_disable(q6v5, false);
>  
> +	/*
> +	 * When attaching to an already-running remote processor,
> +	 * handover_issued is set before the IRQ is unmasked, since the
> +	 * handover already happened out-of-band, before this driver probed.
> +	 * The transition that signals it is latched at the interrupt
> +	 * controller while masked, so unmasking still delivers this one
> +	 * IRQ. Ignore it: this driver instance never enabled the resources
> +	 * (proxy PDs, clocks, etc.) that the handover callback would tear
> +	 * down.
> +	 */
> +	if (q6v5->handover_issued)
> +		return IRQ_HANDLED;
> +

The goal of Abel's patch was to have the handover interrupt unmasked
only when needed. If you just bail out here and do nothing then there
was no need to unmask it in the first place. Can you just drop enabling
the handover IRQ in qcom_pas_attach()?

Thanks,
Stephan

Re: [PATCH 2/2] remoteproc: qcom: q6v5: Ignore stale handover IRQ delivered on attach

[Shawn Guo]

On Fri, Jul 31, 2026 at 10:42:20AM +0200, Stephan Gerhold wrote:
> On Fri, Jul 31, 2026 at 10:56:54AM +0800, Shawn Guo wrote:
> > Commit bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ one-shot")
> > dropped the check that ignored a handover interrupt after handover
> > had already been issued, relying instead on disabling the IRQ after
> > its first delivery to make it one-shot.
> > 
> > That is not sufficient for qcom_pas_attach(): when attaching to a
> > remote processor that was already booted by the bootloader, handover
> > has already happened out-of-band, before this driver ever probed.
> > qcom_pas_attach() sets handover_issued = true and unmasks the
> > handover IRQ to keep the enable/disable tracking consistent, but the
> > transition that signals handover is latched at the interrupt
> > controller while masked, so unmasking still delivers that one IRQ.
> > 
> > Since this driver instance never ran qcom_pas_start() for that boot,
> > it never took the proxy power-domain/clock/regulator votes that
> > q6v5->handover() releases. Running the handover callback for this
> > stale, already-accounted-for signal disables those votes without a
> > matching enable, producing:
> > 
> >   genpd genpd:0:4c00000.remoteproc: Runtime PM usage count underflow!
> >   genpd genpd:1:4c00000.remoteproc: Runtime PM usage count underflow!
> > 
> > Restore the early-return when handover_issued is already set, so a
> > stale IRQ delivered on attach is dropped after being disabled instead
> > of re-running the handover callback.
> > 
> > Fixes: bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ one-shot")
> > Assisted-by: Claude:claude-sonnet-5
> > Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> > ---
> >  drivers/remoteproc/qcom_q6v5.c | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
> > index 10fa38f264c8..e715083846aa 100644
> > --- a/drivers/remoteproc/qcom_q6v5.c
> > +++ b/drivers/remoteproc/qcom_q6v5.c
> > @@ -210,10 +210,23 @@ static irqreturn_t q6v5_handover_interrupt(int irq, void *data)
> >  {
> >  	struct qcom_q6v5 *q6v5 = data;
> >  
> > -	q6v5->handover_issued = true;
> > -
> >  	qcom_q6v5_handover_irq_disable(q6v5, false);
> >  
> > +	/*
> > +	 * When attaching to an already-running remote processor,
> > +	 * handover_issued is set before the IRQ is unmasked, since the
> > +	 * handover already happened out-of-band, before this driver probed.
> > +	 * The transition that signals it is latched at the interrupt
> > +	 * controller while masked, so unmasking still delivers this one
> > +	 * IRQ. Ignore it: this driver instance never enabled the resources
> > +	 * (proxy PDs, clocks, etc.) that the handover callback would tear
> > +	 * down.
> > +	 */
> > +	if (q6v5->handover_issued)
> > +		return IRQ_HANDLED;
> > +
> 
> The goal of Abel's patch was to have the handover interrupt unmasked
> only when needed. If you just bail out here and do nothing then there
> was no need to unmask it in the first place. Can you just drop enabling
> the handover IRQ in qcom_pas_attach()?

Hi Stephen,

Thanks for the comment!

Although it seems working in my limited testing, I'm not sure it's
entirely safe to drop the handover IRQ enable in qcom_pas_attach().

The handover IRQ is requested with IRQF_TRIGGER_RISING. When we attach
to a remote processor the bootloader already booted, that rising edge
happened while the IRQ was masked. Masking doesn't prevent the edge
from being latched at the interrupt controller -- it just defers
delivery. The pending bit sits there until the IRQ is unmasked and
actually taken; unmasking alone doesn't clear it.

If qcom_pas_attach() never unmasks the IRQ, that stale pending edge
doesn't go away -- it just waits for the next unmask, which is the
next real qcom_q6v5_prepare() (i.e. the next stop/start cycle). But by
then handover_issued has already been reset to false for that new
boot (qcom_q6v5_prepare() does this before re-enabling the IRQ), so
the stale edge would be indistinguishable from a genuine handover for
the new cycle. It could run q6v5->handover() and tear down proxy
PDs/clocks/interconnect immediately on prepare, before the firmware
for that cycle has even signaled ready.

So the enable_irq() in attach() isn't dead weight -- it's there to
deliberately unmask and flush that stale latched edge right away,
while handover_issued is still true from attach(), so the IRQ fires
once, gets caught by the one-shot ISR, is dropped harmlessly (because
handover_issued is already set), and the IRQ ends up masked again.
Removing the enable would just defer the spurious handover to a later
and more damaging point instead of eliminating it.

If you agree with the theory above, I can follow up with a comment
update in q6v5_handover_interrupt() to make this rationale explicit,
so it's clear that the attach-time enable exists to flush the latched
edge.

Shawn

Re: [PATCH 2/2] remoteproc: qcom: q6v5: Ignore stale handover IRQ delivered on attach

[Stephan Gerhold]

On Fri, Jul 31, 2026 at 09:02:20PM +0800, Shawn Guo wrote:
> On Fri, Jul 31, 2026 at 10:42:20AM +0200, Stephan Gerhold wrote:
> > On Fri, Jul 31, 2026 at 10:56:54AM +0800, Shawn Guo wrote:
> > > Commit bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ one-shot")
> > > dropped the check that ignored a handover interrupt after handover
> > > had already been issued, relying instead on disabling the IRQ after
> > > its first delivery to make it one-shot.
> > > 
> > > That is not sufficient for qcom_pas_attach(): when attaching to a
> > > remote processor that was already booted by the bootloader, handover
> > > has already happened out-of-band, before this driver ever probed.
> > > qcom_pas_attach() sets handover_issued = true and unmasks the
> > > handover IRQ to keep the enable/disable tracking consistent, but the
> > > transition that signals handover is latched at the interrupt
> > > controller while masked, so unmasking still delivers that one IRQ.
> > > 
> > > Since this driver instance never ran qcom_pas_start() for that boot,
> > > it never took the proxy power-domain/clock/regulator votes that
> > > q6v5->handover() releases. Running the handover callback for this
> > > stale, already-accounted-for signal disables those votes without a
> > > matching enable, producing:
> > > 
> > >   genpd genpd:0:4c00000.remoteproc: Runtime PM usage count underflow!
> > >   genpd genpd:1:4c00000.remoteproc: Runtime PM usage count underflow!
> > > 
> > > Restore the early-return when handover_issued is already set, so a
> > > stale IRQ delivered on attach is dropped after being disabled instead
> > > of re-running the handover callback.
> > > 
> > > Fixes: bb7c5d6f5b41 ("remoteproc: qcom: q6v5: Make handover IRQ one-shot")
> > > Assisted-by: Claude:claude-sonnet-5
> > > Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> > > ---
> > >  drivers/remoteproc/qcom_q6v5.c | 17 +++++++++++++++--
> > >  1 file changed, 15 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
> > > index 10fa38f264c8..e715083846aa 100644
> > > --- a/drivers/remoteproc/qcom_q6v5.c
> > > +++ b/drivers/remoteproc/qcom_q6v5.c
> > > @@ -210,10 +210,23 @@ static irqreturn_t q6v5_handover_interrupt(int irq, void *data)
> > >  {
> > >  	struct qcom_q6v5 *q6v5 = data;
> > >  
> > > -	q6v5->handover_issued = true;
> > > -
> > >  	qcom_q6v5_handover_irq_disable(q6v5, false);
> > >  
> > > +	/*
> > > +	 * When attaching to an already-running remote processor,
> > > +	 * handover_issued is set before the IRQ is unmasked, since the
> > > +	 * handover already happened out-of-band, before this driver probed.
> > > +	 * The transition that signals it is latched at the interrupt
> > > +	 * controller while masked, so unmasking still delivers this one
> > > +	 * IRQ. Ignore it: this driver instance never enabled the resources
> > > +	 * (proxy PDs, clocks, etc.) that the handover callback would tear
> > > +	 * down.
> > > +	 */
> > > +	if (q6v5->handover_issued)
> > > +		return IRQ_HANDLED;
> > > +
> > 
> > The goal of Abel's patch was to have the handover interrupt unmasked
> > only when needed. If you just bail out here and do nothing then there
> > was no need to unmask it in the first place. Can you just drop enabling
> > the handover IRQ in qcom_pas_attach()?
> 
> Hi Stephen,
> 
> Thanks for the comment!
> 
> Although it seems working in my limited testing, I'm not sure it's
> entirely safe to drop the handover IRQ enable in qcom_pas_attach().
> 
> The handover IRQ is requested with IRQF_TRIGGER_RISING. When we attach
> to a remote processor the bootloader already booted, that rising edge
> happened while the IRQ was masked. Masking doesn't prevent the edge
> from being latched at the interrupt controller -- it just defers
> delivery. The pending bit sits there until the IRQ is unmasked and
> actually taken; unmasking alone doesn't clear it.
> 
> If qcom_pas_attach() never unmasks the IRQ, that stale pending edge
> doesn't go away -- it just waits for the next unmask, which is the
> next real qcom_q6v5_prepare() (i.e. the next stop/start cycle). But by
> then handover_issued has already been reset to false for that new
> boot (qcom_q6v5_prepare() does this before re-enabling the IRQ), so
> the stale edge would be indistinguishable from a genuine handover for
> the new cycle. It could run q6v5->handover() and tear down proxy
> PDs/clocks/interconnect immediately on prepare, before the firmware
> for that cycle has even signaled ready.
> 
> So the enable_irq() in attach() isn't dead weight -- it's there to
> deliberately unmask and flush that stale latched edge right away,
> while handover_issued is still true from attach(), so the IRQ fires
> once, gets caught by the one-shot ISR, is dropped harmlessly (because
> handover_issued is already set), and the IRQ ends up masked again.
> Removing the enable would just defer the spurious handover to a later
> and more damaging point instead of eliminating it.
> 
> If you agree with the theory above, I can follow up with a comment
> update in q6v5_handover_interrupt() to make this rationale explicit,
> so it's clear that the attach-time enable exists to flush the latched
> edge.
> 

It's a fair point (that I also considered before writing my original
comment), but I think it does not apply here.

There is no real interrupt controller for the SMP2P interrupts, all the
handling happens in software inside drivers/soc/qcom/smp2p.c. The driver
does not implement any latching. If the interrupt is masked, the bit in
entry->irq_enabled is not set and qcom_smp2p_notify_in() simply skips
signaling the event.

We rely on this also for the normal boot case. After the initial
handover is completed, the handover IRQ is disabled by Abel's patch and
then you have the exact same situation as in the attach case. There may
be additional handover IRQs coming in, but they should not get latched
for the next boot of the remoteproc.

Thanks,
Stephan