[PATCH 0/6] ARM: tegra: enable wlan for t20 and t30

Enable wlan for following tegra board:
Tegra30: Cardhu.
Tegra20: Seaboard, Ventana.

Wei Ni (6):
  ARM: tegra: set up wlan clocks for tegra dt
  brcmfmac: Handling the interrupt in ISR directly for non-OOB
  ARM: dt: t20 seaboard: turn on the power for wlan
  ARM: dt: t20 ventana: set pinmux and power for wlan
  ARM: dt: t30 cardhu: set pinmux and power for wlan
  ARM: tegra: enable wireless in defconfig

 arch/arm/boot/dts/tegra20-seaboard.dts             |    5 +++
 arch/arm/boot/dts/tegra20-ventana.dts              |   15 +++++++++
 arch/arm/boot/dts/tegra30-cardhu.dtsi              |   32 ++++++++++++++++++++
 arch/arm/configs/tegra_defconfig                   |    4 ++
 arch/arm/mach-tegra/board-dt-tegra20.c             |    4 ++
 arch/arm/mach-tegra/board-dt-tegra30.c             |    4 ++
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c   |    2 +
 drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |    8 ++++-
 8 files changed, 73 insertions(+), 1 deletions(-)

In case of inband interrupts, if we handle the interrupt in dpc thread,
two level of thread switching takes place to process wifi interrupts.
One in SDHCI driver and the other in Wifi driver. This may cause the system
instability.
Because the SDHCI calls sdio_irq_thread() to handle the irq, this thread locks
mmc host and calls wifi handler. It expects WiFi handler to be quick and
enables sdio interrupt from card at end. If wifi handler defers this work for
a different thread, sdio_irq_thread() will be stuck on next wifi interrupt
since mmc lock is not freed.

Handling the interrupt in ISR directly will prevent thread context switching in
wifi driver. It can fix the instability problems.

Signed-off-by: Wei Ni <redacted>
---
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c   |    2 ++
 drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |    8 +++++++-
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 8e7e692..5cf6c3b 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c

@@ -121,7 +121,9 @@ static void brcmf_sdio_irqhandler(struct sdio_func *func)
 
 	brcmf_dbg(INTR, "ib intr triggered\n");
 
+	sdio_release_host(sdiodev->func[1]);
 	brcmf_sdbrcm_isr(sdiodev->bus);
+	sdio_claim_host(sdiodev->func[1]);
 }
 
 /* dummy handler for SDIO function 2 interrupt */

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
index 472f2ef..4576d59 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c

@@ -2347,7 +2347,7 @@ static bool brcmf_sdbrcm_dpc(struct brcmf_sdio *bus)
 	uint framecnt = 0;	/* Temporary counter of tx/rx frames */
 	bool rxdone = true;	/* Flag for no more read data */
 	bool resched = false;	/* Flag indicating resched wanted */
-	int err;
+	int err = 0;
 
 	brcmf_dbg(TRACE, "Enter\n");
 

@@ -3786,11 +3786,17 @@ void brcmf_sdbrcm_isr(void *arg)
 	if (!bus->intr)
 		brcmf_dbg(ERROR, "isr w/o interrupt configured!\n");
 
+
+#ifndef CONFIG_BRCMFMAC_SDIO_OOB
+	while (brcmf_sdbrcm_dpc(bus))
+		;
+#else
 	bus->dpc_sched = true;
 	if (bus->dpc_tsk) {
 		brcmf_sdbrcm_adddpctsk(bus);
 		complete(&bus->dpc_wait);
 	}
+#endif
 }
 
 static bool brcmf_sdbrcm_bus_watchdog(struct brcmf_sdio *bus)

-- 
1.7.1

Set up the wlan clock tree for Tegra20 and Tegra30.

Signed-off-by: Wei Ni <redacted>
---
 arch/arm/mach-tegra/board-dt-tegra20.c |    4 ++++
 arch/arm/mach-tegra/board-dt-tegra30.c |    4 ++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-tegra/board-dt-tegra20.c b/arch/arm/mach-tegra/board-dt-tegra20.c
