All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
To: Franky Lin <frankyl-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Cc: Wei Ni <wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	Arend van Spriel <arend-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	"rvossen-dY08KVG/lbpWk0Htik3J/w@public.gmane.org"
	<rvossen-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	Rakesh Kumar <krakesh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	Laxman Dewangan
	<ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	"linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"brcm80211-dev-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org"
	<brcm80211-dev-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Subject: Re: [PATCH 2/6] brcmfmac: Handling the interrupt in ISR directly for non-OOB
Date: Tue, 28 Aug 2012 15:39:40 -0700	[thread overview]
Message-ID: <503D48AC.5040509@wwwdotorg.org> (raw)
In-Reply-To: <503CF5B3.8040201-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

On 08/28/2012 09:45 AM, Franky Lin wrote:
> On 08/28/2012 04:13 AM, Wei Ni wrote:
>> On Tue, 2012-08-28 at 04:06 +0800, Stephen Warren wrote:
>>> On 08/27/2012 09:24 AM, Arend van Spriel wrote:
>>>> 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.
...
>>>> 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!
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren@wwwdotorg.org>
To: Franky Lin <frankyl@broadcom.com>
Cc: Wei Ni <wni@nvidia.com>, Arend van Spriel <arend@broadcom.com>,
	"rvossen@broadcom.com" <rvossen@broadcom.com>,
	Rakesh Kumar <krakesh@nvidia.com>,
	Laxman Dewangan <ldewangan@nvidia.com>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"brcm80211-dev-list@broadcom.com"
	<brcm80211-dev-list@broadcom.com>
Subject: Re: [PATCH 2/6] brcmfmac: Handling the interrupt in ISR directly for non-OOB
Date: Tue, 28 Aug 2012 15:39:40 -0700	[thread overview]
Message-ID: <503D48AC.5040509@wwwdotorg.org> (raw)
In-Reply-To: <503CF5B3.8040201@broadcom.com>

On 08/28/2012 09:45 AM, Franky Lin wrote:
> On 08/28/2012 04:13 AM, Wei Ni wrote:
>> On Tue, 2012-08-28 at 04:06 +0800, Stephen Warren wrote:
>>> On 08/27/2012 09:24 AM, Arend van Spriel wrote:
>>>> 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.
...
>>>> 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!

WARNING: multiple messages have this Message-ID (diff)
From: swarren@wwwdotorg.org (Stephen Warren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/6] brcmfmac: Handling the interrupt in ISR directly for non-OOB
Date: Tue, 28 Aug 2012 15:39:40 -0700	[thread overview]
Message-ID: <503D48AC.5040509@wwwdotorg.org> (raw)
In-Reply-To: <503CF5B3.8040201@broadcom.com>

On 08/28/2012 09:45 AM, Franky Lin wrote:
> On 08/28/2012 04:13 AM, Wei Ni wrote:
>> On Tue, 2012-08-28 at 04:06 +0800, Stephen Warren wrote:
>>> On 08/27/2012 09:24 AM, Arend van Spriel wrote:
>>>> 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.
...
>>>> 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!

  parent reply	other threads:[~2012-08-28 22:39 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-27 10:25 [PATCH 0/6] ARM: tegra: enable wlan for t20 and t30 Wei Ni
2012-08-27 10:25 ` Wei Ni
2012-08-27 10:25 ` Wei Ni
     [not found] ` <1346063114-30361-1-git-send-email-wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2012-08-27 10:25   ` [PATCH 1/6] ARM: tegra: set up wlan clocks for tegra dt Wei Ni
2012-08-27 10:25     ` Wei Ni
2012-08-27 10:25     ` Wei Ni
2012-08-27 10:25 ` [PATCH 2/6] brcmfmac: Handling the interrupt in ISR directly for non-OOB Wei Ni
2012-08-27 10:25   ` Wei Ni
2012-08-27 10:25   ` Wei Ni
     [not found]   ` <1346063114-30361-3-git-send-email-wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2012-08-27 16:24     ` Arend van Spriel
2012-08-27 16:24       ` Arend van Spriel
2012-08-27 16:24       ` Arend van Spriel
2012-08-27 20:06       ` Stephen Warren
2012-08-27 20:06         ` Stephen Warren
     [not found]         ` <503BD345.6030501-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2012-08-28 11:13           ` Wei Ni
2012-08-28 11:13             ` Wei Ni
2012-08-28 11:13             ` Wei Ni
2012-08-28 16:45             ` Franky Lin
2012-08-28 16:45               ` Franky Lin
2012-08-28 16:45               ` Franky Lin
     [not found]               ` <503CF5B3.8040201-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2012-08-28 22:39                 ` Stephen Warren [this message]
2012-08-28 22:39                   ` Stephen Warren
2012-08-28 22:39                   ` Stephen Warren
2012-08-28 23:01                   ` Franky Lin
2012-08-28 23:01                     ` Franky Lin
     [not found]                     ` <503D4DD9.2060808-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2012-08-28 23:04                       ` Stephen Warren
2012-08-28 23:04                         ` Stephen Warren
2012-08-28 23:04                         ` Stephen Warren
     [not found]                         ` <503D4E7F.9010205-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2012-08-28 23:10                           ` Franky Lin
2012-08-28 23:10                             ` Franky Lin
2012-08-28 23:10                             ` Franky Lin
     [not found]       ` <503B9F31.5050502-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2012-08-28  6:08         ` Wei Ni
2012-08-28  6:08           ` Wei Ni
2012-08-28  6:08           ` Wei Ni
2012-08-27 10:25 ` [PATCH 3/6] ARM: dt: t20 seaboard: turn on the power for wlan Wei Ni
2012-08-27 10:25   ` Wei Ni
2012-08-27 10:25   ` Wei Ni
2012-08-27 10:25 ` [PATCH 4/6] ARM: dt: t20 ventana: set pinmux and " Wei Ni
2012-08-27 10:25   ` Wei Ni
2012-08-27 10:25   ` Wei Ni
2012-08-27 10:25 ` [PATCH 5/6] ARM: dt: t30 cardhu: " Wei Ni
2012-08-27 10:25   ` Wei Ni
2012-08-27 10:25   ` Wei Ni
2012-08-27 10:25 ` [PATCH 6/6] ARM: tegra: enable wireless in defconfig Wei Ni
2012-08-27 10:25   ` Wei Ni
2012-08-27 10:25   ` Wei Ni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=503D48AC.5040509@wwwdotorg.org \
    --to=swarren-3lzwwm7+weoh9zmkesr00q@public.gmane.org \
    --cc=arend-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
    --cc=brcm80211-dev-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
    --cc=frankyl-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
    --cc=krakesh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    --cc=ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=rvossen-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
    --cc=wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.