lkml.org 
[lkml]   [2009]   [Jan]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: amd5536udc interrupts bug

> Vadim Lobanov wrote:
>> It is my understanding that a handler for a shared interrupt can be
>> invoked at any time after the corresponding request_irq() call is
>> made, simply because some other device on the same interrupt may
>> already be active. This leaves us with a very noticeable race
>> condition, where udc_irq() can be invoked with uninitialized 'dev'
>> data.
>
> Yes, your analysis appears correct.

Ah, right. Thanks for your analysis! Never ran into this condition as on the
Geode systems I had the IRQ is shared between UDC and UOC/OTG
(both on the CS5536 chip) and UOC never fires interrupts while UDC
driver is loaded as both drivers (UOC/OTG driver is not in the kernel yet)
register each other which makes sure that interrupts are enabled
after udc_pci_probe().

>> more complicated fix may be to try to shuffle all the code around to
>> make sure that 'dev' is fully initialized before we request the irq,
>> but I don't understand the code well enough (yet) to comfortably do
>> this.
>
> Yeah, that's the proper fix.

I could provide a fix within the next weeks. Currently I move to another
company and
have no hardware to test it.

Maybe you want to try this. It should work to place the register init
from udc_probe()

/* udc csr registers base */
dev->csr = dev->virt_addr + UDC_CSR_ADDR;
/* dev registers base */
dev->regs = dev->virt_addr + UDC_DEVCFG_ADDR;
/* ep registers base */
dev->ep_regs = dev->virt_addr + UDC_EPREGS_ADDR;
/* fifo's base */
dev->rxfifo = (u32 __iomem *)(dev->virt_addr + UDC_RXFIFO_ADDR);
dev->txfifo = (u32 __iomem *)(dev->virt_addr + UDC_TXFIFO_ADDR);

just before request_irq(...) to allow the interrupt handler to read the
interrupt status
registers.

Thomas





\
 
 \ /
  Last update: 2009-01-08 17:43    [W:0.952 / U:0.208 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site