| Message ID | 54ac2295-36b4-49fc-9583-a10db8d9d5d6@freebox.fr |
|---|---|
| State | Accepted |
| Commit | 71b6e321e30271beb08772871c4f76777f49e402 |
| Headers | show |
| Series | Work around missing MSA_READY indicator for msm8998 devices | expand |
On Mon, Apr 29, 2024 at 04:04:51PM +0200, Marc Gonzalez wrote: > The ath10k driver waits for an "MSA_READY" indicator > to complete initialization. If the indicator is not > received, then the device remains unusable. > > cf. ath10k_qmi_driver_event_work() > > Several msm8998-based devices are affected by this issue. > Oddly, it seems safe to NOT wait for the indicator, and > proceed immediately when QMI_EVENT_SERVER_ARRIVE. > > Jeff Johnson wrote: > > The feedback I received was "it might be ok to change all ath10k qmi > to skip waiting for msa_ready", and it was pointed out that ath11k > (and ath12k) do not wait for it. > > However with so many deployed devices, "might be ok" isn't a strong > argument for changing the default behavior. > > Kalle Valo first suggested setting a bit in firmware-5.bin to trigger > work-around in the driver. However, firmware-5.bin is parsed too late. > So we are stuck with a DT property. > > Signed-off-by: Pierre-Hugues Husson <phhusson@freebox.fr> > Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> This says "Pierre-Hugues certifies the origin of the patch" then "Marc certifies the origin of the patch". This would have to imply that Pierre-Hugues authored the patch, but you're listed as the author... Perhaps a suitable answer to this question would be to add "Co-developed-by: Pierre-Hugues ..." above his s-o-b, which implies that the two of you jointly came up with this and both certify the origin. Other than that, I think this looks good, so please upon addressing this problem feel free to add my: Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com> Regards, Bjorn > --- > Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml > index 9b3ef4bc37325..9070a41f7fc07 100644 > --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml > +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml > @@ -122,6 +122,11 @@ properties: > Whether to skip executing an SCM call that reassigns the memory > region ownership. > > + qcom,no-msa-ready-indicator: > + type: boolean > + description: > + Don't wait for MSA_READY indicator to complete init. > + > qcom,smem-states: > $ref: /schemas/types.yaml#/definitions/phandle-array > description: State bits used by the AP to signal the WLAN Q6. > -- > 2.34.1 >
On 30/04/2024 06:06, Kalle Valo wrote: > Bjorn Andersson wrote: > >> On Mon, Apr 29, 2024 at 04:04:51PM +0200, Marc Gonzalez wrote: >> >>> The ath10k driver waits for an "MSA_READY" indicator >>> to complete initialization. If the indicator is not >>> received, then the device remains unusable. >>> >>> cf. ath10k_qmi_driver_event_work() >>> >>> Several msm8998-based devices are affected by this issue. >>> Oddly, it seems safe to NOT wait for the indicator, and >>> proceed immediately when QMI_EVENT_SERVER_ARRIVE. >>> >>> Jeff Johnson wrote: >>> >>> The feedback I received was "it might be ok to change all ath10k qmi >>> to skip waiting for msa_ready", and it was pointed out that ath11k >>> (and ath12k) do not wait for it. >>> >>> However with so many deployed devices, "might be ok" isn't a strong >>> argument for changing the default behavior. >>> >>> Kalle Valo first suggested setting a bit in firmware-5.bin to trigger >>> work-around in the driver. However, firmware-5.bin is parsed too late. >>> So we are stuck with a DT property. >>> >>> Signed-off-by: Pierre-Hugues Husson <phhusson@freebox.fr> >>> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> >> >> This says "Pierre-Hugues certifies the origin of the patch" then "Marc >> certifies the origin of the patch". This would have to imply that >> Pierre-Hugues authored the patch, but you're listed as the author... >> >> Perhaps a suitable answer to this question would be to add >> "Co-developed-by: Pierre-Hugues ..." above his s-o-b, which implies that >> the two of you jointly came up with this and both certify the origin. > > BTW I can add that in the pending branch, no need to resend because of > this. Just need guidance from Marc. I typed this patch all by myself with my grubby little paws. You can drop PH's S-o-b. >> Other than that, I think this looks good, so please upon addressing this >> problem feel free to add my: >> >> Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com> > > Thanks, I'll then add this as well. Cool. Almost there :)
Marc Gonzalez <mgonzalez@freebox.fr> writes: > On 30/04/2024 06:06, Kalle Valo wrote: > >> Bjorn Andersson wrote: >> >>> On Mon, Apr 29, 2024 at 04:04:51PM +0200, Marc Gonzalez wrote: >>> >>>> The ath10k driver waits for an "MSA_READY" indicator >>>> to complete initialization. If the indicator is not >>>> received, then the device remains unusable. >>>> >>>> cf. ath10k_qmi_driver_event_work() >>>> >>>> Several msm8998-based devices are affected by this issue. >>>> Oddly, it seems safe to NOT wait for the indicator, and >>>> proceed immediately when QMI_EVENT_SERVER_ARRIVE. >>>> >>>> Jeff Johnson wrote: >>>> >>>> The feedback I received was "it might be ok to change all ath10k qmi >>>> to skip waiting for msa_ready", and it was pointed out that ath11k >>>> (and ath12k) do not wait for it. >>>> >>>> However with so many deployed devices, "might be ok" isn't a strong >>>> argument for changing the default behavior. >>>> >>>> Kalle Valo first suggested setting a bit in firmware-5.bin to trigger >>>> work-around in the driver. However, firmware-5.bin is parsed too late. >>>> So we are stuck with a DT property. >>>> >>>> Signed-off-by: Pierre-Hugues Husson <phhusson@freebox.fr> >>>> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> >>> >>> This says "Pierre-Hugues certifies the origin of the patch" then "Marc >>> certifies the origin of the patch". This would have to imply that >>> Pierre-Hugues authored the patch, but you're listed as the author... >>> >>> Perhaps a suitable answer to this question would be to add >>> "Co-developed-by: Pierre-Hugues ..." above his s-o-b, which implies that >>> the two of you jointly came up with this and both certify the origin. >> >> BTW I can add that in the pending branch, no need to resend because of >> this. Just need guidance from Marc. > > I typed this patch all by myself with my grubby little paws. > You can drop PH's S-o-b. > >>> Other than that, I think this looks good, so please upon addressing this >>> problem feel free to add my: >>> >>> Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com> >> >> Thanks, I'll then add this as well. > > Cool. Almost there :) All I need is an ack from DT maintainers for this patch. DT maintainers: I think this is the best option and I can't think of any other solution so I would prefer to take this approach to our ath.git tree if it's ok for you. IIRC someone suggested testing for firmware version string but I suspect that has the same problem as the firmware-N.bin approach: ath10k gets the firmware version too late. And besides it's difficult to maintain such a list in ath10k, it would always need kernel updates when there's a new firmware etc.
On Mon, 29 Apr 2024 16:04:51 +0200, Marc Gonzalez wrote: > The ath10k driver waits for an "MSA_READY" indicator > to complete initialization. If the indicator is not > received, then the device remains unusable. > > cf. ath10k_qmi_driver_event_work() > > Several msm8998-based devices are affected by this issue. > Oddly, it seems safe to NOT wait for the indicator, and > proceed immediately when QMI_EVENT_SERVER_ARRIVE. > > Jeff Johnson wrote: > > The feedback I received was "it might be ok to change all ath10k qmi > to skip waiting for msa_ready", and it was pointed out that ath11k > (and ath12k) do not wait for it. > > However with so many deployed devices, "might be ok" isn't a strong > argument for changing the default behavior. > > Kalle Valo first suggested setting a bit in firmware-5.bin to trigger > work-around in the driver. However, firmware-5.bin is parsed too late. > So we are stuck with a DT property. > > Signed-off-by: Pierre-Hugues Husson <phhusson@freebox.fr> > Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> > --- > Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml | 5 +++++ > 1 file changed, 5 insertions(+) > Acked-by: Rob Herring (Arm) <robh@kernel.org>
Marc Gonzalez <mgonzalez@freebox.fr> writes: > On 30/04/2024 06:06, Kalle Valo wrote: > >> Bjorn Andersson wrote: >> >>> On Mon, Apr 29, 2024 at 04:04:51PM +0200, Marc Gonzalez wrote: >>> >>>> The ath10k driver waits for an "MSA_READY" indicator >>>> to complete initialization. If the indicator is not >>>> received, then the device remains unusable. >>>> >>>> cf. ath10k_qmi_driver_event_work() >>>> >>>> Several msm8998-based devices are affected by this issue. >>>> Oddly, it seems safe to NOT wait for the indicator, and >>>> proceed immediately when QMI_EVENT_SERVER_ARRIVE. >>>> >>>> Jeff Johnson wrote: >>>> >>>> The feedback I received was "it might be ok to change all ath10k qmi >>>> to skip waiting for msa_ready", and it was pointed out that ath11k >>>> (and ath12k) do not wait for it. >>>> >>>> However with so many deployed devices, "might be ok" isn't a strong >>>> argument for changing the default behavior. >>>> >>>> Kalle Valo first suggested setting a bit in firmware-5.bin to trigger >>>> work-around in the driver. However, firmware-5.bin is parsed too late. >>>> So we are stuck with a DT property. >>>> >>>> Signed-off-by: Pierre-Hugues Husson <phhusson@freebox.fr> >>>> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> >>> >>> This says "Pierre-Hugues certifies the origin of the patch" then "Marc >>> certifies the origin of the patch". This would have to imply that >>> Pierre-Hugues authored the patch, but you're listed as the author... >>> >>> Perhaps a suitable answer to this question would be to add >>> "Co-developed-by: Pierre-Hugues ..." above his s-o-b, which implies that >>> the two of you jointly came up with this and both certify the origin. >> >> BTW I can add that in the pending branch, no need to resend because of >> this. Just need guidance from Marc. > > I typed this patch all by myself with my grubby little paws. > You can drop PH's S-o-b. Thanks. Please check that my modifications in the pending branch are correct: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=3aec20a8e797b28d32e75291cc070d5913bf6dab https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=df5b4bec31b0736a453d507762c5b3d098d5c733 I can freely edit commits in the pending branch, it's just a temporary branch for testing.
On 07/05/2024 19:03, Kalle Valo wrote: > Thanks. Please check that my modifications in the pending branch are > correct: > > https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=3aec20a8e797b28d32e75291cc070d5913bf6dab > > https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=df5b4bec31b0736a453d507762c5b3d098d5c733 > > I can freely edit commits in the pending branch, it's just a temporary > branch for testing. Looks good to me. Bjorn, can you take patch 3 in your fix branch for msm? Regards
Marc Gonzalez <mgonzalez@freebox.fr> wrote: > The ath10k driver waits for an "MSA_READY" indicator > to complete initialization. If the indicator is not > received, then the device remains unusable. > > cf. ath10k_qmi_driver_event_work() > > Several msm8998-based devices are affected by this issue. > Oddly, it seems safe to NOT wait for the indicator, and > proceed immediately when QMI_EVENT_SERVER_ARRIVE. > > Jeff Johnson wrote: > > The feedback I received was "it might be ok to change all ath10k qmi > to skip waiting for msa_ready", and it was pointed out that ath11k > (and ath12k) do not wait for it. > > However with so many deployed devices, "might be ok" isn't a strong > argument for changing the default behavior. > > Kalle Valo first suggested setting a bit in firmware-5.bin to trigger > work-around in the driver. However, firmware-5.bin is parsed too late. > So we are stuck with a DT property. > > Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> > Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com> > Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com> > Acked-by: Rob Herring (Arm) <robh@kernel.org> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> 2 patches applied to ath-next branch of ath.git, thanks. 71b6e321e302 dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop 6d67d18014a8 wifi: ath10k: do not always wait for MSA_READY indicator
On 13/05/2024 16:16, Kalle Valo wrote: > Marc Gonzalez wrote: > >> The ath10k driver waits for an "MSA_READY" indicator >> to complete initialization. If the indicator is not >> received, then the device remains unusable. >> >> cf. ath10k_qmi_driver_event_work() >> >> Several msm8998-based devices are affected by this issue. >> Oddly, it seems safe to NOT wait for the indicator, and >> proceed immediately when QMI_EVENT_SERVER_ARRIVE. >> >> Jeff Johnson wrote: >> >> The feedback I received was "it might be ok to change all ath10k qmi >> to skip waiting for msa_ready", and it was pointed out that ath11k >> (and ath12k) do not wait for it. >> >> However with so many deployed devices, "might be ok" isn't a strong >> argument for changing the default behavior. >> >> Kalle Valo first suggested setting a bit in firmware-5.bin to trigger >> work-around in the driver. However, firmware-5.bin is parsed too late. >> So we are stuck with a DT property. >> >> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> >> Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com> >> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com> >> Acked-by: Rob Herring (Arm) <robh@kernel.org> >> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> > > 2 patches applied to ath-next branch of ath.git, thanks. > > 71b6e321e302 dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop > 6d67d18014a8 wifi: ath10k: do not always wait for MSA_READY indicator Hello Kalle, What version of Linux will these be included in? (I don't see them in v6.10-rc1. Are they considered a new feature, rather than a fix, and thus 6.11?) Hello Bjorn, Will you pick up patch 3 ? Regards
Marc Gonzalez <mgonzalez@freebox.fr> writes: > On 13/05/2024 16:16, Kalle Valo wrote: > >> Marc Gonzalez wrote: >> >>> The ath10k driver waits for an "MSA_READY" indicator >>> to complete initialization. If the indicator is not >>> received, then the device remains unusable. >>> >>> cf. ath10k_qmi_driver_event_work() >>> >>> Several msm8998-based devices are affected by this issue. >>> Oddly, it seems safe to NOT wait for the indicator, and >>> proceed immediately when QMI_EVENT_SERVER_ARRIVE. >>> >>> Jeff Johnson wrote: >>> >>> The feedback I received was "it might be ok to change all ath10k qmi >>> to skip waiting for msa_ready", and it was pointed out that ath11k >>> (and ath12k) do not wait for it. >>> >>> However with so many deployed devices, "might be ok" isn't a strong >>> argument for changing the default behavior. >>> >>> Kalle Valo first suggested setting a bit in firmware-5.bin to trigger >>> work-around in the driver. However, firmware-5.bin is parsed too late. >>> So we are stuck with a DT property. >>> >>> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> >>> Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com> >>> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com> >>> Acked-by: Rob Herring (Arm) <robh@kernel.org> >>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> >> >> 2 patches applied to ath-next branch of ath.git, thanks. >> >> 71b6e321e302 dt-bindings: net: wireless: ath10k: add >> qcom,no-msa-ready-indicator prop >> 6d67d18014a8 wifi: ath10k: do not always wait for MSA_READY indicator > > Hello Kalle, > What version of Linux will these be included in? > (I don't see them in v6.10-rc1. Are they considered > a new feature, rather than a fix, and thus 6.11?) Yeah, these commits will go to v6.11. Because of the multiple trees involved (ath-next -> wireless-next -> net-next -> linus) we need to have ath.git pull request ready well before the merge window opens and these commits missed the last pull request. Yes, we are slow :)
On 28/05/2024 12:11, Kalle Valo wrote: > Marc Gonzalez writes: > >> On 13/05/2024 16:16, Kalle Valo wrote: >> >>> Marc Gonzalez wrote: >>> >>>> The ath10k driver waits for an "MSA_READY" indicator >>>> to complete initialization. If the indicator is not >>>> received, then the device remains unusable. >>>> >>>> cf. ath10k_qmi_driver_event_work() >>>> >>>> Several msm8998-based devices are affected by this issue. >>>> Oddly, it seems safe to NOT wait for the indicator, and >>>> proceed immediately when QMI_EVENT_SERVER_ARRIVE. >>>> >>>> Jeff Johnson wrote: >>>> >>>> The feedback I received was "it might be ok to change all ath10k qmi >>>> to skip waiting for msa_ready", and it was pointed out that ath11k >>>> (and ath12k) do not wait for it. >>>> >>>> However with so many deployed devices, "might be ok" isn't a strong >>>> argument for changing the default behavior. >>>> >>>> Kalle Valo first suggested setting a bit in firmware-5.bin to trigger >>>> work-around in the driver. However, firmware-5.bin is parsed too late. >>>> So we are stuck with a DT property. >>>> >>>> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> >>>> Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com> >>>> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com> >>>> Acked-by: Rob Herring (Arm) <robh@kernel.org> >>>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> >>> >>> 2 patches applied to ath-next branch of ath.git, thanks. >>> >>> 71b6e321e302 dt-bindings: net: wireless: ath10k: add >>> qcom,no-msa-ready-indicator prop >>> 6d67d18014a8 wifi: ath10k: do not always wait for MSA_READY indicator >> >> Hello Kalle, >> What version of Linux will these be included in? >> (I don't see them in v6.10-rc1. Are they considered >> a new feature, rather than a fix, and thus 6.11?) > > Yeah, these commits will go to v6.11. Because of the multiple trees > involved (ath-next -> wireless-next -> net-next -> linus) we need to > have ath.git pull request ready well before the merge window opens and > these commits missed the last pull request. > > Yes, we are slow :) My understanding of the merging process was that - new features are queued for the next cycle, so vN+1-rc1, or vN+2-rc1 if the submission came too late (after ~rc6) in cycle N - fixes are queued for the fixes batch in the same cycle This patch series is handled like a feature rather than a fix? (To me, it fixed broken behavior in the FW, but I understand if the nature of the changes require a more prudent approach. Though they are disabled for everyone by default.) Regards
Marc Gonzalez <mgonzalez@freebox.fr> writes: > On 28/05/2024 12:11, Kalle Valo wrote: > >> Marc Gonzalez writes: >> >>> On 13/05/2024 16:16, Kalle Valo wrote: >>> >>>> Marc Gonzalez wrote: >>>> >>>>> The ath10k driver waits for an "MSA_READY" indicator >>>>> to complete initialization. If the indicator is not >>>>> received, then the device remains unusable. >>>>> >>>>> cf. ath10k_qmi_driver_event_work() >>>>> >>>>> Several msm8998-based devices are affected by this issue. >>>>> Oddly, it seems safe to NOT wait for the indicator, and >>>>> proceed immediately when QMI_EVENT_SERVER_ARRIVE. >>>>> >>>>> Jeff Johnson wrote: >>>>> >>>>> The feedback I received was "it might be ok to change all ath10k qmi >>>>> to skip waiting for msa_ready", and it was pointed out that ath11k >>>>> (and ath12k) do not wait for it. >>>>> >>>>> However with so many deployed devices, "might be ok" isn't a strong >>>>> argument for changing the default behavior. >>>>> >>>>> Kalle Valo first suggested setting a bit in firmware-5.bin to trigger >>>>> work-around in the driver. However, firmware-5.bin is parsed too late. >>>>> So we are stuck with a DT property. >>>>> >>>>> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> >>>>> Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com> >>>>> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com> >>>>> Acked-by: Rob Herring (Arm) <robh@kernel.org> >>>>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> >>>> >>>> 2 patches applied to ath-next branch of ath.git, thanks. >>>> >>>> 71b6e321e302 dt-bindings: net: wireless: ath10k: add >>>> qcom,no-msa-ready-indicator prop >>>> 6d67d18014a8 wifi: ath10k: do not always wait for MSA_READY indicator >>> >>> Hello Kalle, >>> What version of Linux will these be included in? >>> (I don't see them in v6.10-rc1. Are they considered >>> a new feature, rather than a fix, and thus 6.11?) >> >> Yeah, these commits will go to v6.11. Because of the multiple trees >> involved (ath-next -> wireless-next -> net-next -> linus) we need to >> have ath.git pull request ready well before the merge window opens and >> these commits missed the last pull request. >> >> Yes, we are slow :) > > My understanding of the merging process was that > > - new features are queued for the next cycle, > so vN+1-rc1, or vN+2-rc1 if the submission came too late (after ~rc6) in cycle N > > - fixes are queued for the fixes batch in the same cycle > > This patch series is handled like a feature rather than a fix? > (To me, it fixed broken behavior in the FW, but I understand > if the nature of the changes require a more prudent approach. > Though they are disabled for everyone by default.) So the path for ath10k/ath11k/ath12k fixes to current -rc release is: ath-current -> wireless -> net -> linus For new features going to the next release: ath-next -> wireless-next -> net-next -> (in merge window) linus To reduce conflicts between trees most of the patches I apply go to -next, I usually take only important regression fixes to -current. In this case I didn't even consider taking the patches to -current as there were changes in DT and I just assumed this is for -next. If you considered otherwise I didn't realise it, sorry about that. In future, if you think a patch should go to -current please mention it somewhere, preferably something like tagging it with "[PATCH wireless]" or "[PATCH ath-current]" etc. to document which tree it is for. Or just as a simple reply.
diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml index 9b3ef4bc37325..9070a41f7fc07 100644 --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml @@ -122,6 +122,11 @@ properties: Whether to skip executing an SCM call that reassigns the memory region ownership. + qcom,no-msa-ready-indicator: + type: boolean + description: + Don't wait for MSA_READY indicator to complete init. + qcom,smem-states: $ref: /schemas/types.yaml#/definitions/phandle-array description: State bits used by the AP to signal the WLAN Q6.