Hi! > +#ifndef _SDRAM_H_ > +#define _SDRAM_H_ > + > +#ifndef __ASSEMBLY__ > + > +/* function declaration */
You can delete this comment. > +#define \ > +SDR_CTRLGRP_MPTHRESHOLDRST_0_THRESHOLDRSTCYCLES_31_0_LSB 0 > +#define \ > +SDR_CTRLGRP_MPTHRESHOLDRST_0_THRESHOLDRSTCYCLES_31_0_MASK \ > +0xffffffff > +/* Register template: sdr::ctrlgrp::mpthresholdrst::mpthresholdrst_1 */ > +#define \ > +SDR_CTRLGRP_MPTHRESHOLDRST_1_THRESHOLDRSTCYCLES_63_32_LSB 0 > +#define \ > +SDR_CTRLGRP_MPTHRESHOLDRST_1_THRESHOLDRSTCYCLES_63_32_MASK \ > +0xffffffff > +/* Register template: sdr::ctrlgrp::mpthresholdrst::mpthresholdrst_2 */ > +#define \ > +SDR_CTRLGRP_MPTHRESHOLDRST_2_THRESHOLDRSTCYCLES_79_64_LSB 0 > +#define \ > +SDR_CTRLGRP_MPTHRESHOLDRST_2_THRESHOLDRSTCYCLES_79_64_MASK \ > +0x0000ffff Can we get slightly shorter define names? > +/* Register template: sdr::ctrlgrp::remappriority */ > +#define SDR_CTRLGRP_REMAPPRIORITY_PRIORITYREMAP_LSB 0 > +#define SDR_CTRLGRP_REMAPPRIORITY_PRIORITYREMAP_MASK 0x000000ff > +/* Register template: sdr::ctrlgrp::phyctrl::phyctrl_0 */ > +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_SAMPLECOUNT_19_0_LSB 12 > +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_SAMPLECOUNT_19_0_WIDTH 20 > +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_SAMPLECOUNT_19_0_SET(x) \ > + (((x) << 12) & 0xfffff000) > +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_ADDLATSEL_SET(x) \ > + (((x) << 10) & 0x00000c00) > +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_DQSLOGICDELAYEN_SET(x) \ > + (((x) << 6) & 0x000000c0) > +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_RESETDELAYEN_SET(x) \ > + (((x) << 8) & 0x00000100) > +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_LPDDRDIS_SET(x) \ > + (((x) << 9) & 0x00000200) > +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_DQSDELAYEN_SET(x) \ > + (((x) << 4) & 0x00000030) > +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_DQDELAYEN_SET(x) \ > + (((x) << 2) & 0x0000000c) > +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_ACDELAYEN_SET(x) \ > + (((x) << 0) & 0x00000003) > +/* Register template: sdr::ctrlgrp::phyctrl::phyctrl_1 */ > +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_1_LONGIDLESAMPLECOUNT_19_0_WIDTH 20 > +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_1_LONGIDLESAMPLECOUNT_19_0_SET(x) \ > + (((x) << 12) & 0xfffff000) > +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_1_SAMPLECOUNT_31_20_SET(x) \ > + (((x) << 0) & 0x00000fff) > +/* Register template: sdr::ctrlgrp::phyctrl::phyctrl_2 */ > +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_2_LONGIDLESAMPLECOUNT_31_20_SET(x) \ > + (((x) << 0) & 0x00000fff) Too long names here, too.. > --- /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 > + */ > + If this file is autogenerated, you should mention it here. > +#ifdef CONFIG_SOCFPGA_ARRIA5 > +/* The if..else... is not required if generated by tools */ What does this comment say? > +#define CONFIG_HPS_SDR_CTRLCFG_MPPACING_1_THRESHOLD2_3_0 0 > +#define CONFIG_HPS_SDR_CTRLCFG_MPPACING_2_THRESHOLD2_35_4 0x41041041 > +#define CONFIG_HPS_SDR_CTRLCFG_MPPACING_3_THRESHOLD2_59_36 0x410410 > +#define CONFIG_HPS_SDR_CTRLCFG_MPTHRESHOLDRST_0_THRESHOLDRSTCYCLES_31_0 \ > +0x01010101 > +#define CONFIG_HPS_SDR_CTRLCFG_MPTHRESHOLDRST_1_THRESHOLDRSTCYCLES_63_32 \ > +0x01010101 > +#define CONFIG_HPS_SDR_CTRLCFG_MPTHRESHOLDRST_2_THRESHOLDRSTCYCLES_79_64 \ > +0x0101 Drop "HPS" and "CTRLCFG" from the config names... they should still be unique and you'll not hit 80 column limits with just the name? > +#define COMPARE_FAIL_ACTION return 1; Macros that change control flow are nasty. > +/* Below function only applicable for SPL */ "Function below"? Add ifdef so that it is not available for u-boot proper? > +typedef 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*/ > + > + uint32_t security; > + uint32_t portmask; > + uint32_t result; > + uint32_t lo_prot_id; > + uint32_t hi_prot_id; > +} sdram_prot_rule, *psdram_prot_rule; Struct typedefs are nasty. Just use "struct sdram_prot_rule"? > +static void sdram_get_rule(psdram_prot_rule prule) > +{ > + uint32_t protruleaddr; > + uint32_t protruleid; > + uint32_t protruledata; Remove "protrule" from local variables, as it is clear from context? > +static void sdram_set_protection_config(uint64_t sdram_start, uint64_t > sdram_end) > +{ > + sdram_prot_rule rule; > + int rules; > + > + /* Start with accepting all SDRAM transaction */ > + writel(0x0, &sdr_ctrl->protport_default); > + > + /* Clear all protection rules for warm boot case */ > + > + rule.sdram_start = 0; Kill last empty line. And actually... maybe memset? > +static void set_sdr_addr_rw(void) > +{ > + int cs = CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_CSBITS; > + int width = 8; > + int rows = CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_ROWBITS; > + int banks = CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_BANKBITS; > + int cols = CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_COLBITS; > + unsigned long long workaround_memsize = MEMSIZE_4G; > + > + debug("Configuring DRAMADDRW\n"); > + clrsetbits_le32(&sdr_ctrl->dram_addrw, > SDR_CTRLGRP_DRAMADDRW_COLBITS_MASK, > + CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_COLBITS << > + SDR_CTRLGRP_DRAMADDRW_COLBITS_LSB); > + /* SDRAM Failure When Accessing Non-Existent Memory > + * Update Preloader to artificially increase the number of rows so > + * that the memory thinks it has 4GB of RAM. > + */ Comment style, "rows, so"? > +/* To calculate SDRAM device size based on SDRAM controller parameters. Drop "To". > + * 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. > + */ Is that worth big note and four exclamation marks? Compiler should take care of recompilation... Ok, this starts to look like something that we could actually merge. 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