[PATCH v7 22/50] powerpc/powernv: Introduce pnv_ioda_init_pe()

[Gavin Shan]

On Tue, Nov 17, 2015 at 01:37:33PM +1100, Alexey Kardashevskiy wrote:
>On 11/17/2015 12:58 PM, Gavin Shan wrote:
>>On Tue, Nov 17, 2015 at 11:30:49AM +1100, Daniel Axtens wrote:
>>>Gavin Shan <gwshan at linux.vnet.ibm.com> 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 at linux.vnet.ibm.com>
>>>>---
>>>>  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.
>
>This patch merged into the next one will make the next one easier to review
>because you won't have to change there the code which you just added in this
>patch (which is always good).
>

Ok & will do in next revision.

Thanks,
Gavin