index d9eeae6..5efc219 100644
--- a/arch/arm/mach-tegra/board-dt-tegra20.c
+++ b/arch/arm/mach-tegra/board-dt-tegra20.c

@@ -79,8 +79,12 @@ static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
 	{ "pll_a",      "pll_p_out1",   56448000,       true },
 	{ "pll_a_out0", "pll_a",        11289600,       true },
 	{ "cdev1",      NULL,           0,              true },
+	{ "blink",      "clk_32k",      32768,          true },
 	{ "i2s1",       "pll_a_out0",   11289600,       false},
 	{ "i2s2",       "pll_a_out0",   11289600,       false},
+	{ "sdmmc1",	"pll_p",	48000000,	false},
+	{ "sdmmc3",	"pll_p",	48000000,	false},
+	{ "sdmmc4",	"pll_p",	48000000,	false},
 	{ NULL,		NULL,		0,		0},
 };
 

diff --git a/arch/arm/mach-tegra/board-dt-tegra30.c b/arch/arm/mach-tegra/board-dt-tegra30.c
index 53bf60f..3277d1e 100644
--- a/arch/arm/mach-tegra/board-dt-tegra30.c
+++ b/arch/arm/mach-tegra/board-dt-tegra30.c

@@ -61,11 +61,15 @@ static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
 	{ "pll_a_out0",	"pll_a",	11289600,	true },
 	{ "extern1",	"pll_a_out0",	0,		true },
 	{ "clk_out_1",	"extern1",	0,		true },
+	{ "blink",	"clk_32k",	32768,		true },
 	{ "i2s0",	"pll_a_out0",	11289600,	false},
 	{ "i2s1",	"pll_a_out0",	11289600,	false},
 	{ "i2s2",	"pll_a_out0",	11289600,	false},
 	{ "i2s3",	"pll_a_out0",	11289600,	false},
 	{ "i2s4",	"pll_a_out0",	11289600,	false},
+	{ "sdmmc1",	"pll_p",	48000000,	false},
+	{ "sdmmc3",	"pll_p",	48000000,	false},
+	{ "sdmmc4",	"pll_p",	48000000,	false},
 	{ NULL,		NULL,		0,		0},
 };
 

-- 
1.7.1

New options enabled:
* WIRELESS: (dependency)
* CFG80211_WEXT: (dependency)
* WLAN: (dependency)
* BRCMFMAC: wlan driver, enable as module.

Signed-off-by: Wei Ni <redacted>
---
 arch/arm/configs/tegra_defconfig |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index ef2c679..8fab9e0 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig

@@ -73,7 +73,11 @@ CONFIG_BT_RFCOMM=y
 CONFIG_BT_BNEP=y
 CONFIG_BT_HIDP=y
 CONFIG_BT_HCIBTUSB=m
+CONFIG_WLAN=y
+CONFIG_BRCMFMAC=m
+CONFIG_WIRELESS=y
 CONFIG_CFG80211=y
+CONFIG_CFG80211_WEXT=y
 CONFIG_MAC80211=y
 CONFIG_RFKILL=y
 CONFIG_RFKILL_INPUT=y

-- 
1.7.1

Configure pinmux as required for WiFi.
Enable the SDHCI1 controller. This is connectted to the WiFi module.
For now, always enable the regulator that provides power to the Wifi module.

Signed-off-by: Wei Ni <redacted>
---
 arch/arm/boot/dts/tegra30-cardhu.dtsi |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi
index 4794693..ed9b530 100644
--- a/arch/arm/boot/dts/tegra30-cardhu.dtsi
+++ b/arch/arm/boot/dts/tegra30-cardhu.dtsi

@@ -52,6 +52,22 @@
 				nvidia,pull = <2>;
 				nvidia,tristate = <0>;
 			};
