[U-Boot] [PATCH v3 1/5] kirkwood: add save functionality kirkwood_mpp_conf function

[Prafulla Wadaskar]

> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
> Sent: 01 June 2012 12:33
> To: uboot at lukaperkov.net
> Cc: Prafulla Wadaskar; Brunck, Holger; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH v3 1/5] kirkwood: add save functionality
> kirkwood_mpp_conf function
> 
> Hi Luka,
> 
> On 06/01/2012 01:02 AM, Luka Perkov wrote:
> > Hi Valentin,
> >
> > On Thu, May 31, 2012 at 04:17:52PM +0200, Valentin Longchamp wrote:
> >> If a second non NULL argument is given to the kirkwood_mpp_conf
> >> function, it will be used to store the current configuration of the
> MPP
> >> registers. mpp_save  must be a preallocated table of the same size
> as
> >> mpp_list and it must be zero terminated as well.
> >>
> >> A later call to kirkwood_mpp_conf function with this saved list as
> first
> >> (mpp_conf) argment will set the configuration back.
> >>
> >> Signed-off-by: Valentin Longchamp <valentin.longchamp at keymile.com>
> >> cc: Holger Brunck <holger.brunck at keymile.com>
> >> cc: Prafulla Wadaskar <prafulla at marvell.com>
> >> ---
> >>  arch/arm/cpu/arm926ejs/kirkwood/mpp.c    |   14 ++++++++++++--
> >>  arch/arm/include/asm/arch-kirkwood/mpp.h |    2 +-
> >>  2 files changed, 13 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> >> index 3da6c98..158ea84 100644
> >> --- a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> >> +++ b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> >> @@ -31,11 +31,11 @@ static u32 kirkwood_variant(void)
> >>  #define MPP_CTRL(i)	(KW_MPP_BASE + (i* 4))
> >>  #define MPP_NR_REGS	(1 + MPP_MAX/8)
> >>
> >> -void kirkwood_mpp_conf(u32 *mpp_list)
> >> +void kirkwood_mpp_conf(u32 *mpp_list, u32 *mpp_save)
> >>  {
> >>  	u32 mpp_ctrl[MPP_NR_REGS];
> >>  	unsigned int variant_mask;
> >> -	int i;
> >> +	int i, save = 0;
> >>
> >>  	variant_mask = kirkwood_variant();
> >>  	if (!variant_mask)
> >> @@ -48,10 +48,13 @@ void kirkwood_mpp_conf(u32 *mpp_list)
> >>  	}
> >>  	debug("\n");
> >>
> >> +	if (mpp_save)
> >> +		save = 1;
> >>
> >>  	while (*mpp_list) {
> >>  		unsigned int num = MPP_NUM(*mpp_list);
> >>  		unsigned int sel = MPP_SEL(*mpp_list);
> >> +		unsigned int sel_save;
> >>  		int shift;
> >>
> >>  		if (num > MPP_MAX) {
> >> @@ -66,6 +69,13 @@ void kirkwood_mpp_conf(u32 *mpp_list)
> >>  		}
> >>
> >>  		shift = (num & 7) << 2;
> >> +
> >> +		if (save) {
> >
> > Why using new variable if it's only used in one place? Why not use
> this
> > here:
> >
> > 		if (mpp_save) {
> >
> > Then we don't need save variable at all.
> >
> 
> Yeah you are right, I can directly test on mpp_save, since it should
> remain NULL
> during the whole while loop if NULL at the beginning.

Pls port v4 with this fix, rest of the patch series looks okay to me.

Regards..
Prafulla . . .