diff mbox

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

Message ID 1429223817-10076-1-git-send-email-picmaster@mail.bg
State Awaiting Upstream
Delegated to: Stefano Babic
Headers show

Commit Message

Nikolay Dimitrov April 16, 2015, 10:36 p.m. UTC
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(-)

Comments

Otavio Salvador April 17, 2015, 12:53 a.m. UTC | #1
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>
Tim Harvey April 21, 2015, 4:17 a.m. UTC | #2
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>
Igor Grinberg April 22, 2015, 6:08 a.m. UTC | #3
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>
Stefano Babic April 22, 2015, 12:12 p.m. UTC | #4
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
Nikolay Dimitrov April 22, 2015, 12:22 p.m. UTC | #5
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
Stefano Babic April 22, 2015, 12:47 p.m. UTC | #6
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
Stefano Babic April 22, 2015, 2:02 p.m. UTC | #7
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
Nikolay Dimitrov April 23, 2015, 2:26 p.m. UTC | #8
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
Stefano Babic April 23, 2015, 2:28 p.m. UTC | #9
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
diff mbox

Patch

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;