On Tue, 2008-07-29 at 15:15 +0900, Hidetoshi Seto wrote:
> The msi_free_irqs() and msix_free_all_irqs() do not free "irqs",
> but free entries of msi_desc which have attached to the device.
Hmm, sort of. It also calls arch_teardown_msi_irqs(), which at least in
the case of powerpc will actually free the irq (in the sense that
something else might be allocated that irq number in future).
But the rest of the patch is nice and I'm not really fussed about the
naming. And just to be clear you haven't changed the logic of
free_msi_entries() at all, just moved it to avoid the forward
declaration - right?
cheers
> Signed-off-by: Hidetoshi Seto <seto.hidetoshi@xxxxxxxxxxxxxx>
> ---
> drivers/pci/msi.c | 71 +++++++++++++++++++++-------------------------------
> 1 files changed, 29 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index e6e3fc5..07ff8aa 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -261,8 +261,30 @@ void unmask_msi_irq(unsigned int irq)
> msix_flush_writes(irq);
> }
>
> -static int msi_free_irqs(struct pci_dev* dev);
> +static void free_msi_entries(struct pci_dev *dev)
> +{
> + struct msi_desc *entry, *tmp;
> +
> + list_for_each_entry(entry, &dev->msi_list, list) {
> + if (entry->irq)
> + BUG_ON(irq_has_action(entry->irq));
> + }
> +
> + arch_teardown_msi_irqs(dev);
> +
> + list_for_each_entry_safe(entry, tmp, &dev->msi_list, list) {
> + if (entry->msi_attrib.type == PCI_CAP_ID_MSIX) {
> + writel(1, entry->mask_base + entry->msi_attrib.entry_nr
> + * PCI_MSIX_ENTRY_SIZE
> + + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
>
> + if (list_is_last(&entry->list, &dev->msi_list))
> + iounmap(entry->mask_base);
> + }
> + list_del(&entry->list);
> + kfree(entry);
> + }
> +}
>
> static struct msi_desc* alloc_msi_entry(void)
> {
> @@ -400,7 +422,7 @@ static int msi_capability_init(struct pci_dev *dev)
> /* Configure MSI capability structure */
> ret = arch_setup_msi_irqs(dev, 1, PCI_CAP_ID_MSI);
> if (ret) {
> - msi_free_irqs(dev);
> + free_msi_entries(dev);
> return ret;
> }
>
> @@ -478,7 +500,7 @@ static int msix_capability_init(struct pci_dev *dev,
> }
> }
>
> - msi_free_irqs(dev);
> + free_msi_entries(dev);
>
> /* If we had some success report the number of irqs
> * we succeeded in setting up.
> @@ -617,37 +639,10 @@ void pci_disable_msi(struct pci_dev* dev)
> if (!entry->dev || entry->msi_attrib.type != PCI_CAP_ID_MSI)
> return;
>
> - msi_free_irqs(dev);
> + free_msi_entries(dev);
> }
> EXPORT_SYMBOL(pci_disable_msi);
>
> -static int msi_free_irqs(struct pci_dev* dev)
> -{
> - struct msi_desc *entry, *tmp;
> -
> - list_for_each_entry(entry, &dev->msi_list, list) {
> - if (entry->irq)
> - BUG_ON(irq_has_action(entry->irq));
> - }
> -
> - arch_teardown_msi_irqs(dev);
> -
> - list_for_each_entry_safe(entry, tmp, &dev->msi_list, list) {
> - if (entry->msi_attrib.type == PCI_CAP_ID_MSIX) {
> - writel(1, entry->mask_base + entry->msi_attrib.entry_nr
> - * PCI_MSIX_ENTRY_SIZE
> - + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
> -
> - if (list_is_last(&entry->list, &dev->msi_list))
> - iounmap(entry->mask_base);
> - }
> - list_del(&entry->list);
> - kfree(entry);
> - }
> -
> - return 0;
> -}
> -
> /**
> * pci_enable_msix - configure device's MSI-X capability structure
> * @dev: pointer to the pci_dev data structure of MSI-X device function
> @@ -704,11 +699,6 @@ int pci_enable_msix(struct pci_dev* dev, struct msix_entry *entries, int nvec)
> }
> EXPORT_SYMBOL(pci_enable_msix);
>
> -static void msix_free_all_irqs(struct pci_dev *dev)
> -{
> - msi_free_irqs(dev);
> -}
> -
> void pci_msix_shutdown(struct pci_dev* dev)
> {
> if (!pci_msi_enable || !dev || !dev->msix_enabled)
> @@ -725,7 +715,7 @@ void pci_disable_msix(struct pci_dev* dev)
>
> pci_msix_shutdown(dev);
>
> - msix_free_all_irqs(dev);
> + free_msi_entries(dev);
> }
> EXPORT_SYMBOL(pci_disable_msix);
>
> @@ -743,11 +733,8 @@ void msi_remove_pci_irq_vectors(struct pci_dev* dev)
> if (!pci_msi_enable || !dev)
> return;
>
> - if (dev->msi_enabled)
> - msi_free_irqs(dev);
> -
> - if (dev->msix_enabled)
> - msix_free_all_irqs(dev);
> + if (dev->msi_enabled || dev->msix_enabled)
> + free_msi_entries(dev);
> }
>
> void pci_no_msi(void)
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
Attachment:
signature.asc
Description: This is a digitally signed message part