+			sdmmc3_clk_pa6 {
+				nvidia,pins = "sdmmc3_clk_pa6";
+				nvidia,function = "sdmmc3";
+				nvidia,pull = <0>;
+				nvidia,tristate = <0>;
+			};
+			sdmmc3_cmd_pa7 {
+				nvidia,pins =	"sdmmc3_cmd_pa7",
+						"sdmmc3_dat0_pb7",
+						"sdmmc3_dat1_pb6",
+						"sdmmc3_dat2_pb5",
+						"sdmmc3_dat3_pb4";
+				nvidia,function = "sdmmc3";
+				nvidia,pull = <2>;
+				nvidia,tristate = <0>;
+			};
 			sdmmc4_clk_pcc4 {
 				nvidia,pins =	"sdmmc4_clk_pcc4",
 						"sdmmc4_rst_n_pcc3";

@@ -81,6 +97,15 @@
 				nvidia,pull = <0>;
 				nvidia,tristate = <0>;
 			};
+			sdio3 {
+				nvidia,pins = "drive_sdio3";
+				nvidia,high-speed-mode = <0>;
+				nvidia,schmitt = <0>;
+				nvidia,pull-down-strength = <46>;
+				nvidia,pull-up-strength = <42>;
+				nvidia,slew-rate-rising = <1>;
+				nvidia,slew-rate-falling = <1>;
+			};
 		};
 	};
 

@@ -292,6 +317,11 @@
 		bus-width = <4>;
 	};
 
+	sdhci at 78000400 {
+		status = "okay";
+		power-gpios = <&gpio 28 0>; /* gpio PD4 */
+	};
+
 	sdhci at 78000600 {
 		status = "okay";
 		bus-width = <8>;

@@ -407,6 +437,8 @@
 			regulator-name = "vdd_com";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
+			regulator-always-on;
+			regulator-boot-on;
 			enable-active-high;
 			gpio = <&gpio 24 0>; /* gpio PD0 */
 			vin-supply = <&sys_3v3_reg>;

-- 
1.7.1

Configure pinmux as required for WiFi.
Enable the SDHCI1 controller, which is connectted to the WiFi module.

Signed-off-by: Wei Ni <redacted>
---
 arch/arm/boot/dts/tegra20-ventana.dts |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
index 4ec6b4c..3cbe54c 100644
--- a/arch/arm/boot/dts/tegra20-ventana.dts
+++ b/arch/arm/boot/dts/tegra20-ventana.dts

@@ -237,6 +237,16 @@
 					"ld23_22";
 				nvidia,pull = <1>;
 			};
+			drive_sdio1 {
+				nvidia,pins = "drive_sdio1";
+				nvidia,high-speed-mode = <0>;
+				nvidia,schmitt = <1>;
+				nvidia,low-power-mode = <3>;
+				nvidia,pull-down-strength = <31>;
+				nvidia,pull-up-strength = <31>;
+				nvidia,slew-rate-rising = <3>;
+				nvidia,slew-rate-falling = <3>;
+			};
 		};
 	};
 

@@ -454,6 +464,11 @@
 		status = "okay";
 	};
 
+	sdhci at c8000000 {
+		status = "okay";
+		power-gpios = <&gpio 86 0>; /* gpio PK6 */
+	};
+
 	sdhci at c8000400 {
 		status = "okay";
 		cd-gpios = <&gpio 69 0>; /* gpio PI5 */

-- 
1.7.1

Enable the SDHCI1 controller. This is connected to the WiFi module.

Signed-off-by: Wei Ni <redacted>
---
 arch/arm/boot/dts/tegra20-seaboard.dts |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/tegra20-seaboard.dts b/arch/arm/boot/dts/tegra20-seaboard.dts
index 92deb36..f783f31 100644
--- a/arch/arm/boot/dts/tegra20-seaboard.dts
+++ b/arch/arm/boot/dts/tegra20-seaboard.dts

@@ -590,6 +590,11 @@
 		status = "okay";
 	};
 
