From: Amitkumar Karwar <[email protected]>
601216e "mwifiex: process RX packets in SDIO IRQ thread directly"
introduced a command timeout issue which can be reproduced easily on
an AM33xx platform using a test application written by Daniel Mack:
https://gist.github.com/zonque/6579314
mwifiex_main_process() is called from both the SDIO handler and
the workqueue. In case an interrupt occurs right after the
int_status check, but before updating the mwifiex_processing flag,
this interrupt gets lost, resulting in a command timeout and
consequently a card reset.
Let main_proc_lock protect both int_status and mwifiex_processing
flag. This fixes the interrupt lost issue.
Cc: <[email protected]> # 3.7+
Reported-by: Sven Neumann <[email protected]>
Reported-by: Andreas Fenkart <[email protected]>
Tested-by: Daniel Mack <[email protected]>
Reviewed-by: Dylan Reid <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
Signed-off-by: Paul Stewart <[email protected]>
---
drivers/net/wireless/mwifiex/main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index fd77833..c2b91f5 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -358,10 +358,12 @@ process_start:
}
} while (true);
- if ((adapter->int_status) || IS_CARD_RX_RCVD(adapter))
+ spin_lock_irqsave(&adapter->main_proc_lock, flags);
+ if ((adapter->int_status) || IS_CARD_RX_RCVD(adapter)) {
+ spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
goto process_start;
+ }
- spin_lock_irqsave(&adapter->main_proc_lock, flags);
adapter->mwifiex_processing = false;
spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
--
1.8.2.3
On 27.09.2013 19:55, Bing Zhao wrote:
> From: Amitkumar Karwar <[email protected]>
>
> 601216e "mwifiex: process RX packets in SDIO IRQ thread directly"
> introduced a command timeout issue which can be reproduced easily on
> an AM33xx platform using a test application written by Daniel Mack:
>
> https://gist.github.com/zonque/6579314
Actually, just out of pure curiosity, I would be interested whether the
issue is also as easily reproducible on other platform like Chromebooks.
Could anyone give that a try maybe?
Thanks,
Daniel
>
> mwifiex_main_process() is called from both the SDIO handler and
> the workqueue. In case an interrupt occurs right after the
> int_status check, but before updating the mwifiex_processing flag,
> this interrupt gets lost, resulting in a command timeout and
> consequently a card reset.
>
> Let main_proc_lock protect both int_status and mwifiex_processing
> flag. This fixes the interrupt lost issue.
>
> Cc: <[email protected]> # 3.7+
> Reported-by: Sven Neumann <[email protected]>
> Reported-by: Andreas Fenkart <[email protected]>
> Tested-by: Daniel Mack <[email protected]>
> Reviewed-by: Dylan Reid <[email protected]>
> Signed-off-by: Amitkumar Karwar <[email protected]>
> Signed-off-by: Bing Zhao <[email protected]>
> Signed-off-by: Paul Stewart <[email protected]>
> ---
> drivers/net/wireless/mwifiex/main.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
> index fd77833..c2b91f5 100644
> --- a/drivers/net/wireless/mwifiex/main.c
> +++ b/drivers/net/wireless/mwifiex/main.c
> @@ -358,10 +358,12 @@ process_start:
> }
> } while (true);
>
> - if ((adapter->int_status) || IS_CARD_RX_RCVD(adapter))
> + spin_lock_irqsave(&adapter->main_proc_lock, flags);
> + if ((adapter->int_status) || IS_CARD_RX_RCVD(adapter)) {
> + spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
> goto process_start;
> + }
>
> - spin_lock_irqsave(&adapter->main_proc_lock, flags);
> adapter->mwifiex_processing = false;
> spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
>
>
On 27.09.2013 23:11, Paul Stewart wrote:
> On Fri, Sep 27, 2013 at 11:55 AM, Daniel Mack <[email protected]
> <mailto:[email protected]>> wrote:
>
> On 27.09.2013 19 <tel:27.09.2013%2019>:55, Bing Zhao wrote:
> > From: Amitkumar Karwar <[email protected]
> <mailto:[email protected]>>
> >
> > 601216e "mwifiex: process RX packets in SDIO IRQ thread directly"
> > introduced a command timeout issue which can be reproduced easily on
> > an AM33xx platform using a test application written by Daniel Mack:
> >
> > https://gist.github.com/zonque/6579314
>
> Actually, just out of pure curiosity, I would be interested whether the
> issue is also as easily reproducible on other platform like Chromebooks.
> Could anyone give that a try maybe?
>
> I would hesitate to say "easily". Even on a kernel before your change,
> running your test application
That might be a dumb remark, but you noticed that the interface name in
the code doesn't actually match the name of the interface mwifiex
creates by default? We changed that to "wlan0" in our system to bring it
in line with other platforms.
I changed the test tool in the gist so it requires you to pass the name
as first argument.
Daniel
On Tue, Oct 15, 2013 at 12:38:50PM +0200, Daniel Mack wrote: > On 27.09.2013 19:55, Bing Zhao wrote:
> > From: Amitkumar Karwar <[email protected]>
> >
> > 601216e "mwifiex: process RX packets in SDIO IRQ thread directly"
> > introduced a command timeout issue which can be reproduced easily on
> > an AM33xx platform using a test application written by Daniel Mack:
> >
> > https://gist.github.com/zonque/6579314
> >
> > mwifiex_main_process() is called from both the SDIO handler and
> > the workqueue. In case an interrupt occurs right after the
> > int_status check, but before updating the mwifiex_processing flag,
> > this interrupt gets lost, resulting in a command timeout and
> > consequently a card reset.
> >
> > Let main_proc_lock protect both int_status and mwifiex_processing
> > flag. This fixes the interrupt lost issue.
>
> John, could you take this patch trough your tree?
I already did, and it is already in Dave's tree too...
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
On 27.09.2013 19:55, Bing Zhao wrote:
> From: Amitkumar Karwar <[email protected]>
>
> 601216e "mwifiex: process RX packets in SDIO IRQ thread directly"
> introduced a command timeout issue which can be reproduced easily on
> an AM33xx platform using a test application written by Daniel Mack:
>
> https://gist.github.com/zonque/6579314
>
> mwifiex_main_process() is called from both the SDIO handler and
> the workqueue. In case an interrupt occurs right after the
> int_status check, but before updating the mwifiex_processing flag,
> this interrupt gets lost, resulting in a command timeout and
> consequently a card reset.
>
> Let main_proc_lock protect both int_status and mwifiex_processing
> flag. This fixes the interrupt lost issue.
John, could you take this patch trough your tree?
Thanks,
Daniel
>
> Cc: <[email protected]> # 3.7+
> Reported-by: Sven Neumann <[email protected]>
> Reported-by: Andreas Fenkart <[email protected]>
> Tested-by: Daniel Mack <[email protected]>
> Reviewed-by: Dylan Reid <[email protected]>
> Signed-off-by: Amitkumar Karwar <[email protected]>
> Signed-off-by: Bing Zhao <[email protected]>
> Signed-off-by: Paul Stewart <[email protected]>
> ---
> drivers/net/wireless/mwifiex/main.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
> index fd77833..c2b91f5 100644
> --- a/drivers/net/wireless/mwifiex/main.c
> +++ b/drivers/net/wireless/mwifiex/main.c
> @@ -358,10 +358,12 @@ process_start:
> }
> } while (true);
>
> - if ((adapter->int_status) || IS_CARD_RX_RCVD(adapter))
> + spin_lock_irqsave(&adapter->main_proc_lock, flags);
> + if ((adapter->int_status) || IS_CARD_RX_RCVD(adapter)) {
> + spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
> goto process_start;
> + }
>
> - spin_lock_irqsave(&adapter->main_proc_lock, flags);
> adapter->mwifiex_processing = false;
> spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
>
>