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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



--
Alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux