Messages in this thread | | | Date | Thu, 08 Jan 2009 17:40:28 +0100 | From | Thomas Dahlmann <> | Subject | Re: 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
| |