[U-Boot] [PATCHv2] fsl_ddr: Don't use full 64-bit divides on32-bit PowerPC

[Moffett, Kyle D]

On Jul 28, 2011, at 17:41, York Sun wrote:
> On Mon, 2011-03-14 at 16:35 -0500, Moffett, Kyle D wrote:
>> On Mar 14, 2011, at 16:22, York Sun wrote:
>>> On Wed, 2011-02-23 at 11:35 -0500, Kyle Moffett wrote:
>>>> +	 * Now divide by 5^12 and track the 32-bit remainder, then divide
>>>> +	 * by 2*(2^12) using shifts (and updating the remainder).
>>>> +	 */
>>>> +	clks_rem = do_div(clks, UL_5pow12);
>>>> +	clks_rem <<= 13;
>>> 
>>> Shouldn't this be clks_rem >>= 13 ?
>>>> 
>>>> +	clks_rem |= clks & (UL_2pow13-1);
>>>> +	clks >>= 13;
>>>> +
>>>> +	/* If we had a remainder, then round up */
>>>> +	if (clks_rem)
>>>> 		clks++;
>>>> -	}
> 
> 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.

The function has obviously always done rounding, even before I made
any changes.  Have you retested what the math does with the patch
reverted to see if it makes any difference?

Hmm, looking at it, the obvious problem case would be mclk_to_picos()
using a rounded result in a subsequent multiply.  Can you retest by
replacing mclk_to_picos() with the following code; this should keep
the error down by doing the rounding-to-10ps *after* the multiply.

unsigned int mclk_to_picos(unsigned int mclk)
{
        unsigned int data_rate = get_ddr_freq(0);
        unsigned int result;

        /* Round to nearest 10ps, being careful about 64-bit multiply/divide */
        unsigned long long mclk_ps = ULL_2e12 * mclk;

        /* Add 5*data_rate, for rounding */
        mclk_ps += 5*(unsigned long long)data_rate;
 
        /* Now perform the big divide, the result fits in 32-bits */
        do_div(mclk_ps, data_rate);
        result = mclk_ps;

        /* We still need to round to 10ps */
        return 10 * (result/10);
}

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...

Cheers,
Kyle Moffett