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

login
register
mail settings
Submitter Valentin Longchamp
Date May 30, 2012, 3:42 p.m.
Message ID <1338392526-5754-2-git-send-email-valentin.longchamp@keymile.com>
Download mbox | patch
Permalink /patch/162012/
State Superseded
Headers show

Comments

Valentin Longchamp - May 30, 2012, 3:42 p.m.
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@keymile.com>
cc: Holger Brunck <holger.brunck@keymile.com>
cc: Prafulla Wadaskar <prafulla@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(-)
Prafulla Wadaskar - May 31, 2012, 8:30 a.m.
> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com]
> Sent: 30 May 2012 21:12
> To: Prafulla Wadaskar
> Cc: Valentin Longchamp; holger.brunck@keymile.com; u-
> boot@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@keymile.com>
> cc: Holger Brunck <holger.brunck@keymile.com>
> cc: Prafulla Wadaskar <prafulla@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

Regards..
Prafulla . . .

> diff --git a/arch/arm/include/asm/arch-kirkwood/mpp.h
> b/arch/arm/include/asm/arch-kirkwood/mpp.h
> index b3c090e..22a64b8 100644
> --- a/arch/arm/include/asm/arch-kirkwood/mpp.h
> +++ b/arch/arm/include/asm/arch-kirkwood/mpp.h
> @@ -313,5 +313,6 @@
>  #define MPP_MAX			49
> 
>  void kirkwood_mpp_conf(unsigned int *mpp_list);
> +void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved);
> 
>  #endif
> --
> 1.7.1
Valentin Longchamp - May 31, 2012, 8:43 a.m.
Hi Prafulla,

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

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 ?

Val
Prafulla Wadaskar - May 31, 2012, 8:49 a.m.
> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com]
> Sent: 31 May 2012 14:14
> To: Prafulla Wadaskar
> Cc: holger.brunck@keymile.com; u-boot@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@keymile.com]
> >> Sent: 30 May 2012 21:12
> >> To: Prafulla Wadaskar
> >> Cc: Valentin Longchamp; holger.brunck@keymile.com; u-
> >> boot@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@keymile.com>
> >> cc: Holger Brunck <holger.brunck@keymile.com>
> >> cc: Prafulla Wadaskar <prafulla@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.

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

Patch

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++;
+	}
+}
+
diff --git a/arch/arm/include/asm/arch-kirkwood/mpp.h b/arch/arm/include/asm/arch-kirkwood/mpp.h
index b3c090e..22a64b8 100644
--- a/arch/arm/include/asm/arch-kirkwood/mpp.h
+++ b/arch/arm/include/asm/arch-kirkwood/mpp.h
@@ -313,5 +313,6 @@ 
 #define MPP_MAX			49
 
 void kirkwood_mpp_conf(unsigned int *mpp_list);
+void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved);
 
 #endif