From patchwork Thu May 14 13:45:50 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthew Wilcox X-Patchwork-Id: 23771 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n4EDjqtv030490 for ; Thu, 14 May 2009 13:45:52 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751945AbZENNpu (ORCPT ); Thu, 14 May 2009 09:45:50 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752458AbZENNpu (ORCPT ); Thu, 14 May 2009 09:45:50 -0400 Received: from palinux.external.hp.com ([192.25.206.14]:49033 "EHLO mail.parisc-linux.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751945AbZENNpt (ORCPT ); Thu, 14 May 2009 09:45:49 -0400 Received: by mail.parisc-linux.org (Postfix, from userid 26919) id 2059A49400B; Thu, 14 May 2009 07:45:51 -0600 (MDT) Date: Thu, 14 May 2009 07:45:50 -0600 From: Matthew Wilcox To: Hidetoshi Seto Cc: michael@ellerman.id.au, linux-pci@vger.kernel.org Subject: Re: [PATCH] incremental fix for NIU MSI-X problem Message-ID: <20090514134550.GR15360@parisc-linux.org> References: <20090513214309.GL15360@parisc-linux.org> <1242272519.7408.26.camel@concordia> <4A0BCFFE.2060303@jp.fujitsu.com> <20090514134433.GQ15360@parisc-linux.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20090514134433.GQ15360@parisc-linux.org> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Thu, May 14, 2009 at 07:44:34AM -0600, Matthew Wilcox wrote: > I think the real problem is that msix_capability_init() is 90 lines > of code with 11 local variables. The fix for this, however, is not > to encapsulate the function control elsewhere, but to create more > subfunctions. I shrunk it to 45 lines by doing that ... I'll send > that patch as a followup (note I didn't even boot the result, but it > does compile). As promised ... diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index 6faff6a..73e2dbd 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -404,6 +404,62 @@ static int msi_capability_init(struct pci_dev *dev, int nvec) return 0; } +static void __iomem *msix_map_region(struct pci_dev *dev, unsigned pos, + unsigned nr_entries) +{ + unsigned long phys_addr; + u32 table_offset; + u8 bir; + + pci_read_config_dword(dev, msix_table_offset_reg(pos), &table_offset); + bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK); + table_offset &= ~PCI_MSIX_FLAGS_BIRMASK; + phys_addr = pci_resource_start(dev, bir) + table_offset; + return ioremap_nocache(phys_addr, nr_entries * PCI_MSIX_ENTRY_SIZE); +} + +static int msix_setup_entries(struct pci_dev *dev, unsigned pos, + struct msix_entry *entries, int nvec) +{ + struct msi_desc *entry; + int i, j; + + for (i = 0; i < nvec; i++) { + entry = alloc_msi_entry(dev); + if (!entry) + return i; + + j = entries[i].entry; + entry->msi_attrib.is_msix = 1; + entry->msi_attrib.is_64 = 1; + entry->msi_attrib.entry_nr = j; + entry->msi_attrib.default_irq = dev->irq; + entry->msi_attrib.pos = pos; + + list_add_tail(&entry->list, &dev->msi_list); + } + + return nvec; +} + +static void msix_program_entries(struct pci_dev *dev, void __iomem *base, + struct msix_entry *entries) +{ + struct msi_desc *entry; + int i = 0; + + list_for_each_entry(entry, &dev->msi_list, list) { + int j = entries[i].entry; + entries[i].vector = entry->irq; + entry->mask_base = base; + set_irq_msi(entry->irq, entry); + entry->masked = readl(base + j * PCI_MSIX_ENTRY_SIZE + + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET); + msix_mask_irq(entry, 1); + i++; + } +} + /** * msix_capability_init - configure device's MSI-X capability * @dev: pointer to the pci_dev data structure of MSI-X device function @@ -417,12 +473,8 @@ static int msi_capability_init(struct pci_dev *dev, int nvec) static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries, int nvec) { - struct msi_desc *entry; - int pos, i, j, nr_entries, ret; - unsigned long phys_addr; - u32 table_offset; + int pos, nr_entries, ret; u16 control; - u8 bir; void __iomem *base; pos = pci_find_capability(dev, PCI_CAP_ID_MSIX); @@ -432,47 +484,15 @@ static int msix_capability_init(struct pci_dev *dev, control &= ~PCI_MSIX_FLAGS_ENABLE; pci_write_config_word(dev, pos + PCI_MSIX_FLAGS, control); - /* Request & Map MSI-X table region */ - nr_entries = multi_msix_capable(control); - - pci_read_config_dword(dev, msix_table_offset_reg(pos), &table_offset); - bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK); - table_offset &= ~PCI_MSIX_FLAGS_BIRMASK; - phys_addr = pci_resource_start (dev, bir) + table_offset; - base = ioremap_nocache(phys_addr, nr_entries * PCI_MSIX_ENTRY_SIZE); + base = msix_map_region(dev, pos, multi_msix_capable(control)); if (base == NULL) return -ENOMEM; - for (i = 0; i < nvec; i++) { - entry = alloc_msi_entry(dev); - if (!entry) - break; - - j = entries[i].entry; - entry->msi_attrib.is_msix = 1; - entry->msi_attrib.is_64 = 1; - entry->msi_attrib.entry_nr = j; - entry->msi_attrib.default_irq = dev->irq; - entry->msi_attrib.pos = pos; - entry->mask_base = base; - - list_add_tail(&entry->list, &dev->msi_list); - } + nr_entries = msix_setup_entries(dev, pos, entries, nvec); ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX); - if (ret < 0) { - /* If we had some success report the number of irqs - * we succeeded in setting up. */ - int avail = 0; - list_for_each_entry(entry, &dev->msi_list, list) { - if (entry->irq != 0) { - avail++; - } - } - - if (avail != 0) - ret = avail; - } + if (ret < 0 && nr_entries > 0) + ret = nr_entries; if (ret) { msi_free_irqs(dev); @@ -487,16 +507,7 @@ static int msix_capability_init(struct pci_dev *dev, control |= PCI_MSIX_FLAGS_MASKALL | PCI_MSIX_FLAGS_ENABLE; pci_write_config_word(dev, pos + PCI_MSIX_FLAGS, control); - i = 0; - list_for_each_entry(entry, &dev->msi_list, list) { - entries[i].vector = entry->irq; - set_irq_msi(entry->irq, entry); - j = entries[i].entry; - entry->masked = readl(base + j * PCI_MSIX_ENTRY_SIZE + - PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET); - msix_mask_irq(entry, 1); - i++; - } + msix_program_entries(dev, base, entries); /* Set MSI-X enabled bits and unmask the function */ pci_intx_for_msi(dev, 0);