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

[Valentin Longchamp <valentin.longchamp@keymile.com>]

On 05/24/2012 10:26 AM, Prafulla Wadaskar wrote:
> 
> 
>> -----Original Message-----
>> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
>> Sent: 16 May 2012 16:24
>> To: Prafulla Wadaskar; holger.brunck at keymile.com
>> Cc: Valentin Longchamp; u-boot at 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 . . .