All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] ath10k: fixes 2014-08-07, part 1
@ 2014-08-07  9:03 ` Michal Kazior
  0 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-07  9:03 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Hi,

This is part 1 (of 3) of my patches. Split for
your reviewing pleasure.

This one is mostly non-functional (just cleanups)
except the removal of soc powersave.


Michal Kazior (6):
  ath10k: embed ar_pci inside ar
  ath10k: remove target soc ps code
  ath10k: remove pci features var
  ath10k: group some pci probing helpers
  ath10k: remove htc->stopped
  ath10k: move fw init print

 drivers/net/wireless/ath/ath10k/ce.c   |  68 +------
 drivers/net/wireless/ath/ath10k/core.c |  50 ++---
 drivers/net/wireless/ath/ath10k/core.h |   6 +-
 drivers/net/wireless/ath/ath10k/htc.c  |  16 --
 drivers/net/wireless/ath/ath10k/htc.h  |   5 +-
 drivers/net/wireless/ath/ath10k/mac.c  |   4 +-
 drivers/net/wireless/ath/ath10k/mac.h  |   2 +-
 drivers/net/wireless/ath/ath10k/pci.c  | 358 +++++++++++----------------------
 drivers/net/wireless/ath/ath10k/pci.h  |  78 ++-----
 9 files changed, 163 insertions(+), 424 deletions(-)

-- 
1.8.5.3


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 0/6] ath10k: fixes 2014-08-07, part 1
@ 2014-08-07  9:03 ` Michal Kazior
  0 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-07  9:03 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Hi,

This is part 1 (of 3) of my patches. Split for
your reviewing pleasure.

This one is mostly non-functional (just cleanups)
except the removal of soc powersave.


Michal Kazior (6):
  ath10k: embed ar_pci inside ar
  ath10k: remove target soc ps code
  ath10k: remove pci features var
  ath10k: group some pci probing helpers
  ath10k: remove htc->stopped
  ath10k: move fw init print

 drivers/net/wireless/ath/ath10k/ce.c   |  68 +------
 drivers/net/wireless/ath/ath10k/core.c |  50 ++---
 drivers/net/wireless/ath/ath10k/core.h |   6 +-
 drivers/net/wireless/ath/ath10k/htc.c  |  16 --
 drivers/net/wireless/ath/ath10k/htc.h  |   5 +-
 drivers/net/wireless/ath/ath10k/mac.c  |   4 +-
 drivers/net/wireless/ath/ath10k/mac.h  |   2 +-
 drivers/net/wireless/ath/ath10k/pci.c  | 358 +++++++++++----------------------
 drivers/net/wireless/ath/ath10k/pci.h  |  78 ++-----
 9 files changed, 163 insertions(+), 424 deletions(-)

-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/6] ath10k: embed ar_pci inside ar
  2014-08-07  9:03 ` Michal Kazior
@ 2014-08-07  9:03   ` Michal Kazior
  -1 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-07  9:03 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Use the common convention of embedding private
structures inside parent structures. This
reduces allocations and simplifies pci probing
code.

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

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 440c3ff..2790aad 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1060,12 +1060,12 @@ void ath10k_core_unregister(struct ath10k *ar)
 }
 EXPORT_SYMBOL(ath10k_core_unregister);
 
-struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
+struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
 				  const struct ath10k_hif_ops *hif_ops)
 {
 	struct ath10k *ar;
 
-	ar = ath10k_mac_create();
+	ar = ath10k_mac_create(priv_size);
 	if (!ar)
 		return NULL;
 
@@ -1075,7 +1075,6 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
 	ar->p2p = !!ath10k_p2p;
 	ar->dev = dev;
 
-	ar->hif.priv = hif_priv;
 	ar->hif.ops = hif_ops;
 
 	init_completion(&ar->scan.started);
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index d5c95d4..2694bd6 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -375,7 +375,6 @@ struct ath10k {
 	bool p2p;
 
 	struct {
-		void *priv;
 		const struct ath10k_hif_ops *ops;
 	} hif;
 
@@ -510,9 +509,12 @@ struct ath10k {
 		enum ath10k_spectral_mode mode;
 		struct ath10k_spec_scan config;
 	} spectral;
+
+	/* must be last */
+	u8 drv_priv[0] __aligned(sizeof(void *));
 };
 
-struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
+struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
 				  const struct ath10k_hif_ops *hif_ops);
 void ath10k_core_destroy(struct ath10k *ar);
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 3c794203..908d5f5 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4514,12 +4514,12 @@ static struct ieee80211_rate ath10k_rates[] = {
 #define ath10k_g_rates (ath10k_rates + 0)
 #define ath10k_g_rates_size (ARRAY_SIZE(ath10k_rates))
 
-struct ath10k *ath10k_mac_create(void)
+struct ath10k *ath10k_mac_create(size_t priv_size)
 {
 	struct ieee80211_hw *hw;
 	struct ath10k *ar;
 
-	hw = ieee80211_alloc_hw(sizeof(struct ath10k), &ath10k_ops);
+	hw = ieee80211_alloc_hw(sizeof(struct ath10k) + priv_size, &ath10k_ops);
 	if (!hw)
 		return NULL;
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.h b/drivers/net/wireless/ath/ath10k/mac.h
index ef4f843..720cb1a 100644
--- a/drivers/net/wireless/ath/ath10k/mac.h
+++ b/drivers/net/wireless/ath/ath10k/mac.h
@@ -26,7 +26,7 @@ struct ath10k_generic_iter {
 	int ret;
 };
 
-struct ath10k *ath10k_mac_create(void);
+struct ath10k *ath10k_mac_create(size_t priv_size);
 void ath10k_mac_destroy(struct ath10k *ar);
 int ath10k_mac_register(struct ath10k *ar);
 void ath10k_mac_unregister(struct ath10k *ar);
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 2d340cc..a2003b6 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2621,10 +2621,14 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 
 	ath10k_dbg(ATH10K_DBG_PCI, "pci probe\n");
 
-	ar_pci = kzalloc(sizeof(*ar_pci), GFP_KERNEL);
-	if (ar_pci == NULL)
+	ar = ath10k_core_create(sizeof(*ar_pci), &pdev->dev,
+				&ath10k_pci_hif_ops);
+	if (!ar) {
+		ath10k_err("failed to allocate core\n");
 		return -ENOMEM;
+	}
 
+	ar_pci = ath10k_pci_priv(ar);
 	ar_pci->pdev = pdev;
 	ar_pci->dev = &pdev->dev;
 
@@ -2635,7 +2639,7 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	default:
 		ret = -ENODEV;
 		ath10k_err("Unknown device ID: %d\n", pci_dev->device);
-		goto err_ar_pci;
+		goto err_core_destroy;
 	}
 
 	if (ath10k_pci_target_ps)
@@ -2643,13 +2647,6 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 
 	ath10k_pci_dump_features(ar_pci);
 
-	ar = ath10k_core_create(ar_pci, ar_pci->dev, &ath10k_pci_hif_ops);
-	if (!ar) {
-		ath10k_err("failed to create driver core\n");
-		ret = -EINVAL;
-		goto err_ar_pci;
-	}
-
 	ar_pci->ar = ar;
 	atomic_set(&ar_pci->keep_awake_count, 0);
 
@@ -2658,7 +2655,7 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	ret = pci_enable_device(pdev);
 	if (ret) {
 		ath10k_err("failed to enable PCI device: %d\n", ret);
-		goto err_ar;
+		goto err_core_destroy;
 	}
 
 	/* Request MMIO resources */
@@ -2742,11 +2739,8 @@ err_region:
 	pci_release_region(pdev, BAR_NUM);
 err_device:
 	pci_disable_device(pdev);
-err_ar:
+err_core_destroy:
 	ath10k_core_destroy(ar);
-err_ar_pci:
-	/* call HIF PCI free here */
-	kfree(ar_pci);
 
 	return ret;
 }
@@ -2775,7 +2769,6 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 	pci_disable_device(pdev);
 
 	ath10k_core_destroy(ar);
-	kfree(ar_pci);
 }
 
 MODULE_DEVICE_TABLE(pci, ath10k_pci_id_table);
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 9401292..531c98a 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -202,7 +202,7 @@ struct ath10k_pci {
 
 static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)
 {
-	return ar->hif.priv;
+	return (struct ath10k_pci *)ar->drv_priv;
 }
 
 static inline u32 ath10k_pci_reg_read32(struct ath10k *ar, u32 addr)
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 1/6] ath10k: embed ar_pci inside ar
@ 2014-08-07  9:03   ` Michal Kazior
  0 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-07  9:03 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Use the common convention of embedding private
structures inside parent structures. This
reduces allocations and simplifies pci probing
code.

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

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 440c3ff..2790aad 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1060,12 +1060,12 @@ void ath10k_core_unregister(struct ath10k *ar)
 }
 EXPORT_SYMBOL(ath10k_core_unregister);
 
-struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
+struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
 				  const struct ath10k_hif_ops *hif_ops)
 {
 	struct ath10k *ar;
 
-	ar = ath10k_mac_create();
+	ar = ath10k_mac_create(priv_size);
 	if (!ar)
 		return NULL;
 
@@ -1075,7 +1075,6 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
 	ar->p2p = !!ath10k_p2p;
 	ar->dev = dev;
 
-	ar->hif.priv = hif_priv;
 	ar->hif.ops = hif_ops;
 
 	init_completion(&ar->scan.started);
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index d5c95d4..2694bd6 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -375,7 +375,6 @@ struct ath10k {
 	bool p2p;
 
 	struct {
-		void *priv;
 		const struct ath10k_hif_ops *ops;
 	} hif;
 
@@ -510,9 +509,12 @@ struct ath10k {
 		enum ath10k_spectral_mode mode;
 		struct ath10k_spec_scan config;
 	} spectral;
+
+	/* must be last */
+	u8 drv_priv[0] __aligned(sizeof(void *));
 };
 
-struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
+struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
 				  const struct ath10k_hif_ops *hif_ops);
 void ath10k_core_destroy(struct ath10k *ar);
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 3c794203..908d5f5 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4514,12 +4514,12 @@ static struct ieee80211_rate ath10k_rates[] = {
 #define ath10k_g_rates (ath10k_rates + 0)
 #define ath10k_g_rates_size (ARRAY_SIZE(ath10k_rates))
 
-struct ath10k *ath10k_mac_create(void)
+struct ath10k *ath10k_mac_create(size_t priv_size)
 {
 	struct ieee80211_hw *hw;
 	struct ath10k *ar;
 
-	hw = ieee80211_alloc_hw(sizeof(struct ath10k), &ath10k_ops);
+	hw = ieee80211_alloc_hw(sizeof(struct ath10k) + priv_size, &ath10k_ops);
 	if (!hw)
 		return NULL;
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.h b/drivers/net/wireless/ath/ath10k/mac.h
index ef4f843..720cb1a 100644
--- a/drivers/net/wireless/ath/ath10k/mac.h
+++ b/drivers/net/wireless/ath/ath10k/mac.h
@@ -26,7 +26,7 @@ struct ath10k_generic_iter {
 	int ret;
 };
 
-struct ath10k *ath10k_mac_create(void);
+struct ath10k *ath10k_mac_create(size_t priv_size);
 void ath10k_mac_destroy(struct ath10k *ar);
 int ath10k_mac_register(struct ath10k *ar);
 void ath10k_mac_unregister(struct ath10k *ar);
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 2d340cc..a2003b6 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2621,10 +2621,14 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 
 	ath10k_dbg(ATH10K_DBG_PCI, "pci probe\n");
 
-	ar_pci = kzalloc(sizeof(*ar_pci), GFP_KERNEL);
-	if (ar_pci == NULL)
+	ar = ath10k_core_create(sizeof(*ar_pci), &pdev->dev,
+				&ath10k_pci_hif_ops);
+	if (!ar) {
+		ath10k_err("failed to allocate core\n");
 		return -ENOMEM;
+	}
 
+	ar_pci = ath10k_pci_priv(ar);
 	ar_pci->pdev = pdev;
 	ar_pci->dev = &pdev->dev;
 
@@ -2635,7 +2639,7 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	default:
 		ret = -ENODEV;
 		ath10k_err("Unknown device ID: %d\n", pci_dev->device);
-		goto err_ar_pci;
+		goto err_core_destroy;
 	}
 
 	if (ath10k_pci_target_ps)
@@ -2643,13 +2647,6 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 
 	ath10k_pci_dump_features(ar_pci);
 
-	ar = ath10k_core_create(ar_pci, ar_pci->dev, &ath10k_pci_hif_ops);
-	if (!ar) {
-		ath10k_err("failed to create driver core\n");
-		ret = -EINVAL;
-		goto err_ar_pci;
-	}
-
 	ar_pci->ar = ar;
 	atomic_set(&ar_pci->keep_awake_count, 0);
 
@@ -2658,7 +2655,7 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	ret = pci_enable_device(pdev);
 	if (ret) {
 		ath10k_err("failed to enable PCI device: %d\n", ret);
-		goto err_ar;
+		goto err_core_destroy;
 	}
 
 	/* Request MMIO resources */
@@ -2742,11 +2739,8 @@ err_region:
 	pci_release_region(pdev, BAR_NUM);
 err_device:
 	pci_disable_device(pdev);
-err_ar:
+err_core_destroy:
 	ath10k_core_destroy(ar);
-err_ar_pci:
-	/* call HIF PCI free here */
-	kfree(ar_pci);
 
 	return ret;
 }
@@ -2775,7 +2769,6 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 	pci_disable_device(pdev);
 
 	ath10k_core_destroy(ar);
-	kfree(ar_pci);
 }
 
 MODULE_DEVICE_TABLE(pci, ath10k_pci_id_table);
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 9401292..531c98a 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -202,7 +202,7 @@ struct ath10k_pci {
 
 static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)
 {
-	return ar->hif.priv;
+	return (struct ath10k_pci *)ar->drv_priv;
 }
 
 static inline u32 ath10k_pci_reg_read32(struct ath10k *ar, u32 addr)
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/6] ath10k: remove target soc ps code
  2014-08-07  9:03 ` Michal Kazior
