* [PATCH] remoteproc: qcom_q6v5_pas: Drop early_boot flag for Nord ADSP
@ 2026-09-30 2:07 Shawn Guo
2026-10-01 8:38 ` Bartosz Golaszewski
2026-10-01 8:42 ` Konrad Dybcio
0 siblings, 2 replies; 5+ messages in thread
From: Shawn Guo @ 2026-09-30 2:07 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Mathieu Poirier, Bartosz Golaszewski, linux-arm-msm,
linux-remoteproc, linux-kernel, Shawn Guo
Qualcomm NHLOS team changes XBL for Nord IOT/Embedded variant, leaving
ADSP to be powered up by Linux remoteproc, so that Nord IQ10 Qualcomm
Linux (QLI) behavior gets aligned with IQ8/9. Drop early_boot flag from
Nord ADSP for that purpose.
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
drivers/remoteproc/qcom_q6v5_pas.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 2e1e39826ffa..9e7fb5c5610e 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -1582,7 +1582,6 @@ static const struct qcom_pas_data nord_adsp_resource = {
.dtb_pas_id = 36,
.minidump_id = 5,
.auto_boot = true,
- .early_boot = true,
.proxy_pd_names = (char*[]){
"cx",
"mx",
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] remoteproc: qcom_q6v5_pas: Drop early_boot flag for Nord ADSP
2026-09-30 2:07 [PATCH] remoteproc: qcom_q6v5_pas: Drop early_boot flag for Nord ADSP Shawn Guo
@ 2026-10-01 8:38 ` Bartosz Golaszewski
2026-10-01 8:42 ` Konrad Dybcio
1 sibling, 0 replies; 5+ messages in thread
From: Bartosz Golaszewski @ 2026-10-01 8:38 UTC (permalink / raw)
To: Shawn Guo
Cc: Mathieu Poirier, Bartosz Golaszewski, linux-arm-msm,
linux-remoteproc, linux-kernel, Bjorn Andersson
On Wed, 30 Sep 2026 04:07:45 +0200, Shawn Guo
<shengchao.guo@oss.qualcomm.com> said:
> Qualcomm NHLOS team changes XBL for Nord IOT/Embedded variant, leaving
> ADSP to be powered up by Linux remoteproc, so that Nord IQ10 Qualcomm
> Linux (QLI) behavior gets aligned with IQ8/9. Drop early_boot flag from
> Nord ADSP for that purpose.
>
> Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> ---
> drivers/remoteproc/qcom_q6v5_pas.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 2e1e39826ffa..9e7fb5c5610e 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -1582,7 +1582,6 @@ static const struct qcom_pas_data nord_adsp_resource = {
> .dtb_pas_id = 36,
> .minidump_id = 5,
> .auto_boot = true,
> - .early_boot = true,
> .proxy_pd_names = (char*[]){
> "cx",
> "mx",
> --
> 2.43.0
>
>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] remoteproc: qcom_q6v5_pas: Drop early_boot flag for Nord ADSP
2026-09-30 2:07 [PATCH] remoteproc: qcom_q6v5_pas: Drop early_boot flag for Nord ADSP Shawn Guo
2026-10-01 8:38 ` Bartosz Golaszewski
@ 2026-10-01 8:42 ` Konrad Dybcio
2026-10-04 15:26 ` Shawn Guo
1 sibling, 1 reply; 5+ messages in thread
From: Konrad Dybcio @ 2026-10-01 8:42 UTC (permalink / raw)
To: Shawn Guo, Bjorn Andersson
Cc: Mathieu Poirier, Bartosz Golaszewski, linux-arm-msm,
linux-remoteproc, linux-kernel
On 9/30/26 4:07 AM, Shawn Guo wrote:
> Qualcomm NHLOS team changes XBL for Nord IOT/Embedded variant, leaving
> ADSP to be powered up by Linux remoteproc, so that Nord IQ10 Qualcomm
> Linux (QLI) behavior gets aligned with IQ8/9. Drop early_boot flag from
> Nord ADSP for that purpose.
>
> Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> ---
What happens if this patch is absent?
When the early_boot path was first introduced I was really hoping
that this behavior could be made unconditional and Linux would
figure out if the rproc may be active by virtue of it sending
back signals or not, unfortunately that hasn't made it into the
tree..
Konrad
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] remoteproc: qcom_q6v5_pas: Drop early_boot flag for Nord ADSP
2026-10-01 8:42 ` Konrad Dybcio
@ 2026-10-04 15:26 ` Shawn Guo
2026-10-05 7:34 ` Stephan Gerhold
0 siblings, 1 reply; 5+ messages in thread
From: Shawn Guo @ 2026-10-04 15:26 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Mathieu Poirier, Bartosz Golaszewski,
linux-arm-msm, linux-remoteproc, linux-kernel
On Thu, Oct 01, 2026 at 10:42:39AM +0200, Konrad Dybcio wrote:
> On 9/30/26 4:07 AM, Shawn Guo wrote:
> > Qualcomm NHLOS team changes XBL for Nord IOT/Embedded variant, leaving
> > ADSP to be powered up by Linux remoteproc, so that Nord IQ10 Qualcomm
> > Linux (QLI) behavior gets aligned with IQ8/9. Drop early_boot flag from
> > Nord ADSP for that purpose.
> >
> > Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> > ---
>
> What happens if this patch is absent?
ADSP never comes up:
remoteproc remoteproc0: attaching to adsp
remoteproc remoteproc0: can't attach to rproc adsp: -19
Since XBL no longer boots ADSP, the remote never publishes its SMP2P
inbound item. The smp2p entry stays unmapped, and irq_get_irqchip_state()
on fatal_irq returns -ENODEV. qcom_pas_attach() treats that as a hard
error rather than falling back to a firmware boot.
Even the existing fallback (ready bit clear -> RPROC_OFFLINE) doesn't
work: rproc_boot() takes the attach branch, sees the error and returns
without loading firmware.
>
> When the early_boot path was first introduced I was really hoping
> that this behavior could be made unconditional and Linux would
> figure out if the rproc may be active by virtue of it sending
> back signals or not, unfortunately that hasn't made it into the
> tree..
The difficulty is with the positive signal. As Stephan pointed out [1],
the ready bit is not cleared when the remote is stopped or force-shutdown,
so a stop followed by rmmod/modprobe of qcom_q6v5_pas makes the driver
attach to a remote that is not running. Without ping-pong or a PAS query
for the remote state, I don't think we can reliably say that a remote
*is* running.
The negative signal is reliable though. The -ENODEV from
irq_get_irqchip_state() means the remote has never populated its SMP2P
entry since cold boot, so it cannot be running. We can fall back to a
firmware boot in that case. early_boot then becomes a hint that the
bootloader *may* have started the remote, which matches Nord ADSP:
it is started by XBL on the Auto variant and by Linux remoteproc on
the IoT variant.
I'll drop this patch and send two patches instead:
- remoteproc: core: continue to the firmware boot path when .attach()
leaves the rproc in RPROC_OFFLINE
- remoteproc: qcom: pas: treat -ENODEV from the SMP2P state read as
"not running" and fall back to start
With those, Nord ADSP keeps early_boot and I've tested it with both XBL
variants. With the old XBL it attaches as before; with the new one
it falls back to loading firmware.
Shawn
[1] https://lore.kernel.org/all/ahBG6jKYdSAboWjs@linaro.org/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] remoteproc: qcom_q6v5_pas: Drop early_boot flag for Nord ADSP
2026-10-04 15:26 ` Shawn Guo
@ 2026-10-05 7:34 ` Stephan Gerhold
0 siblings, 0 replies; 5+ messages in thread
From: Stephan Gerhold @ 2026-10-05 7:34 UTC (permalink / raw)
To: Shawn Guo
Cc: Konrad Dybcio, Bjorn Andersson, Mathieu Poirier,
Bartosz Golaszewski, linux-arm-msm, linux-remoteproc,
linux-kernel, Jingyi Wang, Aiqun(Maria) Yu
On Sun, Oct 04, 2026 at 11:26:31PM +0800, Shawn Guo wrote:
> On Thu, Oct 01, 2026 at 10:42:39AM +0200, Konrad Dybcio wrote:
> > On 9/30/26 4:07 AM, Shawn Guo wrote:
> > > Qualcomm NHLOS team changes XBL for Nord IOT/Embedded variant, leaving
> > > ADSP to be powered up by Linux remoteproc, so that Nord IQ10 Qualcomm
> > > Linux (QLI) behavior gets aligned with IQ8/9. Drop early_boot flag from
> > > Nord ADSP for that purpose.
> > >
> > > Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> > > ---
> >
> > What happens if this patch is absent?
>
> ADSP never comes up:
>
> remoteproc remoteproc0: attaching to adsp
> remoteproc remoteproc0: can't attach to rproc adsp: -19
>
> Since XBL no longer boots ADSP, the remote never publishes its SMP2P
> inbound item. The smp2p entry stays unmapped, and irq_get_irqchip_state()
> on fatal_irq returns -ENODEV. qcom_pas_attach() treats that as a hard
> error rather than falling back to a firmware boot.
>
> Even the existing fallback (ready bit clear -> RPROC_OFFLINE) doesn't
> work: rproc_boot() takes the attach branch, sees the error and returns
> without loading firmware.
>
> >
> > When the early_boot path was first introduced I was really hoping
> > that this behavior could be made unconditional and Linux would
> > figure out if the rproc may be active by virtue of it sending
> > back signals or not, unfortunately that hasn't made it into the
> > tree..
>
> The difficulty is with the positive signal. As Stephan pointed out [1],
> the ready bit is not cleared when the remote is stopped or force-shutdown,
> so a stop followed by rmmod/modprobe of qcom_q6v5_pas makes the driver
> attach to a remote that is not running. Without ping-pong or a PAS query
> for the remote state, I don't think we can reliably say that a remote
> *is* running.
>
> The negative signal is reliable though. The -ENODEV from
> irq_get_irqchip_state() means the remote has never populated its SMP2P
> entry since cold boot, so it cannot be running. We can fall back to a
> firmware boot in that case. early_boot then becomes a hint that the
> bootloader *may* have started the remote, which matches Nord ADSP:
> it is started by XBL on the Auto variant and by Linux remoteproc on
> the IoT variant.
>
> I'll drop this patch and send two patches instead:
>
> - remoteproc: core: continue to the firmware boot path when .attach()
> leaves the rproc in RPROC_OFFLINE
I don't think you need this change in the remoteproc core. In the
current upstream state - without the ping-pong implementation - the
stop/shutdown/ready detection should remain static during the
initialization of qcom_q6v5_pas. The boot firmware has either started
it, stopped it, or never started it at all. Those signals should not
change state until we take some action.
IMO the proper solution is to move the checks inside qcom_pas_attach()
to the probe function and never mark the remoteproc as RPROC_DETACHED in
the first place if we already know it was not started.
The fatal/crash IRQ checks can probably remain inside qcom_pas_attach(),
I would assume the remoteproc core does not handle the case where a
remoteproc appears immediately in RPROC_CRASHED state.
BTW the issue you are running into was pointed out by Sashiko during the
review of the original patch, see the second comment here:
https://sashiko.dev/#/patchset/20260623-knp-soccp-v7-5-1ec7bb5c9fec%40oss.qualcomm.com?part=5
The first comment from Sashiko about the broken crash handling during
attach could also still be valid. I pointed out the same problem
multiple times during the review process [1]. Unfortunately, Jingyi
never fully addressed it. Eventually, the fixes were moved out into a
separate series [2] and AFAICT abandoned as soon as the main series was
merged with all the open problems. I'm quite frustrated about how the
review process went for this change. :(
The crash handling may have improved a bit with the fixes Bjorn did
recently [3], but someone with access to the hardware for testing should
really go complete the work that should have been done before merging
the original series and test that *all* the error cases are handled
correctly (remoteproc running, not running, crashed).
In other words, if you could also do some more testing for the "crashed"
case while fixing the "not running" case that would be much appreciated!
With the state of LLMs today, all the discussions and my lengthy
comments on the original series are probably perfect as verbatim input
context for an LLM to make it do the dirty work... :-)
Thanks,
Stephan
[1]: https://lore.kernel.org/r/aUsUhX8Km275qonq@linaro.org/
[2]: https://lore.kernel.org/r/20260409-rproc-attach-issue-v1-0-088a1c348e7a@oss.qualcomm.com/
[3]: https://lore.kernel.org/r/20260723-rproc-rmmod-not-crashing-v1-0-546dfd5de0e6@oss.qualcomm.com/
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-05 7:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-30 2:07 [PATCH] remoteproc: qcom_q6v5_pas: Drop early_boot flag for Nord ADSP Shawn Guo
2026-10-01 8:38 ` Bartosz Golaszewski
2026-10-01 8:42 ` Konrad Dybcio
2026-10-04 15:26 ` Shawn Guo
2026-10-05 7:34 ` Stephan Gerhold
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®