On Jul 29, 2011, at 17:46, York Sun wrote:
> On Fri, 2011-07-29 at 16:26 -0500, Moffett, Kyle D wrote:
>> On Jul 29, 2011, at 14:46, York Sun wrote:
>>> On Fri, 2011-07-29 at 11:35 -0700, York Sun wrote:
>>>> On Fri, 2011-07-29 at 12:20 -0500, Moffett, Kyle D wrote:
>>>>> On Jul 28, 2011, at 17:41, York Sun wrote:
>>>>>> I found a problem with the round up. Please try
>>>>>>
>>>>>> picos_to_mclk(mclk_to_picos(3))
>>>>>>
>>>>>> You will notice the result is round up. I am sure it is unexpected.
>>>>>
>>>>> Hmm... what does fsl_ddr_get_mem_data_rate() return on your system?
>>>>> I cannot reproduce this here with my memory freq of 800MHz.
>>>>>
>>>>> [...snip...]
>>>>>
>>>>> Obviously you could also get rid of the rounding alltogether, but I
>>>>> don't know why it was put in the code in the first place...
>>>>
>>>> I think the problem comes from the round up. But your calculation of
>>>> remainder is not right.
>>
>> Can you tell me what your value of get_ddr_freq() is? I may be able to
>> reproduce the issue here.
>
> 264000000
Ok, so that's a 264MHz DDR frequency, right? 264 * 10^6?
If I plug that number in to the code I can reproduce your result:
picos_to_mclk(mclk_to_picos(3)) == 4
But that happens with both the new code *AND* the old code. I ran them
side-by-side in the same test-harness:
OLD:
get_memory_clk_period_ps(): 7580
picos_to_mclk(mclk_to_picos(3)): 4
----------------------
NEW:
get_memory_clk_period_ps(): 7580
picos_to_mclk(mclk_to_picos(3)): 4
So there's a bug, but it's always been there...
Just to show the floating-point math really quick:
2 * (10^12 picosec/sec) / (264 * 10^6 cyc/sec)
== 7575.757575..... picoseconds
Since the code rounds to the nearest 10ps, we get 7580ps. Then 3 mclk is
22740ps, but it should be 22727.272727.....
The picos_to_mclk() function always rounds up, because we must avoid
undercutting the SPD timing margin. If your SPD specifies a minimum of a
7590ps timing window, you MUST use a 2 mclk delay (even though it's
painfully close to 1 mclk), because otherwise you will get RAM errors.
Therefore mclk_to_picos() and get_memory_clk_period_ps() should always
round down, even though they currently round to the closest 10ps.
Continuing the example: If we later we do picos_to_mclk(22740):
22740ps * (264 * 10^5 cyc/sec) * 0.5 / (10^12 picosec/sec)
== 3.00168
When we do the integer math with remainder, we get:
3 rem 3360000000
In both cases, we correctly automatically round up to 4.
I believe I have a fix for the problem; here's my fixed test case log:
get_memory_clk_period_ps(): 7575
picos_to_mclk(mclk_to_picos(3)): 3
And here is the new code (the picos_to_mclk() function is unchanged):
unsigned int mclk_to_picos(unsigned int mclk)
{
unsigned int data_rate = get_ddr_freq(0);
/* Be careful to avoid a 64-bit full-divide (result fits in 32) */
unsigned long long mclk_ps = ULL_2E12 * mclk;
do_div(mclk_ps, data_rate);
return mclk_ps;
}
unsigned int get_memory_clk_period_ps(void)
{
return mclk_to_picos(1);
}
Cheers,
Kyle Moffett
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
