[ath9k-devel] [PATCH 00/10] ath10k: device setup refactor

[ath9k-devel] [PATCH 00/10] ath10k: device setup refactor

[Michal Kazior]

This is part 3 of 4 of a bigger patchset.
Split for easier review.

This fixes suspend to disk and changes the
lifetime of device configuration.

Until now FW was loaded once on device probing.

Now FW is loaded once on probing (to get hw
capabilities) and unloaded immediately. Then it is
loaded/unloaded again via mac80211's start/stop
hooks. Since FW is effectively loaded twice
initial startup time is longer.

ieee80211_restart_hw() seems to work, but is not
tested thoroughly yet (I only did some ad-hoc
checks). FW/HW recovery is not implemented yet.

Known issues:
  * hw reconfiguration doesn't work if resume
    fails (the device doesn't respond despite
    going through power-cycle/setup in the code;
    this is not a regression as it wasn't handled
    before at all)
  * wowlan is not supported, yet

Note: it is not based on master branch.

Michal Kazior (10):
  ath10k: decouple pci init/deinit logic
  ath10k: decouple core init/deinit logic
  ath10k: allow deferred regd update
  ath10k: reset BMI state upon init
  ath10k: decouple suspend code
  ath10k: defer hw setup to start/stop mac80211 hooks
  ath10k: store firmware files in memory
  ath10k: make sure all resources are freed upon ath10k_stop()
  ath10k: handle hw reconfiguration properly
  ath10k: move free_vdev_map initialization

 drivers/net/wireless/ath/ath10k/core.c |  250 ++++++++++++++++++----------
 drivers/net/wireless/ath/ath10k/core.h |   11 +-
 drivers/net/wireless/ath/ath10k/hif.h  |   39 +++++
 drivers/net/wireless/ath/ath10k/mac.c  |  130 ++++++++++++++-
 drivers/net/wireless/ath/ath10k/pci.c  |  284 +++++++++++++-------------------
 5 files changed, 452 insertions(+), 262 deletions(-)

-- 
1.7.9.5

[ath9k-devel] [PATCH 01/10] ath10k: decouple pci init/deinit logic

[Michal Kazior]

Split logic that prepares the device for BMI
phase/cleans up related resources.

This is necessary for ath10k to be able to restart
hw on the fly without reloading the module.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/hif.h |   20 ++++++
 drivers/net/wireless/ath/ath10k/pci.c |  110 +++++++++++++++++++++------------
 2 files changed, 91 insertions(+), 39 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/hif.h b/drivers/net/wireless/ath/ath10k/hif.h
index 010e265..3a257d1 100644
--- a/drivers/net/wireless/ath/ath10k/hif.h
+++ b/drivers/net/wireless/ath/ath10k/hif.h
@@ -46,8 +46,11 @@ struct ath10k_hif_ops {
 				void *request, u32 request_len,
 				void *response, u32 *response_len);
 
+	/* Post BMI phase, after FW is loaded. Starts regular operation */
 	int (*start)(struct ath10k *ar);
 
+	/* Clean up what start() did. This does not revert to BMI phase. If
+	 * desired so, call deinit() and init() */
 	void (*stop)(struct ath10k *ar);
 
 	int (*map_service_to_pipe)(struct ath10k *ar, u16 service_id,
@@ -70,6 +73,13 @@ struct ath10k_hif_ops {
 			      struct ath10k_hif_cb *callbacks);
 
 	u16 (*get_free_queue_number)(struct ath10k *ar, u8 pipe_id);
+
+	/* Power up the device and enter BMI transfer mode for FW download */
+	int (*init)(struct ath10k *ar);
+
+	/* Power down the device and free up resources. stop() must be called
+	 * before this if start() was called earlier */
+	void (*deinit)(struct ath10k *ar);
 };
 
 
@@ -134,4 +144,14 @@ static inline u16 ath10k_hif_get_free_queue_number(struct ath10k *ar,
 	return ar->hif.ops->get_free_queue_number(ar, pipe_id);
 }
 
+static inline int ath10k_hif_init(struct ath10k *ar)
+{
+	return ar->hif.ops->init(ar);
+}
+
+static inline void ath10k_hif_deinit(struct ath10k *ar)
+{
+	ar->hif.ops->deinit(ar);
+}
+
 #endif /* _HIF_H_ */
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 1a59638..bcd7184 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -54,6 +54,8 @@ static int ath10k_pci_post_rx_pipe(struct hif_ce_pipe_info *pipe_info,
 					     int num);
 static void ath10k_pci_rx_pipe_cleanup(struct hif_ce_pipe_info *pipe_info);
 static void ath10k_pci_stop_ce(struct ath10k *ar);
+static void ath10k_pci_device_reset(struct ath10k *ar);
+static int ath10k_pci_reset_target(struct ath10k *ar);
 
 static const struct ce_attr host_ce_config_wlan[] = {
 	/* host->target HTC control and raw streams */
@@ -1734,6 +1736,66 @@ static void ath10k_pci_fw_interrupt_handler(struct ath10k *ar)
 	ath10k_pci_sleep(ar);
 }
 
+static int ath10k_pci_hif_init(struct ath10k *ar)
+{
+	int ret;
+
+	/*
+	 * Bring the target up cleanly.
+	 *
+	 * The target may be in an undefined state with an AUX-powered Target
+	 * and a Host in WoW mode. If the Host crashes, loses power, or is
+	 * restarted (without unloading the driver) then the Target is left
+	 * (aux) powered and running. On a subsequent driver load, the Target
+	 * is in an unexpected state. We try to catch that here in order to
+	 * reset the Target and retry the probe.
+	 */
+	ath10k_pci_device_reset(ar);
+
+	ret = ath10k_pci_reset_target(ar);
+	if (ret)
+		goto err;
+
+	if (ath10k_target_ps) {
+		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save enabled\n");
+	} else {
+		/* Force AWAKE forever */
+		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save disabled\n");
+		ath10k_do_pci_wake(ar);
+	}
+
+	ret = ath10k_pci_ce_init(ar);
+	if (ret)
+		goto err_ps;
+
+	ret = ath10k_pci_init_config(ar);
+	if (ret)
+		goto err_ce;
+
+	ret = ath10k_pci_wake_target_cpu(ar);
+	if (ret) {
+		ath10k_err("could not wake up target CPU (%d)\n", ret);
+		goto err_ce;
+	}
+
+	return 0;
+
+err_ce:
+	ath10k_pci_ce_deinit(ar);
+err_ps:
+	if (!ath10k_target_ps)
+		ath10k_do_pci_sleep(ar);
+err:
+	return ret;
+}
+
+static void ath10k_pci_hif_deinit(struct ath10k *ar)
+{
+	ath10k_pci_ce_deinit(ar);
+	if (!ath10k_target_ps)
+		ath10k_do_pci_sleep(ar);
+}
+
 static const struct ath10k_hif_ops ath10k_pci_hif_ops = {
 	.send_head		= ath10k_pci_hif_send_head,
 	.exchange_bmi_msg	= ath10k_pci_hif_exchange_bmi_msg,
@@ -1744,6 +1806,8 @@ static const struct ath10k_hif_ops ath10k_pci_hif_ops = {
 	.send_complete_check	= ath10k_pci_hif_send_complete_check,
 	.set_callbacks		= ath10k_pci_hif_set_callbacks,
 	.get_free_queue_number	= ath10k_pci_hif_get_free_queue_number,
+	.init			= ath10k_pci_hif_init,
+	.deinit			= ath10k_pci_hif_deinit,
 };
 
 static void ath10k_pci_ce_tasklet(unsigned long ptr)
@@ -2244,54 +2308,22 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 		goto err_iomap;
 	}
 
