Preempt-RT on OMAP3?

Commit Message

David Brownell April 8, 2009, 10:21 p.m. UTC

On Tuesday 07 April 2009, Hugo Vincent wrote:
> > http://hugovincent.com/files/lkml-20090407/boot3.log
>
> Can anyone give me any pointers on where to start for fixing the
> problems shown in the above boot log?

You'll have to start finding out about all the funky rules
which apply to the -RT kernels, I think.

As a rule, getting drivers to work in RT kernels involves
some changes.  Not all of those changes can work in mainline
kernels.  Some of the changes are driver issues; while some
are changes in RT framework code.  Some are bugfixes.

There are a lot of reasons why not all the RT patches have
made it to mainline ...

 
> It looks like some fairly low level locking bugs (spinlock vs
> raw_spinlock maybe?) in twl4030 IRQ handling and GP timer/clock event
> source setup.

The first lockdep warning is about some IRQ dispatch code.

I happen to have the appended patches sitting around; they
might affect this particular problem (or might not).  I don't
know if they'd even apply to the RT kernel.

Alternatively, there's some odd rule about using workqueues
in the current RT code.  Like not being able to trigger them
from all the usual contexts.  If so, that seems buglike to me.

- Dave

Comments

Hugo Vincent April 8, 2009, 11:34 p.m. UTC | #1

Cheers for that David.

Those patches still apply cleanly, but don't seem to affect this
particular problem.

I'm now using the 2.6.29.1 patch and the 2.6.29.1-rt5 patch against
Linux-omap 90e758af52ba803cba233fabee81176d99589f09. Here is the log
now:
http://hugovincent.com/files/lkml-20090407/boot4.log

I've spent enough time on this already, so I think I'll wait and see
if anyone takes on fixing these issues, and hope that my MUSB Rx DMA
problems go away when the pending patches hit. The RT patchset already
is working well enough for me in my application, and the only reason I
would absolutely *need* to fix it is because of nagging doubts about
possible deadlocks or crashes after I've deployed my board into the
field...

Thanks for all your help.

Hugo

On Thu, Apr 9, 2009 at 10:21 AM, David Brownell <[e-mail]> wrote:
> On Tuesday 07 April 2009, Hugo Vincent wrote:
>> > http://hugovincent.com/files/lkml-20090407/boot3.log
>>
>> Can anyone give me any pointers on where to start for fixing the
>> problems shown in the above boot log?
>
> You'll have to start finding out about all the funky rules
> which apply to the -RT kernels, I think.
>
> As a rule, getting drivers to work in RT kernels involves
> some changes.  Not all of those changes can work in mainline
> kernels.  Some of the changes are driver issues; while some
> are changes in RT framework code.  Some are bugfixes.
>
> There are a lot of reasons why not all the RT patches have
> made it to mainline ...
>
>
>> It looks like some fairly low level locking bugs (spinlock vs
>> raw_spinlock maybe?) in twl4030 IRQ handling and GP timer/clock event
>> source setup.
>
> The first lockdep warning is about some IRQ dispatch code.
>
> I happen to have the appended patches sitting around; they
> might affect this particular problem (or might not).  I don't
> know if they'd even apply to the RT kernel.
>
> Alternatively, there's some odd rule about using workqueues
> in the current RT code.  Like not being able to trigger them
> from all the usual contexts.  If so, that seems buglike to me.
>
> - Dave
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

From: David Brownell <[e-mail]>
Subject: twl4030: use new handle_threaded_irq() flow handler

Make the toplevel twl4030 irq dispatch code use the new 
handle_threaded_irq() flow handler.  Also, minor cleanup,
use the newish generic_handle_irq_desc().

Since that flow handler guarantees the IRQ handlers are
called only in a normal (sleeping) thread context, remove
some of the workarounds for the lockdep goofage whereby
it breaks various drivers by forcing IRQF_DISABLED on.
 
Signed-off-by: David Brownell <[e-mail]>
---
 drivers/mfd/twl4030-irq.c     |   15 +++++----------
 drivers/rtc/rtc-twl4030.c     |    8 --------
 drivers/usb/otg/twl4030-usb.c |    8 --------
 3 files changed, 5 insertions(+), 26 deletions(-)

