All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper
@ 2018-10-02 11:22 Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 01/23] log: Correct definition of log_msg_ret() Simon Glass
                   ` (22 more replies)
  0 siblings, 23 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

At present we have no standard way of passing information from SPL to
U-Boot. Such information may be the size of DRAM banks or some information
about the reset state of the machine,for example.

This series first adds a bloblist, which allows a list of 'blobs' to be
created, each with a tag so that subsystems can store data and retrieve it
later. Then it adds the SPL 'handoff' information, which uses bloblist.

Various minor sandbox enhancements are provided to make this easier, or to
support testing.

Changes in v2:
- Fix several typos
- Add back a blank line, and put one in the new function too
- Drop the wildcard in the .lds file as it is not needed

Simon Glass (23):
  log: Correct definition of log_msg_ret()
  spl: Add support for logging in SPL and TPL
  Add core support for a bloblist to convey data from SPL
  spl: Set up the bloblist in SPL
  bloblist: Locate bloblist in U-Boot
  test: Add a simple test for bloblist
  Add bloblist documentation
  spl: Support hash, input, pch, pci, rtc, tpm in SPL
  spl: Add a define for SPL_TPL_PROMPT
  spl: Make SPL_DISABLE_BANNER_PRINT a positive option
  spl: Add a comment to spl_set_bd()
  spl: Print a message if we are unable to load an image
  sandbox: Add a memory map to the sandbox README
  test/py: Add a way to pass flags to sandbox
  sandbox: Add an option to display of-platdata in SPL
  sandbox: Drop the deprecated 'sb' command
  sandbox: Add a new 'sb' command
  sandbox: Allow puts() output before global_data is set up
  sandbox: Refactor code to create os_jump_to_file()
  sandbox: Use malloc() and free() from os layer
  sandbox: Filter arguments when starting U-Boot
  sandbox: Boot in U-Boot through the standard call
  spl: Add support for passing handoff info to U-Boot proper

 arch/Kconfig                       |   1 +
 arch/powerpc/include/asm/spl.h     |   3 -
 arch/sandbox/cpu/eth-raw-os.c      |   9 +-
 arch/sandbox/cpu/os.c              | 115 +++++++++-----
 arch/sandbox/cpu/spl.c             |  31 +++-
 arch/sandbox/cpu/start.c           |  19 +++
 arch/sandbox/cpu/u-boot-spl.lds    |   2 +-
 arch/sandbox/include/asm/handoff.h |  18 +++
 arch/sandbox/include/asm/state.h   |   8 +
 board/sandbox/README.sandbox       |  14 +-
 cmd/Makefile                       |   1 +
 cmd/host.c                         |   5 -
 cmd/sb.c                           |  65 ++++++++
 common/Kconfig                     |  84 +++++++++-
 common/Makefile                    |   5 +-
 common/bloblist.c                  | 239 +++++++++++++++++++++++++++++
 common/board_f.c                   |  49 ++++++
 common/console.c                   |   7 +
 common/init/Makefile               |   1 +
 common/init/handoff.c              |  47 ++++++
 common/spl/Kconfig                 |  95 +++++++++++-
 common/spl/spl.c                   | 142 +++++++++++++----
 configs/sandbox_spl_defconfig      |   1 +
 doc/README.bloblist                |  82 ++++++++++
 doc/README.trace                   |   2 +-
 drivers/Makefile                   |  11 +-
 include/asm-generic/global_data.h  |   7 +
 include/bloblist.h                 | 195 +++++++++++++++++++++++
 include/handoff.h                  |  36 +++++
 include/log.h                      |  10 +-
 include/spl.h                      |  41 +++++
 include/test/suites.h              |   1 +
 test/Makefile                      |   1 +
 test/bloblist.c                    | 187 ++++++++++++++++++++++
 test/cmd_ut.c                      |   3 +
 test/dm/sf.c                       |   2 +-
 test/py/tests/test_fit.py          |  12 +-
 test/py/tests/test_handoff.py      |  14 ++
 test/py/tests/test_ofplatdata.py   |  31 +++-
 test/py/tests/test_vboot.py        |   2 +-
 test/py/u_boot_console_base.py     |   2 +-
 test/py/u_boot_console_sandbox.py  |  18 +++
 test/run                           |   2 +-
 43 files changed, 1510 insertions(+), 110 deletions(-)
 create mode 100644 arch/sandbox/include/asm/handoff.h
 create mode 100644 cmd/sb.c
 create mode 100644 common/bloblist.c
 create mode 100644 common/init/handoff.c
 create mode 100644 doc/README.bloblist
 create mode 100644 include/bloblist.h
 create mode 100644 include/handoff.h
 create mode 100644 test/bloblist.c
 create mode 100644 test/py/tests/test_handoff.py

-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 01/23] log: Correct definition of log_msg_ret()
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-10-04  9:25   ` Bin Meng
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 02/23] spl: Add support for logging in SPL and TPL Simon Glass
                   ` (21 subsequent siblings)
  22 siblings, 1 reply; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

This macro should have two parameters, not one. Fix it so that it
correctly resolves to _ret when logging is disabled.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 include/log.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/log.h b/include/log.h
index 653fb8d853e..75ff1e1160c 100644
--- a/include/log.h
+++ b/include/log.h
@@ -175,7 +175,7 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line,
 	})
 #else
 #define log_ret(_ret) (_ret)
-#define log_msg_ret(_ret) (_ret)
+#define log_msg_ret(_msg, _ret) (_ret)
 #endif
 
 /**
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 02/23] spl: Add support for logging in SPL and TPL
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 01/23] log: Correct definition of log_msg_ret() Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-11-09 18:42   ` [U-Boot] [U-Boot, v2, " Tom Rini
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 03/23] Add core support for a bloblist to convey data from SPL Simon Glass
                   ` (20 subsequent siblings)
  22 siblings, 1 reply; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

It is sometimes useful to log information in SPL and TPL. Add support for
this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/Kconfig   | 36 ++++++++++++++++++++++++++++++++++--
 common/Makefile  |  4 ++--
 common/spl/spl.c |  7 +++++++
 include/log.h    |  7 ++++++-
 4 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index 41f27a13383..3bb9571b710 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -290,6 +290,10 @@ config SPL_LOGLEVEL
 	int
 	default LOGLEVEL
 
+config TPL_LOGLEVEL
+	int
+	default LOGLEVEL
+
 config SILENT_CONSOLE
 	bool "Support a silent console"
 	help
@@ -486,6 +490,24 @@ config SPL_LOG_MAX_LEVEL
 	    6 - detail
 	    7 - debug
 
+config TPL_LOG_MAX_LEVEL
+	int "Maximum log level to record in TPL"
+	depends on TPL_LOG
+	default 3
+	help
+	  This selects the maximum log level that will be recorded. Any value
+	  higher than this will be ignored. If possible log statements below
+	  this level will be discarded at build time. Levels:
+
+	    0 - panic
+	    1 - critical
+	    2 - error
+	    3 - warning
+	    4 - note
+	    5 - info
+	    6 - detail
+	    7 - debug
+
 config LOG_CONSOLE
 	bool "Allow log output to the console"
 	depends on LOG
@@ -496,9 +518,19 @@ config LOG_CONSOLE
 	  log message is shown - other details like level, category, file and
 	  line number are omitted.
 
-config LOG_SPL_CONSOLE
+config SPL_LOG_CONSOLE
+	bool "Allow log output to the console in SPL"
+	depends on SPL_LOG
+	default y
+	help
+	  Enables a log driver which writes log records to the console.
+	  Generally the console is the serial port or LCD display. Only the
+	  log message is shown - other details like level, category, file and
+	  line number are omitted.
+
+config TPL_LOG_CONSOLE
 	bool "Allow log output to the console in SPL"
-	depends on LOG_SPL
+	depends on TPL_LOG
 	default y
 	help
 	  Enables a log driver which writes log records to the console.
diff --git a/common/Makefile b/common/Makefile
index 7473b850115..cbca4ff2da6 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -118,8 +118,8 @@ obj-y += cli.o
 obj-$(CONFIG_FSL_DDR_INTERACTIVE) += cli_simple.o cli_readline.o
 obj-$(CONFIG_DFU_OVER_USB) += dfu.o
 obj-y += command.o
-obj-$(CONFIG_$(SPL_)LOG) += log.o
-obj-$(CONFIG_$(SPL_)LOG_CONSOLE) += log_console.o
+obj-$(CONFIG_$(SPL_TPL_)LOG) += log.o
+obj-$(CONFIG_$(SPL_TPL_)LOG_CONSOLE) += log_console.o
 obj-y += s_record.o
 obj-y += xyzModem.o
 
diff --git a/common/spl/spl.c b/common/spl/spl.c
index 038f2b0e83c..b917624e61d 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -338,6 +338,13 @@ static int spl_common_init(bool setup_malloc)
 		return ret;
 	}
 	bootstage_mark_name(BOOTSTAGE_ID_START_SPL, "spl");
+#if CONFIG_IS_ENABLED(LOG)
+	ret = log_init();
+	if (ret) {
+		debug("%s: Failed to set up logging\n", __func__);
+		return ret;
+	}
+#endif
 	if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) {
 		ret = fdtdec_setup();
 		if (ret) {
diff --git a/include/log.h b/include/log.h
index 75ff1e1160c..1146f423a7a 100644
--- a/include/log.h
+++ b/include/log.h
@@ -92,6 +92,8 @@ int _log(enum log_category_t cat, enum log_level_t level, const char *file,
 #define _LOG_MAX_LEVEL LOGL_INFO
 #endif
 
+#if CONFIG_IS_ENABLED(LOG)
+
 /* Emit a log record if the level is less that the maximum */
 #define log(_cat, _level, _fmt, _args...) ({ \
 	int _l = _level; \
@@ -100,6 +102,9 @@ int _log(enum log_category_t cat, enum log_level_t level, const char *file,
 		      __func__, \
 		      pr_fmt(_fmt), ##_args); \
 	})
+#else
+#define log(_cat, _level, _fmt, _args...)
+#endif
 
 #ifdef DEBUG
 #define _DEBUG	1
@@ -159,7 +164,7 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line,
 	({ if (!(x) && _DEBUG) \
 		__assert_fail(#x, __FILE__, __LINE__, __func__); })
 
-#ifdef CONFIG_LOG_ERROR_RETURN
+#if CONFIG_IS_ENABLED(LOG) && defined(CONFIG_LOG_ERROR_RETURN)
 #define log_ret(_ret) ({ \
 	int __ret = (_ret); \
 	if (__ret < 0) \
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 03/23] Add core support for a bloblist to convey data from SPL
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 01/23] log: Correct definition of log_msg_ret() Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 02/23] spl: Add support for logging in SPL and TPL Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-10-08  2:42   ` Kever Yang
  2018-11-09 18:42   ` [U-Boot] [U-Boot, v2, " Tom Rini
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 04/23] spl: Set up the bloblist in SPL Simon Glass
                   ` (19 subsequent siblings)
  22 siblings, 2 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

At present there is no standard way in U-Boot to pass information from SPL
to U-Boot proper. But sometimes SPL wants to convey information to U-Boot
that U-Boot cannot easily figure out. For example, if SPL sets up SDRAM
then it might want to pass the size of SDRAM, or the location of each
bank, to U-Boot proper.

Add a new 'bloblist' feature which provides this. A bloblist is set up in
the first phase of U-Boot that runs (i.e. TPL or SPL). The location of
this info may be in SRAM or CAR (x86 cache-as-RAM) or somewhere else.

Information placed in this region is preserved (with a checksum) through
TPL and SPL and ends up in U-Boot. At this point it is copied into SDRAM
so it can be used after relocation.

Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Andreas Dannenberg <dannenberg@ti.com>
---

Changes in v2:
- Fix several typos

 common/Kconfig     |  48 +++++++++
 common/Makefile    |   1 +
 common/bloblist.c  | 239 +++++++++++++++++++++++++++++++++++++++++++++
 include/bloblist.h | 195 ++++++++++++++++++++++++++++++++++++
 include/log.h      |   1 +
 5 files changed, 484 insertions(+)
 create mode 100644 common/bloblist.c
 create mode 100644 include/bloblist.h

diff --git a/common/Kconfig b/common/Kconfig
index 3bb9571b710..2e72b3c83c6 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -715,4 +715,52 @@ config UPDATE_TFTP_MSEC_MAX
 
 endmenu
 
+menu "Blob list"
+
+config BLOBLIST
+	bool "Support for a bloblist"
+	help
+	  This enables support for a bloblist in U-Boot, which can be passed
+	  from TPL to SPL to U-Boot proper (and potentially to Linux). The
+	  blob list supports multiple binary blobs of data, each with a tag,
+	  so that different U-Boot components can store data which can survive
+	  through to the next stage of the boot.
+
+config SPL_BLOBLIST
+	bool "Support for a bloblist in SPL"
+	depends on BLOBLIST
+	default y if SPL
+	help
+	  This enables a bloblist in SPL. If this is the first part of U-Boot
+	  to run, then the bloblist is set up in SPL and passed to U-Boot
+	  proper. If TPL also has a bloblist, then SPL uses the one from there.
+
+config TPL_BLOBLIST
+	bool "Support for a bloblist in TPL"
+	depends on BLOBLIST
+	default y if TPL
+	help
+	  This enables a bloblist in TPL. The bloblist is set up in TPL and
+	  passed to SPL and U-Boot proper.
+
+config BLOBLIST_SIZE
+	hex "Size of bloblist"
+	depends on BLOBLIST
+	default 0x400
+	help
+	  Sets the size of the bloblist in bytes. This must include all
+	  overhead (alignment, bloblist header, record header). The bloblist
+	  is set up in the first part of U-Boot to run (TPL, SPL or U-Boot
+	  proper), and this sane bloblist is used for subsequent stages.
+
+config BLOBLIST_ADDR
+	hex "Address of bloblist"
+	depends on BLOBLIST
+	default 0xe000
+	help
+	  Sets the address of the bloblist, set up by the first part of U-Boot
+	  which runs. Subsequent U-Boot stages typically use the same address.
+
+endmenu
+
 source "common/spl/Kconfig"
diff --git a/common/Makefile b/common/Makefile
index cbca4ff2da6..6aff2b1a6e3 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_CMDLINE) += cli_readline.o cli_simple.o
 endif # !CONFIG_SPL_BUILD
 
 obj-$(CONFIG_$(SPL_TPL_)BOOTSTAGE) += bootstage.o
+obj-$(CONFIG_$(SPL_TPL_)BLOBLIST) += bloblist.o
 
 ifdef CONFIG_SPL_BUILD
 ifdef CONFIG_SPL_DFU_SUPPORT
diff --git a/common/bloblist.c b/common/bloblist.c
new file mode 100644
index 00000000000..b4cf169b05a
--- /dev/null
+++ b/common/bloblist.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2018 Google, Inc
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <common.h>
+#include <bloblist.h>
+#include <log.h>
+#include <mapmem.h>
+#include <spl.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+struct bloblist_rec *bloblist_first_blob(struct bloblist_hdr *hdr)
+{
+	if (hdr->alloced <= hdr->hdr_size)
+		return NULL;
+	return (struct bloblist_rec *)((void *)hdr + hdr->hdr_size);
+}
+
+struct bloblist_rec *bloblist_next_blob(struct bloblist_hdr *hdr,
+					struct bloblist_rec *rec)
+{
+	ulong offset;
+
+	offset = (void *)rec - (void *)hdr;
+	offset += rec->hdr_size + ALIGN(rec->size, BLOBLIST_ALIGN);
+	if (offset >= hdr->alloced)
+		return NULL;
+	return (struct bloblist_rec *)((void *)hdr + offset);
+}
+
+#define foreach_rec(_rec, _hdr) \
+	for (_rec = bloblist_first_blob(_hdr); \
+	     _rec; \
+	     _rec = bloblist_next_blob(_hdr, _rec))
+
+static struct bloblist_rec *bloblist_findrec(uint tag)
+{
+	struct bloblist_hdr *hdr = gd->bloblist;
+	struct bloblist_rec *rec;
+
+	if (!hdr)
+		return NULL;
+
+	foreach_rec(rec, hdr) {
+		if (rec->tag == tag)
+			return rec;
+	}
+
+	return NULL;
+}
+
+static int bloblist_addrec(uint tag, int size, struct bloblist_rec **recp)
+{
+	struct bloblist_hdr *hdr = gd->bloblist;
+	struct bloblist_rec *rec;
+	int new_alloced;
+
+	new_alloced = hdr->alloced + sizeof(*rec) +
+			ALIGN(size, BLOBLIST_ALIGN);
+	if (new_alloced >= hdr->size) {
+		log(LOGC_BLOBLIST, LOGL_ERR,
+		    "Failed to allocate %x bytes size=%x, need size>=%x\n",
+		    size, hdr->size, new_alloced);
+		return log_msg_ret("bloblist add", -ENOSPC);
+	}
+	rec = (void *)hdr + hdr->alloced;
+	hdr->alloced = new_alloced;
+
+	rec->tag = tag;
+	rec->hdr_size = sizeof(*rec);
+	rec->size = size;
+	rec->spare = 0;
+	*recp = rec;
+
+	return 0;
+}
+
+static int bloblist_ensurerec(uint tag, struct bloblist_rec **recp, int size)
+{
+	struct bloblist_rec *rec;
+
+	rec = bloblist_findrec(tag);
+	if (rec) {
+		if (size && size != rec->size)
+			return -ESPIPE;
+	} else {
+		int ret;
+
+		ret = bloblist_addrec(tag, size, &rec);
+		if (ret)
+			return ret;
+	}
+	*recp = rec;
+
+	return 0;
+}
+
+void *bloblist_find(uint tag, int size)
+{
+	struct bloblist_rec *rec;
+
+	rec = bloblist_findrec(tag);
+	if (!rec)
+		return NULL;
+	if (size && size != rec->size)
+		return NULL;
+
+	return (void *)rec + rec->hdr_size;
+}
+
+void *bloblist_add(uint tag, int size)
+{
+	struct bloblist_rec *rec;
+
+	if (bloblist_addrec(tag, size, &rec))
+		return NULL;
+
+	return rec + 1;
+}
+
+int bloblist_ensure_size(uint tag, int size, void **blobp)
+{
+	struct bloblist_rec *rec;
+	int ret;
+
+	ret = bloblist_ensurerec(tag, &rec, size);
+	if (ret)
+		return ret;
+	*blobp = (void *)rec + rec->hdr_size;
+
+	return 0;
+}
+
+void *bloblist_ensure(uint tag, int size)
+{
+	struct bloblist_rec *rec;
+
+	if (bloblist_ensurerec(tag, &rec, size))
+		return NULL;
+
+	return (void *)rec + rec->hdr_size;
+}
+
+static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr)
+{
+	struct bloblist_rec *rec;
+	u32 chksum;
+
+	chksum = crc32(0, (unsigned char *)hdr,
+		       offsetof(struct bloblist_hdr, chksum));
+	foreach_rec(rec, hdr) {
+		chksum = crc32(chksum, (void *)rec, rec->hdr_size);
+		chksum = crc32(chksum, (void *)rec + rec->hdr_size, rec->size);
+	}
+
+	return chksum;
+}
+
+int bloblist_new(ulong addr, uint size, uint flags)
+{
+	struct bloblist_hdr *hdr;
+
+	if (size < sizeof(*hdr))
+		return log_ret(-ENOSPC);
+	if (addr & (BLOBLIST_ALIGN - 1))
+		return log_ret(-EFAULT);
+	hdr = map_sysmem(addr, size);
+	memset(hdr, '\0', sizeof(*hdr));
+	hdr->version = BLOBLIST_VERSION;
+	hdr->hdr_size = sizeof(*hdr);
+	hdr->flags = flags;
+	hdr->magic = BLOBLIST_MAGIC;
+	hdr->size = size;
+	hdr->alloced = hdr->hdr_size;
+	hdr->chksum = 0;
+	gd->bloblist = hdr;
+
+	return 0;
+}
+
+int bloblist_check(ulong addr, uint size)
+{
+	struct bloblist_hdr *hdr;
+	u32 chksum;
+
+	hdr = map_sysmem(addr, sizeof(*hdr));
+	if (hdr->magic != BLOBLIST_MAGIC)
+		return log_msg_ret("Bad magic", -ENOENT);
+	if (hdr->version != BLOBLIST_VERSION)
+		return log_msg_ret("Bad version", -EPROTONOSUPPORT);
+	if (size && hdr->size != size)
+		return log_msg_ret("Bad size", -EFBIG);
+	chksum = bloblist_calc_chksum(hdr);
+	if (hdr->chksum != chksum) {
+		log(LOGC_BLOBLIST, LOGL_ERR, "Checksum %x != %x\n", hdr->chksum,
+		    chksum);
+		return log_msg_ret("Bad checksum", -EIO);
+	}
+	gd->bloblist = hdr;
+
+	return 0;
+}
+
+int bloblist_finish(void)
+{
+	struct bloblist_hdr *hdr = gd->bloblist;
+
+	hdr->chksum = bloblist_calc_chksum(hdr);
+
+	return 0;
+}
+
+int bloblist_init(void)
+{
+	bool expected;
+	int ret = -ENOENT;
+
+	/**
+	 * Wed expect to find an existing bloblist in the first phase of U-Boot
+	 * that runs
+	 */
+	expected = !u_boot_first_phase();
+	if (expected)
+		ret = bloblist_check(CONFIG_BLOBLIST_ADDR,
+				     CONFIG_BLOBLIST_SIZE);
+	if (ret) {
+		log(LOGC_BLOBLIST, expected ? LOGL_WARNING : LOGL_DEBUG,
+		    "Existing bloblist not found: creating new bloblist\n");
+		ret = bloblist_new(CONFIG_BLOBLIST_ADDR, CONFIG_BLOBLIST_SIZE,
+				   0);
+	} else {
+		log(LOGC_BLOBLIST, LOGL_DEBUG, "Found existing bloblist\n");
+	}
+
+	return ret;
+}
diff --git a/include/bloblist.h b/include/bloblist.h
new file mode 100644
index 00000000000..413736a9080
--- /dev/null
+++ b/include/bloblist.h
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * This provides a standard way of passing information between boot phases
+ * (TPL -> SPL -> U-Boot proper.)
+ *
+ * A list of blobs of data, tagged with their owner. The list resides in memory
+ * and can be updated by SPL, U-Boot, etc.
+ *
+ * Copyright 2018 Google, Inc
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#ifndef __BLOBLIST_H
+#define __BLOBLIST_H
+
+enum {
+	BLOBLIST_VERSION	= 0,
+	BLOBLIST_MAGIC		= 0xb00757a3,
+	BLOBLIST_ALIGN		= 16,
+};
+
+enum bloblist_tag_t {
+	BLOBLISTT_NONE = 0,
+
+	/* Vendor-specific tags are permitted here */
+	BLOBLISTT_EC_HOSTEVENT,		/* Chromium OS EC host-event mask */
+	BLOBLISTT_SPL_HANDOFF,		/* Hand-off info from SPL */
+	BLOBLISTT_VBOOT_CTX,		/* Chromium OS verified boot context */
+	BLOBLISTT_VBOOT_HANDOFF,	/* Chromium OS internal handoff info */
+};
+
+/**
+ * struct bloblist_hdr - header for the bloblist
+ *
+ * This is stored at the start of the bloblist which is always on a 16-byte
+ * boundary. Records follow this header. The bloblist normally stays in the
+ * same place in memory as SPL and U-Boot execute, but it can be safely moved
+ * around.
+ *
+ * None of the bloblist structures contain pointers but it is possible to put
+ * pointers inside a bloblist record if desired. This is not encouraged,
+ * since it can make part of the bloblist inaccessible if the pointer is
+ * no-longer valid. It is better to just store all the data inside a bloblist
+ * record.
+ *
+ * Each bloblist record is aligned to a 16-byte boundary and follows immediately
+ * from the last.
+ *
+ * @version: BLOBLIST_VERSION
+ * @hdr_size: Size of this header, normally sizeof(struct bloblist_hdr). The
+ *	first bloblist_rec starts at this offset from the start of the header
+ * @flags: Space for BLOBLISTF_... flags (none yet)
+ * @magic: BLOBLIST_MAGIC
+ * @size: Total size of all records (non-zero if valid) including this header.
+ *	The bloblist extends for this many bytes from the start of this header.
+ * @alloced: Total size allocated for this bloblist. When adding new records,
+ *	the bloblist can grow up to this size. This starts out as
+ *	sizeof(bloblist_hdr) since we need at least that much space to store a
+ *	valid bloblist
+ * @spare: Space space
+ * @chksum: CRC32 for the entire bloblist allocated area. Since any of the
+ *	blobs can be altered after being created, this checksum is only valid
+ *	when the bloblist is finalised before jumping to the next stage of boot.
+ *	Note: @chksum is last to make it easier to exclude it from the checksum
+ *	calculation.
+ */
+struct bloblist_hdr {
+	u32 version;
+	u32 hdr_size;
+	u32 flags;
+	u32 magic;
+
+	u32 size;
+	u32 alloced;
+	u32 spare;
+	u32 chksum;
+};
+
+/**
+ * struct bloblist_rec - record for the bloblist
+ *
+ * NOTE: Only exported for testing purposes. Do not use this struct.
+ *
+ * The bloblist contains a number of records each consisting of this record
+ * structure followed by the data contained. Each records is 16-byte aligned.
+ *
+ * @tag: Tag indicating what the record contains
+ * @hdr_size: Size of this header, normally sizeof(struct bloblist_rec). The
+ *	record's data starts at this offset from the start of the record
+ * @size: Size of record in bytes, excluding the header size. This does not
+ *	need to be aligned (e.g. 3 is OK).
+ * @spare: Spare space for other things
+ */
+struct bloblist_rec {
+	u32 tag;
+	u32 hdr_size;
+	u32 size;
+	u32 spare;
+};
+
+/**
+ * bloblist_find() - Find a blob
+ *
+ * Searches the bloblist and returns the blob with the matching tag
+ *
+ * @tag:	Tag to search for (enum bloblist_tag_t)
+ * @size:	Expected size of the blob
+ * @return pointer to blob if found, or NULL if not found, or a blob was found
+ *	but it is the wrong size
+ */
+void *bloblist_find(uint tag, int size);
+
+/**
+ * bloblist_add() - Add a new blob
+ *
+ * Add a new blob to the bloblist
+ *
+ * This should only be called if you konw there is no existing blob for a
+ * particular tag. It is typically safe to call in the first phase of U-Boot
+ * (e.g. TPL or SPL). After that, bloblist_ensure() should be used instead.
+ *
+ * @tag:	Tag to add (enum bloblist_tag_t)
+ * @size:	Size of the blob
+ * @return pointer to the newly added block, or NULL if there is not enough
+ *	space for the blob
+ */
+void *bloblist_add(uint tag, int size);
+
+/**
+ * bloblist_ensure_size() - Find or add a blob
+ *
+ * Find an existing blob, or add a new one if not found
+ *
+ * @tag:	Tag to add (enum bloblist_tag_t)
+ * @size:	Size of the blob
+ * @blobp:	Returns a pointer to blob on success
+ * @return 0 if OK, -ENOSPC if it is missing and could not be added due to lack
+ *	of space, or -ESPIPE it exists but has the wrong size
+ */
+int bloblist_ensure_size(uint tag, int size, void **blobp);
+
+/**
+ * bloblist_ensure() - Find or add a blob
+ *
+ * Find an existing blob, or add a new one if not found
+ *
+ * @tag:	Tag to add (enum bloblist_tag_t)
+ * @size:	Size of the blob
+ * @return pointer to blob, or NULL if it is missing and could not be added due
+ *	to lack of space, or it exists but has the wrong size
+ */
+void *bloblist_ensure(uint tag, int size);
+
+/**
+ * bloblist_new() - Create a new, empty bloblist of a given size
+ *
+ * @addr: Address of bloblist
+ * @size: Initial size for bloblist
+ * @flags: Flags to use for bloblist
+ * @return 0 if OK, -EFAULT if addr is not aligned correctly, -ENOSPC is the
+ *	area is not large enough
+ */
+int bloblist_new(ulong addr, uint size, uint flags);
+
+/**
+ * bloblist_check() - Check if a bloblist exists
+ *
+ * @addr: Address of bloblist
+ * @size: Expected size of blobsize, or 0 to detect the size
+ * @return 0 if OK, -ENOENT if the magic number doesn't match (indicating that
+ *	there problem is no bloblist at the given address), -EPROTONOSUPPORT
+ *	if the version does not match, -EIO if the checksum does not match,
+ *	-EFBIG if the expected size does not match the detected size
+ */
+int bloblist_check(ulong addr, uint size);
+
+/**
+ * bloblist_finish() - Set up the bloblist for the next U-Boot part
+ *
+ * This sets the correct checksum for the bloblist. This ensures that the
+ * bloblist will be detected correctly by the next phase of U-Boot.
+ *
+ * @return 0
+ */
+int bloblist_finish(void);
+
+/**
+ * bloblist_init() - Init the bloblist system with a single bloblist
+ *
+ * This uses CONFIG_BLOBLIST_ADDR and CONFIG_BLOBLIST_SIZE to set up a bloblist
+ * for use by U-Boot.
+ */
+int bloblist_init(void);
+
+#endif /* __BLOBLIST_H */
diff --git a/include/log.h b/include/log.h
index 1146f423a7a..61411b72eac 100644
--- a/include/log.h
+++ b/include/log.h
@@ -46,6 +46,7 @@ enum log_category_t {
 	LOGC_DM,	/* Core driver-model */
 	LOGC_DT,	/* Device-tree */
 	LOGC_EFI,	/* EFI implementation */
+	LOGC_BLOBLIST,	/* Bloblist */
 
 	LOGC_COUNT,
 	LOGC_END,
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 04/23] spl: Set up the bloblist in SPL
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (2 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 03/23] Add core support for a bloblist to convey data from SPL Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-11-09 18:43   ` [U-Boot] [U-Boot,v2,04/23] " Tom Rini
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 05/23] bloblist: Locate bloblist in U-Boot Simon Glass
                   ` (18 subsequent siblings)
  22 siblings, 1 reply; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

