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