All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Dahlmann <dahlmann.thomas@arcor.de>
To: linux-kernel@vger.kernel.org
Subject: Re: amd5536udc interrupts bug
Date: Thu, 08 Jan 2009 17:40:28 +0100	[thread overview]
Message-ID: <49662C7C.2040500@arcor.de> (raw)
In-Reply-To: <49661E83.2070703@arcor.de>


> 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




       reply	other threads:[~2009-01-08 16:40 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <49661E83.2070703@arcor.de>
2009-01-08 16:40 ` Thomas Dahlmann [this message]
2009-01-08 18:27   ` amd5536udc interrupts bug Vadim Lobanov
2009-01-09  2:02   ` Vadim Lobanov
2009-01-09 11:41     ` Thomas Dahlmann
2009-01-09 22:40       ` Vadim Lobanov
2009-01-10 20:28         ` Thomas Dahlmann
2009-01-12 19:02           ` Vadim Lobanov
2009-01-13 19:19           ` Vadim Lobanov
2009-01-14 12:43             ` Thomas Dahlmann
2009-01-14 22:49               ` Vadim Lobanov
2009-01-15  9:26                 ` Thomas Dahlmann
2009-01-17  0:17                   ` Vadim Lobanov
2009-01-19 12:23                     ` Thomas Dahlmann
2009-01-07 23:10 Vadim Lobanov
2009-01-08  2:32 ` Robert Hancock
2009-01-08  3:30   ` Vadim Lobanov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49662C7C.2040500@arcor.de \
    --to=dahlmann.thomas@arcor.de \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.