The bloblist is normally set up in SPL ready for use by U-Boot. Add
a simple implementation of this to the common SPL code.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/spl/spl.c | 18 ++++++++++++++++--
 include/spl.h    | 27 +++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index b917624e61d..2ca900ae9ed 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -7,6 +7,7 @@
  */
 
 #include <common.h>
+#include <bloblist.h>
 #include <binman_sym.h>
 #include <dm.h>
 #include <spl.h>
@@ -345,6 +346,14 @@ static int spl_common_init(bool setup_malloc)
 		return ret;
 	}
 #endif
+	if (CONFIG_IS_ENABLED(BLOBLIST)) {
+		ret = bloblist_init();
+		if (ret) {
+			debug("%s: Failed to set up bloblist: ret=%d\n",
+			      __func__, ret);
+			return ret;
+		}
+	}
 	if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) {
 		ret = fdtdec_setup();
 		if (ret) {
@@ -483,6 +492,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 		BOOT_DEVICE_NONE,
 	};
 	struct spl_image_info spl_image;
+	int ret;
 
 	debug(">>spl:board_init_r()\n");
 
@@ -529,6 +539,12 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 	}
 
 	spl_perform_fixups(&spl_image);
+	if (CONFIG_IS_ENABLED(BLOBLIST)) {
+		ret = bloblist_finish();
+		if (ret)
+			printf("Warning: Failed to finish bloblist (ret=%d)\n",
+			       ret);
+	}
 
 #ifdef CONFIG_CPU_V7M
 	spl_image.entry_point |= 0x1;
@@ -558,8 +574,6 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 	      gd->malloc_ptr / 1024);
 #endif
 #ifdef CONFIG_BOOTSTAGE_STASH
-	int ret;
-
 	bootstage_mark_name(BOOTSTAGE_ID_END_SPL, "end_spl");
 	ret = bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
 			      CONFIG_BOOTSTAGE_STASH_SIZE);
diff --git a/include/spl.h b/include/spl.h
index b42683c9e71..123244f50bb 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -21,6 +21,33 @@
 #define MMCSD_MODE_FS		2
 #define MMCSD_MODE_EMMCBOOT	3
 
+/*
+ * u_boot_first_phase() - check if this is the first U-Boot phase
+ *
+ * U-Boot has up to three phases: TPL, SPL and U-Boot proper. Depending on the
+ * build flags we can determine whether the current build is for the first
+ * phase of U-Boot or not. If there is no SPL, then this is U-Boot proper. If
+ * there is SPL but no TPL, the the first phase is SPL. If there is TPL, then
+ * it is the first phase.
+ *
+ * @returns true if this is the first phase of U-Boot
+ *
+ */
+static inline bool u_boot_first_phase(void)
+{
+	if (IS_ENABLED(CONFIG_TPL)) {
+		if (IS_ENABLED(CONFIG_TPL_BUILD))
+			return true;
+	} else if (IS_ENABLED(CONFIG_SPL)) {
+		if (IS_ENABLED(CONFIG_SPL_BUILD))
+			return true;
+	} else {
+		return true;
+	}
+
+	return false;
+}
+
 struct spl_image_info {
 	const char *name;
 	u8 os;
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 05/23] bloblist: Locate bloblist in U-Boot
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (3 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 04/23] spl: Set up the bloblist in SPL Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-11-09 18:43   ` [U-Boot] [U-Boot,v2,05/23] " Tom Rini
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 06/23] test: Add a simple test for bloblist Simon Glass
                   ` (17 subsequent siblings)
  22 siblings, 1 reply; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

Add support for locating a bloblist in U-Boot that has been set up by SPL.
It is copied into RAM during relocation.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/board_f.c                  | 34 +++++++++++++++++++++++++++++++
 include/asm-generic/global_data.h |  4 ++++
 2 files changed, 38 insertions(+)

diff --git a/common/board_f.c b/common/board_f.c
index 213d0440667..d272e0aba62 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -10,6 +10,7 @@
  */
 
 #include <common.h>
+#include <bloblist.h>
 #include <console.h>
 #include <cpu.h>
 #include <dm.h>
@@ -560,6 +561,16 @@ static int reserve_stacks(void)
 	return arch_reserve_stacks();
 }
 
+static int reserve_bloblist(void)
+{
+#ifdef CONFIG_BLOBLIST
+	gd->start_addr_sp -= CONFIG_BLOBLIST_SIZE;
+	gd->new_bloblist = map_sysmem(gd->start_addr_sp, CONFIG_BLOBLIST_SIZE);
+#endif
+
+	return 0;
+}
+
 static int display_new_sp(void)
 {
 	debug("New Stack Pointer is: %08lx\n", gd->start_addr_sp);
@@ -666,6 +677,24 @@ static int reloc_bootstage(void)
 	return 0;
 }
 
