On Tue 2015-07-28 15:30:06, Marek Vasut wrote: > On Tuesday, July 28, 2015 at 03:13:09 PM, Pavel Machek wrote: > > Hi! > > > > > This series fixes the SPL support on SoCFPGA and cleans up the DDR > > > init code such that it is becoming remotely mainlinable. After this > > > series, the SPL is capable of booting from both SD/MMC and QSPI NOR. > > > > > > There is still work to be done, but I'd like to start picking it up > > > so it can land in 2015.10 . Reviews and comments are welcome. > > > > Do you have series in git somewhere? I guess I'd like to review > > different diffs than these... > > > > Here is what you'd probably make most use of -- complete sockit support: > http://git.denx.de/?p=u-boot/u-boot- > socfpga.git;a=shortlog;h=refs/heads/wip/sockit
Thanks! Random notes: +static int check_cache_range(unsigned long start, unsigned long stop) +{ + int ok = 1; + + if (start & (CONFIG_SYS_CACHELINE_SIZE - 1)) + ok = 0; + + if (stop & (CONFIG_SYS_CACHELINE_SIZE - 1)) + ok = 0; + + if (!ok) + debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n", + start, stop); + if ((start | end) & (...)) debug() ..and you get rid of a variable. +int iocsr_get_config_table(const unsigned int chain_id, + const unsigned long **table, + unsigned int *table_len) +{ + switch (chain_id) { + case 0: + *table = iocsr_scan_chain0_table; + *table_len = CONFIG_HPS_IOCSR_SCANCHAIN0_LENGTH; + break; + case 1: + *table = iocsr_scan_chain1_table; + *table_len = CONFIG_HPS_IOCSR_SCANCHAIN1_LENGTH; + break; + case 2: + *table = iocsr_scan_chain2_table; + *table_len = CONFIG_HPS_IOCSR_SCANCHAIN2_LENGTH; + break; + case 3: + *table = iocsr_scan_chain3_table; + *table_len = CONFIG_HPS_IOCSR_SCANCHAIN3_LENGTH; + break; + default: + return -EINVAL; I'd do *table = NULL, *table_len = 0 here, to catch any mistakes. +static void sdram_set_rule(struct sdram_prot_rule *prule) +{ + uint32_t lo_addr_bits; + uint32_t hi_addr_bits; Can we do u32 here, and in other places like this? + rw_mgr_mem_load_user(RW_MGR_MRS0_USER_MIRR, RW_MGR_MRS0_USER, 1); + /* + * Need to wait tMOD (12CK or 15ns) time before issuing other + * commands, but we will have plenty of NIOS cycles before actual + * handoff so its okay. + */ I don't understand this comment. + return 0; + + /* Calibration Stage 1 completed OK. */ +cal_done_ok: + /* + * Reset the delay chains back to zero if they have moved > 1 + * (check for > 1 because loop will increase d even when pass in + * first case). + */ + if (d > 2) + scc_mgr_zero_group(rw_group, 1); + + return 1; +} Other functions use 0/-EIO protocol here, so it would be good to be consistent. It looks much better now, thanks, 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