On Tue, Nov 17, 2015 at 11:30:49AM +1100, Daniel Axtens wrote: >Gavin Shan <gwshan@xxxxxxxxxxxxxxxxxx> writes: > >> This introduces pnv_ioda_init_pe() to initialize the specified PE >> instance (phb->ioda.pe_array[x]). It's used by pnv_ioda_alloc_pe() >> and pnv_ioda_reserve_pe(). No logical changes introduced. >> >> Signed-off-by: Gavin Shan <gwshan@xxxxxxxxxxxxxxxxxx> >> --- >> arch/powerpc/platforms/powernv/pci-ioda.c | 14 ++++++++++---- >> 1 file changed, 10 insertions(+), 4 deletions(-) >> >> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c >> index ef93a01..488e0f8 100644 >> --- a/arch/powerpc/platforms/powernv/pci-ioda.c >> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c >> @@ -129,6 +129,14 @@ static inline bool pnv_pci_is_mem_pref_64(unsigned long flags) >> (IORESOURCE_MEM_64 | IORESOURCE_PREFETCH)); >> } >> >> +static struct pnv_ioda_pe *pnv_ioda_init_pe(struct pnv_phb *phb, int pe_no) >> +{ >> + phb->ioda.pe_array[pe_no].phb = phb; >> + phb->ioda.pe_array[pe_no].pe_number = pe_no; >> + >> + return &phb->ioda.pe_array[pe_no]; >You have the function returning the newly initalized PE here... > >> +} >> + >> static void pnv_ioda_reserve_pe(struct pnv_phb *phb, int pe_no) >> { >> if (!(pe_no >= 0 && pe_no < phb->ioda.total_pe_num)) { >> @@ -141,8 +149,7 @@ static void pnv_ioda_reserve_pe(struct pnv_phb *phb, int pe_no) >> pr_debug("%s: PE %d was reserved on PHB#%x\n", >> __func__, pe_no, phb->hose->global_number); >> >> - phb->ioda.pe_array[pe_no].phb = phb; >> - phb->ioda.pe_array[pe_no].pe_number = pe_no; >> + pnv_ioda_init_pe(phb, pe_no); >... but then you ignore the result here and in the other function you've >modified. > >It looks like you're using the result in the next patch though, so I >wonder if you would be better to merge this patch with the next >one. However, as I said before I'll defer to Alexey on decisions about >how to split the patch series if he has a different opinion. > I'd like to keep this separate when thinking about the rule I was told before: one patch does one thing if it can. Also, merging it to next one will make next one harder to be reiview. Thanks, Gavin -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html