[ath9k-devel] [PATCH 0/7] ath10k: random fixes/cleanups

[ath9k-devel] [PATCH 0/7] ath10k: random fixes/cleanups

[Michal Kazior]

Michal Kazior (7):
  ath10k: add error handling to some functions
  ath10k: remove unnecessary check
  ath10k: remove unnecessary prints
  ath10k: cleanup warning/error prints
  ath10k: move htc creation error handling
  ath10k: cleanup pci suspend
  ath10k: use ath10k_err for non-recoverable failures

 drivers/net/wireless/ath/ath10k/core.c   |   19 ++++++++++++++-----
 drivers/net/wireless/ath/ath10k/debug.c  |    5 ++++-
 drivers/net/wireless/ath/ath10k/htc.c    |    6 ++----
 drivers/net/wireless/ath/ath10k/htt_rx.c |    2 +-
 drivers/net/wireless/ath/ath10k/mac.c    |   24 ++++++++++++++----------
 drivers/net/wireless/ath/ath10k/pci.c    |   30 +++++++++++++++++++-----------
 drivers/net/wireless/ath/ath10k/wmi.c    |   23 ++++++-----------------
 7 files changed, 60 insertions(+), 49 deletions(-)

-- 
1.7.9.5

[ath9k-devel] [PATCH 1/7] ath10k: add error handling to some functions

[Michal Kazior]

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c  |   10 +++++++++-
 drivers/net/wireless/ath/ath10k/debug.c |    5 ++++-
 drivers/net/wireless/ath/ath10k/mac.c   |   19 ++++++++++++-------
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 2be47f3..1c98bbc 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -575,7 +575,11 @@ int ath10k_core_register(struct ath10k *ar)
 	if (status)
 		goto err_disconnect_htc;
 
-	ath10k_wmi_cmd_init(ar);
+	status = ath10k_wmi_cmd_init(ar);
+	if (status) {
+		ath10k_err("could not send WMI init command (%d)\n", status);
+		goto err_disconnect_htc;
+	}
 
 	status = ath10k_wmi_wait_for_unified_ready(ar);
 	if (status <= 0) {
@@ -635,6 +639,8 @@ int ath10k_core_target_suspend(struct ath10k *ar)
 	ath10k_dbg(ATH10K_DBG_CORE, "%s: called", __func__);
 
 	ret = ath10k_wmi_pdev_suspend_target(ar);
+	if (ret)
+		ath10k_warn("could not suspend target (%d)\n", ret);
 
 	return ret;
 }
@@ -647,6 +653,8 @@ int ath10k_core_target_resume(struct ath10k *ar)
 	ath10k_dbg(ATH10K_DBG_CORE, "%s: called", __func__);
 
 	ret = ath10k_wmi_pdev_resume_target(ar);
