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 <gwshan@xxxxxxxxxxxxxxxxxx> >>--- >> 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 > -- 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