[U-Boot,1/3] kirkwood: add kirkwood_mpp_save/restore functions

These 2 functions can be used in pair if one needs to set a mpp
configuration only for a given time and then switch back to the previous
mpp config.

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    |   18 ++++++++++++++++++
 arch/arm/include/asm/arch-kirkwood/mpp.h |    2 ++
 2 files changed, 20 insertions(+), 0 deletions(-)

> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com]
> Sent: 16 May 2012 16:24
> To: Prafulla Wadaskar; holger.brunck@keymile.com
> Cc: Valentin Longchamp; u-boot@lists.denx.de; Holger Brunck; Prafulla
> Wadaskar
> Subject: [PATCH 1/3] kirkwood: add kirkwood_mpp_save/restore functions
> 
> These 2 functions can be used in pair if one needs to set a mpp
> configuration only for a given time and then switch back to the
> previous
> mpp config.
> 
> 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    |   18 ++++++++++++++++++
>  arch/arm/include/asm/arch-kirkwood/mpp.h |    2 ++
>  2 files changed, 20 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> index 3da6c98..43f5053 100644
> --- a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> +++ b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> @@ -80,3 +80,21 @@ void kirkwood_mpp_conf(u32 *mpp_list)
>  	debug("\n");
> 
>  }
> +
> +u32 mpp_regs[MPP_NR_REGS];

This is optional feature only used in this specific case.
No global here, this should be part of caller function.

> +
> +void kirkwood_mpp_save(void)
This should be 
void kirkwood_mpp_save(unsigned int *mpp_ctrl, int len)

> +{
> +	int i;
> +
> +	for (i = 0; i < MPP_NR_REGS; i++)
> +		mpp_regs[i] = readl(MPP_CTRL(i));
> +}
> +
> +void kirkwood_mpp_restore(void)
Same here
void kirkwood_mpp_restore(unsigned int *mpp_ctrl, int len)

> +{
> +	int i;
> +
> +	for (i = 0; i < MPP_NR_REGS; i++)
> +		writel(mpp_regs[i], MPP_CTRL(i));
> +}
> diff --git a/arch/arm/include/asm/arch-kirkwood/mpp.h
> b/arch/arm/include/asm/arch-kirkwood/mpp.h
> index b3c090e..da65b4d 100644
> --- a/arch/arm/include/asm/arch-kirkwood/mpp.h
> +++ b/arch/arm/include/asm/arch-kirkwood/mpp.h
> @@ -313,5 +313,7 @@
>  #define MPP_MAX			49
> 
>  void kirkwood_mpp_conf(unsigned int *mpp_list);
> +void kirkwood_mpp_save(void);
> +void kirkwood_mpp_restore(void);

Regards..
Prafulla . . .

Am Donnerstag 24 Mai 2012, 10:26:44 schrieb Prafulla Wadaskar:
> > +
> > +void kirkwood_mpp_save(void)
> 
> This should be
> void kirkwood_mpp_save(unsigned int *mpp_ctrl, int len)
> 
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < MPP_NR_REGS; i++)
> > +		mpp_regs[i] = readl(MPP_CTRL(i));
> > +}
> > +
> > +void kirkwood_mpp_restore(void)
> 
> Same here
> void kirkwood_mpp_restore(unsigned int *mpp_ctrl, int len)
or even better kirkwood_mpp_restore(const unsigned int *mpp_ctrl, int len)

btw. sometimes "unsigned int" is used and sometimes "u32", could we agree on 
one? Eg. kirkwood_mpp_conf() is declared with unsigned int but defined with 
u32.