+	sdhci at c8000000 {
+		status = "okay";
+		power-gpios = <&gpio 86 0>; /* gpio PK6 */
+	};
+
 	sdhci at c8000400 {
 		status = "okay";
 		cd-gpios = <&gpio 69 0>; /* gpio PI5 */

-- 
1.7.1

On 08/27/2012 12:25 PM, Wei Ni wrote:

In case of inband interrupts, if we handle the interrupt in dpc thread,
two level of thread switching takes place to process wifi interrupts.
One in SDHCI driver and the other in Wifi driver. This may cause the system
instability.

Looking into the sdhci/mmc code indeed shows that the brcmfmac irq 
handler is not called in true IRQ context. So the dpc thread may add 
unnecessary complexity, but to me there is not indication that there is 
a stability issue.

Because the SDHCI calls sdio_irq_thread() to handle the irq, this thread locks
mmc host and calls wifi handler. It expects WiFi handler to be quick and
enables sdio interrupt from card at end. If wifi handler defers this work for
a different thread, sdio_irq_thread() will be stuck on next wifi interrupt
since mmc lock is not freed.

Not sure if I can follow this explanation. The isr is called with host 
claimed (by sdio_irq_thread) and all it does is at a linked list member 
and signal the dpc thread. After doing this the host is released.

Handling the interrupt in ISR directly will prevent thread context switching in
wifi driver. It can fix the instability problems.

This basically increases the duration of the isr in brcmfmac.

This is probably needed because the ISR now locks the host to long 
because brcmf_sdbrcm_dpc() is called directly.

I would really like to know what issue is solved by this change. Could 
you provide more details.

Franky,

Do you have anything to add here?

Gr. AvS

On 08/27/2012 09:24 AM, Arend van Spriel wrote:

On 08/27/2012 12:25 PM, Wei Ni wrote:

quoted

In case of inband interrupts, if we handle the interrupt in dpc thread,
two level of thread switching takes place to process wifi interrupts.
One in SDHCI driver and the other in Wifi driver. This may cause the
system
instability.

Looking into the sdhci/mmc code indeed shows that the brcmfmac irq
handler is not called in true IRQ context. So the dpc thread may add
unnecessary complexity, but to me there is not indication that there is
a stability issue.

quoted

Because the SDHCI calls sdio_irq_thread() to handle the irq, this
thread locks
mmc host and calls wifi handler. It expects WiFi handler to be quick and
enables sdio interrupt from card at end. If wifi handler defers this
work for
a different thread, sdio_irq_thread() will be stuck on next wifi
interrupt
since mmc lock is not freed.

Not sure if I can follow this explanation. The isr is called with host
claimed (by sdio_irq_thread) and all it does is at a linked list member
and signal the dpc thread. After doing this the host is released.

Is the issue something like the ISR handler or first level of threading
does:

* Trigger DPC
* Re-enable interrupt

So that the interrupt then fires again before the triggered DPC can run
to handle/clear it, thus causing an interrupt storm?

Whereas handling the interrupt directly prevents this race condition?

On Tue, 2012-08-28 at 00:24 +0800, Arend van Spriel wrote:

On 08/27/2012 12:25 PM, Wei Ni wrote:

quoted

In case of inband interrupts, if we handle the interrupt in dpc thread,
two level of thread switching takes place to process wifi interrupts.
One in SDHCI driver and the other in Wifi driver. This may cause the system
instability.

Looking into the sdhci/mmc code indeed shows that the brcmfmac irq 
handler is not called in true IRQ context. So the dpc thread may add 
unnecessary complexity, but to me there is not indication that there is 
a stability issue.

quoted

Because the SDHCI calls sdio_irq_thread() to handle the irq, this thread locks
mmc host and calls wifi handler. It expects WiFi handler to be quick and
enables sdio interrupt from card at end. If wifi handler defers this work for
a different thread, sdio_irq_thread() will be stuck on next wifi interrupt
since mmc lock is not freed.

Not sure if I can follow this explanation. The isr is called with host 
claimed (by sdio_irq_thread) and all it does is at a linked list member 
and signal the dpc thread. After doing this the host is released.

quoted

Handling the interrupt in ISR directly will prevent thread context switching in
wifi driver. It can fix the instability problems.

This basically increases the duration of the isr in brcmfmac.

quoted

Signed-off-by: Wei Ni <redacted>
---
  drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c   |    2 ++
  drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |    8 +++++++-
  2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 8e7e692..5cf6c3b 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c

@@ -121,7 +121,9 @@ static void brcmf_sdio_irqhandler(struct sdio_func *func)

  	brcmf_dbg(INTR, "ib intr triggered\n");

+	sdio_release_host(sdiodev->func[1]);
  	brcmf_sdbrcm_isr(sdiodev->bus);
+	sdio_claim_host(sdiodev->func[1]);

This is probably needed because the ISR now locks the host to long 
because brcmf_sdbrcm_dpc() is called directly.

quoted

  }

  /* dummy handler for SDIO function 2 interrupt */

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
index 472f2ef..4576d59 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c