-	/*
-	 * Bring the target up cleanly.
-	 *
-	 * The target may be in an undefined state with an AUX-powered Target
-	 * and a Host in WoW mode. If the Host crashes, loses power, or is
-	 * restarted (without unloading the driver) then the Target is left
-	 * (aux) powered and running. On a subsequent driver load, the Target
-	 * is in an unexpected state. We try to catch that here in order to
-	 * reset the Target and retry the probe.
-	 */
-	ath10k_pci_device_reset(ar);
-
-	ret = ath10k_pci_reset_target(ar);
-	if (ret)
-		goto err_intr;
-
-	if (ath10k_target_ps) {
-		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save enabled\n");
-	} else {
-		/* Force AWAKE forever */
-		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save disabled\n");
-		ath10k_do_pci_wake(ar);
-	}
-
-	ret = ath10k_pci_ce_init(ar);
-	if (ret)
-		goto err_intr;
-
-	ret = ath10k_pci_init_config(ar);
-	if (ret)
-		goto err_ce;
-
-	ret = ath10k_pci_wake_target_cpu(ar);
+	ret = ath10k_pci_hif_init(ar);
 	if (ret) {
-		ath10k_err("could not wake up target CPU (%d)\n", ret);
-		goto err_ce;
+		ath10k_err("could not start pci hif (%d)\n", ret);
+		goto err_intr;
 	}
 
 	ret = ath10k_core_register(ar);
 	if (ret) {
 		ath10k_err("could not register driver core (%d)\n", ret);
-		goto err_ce;
+		goto err_hif;
 	}
 
 	return 0;
 
-err_ce:
-	ath10k_pci_ce_deinit(ar);
+err_hif:
+	ath10k_pci_hif_deinit(ar);
 err_intr:
 	ath10k_pci_stop_intr(ar);
 err_iomap:
@@ -2330,7 +2362,7 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 	tasklet_kill(&ar_pci->msi_fw_err);
 
 	ath10k_core_unregister(ar);
-	ath10k_pci_ce_deinit(ar);
+	ath10k_pci_hif_deinit(ar);
 	ath10k_pci_stop_intr(ar);
 
 	pci_set_drvdata(pdev, NULL);
-- 
1.7.9.5

[ath9k-devel] [PATCH 02/10] ath10k: decouple core init/deinit logic

[Michal Kazior]

Enables code reuse for proper hw reconfiguration
that is in turn required for proper
suspend/hibernation/wowlan support.

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

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index aabe166..87d6536 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -502,8 +502,7 @@ void ath10k_core_destroy(struct ath10k *ar)
 }
 EXPORT_SYMBOL(ath10k_core_destroy);
 
-
-int ath10k_core_register(struct ath10k *ar)
+int ath10k_core_init(struct ath10k *ar)
 {
 	struct ath10k_htc_ops htc_ops;
 	struct bmi_target_info target_info;
@@ -590,9 +589,39 @@ int ath10k_core_register(struct ath10k *ar)
 	if (status)
 		goto err_disconnect_htc;
 
+	return 0;
+
+err_disconnect_htc:
+	ath10k_htc_stop(ar->htc);
+err_htt_detach:
+	ath10k_htt_detach(ar->htt);
+err_wmi_detach:
+	ath10k_wmi_detach(ar);
+err_htc_destroy:
+	ath10k_htc_destroy(ar->htc);
+err:
+	return status;
+}
+
+void ath10k_core_deinit(struct ath10k *ar)
+{
+	ath10k_htc_stop(ar->htc);
+	ath10k_htt_detach(ar->htt);
+	ath10k_wmi_detach(ar);
+	ath10k_htc_destroy(ar->htc);
+}
+
+int ath10k_core_register(struct ath10k *ar)
+{
+	int status;
+
+	status = ath10k_core_init(ar);
+	if (status)
+		goto err;
+
 	status = ath10k_mac_register(ar);
 	if (status)
-		goto err_disconnect_htc;
+		goto err_core_deinit;
 
 	status = ath10k_debug_create(ar);
 	if (status) {
@@ -604,14 +633,8 @@ int ath10k_core_register(struct ath10k *ar)
 
 err_unregister_mac:
 	ath10k_mac_unregister(ar);
-err_disconnect_htc:
-	ath10k_htc_stop(ar->htc);
-err_htt_detach:
-	ath10k_htt_detach(ar->htt);
-err_wmi_detach:
-	ath10k_wmi_detach(ar);
-err_htc_destroy:
-	ath10k_htc_destroy(ar->htc);
+err_core_deinit:
+	ath10k_core_deinit(ar);
 err:
 	return status;
 }
@@ -623,10 +646,7 @@ void ath10k_core_unregister(struct ath10k *ar)
 	 * Otherwise we will fail to submit commands to FW and mac80211 will be
 	 * unhappy about callback failures. */
 	ath10k_mac_unregister(ar);
-	ath10k_htc_stop(ar->htc);
-	ath10k_htt_detach(ar->htt);
-	ath10k_wmi_detach(ar);
-	ath10k_htc_destroy(ar->htc);
+	ath10k_core_deinit(ar);
 }
 EXPORT_SYMBOL(ath10k_core_unregister);
 
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 7489770..c718b80 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -354,6 +354,8 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
 				  const struct ath10k_hif_ops *hif_ops);
 void ath10k_core_destroy(struct ath10k *ar);
 
+int ath10k_core_init(struct ath10k *ar);
+void ath10k_core_deinit(struct ath10k *ar);
 int ath10k_core_register(struct ath10k *ar);
 void ath10k_core_unregister(struct ath10k *ar);
 
-- 
1.7.9.5

[ath9k-devel] [PATCH 02/10] ath10k: decouple core init/deinit logic

[Kalle Valo]

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

> Enables code reuse for proper hw reconfiguration
> that is in turn required for proper
> suspend/hibernation/wowlan support.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

Let's have a terminology check. I have understood we use terms with
meaning like this (maybe you have been thinking something else):

init(): initilises the module in question from host side, allocates
        memory etc. Called only once when driver is loaded.

deinit(): reverse action for init(), frees allocated memory etc. Called
          only once.

start(): Initialises hw/fw. Called every time we start the firmware.

stop(): Kills the running fw. Shuts down all power the device (when
        possible)

But here ath10k_core_init() seems to be initialising hw/fw. So should it
be renamed to ath10k_core_start()?

-- 
Kalle Valo

[ath9k-devel] [PATCH 02/10] ath10k: decouple core init/deinit logic

[Michal Kazior]

On 10/06/13 18:53, Kalle Valo wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> Enables code reuse for proper hw reconfiguration
>> that is in turn required for proper
>> suspend/hibernation/wowlan support.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
> Let's have a terminology check. I have understood we use terms with
> meaning like this (maybe you have been thinking something else):
>
> init(): initilises the module in question from host side, allocates
>          memory etc. Called only once when driver is loaded.
>
> deinit(): reverse action for init(), frees allocated memory etc. Called
>            only once.
>
> start(): Initialises hw/fw. Called every time we start the firmware.
>
> stop(): Kills the running fw. Shuts down all power the device (when
>          possible)
>
> But here ath10k_core_init() seems to be initialising hw/fw. So should it
> be renamed to ath10k_core_start()?

You're right. I'll fix this and resend.


-- Pozdrawiam / Best regards, Michal Kazior.

[ath9k-devel] [PATCH 03/10] ath10k: allow deferred regd update

[Michal Kazior]

