From: Mark Lord <liml@rtr.ca>
To: Grant Grundler <grundler@google.com>
Cc: IDE/ATA development list <linux-ide@vger.kernel.org>,
Linux Kernel <linux-kernel@vger.kernel.org>,
Tejun Heo <htejun@gmail.com>, Jeff Garzik <jgarzik@pobox.com>,
linux-pci@vger.kernel.org
Subject: Re: libata, devm_*, and MSI ?
Date: Tue, 20 Jan 2009 13:16:21 -0500 [thread overview]
Message-ID: <497614F5.6000301@rtr.ca> (raw)
In-Reply-To: <da824cf30901200944x545431b2va0233abc14e81f9b@mail.gmail.com>
Grant Grundler wrote:
> On Tue, Jan 20, 2009 at 8:03 AM, Mark Lord <liml@rtr.ca> wrote:
..
>> For starters, the MSI HOW-TO suggests that drivers must be careful
>> to invoke pci_disable_msi() on module unload, but I don't see that
>> happening anywhere in libata.
>
> I don't think that's necessary if free_irq() or disable_irq() are called.
> However, I'm not seeing those get called either.
..
The linux/Documentation/PCI/MSI-HOWTO.txt information is rather explicit
about it, and the pci core code does seem to expect it.
> However, I'm not seeing those get called either.
..
The devres code takes care of the free_irq() normally,
but does not do the pci_disable_msi(), because it doesn't
know enough about the device to do it right now.
..
>> Eg. from ahci.c, we have this:
>>
>> if ((hpriv->flags & AHCI_HFLAG_NO_MSI) || pci_enable_msi(pdev))
>> pci_intx(pdev, 1);
>>
>> Which agrees with the existing code in sata_mv:
>>
>> if (msi && hi jiqi(pdev))
>> pci_intx(pdev, 1);
..
>> if (pci_enable_msi(pdev) == 0)
>> pci_intx(pdev, 0);
>>
>> Either that one is wrong, or pci_intx() is unnecessary in all cases.
..
Those calls to pci_intx() are definitely redundant,
as the pci core already does that for us on the relevant paths.
>> Perhaps somebody from the PCI side of things might enlighten us all.
>
> http://lists.linuxcoding.com/kernel/2005-q3/msg11296.html
>
> Shows when pci_intx() was added. linux-pci was NOT cc'd on that email.
> And no one asked for Documentation/ update. C'est la vie.
..
I still think the documentation matches the code in the pci core, though.
Patch for sata_mv coming shortly.
Cheers
next prev parent reply other threads:[~2009-01-20 18:16 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-01-20 16:03 libata, devm_*, and MSI ? Mark Lord
2009-01-20 16:07 ` Mark Lord
2009-01-20 17:44 ` Grant Grundler
2009-01-20 18:16 ` Mark Lord [this message]
2009-01-20 18:51 ` Grant Grundler
2009-01-20 19:54 ` Mark Lord
2009-01-21 11:59 ` Tejun Heo
2009-01-20 21:50 ` Daniel Barkalow
2009-01-21 3:39 ` Mark Lord
2009-01-21 4:02 ` Grant Grundler
2009-01-21 4:16 ` Michael Ellerman
2009-01-21 15:05 ` Mark Lord
2009-01-22 0:33 ` Robert Hancock
2009-01-23 18:11 ` Mark Lord
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=497614F5.6000301@rtr.ca \
--to=liml@rtr.ca \
--cc=grundler@google.com \
--cc=htejun@gmail.com \
--cc=jgarzik@pobox.com \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@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.