@ 2014-08-07  9:03   ` Michal Kazior
  -1 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-07  9:03 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

The soc powersave was disabled by default. It
never was fully tested. Some hw apparently had
problems with it and the implementation itself had
a possible race.

Just remove the refcounting and simply wake up the
device when probing and put to sleep when
removing.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/ce.c  |  68 +------------
 drivers/net/wireless/ath/ath10k/pci.c | 182 +++++++---------------------------
 drivers/net/wireless/ath/ath10k/pci.h |  67 +++----------
 3 files changed, 54 insertions(+), 263 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 4333107..5117705 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -287,10 +287,6 @@ int ath10k_ce_send_nolock(struct ath10k_ce_pipe *ce_state,
 		ath10k_warn("%s: send more we can (nbytes: %d, max: %d)\n",
 			    __func__, nbytes, ce_state->src_sz_max);
 
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		return ret;
-
 	if (unlikely(CE_RING_DELTA(nentries_mask,
 				   write_index, sw_index - 1) <= 0)) {
 		ret = -ENOSR;
@@ -325,7 +321,6 @@ int ath10k_ce_send_nolock(struct ath10k_ce_pipe *ce_state,
 
 	src_ring->write_index = write_index;
 exit:
-	ath10k_pci_sleep(ar);
 	return ret;
 }
 
@@ -407,10 +402,6 @@ int ath10k_ce_recv_buf_enqueue(struct ath10k_ce_pipe *ce_state,
 	write_index = dest_ring->write_index;
 	sw_index = dest_ring->sw_index;
 
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		goto out;
-
 	if (CE_RING_DELTA(nentries_mask, write_index, sw_index - 1) > 0) {
 		struct ce_desc *base = dest_ring->base_addr_owner_space;
 		struct ce_desc *desc = CE_DEST_RING_TO_DESC(base, write_index);
@@ -430,11 +421,8 @@ int ath10k_ce_recv_buf_enqueue(struct ath10k_ce_pipe *ce_state,
 	} else {
 		ret = -EIO;
 	}
-	ath10k_pci_sleep(ar);
 
-out:
 	spin_unlock_bh(&ar_pci->ce_lock);
-
 	return ret;
 }
 
@@ -588,7 +576,6 @@ static int ath10k_ce_completed_send_next_nolock(struct ath10k_ce_pipe *ce_state,
 	unsigned int sw_index = src_ring->sw_index;
 	struct ce_desc *sdesc, *sbase;
 	unsigned int read_index;
-	int ret;
 
 	if (src_ring->hw_index == sw_index) {
 		/*
@@ -599,18 +586,12 @@ static int ath10k_ce_completed_send_next_nolock(struct ath10k_ce_pipe *ce_state,
 		 * value of the HW index has become stale.
 		 */
 
-		ret = ath10k_pci_wake(ar);
-		if (ret)
-			return ret;
-
 		read_index = ath10k_ce_src_ring_read_index_get(ar, ctrl_addr);
 		if (read_index == 0xffffffff)
 			return -ENODEV;
 
 		read_index &= nentries_mask;
 		src_ring->hw_index = read_index;
-
-		ath10k_pci_sleep(ar);
 	}
 
 	read_index = src_ring->hw_index;
@@ -731,11 +712,6 @@ void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id)
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	struct ath10k_ce_pipe *ce_state = &ar_pci->ce_states[ce_id];
 	u32 ctrl_addr = ce_state->ctrl_addr;
-	int ret;
-
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		return;
 
 	spin_lock_bh(&ar_pci->ce_lock);
 
@@ -760,7 +736,6 @@ void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id)
 	ath10k_ce_engine_int_status_clear(ar, ctrl_addr, CE_WATERMARK_MASK);
 
 	spin_unlock_bh(&ar_pci->ce_lock);
-	ath10k_pci_sleep(ar);
 }
 
 /*
@@ -771,13 +746,9 @@ void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id)
 
 void ath10k_ce_per_engine_service_any(struct ath10k *ar)
 {
-	int ce_id, ret;
+	int ce_id;
 	u32 intr_summary;
 
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		return;
-
 	intr_summary = CE_INTERRUPT_SUMMARY(ar);
 
 	for (ce_id = 0; intr_summary && (ce_id < CE_COUNT); ce_id++) {
@@ -789,8 +760,6 @@ void ath10k_ce_per_engine_service_any(struct ath10k *ar)
 
 		ath10k_ce_per_engine_service(ar, ce_id);
 	}
-
-	ath10k_pci_sleep(ar);
 }
 
 /*
@@ -805,11 +774,6 @@ static void ath10k_ce_per_engine_handler_adjust(struct ath10k_ce_pipe *ce_state,
 {
 	u32 ctrl_addr = ce_state->ctrl_addr;
 	struct ath10k *ar = ce_state->ar;
-	int ret;
-
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		return;
 
 	if ((!disable_copy_compl_intr) &&
 	    (ce_state->send_cb || ce_state->recv_cb))
@@ -818,17 +782,11 @@ static void ath10k_ce_per_engine_handler_adjust(struct ath10k_ce_pipe *ce_state,
 		ath10k_ce_copy_complete_intr_disable(ar, ctrl_addr);
 
 	ath10k_ce_watermark_intr_disable(ar, ctrl_addr);
-
-	ath10k_pci_sleep(ar);
 }
 
 int ath10k_ce_disable_interrupts(struct ath10k *ar)
 {
-	int ce_id, ret;
-
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		return ret;
+	int ce_id;
 
 	for (ce_id = 0; ce_id < CE_COUNT; ce_id++) {
 		u32 ctrl_addr = ath10k_ce_base_address(ce_id);
@@ -838,8 +796,6 @@ int ath10k_ce_disable_interrupts(struct ath10k *ar)
 		ath10k_ce_watermark_intr_disable(ar, ctrl_addr);
 	}
 
-	ath10k_pci_sleep(ar);
-
 	return 0;
 }
 
@@ -1084,10 +1040,6 @@ int ath10k_ce_init_pipe(struct ath10k *ar, unsigned int ce_id,
 	BUILD_BUG_ON(2*TARGET_10X_NUM_MSDU_DESC >
 		     (CE_HTT_H2T_MSG_SRC_NENTRIES - 1));
 
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		return ret;
-
 	spin_lock_bh(&ar_pci->ce_lock);
 	ce_state->ar = ar;
 	ce_state->id = ce_id;
@@ -1101,7 +1053,7 @@ int ath10k_ce_init_pipe(struct ath10k *ar, unsigned int ce_id,
 		if (ret) {
 			ath10k_err("Failed to initialize CE src ring for ID: %d (%d)\n",
 				   ce_id, ret);
-			goto out;
+			return ret;
 		}
 	}
 
@@ -1110,13 +1062,11 @@ int ath10k_ce_init_pipe(struct ath10k *ar, unsigned int ce_id,
 		if (ret) {
 			ath10k_err("Failed to initialize CE dest ring for ID: %d (%d)\n",
 				   ce_id, ret);
-			goto out;
+			return ret;
 		}
 	}
 
-out:
-	ath10k_pci_sleep(ar);
-	return ret;
+	return 0;
 }
 
 static void ath10k_ce_deinit_src_ring(struct ath10k *ar, unsigned int ce_id)
@@ -1140,16 +1090,8 @@ static void ath10k_ce_deinit_dest_ring(struct ath10k *ar, unsigned int ce_id)
 
 void ath10k_ce_deinit_pipe(struct ath10k *ar, unsigned int ce_id)
 {
-	int ret;
-
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		return;
-
 	ath10k_ce_deinit_src_ring(ar, ce_id);
 	ath10k_ce_deinit_dest_ring(ar, ce_id);
-
-	ath10k_pci_sleep(ar);
 }
 
 int ath10k_ce_alloc_pipe(struct ath10k *ar, int ce_id,
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index a2003b6..5fa45db 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -44,13 +44,9 @@ enum ath10k_pci_reset_mode {
 	ATH10K_PCI_RESET_WARM_ONLY = 1,
 };
 
-static unsigned int ath10k_pci_target_ps;
 static unsigned int ath10k_pci_irq_mode = ATH10K_PCI_IRQ_AUTO;
 static unsigned int ath10k_pci_reset_mode = ATH10K_PCI_RESET_AUTO;
 
-module_param_named(target_ps, ath10k_pci_target_ps, uint, 0644);
-MODULE_PARM_DESC(target_ps, "Enable ath10k Target (SoC) PS option");
-
 module_param_named(irq_mode, ath10k_pci_irq_mode, uint, 0644);
 MODULE_PARM_DESC(irq_mode, "0: auto, 1: legacy, 2: msi (default: 0)");
 
@@ -389,10 +385,8 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar, u32 address, void *data,
 		 * convert it from Target CPU virtual address space
 		 * to CE address space
 		 */
-		ath10k_pci_wake(ar);
 		address = TARG_CPU_SPACE_TO_CE_SPACE(ar, ar_pci->mem,
 						     address);
-		ath10k_pci_sleep(ar);
 
 		ret = ath10k_ce_send(ce_diag, NULL, (u32)address, nbytes, 0,
 				 0);
@@ -474,9 +468,7 @@ static int ath10k_pci_diag_read_access(struct ath10k *ar, u32 address,
 	if (address >= DRAM_BASE_ADDRESS)
 		return ath10k_pci_diag_read_mem(ar, address, data, sizeof(u32));
 
-	ath10k_pci_wake(ar);
 	*data = ath10k_pci_read32(ar, address);
-	ath10k_pci_sleep(ar);
 	return 0;
 }
 
@@ -528,9 +520,7 @@ static int ath10k_pci_diag_write_mem(struct ath10k *ar, u32 address,
 	 * to
 	 *    CE address space
 	 */
-	ath10k_pci_wake(ar);
 	address = TARG_CPU_SPACE_TO_CE_SPACE(ar, ar_pci->mem, address);
-	ath10k_pci_sleep(ar);
 
 	remaining_bytes = orig_nbytes;
 	ce_data = ce_data_base;
@@ -623,51 +613,25 @@ static int ath10k_pci_diag_write_access(struct ath10k *ar, u32 address,
 		return ath10k_pci_diag_write_mem(ar, address, &data,
 						 sizeof(u32));
 
-	ath10k_pci_wake(ar);
 	ath10k_pci_write32(ar, address, data);
-	ath10k_pci_sleep(ar);
 	return 0;
 }
 
-static bool ath10k_pci_target_is_awake(struct ath10k *ar)
+static bool ath10k_pci_is_awake(struct ath10k *ar)
 {
-	void __iomem *mem = ath10k_pci_priv(ar)->mem;
-	u32 val;
-	val = ioread32(mem + PCIE_LOCAL_BASE_ADDRESS +
-		       RTC_STATE_ADDRESS);
-	return (RTC_STATE_V_GET(val) == RTC_STATE_V_ON);
+	u32 val = ath10k_pci_reg_read32(ar, RTC_STATE_ADDRESS);
+
+	return RTC_STATE_V_GET(val) == RTC_STATE_V_ON;
 }
 
-int ath10k_do_pci_wake(struct ath10k *ar)
+static int ath10k_pci_wake_wait(struct ath10k *ar)
 {
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	void __iomem *pci_addr = ar_pci->mem;
 	int tot_delay = 0;
 	int curr_delay = 5;
 
-	if (atomic_read(&ar_pci->keep_awake_count) == 0) {
-		/* Force AWAKE */
-		iowrite32(PCIE_SOC_WAKE_V_MASK,
-			  pci_addr + PCIE_LOCAL_BASE_ADDRESS +
-			  PCIE_SOC_WAKE_ADDRESS);
-	}
-	atomic_inc(&ar_pci->keep_awake_count);
-
-	if (ar_pci->verified_awake)
-		return 0;
-
-	for (;;) {
-		if (ath10k_pci_target_is_awake(ar)) {
-			ar_pci->verified_awake = true;
+	while (tot_delay < PCIE_WAKE_TIMEOUT) {
+		if (ath10k_pci_is_awake(ar))
 			return 0;
-		}
-
-		if (tot_delay > PCIE_WAKE_TIMEOUT) {
-			ath10k_warn("target took longer %d us to wake up (awake count %d)\n",
-				    PCIE_WAKE_TIMEOUT,
-				    atomic_read(&ar_pci->keep_awake_count));
-			return -ETIMEDOUT;
-		}
 
 		udelay(curr_delay);
 		tot_delay += curr_delay;
@@ -675,20 +639,21 @@ int ath10k_do_pci_wake(struct ath10k *ar)
 		if (curr_delay < 50)
 			curr_delay += 5;
 	}
+
+	return -ETIMEDOUT;
 }
 
-void ath10k_do_pci_sleep(struct ath10k *ar)
+int ath10k_pci_wake(struct ath10k *ar)
 {
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	void __iomem *pci_addr = ar_pci->mem;
+	ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS,
+			       PCIE_SOC_WAKE_V_MASK);
+	return ath10k_pci_wake_wait(ar);
+}
 
-	if (atomic_dec_and_test(&ar_pci->keep_awake_count)) {
-		/* Allow sleep */
-		ar_pci->verified_awake = false;
-		iowrite32(PCIE_SOC_WAKE_RESET,
-			  pci_addr + PCIE_LOCAL_BASE_ADDRESS +
-			  PCIE_SOC_WAKE_ADDRESS);
-	}
+void ath10k_pci_sleep(struct ath10k *ar)
+{
+	ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS,
+			       PCIE_SOC_WAKE_RESET);
 }
 
 /* Called by lower (CE) layer when a send to Target completes. */
@@ -1788,8 +1753,6 @@ static void ath10k_pci_fw_interrupt_handler(struct ath10k *ar)
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	u32 fw_indicator;
 
-	ath10k_pci_wake(ar);
-
 	fw_indicator = ath10k_pci_read32(ar, FW_INDICATOR_ADDRESS);
 
 	if (fw_indicator & FW_IND_EVENT_PENDING) {
@@ -1807,8 +1770,6 @@ static void ath10k_pci_fw_interrupt_handler(struct ath10k *ar)
 			ath10k_warn("early firmware event indicated\n");
 		}
 	}
-
-	ath10k_pci_sleep(ar);
 }
 
 /* this function effectively clears target memory controller assert line */
@@ -1838,12 +1799,6 @@ static int ath10k_pci_warm_reset(struct ath10k *ar)
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot warm reset\n");
 
-	ret = ath10k_do_pci_wake(ar);
-	if (ret) {
-		ath10k_err("failed to wake up target: %d\n", ret);
-		return ret;
-	}
-
 	/* debug */
 	val = ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
 				PCIE_INTR_CAUSE_ADDRESS);
@@ -1915,7 +1870,6 @@ static int ath10k_pci_warm_reset(struct ath10k *ar)
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot warm reset complete\n");
 
-	ath10k_do_pci_sleep(ar);
 	return ret;
 }
 
@@ -1945,14 +1899,10 @@ static int __ath10k_pci_hif_power_up(struct ath10k *ar, bool cold_reset)
 		goto err;
 	}
 
-	if (!test_bit(ATH10K_PCI_FEATURE_SOC_POWER_SAVE, ar_pci->features))
-		/* Force AWAKE forever */
-		ath10k_do_pci_wake(ar);
-
 	ret = ath10k_pci_ce_init(ar);
 	if (ret) {
 		ath10k_err("failed to initialize CE: %d\n", ret);
-		goto err_ps;
+		goto err;
 	}
 
 	ret = ath10k_ce_disable_interrupts(ar);
@@ -2012,9 +1962,6 @@ err_deinit_irq:
 err_ce:
 	ath10k_pci_ce_deinit(ar);
 	ath10k_pci_warm_reset(ar);
-err_ps:
-	if (!test_bit(ATH10K_PCI_FEATURE_SOC_POWER_SAVE, ar_pci->features))
-		ath10k_do_pci_sleep(ar);
 err:
 	return ret;
 }
@@ -2078,8 +2025,6 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
 
 static void ath10k_pci_hif_power_down(struct ath10k *ar)
 {
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot hif power down\n");
 
 	ath10k_pci_free_early_irq(ar);
@@ -2087,9 +2032,6 @@ static void ath10k_pci_hif_power_down(struct ath10k *ar)
 	ath10k_pci_deinit_irq(ar);
 	ath10k_pci_ce_deinit(ar);
 	ath10k_pci_warm_reset(ar);
-
-	if (!test_bit(ATH10K_PCI_FEATURE_SOC_POWER_SAVE, ar_pci->features))
-		ath10k_do_pci_sleep(ar);
 }
 
 #ifdef CONFIG_PM
@@ -2236,14 +2178,6 @@ static void ath10k_pci_early_irq_tasklet(unsigned long data)
 {
 	struct ath10k *ar = (struct ath10k *)data;
 	u32 fw_ind;
-	int ret;
-
-	ret = ath10k_pci_wake(ar);
-	if (ret) {
-		ath10k_warn("failed to wake target in early irq tasklet: %d\n",
-			    ret);
-		return;
-	}
 
 	fw_ind = ath10k_pci_read32(ar, FW_INDICATOR_ADDRESS);
 	if (fw_ind & FW_IND_EVENT_PENDING) {
@@ -2252,7 +2186,6 @@ static void ath10k_pci_early_irq_tasklet(unsigned long data)
 		ath10k_pci_hif_dump_area(ar);
 	}
 
-	ath10k_pci_sleep(ar);
 	ath10k_pci_enable_legacy_irq(ar);
 }
 
@@ -2426,34 +2359,16 @@ static int ath10k_pci_init_irq(struct ath10k *ar)
 	 * synchronization checking. */
 	ar_pci->num_msi_intrs = 0;
 
-	ret = ath10k_pci_wake(ar);
-	if (ret) {
-		ath10k_warn("failed to wake target: %d\n", ret);
-		return ret;
-	}
-
 	ath10k_pci_write32(ar, SOC_CORE_BASE_ADDRESS + PCIE_INTR_ENABLE_ADDRESS,
 			   PCIE_INTR_FIRMWARE_MASK | PCIE_INTR_CE_MASK_ALL);
-	ath10k_pci_sleep(ar);
 
 	return 0;
 }
 
-static int ath10k_pci_deinit_irq_legacy(struct ath10k *ar)
+static void ath10k_pci_deinit_irq_legacy(struct ath10k *ar)
 {
-	int ret;
-
-	ret = ath10k_pci_wake(ar);
-	if (ret) {
-		ath10k_warn("failed to wake target: %d\n", ret);
-		return ret;
-	}
-
 	ath10k_pci_write32(ar, SOC_CORE_BASE_ADDRESS + PCIE_INTR_ENABLE_ADDRESS,
 			   0);
-	ath10k_pci_sleep(ar);
-
-	return 0;
 }
 
 static int ath10k_pci_deinit_irq(struct ath10k *ar)
@@ -2462,7 +2377,8 @@ static int ath10k_pci_deinit_irq(struct ath10k *ar)
 
 	switch (ar_pci->num_msi_intrs) {
 	case 0:
-		return ath10k_pci_deinit_irq_legacy(ar);
+		ath10k_pci_deinit_irq_legacy(ar);
+		return 0;
 	case 1:
 		/* fall-through */
 	case MSI_NUM_REQUEST:
@@ -2480,17 +2396,10 @@ static int ath10k_pci_wait_for_target_init(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	unsigned long timeout;
-	int ret;
 	u32 val;
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot waiting target to initialise\n");
 
-	ret = ath10k_pci_wake(ar);
-	if (ret) {
-		ath10k_err("failed to wake up target for init: %d\n", ret);
-		return ret;
-	}
-
 	timeout = jiffies + msecs_to_jiffies(ATH10K_PCI_TARGET_WAIT);
 
 	do {
@@ -2520,8 +2429,7 @@ static int ath10k_pci_wait_for_target_init(struct ath10k *ar)
 
 	if (val == 0xffffffff) {
 		ath10k_err("failed to read device register, device is gone\n");
-		ret = -EIO;
-		goto out;
+		return -EIO;
 	}
 
 	if (val & FW_IND_EVENT_PENDING) {
@@ -2529,38 +2437,26 @@ static int ath10k_pci_wait_for_target_init(struct ath10k *ar)
 		ath10k_pci_write32(ar, FW_INDICATOR_ADDRESS,
 				   val & ~FW_IND_EVENT_PENDING);
 		ath10k_pci_hif_dump_area(ar);
-		ret = -ECOMM;
-		goto out;
+		return -ECOMM;
 	}
 
 	if (!(val & FW_IND_INITIALIZED)) {
 		ath10k_err("failed to receive initialized event from target: %08x\n",
 			   val);
-		ret = -ETIMEDOUT;
-		goto out;
+		return -ETIMEDOUT;
 	}
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot target initialised\n");
-
-out:
-	ath10k_pci_sleep(ar);
-	return ret;
+	return 0;
 }
 
 static int ath10k_pci_cold_reset(struct ath10k *ar)
 {
-	int i, ret;
+	int i;
 	u32 val;
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot cold reset\n");
 
-	ret = ath10k_do_pci_wake(ar);
-	if (ret) {
-		ath10k_err("failed to wake up target: %d\n",
-			   ret);
-		return ret;
-	}
-
 	/* Put Target, including PCIe, into RESET. */
 	val = ath10k_pci_reg_read32(ar, SOC_GLOBAL_RESET_ADDRESS);
 	val |= 1;
@@ -2584,8 +2480,6 @@ static int ath10k_pci_cold_reset(struct ath10k *ar)
 		msleep(1);
 	}
 
-	ath10k_do_pci_sleep(ar);
-
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot cold reset complete\n");
 
 	return 0;
@@ -2603,9 +2497,6 @@ static void ath10k_pci_dump_features(struct ath10k_pci *ar_pci)
 		case ATH10K_PCI_FEATURE_MSI_X:
 			ath10k_dbg(ATH10K_DBG_BOOT, "device supports MSI-X\n");
 			break;
-		case ATH10K_PCI_FEATURE_SOC_POWER_SAVE:
-			ath10k_dbg(ATH10K_DBG_BOOT, "QCA98XX SoC power save enabled\n");
-			break;
 		}
 	}
 }
@@ -2642,13 +2533,9 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 		goto err_core_destroy;
 	}
 
-	if (ath10k_pci_target_ps)
-		set_bit(ATH10K_PCI_FEATURE_SOC_POWER_SAVE, ar_pci->features);
-
 	ath10k_pci_dump_features(ar_pci);
 
 	ar_pci->ar = ar;
-	atomic_set(&ar_pci->keep_awake_count, 0);
 
 	pci_set_drvdata(pdev, ar);
 
@@ -2703,20 +2590,22 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 
 	spin_lock_init(&ar_pci->ce_lock);
 
-	ret = ath10k_do_pci_wake(ar);
+	ret = ath10k_pci_wake(ar);
 	if (ret) {
-		ath10k_err("Failed to get chip id: %d\n", ret);
+		ath10k_err("failed to wake up: %d\n", ret);
 		goto err_iomap;
 	}
 
 	chip_id = ath10k_pci_soc_read32(ar, SOC_CHIP_ID_ADDRESS);
-
-	ath10k_do_pci_sleep(ar);
+	if (chip_id == 0xffffffff) {
+		ath10k_err("failed to get chip id\n");
+		goto err_sleep;
+	}
 
 	ret = ath10k_pci_alloc_ce(ar);
 	if (ret) {
 		ath10k_err("failed to allocate copy engine pipes: %d\n", ret);
-		goto err_iomap;
+		goto err_sleep;
 	}
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot pci_mem 0x%p\n", ar_pci->mem);
@@ -2731,6 +2620,8 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 
 err_free_ce:
 	ath10k_pci_free_ce(ar);
+err_sleep:
+	ath10k_pci_sleep(ar);
 err_iomap:
 	pci_iounmap(pdev, mem);
 err_master:
@@ -2762,6 +2653,7 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 
 	ath10k_core_unregister(ar);
 	ath10k_pci_free_ce(ar);
+	ath10k_pci_sleep(ar);
 
 	pci_iounmap(pdev, ar_pci->mem);
 	pci_release_region(pdev, BAR_NUM);
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 531c98a..65b1b90 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -137,7 +137,6 @@ struct service_to_pipe {
 
 enum ath10k_pci_features {
 	ATH10K_PCI_FEATURE_MSI_X		= 0,
-	ATH10K_PCI_FEATURE_SOC_POWER_SAVE	= 1,
 
 	/* keep last */
 	ATH10K_PCI_FEATURE_COUNT
@@ -183,9 +182,6 @@ struct ath10k_pci {
 
 	int started;
 
-	atomic_t keep_awake_count;
-	bool verified_awake;
-
 	struct ath10k_pci_pipe pipe_info[CE_COUNT_MAX];
 
 	struct ath10k_hif_cb msg_callbacks_current;
@@ -205,20 +201,6 @@ static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)
 	return (struct ath10k_pci *)ar->drv_priv;
 }
 
-static inline u32 ath10k_pci_reg_read32(struct ath10k *ar, u32 addr)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-
-	return ioread32(ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS + addr);
-}
-
-static inline void ath10k_pci_reg_write32(struct ath10k *ar, u32 addr, u32 val)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-
-	iowrite32(val, ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS + addr);
-}
-
 #define ATH_PCI_RESET_WAIT_MAX 10 /* ms */
 #define PCIE_WAKE_TIMEOUT 5000	/* 5ms */
 
@@ -242,35 +224,17 @@ static inline void ath10k_pci_reg_write32(struct ath10k *ar, u32 addr, u32 val)
 /* Wait up to this many Ms for a Diagnostic Access CE operation to complete */
 #define DIAG_ACCESS_CE_TIMEOUT_MS 10
 
-/*
- * This API allows the Host to access Target registers directly
- * and relatively efficiently over PCIe.
- * This allows the Host to avoid extra overhead associated with
- * sending a message to firmware and waiting for a response message
- * from firmware, as is done on other interconnects.
- *
- * Yet there is some complexity with direct accesses because the
- * Target's power state is not known a priori. The Host must issue
- * special PCIe reads/writes in order to explicitly wake the Target
- * and to verify that it is awake and will remain awake.
+/* Target exposes its registers for direct access. However before host can
+ * access them it needs to make sure the target is awake (ath10k_pci_wake,
+ * ath10k_pci_wake_wait, ath10k_pci_is_awake). Once target is awake it won't go
+ * to sleep unless host tells it to (ath10k_pci_sleep).
  *
- * Usage:
+ * If host tries to access target registers without waking it up it can
+ * scribble over host memory.
  *
- *   Use ath10k_pci_read32 and ath10k_pci_write32 to access Target space.
- *   These calls must be bracketed by ath10k_pci_wake and
- *   ath10k_pci_sleep.  A single BEGIN/END pair is adequate for
- *   multiple READ/WRITE operations.
- *
- *   Use ath10k_pci_wake to put the Target in a state in
- *   which it is legal for the Host to directly access it. This
- *   may involve waking the Target from a low power state, which
- *   may take up to 2Ms!
- *
- *   Use ath10k_pci_sleep to tell the Target that as far as
- *   this code path is concerned, it no longer needs to remain
- *   directly accessible.  BEGIN/END is under a reference counter;
- *   multiple code paths may issue BEGIN/END on a single targid.
+ * If target is asleep waking it up may take up to even 2ms.
  */
+
 static inline void ath10k_pci_write32(struct ath10k *ar, u32 offset,
 				      u32 value)
 {
@@ -296,25 +260,18 @@ static inline void ath10k_pci_soc_write32(struct ath10k *ar, u32 addr, u32 val)
 	ath10k_pci_write32(ar, RTC_SOC_BASE_ADDRESS + addr, val);
 }
 
-int ath10k_do_pci_wake(struct ath10k *ar);
-void ath10k_do_pci_sleep(struct ath10k *ar);
-
-static inline int ath10k_pci_wake(struct ath10k *ar)
+static inline u32 ath10k_pci_reg_read32(struct ath10k *ar, u32 addr)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 
-	if (test_bit(ATH10K_PCI_FEATURE_SOC_POWER_SAVE, ar_pci->features))
-		return ath10k_do_pci_wake(ar);
-
-	return 0;
+	return ioread32(ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS + addr);
 }
 
-static inline void ath10k_pci_sleep(struct ath10k *ar)
+static inline void ath10k_pci_reg_write32(struct ath10k *ar, u32 addr, u32 val)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 
-	if (test_bit(ATH10K_PCI_FEATURE_SOC_POWER_SAVE, ar_pci->features))
-		ath10k_do_pci_sleep(ar);
+	iowrite32(val, ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS + addr);
 }
 
 #endif /* _PCI_H_ */
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/6] ath10k: remove target soc ps code
@ 2014-08-07  9:03   ` Michal Kazior
  0 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-07  9:03 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