Regulatory domain notification hook can be called
regardless of the hw state (i.e. before start
mac80211 callback).

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

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index c718b80..a3ab3eb 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -345,6 +345,8 @@ struct ath10k {
 	struct completion offchan_tx_completed;
 	struct sk_buff *offchan_tx_skb;
 
+	bool started;
+
 #ifdef CONFIG_ATH10K_DEBUGFS
 	struct ath10k_debug debug;
 #endif
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index cdee800..c1729fa 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1273,21 +1273,20 @@ static int ath10k_update_channel_list(struct ath10k *ar)
 	return ret;
 }
 
-static void ath10k_reg_notifier(struct wiphy *wiphy,
-				struct regulatory_request *request)
+static void ath10k_regd_update(struct ath10k *ar)
 {
-	struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
 	struct reg_dmn_pair_mapping *regpair;
-	struct ath10k *ar = hw->priv;
 	int ret;
 
-	ath_reg_notifier_apply(wiphy, request, &ar->ath_common.regulatory);
+	if (!ar->started)
+		return;
 
 	ret = ath10k_update_channel_list(ar);
 	if (ret)
 		ath10k_warn("could not update channel list (%d)\n", ret);
 
 	regpair = ar->ath_common.regulatory.regpair;
+
 	/* Target allows setting up per-band regdomain but ath_common provides
 	 * a combined one only */
 	ret = ath10k_wmi_pdev_set_regdomain(ar,
@@ -1300,6 +1299,16 @@ static void ath10k_reg_notifier(struct wiphy *wiphy,
 		ath10k_warn("could not set pdev regdomain (%d)\n", ret);
 }
 
+static void ath10k_reg_notifier(struct wiphy *wiphy,
+				struct regulatory_request *request)
+{
+	struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
+	struct ath10k *ar = hw->priv;
+
+	ath_reg_notifier_apply(wiphy, request, &ar->ath_common.regulatory);
+	ath10k_regd_update(ar);
+}
+
 /***************/
 /* TX handlers */
 /***************/
@@ -1685,6 +1694,8 @@ static int ath10k_start(struct ieee80211_hw *hw)
 		ath10k_warn("could not init WMI_PDEV_PARAM_DYNAMIC_BW (%d)\n",
 			    ret);
 
+	ar->started = true;
+	ath10k_regd_update(ar);
 	return 0;
 }
 
@@ -1695,6 +1706,8 @@ static void ath10k_stop(struct ieee80211_hw *hw)
 	/* avoid leaks in case FW never confirms scan for offchannel */
 	cancel_work_sync(&ar->offchan_tx_work);
 	ath10k_offchan_tx_purge(ar);
+
+	ar->started = false;
 }
 
 static int ath10k_config(struct ieee80211_hw *hw, u32 changed)
-- 
1.7.9.5

[ath9k-devel] [PATCH 03/10] ath10k: allow deferred regd update

[Sujith Manoharan]

Michal Kazior wrote:
> Regulatory domain notification hook can be called
> regardless of the hw state (i.e. before start
> mac80211 callback).
> 
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>  drivers/net/wireless/ath/ath10k/core.h |    2 ++
>  drivers/net/wireless/ath/ath10k/mac.c  |   23 ++++++++++++++++++-----
>  2 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
> index c718b80..a3ab3eb 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -345,6 +345,8 @@ struct ath10k {
>  	struct completion offchan_tx_completed;
>  	struct sk_buff *offchan_tx_skb;
>  
> +	bool started;

I think we can start gathering such flags into a single place
and use test_bit/set_bit etc. to rule out race conditions.

Sujith

[ath9k-devel] [PATCH 03/10] ath10k: allow deferred regd update

[Kalle Valo]

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

> Regulatory domain notification hook can be called
> regardless of the hw state (i.e. before start
> mac80211 callback).
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>  drivers/net/wireless/ath/ath10k/core.h |    2 ++
>  drivers/net/wireless/ath/ath10k/mac.c  |   23 ++++++++++++++++++-----
>  2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
> index c718b80..a3ab3eb 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -345,6 +345,8 @@ struct ath10k {
>  	struct completion offchan_tx_completed;
>  	struct sk_buff *offchan_tx_skb;
>  
> +	bool started;

I would prefer to have a proper state variable right from the start.
Sooner or later we need it anyway. Otherwise we end up to "flag hell"
(checking multiple flags).

For ath6kl we currently have:

enum ath6kl_state {
	ATH6KL_STATE_OFF,
	ATH6KL_STATE_ON,
	ATH6KL_STATE_SUSPENDING,
	ATH6KL_STATE_RESUMING,
	ATH6KL_STATE_DEEPSLEEP,
	ATH6KL_STATE_CUTPOWER,
	ATH6KL_STATE_WOW,
	ATH6KL_STATE_RECOVERY,
};

I think ON and OFF states would be enough for ath10k currently. We can
extend them as we add more features.

-- 
Kalle Valo

[ath9k-devel] [PATCH 03/10] ath10k: allow deferred regd update

[Michal Kazior]

On 10/06/13 18:56, Kalle Valo wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> Regulatory domain notification hook can be called
>> regardless of the hw state (i.e. before start
>> mac80211 callback).
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>> ---
>>   drivers/net/wireless/ath/ath10k/core.h |    2 ++
>>   drivers/net/wireless/ath/ath10k/mac.c  |   23 ++++++++++++++++++-----
>>   2 files changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
>> index c718b80..a3ab3eb 100644
>> --- a/drivers/net/wireless/ath/ath10k/core.h
>> +++ b/drivers/net/wireless/ath/ath10k/core.h
>> @@ -345,6 +345,8 @@ struct ath10k {
>>   	struct completion offchan_tx_completed;
>>   	struct sk_buff *offchan_tx_skb;
>>
>> +	bool started;
>
> I would prefer to have a proper state variable right from the start.
> Sooner or later we need it anyway. Otherwise we end up to "flag hell"
> (checking multiple flags).
>
> For ath6kl we currently have:
>
> enum ath6kl_state {
> 	ATH6KL_STATE_OFF,
> 	ATH6KL_STATE_ON,
> 	ATH6KL_STATE_SUSPENDING,
> 	ATH6KL_STATE_RESUMING,
> 	ATH6KL_STATE_DEEPSLEEP,
> 	ATH6KL_STATE_CUTPOWER,
> 	ATH6KL_STATE_WOW,
> 	ATH6KL_STATE_RECOVERY,
> };
>
> I think ON and OFF states would be enough for ath10k currently. We can
> extend them as we add more features.

Sounds good. I'll fix this and resend later.


-- Pozdrawiam / Best regards, Michal Kazior.

[ath9k-devel] [PATCH 04/10] ath10k: reset BMI state upon init

[Michal Kazior]

This is necessary if we want to be able to restart
hw on-the-fly.

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

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 87d6536..911d41e 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -508,6 +508,8 @@ int ath10k_core_init(struct ath10k *ar)
 	struct bmi_target_info target_info;
 	int status;
 
+	ar->bmi.done_sent = false;
+
 	memset(&target_info, 0, sizeof(target_info));
 	status = ath10k_bmi_get_target_info(ar, &target_info);
 	if (status)
-- 
1.7.9.5

[ath9k-devel] [PATCH 04/10] ath10k: reset BMI state upon init

[Kalle Valo]

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

> This is necessary if we want to be able to restart
> hw on-the-fly.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>  drivers/net/wireless/ath/ath10k/core.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
> index 87d6536..911d41e 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -508,6 +508,8 @@ int ath10k_core_init(struct ath10k *ar)
>  	struct bmi_target_info target_info;
>  	int status;
>  
> +	ar->bmi.done_sent = false;

We should do this in ath10k_bmi_start() so that all BMI related code is
in bmi.c.

-- 
Kalle Valo

[ath9k-devel] [PATCH 05/10] ath10k: decouple suspend code

[Michal Kazior]

Split up fw-related and hw-related suspension code.

Although we don't advertise WoW support to
mac80211 yet it's useful to keep the code in
suspend/resume hooks.

At this point there's no need to keep pci pm ops.
In case of WoW mac80211 calls ath10k_suspend()
which should take care of entering low-power mode.
In case WoW is not available mac80211 will go
through regular interface teradown and use start/stop.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c |   28 -----
 drivers/net/wireless/ath/ath10k/core.h |    3 -
 drivers/net/wireless/ath/ath10k/hif.h  |   19 ++++
 drivers/net/wireless/ath/ath10k/mac.c  |   66 ++++++++++++
 drivers/net/wireless/ath/ath10k/pci.c  |  176 ++++++++++----------------------
 5 files changed, 138 insertions(+), 154 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 911d41e..6d378d5 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -652,34 +652,6 @@ void ath10k_core_unregister(struct ath10k *ar)
 }
 EXPORT_SYMBOL(ath10k_core_unregister);
 
-int ath10k_core_target_suspend(struct ath10k *ar)
-{
-	int ret;
-
-	ath10k_dbg(ATH10K_DBG_CORE, "%s: called", __func__);
-
-	ret = ath10k_wmi_pdev_suspend_target(ar);
-	if (ret)
-		ath10k_warn("could not suspend target (%d)\n", ret);
-
-	return ret;
-}
-EXPORT_SYMBOL(ath10k_core_target_suspend);
-
-int ath10k_core_target_resume(struct ath10k *ar)
-{
-	int ret;
-
-	ath10k_dbg(ATH10K_DBG_CORE, "%s: called", __func__);
-
-	ret = ath10k_wmi_pdev_resume_target(ar);
-	if (ret)
-		ath10k_warn("could not resume target (%d)\n", ret);
-
-	return ret;
-}
-EXPORT_SYMBOL(ath10k_core_target_resume);
-
 MODULE_AUTHOR("Qualcomm Atheros");
 MODULE_DESCRIPTION("Core module for QCA988X PCIe devices.");
 MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index a3ab3eb..083185c 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -361,7 +361,4 @@ void ath10k_core_deinit(struct ath10k *ar);
 int ath10k_core_register(struct ath10k *ar);
 void ath10k_core_unregister(struct ath10k *ar);
 
-int ath10k_core_target_suspend(struct ath10k *ar);
-int ath10k_core_target_resume(struct ath10k *ar);
-
 #endif /* _CORE_H_ */
diff --git a/drivers/net/wireless/ath/ath10k/hif.h b/drivers/net/wireless/ath/ath10k/hif.h
index 3a257d1..00bfb03 100644
--- a/drivers/net/wireless/ath/ath10k/hif.h
+++ b/drivers/net/wireless/ath/ath10k/hif.h
@@ -80,6 +80,9 @@ struct ath10k_hif_ops {
 	/* Power down the device and free up resources. stop() must be called
 	 * before this if start() was called earlier */
 	void (*deinit)(struct ath10k *ar);
+
+	int (*suspend)(struct ath10k *ar);
+	int (*resume)(struct ath10k *ar);
 };
 
 
@@ -154,4 +157,20 @@ static inline void ath10k_hif_deinit(struct ath10k *ar)
 	ar->hif.ops->deinit(ar);
 }
 
+static inline int ath10k_hif_suspend(struct ath10k *ar)
+{
+	if (!ar->hif.ops->suspend)
+		return -EOPNOTSUPP;
+
+	return ar->hif.ops->suspend(ar);
+}
+
+static inline int ath10k_hif_resume(struct ath10k *ar)
+{
+	if (!ar->hif.ops->resume)
+		return -EOPNOTSUPP;
+
+	return ar->hif.ops->resume(ar);
+}
+
 #endif /* _HIF_H_ */
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index c1729fa..0f339e6 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -20,6 +20,7 @@
 #include <net/mac80211.h>
 #include <linux/etherdevice.h>
 
+#include "hif.h"
 #include "core.h"
 #include "debug.h"
 #include "wmi.h"
@@ -2690,6 +2691,67 @@ static int ath10k_tx_last_beacon(struct ieee80211_hw *hw)
 	return 1;
 }
 