@@ -2347,7 +2347,7 @@ static bool brcmf_sdbrcm_dpc(struct brcmf_sdio *bus)
  	uint framecnt = 0;	/* Temporary counter of tx/rx frames */
  	bool rxdone = true;	/* Flag for no more read data */
  	bool resched = false;	/* Flag indicating resched wanted */
-	int err;
+	int err = 0;

  	brcmf_dbg(TRACE, "Enter\n");

@@ -3786,11 +3786,17 @@ void brcmf_sdbrcm_isr(void *arg)
  	if (!bus->intr)
  		brcmf_dbg(ERROR, "isr w/o interrupt configured!\n");

+
+#ifndef CONFIG_BRCMFMAC_SDIO_OOB
+	while (brcmf_sdbrcm_dpc(bus))
+		;
+#else
  	bus->dpc_sched = true;
  	if (bus->dpc_tsk) {
  		brcmf_sdbrcm_adddpctsk(bus);
  		complete(&bus->dpc_wait);
  	}
+#endif
  }

  static bool brcmf_sdbrcm_bus_watchdog(struct brcmf_sdio *bus)

I would really like to know what issue is solved by this change. Could 
you provide more details.

If without this fix, the system is instability, we observed interrupt
from Wi-Fi cards pilling up in the system. We observed following issue
because of this:
1. Device is slow while downloading file through WiFi
2. WiFi performance is bad
3. WiFi does not turn on single CPU
4. Sometimes it will show following spew from kernel, and system will
hang up, it was caused by the semaphore which didn't be released.

INFO: task brcmf_watchdog:248 blocked for more than 120 seconds.
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this
message.
brcmf_watchdog  D c041aeac     0   248      2 0x00000000
[<c041aeac>] (__schedule+0x3c4/0x6d0) from [<c0418fe0>]
(schedule_timeout+0x1b0/0x218)
[<c0418fe0>] (schedule_timeout+0x1b0/0x218) from [<c041a748>] (__down
+0x68/0x98)
[<c041a748>] (__down+0x68/0x98) from [<c004c13c>] (down+0x44/0x4c)
[<c004c13c>] (down+0x44/0x4c) from [<bf017210>]
(brcmf_sdbrcm_bus_watchdog+0x28/0x1d0 [brcmfmac])
[<bf017210>] (brcmf_sdbrcm_bus_watchdog+0x28/0x1d0 [brcmfmac]) from
[<bf0173e4>] (brcmf_sdbrcm_watchdog_thread+0x2c/0x50 [brcmfmac])
[<bf0173e4>] (brcmf_sdbrcm_watchdog_thread+0x2c/0x50 [brcmfmac]) from
[<c0046328>] (kthread+0x8c/0x98)
[<c0046328>] (kthread+0x8c/0x98) from [<c000f5e4>] (kernel_thread_exit
+0x0/0x8)

After add this fix, everything looks ok.

I noticed that in the old version driver, it also has this fix, but
removed in http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-
2.6.git;a=commit;h=b61c23c846978a4381fdc499055bd66b7bd24120

Thanks.
Wei.

Franky,

Do you have anything to add here?

Gr. AvS

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, 2012-08-28 at 04:06 +0800, Stephen Warren wrote:

On 08/27/2012 09:24 AM, Arend van Spriel wrote:

quoted

On 08/27/2012 12:25 PM, Wei Ni wrote:

quoted

In case of inband interrupts, if we handle the interrupt in dpc thread,
two level of thread switching takes place to process wifi interrupts.
One in SDHCI driver and the other in Wifi driver. This may cause the
system
instability.

Looking into the sdhci/mmc code indeed shows that the brcmfmac irq
handler is not called in true IRQ context. So the dpc thread may add
unnecessary complexity, but to me there is not indication that there is
a stability issue.

