Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v3 0/3] Fixes and improvements for brcmfmac driver
@ 2020-08-30 19:14 Dmitry Osipenko
  2020-08-30 19:14 ` [PATCH v3 1/3] brcmfmac: increase F2 watermark for BCM4329 Dmitry Osipenko
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Dmitry Osipenko @ 2020-08-30 19:14 UTC (permalink / raw)
  To: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, Kalle Valo
  Cc: linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
	netdev, linux-tegra, linux-kernel

Hello!

Recently I was debugging WiFi performance problems on Acer A500 tablet
device that got upstreamed recently. This is an older Android device from
2011-2012 that is powered by NVIDIA Tegra20 SoC and it has BCM4329 chip
that provides WiFi (SDIO) and Bluetooth (UART). I noticed that WiFi
throughput on a recent Linux kernel wasn't as great as it could be in
comparison to older 3.18 kernel that is uses downstream BCMDHD driver
and this series fixes a major part of the problems that I found.

Found problems:

1. The WiFi SDIO pinmux configuration had a bug in Acer A500 device-tree
   and MMC refused to work if it was clocked above 25MHz and legacy
   signaling mode was used. After fixing the bug, WiFi SDIO works perfectly
   well at 50MHz and this increases TX throughput by 5-10 Mbit/s. I already
   sent out patches that fix this bug to the Tegra ML.

2. There are occasional SDHCI CRC errors if SDIO is clocked above 25Mhz.
   The "increase F2 watermark" patch fixes this problem.

3. WiFi TX throughput is lower by 10 Mbit/s than it should be using 512B
   for maximum F2 SDIO block size. Reducing block size to 128B fixes this
   problem. The "set F2 SDIO block size to 128 bytes" patch addresses this
   issue. The exact reason why 128B is more efficient than 512B is unknown,
   this optimization is borrowed from the BCMDHD driver.

4. While I was bisecting upstream kernel, I found that WiFi RX/TX throughput
   dropped by 5-10 Mbit/s after 5.2 kernel and reverting the following commit
   from linux-next resolves the problem:

   commit c07a48c2651965e84d35cf193dfc0e5f7892d612
   Author: Adrian Hunter <adrian.hunter@intel.com>
   Date:   Fri Apr 5 15:40:20 2019 +0300

       mmc: sdhci: Remove finish_tasklet

   I'll sent a separate email for discussing this problem [1].

   [1] https://lkml.org/lkml/2020/8/27/54

After fixing all the above problems, I'm now getting a solid 40 Mbit/s
up/down on Acer A500 on a recent linux-next in comparison to 15 Mbit/s
that I was getting before the fixes.

Big thanks to Wright Feng who helped me to find and fix some of the problems!

Changelog:

v3: - The v2 "drop unnecessary "fallthrough" comments" patch isn't needed
      anymore because that change is already applied.

    - Added r-b from Arend van Spriel that he gave to the v1 of "increase F2
      watermark for BCM4329" patch.

v2: - Added "drop chip id from debug messages" as was requested by
      Arend Van Spriel in the review comment to v1 of the "increase F2
      watermark" patch.

    - Added patches that remove unnecessary "fallthrough" comments and
      change F2 SDIO block size to 128 bytes for BCM4329.

Dmitry Osipenko (3):
  brcmfmac: increase F2 watermark for BCM4329
  brcmfmac: drop chip id from debug messages
  brcmfmac: set F2 SDIO block size to 128 bytes for BCM4329

 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 4 ++++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c   | 5 +++--
 2 files changed, 7 insertions(+), 2 deletions(-)

-- 
2.27.0


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

* [PATCH v3 1/3] brcmfmac: increase F2 watermark for BCM4329
  2020-08-30 19:14 [PATCH v3 0/3] Fixes and improvements for brcmfmac driver Dmitry Osipenko
@ 2020-08-30 19:14 ` Dmitry Osipenko
  2020-09-07  8:52   ` Kalle Valo
  2020-08-30 19:14 ` [PATCH v3 2/3] brcmfmac: drop chip id from debug messages Dmitry Osipenko
  2020-08-30 19:14 ` [PATCH v3 3/3] brcmfmac: set F2 SDIO block size to 128 bytes for BCM4329 Dmitry Osipenko
  2 siblings, 1 reply; 5+ messages in thread
From: Dmitry Osipenko @ 2020-08-30 19:14 UTC (permalink / raw)
  To: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, Kalle Valo
  Cc: linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
	netdev, linux-tegra, linux-kernel

This patch fixes SDHCI CRC errors during of RX throughput testing on
BCM4329 chip if SDIO BUS is clocked above 25MHz. In particular the
checksum problem is observed on NVIDIA Tegra20 SoCs. The good watermark
value is borrowed from downstream BCMDHD driver and it's matching to the
value that is already used for the BCM4339 chip, hence let's re-use it
for BCM4329.

Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index d1b96bad2718..b16944a898f9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -4278,6 +4278,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 			brcmf_sdiod_writeb(sdiod, SBSDIO_FUNC1_MESBUSYCTRL,
 					   CY_43012_MESBUSYCTRL, &err);
 			break;
+		case SDIO_DEVICE_ID_BROADCOM_4329:
 		case SDIO_DEVICE_ID_BROADCOM_4339:
 			brcmf_dbg(INFO, "set F2 watermark to 0x%x*4 bytes for 4339\n",
 				  CY_4339_F2_WATERMARK);
-- 
2.27.0


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

* [PATCH v3 2/3] brcmfmac: drop chip id from debug messages
  2020-08-30 19:14 [PATCH v3 0/3] Fixes and improvements for brcmfmac driver Dmitry Osipenko
  2020-08-30 19:14 ` [PATCH v3 1/3] brcmfmac: increase F2 watermark for BCM4329 Dmitry Osipenko