On 05/24/2012 10:26 AM, Prafulla Wadaskar wrote:
> 
> 
>> -----Original Message-----
>> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com]
>> Sent: 16 May 2012 16:24
>> To: Prafulla Wadaskar; holger.brunck@keymile.com
>> Cc: Valentin Longchamp; u-boot@lists.denx.de; Holger Brunck; Prafulla
>> Wadaskar
>> Subject: [PATCH 1/3] kirkwood: add kirkwood_mpp_save/restore functions
>>
>> These 2 functions can be used in pair if one needs to set a mpp
>> configuration only for a given time and then switch back to the
>> previous
>> mpp config.
>>
>> 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    |   18 ++++++++++++++++++
>>  arch/arm/include/asm/arch-kirkwood/mpp.h |    2 ++
>>  2 files changed, 20 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
>> b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
>> index 3da6c98..43f5053 100644
>> --- a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
>> +++ b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
>> @@ -80,3 +80,21 @@ void kirkwood_mpp_conf(u32 *mpp_list)
>>  	debug("\n");
>>
>>  }
>> +
>> +u32 mpp_regs[MPP_NR_REGS];
> 
> This is optional feature only used in this specific case.
> No global here, this should be part of caller function.

I wanted this patch to be independant of the SPI claim bus that _currently_ is
the only one using it, but who knows in the future.

Anyway if this isn't global here, it will have to be global in the
kirkwood_spi.c driver since it would have to be shared between spi_claim_bus and
spi_release_bus.

> 
>> +
>> +void kirkwood_mpp_save(void)
> This should be 
> void kirkwood_mpp_save(unsigned int *mpp_ctrl, int len)

Here we save _all_ mpp registers, with direct access to the registers. With your
proposed solution, I would save it in a dynamically allocated table of size len.

That's fine for me, but I would then need to export MPP_NR_REGS, because that is
what I would pass as len arg, is that OK ?

> 
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < MPP_NR_REGS; i++)
>> +		mpp_regs[i] = readl(MPP_CTRL(i));
>> +}
>> +
>> +void kirkwood_mpp_restore(void)
> Same here
> void kirkwood_mpp_restore(unsigned int *mpp_ctrl, int len)
> 
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < MPP_NR_REGS; i++)
>> +		writel(mpp_regs[i], MPP_CTRL(i));
>> +}
>> diff --git a/arch/arm/include/asm/arch-kirkwood/mpp.h
>> b/arch/arm/include/asm/arch-kirkwood/mpp.h
>> index b3c090e..da65b4d 100644
>> --- a/arch/arm/include/asm/arch-kirkwood/mpp.h
>> +++ b/arch/arm/include/asm/arch-kirkwood/mpp.h
>> @@ -313,5 +313,7 @@
>>  #define MPP_MAX			49
>>
>>  void kirkwood_mpp_conf(unsigned int *mpp_list);
>> +void kirkwood_mpp_save(void);
>> +void kirkwood_mpp_restore(void);
> 
> Regards..
> Prafulla . . .

> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com]
> Sent: 29 May 2012 14:15
> To: Prafulla Wadaskar
> Cc: holger.brunck@keymile.com; u-boot@lists.denx.de
> Subject: Re: [PATCH 1/3] kirkwood: add kirkwood_mpp_save/restore
> functions
> 
...snip...
> >
> >> +
> >> +void kirkwood_mpp_save(void)
> > This should be
> > void kirkwood_mpp_save(unsigned int *mpp_ctrl, int len)
> 
> Here we save _all_ mpp registers, with direct access to the registers.
> With your
> proposed solution, I would save it in a dynamically allocated table of
> size len.
> 
> That's fine for me, but I would then need to export MPP_NR_REGS,
> because that is
> what I would pass as len arg, is that OK ?

I think in your case you need configuration of 4 MPPs, i.e. from MPP6 to MPP11, so you may declare array of length 7 and backup and restore the same MPPs using len = 6.

No Need to backup and restore all MPPs.

Regards...
Prafulla . . .

>> -----Original Message-----
>> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com]
>> Sent: 29 May 2012 14:15
>> To: Prafulla Wadaskar
>> Cc: holger.brunck@keymile.com; u-boot@lists.denx.de
>> Subject: Re: [PATCH 1/3] kirkwood: add kirkwood_mpp_save/restore
>> functions
>>
> ...snip...
>>>
>>>> +
>>>> +void kirkwood_mpp_save(void)
>>> This should be
>>> void kirkwood_mpp_save(unsigned int *mpp_ctrl, int len)
>>
>> Here we save _all_ mpp registers, with direct access to the registers.
>> With your
>> proposed solution, I would save it in a dynamically allocated table of
>> size len.
>>
>> That's fine for me, but I would then need to export MPP_NR_REGS,
>> because that is
>> what I would pass as len arg, is that OK ?
> 
> I think in your case you need configuration of 4 MPPs, i.e. from MPP6 to MPP11, so you may declare array of length 7 and backup and restore the same MPPs using len = 6.
> 
> No Need to backup and restore all MPPs.
> 