+	if (ret)
+		ath10k_warn("could not resume target (%d)\n", ret);
 
 	return ret;
 }
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 040cdcd..188a4f9 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -278,6 +278,7 @@ static ssize_t ath10k_read_fw_stats(struct file *file, char __user *user_buf,
 	ssize_t ret_cnt;
 	long left;
 	int i;
+	int ret;
 
 	fw_stats = &ar->debug.target_stats;
 
@@ -285,7 +286,9 @@ static ssize_t ath10k_read_fw_stats(struct file *file, char __user *user_buf,
 	if (!buf)
 		return -ENOMEM;
 
-	if (ath10k_wmi_request_stats(ar, WMI_REQUEST_PEER_STAT)) {
+	ret = ath10k_wmi_request_stats(ar, WMI_REQUEST_PEER_STAT);
+	if (ret) {
+		ath10k_warn("could not request stats (%d)\n", ret);
 		kfree(buf);
 		return -EIO;
 	}
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index b5cc3c8..7af5c40 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1077,7 +1077,7 @@ static int ath10k_station_disassoc(struct ath10k *ar, struct ath10k_vif *arvif,
 /* Regulatory */
 /**************/
 
-static void ath10k_update_channel_list(struct ath10k *ar)
+static int ath10k_update_channel_list(struct ath10k *ar)
 {
 	struct ieee80211_hw *hw = ar->hw;
 	struct ieee80211_supported_band **bands;
@@ -1087,6 +1087,7 @@ static void ath10k_update_channel_list(struct ath10k *ar)
 	struct wmi_channel_arg *ch;
 	bool passive;
 	int len;
+	int ret;
 	int i;
 
 	bands = hw->wiphy->bands;
@@ -1105,10 +1106,8 @@ static void ath10k_update_channel_list(struct ath10k *ar)
 
 	len = sizeof(struct wmi_channel_arg) * arg.n_channels;
 	arg.channels = kzalloc(len, GFP_KERNEL);
-	if (!arg.channels) {
-		ath10k_warn("%s alloc failed\n", __func__);
-		return;
-	}
+	if (!arg.channels)
+		return -ENOMEM;
 
 	ch = arg.channels;
 	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
@@ -1153,8 +1152,10 @@ static void ath10k_update_channel_list(struct ath10k *ar)
 		}
 	}
 
-	ath10k_wmi_scan_chan_list(ar, &arg);
+	ret = ath10k_wmi_scan_chan_list(ar, &arg);
 	kfree(arg.channels);
+
+	return ret;
 }
 
 static void ath10k_reg_notifier(struct wiphy *wiphy,
@@ -1162,9 +1163,13 @@ static void ath10k_reg_notifier(struct wiphy *wiphy,
 {
 	struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
 	struct ath10k *ar = hw->priv;
+	int ret;
 
 	ath_reg_notifier_apply(wiphy, request, &ath10k_common(ar)->regulatory);
-	ath10k_update_channel_list(ar);
+
+	ret = ath10k_update_channel_list(ar);
+	if (ret)
+		ath10k_warn("could not update channel list (%d)\n", ret);
 }
 
 /***************/
-- 
1.7.9.5

[ath9k-devel] [PATCH 2/7] ath10k: remove unnecessary check

[Michal Kazior]

rx skbuff is never NULL.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/wmi.c |    5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 3b819c8..2bb9411 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1084,11 +1084,6 @@ static void ath10k_wmi_process_rx(struct ath10k *ar, struct sk_buff *skb)
 
 	event_id = MS(__le32_to_cpu(cmd_hdr->cmd_id), WMI_CMD_HDR_CMD_ID);
 
-	if (!skb) {
-		ath10k_warn("%s: missing skb\n", __func__);
-		return;
-	}
-
 	/* some events require to be handled ASAP
 	 * thus can't be defered to a worker thread */
 	switch (event_id) {
-- 
1.7.9.5

[ath9k-devel] [PATCH 3/7] ath10k: remove unnecessary prints

[Michal Kazior]

Error codes are already propagated and printed.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/wmi.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 2bb9411..678f201 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -92,10 +92,8 @@ static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb,
 	int status;
 	u32 cmd = 0;
 
-	if (skb_push(skb, sizeof(struct wmi_cmd_hdr)) == NULL) {
-		ath10k_warn("%s: skb_push failed\n", __func__);
+	if (skb_push(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
 		return -ENOMEM;
-	}
 
 	cmd |= SM(cmd_id, WMI_CMD_HDR_CMD_ID);
 
@@ -118,10 +116,10 @@ static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb,
 	if (status) {
 		dev_kfree_skb_any(skb);
 		atomic_dec(&ar->wmi.pending_tx_count);
-		ath10k_warn("%s(): htc_send failed (%d)\n", __func__, status);
+		return status;
 	}
 
-	return status;
+	return 0;
 }
 
 static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
@@ -1221,10 +1219,8 @@ int ath10k_wmi_pdev_resume_target(struct ath10k *ar)
 	struct sk_buff *skb;
 
 	skb = ath10k_wmi_alloc_skb(0);
-	if (skb == NULL) {
-		ath10k_warn("%s: ath10k_wmi_alloc_skb() failed\n", __func__);
+	if (skb == NULL)
 		return -ENOMEM;
-	}
 
 	return ath10k_wmi_cmd_send(ar, skb, WMI_PDEV_RESUME_CMDID);
 }
-- 
1.7.9.5

[ath9k-devel] [PATCH 4/7] ath10k: cleanup warning/error prints

[Michal Kazior]

Gets rid of __func__ usage except debug prints.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c |    2 +-
 drivers/net/wireless/ath/ath10k/mac.c    |    5 ++---
 drivers/net/wireless/ath/ath10k/pci.c    |    7 +++----
 drivers/net/wireless/ath/ath10k/wmi.c    |    6 ++----
 4 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index cdedc5f..fd8dca0 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -285,7 +285,7 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 		ath10k_warn("htt rx ring is empty!\n");
 
 	if (htt->rx_confused) {
-		ath10k_warn("%s: htt failure: cannot rx\n", __func__);
+		ath10k_warn("htt is confused. refusing rx\n");
 		return 0;
 	}
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 7af5c40..bb5d9db 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1446,8 +1446,7 @@ static void ath10k_abort_scan(struct ath10k *ar)
 
 	ret = ath10k_wmi_stop_scan(ar, &arg);
 	if (ret)
-		ath10k_warn("%s: ath10k_wmi_stop_scan failed (%d)\n",
-			    __func__, ret);
+		ath10k_warn("could not submit wmi stop scan (%d)\n", ret);
 
 	ath10k_wmi_flush_tx(ar);
 
@@ -1457,7 +1456,7 @@ static void ath10k_abort_scan(struct ath10k *ar)
 
 	spin_lock_bh(&ar->data_lock);
 	if (ar->scan.in_progress) {
-		ath10k_warn("%s: could not stop scan (%d)\n", __func__, ret);
+		ath10k_warn("could not stop scan. its still in progress\n");
 		ar->scan.in_progress = false;
 		ath10k_offchan_tx_purge(ar);
 	}
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index ca09a44..a1e7650 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -926,8 +926,8 @@ static void ath10k_pci_process_ce(struct ath10k *ar)
 				cb->rx_completion(ar, skb,
 						  compl->pipe_info->pipe_num);
 			} else {
-				ath10k_warn("%s: rxed more than expected (nbytes %d, max %d)",
-					    __func__, nbytes,
+				ath10k_warn("rxed more than expected (nbytes %d, max %d)",
+					    nbytes,
 					    skb->len + skb_tailroom(skb));
 			}
 		}
@@ -1766,8 +1766,7 @@ static irqreturn_t ath10k_pci_per_engine_handler(int irq, void *arg)
 	int ce_id = irq - ar_pci->pdev->irq - MSI_ASSIGN_CE_INITIAL;
 
 	if (ce_id < 0 || ce_id > ARRAY_SIZE(ar_pci->pipe_info)) {
-		ath10k_warn("%s: unexpected/invalid irq %d ce_id %d\n",
-			    __func__, irq, ce_id);
+		ath10k_warn("unexpected/invalid irq %d ce_id %d\n", irq, ce_id);
 		return IRQ_HANDLED;
 	}
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 678f201..a911dd7 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -104,7 +104,6 @@ static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb,
 	    WMI_MAX_PENDING_TX_COUNT) {
 		/* avoid using up memory when FW hangs */
 		atomic_dec(&ar->wmi.pending_tx_count);
-		ath10k_warn("%s: too many tx packets pending\n", __func__);
 		return -EBUSY;
 	}
 
@@ -681,7 +680,7 @@ static void ath10k_wmi_event_host_swba(struct ath10k *ar, struct sk_buff *skb)
 		i++;
 
 		if (i >= WMI_MAX_AP_VDEV) {
-			ath10k_warn("%s: incorect vdev map\n", __func__);
+			ath10k_warn("swba has corrupted vdev map\n");
 			break;
 		}
 
@@ -706,8 +705,7 @@ static void ath10k_wmi_event_host_swba(struct ath10k *ar, struct sk_buff *skb)
 
 		arvif = ath10k_get_arvif(ar, vdev_id);
 		if (arvif == NULL) {
-			ath10k_warn("%s: no vif for vdev_id %d\n",
-				    __func__, vdev_id);
+			ath10k_warn("no vif for vdev_id %d found\n", vdev_id);
 			continue;
 		}
 
-- 
1.7.9.5

[ath9k-devel] [PATCH 5/7] ath10k: move htc creation error handling

[Michal Kazior]

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c |    5 +++--
 drivers/net/wireless/ath/ath10k/htc.c  |    6 ++----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 1c98bbc..3cc5ee7 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -540,8 +540,9 @@ int ath10k_core_register(struct ath10k *ar)
 	htc_ops.target_send_suspend_complete = ath10k_send_suspend_complete;
 
 	ar->htc = ath10k_htc_create(ar, &htc_ops);
-	if (ar->htc == NULL) {
-		status = -ENOMEM;
+	if (IS_ERR(ar->htc)) {
+		status = PTR_ERR(ar->htc);
+		ath10k_err("could not create HTC (%d)\n", status);
 		goto err;
 	}
 
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 22abc6f..af95e87 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -1031,10 +1031,8 @@ struct ath10k_htc *ath10k_htc_create(struct ath10k *ar,
 
 	/* FIXME: use struct ath10k instead */
 	htc = kzalloc(sizeof(struct ath10k_htc), GFP_KERNEL);
-	if (!htc) {
-		ath10k_err("%s: unable to allocate memory\n", __func__);
-		return NULL;
-	}
+	if (!htc)
+		return ERR_PTR(-ENOMEM);
 
 	spin_lock_init(&htc->tx_lock);
 
-- 
1.7.9.5

[ath9k-devel] [PATCH 6/7] ath10k: cleanup pci suspend

[Michal Kazior]

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/pci.c |   23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index a1e7650..77add02 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2347,7 +2347,7 @@ static int ath10k_pci_suspend(struct device *device)
 	struct ath10k *ar = pci_get_drvdata(pdev);
 	struct ath10k_pci *ar_pci;
 	u32 val;
-	u32 left;
+	int ret, retval;
 
 	ath10k_dbg(ATH10K_DBG_PCI, "%s\n", __func__);
 
@@ -2361,14 +2361,17 @@ static int ath10k_pci_suspend(struct device *device)
 	if (ath10k_core_target_suspend(ar))
 		return -EBUSY;
 
-	left = wait_event_interruptible_timeout(ar->event_queue,
+	ret = wait_event_interruptible_timeout(ar->event_queue,
 						ar->is_target_paused == true,
 						1 * HZ);
-
-	if (!left) {
-		ath10k_warn("failed to receive target pasused event [left=%d]\n",
-			    left);
-		return -EIO;
+	if (ret < 0) {
+		ath10k_warn("suspend interrupted (%d)\n", ret);
+		retval = ret;
+		goto resume;
+	} else if (ret == 0) {
+		ath10k_warn("suspend timed out - target pause event never came\n");
+		retval = EIO;
+		goto resume;
 	}
 
 	/*
@@ -2389,6 +2392,12 @@ static int ath10k_pci_suspend(struct device *device)
 	}
 
 	return 0;
+resume:
+	ret = ath10k_core_target_resume(ar);
+	if (ret)
+		ath10k_warn("could not resume (%d)\n", ret);
+
+	return retval;
 }
 
 static int ath10k_pci_resume(struct device *device)
-- 
1.7.9.5

[ath9k-devel] [PATCH 7/7] ath10k: use ath10k_err for non-recoverable failures

[Michal Kazior]

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 3cc5ee7..75d52de 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -584,7 +584,7 @@ int ath10k_core_register(struct ath10k *ar)
 
 	status = ath10k_wmi_wait_for_unified_ready(ar);
 	if (status <= 0) {
-		ath10k_warn("wmi unified ready event not received\n");
+		ath10k_err("wmi unified ready event not received\n");
 		status = -ETIMEDOUT;
 		goto err_disconnect_htc;
 	}
@@ -599,7 +599,7 @@ int ath10k_core_register(struct ath10k *ar)
 
 	status = ath10k_debug_create(ar);
 	if (status) {
-		ath10k_warn("unable to initialize debugfs\n");
+		ath10k_err("unable to initialize debugfs\n");
 		goto err_unregister_mac;
 	}
 
-- 
1.7.9.5

[ath9k-devel] [PATCH 6/7] ath10k: cleanup pci suspend

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> -	left = wait_event_interruptible_timeout(ar->event_queue,
> +	ret = wait_event_interruptible_timeout(ar->event_queue,
>  						ar->is_target_paused == true,
>  						1 * HZ);

Actually I would prefered the name "left" as the variable contains more
than just a normal error code, it also contains the time left before
timeout. But I have applied the patch anyway.

-- 
Kalle Valo

[ath9k-devel] [PATCH 0/7] ath10k: random fixes/cleanups

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> Michal Kazior (7):
>   ath10k: add error handling to some functions
>   ath10k: remove unnecessary check
>   ath10k: remove unnecessary prints
>   ath10k: cleanup warning/error prints
>   ath10k: move htc creation error handling
>   ath10k: cleanup pci suspend
>   ath10k: use ath10k_err for non-recoverable failures

Thanks, all applied.

-- 
Kalle Valo