--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -215,7 +215,6 @@  static int twl4030_irq_thread(void *data
 		}
 
 		/* these handlers deal with the relevant SIH irq status */
-		local_irq_disable();
 		for (module_irq = twl4030_irq_base;
 				pih_isr;
 				pih_isr >>= 1, module_irq++) {
@@ -235,10 +234,9 @@  static int twl4030_irq_thread(void *data
 					note_interrupt(module_irq, d,
 							IRQ_NONE);
 				else
-					d->handle_irq(module_irq, d);
+					generic_handle_irq_desc(module_irq, d);
 			}
 		}
-		local_irq_enable();
 
 		desc->chip->unmask(irq);
 	}
@@ -578,7 +576,7 @@  static inline int sih_read_isr(const str
 }
 
 /*
- * Generic handler for SIH interrupts ... we "know" this is called
+ * Generic handler for SIH interrupts ... we know this is called
  * in task context, with IRQs enabled.
  */
 static void handle_twl4030_sih(unsigned irq, struct irq_desc *desc)
@@ -588,10 +586,7 @@  static void handle_twl4030_sih(unsigned 
 	int isr;
 
 	/* reading ISR acks the IRQs, using clear-on-read mode */
-	local_irq_enable();
 	isr = sih_read_isr(sih);
-	local_irq_disable();
-
 	if (isr < 0) {
 		pr_err("twl4030: %s SIH, read ISR error %d\n",
 			sih->name, isr);
@@ -658,7 +653,7 @@  int twl4030_sih_setup(int module)
 		irq = irq_base + i;
 
 		set_irq_chip_and_handler(irq, &twl4030_sih_irq_chip,
-				handle_edge_irq);
+				handle_threaded_irq);
 		set_irq_chip_data(irq, agent);
 		activate_irq(irq);
 	}
@@ -666,7 +661,7 @@  int twl4030_sih_setup(int module)
 	status = irq_base;
 	twl4030_irq_next += i;
 
-	/* replace generic PIH handler (handle_simple_irq) */
+	/* replace generic PIH handler (handle_threaded_irq) */
 	irq = sih_mod + twl4030_irq_base;
 	set_irq_data(irq, agent);
 	set_irq_chained_handler(irq, handle_twl4030_sih);
@@ -719,7 +714,7 @@  int twl_init_irq(int irq_num, unsigned i
 
 	for (i = irq_base; i < irq_end; i++) {
 		set_irq_chip_and_handler(i, &twl4030_irq_chip,
-				handle_simple_irq);
+				handle_threaded_irq);
 		activate_irq(i);
 	}
 	twl4030_irq_next = i;
--- a/drivers/rtc/rtc-twl4030.c
+++ b/drivers/rtc/rtc-twl4030.c
@@ -325,14 +325,6 @@  static irqreturn_t twl4030_rtc_interrupt
 	int res;
 	u8 rd_reg;
 
-#ifdef CONFIG_LOCKDEP
-	/* WORKAROUND for lockdep forcing IRQF_DISABLED on us, which
-	 * we don't want and can't tolerate.  Although it might be
-	 * friendlier not to borrow this thread context...
-	 */
-	local_irq_enable();
-#endif
-
 	res = twl4030_rtc_read_u8(&rd_reg, REG_RTC_STATUS_REG);
 	if (res)
 		goto out;
--- a/drivers/usb/otg/twl4030-usb.c
+++ b/drivers/usb/otg/twl4030-usb.c
@@ -576,14 +576,6 @@  static irqreturn_t twl4030_usb_irq(int i
 	struct twl4030_usb *twl = _twl;
 	int status;
 
-#ifdef CONFIG_LOCKDEP
-	/* WORKAROUND for lockdep forcing IRQF_DISABLED on us, which
-	 * we don't want and can't tolerate.  Although it might be
-	 * friendlier not to borrow this thread context...
-	 */
-	local_irq_enable();
-#endif
-
 	status = twl4030_usb_linkstat(twl);
 	if (status != USB_LINK_UNKNOWN) {