Sorry, but this is exactly what you did in the kirkwood_mpp_conf function, you
read and write all the registers every time you need to change only one pin, I
took it from there for consistency:

> 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar  2009-06-20 11:01:53 +0200  76)       for (i = 0; i < MPP_NR_REGS; i++) {
> 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar  2009-06-20 11:01:53 +0200  77)               writel(mpp_ctrl[i], MPP_CTRL(i));
> 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar  2009-06-20 11:01:53 +0200  78)               debug(" %08x", mpp_ctrl[i]);
> 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar  2009-06-20 11:01:53 +0200  79)       }

And the way you did it is logical, if it was not done like that, a lot of
reading/masking/rewriting would be needed, and this for every single pin, so it
would be much less efficient than just reading all the regs and write them all back.

> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com]
> Sent: 29 May 2012 16:59
> To: Prafulla Wadaskar
> Cc: holger.brunck@keymile.com; u-boot@lists.denx.de
> Subject: Re: [PATCH 1/3] kirkwood: add kirkwood_mpp_save/restore
> functions
> 
> >> -----Original Message-----
> >> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com]
> >> Sent: 29 May 2012 14:15
> >> To: Prafulla Wadaskar
> >> Cc: holger.brunck@keymile.com; u-boot@lists.denx.de
> >> Subject: Re: [PATCH 1/3] kirkwood: add kirkwood_mpp_save/restore
> >> functions
> >>
> > ...snip...
> >>>
> >>>> +
> >>>> +void kirkwood_mpp_save(void)
> >>> This should be
> >>> void kirkwood_mpp_save(unsigned int *mpp_ctrl, int len)
> >>
> >> Here we save _all_ mpp registers, with direct access to the
> registers.
> >> With your
> >> proposed solution, I would save it in a dynamically allocated table
> of
> >> size len.
> >>
> >> That's fine for me, but I would then need to export MPP_NR_REGS,
> >> because that is
> >> what I would pass as len arg, is that OK ?
> >
> > I think in your case you need configuration of 4 MPPs, i.e. from
> MPP6 to MPP11, so you may declare array of length 7 and backup and
> restore the same MPPs using len = 6.
> >
> > No Need to backup and restore all MPPs.
> >
> 
> Sorry, but this is exactly what you did in the kirkwood_mpp_conf
> function, you
> read and write all the registers every time you need to change only
> one pin, I
> took it from there for consistency:
> 
> > 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar
> 2009-06-20 11:01:53 +0200  76)       for (i = 0; i < MPP_NR_REGS; i++)
> {
> > 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar
> 2009-06-20 11:01:53 +0200  77)               writel(mpp_ctrl[i],
> MPP_CTRL(i));
> > 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar
> 2009-06-20 11:01:53 +0200  78)               debug(" %08x",
> mpp_ctrl[i]);
> > 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar
> 2009-06-20 11:01:53 +0200  79)       }
> 
> And the way you did it is logical, if it was not done like that, a lot
> of
> reading/masking/rewriting would be needed, and this for every single
> pin, so it
> would be much less efficient than just reading all the regs and write
> them all back.

Yes, but the idea is that mpp_config function is called just once during initialization, we have exposed a simple array to do this configuration.