+#if CONFIG_PM
+static int ath10k_suspend(struct ieee80211_hw *hw,
+			  struct cfg80211_wowlan *wowlan)
+{
+	struct ath10k *ar = hw->priv;
+	int ret;
+
+	ar->is_target_paused = false;
+
+	ret = ath10k_wmi_pdev_suspend_target(ar);
+	if (ret) {
+		ath10k_warn("could not suspend target (%d)\n", ret);
+		return 1;
+	}
+
+	ret = wait_event_interruptible_timeout(ar->event_queue,
+					       ar->is_target_paused == true,
+					       1 * HZ);
+	if (ret < 0) {
+		ath10k_warn("suspend interrupted (%d)\n", ret);
+		goto resume;
+	} else if (ret == 0) {
+		ath10k_warn("suspend timed out - target pause event never came\n");
+		goto resume;
+	}
+
+	ret = ath10k_hif_suspend(ar);
+	if (ret) {
+		ath10k_warn("could not suspend hif (%d)\n", ret);
+		goto resume;
+	}
+
+	return 0;
+resume:
+	ret = ath10k_wmi_pdev_resume_target(ar);
+	if (ret)
+		ath10k_warn("could not resume target (%d)\n", ret);
+	return 1;
+}
+
+static int ath10k_resume(struct ieee80211_hw *hw)
+{
+	struct ath10k *ar = hw->priv;
+	int ret;
+
+	ret = ath10k_hif_resume(ar);
+	if (ret) {
+		ath10k_warn("could not resume hif (%d)\n", ret);
+		return 1;
+	}
+
+	ret = ath10k_wmi_pdev_resume_target(ar);
+	if (ret) {
+		ath10k_warn("could not resume target (%d)\n", ret);
+		return 1;
+	}
+
+	return 0;
+}
+#endif
+
 static const struct ieee80211_ops ath10k_ops = {
 	.tx				= ath10k_tx,
 	.start				= ath10k_start,
@@ -2710,6 +2772,10 @@ static const struct ieee80211_ops ath10k_ops = {
 	.set_frag_threshold		= ath10k_set_frag_threshold,
 	.flush				= ath10k_flush,
 	.tx_last_beacon			= ath10k_tx_last_beacon,
+#if CONFIG_PM
+	.suspend			= ath10k_suspend,
+	.resume				= ath10k_resume,
+#endif
 };
 
 #define RATETAB_ENT(_rate, _rateid, _flags) { \
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index bcd7184..63c5a71 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1796,6 +1796,55 @@ static void ath10k_pci_hif_deinit(struct ath10k *ar)
 		ath10k_do_pci_sleep(ar);
 }
 
