From mboxrd@z Thu Jan  1 00:00:00 1970
From: Nikolay Dimitrov <picmaster@mail.bg>
Date: Thu, 23 Apr 2015 17:26:03 +0300
Subject: [U-Boot] [RFC PATCH] arm: mx6: Clamp MMDC and DDR3 clocks for
	timing calculations
In-Reply-To: <5537A9DF.2020709@denx.de>
References: <1429223817-10076-1-git-send-email-picmaster@mail.bg>
 <5537901B.8050106@denx.de> <55379271.10501@mail.bg>
 <5537A9DF.2020709@denx.de>
Message-ID: <553900FB.8040703@mail.bg>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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