but save/restore will be called very frequently, may be for each SPI transaction. So there must be some optimisation.
Secondly, we should only tweak only needed MPPs in run time, why all? It may lead to some other side effects (I don't know).

Regards..
Prafulla . . .

> -----Original Message-----
> From: Michael Walle [mailto:michael@walle.cc]
> Sent: 29 May 2012 03:38
> To: u-boot@lists.denx.de
> Cc: Prafulla Wadaskar; Valentin Longchamp; holger.brunck@keymile.com
> Subject: Re: [U-Boot] [PATCH 1/3] kirkwood: add
> kirkwood_mpp_save/restore functions
> 
> Am Donnerstag 24 Mai 2012, 10:26:44 schrieb Prafulla Wadaskar:
> > > +
> > > +void kirkwood_mpp_save(void)
> >
> > This should be
> > void kirkwood_mpp_save(unsigned int *mpp_ctrl, int len)
> >
> > > +{
> > > +	int i;
> > > +
> > > +	for (i = 0; i < MPP_NR_REGS; i++)
> > > +		mpp_regs[i] = readl(MPP_CTRL(i));
> > > +}
> > > +
> > > +void kirkwood_mpp_restore(void)
> >
> > Same here
> > void kirkwood_mpp_restore(unsigned int *mpp_ctrl, int len)
> or even better kirkwood_mpp_restore(const unsigned int *mpp_ctrl, int
> len)
> 
> btw. sometimes "unsigned int" is used and sometimes "u32", could we
> agree on
> one? Eg. kirkwood_mpp_conf() is declared with unsigned int but defined
> with
> u32.

Hi Michael

Thanks for your comments.

Yes, we should use u32.
And const will be a problem since mpp_ctrl will be array that will be dynamically modified.

Regards..
Prafulla . . .

On 05/29/2012 02:06 PM, Prafulla Wadaskar wrote:
> 
> 
>> -----Original Message-----
>> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com]
>> Sent: 29 May 2012 16:59
>> To: Prafulla Wadaskar
>> Cc: holger.brunck@keymile.com; u-boot@lists.denx.de
>> Subject: Re: [PATCH 1/3] kirkwood: add kirkwood_mpp_save/restore
>> functions
>>
>>>> -----Original Message-----
>>>> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com]
>>>> Sent: 29 May 2012 14:15
>>>> To: Prafulla Wadaskar
>>>> Cc: holger.brunck@keymile.com; u-boot@lists.denx.de
>>>> Subject: Re: [PATCH 1/3] kirkwood: add kirkwood_mpp_save/restore
>>>> functions
>>>>
>>> ...snip...
>>>>>
>>>>>> +
>>>>>> +void kirkwood_mpp_save(void)
>>>>> This should be
>>>>> void kirkwood_mpp_save(unsigned int *mpp_ctrl, int len)
>>>>
>>>> Here we save _all_ mpp registers, with direct access to the
>> registers.
>>>> With your
>>>> proposed solution, I would save it in a dynamically allocated table
>> of
>>>> size len.
>>>>
>>>> That's fine for me, but I would then need to export MPP_NR_REGS,
>>>> because that is
>>>> what I would pass as len arg, is that OK ?
>>>
>>> I think in your case you need configuration of 4 MPPs, i.e. from
>> MPP6 to MPP11, so you may declare array of length 7 and backup and
>> restore the same MPPs using len = 6.
>>>
>>> No Need to backup and restore all MPPs.
>>>
>>
>> Sorry, but this is exactly what you did in the kirkwood_mpp_conf
>> function, you
>> read and write all the registers every time you need to change only
>> one pin, I
>> took it from there for consistency:
>>
>>> 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar
>> 2009-06-20 11:01:53 +0200  76)       for (i = 0; i < MPP_NR_REGS; i++)
>> {
>>> 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar
>> 2009-06-20 11:01:53 +0200  77)               writel(mpp_ctrl[i],
>> MPP_CTRL(i));
>>> 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar
>> 2009-06-20 11:01:53 +0200  78)               debug(" %08x",
>> mpp_ctrl[i]);
>>> 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar
>> 2009-06-20 11:01:53 +0200  79)       }
>>
>> And the way you did it is logical, if it was not done like that, a lot
>> of
>> reading/masking/rewriting would be needed, and this for every single
>> pin, so it
>> would be much less efficient than just reading all the regs and write
>> them all back.
> 
> Yes, but the idea is that mpp_config function is called just once during initialization, we have exposed a simple array to do this configuration.
> 
> but save/restore will be called very frequently, may be for each SPI transaction. So there must be some optimisation.

> Secondly, we should only tweak only needed MPPs in run time, why all? It may lead to some other side effects (I don't know).
> 

With the proposed code, all are saved and restored, but not all are tweaked.
Furthermore, maybe reading and writing back 7 registers and is more efficient
than determining which one of the 7 have to be read/written back and then
performing the accesses.

However, if you tell me that mpp_config function is supposed to be called just
once during initialization, I will propose a new function that optimizes these
reg accesses as you would prefer it, but the best optimization potential is to
avoid to call the 2 mpp_config and mpp_save functions.

> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com]
> Sent: 29 May 2012 18:20
> To: Prafulla Wadaskar
> Cc: holger.brunck@keymile.com; u-boot@lists.denx.de
> Subject: Re: [PATCH 1/3] kirkwood: add kirkwood_mpp_save/restore
> functions
> 
> On 05/29/2012 02:06 PM, Prafulla Wadaskar wrote:
> >
> >
> >> -----Original Message-----
> >> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com]
> >> Sent: 29 May 2012 16:59
> >> To: Prafulla Wadaskar
> >> Cc: holger.brunck@keymile.com; u-boot@lists.denx.de
> >> Subject: Re: [PATCH 1/3] kirkwood: add kirkwood_mpp_save/restore
> >> functions
> >>
> >>>> -----Original Message-----
> >>>> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com]
> >>>> Sent: 29 May 2012 14:15
> >>>> To: Prafulla Wadaskar
> >>>> Cc: holger.brunck@keymile.com; u-boot@lists.denx.de
> >>>> Subject: Re: [PATCH 1/3] kirkwood: add kirkwood_mpp_save/restore
> >>>> functions
> >>>>
> >>> ...snip...
> >>>>>
> >>>>>> +
> >>>>>> +void kirkwood_mpp_save(void)
> >>>>> This should be
> >>>>> void kirkwood_mpp_save(unsigned int *mpp_ctrl, int len)
> >>>>
> >>>> Here we save _all_ mpp registers, with direct access to the
> >> registers.
> >>>> With your
> >>>> proposed solution, I would save it in a dynamically allocated
> table
> >> of
> >>>> size len.
> >>>>
> >>>> That's fine for me, but I would then need to export MPP_NR_REGS,
> >>>> because that is
> >>>> what I would pass as len arg, is that OK ?
> >>>
> >>> I think in your case you need configuration of 4 MPPs, i.e. from
> >> MPP6 to MPP11, so you may declare array of length 7 and backup and
> >> restore the same MPPs using len = 6.
> >>>
> >>> No Need to backup and restore all MPPs.
> >>>
> >>
> >> Sorry, but this is exactly what you did in the kirkwood_mpp_conf
> >> function, you
> >> read and write all the registers every time you need to change only
> >> one pin, I
> >> took it from there for consistency:
> >>
> >>> 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar
> >> 2009-06-20 11:01:53 +0200  76)       for (i = 0; i < MPP_NR_REGS;
> i++)
> >> {
> >>> 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar
> >> 2009-06-20 11:01:53 +0200  77)               writel(mpp_ctrl[i],
> >> MPP_CTRL(i));
> >>> 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar
> >> 2009-06-20 11:01:53 +0200  78)               debug(" %08x",
> >> mpp_ctrl[i]);
> >>> 4efb77d4 cpu/arm926ejs/kirkwood/mpp.c          (Prafulla Wadaskar
> >> 2009-06-20 11:01:53 +0200  79)       }
> >>
> >> And the way you did it is logical, if it was not done like that, a
> lot
> >> of
> >> reading/masking/rewriting would be needed, and this for every
> single
> >> pin, so it
> >> would be much less efficient than just reading all the regs and
> write
> >> them all back.
> >
> > Yes, but the idea is that mpp_config function is called just once
> during initialization, we have exposed a simple array to do this
> configuration.
> >
> > but save/restore will be called very frequently, may be for each SPI
> transaction. So there must be some optimisation.
> 
> > Secondly, we should only tweak only needed MPPs in run time, why
> all? It may lead to some other side effects (I don't know).
> >
> 
> With the proposed code, all are saved and restored, but not all are
> tweaked.
> Furthermore, maybe reading and writing back 7 registers and is more
> efficient
> than determining which one of the 7 have to be read/written back and
> then
> performing the accesses.
> 
> However, if you tell me that mpp_config function is supposed to be
> called just
> once during initialization, I will propose a new function that
> optimizes these

New ideas are always welcomed, let's keep is separate from this context.

> reg accesses as you would prefer it, but the best optimization
> potential is to
> avoid to call the 2 mpp_config and mpp_save functions.

I think, Kirkwood_mpp_config (already supported) finally writes the said MPP configurations to the SoC,

If we can have just Kirkwood_mpp_read() function that reads the MPP configuration for the said MPP will be enough from mpp.c prospective.
This function should return the MPP config value in the same way it is being programmed by Kirkwood_mpp_config() function.

In SPI driver we will use these functions to handle the claim/release/backup the MPP attributes.

Regards..
Prafulla . . .

Dear Prafulla Wadaskar,

this is offtopic in this thread, but I tried contacting you about thrice by now 
via email, maybe you didn't get those mails. To get quickly to the point, can 
you please try cleaning up the patches in patchwork?

Thanks!

Best regards,
Marek Vasut

Am Dienstag 29 Mai 2012, 14:42:08 schrieb Prafulla Wadaskar:
> And const will be a problem since mpp_ctrl will be array that will be
> dynamically modified.
But not within kirkwood_mpp_restore(), right? So an mpp_ctrl array would be 
implicitly casted from u32* to const u32*.

so we would have the following declarations:
 kirkwood_mpp_restore(const u32 *mpp_list)
 kirkwood_mpp_conf(const u32 *mpp_list)
 kirkwood_mpp_save(u32 *mpp_list)

where only the latter modifies mpp_list.

Dear Marek

I did few cleanups in patchwork?
I will check if further more is needed.

Regards..
Prafulla . . .


> -----Original Message-----
> From: Marek Vasut [mailto:marek.vasut@gmail.com]
> Sent: 29 May 2012 20:17
> To: u-boot@lists.denx.de
> Cc: Prafulla Wadaskar; Valentin Longchamp; holger.brunck@keymile.com
> Subject: Re: [U-Boot] [PATCH 1/3] kirkwood: add
> kirkwood_mpp_save/restore functions
> 
> Dear Prafulla Wadaskar,
> 
> this is offtopic in this thread, but I tried contacting you about
> thrice by now
> via email, maybe you didn't get those mails. To get quickly to the
> point, can
> you please try cleaning up the patches in patchwork?
> 
> Thanks!
> 
> Best regards,
> Marek Vasut

Dear Prafulla Wadaskar,

> Dear Marek
> 
> I did few cleanups in patchwork?
> I will check if further more is needed.

I just checked, it seems it's all gone now, only a few crumbles left :)

Sorry I kept pestering you so much.

Thanks a lot for cleaning it up!

> 
> Regards..
> Prafulla . . .
> 
> > -----Original Message-----
> > From: Marek Vasut [mailto:marek.vasut@gmail.com]
> > Sent: 29 May 2012 20:17
> > To: u-boot@lists.denx.de
> > Cc: Prafulla Wadaskar; Valentin Longchamp; holger.brunck@keymile.com
> > Subject: Re: [U-Boot] [PATCH 1/3] kirkwood: add
> > kirkwood_mpp_save/restore functions
> > 
> > Dear Prafulla Wadaskar,
> > 
> > this is offtopic in this thread, but I tried contacting you about
> > thrice by now
> > via email, maybe you didn't get those mails. To get quickly to the
> > point, can
> > you please try cleaning up the patches in patchwork?
> > 
> > Thanks!
> > 
> > Best regards,
> > Marek Vasut

Best regards,
Marek Vasut