[PATCH] wifi: ath11k: Defer on rproc_get failure

[PATCH] wifi: ath11k: Defer on rproc_get failure

[Luca Weiss]

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(-)

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;
 

---
base-commit: 2ef7141596eed0b4b45ef18b3626f428a6b0a822
change-id: 20231027-ath11k-rproc-defer-d166779fa113

Best regards,
-- 
Luca Weiss <luca.weiss@fairphone.com>

Re: [PATCH] wifi: ath11k: Defer on rproc_get failure

[Kalle Valo]

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

I can add that in the pending branch if you provide the info.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] wifi: ath11k: Defer on rproc_get failure

[Luca Weiss]

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.

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


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")

>
> I can add that in the pending branch if you provide the info.

Thanks!

Regards
Luca

Re: [PATCH] wifi: ath11k: Defer on rproc_get failure

[Kalle Valo]

"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.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] wifi: ath11k: Defer on rproc_get failure

[Luca Weiss]

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

Re: [PATCH] wifi: ath11k: Defer on rproc_get failure

[Kalle Valo]

Luca Weiss <luca.weiss@fairphone.com> wrote:

> 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.
> 
> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-01264-QCAMSLSWPLZ-1.37886.3
> 
> Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

Patch applied to ath-next branch of ath.git, thanks.

2a3ec40b98b4 wifi: ath11k: Defer on rproc_get failure

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20231027-ath11k-rproc-defer-v1-1-f6b6a812cd18@fairphone.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches