| Message ID | 1338392526-5754-2-git-send-email-valentin.longchamp@keymile.com |
|---|---|
| State | Superseded |
| Headers | show |
> -----Original Message----- > From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com] > Sent: 30 May 2012 21:12 > To: Prafulla Wadaskar > Cc: Valentin Longchamp; holger.brunck@keymile.com; u- > boot@lists.denx.de; Prafulla Wadaskar > Subject: [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function > > This function can be used to save current mpp state of all mpp pins > given in the mpp_list argument by reading the mpp registers, in the > second mpp_saved argument. > > A later call to kirkwood_mpp_conf function with this saved list will > reset the mpp configuration as it was when kirkwood_mpp_read was > called. > > 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 | 41 > ++++++++++++++++++++++++++++++ > arch/arm/include/asm/arch-kirkwood/mpp.h | 1 + > 2 files changed, 42 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c > b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c > index 3da6c98..9fb9aea 100644 > --- a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c > +++ b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c > @@ -80,3 +80,44 @@ void kirkwood_mpp_conf(u32 *mpp_list) > debug("\n"); > > } > + > +void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved) > +{ > + u32 mpp_ctrl[MPP_NR_REGS]; > + unsigned int variant_mask; > + int i; > + > + variant_mask = kirkwood_variant(); > + if (!variant_mask) > + return; > + > + for (i = 0; i < MPP_NR_REGS; i++) { > + mpp_ctrl[i] = readl(MPP_CTRL(i)); > + debug(" %08x", mpp_ctrl[i]); > + } > + > + while (*mpp_list) { > + unsigned int num = MPP_NUM(*mpp_list); > + unsigned int sel; > + int shift; > + > + if (num > MPP_MAX) { > + debug("kirkwood_mpp_conf: invalid MPP " > + "number (%u)\n", num); > + continue; > + } > + if (!(*mpp_list & variant_mask)) { > + debug("kirkwood_mpp_conf: requested MPP%u config " > + "unavailable on this hardware\n", num); > + continue; > + } > + > + shift = (num & 7) << 2; > + sel = (mpp_ctrl[num / 8] >> shift) & 0xf; > + *mpp_saved = num | (sel << 8) | variant_mask; > + > + mpp_list++; > + mpp_saved++; > + } > +} > + Hi Valentin There is code duplication, similar code it already there in function kirkwood_mpp_conf(), to make it short you should use kirkwood_mpp_read function within kirkwood_mpp_conf Regards.. Prafulla . . . > diff --git a/arch/arm/include/asm/arch-kirkwood/mpp.h > b/arch/arm/include/asm/arch-kirkwood/mpp.h > index b3c090e..22a64b8 100644 > --- a/arch/arm/include/asm/arch-kirkwood/mpp.h > +++ b/arch/arm/include/asm/arch-kirkwood/mpp.h > @@ -313,5 +313,6 @@ > #define MPP_MAX 49 > > void kirkwood_mpp_conf(unsigned int *mpp_list); > +void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved); > > #endif > -- > 1.7.1
Hi Prafulla, On 05/31/2012 10:30 AM, Prafulla Wadaskar wrote: >> -----Original Message----- >> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com] >> Sent: 30 May 2012 21:12 >> To: Prafulla Wadaskar >> Cc: Valentin Longchamp; holger.brunck@keymile.com; u- >> boot@lists.denx.de; Prafulla Wadaskar >> Subject: [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function >> >> This function can be used to save current mpp state of all mpp pins >> given in the mpp_list argument by reading the mpp registers, in the >> second mpp_saved argument. >> >> A later call to kirkwood_mpp_conf function with this saved list will >> reset the mpp configuration as it was when kirkwood_mpp_read was >> called. >> >> 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 | 41 >> ++++++++++++++++++++++++++++++ >> arch/arm/include/asm/arch-kirkwood/mpp.h | 1 + >> 2 files changed, 42 insertions(+), 0 deletions(-) >> >> diff --git a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c >> b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c >> index 3da6c98..9fb9aea 100644 >> --- a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c >> +++ b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c >> @@ -80,3 +80,44 @@ void kirkwood_mpp_conf(u32 *mpp_list) >> debug("\n"); >> >> } >> + >> +void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved) >> +{ >> + u32 mpp_ctrl[MPP_NR_REGS]; >> + unsigned int variant_mask; >> + int i; >> + >> + variant_mask = kirkwood_variant(); >> + if (!variant_mask) >> + return; >> + >> + for (i = 0; i < MPP_NR_REGS; i++) { >> + mpp_ctrl[i] = readl(MPP_CTRL(i)); >> + debug(" %08x", mpp_ctrl[i]); >> + } >> + >> + while (*mpp_list) { >> + unsigned int num = MPP_NUM(*mpp_list); >> + unsigned int sel; >> + int shift; >> + >> + if (num > MPP_MAX) { >> + debug("kirkwood_mpp_conf: invalid MPP " >> + "number (%u)\n", num); >> + continue; >> + } >> + if (!(*mpp_list & variant_mask)) { >> + debug("kirkwood_mpp_conf: requested MPP%u config " >> + "unavailable on this hardware\n", num); >> + continue; >> + } >> + >> + shift = (num & 7) << 2; >> + sel = (mpp_ctrl[num / 8] >> shift) & 0xf; >> + *mpp_saved = num | (sel << 8) | variant_mask; >> + >> + mpp_list++; >> + mpp_saved++; >> + } >> +} >> + > > Hi Valentin > There is code duplication, similar code it already there in function kirkwood_mpp_conf(), to make it short you should use kirkwood_mpp_read function within kirkwood_mpp_conf > Not sure I understand what you mean. You want me to implement the kirkwood_mpp_read functionnality directly into kirkwood_mpp_conf ? If this is so, it would mean that I would have to change kirkwood_mpp_conf "API" to add the second argument (mpp_saved) and then I would have to fix all the calls to this function. Is that what you mean ? Val
> -----Original Message----- > From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com] > Sent: 31 May 2012 14:14 > To: Prafulla Wadaskar > Cc: holger.brunck@keymile.com; u-boot@lists.denx.de > Subject: Re: [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function > > Hi Prafulla, > > On 05/31/2012 10:30 AM, Prafulla Wadaskar wrote: > >> -----Original Message----- > >> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com] > >> Sent: 30 May 2012 21:12 > >> To: Prafulla Wadaskar > >> Cc: Valentin Longchamp; holger.brunck@keymile.com; u- > >> boot@lists.denx.de; Prafulla Wadaskar > >> Subject: [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function > >> > >> This function can be used to save current mpp state of all mpp pins > >> given in the mpp_list argument by reading the mpp registers, in the > >> second mpp_saved argument. > >> > >> A later call to kirkwood_mpp_conf function with this saved list > will > >> reset the mpp configuration as it was when kirkwood_mpp_read was > >> called. > >> > >> 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 | 41 > >> ++++++++++++++++++++++++++++++ > >> arch/arm/include/asm/arch-kirkwood/mpp.h | 1 + > >> 2 files changed, 42 insertions(+), 0 deletions(-) > >> > >> diff --git a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c > >> b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c > >> index 3da6c98..9fb9aea 100644 > >> --- a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c > >> +++ b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c > >> @@ -80,3 +80,44 @@ void kirkwood_mpp_conf(u32 *mpp_list) > >> debug("\n"); > >> > >> } > >> + > >> +void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved) > >> +{ > >> + u32 mpp_ctrl[MPP_NR_REGS]; > >> + unsigned int variant_mask; > >> + int i; > >> + > >> + variant_mask = kirkwood_variant(); > >> + if (!variant_mask) > >> + return; > >> + > >> + for (i = 0; i < MPP_NR_REGS; i++) { > >> + mpp_ctrl[i] = readl(MPP_CTRL(i)); > >> + debug(" %08x", mpp_ctrl[i]); > >> + } > >> + > >> + while (*mpp_list) { > >> + unsigned int num = MPP_NUM(*mpp_list); > >> + unsigned int sel; > >> + int shift; > >> + > >> + if (num > MPP_MAX) { > >> + debug("kirkwood_mpp_conf: invalid MPP " > >> + "number (%u)\n", num); >> + continue; > >> + } > >> + if (!(*mpp_list & variant_mask)) { > >> + debug("kirkwood_mpp_conf: requested MPP%u config " > >> + "unavailable on this hardware\n", num); > >> + continue; > >> + } > >> + > >> + shift = (num & 7) << 2; > >> + sel = (mpp_ctrl[num / 8] >> shift) & 0xf; > >> + *mpp_saved = num | (sel << 8) | variant_mask; > >> + > >> + mpp_list++; > >> + mpp_saved++; > >> + } > >> +} > >> + > > > > Hi Valentin > > There is code duplication, similar code it already there in function > kirkwood_mpp_conf(), to make it short you should use kirkwood_mpp_read > function within kirkwood_mpp_conf > > > > Not sure I understand what you mean. You want me to implement the > kirkwood_mpp_read functionnality directly into kirkwood_mpp_conf ? Yes, > > If this is so, it would mean that I would have to change > kirkwood_mpp_conf "API" > to add the second argument (mpp_saved) and then I would have to fix > all the > calls to this function. Is that what you mean ? Yes, my objective here is - how good we can optimise the code. I will not stretch it further, it's up to you. Regards.. Prafulla . . .
On 05/31/2012 10:49 AM, Prafulla Wadaskar wrote: > > >> -----Original Message----- >> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com] >> Sent: 31 May 2012 14:14 >> To: Prafulla Wadaskar >> Cc: holger.brunck@keymile.com; u-boot@lists.denx.de >> Subject: Re: [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function >> >> Hi Prafulla, >> >> On 05/31/2012 10:30 AM, Prafulla Wadaskar wrote: >>>> -----Original Message----- >>>> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com] >>>> Sent: 30 May 2012 21:12 >>>> To: Prafulla Wadaskar >>>> Cc: Valentin Longchamp; holger.brunck@keymile.com; u- >>>> boot@lists.denx.de; Prafulla Wadaskar >>>> Subject: [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function >>>> >>>> This function can be used to save current mpp state of all mpp pins >>>> given in the mpp_list argument by reading the mpp registers, in the >>>> second mpp_saved argument. >>>> >>>> A later call to kirkwood_mpp_conf function with this saved list >> will >>>> reset the mpp configuration as it was when kirkwood_mpp_read was >>>> called. >>>> >>>> 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 | 41 >>>> ++++++++++++++++++++++++++++++ >>>> arch/arm/include/asm/arch-kirkwood/mpp.h | 1 + >>>> 2 files changed, 42 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c >>>> b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c >>>> index 3da6c98..9fb9aea 100644 >>>> --- a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c >>>> +++ b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c >>>> @@ -80,3 +80,44 @@ void kirkwood_mpp_conf(u32 *mpp_list) >>>> debug("\n"); >>>> >>>> } >>>> + >>>> +void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved) >>>> +{ >>>> + u32 mpp_ctrl[MPP_NR_REGS]; >>>> + unsigned int variant_mask; >>>> + int i; >>>> + >>>> + variant_mask = kirkwood_variant(); >>>> + if (!variant_mask) >>>> + return; >>>> + >>>> + for (i = 0; i < MPP_NR_REGS; i++) { >>>> + mpp_ctrl[i] = readl(MPP_CTRL(i)); >>>> + debug(" %08x", mpp_ctrl[i]); >>>> + } >>>> + >>>> + while (*mpp_list) { >>>> + unsigned int num = MPP_NUM(*mpp_list); >>>> + unsigned int sel; >>>> + int shift; >>>> + >>>> + if (num > MPP_MAX) { >>>> + debug("kirkwood_mpp_conf: invalid MPP " >>>> + "number (%u)\n", num); >>> + continue; >>>> + } >>>> + if (!(*mpp_list & variant_mask)) { >>>> + debug("kirkwood_mpp_conf: requested MPP%u config " >>>> + "unavailable on this hardware\n", num); >>>> + continue; >>>> + } >>>> + >>>> + shift = (num & 7) << 2; >>>> + sel = (mpp_ctrl[num / 8] >> shift) & 0xf; >>>> + *mpp_saved = num | (sel << 8) | variant_mask; >>>> + >>>> + mpp_list++; >>>> + mpp_saved++; >>>> + } >>>> +} >>>> + >>> >>> Hi Valentin >>> There is code duplication, similar code it already there in function >> kirkwood_mpp_conf(), to make it short you should use kirkwood_mpp_read >> function within kirkwood_mpp_conf >>> >> >> Not sure I understand what you mean. You want me to implement the >> kirkwood_mpp_read functionnality directly into kirkwood_mpp_conf ? > > Yes, > >> >> If this is so, it would mean that I would have to change >> kirkwood_mpp_conf "API" >> to add the second argument (mpp_saved) and then I would have to fix >> all the >> calls to this function. Is that what you mean ? > > Yes, my objective here is - how good we can optimise the code. > > I will not stretch it further, it's up to you. > OK, I have done it. Sending v3 of the series in a few minutes.
diff --git a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c index 3da6c98..9fb9aea 100644 --- a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c +++ b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c @@ -80,3 +80,44 @@ void kirkwood_mpp_conf(u32 *mpp_list) debug("\n"); } + +void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved) +{ + u32 mpp_ctrl[MPP_NR_REGS]; + unsigned int variant_mask; + int i; + + variant_mask = kirkwood_variant(); + if (!variant_mask) + return; + + for (i = 0; i < MPP_NR_REGS; i++) { + mpp_ctrl[i] = readl(MPP_CTRL(i)); + debug(" %08x", mpp_ctrl[i]); + } + + while (*mpp_list) { + unsigned int num = MPP_NUM(*mpp_list); + unsigned int sel; + int shift; + + if (num > MPP_MAX) { + debug("kirkwood_mpp_conf: invalid MPP " + "number (%u)\n", num); + continue; + } + if (!(*mpp_list & variant_mask)) { + debug("kirkwood_mpp_conf: requested MPP%u config " + "unavailable on this hardware\n", num); + continue; + } + + shift = (num & 7) << 2; + sel = (mpp_ctrl[num / 8] >> shift) & 0xf; + *mpp_saved = num | (sel << 8) | variant_mask; + + mpp_list++; + mpp_saved++; + } +} + diff --git a/arch/arm/include/asm/arch-kirkwood/mpp.h b/arch/arm/include/asm/arch-kirkwood/mpp.h index b3c090e..22a64b8 100644 --- a/arch/arm/include/asm/arch-kirkwood/mpp.h +++ b/arch/arm/include/asm/arch-kirkwood/mpp.h @@ -313,5 +313,6 @@ #define MPP_MAX 49 void kirkwood_mpp_conf(unsigned int *mpp_list); +void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved); #endif
This function can be used to save current mpp state of all mpp pins given in the mpp_list argument by reading the mpp registers, in the second mpp_saved argument. A later call to kirkwood_mpp_conf function with this saved list will reset the mpp configuration as it was when kirkwood_mpp_read was called. 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 | 41 ++++++++++++++++++++++++++++++ arch/arm/include/asm/arch-kirkwood/mpp.h | 1 + 2 files changed, 42 insertions(+), 0 deletions(-)