The brcmfmac irq handler is called in the thread sdio_irq_thread(), this
thread indeed is driven by the sdhci irq, although it's not the true IRQ
context. If the brcmfmac doesn't clear the IRQ condition ASAP, the
sdio_irq_thread will be triggered again and again, and in this condition
it's too difficult to run the brcmfmac dpc thread, more and more
interrupt can't be handled.

quoted

quoted

Because the SDHCI calls sdio_irq_thread() to handle the irq, this
thread locks
mmc host and calls wifi handler. It expects WiFi handler to be quick and
enables sdio interrupt from card at end. If wifi handler defers this
work for
a different thread, sdio_irq_thread() will be stuck on next wifi
interrupt
since mmc lock is not freed.

Not sure if I can follow this explanation. The isr is called with host
claimed (by sdio_irq_thread) and all it does is at a linked list member
and signal the dpc thread. After doing this the host is released.

Is the issue something like the ISR handler or first level of threading
does:

* Trigger DPC
* Re-enable interrupt

So that the interrupt then fires again before the triggered DPC can run
to handle/clear it, thus causing an interrupt storm?

Whereas handling the interrupt directly prevents this race condition?

Above is my understanding.

On 08/28/2012 04:13 AM, Wei Ni wrote:

On Tue, 2012-08-28 at 04:06 +0800, Stephen Warren wrote:

quoted

On 08/27/2012 09:24 AM, Arend van Spriel wrote:

quoted

On 08/27/2012 12:25 PM, Wei Ni wrote:

quoted

In case of inband interrupts, if we handle the interrupt in dpc thread,
two level of thread switching takes place to process wifi interrupts.
One in SDHCI driver and the other in Wifi driver. This may cause the
system
instability.

Looking into the sdhci/mmc code indeed shows that the brcmfmac irq
handler is not called in true IRQ context. So the dpc thread may add
unnecessary complexity, but to me there is not indication that there is
a stability issue.

The brcmfmac irq handler is called in the thread sdio_irq_thread(), this
thread indeed is driven by the sdhci irq, although it's not the true IRQ
context. If the brcmfmac doesn't clear the IRQ condition ASAP, the
sdio_irq_thread will be triggered again and again, and in this condition
it's too difficult to run the brcmfmac dpc thread, more and more
interrupt can't be handled.

quoted

quoted

quoted

Because the SDHCI calls sdio_irq_thread() to handle the irq, this
thread locks
mmc host and calls wifi handler. It expects WiFi handler to be quick and
enables sdio interrupt from card at end. If wifi handler defers this
work for
a different thread, sdio_irq_thread() will be stuck on next wifi
interrupt
since mmc lock is not freed.

Not sure if I can follow this explanation. The isr is called with host
claimed (by sdio_irq_thread) and all it does is at a linked list member
and signal the dpc thread. After doing this the host is released.

Is the issue something like the ISR handler or first level of threading
does:

* Trigger DPC
* Re-enable interrupt

So that the interrupt then fires again before the triggered DPC can run
to handle/clear it, thus causing an interrupt storm?

Whereas handling the interrupt directly prevents this race condition?

Above is my understanding.

Hi Wei,

I understand the issue here and totally agree that we should treat 
in-band and out-band interrupts differently. But my concern is that the 
behavior of releasing the host before calling brcmf_sdbrcm_isr and grab 
it after is likely error prone. Also we are restructuring the dpc 
routine internally and it's almost done. I will find a better solution 
for in-band interrupt and get it the queue as well. So I suggest 
dropping this patch.

Thanks,
Franky

On 08/28/2012 09:45 AM, Franky Lin wrote:

On 08/28/2012 04:13 AM, Wei Ni wrote:

quoted

On Tue, 2012-08-28 at 04:06 +0800, Stephen Warren wrote:

quoted

On 08/27/2012 09:24 AM, Arend van Spriel wrote:

quoted

On 08/27/2012 12:25 PM, Wei Ni wrote:

quoted

In case of inband interrupts, if we handle the interrupt in dpc
thread,
two level of thread switching takes place to process wifi interrupts.
One in SDHCI driver and the other in Wifi driver. This may cause the
system
instability.

...

quoted

quoted

quoted