@ 2020-08-30 19:14 ` Dmitry Osipenko
  2020-08-30 19:14 ` [PATCH v3 3/3] brcmfmac: set F2 SDIO block size to 128 bytes for BCM4329 Dmitry Osipenko
  2 siblings, 0 replies; 5+ messages in thread
From: Dmitry Osipenko @ 2020-08-30 19:14 UTC (permalink / raw)
  To: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, Kalle Valo
  Cc: linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
	netdev, linux-tegra, linux-kernel

The chip ID was already printed out at the time when debug message about
the changed F2 watermark is printed, hence let's drop the unnecessary part
of the debug messages. This cleans code a tad and also allows to re-use
the F2 watermark debug messages by multiple chips.

Suggested-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index b16944a898f9..d4989e0cd7be 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -4280,7 +4280,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 			break;
 		case SDIO_DEVICE_ID_BROADCOM_4329:
 		case SDIO_DEVICE_ID_BROADCOM_4339:
-			brcmf_dbg(INFO, "set F2 watermark to 0x%x*4 bytes for 4339\n",
+			brcmf_dbg(INFO, "set F2 watermark to 0x%x*4 bytes\n",
 				  CY_4339_F2_WATERMARK);
 			brcmf_sdiod_writeb(sdiod, SBSDIO_WATERMARK,
 					   CY_4339_F2_WATERMARK, &err);
@@ -4293,7 +4293,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 					   CY_4339_MESBUSYCTRL, &err);
 			break;
 		case SDIO_DEVICE_ID_BROADCOM_43455:
