[PATCH 0/3] wl12xx: improve firmware transfer efficiency

[PATCH 0/3] wl12xx: improve firmware transfer efficiency

[Luciano Coelho]

Hello,

These patches increase the FW upload efficiency by using multiple SDIO
blocks in a single write.  After a lot of measurements, studies and
discussions, I came to the conclusion that 16K chunks is the optimal
size to use.  This reduces the upload time by a factor of 0.43 on
wl127x and by a factor of 0.17 on wl128x.  In absolute numbers this
means 75ms and 179ms respectively.

The first patch changes the semantics and name of the blocksize
alignment quirk to negative, to simplify the code in the next two
patches.

Cheers,
Luca.

Luciano Coelho (3):
  wl12xx: change blocksize alignment quirk to negative
  wl12xx: use the same SDIO block size for all different chips
  wl12xx: increase firmware upload chunk size

 drivers/net/wireless/wl12xx/init.c   |    2 +-
 drivers/net/wireless/wl12xx/main.c   |   18 ++++++++++++++----
 drivers/net/wireless/wl12xx/reg.h    |    2 +-
 drivers/net/wireless/wl12xx/tx.c     |    6 +++---
 drivers/net/wireless/wl12xx/wl12xx.h |    4 ++--
 5 files changed, 21 insertions(+), 11 deletions(-)

-- 
1.7.4.1

[PATCH 1/3] wl12xx: change blocksize alignment quirk to negative

[Luciano Coelho]

SDIO blocksize alignment support is now the rule, not the exception.
To simplify the code in patches to come, invert the meaning of the
quirk to be negative (ie. the quirk is set if the device does _not_
support blocksize alignment).

