On Fri, Oct 09, 2026 at 04:38:14PM +0200, Stephan Gerhold wrote: > On Tue, Oct 06, 2026 at 11:35:50AM +0800, Shawn Guo wrote: > > When qcom_pas_attach() finds the fatal SMP2P bit already set, it reports > > a crash and then fails the attach. The core unwinds the attach while the > > crash work is queued: rproc_boot() drops the power refcount back to 0, > > and __rproc_attach()/rproc_attach() unprepare the subdevices, clean up > > the resource table and disable the IOMMU. Once rproc_boot() releases > > rproc->lock, the crash handler finds the rproc still RPROC_DETACHED, > > marks it RPROC_CRASHED and runs rproc_boot_recovery() on top of that > > torn-down state: > > > > - the subdevices are unprepared a second time by rproc_stop(), so SSR > > and sysmon notifiers see a duplicate shutdown; > > - qcom_pas_stop() and the subsequent rproc_start() operate on resources > > that rproc_attach() already released, e.g. iommu_unmap() on a > > disabled domain for rproc->has_iommu; > > - recovery leaves the rproc RPROC_RUNNING with power == 0. A later > > "stop" via sysfs takes power to -1 and returns success without > > stopping anything, so the remote stays running. A following "start" > > then boots the firmware again on the running remote and fails to > > re-add the still registered subdevices: > > > > sysfs: cannot create duplicate filename '.../qcom_common.pd-mapper.0' > > remoteproc remoteproc0: failed to prepare subdevices for adsp: -17 > > remoteproc remoteproc0: Boot failed: -17 > > > > A crash found at attach time is no different from one reported by the > > fatal interrupt right after attaching. Treat it the same way as > > q6v5_fatal_interrupt() does: clear q6v5.running, report the crash and > > let the attach succeed. The core completes the attach with balanced > > bookkeeping, and the crash handler then recovers the subsystem through > > the regular RPROC_ATTACHED -> RPROC_CRASHED path. With running cleared, > > qcom_q6v5_request_stop() does not wait for a stop ack the dead remote > > cannot send, and handover_issued already prevents a spurious handover > > on stop. > > > > Hmmmm okay, I have never thought of this option, but reusing the > existing crash path as-is by making the attach succeed feels actually > quite clever! I suppose starting all the subdevs is really redundant in > this case, but the crash case is also nothing worth optimizing for. > > > Assisted-by: LLM > > Signed-off-by: Shawn Guo <[email protected]> > > --- > > drivers/remoteproc/qcom_q6v5_pas.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c > > b/drivers/remoteproc/qcom_q6v5_pas.c > > index 8f3d45c604c9..0ecd7688a54d 100644 > > --- a/drivers/remoteproc/qcom_q6v5_pas.c > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > > @@ -562,10 +562,14 @@ static int qcom_pas_attach(struct rproc *rproc) > > goto disable_running; > > > > if (crash_state) { > > + /* > > + * Complete the attach and let the crash handler recover the > > + * subsystem from the attached state, as for a crash reported > > + * by the fatal interrupt. > > + */ > > dev_err(pas->dev, "Subsystem has crashed before driver > > probe\n"); > > + pas->q6v5.running = false; > > rproc_report_crash(rproc, RPROC_FATAL_ERROR); > > - ret = -EINVAL; > > - goto disable_running; > > } > > My only nitpick is that it would be nice to fully reuse the normal crash > handling in this case, in particular the crash_reason logging inside > q6v5_fatal_interrupt(). Can you extract that part into a common function > and call it from here?
That's a sensible suggestion. Will update. > (I guess you could also call the existing q6v5_fatal_interrupt() > function since it should work as-is, but externally calling an interrupt > handler might be a bit weird...) I share the same view. Shawn