+#if CONFIG_PM
+
+#define ATH10K_PCI_PM_CONTROL 0x44
+
+static int ath10k_pci_hif_suspend(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	struct pci_dev *pdev = ar_pci->pdev;
+	u32 val;
+
+	pci_read_config_dword(pdev, ATH10K_PCI_PM_CONTROL, &val);
+
+	if ((val & 0x000000ff) != 0x3) {
+		pci_save_state(pdev);
+		pci_disable_device(pdev);
+		pci_write_config_dword(pdev, ATH10K_PCI_PM_CONTROL,
+				       (val & 0xffffff00) | 0x03);
+	}
+
+	return 0;
+}
+
+static int ath10k_pci_hif_resume(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	struct pci_dev *pdev = ar_pci->pdev;
+	u32 val;
+
+	pci_read_config_dword(pdev, ATH10K_PCI_PM_CONTROL, &val);
+
+	if ((val & 0x000000ff) != 0) {
+		pci_restore_state(pdev);
+		pci_write_config_dword(pdev, ATH10K_PCI_PM_CONTROL,
+				       val & 0xffffff00);
+		/*
+		 * Suspend/Resume resets the PCI configuration space,
+		 * so we have to re-disable the RETRY_TIMEOUT register (0x41)
+		 * to keep PCI Tx retries from interfering with C3 CPU state
+		 */
+		pci_read_config_dword(pdev, 0x40, &val);
+
+		if ((val & 0x0000ff00) != 0)
+			pci_write_config_dword(pdev, 0x40, val & 0xffff00ff);
+	}
+
+	return 0;
+}
+#endif
+
 static const struct ath10k_hif_ops ath10k_pci_hif_ops = {
 	.send_head		= ath10k_pci_hif_send_head,
 	.exchange_bmi_msg	= ath10k_pci_hif_exchange_bmi_msg,
@@ -1808,6 +1857,10 @@ static const struct ath10k_hif_ops ath10k_pci_hif_ops = {
 	.get_free_queue_number	= ath10k_pci_hif_get_free_queue_number,
 	.init			= ath10k_pci_hif_init,
 	.deinit			= ath10k_pci_hif_deinit,
+#if CONFIG_PM
+	.suspend		= ath10k_pci_hif_suspend,
+	.resume			= ath10k_pci_hif_resume,
+#endif
 };
 
 static void ath10k_pci_ce_tasklet(unsigned long ptr)
@@ -2375,128 +2428,6 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 	kfree(ar_pci);
 }
 
-#if defined(CONFIG_PM_SLEEP)
-
-#define ATH10K_PCI_PM_CONTROL 0x44
-
-static int ath10k_pci_suspend(struct device *device)
-{
-	struct pci_dev *pdev = to_pci_dev(device);
-	struct ath10k *ar = pci_get_drvdata(pdev);
-	struct ath10k_pci *ar_pci;
-	u32 val;
-	int ret, retval;
-
-	ath10k_dbg(ATH10K_DBG_PCI, "%s\n", __func__);
-
-	if (!ar)
-		return -ENODEV;
-
-	ar_pci = ath10k_pci_priv(ar);
-	if (!ar_pci)
-		return -ENODEV;
-
-	if (ath10k_core_target_suspend(ar))
-		return -EBUSY;
-
-	ret = wait_event_interruptible_timeout(ar->event_queue,
-						ar->is_target_paused == true,
-						1 * HZ);
-	if (ret < 0) {
-		ath10k_warn("suspend interrupted (%d)\n", ret);
-		retval = ret;
-		goto resume;
-	} else if (ret == 0) {
-		ath10k_warn("suspend timed out - target pause event never came\n");
-		retval = EIO;
-		goto resume;
-	}
-
-	/*
-	 * reset is_target_paused and host can check that in next time,
-	 * or it will always be TRUE and host just skip the waiting
-	 * condition, it causes target assert due to host already
-	 * suspend
-	 */
-	ar->is_target_paused = false;
-
-	pci_read_config_dword(pdev, ATH10K_PCI_PM_CONTROL, &val);
-
-	if ((val & 0x000000ff) != 0x3) {
-		pci_save_state(pdev);
-		pci_disable_device(pdev);
-		pci_write_config_dword(pdev, ATH10K_PCI_PM_CONTROL,
-				       (val & 0xffffff00) | 0x03);
-	}
-
-	return 0;
-resume:
-	ret = ath10k_core_target_resume(ar);
-	if (ret)
-		ath10k_warn("could not resume (%d)\n", ret);
-
-	return retval;
-}
-
-static int ath10k_pci_resume(struct device *device)
-{
-	struct pci_dev *pdev = to_pci_dev(device);
-	struct ath10k *ar = pci_get_drvdata(pdev);
-	struct ath10k_pci *ar_pci;
-	int ret;
-	u32 val;
-
-	ath10k_dbg(ATH10K_DBG_PCI, "%s\n", __func__);
-
-	if (!ar)
-		return -ENODEV;
-	ar_pci = ath10k_pci_priv(ar);
-
-	if (!ar_pci)
-		return -ENODEV;
-
-	ret = pci_enable_device(pdev);
-	if (ret) {
-		ath10k_warn("cannot enable PCI device: %d\n", ret);
-		return ret;
-	}
-
-	pci_read_config_dword(pdev, ATH10K_PCI_PM_CONTROL, &val);
-
-	if ((val & 0x000000ff) != 0) {
-		pci_restore_state(pdev);
-		pci_write_config_dword(pdev, ATH10K_PCI_PM_CONTROL,
-				       val & 0xffffff00);
-		/*
-		 * Suspend/Resume resets the PCI configuration space,
-		 * so we have to re-disable the RETRY_TIMEOUT register (0x41)
-		 * to keep PCI Tx retries from interfering with C3 CPU state
-		 */
-		pci_read_config_dword(pdev, 0x40, &val);
-
-		if ((val & 0x0000ff00) != 0)
-			pci_write_config_dword(pdev, 0x40, val & 0xffff00ff);
-	}
-
-	ret = ath10k_core_target_resume(ar);
-	if (ret)
-		ath10k_warn("target resume failed: %d\n", ret);
-
-	return ret;
-}
-
-static SIMPLE_DEV_PM_OPS(ath10k_dev_pm_ops,
-			 ath10k_pci_suspend,
-			 ath10k_pci_resume);
-
-#define ATH10K_PCI_PM_OPS (&ath10k_dev_pm_ops)
-
-#else
-
-#define ATH10K_PCI_PM_OPS NULL
-
-#endif /* CONFIG_PM_SLEEP */
-
 MODULE_DEVICE_TABLE(pci, ath10k_pci_id_table);
 
 static struct pci_driver ath10k_pci_driver = {
@@ -2504,7 +2435,6 @@ static struct pci_driver ath10k_pci_driver = {
 	.id_table = ath10k_pci_id_table,
 	.probe = ath10k_pci_probe,
 	.remove = ath10k_pci_remove,
-	.driver.pm = ATH10K_PCI_PM_OPS,
 };
 
 static int __init ath10k_pci_init(void)
-- 
1.7.9.5

[ath9k-devel] [PATCH 05/10] ath10k: decouple suspend code

[Sujith Manoharan]

Michal Kazior wrote:
> Split up fw-related and hw-related suspension code.
> 
> Although we don't advertise WoW support to
> mac80211 yet it's useful to keep the code in
> suspend/resume hooks.
> 
> At this point there's no need to keep pci pm ops.
> In case of WoW mac80211 calls ath10k_suspend()
> which should take care of entering low-power mode.
> In case WoW is not available mac80211 will go
> through regular interface teradown and use start/stop.

The usage "#if CONFIG_PM" seems a bit weird - "#ifdef" ?

Sujith

[ath9k-devel] [PATCH 05/10] ath10k: decouple suspend code

[Michal Kazior]

On 06/06/13 13:48, Sujith Manoharan wrote:
> Michal Kazior wrote:
>> Split up fw-related and hw-related suspension code.
>>
>> Although we don't advertise WoW support to
>> mac80211 yet it's useful to keep the code in
>> suspend/resume hooks.
>>
>> At this point there's no need to keep pci pm ops.
>> In case of WoW mac80211 calls ath10k_suspend()
>> which should take care of entering low-power mode.
>> In case WoW is not available mac80211 will go
>> through regular interface teradown and use start/stop.
>
> The usage "#if CONFIG_PM" seems a bit weird - "#ifdef" ?

Yes, it should be an #ifdef. Good catch, thanks!


-- Pozdrawiam / Best regards, Michal Kazior.

[ath9k-devel] [PATCH 06/10] ath10k: defer hw setup to start/stop mac80211 hooks

[Michal Kazior]

This fixes suspend-to-disk. The hardware is now
re-initialized upon freeze/thaw properly.

This also makes suspend/resume re-initialize the
hardware as WoWLAN support is not done yet.

With some little work it should be possible to
support hw reconfiguration for hw/fw recovery.

HW must be initialized once before registering to
mac80211 because FW determinates what hw
capabilities can be advertised.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c |   46 +++++++++++++++++++++++++-------
 drivers/net/wireless/ath/ath10k/mac.c  |   15 +++++++++++
 drivers/net/wireless/ath/ath10k/pci.c  |   10 +------
 3 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 6d378d5..272513d 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -604,6 +604,7 @@ err_htc_destroy:
 err:
 	return status;
 }
