Devicetree
 help / color / mirror / Atom feed
From: Stephan Gerhold <stephan.gerhold@linaro.org>
To: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
Cc: Bjorn Andersson <andersson@kernel.org>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Manivannan Sadhasivam <mani@kernel.org>,
	Luca Weiss <luca.weiss@fairphone.com>,
	Bartosz Golaszewski <brgl@kernel.org>,
	Konrad Dybcio <konradybcio@kernel.org>,
	shengchao.guo@oss.qualcomm.com, aiqun.yu@oss.qualcomm.com,
	tingwei.zhang@oss.qualcomm.com, trilok.soni@oss.qualcomm.com,
	yijie.yang@oss.qualcomm.com, linux-arm-msm@vger.kernel.org,
	linux-remoteproc@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Gokul Krishna Krishnakumar <gokul.krishnakumar@oss.qualcomm.com>
Subject: Re: [PATCH v6 5/6] remoteproc: qcom: pas: Add late attach support for subsystems
Date: Fri, 22 May 2026 14:07:06 +0200	[thread overview]
Message-ID: <ahBG6jKYdSAboWjs@linaro.org> (raw)
In-Reply-To: <20260519-knp-soccp-v6-5-cf5d0e194b5f@oss.qualcomm.com>

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>

Unfortunately, removing the ping-pong functionality that was present in
previous patch versions makes the whole mechanism a lot more fragile.
I'm not entirely sure if this has changed in SMP2P v2 or more recent
firmware versions, but in my experience the SMP2P "ready" bit does not
tell you if the remoteproc is actually running. The problem is that the
"ready" bit is asserted by the remoteproc when the firmware is ready,
but it is not cleared when you shutdown or forcibly stop the remoteproc.

If this is still the case, you can easily reproduce that with the
following test:

 1. Start the system as usual and let it attach the remoteproc
 2. Manually stop the remoteproc in sysfs (echo stop > state)
 3. modprobe -r qcom_q6v5_pas
 4. modprobe qcom_q6v5_pas
 5. If the "ready" bit is still set, the driver will try attaching the
    remoteproc, but it's actually not running. No recovery will happen.

In this situation, it is very difficult to detect the correct remoteproc
state without relying on an additional query mechanism like the
ping-pong feature.

You can make it a bit more reliable if you also check the status of the
"stop-ack" bit. This would tell you if the remoteproc was cleanly
stopped with the SMP2P "stop" mechanism. However, that will typically
still not fix the case above since nowadays remoteprocs are typically
stopped via the QMI qcom_sysmon and the "stop-ack" is not set in that
case. I believe this might set the separate "shutdown-ack" bit though
that is described for some SoCs, I never finished testing that.

And even if you check both "stop-ack" and "shutdown-ack", that doesn't
tell you if the remoteproc was forcibly killed using
qcom_scm_pas_shutdown() without gracefully stopping it first. The ideal
solution would be querying the PAS API to tell us if the remoteproc is
actively running, but the last time I checked I was unfortunately not
able to find a documented call that would tell us that.

Thanks,
Stephan

  parent reply	other threads:[~2026-05-22 12:07 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-19  7:24 [PATCH v6 0/6] Add binding and driver for Kaanapali SoCCP Jingyi Wang
2026-05-19  7:24 ` [PATCH v6 1/6] dt-bindings: remoteproc: qcom: cleanup qcom,adsp.yaml Jingyi Wang
2026-05-19  7:24 ` [PATCH v6 2/6] dt-bindings: remoteproc: qcom: move interrupts and interrupt-names list out of pas-common Jingyi Wang
2026-05-19  7:24 ` [PATCH v6 3/6] dt-bindings: remoteproc: qcom: Document pas for SoCCP on Kaanapali and Glymur platforms Jingyi Wang
2026-05-19  8:05   ` sashiko-bot
2026-05-19  8:45   ` Shawn Guo
2026-05-20  2:44     ` Jingyi Wang
2026-05-19  7:24 ` [PATCH v6 4/6] dt-bindings: remoteproc: qcom: Document pas for SoCCP on Hawi SoC Jingyi Wang
2026-05-19  9:54   ` Rob Herring (Arm)
2026-05-20  4:11     ` Jingyi Wang
2026-05-19  7:24 ` [PATCH v6 5/6] remoteproc: qcom: pas: Add late attach support for subsystems Jingyi Wang
2026-05-19  8:33   ` Shawn Guo
2026-05-19  8:50   ` sashiko-bot
2026-05-20  8:27   ` Mukesh Ojha
2026-05-20 10:18     ` Shawn Guo
2026-05-21  3:42     ` Jingyi Wang
2026-05-21 11:22       ` Mukesh Ojha
2026-05-22 12:07   ` Stephan Gerhold [this message]
2026-05-19  7:24 ` [PATCH v6 6/6] remoteproc: qcom_q6v5_pas: Add SoCCP node on Kaanapali Jingyi Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ahBG6jKYdSAboWjs@linaro.org \
    --to=stephan.gerhold@linaro.org \
    --cc=aiqun.yu@oss.qualcomm.com \
    --cc=andersson@kernel.org \
    --cc=brgl@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=gokul.krishnakumar@oss.qualcomm.com \
    --cc=jingyi.wang@oss.qualcomm.com \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=luca.weiss@fairphone.com \
    --cc=mani@kernel.org \
    --cc=mathieu.poirier@linaro.org \
    --cc=robh@kernel.org \
    --cc=shengchao.guo@oss.qualcomm.com \
    --cc=tingwei.zhang@oss.qualcomm.com \
    --cc=trilok.soni@oss.qualcomm.com \
    --cc=yijie.yang@oss.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox