* [PATCH 1/5] wl12xx: Clean up and fix the 128x boot sequence
2011-03-31 8:06 [PATCH 0/5] wl12xx: Cleanups & fixes Ido Yariv
@ 2011-03-31 8:06 ` Ido Yariv
2011-04-01 10:43 ` Luciano Coelho
2011-03-31 8:06 ` [PATCH 2/5] wl12xx: Clean up the block size alignment code Ido Yariv
` (4 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Ido Yariv @ 2011-03-31 8:06 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless, Ido Yariv
Clean up the boot sequence code & fix the following issues:
1. Always read the registers' values and set the relevant bits instead of
zeroing all other bits
2. Handle cases where wl1271_top_reg_read returns an error
3. Verify that the HW can detect the selected clock source
4. Remove 128x PG10 initialization code
5. Configure the MCS PLL to work in HP mode
Signed-off-by: Ido Yariv <ido@wizery.com>
---
drivers/net/wireless/wl12xx/boot.c | 235 ++++++++++++++++++------------------
drivers/net/wireless/wl12xx/boot.h | 1 +
include/linux/wl12xx.h | 10 +-
3 files changed, 123 insertions(+), 123 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/boot.c b/drivers/net/wireless/wl12xx/boot.c
index 34bf2fe..b5ec2c2 100644
--- a/drivers/net/wireless/wl12xx/boot.c
+++ b/drivers/net/wireless/wl12xx/boot.c
@@ -523,137 +523,137 @@ static void wl1271_boot_hw_version(struct wl1271 *wl)
wl->quirks |= WL12XX_QUIRK_END_OF_TRANSACTION;
}
-/*
- * WL128x has two clocks input - TCXO and FREF.
- * TCXO is the main clock of the device, while FREF is used to sync
- * between the GPS and the cellular modem.
- * In cases where TCXO is 32.736MHz or 16.368MHz, the FREF will be used
- * as the WLAN/BT main clock.
- */
-static int wl128x_switch_fref(struct wl1271 *wl, bool *is_ref_clk)
+static int wl128x_switch_tcxo_to_fref(struct wl1271 *wl)
{
- u16 sys_clk_cfg_val;
+ u16 spare_reg;
- /* if working on XTAL-only mode go directly to TCXO TO FREF SWITCH */
- if ((wl->ref_clock == CONF_REF_CLK_38_4_M_XTAL) ||
- (wl->ref_clock == CONF_REF_CLK_26_M_XTAL))
- return true;
+ /* Mask bits [2] & [8:4] in the sys_clk_cfg register */
+ spare_reg = wl1271_top_reg_read(wl, WL_SPARE_REG);
+ if (spare_reg == 0xFFFF)
+ return -EFAULT;
+ spare_reg |= (BIT(3) | BIT(5) | BIT(6));
+ wl1271_top_reg_write(wl, WL_SPARE_REG, spare_reg);
- /* Read clock source FREF or TCXO */
- sys_clk_cfg_val = wl1271_top_reg_read(wl, SYS_CLK_CFG_REG);
+ /* Enable FREF_CLK_REQ & mux MCS and coex PLLs to FREF */
+ wl1271_top_reg_write(wl, SYS_CLK_CFG_REG,
+ WL_CLK_REQ_TYPE_PG2 | MCS_PLL_CLK_SEL_FREF);
- if (sys_clk_cfg_val & PRCM_CM_EN_MUX_WLAN_FREF) {
- /* if bit 3 is set - working with FREF clock */
- wl1271_debug(DEBUG_BOOT, "working with FREF clock, skip"
- " to FREF");
+ /* Delay execution for 15msec, to let the HW settle */
+ mdelay(15);
- *is_ref_clk = true;
- } else {
- /* if bit 3 is clear - working with TCXO clock */
- wl1271_debug(DEBUG_BOOT, "working with TCXO clock");
-
- /* TCXO to FREF switch, check TXCO clock config */
- if ((wl->tcxo_clock != WL12XX_TCXOCLOCK_16_368) &&
- (wl->tcxo_clock != WL12XX_TCXOCLOCK_32_736)) {
- /*
- * not 16.368Mhz and not 32.736Mhz - skip to
- * configure ELP stage
- */
- wl1271_debug(DEBUG_BOOT, "NEW PLL ALGO:"
- " TcxoRefClk=%d - not 16.368Mhz and not"
- " 32.736Mhz - skip to configure ELP"
- " stage", wl->tcxo_clock);
-
- *is_ref_clk = false;
- } else {
- wl1271_debug(DEBUG_BOOT, "NEW PLL ALGO:"
- "TcxoRefClk=%d - 16.368Mhz or 32.736Mhz"
- " - TCXO to FREF switch",
- wl->tcxo_clock);
+ return 0;
+}
- return true;
- }
- }
+static bool wl128x_is_tcxo_valid(struct wl1271 *wl)
+{
+ u16 tcxo_detection;
+
+ tcxo_detection = wl1271_top_reg_read(wl, TCXO_CLK_DETECT_REG);
+ if (tcxo_detection & TCXO_DET_FAILED)
+ return false;
- return false;
+ return true;
}
-static int wl128x_boot_clk(struct wl1271 *wl, bool *is_ref_clk)
+static bool wl128x_is_fref_valid(struct wl1271 *wl)
{
- if (wl128x_switch_fref(wl, is_ref_clk)) {
- wl1271_debug(DEBUG_BOOT, "XTAL-only mode go directly to"
- " TCXO TO FREF SWITCH");
- /* TCXO to FREF switch - for PG2.0 */
- wl1271_top_reg_write(wl, WL_SPARE_REG,
- WL_SPARE_MASK_8526);
-
- wl1271_top_reg_write(wl, SYS_CLK_CFG_REG,
- WL_CLK_REQ_TYPE_PG2 | MCS_PLL_CLK_SEL_FREF);
-
- *is_ref_clk = true;
- mdelay(15);
- }
+ u16 fref_detection;
- /* Set bit 2 in spare register to avoid illegal access */
- wl1271_top_reg_write(wl, WL_SPARE_REG, WL_SPARE_VAL);
+ fref_detection = wl1271_top_reg_read(wl, FREF_CLK_DETECT_REG);
+ if (fref_detection & FREF_CLK_DETECT_FAIL)
+ return false;
- /* working with TCXO clock */
- if ((*is_ref_clk == false) &&
- ((wl->tcxo_clock == WL12XX_TCXOCLOCK_16_8) ||
- (wl->tcxo_clock == WL12XX_TCXOCLOCK_33_6))) {
- wl1271_debug(DEBUG_BOOT, "16_8_M or 33_6_M TCXO detected");
+ return true;
+}
- /* Manually Configure MCS PLL settings PG2.0 Only */
- wl1271_top_reg_write(wl, MCS_PLL_M_REG, MCS_PLL_M_REG_VAL);
- wl1271_top_reg_write(wl, MCS_PLL_N_REG, MCS_PLL_N_REG_VAL);
- wl1271_top_reg_write(wl, MCS_PLL_CONFIG_REG,
- MCS_PLL_CONFIG_REG_VAL);
- } else {
- int pll_config;
- u16 mcs_pll_config_val;
+static int wl128x_manually_configure_mcs_pll(struct wl1271 *wl)
+{
+ wl1271_top_reg_write(wl, MCS_PLL_M_REG, MCS_PLL_M_REG_VAL);
+ wl1271_top_reg_write(wl, MCS_PLL_N_REG, MCS_PLL_N_REG_VAL);
+ wl1271_top_reg_write(wl, MCS_PLL_CONFIG_REG, MCS_PLL_CONFIG_REG_VAL);
- /*
- * Configure MCS PLL settings to FREF Freq
- * Set the values that determine the time elapse since the PLL's
- * get their enable signal until the lock indication is set
- */
- wl1271_top_reg_write(wl, PLL_LOCK_COUNTERS_REG,
- PLL_LOCK_COUNTERS_COEX | PLL_LOCK_COUNTERS_MCS);
+ return 0;
+}
- mcs_pll_config_val = wl1271_top_reg_read(wl,
- MCS_PLL_CONFIG_REG);
- /*
- * Set the MCS PLL input frequency value according to the
- * reference clock value detected/read
- */
- if (*is_ref_clk == false) {
- if ((wl->tcxo_clock == WL12XX_TCXOCLOCK_19_2) ||
- (wl->tcxo_clock == WL12XX_TCXOCLOCK_38_4))
- pll_config = 1;
- else if ((wl->tcxo_clock == WL12XX_TCXOCLOCK_26)
- ||
- (wl->tcxo_clock == WL12XX_TCXOCLOCK_52))
- pll_config = 2;
- else
- return -EINVAL;
- } else {
- if ((wl->ref_clock == CONF_REF_CLK_19_2_E) ||
- (wl->ref_clock == CONF_REF_CLK_38_4_E))
- pll_config = 1;
- else if ((wl->ref_clock == CONF_REF_CLK_26_E) ||
- (wl->ref_clock == CONF_REF_CLK_52_E))
- pll_config = 2;
- else
- return -EINVAL;
- }
+static int wl128x_configure_mcs_pll(struct wl1271 *wl, int clk)
+{
+ u16 spare_reg;
+ u16 pll_config;
+ u8 input_freq;
+
+ /* Mask bits [3:1] in the sys_clk_cfg register */
+ spare_reg = wl1271_top_reg_read(wl, WL_SPARE_REG);
+ if (spare_reg == 0xFFFF)
+ return -EFAULT;
+ spare_reg |= BIT(2);
+ wl1271_top_reg_write(wl, WL_SPARE_REG, spare_reg);
+
+ /* Handle special cases of the TCXO clock */
+ if (wl->tcxo_clock == WL12XX_TCXOCLOCK_16_8 ||
+ wl->tcxo_clock == WL12XX_TCXOCLOCK_33_6)
+ return wl128x_manually_configure_mcs_pll(wl);
+
+ /* Set the input frequency according to the selected clock source */
+ input_freq = (clk & 1) + 1;
+
+ pll_config = wl1271_top_reg_read(wl, MCS_PLL_CONFIG_REG);
+ if (pll_config == 0xFFFF)
+ return -EFAULT;
+ pll_config |= (input_freq << MCS_SEL_IN_FREQ_SHIFT);
+ pll_config |= MCS_PLL_ENABLE_HP;
+ wl1271_top_reg_write(wl, MCS_PLL_CONFIG_REG, pll_config);
- mcs_pll_config_val |= (pll_config << (MCS_SEL_IN_FREQ_SHIFT)) &
- (MCS_SEL_IN_FREQ_MASK);
- wl1271_top_reg_write(wl, MCS_PLL_CONFIG_REG,
- mcs_pll_config_val);
+ return 0;
+}
+
+/*
+ * WL128x has two clocks input - TCXO and FREF.
+ * TCXO is the main clock of the device, while FREF is used to sync
+ * between the GPS and the cellular modem.
+ * In cases where TCXO is 32.736MHz or 16.368MHz, the FREF will be used
+ * as the WLAN/BT main clock.
+ */
+static int wl128x_boot_clk(struct wl1271 *wl, int *selected_clock)
+{
+ u16 sys_clk_cfg;
+
+ /* For XTAL-only modes, FREF will be used after switching from TCXO */
+ if (wl->ref_clock == WL12XX_REFCLOCK_26_XTAL ||
+ wl->ref_clock == WL12XX_REFCLOCK_38_XTAL) {
+ if (!wl128x_switch_tcxo_to_fref(wl))
+ return -EINVAL;
+ goto fref_clk;
}
- return 0;
+ /* Query the HW, to determine which clock source we should use */
+ sys_clk_cfg = wl1271_top_reg_read(wl, SYS_CLK_CFG_REG);
+ if (sys_clk_cfg == 0xFFFF)
+ return -EINVAL;
+ if (sys_clk_cfg & PRCM_CM_EN_MUX_WLAN_FREF)
+ goto fref_clk;
+
+ /* If TCXO is either 32.736MHz or 16.368MHz, switch to FREF */
+ if (wl->tcxo_clock == WL12XX_TCXOCLOCK_16_368 ||
+ wl->tcxo_clock == WL12XX_TCXOCLOCK_32_736) {
+ if (!wl128x_switch_tcxo_to_fref(wl))
+ return -EINVAL;
+ goto fref_clk;
+ }
+
+ /* TCXO clock is selected */
+ if (!wl128x_is_tcxo_valid(wl))
+ return -EINVAL;
+ *selected_clock = wl->tcxo_clock;
+ goto config_mcs_pll;
+
+fref_clk:
+ /* FREF clock is selected */
+ if (!wl128x_is_fref_valid(wl))
+ return -EINVAL;
+ *selected_clock = wl->ref_clock;
+
+config_mcs_pll:
+ return wl128x_configure_mcs_pll(wl, *selected_clock);
}
static int wl127x_boot_clk(struct wl1271 *wl)
@@ -713,10 +713,10 @@ int wl1271_load_firmware(struct wl1271 *wl)
{
int ret = 0;
u32 tmp, clk;
- bool is_ref_clk = false;
+ int selected_clock = -1;
if (wl->chip.id == CHIP_ID_1283_PG20) {
- ret = wl128x_boot_clk(wl, &is_ref_clk);
+ ret = wl128x_boot_clk(wl, &selected_clock);
if (ret < 0)
goto out;
} else {
@@ -741,10 +741,7 @@ int wl1271_load_firmware(struct wl1271 *wl)
wl1271_debug(DEBUG_BOOT, "clk2 0x%x", clk);
if (wl->chip.id == CHIP_ID_1283_PG20) {
- if (is_ref_clk == false)
- clk |= ((wl->tcxo_clock & 0x3) << 1) << 4;
- else
- clk |= ((wl->ref_clock & 0x3) << 1) << 4;
+ clk |= ((selected_clock & 0x3) << 1) << 4;
} else {
clk |= (wl->ref_clock << 1) << 4;
}
diff --git a/drivers/net/wireless/wl12xx/boot.h b/drivers/net/wireless/wl12xx/boot.h
index 1f5ee31..d9de64a 100644
--- a/drivers/net/wireless/wl12xx/boot.h
+++ b/drivers/net/wireless/wl12xx/boot.h
@@ -107,6 +107,7 @@ struct wl1271_static_data {
#define MCS_SEL_IN_FREQ_MASK 0x0070
#define MCS_SEL_IN_FREQ_SHIFT 4
#define MCS_PLL_CONFIG_REG_VAL 0x73
+#define MCS_PLL_ENABLE_HP (BIT(0) | BIT(1))
#define MCS_PLL_M_REG 0xD94
#define MCS_PLL_N_REG 0xD96
diff --git a/include/linux/wl12xx.h b/include/linux/wl12xx.h
index eb8aaca..c1a743e 100644
--- a/include/linux/wl12xx.h
+++ b/include/linux/wl12xx.h
@@ -26,10 +26,12 @@
/* Reference clock values */
enum {
- WL12XX_REFCLOCK_19 = 0, /* 19.2 MHz */
- WL12XX_REFCLOCK_26 = 1, /* 26 MHz */
- WL12XX_REFCLOCK_38 = 2, /* 38.4 MHz */
- WL12XX_REFCLOCK_54 = 3, /* 54 MHz */
+ WL12XX_REFCLOCK_19 = 0, /* 19.2 MHz */
+ WL12XX_REFCLOCK_26 = 1, /* 26 MHz */
+ WL12XX_REFCLOCK_38 = 2, /* 38.4 MHz */
+ WL12XX_REFCLOCK_52 = 3, /* 52 MHz */
+ WL12XX_REFCLOCK_38_XTAL = 4, /* 38.4 MHz, XTAL */
+ WL12XX_REFCLOCK_26_XTAL = 5, /* 26 MHz, XTAL */
};
/* TCXO clock values */
--
1.7.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/5] wl12xx: Clean up and fix the 128x boot sequence
2011-03-31 8:06 ` [PATCH 1/5] wl12xx: Clean up and fix the 128x boot sequence Ido Yariv
@ 2011-04-01 10:43 ` Luciano Coelho
0 siblings, 0 replies; 9+ messages in thread
From: Luciano Coelho @ 2011-04-01 10:43 UTC (permalink / raw)
To: Ido Yariv; +Cc: linux-wireless
On Thu, 2011-03-31 at 10:06 +0200, Ido Yariv wrote:
> Clean up the boot sequence code & fix the following issues:
> 1. Always read the registers' values and set the relevant bits instead of
> zeroing all other bits
> 2. Handle cases where wl1271_top_reg_read returns an error
> 3. Verify that the HW can detect the selected clock source
> 4. Remove 128x PG10 initialization code
> 5. Configure the MCS PLL to work in HP mode
>
> Signed-off-by: Ido Yariv <ido@wizery.com>
> ---
This looks nice! Much easier to read now. I checked it with the version
of the system initialization procedures that I got and it looks correct.
Except for some extra register reads in the two validation functions. I
think I don't have the latest version of the initialization document,
since you mentioned to Shahar that in the one you have those registers
are read.
Anyway, this is fine, as you mentioned, reading the registers shouldn't
cause any problems here.
Reviewed-by: Luciano Coelho <coelho@ti.com>
And applied, thanks!
A couple of small comments below...
> + /* Mask bits [2] & [8:4] in the sys_clk_cfg register */
> + spare_reg = wl1271_top_reg_read(wl, WL_SPARE_REG);
> + if (spare_reg == 0xFFFF)
> + return -EFAULT;
> + spare_reg |= (BIT(3) | BIT(5) | BIT(6));
> + wl1271_top_reg_write(wl, WL_SPARE_REG, spare_reg);
The comments of masking bits 2 and 8-4 is quite cryptic here. You
actually set bits 3, 5 and 6, so from the driver point of view the
comment doesn't make much sense. Since it's in the initialization
document, I guess it's something the firmware does internally. This
should be removed, but I'll keep it for now.
> + /* Mask bits [3:1] in the sys_clk_cfg register */
> + spare_reg = wl1271_top_reg_read(wl, WL_SPARE_REG);
> + if (spare_reg == 0xFFFF)
> + return -EFAULT;
> + spare_reg |= BIT(2);
> + wl1271_top_reg_write(wl, WL_SPARE_REG, spare_reg);
Same here, the comment is quite cryptic. Also, the "sys_clk_cfg
register" is wrong, it should be in the "spare register". In any case,
let's keep this comment for now, since it's a good way to map what is
going on with the system documentation. Let's remove them later (maybe
a patch to remove all such information that is still spread around the
driver?).
> + /* Set the input frequency according to the selected clock source */
> + input_freq = (clk & 1) + 1;
Nice trick! :) Maybe some comments about what actually happens here
would have been nice, though.
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/5] wl12xx: Clean up the block size alignment code
2011-03-31 8:06 [PATCH 0/5] wl12xx: Cleanups & fixes Ido Yariv
2011-03-31 8:06 ` [PATCH 1/5] wl12xx: Clean up and fix the 128x boot sequence Ido Yariv
@ 2011-03-31 8:06 ` Ido Yariv
2011-04-01 11:23 ` Luciano Coelho
2011-03-31 8:06 ` [PATCH 3/5] wl12xx: Clean up the dummy packet mechanism Ido Yariv
` (3 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Ido Yariv @ 2011-03-31 8:06 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless, Ido Yariv
Simplify and clean up the block size alignment code:
1. Set the block size according to the padding field type, as it cannot
exceed the maximum value this field can hold.
2. Move the alignment code into a function instead of duplicating it in
multiple places.
3. In the current implementation, the block_size member can be
misleading because a zero value actually means that there's no need to
align. Declare a block size alignment quirk instead.
Signed-off-by: Ido Yariv <ido@wizery.com>
---
drivers/net/wireless/wl12xx/init.c | 2 +-
drivers/net/wireless/wl12xx/io.c | 3 ++-
drivers/net/wireless/wl12xx/main.c | 4 ++--
drivers/net/wireless/wl12xx/sdio.c | 12 ++----------
drivers/net/wireless/wl12xx/tx.c | 26 ++++++++++++++------------
drivers/net/wireless/wl12xx/tx.h | 9 +++++++++
drivers/net/wireless/wl12xx/wl12xx.h | 10 ++++++----
7 files changed, 36 insertions(+), 30 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/init.c b/drivers/net/wireless/wl12xx/init.c
index 2dbc083..cf46607 100644
--- a/drivers/net/wireless/wl12xx/init.c
+++ b/drivers/net/wireless/wl12xx/init.c
@@ -514,7 +514,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 (wl1271_set_block_size(wl))
+ if (wl->quirks & WL12XX_QUIRK_BLOCKSIZE_ALIGNMENT)
/* Enable SDIO padding */
host_cfg_bitmap |= HOST_IF_CFG_TX_PAD_TO_SDIO_BLK;
diff --git a/drivers/net/wireless/wl12xx/io.c b/drivers/net/wireless/wl12xx/io.c
index aa40c98..da5c1ad 100644
--- a/drivers/net/wireless/wl12xx/io.c
+++ b/drivers/net/wireless/wl12xx/io.c
@@ -29,6 +29,7 @@
#include "wl12xx.h"
#include "wl12xx_80211.h"
#include "io.h"
+#include "tx.h"
#define OCP_CMD_LOOP 32
@@ -46,7 +47,7 @@
bool wl1271_set_block_size(struct wl1271 *wl)
{
if (wl->if_ops->set_block_size) {
- wl->if_ops->set_block_size(wl);
+ wl->if_ops->set_block_size(wl, WL12XX_BUS_BLOCK_SIZE);
return true;
}
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 85cb4da..960e993 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -1017,6 +1017,8 @@ 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_BLOCKSIZE_ALIGNMENT;
break;
case CHIP_ID_1283_PG10:
default:
@@ -1472,7 +1474,6 @@ static void __wl1271_op_remove_interface(struct wl1271 *wl)
memset(wl->ap_hlid_map, 0, sizeof(wl->ap_hlid_map));
wl->ap_fw_ps_map = 0;
wl->ap_ps_map = 0;
- wl->block_size = 0;
for (i = 0; i < NUM_TX_QUEUES; i++)
wl->tx_blocks_freed[i] = 0;
@@ -3602,7 +3603,6 @@ struct ieee80211_hw *wl1271_alloc_hw(void)
wl->ap_ps_map = 0;
wl->ap_fw_ps_map = 0;
wl->quirks = 0;
- wl->block_size = 0;
memset(wl->tx_frames_map, 0, sizeof(wl->tx_frames_map));
for (i = 0; i < ACX_TX_DESCRIPTORS; i++)
diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index 26dcd44..15efe0a 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -51,15 +51,10 @@ static const struct sdio_device_id wl1271_devices[] = {
};
MODULE_DEVICE_TABLE(sdio, wl1271_devices);
-/* The max SDIO block size is 256 when working with tx padding to SDIO block */
-#define TX_PAD_SDIO_BLK_SIZE 256
-
-static void wl1271_sdio_set_block_size(struct wl1271 *wl)
+static void wl1271_sdio_set_block_size(struct wl1271 *wl, unsigned int blksz)
{
- wl->block_size = TX_PAD_SDIO_BLK_SIZE;
-
sdio_claim_host(wl->if_priv);
- sdio_set_block_size(wl->if_priv, TX_PAD_SDIO_BLK_SIZE);
+ sdio_set_block_size(wl->if_priv, blksz);
sdio_release_host(wl->if_priv);
}
@@ -178,9 +173,6 @@ static int wl1271_sdio_power_on(struct wl1271 *wl)
sdio_claim_host(func);
sdio_enable_func(func);
- /* Set the default block size in case it was modified */
- sdio_set_block_size(func, 0);
-
out:
return ret;
}
diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
index db9e47e..d422f12 100644
--- a/drivers/net/wireless/wl12xx/tx.c
+++ b/drivers/net/wireless/wl12xx/tx.c
@@ -127,6 +127,15 @@ u8 wl1271_tx_get_hlid(struct sk_buff *skb)
}
}
+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
+ return ALIGN(packet_length, WL1271_TX_ALIGN_TO);
+}
+
static int wl1271_tx_allocate(struct wl1271 *wl, struct sk_buff *skb, u32 extra,
u32 buf_offset, u8 hlid)
{
@@ -152,10 +161,7 @@ static int wl1271_tx_allocate(struct wl1271 *wl, struct sk_buff *skb, u32 extra,
/* approximate the number of blocks required for this packet
in the firmware */
- if (wl->block_size)
- len = ALIGN(total_len, wl->block_size);
- else
- len = total_len;
+ len = wl12xx_calc_packet_alignment(wl, total_len);
total_blocks = (len + TX_HW_BLOCK_SIZE - 1) / TX_HW_BLOCK_SIZE +
spare_blocks;
@@ -269,9 +275,9 @@ static void wl1271_tx_fill_hdr(struct wl1271 *wl, struct sk_buff *skb,
tx_attr |= rate_idx << TX_HW_ATTR_OFST_RATE_POLICY;
desc->reserved = 0;
- if (wl->block_size) {
- aligned_len = ALIGN(skb->len, wl->block_size);
+ aligned_len = wl12xx_calc_packet_alignment(wl, skb->len);
+ if (wl->chip.id == CHIP_ID_1283_PG20) {
desc->wl128x_mem.extra_bytes = aligned_len - skb->len;
desc->length = cpu_to_le16(aligned_len >> 2);
@@ -284,8 +290,7 @@ static void wl1271_tx_fill_hdr(struct wl1271 *wl, struct sk_buff *skb,
} else {
int pad;
- /* align the length (and store in terms of words) */
- aligned_len = ALIGN(skb->len, WL1271_TX_ALIGN_TO);
+ /* Store the aligned length in terms of words */
desc->length = cpu_to_le16(aligned_len >> 2);
/* calculate number of padding bytes */
@@ -362,10 +367,7 @@ static int wl1271_prepare_tx_frame(struct wl1271 *wl, struct sk_buff *skb,
* In special cases, we want to align to a specific block size
* (eg. for wl128x with SDIO we align to 256).
*/
- if (wl->block_size)
- total_len = ALIGN(skb->len, wl->block_size);
- else
- total_len = ALIGN(skb->len, WL1271_TX_ALIGN_TO);
+ total_len = wl12xx_calc_packet_alignment(wl, skb->len);
memcpy(wl->aggr_buf + buf_offset, skb->data, skb->len);
memset(wl->aggr_buf + buf_offset + skb->len, 0, total_len - skb->len);
diff --git a/drivers/net/wireless/wl12xx/tx.h b/drivers/net/wireless/wl12xx/tx.h
index a3877ba..d6b05d9 100644
--- a/drivers/net/wireless/wl12xx/tx.h
+++ b/drivers/net/wireless/wl12xx/tx.h
@@ -89,6 +89,15 @@ struct wl128x_tx_mem {
u8 extra_bytes;
} __packed;
+/*
+ * On wl128x based devices, when TX packets are aggregated, each packet
+ * size must be aligned to the SDIO block size. The maximum block size
+ * is bounded by the type of the padded bytes field that is sent to the
+ * FW. Currently the type is u8, so the maximum block size is 256 bytes.
+ */
+#define WL12XX_BUS_BLOCK_SIZE min(512u, \
+ (1u << (8 * sizeof(((struct wl128x_tx_mem *) 0)->extra_bytes))))
+
struct wl1271_tx_hw_descr {
/* Length of packet in words, including descriptor+header+data */
__le16 length;
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index b04481a..14b8bc6 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -322,7 +322,7 @@ struct wl1271_if_operations {
struct device* (*dev)(struct wl1271 *wl);
void (*enable_irq)(struct wl1271 *wl);
void (*disable_irq)(struct wl1271 *wl);
- void (*set_block_size) (struct wl1271 *wl);
+ void (*set_block_size) (struct wl1271 *wl, unsigned int blksz);
};
#define MAX_NUM_KEYS 14
@@ -556,7 +556,6 @@ struct wl1271 {
bool ba_support;
u8 ba_rx_bitmap;
- u32 block_size;
int tcxo_clock;
/*
@@ -608,12 +607,15 @@ int wl1271_plt_stop(struct wl1271 *wl);
/* Quirks */
/* Each RX/TX transaction requires an end-of-transaction transfer */
-#define WL12XX_QUIRK_END_OF_TRANSACTION BIT(0)
+#define WL12XX_QUIRK_END_OF_TRANSACTION BIT(0)
/*
* Older firmwares use 2 spare TX blocks
* (for STA < 6.1.3.50.58 or for AP < 6.2.0.0.47)
*/
-#define WL12XX_QUIRK_USE_2_SPARE_BLOCKS BIT(1)
+#define WL12XX_QUIRK_USE_2_SPARE_BLOCKS BIT(1)
+
+/* WL128X requires aggregated packets to be aligned to the SDIO block size */
+#define WL12XX_QUIRK_BLOCKSIZE_ALIGNMENT BIT(2)
#endif
--
1.7.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/5] wl12xx: Clean up the block size alignment code
2011-03-31 8:06 ` [PATCH 2/5] wl12xx: Clean up the block size alignment code Ido Yariv
@ 2011-04-01 11:23 ` Luciano Coelho
0 siblings, 0 replies; 9+ messages in thread
From: Luciano Coelho @ 2011-04-01 11:23 UTC (permalink / raw)
To: Ido Yariv; +Cc: linux-wireless
On Thu, 2011-03-31 at 10:06 +0200, Ido Yariv wrote:
> Simplify and clean up the block size alignment code:
> 1. Set the block size according to the padding field type, as it cannot
> exceed the maximum value this field can hold.
> 2. Move the alignment code into a function instead of duplicating it in
> multiple places.
> 3. In the current implementation, the block_size member can be
> misleading because a zero value actually means that there's no need to
> align. Declare a block size alignment quirk instead.
>
> Signed-off-by: Ido Yariv <ido@wizery.com>
> ---
Applied, thanks!
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/5] wl12xx: Clean up the dummy packet mechanism
2011-03-31 8:06 [PATCH 0/5] wl12xx: Cleanups & fixes Ido Yariv
2011-03-31 8:06 ` [PATCH 1/5] wl12xx: Clean up and fix the 128x boot sequence Ido Yariv
2011-03-31 8:06 ` [PATCH 2/5] wl12xx: Clean up the block size alignment code Ido Yariv
@ 2011-03-31 8:06 ` Ido Yariv
2011-03-31 8:07 ` [PATCH 4/5] wl12xx: Simplify TX blocks accounting Ido Yariv
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Ido Yariv @ 2011-03-31 8:06 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless, Ido Yariv
The current implementation allocates a skb each time one is requested by
the firmware. Since dummy packets are handled differently than regular
packets, the skb needs to be marked. Currently, this is done by
setting the pkt_type member to 5. This might not be safe, as we cannot
be sure that there won't be any other packets with this pkt_type value.
Since the packet does not change from one request to another, we can
simply allocate a dummy packet template and always send it. All changes
to the skb done during packet preparation must be reverted, so the same
skb can be reused.
The dummy packets are not transmitted, therefore there's no need to set
the BSSID or our own MAC address.
In addition, the header portion of the packet was zeroed by mistake, so
fix that as well.
Signed-off-by: Ido Yariv <ido@wizery.com>
---
drivers/net/wireless/wl12xx/main.c | 79 +++++++++++++++++++++++-----------
drivers/net/wireless/wl12xx/tx.c | 43 +++++++++++++-----
drivers/net/wireless/wl12xx/tx.h | 2 -
drivers/net/wireless/wl12xx/wl12xx.h | 6 ++-
4 files changed, 90 insertions(+), 40 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 960e993..dea26a9 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -1212,20 +1212,46 @@ static void wl1271_op_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
spin_unlock_irqrestore(&wl->wl_lock, flags);
}
-#define TX_DUMMY_PACKET_SIZE 1400
int wl1271_tx_dummy_packet(struct wl1271 *wl)
{
- struct sk_buff *skb = NULL;
+ unsigned long flags;
+
+ spin_lock_irqsave(&wl->wl_lock, flags);
+ set_bit(WL1271_FLAG_DUMMY_PACKET_PENDING, &wl->flags);
+ wl->tx_queue_count++;
+ spin_unlock_irqrestore(&wl->wl_lock, flags);
+
+ /* The FW is low on RX memory blocks, so send the dummy packet asap */
+ if (!test_bit(WL1271_FLAG_FW_TX_BUSY, &wl->flags))
+ wl1271_tx_work_locked(wl);
+
+ /*
+ * If the FW TX is busy, TX work will be scheduled by the threaded
+ * interrupt handler function
+ */
+ return 0;
+}
+
+/*
+ * The size of the dummy packet should be at least 1400 bytes. However, in
+ * order to minimize the number of bus transactions, aligning it to 512 bytes
+ * boundaries could be beneficial, performance wise
+ */
+#define TOTAL_TX_DUMMY_PACKET_SIZE (ALIGN(1400, 512))
+
+struct sk_buff *wl12xx_alloc_dummy_packet(struct wl1271 *wl)
+{
+ struct sk_buff *skb;
struct ieee80211_hdr_3addr *hdr;
- int ret = 0;
+ unsigned int dummy_packet_size;
+
+ dummy_packet_size = TOTAL_TX_DUMMY_PACKET_SIZE -
+ sizeof(struct wl1271_tx_hw_descr) - sizeof(*hdr);
- skb = dev_alloc_skb(
- sizeof(struct wl1271_tx_hw_descr) + sizeof(*hdr) +
- TX_DUMMY_PACKET_SIZE);
+ skb = dev_alloc_skb(TOTAL_TX_DUMMY_PACKET_SIZE);
if (!skb) {
- wl1271_warning("failed to allocate buffer for dummy packet");
- ret = -ENOMEM;
- goto out;
+ wl1271_warning("Failed to allocate a dummy packet skb");
+ return NULL;
}
skb_reserve(skb, sizeof(struct wl1271_tx_hw_descr));
@@ -1233,29 +1259,22 @@ int wl1271_tx_dummy_packet(struct wl1271 *wl)
hdr = (struct ieee80211_hdr_3addr *) skb_put(skb, sizeof(*hdr));
memset(hdr, 0, sizeof(*hdr));
hdr->frame_control = cpu_to_le16(IEEE80211_FTYPE_DATA |
- IEEE80211_FCTL_TODS |
- IEEE80211_STYPE_NULLFUNC);
-
- memcpy(hdr->addr1, wl->bssid, ETH_ALEN);
- memcpy(hdr->addr2, wl->mac_addr, ETH_ALEN);
- memcpy(hdr->addr3, wl->bssid, ETH_ALEN);
+ IEEE80211_STYPE_NULLFUNC |
+ IEEE80211_FCTL_TODS);
- skb_put(skb, TX_DUMMY_PACKET_SIZE);
+ memset(skb_put(skb, dummy_packet_size), 0, dummy_packet_size);
- memset(skb->data, 0, TX_DUMMY_PACKET_SIZE);
-
- skb->pkt_type = TX_PKT_TYPE_DUMMY_REQ;
/* Dummy packets require the TID to be management */
skb->priority = WL1271_TID_MGMT;
- /* CONF_TX_AC_VO */
- skb->queue_mapping = 0;
- wl1271_op_tx(wl->hw, skb);
+ /* Initialize all fields that might be used */
+ skb->queue_mapping = 0;
+ memset(IEEE80211_SKB_CB(skb), 0, sizeof(struct ieee80211_tx_info));
-out:
- return ret;
+ return skb;
}
+
static struct notifier_block wl1271_dev_notifier = {
.notifier_call = wl1271_dev_notify,
};
@@ -3623,11 +3642,17 @@ struct ieee80211_hw *wl1271_alloc_hw(void)
goto err_hw;
}
+ wl->dummy_packet = wl12xx_alloc_dummy_packet(wl);
+ if (!wl->dummy_packet) {
+ ret = -ENOMEM;
+ goto err_aggr;
+ }
+
/* Register platform device */
ret = platform_device_register(wl->plat_dev);
if (ret) {
wl1271_error("couldn't register platform device");
- goto err_aggr;
+ goto err_dummy_packet;
}
dev_set_drvdata(&wl->plat_dev->dev, wl);
@@ -3653,6 +3678,9 @@ err_bt_coex_state:
err_platform:
platform_device_unregister(wl->plat_dev);
+err_dummy_packet:
+ dev_kfree_skb(wl->dummy_packet);
+
err_aggr:
free_pages((unsigned long)wl->aggr_buf, order);
@@ -3672,6 +3700,7 @@ EXPORT_SYMBOL_GPL(wl1271_alloc_hw);
int wl1271_free_hw(struct wl1271 *wl)
{
platform_device_unregister(wl->plat_dev);
+ dev_kfree_skb(wl->dummy_packet);
free_pages((unsigned long)wl->aggr_buf,
get_order(WL1271_AGGR_BUFFER_SIZE));
kfree(wl->plat_dev);
diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
index d422f12..18d9655 100644
--- a/drivers/net/wireless/wl12xx/tx.c
+++ b/drivers/net/wireless/wl12xx/tx.c
@@ -197,6 +197,11 @@ static int wl1271_tx_allocate(struct wl1271 *wl, struct sk_buff *skb, u32 extra,
return ret;
}
+static bool wl12xx_is_dummy_packet(struct wl1271 *wl, struct sk_buff *skb)
+{
+ return wl->dummy_packet == skb;
+}
+
static void wl1271_tx_fill_hdr(struct wl1271 *wl, struct sk_buff *skb,
u32 extra, struct ieee80211_tx_info *control,
u8 hlid)
@@ -231,7 +236,7 @@ static void wl1271_tx_fill_hdr(struct wl1271 *wl, struct sk_buff *skb,
ac = wl1271_tx_get_queue(skb_get_queue_mapping(skb));
desc->tid = skb->priority;
- if (skb->pkt_type == TX_PKT_TYPE_DUMMY_REQ) {
+ if (wl12xx_is_dummy_packet(wl, skb)) {
/*
* FW expects the dummy packet to have an invalid session id -
* any session id that is different than the one set in the join
@@ -372,6 +377,10 @@ static int wl1271_prepare_tx_frame(struct wl1271 *wl, struct sk_buff *skb,
memcpy(wl->aggr_buf + buf_offset, skb->data, skb->len);
memset(wl->aggr_buf + buf_offset + skb->len, 0, total_len - skb->len);
+ /* Revert side effects in the dummy packet skb, so it can be reused */
+ if (wl12xx_is_dummy_packet(wl, skb))
+ skb_pull(skb, sizeof(struct wl1271_tx_hw_descr));
+
return total_len;
}
@@ -484,10 +493,23 @@ out:
static struct sk_buff *wl1271_skb_dequeue(struct wl1271 *wl)
{
+ unsigned long flags;
+ struct sk_buff *skb = NULL;
+
if (wl->bss_type == BSS_TYPE_AP_BSS)
- return wl1271_ap_skb_dequeue(wl);
+ skb = wl1271_ap_skb_dequeue(wl);
+ else
+ skb = wl1271_sta_skb_dequeue(wl);
- return wl1271_sta_skb_dequeue(wl);
+ if (!skb &&
+ test_and_clear_bit(WL1271_FLAG_DUMMY_PACKET_PENDING, &wl->flags)) {
+ skb = wl->dummy_packet;
+ spin_lock_irqsave(&wl->wl_lock, flags);
+ wl->tx_queue_count--;
+ spin_unlock_irqrestore(&wl->wl_lock, flags);
+ }
+
+ return skb;
}
static void wl1271_skb_queue_head(struct wl1271 *wl, struct sk_buff *skb)
@@ -495,7 +517,9 @@ static void wl1271_skb_queue_head(struct wl1271 *wl, struct sk_buff *skb)
unsigned long flags;
int q = wl1271_tx_get_queue(skb_get_queue_mapping(skb));
- if (wl->bss_type == BSS_TYPE_AP_BSS) {
+ if (wl12xx_is_dummy_packet(wl, skb)) {
+ set_bit(WL1271_FLAG_DUMMY_PACKET_PENDING, &wl->flags);
+ } else if (wl->bss_type == BSS_TYPE_AP_BSS) {
u8 hlid = wl1271_tx_get_hlid(skb);
skb_queue_head(&wl->links[hlid].tx_queue[q], skb);
@@ -608,8 +632,7 @@ static void wl1271_tx_complete_packet(struct wl1271 *wl,
skb = wl->tx_frames[id];
info = IEEE80211_SKB_CB(skb);
- if (skb->pkt_type == TX_PKT_TYPE_DUMMY_REQ) {
- dev_kfree_skb(skb);
+ if (wl12xx_is_dummy_packet(wl, skb)) {
wl1271_free_tx_id(wl, id);
return;
}
@@ -744,9 +767,7 @@ void wl1271_tx_reset(struct wl1271 *wl)
wl1271_debug(DEBUG_TX, "freeing skb 0x%p",
skb);
- if (skb->pkt_type == TX_PKT_TYPE_DUMMY_REQ) {
- dev_kfree_skb(skb);
- } else {
+ if (!wl12xx_is_dummy_packet(wl, skb)) {
info = IEEE80211_SKB_CB(skb);
info->status.rates[0].idx = -1;
info->status.rates[0].count = 0;
@@ -772,9 +793,7 @@ void wl1271_tx_reset(struct wl1271 *wl)
wl1271_free_tx_id(wl, i);
wl1271_debug(DEBUG_TX, "freeing skb 0x%p", skb);
- if (skb->pkt_type == TX_PKT_TYPE_DUMMY_REQ) {
- dev_kfree_skb(skb);
- } else {
+ if (!wl12xx_is_dummy_packet(wl, skb)) {
/*
* Remove private headers before passing the skb to
* mac80211
diff --git a/drivers/net/wireless/wl12xx/tx.h b/drivers/net/wireless/wl12xx/tx.h
index d6b05d9..fc7835c 100644
--- a/drivers/net/wireless/wl12xx/tx.h
+++ b/drivers/net/wireless/wl12xx/tx.h
@@ -42,8 +42,6 @@
#define TX_HW_ATTR_TX_CMPLT_REQ BIT(12)
#define TX_HW_ATTR_TX_DUMMY_REQ BIT(13)
-#define TX_PKT_TYPE_DUMMY_REQ 5
-
#define TX_HW_ATTR_OFST_SAVE_RETRIES 0
#define TX_HW_ATTR_OFST_HEADER_PAD 1
#define TX_HW_ATTR_OFST_SESSION_COUNTER 2
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index 14b8bc6..87e96ba 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -353,7 +353,8 @@ enum wl12xx_flags {
WL1271_FLAG_PSPOLL_FAILURE,
WL1271_FLAG_STA_STATE_SENT,
WL1271_FLAG_FW_TX_BUSY,
- WL1271_FLAG_AP_STARTED
+ WL1271_FLAG_AP_STARTED,
+ WL1271_FLAG_DUMMY_PACKET_PENDING,
};
struct wl1271_link {
@@ -459,6 +460,9 @@ struct wl1271 {
/* Intermediate buffer, used for packet aggregation */
u8 *aggr_buf;
+ /* Reusable dummy packet template */
+ struct sk_buff *dummy_packet;
+
/* Network stack work */
struct work_struct netstack_work;
--
1.7.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/5] wl12xx: Simplify TX blocks accounting
2011-03-31 8:06 [PATCH 0/5] wl12xx: Cleanups & fixes Ido Yariv
` (2 preceding siblings ...)
2011-03-31 8:06 ` [PATCH 3/5] wl12xx: Clean up the dummy packet mechanism Ido Yariv
@ 2011-03-31 8:07 ` Ido Yariv
2011-03-31 8:07 ` [PATCH 5/5] wl12xx: Handle platforms without level trigger interrupts Ido Yariv
2011-04-01 11:30 ` [PATCH 0/5] wl12xx: Cleanups & fixes Luciano Coelho
5 siblings, 0 replies; 9+ messages in thread
From: Ido Yariv @ 2011-03-31 8:07 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless, Ido Yariv, Arik Nemtsov
The total number of TX memory blocks may change when the dynamic memory
option is enabled. The current implementation only tracks the available
memory blocks, which over-complicates TX blocks accounting.
By tracking the number of allocated blocks, calculation of the number of
available blocks becomes simpler and cleaner. It simply equals the total
number of TX memory blocks minus the allocated ones.
Also, remove some unnecessary castings and use union member accesses
instead.
Signed-off-by: Ido Yariv <ido@wizery.com>
Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
drivers/net/wireless/wl12xx/acx.c | 2 -
drivers/net/wireless/wl12xx/main.c | 48 +++++++++++++++------------------
drivers/net/wireless/wl12xx/tx.c | 1 +
drivers/net/wireless/wl12xx/wl12xx.h | 4 +--
4 files changed, 24 insertions(+), 31 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/acx.c b/drivers/net/wireless/wl12xx/acx.c
index e005aa4..b277947 100644
--- a/drivers/net/wireless/wl12xx/acx.c
+++ b/drivers/net/wireless/wl12xx/acx.c
@@ -1081,8 +1081,6 @@ int wl1271_acx_init_mem_config(struct wl1271 *wl)
wl1271_debug(DEBUG_TX, "available tx blocks: %d",
wl->tx_blocks_available);
- wl->tx_new_total = wl->tx_blocks_available;
-
return 0;
}
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index dea26a9..f1c9941 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -622,7 +622,7 @@ static void wl1271_fw_status(struct wl1271 *wl,
struct wl1271_fw_common_status *status = &full_status->common;
struct timespec ts;
u32 old_tx_blk_count = wl->tx_blocks_available;
- u32 total = 0;
+ u32 freed_blocks = 0;
int i;
if (wl->bss_type == BSS_TYPE_AP_BSS) {
@@ -631,15 +631,6 @@ static void wl1271_fw_status(struct wl1271 *wl,
} else {
wl1271_raw_read(wl, FW_STATUS_ADDR, status,
sizeof(struct wl1271_fw_sta_status), false);
-
- /* Update tx total blocks change */
- wl->tx_total_diff +=
- ((struct wl1271_fw_sta_status *)status)->tx_total -
- wl->tx_new_total;
-
- /* Update total tx blocks */
- wl->tx_new_total =
- ((struct wl1271_fw_sta_status *)status)->tx_total;
}
wl1271_debug(DEBUG_IRQ, "intr: 0x%x (fw_rx_counter = %d, "
@@ -651,34 +642,38 @@ static void wl1271_fw_status(struct wl1271 *wl,
/* update number of available TX blocks */
for (i = 0; i < NUM_TX_QUEUES; i++) {
- total += le32_to_cpu(status->tx_released_blks[i]) -
- wl->tx_blocks_freed[i];
+ freed_blocks += le32_to_cpu(status->tx_released_blks[i]) -
+ wl->tx_blocks_freed[i];
wl->tx_blocks_freed[i] =
le32_to_cpu(status->tx_released_blks[i]);
-
}
- /*
- * By adding the freed blocks to tx_total_diff we are actually
- * moving them to the RX pool.
- */
- wl->tx_total_diff += total;
+ wl->tx_allocated_blocks -= freed_blocks;
+
+ if (wl->bss_type == BSS_TYPE_AP_BSS) {
+ /* Update num of allocated TX blocks per link and ps status */
+ wl1271_irq_update_links_status(wl, &full_status->ap);
+ wl->tx_blocks_available += freed_blocks;
+ } else {
+ int avail = full_status->sta.tx_total - wl->tx_allocated_blocks;
- /* if we have positive difference, add the blocks to the TX pool */
- if (wl->tx_total_diff >= 0) {
- wl->tx_blocks_available += wl->tx_total_diff;
- wl->tx_total_diff = 0;
+ /*
+ * The FW might change the total number of TX memblocks before
+ * we get a notification about blocks being released. Thus, the
+ * available blocks calculation might yield a temporary result
+ * which is lower than the actual available blocks. Keeping in
+ * mind that only blocks that were allocated can be moved from
+ * TX to RX, tx_blocks_available should never decrease here.
+ */
+ wl->tx_blocks_available = max((int)wl->tx_blocks_available,
+ avail);
}
/* if more blocks are available now, tx work can be scheduled */
if (wl->tx_blocks_available > old_tx_blk_count)
clear_bit(WL1271_FLAG_FW_TX_BUSY, &wl->flags);
- /* for AP update num of allocated TX blocks per link and ps status */
- if (wl->bss_type == BSS_TYPE_AP_BSS)
- wl1271_irq_update_links_status(wl, &full_status->ap);
-
/* update the host-chipset time offset */
getnstimeofday(&ts);
wl->time_offset = (timespec_to_ns(&ts) >> 10) -
@@ -1479,6 +1474,7 @@ static void __wl1271_op_remove_interface(struct wl1271 *wl)
wl->psm_entry_retry = 0;
wl->power_level = WL1271_DEFAULT_POWER_LEVEL;
wl->tx_blocks_available = 0;
+ wl->tx_allocated_blocks = 0;
wl->tx_results_count = 0;
wl->tx_packets_count = 0;
wl->tx_security_last_seq = 0;
diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
index 18d9655..f0abb71 100644
--- a/drivers/net/wireless/wl12xx/tx.c
+++ b/drivers/net/wireless/wl12xx/tx.c
@@ -181,6 +181,7 @@ static int wl1271_tx_allocate(struct wl1271 *wl, struct sk_buff *skb, u32 extra,
desc->id = id;
wl->tx_blocks_available -= total_blocks;
+ wl->tx_allocated_blocks += total_blocks;
if (wl->bss_type == BSS_TYPE_AP_BSS)
wl->links[hlid].allocated_blks += total_blocks;
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index 87e96ba..34b03eb 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -416,10 +416,8 @@ struct wl1271 {
/* Accounting for allocated / available TX blocks on HW */
u32 tx_blocks_freed[NUM_TX_QUEUES];
u32 tx_blocks_available;
+ u32 tx_allocated_blocks;
u32 tx_results_count;
- /* Indicates how many memory blocks should be moved to the RX pool */
- int tx_total_diff;
- u32 tx_new_total;
/* Transmitted TX packets counter for chipset interface */
u32 tx_packets_count;
--
1.7.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/5] wl12xx: Handle platforms without level trigger interrupts
2011-03-31 8:06 [PATCH 0/5] wl12xx: Cleanups & fixes Ido Yariv
` (3 preceding siblings ...)
2011-03-31 8:07 ` [PATCH 4/5] wl12xx: Simplify TX blocks accounting Ido Yariv
@ 2011-03-31 8:07 ` Ido Yariv
2011-04-01 11:30 ` [PATCH 0/5] wl12xx: Cleanups & fixes Luciano Coelho
5 siblings, 0 replies; 9+ messages in thread
From: Ido Yariv @ 2011-03-31 8:07 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless, Ido Yariv
Some platforms are incapable of triggering on level interrupts. Add a
platform quirks member in the platform data structure, as well as an
edge interrupt quirk which can be set on such platforms.
When the interrupt is requested with IRQF_TRIGGER_RISING, IRQF_ONESHOT
cannot be used, as we might miss interrupts that occur after the FW
status is cleared and before the threaded interrupt handler exits.
Moreover, when IRQF_ONESHOT is not set, iterating more than once in the
threaded interrupt handler introduces a few race conditions between this
handler and the hardirq handler. Currently this is worked around by
limiting the loop to one iteration only. This workaround has an impact
on performance. To remove to this restriction, the race conditions will
need to be addressed.
Signed-off-by: Ido Yariv <ido@wizery.com>
---
drivers/net/wireless/wl12xx/main.c | 9 +++++++++
drivers/net/wireless/wl12xx/sdio.c | 9 ++++++++-
drivers/net/wireless/wl12xx/spi.c | 9 ++++++++-
drivers/net/wireless/wl12xx/wl12xx.h | 3 +++
include/linux/wl12xx.h | 4 ++++
5 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index f1c9941..ccc468a 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -30,6 +30,7 @@
#include <linux/vmalloc.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
+#include <linux/wl12xx.h>
#include "wl12xx.h"
#include "wl12xx_80211.h"
@@ -719,6 +720,13 @@ irqreturn_t wl1271_irq(int irq, void *cookie)
set_bit(WL1271_FLAG_TX_PENDING, &wl->flags);
cancel_work_sync(&wl->tx_work);
+ /*
+ * In case edge triggered interrupt must be used, we cannot iterate
+ * more than once without introducing race conditions with the hardirq.
+ */
+ if (wl->platform_quirks & WL12XX_PLATFORM_QUIRK_EDGE_IRQ)
+ loopcount = 1;
+
mutex_lock(&wl->mutex);
wl1271_debug(DEBUG_IRQ, "IRQ work");
@@ -3618,6 +3626,7 @@ struct ieee80211_hw *wl1271_alloc_hw(void)
wl->ap_ps_map = 0;
wl->ap_fw_ps_map = 0;
wl->quirks = 0;
+ wl->platform_quirks = 0;
memset(wl->tx_frames_map, 0, sizeof(wl->tx_frames_map));
for (i = 0; i < ACX_TX_DESCRIPTORS; i++)
diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index 15efe0a..82844aa 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -220,6 +220,7 @@ static int __devinit wl1271_probe(struct sdio_func *func,
struct ieee80211_hw *hw;
const struct wl12xx_platform_data *wlan_data;
struct wl1271 *wl;
+ unsigned long irqflags;
int ret;
/* We are only able to handle the wlan function */
@@ -251,9 +252,15 @@ static int __devinit wl1271_probe(struct sdio_func *func,
wl->irq = wlan_data->irq;
wl->ref_clock = wlan_data->board_ref_clock;
wl->tcxo_clock = wlan_data->board_tcxo_clock;
+ wl->platform_quirks = wlan_data->platform_quirks;
+
+ if (wl->platform_quirks & WL12XX_PLATFORM_QUIRK_EDGE_IRQ)
+ irqflags = IRQF_TRIGGER_RISING;
+ else
+ irqflags = IRQF_TRIGGER_HIGH | IRQF_ONESHOT;
ret = request_threaded_irq(wl->irq, wl1271_hardirq, wl1271_irq,
- IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ irqflags,
DRIVER_NAME, wl);
if (ret < 0) {
wl1271_error("request_irq() failed: %d", ret);
diff --git a/drivers/net/wireless/wl12xx/spi.c b/drivers/net/wireless/wl12xx/spi.c
index 3b862a2..78c3ce6 100644
--- a/drivers/net/wireless/wl12xx/spi.c
+++ b/drivers/net/wireless/wl12xx/spi.c
@@ -364,6 +364,7 @@ static int __devinit wl1271_probe(struct spi_device *spi)
struct wl12xx_platform_data *pdata;
struct ieee80211_hw *hw;
struct wl1271 *wl;
+ unsigned long irqflags;
int ret;
pdata = spi->dev.platform_data;
@@ -402,6 +403,12 @@ static int __devinit wl1271_probe(struct spi_device *spi)
wl->ref_clock = pdata->board_ref_clock;
wl->tcxo_clock = pdata->board_tcxo_clock;
+ wl->platform_quirks = pdata->platform_quirks;
+
+ if (wl->platform_quirks & WL12XX_PLATFORM_QUIRK_EDGE_IRQ)
+ irqflags = IRQF_TRIGGER_RISING;
+ else
+ irqflags = IRQF_TRIGGER_HIGH | IRQF_ONESHOT;
wl->irq = spi->irq;
if (wl->irq < 0) {
@@ -411,7 +418,7 @@ static int __devinit wl1271_probe(struct spi_device *spi)
}
ret = request_threaded_irq(wl->irq, wl1271_hardirq, wl1271_irq,
- IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ irqflags,
DRIVER_NAME, wl);
if (ret < 0) {
wl1271_error("request_irq() failed: %d", ret);
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index 34b03eb..4137b1d 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -577,6 +577,9 @@ struct wl1271 {
/* Quirks of specific hardware revisions */
unsigned int quirks;
+
+ /* Platform limitations */
+ unsigned int platform_quirks;
};
struct wl1271_station {
diff --git a/include/linux/wl12xx.h b/include/linux/wl12xx.h
index c1a743e..4b69739 100644
--- a/include/linux/wl12xx.h
+++ b/include/linux/wl12xx.h
@@ -53,8 +53,12 @@ struct wl12xx_platform_data {
bool use_eeprom;
int board_ref_clock;
int board_tcxo_clock;
+ unsigned long platform_quirks;
};
+/* Platform does not support level trigger interrupts */
+#define WL12XX_PLATFORM_QUIRK_EDGE_IRQ BIT(0)
+
#ifdef CONFIG_WL12XX_PLATFORM_DATA
int wl12xx_set_platform_data(const struct wl12xx_platform_data *data);
--
1.7.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 0/5] wl12xx: Cleanups & fixes
2011-03-31 8:06 [PATCH 0/5] wl12xx: Cleanups & fixes Ido Yariv
` (4 preceding siblings ...)
2011-03-31 8:07 ` [PATCH 5/5] wl12xx: Handle platforms without level trigger interrupts Ido Yariv
@ 2011-04-01 11:30 ` Luciano Coelho
5 siblings, 0 replies; 9+ messages in thread
From: Luciano Coelho @ 2011-04-01 11:30 UTC (permalink / raw)
To: Ido Yariv; +Cc: linux-wireless
On Thu, 2011-03-31 at 10:06 +0200, Ido Yariv wrote:
> The following patches mostly clean up code, but also fix a few minor bugs.
>
> The first four patches fix a few issues in the recently added 128x support.
> The last patch fixes a regression in the driver, preventing it from working
> properly on platforms which do not support level triggered interrupts.
>
> Ido Yariv (5):
> wl12xx: Clean up and fix the 128x boot sequence
> wl12xx: Clean up the block size alignment code
> wl12xx: Clean up the dummy packet mechanism
> wl12xx: Simplify TX blocks accounting
> wl12xx: Handle platforms without level trigger interrupts
>
> drivers/net/wireless/wl12xx/acx.c | 2 -
> drivers/net/wireless/wl12xx/boot.c | 235 +++++++++++++++++-----------------
> drivers/net/wireless/wl12xx/boot.h | 1 +
> drivers/net/wireless/wl12xx/init.c | 2 +-
> drivers/net/wireless/wl12xx/io.c | 3 +-
> drivers/net/wireless/wl12xx/main.c | 140 +++++++++++++--------
> drivers/net/wireless/wl12xx/sdio.c | 21 ++--
> drivers/net/wireless/wl12xx/spi.c | 9 +-
> drivers/net/wireless/wl12xx/tx.c | 70 +++++++----
> drivers/net/wireless/wl12xx/tx.h | 11 ++-
> drivers/net/wireless/wl12xx/wl12xx.h | 23 ++-
> include/linux/wl12xx.h | 14 ++-
> 12 files changed, 305 insertions(+), 226 deletions(-)
Applied the 3 remaining patches of the series too, so all this patchset
has been applied. Thanks!
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 9+ messages in thread