Hi Manikanta, all,
I have been bringing up the WiFi subsystem on an IPQ5332 board using the
multipd remoteproc driver (qcom_q6v5_mpd) together with the ath12k AHB
backend, on 6.18.44. Two things came out of it that I could not settle
from the code alone. Both look structural rather than like mistakes on
my side, so I would rather report them than sit on them.
Caveat up front: I am running vendor (QSDK) Q6 firmware, as there is no
upstream firmware for this part. If either answer is "that is a
vendor-firmware-only concern", that is a perfectly good answer and I
will stop there.
1. The root PD does not boot without a text PD level, and the binding
has no way to describe one
------------------------------------------------------------------
With the DT shaped as the binding describes -- a root PD with user PDs
pd-1/pd-2 directly underneath -- the root PD never reaches a usable
state:
qcom-q6-mpd d100000.remoteproc: fatal error without message
No further diagnostics, and the user PDs never come up.
The vendor DT has a level between the root PD and the user PDs: a text
PD (compatible "qcom,ipq5332-mpd-upd-text") that owns a separate
firmware image, with the user PDs as its children rather than the
root's:
q6v5_wcss qcom,ipq5332-q6-mpd q6_fw0.mdt
text-pd qcom,ipq5332-mpd-upd-text q6_fw3.mdt
pd1 q6_fw1.mdt
pd2 q6_fw2.mdt
Once I added that level -- a text PD node with its own firmware-name,
the user PDs reparented under it, and a small driver change so that
q6_register_userpd() recurses and the PAS ID is derived from the node
-- the fatal error disappeared and the whole chain came up:
remoteproc remoteproc2: powering up pd1
remoteproc remoteproc1: powering up text-pd3
remoteproc remoteproc0: powering up d100000.remoteproc
remoteproc remoteproc0: remote processor d100000.remoteproc is now up
remoteproc remoteproc1: remote processor text-pd3 is now up
remoteproc remoteproc2: remote processor pd1 is now up
The ordering is the interesting part. "powering up" runs child to parent
as rproc_boot() recurses upward, and "is now up" comes back parent to
child as it unwinds -- which is exactly the parent-first dependency the
multipd model exists to express. The text PD simply was not one of the
levels it can express.
So: is the text PD level deliberately out of scope upstream -- because
upstream firmware for these parts is expected to be packaged without a
separate text image, say -- or is it a level that should exist and is
not there yet?
I ask because if it is the latter, the binding as posted cannot describe
this hardware at all, and that seems worth settling before it lands. I
have a working change against qcom_q6v5_mpd.c and would be glad to send
it as an RFC. I did not want to send it uninvited if the omission is
intentional.
2. The vendor DT and the ath12k AHB backend disagree about what
qcom,rproc points at
------------------------------------------------------------------
This one I can describe precisely, but I cannot tell which side is
meant to be authoritative.
In ath12k's AHB backend, qcom,rproc is the *root* PD. ahb.c resolves the
phandle and boots it via a function named ath12k_ahb_boot_root_pd(),
waits for rootpd_ready, and then does the entire user PD lifecycle
itself: qcom_mdt_load() of amss_<n>.mdt into memory-region[0],
qcom_scm_pas_auth_and_reset() with a PAS ID it computes locally, and a
spawn via qcom_smem_state_update_bits(). It also requests the "spawn",
"ready" and "stop-ack" interrupts by name to drive that sequence.
In the vendor DT, qcom,rproc is the *user* PD:
wifi@c0000000 {
compatible = "qcom,cnss-qca5332", "qcom,ipq5332-wifi";
qcom,rproc = <&remoteproc_pd1>; /* the user PD */
qcom,userpd-subsys-name = "q6v5_wcss_userpd1";
};
There, remoteproc owns the user PD -- image, PAS, spawn and the same
three interrupts -- and the WiFi driver only attaches to it.
These are mutually exclusive, and on this board they collide. With
qcom_q6v5_mpd instantiating user PD rprocs as its binding describes, the
PD boots fine and then ath12k cannot have the interrupt it needs:
remoteproc remoteproc2: remote processor pd1 is now up
genirq: Flags mismatch irq 31. 00002001 (UserPD1-spawn)
vs. 00002001 (q6v5_wcss_userpd1_spawn-ack)
ath12k_ahb c000000.wifi: error -EBUSY: Request spawn irq failed: -16
Same interrupt, same flags, two requesters, neither wrong in isolation.
So which is the intended division of labour?
- Are the pd-1/pd-2 nodes in the multipd binding meant for non-WiFi
PDs only, with WiFi user PDs left entirely to ath12k -- so a board
with only WiFi user PDs should describe none of them, and point
qcom,rproc at the root PD?
- Or is remoteproc meant to own them, as the vendor DT has it, and
ath12k's AHB backend should attach rather than load and spawn when
qcom,rproc resolves to a user PD?
I have deliberately not sent a patch for this one. Either answer implies
a behaviour change in a driver used by boards I cannot test, and
guessing at the ownership model and then changing ath12k for everyone is
not mine to do.
Happy to test patches, provide full boot logs, or run experiments on
this board -- it is on a serial console with a recoverable second
firmware bank, so I can iterate destructively without much risk.
Thanks for the series, and for reading this far.
Regards,
Hideo Sumi