Skip to content

socfpga: SPL and DDR init

Pavel Machek u-boot 1 version New

Get series with b4
b4 am 20150728135833.GA8480@amd

Versions newest first

  1. v1 Latest0 of 172 patches 0 repliesNew

Patches in v1 No applies recorded yet

None of the 172 patches the cover letter announces are in this archive.

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [U-Boot] [PATCH 000/172] socfpga: SPL and DDR init
Quoted 10 lines · Marek Vasut
Quoted 9 lines · Pavel Machek

Hi!

Quoted 6 lines

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.

Quoted 7 lines
Quoted 3 lines

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Read the cover letter in its thread