| Message ID | 20231027-ath11k-rproc-defer-v1-1-f6b6a812cd18@fairphone.com |
|---|---|
| State | Accepted |
| Commit | 2a3ec40b98b46c339adb57313d3b933ee5e7a8e8 |
| Headers | show |
| Series | wifi: ath11k: Defer on rproc_get failure | expand |
"Luca Weiss" <luca.weiss@fairphone.com> writes: > On Fri Oct 27, 2023 at 10:25 AM CEST, Kalle Valo wrote: > >> Luca Weiss <luca.weiss@fairphone.com> writes: >> >> > If we already have gotten the rproc_handle (meaning the "qcom,rproc" >> > property is defined in the devicetree), it's a valid state that the >> > remoteproc module hasn't probed yet so we should defer probing instead >> > of just failing to probe. >> > >> > This resolves a race condition when the ath11k driver probes and fails >> > before the wpss remoteproc driver has probed, like the following: >> > >> > [ 6.232360] ath11k 17a10040.wifi: failed to get rproc >> > [ 6.232366] ath11k 17a10040.wifi: failed to get rproc: -22 >> > [ 6.232478] ath11k: probe of 17a10040.wifi failed with error -22 >> > ... >> > [ 6.252415] remoteproc remoteproc2: 8a00000.remoteproc is available >> > [ 6.252776] remoteproc remoteproc2: powering up 8a00000.remoteproc >> > [ 6.252781] remoteproc remoteproc2: Booting fw image qcom/qcm6490/fairphone5/wpss.mdt, size 7188 >> > >> > So, defer the probe if we hit that so we can retry later once the wpss >> > remoteproc is available. >> > >> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >> >> Did you test this on a real device? If yes, what ath11k hardware and firmware >> did you use? We use Tested-on tag to document that: >> >> https://wireless.wiki.kernel.org/en/users/drivers/ath11k/submittingpatches#tested-on_tag > > Hi, > > Yes I tested this on qcm6490-fairphone-fp5 including some extra patches > for wpss-pas remoteproc support (nothing special, just adding it to the > existing PAS driver) and wifi enablement in dts. Nice, do you have a link to the patches or a git tree which has everything? And how difficult would it be for me to run vanilla kernel.org kernel (no vendor kernels or anything like that) on Fairphone 5? Any documentation available for that? I'm asking because I don't have a test setup for WCN6750 right now. It would be awesome if I could use Fairphone for testing :) > I built this line from info from the dmesg, hope it's okay: > > Tested-on: wcn6750 hw1.0 AHB WLAN.MSL.1.0.1-01264-QCAMSLSWPLZ-1.37886.3 Thanks, I added that to the commit message. > And thinking about it, a Fixes tag would also be appropriate for this > patch. The code was moved to a different file in commit ba929d6fe31a > ("ath11k: Remove rproc references from common core layer") but I think > this tag should be correct. > > Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices") Ok, I added that as well.
On Mon Nov 13, 2023 at 4:37 PM CET, Kalle Valo wrote: > "Luca Weiss" <luca.weiss@fairphone.com> writes: > > > On Fri Oct 27, 2023 at 10:25 AM CEST, Kalle Valo wrote: > > > >> Luca Weiss <luca.weiss@fairphone.com> writes: > >> > >> > If we already have gotten the rproc_handle (meaning the "qcom,rproc" > >> > property is defined in the devicetree), it's a valid state that the > >> > remoteproc module hasn't probed yet so we should defer probing instead > >> > of just failing to probe. > >> > > >> > This resolves a race condition when the ath11k driver probes and fails > >> > before the wpss remoteproc driver has probed, like the following: > >> > > >> > [ 6.232360] ath11k 17a10040.wifi: failed to get rproc > >> > [ 6.232366] ath11k 17a10040.wifi: failed to get rproc: -22 > >> > [ 6.232478] ath11k: probe of 17a10040.wifi failed with error -22 > >> > ... > >> > [ 6.252415] remoteproc remoteproc2: 8a00000.remoteproc is available > >> > [ 6.252776] remoteproc remoteproc2: powering up 8a00000.remoteproc > >> > [ 6.252781] remoteproc remoteproc2: Booting fw image qcom/qcm6490/fairphone5/wpss.mdt, size 7188 > >> > > >> > So, defer the probe if we hit that so we can retry later once the wpss > >> > remoteproc is available. > >> > > >> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > >> > >> Did you test this on a real device? If yes, what ath11k hardware and firmware > >> did you use? We use Tested-on tag to document that: > >> > >> https://wireless.wiki.kernel.org/en/users/drivers/ath11k/submittingpatches#tested-on_tag > > > > Hi, > > > > Yes I tested this on qcm6490-fairphone-fp5 including some extra patches > > for wpss-pas remoteproc support (nothing special, just adding it to the > > existing PAS driver) and wifi enablement in dts. > > Nice, do you have a link to the patches or a git tree which has > everything? And how difficult would it be for me to run vanilla > kernel.org kernel (no vendor kernels or anything like that) on Fairphone > 5? Any documentation available for that? I'm happy you ask ;) Currently pure kernel.org doesn't boot much because quite a few patches are still being upstreamed, like this one. In terms of git tree, this here is the last tag for everything I have working on the device, it's ~100 patches on top of v6.6, a good chunk should be merged for v6.7-rc1 already: https://github.com/z3ntu/linux/commits/v6.6.0-sc7280 You can flash the device with that kernel and postmarketOS using the pmbootstrap tooling: https://wiki.postmarketos.org/wiki/Fairphone_5_(fairphone-fp5) https://wiki.postmarketos.org/wiki/Pmbootstrap Let me know if you hit any issues, or if I can help somehow. > > I'm asking because I don't have a test setup for WCN6750 right now. It > would be awesome if I could use Fairphone for testing :) > > > I built this line from info from the dmesg, hope it's okay: > > > > Tested-on: wcn6750 hw1.0 AHB WLAN.MSL.1.0.1-01264-QCAMSLSWPLZ-1.37886.3 > > Thanks, I added that to the commit message. > > > And thinking about it, a Fixes tag would also be appropriate for this > > patch. The code was moved to a different file in commit ba929d6fe31a > > ("ath11k: Remove rproc references from common core layer") but I think > > this tag should be correct. > > > > Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices") > > Ok, I added that as well. Thanks! Regards Luca
diff --git a/drivers/net/wireless/ath/ath11k/ahb.c b/drivers/net/wireless/ath/ath11k/ahb.c index 235336ef2a7a..f8f5e653cd03 100644 --- a/drivers/net/wireless/ath/ath11k/ahb.c +++ b/drivers/net/wireless/ath/ath11k/ahb.c @@ -803,8 +803,8 @@ static int ath11k_core_get_rproc(struct ath11k_base *ab) prproc = rproc_get_by_phandle(rproc_phandle); if (!prproc) { - ath11k_err(ab, "failed to get rproc\n"); - return -EINVAL; + ath11k_dbg(ab, ATH11K_DBG_AHB, "failed to get rproc, deferring\n"); + return -EPROBE_DEFER; } ab_ahb->tgt_rproc = prproc;
If we already have gotten the rproc_handle (meaning the "qcom,rproc" property is defined in the devicetree), it's a valid state that the remoteproc module hasn't probed yet so we should defer probing instead of just failing to probe. This resolves a race condition when the ath11k driver probes and fails before the wpss remoteproc driver has probed, like the following: [ 6.232360] ath11k 17a10040.wifi: failed to get rproc [ 6.232366] ath11k 17a10040.wifi: failed to get rproc: -22 [ 6.232478] ath11k: probe of 17a10040.wifi failed with error -22 ... [ 6.252415] remoteproc remoteproc2: 8a00000.remoteproc is available [ 6.252776] remoteproc remoteproc2: powering up 8a00000.remoteproc [ 6.252781] remoteproc remoteproc2: Booting fw image qcom/qcm6490/fairphone5/wpss.mdt, size 7188 So, defer the probe if we hit that so we can retry later once the wpss remoteproc is available. Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> --- drivers/net/wireless/ath/ath11k/ahb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- base-commit: 2ef7141596eed0b4b45ef18b3626f428a6b0a822 change-id: 20231027-ath11k-rproc-defer-d166779fa113 Best regards,