No subject

that if drac == 2, then the SDRAM row is a number between 0 and (11+drac0) - 1.
That in turn means that the number of SDRAM rows is : 2 ^ (11 + drac0).
Example:
 - drac0 = 2 : row encoded on 13 bits => 2^13 rows (ie. 1 << (11 + 2)).

>  
>  static u32 mdrefr_dri(unsigned int freq)
>  {
>  	u32 dri = 0;
>  
> -	if (cpu_is_pxa25x())
> -		dri = ((freq * SDRAM_TREF) / (sdram_rows * 32));
> +	/* 
> +	 * PXA255 (6.5.3):
> +	 * 	DRI = (Refresh time / rows * memory clock frequency) / 32
> +	 * PXA270 (Table 91):
> +	 * 	DRI = (Refresh time / rows * memory clock frequency - 31) / 32
> +	 * 
> +	 * Memory clock frequency is in MHz here! (Refresh time is in ms).
> +	 */
I don't agree here either. The "freq" variable is in kHz. It comes from
"new_freq_mem = pxa_freq_settings[idx].membus;" in pxa_set_target(). The membus
value is in kHz, as in tables pxaXXX_freqs (see 104000 and 208000, second column
in pxa27x_freqs for example). As refresh time is in ms, there is no need for a
1000th multiplier.

And my understanding is that the legacy formula is incorrect, as you discovered
it. Yet it should be, IMO :
    if (cpu_is_pxa27x())
-       dri = ((freq * SDRAM_TREF) / (sdram_rows - 31)) / 32;
+       dri = ((freq * SDRAM_TREF) / sdram_rows - 31) / 32;

This is the only change to apply IMHO.

Can you confirm my analysis ?

-- 
Robert