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 "*/".
> 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.
> +#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?
> +#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?
> + 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...
> +/*
> + * 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.
Second part of the comment is probably not relevant any more....?
Acked-by: Pavel Machek <pa...@denx.de>
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