+static int reloc_bloblist(void)
+{
+#ifdef CONFIG_BLOBLIST
+	if (gd->flags & GD_FLG_SKIP_RELOC)
+		return 0;
+	if (gd->new_bloblist) {
+		int size = CONFIG_BLOBLIST_SIZE;
+
+		debug("Copying bloblist from %p to %p, size %x\n",
+		      gd->bloblist, gd->new_bloblist, size);
+		memcpy(gd->new_bloblist, gd->bloblist, size);
+		gd->bloblist = gd->new_bloblist;
+	}
+#endif
+
+	return 0;
+}
+
 static int setup_reloc(void)
 {
 	if (gd->flags & GD_FLG_SKIP_RELOC) {
@@ -813,6 +842,9 @@ static const init_fnc_t init_sequence_f[] = {
 	initf_malloc,
 	log_init,
 	initf_bootstage,	/* uses its own timer, so does not need DM */
+#ifdef CONFIG_BLOBLIST
+	bloblist_init,
+#endif
 	initf_console_record,
 #if defined(CONFIG_HAVE_FSP)
 	arch_fsp_init,
@@ -913,6 +945,7 @@ static const init_fnc_t init_sequence_f[] = {
 	reserve_global_data,
 	reserve_fdt,
 	reserve_bootstage,
+	reserve_bloblist,
 	reserve_arch,
 	reserve_stacks,
 	dram_init_banksize,
@@ -932,6 +965,7 @@ static const init_fnc_t init_sequence_f[] = {
 	INIT_FUNC_WATCHDOG_RESET
 	reloc_fdt,
 	reloc_bootstage,
+	reloc_bloblist,
 	setup_reloc,
 #if defined(CONFIG_X86) || defined(CONFIG_ARC)
 	copy_uboot_to_ram,
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index c83fc01b764..ccf361ed88a 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -122,6 +122,10 @@ typedef struct global_data {
 	struct list_head log_head;	/* List of struct log_device */
 	int log_fmt;			/* Mask containing log format info */
 #endif
+#if CONFIG_IS_ENABLED(BLOBLIST)
+	struct bloblist_hdr *bloblist;	/* Bloblist information */
+	struct bloblist_hdr *new_bloblist;	/* Relocated blolist info */
+#endif
 } gd_t;
 #endif
 
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 06/23] test: Add a simple test for bloblist
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (4 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 05/23] bloblist: Locate bloblist in U-Boot Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 07/23] Add bloblist documentation Simon Glass
                   ` (16 subsequent siblings)
  22 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

Add a unit test for the bloblist functionality and enable bloblist for
sandbox.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/Kconfig          |   1 +
 include/test/suites.h |   1 +
 test/Makefile         |   1 +
 test/bloblist.c       | 187 ++++++++++++++++++++++++++++++++++++++++++
 test/cmd_ut.c         |   3 +
 5 files changed, 193 insertions(+)
 create mode 100644 test/bloblist.c

diff --git a/arch/Kconfig b/arch/Kconfig
index 9b4bcbf2fd2..6dd130052b9 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -79,6 +79,7 @@ config SANDBOX
 	select SPI
 	select SUPPORT_OF_CONTROL
 	imply BITREVERSE
+	select BLOBLIST
 	imply CMD_DM
 	imply CMD_GETTIME
 	imply CMD_HASH
diff --git a/include/test/suites.h b/include/test/suites.h
index abb3a4b8169..77d863b4a6a 100644
--- a/include/test/suites.h
+++ b/include/test/suites.h
@@ -23,6 +23,7 @@ struct unit_test;
 int cmd_ut_category(const char *name, struct unit_test *tests, int n_ents,
 		    int argc, char * const argv[]);
 
+int do_ut_bloblist(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]);
 int do_ut_compression(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]);
 int do_ut_dm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 int do_ut_env(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
diff --git a/test/Makefile b/test/Makefile
index 1e434730b68..2fe41f489c3 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -2,6 +2,7 @@
 #
 # (C) Copyright 2012 The Chromium Authors
 
+obj-$(CONFIG_SANDBOX) += bloblist.o
 obj-$(CONFIG_UNIT_TEST) += cmd_ut.o
 obj-$(CONFIG_UNIT_TEST) += ut.o
 obj-$(CONFIG_SANDBOX) += command_ut.o
diff --git a/test/bloblist.c b/test/bloblist.c
new file mode 100644
index 00000000000..89bdb012e35
--- /dev/null
+++ b/test/bloblist.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2018, Google Inc. All rights reserved.
+ */
+
+#include <common.h>
+#include <bloblist.h>
+#include <log.h>
+#include <mapmem.h>
+#include <test/suites.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* Declare a new compression test */
+#define BLOBLIST_TEST(_name, _flags) \
+		UNIT_TEST(_name, _flags, bloblist_test)
+
+enum {
+	TEST_TAG		= 1,
+	TEST_TAG2		= 2,
+	TEST_TAG_MISSING	= 3,
+
+	TEST_SIZE		= 10,
+	TEST_SIZE2		= 20,
+
+	TEST_ADDR		= CONFIG_BLOBLIST_ADDR,
+	TEST_BLOBLIST_SIZE	= 0x100,
+};
+
+static struct bloblist_hdr *clear_bloblist(void)
+{
+	struct bloblist_hdr *hdr;
+
+	/* Clear out any existing bloblist so we have a clean slate */
+	hdr = map_sysmem(CONFIG_BLOBLIST_ADDR, TEST_BLOBLIST_SIZE);
+	memset(hdr, '\0', TEST_BLOBLIST_SIZE);
+
+	return hdr;
+}
+
+static int bloblist_test_init(struct unit_test_state *uts)
+{
+	struct bloblist_hdr *hdr;
+
+	hdr = clear_bloblist();
+	ut_asserteq(-ENOENT, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	hdr->version++;
+	ut_asserteq(-EPROTONOSUPPORT, bloblist_check(TEST_ADDR,
+						     TEST_BLOBLIST_SIZE));
+
+	ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0x10, 0));
+	ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+
+	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_finish());
+	ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	hdr->flags++;
+	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+
+	return 1;
+}
+BLOBLIST_TEST(bloblist_test_init, 0);
+
+static int bloblist_test_blob(struct unit_test_state *uts)
+{
+	struct bloblist_hdr *hdr;
+	struct bloblist_rec *rec, *rec2;
+	char *data;
+
+	/* At the start there should be no records */
+	hdr = clear_bloblist();
+	ut_assertnull(bloblist_find(TEST_TAG, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+
+	/* Add a record and check that we can find it */
+	data = bloblist_add(TEST_TAG, TEST_SIZE);
+	rec = (void *)(hdr + 1);
+	ut_asserteq_ptr(rec + 1, data);
+	data = bloblist_find(TEST_TAG, TEST_SIZE);
+	ut_asserteq_ptr(rec + 1, data);
+
+	/* Check the 'ensure' method */
+	ut_asserteq_ptr(data, bloblist_ensure(TEST_TAG, TEST_SIZE));
+	ut_assertnull(bloblist_ensure(TEST_TAG, TEST_SIZE2));
+	rec2 = (struct bloblist_rec *)(data + ALIGN(TEST_SIZE, BLOBLIST_ALIGN));
+
+	/* Check for a non-existent record */
+	ut_asserteq_ptr(data, bloblist_ensure(TEST_TAG, TEST_SIZE));
+	ut_asserteq_ptr(rec2 + 1, bloblist_ensure(TEST_TAG2, TEST_SIZE2));
+	ut_assertnull(bloblist_find(TEST_TAG_MISSING, 0));
+
+	return 0;
+}
+BLOBLIST_TEST(bloblist_test_blob, 0);
+
+static int bloblist_test_bad_blob(struct unit_test_state *uts)
+{
+	struct bloblist_hdr *hdr;
+	void *data;
+
+	hdr = clear_bloblist();
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	data = hdr + 1;
+	data += sizeof(struct bloblist_rec);
+	ut_asserteq_ptr(data, bloblist_ensure(TEST_TAG, TEST_SIZE));
+	ut_asserteq_ptr(data, bloblist_ensure(TEST_TAG, TEST_SIZE));
+
+	return 0;
+}
+BLOBLIST_TEST(bloblist_test_bad_blob, 0);
+
+static int bloblist_test_checksum(struct unit_test_state *uts)
+{
+	struct bloblist_hdr *hdr;
+	char *data, *data2;
+
+	hdr = clear_bloblist();
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_finish());
+	ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+
+	/*
+	 * Now change things amd make sure that the checksum notices. We cannot
+	 * change the size or alloced fields, since that will crash the code.
+	 * It has to rely on these being correct.
+	 */
+	hdr->flags--;
+	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	hdr->flags++;
+
+	hdr->size--;
+	ut_asserteq(-EFBIG, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	hdr->size++;
+
+	hdr->spare++;
+	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	hdr->spare--;
+
+	hdr->chksum++;
+	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	hdr->chksum--;
+
+	/* Make sure the checksum changes when we add blobs */
+	data = bloblist_add(TEST_TAG, TEST_SIZE);
+	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+
+	data2 = bloblist_add(TEST_TAG2, TEST_SIZE2);
+	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_finish());
+
+	/* It should also change if we change the data */
+	ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	*data += 1;
+	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	*data -= 1;
+
+	ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	*data2 += 1;
+	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	*data2 -= 1;
+
+	/*
+	 * Changing data outside the range of valid data should not affect
+	 * the checksum.
+	 */
+	ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	data[TEST_SIZE]++;
+	data2[TEST_SIZE2]++;
+	ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+
+	return 0;
+}
+
+BLOBLIST_TEST(bloblist_test_checksum, 0);
+
+int do_ut_bloblist(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	struct unit_test *tests = ll_entry_start(struct unit_test,
+						 bloblist_test);
+	const int n_ents = ll_entry_count(struct unit_test, bloblist_test);
+
+	return cmd_ut_category("bloblist", tests, n_ents, argc, argv);
+}
diff --git a/test/cmd_ut.c b/test/cmd_ut.c
index b7e01a4847e..56924a52726 100644
--- a/test/cmd_ut.c
+++ b/test/cmd_ut.c
@@ -55,6 +55,8 @@ static cmd_tbl_t cmd_ut_sub[] = {
 #ifdef CONFIG_SANDBOX
 	U_BOOT_CMD_MKENT(compression, CONFIG_SYS_MAXARGS, 1, do_ut_compression,
 			 "", ""),
+	U_BOOT_CMD_MKENT(bloblist, CONFIG_SYS_MAXARGS, 1, do_ut_bloblist,
+			 "", ""),
 #endif
 };
 
@@ -97,6 +99,7 @@ static int do_ut(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 static char ut_help_text[] =
 	"all - execute all enabled tests\n"
 #ifdef CONFIG_SANDBOX
+	"ut bloblist - Test bloblist implementation\n"
 	"ut compression - Test compressors and bootm decompression\n"
 #endif
 #ifdef CONFIG_UT_DM
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 07/23] Add bloblist documentation
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (5 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 06/23] test: Add a simple test for bloblist Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 08/23] spl: Support hash, input, pch, pci, rtc, tpm in SPL Simon Glass
                   ` (15 subsequent siblings)
  22 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

Add a description of the purpose of bloblist and how to use it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 doc/README.bloblist | 82 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 doc/README.bloblist

diff --git a/doc/README.bloblist b/doc/README.bloblist
new file mode 100644
index 00000000000..b0e787b97db
--- /dev/null
+++ b/doc/README.bloblist
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+Blob Lists - bloblist
+=====================
+
+Introduction
+------------
+
+A bloblist provides a way to store collections of binary information (blobs) in
+a central structure. Each record of information is assigned a tag so that its
+owner can find it and update it. Each record is generally described by a C
+structure defined by the code that owns it.
+
+
+Passing state through the boot process
+--------------------------------------
+
+The bloblist is created when the first U-Boot component runs (often SPL,
+sometimes TPL). It is passed through to each successive part of the boot and
+can be accessed as needed. This provides a way to transfer state from one part
+to the next. For example, TPL may determine that a watchdog reset occurred by
+reading an SoC register. Reading the register may reset the value, so that it
+cannot be read a second time. So TPL can store that in a bloblist record which
+can be passed through to SPL and U-Boot proper, which can print a message
+indicating that something went wrong and the watchdog fired.
+
+
+Blobs
+-----
+
+While each blob in the bloblist can be of any length, bloblists are designed to
+hold small amounts of data, typically a few KB at most. It is not possible to
+change the length of a blob once it has been written. Each blob is normally
+created from a C structure which can beused to access its fields.
+
+
+Blob tags
+---------
+
+Each blob has a tag which is a 32-bit number. This uniquely identifies the
+owner of the blob. Blob tags are listed in enum blob_tag_t and are named
+with a BLOBT_ prefix.
+
+
+Single structure
+----------------
+
+There is normally only one bloblist in U-Boot. Since a bloblist can store
+multiple blobs it does not seem useful to allow multiple bloblists. Of course
+there could be reasons for this, such as needing to spread the blobs around in
+different memory areas due to fragmented memory, but it is simpler to just have
+a single bloblist.
+
+
+API
+---
+
+Bloblist provides a fairly simple API which allows blobs to be created  and
+found. All access is via the blob's tag.
+
+
+Finishing the bloblist
+----------------------
+
+When a part of U-Boot is about to jump to the next part, it can 'finish' the
+bloblist in preparation for the next stage. This involves adding a checksum so
+that the next stage can make sure that the data arrived safely. While the
+bloblist is in use, changes can be made which will affect the checksum, so it
+is easier to calculate the checksum at the end after all changes are made.
+
+
+Future work
+-----------
+
+Bootstage has a mechanism to 'stash' its records for passing to the next part.
+This should move to using bloblist, to avoid having its own mechanism for
+passing information between U-Boot parts.
+
+
+Simon Glass
+sjg at chromium.org
+12-Aug-2018
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 08/23] spl: Support hash, input, pch, pci, rtc, tpm in SPL
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (6 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 07/23] Add bloblist documentation Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-11-09 18:43   ` [U-Boot] [U-Boot, v2, " Tom Rini
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 09/23] spl: Add a define for SPL_TPL_PROMPT Simon Glass
                   ` (14 subsequent siblings)
  22 siblings, 1 reply; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

At present these subsystems are only supported in U-Boot proper but it is
sometimes necessary to support them in SPL, or even TPL. Update the
Kconfig and Makefile to support this. Also adjust GPIO so that it can be
used in TPL if required.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/spl/Kconfig | 48 +++++++++++++++++++++++++++++++++++++++++++++-
 drivers/Makefile   | 11 ++++++-----
 2 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 280496fbe0b..71d36f087bc 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -295,6 +295,16 @@ config SPL_HASH_SUPPORT
 	  this option to build system-specific drivers for hash acceleration
 	  as part of an SPL build.
 
+config TPL_HASH_SUPPORT
+	bool "Support hashing drivers in TPL"
+	select SHA1
+	select SHA256
+	help
+	  Enable hashing drivers in SPL. These drivers can be used to
+	  accelerate secure boot processing in secure applications. Enable
+	  this option to build system-specific drivers for hash acceleration
+	  as part of an SPL build.
+
 config SPL_DMA_SUPPORT
 	bool "Support DMA drivers"
 	help
@@ -376,7 +386,7 @@ config SPL_FPGA_SUPPORT
 	  within SPL.
 
 config SPL_GPIO_SUPPORT
-	bool "Support GPIO"
+	bool "Support GPIO in SPL"
 	help
 	  Enable support for GPIOs (General-purpose Input/Output) in SPL.
 	  GPIOs allow U-Boot to read the state of an input line (high or
@@ -921,6 +931,17 @@ config TPL_ENV_SUPPORT
 	help
 	  Enable environment support in TPL. See SPL_ENV_SUPPORT for details.
 
+config TPL_GPIO_SUPPORT
+	bool "Support GPIO in TPL"
+	help
+	  Enable support for GPIOs (General-purpose Input/Output) in TPL.
+	  GPIOs allow U-Boot to read the state of an input line (high or
+	  low) and set the state of an output line. This can be used to
+	  drive LEDs, control power to various system parts and read user
+	  input. GPIOs can be useful in TPL to enable a 'sign-of-life' LED,
+	  for example. Enable this option to build the drivers in
+	  drivers/gpio as part of an TPL build.
+
 config TPL_I2C_SUPPORT
 	bool "Support I2C"
 	help
@@ -956,6 +977,22 @@ config TPL_NAND_SUPPORT
 	help
 	  Enable support for NAND in TPL. See SPL_NAND_SUPPORT for details.
 
+config TPL_PCI_SUPPORT
+	bool "Support PCI drivers"
+	help
+	  Enable support for PCI in TPL. For platforms that need PCI to boot,
+	  or must perform some init using PCI in SPL, this provides the
+	  necessary driver support. This enables the drivers in drivers/pci
+	  as part of a TPL build.
+
+config TPL_PCH_SUPPORT
+	bool "Support PCH drivers"
+	help
+	  Enable support for PCH (Platform Controller Hub) devices in TPL.
+	  These are used to set up GPIOs and the SPI peripheral early in
+	  boot. This enables the drivers in drivers/pch as part of a TPL
+	  build.
+
 config TPL_RAM_SUPPORT
 	bool "Support booting from RAM"
 	help
@@ -970,6 +1007,15 @@ config TPL_RAM_DEVICE
 	  be already in memory when TPL takes over, e.g. loaded by the boot
 	  ROM.
 
+config TPL_RTC_SUPPORT
+	bool "Support RTC drivers"
+	help
+	  Enable RTC (Real-time Clock) support in TPL. This includes support
+	  for reading and setting the time. Some RTC devices also have some
+	  non-volatile (battery-backed) memory which is accessible if
+	  needed. This enables the drivers in drivers/rtc as part of an TPL
+	  build.
+
 config TPL_SERIAL_SUPPORT
 	bool "Support serial"
 	select TPL_PRINTF
diff --git a/drivers/Makefile b/drivers/Makefile
index 091b5e81190..63495ee21e9 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -2,20 +2,26 @@
 
 obj-$(CONFIG_$(SPL_TPL_)CLK) += clk/
 obj-$(CONFIG_$(SPL_TPL_)DM) += core/
+obj-$(CONFIG_$(SPL_TPL_)GPIO_SUPPORT) += gpio/
 obj-$(CONFIG_$(SPL_TPL_)DRIVERS_MISC_SUPPORT) += misc/ sysreset/ firmware/
 obj-$(CONFIG_$(SPL_TPL_)I2C_SUPPORT) += i2c/
+obj-$(CONFIG_$(SPL_TPL_)INPUT) += input/
 obj-$(CONFIG_$(SPL_TPL_)LED) += led/
 obj-$(CONFIG_$(SPL_TPL_)MMC_SUPPORT) += mmc/
 obj-$(CONFIG_$(SPL_TPL_)NAND_SUPPORT) += mtd/nand/
+obj-$(CONFIG_$(SPL_TPL_)PCI_SUPPORT) += pci/
+obj-$(CONFIG_$(SPL_TPL_)PCH_SUPPORT) += pch/
 obj-$(CONFIG_$(SPL_TPL_)PHY) += phy/
 obj-$(CONFIG_$(SPL_TPL_)PINCTRL) += pinctrl/
 obj-$(CONFIG_$(SPL_TPL_)RAM) += ram/
+obj-$(CONFIG_$(SPL_TPL_)RTC_SUPPORT) += rtc/
 obj-$(CONFIG_$(SPL_TPL_)SERIAL_SUPPORT) += serial/
 obj-$(CONFIG_$(SPL_TPL_)SPI_FLASH_SUPPORT) += mtd/spi/
 obj-$(CONFIG_$(SPL_TPL_)SPI_SUPPORT) += spi/
 obj-$(CONFIG_$(SPL_TPL_)TIMER) += timer/
 obj-$(CONFIG_$(SPL_)DM_MAILBOX) += mailbox/
 obj-$(CONFIG_$(SPL_)REMOTEPROC) += remoteproc/
+obj-$(CONFIG_$(SPL_TPL_)TPM) += tpm/
 
 ifndef CONFIG_TPL_BUILD
 ifdef CONFIG_SPL_BUILD
@@ -23,7 +29,6 @@ ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_SPL_BOOTCOUNT_LIMIT) += bootcount/
 obj-$(CONFIG_SPL_CPU_SUPPORT) += cpu/
 obj-$(CONFIG_SPL_CRYPTO_SUPPORT) += crypto/
-obj-$(CONFIG_SPL_GPIO_SUPPORT) += gpio/
 obj-$(CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT) += ddr/fsl/
 obj-$(CONFIG_ARMADA_38X) += ddr/marvell/a38x/
 obj-$(CONFIG_ARMADA_XP) += ddr/marvell/axp/
@@ -39,9 +44,6 @@ obj-$(CONFIG_SPL_DMA_SUPPORT) += dma/
 obj-$(CONFIG_SPL_ETH_SUPPORT) += net/
 obj-$(CONFIG_SPL_ETH_SUPPORT) += net/phy/
 obj-$(CONFIG_SPL_USB_ETHER) += net/phy/
-obj-$(CONFIG_SPL_PCI_SUPPORT) += pci/
-obj-$(CONFIG_SPL_PCH_SUPPORT) += pch/
-obj-$(CONFIG_SPL_RTC_SUPPORT) += rtc/
 obj-$(CONFIG_SPL_MUSB_NEW_SUPPORT) += usb/musb-new/
 obj-$(CONFIG_SPL_USB_GADGET_SUPPORT) += usb/gadget/
 obj-$(CONFIG_SPL_USB_GADGET_SUPPORT) += usb/gadget/udc/
@@ -91,7 +93,6 @@ obj-y += scsi/
 obj-y += sound/
 obj-y += spmi/
 obj-y += sysreset/
-obj-y += tpm/
 obj-y += video/
 obj-y += watchdog/
 obj-$(CONFIG_QE) += qe/
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 09/23] spl: Add a define for SPL_TPL_PROMPT
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (7 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 08/23] spl: Support hash, input, pch, pci, rtc, tpm in SPL Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-11-09 18:43   ` [U-Boot] [U-Boot, v2, " Tom Rini
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 10/23] spl: Make SPL_DISABLE_BANNER_PRINT a positive option Simon Glass
                   ` (13 subsequent siblings)
  22 siblings, 1 reply; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

We should use a macro rather than hard-coding the SPL prompt to 'spl'
since the code can be used by TPL too. Add a macro that works for both
and use it in various places.

This allows TPL to use the same code without printing confusing messages.

Note that the string is lower case ('spl', 'tpl') which is a change from
previously.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/spl/spl.c               | 33 ++++++++++++++++++---------------
 include/spl.h                  | 13 +++++++++++++
 test/py/u_boot_console_base.py |  2 +-
 3 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 2ca900ae9ed..d04e2887f34 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -58,8 +58,9 @@ __weak void show_boot_progress(int val) {}
 #ifdef CONFIG_SPL_OS_BOOT
 __weak int spl_start_uboot(void)
 {
-	puts("SPL: Please implement spl_start_uboot() for your board\n");
-	puts("SPL: Direct Linux boot not active!\n");
+	puts(SPL_TPL_PROMPT
+	     "Please implement spl_start_uboot() for your board\n");
+	puts(SPL_TPL_PROMPT "Direct Linux boot not active!\n");
 	return 1;
 }
 
@@ -101,13 +102,13 @@ void spl_fixup_fdt(void)
 	/* fixup the memory dt node */
 	err = fdt_shrink_to_minimum(fdt_blob, 0);
 	if (err == 0) {
-		printf("spl: fdt_shrink_to_minimum err - %d\n", err);
+		printf(SPL_TPL_PROMPT "fdt_shrink_to_minimum err - %d\n", err);
 		return;
 	}
 
 	err = arch_fixup_fdt(fdt_blob);
 	if (err) {
-		printf("spl: arch_fixup_fdt err - %d\n", err);
+		printf(SPL_TPL_PROMPT "arch_fixup_fdt err - %d\n", err);
 		return;
 	}
 #endif
@@ -186,7 +187,7 @@ static int spl_load_fit_image(struct spl_image_info *spl_image,
 	spl_image->os = IH_OS_U_BOOT;
 	spl_image->name = "U-Boot";
 
-	debug("spl: payload image: %.*s load addr: 0x%lx size: %d\n",
+	debug(SPL_TPL_PROMPT "payload image: %.*s load addr: 0x%lx size: %d\n",
 	      (int)sizeof(spl_image->name), spl_image->name,
 		spl_image->load_addr, spl_image->size);
 
@@ -257,9 +258,10 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
 		}
 		spl_image->os = image_get_os(header);
 		spl_image->name = image_get_name(header);
-		debug("spl: payload image: %.*s load addr: 0x%lx size: %d\n",
-			IH_NMLEN, spl_image->name,
-			spl_image->load_addr, spl_image->size);
+		debug(SPL_TPL_PROMPT
+		      "payload image: %.*s load addr: 0x%lx size: %d\n",
+		      IH_NMLEN, spl_image->name, spl_image->load_addr,
+		      spl_image->size);
 #else
 		/* LEGACY image not supported */
 		debug("Legacy boot image support not enabled, proceeding to other boot methods\n");
@@ -287,7 +289,8 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
 			spl_image->load_addr = CONFIG_SYS_LOAD_ADDR;
 			spl_image->entry_point = CONFIG_SYS_LOAD_ADDR;
 			spl_image->size = end - start;
-			debug("spl: payload zImage, load addr: 0x%lx size: %d\n",
+			debug(SPL_TPL_PROMPT
+			      "payload zImage, load addr: 0x%lx size: %d\n",
 			      spl_image->load_addr, spl_image->size);
 			return 0;
 		}
@@ -315,7 +318,7 @@ __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
 	image_entry_noargs_t image_entry =
 		(image_entry_noargs_t)spl_image->entry_point;
 
-	debug("image entry point: 0x%lX\n", spl_image->entry_point);
+	debug("image entry point: #%lx\n", (ulong)spl_image->entry_point);
 	image_entry();
 }
 
@@ -471,7 +474,7 @@ static int boot_from_devices(struct spl_image_info *spl_image,
 		if (loader)
 			printf("Trying to boot from %s\n", loader->name);
 		else
-			puts("SPL: Unsupported Boot Device!\n");
+			puts(SPL_TPL_PROMPT "Unsupported Boot Device!\n");
 #endif
 		if (loader && !spl_load_image(spl_image, loader)) {
 			spl_image->boot_device = spl_boot_list[i];
@@ -494,7 +497,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 	struct spl_image_info spl_image;
 	int ret;
 
-	debug(">>spl:board_init_r()\n");
+	debug(">>" SPL_TPL_PROMPT "board_init_r()\n");
 
 	spl_set_bd();
 
@@ -534,7 +537,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 
 	if (boot_from_devices(&spl_image, spl_boot_list,
 			      ARRAY_SIZE(spl_boot_list))) {
-		puts("SPL: failed to boot from all boot devices\n");
+		puts(SPL_TPL_PROMPT "failed to boot from all boot devices\n");
 		hang();
 	}
 
@@ -600,8 +603,8 @@ void preloader_console_init(void)
 	gd->have_console = 1;
 
 #ifndef CONFIG_SPL_DISABLE_BANNER_PRINT
-	puts("\nU-Boot SPL " PLAIN_VERSION " (" U_BOOT_DATE " - " \
-			U_BOOT_TIME " " U_BOOT_TZ ")\n");
+	puts("\nU-Boot " SPL_TPL_NAME " " PLAIN_VERSION " (" U_BOOT_DATE " - "
+	     U_BOOT_TIME " " U_BOOT_TZ ")\n");
 #endif
 #ifdef CONFIG_SPL_DISPLAY_PRINT
 	spl_display_print();
diff --git a/include/spl.h b/include/spl.h
index 123244f50bb..dbb19c9a310 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -48,6 +48,19 @@ static inline bool u_boot_first_phase(void)
 	return false;
 }
 
+/* A string name for SPL or TPL */
+#ifdef CONFIG_SPL_BUILD
+# ifdef CONFIG_TPL_BUILD
+#  define SPL_TPL_NAME	"tpl"
+# else
+#  define SPL_TPL_NAME	"spl"
+# endif
+# define SPL_TPL_PROMPT	SPL_TPL_NAME ": "
+#else
+# define SPL_TPL_NAME	""
+# define SPL_TPL_PROMPT	""
+#endif
+
 struct spl_image_info {
 	const char *name;
 	u8 os;
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
index 326b2ac51fb..e044eb3ea1d 100644
--- a/test/py/u_boot_console_base.py
+++ b/test/py/u_boot_console_base.py
@@ -16,7 +16,7 @@ import sys
 import u_boot_spawn
 
 # Regexes for text we expect U-Boot to send to the console.
-pattern_u_boot_spl_signon = re.compile('(U-Boot SPL \\d{4}\\.\\d{2}[^\r\n]*\\))')
+pattern_u_boot_spl_signon = re.compile('(U-Boot spl \\d{4}\\.\\d{2}[^\r\n]*\\))')
 pattern_u_boot_main_signon = re.compile('(U-Boot \\d{4}\\.\\d{2}[^\r\n]*\\))')
 pattern_stop_autoboot_prompt = re.compile('Hit any key to stop autoboot: ')
 pattern_unknown_command = re.compile('Unknown command \'.*\' - try \'help\'')
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 10/23] spl: Make SPL_DISABLE_BANNER_PRINT a positive option
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (8 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 09/23] spl: Add a define for SPL_TPL_PROMPT Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-11-09 18:43   ` [U-Boot] [U-Boot, v2, " Tom Rini
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 11/23] spl: Add a comment to spl_set_bd() Simon Glass
                   ` (12 subsequent siblings)
  22 siblings, 1 reply; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

Rather than having a negative option, make this a positive option and
enable it by default. This makes it easier to understand.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/spl/Kconfig | 17 +++++++++++++----
 common/spl/spl.c   |  2 +-
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 71d36f087bc..4baedbfc0e6 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -135,12 +135,21 @@ config SPL_SEPARATE_BSS
 	  location is used. Normally we put the device tree at the end of BSS
 	  but with this option enabled, it goes at _image_binary_end.
 
-config SPL_DISABLE_BANNER_PRINT
-	bool "Disable output of the SPL banner 'U-Boot SPL ...'"
+config SPL_BANNER_PRINT
+	bool "Enable output of the SPL banner 'U-Boot SPL ...'"
+	default y
+	help
+	  If this option is enabled, SPL will print the banner with version
+	  info. Disabling this option could be useful to reduce TPL boot time
+	  (e.g. approx. 6 ms faster, when output on i.MX6 with 115200 baud).
+
+config TPL_BANNER_PRINT
+	bool "Enable output of the TPL banner 'U-Boot TPL ...'"
+	default y
 	help
 	  If this option is enabled, SPL will not print the banner with version
-	  info. Selecting this option could be useful to reduce SPL boot time
-	  (e.g. approx. 6 ms slower, when output on i.MX6 with 115200 baud).
+	  info. Disabling this option could be useful to reduce SPL boot time
+	  (e.g. approx. 6 ms faster, when output on i.MX6 with 115200 baud).
 
 config SPL_DISPLAY_PRINT
 	bool "Display a board-specific message in SPL"
diff --git a/common/spl/spl.c b/common/spl/spl.c
index d04e2887f34..9faed0a2adb 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -602,7 +602,7 @@ void preloader_console_init(void)
 
 	gd->have_console = 1;
 
-#ifndef CONFIG_SPL_DISABLE_BANNER_PRINT
+#if CONFIG_IS_ENABLED(BANNER_PRINT)
 	puts("\nU-Boot " SPL_TPL_NAME " " PLAIN_VERSION " (" U_BOOT_DATE " - "
 	     U_BOOT_TIME " " U_BOOT_TZ ")\n");
 #endif
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 11/23] spl: Add a comment to spl_set_bd()
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (9 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 10/23] spl: Make SPL_DISABLE_BANNER_PRINT a positive option Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-11-09 18:43   ` [U-Boot] [U-Boot,v2,11/23] " Tom Rini
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 12/23] spl: Print a message if we are unable to load an image Simon Glass
                   ` (11 subsequent siblings)
  22 siblings, 1 reply; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

There is a strange feature to set global_data to a data-section variable
early in SPL. This only works if SPL actually has access to SRAM which is
not the case on x86, for eaxmple. Add a comment to this effect.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/spl/spl.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 9faed0a2adb..396c42e1e1b 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -380,6 +380,10 @@ static int spl_common_init(bool setup_malloc)
 
 void spl_set_bd(void)
 {
+	/*
+	 * NOTE: On some platforms (e.g. x86) bdata may be in flash and not
+	 * writeable.
+	 */
 	if (!gd->bd)
 		gd->bd = &bdata;
 }
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 12/23] spl: Print a message if we are unable to load an image
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (10 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 11/23] spl: Add a comment to spl_set_bd() Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-11-09 18:43   ` [U-Boot] [U-Boot, v2, " Tom Rini
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 13/23] sandbox: Add a memory map to the sandbox README Simon Glass
                   ` (10 subsequent siblings)
  22 siblings, 1 reply; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

It can confusing when U-Boot SPL hangs for no obvious reason, when it is
unable to load U-Boot. Add a message to indicate the cause.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/spl/spl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 396c42e1e1b..512141c4139 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -485,6 +485,7 @@ static int boot_from_devices(struct spl_image_info *spl_image,
 			return 0;
 		}
 	}
+	puts(SPL_TPL_PROMPT "No more boot devices\n");
 
 	return -ENODEV;
 }
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 13/23] sandbox: Add a memory map to the sandbox README
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (11 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 12/23] spl: Print a message if we are unable to load an image Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 14/23] test/py: Add a way to pass flags to sandbox Simon Glass
                   ` (9 subsequent siblings)
  22 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

We have a few things in the memory map now, so add documentation for this
to avoid confusion. Also note that it is possible to run all tests now.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 board/sandbox/README.sandbox | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/board/sandbox/README.sandbox b/board/sandbox/README.sandbox
index a28fc9f36c1..6d85901b38d 100644
--- a/board/sandbox/README.sandbox
+++ b/board/sandbox/README.sandbox
@@ -418,7 +418,19 @@ coverage in U-Boot is limited, as we need to work to improve it.
 Note that many of these tests are implemented as commands which you can
 run natively on your board if desired (and enabled).
 
-It would be useful to have a central script to run all of these.
+To run all tests use "make check".
+
+
+Memory Map
+----------
+
+Sandbox has its own emulated memory starting at 0. Here are some of the things
+that are mapped into that memory:
+
+      0   CONFIG_SYS_FDT_LOAD_ADDR   Device tree
+   e000   CONFIG_BLOBLIST_ADDR       Blob list
+  10000   CONFIG_MALLOC_F_ADDR       Early memory allocation
+
 
 --
 Simon Glass <sjg@chromium.org>
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 14/23] test/py: Add a way to pass flags to sandbox
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (12 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 13/23] sandbox: Add a memory map to the sandbox README Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 15/23] sandbox: Add an option to display of-platdata in SPL Simon Glass
                   ` (8 subsequent siblings)
  22 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

It is sometimes useful to restart sandbox with some particular flags to
test certain functionality. Add a new method to ConsoleSandbox to handle
this, without changing the existing APIs.

Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Stephen Warren <swarren@nvidia.com>
---

Changes in v2:
- Add back a blank line, and put one in the new function too

 test/py/u_boot_console_sandbox.py | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/test/py/u_boot_console_sandbox.py b/test/py/u_boot_console_sandbox.py
index 778f6d0983d..836f5a9e2b8 100644
--- a/test/py/u_boot_console_sandbox.py
+++ b/test/py/u_boot_console_sandbox.py
@@ -24,6 +24,7 @@ class ConsoleSandbox(ConsoleBase):
         """
 
         super(ConsoleSandbox, self).__init__(log, config, max_fifo_fill=1024)
+        self.sandbox_flags = []
 
     def get_spawn(self):
         """Connect to a fresh U-Boot instance.
@@ -51,8 +52,25 @@ class ConsoleSandbox(ConsoleBase):
             '-d',
             self.config.dtb
         ]
+        cmd += self.sandbox_flags
         return Spawn(cmd, cwd=self.config.source_dir)
 
