Re: [U-Boot] [PATCHv1 01/22] arm: socfpga: spl: Add main sdram code

Hi!

From: Dinh Nguyen dinguyen@opensource.altera.com

This adds the code to configure the SDRAM controller that is found in the SoCFGPA Cyclone5 and Arria5 platforms.

Signed-off-by: Dinh Nguyen dinguyen@opensource.altera.com

+/******************************************************************************

• ---

• ** NOTE: Special Rules for Globale Variables **

• ---

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

• ---

• ******************************************************************************/

Comment coding style, please?

And they not globale variables.

But more importantly: is it good idea? What state is shared over reset? What are the special cases this is needed for?

What happens, when kernel corrupts that state?

+/*

• • case:56390
• • VFIFO_CONTROL_WIDTH_PER_DQS is the number of VFIFOs actually instantiated
• • per DQS. This is always one except:
• • AV QDRII where it is 2 for x18 and x18w2, and 4 for x36 and x36w2
• • RLDRAMII x36 and x36w2 where it is 2.
• • In 12.0sp1 we set this to 4 for all of the special cases above to
• • keep it simple.
• • In 12.0sp2 or 12.1 this should get moved to generation and unified with
• • the same constant used in the phy mgr
• */

Does case: refer to something public?

+/*

• • Given a rank, select the set of shadow registers that is responsible
• • for the delays of such rank, so that subsequent SCC updates will
• • go to those shadow registers.
• */

+void select_shadow_regs_for_update(uint32_t rank, uint32_t group,

• uint32_t update_scan_chains)

+{ +#if USE_SHADOW_REGS

• uint32_t rank_one_hot = (0xFF & (1 << rank));

Does this need to be configurable?

Best regards,

Pavel