[U-Boot] [RFC PATCH] arm: mx6: Clamp MMDC and DDR3 clocks for timing calculations

[U-Boot] [RFC PATCH] arm: mx6: Clamp MMDC and DDR3 clocks for timing calculations

[Nikolay Dimitrov]

This is proposal for clamping the MMDC/DDR3 clocks to the maximum supported
frequencies as per imx6 SOC models, and for dynamically calculating valid
clock value based on mem_speed.

Currently the code uses impossible values for mem_speed (1333, 1600 MT/s) for
calculating the DDR timings, and uses fixed clock (528 or 400 MHz) which
doesn't take into account DDR3 memory limitations.

Signed-off-by: Nikolay Dimitrov <picmaster@mail.bg>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Eric Nelson <eric.nelson@boundarydevices.com>
---
 arch/arm/cpu/armv7/mx6/ddr.c |   25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/arch/arm/cpu/armv7/mx6/ddr.c b/arch/arm/cpu/armv7/mx6/ddr.c
index fef2231..9daa180 100644
--- a/arch/arm/cpu/armv7/mx6/ddr.c
+++ b/arch/arm/cpu/armv7/mx6/ddr.c
@@ -265,7 +265,7 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo *sysinfo,
 	u16 tdllk = 0x1ff; /* DLL locking time: 512 cycles (JEDEC DDR3) */
 	u8 coladdr;
 	int clkper; /* clock period in picoseconds */
-	int clock; /* clock freq in mHz */
+	int clock; /* clock freq in MHz */
 	int cs;
 
 	mmdc0 = (struct mmdc_p_regs *)MMDC_P0_BASE_ADDR;
@@ -273,16 +273,31 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo *sysinfo,
 	mmdc1 = (struct mmdc_p_regs *)MMDC_P1_BASE_ADDR;
 #endif
 
-	/* MX6D/MX6Q: 1066 MHz memory clock, clkper = 1.894ns = 1894ps */
+	/* Limit mem_speed for MX6D/MX6Q */
 	if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D)) {
-		clock = 528;
+		if (ddr3_cfg->mem_speed > 1066)
+			ddr3_cfg->mem_speed = 1066; /* 1066 MT/s */
+
 		tcwl = 4;
 	}
-	/* MX6S/MX6DL: 800 MHz memory clock, clkper = 2.5ns = 2500ps */
+	/* Limit mem_speed for MX6S/MX6DL */
 	else {
-		clock = 400;
+		if (ddr3_cfg->mem_speed > 800)
+			ddr3_cfg->mem_speed = 800;  /* 800 MT/s */
+
 		tcwl = 3;
 	}
+
+	clock = ddr3_cfg->mem_speed / 2;
+	/*
+	 * Data rate of 1066 MT/s requires 533 MHz DDR3 clock, but MX6D/Q supports
+	 * up to 528 MHz, so reduce the clock to fit chip specs
+	 */
+	if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D)) {
+		if (clock > 528)
+			clock = 528; /* 528 MHz */
+	}
+
 	clkper = (1000 * 1000) / clock; /* pico seconds */
 	todtlon = tcwl;
 	taxpd = tcwl;
-- 
1.7.10.4

[U-Boot] [RFC PATCH] arm: mx6: Clamp MMDC and DDR3 clocks for timing calculations

[Otavio Salvador]

On Thu, Apr 16, 2015 at 7:36 PM, Nikolay Dimitrov <picmaster@mail.bg> wrote:
> This is proposal for clamping the MMDC/DDR3 clocks to the maximum supported
> frequencies as per imx6 SOC models, and for dynamically calculating valid
> clock value based on mem_speed.
>
> Currently the code uses impossible values for mem_speed (1333, 1600 MT/s) for
> calculating the DDR timings, and uses fixed clock (528 or 400 MHz) which
> doesn't take into account DDR3 memory limitations.
>
> Signed-off-by: Nikolay Dimitrov <picmaster@mail.bg>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Eric Nelson <eric.nelson@boundarydevices.com>

It does looks way nicer. It is much easier to understand what is going
behind the scenes.

Acked-by: Otavio Salvador <otavio@ossystems.com.br>

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

[U-Boot] [RFC PATCH] arm: mx6: Clamp MMDC and DDR3 clocks for timing calculations

