2024-04-29 14:08:31

by Marc Gonzalez

[permalink] [raw]
Subject: [PATCH v3 0/3] Work around missing MSA_READY indicator for msm8998 devices

Work around missing MSA_READY indicator in ath10k driver
(apply work-around for all msm8998 devices)

CHANGELOG v3
- Add a paragraph in binding commit to explain why we use
a DT property instead of a firmware feature bit.
- Warn if the "no_msa_ready_indicator" property is true,
but we actually receive the indicator.

Marc Gonzalez (3):
dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop
wifi: ath10k: do not always wait for MSA_READY indicator
arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi

Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml | 5 +++++
arch/arm64/boot/dts/qcom/msm8998.dtsi | 1 +
drivers/net/wireless/ath/ath10k/qmi.c | 11 +++++++++++
drivers/net/wireless/ath/ath10k/qmi.h | 1 +
4 files changed, 18 insertions(+)

--
2.34.1


2024-04-29 14:08:44

by Marc Gonzalez

[permalink] [raw]
Subject: [PATCH v3 2/3] wifi: ath10k: do not always wait for MSA_READY indicator

The ath10k driver waits for an "MSA_READY" indicator
to complete initialization. If the indicator is not
received, then the device remains unusable.

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.

fw_version 0x100204b2
fw_build_timestamp 2019-09-04 03:01
fw_build_id QC_IMAGE_VERSION_STRING=WLAN.HL.1.0-01202-QCAHLSWMTPLZ-1.221523.2

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.

Signed-off-by: Pierre-Hugues Husson <[email protected]>
Signed-off-by: Marc Gonzalez <[email protected]>
---
drivers/net/wireless/ath/ath10k/qmi.c | 11 +++++++++++
drivers/net/wireless/ath/ath10k/qmi.h | 1 +
2 files changed, 12 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/qmi.c b/drivers/net/wireless/ath/ath10k/qmi.c
index 38e939f572a9e..f1f33af0170a0 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.c
+++ b/drivers/net/wireless/ath/ath10k/qmi.c
@@ -1040,6 +1040,10 @@ static void ath10k_qmi_driver_event_work(struct work_struct *work)
switch (event->type) {
case ATH10K_QMI_EVENT_SERVER_ARRIVE:
ath10k_qmi_event_server_arrive(qmi);
+ if (qmi->no_msa_ready_indicator) {
+ ath10k_info(ar, "qmi not waiting for msa_ready indicator");
+ ath10k_qmi_event_msa_ready(qmi);
+ }
break;
case ATH10K_QMI_EVENT_SERVER_EXIT:
ath10k_qmi_event_server_exit(qmi);
@@ -1048,6 +1052,10 @@ static void ath10k_qmi_driver_event_work(struct work_struct *work)
ath10k_qmi_event_fw_ready_ind(qmi);
break;
case ATH10K_QMI_EVENT_MSA_READY_IND:
+ if (qmi->no_msa_ready_indicator) {
+ ath10k_warn(ar, "qmi unexpected msa_ready indicator");
+ break;
+ }
ath10k_qmi_event_msa_ready(qmi);
break;
default:
@@ -1077,6 +1085,9 @@ int ath10k_qmi_init(struct ath10k *ar, u32 msa_size)
if (of_property_read_bool(dev->of_node, "qcom,msa-fixed-perm"))
qmi->msa_fixed_perm = true;

+ if (of_property_read_bool(dev->of_node, "qcom,no-msa-ready-indicator"))
+ qmi->no_msa_ready_indicator = true;
+
ret = qmi_handle_init(&qmi->qmi_hdl,
WLFW_BDF_DOWNLOAD_REQ_MSG_V01_MAX_MSG_LEN,
&ath10k_qmi_ops, qmi_msg_handler);
diff --git a/drivers/net/wireless/ath/ath10k/qmi.h b/drivers/net/wireless/ath/ath10k/qmi.h
index 89464239fe96a..0816eb4e4a18a 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.h
+++ b/drivers/net/wireless/ath/ath10k/qmi.h
@@ -107,6 +107,7 @@ struct ath10k_qmi {
char fw_build_timestamp[MAX_TIMESTAMP_LEN + 1];
struct ath10k_qmi_cal_data cal_data[MAX_NUM_CAL_V01];
bool msa_fixed_perm;
+ bool no_msa_ready_indicator;
enum ath10k_qmi_state state;
};

--
2.34.1


2024-04-29 14:34:39

by Marc Gonzalez

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

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 <[email protected]>
Signed-off-by: Marc Gonzalez <[email protected]>
---
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


2024-04-29 14:34:49

by Marc Gonzalez

[permalink] [raw]
Subject: [PATCH v3 3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi

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.

cf. also
https://wiki.postmarketos.org/wiki/Qualcomm_Snapdragon_835_(MSM8998)#WLAN

Signed-off-by: Marc Gonzalez <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8998.dtsi | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index 67b8374ddf02f..4e6245095adfc 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -3234,6 +3234,7 @@ wifi: wifi@18800000 {
iommus = <&anoc2_smmu 0x1900>,
<&anoc2_smmu 0x1901>;
qcom,snoc-host-cap-8bit-quirk;
+ qcom,no-msa-ready-indicator;
};
};
};
--
2.34.1