+EXPORT_SYMBOL(ath10k_core_init);
 
 void ath10k_core_deinit(struct ath10k *ar)
 {
@@ -612,18 +613,49 @@ void ath10k_core_deinit(struct ath10k *ar)
 	ath10k_wmi_detach(ar);
 	ath10k_htc_destroy(ar->htc);
 }
+EXPORT_SYMBOL(ath10k_core_deinit);
+
+/* mac80211 manages fw/hw initialization through start/stop hooks. However in
+ * order to know what hw capabilities should be advertised to mac80211 it is
+ * necessary to load the firmware (and tear it down immediately since start
+ * hook will try to init it again) before registering */
+static int ath10k_core_probe_fw(struct ath10k *ar)
+{
+	int ret;
+
+	ret = ath10k_hif_init(ar);
+	if (ret) {
+		ath10k_err("could not start pci hif (%d)\n", ret);
+		return ret;
+	}
+
+	ret = ath10k_core_init(ar);
+	if (ret) {
+		ath10k_err("could not init core (%d)\n", ret);
+		ath10k_hif_deinit(ar);
+		return ret;
+	}
+
+	ath10k_core_deinit(ar);
+	ath10k_hif_deinit(ar);
+	return 0;
+}
 
 int ath10k_core_register(struct ath10k *ar)
 {
 	int status;
 
-	status = ath10k_core_init(ar);
-	if (status)
-		goto err;
+	status = ath10k_core_probe_fw(ar);
+	if (status) {
+		ath10k_err("could not probe fw (%d)\n", status);
+		return status;
+	}
 
 	status = ath10k_mac_register(ar);
-	if (status)
-		goto err_core_deinit;
+	if (status) {
+		ath10k_err("could not register to mac80211 (%d)\n", status);
+		return status;
+	}
 
 	status = ath10k_debug_create(ar);
 	if (status) {
@@ -635,9 +667,6 @@ int ath10k_core_register(struct ath10k *ar)
 
 err_unregister_mac:
 	ath10k_mac_unregister(ar);
-err_core_deinit:
-	ath10k_core_deinit(ar);
-err:
 	return status;
 }
 EXPORT_SYMBOL(ath10k_core_register);
@@ -648,7 +677,6 @@ void ath10k_core_unregister(struct ath10k *ar)
 	 * Otherwise we will fail to submit commands to FW and mac80211 will be
 	 * unhappy about callback failures. */
 	ath10k_mac_unregister(ar);
-	ath10k_core_deinit(ar);
 }
 EXPORT_SYMBOL(ath10k_core_unregister);
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 0f339e6..fe72a0c 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1685,6 +1685,19 @@ static int ath10k_start(struct ieee80211_hw *hw)
 	struct ath10k *ar = hw->priv;
 	int ret;
 
+	ret = ath10k_hif_init(ar);
+	if (ret) {
+		ath10k_err("could not init hif (%d)\n", ret);
+		return ret;
+	}
+
+	ret = ath10k_core_init(ar);
+	if (ret) {
+		ath10k_err("could not init core (%d)\n", ret);
+		ath10k_hif_deinit(ar);
+		return ret;
+	}
+
 	ret = ath10k_wmi_pdev_set_param(ar, WMI_PDEV_PARAM_PMF_QOS, 1);
 	if (ret)
 		ath10k_warn("could not enable WMI_PDEV_PARAM_PMF_QOS (%d)\n",
@@ -1707,6 +1720,8 @@ static void ath10k_stop(struct ieee80211_hw *hw)
 	/* avoid leaks in case FW never confirms scan for offchannel */
 	cancel_work_sync(&ar->offchan_tx_work);
 	ath10k_offchan_tx_purge(ar);
+	ath10k_core_deinit(ar);
+	ath10k_hif_deinit(ar);
 
 	ar->started = false;
 }
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 63c5a71..6a7e02f 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2361,22 +2361,14 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 		goto err_iomap;
 	}
 
-	ret = ath10k_pci_hif_init(ar);
-	if (ret) {
-		ath10k_err("could not start pci hif (%d)\n", ret);
-		goto err_intr;
-	}
-
 	ret = ath10k_core_register(ar);
 	if (ret) {
 		ath10k_err("could not register driver core (%d)\n", ret);
-		goto err_hif;
+		goto err_intr;
 	}
 
 	return 0;
 
-err_hif:
-	ath10k_pci_hif_deinit(ar);
 err_intr:
 	ath10k_pci_stop_intr(ar);
 err_iomap:
-- 
1.7.9.5

[ath9k-devel] [PATCH 06/10] ath10k: defer hw setup to start/stop mac80211 hooks

[Sujith Manoharan]

Michal Kazior wrote:
> This fixes suspend-to-disk. The hardware is now
> re-initialized upon freeze/thaw properly.
> 
> This also makes suspend/resume re-initialize the
> hardware as WoWLAN support is not done yet.
> 
> With some little work it should be possible to
> support hw reconfiguration for hw/fw recovery.
> 
> HW must be initialized once before registering to
> mac80211 because FW determinates what hw
> capabilities can be advertised.

Is there really no other alternative to fix s2disk without
moving Fw download to start() ?

Sujith

[ath9k-devel] [PATCH 06/10] ath10k: defer hw setup to start/stop mac80211 hooks

[Sujith Manoharan]

Sujith Manoharan wrote:
> Is there really no other alternative to fix s2disk without
> moving Fw download to start() ?

This approach seems reasonable too.

Sujith

[ath9k-devel] [PATCH 07/10] ath10k: store firmware files in memory

[Michal Kazior]

Different FW versions may provide different
functions thus mean different hw capabilities
advertised to mac80211.

