Re: [PATCH v6 5/6] remoteproc: qcom: pas: Add late attach support for subsystems

[Shawn Guo]

On Wed, May 20, 2026 at 01:57:47PM +0530, Mukesh Ojha wrote:
> On Tue, May 19, 2026 at 12:24:23AM -0700, Jingyi Wang wrote:
> > Subsystems can be brought out of reset by entities such as bootloaders.
> > As the irq enablement could be later than subsystem bring up, the state
> > of subsystem should be checked by reading SMP2P bits.
> > 
> > A new qcom_pas_attach() function is introduced. if a crash state is
> > detected for the subsystem, rproc_report_crash() is called. If the ready
> > state is detected, it will be marked as "attached", otherwise it could
> > be the early boot feature is not supported by other entities. In this
> > case, the state will be marked as RPROC_OFFLINE so that the PAS driver
> > can load the firmware and start the remoteproc.
> > 
> > Co-developed-by: Gokul Krishna Krishnakumar <gokul.krishnakumar@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Gokul Krishna Krishnakumar <gokul.krishnakumar@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Jingyi Wang <jingyi.wang@xxxxxxxxxxxxxxxx>
> > ---
> >  drivers/remoteproc/qcom_q6v5_pas.c | 58 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> > 
> > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> > index da27d1d3c9da..ac2a00aacd2e 100644
> > --- a/drivers/remoteproc/qcom_q6v5_pas.c
> > +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> > @@ -60,6 +60,7 @@ struct qcom_pas_data {
> >  	int region_assign_count;
> >  	bool region_assign_shared;
> >  	int region_assign_vmid;
> > +	bool early_boot;
> >  };
> >  
> >  struct qcom_pas {
> > @@ -510,6 +511,57 @@ static unsigned long qcom_pas_panic(struct rproc *rproc)
> >  	return qcom_q6v5_panic(&pas->q6v5);
> >  }
> >  
> > +static int qcom_pas_attach(struct rproc *rproc)
> > +{
> > +	int ret;
> > +	struct qcom_pas *pas = rproc->priv;
> > +	bool ready_state;
> > +	bool crash_state;
> > +
> > +	pas->q6v5.handover_issued = true;
> > +	enable_irq(pas->q6v5.handover_irq);
> > +
> > +	pas->q6v5.running = true;
> > +	ret = irq_get_irqchip_state(pas->q6v5.fatal_irq,
> > +				    IRQCHIP_STATE_LINE_LEVEL, &crash_state);
> > +
> > +	if (ret)
> > +		goto disable_running;
> > +
> > +	if (crash_state) {
> > +		dev_err(pas->dev, "Subsystem has crashed before driver probe\n");
> > +		rproc_report_crash(rproc, RPROC_FATAL_ERROR);
> 
> I am not sure if this is already discussed, but what if it is the first
> crash with recovery and coredump enabled? What would be in the dump,
> nothing? As there is no segment, is it expected since Linux did not load
> this?
> 
> This is even true if it is a crash after a successful attach.

Good point, Mukesh!

I guess we can have a check in qcom_pas_minidump() to do nothing 
in case state is RPROC_ATTACHED?

Shawn