2024-04-29 23:24:51

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] Work around missing MSA_READY indicator for msm8998 devices

On 29/04/2024 15:01, Marc Gonzalez wrote:
> Work around missing MSA_READY indicator in ath10k driver
> (apply work-around for all msm8998 devices)
>
> CHANGELOG v3
> - Add a paragraph in binding commit to explain why we use
> a DT property instead of a firmware feature bit.
> - Warn if the "no_msa_ready_indicator" property is true,
> but we actually receive the indicator.
>
> Marc Gonzalez (3):
> dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop
> wifi: ath10k: do not always wait for MSA_READY indicator
> arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi
>
> Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml | 5 +++++
> arch/arm64/boot/dts/qcom/msm8998.dtsi | 1 +
> drivers/net/wireless/ath/ath10k/qmi.c | 11 +++++++++++
> drivers/net/wireless/ath/ath10k/qmi.h | 1 +
> 4 files changed, 18 insertions(+)
>

I wonder if you could infer the workaround based on firmware version,
instead of kernel passed flag ?

---
bod

2024-04-30 02:19:14

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] Work around missing MSA_READY indicator for msm8998 devices

On Tue, Apr 30, 2024 at 12:24:40AM +0100, Bryan O'Donoghue wrote:
> On 29/04/2024 15:01, Marc Gonzalez wrote:
> > Work around missing MSA_READY indicator in ath10k driver
> > (apply work-around for all msm8998 devices)
> >
> > CHANGELOG v3
> > - Add a paragraph in binding commit to explain why we use
> > a DT property instead of a firmware feature bit.
> > - Warn if the "no_msa_ready_indicator" property is true,
> > but we actually receive the indicator.
> >
> > Marc Gonzalez (3):
> > dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop
> > wifi: ath10k: do not always wait for MSA_READY indicator
> > arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi
> >
> > Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml | 5 +++++
> > arch/arm64/boot/dts/qcom/msm8998.dtsi | 1 +
> > drivers/net/wireless/ath/ath10k/qmi.c | 11 +++++++++++
> > drivers/net/wireless/ath/ath10k/qmi.h | 1 +
> > 4 files changed, 18 insertions(+)
> >
>
> I wonder if you could infer the workaround based on firmware version,
> instead of kernel passed flag ?
>

It's been a while, but I attempted this for the similar workaround for
SDM845 RB3 et al. I vaguely remember concluding that the different
devices I worked on, had firmware from different branches without any
suitable version scheme to use for such comparison - which is why we
have "qcom,snoc-host-cap-8bit-quirk;" in those nodes (which apparently
is not documented in the yaml).

I'd also rather see us ask Marc spending time on some more fruitful
exercise...

Regards,
Bjorn

2024-04-30 02:24:37

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] wifi: ath10k: do not always wait for MSA_READY indicator

On Mon, Apr 29, 2024 at 04:06:29PM +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.
>
> 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.
>
> fw_version 0x100204b2
> fw_build_timestamp 2019-09-04 03:01
> fw_build_id QC_IMAGE_VERSION_STRING=WLAN.HL.1.0-01202-QCAHLSWMTPLZ-1.221523.2
>
> 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.
>
> Signed-off-by: Pierre-Hugues Husson <[email protected]>
> Signed-off-by: Marc Gonzalez <[email protected]>

