[U-Boot] [PATCH v3 1/5] kirkwood: add save functionality kirkwood_mpp_conf function

[Valentin Longchamp]

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 at keymile.com>
>> cc: Holger Brunck <holger.brunck at keymile.com>
>> cc: Prafulla Wadaskar <prafulla at 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.