[U-Boot] [PATCH] MX51: Moved board specific values in config file

[U-Boot] [PATCH] MX51: Moved board specific values in config file

[Stefano Babic]

The lowlevel_init file contained some hard-coded values
to setup the RAM. These board related values are moved into
the board configuration file.

Signed-off-by: Stefano Babic <sbabic@denx.de>
---
 cpu/arm_cortexa8/mx51/lowlevel_init.S |    5 ++++-
 include/configs/mx51evk.h             |    3 +++
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/cpu/arm_cortexa8/mx51/lowlevel_init.S b/cpu/arm_cortexa8/mx51/lowlevel_init.S
index 31af9e2..1e393bb 100644
--- a/cpu/arm_cortexa8/mx51/lowlevel_init.S
+++ b/cpu/arm_cortexa8/mx51/lowlevel_init.S
@@ -158,6 +158,7 @@
 	/* Switch peripheral to PLL 3 */
 	ldr r0, =CCM_BASE_ADDR
 	ldr r1, =0x000010C0
+ 	orr r1,r1,#CONFIG_SYS_DDR_CLKSEL
 	str r1, [r0, #CLKCTL_CBCMR]
 	ldr r1, =0x13239145
 	str r1, [r0, #CLKCTL_CBCDR]
@@ -171,6 +172,7 @@
 	ldr r1, =0x19239145
 	str r1, [r0, #CLKCTL_CBCDR]
 	ldr r1, =0x000020C0
+	orr r1,r1,#CONFIG_SYS_DDR_CLKSEL
 	str r1, [r0, #CLKCTL_CBCMR]
 
 	mov r3, #DP_OP_216
@@ -201,9 +203,10 @@
 	/* setup the rest */
 	/* Use lp_apm (24MHz) source for perclk */
 	ldr r1, =0x000020C2
+	orr r1,r1,#CONFIG_SYS_DDR_CLKSEL
 	str r1, [r0, #CLKCTL_CBCMR]
 	/* ddr clock from PLL 1, all perclk dividers are 1 since using 24MHz */
-	ldr r1, =0x59E35100
+	ldr r1, =CONFIG_SYS_CLKTL_CBCDR
 	str r1, [r0, #CLKCTL_CBCDR]
 
 	/* Restore the default values in the Gate registers */
diff --git a/include/configs/mx51evk.h b/include/configs/mx51evk.h
index e2daeca..5096ab7 100644
--- a/include/configs/mx51evk.h
+++ b/include/configs/mx51evk.h
@@ -177,6 +177,9 @@
 #define PHYS_SDRAM_1		CSD0_BASE_ADDR
 #define PHYS_SDRAM_1_SIZE	(512 * 1024 * 1024)
 
+#define CONFIG_SYS_DDR_CLKSEL	0
+#define CONFIG_SYS_CLKTL_CBCDR	0x59E35100
+
 /*-----------------------------------------------------------------------
  * FLASH and environment organization
  */
-- 
1.6.3.3

[U-Boot] [PATCH] MX51: Moved board specific values in config file

[Tom]

Stefano Babic wrote:
> The lowlevel_init file contained some hard-coded values
> to setup the RAM. These board related values are moved into
> the board configuration file.

Why was only one value cleaned up?
Just from the patch, there are at least 4 more.

> 
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> ---
>  cpu/arm_cortexa8/mx51/lowlevel_init.S |    5 ++++-
>  include/configs/mx51evk.h             |    3 +++
>  2 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/cpu/arm_cortexa8/mx51/lowlevel_init.S b/cpu/arm_cortexa8/mx51/lowlevel_init.S
> index 31af9e2..1e393bb 100644
> --- a/cpu/arm_cortexa8/mx51/lowlevel_init.S
> +++ b/cpu/arm_cortexa8/mx51/lowlevel_init.S
> @@ -158,6 +158,7 @@
>  	/* Switch peripheral to PLL 3 */
>  	ldr r0, =CCM_BASE_ADDR
>  	ldr r1, =0x000010C0
> + 	orr r1,r1,#CONFIG_SYS_DDR_CLKSEL

git am complains about the leading space
Please remove
>  	str r1, [r0, #CLKCTL_CBCMR]
>  	ldr r1, =0x13239145
>  	str r1, [r0, #CLKCTL_CBCDR]
> @@ -171,6 +172,7 @@
>  	ldr r1, =0x19239145
>  	str r1, [r0, #CLKCTL_CBCDR]
>  	ldr r1, =0x000020C0
> +	orr r1,r1,#CONFIG_SYS_DDR_CLKSEL

This is a noop as it is defined as 0.
Is the value going to change ?

Tom

[U-Boot] [PATCH] MX51: Moved board specific values in config file

[Stefano Babic]

Tom wrote:
> Stefano Babic wrote:
>> The lowlevel_init file contained some hard-coded values
>> to setup the RAM. These board related values are moved into
>> the board configuration file.
> 
> Why was only one value cleaned up?
> Just from the patch, there are at least 4 more.

Well, I prefer do not move a lot of configuration values inside the
board config file and add only what it is really needed and when it is
needed.

The fixed values you have seen set the CCM in a known status before
updating it to the final value, and this procedure should be the same
for all boards, as I undestood from Freescale's doc.

The patch adds configuration values to select the clock for DDR and the
value of the related divider, and both depends on the board implementation.

>> +     orr r1,r1,#CONFIG_SYS_DDR_CLKSEL

I'll fix it, thanks.

> 
> git am complains about the leading space
> Please remove
>>      str r1, [r0, #CLKCTL_CBCMR]
>>      ldr r1, =0x13239145
>>      str r1, [r0, #CLKCTL_CBCDR]
>> @@ -171,6 +172,7 @@
>>      ldr r1, =0x19239145
>>      str r1, [r0, #CLKCTL_CBCDR]
>>      ldr r1, =0x000020C0
>> +    orr r1,r1,#CONFIG_SYS_DDR_CLKSEL
> 
> This is a noop as it is defined as 0.
> Is the value going to change ?

That is correct. The value selects which clock must be used for the DDR.
On the mx51evk, this value must be set to 0. However, I know already
another target that uses another clock source for the RAM.

Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] MX51: Moved board specific values in config file

[Tom]

Stefano Babic wrote:
> Tom wrote:
>> Stefano Babic wrote:
>>> The lowlevel_init file contained some hard-coded values
>>> to setup the RAM. These board related values are moved into
>>> the board configuration file.
>> Why was only one value cleaned up?
>> Just from the patch, there are at least 4 more.
> 
> Well, I prefer do not move a lot of configuration values inside the
> board config file and add only what it is really needed and when it is
> needed.
> 
> The fixed values you have seen set the CCM in a known status before
> updating it to the final value, and this procedure should be the same
> for all boards, as I undestood from Freescale's doc.
> 
> The patch adds configuration values to select the clock for DDR and the
> value of the related divider, and both depends on the board implementation.
> 
>>> +     orr r1,r1,#CONFIG_SYS_DDR_CLKSEL
> 
> I'll fix it, thanks.
> 
>> git am complains about the leading space
>> Please remove
>>>      str r1, [r0, #CLKCTL_CBCMR]
>>>      ldr r1, =0x13239145
>>>      str r1, [r0, #CLKCTL_CBCDR]
>>> @@ -171,6 +172,7 @@
>>>      ldr r1, =0x19239145
>>>      str r1, [r0, #CLKCTL_CBCDR]
>>>      ldr r1, =0x000020C0
>>> +    orr r1,r1,#CONFIG_SYS_DDR_CLKSEL
>> This is a noop as it is defined as 0.
>> Is the value going to change ?
> 
> That is correct. The value selects which clock must be used for the DDR.
> On the mx51evk, this value must be set to 0. However, I know already
> another target that uses another clock source for the RAM.
> 

That is a good reason.
Just a cleanup is needed
Tom

> Stefano
> 

[U-Boot] [PATCH V2] Moved board specific values in config file

[Stefano Babic]

The lowlevel_init file contained some hard-coded values
to setup the RAM. These board related values are moved into
the board configuration file.

Signed-off-by: Stefano Babic <sbabic@denx.de>
---
 cpu/arm_cortexa8/mx51/lowlevel_init.S |    5 ++++-
 include/configs/mx51evk.h             |    3 +++
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/cpu/arm_cortexa8/mx51/lowlevel_init.S b/cpu/arm_cortexa8/mx51/lowlevel_init.S
index 31af9e2..783c81f 100644
--- a/cpu/arm_cortexa8/mx51/lowlevel_init.S
+++ b/cpu/arm_cortexa8/mx51/lowlevel_init.S
@@ -158,6 +158,7 @@
 	/* Switch peripheral to PLL 3 */
 	ldr r0, =CCM_BASE_ADDR
 	ldr r1, =0x000010C0
+	orr r1,r1,#CONFIG_SYS_DDR_CLKSEL
 	str r1, [r0, #CLKCTL_CBCMR]
 	ldr r1, =0x13239145
 	str r1, [r0, #CLKCTL_CBCDR]
@@ -171,6 +172,7 @@
 	ldr r1, =0x19239145
 	str r1, [r0, #CLKCTL_CBCDR]
 	ldr r1, =0x000020C0
+	orr r1,r1,#CONFIG_SYS_DDR_CLKSEL
 	str r1, [r0, #CLKCTL_CBCMR]
 
 	mov r3, #DP_OP_216
@@ -201,9 +203,10 @@
 	/* setup the rest */
 	/* Use lp_apm (24MHz) source for perclk */
 	ldr r1, =0x000020C2
+	orr r1,r1,#CONFIG_SYS_DDR_CLKSEL
 	str r1, [r0, #CLKCTL_CBCMR]
 	/* ddr clock from PLL 1, all perclk dividers are 1 since using 24MHz */
-	ldr r1, =0x59E35100
+	ldr r1, =CONFIG_SYS_CLKTL_CBCDR
 	str r1, [r0, #CLKCTL_CBCDR]
 
 	/* Restore the default values in the Gate registers */
diff --git a/include/configs/mx51evk.h b/include/configs/mx51evk.h
index e2daeca..5096ab7 100644
--- a/include/configs/mx51evk.h
+++ b/include/configs/mx51evk.h
@@ -177,6 +177,9 @@
 #define PHYS_SDRAM_1		CSD0_BASE_ADDR
 #define PHYS_SDRAM_1_SIZE	(512 * 1024 * 1024)
 
+#define CONFIG_SYS_DDR_CLKSEL	0
+#define CONFIG_SYS_CLKTL_CBCDR	0x59E35100
+
 /*-----------------------------------------------------------------------
  * FLASH and environment organization
  */
-- 
1.6.3.3

[U-Boot] [PATCH V2] Moved board specific values in config file

[Liu Hui-R64343]

Stefano ,

> -----Original Message-----
> From: u-boot-bounces at lists.denx.de 
> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Stefano Babic
> Sent: 2010?3?28? 19:43
> To: u-boot at lists.denx.de
> Subject: [U-Boot] [PATCH V2] Moved board specific values in 
> config file
> 
> The lowlevel_init file contained some hard-coded values to 
> setup the RAM. These board related values are moved into the 
> board configuration file.
> 
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> ---
>  cpu/arm_cortexa8/mx51/lowlevel_init.S |    5 ++++-
>  include/configs/mx51evk.h             |    3 +++
>  2 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/cpu/arm_cortexa8/mx51/lowlevel_init.S 
> b/cpu/arm_cortexa8/mx51/lowlevel_init.S
> index 31af9e2..783c81f 100644
> --- a/cpu/arm_cortexa8/mx51/lowlevel_init.S
> +++ b/cpu/arm_cortexa8/mx51/lowlevel_init.S
> @@ -158,6 +158,7 @@
>  	/* Switch peripheral to PLL 3 */
>  	ldr r0, =CCM_BASE_ADDR
>  	ldr r1, =0x000010C0
> +	orr r1,r1,#CONFIG_SYS_DDR_CLKSEL
>  	str r1, [r0, #CLKCTL_CBCMR]
>  	ldr r1, =0x13239145
>  	str r1, [r0, #CLKCTL_CBCDR]
> @@ -171,6 +172,7 @@
>  	ldr r1, =0x19239145
>  	str r1, [r0, #CLKCTL_CBCDR]
>  	ldr r1, =0x000020C0
> +	orr r1,r1,#CONFIG_SYS_DDR_CLKSEL
>  	str r1, [r0, #CLKCTL_CBCMR]
>  
>  	mov r3, #DP_OP_216
> @@ -201,9 +203,10 @@
>  	/* setup the rest */
>  	/* Use lp_apm (24MHz) source for perclk */
>  	ldr r1, =0x000020C2
> +	orr r1,r1,#CONFIG_SYS_DDR_CLKSEL
>  	str r1, [r0, #CLKCTL_CBCMR]
>  	/* ddr clock from PLL 1, all perclk dividers are 1 
> since using 24MHz */
> -	ldr r1, =0x59E35100
> +	ldr r1, =CONFIG_SYS_CLKTL_CBCDR
>  	str r1, [r0, #CLKCTL_CBCDR]
>  
>  	/* Restore the default values in the Gate registers */ 
> diff --git a/include/configs/mx51evk.h 
> b/include/configs/mx51evk.h index e2daeca..5096ab7 100644
> --- a/include/configs/mx51evk.h
> +++ b/include/configs/mx51evk.h
> @@ -177,6 +177,9 @@
>  #define PHYS_SDRAM_1		CSD0_BASE_ADDR
>  #define PHYS_SDRAM_1_SIZE	(512 * 1024 * 1024)
>  
> +#define CONFIG_SYS_DDR_CLKSEL	0
> +#define CONFIG_SYS_CLKTL_CBCDR	0x59E35100

It's nice to have some comments about the value selected.

> +
>  
> /*------------------------------------------------------------
> -----------
>   * FLASH and environment organization
>   */
> --
> 1.6.3.3
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> 
> 

[U-Boot] [PATCH V2] Moved board specific values in config file

[Stefano Babic]

Liu Hui-R64343 wrote:
> Stefano ,
> 
Hi Jason,

>>  
>> +#define CONFIG_SYS_DDR_CLKSEL	0
>> +#define CONFIG_SYS_CLKTL_CBCDR	0x59E35100
> 
> It's nice to have some comments about the value selected.

This patch is more as one month old and was already merged into the
mainline, so we need a new patch to for setting a comment.
As your comment regards only the description to a configuration value, I
will not do it now. However, I write down your comment for the next time
this file will be changed ;)

Regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================