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

[Jingyi Wang <jingyi.wang@oss.qualcomm.com>]

On 5/21/2026 7:22 PM, Mukesh Ojha wrote:
> On Thu, May 21, 2026 at 11:42:49AM +0800, Jingyi Wang wrote:
>>
>>
>> On 5/20/2026 4:27 PM, 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@oss.qualcomm.com>
>>>> Signed-off-by: Gokul Krishna Krishnakumar <gokul.krishnakumar@oss.qualcomm.com>
>>>> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>>>> ---
>>>>    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.
>>>
>>
>> It is suggested by Bjorn:
>> https://lore.kernel.org/all/qfls6xlvfppqw7p6rjpmzqesh6sbob4myfc6dz47qh3jywqrjk@5xiutkbybk5d/
>>
>> I did a hack to test the recovery by setting crash_state true, it can recovery
>> (stop and start) successfully with below patches:
>> https://lore.kernel.org/all/20260519-rproc-attach-issue-v2-0-caa1eaf75081@oss.qualcomm.com/
>>
>> For coredump, it will return from the first "list_empty(&rproc->dump_segments)" check in
>> rproc_coredump as segments are not configured in attach.
> 
> 
> I was not against any of the stuff, but mostly checking, if we agreed on not collecting dump
> for first crash when soccp minidump is not initialized which falls back to full dump of the soccp.
> I see soccp minidump id in the downstream but we have not added in 6/6.
> 

Hi Mukesh,

I prefer to add base rproc attach feature only in this patch and skipping the coredump in attach
workflow.

Thanks,
Jingyi

>>
>> Thanks,
>> Jingyi
>>
>>
>>> @Sibi, has this series been tested on Glymur with KVM?
>>> I don't see the iommu property in the below patch.
>>> https://lore.kernel.org/lkml/20260403-glymur-soccp-v3-1-f0e8d57f11ba@oss.qualcomm.com/
>>>
>>
>