ath10k: Skip sending quiet mode cmd for WCN3990

PATCH In review Govind Singh 2 participants 2 messages Dec 11, 2018 – Dec 14, 2018 Qualcomm Atheros ATH10K wireless driver

lore

Share & tools

lore

Govind Singh

author

2018-12-11 05:39 UTC #0 lore raw

PATCH 1/1 ath10k: Skip sending quiet mode cmd for WCN3990

From: Rakesh Pillai <[email protected]>

HL2.0 firmware does not support setting quiet mode. If the host driver sends the quiet mode setting command to the HL2.0 firmware, it crashes with the below signature.

fatal error received: err_qdi.c:456:EX:wlan_process:1:WLAN RT:207a:PC=b001b4f0

The quiet mode command support is exposed by the firmware via thermal throttle wmi service. Enable ath10k thermal support if thermal throttle wmi service bit is set. 10X firmware version supports this feature by default, hence set this service bit by default.

Tested HW: WCN3990
Tested FW: WLAN.HL.2.0-01188-QCAHLSWMTPLZ-1

Co-developed-by Govind Singh <[email protected]>

Signed-off-by Rakesh Pillai <[email protected]>

Signed-off-by Govind Singh <[email protected]>

---
 drivers/net/wireless/ath/ath10k/core.c    | 19 +++++++++++++------
 drivers/net/wireless/ath/ath10k/debug.c   |  5 +++--
 drivers/net/wireless/ath/ath10k/mac.c     |  3 ++-
 drivers/net/wireless/ath/ath10k/wmi-tlv.h |  3 +++
 drivers/net/wireless/ath/ath10k/wmi.h     |  1 +
 5 files changed, 22 insertions(+), 9 deletions(-)

drivers/net/wireless/ath/ath10k/core.c +13 -6

index 22cbe9a2e646..2bdb632b7b1a 100644
@@ -2621,6 +2621,10 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
 		goto err_hif_stop;
 	}
 
+	if (test_bit(ATH10K_FW_FEATURE_WMI_10X,
+		     ar->normal_mode_fw.fw_file.fw_features)) {
+		set_bit(WMI_SERVICE_THERM_THROT, ar->wmi.svc_map);
+	}
 	/* Some firmware revisions do not properly set up hardware rx filter
 	 * registers.
 	 *

@@ -2919,11 +2923,13 @@ static void ath10k_core_register_work(struct work_struct *work)
 		goto err_debug_destroy;
 	}
 
-	status = ath10k_thermal_register(ar);
-	if (status) {
-		ath10k_err(ar, "could not register thermal device: %d\n",
-			   status);
-		goto err_spectral_destroy;
+	if (test_bit(WMI_SERVICE_THERM_THROT, ar->wmi.svc_map)) {
+		status = ath10k_thermal_register(ar);
+		if (status) {
+			ath10k_err(ar, "could not register thermal device: %d\n",
+				   status);
+			goto err_spectral_destroy;
+		}
 	}
 
 	set_bit(ATH10K_FLAG_CORE_REGISTERED, &ar->dev_flags);

@@ -2964,7 +2970,8 @@ void ath10k_core_unregister(struct ath10k *ar)
 	if (!test_bit(ATH10K_FLAG_CORE_REGISTERED, &ar->dev_flags))
 		return;
 
-	ath10k_thermal_unregister(ar);
+	if (test_bit(WMI_SERVICE_THERM_THROT, ar->wmi.svc_map))
+		ath10k_thermal_unregister(ar);
 	/* Stop spectral before unregistering from mac80211 to remove the
 	 * relayfs debugfs file cleanly. Otherwise the parent debugfs tree
 	 * would be already be free'd recursively, leading to a double free.

drivers/net/wireless/ath/ath10k/debug.c +3 -2

index 15964b374f68..02988fc378a1 100644
@@ -2578,8 +2578,9 @@ int ath10k_debug_register(struct ath10k *ar)
 	debugfs_create_file("pktlog_filter", 0644, ar->debug.debugfs_phy, ar,
 			    &fops_pktlog_filter);
 
-	debugfs_create_file("quiet_period", 0644, ar->debug.debugfs_phy, ar,
-			    &fops_quiet_period);
+	if (test_bit(WMI_SERVICE_THERM_THROT, ar->wmi.svc_map))
+		debugfs_create_file("quiet_period", 0644, ar->debug.debugfs_phy, ar,
+				    &fops_quiet_period);
 
 	debugfs_create_file("tpc_stats", 0400, ar->debug.debugfs_phy, ar,
 			    &fops_tpc_stats);

drivers/net/wireless/ath/ath10k/mac.c +2 -1

index c5130fa264eb..b8a5ec40b949 100644
@@ -4800,7 +4800,8 @@ static int ath10k_start(struct ieee80211_hw *hw)
 	ath10k_regd_update(ar);
 
 	ath10k_spectral_start(ar);
-	ath10k_thermal_set_throttling(ar);
+	if (test_bit(WMI_SERVICE_THERM_THROT, ar->wmi.svc_map))
+		ath10k_thermal_set_throttling(ar);
 
 	ar->radar_conf_state = ATH10K_RADAR_CONFIRMATION_IDLE;

drivers/net/wireless/ath/ath10k/wmi-tlv.h +3 -0

index bf8a4320c39c..e07e9907e355 100644
@@ -1564,6 +1564,9 @@ wmi_tlv_svc_map_ext(const __le32 *in, unsigned long *out, size_t len)
 	SVCMAP(WMI_TLV_SERVICE_SPOOF_MAC_SUPPORT,
 	       WMI_SERVICE_SPOOF_MAC_SUPPORT,
 	       WMI_TLV_MAX_SERVICE);
+	SVCMAP(WMI_TLV_SERVICE_THERM_THROT,
+	       WMI_SERVICE_THERM_THROT,
+	       WMI_TLV_MAX_SERVICE);
 }
 
 #undef SVCMAP

drivers/net/wireless/ath/ath10k/wmi.h +1 -0

index 58e33ab9e0e9..66222eeaba4c 100644
@@ -205,6 +205,7 @@ enum wmi_service {
 	WMI_SERVICE_SPOOF_MAC_SUPPORT,
 	WMI_SERVICE_TX_DATA_ACK_RSSI,
 	WMI_SERVICE_VDEV_DIFFERENT_BEACON_INTERVAL_SUPPORT,
+	WMI_SERVICE_THERM_THROT,
 
 	/* keep last */
 	WMI_SERVICE_MAX,

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Kalle Valo