+    def restart_uboot_with_flags(self, flags):
+        """Run U-Boot with the given command-line flags
+
+        Args:
+            flags: List of flags to pass, each a string
+
+        Returns:
+            A u_boot_spawn.Spawn object that is attached to U-Boot.
+        """
+
+        try:
+            self.sandbox_flags = flags
+            return self.restart_uboot()
+        finally:
+            self.sandbox_flags = []
+
     def kill(self, sig):
         """Send a specific Unix signal to the sandbox process.
 
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 15/23] sandbox: Add an option to display of-platdata in SPL
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (13 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 14/23] test/py: Add a way to pass flags to sandbox Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 16/23] sandbox: Drop the deprecated 'sb' command Simon Glass
                   ` (7 subsequent siblings)
  22 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

At present we don't have a test that of-platdata can be accessed in SPL.
Add this in as a command-line option to SPL.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Drop the wildcard in the .lds file as it is not needed

 arch/sandbox/cpu/spl.c           | 14 ++++++++++++++
 arch/sandbox/cpu/start.c         |  9 +++++++++
 arch/sandbox/cpu/u-boot-spl.lds  |  2 +-
 arch/sandbox/include/asm/state.h |  1 +
 test/py/tests/test_ofplatdata.py | 31 ++++++++++++++++++++++++++++++-
 5 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c
index 42c149a4981..49f98644c02 100644
--- a/arch/sandbox/cpu/spl.c
+++ b/arch/sandbox/cpu/spl.c
@@ -44,5 +44,19 @@ SPL_LOAD_IMAGE_METHOD("sandbox", 0, BOOT_DEVICE_BOARD, spl_board_load_image);
 
 void spl_board_init(void)
 {
+	struct sandbox_state *state = state_get_current();
+	struct udevice *dev;
+
 	preloader_console_init();
+	if (state->show_of_platdata) {
+		/*
+		 * Scan all the devices so that we can output their platform
+		 * data. See sandbox_spl_probe().
+		 */
+		printf("Scanning misc devices\n");
+		for (uclass_first_device(UCLASS_MISC, &dev);
+		     dev;
+		     uclass_next_device(&dev))
+			;
+	}
 }
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index 59c68a20c52..2879b438d25 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -273,6 +273,15 @@ static int sandbox_cmdline_cb_verbose(struct sandbox_state *state,
 }
 SANDBOX_CMDLINE_OPT_SHORT(verbose, 'v', 0, "Show test output");
 
+static int sandbox_cmdline_cb_show_of_platdata(struct sandbox_state *state,
+					       const char *arg)
+{
+	state->show_of_platdata = true;
+
+	return 0;
+}
+SANDBOX_CMDLINE_OPT(show_of_platdata, 0, "Show of-platdata in SPL");
+
 int board_run_command(const char *cmdline)
 {
 	printf("## Commands are disabled. Please enable CONFIG_CMDLINE.\n");
diff --git a/arch/sandbox/cpu/u-boot-spl.lds b/arch/sandbox/cpu/u-boot-spl.lds
index f97abdfa050..de65b01b33c 100644
--- a/arch/sandbox/cpu/u-boot-spl.lds
+++ b/arch/sandbox/cpu/u-boot-spl.lds
@@ -14,7 +14,7 @@ SECTIONS
 	}
 
 	__u_boot_sandbox_option_start = .;
-	_u_boot_sandbox_getopt : { *(.u_boot_sandbox_getopt) }
+	_u_boot_sandbox_getopt : { KEEP(*(.u_boot_sandbox_getopt)) }
 	__u_boot_sandbox_option_end = .;
 
 	__bss_start = .;
diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h
index a612ce89447..6506ec9ba31 100644
--- a/arch/sandbox/include/asm/state.h
+++ b/arch/sandbox/include/asm/state.h
@@ -89,6 +89,7 @@ struct sandbox_state {
 	enum state_terminal_raw term_raw;	/* Terminal raw/cooked */
 	bool skip_delays;		/* Ignore any time delays (for test) */
 	bool show_test_output;		/* Don't suppress stdout in tests */
+	bool show_of_platdata;		/* Show of-platdata in SPL */
 
 	/* Pointer to information for each SPI bus/cs */
 	struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS]
diff --git a/test/py/tests/test_ofplatdata.py b/test/py/tests/test_ofplatdata.py
index dd8a09f0324..98103ee71a9 100644
--- a/test/py/tests/test_ofplatdata.py
+++ b/test/py/tests/test_ofplatdata.py
@@ -3,11 +3,40 @@
 
 import pytest
 
-OF_PLATDATA_OUTPUT = ''
+OF_PLATDATA_OUTPUT = '''
+of-platdata probe:
+bool 1
+byte 05
+bytearray 06 00 00
+int 1
+intarray 2 3 4 0
+longbytearray 09 0a 0b 0c 0d 0e 0f 10 11
+string message
+stringarray "multi-word" "message" ""
+of-platdata probe:
+bool 0
+byte 08
+bytearray 01 23 34
+int 3
+intarray 5 0 0 0
+longbytearray 09 00 00 00 00 00 00 00 00
+string message2
+stringarray "another" "multi-word" "message"
+of-platdata probe:
+bool 0
+byte 00
+bytearray 00 00 00
+int 0
+intarray 0 0 0 0
+longbytearray 00 00 00 00 00 00 00 00 00
+string <NULL>
+stringarray "one" "" ""
+'''
 
 @pytest.mark.buildconfigspec('spl_of_platdata')
 def test_ofplatdata(u_boot_console):
     """Test that of-platdata can be generated and used in sandbox"""
     cons = u_boot_console
+    cons.restart_uboot_with_flags(['--show_of_platdata'])
     output = cons.get_spawn_output().replace('\r', '')
     assert OF_PLATDATA_OUTPUT in output
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 16/23] sandbox: Drop the deprecated 'sb' command
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (14 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 15/23] sandbox: Add an option to display of-platdata in SPL Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 17/23] sandbox: Add a new " Simon Glass
                   ` (6 subsequent siblings)
  22 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

The old 'sb' command was deprecated in 2015 and replaced with 'host'.
Remove the remaining users and the command, so that the name is available
for other purposes.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 cmd/host.c                  |  5 -----
 doc/README.trace            |  2 +-
 test/dm/sf.c                |  2 +-
 test/py/tests/test_fit.py   | 12 ++++++------
 test/py/tests/test_vboot.py |  2 +-
 5 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/cmd/host.c b/cmd/host.c
index 645dba4de83..f7d3eae5b1a 100644
--- a/cmd/host.c
+++ b/cmd/host.c
@@ -167,11 +167,6 @@ static int do_host(cmd_tbl_t *cmdtp, int flag, int argc,
 		return CMD_RET_USAGE;
 }
 
-U_BOOT_CMD(
-	sb,	8,	1,	do_host,
-	"Deprecated: use 'host' command instead.", ""
-);
-
 U_BOOT_CMD(
 	host, 8, 1, do_host,
 	"Miscellaneous host commands",
diff --git a/doc/README.trace b/doc/README.trace
index 74ba26a5a48..2e7ca3319a9 100644
--- a/doc/README.trace
+++ b/doc/README.trace
@@ -88,7 +88,7 @@ stdin=serial
 stdout=serial
 
 Environment size: 117/8188 bytes
-=>sb save host 0 trace 0 ${profoffset}
+=>host save host 0 trace 0 ${profoffset}
 11405888 bytes written in 10 ms (1.1 GiB/s)
 =>reset
 
diff --git a/test/dm/sf.c b/test/dm/sf.c
index 35241b9f574..af49d7a2129 100644
--- a/test/dm/sf.c
+++ b/test/dm/sf.c
@@ -28,7 +28,7 @@ static int dm_test_spi_flash(struct unit_test_state *uts)
 	 * benefit is worth the extra complexity.
 	 */
 	ut_asserteq(0, run_command_list(
-		"sb save hostfs - 0 spi.bin 200000;"
+		"host save hostfs - 0 spi.bin 200000;"
 		"sf probe;"
 		"sf test 0 10000", -1,  0));
 	/*
diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py
index 34696e97679..49d6fea5716 100755
--- a/test/py/tests/test_fit.py
+++ b/test/py/tests/test_fit.py
@@ -99,15 +99,15 @@ base_fdt = '''
 # then run the 'bootm' command, then save out memory from the places where
 # we expect 'bootm' to write things. Then quit.
 base_script = '''
-sb load hostfs 0 %(fit_addr)x %(fit)s
+host load hostfs 0 %(fit_addr)x %(fit)s
 fdt addr %(fit_addr)x
 bootm start %(fit_addr)x
 bootm loados
-sb save hostfs 0 %(kernel_addr)x %(kernel_out)s %(kernel_size)x
-sb save hostfs 0 %(fdt_addr)x %(fdt_out)s %(fdt_size)x
-sb save hostfs 0 %(ramdisk_addr)x %(ramdisk_out)s %(ramdisk_size)x
-sb save hostfs 0 %(loadables1_addr)x %(loadables1_out)s %(loadables1_size)x
-sb save hostfs 0 %(loadables2_addr)x %(loadables2_out)s %(loadables2_size)x
+host save hostfs 0 %(kernel_addr)x %(kernel_out)s %(kernel_size)x
+host save hostfs 0 %(fdt_addr)x %(fdt_out)s %(fdt_size)x
+host save hostfs 0 %(ramdisk_addr)x %(ramdisk_out)s %(ramdisk_size)x
+host save hostfs 0 %(loadables1_addr)x %(loadables1_out)s %(loadables1_size)x
+host save hostfs 0 %(loadables2_addr)x %(loadables2_out)s %(loadables2_size)x
 '''
 
 @pytest.mark.boardspec('sandbox')
diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index e9cbd57fbab..92144d4c1e3 100644
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -74,7 +74,7 @@ def test_vboot(u_boot_console):
         cons.restart_uboot()
         with cons.log.section('Verified boot %s %s' % (sha_algo, test_type)):
             output = cons.run_command_list(
-                ['sb load hostfs - 100 %stest.fit' % tmpdir,
+                ['host load hostfs - 100 %stest.fit' % tmpdir,
                 'fdt addr 100',
                 'bootm 100'])
         assert(expect_string in ''.join(output))
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 17/23] sandbox: Add a new 'sb' command
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (15 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 16/23] sandbox: Drop the deprecated 'sb' command Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 18/23] sandbox: Allow puts() output before global_data is set up Simon Glass
                   ` (5 subsequent siblings)
  22 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

The old 'sb' command was deprecated in 2015 and replaced with 'host'. It
is useful to be able to access some internal sandbox state, particularly
for testing.

Resurrect the old command and provide a way to print some basic state
information (currently just the arguments to sandbox).

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/sandbox/cpu/start.c         | 10 +++++++
 arch/sandbox/include/asm/state.h |  7 +++++
 cmd/Makefile                     |  1 +
 cmd/sb.c                         | 46 ++++++++++++++++++++++++++++++++
 4 files changed, 64 insertions(+)
 create mode 100644 cmd/sb.c

diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index 2879b438d25..2251ec4c53f 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -295,6 +295,16 @@ static void setup_ram_buf(struct sandbox_state *state)
 	gd->ram_size = state->ram_size;
 }
 
+void state_show(struct sandbox_state *state)
+{
+	char **p;
+
+	printf("Arguments:\n");
+	for (p = state->argv; *p; p++)
+		printf("%s ", *p);
+	printf("\n");
+}
+
 int main(int argc, char *argv[])
 {
 	struct sandbox_state *state;
diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h
index 6506ec9ba31..4e4c71b8062 100644
--- a/arch/sandbox/include/asm/state.h
+++ b/arch/sandbox/include/asm/state.h
@@ -242,6 +242,13 @@ bool state_get_skip_delays(void);
  */
 void state_reset_for_test(struct sandbox_state *state);
 
+/**
+ * state_show() - Show information about the sandbox state
+ *
+ * @param state		Sandbox state to show
+ */
+void state_show(struct sandbox_state *state);
+
 /**
  * Initialize the test system state
  */
diff --git a/cmd/Makefile b/cmd/Makefile
index 2c858f9500a..33604716c4b 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -114,6 +114,7 @@ obj-$(CONFIG_CMD_ROCKUSB) += rockusb.o
 obj-$(CONFIG_SANDBOX) += host.o
 obj-$(CONFIG_CMD_SATA) += sata.o
 obj-$(CONFIG_CMD_NVME) += nvme.o
+obj-$(CONFIG_SANDBOX) += sb.o
 obj-$(CONFIG_CMD_SF) += sf.o
 obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o
 obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o
diff --git a/cmd/sb.c b/cmd/sb.c
new file mode 100644
index 00000000000..6ca3361d7e3
--- /dev/null
+++ b/cmd/sb.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2018, Google Inc.
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <spl.h>
+#include <asm/state.h>
+
+static int do_sb_state(cmd_tbl_t *cmdtp, int flag, int argc,
+		       char * const argv[])
+{
+	struct sandbox_state *state;
+
+	state = state_get_current();
+	state_show(state);
+
+	return 0;
+}
+
+static cmd_tbl_t cmd_sb_sub[] = {
+	U_BOOT_CMD_MKENT(state, 1, 0, do_sb_state, "", ""),
+};
+
+static int do_sb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	cmd_tbl_t *c;
+
+	/* Skip past 'sb' */
+	argc--;
+	argv++;
+
+	c = find_cmd_tbl(argv[0], cmd_sb_sub, ARRAY_SIZE(cmd_sb_sub));
+	if (c)
+		return c->cmd(cmdtp, flag, argc, argv);
+	else
+		return CMD_RET_USAGE;
+}
+
+U_BOOT_CMD(
+	sb,	8,	1,	do_sb,
+	"Sandbox status commands",
+	"state       - Show sandbox state"
+);
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 18/23] sandbox: Allow puts() output before global_data is set up
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (16 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 17/23] sandbox: Add a new " Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 19/23] sandbox: Refactor code to create os_jump_to_file() Simon Glass
                   ` (4 subsequent siblings)
  22 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

We support putc() in this case but not puts(), but this is more useful
since it is what printf() uses.

This particularly affects debugging early in SPL, where currently printf()
statements result in no output. Fix this by adding a special case into
puts() for sandbox, just like putc().

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
---

Changes in v2: None

 common/console.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/common/console.c b/common/console.c
index 9a94f321922..0b0dd76256c 100644
--- a/common/console.c
+++ b/common/console.c
@@ -535,6 +535,13 @@ void putc(const char c)
 
 void puts(const char *s)
 {
+#ifdef CONFIG_SANDBOX
+	/* sandbox can send characters to stdout before it has a console */
+	if (!gd || !(gd->flags & GD_FLG_SERIAL_READY)) {
+		os_puts(s);
+		return;
+	}
+#endif
 #ifdef CONFIG_DEBUG_UART
 	if (!gd || !(gd->flags & GD_FLG_SERIAL_READY)) {
 		while (*s) {
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 19/23] sandbox: Refactor code to create os_jump_to_file()
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (17 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 18/23] sandbox: Allow puts() output before global_data is set up Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 20/23] sandbox: Use malloc() and free() from os layer Simon Glass
                   ` (3 subsequent siblings)
  22 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

At present os_jump_to_image() jumps to a given image, and this is written
to a file. But it is useful to be able to jump to a file also.

To avoid duplicating code, split out the implementation of
os_jump_to_image() into a new function that jumps to a file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/sandbox/cpu/os.c | 48 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index d4d6d78dc74..1723e274535 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -491,7 +491,18 @@ static int make_exec(char *fname, const void *data, int size)
 	return 0;
 }
 
-static int add_args(char ***argvp, const char *add_args[], int count)
+/**
+ * add_args() - Allocate a new argv with the given args
+ *
+ * This is used to create a new argv array with all the old arguments and some
+ * new ones that are passed in
+ *
+ * @argvp:  Returns newly allocated args list
+ * @add_args: Arguments to add, each a string
+ * @count: Number of arguments in @add_args
+ * @return 0 if OK, -ENOMEM if out of memory
+ */
+static int add_args(char ***argvp, char *add_args[], int count)
 {
 	char **argv;
 	int argc;
@@ -512,21 +523,26 @@ static int add_args(char ***argvp, const char *add_args[], int count)
 	return 0;
 }
 
-int os_jump_to_image(const void *dest, int size)
+/**
+ * os_jump_to_file() - Jump to a new program
+ *
+ * This saves the memory buffer, sets up arguments to the new process, then
+ * execs it.
+ *
+ * @fname: Filename to exec
+ * @return does not return on success, any return value is an error
+ */
+static int os_jump_to_file(const char *fname)
 {
 	struct sandbox_state *state = state_get_current();
-	char fname[30], mem_fname[30];
+	char mem_fname[30];
 	int fd, err;
-	const char *extra_args[5];
+	char *extra_args[5];
 	char **argv = state->argv;
 #ifdef DEBUG
-	int argc, i;
+	int i;
 #endif
 
-	err = make_exec(fname, dest, size);
-	if (err)
-		return err;
-
 	strcpy(mem_fname, "/tmp/u-boot.mem.XXXXXX");
 	fd = mkstemp(mem_fname);
 	if (fd < 0)
@@ -539,7 +555,7 @@ int os_jump_to_image(const void *dest, int size)
 	os_fd_restore();
 
 	extra_args[0] = "-j";
-	extra_args[1] = fname;
+	extra_args[1] = (char *)fname;
 	extra_args[2] = "-m";
 	extra_args[3] = mem_fname;
 	extra_args[4] = "--rm_memory";
@@ -564,6 +580,18 @@ int os_jump_to_image(const void *dest, int size)
 	return unlink(fname);
 }
 
+int os_jump_to_image(const void *dest, int size)
+{
+	char fname[30];
+	int err;
+
+	err = make_exec(fname, dest, size);
+	if (err)
+		return err;
+
+	return os_jump_to_file(fname);
+}
+
 int os_find_u_boot(char *fname, int maxlen)
 {
 	struct sandbox_state *state = state_get_current();
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 20/23] sandbox: Use malloc() and free() from os layer
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (18 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 19/23] sandbox: Refactor code to create os_jump_to_file() Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 21/23] sandbox: Filter arguments when starting U-Boot Simon Glass
                   ` (2 subsequent siblings)
  22 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

At present sandbox calls malloc() from various places in the OS layer and
this results in calls to U-Boot's malloc() implementation. It is better to
use the on in the OS layer, since it does not mix allocations with the
main U-Boot code.

Fix this by replacing calls with malloc() to os_malloc(), etc.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
---

Changes in v2: None

 arch/sandbox/cpu/eth-raw-os.c |  9 ++++++---
 arch/sandbox/cpu/os.c         | 18 +++++++++---------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/arch/sandbox/cpu/eth-raw-os.c b/arch/sandbox/cpu/eth-raw-os.c
index 75bfaa4c90a..8d05bc2eda0 100644
--- a/arch/sandbox/cpu/eth-raw-os.c
+++ b/arch/sandbox/cpu/eth-raw-os.c
@@ -11,6 +11,7 @@
 #include <netinet/in.h>
 #include <netinet/ip.h>
 #include <netinet/udp.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -23,6 +24,8 @@
 #include <linux/if_ether.h>
 #include <linux/if_packet.h>
 
+#include <os.h>
+
 struct sandbox_eth_raw_if_nameindex *sandbox_eth_raw_if_nameindex(void)
 {
 	return (struct sandbox_eth_raw_if_nameindex *)if_nameindex();
@@ -71,7 +74,7 @@ static int _raw_packet_start(struct eth_sandbox_raw_priv *priv,
 
 	/* Prepare device struct */
 	priv->local_bind_sd = -1;
-	priv->device = malloc(sizeof(struct sockaddr_ll));
+	priv->device = os_malloc(sizeof(struct sockaddr_ll));
 	if (priv->device == NULL)
 		return -ENOMEM;
 	device = priv->device;
@@ -144,7 +147,7 @@ static int _local_inet_start(struct eth_sandbox_raw_priv *priv)
 	/* Prepare device struct */
 	priv->local_bind_sd = -1;
 	priv->local_bind_udp_port = 0;
-	priv->device = malloc(sizeof(struct sockaddr_in));
+	priv->device = os_malloc(sizeof(struct sockaddr_in));
 	if (priv->device == NULL)
 		return -ENOMEM;
 	device = priv->device;
@@ -279,7 +282,7 @@ int sandbox_eth_raw_os_recv(void *packet, int *length,
 
 void sandbox_eth_raw_os_stop(struct eth_sandbox_raw_priv *priv)
 {
-	free(priv->device);
+	os_free(priv->device);
 	priv->device = NULL;
 	close(priv->sd);
 	priv->sd = -1;
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index 1723e274535..a91fd069dfe 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -309,7 +309,7 @@ void os_dirent_free(struct os_dirent_node *node)
 
 	while (node) {
 		next = node->next;
-		free(node);
+		os_free(node);
 		node = next;
 	}
 }
@@ -334,7 +334,7 @@ int os_dirent_ls(const char *dirname, struct os_dirent_node **headp)
 	/* Create a buffer upfront, with typically sufficient size */
 	dirlen = strlen(dirname) + 2;
 	len = dirlen + 256;
-	fname = malloc(len);
+	fname = os_malloc(len);
 	if (!fname) {
 		ret = -ENOMEM;
 		goto done;
@@ -347,7 +347,7 @@ int os_dirent_ls(const char *dirname, struct os_dirent_node **headp)
 			ret = errno;
 			break;
 		}
-		next = malloc(sizeof(*node) + strlen(entry->d_name) + 1);
+		next = os_malloc(sizeof(*node) + strlen(entry->d_name) + 1);
 		if (!next) {
 			os_dirent_free(head);
 			ret = -ENOMEM;
@@ -356,10 +356,10 @@ int os_dirent_ls(const char *dirname, struct os_dirent_node **headp)
 		if (dirlen + strlen(entry->d_name) > len) {
 			len = dirlen + strlen(entry->d_name);
 			old_fname = fname;
-			fname = realloc(fname, len);
+			fname = os_realloc(fname, len);
 			if (!fname) {
-				free(old_fname);
-				free(next);
+				os_free(old_fname);
+				os_free(next);
 				os_dirent_free(head);
 				ret = -ENOMEM;
 				goto done;
@@ -393,7 +393,7 @@ int os_dirent_ls(const char *dirname, struct os_dirent_node **headp)
 
 done:
 	closedir(dir);
-	free(fname);
+	os_free(fname);
 	return ret;
 }
 
@@ -510,7 +510,7 @@ static int add_args(char ***argvp, char *add_args[], int count)
 	for (argv = *argvp, argc = 0; (*argvp)[argc]; argc++)
 		;
 
-	argv = malloc((argc + count + 1) * sizeof(char *));
+	argv = os_malloc((argc + count + 1) * sizeof(char *));
 	if (!argv) {
 		printf("Out of memory for %d argv\n", count);
 		return -ENOMEM;
@@ -573,7 +573,7 @@ static int os_jump_to_file(const char *fname)
 		os_exit(2);
 
 	err = execv(fname, argv);
-	free(argv);
+	os_free(argv);
 	if (err)
 		return err;
 
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 21/23] sandbox: Filter arguments when starting U-Boot
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (19 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 20/23] sandbox: Use malloc() and free() from os layer Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 22/23] sandbox: Boot in U-Boot through the standard call Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 23/23] spl: Add support for passing handoff info to U-Boot proper Simon Glass
  22 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

The current method of starting U-Boot from U-Boot adds arguments to pass
the memory file through, so that memory is preserved. This is fine for a
single call, but if we call from TPL -> SPL -> U-Boot the arguments build
up and we have several memory files in the argument list.

Adjust the implementation to filter out arguments that we want to replace
with new ones. Also print a useful error if the exec() call fails.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/sandbox/cpu/os.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index a91fd069dfe..8f66e22d2e8 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -504,10 +504,10 @@ static int make_exec(char *fname, const void *data, int size)
  */
 static int add_args(char ***argvp, char *add_args[], int count)
 {
-	char **argv;
+	char **argv, **ap;
 	int argc;
 
-	for (argv = *argvp, argc = 0; (*argvp)[argc]; argc++)
+	for (argc = 0; (*argvp)[argc]; argc++)
 		;
 
 	argv = os_malloc((argc + count + 1) * sizeof(char *));
@@ -515,7 +515,24 @@ static int add_args(char ***argvp, char *add_args[], int count)
 		printf("Out of memory for %d argv\n", count);
 		return -ENOMEM;
 	}
-	memcpy(argv, *argvp, argc * sizeof(char *));
+	for (ap = *argvp, argc = 0; *ap; ap++) {
+		char *arg = *ap;
+
+		/* Drop args that we don't want to propagate */
+		if (*arg == '-' && strlen(arg) == 2) {
+			switch (arg[1]) {
+			case 'j':
+			case 'm':
+				ap++;
+				continue;
+			}
+		} else if (!strcmp(arg, "--rm_memory")) {
+			ap++;
+			continue;
+		}
+		argv[argc++] = arg;
+	}
+
 	memcpy(argv + argc, add_args, count * sizeof(char *));
 	argv[argc + count] = NULL;
 
@@ -539,6 +556,7 @@ static int os_jump_to_file(const char *fname)
 	int fd, err;
 	char *extra_args[5];
 	char **argv = state->argv;
+	int argc;
 #ifdef DEBUG
 	int i;
 #endif
@@ -558,11 +576,13 @@ static int os_jump_to_file(const char *fname)
 	extra_args[1] = (char *)fname;
 	extra_args[2] = "-m";
 	extra_args[3] = mem_fname;
-	extra_args[4] = "--rm_memory";
-	err = add_args(&argv, extra_args,
-		       sizeof(extra_args) / sizeof(extra_args[0]));
+	argc = 4;
+	if (state->ram_buf_rm)
+		extra_args[argc++] = "--rm_memory";
+	err = add_args(&argv, extra_args, argc);
 	if (err)
 		return err;
+	argv[0] = (char *)fname;
 
 #ifdef DEBUG
 	for (i = 0; argv[i]; i++)
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 22/23] sandbox: Boot in U-Boot through the standard call
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (20 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 21/23] sandbox: Filter arguments when starting U-Boot Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 23/23] spl: Add support for passing handoff info to U-Boot proper Simon Glass
  22 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

Most architectures use jump_to_image_no_args() to jump from SPL to U-Boot.
At present sandbox is special in that it jumps in its
spl_board_load_image() call. This is not strictly correct, and means that
sandbox misses out some parts of board_init_r(), just as calling
bloblist_finish(), for example.

Change spl_board_load_image() to just identify the filename to boot, and
implement jump_to_image_no_args() to actually jump to it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/sandbox/cpu/os.c  | 17 +++++------------
 arch/sandbox/cpu/spl.c | 17 +++++++++++++++--
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index 8f66e22d2e8..071e2074ef4 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -594,8 +594,11 @@ static int os_jump_to_file(const char *fname)
 
 	err = execv(fname, argv);
 	os_free(argv);
-	if (err)
+	if (err) {
+		perror("Unable to run image");
+		printf("Image filename '%s'\n", mem_fname);
 		return err;
+	}
 
 	return unlink(fname);
 }
@@ -650,17 +653,7 @@ int os_find_u_boot(char *fname, int maxlen)
 
 int os_spl_to_uboot(const char *fname)
 {
-	struct sandbox_state *state = state_get_current();
-	char *argv[state->argc + 1];
-	int ret;
-
-	memcpy(argv, state->argv, sizeof(char *) * (state->argc + 1));
-	argv[0] = (char *)fname;
-	ret = execv(fname, argv);
-	if (ret)
-		return ret;
-
-	return unlink(fname);
+	return os_jump_to_file(fname);
 }
 
 void os_localtime(struct rtc_time *rt)
diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c
index 49f98644c02..5005ed2f54a 100644
--- a/arch/sandbox/cpu/spl.c
+++ b/arch/sandbox/cpu/spl.c
@@ -37,8 +37,12 @@ static int spl_board_load_image(struct spl_image_info *spl_image,
 		return ret;
 	}
 
-	/* Hopefully this will not return */
-	return os_spl_to_uboot(fname);
+	/* Set up spl_image to boot from jump_to_image_no_args() */
+	spl_image->arg = strdup(fname);
+	if (!spl_image->arg)
+		return log_msg_ret("Setup exec filename", -ENOMEM);
+
+	return 0;
 }
 SPL_LOAD_IMAGE_METHOD("sandbox", 0, BOOT_DEVICE_BOARD, spl_board_load_image);
 
@@ -60,3 +64,12 @@ void spl_board_init(void)
 			;
 	}
 }
+
+void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
+{
+	const char *fname = spl_image->arg;
+
+	os_fd_restore();
+	os_spl_to_uboot(fname);
+	hang();
+}
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 23/23] spl: Add support for passing handoff info to U-Boot proper
  2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
                   ` (21 preceding siblings ...)
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 22/23] sandbox: Boot in U-Boot through the standard call Simon Glass
@ 2018-10-02 11:22 ` Simon Glass
  2018-11-09 18:43   ` [U-Boot] [U-Boot, v2, " Tom Rini
  22 siblings, 1 reply; 48+ messages in thread
From: Simon Glass @ 2018-10-02 11:22 UTC (permalink / raw)
  To: u-boot

There is some basic informaton that SPL normally wants to pass through to
U-Boot, such as the SDRAM size and bank information.

Mkae use of the new bloblist structure for this. Add a new 'handoff' blob
which is set up in SPL and passed to U-Boot proper. Also adda  test for
sandbox_spl that checks that this works correctly and a new 'sb' command
to show the information passed from SPL.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/powerpc/include/asm/spl.h     |  3 --
 arch/sandbox/include/asm/handoff.h | 18 +++++++
 cmd/sb.c                           | 21 +++++++-
 common/board_f.c                   | 15 ++++++
 common/init/Makefile               |  1 +
 common/init/handoff.c              | 47 ++++++++++++++++++
 common/spl/Kconfig                 | 30 ++++++++++++
 common/spl/spl.c                   | 77 +++++++++++++++++++++++++-----
 configs/sandbox_spl_defconfig      |  1 +
 include/asm-generic/global_data.h  |  3 ++
 include/handoff.h                  | 36 ++++++++++++++
 include/spl.h                      |  1 +
 test/py/tests/test_handoff.py      | 14 ++++++
 test/run                           |  2 +-
 14 files changed, 252 insertions(+), 17 deletions(-)
 create mode 100644 arch/sandbox/include/asm/handoff.h
 create mode 100644 common/init/handoff.c
 create mode 100644 include/handoff.h
 create mode 100644 test/py/tests/test_handoff.py

diff --git a/arch/powerpc/include/asm/spl.h b/arch/powerpc/include/asm/spl.h
index cd6d31c71a7..60a7d37d30b 100644
--- a/arch/powerpc/include/asm/spl.h
+++ b/arch/powerpc/include/asm/spl.h
@@ -8,7 +8,4 @@
 
 #define BOOT_DEVICE_NOR		1
 
-/* Linker symbols */
-extern char __bss_start[], __bss_end[];
-
 #endif
diff --git a/arch/sandbox/include/asm/handoff.h b/arch/sandbox/include/asm/handoff.h
new file mode 100644
index 00000000000..a4f4873fb31
--- /dev/null
+++ b/arch/sandbox/include/asm/handoff.h
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Architecture-specific SPL handoff information for sandbox
+ *
+ * Copyright 2018 Google, Inc
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#ifndef __handoff_h
+#define __handoff_h
+
+#define TEST_HANDOFF_MAGIC	0x14f93c7b
+
+struct arch_spl_handoff {
+	ulong	magic;		/* Used for testing */
+};
+
+#endif
diff --git a/cmd/sb.c b/cmd/sb.c
index 6ca3361d7e3..5701e03797c 100644
--- a/cmd/sb.c
+++ b/cmd/sb.c
@@ -9,6 +9,23 @@
 #include <spl.h>
 #include <asm/state.h>
 
+static int do_sb_handoff(cmd_tbl_t *cmdtp, int flag, int argc,
+			 char *const argv[])
+{
+#if CONFIG_IS_ENABLED(HANDOFF)
+	if (gd->spl_handoff)
+		printf("SPL handoff magic %lx\n", gd->spl_handoff->arch.magic);
+	else
+		printf("SPL handoff info not received\n");
+
+	return 0;
+#else
+	printf("Command not supported\n");
+
+	return CMD_RET_USAGE;
+#endif
+}
+
 static int do_sb_state(cmd_tbl_t *cmdtp, int flag, int argc,
 		       char * const argv[])
 {
@@ -21,6 +38,7 @@ static int do_sb_state(cmd_tbl_t *cmdtp, int flag, int argc,
 }
 
 static cmd_tbl_t cmd_sb_sub[] = {
+	U_BOOT_CMD_MKENT(handoff, 1, 0, do_sb_handoff, "", ""),
 	U_BOOT_CMD_MKENT(state, 1, 0, do_sb_state, "", ""),
 };
 
@@ -42,5 +60,6 @@ static int do_sb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 U_BOOT_CMD(
 	sb,	8,	1,	do_sb,
 	"Sandbox status commands",
-	"state       - Show sandbox state"
+	"handoff     - Show handoff data received from SPL\n"
+	"sb state       - Show sandbox state"
 );
diff --git a/common/board_f.c b/common/board_f.c
index d272e0aba62..fd15ddd8c7e 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -25,6 +25,9 @@
 #include <post.h>
 #include <relocate.h>
 #include <spi.h>
+#ifdef CONFIG_SPL
+#include <spl.h>
+#endif
 #include <status_led.h>
 #include <sysreset.h>
 #include <timer.h>
@@ -286,6 +289,17 @@ static int setup_mon_len(void)
 	return 0;
 }
 
+static int setup_spl_handoff(void)
+{
+#if CONFIG_IS_ENABLED(HANDOFF)
+	gd->spl_handoff = bloblist_find(BLOBLISTT_SPL_HANDOFF,
+					sizeof(struct spl_handoff));
+	debug("Found SPL hand-off info %p\n", gd->spl_handoff);
+#endif
+
+	return 0;
+}
+
 __weak int arch_cpu_init(void)
 {
 	return 0;
@@ -845,6 +859,7 @@ static const init_fnc_t init_sequence_f[] = {
 #ifdef CONFIG_BLOBLIST
 	bloblist_init,
 #endif
+	setup_spl_handoff,
 	initf_console_record,
 #if defined(CONFIG_HAVE_FSP)
 	arch_fsp_init,
diff --git a/common/init/Makefile b/common/init/Makefile
index 4902635f535..853b56d1e57 100644
--- a/common/init/Makefile
+++ b/common/init/Makefile
@@ -5,3 +5,4 @@
 #
 
 obj-y += board_init.o
+obj-$(CONFIG_$(SPL_TPL_)HANDOFF) += handoff.o
diff --git a/common/init/handoff.c b/common/init/handoff.c
new file mode 100644
index 00000000000..e00b43e6a7b
--- /dev/null
+++ b/common/init/handoff.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Passing basic information from SPL to U-Boot proper
+ *
+ * Copyright 2018 Google, Inc
+ */
+
+#include <common.h>
+#include <handoff.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+void handoff_save_dram(struct spl_handoff *ho)
+{
+	ho->ram_size = gd->ram_size;
+#ifdef CONFIG_NR_DRAM_BANKS
+	{
+		struct bd_info *bd = gd->bd;
+		int i;
+
+		for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
+			ho->ram_bank[i].start = bd->bi_dram[i].start;
+			ho->ram_bank[i].size = bd->bi_dram[i].size;
+		}
+	}
+#endif
+}
+
+void handoff_load_dram_size(struct spl_handoff *ho)
+{
+	gd->ram_size = ho->ram_size;
+}
+
+void handoff_load_dram_banks(struct spl_handoff *ho)
+{
+#ifdef CONFIG_NR_DRAM_BANKS
+	{
+		struct bd_info *bd = gd->bd;
+		int i;
+
+		for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
+			bd->bi_dram[i].start = ho->ram_bank[i].start;
+			bd->bi_dram[i].size = ho->ram_bank[i].size;
+		}
+	}
+#endif
+}
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 4baedbfc0e6..98c5b4d3a8b 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -25,8 +25,28 @@ config SPL_FRAMEWORK
 	  supports MMC, NAND and YMODEM and other methods loading of U-Boot
 	  and the Linux Kernel.  If unsure, say Y.
 
+config HANDOFF
+	bool "Pass hand-off information from SPL to U-Boot proper"
+	depends on BLOBLIST
+	help
+	  It is useful to be able to pass information from SPL to U-Boot
+	  proper to preserve state that is known in SPL and is needed in U-Boot.
+	  Enable this to locate the handoff information in U-Boot proper, early
+	  in boot. It is available in gd->handoff. The state state is set up
+	  in SPL (or TPL if that is being used).
+
 if SPL
 
+config SPL_HANDOFF
+	bool "Pass hand-off information from SPL to U-Boot proper"
+	depends on HANDOFF
+	default y
+	help
+	  This option enables SPL to write handoff information. This can be
+	  used to pass information like the size of SDRAM from SPL to U-Boot
+	  proper. Also SPL can receive information from TPL in the same place
+	  if that is enabled.
+
 config SPL_LDSCRIPT
 	string "Linker script for the SPL stage"
 	default "arch/$(ARCH)/cpu/u-boot-spl.lds"
@@ -860,6 +880,16 @@ config TPL
 
 if TPL
 
+config TPL_HANDOFF
+	bool "Pass hand-off information from TPL to SPL and U-Boot proper"
+	depends on HANDOFF
+	default y
+	help
+	  This option enables TPL to write handoff information. This can be
+	  used to pass information like the size of SDRAM from TPL to U-Boot
+	  proper. The information is also available to SPL if it is useful
+	  there.
+
 config TPL_BOARD_INIT
 	bool "Call board-specific initialization in TPL"
 	help
diff --git a/common/spl/spl.c b/common/spl/spl.c
index 512141c4139..86ba0fda02a 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -10,6 +10,7 @@
 #include <bloblist.h>
 #include <binman_sym.h>
 #include <dm.h>
+#include <handoff.h>
 #include <spl.h>
 #include <asm/u-boot.h>
 #include <nand.h>
@@ -45,6 +46,14 @@ static bd_t bdata __attribute__ ((section(".data")));
  */
 __weak void show_boot_progress(int val) {}
 
+#if defined(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF)
+/* weak, default platform-specific function to initialize dram banks */
+__weak int dram_init_banksize(void)
+{
+	return 0;
+}
+#endif
+
 /*
  * Default function to determine if u-boot or the OS should
  * be started. This implementation always returns 1.
@@ -64,14 +73,6 @@ __weak int spl_start_uboot(void)
 	return 1;
 }
 
-/* weak default platform specific function to initialize
- * dram banks
- */
-__weak int dram_init_banksize(void)
-{
-	return 0;
-}
-
 /*
  * Weak default function for arch specific zImage check. Return zero
  * and fill start and end address if image is recognized.
@@ -322,6 +323,44 @@ __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
 	image_entry();
 }
 
+#if CONFIG_IS_ENABLED(HANDOFF)
+/**
+ * Set up the SPL hand-off information
+ *
+ * This is initially empty (zero) but can be written by
+ */
+static int setup_spl_handoff(void)
+{
+	struct spl_handoff *ho;
+
+	ho = bloblist_ensure(BLOBLISTT_SPL_HANDOFF, sizeof(struct spl_handoff));
+	if (!ho)
+		return -ENOENT;
+
+	return 0;
+}
+
+static int write_spl_handoff(void)
+{
+	struct spl_handoff *ho;
+
+	ho = bloblist_find(BLOBLISTT_SPL_HANDOFF, sizeof(struct spl_handoff));
+	if (!ho)
+		return -ENOENT;
+	handoff_save_dram(ho);
+#ifdef CONFIG_SANDBOX
+	ho->arch.magic = TEST_HANDOFF_MAGIC;
+#endif
+	debug(SPL_TPL_PROMPT "Wrote SPL handoff\n");
+
+	return 0;
+}
+#else
+static inline int setup_spl_handoff(void) { return 0; }
+static inline int write_spl_handoff(void) { return 0; }
+
+#endif /* HANDOFF */
+
 static int spl_common_init(bool setup_malloc)
 {
 	int ret;
@@ -357,6 +396,15 @@ static int spl_common_init(bool setup_malloc)
 			return ret;
 		}
 	}
+	if (CONFIG_IS_ENABLED(HANDOFF)) {
+		int ret;
+
+		ret = setup_spl_handoff();
+		if (ret) {
+			puts(SPL_TPL_PROMPT "Cannot set up SPL handoff\n");
+			hang();
+		}
+	}
 	if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) {
 		ret = fdtdec_setup();
 		if (ret) {
@@ -506,10 +554,6 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 
 	spl_set_bd();
 
-#ifdef CONFIG_SPL_OS_BOOT
-	dram_init_banksize();
-#endif
-
 #if defined(CONFIG_SYS_SPL_MALLOC_START)
 	mem_malloc_init(CONFIG_SYS_SPL_MALLOC_START,
 			CONFIG_SYS_SPL_MALLOC_SIZE);
@@ -531,6 +575,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 	spl_board_init();
 #endif
 
+	if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF))
+		dram_init_banksize();
+
 	bootcount_inc();
 
 	memset(&spl_image, '\0', sizeof(spl_image));
@@ -547,6 +594,12 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 	}
 
 	spl_perform_fixups(&spl_image);
+	if (CONFIG_IS_ENABLED(HANDOFF)) {
+		ret = write_spl_handoff();
+		if (ret)
+			printf(SPL_TPL_PROMPT
+			       "SPL hand-off write failed (err=%d)\n", ret);
+	}
 	if (CONFIG_IS_ENABLED(BLOBLIST)) {
 		ret = bloblist_finish();
 		if (ret)
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index c019eeb65bd..1a07cf2a4bc 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -23,6 +23,7 @@ CONFIG_CONSOLE_RECORD=y
 CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
 CONFIG_SILENT_CONSOLE=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_HANDOFF=y
 CONFIG_SPL_BOARD_INIT=y
 CONFIG_SPL_ENV_SUPPORT=y
 CONFIG_CMD_CPU=y
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index ccf361ed88a..dffd6b26026 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -125,6 +125,9 @@ typedef struct global_data {
 #if CONFIG_IS_ENABLED(BLOBLIST)
 	struct bloblist_hdr *bloblist;	/* Bloblist information */
 	struct bloblist_hdr *new_bloblist;	/* Relocated blolist info */
+# ifdef CONFIG_SPL
+	struct spl_handoff *spl_handoff;
+# endif
 #endif
 } gd_t;
 #endif
diff --git a/include/handoff.h b/include/handoff.h
new file mode 100644
index 00000000000..f932a98a2b0
--- /dev/null
+++ b/include/handoff.h
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Passing basic information from SPL to U-Boot proper
+ *
+ * Copyright 2018 Google, Inc
+ */
+
+#ifndef __HANDOFF_H
+#define __HANDOFF_H
+
+#if CONFIG_IS_ENABLED(HANDOFF)
+
+#include <asm/handoff.h>
+
+/**
+ * struct spl_handoff - information passed from SPL to U-Boot proper
+ *
+ * @ram_size: Value to use for gd->ram_size
+ */
+struct spl_handoff {
+	struct arch_spl_handoff arch;
+	u64 ram_size;
+#ifdef CONFIG_NR_DRAM_BANKS
+	struct {
+		u64 start;
+		u64 size;
+	} ram_bank[CONFIG_NR_DRAM_BANKS];
+#endif
+};
+
+void handoff_save_dram(struct spl_handoff *ho);
+void handoff_load_dram_size(struct spl_handoff *ho);
+void handoff_load_dram_banks(struct spl_handoff *ho);
+#endif
+
+#endif
diff --git a/include/spl.h b/include/spl.h
index dbb19c9a310..383bc4596ca 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -11,6 +11,7 @@
 /* Platform-specific defines */
 #include <linux/compiler.h>
 #include <asm/spl.h>
+#include <handoff.h>
 
 /* Value in r0 indicates we booted from U-Boot */
 #define UBOOT_NOT_LOADED_FROM_SPL	0x13578642
diff --git a/test/py/tests/test_handoff.py b/test/py/tests/test_handoff.py
new file mode 100644
index 00000000000..4540441fae7
--- /dev/null
+++ b/test/py/tests/test_handoff.py
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2016 Google, Inc
+
+import pytest
+
+# Magic number to check that SPL handoff is working
+TEST_HANDOFF_MAGIC = 0x14f93c7b
+
+ at pytest.mark.buildconfigspec('spl')
+def test_handoff(u_boot_console):
+    """Test that of-platdata can be generated and used in sandbox"""
+    cons = u_boot_console
+    response = cons.run_command('sb handoff')
+    assert ('SPL handoff magic %x' % TEST_HANDOFF_MAGIC) in response
diff --git a/test/run b/test/run
index fb8ff5da0cb..cd323b0a85e 100755
--- a/test/run
+++ b/test/run
@@ -19,7 +19,7 @@ run_test "sandbox" ./test/py/test.py --bd sandbox --build
 
 # Run tests which require sandbox_spl
 run_test "sandbox_spl" ./test/py/test.py --bd sandbox_spl --build \
-	-k test_ofplatdata.py
+	-k 'test_ofplatdata or test_handoff'
 
 # Run tests for the flat-device-tree version of sandbox. This is a special
 # build which does not enable CONFIG_OF_LIVE for the live device tree, so we can
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 01/23] log: Correct definition of log_msg_ret()
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 01/23] log: Correct definition of log_msg_ret() Simon Glass
@ 2018-10-04  9:25   ` Bin Meng
  2018-11-16  0:07     ` Simon Glass
  0 siblings, 1 reply; 48+ messages in thread
From: Bin Meng @ 2018-10-04  9:25 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Tue, Oct 2, 2018 at 8:25 PM Simon Glass <sjg@chromium.org> wrote:
>
> This macro should have two parameters, not one. Fix it so that it
> correctly resolves to _ret when logging is disabled.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2: None
>
>  include/log.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/log.h b/include/log.h
> index 653fb8d853e..75ff1e1160c 100644
> --- a/include/log.h
> +++ b/include/log.h
> @@ -175,7 +175,7 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line,
>         })
>  #else
>  #define log_ret(_ret) (_ret)
> -#define log_msg_ret(_ret) (_ret)
> +#define log_msg_ret(_msg, _ret) (_ret)

This creates a warning still if logging is disabled.

include/log.h:178:33: warning: statement with no effect [-Wunused-value]
 #define log_msg_ret(_msg, _ret) (_ret)

Regards,
Bin

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 03/23] Add core support for a bloblist to convey data from SPL
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 03/23] Add core support for a bloblist to convey data from SPL Simon Glass
@ 2018-10-08  2:42   ` Kever Yang
  2018-10-09  3:40     ` Simon Glass
  2018-11-09 18:42   ` [U-Boot] [U-Boot, v2, " Tom Rini
  1 sibling, 1 reply; 48+ messages in thread
From: Kever Yang @ 2018-10-08  2:42 UTC (permalink / raw)
  To: u-boot

Hi Simon,

    I didn't notice you already have a V2 for this patch set just now
and have send a reply to V1.

    My question is:
    Is it possible to use ATAGs instead of a new 'bloblist'?

Thanks,
- Kever
On 10/02/2018 07:22 PM, Simon Glass wrote:
> At present there is no standard way in U-Boot to pass information from SPL
> to U-Boot proper. But sometimes SPL wants to convey information to U-Boot
> that U-Boot cannot easily figure out. For example, if SPL sets up SDRAM
> then it might want to pass the size of SDRAM, or the location of each
> bank, to U-Boot proper.
>
> Add a new 'bloblist' feature which provides this. A bloblist is set up in
> the first phase of U-Boot that runs (i.e. TPL or SPL). The location of
> this info may be in SRAM or CAR (x86 cache-as-RAM) or somewhere else.
>
> Information placed in this region is preserved (with a checksum) through
> TPL and SPL and ends up in U-Boot. At this point it is copied into SDRAM
> so it can be used after relocation.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Acked-by: Andreas Dannenberg <dannenberg@ti.com>
> ---
>
> Changes in v2:
> - Fix several typos
>
>  common/Kconfig     |  48 +++++++++
>  common/Makefile    |   1 +
>  common/bloblist.c  | 239 +++++++++++++++++++++++++++++++++++++++++++++
>  include/bloblist.h | 195 ++++++++++++++++++++++++++++++++++++
>  include/log.h      |   1 +
>  5 files changed, 484 insertions(+)
>  create mode 100644 common/bloblist.c
>  create mode 100644 include/bloblist.h
>
> diff --git a/common/Kconfig b/common/Kconfig
> index 3bb9571b710..2e72b3c83c6 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -715,4 +715,52 @@ config UPDATE_TFTP_MSEC_MAX
>  
>  endmenu
>  
> +menu "Blob list"
> +
> +config BLOBLIST
> +	bool "Support for a bloblist"
> +	help
> +	  This enables support for a bloblist in U-Boot, which can be passed
> +	  from TPL to SPL to U-Boot proper (and potentially to Linux). The
> +	  blob list supports multiple binary blobs of data, each with a tag,
> +	  so that different U-Boot components can store data which can survive
> +	  through to the next stage of the boot.
> +
> +config SPL_BLOBLIST
> +	bool "Support for a bloblist in SPL"
> +	depends on BLOBLIST
> +	default y if SPL
> +	help
> +	  This enables a bloblist in SPL. If this is the first part of U-Boot
> +	  to run, then the bloblist is set up in SPL and passed to U-Boot
> +	  proper. If TPL also has a bloblist, then SPL uses the one from there.
> +
> +config TPL_BLOBLIST
> +	bool "Support for a bloblist in TPL"
> +	depends on BLOBLIST
> +	default y if TPL
> +	help
> +	  This enables a bloblist in TPL. The bloblist is set up in TPL and
> +	  passed to SPL and U-Boot proper.
> +
> +config BLOBLIST_SIZE
> +	hex "Size of bloblist"
> +	depends on BLOBLIST
> +	default 0x400
> +	help
> +	  Sets the size of the bloblist in bytes. This must include all
> +	  overhead (alignment, bloblist header, record header). The bloblist
> +	  is set up in the first part of U-Boot to run (TPL, SPL or U-Boot
> +	  proper), and this sane bloblist is used for subsequent stages.
> +
> +config BLOBLIST_ADDR
> +	hex "Address of bloblist"
> +	depends on BLOBLIST
> +	default 0xe000
> +	help
> +	  Sets the address of the bloblist, set up by the first part of U-Boot
> +	  which runs. Subsequent U-Boot stages typically use the same address.
> +
> +endmenu
> +
>  source "common/spl/Kconfig"
> diff --git a/common/Makefile b/common/Makefile
> index cbca4ff2da6..6aff2b1a6e3 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -61,6 +61,7 @@ obj-$(CONFIG_CMDLINE) += cli_readline.o cli_simple.o
>  endif # !CONFIG_SPL_BUILD
>  
>  obj-$(CONFIG_$(SPL_TPL_)BOOTSTAGE) += bootstage.o
> +obj-$(CONFIG_$(SPL_TPL_)BLOBLIST) += bloblist.o
>  
>  ifdef CONFIG_SPL_BUILD
>  ifdef CONFIG_SPL_DFU_SUPPORT
> diff --git a/common/bloblist.c b/common/bloblist.c
> new file mode 100644
> index 00000000000..b4cf169b05a
> --- /dev/null
> +++ b/common/bloblist.c
> @@ -0,0 +1,239 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2018 Google, Inc
> + * Written by Simon Glass <sjg@chromium.org>
> + */
> +
> +#include <common.h>
> +#include <bloblist.h>
> +#include <log.h>
> +#include <mapmem.h>
> +#include <spl.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +struct bloblist_rec *bloblist_first_blob(struct bloblist_hdr *hdr)
> +{
> +	if (hdr->alloced <= hdr->hdr_size)
> +		return NULL;
> +	return (struct bloblist_rec *)((void *)hdr + hdr->hdr_size);
> +}
> +
> +struct bloblist_rec *bloblist_next_blob(struct bloblist_hdr *hdr,
> +					struct bloblist_rec *rec)
> +{
> +	ulong offset;
> +
> +	offset = (void *)rec - (void *)hdr;
> +	offset += rec->hdr_size + ALIGN(rec->size, BLOBLIST_ALIGN);
> +	if (offset >= hdr->alloced)
> +		return NULL;
> +	return (struct bloblist_rec *)((void *)hdr + offset);
> +}
> +
> +#define foreach_rec(_rec, _hdr) \
> +	for (_rec = bloblist_first_blob(_hdr); \
> +	     _rec; \
> +	     _rec = bloblist_next_blob(_hdr, _rec))
> +
> +static struct bloblist_rec *bloblist_findrec(uint tag)
> +{
> +	struct bloblist_hdr *hdr = gd->bloblist;
> +	struct bloblist_rec *rec;
> +
> +	if (!hdr)
> +		return NULL;
> +
> +	foreach_rec(rec, hdr) {
> +		if (rec->tag == tag)
> +			return rec;
> +	}
> +
> +	return NULL;
> +}
> +
> +static int bloblist_addrec(uint tag, int size, struct bloblist_rec **recp)
> +{
> +	struct bloblist_hdr *hdr = gd->bloblist;
> +	struct bloblist_rec *rec;
> +	int new_alloced;
> +
> +	new_alloced = hdr->alloced + sizeof(*rec) +
> +			ALIGN(size, BLOBLIST_ALIGN);
> +	if (new_alloced >= hdr->size) {
> +		log(LOGC_BLOBLIST, LOGL_ERR,
> +		    "Failed to allocate %x bytes size=%x, need size>=%x\n",
> +		    size, hdr->size, new_alloced);
> +		return log_msg_ret("bloblist add", -ENOSPC);
> +	}
> +	rec = (void *)hdr + hdr->alloced;
> +	hdr->alloced = new_alloced;
> +
> +	rec->tag = tag;
> +	rec->hdr_size = sizeof(*rec);
> +	rec->size = size;
> +	rec->spare = 0;
> +	*recp = rec;
> +
> +	return 0;
> +}
> +
> +static int bloblist_ensurerec(uint tag, struct bloblist_rec **recp, int size)
> +{
> +	struct bloblist_rec *rec;
> +
> +	rec = bloblist_findrec(tag);
> +	if (rec) {
> +		if (size && size != rec->size)
> +			return -ESPIPE;
> +	} else {
> +		int ret;
> +
> +		ret = bloblist_addrec(tag, size, &rec);
> +		if (ret)
> +			return ret;
> +	}
> +	*recp = rec;
> +
> +	return 0;
> +}
> +
> +void *bloblist_find(uint tag, int size)
> +{
> +	struct bloblist_rec *rec;
> +
> +	rec = bloblist_findrec(tag);
> +	if (!rec)
> +		return NULL;
> +	if (size && size != rec->size)
> +		return NULL;
> +
> +	return (void *)rec + rec->hdr_size;
> +}
> +
> +void *bloblist_add(uint tag, int size)
> +{
> +	struct bloblist_rec *rec;
> +
> +	if (bloblist_addrec(tag, size, &rec))
> +		return NULL;
> +
> +	return rec + 1;
> +}
> +
> +int bloblist_ensure_size(uint tag, int size, void **blobp)
> +{
> +	struct bloblist_rec *rec;
> +	int ret;
> +
> +	ret = bloblist_ensurerec(tag, &rec, size);
> +	if (ret)
> +		return ret;
> +	*blobp = (void *)rec + rec->hdr_size;
> +
> +	return 0;
> +}
> +
> +void *bloblist_ensure(uint tag, int size)
> +{
> +	struct bloblist_rec *rec;
> +
> +	if (bloblist_ensurerec(tag, &rec, size))
> +		return NULL;
> +
> +	return (void *)rec + rec->hdr_size;
> +}
> +
> +static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr)
> +{
> +	struct bloblist_rec *rec;
> +	u32 chksum;
> +
> +	chksum = crc32(0, (unsigned char *)hdr,
> +		       offsetof(struct bloblist_hdr, chksum));
> +	foreach_rec(rec, hdr) {
> +		chksum = crc32(chksum, (void *)rec, rec->hdr_size);
> +		chksum = crc32(chksum, (void *)rec + rec->hdr_size, rec->size);
> +	}
> +
> +	return chksum;
> +}
> +
> +int bloblist_new(ulong addr, uint size, uint flags)
> +{
> +	struct bloblist_hdr *hdr;
> +
> +	if (size < sizeof(*hdr))
> +		return log_ret(-ENOSPC);
> +	if (addr & (BLOBLIST_ALIGN - 1))
> +		return log_ret(-EFAULT);
> +	hdr = map_sysmem(addr, size);
> +	memset(hdr, '\0', sizeof(*hdr));
> +	hdr->version = BLOBLIST_VERSION;
> +	hdr->hdr_size = sizeof(*hdr);
> +	hdr->flags = flags;
> +	hdr->magic = BLOBLIST_MAGIC;
> +	hdr->size = size;
> +	hdr->alloced = hdr->hdr_size;
> +	hdr->chksum = 0;
> +	gd->bloblist = hdr;
> +
> +	return 0;
> +}
> +
> +int bloblist_check(ulong addr, uint size)
> +{
> +	struct bloblist_hdr *hdr;
> +	u32 chksum;
> +
> +	hdr = map_sysmem(addr, sizeof(*hdr));
> +	if (hdr->magic != BLOBLIST_MAGIC)
> +		return log_msg_ret("Bad magic", -ENOENT);
> +	if (hdr->version != BLOBLIST_VERSION)
> +		return log_msg_ret("Bad version", -EPROTONOSUPPORT);
> +	if (size && hdr->size != size)
> +		return log_msg_ret("Bad size", -EFBIG);
> +	chksum = bloblist_calc_chksum(hdr);
> +	if (hdr->chksum != chksum) {
> +		log(LOGC_BLOBLIST, LOGL_ERR, "Checksum %x != %x\n", hdr->chksum,
> +		    chksum);
> +		return log_msg_ret("Bad checksum", -EIO);
> +	}
> +	gd->bloblist = hdr;
> +
> +	return 0;
> +}
> +
> +int bloblist_finish(void)
> +{
> +	struct bloblist_hdr *hdr = gd->bloblist;
> +
> +	hdr->chksum = bloblist_calc_chksum(hdr);
> +
> +	return 0;
> +}
> +
> +int bloblist_init(void)
> +{
> +	bool expected;
> +	int ret = -ENOENT;
> +
> +	/**
> +	 * Wed expect to find an existing bloblist in the first phase of U-Boot
> +	 * that runs
> +	 */
> +	expected = !u_boot_first_phase();
> +	if (expected)
> +		ret = bloblist_check(CONFIG_BLOBLIST_ADDR,
> +				     CONFIG_BLOBLIST_SIZE);
> +	if (ret) {
> +		log(LOGC_BLOBLIST, expected ? LOGL_WARNING : LOGL_DEBUG,
> +		    "Existing bloblist not found: creating new bloblist\n");
> +		ret = bloblist_new(CONFIG_BLOBLIST_ADDR, CONFIG_BLOBLIST_SIZE,
> +				   0);
> +	} else {
> +		log(LOGC_BLOBLIST, LOGL_DEBUG, "Found existing bloblist\n");
> +	}
> +
> +	return ret;
> +}
> diff --git a/include/bloblist.h b/include/bloblist.h
> new file mode 100644
> index 00000000000..413736a9080
> --- /dev/null
> +++ b/include/bloblist.h
> @@ -0,0 +1,195 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * This provides a standard way of passing information between boot phases
> + * (TPL -> SPL -> U-Boot proper.)
> + *
> + * A list of blobs of data, tagged with their owner. The list resides in memory
> + * and can be updated by SPL, U-Boot, etc.
> + *
> + * Copyright 2018 Google, Inc
> + * Written by Simon Glass <sjg@chromium.org>
> + */
> +
> +#ifndef __BLOBLIST_H
> +#define __BLOBLIST_H
> +
> +enum {
> +	BLOBLIST_VERSION	= 0,
> +	BLOBLIST_MAGIC		= 0xb00757a3,
> +	BLOBLIST_ALIGN		= 16,
> +};
> +
> +enum bloblist_tag_t {
> +	BLOBLISTT_NONE = 0,
> +
> +	/* Vendor-specific tags are permitted here */
> +	BLOBLISTT_EC_HOSTEVENT,		/* Chromium OS EC host-event mask */
> +	BLOBLISTT_SPL_HANDOFF,		/* Hand-off info from SPL */
> +	BLOBLISTT_VBOOT_CTX,		/* Chromium OS verified boot context */
> +	BLOBLISTT_VBOOT_HANDOFF,	/* Chromium OS internal handoff info */
> +};
> +
> +/**
> + * struct bloblist_hdr - header for the bloblist
> + *
> + * This is stored at the start of the bloblist which is always on a 16-byte
> + * boundary. Records follow this header. The bloblist normally stays in the
> + * same place in memory as SPL and U-Boot execute, but it can be safely moved
> + * around.
> + *
> + * None of the bloblist structures contain pointers but it is possible to put
> + * pointers inside a bloblist record if desired. This is not encouraged,
> + * since it can make part of the bloblist inaccessible if the pointer is
> + * no-longer valid. It is better to just store all the data inside a bloblist
> + * record.
> + *
> + * Each bloblist record is aligned to a 16-byte boundary and follows immediately
> + * from the last.
> + *
> + * @version: BLOBLIST_VERSION
> + * @hdr_size: Size of this header, normally sizeof(struct bloblist_hdr). The
> + *	first bloblist_rec starts at this offset from the start of the header
> + * @flags: Space for BLOBLISTF_... flags (none yet)
> + * @magic: BLOBLIST_MAGIC
> + * @size: Total size of all records (non-zero if valid) including this header.
> + *	The bloblist extends for this many bytes from the start of this header.
> + * @alloced: Total size allocated for this bloblist. When adding new records,
> + *	the bloblist can grow up to this size. This starts out as
> + *	sizeof(bloblist_hdr) since we need at least that much space to store a
> + *	valid bloblist
> + * @spare: Space space
> + * @chksum: CRC32 for the entire bloblist allocated area. Since any of the
> + *	blobs can be altered after being created, this checksum is only valid
> + *	when the bloblist is finalised before jumping to the next stage of boot.
> + *	Note: @chksum is last to make it easier to exclude it from the checksum
> + *	calculation.
> + */
> +struct bloblist_hdr {
> +	u32 version;
> +	u32 hdr_size;
> +	u32 flags;
> +	u32 magic;
> +
> +	u32 size;
> +	u32 alloced;
> +	u32 spare;
> +	u32 chksum;
> +};
> +
> +/**
> + * struct bloblist_rec - record for the bloblist
> + *
> + * NOTE: Only exported for testing purposes. Do not use this struct.
> + *
> + * The bloblist contains a number of records each consisting of this record
> + * structure followed by the data contained. Each records is 16-byte aligned.
> + *
> + * @tag: Tag indicating what the record contains
> + * @hdr_size: Size of this header, normally sizeof(struct bloblist_rec). The
> + *	record's data starts at this offset from the start of the record
> + * @size: Size of record in bytes, excluding the header size. This does not
> + *	need to be aligned (e.g. 3 is OK).
> + * @spare: Spare space for other things
> + */
> +struct bloblist_rec {
> +	u32 tag;
> +	u32 hdr_size;
> +	u32 size;
> +	u32 spare;
> +};
> +
> +/**
> + * bloblist_find() - Find a blob
> + *
> + * Searches the bloblist and returns the blob with the matching tag
> + *
> + * @tag:	Tag to search for (enum bloblist_tag_t)
> + * @size:	Expected size of the blob
> + * @return pointer to blob if found, or NULL if not found, or a blob was found
> + *	but it is the wrong size
> + */
> +void *bloblist_find(uint tag, int size);
> +
> +/**
> + * bloblist_add() - Add a new blob
> + *
> + * Add a new blob to the bloblist
> + *
> + * This should only be called if you konw there is no existing blob for a
> + * particular tag. It is typically safe to call in the first phase of U-Boot
> + * (e.g. TPL or SPL). After that, bloblist_ensure() should be used instead.
> + *
> + * @tag:	Tag to add (enum bloblist_tag_t)
> + * @size:	Size of the blob
> + * @return pointer to the newly added block, or NULL if there is not enough
> + *	space for the blob
> + */
> +void *bloblist_add(uint tag, int size);
> +
> +/**
> + * bloblist_ensure_size() - Find or add a blob
> + *
> + * Find an existing blob, or add a new one if not found
> + *
> + * @tag:	Tag to add (enum bloblist_tag_t)
> + * @size:	Size of the blob
> + * @blobp:	Returns a pointer to blob on success
> + * @return 0 if OK, -ENOSPC if it is missing and could not be added due to lack
> + *	of space, or -ESPIPE it exists but has the wrong size
> + */
> +int bloblist_ensure_size(uint tag, int size, void **blobp);
> +
> +/**
> + * bloblist_ensure() - Find or add a blob
> + *
> + * Find an existing blob, or add a new one if not found
> + *
> + * @tag:	Tag to add (enum bloblist_tag_t)
> + * @size:	Size of the blob
> + * @return pointer to blob, or NULL if it is missing and could not be added due
> + *	to lack of space, or it exists but has the wrong size
> + */
> +void *bloblist_ensure(uint tag, int size);
> +
> +/**
> + * bloblist_new() - Create a new, empty bloblist of a given size
> + *
> + * @addr: Address of bloblist
> + * @size: Initial size for bloblist
> + * @flags: Flags to use for bloblist
> + * @return 0 if OK, -EFAULT if addr is not aligned correctly, -ENOSPC is the
> + *	area is not large enough
> + */
> +int bloblist_new(ulong addr, uint size, uint flags);
> +
> +/**
> + * bloblist_check() - Check if a bloblist exists
> + *
> + * @addr: Address of bloblist
> + * @size: Expected size of blobsize, or 0 to detect the size
> + * @return 0 if OK, -ENOENT if the magic number doesn't match (indicating that
> + *	there problem is no bloblist at the given address), -EPROTONOSUPPORT
> + *	if the version does not match, -EIO if the checksum does not match,
> + *	-EFBIG if the expected size does not match the detected size
> + */
> +int bloblist_check(ulong addr, uint size);
> +
> +/**
> + * bloblist_finish() - Set up the bloblist for the next U-Boot part
> + *
> + * This sets the correct checksum for the bloblist. This ensures that the
> + * bloblist will be detected correctly by the next phase of U-Boot.
> + *
> + * @return 0
> + */
> +int bloblist_finish(void);
> +
> +/**
> + * bloblist_init() - Init the bloblist system with a single bloblist
> + *
> + * This uses CONFIG_BLOBLIST_ADDR and CONFIG_BLOBLIST_SIZE to set up a bloblist
> + * for use by U-Boot.
> + */
> +int bloblist_init(void);
> +
> +#endif /* __BLOBLIST_H */
> diff --git a/include/log.h b/include/log.h
> index 1146f423a7a..61411b72eac 100644
> --- a/include/log.h
> +++ b/include/log.h
> @@ -46,6 +46,7 @@ enum log_category_t {
>  	LOGC_DM,	/* Core driver-model */
>  	LOGC_DT,	/* Device-tree */
>  	LOGC_EFI,	/* EFI implementation */
> +	LOGC_BLOBLIST,	/* Bloblist */
>  
>  	LOGC_COUNT,
>  	LOGC_END,

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 03/23] Add core support for a bloblist to convey data from SPL
  2018-10-08  2:42   ` Kever Yang
@ 2018-10-09  3:40     ` Simon Glass
  0 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-10-09  3:40 UTC (permalink / raw)
  To: u-boot

Hi Kever,

On 7 October 2018 at 20:42, Kever Yang <kever.yang@rock-chips.com> wrote:

> Hi Simon,
>
>     I didn't notice you already have a V2 for this patch set just now
> and have send a reply to V1.
>
>     My question is:
>     Is it possible to use ATAGs instead of a new 'bloblist'?
>

I thought that was a way of passing things to the kernel, and was dropped
in favour of device tree? Also, it is ARM only, right?

Regards,
Simon


>
> Thanks,
> - Kever
> On 10/02/2018 07:22 PM, Simon Glass wrote:
> > At present there is no standard way in U-Boot to pass information from
> SPL
> > to U-Boot proper. But sometimes SPL wants to convey information to U-Boot
> > that U-Boot cannot easily figure out. For example, if SPL sets up SDRAM
> > then it might want to pass the size of SDRAM, or the location of each
> > bank, to U-Boot proper.
> >
> > Add a new 'bloblist' feature which provides this. A bloblist is set up in
> > the first phase of U-Boot that runs (i.e. TPL or SPL). The location of
> > this info may be in SRAM or CAR (x86 cache-as-RAM) or somewhere else.
> >
> > Information placed in this region is preserved (with a checksum) through
> > TPL and SPL and ends up in U-Boot. At this point it is copied into SDRAM
> > so it can be used after relocation.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > Acked-by: Andreas Dannenberg <dannenberg@ti.com>
> > ---
> >
> > Changes in v2:
> > - Fix several typos
> >
> >  common/Kconfig     |  48 +++++++++
> >  common/Makefile    |   1 +
> >  common/bloblist.c  | 239 +++++++++++++++++++++++++++++++++++++++++++++
> >  include/bloblist.h | 195 ++++++++++++++++++++++++++++++++++++
> >  include/log.h      |   1 +
> >  5 files changed, 484 insertions(+)
> >  create mode 100644 common/bloblist.c
> >  create mode 100644 include/bloblist.h
> >
> > diff --git a/common/Kconfig b/common/Kconfig
> > index 3bb9571b710..2e72b3c83c6 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -715,4 +715,52 @@ config UPDATE_TFTP_MSEC_MAX
> >
> >  endmenu
> >
> > +menu "Blob list"
> > +
> > +config BLOBLIST
> > +     bool "Support for a bloblist"
> > +     help
> > +       This enables support for a bloblist in U-Boot, which can be
> passed
> > +       from TPL to SPL to U-Boot proper (and potentially to Linux). The
> > +       blob list supports multiple binary blobs of data, each with a
> tag,
> > +       so that different U-Boot components can store data which can
> survive
> > +       through to the next stage of the boot.
> > +
> > +config SPL_BLOBLIST
> > +     bool "Support for a bloblist in SPL"
> > +     depends on BLOBLIST
> > +     default y if SPL
> > +     help
> > +       This enables a bloblist in SPL. If this is the first part of
> U-Boot
> > +       to run, then the bloblist is set up in SPL and passed to U-Boot
> > +       proper. If TPL also has a bloblist, then SPL uses the one from
> there.
> > +
> > +config TPL_BLOBLIST
> > +     bool "Support for a bloblist in TPL"
> > +     depends on BLOBLIST
> > +     default y if TPL
> > +     help
> > +       This enables a bloblist in TPL. The bloblist is set up in TPL and
> > +       passed to SPL and U-Boot proper.
> > +
> > +config BLOBLIST_SIZE
> > +     hex "Size of bloblist"
> > +     depends on BLOBLIST
> > +     default 0x400
> > +     help
> > +       Sets the size of the bloblist in bytes. This must include all
> > +       overhead (alignment, bloblist header, record header). The
> bloblist
> > +       is set up in the first part of U-Boot to run (TPL, SPL or U-Boot
> > +       proper), and this sane bloblist is used for subsequent stages.
> > +
> > +config BLOBLIST_ADDR
> > +     hex "Address of bloblist"
> > +     depends on BLOBLIST
> > +     default 0xe000
> > +     help
> > +       Sets the address of the bloblist, set up by the first part of
> U-Boot
> > +       which runs. Subsequent U-Boot stages typically use the same
> address.
> > +
> > +endmenu
> > +
> >  source "common/spl/Kconfig"
> > diff --git a/common/Makefile b/common/Makefile
> > index cbca4ff2da6..6aff2b1a6e3 100644
> > --- a/common/Makefile
> > +++ b/common/Makefile
> > @@ -61,6 +61,7 @@ obj-$(CONFIG_CMDLINE) += cli_readline.o cli_simple.o
> >  endif # !CONFIG_SPL_BUILD
> >
> >  obj-$(CONFIG_$(SPL_TPL_)BOOTSTAGE) += bootstage.o
> > +obj-$(CONFIG_$(SPL_TPL_)BLOBLIST) += bloblist.o
> >
> >  ifdef CONFIG_SPL_BUILD
> >  ifdef CONFIG_SPL_DFU_SUPPORT
> > diff --git a/common/bloblist.c b/common/bloblist.c
> > new file mode 100644
> > index 00000000000..b4cf169b05a
> > --- /dev/null
> > +++ b/common/bloblist.c
> > @@ -0,0 +1,239 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2018 Google, Inc
> > + * Written by Simon Glass <sjg@chromium.org>
> > + */
> > +
> > +#include <common.h>
> > +#include <bloblist.h>
> > +#include <log.h>
> > +#include <mapmem.h>
> > +#include <spl.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +struct bloblist_rec *bloblist_first_blob(struct bloblist_hdr *hdr)
> > +{
> > +     if (hdr->alloced <= hdr->hdr_size)
> > +             return NULL;
> > +     return (struct bloblist_rec *)((void *)hdr + hdr->hdr_size);
> > +}
> > +
> > +struct bloblist_rec *bloblist_next_blob(struct bloblist_hdr *hdr,
> > +                                     struct bloblist_rec *rec)
> > +{
> > +     ulong offset;
> > +
> > +     offset = (void *)rec - (void *)hdr;
> > +     offset += rec->hdr_size + ALIGN(rec->size, BLOBLIST_ALIGN);
> > +     if (offset >= hdr->alloced)
> > +             return NULL;
> > +     return (struct bloblist_rec *)((void *)hdr + offset);
> > +}
> > +
> > +#define foreach_rec(_rec, _hdr) \
> > +     for (_rec = bloblist_first_blob(_hdr); \
> > +          _rec; \
> > +          _rec = bloblist_next_blob(_hdr, _rec))
> > +
> > +static struct bloblist_rec *bloblist_findrec(uint tag)
> > +{
> > +     struct bloblist_hdr *hdr = gd->bloblist;
> > +     struct bloblist_rec *rec;
> > +
> > +     if (!hdr)
> > +             return NULL;
> > +
> > +     foreach_rec(rec, hdr) {
> > +             if (rec->tag == tag)
> > +                     return rec;
> > +     }
> > +
> > +     return NULL;
> > +}
> > +
> > +static int bloblist_addrec(uint tag, int size, struct bloblist_rec
> **recp)
> > +{
> > +     struct bloblist_hdr *hdr = gd->bloblist;
> > +     struct bloblist_rec *rec;
> > +     int new_alloced;
> > +
> > +     new_alloced = hdr->alloced + sizeof(*rec) +
> > +                     ALIGN(size, BLOBLIST_ALIGN);
> > +     if (new_alloced >= hdr->size) {
> > +             log(LOGC_BLOBLIST, LOGL_ERR,
> > +                 "Failed to allocate %x bytes size=%x, need size>=%x\n",
> > +                 size, hdr->size, new_alloced);
> > +             return log_msg_ret("bloblist add", -ENOSPC);
> > +     }
> > +     rec = (void *)hdr + hdr->alloced;
> > +     hdr->alloced = new_alloced;
> > +
> > +     rec->tag = tag;
> > +     rec->hdr_size = sizeof(*rec);
> > +     rec->size = size;
> > +     rec->spare = 0;
> > +     *recp = rec;
> > +
> > +     return 0;
> > +}
> > +
> > +static int bloblist_ensurerec(uint tag, struct bloblist_rec **recp, int
> size)
> > +{
> > +     struct bloblist_rec *rec;
> > +
> > +     rec = bloblist_findrec(tag);
> > +     if (rec) {
> > +             if (size && size != rec->size)
> > +                     return -ESPIPE;
> > +     } else {
> > +             int ret;
> > +
> > +             ret = bloblist_addrec(tag, size, &rec);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +     *recp = rec;
> > +
> > +     return 0;
> > +}
> > +
> > +void *bloblist_find(uint tag, int size)
> > +{
> > +     struct bloblist_rec *rec;
> > +
> > +     rec = bloblist_findrec(tag);
> > +     if (!rec)
> > +             return NULL;
> > +     if (size && size != rec->size)
> > +             return NULL;
> > +
> > +     return (void *)rec + rec->hdr_size;
> > +}
> > +
> > +void *bloblist_add(uint tag, int size)
> > +{
> > +     struct bloblist_rec *rec;
> > +
> > +     if (bloblist_addrec(tag, size, &rec))
> > +             return NULL;
> > +
> > +     return rec + 1;
> > +}
> > +
> > +int bloblist_ensure_size(uint tag, int size, void **blobp)
> > +{
> > +     struct bloblist_rec *rec;
> > +     int ret;
> > +
> > +     ret = bloblist_ensurerec(tag, &rec, size);
> > +     if (ret)
> > +             return ret;
> > +     *blobp = (void *)rec + rec->hdr_size;
> > +
> > +     return 0;
> > +}
> > +
> > +void *bloblist_ensure(uint tag, int size)
> > +{
> > +     struct bloblist_rec *rec;
> > +
> > +     if (bloblist_ensurerec(tag, &rec, size))
> > +             return NULL;
> > +
> > +     return (void *)rec + rec->hdr_size;
> > +}
> > +
> > +static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr)
> > +{
> > +     struct bloblist_rec *rec;
> > +     u32 chksum;
> > +
> > +     chksum = crc32(0, (unsigned char *)hdr,
> > +                    offsetof(struct bloblist_hdr, chksum));
> > +     foreach_rec(rec, hdr) {
> > +             chksum = crc32(chksum, (void *)rec, rec->hdr_size);
> > +             chksum = crc32(chksum, (void *)rec + rec->hdr_size,
> rec->size);
> > +     }
> > +
> > +     return chksum;
> > +}
> > +
> > +int bloblist_new(ulong addr, uint size, uint flags)
> > +{
> > +     struct bloblist_hdr *hdr;
> > +
> > +     if (size < sizeof(*hdr))
> > +             return log_ret(-ENOSPC);
> > +     if (addr & (BLOBLIST_ALIGN - 1))
> > +             return log_ret(-EFAULT);
> > +     hdr = map_sysmem(addr, size);
> > +     memset(hdr, '\0', sizeof(*hdr));
> > +     hdr->version = BLOBLIST_VERSION;
> > +     hdr->hdr_size = sizeof(*hdr);
> > +     hdr->flags = flags;
> > +     hdr->magic = BLOBLIST_MAGIC;
> > +     hdr->size = size;
> > +     hdr->alloced = hdr->hdr_size;
> > +     hdr->chksum = 0;
> > +     gd->bloblist = hdr;
> > +
> > +     return 0;
> > +}
> > +
> > +int bloblist_check(ulong addr, uint size)
> > +{
> > +     struct bloblist_hdr *hdr;
> > +     u32 chksum;
> > +
> > +     hdr = map_sysmem(addr, sizeof(*hdr));
> > +     if (hdr->magic != BLOBLIST_MAGIC)
> > +             return log_msg_ret("Bad magic", -ENOENT);
> > +     if (hdr->version != BLOBLIST_VERSION)
> > +             return log_msg_ret("Bad version", -EPROTONOSUPPORT);
> > +     if (size && hdr->size != size)
> > +             return log_msg_ret("Bad size", -EFBIG);
> > +     chksum = bloblist_calc_chksum(hdr);
> > +     if (hdr->chksum != chksum) {
> > +             log(LOGC_BLOBLIST, LOGL_ERR, "Checksum %x != %x\n",
> hdr->chksum,
> > +                 chksum);
> > +             return log_msg_ret("Bad checksum", -EIO);
> > +     }
> > +     gd->bloblist = hdr;
> > +
> > +     return 0;
> > +}
> > +
> > +int bloblist_finish(void)
> > +{
> > +     struct bloblist_hdr *hdr = gd->bloblist;
> > +
> > +     hdr->chksum = bloblist_calc_chksum(hdr);
> > +
> > +     return 0;
> > +}
> > +
> > +int bloblist_init(void)
> > +{
> > +     bool expected;
> > +     int ret = -ENOENT;
> > +
> > +     /**
> > +      * Wed expect to find an existing bloblist in the first phase of
> U-Boot
> > +      * that runs
> > +      */
> > +     expected = !u_boot_first_phase();
> > +     if (expected)
> > +             ret = bloblist_check(CONFIG_BLOBLIST_ADDR,
> > +                                  CONFIG_BLOBLIST_SIZE);
> > +     if (ret) {
> > +             log(LOGC_BLOBLIST, expected ? LOGL_WARNING : LOGL_DEBUG,
> > +                 "Existing bloblist not found: creating new
> bloblist\n");
> > +             ret = bloblist_new(CONFIG_BLOBLIST_ADDR,
> CONFIG_BLOBLIST_SIZE,
> > +                                0);
> > +     } else {
> > +             log(LOGC_BLOBLIST, LOGL_DEBUG, "Found existing
> bloblist\n");
> > +     }
> > +
> > +     return ret;
> > +}
> > diff --git a/include/bloblist.h b/include/bloblist.h
> > new file mode 100644
> > index 00000000000..413736a9080
> > --- /dev/null
> > +++ b/include/bloblist.h
> > @@ -0,0 +1,195 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * This provides a standard way of passing information between boot
> phases
> > + * (TPL -> SPL -> U-Boot proper.)
> > + *
> > + * A list of blobs of data, tagged with their owner. The list resides
> in memory
> > + * and can be updated by SPL, U-Boot, etc.
> > + *
> > + * Copyright 2018 Google, Inc
> > + * Written by Simon Glass <sjg@chromium.org>
> > + */
> > +
> > +#ifndef __BLOBLIST_H
> > +#define __BLOBLIST_H
> > +
> > +enum {
> > +     BLOBLIST_VERSION        = 0,
> > +     BLOBLIST_MAGIC          = 0xb00757a3,
> > +     BLOBLIST_ALIGN          = 16,
> > +};
> > +
> > +enum bloblist_tag_t {
> > +     BLOBLISTT_NONE = 0,
> > +
> > +     /* Vendor-specific tags are permitted here */
> > +     BLOBLISTT_EC_HOSTEVENT,         /* Chromium OS EC host-event mask
> */
> > +     BLOBLISTT_SPL_HANDOFF,          /* Hand-off info from SPL */
> > +     BLOBLISTT_VBOOT_CTX,            /* Chromium OS verified boot
> context */
> > +     BLOBLISTT_VBOOT_HANDOFF,        /* Chromium OS internal handoff
> info */
> > +};
> > +
> > +/**
> > + * struct bloblist_hdr - header for the bloblist
> > + *
> > + * This is stored at the start of the bloblist which is always on a
> 16-byte
> > + * boundary. Records follow this header. The bloblist normally stays in
> the
> > + * same place in memory as SPL and U-Boot execute, but it can be safely
> moved
> > + * around.
> > + *
> > + * None of the bloblist structures contain pointers but it is possible
> to put
> > + * pointers inside a bloblist record if desired. This is not encouraged,
> > + * since it can make part of the bloblist inaccessible if the pointer is
> > + * no-longer valid. It is better to just store all the data inside a
> bloblist
> > + * record.
> > + *
> > + * Each bloblist record is aligned to a 16-byte boundary and follows
> immediately
> > + * from the last.
> > + *
> > + * @version: BLOBLIST_VERSION
> > + * @hdr_size: Size of this header, normally sizeof(struct
> bloblist_hdr). The
> > + *   first bloblist_rec starts at this offset from the start of the
> header
> > + * @flags: Space for BLOBLISTF_... flags (none yet)
> > + * @magic: BLOBLIST_MAGIC
> > + * @size: Total size of all records (non-zero if valid) including this
> header.
> > + *   The bloblist extends for this many bytes from the start of this
> header.
> > + * @alloced: Total size allocated for this bloblist. When adding new
> records,
> > + *   the bloblist can grow up to this size. This starts out as
> > + *   sizeof(bloblist_hdr) since we need at least that much space to
> store a
> > + *   valid bloblist
> > + * @spare: Space space
> > + * @chksum: CRC32 for the entire bloblist allocated area. Since any of
> the
> > + *   blobs can be altered after being created, this checksum is only
> valid
> > + *   when the bloblist is finalised before jumping to the next stage of
> boot.
> > + *   Note: @chksum is last to make it easier to exclude it from the
> checksum
> > + *   calculation.
> > + */
> > +struct bloblist_hdr {
> > +     u32 version;
> > +     u32 hdr_size;
> > +     u32 flags;
> > +     u32 magic;
> > +
> > +     u32 size;
> > +     u32 alloced;
> > +     u32 spare;
> > +     u32 chksum;
> > +};
> > +
> > +/**
> > + * struct bloblist_rec - record for the bloblist
> > + *
> > + * NOTE: Only exported for testing purposes. Do not use this struct.
> > + *
> > + * The bloblist contains a number of records each consisting of this
> record
> > + * structure followed by the data contained. Each records is 16-byte
> aligned.
> > + *
> > + * @tag: Tag indicating what the record contains
> > + * @hdr_size: Size of this header, normally sizeof(struct
> bloblist_rec). The
> > + *   record's data starts at this offset from the start of the record
> > + * @size: Size of record in bytes, excluding the header size. This does
> not
> > + *   need to be aligned (e.g. 3 is OK).
> > + * @spare: Spare space for other things
> > + */
> > +struct bloblist_rec {
> > +     u32 tag;
> > +     u32 hdr_size;
> > +     u32 size;
> > +     u32 spare;
> > +};
> > +
> > +/**
> > + * bloblist_find() - Find a blob
> > + *
> > + * Searches the bloblist and returns the blob with the matching tag
> > + *
> > + * @tag:     Tag to search for (enum bloblist_tag_t)
> > + * @size:    Expected size of the blob
> > + * @return pointer to blob if found, or NULL if not found, or a blob
> was found
> > + *   but it is the wrong size
> > + */
> > +void *bloblist_find(uint tag, int size);
> > +
> > +/**
> > + * bloblist_add() - Add a new blob
> > + *
> > + * Add a new blob to the bloblist
> > + *
> > + * This should only be called if you konw there is no existing blob for
> a
> > + * particular tag. It is typically safe to call in the first phase of
> U-Boot
> > + * (e.g. TPL or SPL). After that, bloblist_ensure() should be used
> instead.
> > + *
> > + * @tag:     Tag to add (enum bloblist_tag_t)
> > + * @size:    Size of the blob
> > + * @return pointer to the newly added block, or NULL if there is not
> enough
> > + *   space for the blob
> > + */
> > +void *bloblist_add(uint tag, int size);
> > +
> > +/**
> > + * bloblist_ensure_size() - Find or add a blob
> > + *
> > + * Find an existing blob, or add a new one if not found
> > + *
> > + * @tag:     Tag to add (enum bloblist_tag_t)
> > + * @size:    Size of the blob
> > + * @blobp:   Returns a pointer to blob on success
> > + * @return 0 if OK, -ENOSPC if it is missing and could not be added due
> to lack
> > + *   of space, or -ESPIPE it exists but has the wrong size
> > + */
> > +int bloblist_ensure_size(uint tag, int size, void **blobp);
> > +
> > +/**
> > + * bloblist_ensure() - Find or add a blob
> > + *
> > + * Find an existing blob, or add a new one if not found
> > + *
> > + * @tag:     Tag to add (enum bloblist_tag_t)
> > + * @size:    Size of the blob
> > + * @return pointer to blob, or NULL if it is missing and could not be
> added due
> > + *   to lack of space, or it exists but has the wrong size
> > + */
> > +void *bloblist_ensure(uint tag, int size);
> > +
> > +/**
> > + * bloblist_new() - Create a new, empty bloblist of a given size
> > + *
> > + * @addr: Address of bloblist
> > + * @size: Initial size for bloblist
> > + * @flags: Flags to use for bloblist
> > + * @return 0 if OK, -EFAULT if addr is not aligned correctly, -ENOSPC
> is the
> > + *   area is not large enough
> > + */
> > +int bloblist_new(ulong addr, uint size, uint flags);
> > +
> > +/**
> > + * bloblist_check() - Check if a bloblist exists
> > + *
> > + * @addr: Address of bloblist
> > + * @size: Expected size of blobsize, or 0 to detect the size
> > + * @return 0 if OK, -ENOENT if the magic number doesn't match
> (indicating that
> > + *   there problem is no bloblist at the given address),
> -EPROTONOSUPPORT
> > + *   if the version does not match, -EIO if the checksum does not match,
> > + *   -EFBIG if the expected size does not match the detected size
> > + */
> > +int bloblist_check(ulong addr, uint size);
> > +
> > +/**
> > + * bloblist_finish() - Set up the bloblist for the next U-Boot part
> > + *
> > + * This sets the correct checksum for the bloblist. This ensures that
> the
> > + * bloblist will be detected correctly by the next phase of U-Boot.
> > + *
> > + * @return 0
> > + */
> > +int bloblist_finish(void);
> > +
> > +/**
> > + * bloblist_init() - Init the bloblist system with a single bloblist
> > + *
> > + * This uses CONFIG_BLOBLIST_ADDR and CONFIG_BLOBLIST_SIZE to set up a
> bloblist
> > + * for use by U-Boot.
> > + */
> > +int bloblist_init(void);
> > +
> > +#endif /* __BLOBLIST_H */
> > diff --git a/include/log.h b/include/log.h
> > index 1146f423a7a..61411b72eac 100644
> > --- a/include/log.h
> > +++ b/include/log.h
> > @@ -46,6 +46,7 @@ enum log_category_t {
> >       LOGC_DM,        /* Core driver-model */
> >       LOGC_DT,        /* Device-tree */
> >       LOGC_EFI,       /* EFI implementation */
> > +     LOGC_BLOBLIST,  /* Bloblist */
> >
> >       LOGC_COUNT,
> >       LOGC_END,
>
>
>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v2, 02/23] spl: Add support for logging in SPL and TPL
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 02/23] spl: Add support for logging in SPL and TPL Simon Glass
@ 2018-11-09 18:42   ` Tom Rini
  0 siblings, 0 replies; 48+ messages in thread
From: Tom Rini @ 2018-11-09 18:42 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 02, 2018 at 05:22:32AM -0600, Simon Glass wrote:

> It is sometimes useful to log information in SPL and TPL. Add support for
> this.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181109/87ae8ce8/attachment.sig>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v2, 03/23] Add core support for a bloblist to convey data from SPL
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 03/23] Add core support for a bloblist to convey data from SPL Simon Glass
  2018-10-08  2:42   ` Kever Yang
@ 2018-11-09 18:42   ` Tom Rini
  1 sibling, 0 replies; 48+ messages in thread
From: Tom Rini @ 2018-11-09 18:42 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 02, 2018 at 05:22:33AM -0600, Simon Glass wrote:

> At present there is no standard way in U-Boot to pass information from SPL
> to U-Boot proper. But sometimes SPL wants to convey information to U-Boot
> that U-Boot cannot easily figure out. For example, if SPL sets up SDRAM
> then it might want to pass the size of SDRAM, or the location of each
> bank, to U-Boot proper.
> 
> Add a new 'bloblist' feature which provides this. A bloblist is set up in
> the first phase of U-Boot that runs (i.e. TPL or SPL). The location of
> this info may be in SRAM or CAR (x86 cache-as-RAM) or somewhere else.
> 
> Information placed in this region is preserved (with a checksum) through
> TPL and SPL and ends up in U-Boot. At this point it is copied into SDRAM
> so it can be used after relocation.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Acked-by: Andreas Dannenberg <dannenberg@ti.com>

In general:

Reviewed-by: Tom Rini <trini@konsulko.com>

But:
> +config BLOBLIST_ADDR
> +	hex "Address of bloblist"
> +	depends on BLOBLIST
> +	default 0xe000

This default needs to be if WHATEVER, for whatever platform that's a
valid address for using by default.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181109/62daf2de/attachment.sig>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v2,04/23] spl: Set up the bloblist in SPL
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 04/23] spl: Set up the bloblist in SPL Simon Glass
@ 2018-11-09 18:43   ` Tom Rini
  2018-11-16  0:07     ` Simon Glass
  0 siblings, 1 reply; 48+ messages in thread
From: Tom Rini @ 2018-11-09 18:43 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 02, 2018 at 05:22:34AM -0600, Simon Glass wrote:
> The bloblist is normally set up in SPL ready for use by U-Boot. Add
> a simple implementation of this to the common SPL code.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v2: None
> 
>  common/spl/spl.c | 18 ++++++++++++++++--
>  include/spl.h    | 27 +++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index b917624e61d..2ca900ae9ed 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -7,6 +7,7 @@
>   */
>  
>  #include <common.h>
> +#include <bloblist.h>
>  #include <binman_sym.h>
>  #include <dm.h>
>  #include <spl.h>
> @@ -345,6 +346,14 @@ static int spl_common_init(bool setup_malloc)
>  		return ret;
>  	}
>  #endif
> +	if (CONFIG_IS_ENABLED(BLOBLIST)) {
> +		ret = bloblist_init();
> +		if (ret) {
> +			debug("%s: Failed to set up bloblist: ret=%d\n",
> +			      __func__, ret);
> +			return ret;
> +		}
> +	}
>  	if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) {
>  		ret = fdtdec_setup();
>  		if (ret) {
> @@ -483,6 +492,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>  		BOOT_DEVICE_NONE,
>  	};
>  	struct spl_image_info spl_image;
> +	int ret;
>  
>  	debug(">>spl:board_init_r()\n");
>  
> @@ -529,6 +539,12 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>  	}
>  
>  	spl_perform_fixups(&spl_image);
> +	if (CONFIG_IS_ENABLED(BLOBLIST)) {
> +		ret = bloblist_finish();
> +		if (ret)
> +			printf("Warning: Failed to finish bloblist (ret=%d)\n",
> +			       ret);
> +	}
>  
>  #ifdef CONFIG_CPU_V7M
>  	spl_image.entry_point |= 0x1;
> @@ -558,8 +574,6 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>  	      gd->malloc_ptr / 1024);
>  #endif
>  #ifdef CONFIG_BOOTSTAGE_STASH
> -	int ret;
> -
>  	bootstage_mark_name(BOOTSTAGE_ID_END_SPL, "end_spl");
>  	ret = bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
>  			      CONFIG_BOOTSTAGE_STASH_SIZE);

I think we'll need a __maybe_unused on ret above for !BOOTSTAGE_STASH &&
!BLOBLIST.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181109/ced53b55/attachment.sig>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v2,05/23] bloblist: Locate bloblist in U-Boot
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 05/23] bloblist: Locate bloblist in U-Boot Simon Glass
@ 2018-11-09 18:43   ` Tom Rini
  0 siblings, 0 replies; 48+ messages in thread
From: Tom Rini @ 2018-11-09 18:43 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 02, 2018 at 05:22:35AM -0600, Simon Glass wrote:

> Add support for locating a bloblist in U-Boot that has been set up by SPL.
> It is copied into RAM during relocation.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181109/11066a39/attachment.sig>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v2, 08/23] spl: Support hash, input, pch, pci, rtc, tpm in SPL
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 08/23] spl: Support hash, input, pch, pci, rtc, tpm in SPL Simon Glass
@ 2018-11-09 18:43   ` Tom Rini
  0 siblings, 0 replies; 48+ messages in thread
From: Tom Rini @ 2018-11-09 18:43 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 02, 2018 at 05:22:38AM -0600, Simon Glass wrote:

> At present these subsystems are only supported in U-Boot proper but it is
> sometimes necessary to support them in SPL, or even TPL. Update the
> Kconfig and Makefile to support this. Also adjust GPIO so that it can be
> used in TPL if required.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181109/e9847762/attachment.sig>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v2, 09/23] spl: Add a define for SPL_TPL_PROMPT
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 09/23] spl: Add a define for SPL_TPL_PROMPT Simon Glass
@ 2018-11-09 18:43   ` Tom Rini
  0 siblings, 0 replies; 48+ messages in thread
From: Tom Rini @ 2018-11-09 18:43 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 02, 2018 at 05:22:39AM -0600, Simon Glass wrote:

> We should use a macro rather than hard-coding the SPL prompt to 'spl'
> since the code can be used by TPL too. Add a macro that works for both
> and use it in various places.
> 
> This allows TPL to use the same code without printing confusing messages.
> 
> Note that the string is lower case ('spl', 'tpl') which is a change from
> previously.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181109/7854765f/attachment.sig>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v2, 10/23] spl: Make SPL_DISABLE_BANNER_PRINT a positive option
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 10/23] spl: Make SPL_DISABLE_BANNER_PRINT a positive option Simon Glass
@ 2018-11-09 18:43   ` Tom Rini
  0 siblings, 0 replies; 48+ messages in thread
From: Tom Rini @ 2018-11-09 18:43 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 02, 2018 at 05:22:40AM -0600, Simon Glass wrote:

> Rather than having a negative option, make this a positive option and
> enable it by default. This makes it easier to understand.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181109/c1229af9/attachment.sig>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 11/23] spl: Add a comment to spl_set_bd() Simon Glass
@ 2018-11-09 18:43   ` Tom Rini
  2019-02-11 21:00     ` Simon Goldschmidt
  0 siblings, 1 reply; 48+ messages in thread
From: Tom Rini @ 2018-11-09 18:43 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote:

> There is a strange feature to set global_data to a data-section variable
> early in SPL. This only works if SPL actually has access to SRAM which is
> not the case on x86, for eaxmple. Add a comment to this effect.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181109/b7e52ba6/attachment.sig>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v2, 12/23] spl: Print a message if we are unable to load an image
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 12/23] spl: Print a message if we are unable to load an image Simon Glass
@ 2018-11-09 18:43   ` Tom Rini
  2018-11-16  0:08     ` Simon Glass
  0 siblings, 1 reply; 48+ messages in thread
From: Tom Rini @ 2018-11-09 18:43 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 02, 2018 at 05:22:42AM -0600, Simon Glass wrote:

> It can confusing when U-Boot SPL hangs for no obvious reason, when it is
> unable to load U-Boot. Add a message to indicate the cause.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v2: None
> 
>  common/spl/spl.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 396c42e1e1b..512141c4139 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -485,6 +485,7 @@ static int boot_from_devices(struct spl_image_info *spl_image,
>  			return 0;
>  		}
>  	}
> +	puts(SPL_TPL_PROMPT "No more boot devices\n");
>  
>  	return -ENODEV;
>  }

Don't we have a similar debug() statement around this part of the boot
flow?  It is annoying but I also think we ended up dropping it due to
binary size issues.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181109/9ff440c3/attachment.sig>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v2, 23/23] spl: Add support for passing handoff info to U-Boot proper
  2018-10-02 11:22 ` [U-Boot] [PATCH v2 23/23] spl: Add support for passing handoff info to U-Boot proper Simon Glass
@ 2018-11-09 18:43   ` Tom Rini
  0 siblings, 0 replies; 48+ messages in thread
From: Tom Rini @ 2018-11-09 18:43 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 02, 2018 at 05:22:53AM -0600, Simon Glass wrote:

> There is some basic informaton that SPL normally wants to pass through to
> U-Boot, such as the SDRAM size and bank information.
> 
> Mkae use of the new bloblist structure for this. Add a new 'handoff' blob
> which is set up in SPL and passed to U-Boot proper. Also adda  test for
> sandbox_spl that checks that this works correctly and a new 'sb' command
> to show the information passed from SPL.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181109/12ca74bb/attachment.sig>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 01/23] log: Correct definition of log_msg_ret()
  2018-10-04  9:25   ` Bin Meng
@ 2018-11-16  0:07     ` Simon Glass
  2018-11-16  1:43       ` Bin Meng
  0 siblings, 1 reply; 48+ messages in thread
From: Simon Glass @ 2018-11-16  0:07 UTC (permalink / raw)
  To: u-boot

Hi Bin,

On 4 October 2018 at 02:25, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Tue, Oct 2, 2018 at 8:25 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > This macro should have two parameters, not one. Fix it so that it
> > correctly resolves to _ret when logging is disabled.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v2: None
> >
> >  include/log.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/log.h b/include/log.h
> > index 653fb8d853e..75ff1e1160c 100644
> > --- a/include/log.h
> > +++ b/include/log.h
> > @@ -175,7 +175,7 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line,
> >         })
> >  #else
> >  #define log_ret(_ret) (_ret)
> > -#define log_msg_ret(_ret) (_ret)
> > +#define log_msg_ret(_msg, _ret) (_ret)
>
> This creates a warning still if logging is disabled.
>
> include/log.h:178:33: warning: statement with no effect [-Wunused-value]
>  #define log_msg_ret(_msg, _ret) (_ret)

It has to be used in a 'return' statement. I'll add a comment in the
first patch of the spl handoff series.

Regards,
Simon

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v2,04/23] spl: Set up the bloblist in SPL
  2018-11-09 18:43   ` [U-Boot] [U-Boot,v2,04/23] " Tom Rini
@ 2018-11-16  0:07     ` Simon Glass
  2018-11-16  0:10       ` Tom Rini
  0 siblings, 1 reply; 48+ messages in thread
From: Simon Glass @ 2018-11-16  0:07 UTC (permalink / raw)
  To: u-boot

Hi Tom,

On 9 November 2018 at 10:43, Tom Rini <trini@konsulko.com> wrote:
> On Tue, Oct 02, 2018 at 05:22:34AM -0600, Simon Glass wrote:
>> The bloblist is normally set up in SPL ready for use by U-Boot. Add
>> a simple implementation of this to the common SPL code.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v2: None
>>
>>  common/spl/spl.c | 18 ++++++++++++++++--
>>  include/spl.h    | 27 +++++++++++++++++++++++++++
>>  2 files changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>> index b917624e61d..2ca900ae9ed 100644
>> --- a/common/spl/spl.c
>> +++ b/common/spl/spl.c
>> @@ -7,6 +7,7 @@
>>   */
>>
>>  #include <common.h>
>> +#include <bloblist.h>
>>  #include <binman_sym.h>
>>  #include <dm.h>
>>  #include <spl.h>
>> @@ -345,6 +346,14 @@ static int spl_common_init(bool setup_malloc)
>>               return ret;
>>       }
>>  #endif
>> +     if (CONFIG_IS_ENABLED(BLOBLIST)) {
>> +             ret = bloblist_init();
>> +             if (ret) {
>> +                     debug("%s: Failed to set up bloblist: ret=%d\n",
>> +                           __func__, ret);
>> +                     return ret;
>> +             }
>> +     }
>>       if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) {
>>               ret = fdtdec_setup();
>>               if (ret) {
>> @@ -483,6 +492,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>>               BOOT_DEVICE_NONE,
>>       };
>>       struct spl_image_info spl_image;
>> +     int ret;
>>
>>       debug(">>spl:board_init_r()\n");
>>
>> @@ -529,6 +539,12 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>>       }
>>
>>       spl_perform_fixups(&spl_image);
>> +     if (CONFIG_IS_ENABLED(BLOBLIST)) {
>> +             ret = bloblist_finish();
>> +             if (ret)
>> +                     printf("Warning: Failed to finish bloblist (ret=%d)\n",
>> +                            ret);
>> +     }
>>
>>  #ifdef CONFIG_CPU_V7M
>>       spl_image.entry_point |= 0x1;
>> @@ -558,8 +574,6 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>>             gd->malloc_ptr / 1024);
>>  #endif
>>  #ifdef CONFIG_BOOTSTAGE_STASH
>> -     int ret;
>> -
>>       bootstage_mark_name(BOOTSTAGE_ID_END_SPL, "end_spl");
>>       ret = bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
>>                             CONFIG_BOOTSTAGE_STASH_SIZE);
>
> I think we'll need a __maybe_unused on ret above for !BOOTSTAGE_STASH &&
> !BLOBLIST.
>

The code for BLOBLIST uses if() rather than #if so it seems OK:

if (CONFIG_IS_ENABLED(BLOBLIST)) {

Regards,
Simon

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot, v2, 12/23] spl: Print a message if we are unable to load an image
  2018-11-09 18:43   ` [U-Boot] [U-Boot, v2, " Tom Rini
@ 2018-11-16  0:08     ` Simon Glass
  0 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-11-16  0:08 UTC (permalink / raw)
  To: u-boot

Hi Tom,

On 9 November 2018 at 10:43, Tom Rini <trini@konsulko.com> wrote:
> On Tue, Oct 02, 2018 at 05:22:42AM -0600, Simon Glass wrote:
>
>> It can confusing when U-Boot SPL hangs for no obvious reason, when it is
>> unable to load U-Boot. Add a message to indicate the cause.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v2: None
>>
>>  common/spl/spl.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>> index 396c42e1e1b..512141c4139 100644
>> --- a/common/spl/spl.c
>> +++ b/common/spl/spl.c
>> @@ -485,6 +485,7 @@ static int boot_from_devices(struct spl_image_info *spl_image,
>>                       return 0;
>>               }
>>       }
>> +     puts(SPL_TPL_PROMPT "No more boot devices\n");
>>
>>       return -ENODEV;
>>  }
>
> Don't we have a similar debug() statement around this part of the boot
> flow?  It is annoying but I also think we ended up dropping it due to
> binary size issues.

Er yes you are right:

   puts(SPL_TPL_PROMPT "failed to boot from all boot devices\n");

I'll drop this patch.

Regards,
Simon

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v2,04/23] spl: Set up the bloblist in SPL
  2018-11-16  0:07     ` Simon Glass
@ 2018-11-16  0:10       ` Tom Rini
  2018-11-16  0:29         ` Simon Glass
  0 siblings, 1 reply; 48+ messages in thread
From: Tom Rini @ 2018-11-16  0:10 UTC (permalink / raw)
  To: u-boot

On Thu, Nov 15, 2018 at 04:07:59PM -0800, Simon Glass wrote:
> Hi Tom,
> 
> On 9 November 2018 at 10:43, Tom Rini <trini@konsulko.com> wrote:
> > On Tue, Oct 02, 2018 at 05:22:34AM -0600, Simon Glass wrote:
> >> The bloblist is normally set up in SPL ready for use by U-Boot. Add
> >> a simple implementation of this to the common SPL code.
> >>
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >> ---
> >>
> >> Changes in v2: None
> >>
> >>  common/spl/spl.c | 18 ++++++++++++++++--
> >>  include/spl.h    | 27 +++++++++++++++++++++++++++
> >>  2 files changed, 43 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/common/spl/spl.c b/common/spl/spl.c
> >> index b917624e61d..2ca900ae9ed 100644
> >> --- a/common/spl/spl.c
> >> +++ b/common/spl/spl.c
> >> @@ -7,6 +7,7 @@
> >>   */
> >>
> >>  #include <common.h>
> >> +#include <bloblist.h>
> >>  #include <binman_sym.h>
> >>  #include <dm.h>
> >>  #include <spl.h>
> >> @@ -345,6 +346,14 @@ static int spl_common_init(bool setup_malloc)
> >>               return ret;
> >>       }
> >>  #endif
> >> +     if (CONFIG_IS_ENABLED(BLOBLIST)) {
> >> +             ret = bloblist_init();
> >> +             if (ret) {
> >> +                     debug("%s: Failed to set up bloblist: ret=%d\n",
> >> +                           __func__, ret);
> >> +                     return ret;
> >> +             }
> >> +     }
> >>       if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) {
> >>               ret = fdtdec_setup();
> >>               if (ret) {
> >> @@ -483,6 +492,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
> >>               BOOT_DEVICE_NONE,
> >>       };
> >>       struct spl_image_info spl_image;
> >> +     int ret;
> >>
> >>       debug(">>spl:board_init_r()\n");
> >>
> >> @@ -529,6 +539,12 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
> >>       }
> >>
> >>       spl_perform_fixups(&spl_image);
> >> +     if (CONFIG_IS_ENABLED(BLOBLIST)) {
> >> +             ret = bloblist_finish();
> >> +             if (ret)
> >> +                     printf("Warning: Failed to finish bloblist (ret=%d)\n",
> >> +                            ret);
> >> +     }
> >>
> >>  #ifdef CONFIG_CPU_V7M
> >>       spl_image.entry_point |= 0x1;
> >> @@ -558,8 +574,6 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
> >>             gd->malloc_ptr / 1024);
> >>  #endif
> >>  #ifdef CONFIG_BOOTSTAGE_STASH
> >> -     int ret;
> >> -
> >>       bootstage_mark_name(BOOTSTAGE_ID_END_SPL, "end_spl");
> >>       ret = bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
> >>                             CONFIG_BOOTSTAGE_STASH_SIZE);
> >
> > I think we'll need a __maybe_unused on ret above for !BOOTSTAGE_STASH &&
> > !BLOBLIST.
> >
> 
> The code for BLOBLIST uses if() rather than #if so it seems OK:
> 
> if (CONFIG_IS_ENABLED(BLOBLIST)) {

So long as it doesn't warn, OK :)

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181115/cca38c40/attachment.sig>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v2,04/23] spl: Set up the bloblist in SPL
  2018-11-16  0:10       ` Tom Rini
@ 2018-11-16  0:29         ` Simon Glass
  0 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-11-16  0:29 UTC (permalink / raw)
  To: u-boot

Hi Tom,

On 15 November 2018 at 16:10, Tom Rini <trini@konsulko.com> wrote:
> On Thu, Nov 15, 2018 at 04:07:59PM -0800, Simon Glass wrote:
>> Hi Tom,
>>
>> On 9 November 2018 at 10:43, Tom Rini <trini@konsulko.com> wrote:
>> > On Tue, Oct 02, 2018 at 05:22:34AM -0600, Simon Glass wrote:
>> >> The bloblist is normally set up in SPL ready for use by U-Boot. Add
>> >> a simple implementation of this to the common SPL code.
>> >>
>> >> Signed-off-by: Simon Glass <sjg@chromium.org>
>> >> ---
>> >>
>> >> Changes in v2: None
>> >>
>> >>  common/spl/spl.c | 18 ++++++++++++++++--
>> >>  include/spl.h    | 27 +++++++++++++++++++++++++++
>> >>  2 files changed, 43 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/common/spl/spl.c b/common/spl/spl.c
>> >> index b917624e61d..2ca900ae9ed 100644
>> >> --- a/common/spl/spl.c
>> >> +++ b/common/spl/spl.c
>> >> @@ -7,6 +7,7 @@
>> >>   */
>> >>
>> >>  #include <common.h>
>> >> +#include <bloblist.h>
>> >>  #include <binman_sym.h>
>> >>  #include <dm.h>
>> >>  #include <spl.h>
>> >> @@ -345,6 +346,14 @@ static int spl_common_init(bool setup_malloc)
>> >>               return ret;
>> >>       }
>> >>  #endif
>> >> +     if (CONFIG_IS_ENABLED(BLOBLIST)) {
>> >> +             ret = bloblist_init();
>> >> +             if (ret) {
>> >> +                     debug("%s: Failed to set up bloblist: ret=%d\n",
>> >> +                           __func__, ret);
>> >> +                     return ret;
>> >> +             }
>> >> +     }
>> >>       if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) {
>> >>               ret = fdtdec_setup();
>> >>               if (ret) {
>> >> @@ -483,6 +492,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>> >>               BOOT_DEVICE_NONE,
>> >>       };
>> >>       struct spl_image_info spl_image;
>> >> +     int ret;
>> >>
>> >>       debug(">>spl:board_init_r()\n");
>> >>
>> >> @@ -529,6 +539,12 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>> >>       }
>> >>
>> >>       spl_perform_fixups(&spl_image);
>> >> +     if (CONFIG_IS_ENABLED(BLOBLIST)) {
>> >> +             ret = bloblist_finish();
>> >> +             if (ret)
>> >> +                     printf("Warning: Failed to finish bloblist (ret=%d)\n",
>> >> +                            ret);
>> >> +     }
>> >>
>> >>  #ifdef CONFIG_CPU_V7M
>> >>       spl_image.entry_point |= 0x1;
>> >> @@ -558,8 +574,6 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>> >>             gd->malloc_ptr / 1024);
>> >>  #endif
>> >>  #ifdef CONFIG_BOOTSTAGE_STASH
>> >> -     int ret;
>> >> -
>> >>       bootstage_mark_name(BOOTSTAGE_ID_END_SPL, "end_spl");
>> >>       ret = bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
>> >>                             CONFIG_BOOTSTAGE_STASH_SIZE);
>> >
>> > I think we'll need a __maybe_unused on ret above for !BOOTSTAGE_STASH &&
>> > !BLOBLIST.
>> >
>>
>> The code for BLOBLIST uses if() rather than #if so it seems OK:
>>
>> if (CONFIG_IS_ENABLED(BLOBLIST)) {
>
> So long as it doesn't warn, OK :)

That's right. One of the benefits of if() is we can avoid #ifdefs
around local variables .

Regards,
Simon

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 01/23] log: Correct definition of log_msg_ret()
  2018-11-16  0:07     ` Simon Glass
@ 2018-11-16  1:43       ` Bin Meng
  2018-11-16  2:44         ` Simon Glass
  0 siblings, 1 reply; 48+ messages in thread
From: Bin Meng @ 2018-11-16  1:43 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Fri, Nov 16, 2018 at 8:08 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On 4 October 2018 at 02:25, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Tue, Oct 2, 2018 at 8:25 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > This macro should have two parameters, not one. Fix it so that it
> > > correctly resolves to _ret when logging is disabled.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > > Changes in v2: None
> > >
> > >  include/log.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/include/log.h b/include/log.h
> > > index 653fb8d853e..75ff1e1160c 100644
> > > --- a/include/log.h
> > > +++ b/include/log.h
> > > @@ -175,7 +175,7 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line,
> > >         })
> > >  #else
> > >  #define log_ret(_ret) (_ret)
> > > -#define log_msg_ret(_ret) (_ret)
> > > +#define log_msg_ret(_msg, _ret) (_ret)
> >
> > This creates a warning still if logging is disabled.
> >
> > include/log.h:178:33: warning: statement with no effect [-Wunused-value]
> >  #define log_msg_ret(_msg, _ret) (_ret)
>
> It has to be used in a 'return' statement. I'll add a comment in the
> first patch of the spl handoff series.
>

Yes, it seems I have noticed this and see my patch here :)
http://patchwork.ozlabs.org/patch/996894/

Regards,
Bin

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [PATCH v2 01/23] log: Correct definition of log_msg_ret()
  2018-11-16  1:43       ` Bin Meng
@ 2018-11-16  2:44         ` Simon Glass
  0 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2018-11-16  2:44 UTC (permalink / raw)
  To: u-boot

Hi Bin,

On 15 November 2018 at 17:43, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Fri, Nov 16, 2018 at 8:08 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On 4 October 2018 at 02:25, Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Tue, Oct 2, 2018 at 8:25 PM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > This macro should have two parameters, not one. Fix it so that it
> > > > correctly resolves to _ret when logging is disabled.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > >
> > > > Changes in v2: None
> > > >
> > > >  include/log.h | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/include/log.h b/include/log.h
> > > > index 653fb8d853e..75ff1e1160c 100644
> > > > --- a/include/log.h
> > > > +++ b/include/log.h
> > > > @@ -175,7 +175,7 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line,
> > > >         })
> > > >  #else
> > > >  #define log_ret(_ret) (_ret)
> > > > -#define log_msg_ret(_ret) (_ret)
> > > > +#define log_msg_ret(_msg, _ret) (_ret)
> > >
> > > This creates a warning still if logging is disabled.
> > >
> > > include/log.h:178:33: warning: statement with no effect [-Wunused-value]
> > >  #define log_msg_ret(_msg, _ret) (_ret)
> >
> > It has to be used in a 'return' statement. I'll add a comment in the
> > first patch of the spl handoff series.
> >
>
> Yes, it seems I have noticed this and see my patch here :)
> http://patchwork.ozlabs.org/patch/996894/

OK good. I'm not sure what I was thinking when I omitted the documentation...

Regards,
Simon

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()
  2018-11-09 18:43   ` [U-Boot] [U-Boot,v2,11/23] " Tom Rini
@ 2019-02-11 21:00     ` Simon Goldschmidt
  2019-02-21  2:47       ` Simon Glass
  0 siblings, 1 reply; 48+ messages in thread
From: Simon Goldschmidt @ 2019-02-11 21:00 UTC (permalink / raw)
  To: u-boot

Am 09.11.2018 um 19:43 schrieb Tom Rini:
> On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote:
> 
>> There is a strange feature to set global_data to a data-section variable
>> early in SPL. This only works if SPL actually has access to SRAM which is
>> not the case on x86, for eaxmple. Add a comment to this effect.

Seems like I missed that one back in October.

Does anyone have an idea why this variable ('static bd_t bdata') is 
hard-coded into section ".data"? To me this seems pretty unportable...

For example, for a specific feature on socfpga (warm reboot CRC check), 
I would prefer to have the ".data" section empty and put 'bdata' into 
bss. This is currently not possible.

But before sending a patch that somehow changes this behaviour, I'd like 
to know why this variable is put into ".data" instead of ".bss"

Thanks,
Simon

>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
> 
> Reviewed-by: Tom Rini <trini@konsulko.com>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()
  2019-02-11 21:00     ` Simon Goldschmidt
@ 2019-02-21  2:47       ` Simon Glass
  2019-02-21  5:43         ` Simon Goldschmidt
  0 siblings, 1 reply; 48+ messages in thread
From: Simon Glass @ 2019-02-21  2:47 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Mon, 11 Feb 2019 at 14:00, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> Am 09.11.2018 um 19:43 schrieb Tom Rini:
> > On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote:
> >
> >> There is a strange feature to set global_data to a data-section variable
> >> early in SPL. This only works if SPL actually has access to SRAM which is
> >> not the case on x86, for eaxmple. Add a comment to this effect.
>
> Seems like I missed that one back in October.
>
> Does anyone have an idea why this variable ('static bd_t bdata') is
> hard-coded into section ".data"? To me this seems pretty unportable...
>
> For example, for a specific feature on socfpga (warm reboot CRC check),
> I would prefer to have the ".data" section empty and put 'bdata' into
> bss. This is currently not possible.
>
> But before sending a patch that somehow changes this behaviour, I'd like
> to know why this variable is put into ".data" instead of ".bss"

BSS normally appears after data and is not actually part of the
u-boot-spi.bin image. If the device tree is appended to SPL then it
overlaps with BSS. Thus accessing BSS overwrites the DT. There are two
features to get around that - one is to pad out the BSS space so that:

cat u-boot-spl-nodtb.bin u-boot-spl.dtb >u-boot-spl.bin

places the DT after BSS.

The other is to specify an address (typically in SDRAM) for BSS, so
that it is completely separate from the image.

Note also that BSS is not always available in SPL. For example if SPL
is running from flash then the BSS may be mapped either to SDRAM
(which is not set up until later in SPL) or flash (which is not
writable at all). Of course the data section has the same problem.

Putting gd in the data section avoids one of the above problems.

Others may have more insight here.

Regards,
Simon

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()
  2019-02-21  2:47       ` Simon Glass
@ 2019-02-21  5:43         ` Simon Goldschmidt
  2019-03-10 21:50           ` Simon Glass
  0 siblings, 1 reply; 48+ messages in thread
From: Simon Goldschmidt @ 2019-02-21  5:43 UTC (permalink / raw)
  To: u-boot

Am Do., 21. Feb. 2019, 03:48 hat Simon Glass <sjg@chromium.org> geschrieben:

> Hi Simon,
>
> On Mon, 11 Feb 2019 at 14:00, Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
> >
> > Am 09.11.2018 um 19:43 schrieb Tom Rini:
> > > On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote:
> > >
> > >> There is a strange feature to set global_data to a data-section
> variable
> > >> early in SPL. This only works if SPL actually has access to SRAM
> which is
> > >> not the case on x86, for eaxmple. Add a comment to this effect.
> >
> > Seems like I missed that one back in October.
> >
> > Does anyone have an idea why this variable ('static bd_t bdata') is
> > hard-coded into section ".data"? To me this seems pretty unportable...
> >
> > For example, for a specific feature on socfpga (warm reboot CRC check),
> > I would prefer to have the ".data" section empty and put 'bdata' into
> > bss. This is currently not possible.
> >
> > But before sending a patch that somehow changes this behaviour, I'd like
> > to know why this variable is put into ".data" instead of ".bss"
>
> BSS normally appears after data and is not actually part of the
> u-boot-spi.bin image. If the device tree is appended to SPL then it
> overlaps with BSS. Thus accessing BSS overwrites the DT. There are two
> features to get around that - one is to pad out the BSS space so that:
>
> cat u-boot-spl-nodtb.bin u-boot-spl.dtb >u-boot-spl.bin
>
> places the DT after BSS.
>
> The other is to specify an address (typically in SDRAM) for BSS, so
> that it is completely separate from the image.
>
> Note also that BSS is not always available in SPL. For example if SPL
> is running from flash then the BSS may be mapped either to SDRAM
> (which is not set up until later in SPL) or flash (which is not
> writable at all). Of course the data section has the same problem.
>

Ok, thanks for the insight.

Putting gd in the data section avoids one of the above problems.
>

Actually it's bd here, not gd, so it's a little less widely used.

And while I can follow your explanation, it convinces me even more that
this should be configurable, not hadcoded. It wastes space in SPL binary
and may not even work on some platforms...

Regards,
Simon


> Others may have more insight here.
>
> Regards,
> Simon
>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()
  2019-02-21  5:43         ` Simon Goldschmidt
@ 2019-03-10 21:50           ` Simon Glass
  0 siblings, 0 replies; 48+ messages in thread
From: Simon Glass @ 2019-03-10 21:50 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Wed, 20 Feb 2019 at 22:44, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
>
>
> Am Do., 21. Feb. 2019, 03:48 hat Simon Glass <sjg@chromium.org> geschrieben:
>>
>> Hi Simon,
>>
>> On Mon, 11 Feb 2019 at 14:00, Simon Goldschmidt
>> <simon.k.r.goldschmidt@gmail.com> wrote:
>> >
>> > Am 09.11.2018 um 19:43 schrieb Tom Rini:
>> > > On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote:
>> > >
>> > >> There is a strange feature to set global_data to a data-section variable
>> > >> early in SPL. This only works if SPL actually has access to SRAM which is
>> > >> not the case on x86, for eaxmple. Add a comment to this effect.
>> >
>> > Seems like I missed that one back in October.
>> >
>> > Does anyone have an idea why this variable ('static bd_t bdata') is
>> > hard-coded into section ".data"? To me this seems pretty unportable...
>> >
>> > For example, for a specific feature on socfpga (warm reboot CRC check),
>> > I would prefer to have the ".data" section empty and put 'bdata' into
>> > bss. This is currently not possible.
>> >
>> > But before sending a patch that somehow changes this behaviour, I'd like
>> > to know why this variable is put into ".data" instead of ".bss"
>>
>> BSS normally appears after data and is not actually part of the
>> u-boot-spi.bin image. If the device tree is appended to SPL then it
>> overlaps with BSS. Thus accessing BSS overwrites the DT. There are two
>> features to get around that - one is to pad out the BSS space so that:
>>
>> cat u-boot-spl-nodtb.bin u-boot-spl.dtb >u-boot-spl.bin
>>
>> places the DT after BSS.
>>
>> The other is to specify an address (typically in SDRAM) for BSS, so
>> that it is completely separate from the image.
>>
>> Note also that BSS is not always available in SPL. For example if SPL
>> is running from flash then the BSS may be mapped either to SDRAM
>> (which is not set up until later in SPL) or flash (which is not
>> writable at all). Of course the data section has the same problem.
>
>
> Ok, thanks for the insight.
>
>> Putting gd in the data section avoids one of the above problems.
>
>
> Actually it's bd here, not gd, so it's a little less widely used.
>
> And while I can follow your explanation, it convinces me even more that this should be configurable, not hadcoded. It wastes space in SPL binary and may not even work on some platforms...

Yes my comments were on gd, sorry about that.

Ideally it would default to something safe (i.e. not being in BSS or
data). The best things is probably to malloc() it, assuming that this
is enabled in SPL.

Regards,
Simon

^ permalink raw reply	[flat|nested] 48+ messages in thread

end of thread, other threads:[~2019-03-10 21:50 UTC | newest]

Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-02 11:22 [U-Boot] [PATCH v2 00/23] spl: Add features for passing info from SPL to U-Boot proper Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 01/23] log: Correct definition of log_msg_ret() Simon Glass
2018-10-04  9:25   ` Bin Meng
2018-11-16  0:07     ` Simon Glass
2018-11-16  1:43       ` Bin Meng
2018-11-16  2:44         ` Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 02/23] spl: Add support for logging in SPL and TPL Simon Glass
2018-11-09 18:42   ` [U-Boot] [U-Boot, v2, " Tom Rini
2018-10-02 11:22 ` [U-Boot] [PATCH v2 03/23] Add core support for a bloblist to convey data from SPL Simon Glass
2018-10-08  2:42   ` Kever Yang
2018-10-09  3:40     ` Simon Glass
2018-11-09 18:42   ` [U-Boot] [U-Boot, v2, " Tom Rini
2018-10-02 11:22 ` [U-Boot] [PATCH v2 04/23] spl: Set up the bloblist in SPL Simon Glass
2018-11-09 18:43   ` [U-Boot] [U-Boot,v2,04/23] " Tom Rini
2018-11-16  0:07     ` Simon Glass
2018-11-16  0:10       ` Tom Rini
2018-11-16  0:29         ` Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 05/23] bloblist: Locate bloblist in U-Boot Simon Glass
2018-11-09 18:43   ` [U-Boot] [U-Boot,v2,05/23] " Tom Rini
2018-10-02 11:22 ` [U-Boot] [PATCH v2 06/23] test: Add a simple test for bloblist Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 07/23] Add bloblist documentation Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 08/23] spl: Support hash, input, pch, pci, rtc, tpm in SPL Simon Glass
2018-11-09 18:43   ` [U-Boot] [U-Boot, v2, " Tom Rini
2018-10-02 11:22 ` [U-Boot] [PATCH v2 09/23] spl: Add a define for SPL_TPL_PROMPT Simon Glass
2018-11-09 18:43   ` [U-Boot] [U-Boot, v2, " Tom Rini
2018-10-02 11:22 ` [U-Boot] [PATCH v2 10/23] spl: Make SPL_DISABLE_BANNER_PRINT a positive option Simon Glass
2018-11-09 18:43   ` [U-Boot] [U-Boot, v2, " Tom Rini
2018-10-02 11:22 ` [U-Boot] [PATCH v2 11/23] spl: Add a comment to spl_set_bd() Simon Glass
2018-11-09 18:43   ` [U-Boot] [U-Boot,v2,11/23] " Tom Rini
2019-02-11 21:00     ` Simon Goldschmidt
2019-02-21  2:47       ` Simon Glass
2019-02-21  5:43         ` Simon Goldschmidt
2019-03-10 21:50           ` Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 12/23] spl: Print a message if we are unable to load an image Simon Glass
2018-11-09 18:43   ` [U-Boot] [U-Boot, v2, " Tom Rini
2018-11-16  0:08     ` Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 13/23] sandbox: Add a memory map to the sandbox README Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 14/23] test/py: Add a way to pass flags to sandbox Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 15/23] sandbox: Add an option to display of-platdata in SPL Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 16/23] sandbox: Drop the deprecated 'sb' command Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 17/23] sandbox: Add a new " Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 18/23] sandbox: Allow puts() output before global_data is set up Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 19/23] sandbox: Refactor code to create os_jump_to_file() Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 20/23] sandbox: Use malloc() and free() from os layer Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 21/23] sandbox: Filter arguments when starting U-Boot Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 22/23] sandbox: Boot in U-Boot through the standard call Simon Glass
2018-10-02 11:22 ` [U-Boot] [PATCH v2 23/23] spl: Add support for passing handoff info to U-Boot proper Simon Glass
2018-11-09 18:43   ` [U-Boot] [U-Boot, v2, " Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.