diff mbox series

[v3,2/3] wifi: ath10k: do not always wait for MSA_READY indicator

Message ID 23540303-5816-45d5-a1af-5f09d645a73b@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:06 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.

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 <phhusson@freebox.fr>
Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>
---
 drivers/net/wireless/ath/ath10k/qmi.c | 11 +++++++++++
 drivers/net/wireless/ath/ath10k/qmi.h |  1 +
 2 files changed, 12 insertions(+)

Comments

Bjorn Andersson April 30, 2024, 2:24 a.m. UTC | #1
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 <phhusson@freebox.fr>
> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>

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

Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com>

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
> 
>
Marc Gonzalez April 30, 2024, 11:15 a.m. UTC | #2
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 <phhusson@freebox.fr>
>> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>
> 
> As with patch 1, please address the s-o-b and accept my:
> 
> Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com>

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
Jeff Johnson April 30, 2024, 2:39 p.m. UTC | #3
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 <phhusson@freebox.fr>
> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>
> ---
with SOB cleaned up:
Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
diff mbox series

Patch

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;
 };