The soc powersave was disabled by default. It
never was fully tested. Some hw apparently had
problems with it and the implementation itself had
a possible race.

Just remove the refcounting and simply wake up the
device when probing and put to sleep when
removing.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/ce.c  |  68 +------------
 drivers/net/wireless/ath/ath10k/pci.c | 182 +++++++---------------------------
 drivers/net/wireless/ath/ath10k/pci.h |  67 +++----------
 3 files changed, 54 insertions(+), 263 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 4333107..5117705 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -287,10 +287,6 @@ int ath10k_ce_send_nolock(struct ath10k_ce_pipe *ce_state,
 		ath10k_warn("%s: send more we can (nbytes: %d, max: %d)\n",
 			    __func__, nbytes, ce_state->src_sz_max);
 
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		return ret;
-
 	if (unlikely(CE_RING_DELTA(nentries_mask,
 				   write_index, sw_index - 1) <= 0)) {
 		ret = -ENOSR;
@@ -325,7 +321,6 @@ int ath10k_ce_send_nolock(struct ath10k_ce_pipe *ce_state,
 
 	src_ring->write_index = write_index;
 exit:
-	ath10k_pci_sleep(ar);
 	return ret;
 }
 
@@ -407,10 +402,6 @@ int ath10k_ce_recv_buf_enqueue(struct ath10k_ce_pipe *ce_state,
 	write_index = dest_ring->write_index;
 	sw_index = dest_ring->sw_index;
 
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		goto out;
-
 	if (CE_RING_DELTA(nentries_mask, write_index, sw_index - 1) > 0) {
 		struct ce_desc *base = dest_ring->base_addr_owner_space;
 		struct ce_desc *desc = CE_DEST_RING_TO_DESC(base, write_index);
@@ -430,11 +421,8 @@ int ath10k_ce_recv_buf_enqueue(struct ath10k_ce_pipe *ce_state,
 	} else {
 		ret = -EIO;
 	}
-	ath10k_pci_sleep(ar);
 
-out:
 	spin_unlock_bh(&ar_pci->ce_lock);
-
 	return ret;
 }
 
@@ -588,7 +576,6 @@ static int ath10k_ce_completed_send_next_nolock(struct ath10k_ce_pipe *ce_state,
 	unsigned int sw_index = src_ring->sw_index;
 	struct ce_desc *sdesc, *sbase;
 	unsigned int read_index;
-	int ret;
 
 	if (src_ring->hw_index == sw_index) {
 		/*
@@ -599,18 +586,12 @@ static int ath10k_ce_completed_send_next_nolock(struct ath10k_ce_pipe *ce_state,
 		 * value of the HW index has become stale.
 		 */
 
-		ret = ath10k_pci_wake(ar);
-		if (ret)
-			return ret;
-
 		read_index = ath10k_ce_src_ring_read_index_get(ar, ctrl_addr);
 		if (read_index == 0xffffffff)
 			return -ENODEV;
 
 		read_index &= nentries_mask;
 		src_ring->hw_index = read_index;
-
-		ath10k_pci_sleep(ar);
 	}
 
 	read_index = src_ring->hw_index;
@@ -731,11 +712,6 @@ void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id)
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	struct ath10k_ce_pipe *ce_state = &ar_pci->ce_states[ce_id];
 	u32 ctrl_addr = ce_state->ctrl_addr;
-	int ret;
-
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		return;
 
 	spin_lock_bh(&ar_pci->ce_lock);
 
@@ -760,7 +736,6 @@ void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id)
 	ath10k_ce_engine_int_status_clear(ar, ctrl_addr, CE_WATERMARK_MASK);
 
 	spin_unlock_bh(&ar_pci->ce_lock);
-	ath10k_pci_sleep(ar);
 }
 
 /*
@@ -771,13 +746,9 @@ void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id)
 
 void ath10k_ce_per_engine_service_any(struct ath10k *ar)
 {
-	int ce_id, ret;
+	int ce_id;
 	u32 intr_summary;
 
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		return;
-
 	intr_summary = CE_INTERRUPT_SUMMARY(ar);
 
 	for (ce_id = 0; intr_summary && (ce_id < CE_COUNT); ce_id++) {
@@ -789,8 +760,6 @@ void ath10k_ce_per_engine_service_any(struct ath10k *ar)
 
 		ath10k_ce_per_engine_service(ar, ce_id);
 	}
-
-	ath10k_pci_sleep(ar);
 }
 
 /*
@@ -805,11 +774,6 @@ static void ath10k_ce_per_engine_handler_adjust(struct ath10k_ce_pipe *ce_state,
 {
 	u32 ctrl_addr = ce_state->ctrl_addr;
 	struct ath10k *ar = ce_state->ar;
-	int ret;
-
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		return;
 
 	if ((!disable_copy_compl_intr) &&
 	    (ce_state->send_cb || ce_state->recv_cb))
@@ -818,17 +782,11 @@ static void ath10k_ce_per_engine_handler_adjust(struct ath10k_ce_pipe *ce_state,
 		ath10k_ce_copy_complete_intr_disable(ar, ctrl_addr);
 
 	ath10k_ce_watermark_intr_disable(ar, ctrl_addr);
-
-	ath10k_pci_sleep(ar);
 }
 
 int ath10k_ce_disable_interrupts(struct ath10k *ar)
 {
-	int ce_id, ret;
-
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		return ret;
+	int ce_id;
 
 	for (ce_id = 0; ce_id < CE_COUNT; ce_id++) {
 		u32 ctrl_addr = ath10k_ce_base_address(ce_id);
@@ -838,8 +796,6 @@ int ath10k_ce_disable_interrupts(struct ath10k *ar)
 		ath10k_ce_watermark_intr_disable(ar, ctrl_addr);
 	}
 
-	ath10k_pci_sleep(ar);
-
 	return 0;
 }
 
@@ -1084,10 +1040,6 @@ int ath10k_ce_init_pipe(struct ath10k *ar, unsigned int ce_id,
 	BUILD_BUG_ON(2*TARGET_10X_NUM_MSDU_DESC >
 		     (CE_HTT_H2T_MSG_SRC_NENTRIES - 1));
 
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		return ret;
-
 	spin_lock_bh(&ar_pci->ce_lock);
 	ce_state->ar = ar;
 	ce_state->id = ce_id;
@@ -1101,7 +1053,7 @@ int ath10k_ce_init_pipe(struct ath10k *ar, unsigned int ce_id,
 		if (ret) {
 			ath10k_err("Failed to initialize CE src ring for ID: %d (%d)\n",
 				   ce_id, ret);
-			goto out;
+			return ret;
 		}
 	}
 
@@ -1110,13 +1062,11 @@ int ath10k_ce_init_pipe(struct ath10k *ar, unsigned int ce_id,
 		if (ret) {
 			ath10k_err("Failed to initialize CE dest ring for ID: %d (%d)\n",
 				   ce_id, ret);
-			goto out;
+			return ret;
 		}
 	}
 
-out:
-	ath10k_pci_sleep(ar);
-	return ret;
+	return 0;
 }
 
 static void ath10k_ce_deinit_src_ring(struct ath10k *ar, unsigned int ce_id)
@@ -1140,16 +1090,8 @@ static void ath10k_ce_deinit_dest_ring(struct ath10k *ar, unsigned int ce_id)
 
 void ath10k_ce_deinit_pipe(struct ath10k *ar, unsigned int ce_id)
 {
-	int ret;
-
-	ret = ath10k_pci_wake(ar);
-	if (ret)
-		return;
-
 	ath10k_ce_deinit_src_ring(ar, ce_id);
 	ath10k_ce_deinit_dest_ring(ar, ce_id);
-
-	ath10k_pci_sleep(ar);
 }
 
 int ath10k_ce_alloc_pipe(struct ath10k *ar, int ce_id,
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index a2003b6..5fa45db 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -44,13 +44,9 @@ enum ath10k_pci_reset_mode {
 	ATH10K_PCI_RESET_WARM_ONLY = 1,
 };
 
-static unsigned int ath10k_pci_target_ps;
 static unsigned int ath10k_pci_irq_mode = ATH10K_PCI_IRQ_AUTO;
 static unsigned int ath10k_pci_reset_mode = ATH10K_PCI_RESET_AUTO;
 
-module_param_named(target_ps, ath10k_pci_target_ps, uint, 0644);
-MODULE_PARM_DESC(target_ps, "Enable ath10k Target (SoC) PS option");
-
 module_param_named(irq_mode, ath10k_pci_irq_mode, uint, 0644);
 MODULE_PARM_DESC(irq_mode, "0: auto, 1: legacy, 2: msi (default: 0)");
 
@@ -389,10 +385,8 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar, u32 address, void *data,
 		 * convert it from Target CPU virtual address space
 		 * to CE address space
 		 */
-		ath10k_pci_wake(ar);
 		address = TARG_CPU_SPACE_TO_CE_SPACE(ar, ar_pci->mem,
 						     address);
-		ath10k_pci_sleep(ar);
 
 		ret = ath10k_ce_send(ce_diag, NULL, (u32)address, nbytes, 0,
 				 0);
@@ -474,9 +468,7 @@ static int ath10k_pci_diag_read_access(struct ath10k *ar, u32 address,
 	if (address >= DRAM_BASE_ADDRESS)
 		return ath10k_pci_diag_read_mem(ar, address, data, sizeof(u32));
 
-	ath10k_pci_wake(ar);
 	*data = ath10k_pci_read32(ar, address);
-	ath10k_pci_sleep(ar);
 	return 0;
 }
 
@@ -528,9 +520,7 @@ static int ath10k_pci_diag_write_mem(struct ath10k *ar, u32 address,
 	 * to
 	 *    CE address space
 	 */
-	ath10k_pci_wake(ar);
 	address = TARG_CPU_SPACE_TO_CE_SPACE(ar, ar_pci->mem, address);
-	ath10k_pci_sleep(ar);
 
 	remaining_bytes = orig_nbytes;
 	ce_data = ce_data_base;
@@ -623,51 +613,25 @@ static int ath10k_pci_diag_write_access(struct ath10k *ar, u32 address,
 		return ath10k_pci_diag_write_mem(ar, address, &data,
 						 sizeof(u32));
 
-	ath10k_pci_wake(ar);
 	ath10k_pci_write32(ar, address, data);
-	ath10k_pci_sleep(ar);
 	return 0;
 }
 
-static bool ath10k_pci_target_is_awake(struct ath10k *ar)
+static bool ath10k_pci_is_awake(struct ath10k *ar)
 {
-	void __iomem *mem = ath10k_pci_priv(ar)->mem;
-	u32 val;
-	val = ioread32(mem + PCIE_LOCAL_BASE_ADDRESS +
-		       RTC_STATE_ADDRESS);
-	return (RTC_STATE_V_GET(val) == RTC_STATE_V_ON);
+	u32 val = ath10k_pci_reg_read32(ar, RTC_STATE_ADDRESS);
+
+	return RTC_STATE_V_GET(val) == RTC_STATE_V_ON;
 }
 
-int ath10k_do_pci_wake(struct ath10k *ar)
+static int ath10k_pci_wake_wait(struct ath10k *ar)
 {
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	void __iomem *pci_addr = ar_pci->mem;
 	int tot_delay = 0;
 	int curr_delay = 5;
 
-	if (atomic_read(&ar_pci->keep_awake_count) == 0) {
-		/* Force AWAKE */
-		iowrite32(PCIE_SOC_WAKE_V_MASK,
-			  pci_addr + PCIE_LOCAL_BASE_ADDRESS +
-			  PCIE_SOC_WAKE_ADDRESS);
-	}
-	atomic_inc(&ar_pci->keep_awake_count);
-
-	if (ar_pci->verified_awake)
-		return 0;
-
-	for (;;) {
-		if (ath10k_pci_target_is_awake(ar)) {
-			ar_pci->verified_awake = true;
+	while (tot_delay < PCIE_WAKE_TIMEOUT) {
+		if (ath10k_pci_is_awake(ar))
 			return 0;
-		}
-
-		if (tot_delay > PCIE_WAKE_TIMEOUT) {
-			ath10k_warn("target took longer %d us to wake up (awake count %d)\n",
-				    PCIE_WAKE_TIMEOUT,
-				    atomic_read(&ar_pci->keep_awake_count));
-			return -ETIMEDOUT;
-		}
 
 		udelay(curr_delay);
 		tot_delay += curr_delay;
@@ -675,20 +639,21 @@ int ath10k_do_pci_wake(struct ath10k *ar)
 		if (curr_delay < 50)
 			curr_delay += 5;
 	}
+
+	return -ETIMEDOUT;
 }
 
-void ath10k_do_pci_sleep(struct ath10k *ar)
+int ath10k_pci_wake(struct ath10k *ar)
 {
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	void __iomem *pci_addr = ar_pci->mem;
+	ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS,
+			       PCIE_SOC_WAKE_V_MASK);
+	return ath10k_pci_wake_wait(ar);
+}
 
-	if (atomic_dec_and_test(&ar_pci->keep_awake_count)) {
-		/* Allow sleep */
-		ar_pci->verified_awake = false;
-		iowrite32(PCIE_SOC_WAKE_RESET,
-			  pci_addr + PCIE_LOCAL_BASE_ADDRESS +
-			  PCIE_SOC_WAKE_ADDRESS);
-	}
+void ath10k_pci_sleep(struct ath10k *ar)
+{
+	ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS,
+			       PCIE_SOC_WAKE_RESET);
 }
 
 /* Called by lower (CE) layer when a send to Target completes. */
@@ -1788,8 +1753,6 @@ static void ath10k_pci_fw_interrupt_handler(struct ath10k *ar)
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	u32 fw_indicator;
 
-	ath10k_pci_wake(ar);
-
 	fw_indicator = ath10k_pci_read32(ar, FW_INDICATOR_ADDRESS);
 
 	if (fw_indicator & FW_IND_EVENT_PENDING) {
@@ -1807,8 +1770,6 @@ static void ath10k_pci_fw_interrupt_handler(struct ath10k *ar)
 			ath10k_warn("early firmware event indicated\n");
 		}
 	}
