Re: amd5536udc interrupts bug

[Thomas Dahlmann]

> 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



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