Re: TI wl1271 wireless bug with 3.4-rt17

[Gregoire Gentil <gregoiregentil@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4144 bytes --]



On 09/05/2012 09:22 PM, Gregoire Gentil wrote:
>
>
> On 09/05/2012 08:48 PM, Josh Cartwright wrote:
>> CC'd Luciano to let him know about breakage of wl12xx on -rt.
>>
>> On Tue, Sep 04, 2012 at 06:22:33PM -0700, Gregoire Gentil wrote:
>>> On 09/04/2012 07:11 AM, Josh Cartwright wrote:
>>>> On Sun, Sep 02, 2012 at 11:24:47PM -0700, Gregoire Gentil wrote:
>>>>> Hello,
>>>>>
>>>>> I'm trying to debug a wifi bug with 3.4-rt17 applied, running on an
>>>>> OMAP4 ARM board such as Pandaboard.
>>>>>
>>>>> Wi-Fi works perfectly well without rt patches. It also works quite
>>>>> well with rt patches AND without wifi module loaded. But with both
>>>>> rt patches and wifi module, the system is very flaky and even if I
>>>>> manage to launch a big download, I get a kernel hang. I managed to
>>>>> get a trace:
>>>>>
>>>>> BUG: scheduling while atomic: irq/213-wl12xx/1588/0x00010002
>>>>> Modules linked in: omapdce(C) wl12xx wlcore omaprpc(C) mac80211 d
>>>>> [<c001beb4>] (unwind_backtrace+0x0/0xf0) from [<c0613548>] (dump)
>>>>> [<c0613548>] (dump_stack+0x20/0x24) from [<c0073908>] (__schedul)
>>>>> [<c0073908>] (__schedule_bug+0x54/0x60) from [<c0614818>] (__sch)
>>>>> [<c0614818>] (__schedule+0x74/0x6c0) from [<c0614f60>] (schedule)
>>>>> [<c0614f60>] (schedule+0xa0/0xb8) from [<c0615eb8>] (rt_spin_loc)
>>>>> [<c0615eb8>] (rt_spin_lock_slowlock+0x198/0x288) from [<c06160a8)
>>>>> [<c06160a8>] (rt_spin_lock+0x18/0x1c) from [<bf0c6b24>] (wl12xx_)
>>>>> [<bf0c6b24>] (wl12xx_hardirq+0x2c/0xa4 [wlcore]) from [<c00bd4f0)
>>>>> [<c00bd4f0>] (handle_irq_event_percpu+0xac/0x24c) from [<c00bd70)
>>>>> [<c00bd70c>] (handle_irq_event+0x7c/0x9c) from [<c00c08f0>] (han)
>>>>> [<c00c08f0>] (handle_level_irq+0xe4/0x134) from [<c00bcf58>] (ge)
>>>>> [<c00bcf58>] (generic_handle_irq+0x34/0x3c) from [<c03217e8>] (g)
>>>>> [<c03217e8>] (gpio_irq_handler+0x160/0x1a4) from [<c00bcf58>] (g)
>>>>> [<c00bcf58>] (generic_handle_irq+0x34/0x3c) from [<c001449c>] (h)
>>>>> [<c001449c>] (handle_IRQ+0x88/0xc8)
>>>>>
>>>>> Source code including the function wl12xx_hardirq is here:
>>>>> http://dev.omapzoom.org/?p=integration/kernel-ubuntu.git;a=blob;f=drivers/net/wireless/ti/wlcore/main.c;h=45fe911a6504f92dddff5a9415bb77a643b3c4a9;hb=f84c72f6b36418ff11d16808c16a7c3216730bb0
>>>>>
>>>>>
>>>>> Any idea what could be wrong and how I could debug and fix this
>>>>> situation?
>>>>
>>>> On first glance, it looks like the driver uses request_threaded_irq(),
>>>> to register its handlers, but is trying to acquire a regular spin_lock
>>>> in its primary handler.  That's bad news, since spin_locks' can
>>>> schedule() when contended with CONFIG_PREEMPT_RT.
>>>>
>>>> And it's not just that, unfortunately, since the primary handler also
>>>> complete()s a completion, which also can schedule().
>>>>
>>>> It looks like the overall interrupt handling strategy of this driver
>>>> probably needs to be revisited. :(.
>>>
>>> Josh,
>>>
>>> I really appreciate the answer. Thank you. Though I'm definitely not
>>> a RT expert, I really would like to make this work... Could you
>>> provide a little bit more guideline what I should patch to fix this
>>> situation? Could I change request_threaded_irq to something else?
>>> Would raw_spin* help?
>>
>> In general, the hardirq handler should be doing the absolute bare
>> minimum necessary to determine whether or not the device is interrupting
>> (and if so, quiet it down and return IRQ_WAKE_THREAD), since
>> longer-running handlers have detrimental impact to the system
>> determinism.
>>
>> The right solution for wl12xx seems to be pushing the logic currently
>> implemented in the hardirq handler into the threaded handler, but
>> without knowing too many details about the driver, its difficult to
>> judge the viability/impact of this solution.
> [G2]. Luciano,
>
> Could you please comment on this suggestion? With the right guideline,
> I'm willing to patch and test and see if we can improve this buggy
> situation.
>
> Many thanks in advance,
>
> Grégoire
Find attached a patch which seems to work but I really don't know what 
I'm doing here! Any comment would be appreciated,

Grégoire


[-- Attachment #2: rt-wifi.patch --]
[-- Type: text/x-patch, Size: 1085 bytes --]

--- a/drivers/net/wireless/ti/wlcore/main.c	2012-09-06 08:35:44.821870411 -0700
+++ b/drivers/net/wireless/ti/wlcore/main.c	2012-09-06 08:38:32.994704341 -0700
@@ -5234,6 +5234,11 @@
 
 static irqreturn_t wl12xx_hardirq(int irq, void *cookie)
 {
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t wl1271_irq_special(int irq, void *cookie)
+{
 	struct wl1271 *wl = cookie;
 	unsigned long flags;
 
@@ -5253,12 +5258,10 @@
 		wl1271_debug(DEBUG_IRQ, "should not enqueue work");
 		disable_irq_nosync(wl->irq);
 		pm_wakeup_event(wl->dev, 0);
-		spin_unlock_irqrestore(&wl->wl_lock, flags);
-		return IRQ_HANDLED;
 	}
 	spin_unlock_irqrestore(&wl->wl_lock, flags);
 
-	return IRQ_WAKE_THREAD;
+	return wl1271_irq(irq, cookie);
 }
 
 int __devinit wlcore_probe(struct wl1271 *wl, struct platform_device *pdev)
@@ -5292,7 +5295,7 @@
 	else
 		irqflags = IRQF_TRIGGER_HIGH | IRQF_ONESHOT;
 
-	ret = request_threaded_irq(wl->irq, wl12xx_hardirq, wl1271_irq,
+	ret = request_threaded_irq(wl->irq, wl12xx_hardirq, wl1271_irq_special,
 				   irqflags,
 				   pdev->name, wl);
 	if (ret < 0) {