[U-Boot] [RFC PATCH] arm: mx6: Clamp MMDC and DDR3 clocks for timing calculations
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;
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
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
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
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
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
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, butMX6D/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
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
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
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
participants (5)
-
Igor Grinberg -
Nikolay Dimitrov -
Otavio Salvador -
Stefano Babic -
Tim Harvey