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

[Alexey Kardashevskiy]

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).



-- 
Alexey