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

[Valentin Longchamp]

>> -----Original Message-----
>> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
>> Sent: 29 May 2012 14:15
>> To: Prafulla Wadaskar
>> Cc: holger.brunck at keymile.com; u-boot at 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.