Re: [PATCH] [MUSB] Fix for crash in DM646x USB when (CPPI)DMA is enabled.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

Subbrathnam, Swaminathan wrote:
Sergei,
	Pl. read through the patch description (and the associated code) before expressing your opinion.  This helps to avoid the unnecessary traffic for all of us.

	As in the patch description DM646x has 2 separate interrupt relating to USB subsystem.  One handles DMA interrupts while other handles non-DMA interrupt.

  I know -- because I was the author of the cppi_interrupt(). :-)
Unfortunately I have completely overlooked the locking problem (and I yet had some strange things happening with the USB storage into which I hadn't looked enough it seems). :-<

	Davinci_interrupt() handler handles non-DMA interrupts while cppi_interrupt() handler handles the DMA interrupts.

	In the davinci_interrupt() handler refer to the below code snippet

---------------------------------------------------------------------------
        cppi = container_of(musb->dma_controller, struct cppi, controller);
        if (is_cppi_enabled() && musb->dma_controller && !cppi->irq)
                retval = cppi_interrupt(irq, __hci);
----------------------------------------------------------------------------

	From the above snippet you can see if the DMA irq is subscribed then davinci_interrupt() does not call the cppi_interrupt() handler.  This is the case in relation to DM646x platform.

Sigh. Look at the code just above that snippet, i.e. spiun_lock_irqsave(&musb->lock, flags), and think about *non-DM646x* case. You're causing the lock recursion in this case.

	I expect you to look in the code, understand the patch and then provide you observation in future and do not expect others to do this for you.


Regards
swami

-----Original Message-----
From: Sergei Shtylyov [mailto:sshtylyov@xxxxxxxxxxxxx] Sent: Thursday, September 24, 2009 7:01 PM
To: Subbrathnam, Swaminathan
Cc: linux-usb@xxxxxxxxxxxxxxx; davinci-linux-open-source@xxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH] [MUSB] Fix for crash in DM646x USB when (CPPI)DMA is enabled.

Swaminathan S wrote:
Race condition exists between the cppi_interrupt handler and davinci_interrupt
handler w.r.t completing a TX IO.  Since DM646x has seperate DMA and USB
Endpoint interrupts cppi_interrupt handler needs to hold the lock while
operating on the endpoint.

Signed-off-by: Swaminathan S <swami.iyer@xxxxxx>

NAK. davinci_interrupt() calls cppi_interrupt() with this lock already held. We probably need to move the lock acquisition below that call first.
WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux