ath10k: fix disabling of UART debugging
diff mbox series

Message ID d492617ed38eb7953401c3171bb3e08c48b7b431.1570976384.git.ps@pks.im
State New
Headers show
Series
  • ath10k: fix disabling of UART debugging
Related show

Commit Message

Patrick Steinhardt Oct. 13, 2019, 2:20 p.m. UTC
Starting with v5.3.0, it was observed that throughput of an access point
with QCA988x-based wireless chip is severely degraded from at least
10MB/s to roughly 200KB/s. A bisect of the issue points to commit
4504f0e5b571 (ath10k: sdio: workaround firmware UART pin configuration
bug, 2019-04-19), which employs a workaround for a firmware bug in some
QCA6174 SDIO devices.

If UART debugging is disabled via the "ath10k.uart_print" module
parameter, then the UART initialization code is skipped. With the new
workaround introduced in the mentioned commit, devices that need the
workaround will also re-set the UART pin of the device to avoid a
failing SDIO interrupt. But in fact, the change effectively removed the
early return for all devices that do not use the workaround, and as a
result UART debugging is now unconditionally turned on disregarding the
value of "ath10k.uart_print", causing the decrease in throughput.

Fix the issue by re-introducing the early return for all devices again,
independent of the UART pin workaround. Tests show that throughput is
back to normal levels again with this fix.

Fixes: 4504f0e5b571 ("ath10k: sdio: workaround firmware UART pin configuration bug")
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 drivers/net/wireless/ath/ath10k/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Kalle Valo Oct. 14, 2019, 8:53 a.m. UTC | #1
+ linux-wireless

Patrick Steinhardt <ps@pks.im> writes:

> Starting with v5.3.0, it was observed that throughput of an access point
> with QCA988x-based wireless chip is severely degraded from at least
> 10MB/s to roughly 200KB/s. A bisect of the issue points to commit
> 4504f0e5b571 (ath10k: sdio: workaround firmware UART pin configuration
> bug, 2019-04-19), which employs a workaround for a firmware bug in some
> QCA6174 SDIO devices.
>
> If UART debugging is disabled via the "ath10k.uart_print" module
> parameter, then the UART initialization code is skipped. With the new
> workaround introduced in the mentioned commit, devices that need the
> workaround will also re-set the UART pin of the device to avoid a
> failing SDIO interrupt. But in fact, the change effectively removed the
> early return for all devices that do not use the workaround, and as a
> result UART debugging is now unconditionally turned on disregarding the
> value of "ath10k.uart_print", causing the decrease in throughput.
>
> Fix the issue by re-introducing the early return for all devices again,
> independent of the UART pin workaround. Tests show that throughput is
> back to normal levels again with this fix.
>
> Fixes: 4504f0e5b571 ("ath10k: sdio: workaround firmware UART pin configuration bug")
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  drivers/net/wireless/ath/ath10k/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
> index dc45d16e8d21..8e4ca231634d 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -2125,9 +2125,10 @@ static int ath10k_init_uart(struct ath10k *ar)
>  			ath10k_warn(ar, "failed to set UART TX pin: %d", ret);
>  			return ret;
>  		}
> +	}
>  
> +	if (!uart_print)
>  		return 0;
> -	}

This was fixed in -next with a similar (but not identical) patch, but as
there have been multiple reports of this issues I have now cherry picked
it also for v5.4:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git/commit/?id=d79749f7716d9dc32fa2d5075f6ec29aac63c76d

Patch
diff mbox series

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index dc45d16e8d21..8e4ca231634d 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2125,9 +2125,10 @@  static int ath10k_init_uart(struct ath10k *ar)
 			ath10k_warn(ar, "failed to set UART TX pin: %d", ret);
 			return ret;
 		}
+	}
 
+	if (!uart_print)
 		return 0;
-	}
 
 	ret = ath10k_bmi_write32(ar, hi_dbg_uart_txpin, ar->hw_params.uart_pin);
 	if (ret) {