Signed-off-by: Luciano Coelho <coelho@ti.com>
---
 drivers/net/wireless/wl12xx/init.c   |    2 +-
 drivers/net/wireless/wl12xx/main.c   |    8 ++++++--
 drivers/net/wireless/wl12xx/tx.c     |    6 +++---
 drivers/net/wireless/wl12xx/wl12xx.h |    4 ++--
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/init.c b/drivers/net/wireless/wl12xx/init.c
index c6084f8..6d06188 100644
--- a/drivers/net/wireless/wl12xx/init.c
+++ b/drivers/net/wireless/wl12xx/init.c
@@ -501,7 +501,7 @@ int wl1271_chip_specific_init(struct wl1271 *wl)
 	if (wl->chip.id == CHIP_ID_1283_PG20) {
 		u32 host_cfg_bitmap = HOST_IF_CFG_RX_FIFO_ENABLE;
 
-		if (wl->quirks & WL12XX_QUIRK_BLOCKSIZE_ALIGNMENT)
+		if (!(wl->quirks & WL12XX_QUIRK_NO_BLOCKSIZE_ALIGNMENT))
 			/* Enable SDIO padding */
 			host_cfg_bitmap |= HOST_IF_CFG_TX_PAD_TO_SDIO_BLK;
 
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index f76be5a..bddf289 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -1323,7 +1323,9 @@ static int wl1271_chip_wakeup(struct wl1271 *wl)
 		ret = wl1271_setup(wl);
 		if (ret < 0)
 			goto out;
+		wl->quirks |= WL12XX_QUIRK_NO_BLOCKSIZE_ALIGNMENT;
 		break;
+
 	case CHIP_ID_1271_PG20:
 		wl1271_debug(DEBUG_BOOT, "chip id 0x%x (1271 PG20)",
 			     wl->chip.id);
@@ -1331,7 +1333,9 @@ static int wl1271_chip_wakeup(struct wl1271 *wl)
 		ret = wl1271_setup(wl);
 		if (ret < 0)
 			goto out;
+		wl->quirks |= WL12XX_QUIRK_NO_BLOCKSIZE_ALIGNMENT;
 		break;
+
 	case CHIP_ID_1283_PG20:
 		wl1271_debug(DEBUG_BOOT, "chip id 0x%x (1283 PG20)",
 			     wl->chip.id);
@@ -1340,8 +1344,8 @@ static int wl1271_chip_wakeup(struct wl1271 *wl)
 		if (ret < 0)
 			goto out;
 
-		if (wl1271_set_block_size(wl))
-			wl->quirks |= WL12XX_QUIRK_BLOCKSIZE_ALIGNMENT;
+		if (!wl1271_set_block_size(wl))
+			wl->quirks |= WL12XX_QUIRK_NO_BLOCKSIZE_ALIGNMENT;
 		break;
 	case CHIP_ID_1283_PG10:
 	default:
diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
index c7ad4f5..686c5e5 100644
--- a/drivers/net/wireless/wl12xx/tx.c
+++ b/drivers/net/wireless/wl12xx/tx.c
@@ -205,10 +205,10 @@ u8 wl12xx_tx_get_hlid(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 static unsigned int wl12xx_calc_packet_alignment(struct wl1271 *wl,
 						unsigned int packet_length)
 {
-	if (wl->quirks & WL12XX_QUIRK_BLOCKSIZE_ALIGNMENT)
-		return ALIGN(packet_length, WL12XX_BUS_BLOCK_SIZE);
-	else
+	if (wl->quirks & WL12XX_QUIRK_NO_BLOCKSIZE_ALIGNMENT)
 		return ALIGN(packet_length, WL1271_TX_ALIGN_TO);
+	else
+		return ALIGN(packet_length, WL12XX_BUS_BLOCK_SIZE);
 }
 
 static int wl1271_tx_allocate(struct wl1271 *wl, struct wl12xx_vif *wlvif,
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index b7036df..e58e801 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -669,8 +669,8 @@ size_t wl12xx_copy_fwlog(struct wl1271 *wl, u8 *memblock, size_t maxlen);
 /* Each RX/TX transaction requires an end-of-transaction transfer */
 #define WL12XX_QUIRK_END_OF_TRANSACTION		BIT(0)
 
-/* WL128X requires aggregated packets to be aligned to the SDIO block size */
-#define WL12XX_QUIRK_BLOCKSIZE_ALIGNMENT	BIT(2)
+/* wl127x and SPI don't support SDIO block size alignment */
+#define WL12XX_QUIRK_NO_BLOCKSIZE_ALIGNMENT	BIT(2)
 
 /* Older firmwares did not implement the FW logger over bus feature */
 #define WL12XX_QUIRK_FWLOG_NOT_IMPLEMENTED	BIT(4)
-- 
1.7.4.1

[PATCH 2/3] wl12xx: use the same SDIO block size for all different chips

[Luciano Coelho]

The sdio driver uses a block size of 512 bytes by default.  With our
card, this doesn't work correctly because it sets the block size FBR
in the chip too early (ie. before the chip is powered on).  Thus, if
we don't set it explicitly, block mode remains disabled in the chip.
If we try to send more data than fits in one block, the sdio driver
will split it into separate blocks before sending to the chip.  This
causes problems because the chip is not expecting multiple blocks.

At the moment this is not a problem, because we use chunks of 512
bytes for firmware upload and the data is always sent in byte mode.
In the next patch, we will change the chunk size to a bigger value, so
this patch is a preparation for that.

Signed-off-by: Luciano Coelho <coelho@ti.com>
---
 drivers/net/wireless/wl12xx/main.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index bddf289..0532630 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -1313,7 +1313,16 @@ static int wl1271_chip_wakeup(struct wl1271 *wl)
 	/* 0. read chip id from CHIP_ID */
 	wl->chip.id = wl1271_read32(wl, CHIP_ID_B);
 
-	/* 1. check if chip id is valid */
+	/*
+	 * For wl127x based devices we could use the default block
+	 * size (512 bytes), but due to a bug in the sdio driver, we
+	 * need to set it explicitly after the chip is powered on.  To
+	 * simplify the code and since the performance impact is
+	 * negligible, we use the same block size for all different
+	 * chip types.
+	 */
+	if (!wl1271_set_block_size(wl))
+		wl->quirks |= WL12XX_QUIRK_NO_BLOCKSIZE_ALIGNMENT;
 
 	switch (wl->chip.id) {
 	case CHIP_ID_1271_PG10:
@@ -1343,9 +1352,6 @@ static int wl1271_chip_wakeup(struct wl1271 *wl)
 		ret = wl1271_setup(wl);
 		if (ret < 0)
 			goto out;
-
-		if (!wl1271_set_block_size(wl))
-			wl->quirks |= WL12XX_QUIRK_NO_BLOCKSIZE_ALIGNMENT;
 		break;
 	case CHIP_ID_1283_PG10:
 	default:
-- 
1.7.4.1

[PATCH 3/3] wl12xx: increase firmware upload chunk size

[Luciano Coelho]

The chunk size used during firmware upload was set to 512, which is
the size of a single SDIO block (or two).  This is very inneficient
because we send one or two blocks only per SDIO transaction and don't
get the full benefits of sdio block transfers.

This patch increases the chunk size to 16K.  This more than doubles
the transfer speed both in wl127x and wl128x chips, with greater
impact on the latter:

wl127x:   512 bytes chunk -> ~132ms
        16384 bytes chunk ->  ~57ms
wl128x:   512 bytes chunk -> ~216ms
        16384 bytes chunk ->  ~37ms

Signed-off-by: Luciano Coelho <coelho@ti.com>
---
 drivers/net/wireless/wl12xx/reg.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/reg.h b/drivers/net/wireless/wl12xx/reg.h
index 3f570f3..df34d59 100644
--- a/drivers/net/wireless/wl12xx/reg.h
+++ b/drivers/net/wireless/wl12xx/reg.h
@@ -408,7 +408,7 @@
 
 
 /* Firmware image load chunk size */
-#define CHUNK_SIZE          512
+#define CHUNK_SIZE	16384
 
 /* Firmware image header size */
 #define FW_HDR_SIZE 8
-- 
1.7.4.1

Re: [PATCH 0/3] wl12xx: improve firmware transfer efficiency

[Luciano Coelho]

On Thu, 2011-11-03 at 08:44 +0200, Luciano Coelho wrote: 
> Hello,
> 
> These patches increase the FW upload efficiency by using multiple SDIO
> blocks in a single write.  After a lot of measurements, studies and
> discussions, I came to the conclusion that 16K chunks is the optimal
> size to use.  This reduces the upload time by a factor of 0.43 on
> wl127x and by a factor of 0.17 on wl128x.  In absolute numbers this
> means 75ms and 179ms respectively.
> 
> The first patch changes the semantics and name of the blocksize
> alignment quirk to negative, to simplify the code in the next two
> patches.

Applied this series.

-- 
Cheers,
Luca.