On Wed, Apr 13, 2016 at 04:45:39PM +1000, Alexey Kardashevskiy wrote:
>On 02/17/2016 02:43 PM, Gavin Shan wrote:
>>The original implementation of pnv_ioda_setup_pe_seg() configures
>>IO and M32 segments by separate logics, which can be merged by
>>by caching @segmap, @seg_size, @win in advance. This shouldn't
>>cause any behavioural changes.
>>
>>Signed-off-by: Gavin Shan <gws...@linux.vnet.ibm.com>
>>---
>>  arch/powerpc/platforms/powernv/pci-ioda.c | 62 
>> ++++++++++++++-----------------
>>  1 file changed, 28 insertions(+), 34 deletions(-)
>>
>>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c 
>>b/arch/powerpc/platforms/powernv/pci-ioda.c
>>index 44cc5f3..fd7d382 100644
>>--- a/arch/powerpc/platforms/powernv/pci-ioda.c
>>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>>@@ -2940,8 +2940,10 @@ static void pnv_ioda_setup_pe_seg(struct 
>>pci_controller *hose,
>>      struct pnv_phb *phb = hose->private_data;
>>      struct pci_bus_region region;
>>      struct resource *res;
>>-     int i, index;
>>-     int rc;
>>+     unsigned int segsize;
>>+     int *segmap, index, i;
>>+     uint16_t win;
>>+     int64_t rc;
>>
>>      /*
>>       * NOTE: We only care PCI bus based PE for now. For PCI
>>@@ -2958,23 +2960,9 @@ static void pnv_ioda_setup_pe_seg(struct 
>>pci_controller *hose,
>>              if (res->flags & IORESOURCE_IO) {
>>                      region.start = res->start - phb->ioda.io_pci_base;
>>                      region.end   = res->end - phb->ioda.io_pci_base;
>>-                     index = region.start / phb->ioda.io_segsize;
>>-
>>-                     while (index < phb->ioda.total_pe_num &&
>>-                            region.start <= region.end) {
>>-                             phb->ioda.io_segmap[index] = pe->pe_number;
>>-                             rc = opal_pci_map_pe_mmio_window(phb->opal_id,
>>-                                     pe->pe_number, OPAL_IO_WINDOW_TYPE, 0, 
>>index);
>>-                             if (rc != OPAL_SUCCESS) {
>>-                                     pr_err("%s: OPAL error %d when mapping 
>>IO "
>>-                                            "segment #%d to PE#%d\n",
>>-                                            __func__, rc, index, 
>>pe->pe_number);
>>-                                     break;
>>-                             }
>>-
>>-                             region.start += phb->ioda.io_segsize;
>>-                             index++;
>>-                     }
>>+                     segsize      = phb->ioda.io_segsize;
>>+                     segmap       = phb->ioda.io_segmap;
>>+                     win          = OPAL_IO_WINDOW_TYPE;
>>              } else if ((res->flags & IORESOURCE_MEM) &&
>>                         !pnv_pci_is_mem_pref_64(res->flags)) {
>>                      region.start = res->start -
>>@@ -2983,23 +2971,29 @@ static void pnv_ioda_setup_pe_seg(struct 
>>pci_controller *hose,
>>                      region.end   = res->end -
>>                                     hose->mem_offset[0] -
>>                                     phb->ioda.m32_pci_base;
>>-                     index = region.start / phb->ioda.m32_segsize;
>>-
>>-                     while (index < phb->ioda.total_pe_num &&
>>-                            region.start <= region.end) {
>>-                             phb->ioda.m32_segmap[index] = pe->pe_number;
>>-                             rc = opal_pci_map_pe_mmio_window(phb->opal_id,
>>-                                     pe->pe_number, OPAL_M32_WINDOW_TYPE, 0, 
>>index);
>>-                             if (rc != OPAL_SUCCESS) {
>>-                                     pr_err("%s: OPAL error %d when mapping 
>>M32 "
>>-                                            "segment#%d to PE#%d",
>>-                                            __func__, rc, index, 
>>pe->pe_number);
>>-                                     break;
>>-                             }
>>+                     segsize      = phb->ioda.m32_segsize;
>>+                     segmap       = phb->ioda.m32_segmap;
>>+                     win          = OPAL_M32_WINDOW_TYPE;
>>+             } else {
>>+                     continue;
>>+             }
>>
>>-                             region.start += phb->ioda.m32_segsize;
>>-                             index++;
>>+             index = region.start / segsize;
>>+             while (index < phb->ioda.total_pe_num &&
>>+                    region.start <= region.end) {
>>+                     segmap[index] = pe->pe_number;
>>+                     rc = opal_pci_map_pe_mmio_window(phb->opal_id,
>>+                                     pe->pe_number, win, 0, index);
>>+                     if (rc != OPAL_SUCCESS) {
>>+                             pr_warn("%s: Error %lld mapping (%d) seg#%d to 
>>PHB#%d-PE#%d\n",
>>+                                     __func__, rc, win, index,
>>+                                     pe->phb->hose->global_number,
>>+                                     pe->pe_number);
>>+                             break;
>
>Please move this loop to a helper and stop caching segsize/segmap/win; this
>will make the code easier to read and the next patch will look much cleaner
>as it will not have to move this exact loop.
>

Thanks. It's good idea and I'll change the code accordingly in next revision.

>>                      }
>>+
>>+                     region.start += segsize;
>>+                     index++;
>>              }
>>      }
>>  }
>>
>
>
>-- 
>Alexey
>

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to