[U-Boot] [PATCHv3 2/3] driver/ddr/altera/: Add the sdram calibration portion

[Dinh Nguyen]

On 05/28/2015 01:18 PM, Marek Vasut wrote:
> On Thursday, May 28, 2015 at 05:41:26 PM, Dinh Nguyen wrote:
>> On 05/25/2015 08:23 AM, Wolfgang Denk wrote:
>>> Dear Pavel,
>>>
>>> In message <20150525123750.GD9943 at amd> you wrote:
>>>>> + ** All global variables that are explicitly initialized (including   
>>>>>       ** + ** explicitly initialized to zero), are only initialized
>>>>> once, during       ** + ** configuration time, and not again on reset.
>>>>>  This means that they        ** + ** preserve their current contents
>>>>> across resets, which is needed for some  ** + ** special cases
>>>>> involving communication with external modules.  In         ** + **
>>>>> addition, this avoids paying the price to have the memory initialized,
>>>>>   ** + ** even for zeroed data, provided it is explicitly set to zero
>>>>> in the code, ** + ** and doesn't rely on implicit initialization.     
>>>>>                        ** +
>>>>> **********************************************************************
>>>>> ******** +
>>>>
>>>> Is this sane thing to do? How does it work for variables in other
>>>> sources?
>>>
>>> My concern is if this is actually true (and I asked this before, in an
>>> earlier round ov reviews).   I cannot make heads or tails of this
>>> comment, as I don't understand what "configuration time" and "reset"
>>> are supposed to mean in U-Boot context.  In my understanding, after a
>>> reset the memory content is uninitialized, i. e. random, and thus MUST
>>> always be properly initialized.
> 
> Meh, since there's a pushback, I'll wait a bit with applying these until
> these remaining concerns settle :-/

That's fine. I'll send a v4 with further clean ups.

> 
>> This comment is related to the configuration where we have the NiOS cpu
>> doing the ddr calibration and is not applicable for the Cyclone5/Arria5.
>> So I think I can remove the comment for the the A5/C5 configuration.
> 
> OK, then this should be removed. A10 support should then be added in a
> separate patch please.
> 
>> This situation will come into play for the Arria10 SoCFPGA, because that
>> part will have a NiOS cpu that will do the DDR configuration.
>> "configuration time" happens at power-up
> 
> So "configuration time" happens if the FPGA loads itself from EPCQ or
> does it happen also if the FPGA is not loaded at all ?

I think configuration time is only applicable when FPGA is programmed.

> 
>> and "reset" is a warm reset.
>> From what I was told, the situation where we might want to preserve a
>> variable after a reset is to avoid reconfiguring the NiOS in the FPGA
>> for DDR operations.
> 
> Can't you synthesize a sticky register for this purpose in the FPGA instead?
> Or is it that this NIOS2 which configures the DRAM is actually a hardware,
> not a softcore ?
> 

On the Arria10, the NIOS2 is a hardened block and not a softcore.

>>> Also, what are "external modules"?
>>
>> I think these could be different FPGA instances that needs these
>> variables that have survived a warm reset to the CPU.
> 
> I _think_ I understand most of the above, but I kinda wonder why don't you
> cook a small IP block on the avalon bus in the FPGA which would contain a
> sticky register to hold all those configuration sticky bits. That'd look
> much more sensible to me.
> 
That does sound like a great idea. I think some of our customers are
probably doing that already that.

Dinh