[Tim Harvey]

On Thu, Apr 16, 2015 at 3:36 PM, Nikolay Dimitrov <picmaster@mail.bg> wrote:
> This is proposal for clamping the MMDC/DDR3 clocks to the maximum supported
> frequencies as per imx6 SOC models, and for dynamically calculating valid
> clock value based on mem_speed.
>
> Currently the code uses impossible values for mem_speed (1333, 1600 MT/s) for
> calculating the DDR timings, and uses fixed clock (528 or 400 MHz) which
> doesn't take into account DDR3 memory limitations.
>
> Signed-off-by: Nikolay Dimitrov <picmaster@mail.bg>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Eric Nelson <eric.nelson@boundarydevices.com>

Nikolay,

Makes sense to me.

Acked-by: Tim Harvey <tharvey@gateworks.com>

[U-Boot] [RFC PATCH] arm: mx6: Clamp MMDC and DDR3 clocks for timing calculations

[Igor Grinberg]

On 04/17/15 01:36, Nikolay Dimitrov wrote:
> This is proposal for clamping the MMDC/DDR3 clocks to the maximum supported
> frequencies as per imx6 SOC models, and for dynamically calculating valid
> clock value based on mem_speed.
> 
> Currently the code uses impossible values for mem_speed (1333, 1600 MT/s) for
> calculating the DDR timings, and uses fixed clock (528 or 400 MHz) which
> doesn't take into account DDR3 memory limitations.
> 
> Signed-off-by: Nikolay Dimitrov <picmaster@mail.bg>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Eric Nelson <eric.nelson@boundarydevices.com>

Acked-by: Igor Grinberg <grinberg@compulab.co.il>


-- 
Regards,
Igor.

[U-Boot] [RFC PATCH] arm: mx6: Clamp MMDC and DDR3 clocks for timing calculations

[Stefano Babic]

Hi Nikolay,

On 17/04/2015 00:36, Nikolay Dimitrov wrote:
> This is proposal for clamping the MMDC/DDR3 clocks to the maximum supported
> frequencies as per imx6 SOC models, and for dynamically calculating valid
> clock value based on mem_speed.
> 
> Currently the code uses impossible values for mem_speed (1333, 1600 MT/s) for
> calculating the DDR timings, and uses fixed clock (528 or 400 MHz) which
> doesn't take into account DDR3 memory limitations.
> 
> Signed-off-by: Nikolay Dimitrov <picmaster@mail.bg>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Eric Nelson <eric.nelson@boundarydevices.com>
> ---
>  arch/arm/cpu/armv7/mx6/ddr.c |   25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/mx6/ddr.c b/arch/arm/cpu/armv7/mx6/ddr.c
> index fef2231..9daa180 100644
> --- a/arch/arm/cpu/armv7/mx6/ddr.c
> +++ b/arch/arm/cpu/armv7/mx6/ddr.c
> @@ -265,7 +265,7 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo *sysinfo,
>  	u16 tdllk = 0x1ff; /* DLL locking time: 512 cycles (JEDEC DDR3) */
>  	u8 coladdr;
>  	int clkper; /* clock period in picoseconds */
> -	int clock; /* clock freq in mHz */
> +	int clock; /* clock freq in MHz */
>  	int cs;
>  
>  	mmdc0 = (struct mmdc_p_regs *)MMDC_P0_BASE_ADDR;
> @@ -273,16 +273,31 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo *sysinfo,
>  	mmdc1 = (struct mmdc_p_regs *)MMDC_P1_BASE_ADDR;
>  #endif
>  
> -	/* MX6D/MX6Q: 1066 MHz memory clock, clkper = 1.894ns = 1894ps */
> +	/* Limit mem_speed for MX6D/MX6Q */
>  	if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D)) {
> -		clock = 528;
> +		if (ddr3_cfg->mem_speed > 1066)
> +			ddr3_cfg->mem_speed = 1066; /* 1066 MT/s */
> +
>  		tcwl = 4;
>  	}
> -	/* MX6S/MX6DL: 800 MHz memory clock, clkper = 2.5ns = 2500ps */
> +	/* Limit mem_speed for MX6S/MX6DL */
>  	else {
> -		clock = 400;
> +		if (ddr3_cfg->mem_speed > 800)
> +			ddr3_cfg->mem_speed = 800;  /* 800 MT/s */
> +
>  		tcwl = 3;
>  	}
> +
> +	clock = ddr3_cfg->mem_speed / 2;
> +	/*
> +	 * Data rate of 1066 MT/s requires 533 MHz DDR3 clock, but MX6D/Q supports
> +	 * up to 528 MHz, so reduce the clock to fit chip specs
> +	 */
> +	if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D)) {
> +		if (clock > 528)
> +			clock = 528; /* 528 MHz */
> +	}
> +
>  	clkper = (1000 * 1000) / clock; /* pico seconds */
>  	todtlon = tcwl;
>  	taxpd = tcwl;
> 

