* [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®