Re: [PATCH 2/2] ARM: OMAP: MUSB: BUG on potential deadlocks

[Kevin Hilman <khilman@mvista.com>]

David Brownell wrote:
>>> This isn't a good patch.  *ADDING* a BUG() is wrong, and that's
>>> especially true for things like spin_is_locked() 
>> I probably should've made this an RFC instead of a patch.  I meant for
>> this to be a debug aid, not to actually go into the tree.  I'm using
>> this to try to track down some deadlocks and thought it may be useful to
>> others.  It has already turned up a couple problems (see below.)
> 
> OK, that's better.  As I presume you've deduced, a lot of the recent
> cleanup effort in this driver is so we can try pushing it upstream
> for 2.6.24 ... abuse of BUG() wouldn't help that!
> 
> 
>>> It'd be much better to actually submit fixes for any locking bugs
>>> than to add BUG_ON() statements.
>> I agree, and I'm working on the fixes.  Just not sure I have the right
>> fix yet.
>>
>> For exaple, I've found a couple paths where the BUG_ON() has definitely
>> turned up a problem:
>>
>> dma_controller_irq
>>  musb_dma_completion
>>   musb_g_rx  (or musb_host_rx in host mode)
>>    musb_g_giveback (musb_advance_schedule --> musb_giveback)
>>
>> In both host and gadget cases, the giveback function is entered without
>> the lock being held and tries to unlock the lock.  The comment in
>> musb_dma_completion says that the lock is held, but it's not when called
>> from the DMA IRQ ( but is when called from davinci_irq via
>> cppi_completion.)
> 
> I presume you mean the OMAP2430 support?  Looked to me like TUSB6010
> got that OK too.  That code for Mentor's DMA engine still has obvious
> bugs; and I don't know that it's ever had more than basic cleanups.

Yes, this is 2430.

So is nobody else trying to use the Mentor's DMA engine?  I know it's
typically disabled on DaVinci, but is it used on tusb?

Kevin