[RFC,v3,04/11] ath10k: add start_once support
diff mbox

Message ID 20170917194013.8658-5-erik.stromdahl@gmail.com
State New
Headers show

Commit Message

Erik Stromdahl Sept. 17, 2017, 7:40 p.m. UTC
Add possibility to configure the driver to only start target once.
This can reduce startup time of SDIO devices significantly since
loading the firmware can take a substantial amount of time.

The patch is also necessary for high latency devices in general
since it does not seem to be possible to rerun the BMI phase
(fw upload) without power-cycling the device.

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 drivers/net/wireless/ath/ath10k/core.c | 19 +++++++++++++++----
 drivers/net/wireless/ath/ath10k/core.h |  2 ++
 drivers/net/wireless/ath/ath10k/hw.h   |  6 ++++++
 3 files changed, 23 insertions(+), 4 deletions(-)

Comments

Kalle Valo Dec. 22, 2017, 3:25 p.m. UTC | #1
Erik Stromdahl <erik.stromdahl@gmail.com> writes:

> Add possibility to configure the driver to only start target once.
> This can reduce startup time of SDIO devices significantly since
> loading the firmware can take a substantial amount of time.

But it also makes it impossible to restart the firmware if it crashes,
right? Good to mention that in the commit log.

> The patch is also necessary for high latency devices in general since
> it does not seem to be possible to rerun the BMI phase (fw upload)
> without power-cycling the device.
>
> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>

[...]

> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -784,6 +784,8 @@ struct ath10k {
>  
>  	bool is_high_latency;
>  
> +	bool is_started;

Is a separate boolean really needed? State management becomes really
difficult if an enum ath10k_state and this boolean to define the state
of the device. Can't you use ar->state?

> --- a/drivers/net/wireless/ath/ath10k/hw.h
> +++ b/drivers/net/wireless/ath/ath10k/hw.h
> @@ -569,6 +569,12 @@ struct ath10k_hw_params {
>  	bool is_high_latency;
>  
>  	enum ath10k_bus bus;
> +
> +	/* Specifies whether or not the device should be started once.
> +	 * If set, the device will be started once by the early fw probe
> +	 * and it will not be terminated afterwards.
> +	 */
> +	bool start_once;
>  };

I would actually prefer that the bus driver (eg. usb.c) decides this and
provides it though ath10k_core_register(). It might be that some SDIO
devices have a GPIO line to reset the device etc.

The struct ath10k_bus_params I mentioned earlier might be handy also
here.

Patch
diff mbox

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index a4a326c89e0d..c21227a74996 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1999,6 +1999,9 @@  int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
 	int status;
 	u32 val;
 
+	if (ar->is_started && ar->hw_params.start_once)
+		return 0;
+
 	lockdep_assert_held(&ar->conf_mutex);
 
 	clear_bit(ATH10K_FLAG_CRASH_FLUSH, &ar->dev_flags);
@@ -2226,6 +2229,7 @@  int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
 	if (status)
 		goto err_hif_stop;
 
+	ar->is_started = true;
 	return 0;
 
 err_hif_stop:
@@ -2278,6 +2282,7 @@  void ath10k_core_stop(struct ath10k *ar)
 	ath10k_htt_tx_stop(&ar->htt);
 	ath10k_htt_rx_free(&ar->htt);
 	ath10k_wmi_detach(ar);
+	ar->is_started = false;
 }
 EXPORT_SYMBOL(ath10k_core_stop);
 
@@ -2380,12 +2385,18 @@  static int ath10k_core_probe_fw(struct ath10k *ar)
 		goto err_unlock;
 	}
 
-	ath10k_debug_print_boot_info(ar);
-	ath10k_core_stop(ar);
+	/* Leave target running if hw_params.start_once is set */
+	if (ar->hw_params.start_once) {
+		mutex_unlock(&ar->conf_mutex);
+	} else {
+		ath10k_debug_print_boot_info(ar);
+		ath10k_core_stop(ar);
 
-	mutex_unlock(&ar->conf_mutex);
+		mutex_unlock(&ar->conf_mutex);
+
+		ath10k_hif_power_down(ar);
+	}
 
-	ath10k_hif_power_down(ar);
 	return 0;
 
 err_unlock:
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 64dadcd6e531..0b5b1dd00e16 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -784,6 +784,8 @@  struct ath10k {
 
 	bool is_high_latency;
 
+	bool is_started;
+
 	struct {
 		enum ath10k_bus bus;
 		const struct ath10k_hif_ops *ops;
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 8cf7b963f3d4..fd0536077404 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -569,6 +569,12 @@  struct ath10k_hw_params {
 	bool is_high_latency;
 
 	enum ath10k_bus bus;
+
+	/* Specifies whether or not the device should be started once.
+	 * If set, the device will be started once by the early fw probe
+	 * and it will not be terminated afterwards.
+	 */
+	bool start_once;
 };
 
 struct htt_rx_desc;