As with patch 1, please address the s-o-b and accept my:

Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
> drivers/net/wireless/ath/ath10k/qmi.c | 11 +++++++++++
> drivers/net/wireless/ath/ath10k/qmi.h | 1 +
> 2 files changed, 12 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/qmi.c b/drivers/net/wireless/ath/ath10k/qmi.c
> index 38e939f572a9e..f1f33af0170a0 100644
> --- a/drivers/net/wireless/ath/ath10k/qmi.c
> +++ b/drivers/net/wireless/ath/ath10k/qmi.c
> @@ -1040,6 +1040,10 @@ static void ath10k_qmi_driver_event_work(struct work_struct *work)
> switch (event->type) {
> case ATH10K_QMI_EVENT_SERVER_ARRIVE:
> ath10k_qmi_event_server_arrive(qmi);
> + if (qmi->no_msa_ready_indicator) {
> + ath10k_info(ar, "qmi not waiting for msa_ready indicator");
> + ath10k_qmi_event_msa_ready(qmi);
> + }
> break;
> case ATH10K_QMI_EVENT_SERVER_EXIT:
> ath10k_qmi_event_server_exit(qmi);
> @@ -1048,6 +1052,10 @@ static void ath10k_qmi_driver_event_work(struct work_struct *work)
> ath10k_qmi_event_fw_ready_ind(qmi);
> break;
> case ATH10K_QMI_EVENT_MSA_READY_IND:
> + if (qmi->no_msa_ready_indicator) {
> + ath10k_warn(ar, "qmi unexpected msa_ready indicator");
> + break;
> + }
> ath10k_qmi_event_msa_ready(qmi);
> break;
> default:
> @@ -1077,6 +1085,9 @@ int ath10k_qmi_init(struct ath10k *ar, u32 msa_size)
> if (of_property_read_bool(dev->of_node, "qcom,msa-fixed-perm"))
> qmi->msa_fixed_perm = true;
>
> + if (of_property_read_bool(dev->of_node, "qcom,no-msa-ready-indicator"))
> + qmi->no_msa_ready_indicator = true;
> +
> ret = qmi_handle_init(&qmi->qmi_hdl,
> WLFW_BDF_DOWNLOAD_REQ_MSG_V01_MAX_MSG_LEN,
> &ath10k_qmi_ops, qmi_msg_handler);
> diff --git a/drivers/net/wireless/ath/ath10k/qmi.h b/drivers/net/wireless/ath/ath10k/qmi.h
> index 89464239fe96a..0816eb4e4a18a 100644
> --- a/drivers/net/wireless/ath/ath10k/qmi.h
> +++ b/drivers/net/wireless/ath/ath10k/qmi.h
> @@ -107,6 +107,7 @@ struct ath10k_qmi {
> char fw_build_timestamp[MAX_TIMESTAMP_LEN + 1];
> struct ath10k_qmi_cal_data cal_data[MAX_NUM_CAL_V01];
> bool msa_fixed_perm;
> + bool no_msa_ready_indicator;
> enum ath10k_qmi_state state;
> };
>
> --
> 2.34.1
>
>

2024-04-30 02:24:43

by Bjorn Andersson

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

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 <[email protected]>
> Signed-off-by: Marc Gonzalez <[email protected]>

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 <[email protected]>

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
>

2024-04-30 04:07:10

by Kalle Valo

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

Bjorn Andersson <[email protected]> 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 <[email protected]>
>> Signed-off-by: Marc Gonzalez <[email protected]>
>
> 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 <[email protected]>

Thanks, I'll then add this as well.

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

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

2024-04-30 11:11:04

by Marc Gonzalez

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

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 <[email protected]>
>>> Signed-off-by: Marc Gonzalez <[email protected]>
>>
>> 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 <[email protected]>
>
> Thanks, I'll then add this as well.

Cool. Almost there :)


2024-04-30 11:16:17

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] wifi: ath10k: do not always wait for MSA_READY indicator

On 30/04/2024 04:24, Bjorn Andersson wrote:

