diff mbox series

[v3,1/3] dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop

Message ID 54ac2295-36b4-49fc-9583-a10db8d9d5d6@freebox.fr
State New
Headers show
Series Work around missing MSA_READY indicator for msm8998 devices | expand

Commit Message

Marc Gonzalez April 29, 2024, 2:04 p.m. UTC
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(+)

Comments

Bjorn Andersson April 30, 2024, 2:22 a.m. UTC | #1
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
>
Kalle Valo April 30, 2024, 4:06 a.m. UTC | #2
Bjorn Andersson <quic_bjorande@quicinc.com> writes:

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

> 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.
Rob Herring (Arm) May 7, 2024, 3:05 p.m. UTC | #3
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>
Kalle Valo May 13, 2024, 2:16 p.m. UTC | #4
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
Marc Gonzalez May 28, 2024, 9:54 a.m. UTC | #5
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
Kalle Valo May 28, 2024, 10:11 a.m. UTC | #6
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 :)
Marc Gonzalez May 28, 2024, 12:36 p.m. UTC | #7
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
Kalle Valo May 28, 2024, 1:34 p.m. UTC | #8
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 mbox series

Patch

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.