Re: [PATCH V7 2/3] qe_common: add qe_muram_ functions to manage muram

On Thu, 2015-09-10 at 20:59 -0500, Zhao Qiang-B45475 wrote:

On Mon, 2015-09-11 at 06:09 -0500, Wood Scott-B07421 wrote:

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Friday, September 11, 2015 6:09 AM
To: Zhao Qiang-B45475
Cc: linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
lauraa@codeaurora.org; Xie Xiaobo-R63061; benh@kernel.crashing.org; Li
Yang-Leo-R58472; paulus@samba.org
Subject: Re: [PATCH V7 2/3] qe_common: add qe_muram_ functions to manage
muram

On Wed, 2015-09-09 at 21:34 -0500, Zhao Qiang-B45475 wrote:

quoted

On Mon, 2015-09-10 at 12:39 -0500, Wood Scott-B07421 wrote:

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Thursday, September 10, 2015 12:39 AM
To: Zhao Qiang-B45475
Cc: linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
lauraa@codeaurora.org; Xie Xiaobo-R63061; benh@kernel.crashing.org;
Li Yang-Leo-R58472; paulus@samba.org
Subject: Re: [PATCH V7 2/3] qe_common: add qe_muram_ functions to
manage muram

On Sun, 2015-09-06 at 01:37 -0500, Zhao Qiang-B45475 wrote:

quoted

On Mon, 2015-09-2 at 8:34 +0800, Wood Scott-B07421 wrote:

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Wednesday, September 02, 2015 8:34 AM
To: Zhao Qiang-B45475
Cc: linux-kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
lauraa@codeaurora.org; Xie Xiaobo-R63061;
benh@kernel.crashing.org; Li Yang-Leo-R58472; paulus@samba.org
Subject: Re: [PATCH V7 2/3] qe_common: add qe_muram_ functions
to manage muram

On Mon, 2015-08-31 at 16:58 +0800, Zhao Qiang wrote:

quoted

@@ -187,12 +190,25 @@ static inline int

qe_alive_during_sleep(void)  }

 /* we actually use cpm_muram implementation, define this for
convenience */ -#define qe_muram_init cpm_muram_init -#define
qe_muram_alloc cpm_muram_alloc -#define qe_muram_alloc_fixed
cpm_muram_alloc_fixed -#define qe_muram_free cpm_muram_free
-#define qe_muram_addr cpm_muram_addr -#define qe_muram_offset
cpm_muram_offset
+#define cpm_muram_init qe_muram_init #define cpm_muram_alloc
+qe_muram_alloc #define cpm_muram_alloc_fixed
+qe_muram_alloc_fixed #define cpm_muram_free qe_muram_free
+#define cpm_muram_addr qe_muram_addr #define cpm_muram_offset
+qe_muram_offset

Why?  This is unnecessary churn.

This is necessary. QE is on both ARM and PowerPC, its code is
under public code.
But CPM is only on PowerPC and its code is under PowerPC.
So when build ARM, QE will not find cpm_muram_* function.

If you move the cpm_muram functions to drivers/soc, then ARM will
find them.
There is no need to rename them.

Yes, moving cpm_muram can handle this issue. However, cpm is not
necessary To move to public code, and a churn is simpler than moving

cpm_muram.

quoted

Please consider my suggestion, Thank you!

What do you mean by "public code"?  You're already moving cpm_muram.  I'm
just asking that you not rename it while you do so.  If it absolutely
must be renamed, do it in a different patch from the one that moves the
code, though I don't see why the rename is helpful.

"Public code" is "driver/soc" here. I moved the qe_muram into driver/soc, 
not cpm_muram,

They are the same thing.

And the functions are named qe_muram_*, 

Only after you renamed them.  Before that they were just #defined aliases.

and removed cpm_muram_*, let cpm_muram
Use the same functions code with qe_muram, and define cpm_muram_* 
qe_muram_*. 

Previously, cpm/qe-muram functions are named cpm_muram_* because CPM is the 
predecessor
Of QE,

Yes, and just because marketing decided they wanted to change the name when 
they came out with a new version doesn't mean we need to rename 
infrastructure that is common between them.  E.g. we still classify PPC QorIQ 
chips as "mpc85xx".

Again, if you really want to do the rename, at least do it in a separate 
patch from moving the code, so that git will detect the code movement and we 
can see if anything else changed during the move.

-Scott