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

From: Gavin Shan <[email-masked]>
Date: Tue, 17 Nov 2015 12:58:17 +1100

On Tue, Nov 17, 2015 at 11:30:49AM +1100, Daniel Axtens wrote:
>Gavin Shan <[email-masked]> 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 <[email-masked]>
>> ---
>>  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