-
-	ath10k_pci_sleep(ar);
 }
 
 /* this function effectively clears target memory controller assert line */
@@ -1838,12 +1799,6 @@ static int ath10k_pci_warm_reset(struct ath10k *ar)
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot warm reset\n");
 
-	ret = ath10k_do_pci_wake(ar);
-	if (ret) {
-		ath10k_err("failed to wake up target: %d\n", ret);
-		return ret;
-	}
-
 	/* debug */
 	val = ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
 				PCIE_INTR_CAUSE_ADDRESS);
@@ -1915,7 +1870,6 @@ static int ath10k_pci_warm_reset(struct ath10k *ar)
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot warm reset complete\n");
 
-	ath10k_do_pci_sleep(ar);
 	return ret;
 }
 
@@ -1945,14 +1899,10 @@ static int __ath10k_pci_hif_power_up(struct ath10k *ar, bool cold_reset)
 		goto err;
 	}
 
-	if (!test_bit(ATH10K_PCI_FEATURE_SOC_POWER_SAVE, ar_pci->features))
-		/* Force AWAKE forever */
-		ath10k_do_pci_wake(ar);
-
 	ret = ath10k_pci_ce_init(ar);
 	if (ret) {
 		ath10k_err("failed to initialize CE: %d\n", ret);
-		goto err_ps;
+		goto err;
 	}
 
 	ret = ath10k_ce_disable_interrupts(ar);
@@ -2012,9 +1962,6 @@ err_deinit_irq:
 err_ce:
 	ath10k_pci_ce_deinit(ar);
 	ath10k_pci_warm_reset(ar);
-err_ps:
-	if (!test_bit(ATH10K_PCI_FEATURE_SOC_POWER_SAVE, ar_pci->features))
-		ath10k_do_pci_sleep(ar);
 err:
 	return ret;
 }
@@ -2078,8 +2025,6 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
 
 static void ath10k_pci_hif_power_down(struct ath10k *ar)
 {
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot hif power down\n");
 
 	ath10k_pci_free_early_irq(ar);
@@ -2087,9 +2032,6 @@ static void ath10k_pci_hif_power_down(struct ath10k *ar)
 	ath10k_pci_deinit_irq(ar);
 	ath10k_pci_ce_deinit(ar);
 	ath10k_pci_warm_reset(ar);
-
-	if (!test_bit(ATH10K_PCI_FEATURE_SOC_POWER_SAVE, ar_pci->features))
-		ath10k_do_pci_sleep(ar);
 }
 
 #ifdef CONFIG_PM
@@ -2236,14 +2178,6 @@ static void ath10k_pci_early_irq_tasklet(unsigned long data)
 {
 	struct ath10k *ar = (struct ath10k *)data;
 	u32 fw_ind;
-	int ret;
-
-	ret = ath10k_pci_wake(ar);
-	if (ret) {
-		ath10k_warn("failed to wake target in early irq tasklet: %d\n",
-			    ret);
-		return;
-	}
 
 	fw_ind = ath10k_pci_read32(ar, FW_INDICATOR_ADDRESS);
 	if (fw_ind & FW_IND_EVENT_PENDING) {
@@ -2252,7 +2186,6 @@ static void ath10k_pci_early_irq_tasklet(unsigned long data)
 		ath10k_pci_hif_dump_area(ar);
 	}
 
-	ath10k_pci_sleep(ar);
 	ath10k_pci_enable_legacy_irq(ar);
 }
 
@@ -2426,34 +2359,16 @@ static int ath10k_pci_init_irq(struct ath10k *ar)
 	 * synchronization checking. */
 	ar_pci->num_msi_intrs = 0;
 
-	ret = ath10k_pci_wake(ar);
-	if (ret) {
-		ath10k_warn("failed to wake target: %d\n", ret);
-		return ret;
-	}
-
 	ath10k_pci_write32(ar, SOC_CORE_BASE_ADDRESS + PCIE_INTR_ENABLE_ADDRESS,
 			   PCIE_INTR_FIRMWARE_MASK | PCIE_INTR_CE_MASK_ALL);
-	ath10k_pci_sleep(ar);
 
 	return 0;
 }
 
-static int ath10k_pci_deinit_irq_legacy(struct ath10k *ar)
+static void ath10k_pci_deinit_irq_legacy(struct ath10k *ar)
 {
-	int ret;
-
-	ret = ath10k_pci_wake(ar);
-	if (ret) {
-		ath10k_warn("failed to wake target: %d\n", ret);
-		return ret;
-	}
-
 	ath10k_pci_write32(ar, SOC_CORE_BASE_ADDRESS + PCIE_INTR_ENABLE_ADDRESS,
 			   0);
-	ath10k_pci_sleep(ar);
-
-	return 0;
 }
 
 static int ath10k_pci_deinit_irq(struct ath10k *ar)
@@ -2462,7 +2377,8 @@ static int ath10k_pci_deinit_irq(struct ath10k *ar)
 
 	switch (ar_pci->num_msi_intrs) {
 	case 0:
-		return ath10k_pci_deinit_irq_legacy(ar);
+		ath10k_pci_deinit_irq_legacy(ar);
+		return 0;
 	case 1:
 		/* fall-through */
 	case MSI_NUM_REQUEST:
@@ -2480,17 +2396,10 @@ static int ath10k_pci_wait_for_target_init(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	unsigned long timeout;
-	int ret;
 	u32 val;
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot waiting target to initialise\n");
 
-	ret = ath10k_pci_wake(ar);
-	if (ret) {
-		ath10k_err("failed to wake up target for init: %d\n", ret);
-		return ret;
-	}
-
 	timeout = jiffies + msecs_to_jiffies(ATH10K_PCI_TARGET_WAIT);
 
 	do {
@@ -2520,8 +2429,7 @@ static int ath10k_pci_wait_for_target_init(struct ath10k *ar)
 
 	if (val == 0xffffffff) {
 		ath10k_err("failed to read device register, device is gone\n");
-		ret = -EIO;
-		goto out;
+		return -EIO;
 	}
 
 	if (val & FW_IND_EVENT_PENDING) {
@@ -2529,38 +2437,26 @@ static int ath10k_pci_wait_for_target_init(struct ath10k *ar)
 		ath10k_pci_write32(ar, FW_INDICATOR_ADDRESS,
 				   val & ~FW_IND_EVENT_PENDING);
 		ath10k_pci_hif_dump_area(ar);
-		ret = -ECOMM;
-		goto out;
+		return -ECOMM;
 	}
 
 	if (!(val & FW_IND_INITIALIZED)) {
 		ath10k_err("failed to receive initialized event from target: %08x\n",
 			   val);
-		ret = -ETIMEDOUT;
-		goto out;
+		return -ETIMEDOUT;
 	}
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot target initialised\n");
-
-out:
-	ath10k_pci_sleep(ar);
-	return ret;
+	return 0;
 }
 
 static int ath10k_pci_cold_reset(struct ath10k *ar)
 {
-	int i, ret;
+	int i;
 	u32 val;
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot cold reset\n");
 
-	ret = ath10k_do_pci_wake(ar);
-	if (ret) {
-		ath10k_err("failed to wake up target: %d\n",
-			   ret);
-		return ret;
-	}
-
 	/* Put Target, including PCIe, into RESET. */
 	val = ath10k_pci_reg_read32(ar, SOC_GLOBAL_RESET_ADDRESS);
 	val |= 1;
@@ -2584,8 +2480,6 @@ static int ath10k_pci_cold_reset(struct ath10k *ar)
 		msleep(1);
 	}
 
-	ath10k_do_pci_sleep(ar);
-
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot cold reset complete\n");
 
 	return 0;
@@ -2603,9 +2497,6 @@ static void ath10k_pci_dump_features(struct ath10k_pci *ar_pci)
 		case ATH10K_PCI_FEATURE_MSI_X:
 			ath10k_dbg(ATH10K_DBG_BOOT, "device supports MSI-X\n");
 			break;
-		case ATH10K_PCI_FEATURE_SOC_POWER_SAVE:
-			ath10k_dbg(ATH10K_DBG_BOOT, "QCA98XX SoC power save enabled\n");
-			break;
 		}
 	}
 }
@@ -2642,13 +2533,9 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 		goto err_core_destroy;
 	}
 
-	if (ath10k_pci_target_ps)
-		set_bit(ATH10K_PCI_FEATURE_SOC_POWER_SAVE, ar_pci->features);
-
 	ath10k_pci_dump_features(ar_pci);
 
 	ar_pci->ar = ar;
-	atomic_set(&ar_pci->keep_awake_count, 0);
 
 	pci_set_drvdata(pdev, ar);
 
@@ -2703,20 +2590,22 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 
 	spin_lock_init(&ar_pci->ce_lock);
 
-	ret = ath10k_do_pci_wake(ar);
+	ret = ath10k_pci_wake(ar);
 	if (ret) {
-		ath10k_err("Failed to get chip id: %d\n", ret);
+		ath10k_err("failed to wake up: %d\n", ret);
 		goto err_iomap;
 	}
 
 	chip_id = ath10k_pci_soc_read32(ar, SOC_CHIP_ID_ADDRESS);
