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