> -----Original Message----- > From: Valentin Longchamp [mailto:valentin.longch...@keymile.com] > Sent: 01 June 2012 12:33 > To: ub...@lukaperkov.net > Cc: Prafulla Wadaskar; Brunck, Holger; u-boot@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.longch...@keymile.com> > >> cc: Holger Brunck <holger.bru...@keymile.com> > >> cc: Prafulla Wadaskar <prafu...@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 . . . _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