-
-	ath10k_do_pci_sleep(ar);
+	if (chip_id == 0xffffffff) {
+		ath10k_err("failed to get chip id\n");
+		goto err_sleep;
+	}
 
 	ret = ath10k_pci_alloc_ce(ar);
 	if (ret) {
 		ath10k_err("failed to allocate copy engine pipes: %d\n", ret);
-		goto err_iomap;
+		goto err_sleep;
 	}
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot pci_mem 0x%p\n", ar_pci->mem);
@@ -2731,6 +2620,8 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 
 err_free_ce:
 	ath10k_pci_free_ce(ar);
+err_sleep:
+	ath10k_pci_sleep(ar);
 err_iomap:
 	pci_iounmap(pdev, mem);
 err_master:
@@ -2762,6 +2653,7 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 
 	ath10k_core_unregister(ar);
 	ath10k_pci_free_ce(ar);
+	ath10k_pci_sleep(ar);
 
 	pci_iounmap(pdev, ar_pci->mem);
 	pci_release_region(pdev, BAR_NUM);
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 531c98a..65b1b90 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -137,7 +137,6 @@ struct service_to_pipe {
 
 enum ath10k_pci_features {
 	ATH10K_PCI_FEATURE_MSI_X		= 0,
-	ATH10K_PCI_FEATURE_SOC_POWER_SAVE	= 1,
 
 	/* keep last */
 	ATH10K_PCI_FEATURE_COUNT
@@ -183,9 +182,6 @@ struct ath10k_pci {
 
 	int started;
 
-	atomic_t keep_awake_count;
-	bool verified_awake;
-
 	struct ath10k_pci_pipe pipe_info[CE_COUNT_MAX];
 
 	struct ath10k_hif_cb msg_callbacks_current;
@@ -205,20 +201,6 @@ static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)
 	return (struct ath10k_pci *)ar->drv_priv;
 }
 
-static inline u32 ath10k_pci_reg_read32(struct ath10k *ar, u32 addr)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-
-	return ioread32(ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS + addr);
-}
-
-static inline void ath10k_pci_reg_write32(struct ath10k *ar, u32 addr, u32 val)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-
-	iowrite32(val, ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS + addr);
-}
-
 #define ATH_PCI_RESET_WAIT_MAX 10 /* ms */
 #define PCIE_WAKE_TIMEOUT 5000	/* 5ms */
 
@@ -242,35 +224,17 @@ static inline void ath10k_pci_reg_write32(struct ath10k *ar, u32 addr, u32 val)
 /* Wait up to this many Ms for a Diagnostic Access CE operation to complete */
 #define DIAG_ACCESS_CE_TIMEOUT_MS 10
 
-/*
- * This API allows the Host to access Target registers directly
- * and relatively efficiently over PCIe.
- * This allows the Host to avoid extra overhead associated with
- * sending a message to firmware and waiting for a response message
- * from firmware, as is done on other interconnects.
- *
- * Yet there is some complexity with direct accesses because the
- * Target's power state is not known a priori. The Host must issue
- * special PCIe reads/writes in order to explicitly wake the Target
- * and to verify that it is awake and will remain awake.
+/* Target exposes its registers for direct access. However before host can
+ * access them it needs to make sure the target is awake (ath10k_pci_wake,
+ * ath10k_pci_wake_wait, ath10k_pci_is_awake). Once target is awake it won't go
+ * to sleep unless host tells it to (ath10k_pci_sleep).
  *
- * Usage:
+ * If host tries to access target registers without waking it up it can
+ * scribble over host memory.
  *
- *   Use ath10k_pci_read32 and ath10k_pci_write32 to access Target space.
- *   These calls must be bracketed by ath10k_pci_wake and
- *   ath10k_pci_sleep.  A single BEGIN/END pair is adequate for
- *   multiple READ/WRITE operations.
- *
- *   Use ath10k_pci_wake to put the Target in a state in
- *   which it is legal for the Host to directly access it. This
- *   may involve waking the Target from a low power state, which
- *   may take up to 2Ms!
- *
- *   Use ath10k_pci_sleep to tell the Target that as far as
- *   this code path is concerned, it no longer needs to remain
- *   directly accessible.  BEGIN/END is under a reference counter;
- *   multiple code paths may issue BEGIN/END on a single targid.
+ * If target is asleep waking it up may take up to even 2ms.
  */
+
 static inline void ath10k_pci_write32(struct ath10k *ar, u32 offset,
 				      u32 value)
 {
@@ -296,25 +260,18 @@ static inline void ath10k_pci_soc_write32(struct ath10k *ar, u32 addr, u32 val)
 	ath10k_pci_write32(ar, RTC_SOC_BASE_ADDRESS + addr, val);
 }
 
-int ath10k_do_pci_wake(struct ath10k *ar);
-void ath10k_do_pci_sleep(struct ath10k *ar);
-
-static inline int ath10k_pci_wake(struct ath10k *ar)
+static inline u32 ath10k_pci_reg_read32(struct ath10k *ar, u32 addr)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 
-	if (test_bit(ATH10K_PCI_FEATURE_SOC_POWER_SAVE, ar_pci->features))
-		return ath10k_do_pci_wake(ar);
-
-	return 0;
+	return ioread32(ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS + addr);
 }
 
-static inline void ath10k_pci_sleep(struct ath10k *ar)
+static inline void ath10k_pci_reg_write32(struct ath10k *ar, u32 addr, u32 val)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 
-	if (test_bit(ATH10K_PCI_FEATURE_SOC_POWER_SAVE, ar_pci->features))
-		ath10k_do_pci_sleep(ar);
+	iowrite32(val, ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS + addr);
 }
 
 #endif /* _PCI_H_ */
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 3/6] ath10k: remove pci features var
  2014-08-07  9:03 ` Michal Kazior
@ 2014-08-07  9:03   ` Michal Kazior
  -1 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-07  9:03 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