> On Mon, Apr 29, 2024 at 04:06:29PM +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.
>>
>> 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.
>>
>> fw_version 0x100204b2
>> fw_build_timestamp 2019-09-04 03:01
>> fw_build_id QC_IMAGE_VERSION_STRING=WLAN.HL.1.0-01202-QCAHLSWMTPLZ-1.221523.2
>>
>> 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.
>>
>> Signed-off-by: Pierre-Hugues Husson <[email protected]>
>> Signed-off-by: Marc Gonzalez <[email protected]>
>
> As with patch 1, please address the s-o-b and accept my:
>
> Reviewed-by: Bjorn Andersson <[email protected]>

As with patch 1,
I typed this patch all by myself with my grubby little paws.
You can drop PH's S-o-b.

Regards


2024-04-30 14:43:37

by Jeff Johnson

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

On 4/29/2024 7:04 AM, 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 <[email protected]>
> Signed-off-by: Marc Gonzalez <[email protected]>
> ---
> 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.
with SOB cleaned up:
Acked-by: Jeff Johnson <[email protected]>


2024-04-30 14:44:11

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] wifi: ath10k: do not always wait for MSA_READY indicator

On 4/29/2024 7:06 AM, 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.
>
> 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.
>
> fw_version 0x100204b2
> fw_build_timestamp 2019-09-04 03:01
> fw_build_id QC_IMAGE_VERSION_STRING=WLAN.HL.1.0-01202-QCAHLSWMTPLZ-1.221523.2
>
> 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.
>
> Signed-off-by: Pierre-Hugues Husson <[email protected]>
> Signed-off-by: Marc Gonzalez <[email protected]>
> ---
with SOB cleaned up:
Acked-by: Jeff Johnson <[email protected]>


2024-04-30 14:50:15

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi

On 4/29/2024 7:07 AM, 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.
>
> cf. also
> https://wiki.postmarketos.org/wiki/Qualcomm_Snapdragon_835_(MSM8998)#WLAN
>
> Signed-off-by: Marc Gonzalez <[email protected]>
Acked-by: Jeff Johnson <[email protected]>


2024-05-06 10:39:41

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi

On 29/04/2024 16:07, 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.
>
> cf. also
> https://wiki.postmarketos.org/wiki/Qualcomm_Snapdragon_835_(MSM8998)#WLAN
>
> Signed-off-by: Marc Gonzalez <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/msm8998.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> index 67b8374ddf02f..4e6245095adfc 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> @@ -3234,6 +3234,7 @@ wifi: wifi@18800000 {
> iommus = <&anoc2_smmu 0x1900>,
> <&anoc2_smmu 0x1901>;
> qcom,snoc-host-cap-8bit-quirk;
> + qcom,no-msa-ready-indicator;
> };
> };
> };


Bjorn,

This patch is supposed to go through your tree, right?

Regards


2024-05-06 12:17:25

by Kalle Valo

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

Marc Gonzalez <[email protected]> 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 <[email protected]>
>>>> Signed-off-by: Marc Gonzalez <[email protected]>
>>>
>>> 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 <[email protected]>
>>
>> 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.

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

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

2024-05-07 11:07:36

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi



On 5/6/24 12:39, Marc Gonzalez wrote:
> On 29/04/2024 16:07, 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.
>>
>> cf. also
>> https://wiki.postmarketos.org/wiki/Qualcomm_Snapdragon_835_(MSM8998)#WLAN
>>
>> Signed-off-by: Marc Gonzalez <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/msm8998.dtsi | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
>> index 67b8374ddf02f..4e6245095adfc 100644
>> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
>> @@ -3234,6 +3234,7 @@ wifi: wifi@18800000 {
>> iommus = <&anoc2_smmu 0x1900>,
>> <&anoc2_smmu 0x1901>;
>> qcom,snoc-host-cap-8bit-quirk;
>> + qcom,no-msa-ready-indicator;
>> };
>> };
>> };
>
>
> Bjorn,
>
> This patch is supposed to go through your tree, right?

Reviewed-by: Konrad Dybcio <[email protected]>

Konrad

2024-05-07 15:05:43

by Rob Herring

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


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 <[email protected]>
> Signed-off-by: Marc Gonzalez <[email protected]>
> ---
> Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>

Acked-by: Rob Herring (Arm) <[email protected]>


2024-05-07 17:03:21

by Kalle Valo

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

Marc Gonzalez <[email protected]> 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 <[email protected]>
>>>> Signed-off-by: Marc Gonzalez <[email protected]>
>>>
>>> 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.

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

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