Not sure if I can follow this explanation. The isr is called with host
claimed (by sdio_irq_thread) and all it does is at a linked list member
and signal the dpc thread. After doing this the host is released.

Is the issue something like the ISR handler or first level of threading
does:

* Trigger DPC
* Re-enable interrupt

So that the interrupt then fires again before the triggered DPC can run
to handle/clear it, thus causing an interrupt storm?

Whereas handling the interrupt directly prevents this race condition?

Above is my understanding.

I understand the issue here and totally agree that we should treat
in-band and out-band interrupts differently. But my concern is that the
behavior of releasing the host before calling brcmf_sdbrcm_isr and grab
it after is likely error prone. Also we are restructuring the dpc
routine internally and it's almost done. I will find a better solution
for in-band interrupt and get it the queue as well. So I suggest
dropping this patch.

Franky, do you know which kernel release the DPC restructuring will make
it into? I ask because I can't apply the rest of the patches in this
series without first resolving the stability issues with the Broadcom
WiFi enabled, since that'd de-stabilize the Tegra platform
significantly, and I'd like to plan when we can apply these patches to
Tegra. Thanks!

On 08/28/2012 03:39 PM, Stephen Warren wrote:

On 08/28/2012 09:45 AM, Franky Lin wrote:

quoted

On 08/28/2012 04:13 AM, Wei Ni wrote:

quoted

On Tue, 2012-08-28 at 04:06 +0800, Stephen Warren wrote:

quoted

On 08/27/2012 09:24 AM, Arend van Spriel wrote:

quoted

On 08/27/2012 12:25 PM, Wei Ni wrote:

quoted

In case of inband interrupts, if we handle the interrupt in dpc
thread,
two level of thread switching takes place to process wifi interrupts.
One in SDHCI driver and the other in Wifi driver. This may cause the
system
instability.

...

quoted

quoted

quoted

quoted

Not sure if I can follow this explanation. The isr is called with host
claimed (by sdio_irq_thread) and all it does is at a linked list member
and signal the dpc thread. After doing this the host is released.

Is the issue something like the ISR handler or first level of threading
does:

* Trigger DPC
* Re-enable interrupt

So that the interrupt then fires again before the triggered DPC can run
to handle/clear it, thus causing an interrupt storm?

Whereas handling the interrupt directly prevents this race condition?

Above is my understanding.

I understand the issue here and totally agree that we should treat
in-band and out-band interrupts differently. But my concern is that the
behavior of releasing the host before calling brcmf_sdbrcm_isr and grab
it after is likely error prone. Also we are restructuring the dpc
routine internally and it's almost done. I will find a better solution
for in-band interrupt and get it the queue as well. So I suggest
dropping this patch.

Franky, do you know which kernel release the DPC restructuring will make
it into? I ask because I can't apply the rest of the patches in this
series without first resolving the stability issues with the Broadcom
WiFi enabled, since that'd de-stabilize the Tegra platform
significantly, and I'd like to plan when we can apply these patches to
Tegra. Thanks!

Hi Stephen,

Since we submit patches through linux-wireless tree, you may only be 
able to pick it up at 3.7-rc1. It's quite a big change so I don't think 
it will qualify as a bug fix to get into 3.6-rcX.

Regards,
Franky

On 08/28/2012 04:01 PM, Franky Lin wrote:

On 08/28/2012 03:39 PM, Stephen Warren wrote:

quoted

On 08/28/2012 09:45 AM, Franky Lin wrote:

quoted

On 08/28/2012 04:13 AM, Wei Ni wrote:

quoted

On Tue, 2012-08-28 at 04:06 +0800, Stephen Warren wrote:

quoted

On 08/27/2012 09:24 AM, Arend van Spriel wrote:

quoted

On 08/27/2012 12:25 PM, Wei Ni wrote:

quoted

In case of inband interrupts, if we handle the interrupt in dpc
thread,
two level of thread switching takes place to process wifi
interrupts.
One in SDHCI driver and the other in Wifi driver. This may cause the
system
instability.

...

quoted

quoted

quoted

quoted

Not sure if I can follow this explanation. The isr is called with
host
claimed (by sdio_irq_thread) and all it does is at a linked list
member
and signal the dpc thread. After doing this the host is released.