The ATH10K_PCI_FEATURE_MSI_X was originally
introduced to support both chips QCA988Xv1 and
QCA988Xv2. Since v1 isn't supported anymore it
doesn't make sense to keep the feature flag
around. Since this is the last one remove the
whole thing.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/pci.c | 33 +--------------------------------
 drivers/net/wireless/ath/ath10k/pci.h |  9 ---------
 2 files changed, 1 insertion(+), 41 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 5fa45db..c31edb6 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2318,8 +2318,6 @@ static void ath10k_pci_init_irq_tasklets(struct ath10k *ar)
 static int ath10k_pci_init_irq(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	bool msix_supported = test_bit(ATH10K_PCI_FEATURE_MSI_X,
-				       ar_pci->features);
 	int ret;
 
 	ath10k_pci_init_irq_tasklets(ar);
@@ -2329,7 +2327,7 @@ static int ath10k_pci_init_irq(struct ath10k *ar)
 		ath10k_info("limiting irq mode to: %d\n", ath10k_pci_irq_mode);
 
 	/* Try MSI-X */
-	if (ath10k_pci_irq_mode == ATH10K_PCI_IRQ_AUTO && msix_supported) {
+	if (ath10k_pci_irq_mode == ATH10K_PCI_IRQ_AUTO) {
 		ar_pci->num_msi_intrs = MSI_NUM_REQUEST;
 		ret = pci_enable_msi_range(ar_pci->pdev, ar_pci->num_msi_intrs,
 							 ar_pci->num_msi_intrs);
@@ -2485,22 +2483,6 @@ static int ath10k_pci_cold_reset(struct ath10k *ar)
 	return 0;
 }
 
-static void ath10k_pci_dump_features(struct ath10k_pci *ar_pci)
-{
-	int i;
-
-	for (i = 0; i < ATH10K_PCI_FEATURE_COUNT; i++) {
-		if (!test_bit(i, ar_pci->features))
-			continue;
-
-		switch (i) {
-		case ATH10K_PCI_FEATURE_MSI_X:
-			ath10k_dbg(ATH10K_DBG_BOOT, "device supports MSI-X\n");
-			break;
-		}
-	}
-}
-
 static int ath10k_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *pci_dev)
 {
@@ -2522,19 +2504,6 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	ar_pci = ath10k_pci_priv(ar);
 	ar_pci->pdev = pdev;
 	ar_pci->dev = &pdev->dev;
-
-	switch (pci_dev->device) {
-	case QCA988X_2_0_DEVICE_ID:
-		set_bit(ATH10K_PCI_FEATURE_MSI_X, ar_pci->features);
-		break;
-	default:
-		ret = -ENODEV;
-		ath10k_err("Unknown device ID: %d\n", pci_dev->device);
-		goto err_core_destroy;
-	}
-
-	ath10k_pci_dump_features(ar_pci);
-
 	ar_pci->ar = ar;
 
 	pci_set_drvdata(pdev, ar);
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 65b1b90..662bca3 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -135,13 +135,6 @@ struct service_to_pipe {
 	u32 pipenum;
 };
 
-enum ath10k_pci_features {
-	ATH10K_PCI_FEATURE_MSI_X		= 0,
-
-	/* keep last */
-	ATH10K_PCI_FEATURE_COUNT
-};
-
 /* Per-pipe state. */
 struct ath10k_pci_pipe {
 	/* Handle of underlying Copy Engine */
@@ -168,8 +161,6 @@ struct ath10k_pci {
 	struct ath10k *ar;
 	void __iomem *mem;
 
-	DECLARE_BITMAP(features, ATH10K_PCI_FEATURE_COUNT);
-
 	/*
 	 * Number of MSI interrupts granted, 0 --> using legacy PCI line
 	 * interrupts.
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 3/6] ath10k: remove pci features var
@ 2014-08-07  9:03   ` Michal Kazior
  0 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-07  9:03 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

The ATH10K_PCI_FEATURE_MSI_X was originally
introduced to support both chips QCA988Xv1 and
QCA988Xv2. Since v1 isn't supported anymore it
doesn't make sense to keep the feature flag
around. Since this is the last one remove the
whole thing.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/pci.c | 33 +--------------------------------
 drivers/net/wireless/ath/ath10k/pci.h |  9 ---------
 2 files changed, 1 insertion(+), 41 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 5fa45db..c31edb6 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2318,8 +2318,6 @@ static void ath10k_pci_init_irq_tasklets(struct ath10k *ar)
 static int ath10k_pci_init_irq(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	bool msix_supported = test_bit(ATH10K_PCI_FEATURE_MSI_X,
-				       ar_pci->features);
 	int ret;
 
 	ath10k_pci_init_irq_tasklets(ar);
@@ -2329,7 +2327,7 @@ static int ath10k_pci_init_irq(struct ath10k *ar)
 		ath10k_info("limiting irq mode to: %d\n", ath10k_pci_irq_mode);
 
 	/* Try MSI-X */
-	if (ath10k_pci_irq_mode == ATH10K_PCI_IRQ_AUTO && msix_supported) {
+	if (ath10k_pci_irq_mode == ATH10K_PCI_IRQ_AUTO) {
 		ar_pci->num_msi_intrs = MSI_NUM_REQUEST;
 		ret = pci_enable_msi_range(ar_pci->pdev, ar_pci->num_msi_intrs,
 							 ar_pci->num_msi_intrs);
@@ -2485,22 +2483,6 @@ static int ath10k_pci_cold_reset(struct ath10k *ar)
 	return 0;
 }
 
-static void ath10k_pci_dump_features(struct ath10k_pci *ar_pci)
-{
-	int i;
-
-	for (i = 0; i < ATH10K_PCI_FEATURE_COUNT; i++) {
-		if (!test_bit(i, ar_pci->features))
-			continue;
-
-		switch (i) {
-		case ATH10K_PCI_FEATURE_MSI_X:
-			ath10k_dbg(ATH10K_DBG_BOOT, "device supports MSI-X\n");
-			break;
-		}
-	}
-}
-
 static int ath10k_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *pci_dev)
 {
@@ -2522,19 +2504,6 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	ar_pci = ath10k_pci_priv(ar);
 	ar_pci->pdev = pdev;
 	ar_pci->dev = &pdev->dev;
-
-	switch (pci_dev->device) {
-	case QCA988X_2_0_DEVICE_ID:
-		set_bit(ATH10K_PCI_FEATURE_MSI_X, ar_pci->features);
-		break;
-	default:
-		ret = -ENODEV;
-		ath10k_err("Unknown device ID: %d\n", pci_dev->device);
-		goto err_core_destroy;
-	}
-
-	ath10k_pci_dump_features(ar_pci);
-
 	ar_pci->ar = ar;
 
 	pci_set_drvdata(pdev, ar);
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 65b1b90..662bca3 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -135,13 +135,6 @@ struct service_to_pipe {
 	u32 pipenum;
 };
 
-enum ath10k_pci_features {
-	ATH10K_PCI_FEATURE_MSI_X		= 0,
-
-	/* keep last */
-	ATH10K_PCI_FEATURE_COUNT
-};
-
 /* Per-pipe state. */
 struct ath10k_pci_pipe {
 	/* Handle of underlying Copy Engine */
@@ -168,8 +161,6 @@ struct ath10k_pci {
 	struct ath10k *ar;
 	void __iomem *mem;
 
-	DECLARE_BITMAP(features, ATH10K_PCI_FEATURE_COUNT);
-
 	/*
 	 * Number of MSI interrupts granted, 0 --> using legacy PCI line
 	 * interrupts.
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 4/6] ath10k: group some pci probing helpers
  2014-08-07  9:03 ` Michal Kazior
@ 2014-08-07  9:03   ` Michal Kazior
  -1 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-07  9:03 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Make probe/remove functions shorter and easier to
understand.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index c31edb6..cfc1da2 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2483,86 +2483,116 @@ static int ath10k_pci_cold_reset(struct ath10k *ar)
 	return 0;
 }
 
-static int ath10k_pci_probe(struct pci_dev *pdev,
-			    const struct pci_device_id *pci_dev)
+static int ath10k_pci_claim(struct ath10k *ar)
 {
-	void __iomem *mem;
-	int ret = 0;
-	struct ath10k *ar;
-	struct ath10k_pci *ar_pci;
-	u32 lcr_val, chip_id;
-
-	ath10k_dbg(ATH10K_DBG_PCI, "pci probe\n");
-
-	ar = ath10k_core_create(sizeof(*ar_pci), &pdev->dev,
-				&ath10k_pci_hif_ops);
-	if (!ar) {
-		ath10k_err("failed to allocate core\n");
-		return -ENOMEM;
-	}
-
-	ar_pci = ath10k_pci_priv(ar);
-	ar_pci->pdev = pdev;
-	ar_pci->dev = &pdev->dev;
-	ar_pci->ar = ar;
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	struct pci_dev *pdev = ar_pci->pdev;
+	u32 lcr_val;
+	int ret;
 
 	pci_set_drvdata(pdev, ar);
 
 	ret = pci_enable_device(pdev);
 	if (ret) {
-		ath10k_err("failed to enable PCI device: %d\n", ret);
-		goto err_core_destroy;
+		ath10k_err("failed to enable pci device: %d\n", ret);
+		return ret;
 	}
 
-	/* Request MMIO resources */
 	ret = pci_request_region(pdev, BAR_NUM, "ath");
 	if (ret) {
-		ath10k_err("failed to request MMIO region: %d\n", ret);
+		ath10k_err("failed to request region BAR%d: %d\n", BAR_NUM,
+			   ret);
 		goto err_device;
 	}
 
-	/*
-	 * Target structures have a limit of 32 bit DMA pointers.
-	 * DMA pointers can be wider than 32 bits by default on some systems.
-	 */
+	/* Target expects 32 bit DMA. Enforce it. */
 	ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
 	if (ret) {
-		ath10k_err("failed to set DMA mask to 32-bit: %d\n", ret);
+		ath10k_err("failed to set dma mask to 32-bit: %d\n", ret);
 		goto err_region;
 	}
 
 	ret = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
 	if (ret) {
-		ath10k_err("failed to set consistent DMA mask to 32-bit\n");
+		ath10k_err("failed to set consistent dma mask to 32-bit: %d\n",
+			   ret);
 		goto err_region;
 	}
 
-	/* Set bus master bit in PCI_COMMAND to enable DMA */
 	pci_set_master(pdev);
 
-	/*
-	 * Temporary FIX: disable ASPM
-	 * Will be removed after the OTP is programmed
-	 */
+	/* Workaround: Disable ASPM */
 	pci_read_config_dword(pdev, 0x80, &lcr_val);
 	pci_write_config_dword(pdev, 0x80, (lcr_val & 0xffffff00));
 
 	/* Arrange for access to Target SoC registers. */
-	mem = pci_iomap(pdev, BAR_NUM, 0);
-	if (!mem) {
-		ath10k_err("failed to perform IOMAP for BAR%d\n", BAR_NUM);
+	ar_pci->mem = pci_iomap(pdev, BAR_NUM, 0);
+	if (!ar_pci->mem) {
+		ath10k_err("failed to iomap BAR%d\n", BAR_NUM);
 		ret = -EIO;
 		goto err_master;
 	}
 
-	ar_pci->mem = mem;
+	ath10k_dbg(ATH10K_DBG_BOOT, "boot pci_mem 0x%p\n", ar_pci->mem);
+	return 0;
+
+err_master:
+	pci_clear_master(pdev);
+
+err_region:
+	pci_release_region(pdev, BAR_NUM);
+
+err_device:
+	pci_disable_device(pdev);
+
+	return ret;
+}
+
+static void ath10k_pci_release(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	struct pci_dev *pdev = ar_pci->pdev;
+
+	pci_iounmap(pdev, ar_pci->mem);
+	pci_release_region(pdev, BAR_NUM);
+	pci_clear_master(pdev);
+	pci_disable_device(pdev);
+}
+
+static int ath10k_pci_probe(struct pci_dev *pdev,
+			    const struct pci_device_id *pci_dev)
+{
+	int ret = 0;
+	struct ath10k *ar;
+	struct ath10k_pci *ar_pci;
+	u32 chip_id;
+
+	ath10k_dbg(ATH10K_DBG_PCI, "pci probe\n");
+
+	ar = ath10k_core_create(sizeof(*ar_pci), &pdev->dev,
+				&ath10k_pci_hif_ops);
+	if (!ar) {
+		ath10k_err("failed to allocate core\n");
+		return -ENOMEM;
+	}
+
+	ar_pci = ath10k_pci_priv(ar);
+	ar_pci->pdev = pdev;
+	ar_pci->dev = &pdev->dev;
+	ar_pci->ar = ar;
 
 	spin_lock_init(&ar_pci->ce_lock);
 
+	ret = ath10k_pci_claim(ar);
+	if (ret) {
+		ath10k_err("failed to claim device: %d\n", ret);
+		goto err_core_destroy;
+	}
+
 	ret = ath10k_pci_wake(ar);
 	if (ret) {
 		ath10k_err("failed to wake up: %d\n", ret);
-		goto err_iomap;
+		goto err_release;
 	}
 
 	chip_id = ath10k_pci_soc_read32(ar, SOC_CHIP_ID_ADDRESS);
@@ -2577,8 +2607,6 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 		goto err_sleep;
 	}
 
-	ath10k_dbg(ATH10K_DBG_BOOT, "boot pci_mem 0x%p\n", ar_pci->mem);
-
 	ret = ath10k_core_register(ar, chip_id);
 	if (ret) {
 		ath10k_err("failed to register driver core: %d\n", ret);
@@ -2589,16 +2617,13 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 
 err_free_ce:
 	ath10k_pci_free_ce(ar);
+
 err_sleep:
 	ath10k_pci_sleep(ar);
-err_iomap:
-	pci_iounmap(pdev, mem);
-err_master:
-	pci_clear_master(pdev);
-err_region:
-	pci_release_region(pdev, BAR_NUM);
-err_device:
-	pci_disable_device(pdev);
+
+err_release:
+	ath10k_pci_release(ar);
+
 err_core_destroy:
 	ath10k_core_destroy(ar);
 
@@ -2623,12 +2648,7 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 	ath10k_core_unregister(ar);
 	ath10k_pci_free_ce(ar);
 	ath10k_pci_sleep(ar);
-
-	pci_iounmap(pdev, ar_pci->mem);
-	pci_release_region(pdev, BAR_NUM);
-	pci_clear_master(pdev);
-	pci_disable_device(pdev);
-
+	ath10k_pci_release(ar);
 	ath10k_core_destroy(ar);
 }
 
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 4/6] ath10k: group some pci probing helpers
@ 2014-08-07  9:03   ` Michal Kazior
  0 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-07  9:03 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Make probe/remove functions shorter and easier to
understand.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index c31edb6..cfc1da2 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2483,86 +2483,116 @@ static int ath10k_pci_cold_reset(struct ath10k *ar)
 	return 0;
 }
 
-static int ath10k_pci_probe(struct pci_dev *pdev,
-			    const struct pci_device_id *pci_dev)
+static int ath10k_pci_claim(struct ath10k *ar)
 {
-	void __iomem *mem;
-	int ret = 0;
-	struct ath10k *ar;
-	struct ath10k_pci *ar_pci;
-	u32 lcr_val, chip_id;
-
-	ath10k_dbg(ATH10K_DBG_PCI, "pci probe\n");
-
-	ar = ath10k_core_create(sizeof(*ar_pci), &pdev->dev,
-				&ath10k_pci_hif_ops);
-	if (!ar) {
-		ath10k_err("failed to allocate core\n");
-		return -ENOMEM;
-	}
-
-	ar_pci = ath10k_pci_priv(ar);
-	ar_pci->pdev = pdev;
-	ar_pci->dev = &pdev->dev;
-	ar_pci->ar = ar;
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	struct pci_dev *pdev = ar_pci->pdev;
+	u32 lcr_val;
+	int ret;
 
 	pci_set_drvdata(pdev, ar);
 
 	ret = pci_enable_device(pdev);
 	if (ret) {
-		ath10k_err("failed to enable PCI device: %d\n", ret);
-		goto err_core_destroy;
+		ath10k_err("failed to enable pci device: %d\n", ret);
+		return ret;
 	}
 
-	/* Request MMIO resources */
 	ret = pci_request_region(pdev, BAR_NUM, "ath");
 	if (ret) {
-		ath10k_err("failed to request MMIO region: %d\n", ret);
+		ath10k_err("failed to request region BAR%d: %d\n", BAR_NUM,
+			   ret);
 		goto err_device;
 	}
 
-	/*
-	 * Target structures have a limit of 32 bit DMA pointers.
-	 * DMA pointers can be wider than 32 bits by default on some systems.
-	 */
+	/* Target expects 32 bit DMA. Enforce it. */
 	ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
 	if (ret) {
-		ath10k_err("failed to set DMA mask to 32-bit: %d\n", ret);
+		ath10k_err("failed to set dma mask to 32-bit: %d\n", ret);
 		goto err_region;
 	}
 
 	ret = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
 	if (ret) {
-		ath10k_err("failed to set consistent DMA mask to 32-bit\n");
+		ath10k_err("failed to set consistent dma mask to 32-bit: %d\n",
+			   ret);
 		goto err_region;
 	}
 
-	/* Set bus master bit in PCI_COMMAND to enable DMA */
 	pci_set_master(pdev);
 
-	/*
-	 * Temporary FIX: disable ASPM
-	 * Will be removed after the OTP is programmed
-	 */
+	/* Workaround: Disable ASPM */
 	pci_read_config_dword(pdev, 0x80, &lcr_val);
 	pci_write_config_dword(pdev, 0x80, (lcr_val & 0xffffff00));
 
 	/* Arrange for access to Target SoC registers. */
-	mem = pci_iomap(pdev, BAR_NUM, 0);
-	if (!mem) {
-		ath10k_err("failed to perform IOMAP for BAR%d\n", BAR_NUM);
+	ar_pci->mem = pci_iomap(pdev, BAR_NUM, 0);
+	if (!ar_pci->mem) {
+		ath10k_err("failed to iomap BAR%d\n", BAR_NUM);
 		ret = -EIO;
 		goto err_master;
 	}
 
-	ar_pci->mem = mem;
+	ath10k_dbg(ATH10K_DBG_BOOT, "boot pci_mem 0x%p\n", ar_pci->mem);
+	return 0;
+
+err_master:
+	pci_clear_master(pdev);
+
+err_region:
+	pci_release_region(pdev, BAR_NUM);
+
+err_device:
+	pci_disable_device(pdev);
+
+	return ret;
+}
+
+static void ath10k_pci_release(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	struct pci_dev *pdev = ar_pci->pdev;
+
+	pci_iounmap(pdev, ar_pci->mem);
+	pci_release_region(pdev, BAR_NUM);
+	pci_clear_master(pdev);
+	pci_disable_device(pdev);
+}
+
+static int ath10k_pci_probe(struct pci_dev *pdev,
+			    const struct pci_device_id *pci_dev)
+{
+	int ret = 0;
+	struct ath10k *ar;
+	struct ath10k_pci *ar_pci;
+	u32 chip_id;
+
+	ath10k_dbg(ATH10K_DBG_PCI, "pci probe\n");
+
+	ar = ath10k_core_create(sizeof(*ar_pci), &pdev->dev,
+				&ath10k_pci_hif_ops);
+	if (!ar) {
+		ath10k_err("failed to allocate core\n");
+		return -ENOMEM;
+	}
+
+	ar_pci = ath10k_pci_priv(ar);
+	ar_pci->pdev = pdev;
+	ar_pci->dev = &pdev->dev;
+	ar_pci->ar = ar;
 
 	spin_lock_init(&ar_pci->ce_lock);
 
+	ret = ath10k_pci_claim(ar);
+	if (ret) {
+		ath10k_err("failed to claim device: %d\n", ret);
+		goto err_core_destroy;
+	}
+
 	ret = ath10k_pci_wake(ar);
 	if (ret) {
 		ath10k_err("failed to wake up: %d\n", ret);
-		goto err_iomap;
+		goto err_release;
 	}
 
 	chip_id = ath10k_pci_soc_read32(ar, SOC_CHIP_ID_ADDRESS);
@@ -2577,8 +2607,6 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 		goto err_sleep;
 	}
 
-	ath10k_dbg(ATH10K_DBG_BOOT, "boot pci_mem 0x%p\n", ar_pci->mem);
-
 	ret = ath10k_core_register(ar, chip_id);
 	if (ret) {
 		ath10k_err("failed to register driver core: %d\n", ret);
@@ -2589,16 +2617,13 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 
 err_free_ce:
 	ath10k_pci_free_ce(ar);
+
 err_sleep:
 	ath10k_pci_sleep(ar);
-err_iomap:
-	pci_iounmap(pdev, mem);
-err_master:
-	pci_clear_master(pdev);
-err_region:
-	pci_release_region(pdev, BAR_NUM);
-err_device:
-	pci_disable_device(pdev);
+
+err_release:
+	ath10k_pci_release(ar);
+
 err_core_destroy:
 	ath10k_core_destroy(ar);
 
@@ -2623,12 +2648,7 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 	ath10k_core_unregister(ar);
 	ath10k_pci_free_ce(ar);
 	ath10k_pci_sleep(ar);
-
-	pci_iounmap(pdev, ar_pci->mem);
-	pci_release_region(pdev, BAR_NUM);
-	pci_clear_master(pdev);
-	pci_disable_device(pdev);
-
+	ath10k_pci_release(ar);
 	ath10k_core_destroy(ar);
 }
 
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 5/6] ath10k: remove htc->stopped
  2014-08-07  9:03 ` Michal Kazior
@ 2014-08-07  9:03   ` Michal Kazior
  -1 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-07  9:03 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

This is not necessary anymore. There are no more
uncontrolled htc tx entry points.

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

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 2790aad..8eae2aa 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -780,7 +780,7 @@ int ath10k_core_start(struct ath10k *ar)
 	if (status <= 0) {
 		ath10k_warn("wmi service ready event not received");
 		status = -ETIMEDOUT;
-		goto err_htc_stop;
+		goto err_hif_stop;
 	}
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "firmware %s booted\n",
@@ -789,25 +789,25 @@ int ath10k_core_start(struct ath10k *ar)
 	status = ath10k_wmi_cmd_init(ar);
 	if (status) {
 		ath10k_err("could not send WMI init command (%d)\n", status);
-		goto err_htc_stop;
+		goto err_hif_stop;
 	}
 
 	status = ath10k_wmi_wait_for_unified_ready(ar);
 	if (status <= 0) {
 		ath10k_err("wmi unified ready event not received\n");
 		status = -ETIMEDOUT;
-		goto err_htc_stop;
+		goto err_hif_stop;
 	}
 
 	status = ath10k_htt_setup(&ar->htt);
 	if (status) {
 		ath10k_err("failed to setup htt: %d\n", status);
-		goto err_htc_stop;
+		goto err_hif_stop;
 	}
 
 	status = ath10k_debug_start(ar);
 	if (status)
-		goto err_htc_stop;
+		goto err_hif_stop;
 
 	if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
 		ar->free_vdev_map = (1 << TARGET_10X_NUM_VDEVS) - 1;
@@ -836,8 +836,6 @@ int ath10k_core_start(struct ath10k *ar)
 
 	return 0;
 
-err_htc_stop:
-	ath10k_htc_stop(&ar->htc);
 err_hif_stop:
 	ath10k_hif_stop(ar);
 err_htt_rx_detach:
@@ -882,7 +880,6 @@ void ath10k_core_stop(struct ath10k *ar)
 		ath10k_wait_for_suspend(ar, WMI_PDEV_SUSPEND_AND_DISABLE_INTR);
 
 	ath10k_debug_stop(ar);
-	ath10k_htc_stop(&ar->htc);
 	ath10k_hif_stop(ar);
 	ath10k_htt_tx_free(&ar->htt);
 	ath10k_htt_rx_free(&ar->htt);
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 5fdc40d..7e08bb3 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -138,14 +138,6 @@ int ath10k_htc_send(struct ath10k_htc *htc,
 		return -ENOENT;
 	}
 
-	/* FIXME: This looks ugly, can we fix it? */
-	spin_lock_bh(&htc->tx_lock);
-	if (htc->stopped) {
-		spin_unlock_bh(&htc->tx_lock);
-		return -ESHUTDOWN;
-	}
-	spin_unlock_bh(&htc->tx_lock);
-
 	skb_push(skb, sizeof(struct ath10k_htc_hdr));
 
 	if (ep->tx_credit_flow_enabled) {
@@ -846,13 +838,6 @@ int ath10k_htc_start(struct ath10k_htc *htc)
 	return 0;
 }
 
-void ath10k_htc_stop(struct ath10k_htc *htc)
-{
-	spin_lock_bh(&htc->tx_lock);
-	htc->stopped = true;
-	spin_unlock_bh(&htc->tx_lock);
-}
-
 /* registered target arrival callback from the HIF layer */
 int ath10k_htc_init(struct ath10k *ar)
 {
@@ -862,7 +847,6 @@ int ath10k_htc_init(struct ath10k *ar)
 
 	spin_lock_init(&htc->tx_lock);
 
-	htc->stopped = false;
 	ath10k_htc_reset_endpoint_states(htc);
 
 	/* setup HIF layer callbacks */
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index 4716d33..b5a9daa 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -332,7 +332,7 @@ struct ath10k_htc {
 	struct ath10k *ar;
 	struct ath10k_htc_ep endpoint[ATH10K_HTC_EP_COUNT];
 
-	/* protects endpoint and stopped fields */
+	/* protects endpoints */
 	spinlock_t tx_lock;
 
 	struct ath10k_htc_ops htc_ops;
@@ -345,8 +345,6 @@ struct ath10k_htc {
 	int total_transmit_credits;
 	struct ath10k_htc_svc_tx_credits service_tx_alloc[ATH10K_HTC_EP_COUNT];
 	int target_credit_size;
-
-	bool stopped;
 };
 
 int ath10k_htc_init(struct ath10k *ar);
@@ -357,7 +355,6 @@ int ath10k_htc_connect_service(struct ath10k_htc *htc,
 			       struct ath10k_htc_svc_conn_resp *conn_resp);
 int ath10k_htc_send(struct ath10k_htc *htc, enum ath10k_htc_ep_id eid,
 		    struct sk_buff *packet);
-void ath10k_htc_stop(struct ath10k_htc *htc);
 struct sk_buff *ath10k_htc_alloc_skb(int size);
 
 #endif
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 5/6] ath10k: remove htc->stopped
@ 2014-08-07  9:03   ` Michal Kazior
  0 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-07  9:03 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

This is not necessary anymore. There are no more
uncontrolled htc tx entry points.

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

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 2790aad..8eae2aa 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -780,7 +780,7 @@ int ath10k_core_start(struct ath10k *ar)
 	if (status <= 0) {
 		ath10k_warn("wmi service ready event not received");
 		status = -ETIMEDOUT;
-		goto err_htc_stop;
+		goto err_hif_stop;
 	}
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "firmware %s booted\n",
@@ -789,25 +789,25 @@ int ath10k_core_start(struct ath10k *ar)
 	status = ath10k_wmi_cmd_init(ar);
 	if (status) {
 		ath10k_err("could not send WMI init command (%d)\n", status);
-		goto err_htc_stop;
+		goto err_hif_stop;
 	}
 
 	status = ath10k_wmi_wait_for_unified_ready(ar);
 	if (status <= 0) {
 		ath10k_err("wmi unified ready event not received\n");
 		status = -ETIMEDOUT;
-		goto err_htc_stop;
+		goto err_hif_stop;
 	}
 
 	status = ath10k_htt_setup(&ar->htt);
 	if (status) {
 		ath10k_err("failed to setup htt: %d\n", status);
-		goto err_htc_stop;
+		goto err_hif_stop;
 	}
 
 	status = ath10k_debug_start(ar);
 	if (status)
-		goto err_htc_stop;
+		goto err_hif_stop;
 
 	if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
 		ar->free_vdev_map = (1 << TARGET_10X_NUM_VDEVS) - 1;
@@ -836,8 +836,6 @@ int ath10k_core_start(struct ath10k *ar)
 
 	return 0;
 
-err_htc_stop:
-	ath10k_htc_stop(&ar->htc);
 err_hif_stop:
 	ath10k_hif_stop(ar);
 err_htt_rx_detach:
@@ -882,7 +880,6 @@ void ath10k_core_stop(struct ath10k *ar)
 		ath10k_wait_for_suspend(ar, WMI_PDEV_SUSPEND_AND_DISABLE_INTR);
 
 	ath10k_debug_stop(ar);
-	ath10k_htc_stop(&ar->htc);
 	ath10k_hif_stop(ar);
 	ath10k_htt_tx_free(&ar->htt);
 	ath10k_htt_rx_free(&ar->htt);
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 5fdc40d..7e08bb3 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -138,14 +138,6 @@ int ath10k_htc_send(struct ath10k_htc *htc,
 		return -ENOENT;
 	}
 
-	/* FIXME: This looks ugly, can we fix it? */
-	spin_lock_bh(&htc->tx_lock);
-	if (htc->stopped) {
-		spin_unlock_bh(&htc->tx_lock);
-		return -ESHUTDOWN;
-	}
-	spin_unlock_bh(&htc->tx_lock);
-
 	skb_push(skb, sizeof(struct ath10k_htc_hdr));
 
 	if (ep->tx_credit_flow_enabled) {
@@ -846,13 +838,6 @@ int ath10k_htc_start(struct ath10k_htc *htc)
 	return 0;
 }
 
-void ath10k_htc_stop(struct ath10k_htc *htc)
-{
-	spin_lock_bh(&htc->tx_lock);
-	htc->stopped = true;
-	spin_unlock_bh(&htc->tx_lock);
-}
-
 /* registered target arrival callback from the HIF layer */
 int ath10k_htc_init(struct ath10k *ar)
 {
@@ -862,7 +847,6 @@ int ath10k_htc_init(struct ath10k *ar)
 
 	spin_lock_init(&htc->tx_lock);
 
-	htc->stopped = false;
 	ath10k_htc_reset_endpoint_states(htc);
 
 	/* setup HIF layer callbacks */
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index 4716d33..b5a9daa 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -332,7 +332,7 @@ struct ath10k_htc {
 	struct ath10k *ar;
 	struct ath10k_htc_ep endpoint[ATH10K_HTC_EP_COUNT];
 
-	/* protects endpoint and stopped fields */
+	/* protects endpoints */
 	spinlock_t tx_lock;
 
 	struct ath10k_htc_ops htc_ops;
@@ -345,8 +345,6 @@ struct ath10k_htc {
 	int total_transmit_credits;
 	struct ath10k_htc_svc_tx_credits service_tx_alloc[ATH10K_HTC_EP_COUNT];
 	int target_credit_size;
-
-	bool stopped;
 };
 
 int ath10k_htc_init(struct ath10k *ar);
@@ -357,7 +355,6 @@ int ath10k_htc_connect_service(struct ath10k_htc *htc,
 			       struct ath10k_htc_svc_conn_resp *conn_resp);
 int ath10k_htc_send(struct ath10k_htc *htc, enum ath10k_htc_ep_id eid,
 		    struct sk_buff *packet);
-void ath10k_htc_stop(struct ath10k_htc *htc);
 struct sk_buff *ath10k_htc_alloc_skb(int size);
 
 #endif
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 6/6] ath10k: move fw init print
  2014-08-07  9:03 ` Michal Kazior
@ 2014-08-07  9:03   ` Michal Kazior
  -1 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-07  9:03 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Firmware probing is done only once when driver is
registered and firmware version is guaranteed to
remain the same until driver is unregistered.

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

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 8eae2aa..b5161c8 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -816,24 +816,6 @@ int ath10k_core_start(struct ath10k *ar)
 
 	INIT_LIST_HEAD(&ar->arvifs);
 
-	if (!test_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags)) {
-		ath10k_info("%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d\n",
-			    ar->hw_params.name,
-			    ar->target_version,
-			    ar->chip_id,
-			    ar->hw->wiphy->fw_version,
-			    ar->fw_api,
-			    ar->htt.target_version_major,
-			    ar->htt.target_version_minor);
-		ath10k_info("debug %d debugfs %d tracing %d dfs %d\n",
-			    config_enabled(CONFIG_ATH10K_DEBUG),
-			    config_enabled(CONFIG_ATH10K_DEBUGFS),
-			    config_enabled(CONFIG_ATH10K_TRACING),
-			    config_enabled(CONFIG_ATH10K_DFS_CERTIFIED));
-	}
-
-	__set_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags);
-
 	return 0;
 
 err_hif_stop:
@@ -938,6 +920,20 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
 		return ret;
 	}
 
+	ath10k_info("%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d\n",
+		    ar->hw_params.name,
+		    ar->target_version,
+		    ar->chip_id,
+		    ar->hw->wiphy->fw_version,
+		    ar->fw_api,
+		    ar->htt.target_version_major,
+		    ar->htt.target_version_minor);
+	ath10k_info("debug %d debugfs %d tracing %d dfs %d\n",
+		    config_enabled(CONFIG_ATH10K_DEBUG),
+		    config_enabled(CONFIG_ATH10K_DEBUGFS),
+		    config_enabled(CONFIG_ATH10K_TRACING),
+		    config_enabled(CONFIG_ATH10K_DFS_CERTIFIED));
+
 	ath10k_core_stop(ar);
 
 	mutex_unlock(&ar->conf_mutex);
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 6/6] ath10k: move fw init print
@ 2014-08-07  9:03   ` Michal Kazior
  0 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-07  9:03 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Firmware probing is done only once when driver is
registered and firmware version is guaranteed to
remain the same until driver is unregistered.

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

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 8eae2aa..b5161c8 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -816,24 +816,6 @@ int ath10k_core_start(struct ath10k *ar)
 
 	INIT_LIST_HEAD(&ar->arvifs);
 
-	if (!test_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags)) {
-		ath10k_info("%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d\n",
-			    ar->hw_params.name,
-			    ar->target_version,
-			    ar->chip_id,
-			    ar->hw->wiphy->fw_version,
-			    ar->fw_api,
-			    ar->htt.target_version_major,
-			    ar->htt.target_version_minor);
-		ath10k_info("debug %d debugfs %d tracing %d dfs %d\n",
-			    config_enabled(CONFIG_ATH10K_DEBUG),
-			    config_enabled(CONFIG_ATH10K_DEBUGFS),
-			    config_enabled(CONFIG_ATH10K_TRACING),
-			    config_enabled(CONFIG_ATH10K_DFS_CERTIFIED));
-	}
-
-	__set_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags);
-
 	return 0;
 
 err_hif_stop:
@@ -938,6 +920,20 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
 		return ret;
 	}
 
+	ath10k_info("%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d\n",
+		    ar->hw_params.name,
+		    ar->target_version,
+		    ar->chip_id,
+		    ar->hw->wiphy->fw_version,
+		    ar->fw_api,
+		    ar->htt.target_version_major,
+		    ar->htt.target_version_minor);
+	ath10k_info("debug %d debugfs %d tracing %d dfs %d\n",
+		    config_enabled(CONFIG_ATH10K_DEBUG),
+		    config_enabled(CONFIG_ATH10K_DEBUGFS),
+		    config_enabled(CONFIG_ATH10K_TRACING),
+		    config_enabled(CONFIG_ATH10K_DFS_CERTIFIED));
+
 	ath10k_core_stop(ar);
 
 	mutex_unlock(&ar->conf_mutex);
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/6] ath10k: remove target soc ps code
  2014-08-07  9:03   ` Michal Kazior
@ 2014-08-08 10:45     ` Kalle Valo
  -1 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-08 10:45 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k, linux-wireless

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

> The soc powersave was disabled by default. It
> never was fully tested. Some hw apparently had
> problems with it and the implementation itself had
> a possible race.
>
> Just remove the refcounting and simply wake up the
> device when probing and put to sleep when
> removing.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

This one introduced new warnings:

drivers/net/wireless/ath/ath10k/pci.c:646:5: warning: symbol 'ath10k_pci_wake' was not declared. Should it be static?
drivers/net/wireless/ath/ath10k/pci.c:653:6: warning: symbol 'ath10k_pci_sleep' was not declared. Should it be static?

Is it okay if I fix it like this:

--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -643,14 +643,14 @@ static int ath10k_pci_wake_wait(struct ath10k *ar)
        return -ETIMEDOUT;
 }
 
-int ath10k_pci_wake(struct ath10k *ar)
+static int ath10k_pci_wake(struct ath10k *ar)
 {
        ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS,
                               PCIE_SOC_WAKE_V_MASK);
        return ath10k_pci_wake_wait(ar);
 }
 
-void ath10k_pci_sleep(struct ath10k *ar)
+static void ath10k_pci_sleep(struct ath10k *ar)
 {
        ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS,
                               PCIE_SOC_WAKE_RESET);


-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/6] ath10k: remove target soc ps code
@ 2014-08-08 10:45     ` Kalle Valo
  0 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-08 10:45 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

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

> The soc powersave was disabled by default. It
> never was fully tested. Some hw apparently had
> problems with it and the implementation itself had
> a possible race.
>
> Just remove the refcounting and simply wake up the
> device when probing and put to sleep when
> removing.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

This one introduced new warnings:

drivers/net/wireless/ath/ath10k/pci.c:646:5: warning: symbol 'ath10k_pci_wake' was not declared. Should it be static?
drivers/net/wireless/ath/ath10k/pci.c:653:6: warning: symbol 'ath10k_pci_sleep' was not declared. Should it be static?

Is it okay if I fix it like this:

--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -643,14 +643,14 @@ static int ath10k_pci_wake_wait(struct ath10k *ar)
        return -ETIMEDOUT;
 }
 
-int ath10k_pci_wake(struct ath10k *ar)
+static int ath10k_pci_wake(struct ath10k *ar)
 {
        ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS,
                               PCIE_SOC_WAKE_V_MASK);
        return ath10k_pci_wake_wait(ar);
 }
 
-void ath10k_pci_sleep(struct ath10k *ar)
+static void ath10k_pci_sleep(struct ath10k *ar)
 {
        ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS,
                               PCIE_SOC_WAKE_RESET);


-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/6] ath10k: remove target soc ps code
  2014-08-08 10:45     ` Kalle Valo
@ 2014-08-08 11:00       ` Michal Kazior
  -1 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-08 11:00 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath10k, linux-wireless

