Michael Ellerman wrote:
> 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?
You are right. Nothing changed in the logic, but just the location.
It would be reasonable that we place msi_free_irqs() nearby
msi-specific fuctions and msix_free_all_irqs() nearby msix-specific
functions, if they are actually such specific.
But in fact both are same, one common function. Then it is better
location where the forward declaration is.
Thanks,
H.Seto
>> 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)
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