2018-12-14 10:09 UTC #1 lore raw

Quoted 9 lines · Govind Singh

From: Rakesh Pillai <[email protected]>
HL2.0 firmware does not support setting quiet mode. If the host driver sends the quiet mode setting command to the HL2.0 firmware, it crashes with the below signature.
fatal error received: err_qdi.c:456:EX:wlan_process:1:WLAN RT:207a:PC=b001b4f0

Quoted 8 lines

The quiet mode command support is exposed by the firmware via thermal throttle wmi service. Enable ath10k thermal support if thermal throttle wmi service bit is set. 10X firmware version supports this feature by default, hence set this service bit by default.
Tested HW: WCN3990
Tested FW: WLAN.HL.2.0-01188-QCAHLSWMTPLZ-1

So this would also need testing with QCA988X or QCA9884 family of devices, I don't think this patch would have worked as is.

Quoted 3 lines

Co-developed-by Govind Singh <[email protected]>
Signed-off-by Rakesh Pillai <[email protected]>
Signed-off-by Govind Singh <[email protected]>

I had few comments below which I fixed myself. I'll send v2 soon.

drivers/net/wireless/ath/ath10k/core.c int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode, · line 2627

Quoted 8 lines

drivers/net/wireless/ath/ath10k/core.c +4 -0

@@ -2621,6 +2621,10 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
 		goto err_hif_stop;
 	}
 
+	if (test_bit(ATH10K_FW_FEATURE_WMI_10X,
+		     ar->normal_mode_fw.fw_file.fw_features)) {
+		set_bit(WMI_SERVICE_THERM_THROT, ar->wmi.svc_map);
+	}

I don't think testing for ATH10K_FW_FEATURE_WMI_10X works, that's a really old and deprecated flag. wmi_op_version should be more reliable.

Also I refactored this to it's own function.

Quoted 10 lines

drivers/net/wireless/ath/ath10k/core.c +0 -3

 	/* Some firmware revisions do not properly set up hardware rx filter
 	 * registers.
 	 *

@@ -2919,11 +2923,13 @@ static void ath10k_core_register_work(struct work_struct *work)
 		goto err_debug_destroy;
 	}
 
-	status = ath10k_thermal_register(ar);
-	if (status) {
-		ath10k_err(ar, "could not register thermal device: %d\n",

drivers/net/wireless/ath/ath10k/core.c static void ath10k_core_register_work(struct work_struct *work) · line 2933

Quoted 10 lines

drivers/net/wireless/ath/ath10k/core.c +7 -2

-			   status);
-		goto err_spectral_destroy;
+	if (test_bit(WMI_SERVICE_THERM_THROT, ar->wmi.svc_map)) {
+		status = ath10k_thermal_register(ar);
+		if (status) {
+			ath10k_err(ar, "could not register thermal device: %d\n",
+				   status);
+			goto err_spectral_destroy;
+		}
 	}

The code becomes cleaner if the test is in thermal.c so I moved it there.

drivers/net/wireless/ath/ath10k/core.c void ath10k_core_unregister(struct ath10k *ar) · line 2974

Quoted 7 lines

drivers/net/wireless/ath/ath10k/core.c +2 -1

@@ -2964,7 +2970,8 @@ void ath10k_core_unregister(struct ath10k *ar)
 	if (!test_bit(ATH10K_FLAG_CORE_REGISTERED, &ar->dev_flags))
 		return;
 
-	ath10k_thermal_unregister(ar);
+	if (test_bit(WMI_SERVICE_THERM_THROT, ar->wmi.svc_map))
+		ath10k_thermal_unregister(ar);

Same here.

drivers/net/wireless/ath/ath10k/mac.c static int ath10k_start(struct ieee80211_hw *hw) · line 4804

Quoted 9 lines

drivers/net/wireless/ath/ath10k/mac.c +2 -1

@@ -4800,7 +4800,8 @@ static int ath10k_start(struct ieee80211_hw *hw)
 	ath10k_regd_update(ar);
 
 	ath10k_spectral_start(ar);
-	ath10k_thermal_set_throttling(ar);
+	if (test_bit(WMI_SERVICE_THERM_THROT, ar->wmi.svc_map))
+		ath10k_thermal_set_throttling(ar);

And here.

Signature · 1 line

Kalle Valo