On 8 August 2014 12:45, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> The soc powersave was disabled by default. It
>> never was fully tested. Some hw apparently had
>> problems with it and the implementation itself had
>> a possible race.
>>
>> Just remove the refcounting and simply wake up the
>> device when probing and put to sleep when
>> removing.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
> This one introduced new warnings:
>
> drivers/net/wireless/ath/ath10k/pci.c:646:5: warning: symbol 'ath10k_pci_wake' was not declared. Should it be static?
> drivers/net/wireless/ath/ath10k/pci.c:653:6: warning: symbol 'ath10k_pci_sleep' was not declared. Should it be static?

I didn't catch this for some reason. Can you share how you run
checkpatch, please?


> Is it okay if I fix it like this:
>
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c
> @@ -643,14 +643,14 @@ static int ath10k_pci_wake_wait(struct ath10k *ar)
>         return -ETIMEDOUT;
>  }
>
> -int ath10k_pci_wake(struct ath10k *ar)
> +static int ath10k_pci_wake(struct ath10k *ar)
>  {
>         ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS,
>                                PCIE_SOC_WAKE_V_MASK);
>         return ath10k_pci_wake_wait(ar);
>  }
>
> -void ath10k_pci_sleep(struct ath10k *ar)
> +static void ath10k_pci_sleep(struct ath10k *ar)
>  {
>         ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS,
>                                PCIE_SOC_WAKE_RESET);

Looks good, thanks.


Michał

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/6] ath10k: remove target soc ps code
@ 2014-08-08 11:00       ` Michal Kazior
  0 siblings, 0 replies; 32+ messages in thread
From: Michal Kazior @ 2014-08-08 11:00 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, ath10k

On 8 August 2014 12:45, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> The soc powersave was disabled by default. It
>> never was fully tested. Some hw apparently had
>> problems with it and the implementation itself had
>> a possible race.
>>
>> Just remove the refcounting and simply wake up the
>> device when probing and put to sleep when
>> removing.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
> This one introduced new warnings:
>
> drivers/net/wireless/ath/ath10k/pci.c:646:5: warning: symbol 'ath10k_pci_wake' was not declared. Should it be static?
> drivers/net/wireless/ath/ath10k/pci.c:653:6: warning: symbol 'ath10k_pci_sleep' was not declared. Should it be static?

I didn't catch this for some reason. Can you share how you run
checkpatch, please?


> Is it okay if I fix it like this:
>
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c
> @@ -643,14 +643,14 @@ static int ath10k_pci_wake_wait(struct ath10k *ar)
>         return -ETIMEDOUT;
>  }
>
> -int ath10k_pci_wake(struct ath10k *ar)
> +static int ath10k_pci_wake(struct ath10k *ar)
>  {
>         ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS,
>                                PCIE_SOC_WAKE_V_MASK);
>         return ath10k_pci_wake_wait(ar);
>  }
>
> -void ath10k_pci_sleep(struct ath10k *ar)
> +static void ath10k_pci_sleep(struct ath10k *ar)
>  {
>         ath10k_pci_reg_write32(ar, PCIE_SOC_WAKE_ADDRESS,
>                                PCIE_SOC_WAKE_RESET);

Looks good, thanks.


Michał

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/6] ath10k: remove pci features var
  2014-08-07  9:03   ` Michal Kazior
@ 2014-08-08 11:01     ` Kalle Valo
  -1 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-08 11:01 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k, linux-wireless

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

> The ATH10K_PCI_FEATURE_MSI_X was originally
> introduced to support both chips QCA988Xv1 and
> QCA988Xv2. Since v1 isn't supported anymore it
> doesn't make sense to keep the feature flag
> around. Since this is the last one remove the
> whole thing.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

[...]

> --- a/drivers/net/wireless/ath/ath10k/pci.h
> +++ b/drivers/net/wireless/ath/ath10k/pci.h
> @@ -135,13 +135,6 @@ struct service_to_pipe {
>  	u32 pipenum;
>  };
>  
> -enum ath10k_pci_features {
> -	ATH10K_PCI_FEATURE_MSI_X		= 0,
> -
> -	/* keep last */
> -	ATH10K_PCI_FEATURE_COUNT
> -};

I wouldn't be too surprised if we need this again at some point. But we
can then add it back.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/6] ath10k: remove pci features var
@ 2014-08-08 11:01     ` Kalle Valo
  0 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-08 11:01 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

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

> The ATH10K_PCI_FEATURE_MSI_X was originally
> introduced to support both chips QCA988Xv1 and
> QCA988Xv2. Since v1 isn't supported anymore it
> doesn't make sense to keep the feature flag
> around. Since this is the last one remove the
> whole thing.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

[...]

> --- a/drivers/net/wireless/ath/ath10k/pci.h
> +++ b/drivers/net/wireless/ath/ath10k/pci.h
> @@ -135,13 +135,6 @@ struct service_to_pipe {
>  	u32 pipenum;
>  };
>  
> -enum ath10k_pci_features {
> -	ATH10K_PCI_FEATURE_MSI_X		= 0,
> -
> -	/* keep last */
> -	ATH10K_PCI_FEATURE_COUNT
> -};

I wouldn't be too surprised if we need this again at some point. But we
can then add it back.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/6] ath10k: remove target soc ps code
  2014-08-08 11:00       ` Michal Kazior
@ 2014-08-08 11:04         ` Kalle Valo
  -1 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-08 11:04 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k, linux-wireless

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