Well done. I think we can forget the RFC of the title and apply it.

Regards,
Stefano


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [RFC PATCH] arm: mx6: Clamp MMDC and DDR3 clocks for timing calculations

[Nikolay Dimitrov]

Hi Stefano,

On 04/22/2015 03:12 PM, Stefano Babic wrote:
> Hi Nikolay,
>
> On 17/04/2015 00:36, Nikolay Dimitrov wrote:
>> This is proposal for clamping the MMDC/DDR3 clocks to the maximum supported
>> frequencies as per imx6 SOC models, and for dynamically calculating valid
>> clock value based on mem_speed.
>>
>> Currently the code uses impossible values for mem_speed (1333, 1600 MT/s) for
>> calculating the DDR timings, and uses fixed clock (528 or 400 MHz) which
>> doesn't take into account DDR3 memory limitations.
>>
>> Signed-off-by: Nikolay Dimitrov <picmaster@mail.bg>
>> Cc: Fabio Estevam <festevam@gmail.com>
>> Cc: Stefano Babic <sbabic@denx.de>
>> Cc: Tim Harvey <tharvey@gateworks.com>
>> Cc: Eric Nelson <eric.nelson@boundarydevices.com>
>> ---
>>   arch/arm/cpu/armv7/mx6/ddr.c |   25 ++++++++++++++++++++-----
>>   1 file changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/mx6/ddr.c b/arch/arm/cpu/armv7/mx6/ddr.c
>> index fef2231..9daa180 100644
>> --- a/arch/arm/cpu/armv7/mx6/ddr.c
>> +++ b/arch/arm/cpu/armv7/mx6/ddr.c
>> @@ -265,7 +265,7 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo *sysinfo,
>>   	u16 tdllk = 0x1ff; /* DLL locking time: 512 cycles (JEDEC DDR3) */
>>   	u8 coladdr;
>>   	int clkper; /* clock period in picoseconds */
>> -	int clock; /* clock freq in mHz */
>> +	int clock; /* clock freq in MHz */
>>   	int cs;
>>
>>   	mmdc0 = (struct mmdc_p_regs *)MMDC_P0_BASE_ADDR;
>> @@ -273,16 +273,31 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo *sysinfo,
>>   	mmdc1 = (struct mmdc_p_regs *)MMDC_P1_BASE_ADDR;
>>   #endif
>>
>> -	/* MX6D/MX6Q: 1066 MHz memory clock, clkper = 1.894ns = 1894ps */
>> +	/* Limit mem_speed for MX6D/MX6Q */
>>   	if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D)) {
>> -		clock = 528;
>> +		if (ddr3_cfg->mem_speed > 1066)
>> +			ddr3_cfg->mem_speed = 1066; /* 1066 MT/s */
>> +
>>   		tcwl = 4;
>>   	}
>> -	/* MX6S/MX6DL: 800 MHz memory clock, clkper = 2.5ns = 2500ps */
>> +	/* Limit mem_speed for MX6S/MX6DL */
>>   	else {
>> -		clock = 400;
>> +		if (ddr3_cfg->mem_speed > 800)
>> +			ddr3_cfg->mem_speed = 800;  /* 800 MT/s */
>> +
>>   		tcwl = 3;
>>   	}
>> +
>> +	clock = ddr3_cfg->mem_speed / 2;
>> +	/*
>> +	 * Data rate of 1066 MT/s requires 533 MHz DDR3 clock, but MX6D/Q supports
>> +	 * up to 528 MHz, so reduce the clock to fit chip specs
>> +	 */
>> +	if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D)) {
>> +		if (clock > 528)
>> +			clock = 528; /* 528 MHz */
>> +	}
>> +
>>   	clkper = (1000 * 1000) / clock; /* pico seconds */
>>   	todtlon = tcwl;
>>   	taxpd = tcwl;
>>
>
> Well done. I think we can forget the RFC of the title and apply it.

