Re: [PATCH v4 6/7] remoteproc: qcom: pas: Add late attach support for subsystems
From: Jingyi Wang
Date: Mon Apr 06 2026 - 22:45:03 EST
On 4/6/2026 10:59 PM, Bjorn Andersson wrote:
On Tue, Mar 10, 2026 at 03:03:22AM -0700, Jingyi Wang wrote:
From: Gokul Krishna Krishnakumar <gokul.krishnakumar@xxxxxxxxxxxxxxxx>
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 and performing ping
test.
A new qcom_pas_attach() function is introduced. if a crash state is
detected for the subsystem, rproc_report_crash() is called. If the
subsystem is ready either at the first check or within a 5-second timeout
and the ping is successful, it will be marked as "attached". The ready
state could be set by either ready interrupt or handover interrupt.
The whole use case of early booting SoCCP is to get the charger and USB
Type-C running early - so that charging and USB Type-C works in UEFI.
If SMP2P indicates that it was booted, but it's still not there...then
there's no reason to wait another 5 seconds - it's not there.
If "early_boot" is set by kernel but "subsys_booted" is not completed[..]
within the timeout, 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. As
the running state is set once attach function is called, the watchdog or
fatal interrupt received can be handled correctly.
Signed-off-by: Gokul Krishna Krishnakumar <gokul.krishnakumar@xxxxxxxxxxxxxxxx>
Co-developed-by: Jingyi Wang <jingyi.wang@xxxxxxxxxxxxxxxx>
Signed-off-by: Jingyi Wang <jingyi.wang@xxxxxxxxxxxxxxxx>
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c[..]
+static int qcom_pas_attach(struct rproc *rproc)[..]
+ if (!ret)
+ ret = irq_get_irqchip_state(pas->q6v5.ready_irq,
+ IRQCHIP_STATE_LINE_LEVEL, &ready_state);
+
+ /*
+ * smp2p allocate irq entry can be delayed, irq_get_irqchip_state will get -ENODEV,
This on the other hand, sounds like a bug in the smp2p driver. If we can
acquire the interrupt without getting EPROBE_DEFER, then we should not
get -ENODEV when reading the irq state.
+ * the 5 seconds timeout is set to wait for this, after the entry is allocated, smp2p
+ * will call the qcom_smp2p_intr and complete the timeout in the ISR.
If this indeed is the problem you're working around with the 5 second
delay - then stop. Fix the issue instead!
Also, this comment conflicts with the reasoning for the ping and the 5
second thing in the commit message.
Regards,
Bjorn
It is a design in downstream code to avoid the irq is not received when
we check the irq state, but indeed it seems like a redundant design and
I didn't see the delay in the test for the soccp attach. So we will remove
this 5 seconds wait in next series.
Thanks,
Jingyi
+ */
+ if (unlikely(ret == -ENODEV) || unlikely(!ready_state)) {
+ ret = wait_for_completion_timeout(&pas->q6v5.subsys_booted,
+ msecs_to_jiffies(EARLY_ATTACH_TIMEOUT_MS));
+
+ /*
+ * The bootloader may not support early boot, mark the state as
+ * RPROC_OFFLINE so that the PAS driver can load the firmware and
+ * start the remoteproc.
+ */
+ if (!ret) {
+ dev_err(pas->dev, "Timeout on waiting for subsystem interrupt\n");
+ pas->rproc->state = RPROC_OFFLINE;
+ ret = -ETIMEDOUT;
+ goto disable_running;
+ }
+
+ /* Only ping the subsystem if ready_state is set */
+ ret = irq_get_irqchip_state(pas->q6v5.ready_irq,
+ IRQCHIP_STATE_LINE_LEVEL, &ready_state);
+
+ if (ret)
+ goto disable_running;
+
+ if (!ready_state) {
+ ret = -EINVAL;
+ goto disable_running;
+ }
+ }
+
+ ret = qcom_q6v5_ping_subsystem(&pas->q6v5);
+
+ if (ret) {
+ dev_err(pas->dev, "Failed to ping subsystem, assuming device crashed\n");
+ rproc_report_crash(rproc, RPROC_FATAL_ERROR);
+ goto disable_running;
+ }
+
+ pas->q6v5.handover_issued = true;
+
+ return 0;
+
+disable_running:
+ pas->q6v5.running = false;
+
+ return ret;
+}
+
static const struct rproc_ops qcom_pas_ops = {
.unprepare = qcom_pas_unprepare,
.start = qcom_pas_start,
@@ -518,6 +603,7 @@ static const struct rproc_ops qcom_pas_ops = {
.parse_fw = qcom_pas_parse_firmware,
.load = qcom_pas_load,
.panic = qcom_pas_panic,
+ .attach = qcom_pas_attach,
};
static const struct rproc_ops qcom_pas_minidump_ops = {
@@ -823,7 +909,7 @@ static int qcom_pas_probe(struct platform_device *pdev)
pas->proxy_pd_count = ret;
ret = qcom_q6v5_init(&pas->q6v5, pdev, rproc, desc->crash_reason_smem,
- desc->load_state, qcom_pas_handover);
+ desc->load_state, desc->early_boot, qcom_pas_handover);
if (ret)
goto detach_proxy_pds;
@@ -855,6 +941,15 @@ static int qcom_pas_probe(struct platform_device *pdev)
pas->pas_ctx->use_tzmem = rproc->has_iommu;
pas->dtb_pas_ctx->use_tzmem = rproc->has_iommu;
+
+ if (pas->q6v5.early_boot) {
+ ret = qcom_q6v5_ping_subsystem_init(&pas->q6v5, pdev);
+ if (ret)
+ dev_warn(&pdev->dev, "Falling back to firmware load\n");
+ else
+ pas->rproc->state = RPROC_DETACHED;
+ }
+
ret = rproc_add(rproc);
if (ret)
goto remove_ssr_sysmon;
diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index c27200159a88..859141589ed7 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -1011,7 +1011,7 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = qcom_q6v5_init(&wcss->q6v5, pdev, rproc, desc->crash_reason_smem, NULL, NULL);
+ ret = qcom_q6v5_init(&wcss->q6v5, pdev, rproc, desc->crash_reason_smem, NULL, false, NULL);
if (ret)
return ret;
--
2.25.1