| Message ID | 1338473876-26278-2-git-send-email-valentin.longchamp@keymile.com |
|---|---|
| State | Superseded |
| Headers | show |
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@keymile.com> > cc: Holger Brunck <holger.brunck@keymile.com> > cc: Prafulla Wadaskar <prafulla@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. Luka
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@keymile.com> >> cc: Holger Brunck <holger.brunck@keymile.com> >> cc: Prafulla Wadaskar <prafulla@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.
> -----Original Message----- > From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com] > Sent: 01 June 2012 12:33 > To: uboot@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.longchamp@keymile.com> > >> cc: Holger Brunck <holger.brunck@keymile.com> > >> cc: Prafulla Wadaskar <prafulla@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 . . .
This series adds generic support for the spi_claim/release_bus functions for the kirkwood processors. The implementation was already discussed in another thread following my first board specific submission of the patch. The series adds two functions to the kirkwood mpp code to be able to temporarily save and then restore the mpp configuration. Changes for v2: - save MPP configuration with mpp_read function as dicussed on ML - moved CS pin MPP config to spi_setup_slave only - add backup fo CS pin in spi_setup_slave and reset in spi_free_slave Changes for v3: - moved mpp_read function functionality into mpp_conf function - fixed all calls to mpp_conf so that they are compliant with the newly necessary mpp_conf prototype Changes for v4: - minor fix in the mpp_conf function Valentin Longchamp (5): kirkwood: add save functionality kirkwood_mpp_conf function kirkwood: fix calls to kirkwood_mpp_conf kw_spi: backup and reset the MPP of the chosen CS pin kw_spi: support spi_claim/release_bus functions kw_spi: add weak functions board_spi_claim/release_bus arch/arm/cpu/arm926ejs/kirkwood/mpp.c | 10 +++- arch/arm/include/asm/arch-kirkwood/mpp.h | 2 +- arch/arm/include/asm/arch-kirkwood/spi.h | 11 ++++ board/LaCie/net2big_v2/net2big_v2.c | 2 +- board/LaCie/netspace_v2/netspace_v2.c | 2 +- board/Marvell/dreamplug/dreamplug.c | 2 +- board/Marvell/guruplug/guruplug.c | 2 +- board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.c | 2 +- board/Marvell/openrd/openrd.c | 2 +- board/Marvell/rd6281a/rd6281a.c | 2 +- board/Marvell/sheevaplug/sheevaplug.c | 2 +- board/Seagate/dockstar/dockstar.c | 2 +- board/cloudengines/pogo_e02/pogo_e02.c | 2 +- board/d-link/dns325/dns325.c | 2 +- board/keymile/km_arm/km_arm.c | 6 +- board/raidsonic/ib62x0/ib62x0.c | 2 +- drivers/spi/kirkwood_spi.c | 64 +++++++++++++++++++---- 17 files changed, 90 insertions(+), 27 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) { + sel_save = (mpp_ctrl[num / 8] >> shift) & 0xf; + *mpp_save = num | (sel_save << 8) | variant_mask; + mpp_save++; + } + mpp_ctrl[num / 8] &= ~(0xf << shift); mpp_ctrl[num / 8] |= sel << shift; diff --git a/arch/arm/include/asm/arch-kirkwood/mpp.h b/arch/arm/include/asm/arch-kirkwood/mpp.h index b3c090e..8e50ee7 100644 --- a/arch/arm/include/asm/arch-kirkwood/mpp.h +++ b/arch/arm/include/asm/arch-kirkwood/mpp.h @@ -312,6 +312,6 @@ #define MPP_MAX 49 -void kirkwood_mpp_conf(unsigned int *mpp_list); +void kirkwood_mpp_conf(u32 *mpp_list, u32 *mpp_save); #endif
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@keymile.com> cc: Holger Brunck <holger.brunck@keymile.com> cc: Prafulla Wadaskar <prafulla@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(-)