Do you need me to reword the commit message?

Regards,
Nikolay

[U-Boot] [RFC PATCH] arm: mx6: Clamp MMDC and DDR3 clocks for timing calculations

[Stefano Babic]

On 22/04/2015 14:22, Nikolay Dimitrov wrote:
> Hi Stefano,
> 
> On 04/22/2015 03:12 PM, Stefano Babic wrote:
>> Hi Nikolay,
>>
>> On 17/04/2015 00:36, Nikolay Dimitrov wrote:
>>> This is proposal for clamping the MMDC/DDR3 clocks to the maximum
>>> supported
>>> frequencies as per imx6 SOC models, and for dynamically calculating
>>> valid
>>> clock value based on mem_speed.
>>>
>>> Currently the code uses impossible values for mem_speed (1333, 1600
>>> MT/s) for
>>> calculating the DDR timings, and uses fixed clock (528 or 400 MHz) which
>>> doesn't take into account DDR3 memory limitations.
>>>
>>> Signed-off-by: Nikolay Dimitrov <picmaster@mail.bg>
>>> Cc: Fabio Estevam <festevam@gmail.com>
>>> Cc: Stefano Babic <sbabic@denx.de>
>>> Cc: Tim Harvey <tharvey@gateworks.com>
>>> Cc: Eric Nelson <eric.nelson@boundarydevices.com>
>>> ---
>>>   arch/arm/cpu/armv7/mx6/ddr.c |   25 ++++++++++++++++++++-----
>>>   1 file changed, 20 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/armv7/mx6/ddr.c b/arch/arm/cpu/armv7/mx6/ddr.c
>>> index fef2231..9daa180 100644
>>> --- a/arch/arm/cpu/armv7/mx6/ddr.c
>>> +++ b/arch/arm/cpu/armv7/mx6/ddr.c
>>> @@ -265,7 +265,7 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo
>>> *sysinfo,
>>>       u16 tdllk = 0x1ff; /* DLL locking time: 512 cycles (JEDEC DDR3) */
>>>       u8 coladdr;
>>>       int clkper; /* clock period in picoseconds */
>>> -    int clock; /* clock freq in mHz */
>>> +    int clock; /* clock freq in MHz */
>>>       int cs;
>>>
>>>       mmdc0 = (struct mmdc_p_regs *)MMDC_P0_BASE_ADDR;
>>> @@ -273,16 +273,31 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo
>>> *sysinfo,
>>>       mmdc1 = (struct mmdc_p_regs *)MMDC_P1_BASE_ADDR;
>>>   #endif
>>>
>>> -    /* MX6D/MX6Q: 1066 MHz memory clock, clkper = 1.894ns = 1894ps */
>>> +    /* Limit mem_speed for MX6D/MX6Q */
>>>       if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D)) {
>>> -        clock = 528;
>>> +        if (ddr3_cfg->mem_speed > 1066)
>>> +            ddr3_cfg->mem_speed = 1066; /* 1066 MT/s */
>>> +
>>>           tcwl = 4;
>>>       }
>>> -    /* MX6S/MX6DL: 800 MHz memory clock, clkper = 2.5ns = 2500ps */
>>> +    /* Limit mem_speed for MX6S/MX6DL */
>>>       else {
>>> -        clock = 400;
>>> +        if (ddr3_cfg->mem_speed > 800)
>>> +            ddr3_cfg->mem_speed = 800;  /* 800 MT/s */
>>> +
>>>           tcwl = 3;
>>>       }
>>> +
>>> +    clock = ddr3_cfg->mem_speed / 2;
>>> +    /*
>>> +     * Data rate of 1066 MT/s requires 533 MHz DDR3 clock, but
>>> MX6D/Q supports
>>> +     * up to 528 MHz, so reduce the clock to fit chip specs
>>> +     */
>>> +    if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D)) {
>>> +        if (clock > 528)
>>> +            clock = 528; /* 528 MHz */
>>> +    }
>>> +
>>>       clkper = (1000 * 1000) / clock; /* pico seconds */
>>>       todtlon = tcwl;
>>>       taxpd = tcwl;
>>>
>>
>> Well done. I think we can forget the RFC of the title and apply it.
> 
> Do you need me to reword the commit message?