Is the issue something like the ISR handler or first level of
threading
does:

* Trigger DPC
* Re-enable interrupt

So that the interrupt then fires again before the triggered DPC can
run
to handle/clear it, thus causing an interrupt storm?

Whereas handling the interrupt directly prevents this race condition?

Above is my understanding.

I understand the issue here and totally agree that we should treat
in-band and out-band interrupts differently. But my concern is that the
behavior of releasing the host before calling brcmf_sdbrcm_isr and grab
it after is likely error prone. Also we are restructuring the dpc
routine internally and it's almost done. I will find a better solution
for in-band interrupt and get it the queue as well. So I suggest
dropping this patch.

Franky, do you know which kernel release the DPC restructuring will make
it into? I ask because I can't apply the rest of the patches in this
series without first resolving the stability issues with the Broadcom
WiFi enabled, since that'd de-stabilize the Tegra platform
significantly, and I'd like to plan when we can apply these patches to
Tegra. Thanks!

Hi Stephen,

Since we submit patches through linux-wireless tree, you may only be
able to pick it up at 3.7-rc1. It's quite a big change so I don't think
it will qualify as a bug fix to get into 3.6-rcX.

That's as quick as I expected it to show up, so that's great. I don't
suppose you could mail Wei and myself once the patch gets into the
linux-wireless tree, so we can test it out on Tegra. If the patch could
possibly go into a topic branch in the wireless tree so it can be merged
into the Tegra tree before this series, that would be awesome. Thanks.

On 08/28/2012 04:04 PM, Stephen Warren wrote:

On 08/28/2012 04:01 PM, Franky Lin wrote:

quoted

On 08/28/2012 03:39 PM, Stephen Warren wrote:

quoted

On 08/28/2012 09:45 AM, Franky Lin wrote:

quoted

On 08/28/2012 04:13 AM, Wei Ni wrote:

quoted

On Tue, 2012-08-28 at 04:06 +0800, Stephen Warren wrote:

quoted

On 08/27/2012 09:24 AM, Arend van Spriel wrote:

quoted

On 08/27/2012 12:25 PM, Wei Ni wrote:

quoted

In case of inband interrupts, if we handle the interrupt in dpc
thread,
two level of thread switching takes place to process wifi
interrupts.
One in SDHCI driver and the other in Wifi driver. This may cause the
system
instability.

...

quoted

quoted

quoted

quoted

Not sure if I can follow this explanation. The isr is called with
host
claimed (by sdio_irq_thread) and all it does is at a linked list
member
and signal the dpc thread. After doing this the host is released.

Is the issue something like the ISR handler or first level of
threading
does:

* Trigger DPC
* Re-enable interrupt

So that the interrupt then fires again before the triggered DPC can
run
to handle/clear it, thus causing an interrupt storm?

Whereas handling the interrupt directly prevents this race condition?

Above is my understanding.

I understand the issue here and totally agree that we should treat
in-band and out-band interrupts differently. But my concern is that the
behavior of releasing the host before calling brcmf_sdbrcm_isr and grab
it after is likely error prone. Also we are restructuring the dpc
routine internally and it's almost done. I will find a better solution
for in-band interrupt and get it the queue as well. So I suggest
dropping this patch.

Franky, do you know which kernel release the DPC restructuring will make
it into? I ask because I can't apply the rest of the patches in this
series without first resolving the stability issues with the Broadcom
WiFi enabled, since that'd de-stabilize the Tegra platform
significantly, and I'd like to plan when we can apply these patches to
Tegra. Thanks!

Hi Stephen,

Since we submit patches through linux-wireless tree, you may only be
able to pick it up at 3.7-rc1. It's quite a big change so I don't think
it will qualify as a bug fix to get into 3.6-rcX.

That's as quick as I expected it to show up, so that's great. I don't
suppose you could mail Wei and myself once the patch gets into the
linux-wireless tree, so we can test it out on Tegra. If the patch could
possibly go into a topic branch in the wireless tree so it can be merged
into the Tegra tree before this series, that would be awesome. Thanks.

Will keep you posted.

Franky