On 6/9/15 6:55 AM, Pavel Machek wrote:
Hi!
+struct sdram_prot_rule {
- uint64_t sdram_start; /* SDRAM start address */
- uint64_t sdram_end; /* SDRAM end address */
- uint32_t rule; /* SDRAM protection rule number: 0-19 */
- int valid; /* Rule valid or not? 1 - valid, 0 not*/
There should be space before "*/".
Ok...
diff --git a/arch/arm/include/asm/arch-socfpga/sdram_config.h b/arch/arm/include/asm/arch-socfpga/sdram_config.h new file mode 100644 index 0000000..f6d51ca --- /dev/null +++ b/arch/arm/include/asm/arch-socfpga/sdram_config.h @@ -0,0 +1,100 @@ +/*
- Copyright Altera Corporation (C) 2012-2015
- SPDX-License-Identifier: BSD-3-Clause
- */
+/* This file is autogenerated from tools provided by Altera.*/
Here too.
Ok...
+#endif /*#ifndef__SDRAM_CONFIG_H*/
You should not need to comment for include guards... (and comment style).
+static int compute_errata_rows(unsigned long long memsize, int cs, int width,
int rows, int banks, int cols)+{
Comment what kind of errata this is working around?
I'll have to ask around.
+#if defined(CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_CSBITS) && \ +defined(CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_ROWBITS) && \ +defined(CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_BANKBITS) && \ +defined(CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_COLBITS) && \ +defined(CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_ROWBITS)
Hmm? Is this really neccessary? Is it valid to provide configuration w/o those defines?
These defines are necessary as I want to keep some level of continuity with the Altera tools that generates these config files to this.
- writel(CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_ROWBITS,
&sysmgr_regs->iswgrp_handoff[4]);+#endif
- /* Restore the SDR PHY Register if valid */
- if (sdr_phy_reg != 0xffffffff)
writel(sdr_phy_reg, &sdr_ctrl->phy_ctrl0);+/***** Final step - apply configuration changes *****/
Comment style...
Ok..
+/*
- To calculate SDRAM device size based on SDRAM controller parameters.
- Size is specified in bytes.
- NOTE:
- This function is compiled and linked into the preloader and
- Uboot (there may be others). So if this function changes, the Preloader
- and UBoot must be updated simultaneously.
- */
+unsigned long sdram_calculate_size(void) +{
- unsigned long temp;
- unsigned long row, bank, col, cs, width;
- temp = readl(&sdr_ctrl->dram_addrw);
- col = (temp & SDR_CTRLGRP_DRAMADDRW_COLBITS_MASK) >>
SDR_CTRLGRP_DRAMADDRW_COLBITS_LSB;- /* SDRAM Failure When Accessing Non-Existent Memory
* Use ROWBITS from Quartus/QSys to calculate SDRAM size* since the FB specifies we modify ROWBITs to work around SDRAM* controller issue.** If the stored handoff value for rows is 0, it probably means* the preloader is older than UBoot. Use the* #define from the SOCEDS Tools per Crucible review* uboot-socfpga-204. Note that this is not a supported* configuration and is not tested. The customer* should be using preloader and uboot built from the* same tag.*/U-Boot is normally spelled "U-Boot". You have two different variants in comments here.
Thanks for the comment here, and will be more cognizant in the future on this fact.
Second part of the comment is probably not relevant any more....?
removed...
Acked-by: Pavel Machek pavel@denx.de Pavel
Thanks,
Dinh