-			brcmf_dbg(INFO, "set F2 watermark to 0x%x*4 bytes for 43455\n",
+			brcmf_dbg(INFO, "set F2 watermark to 0x%x*4 bytes\n",
 				  CY_43455_F2_WATERMARK);
 			brcmf_sdiod_writeb(sdiod, SBSDIO_WATERMARK,
 					   CY_43455_F2_WATERMARK, &err);
-- 
2.27.0


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

* [PATCH v3 3/3] brcmfmac: set F2 SDIO block size to 128 bytes for BCM4329
  2020-08-30 19:14 [PATCH v3 0/3] Fixes and improvements for brcmfmac driver Dmitry Osipenko
  2020-08-30 19:14 ` [PATCH v3 1/3] brcmfmac: increase F2 watermark for BCM4329 Dmitry Osipenko
  2020-08-30 19:14 ` [PATCH v3 2/3] brcmfmac: drop chip id from debug messages Dmitry Osipenko
@ 2020-08-30 19:14 ` Dmitry Osipenko
  2 siblings, 0 replies; 5+ messages in thread
From: Dmitry Osipenko @ 2020-08-30 19:14 UTC (permalink / raw)
  To: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, Kalle Valo
  Cc: linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
	netdev, linux-tegra, linux-kernel

Setting F2 block size to 128 bytes for BCM4329 allows to significantly
improve RX throughput on NVIDIA Tegra20. Before this change the throughput
was capped to 30 Mbit/s on Tegra, now throughput is at 40 Mbit/s, which is
a maximum throughput for the BCM4329 chip. The F2 block size is borrowed
from the downstream BCMDHD driver. The comment in the BCMDHD driver says
that 128B improves throughput and turns out that it works for the brcmfmac
as well.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 0dc4de2fa9f6..318bd00bf94f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -45,6 +45,7 @@
 #define SDIO_FUNC2_BLOCKSIZE		512
 #define SDIO_4373_FUNC2_BLOCKSIZE	256
 #define SDIO_435X_FUNC2_BLOCKSIZE	256
+#define SDIO_4329_FUNC2_BLOCKSIZE	128
 /* Maximum milliseconds to wait for F2 to come up */
 #define SDIO_WAIT_F2RDY	3000
 
@@ -920,6 +921,9 @@ int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
 	case SDIO_DEVICE_ID_BROADCOM_4356:
 		f2_blksz = SDIO_435X_FUNC2_BLOCKSIZE;
 		break;
+	case SDIO_DEVICE_ID_BROADCOM_4329:
+		f2_blksz = SDIO_4329_FUNC2_BLOCKSIZE;
+		break;
 	default:
 		break;
 	}
-- 
2.27.0


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

* Re: [PATCH v3 1/3] brcmfmac: increase F2 watermark for BCM4329
  2020-08-30 19:14 ` [PATCH v3 1/3] brcmfmac: increase F2 watermark for BCM4329 Dmitry Osipenko
@ 2020-09-07  8:52   ` Kalle Valo
  0 siblings, 0 replies; 5+ messages in thread
From: Kalle Valo @ 2020-09-07  8:52 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, linux-wireless, brcm80211-dev-list.pdl,
	brcm80211-dev-list, netdev, linux-tegra, linux-kernel

Dmitry Osipenko <digetx@gmail.com> wrote:

> This patch fixes SDHCI CRC errors during of RX throughput testing on
> BCM4329 chip if SDIO BUS is clocked above 25MHz. In particular the
> checksum problem is observed on NVIDIA Tegra20 SoCs. The good watermark
> value is borrowed from downstream BCMDHD driver and it's matching to the
> value that is already used for the BCM4339 chip, hence let's re-use it
> for BCM4329.
> 
> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

3 patches applied to wireless-drivers-next.git, thanks.

317da69d10b0 brcmfmac: increase F2 watermark for BCM4329
1a867a6230db brcmfmac: drop chip id from debug messages
cc95fa81524a brcmfmac: set F2 SDIO block size to 128 bytes for BCM4329

-- 
https://patchwork.kernel.org/patch/11745283/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


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

end of thread, other threads:[~2020-09-07  8:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-30 19:14 [PATCH v3 0/3] Fixes and improvements for brcmfmac driver Dmitry Osipenko
2020-08-30 19:14 ` [PATCH v3 1/3] brcmfmac: increase F2 watermark for BCM4329 Dmitry Osipenko
2020-09-07  8:52   ` Kalle Valo
2020-08-30 19:14 ` [PATCH v3 2/3] brcmfmac: drop chip id from debug messages Dmitry Osipenko
2020-08-30 19:14 ` [PATCH v3 3/3] brcmfmac: set F2 SDIO block size to 128 bytes for BCM4329 Dmitry Osipenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).