[alsa-devel] [PATCH] fm801: move to pcim_* and devm_* functions

[Takashi Iwai]

At Tue, 20 Jan 2015 15:51:05 +0200,
Andy Shevchenko wrote:
> 
> On Tue, Jan 20, 2015 at 3:48 PM, Takashi Iwai <tiwai at suse.de> wrote:
> > At Tue, 20 Jan 2015 15:46:13 +0200,
> > Andy Shevchenko wrote:
> >>
> >> On Tue, Jan 20, 2015 at 2:42 PM, Takashi Iwai <tiwai at suse.de> wrote:
> >> > At Wed, 07 Jan 2015 15:59:35 +0100,
> >> > Takashi Iwai wrote:
> >>
> >> []
> >>
> >>
> >> > I recalled finally why I didn't want this sort of changes.  Namely,
> >> > devm_request_irq() can't be used safely for the shared PCI
> >> > interrupts.
> >> >
> >> > There is a small open window between the driver's remove call
> >> > (i.e. card's private_free or device free calls) and the call of
> >> > devres_release_all().  The registered irq handler still remains during
> >> > this window.  When an irq is triggered from another shared device
> >> > during this, it goes to the remaining irq handler and accesses the
> >> > hardware unexpectedly.  In the case of snd_fm801_interrupt(), it's not
> >> > too bad, though.
> >>
> >> Don't we have interrupts disabled on ->remove() stage?
> >
> > It's a shared irq, so it doesn't help even if your device disables the
> > irq.  The other device can trigger the irq line at any time.
> 
> Got it. We can call devm_free_irq() explicitly in the ->remove() then.

Right, but then there is no much merit to use devm_request_irq(), too
:)

As mentioned, in the case of fm801, it's not too bad.  It's just a
single register read, should read zero, and skip the rest.  So we can
live as is.  My point is that we can't take this approach blindly
"just because others do".


Takashi