[U-Boot] [PATCH] mx6: Add initial SPL support for HummingBoard-i2eX

[Nikolay Dimitrov <picmaster@mail.bg>]

Hi Fabio,

On 04/16/2015 03:48 AM, Fabio Estevam wrote:
> Hi Nikolay,
>
> On Wed, Apr 15, 2015 at 8:18 PM, Nikolay Dimitrov <picmaster@mail.bg> wrote:
>
>> This mem_speed (1600) means that the MMDC imx6 code will calculate
>> timings for 1600 MT/s, but in reality the memory clock will be 528 or
>> 532 MHz. It would be better to override the speed to 1066 to make
>> sure imx6 ddr code calculates proper timings.
>
> mem_speed should be retrieved from the DDR3 datasheet.

The DDR3 clock is defined by the common range of DDR3 chip and memory
controller, not only from the DDR3 chip.

> For clarification, take a look at: board/gateworks/gw_ventana/gw_ventana_spl.c
>
> /* MT41K128M16JT-125 */
> static struct mx6_ddr3_cfg mt41k128m16jt_125 = {
>      .mem_speed = 1600,
>
> Looking at the MT41K128M16JT-125 datasheet it says it has a cycle time
> of 1.25ns, which corresponds
> to 800MHz. Since this is DDR (double rate) then you get 1600MHz.

This memory chip supports up to 800 MHz DDR3 clock, which makes 1600
MT/s data rate. It's the data rate which is doubled (MT/s), not the
DDR3 clock (MHz).

>> To be honest, I don't understand why the code (arch/arm/cpu/armv7
>> /mx6/ddr.c) supports 1333/1600 MT/s when it's obvious that it's outside
>> chip specs.
>
> The chip is not out of spec. In this ddr.c file we have:
>
>      /* MX6D/MX6Q: 1066 MHz memory clock, clkper = 1.894ns = 1894ps */
>      if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D)) {
>          clock = 528;
>          tcwl = 4;
>      }

imx6 supports up to 528 MHz DDR3 clock as per datasheet, which makes
1058 MT/s data rate. Unfortunately such comments (like above) in the
code doesn't help to clarify the differences. Which is a nice way to
say the comments are wrong :).

Here's what happens - chip specs say that the chip supports 1600 MT/s
(800 MHz DDR3 clock). The imx6 ddr code looks up in the switch/case and
calculates timings for 800 MHz, while in reality the imx6 MMDC
officially supports up to 528 MHz. What should happen - mem_speed
should be clamped to imx6 model-specific speed limit, so the MMDC code
can calculate valid timings.

>> And a couple of more notes:
>> - I see that you don't use the extlinux boot mechanism (from distro
>> defaults). Do you see any advantages of not using it?
>
> No, I don't. This could be added later.
>
>> - Would be nice to have HDMI support :)
>
> Yes, HDMI, USB, PCI, SATA etc. One step at time ;-)
>
>
>> - git patch gives a warning about one empty line
>
> Thanks, I noticed this on the README file and will fix it in v2.
>
> Regards,
>
> Fabio Estevam

Kind regards,
Nikolay