Re: [PATCH] remoteproc: qcom_q6v5_pas: Drop early_boot flag for Nord ADSP

[Stephan Gerhold]

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@xxxxxxxxxxxxxxxx>
> > > ---
> > 
> > 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@xxxxxxxxxx/
[2]: https://lore.kernel.org/r/20260409-rproc-attach-issue-v1-0-088a1c348e7a@xxxxxxxxxxxxxxxx/
[3]: https://lore.kernel.org/r/20260723-rproc-rmmod-not-crashing-v1-0-546dfd5de0e6@xxxxxxxxxxxxxxxx/