> On 8 August 2014 12:45, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>
>>> The soc powersave was disabled by default. It
>>> never was fully tested. Some hw apparently had
>>> problems with it and the implementation itself had
>>> a possible race.
>>>
>>> Just remove the refcounting and simply wake up the
>>> device when probing and put to sleep when
>>> removing.
>>>
>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>
>> This one introduced new warnings:
>>
>> drivers/net/wireless/ath/ath10k/pci.c:646:5: warning: symbol 'ath10k_pci_wake' was not declared. Should it be static?
>> drivers/net/wireless/ath/ath10k/pci.c:653:6: warning: symbol 'ath10k_pci_sleep' was not declared. Should it be static?
>
> I didn't catch this for some reason. Can you share how you run
> checkpatch, please?

I think these are coming from sparse, in essence I run this:

make drivers/net/wireless/ath/ath10k/ C=2 CF="-D__CHECK_ENDIAN__"

Which reminds me that I should add -D__CHECK_ENDIAN__ to the makefile.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/6] ath10k: remove target soc ps code
@ 2014-08-08 11:04         ` Kalle Valo
  0 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-08 11:04 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

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

> On 8 August 2014 12:45, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>
>>> The soc powersave was disabled by default. It
>>> never was fully tested. Some hw apparently had
>>> problems with it and the implementation itself had
>>> a possible race.
>>>
>>> Just remove the refcounting and simply wake up the
>>> device when probing and put to sleep when
>>> removing.
>>>
>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>
>> This one introduced new warnings:
>>
>> drivers/net/wireless/ath/ath10k/pci.c:646:5: warning: symbol 'ath10k_pci_wake' was not declared. Should it be static?
>> drivers/net/wireless/ath/ath10k/pci.c:653:6: warning: symbol 'ath10k_pci_sleep' was not declared. Should it be static?
>
> I didn't catch this for some reason. Can you share how you run
> checkpatch, please?

I think these are coming from sparse, in essence I run this:

make drivers/net/wireless/ath/ath10k/ C=2 CF="-D__CHECK_ENDIAN__"

Which reminds me that I should add -D__CHECK_ENDIAN__ to the makefile.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/6] ath10k: remove target soc ps code
  2014-08-07  9:03   ` Michal Kazior
@ 2014-08-09 18:21     ` Kalle Valo
  -1 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-09 18:21 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k, linux-wireless

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

> The soc powersave was disabled by default. It
> never was fully tested. Some hw apparently had
> problems with it and the implementation itself had
> a possible race.
>
> Just remove the refcounting and simply wake up the
> device when probing and put to sleep when
> removing.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

[...]

> @@ -1838,12 +1799,6 @@ static int ath10k_pci_warm_reset(struct ath10k *ar)
>  
>  	ath10k_dbg(ATH10K_DBG_BOOT, "boot warm reset\n");
>  
> -	ret = ath10k_do_pci_wake(ar);
> -	if (ret) {
> -		ath10k_err("failed to wake up target: %d\n", ret);
> -		return ret;
> -	}
> -
>  	/* debug */
>  	val = ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
>  				PCIE_INTR_CAUSE_ADDRESS);
> @@ -1915,7 +1870,6 @@ static int ath10k_pci_warm_reset(struct ath10k *ar)
>  
>  	ath10k_dbg(ATH10K_DBG_BOOT, "boot warm reset complete\n");
>  
> -	ath10k_do_pci_sleep(ar);
>  	return ret;
>  }

For some reason I don't get all kbuild emails, and I don't know if you
got it either, but smatch found a style error here:

>> drivers/net/wireless/ath/ath10k/pci.c:1797:5-8: Unneeded variable: "ret". Return "0" on line 1873

I made this change:

--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1794,7 +1794,6 @@ static void ath10k_pci_warm_reset_si0(struct ath10k *ar)
 
 static int ath10k_pci_warm_reset(struct ath10k *ar)
 {
-       int ret = 0;
        u32 val;
 
        ath10k_dbg(ATH10K_DBG_BOOT, "boot warm reset\n");
@@ -1870,7 +1869,7 @@ static int ath10k_pci_warm_reset(struct ath10k *ar)
 
        ath10k_dbg(ATH10K_DBG_BOOT, "boot warm reset complete\n");
 
-       return ret;
+       return 0;
 }
 
 static int __ath10k_pci_hif_power_up(struct ath10k *ar, bool cold_reset)

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/6] ath10k: remove target soc ps code
@ 2014-08-09 18:21     ` Kalle Valo
  0 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-09 18:21 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

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

> The soc powersave was disabled by default. It
> never was fully tested. Some hw apparently had
> problems with it and the implementation itself had
> a possible race.
>
> Just remove the refcounting and simply wake up the
> device when probing and put to sleep when
> removing.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

[...]

> @@ -1838,12 +1799,6 @@ static int ath10k_pci_warm_reset(struct ath10k *ar)
>  
>  	ath10k_dbg(ATH10K_DBG_BOOT, "boot warm reset\n");
>  
> -	ret = ath10k_do_pci_wake(ar);
> -	if (ret) {
> -		ath10k_err("failed to wake up target: %d\n", ret);
> -		return ret;
> -	}
> -
>  	/* debug */
>  	val = ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
>  				PCIE_INTR_CAUSE_ADDRESS);
> @@ -1915,7 +1870,6 @@ static int ath10k_pci_warm_reset(struct ath10k *ar)
>  
>  	ath10k_dbg(ATH10K_DBG_BOOT, "boot warm reset complete\n");
>  
> -	ath10k_do_pci_sleep(ar);
>  	return ret;
>  }

For some reason I don't get all kbuild emails, and I don't know if you
got it either, but smatch found a style error here:

>> drivers/net/wireless/ath/ath10k/pci.c:1797:5-8: Unneeded variable: "ret". Return "0" on line 1873

I made this change:

--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1794,7 +1794,6 @@ static void ath10k_pci_warm_reset_si0(struct ath10k *ar)
 
 static int ath10k_pci_warm_reset(struct ath10k *ar)
 {
-       int ret = 0;
        u32 val;
 
        ath10k_dbg(ATH10K_DBG_BOOT, "boot warm reset\n");
@@ -1870,7 +1869,7 @@ static int ath10k_pci_warm_reset(struct ath10k *ar)
 
        ath10k_dbg(ATH10K_DBG_BOOT, "boot warm reset complete\n");
 
-       return ret;
+       return 0;
 }
 
 static int __ath10k_pci_hif_power_up(struct ath10k *ar, bool cold_reset)

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/6] ath10k: remove target soc ps code
  2014-08-09 18:21     ` Kalle Valo
@ 2014-08-09 18:24       ` Kalle Valo
  -1 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-09 18:24 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k, linux-wireless

Kalle Valo <kvalo@qca.qualcomm.com> writes:

>> @@ -1915,7 +1870,6 @@ static int ath10k_pci_warm_reset(struct ath10k *ar)
>>  
>>  	ath10k_dbg(ATH10K_DBG_BOOT, "boot warm reset complete\n");
>>  
>> -	ath10k_do_pci_sleep(ar);
>>  	return ret;
>>  }
>
> For some reason I don't get all kbuild emails, and I don't know if you
> got it either, but smatch found a style error here:
>
>>> drivers/net/wireless/ath/ath10k/pci.c:1797:5-8: Unneeded variable: "ret". Return "0" on line 1873

Forgot the link to the mail:

https://lists.01.org/pipermail/kbuild/2014-August/001684.html

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/6] ath10k: remove target soc ps code
@ 2014-08-09 18:24       ` Kalle Valo
  0 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-09 18:24 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

Kalle Valo <kvalo@qca.qualcomm.com> writes:

>> @@ -1915,7 +1870,6 @@ static int ath10k_pci_warm_reset(struct ath10k *ar)
>>  
>>  	ath10k_dbg(ATH10K_DBG_BOOT, "boot warm reset complete\n");
>>  
>> -	ath10k_do_pci_sleep(ar);
>>  	return ret;
>>  }
>
> For some reason I don't get all kbuild emails, and I don't know if you
> got it either, but smatch found a style error here:
>
>>> drivers/net/wireless/ath/ath10k/pci.c:1797:5-8: Unneeded variable: "ret". Return "0" on line 1873

Forgot the link to the mail:

https://lists.01.org/pipermail/kbuild/2014-August/001684.html

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] ath10k: move fw init print
  2014-08-07  9:03   ` Michal Kazior
@ 2014-08-12  7:53     ` Kalle Valo
  -1 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-12  7:53 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k, linux-wireless

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

> Firmware probing is done only once when driver is
> registered and firmware version is guaranteed to
> remain the same until driver is unregistered.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

[...]

> @@ -816,24 +816,6 @@ int ath10k_core_start(struct ath10k *ar)
>  
>  	INIT_LIST_HEAD(&ar->arvifs);
>  
> -	if (!test_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags)) {
> -		ath10k_info("%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d\n",
> -			    ar->hw_params.name,
> -			    ar->target_version,
> -			    ar->chip_id,
> -			    ar->hw->wiphy->fw_version,
> -			    ar->fw_api,
> -			    ar->htt.target_version_major,
> -			    ar->htt.target_version_minor);
> -		ath10k_info("debug %d debugfs %d tracing %d dfs %d\n",
> -			    config_enabled(CONFIG_ATH10K_DEBUG),
> -			    config_enabled(CONFIG_ATH10K_DEBUGFS),
> -			    config_enabled(CONFIG_ATH10K_TRACING),
> -			    config_enabled(CONFIG_ATH10K_DFS_CERTIFIED));
> -	}
> -
> -	__set_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags);

Doesn't this break this print in pci.c:

        if (!test_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags))
           ath10k_info("pci irq %s irq_mode %d reset_mode %d\n",
                                    irq_mode, ath10k_pci_irq_mode,
                                                    ath10k_pci_reset_mode);

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] ath10k: move fw init print
@ 2014-08-12  7:53     ` Kalle Valo
  0 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-12  7:53 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

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

> Firmware probing is done only once when driver is
> registered and firmware version is guaranteed to
> remain the same until driver is unregistered.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

[...]

> @@ -816,24 +816,6 @@ int ath10k_core_start(struct ath10k *ar)
>  
>  	INIT_LIST_HEAD(&ar->arvifs);
>  
> -	if (!test_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags)) {
> -		ath10k_info("%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d\n",
> -			    ar->hw_params.name,
> -			    ar->target_version,
> -			    ar->chip_id,
> -			    ar->hw->wiphy->fw_version,
> -			    ar->fw_api,
> -			    ar->htt.target_version_major,
> -			    ar->htt.target_version_minor);
> -		ath10k_info("debug %d debugfs %d tracing %d dfs %d\n",
> -			    config_enabled(CONFIG_ATH10K_DEBUG),
> -			    config_enabled(CONFIG_ATH10K_DEBUGFS),
> -			    config_enabled(CONFIG_ATH10K_TRACING),
> -			    config_enabled(CONFIG_ATH10K_DFS_CERTIFIED));
> -	}
> -
> -	__set_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags);

Doesn't this break this print in pci.c:

        if (!test_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags))
           ath10k_info("pci irq %s irq_mode %d reset_mode %d\n",
                                    irq_mode, ath10k_pci_irq_mode,
                                                    ath10k_pci_reset_mode);

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/6] ath10k: fixes 2014-08-07, part 1
  2014-08-07  9:03 ` Michal Kazior
@ 2014-08-12  7:56   ` Kalle Valo
  -1 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-12  7:56 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k, linux-wireless

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

> This is part 1 (of 3) of my patches. Split for
> your reviewing pleasure.
>
> This one is mostly non-functional (just cleanups)
> except the removal of soc powersave.
>
>
> Michal Kazior (6):
>   ath10k: embed ar_pci inside ar
>   ath10k: remove target soc ps code
>   ath10k: remove pci features var
>   ath10k: group some pci probing helpers
>   ath10k: remove htc->stopped

Thanks, these five patches applied.

>   ath10k: move fw init print

I dropped this one as I think it's buggy.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/6] ath10k: fixes 2014-08-07, part 1
@ 2014-08-12  7:56   ` Kalle Valo
  0 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-12  7:56 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

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

> This is part 1 (of 3) of my patches. Split for
> your reviewing pleasure.
>
> This one is mostly non-functional (just cleanups)
> except the removal of soc powersave.
>
>
> Michal Kazior (6):
>   ath10k: embed ar_pci inside ar
>   ath10k: remove target soc ps code
>   ath10k: remove pci features var
>   ath10k: group some pci probing helpers
>   ath10k: remove htc->stopped

Thanks, these five patches applied.

>   ath10k: move fw init print

I dropped this one as I think it's buggy.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] ath10k: move fw init print
  2014-08-12  7:53     ` Kalle Valo
@ 2014-08-14  8:27       ` Kalle Valo
  -1 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-14  8:27 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

Kalle Valo <kvalo@qca.qualcomm.com> writes:

> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> Firmware probing is done only once when driver is
>> registered and firmware version is guaranteed to
>> remain the same until driver is unregistered.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
> [...]
>
>> -	__set_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags);
>
> Doesn't this break this print in pci.c:
>
>         if (!test_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags))
>            ath10k_info("pci irq %s irq_mode %d reset_mode %d\n",
>                                     irq_mode, ath10k_pci_irq_mode,
>                                                     ath10k_pci_reset_mode);

Ah, now I understand. You rework that part in patch "ath10k: setup irq
method in probe" which was just in a different patchset. Then this extra
print during interface up is ok, as it just happens between the two
patches.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/6] ath10k: move fw init print
@ 2014-08-14  8:27       ` Kalle Valo
  0 siblings, 0 replies; 32+ messages in thread
From: Kalle Valo @ 2014-08-14  8:27 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

Kalle Valo <kvalo@qca.qualcomm.com> writes:

> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> Firmware probing is done only once when driver is
>> registered and firmware version is guaranteed to
>> remain the same until driver is unregistered.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
> [...]
>
>> -	__set_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags);
>
> Doesn't this break this print in pci.c:
>
>         if (!test_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags))
>            ath10k_info("pci irq %s irq_mode %d reset_mode %d\n",
>                                     irq_mode, ath10k_pci_irq_mode,
>                                                     ath10k_pci_reset_mode);

Ah, now I understand. You rework that part in patch "ath10k: setup irq
method in probe" which was just in a different patchset. Then this extra
print during interface up is ok, as it just happens between the two
patches.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2014-08-14  8:28 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-07  9:03 [PATCH 0/6] ath10k: fixes 2014-08-07, part 1 Michal Kazior
2014-08-07  9:03 ` Michal Kazior
2014-08-07  9:03 ` [PATCH 1/6] ath10k: embed ar_pci inside ar Michal Kazior
2014-08-07  9:03   ` Michal Kazior
2014-08-07  9:03 ` [PATCH 2/6] ath10k: remove target soc ps code Michal Kazior
2014-08-07  9:03   ` Michal Kazior
2014-08-08 10:45   ` Kalle Valo
2014-08-08 10:45     ` Kalle Valo
2014-08-08 11:00     ` Michal Kazior
2014-08-08 11:00       ` Michal Kazior
2014-08-08 11:04       ` Kalle Valo
2014-08-08 11:04         ` Kalle Valo
2014-08-09 18:21   ` Kalle Valo
2014-08-09 18:21     ` Kalle Valo
2014-08-09 18:24     ` Kalle Valo
2014-08-09 18:24       ` Kalle Valo
2014-08-07  9:03 ` [PATCH 3/6] ath10k: remove pci features var Michal Kazior
2014-08-07  9:03   ` Michal Kazior
2014-08-08 11:01   ` Kalle Valo
2014-08-08 11:01     ` Kalle Valo
2014-08-07  9:03 ` [PATCH 4/6] ath10k: group some pci probing helpers Michal Kazior
2014-08-07  9:03   ` Michal Kazior
2014-08-07  9:03 ` [PATCH 5/6] ath10k: remove htc->stopped Michal Kazior
2014-08-07  9:03   ` Michal Kazior
2014-08-07  9:03 ` [PATCH 6/6] ath10k: move fw init print Michal Kazior
2014-08-07  9:03   ` Michal Kazior
2014-08-12  7:53   ` Kalle Valo
2014-08-12  7:53     ` Kalle Valo
2014-08-14  8:27     ` Kalle Valo
2014-08-14  8:27       ` Kalle Valo
2014-08-12  7:56 ` [PATCH 0/6] ath10k: fixes 2014-08-07, part 1 Kalle Valo
2014-08-12  7:56   ` Kalle Valo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.