It is now impossible to swap firmware files. It is
safe to swap firmware files on disk during
driver/device runtime.

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

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 272513d..1b2a530 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -247,19 +247,11 @@ static int ath10k_push_board_ext_data(struct ath10k *ar,
 
 static int ath10k_download_board_data(struct ath10k *ar)
 {
+	const struct firmware *fw = ar->board_data;
 	u32 board_data_size = QCA988X_BOARD_DATA_SZ;
 	u32 address;
-	const struct firmware *fw;
 	int ret;
 
-	fw = ath10k_fetch_fw_file(ar, ar->hw_params.fw.dir,
-				  ar->hw_params.fw.board);
-	if (IS_ERR(fw)) {
-		ath10k_err("could not fetch board data fw file (%ld)\n",
-			   PTR_ERR(fw));
-		return PTR_ERR(fw);
-	}
-
 	ret = ath10k_push_board_ext_data(ar, fw);
 	if (ret) {
 		ath10k_err("could not push board ext data (%d)\n", ret);
@@ -286,32 +278,20 @@ static int ath10k_download_board_data(struct ath10k *ar)
 	}
 
 exit:
-	release_firmware(fw);
 	return ret;
 }
 
 static int ath10k_download_and_run_otp(struct ath10k *ar)
 {
-	const struct firmware *fw;
-	u32 address;
+	const struct firmware *fw = ar->otp;
+	u32 address = ar->hw_params.patch_load_addr;
 	u32 exec_param;
 	int ret;
 
 	/* OTP is optional */
 
-	if (ar->hw_params.fw.otp == NULL) {
-		ath10k_info("otp file not defined\n");
-		return 0;
-	}
-
-	address = ar->hw_params.patch_load_addr;
-
-	fw = ath10k_fetch_fw_file(ar, ar->hw_params.fw.dir,
-				  ar->hw_params.fw.otp);
-	if (IS_ERR(fw)) {
-		ath10k_warn("could not fetch otp (%ld)\n", PTR_ERR(fw));
+	if (!ar->otp)
 		return 0;
-	}
 
 	ret = ath10k_bmi_fast_download(ar, address, fw->data, fw->size);
 	if (ret) {
@@ -327,28 +307,17 @@ static int ath10k_download_and_run_otp(struct ath10k *ar)
 	}
 
 exit:
-	release_firmware(fw);
 	return ret;
 }
 
 static int ath10k_download_fw(struct ath10k *ar)
 {
-	const struct firmware *fw;
+	const struct firmware *fw = ar->firmware;
 	u32 address;
 	int ret;
 
-	if (ar->hw_params.fw.fw == NULL)
-		return -EINVAL;
-
 	address = ar->hw_params.patch_load_addr;
 
-	fw = ath10k_fetch_fw_file(ar, ar->hw_params.fw.dir,
-				  ar->hw_params.fw.fw);
-	if (IS_ERR(fw)) {
-		ath10k_err("could not fetch fw (%ld)\n", PTR_ERR(fw));
-		return PTR_ERR(fw);
-	}
-
 	ret = ath10k_bmi_fast_download(ar, address, fw->data, fw->size);
 	if (ret) {
 		ath10k_err("could not write fw (%d)\n", ret);
@@ -356,7 +325,74 @@ static int ath10k_download_fw(struct ath10k *ar)
 	}
 
 exit:
-	release_firmware(fw);
+	return ret;
+}
+
+static void ath10k_core_free_firmware_files(struct ath10k *ar)
+{
+	if (ar->board_data && !IS_ERR(ar->board_data))
+		release_firmware(ar->board_data);
+
+	if (ar->otp && !IS_ERR(ar->otp))
+		release_firmware(ar->otp);
+
+	if (ar->firmware && !IS_ERR(ar->firmware))
+		release_firmware(ar->firmware);
+
+	ar->board_data = NULL;
+	ar->otp = NULL;
+	ar->firmware = NULL;
+}
+
+static int ath10k_core_fetch_firmware_files(struct ath10k *ar)
+{
+	int ret = 0;
+
+	if (ar->hw_params.fw.fw == NULL) {
+		ath10k_err("firmware file not defined\n");
+		return -EINVAL;
+	}
+
+	if (ar->hw_params.fw.board == NULL) {
+		ath10k_err("board data file not defined");
+		return -EINVAL;
+	}
+
+	ar->board_data = ath10k_fetch_fw_file(ar,
+					      ar->hw_params.fw.dir,
+					      ar->hw_params.fw.board);
+	if (IS_ERR(ar->board_data)) {
+		ret = PTR_ERR(ar->board_data);
+		ath10k_err("could not fetch board data (%d)\n", ret);
+		goto err;
+	}
+
+	ar->firmware = ath10k_fetch_fw_file(ar,
+					    ar->hw_params.fw.dir,
+					    ar->hw_params.fw.fw);
+	if (IS_ERR(ar->firmware)) {
+		ret = PTR_ERR(ar->firmware);
+		ath10k_err("could not fetch firmware (%d)\n", ret);
+		goto err;
+	}
+
+	/* OTP may be undefined. If so, don't fetch it at all */
+	if (ar->hw_params.fw.otp == NULL)
+		return 0;
+
+	ar->otp = ath10k_fetch_fw_file(ar,
+				       ar->hw_params.fw.dir,
+				       ar->hw_params.fw.otp);
+	if (IS_ERR(ar->otp)) {
+		ret = PTR_ERR(ar->otp);
+		ath10k_err("could not fetch otp (%d)\n", ret);
+		goto err;
+	}
+
+	return 0;
+
+err:
+	ath10k_core_free_firmware_files(ar);
 	return ret;
 }
 
@@ -505,23 +541,10 @@ EXPORT_SYMBOL(ath10k_core_destroy);
 int ath10k_core_init(struct ath10k *ar)
 {
 	struct ath10k_htc_ops htc_ops;
-	struct bmi_target_info target_info;
 	int status;
 
 	ar->bmi.done_sent = false;
 
-	memset(&target_info, 0, sizeof(target_info));
-	status = ath10k_bmi_get_target_info(ar, &target_info);
-	if (status)
-		goto err;
-
-	ar->target_version = target_info.version;
-	ar->hw->wiphy->hw_version = target_info.version;
-
-	status = ath10k_init_hw_params(ar);
-	if (status)
-		goto err;
-
 	if (ath10k_init_configure_target(ar)) {
 		status = -EINVAL;
 		goto err;
@@ -621,7 +644,8 @@ EXPORT_SYMBOL(ath10k_core_deinit);
  * hook will try to init it again) before registering */
 static int ath10k_core_probe_fw(struct ath10k *ar)
 {
-	int ret;
+	struct bmi_target_info target_info;
+	int ret = 0;
 
 	ret = ath10k_hif_init(ar);
 	if (ret) {
@@ -629,9 +653,35 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
 		return ret;
 	}
 
+	memset(&target_info, 0, sizeof(target_info));
+	ret = ath10k_bmi_get_target_info(ar, &target_info);
+	if (ret) {
+		ath10k_err("could not get target info (%d)\n", ret);
+		ath10k_hif_deinit(ar);
+		return ret;
+	}
+
+	ar->target_version = target_info.version;
+	ar->hw->wiphy->hw_version = target_info.version;
+
+	ret = ath10k_init_hw_params(ar);
+	if (ret) {
+		ath10k_err("could not get hw params (%d)\n", ret);
+		ath10k_hif_deinit(ar);
+		return ret;
+	}
+
+	ret = ath10k_core_fetch_firmware_files(ar);
+	if (ret) {
+		ath10k_err("could not fetch firmware files (%d)\n", ret);
+		ath10k_hif_deinit(ar);
+		return ret;
+	}
+
 	ret = ath10k_core_init(ar);
 	if (ret) {
 		ath10k_err("could not init core (%d)\n", ret);
+		ath10k_core_free_firmware_files(ar);
 		ath10k_hif_deinit(ar);
 		return ret;
 	}
@@ -654,7 +704,7 @@ int ath10k_core_register(struct ath10k *ar)
 	status = ath10k_mac_register(ar);
 	if (status) {
 		ath10k_err("could not register to mac80211 (%d)\n", status);
-		return status;
+		goto err_release_fw;
 	}
 
 	status = ath10k_debug_create(ar);
@@ -667,6 +717,8 @@ int ath10k_core_register(struct ath10k *ar)
 
 err_unregister_mac:
 	ath10k_mac_unregister(ar);
+err_release_fw:
+	ath10k_core_free_firmware_files(ar);
 	return status;
 }
 EXPORT_SYMBOL(ath10k_core_register);
@@ -677,6 +729,7 @@ void ath10k_core_unregister(struct ath10k *ar)
 	 * Otherwise we will fail to submit commands to FW and mac80211 will be
 	 * unhappy about callback failures. */
 	ath10k_mac_unregister(ar);
+	ath10k_core_free_firmware_files(ar);
 }
 EXPORT_SYMBOL(ath10k_core_unregister);
 
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 083185c..5fb6e5f 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -296,6 +296,10 @@ struct ath10k {
 		} fw;
 	} hw_params;
 