No, I check myself, thanks.

Regards,
Stefano


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [RFC PATCH] arm: mx6: Clamp MMDC and DDR3 clocks for timing calculations

[Stefano Babic]

Hi Nikolay,

On 22/04/2015 14:22, Nikolay Dimitrov wrote:
> Hi Stefano,
> 
> On 04/22/2015 03:12 PM, Stefano Babic wrote:
>> Hi Nikolay,
>>
>> On 17/04/2015 00:36, Nikolay Dimitrov wrote:
>>> This is proposal for clamping the MMDC/DDR3 clocks to the maximum
>>> supported
>>> frequencies as per imx6 SOC models, and for dynamically calculating
>>> valid
>>> clock value based on mem_speed.
>>>
>>> Currently the code uses impossible values for mem_speed (1333, 1600
>>> MT/s) for
>>> calculating the DDR timings, and uses fixed clock (528 or 400 MHz) which
>>> doesn't take into account DDR3 memory limitations.
>>>
>>> Signed-off-by: Nikolay Dimitrov <picmaster@mail.bg>
>>> Cc: Fabio Estevam <festevam@gmail.com>
>>> Cc: Stefano Babic <sbabic@denx.de>
>>> Cc: Tim Harvey <tharvey@gateworks.com>
>>> Cc: Eric Nelson <eric.nelson@boundarydevices.com>
>>> ---
>>>   arch/arm/cpu/armv7/mx6/ddr.c |   25 ++++++++++++++++++++-----
>>>   1 file changed, 20 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/armv7/mx6/ddr.c b/arch/arm/cpu/armv7/mx6/ddr.c
>>> index fef2231..9daa180 100644
>>> --- a/arch/arm/cpu/armv7/mx6/ddr.c
>>> +++ b/arch/arm/cpu/armv7/mx6/ddr.c
>>> @@ -265,7 +265,7 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo
>>> *sysinfo,
>>>       u16 tdllk = 0x1ff; /* DLL locking time: 512 cycles (JEDEC DDR3) */
>>>       u8 coladdr;
>>>       int clkper; /* clock period in picoseconds */
>>> -    int clock; /* clock freq in mHz */
>>> +    int clock; /* clock freq in MHz */
>>>       int cs;
>>>
>>>       mmdc0 = (struct mmdc_p_regs *)MMDC_P0_BASE_ADDR;
>>> @@ -273,16 +273,31 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo
>>> *sysinfo,
>>>       mmdc1 = (struct mmdc_p_regs *)MMDC_P1_BASE_ADDR;
>>>   #endif
>>>
>>> -    /* MX6D/MX6Q: 1066 MHz memory clock, clkper = 1.894ns = 1894ps */
>>> +    /* Limit mem_speed for MX6D/MX6Q */
>>>       if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D)) {
>>> -        clock = 528;
>>> +        if (ddr3_cfg->mem_speed > 1066)
>>> +            ddr3_cfg->mem_speed = 1066; /* 1066 MT/s */
>>> +

Sorry, but there is an issue. Parameters are const struct *, and you are
setting value here.

By testing I get on most mx6 boards:

+arch/arm/cpu/armv7/mx6/ddr.c: In function 'mx6_dram_cfg':
+arch/arm/cpu/armv7/mx6/ddr.c:279:4: error: assignment of member
'mem_speed' in read-only object
+arch/arm/cpu/armv7/mx6/ddr.c:286:4: error: assignment of member
'mem_speed' in read-only object
+make[4]: *** [spl/arch/arm/cpu/armv7/mx6/ddr.o] Error 1

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [RFC PATCH] arm: mx6: Clamp MMDC and DDR3 clocks for timing calculations

[Nikolay Dimitrov]

Hi Stefano,

On 04/22/2015 05:02 PM, Stefano Babic wrote:
> Hi Nikolay,
>
> On 22/04/2015 14:22, Nikolay Dimitrov wrote:
>> Hi Stefano,
>>
>> On 04/22/2015 03:12 PM, Stefano Babic wrote:
>>> Hi Nikolay,
>>>
>>> On 17/04/2015 00:36, Nikolay Dimitrov wrote:
>>>> This is proposal for clamping the MMDC/DDR3 clocks to the maximum
>>>> supported
>>>> frequencies as per imx6 SOC models, and for dynamically calculating
>>>> valid
>>>> clock value based on mem_speed.
>>>>
>>>> Currently the code uses impossible values for mem_speed (1333, 1600
>>>> MT/s) for
>>>> calculating the DDR timings, and uses fixed clock (528 or 400 MHz) which
>>>> doesn't take into account DDR3 memory limitations.
>>>>
>>>> Signed-off-by: Nikolay Dimitrov <picmaster@mail.bg>
>>>> Cc: Fabio Estevam <festevam@gmail.com>
>>>> Cc: Stefano Babic <sbabic@denx.de>
>>>> Cc: Tim Harvey <tharvey@gateworks.com>
>>>> Cc: Eric Nelson <eric.nelson@boundarydevices.com>
>>>> ---
>>>>    arch/arm/cpu/armv7/mx6/ddr.c |   25 ++++++++++++++++++++-----
>>>>    1 file changed, 20 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/arch/arm/cpu/armv7/mx6/ddr.c b/arch/arm/cpu/armv7/mx6/ddr.c
>>>> index fef2231..9daa180 100644
>>>> --- a/arch/arm/cpu/armv7/mx6/ddr.c
>>>> +++ b/arch/arm/cpu/armv7/mx6/ddr.c
>>>> @@ -265,7 +265,7 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo
>>>> *sysinfo,
>>>>        u16 tdllk = 0x1ff; /* DLL locking time: 512 cycles (JEDEC DDR3) */
>>>>        u8 coladdr;
>>>>        int clkper; /* clock period in picoseconds */
>>>> -    int clock; /* clock freq in mHz */
>>>> +    int clock; /* clock freq in MHz */
>>>>        int cs;
>>>>
>>>>        mmdc0 = (struct mmdc_p_regs *)MMDC_P0_BASE_ADDR;
>>>> @@ -273,16 +273,31 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo
>>>> *sysinfo,
>>>>        mmdc1 = (struct mmdc_p_regs *)MMDC_P1_BASE_ADDR;
>>>>    #endif
>>>>
>>>> -    /* MX6D/MX6Q: 1066 MHz memory clock, clkper = 1.894ns = 1894ps */
>>>> +    /* Limit mem_speed for MX6D/MX6Q */
>>>>        if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D)) {
>>>> -        clock = 528;
>>>> +        if (ddr3_cfg->mem_speed > 1066)
>>>> +            ddr3_cfg->mem_speed = 1066; /* 1066 MT/s */
>>>> +
>
> Sorry, but there is an issue. Parameters are const struct *, and you are
> setting value here.
>
> By testing I get on most mx6 boards:
>
> +arch/arm/cpu/armv7/mx6/ddr.c: In function 'mx6_dram_cfg':
> +arch/arm/cpu/armv7/mx6/ddr.c:279:4: error: assignment of member
> 'mem_speed' in read-only object
> +arch/arm/cpu/armv7/mx6/ddr.c:286:4: error: assignment of member
> 'mem_speed' in read-only object
> +make[4]: *** [spl/arch/arm/cpu/armv7/mx6/ddr.o] Error 1

Sorry for the stupid mistake, I've already sent v2.

Regards,
Nikolay

[U-Boot] [RFC PATCH] arm: mx6: Clamp MMDC and DDR3 clocks for timing calculations

[Stefano Babic]

Hi Nikolay,

On 23/04/2015 16:26, Nikolay Dimitrov wrote:
>>
>> +arch/arm/cpu/armv7/mx6/ddr.c: In function 'mx6_dram_cfg':
>> +arch/arm/cpu/armv7/mx6/ddr.c:279:4: error: assignment of member
>> 'mem_speed' in read-only object
>> +arch/arm/cpu/armv7/mx6/ddr.c:286:4: error: assignment of member
>> 'mem_speed' in read-only object
>> +make[4]: *** [spl/arch/arm/cpu/armv7/mx6/ddr.o] Error 1
> 
> Sorry for the stupid mistake, I've already sent v2.

No probblem at all, I have seen it and I'll pick it up soon !

Regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================