On Tue, Nov 17, 2015 at 12:54:04PM +1100, Alexey Kardashevskiy wrote:
>On 11/05/2015 12:12 AM, Gavin Shan wrote:
>>PEs are put into PHB DMA32 list (phb->ioda.pe_dma_list) according
>>to their DMA32 weight. The PEs on the list are iterated to setup
>>their TCE32 tables at system booting time. The list is used for
>>once and there is no good reason for it to survive.
>
>From the above I concluded that you need a list, just do not need to keep
>after the configuration is done but in fact you remove the list completely so
>just remove "to survive" (s/for it to survive/for keep having it/) :)
>

Thanks & will change it accordingly in next revision :)

>>
>>This moves the logic calculating DMA32 weight of PHB and PE to
>>pnv_pci_ioda1_setup_dma() to drop PHB's DMA32 list.
>>
>>Signed-off-by: Gavin Shan <gws...@linux.vnet.ibm.com>
>>---
>>  arch/powerpc/platforms/powernv/pci-ioda.c | 150 
>> ++++++++++++++----------------
>>  arch/powerpc/platforms/powernv/pci.h      |  19 ----
>>  2 files changed, 68 insertions(+), 101 deletions(-)
>>
>>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c 
>>b/arch/powerpc/platforms/powernv/pci-ioda.c
>>index 4c2e023..20ebe6e 100644
>>--- a/arch/powerpc/platforms/powernv/pci-ioda.c
>>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>>@@ -891,44 +891,6 @@ out:
>>      return 0;
>>  }
>>
>>-static void pnv_ioda_link_pe_by_weight(struct pnv_phb *phb,
>>-                                    struct pnv_ioda_pe *pe)
>>-{
>>-     struct pnv_ioda_pe *lpe;
>>-
>>-     list_for_each_entry(lpe, &phb->ioda.pe_dma_list, dma_link) {
>>-             if (lpe->dma_weight < pe->dma_weight) {
>>-                     list_add_tail(&pe->dma_link, &lpe->dma_link);
>>-                     return;
>>-             }
>>-     }
>>-     list_add_tail(&pe->dma_link, &phb->ioda.pe_dma_list);
>>-}
>>-
>>-static unsigned int pnv_ioda_dma_weight(struct pci_dev *dev)
>>-{
>>-     /* This is quite simplistic. The "base" weight of a device
>>-      * is 10. 0 means no DMA is to be accounted for it.
>>-      */
>>-
>>-     /* If it's a bridge, no DMA */
>>-     if (dev->hdr_type != PCI_HEADER_TYPE_NORMAL)
>>-             return 0;
>>-
>>-     /* Reduce the weight of slow USB controllers */
>>-     if (dev->class == PCI_CLASS_SERIAL_USB_UHCI ||
>>-         dev->class == PCI_CLASS_SERIAL_USB_OHCI ||
>>-         dev->class == PCI_CLASS_SERIAL_USB_EHCI)
>>-             return 3;
>>-
>>-     /* Increase the weight of RAID (includes Obsidian) */
>>-     if ((dev->class >> 8) == PCI_CLASS_STORAGE_RAID)
>>-             return 15;
>>-
>>-     /* Default */
>>-     return 10;
>>-}
>>-
>>  #ifdef CONFIG_PCI_IOV
>>  static int pnv_pci_vf_resource_shift(struct pci_dev *dev, int offset)
>>  {
>>@@ -1009,7 +971,6 @@ static void pnv_ioda_setup_same_PE(struct pci_bus *bus, 
>>struct pnv_ioda_pe *pe)
>>                      continue;
>>              }
>>              pdn->pe_number = pe->pe_number;
>>-             pe->dma_weight += pnv_ioda_dma_weight(dev);
>>              if ((pe->flags & PNV_IODA_PE_BUS_ALL) && dev->subordinate)
>>                      pnv_ioda_setup_same_PE(dev->subordinate, pe);
>>      }
>>@@ -1046,10 +1007,8 @@ static void pnv_ioda_setup_bus_PE(struct pci_bus *bus, 
>>bool all)
>>      pe->flags |= (all ? PNV_IODA_PE_BUS_ALL : PNV_IODA_PE_BUS);
>>      pe->pbus = bus;
>>      pe->pdev = NULL;
>>-     pe->tce32_seg = -1;
>>      pe->mve_number = -1;
>>      pe->rid = bus->busn_res.start << 8;
>>-     pe->dma_weight = 0;
>>
>>      if (all)
>>              pe_info(pe, "Secondary bus %d..%d associated with PE#%d\n",
>>@@ -1071,17 +1030,6 @@ static void pnv_ioda_setup_bus_PE(struct pci_bus *bus, 
>>bool all)
>>
>>      /* Put PE to the list */
>>      list_add_tail(&pe->list, &phb->ioda.pe_list);
>>-
>>-     /* Account for one DMA PE if at least one DMA capable device exist
>>-      * below the bridge
>>-      */
>>-     if (pe->dma_weight != 0) {
>>-             phb->ioda.dma_weight += pe->dma_weight;
>>-             phb->ioda.dma_pe_count++;
>>-     }
>>-
>>-     /* Link the PE */
>>-     pnv_ioda_link_pe_by_weight(phb, pe);
>>  }
>>
>>  static void pnv_ioda_setup_PEs(struct pci_bus *bus)
>>@@ -1389,7 +1337,6 @@ static void pnv_ioda_setup_vf_PE(struct pci_dev *pdev, 
>>u16 num_vfs)
>>              pe->flags = PNV_IODA_PE_VF;
>>              pe->pbus = NULL;
>>              pe->parent_dev = pdev;
>>-             pe->tce32_seg = -1;
>>              pe->mve_number = -1;
>>              pe->rid = (pci_iov_virtfn_bus(pdev, vf_index) << 8) |
>>                         pci_iov_virtfn_devfn(pdev, vf_index);
>>@@ -1842,6 +1789,47 @@ static struct iommu_table_ops pnv_ioda2_iommu_ops = {
>>      .free = pnv_ioda2_table_free,
>>  };
>>
>>+static int pnv_pci_ioda_dev_dma_weight(struct pci_dev *dev, void *data)
>>+{
>>+     unsigned int *weight = (unsigned int *)data;
>>+
>>+     /* This is quite simplistic. The "base" weight of a device
>>+      * is 10. 0 means no DMA is to be accounted for it.
>>+      */
>>+
>>+     if (dev->hdr_type != PCI_HEADER_TYPE_NORMAL)
>>+             return 0;
>>+
>>+     if (dev->class == PCI_CLASS_SERIAL_USB_UHCI ||
>>+         dev->class == PCI_CLASS_SERIAL_USB_OHCI ||
>>+         dev->class == PCI_CLASS_SERIAL_USB_EHCI)
>>+             *weight += 3;
>>+     else if ((dev->class >> 8) == PCI_CLASS_STORAGE_RAID)
>>+             *weight += 15;
>>+     else
>>+             *weight += 10;
>>+
>>+     return 0;
>>+}
>>+
>>+static unsigned int pnv_pci_ioda_pe_dma_weight(struct pnv_ioda_pe *pe)
>>+{
>>+     unsigned int weight = 0;
>>+
>>+     if ((pe->flags & PNV_IODA_PE_DEV) && pe->pdev) {
>>+             pnv_pci_ioda_dev_dma_weight(pe->pdev, &weight);
>>+     } else if ((pe->flags & PNV_IODA_PE_BUS) && pe->pbus) {
>>+             struct pci_dev *pdev;
>>+
>>+             list_for_each_entry(pdev, &pe->pbus->devices, bus_list)
>>+                     pnv_pci_ioda_dev_dma_weight(pdev, &weight);
>>+     } else if ((pe->flags & PNV_IODA_PE_BUS_ALL) && pe->pbus) {
>>+             pci_walk_bus(pe->pbus, pnv_pci_ioda_dev_dma_weight, &weight);
>>+     }
>>+
>>+     return weight;
>>+}
>>+
>>  static void pnv_pci_ioda1_setup_dma_pe(struct pnv_phb *phb,
>>                                     struct pnv_ioda_pe *pe,
>>                                     unsigned int base,
>>@@ -1858,17 +1846,12 @@ static void pnv_pci_ioda1_setup_dma_pe(struct pnv_phb 
>>*phb,
>>      /* XXX FIXME: Provide 64-bit DMA facilities & non-4K TCE tables etc.. */
>>      /* XXX FIXME: Allocate multi-level tables on PHB3 */
>>
>>-     /* We shouldn't already have a 32-bit DMA associated */
>>-     if (WARN_ON(pe->tce32_seg >= 0))
>>-             return;
>>-
>>      tbl = pnv_pci_table_alloc(phb->hose->node);
>>      iommu_register_group(&pe->table_group, phb->hose->global_number,
>>                      pe->pe_number);
>>      pnv_pci_link_table_and_group(phb->hose->node, 0, tbl, &pe->table_group);
>>
>>      /* Grab a 32-bit TCE table */
>>-     pe->tce32_seg = base;
>>      pe_info(pe, " Setting up 32-bit TCE table at %08x..%08x\n",
>>              base * PNV_IODA1_DMA32_SEGSIZE,
>>              (base + segs) * PNV_IODA1_DMA32_SEGSIZE - 1);
>>@@ -1932,8 +1915,6 @@ static void pnv_pci_ioda1_setup_dma_pe(struct pnv_phb 
>>*phb,
>>      return;
>>   fail:
>>      /* XXX Failure: Try to fallback to 64-bit only ? */
>>-     if (pe->tce32_seg >= 0)
>>-             pe->tce32_seg = -1;
>>      if (tce_mem)
>>              __free_pages(tce_mem, get_order(tce32_segsz * segs));
>>      if (tbl) {
>>@@ -2344,10 +2325,6 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb 
>>*phb,
>>  {
>>      int64_t rc;
>>
>>-     /* We shouldn't already have a 32-bit DMA associated */
>>-     if (WARN_ON(pe->tce32_seg >= 0))
>>-             return;
>>-
>>      /* TVE #1 is selected by PCI address bit 59 */
>>      pe->tce_bypass_base = 1ull << 59;
>>
>>@@ -2355,7 +2332,6 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb 
>>*phb,
>>                      pe->pe_number);
>>
>>      /* The PE will reserve all possible 32-bits space */
>>-     pe->tce32_seg = 0;
>>      pe_info(pe, "Setting up 32-bit TCE table at 0..%08x\n",
>>              phb->ioda.m32_pci_base);
>>
>>@@ -2371,11 +2347,8 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb 
>>*phb,
>>  #endif
>>
>>      rc = pnv_pci_ioda2_setup_default_config(pe);
>>-     if (rc) {
>>-             if (pe->tce32_seg >= 0)
>>-                     pe->tce32_seg = -1;
>>+     if (rc)
>>              return;
>>-     }
>>
>>      if (pe->flags & PNV_IODA_PE_DEV)
>>              iommu_add_device(&pe->pdev->dev);
>>@@ -2386,24 +2359,34 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb 
>>*phb,
>>  static void pnv_pci_ioda1_setup_dma(struct pnv_phb *phb)
>>  {
>>      struct pci_controller *hose = phb->hose;
>>-     unsigned int residual, remaining, segs, tw, base;
>>+     unsigned int weight, total_weight, dma_pe_count;
>>+     unsigned int residual, remaining, segs, base;
>>      struct pnv_ioda_pe *pe;
>>
>>+     total_weight = 0;
>>+     dma_pe_count = 0;
>>+     list_for_each_entry(pe, &phb->ioda.pe_list, list) {
>>+             weight = pnv_pci_ioda_pe_dma_weight(pe);
>>+             if (weight > 0)
>>+                     dma_pe_count++;
>>+
>>+             total_weight += weight;
>>+     }
>>+
>>      /* If we have more PE# than segments available, hand out one
>>       * per PE until we run out and let the rest fail. If not,
>>       * then we assign at least one segment per PE, plus more based
>>       * on the amount of devices under that PE
>>       */
>>-     if (phb->ioda.dma_pe_count > phb->ioda.tce32_count)
>>+     if (dma_pe_count > phb->ioda.tce32_count)
>>              residual = 0;
>>      else
>>-             residual = phb->ioda.tce32_count -
>>-                     phb->ioda.dma_pe_count;
>>+             residual = phb->ioda.tce32_count - dma_pe_count;
>>
>>      pr_info("PCI: Domain %04x has %ld available 32-bit DMA segments\n",
>>              hose->global_number, phb->ioda.tce32_count);
>>      pr_info("PCI: %d PE# for a total weight of %d\n",
>>-             phb->ioda.dma_pe_count, phb->ioda.dma_weight);
>>+             dma_pe_count, total_weight);
>>
>>      pnv_pci_ioda_setup_opal_tce_kill(phb);
>>
>>@@ -2412,24 +2395,26 @@ static void pnv_pci_ioda1_setup_dma(struct pnv_phb 
>>*phb)
>>       * weight
>>       */
>>      remaining = phb->ioda.tce32_count;
>>-     tw = phb->ioda.dma_weight;
>>      base = 0;
>>-     list_for_each_entry(pe, &phb->ioda.pe_dma_list, dma_link) {
>>-             if (!pe->dma_weight)
>>+     list_for_each_entry(pe, &phb->ioda.pe_list, list) {
>>+             weight = pnv_pci_ioda_pe_dma_weight(pe);
>>+             if (!weight)
>>                      continue;
>>+
>
>
>Unrelated new line.
>

Will remove it in next revision.

Thanks,
Gavin

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to