+	const struct firmware *board_data;
+	const struct firmware *otp;
+	const struct firmware *firmware;
+
 	struct {
 		struct completion started;
 		struct completion completed;
-- 
1.7.9.5

[ath9k-devel] [PATCH 08/10] ath10k: make sure all resources are freed upon ath10k_stop()

[Michal Kazior]

This is necessary for proper hw reconfiguration
and to avoid memory leaks.

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

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index fe72a0c..06601f5 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -362,6 +362,20 @@ static void ath10k_peer_cleanup(struct ath10k *ar, u32 vdev_id)
 	spin_unlock_bh(&ar->data_lock);
 }
 
+static void ath10k_peer_cleanup_all(struct ath10k *ar)
+{
+	struct ath10k_peer *peer, *tmp;
+
+	mutex_lock(&ar->conf_mutex);
+	spin_lock_bh(&ar->data_lock);
+	list_for_each_entry_safe(peer, tmp, &ar->peers, list) {
+		list_del(&peer->list);
+		kfree(peer);
+	}
+	spin_unlock_bh(&ar->data_lock);
+	mutex_unlock(&ar->conf_mutex);
+}
+
 /************************/
 /* Interface management */
 /************************/
@@ -1717,9 +1731,10 @@ static void ath10k_stop(struct ieee80211_hw *hw)
 {
 	struct ath10k *ar = hw->priv;
 
-	/* avoid leaks in case FW never confirms scan for offchannel */
+	del_timer_sync(&ar->scan.timeout);
 	cancel_work_sync(&ar->offchan_tx_work);
 	ath10k_offchan_tx_purge(ar);
+	ath10k_peer_cleanup_all(ar);
 	ath10k_core_deinit(ar);
 	ath10k_hif_deinit(ar);
 
-- 
1.7.9.5

[ath9k-devel] [PATCH 09/10] ath10k: handle hw reconfiguration properly

[Michal Kazior]

If resume fails mac80211 calls start() hook
without any other notification to the driver.

If that happens we must make sure we free up and
clean up our stuff.

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

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 06601f5..fee4082 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1694,11 +1694,20 @@ static void ath10k_tx(struct ieee80211_hw *hw,
 /*
  * Initialize various parameters with default vaules.
  */
+static void ath10k_stop(struct ieee80211_hw *hw);
+
 static int ath10k_start(struct ieee80211_hw *hw)
 {
 	struct ath10k *ar = hw->priv;
 	int ret;
 
+	if (ar->started) {
+		/* This can be called in case if resume fails or
+		 * ieee80211_hw_restart() was issued */
+		ath10k_info("hw reconfiguration started\n");
+		ath10k_stop(hw);
+	}
+
 	ret = ath10k_hif_init(ar);
 	if (ret) {
 		ath10k_err("could not init hif (%d)\n", ret);
-- 
1.7.9.5

[ath9k-devel] [PATCH 09/10] ath10k: handle hw reconfiguration properly

[Kalle Valo]

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

> If resume fails mac80211 calls start() hook
> without any other notification to the driver.
>
> If that happens we must make sure we free up and
> clean up our stuff.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>  drivers/net/wireless/ath/ath10k/mac.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 06601f5..fee4082 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -1694,11 +1694,20 @@ static void ath10k_tx(struct ieee80211_hw *hw,
>  /*
>   * Initialize various parameters with default vaules.
>   */
> +static void ath10k_stop(struct ieee80211_hw *hw);
> +
>  static int ath10k_start(struct ieee80211_hw *hw)
>  {
>  	struct ath10k *ar = hw->priv;
>  	int ret;
>  
> +	if (ar->started) {
> +		/* This can be called in case if resume fails or
> +		 * ieee80211_hw_restart() was issued */
> +		ath10k_info("hw reconfiguration started\n");
> +		ath10k_stop(hw);
> +	}

This look suspicious to me. Can you describe in detail the failing
resume case, please?

If the idea is to initialise the hardware to a pristine state that we
can boot the firmware, please refactor the code into it's own function.
This just looks wrong.

-- 
Kalle Valo

[ath9k-devel] [PATCH 09/10] ath10k: handle hw reconfiguration properly

[Michal Kazior]

On 10/06/13 19:08, Kalle Valo wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> If resume fails mac80211 calls start() hook
>> without any other notification to the driver.
>>
>> If that happens we must make sure we free up and
>> clean up our stuff.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>> ---
>>   drivers/net/wireless/ath/ath10k/mac.c |    9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
>> index 06601f5..fee4082 100644
>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>> @@ -1694,11 +1694,20 @@ static void ath10k_tx(struct ieee80211_hw *hw,
>>   /*
>>    * Initialize various parameters with default vaules.
>>    */
>> +static void ath10k_stop(struct ieee80211_hw *hw);
>> +
>>   static int ath10k_start(struct ieee80211_hw *hw)
>>   {
>>   	struct ath10k *ar = hw->priv;
>>   	int ret;
>>
>> +	if (ar->started) {
>> +		/* This can be called in case if resume fails or
>> +		 * ieee80211_hw_restart() was issued */
>> +		ath10k_info("hw reconfiguration started\n");
>> +		ath10k_stop(hw);
>> +	}
>
> This look suspicious to me. Can you describe in detail the failing
> resume case, please?

If resume fails mac80211 behaves as if ieee80211_restart_hw() is called, 
i.e. it calls start() hook and proceeds with ressurecting STAs, 
interfaces, keys.


> If the idea is to initialise the hardware to a pristine state that we
> can boot the firmware, please refactor the code into it's own function.
> This just looks wrong.

This is more or less what I've done but haven't uploaded yet (since I'm 
still doing some tweaks/fixes).

Thanks for the review!


-- Pozdrawiam / Best regards, Michal Kazior.

[ath9k-devel] [PATCH 10/10] ath10k: move free_vdev_map initialization

[Michal Kazior]

This is necessary for hw reconfiguration to work.
Since mac80211 is not calling remove_interface()
is such case we must reset free_vdev_map.

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

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 1b2a530..eda4a59 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -494,8 +494,6 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
 	ar->hif.priv = hif_priv;
 	ar->hif.ops = hif_ops;
 
-	ar->free_vdev_map = 0xFF; /* 8 vdevs */
-
 	init_completion(&ar->scan.started);
 	init_completion(&ar->scan.completed);
 	init_completion(&ar->scan.on_channel);
@@ -544,6 +542,7 @@ int ath10k_core_init(struct ath10k *ar)
 	int status;
 
 	ar->bmi.done_sent = false;
+	ar->free_vdev_map = 0xFF; /* 8 vdevs */
 
 	if (ath10k_init_configure_target(ar)) {
 		status = -EINVAL;
-- 
1.7.9.5