On Mon, Oct 05, 2026 at 09:34:03AM +0200, Stephan Gerhold wrote: > 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 <[email protected]> > > > > --- > > > > > > 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.
Great input, Stephan! I agree it's way better and cleaner to fix the problem by not touching remoteproc core. > 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 Ah, it seems we should take Sashiko more seriously! > 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). The crash handling seems still buggy, even with Bjorn's fixes. If attach finds the fatal bit set, it queues the crash work and returns -EINVAL. The core unwinds the attach before the crash handler can take rproc->lock. When the handler runs, it finds DETACHED, sets CRASHED and calls rproc_boot_recovery() on top of a half-torn-down state: - rproc_boot() has already dropped power back to 0, so after recovery the rproc is RUNNING with power == 0; - the subdevices are unprepared twice, once in the attach error path and again in rproc_stop(), so SSR/sysmon notifiers see a duplicate shutdown; - qcom_pas_stop() and rproc_start() run on resources that rproc_attach() already released. With has_iommu, that means iommu_unmap() on a disabled domain. I reproduced this on Nord ADSP by forcing crash_state in qcom_pas_attach(). Recovery itself appears to succeed. But a later sysfs "stop" takes power to -1 and returns success without stopping anything, and the state stays "running". A following "start" then boots the firmware again on top of the running remote: sysfs: cannot create duplicate filename '.../qcom_common.pd-mapper.0' remoteproc remoteproc0: failed to prepare subdevices for adsp: -17 remoteproc remoteproc0: Boot failed: -17 > 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... :-) Indeed! I will try to fix the crash handling with LLM's help. > [1]: https://lore.kernel.org/r/[email protected]/ > [2]: > https://lore.kernel.org/r/[email protected]/ > [3]: > https://lore.kernel.org/r/20260723-rproc-rmmod-not-crashing-v1-0-546dfd5de...@oss.qualcomm.com/ Thanks much for the detailed pointers to the earlier threads! Shawn
