[U-Boot] [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function

[Valentin Longchamp]

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