From 68ff6d365539fd0bb13a219bb4c5bb885ee1f30f Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sun, 13 Mar 2022 16:22:48 -0600
Subject: [PATCH 01/34] bloblist: Describe the design goals

Add a comment explaining the design goals of bloblist, to make it easier
for people to understand and comment on the structure.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 doc/develop/bloblist.rst |  2 ++
 include/bloblist.h       | 62 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/doc/develop/bloblist.rst b/doc/develop/bloblist.rst
index 572aa65d764..81643c7674b 100644
--- a/doc/develop/bloblist.rst
+++ b/doc/develop/bloblist.rst
@@ -11,6 +11,8 @@ 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.
 
+For the design goals of bloblist, please see the comments at the top of the
+`bloblist.h` header file.
 
 Passing state through the boot process
 --------------------------------------
diff --git a/include/bloblist.h b/include/bloblist.h
index d0e128acf10..9684bfd5f4b 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -3,8 +3,66 @@
  * 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.
+ * It consists of a list of blobs of data, tagged with their owner / contents.
+ * The list resides in memory and can be updated by SPL, U-Boot, etc.
+ *
+ * Design goals for bloblist:
+ *
+ * 1. Small and efficient structure. This avoids UUIDs or 16-byte name fields,
+ * since a 32-bit tag provides enough space for all the tags we will even need.
+ * If UUIDs are desired, they can be added inside a particular blob.
+ *
+ * 2. Avoids use of pointers, so the structure can be relocated in memory. The
+ * data in each blob is inline, rather than using pointers.
+ *
+ * 3. Bloblist is designed to start small in TPL or SPL, when only a few things
+ * are needed, like the memory size or whether console output should be enabled.
+ * Then it can grow in U-Boot proper, e.g. to include space for ACPI tables.
+ *
+ * 4. The bloblist structure is simple enough that it can be implemented in a
+ * small amount of C code. The API does not require use of strings or UUIDs,
+ * which would add to code size. For Thumb-2 the code size needed in SPL is
+ * approximately 940 bytes (e.g. for chromebook_bob).
+ *
+ * 5. Bloblist uses 16-byte alignment internally and is designed to start on a
+ * 16-byte boundary. Its headers are multiples of 16 bytes. This makes it easier
+ * to deal with data structures which need this level of alignment, such as ACPI
+ * tables. For use in SPL and TPL the alignment can be relaxed, since it can be
+ * relocated to an aligned address in U-Boot proper.
+ *
+ * 6. Bloblist is designed to be passed to Linux as reserved memory. While linux
+ * doesn't understand the bloblist header, it can be passed the indivdual blobs.
+ * For example, ACPI tables can reside in a blob and the address of those is
+ * passed to Linux, without Linux ever being away of the existence of a
+ * bloblist. Having all the blobs contiguous in memory simplifies the
+ * reserved-memory space.
+ *
+ * 7. Bloblist tags are defined in the enum below. There is an area for
+ * project-specific stuff (e.g. U-Boot, TF-A) and vendor-specific stuff, e.g.
+ * something used only on a particular SoC. There is also a private area for
+ * temporary, local use.
+ *
+ * 8. Bloblist includes a simple checksum, so that each boot phase can update
+ * this and allow the next phase to check that all is well. While the bloblist
+ * is small, this is quite cheap to calculate. When it grows (e.g. in U-Boot\
+ * proper), the CPU is likely running faster, so it is not prohibitive. Having
+ * said that, U-Boot is often the last phase that uses bloblist, so calculating
+ * the checksum there may not be necessary.
+ *
+ * 9. It would be possible to extend bloblist to support a non-contiguous
+ * structure, e.g. by creating a blob type that points to the next bloblist.
+ * This does not seem necessary for now. It adds complexity and code. We can
+ * always just copy it.
+ *
+ * 10. Bloblist is designed for simple structures, those that can be defined by
+ * a single C struct. More complex structures should be passed in a device tree.
+ * There are some exceptions, chiefly the various binary structures that Intel
+ * is fond of creating. But device tree provides a dictionary-type format which
+ * is fairly efficient (for use in U-Boot proper and Linux at least), along with
+ * a schema and a good set of tools. New formats should be designed around
+ * device tree rather than creating new binary formats, unless they are needed
+ * early in boot (where libfdt's 3KB of overhead is too large) and are trival
+ * enough to be described by a C struct.
  *
  * Copyright 2018 Google, Inc
  * Written by Simon Glass <sjg@chromium.org>

From 42ae363ddd99c38ab26434b9d1c8b68610844e79 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Fri, 18 Mar 2022 18:01:50 -0600
Subject: [PATCH 02/34] dtoc: Update fdt tests to use test_util

Use the common functions to run tests and report results. Ensure that the
result code indicates success or failure.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---
 tools/dtoc/test_fdt.py | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index 914ed6aed59..3859af8d032 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -780,25 +780,17 @@ def RunTests(args):
     Args:
         args: List of positional args provided to fdt. This can hold a test
             name to execute (as in 'fdt -t testFdt', for example)
+
+    Returns:
+        Return code, 0 on success
     """
     result = unittest.TestResult()
-    sys.argv = [sys.argv[0]]
     test_name = args and args[0] or None
-    for module in (TestFdt, TestNode, TestProp, TestFdtUtil):
-        if test_name:
-            try:
-                suite = unittest.TestLoader().loadTestsFromName(test_name, module)
-            except AttributeError:
-                continue
-        else:
-            suite = unittest.TestLoader().loadTestsFromTestCase(module)
-        suite.run(result)
+    test_util.run_test_suites(
+        result, False, False, False, None, test_name, None,
+        [TestFdt, TestNode, TestProp, TestFdtUtil])
 
-    print(result)
-    for _, err in result.errors:
-        print(err)
-    for _, err in result.failures:
-        print(err)
+    return test_util.report_result('fdt', test_name, result)
 
 if __name__ != '__main__':
     sys.exit(1)
@@ -816,6 +808,7 @@ parser.add_option('-T', '--test-coverage', action='store_true',
 
 # Run our meagre tests
 if options.test:
-    RunTests(args)
+    ret_code = RunTests(args)
+    sys.exit(ret_code)
 elif options.test_coverage:
     RunTestCoverage()

From 24057fe0a8f70ae872da0a8f4889fe7b8cfa09db Mon Sep 17 00:00:00 2001
From: Sean Anderson <seanga2@gmail.com>
Date: Wed, 23 Mar 2022 18:24:38 -0400
Subject: [PATCH 03/34] sandbox: usb: Fix out-of-bounds read when fd=-1

sandbox_flash_bulk uses priv->read_len to determine if priv->buff contains
the response data (such as from SCSI_INQUIRY). However, if priv->fd=-1 in
handle_read, then priv->read_len is not set even though we are going to
PHASE_DATA. This causes sandbox_flash_bulk to try and read len bytes from
priv->buff, which likely goes past the end of the buffer. Fix this by always
setting priv->read_len even if we aren't going to read anything.

Fixes: f4f715360c ("dm: usb: sandbox: Add an emulator for USB flash devices")
Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 drivers/usb/emul/sandbox_flash.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/emul/sandbox_flash.c b/drivers/usb/emul/sandbox_flash.c
index cc80f671337..01b2b41cce9 100644
--- a/drivers/usb/emul/sandbox_flash.c
+++ b/drivers/usb/emul/sandbox_flash.c
@@ -228,9 +228,9 @@ static void handle_read(struct sandbox_flash_priv *priv, ulong lba,
 			ulong transfer_len)
 {
 	debug("%s: lba=%lx, transfer_len=%lx\n", __func__, lba, transfer_len);
+	priv->read_len = transfer_len;
 	if (priv->fd != -1) {
 		os_lseek(priv->fd, lba * SANDBOX_FLASH_BLOCK_LEN, OS_SEEK_SET);
-		priv->read_len = transfer_len;
 		setup_response(priv, priv->buff,
 			       transfer_len * SANDBOX_FLASH_BLOCK_LEN);
 	} else {
@@ -336,6 +336,9 @@ static int sandbox_flash_bulk(struct udevice *dev, struct usb_device *udev,
 			if (priv->read_len) {
 				ulong bytes_read;
 
+				if (priv->fd == -1)
+					return -EIO;
+
 				bytes_read = os_read(priv->fd, buff, len);
 				if (bytes_read != len)
 					return -EIO;

From 8b52f237f0f8ae63c930fa8459c39ab87367bad1 Mon Sep 17 00:00:00 2001
From: Sean Anderson <sean.anderson@seco.com>
Date: Mon, 28 Mar 2022 18:14:37 -0400
Subject: [PATCH 04/34] dm: core: Provide fallbacks for ofnode_conf_read_...

Because fdt_get_config_str et al. were moved/renamed to
ofnode_conf_read_str, they now depend on CONFIG_DM as well as
CONFIG_OF_CONTROL. Add some fallback implementations, preventing a
linker error when CONFIG_SPL_OF_CONTROL and CONFIG_SPL_ENV_IS_IN_MMC are
enabled and CONFIG_SPL_DM is disabled.

Fixes: 7de8bd03c3 ("treewide: fdt: Move fdt_get_config_... to ofnode_conf_read...")
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---
 include/dm/ofnode.h | 66 ++++++++++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 24 deletions(-)

diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 2c4d72d77f5..bb60433124b 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -1181,6 +1181,33 @@ int ofnode_write_string(ofnode node, const char *propname, const char *value);
  */
 int ofnode_set_enabled(ofnode node, bool value);
 
+/**
+ * ofnode_get_phy_node() - Get PHY node for a MAC (if not fixed-link)
+ *
+ * This function parses PHY handle from the Ethernet controller's ofnode
+ * (trying all possible PHY handle property names), and returns the PHY ofnode.
+ *
+ * Before this is used, ofnode_phy_is_fixed_link() should be checked first, and
+ * if the result to that is true, this function should not be called.
+ *
+ * @eth_node:	ofnode belonging to the Ethernet controller
+ * Return: ofnode of the PHY, if it exists, otherwise an invalid ofnode
+ */
+ofnode ofnode_get_phy_node(ofnode eth_node);
+
+/**
+ * ofnode_read_phy_mode() - Read PHY connection type from a MAC node
+ *
+ * This function parses the "phy-mode" / "phy-connection-type" property and
+ * returns the corresponding PHY interface type.
+ *
+ * @mac_node:	ofnode containing the property
+ * Return: one of PHY_INTERFACE_MODE_* constants, PHY_INTERFACE_MODE_NA on
+ *	   error
+ */
+phy_interface_t ofnode_read_phy_mode(ofnode mac_node);
+
+#if CONFIG_IS_ENABLED(DM)
 /**
  * ofnode_conf_read_bool() - Read a boolean value from the U-Boot config
  *
@@ -1218,30 +1245,21 @@ int ofnode_conf_read_int(const char *prop_name, int default_val);
  */
 const char *ofnode_conf_read_str(const char *prop_name);
 
-/**
- * ofnode_get_phy_node() - Get PHY node for a MAC (if not fixed-link)
- *
- * This function parses PHY handle from the Ethernet controller's ofnode
- * (trying all possible PHY handle property names), and returns the PHY ofnode.
- *
- * Before this is used, ofnode_phy_is_fixed_link() should be checked first, and
- * if the result to that is true, this function should not be called.
- *
- * @eth_node:	ofnode belonging to the Ethernet controller
- * Return: ofnode of the PHY, if it exists, otherwise an invalid ofnode
- */
-ofnode ofnode_get_phy_node(ofnode eth_node);
+#else /* CONFIG_DM */
+static inline bool ofnode_conf_read_bool(const char *prop_name)
+{
+	return false;
+}
 
-/**
- * ofnode_read_phy_mode() - Read PHY connection type from a MAC node
- *
- * This function parses the "phy-mode" / "phy-connection-type" property and
- * returns the corresponding PHY interface type.
- *
- * @mac_node:	ofnode containing the property
- * Return: one of PHY_INTERFACE_MODE_* constants, PHY_INTERFACE_MODE_NA on
- *	   error
- */
-phy_interface_t ofnode_read_phy_mode(ofnode mac_node);
+static inline int ofnode_conf_read_int(const char *prop_name, int default_val)
+{
+	return default_val;
+}
+
+static inline const char *ofnode_conf_read_str(const char *prop_name)
+{
+	return NULL;
+}
+#endif /* CONFIG_DM */
 
 #endif

From 501a9a7ed803ac948adb392e541f9da9bafad60e Mon Sep 17 00:00:00 2001
From: Sean Anderson <seanga2@gmail.com>
Date: Wed, 30 Mar 2022 12:26:40 -0400
Subject: [PATCH 05/34] dm: core: Use device_foreach_child where possible

We have some nice macros for iterating over devices in device.h, but they
are not used by the driver core. Convert all the users I could find.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
 drivers/core/device-remove.c |  4 ++--
 drivers/core/device.c        | 21 ++++++++++-----------
 drivers/core/devres.c        |  2 +-
 drivers/core/dump.c          |  2 +-
 4 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index 73d2e9e4208..a86b9325dd8 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -29,7 +29,7 @@ int device_chld_unbind(struct udevice *dev, struct driver *drv)
 
 	assert(dev);
 
-	list_for_each_entry_safe(pos, n, &dev->child_head, sibling_node) {
+	device_foreach_child_safe(pos, n, dev) {
 		if (drv && (pos->driver != drv))
 			continue;
 
@@ -52,7 +52,7 @@ int device_chld_remove(struct udevice *dev, struct driver *drv,
 
 	assert(dev);
 
-	list_for_each_entry_safe(pos, n, &dev->child_head, sibling_node) {
+	device_foreach_child_safe(pos, n, dev) {
 		int ret;
 
 		if (drv && (pos->driver != drv))
diff --git a/drivers/core/device.c b/drivers/core/device.c
index 3d7fbfe0736..7f71570a940 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -284,8 +284,7 @@ int device_reparent(struct udevice *dev, struct udevice *new_parent)
 	assert(dev);
 	assert(new_parent);
 
-	list_for_each_entry_safe(pos, n, &dev->parent->child_head,
-				 sibling_node) {
+	device_foreach_child_safe(pos, n, dev->parent) {
 		if (pos->driver != dev->driver)
 			continue;
 
@@ -724,7 +723,7 @@ int device_get_child(const struct udevice *parent, int index,
 {
 	struct udevice *dev;
 
-	list_for_each_entry(dev, &parent->child_head, sibling_node) {
+	device_foreach_child(dev, parent) {
 		if (!index--)
 			return device_get_device_tail(dev, 0, devp);
 	}
@@ -737,7 +736,7 @@ int device_get_child_count(const struct udevice *parent)
 	struct udevice *dev;
 	int count = 0;
 
-	list_for_each_entry(dev, &parent->child_head, sibling_node)
+	device_foreach_child(dev, parent)
 		count++;
 
 	return count;
@@ -748,7 +747,7 @@ int device_get_decendent_count(const struct udevice *parent)
 	const struct udevice *dev;
 	int count = 1;
 
-	list_for_each_entry(dev, &parent->child_head, sibling_node)
+	device_foreach_child(dev, parent)
 		count += device_get_decendent_count(dev);
 
 	return count;
@@ -761,7 +760,7 @@ int device_find_child_by_seq(const struct udevice *parent, int seq,
 
 	*devp = NULL;
 
-	list_for_each_entry(dev, &parent->child_head, sibling_node) {
+	device_foreach_child(dev, parent) {
 		if (dev->seq_ == seq) {
 			*devp = dev;
 			return 0;
@@ -790,7 +789,7 @@ int device_find_child_by_of_offset(const struct udevice *parent, int of_offset,
 
 	*devp = NULL;
 
-	list_for_each_entry(dev, &parent->child_head, sibling_node) {
+	device_foreach_child(dev, parent) {
 		if (dev_of_offset(dev) == of_offset) {
 			*devp = dev;
 			return 0;
@@ -819,7 +818,7 @@ static struct udevice *_device_find_global_by_ofnode(struct udevice *parent,
 	if (ofnode_equal(dev_ofnode(parent), ofnode))
 		return parent;
 
-	list_for_each_entry(dev, &parent->child_head, sibling_node) {
+	device_foreach_child(dev, parent) {
 		found = _device_find_global_by_ofnode(dev, ofnode);
 		if (found)
 			return found;
@@ -897,7 +896,7 @@ int device_find_first_inactive_child(const struct udevice *parent,
 	struct udevice *dev;
 
 	*devp = NULL;
-	list_for_each_entry(dev, &parent->child_head, sibling_node) {
+	device_foreach_child(dev, parent) {
 		if (!device_active(dev) &&
 		    device_get_uclass_id(dev) == uclass_id) {
 			*devp = dev;
@@ -915,7 +914,7 @@ int device_find_first_child_by_uclass(const struct udevice *parent,
 	struct udevice *dev;
 
 	*devp = NULL;
-	list_for_each_entry(dev, &parent->child_head, sibling_node) {
+	device_foreach_child(dev, parent) {
 		if (device_get_uclass_id(dev) == uclass_id) {
 			*devp = dev;
 			return 0;
@@ -932,7 +931,7 @@ int device_find_child_by_namelen(const struct udevice *parent, const char *name,
 
 	*devp = NULL;
 
-	list_for_each_entry(dev, &parent->child_head, sibling_node) {
+	device_foreach_child(dev, parent) {
 		if (!strncmp(dev->name, name, len) &&
 		    strlen(dev->name) == len) {
 			*devp = dev;
diff --git a/drivers/core/devres.c b/drivers/core/devres.c
index 313ddc7089c..78914bdf7f2 100644
--- a/drivers/core/devres.c
+++ b/drivers/core/devres.c
@@ -232,7 +232,7 @@ static void dump_resources(struct udevice *dev, int depth)
 		       (unsigned long)dr->size, dr->name,
 		       devres_phase_name[dr->phase]);
 
-	list_for_each_entry(child, &dev->child_head, sibling_node)
+	device_foreach_child(child, dev)
 		dump_resources(child, depth + 1);
 }
 
diff --git a/drivers/core/dump.c b/drivers/core/dump.c
index f2f9cacc56c..fe97dca954d 100644
--- a/drivers/core/dump.c
+++ b/drivers/core/dump.c
@@ -39,7 +39,7 @@ static void show_devices(struct udevice *dev, int depth, int last_flag)
 
 	printf("%s\n", dev->name);
 
-	list_for_each_entry(child, &dev->child_head, sibling_node) {
+	device_foreach_child(child, dev) {
 		is_last = list_is_last(&child->sibling_node, &dev->child_head);
 		show_devices(child, depth + 1, (last_flag << 1) | is_last);
 	}

From 6474aaa1d1de0fe246707ff05816a32776d48fa8 Mon Sep 17 00:00:00 2001
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Date: Sat, 2 Apr 2022 20:06:04 +0300
Subject: [PATCH 06/34] patman: test_util: Fix printing results for failed
 tests

When printing a python tool's test results, the entire list of failed
tests and their tracebacks are reprinted for every failed test. This
makes the test output quite unreadable. Fix the loop to print failures
and tracebacks one at a time.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 tools/patman/test_util.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index c60eb3628e2..8b2220dbbaf 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -127,7 +127,7 @@ def report_result(toolname:str, test_name: str, result: unittest.TestResult):
     for test, err in result.errors:
         print(test.id(), err)
     for test, err in result.failures:
-        print(err, result.failures)
+        print(test.id(), err)
     if result.skipped:
         print('%d %s test%s SKIPPED:' % (len(result.skipped), toolname,
             's' if len(result.skipped) > 1 else ''))

From ce12c47b92152e9457d3daa3ddbf53c1cc3de0bb Mon Sep 17 00:00:00 2001
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Date: Sat, 2 Apr 2022 20:06:05 +0300
Subject: [PATCH 07/34] patman: test_util: Handle nonexistent tests while
 loading tests

It's possible to request a specific test to run when trying to run a
python tool's tests. If we request a nonexistent test, the unittest
loaders generate a fake test that reports this as an error. However, we
get these fake tests even when the test exists, because test_util can
load tests from multiple places one by one and the test we want only
exists in one.

The test_util helpers currently remove these fake tests when printing
test results, but that's more of a workaround than a proper solution.
Instead, don't even try to load the missing tests.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 tools/patman/test_util.py | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index 8b2220dbbaf..a4c2a2c3c0b 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -110,19 +110,6 @@ def report_result(toolname:str, test_name: str, result: unittest.TestResult):
         test_name: Name of test that was run, or None for all
         result: A unittest.TestResult object containing the results
     """
-    # Remove errors which just indicate a missing test. Since Python v3.5 If an
-    # ImportError or AttributeError occurs while traversing name then a
-    # synthetic test that raises that error when run will be returned. These
-    # errors are included in the errors accumulated by result.errors.
-    if test_name:
-        errors = []
-
-        for test, err in result.errors:
-            if ("has no attribute '%s'" % test_name) not in err:
-                errors.append((test, err))
-            result.testsRun -= 1
-        result.errors = errors
-
     print(result)
     for test, err in result.errors:
         print(test.id(), err)
@@ -184,10 +171,12 @@ def run_test_suites(result, debug, verbosity, test_preserve_dirs, processes,
                 preserve_outdirs=test_preserve_dirs and test_name is not None,
                 toolpath=toolpath, verbosity=verbosity)
         if test_name:
-            try:
+            # Since Python v3.5 If an ImportError or AttributeError occurs
+            # while traversing a name then a synthetic test that raises that
+            # error when run will be returned. Check that the requested test
+            # exists, otherwise these errors are included in the results.
+            if test_name in loader.getTestCaseNames(module):
                 suite.addTests(loader.loadTestsFromName(test_name, module))
-            except AttributeError:
-                continue
         else:
             suite.addTests(loader.loadTestsFromTestCase(module))
     if use_concurrent and processes != 1:

From d8318feba1ef3b2a74495ea7dca33ad1276a4ffe Mon Sep 17 00:00:00 2001
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Date: Sat, 2 Apr 2022 20:06:06 +0300
Subject: [PATCH 08/34] patman: test_util: Use unittest text runner to print
 test results

The python tools' test utilities handle printing test results, but the
output is quite bare compared to an ordinary unittest run. Delegate
printing the results to a unittest text runner, which gives us niceties
like clear separation between each test's result and how long it took to
run the test suite.

Unfortunately it does not print info for skipped tests by default, but
this can be handled later by a custom test result subclass. It also does
not print the tool name; manually print a heading that includes the
toolname so that the outputs of each tool's tests are distinguishable in
the CI output.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/main.py      |  8 ++----
 tools/buildman/main.py    |  8 ++----
 tools/dtoc/main.py        |  9 +++---
 tools/dtoc/test_fdt.py    |  8 +++---
 tools/patman/main.py      |  8 ++----
 tools/patman/test_util.py | 58 ++++++++++++++-------------------------
 6 files changed, 38 insertions(+), 61 deletions(-)

diff --git a/tools/binman/main.py b/tools/binman/main.py
index 5fb9404ef6a..14432a8d0dc 100755
--- a/tools/binman/main.py
+++ b/tools/binman/main.py
@@ -13,7 +13,6 @@ import os
 import site
 import sys
 import traceback
-import unittest
 
 # Get the absolute path to this file at run-time
 our_path = os.path.dirname(os.path.realpath(__file__))
@@ -73,19 +72,18 @@ def RunTests(debug, verbosity, processes, test_preserve_dirs, args, toolpath):
     from binman import image_test
     import doctest
 
-    result = unittest.TestResult()
     test_name = args and args[0] or None
 
     # Run the entry tests first ,since these need to be the first to import the
     # 'entry' module.
-    test_util.run_test_suites(
-        result, debug, verbosity, test_preserve_dirs, processes, test_name,
+    result = test_util.run_test_suites(
+        'binman', debug, verbosity, test_preserve_dirs, processes, test_name,
         toolpath,
         [bintool_test.TestBintool, entry_test.TestEntry, ftest.TestFunctional,
          fdt_test.TestFdt, elf_test.TestElf, image_test.TestImage,
          cbfs_util_test.TestCbfs, fip_util_test.TestFip])
 
-    return test_util.report_result('binman', test_name, result)
+    return (0 if result.wasSuccessful() else 1)
 
 def RunTestCoverage(toolpath):
     """Run the tests and check that we get 100% coverage"""
diff --git a/tools/buildman/main.py b/tools/buildman/main.py
index 3b6af240802..67c560c48d3 100755
--- a/tools/buildman/main.py
+++ b/tools/buildman/main.py
@@ -11,7 +11,6 @@ import multiprocessing
 import os
 import re
 import sys
-import unittest
 
 # Bring in the patman libraries
 our_path = os.path.dirname(os.path.realpath(__file__))
@@ -34,19 +33,18 @@ def RunTests(skip_net_tests, verboose, args):
     from buildman import test
     import doctest
 
-    result = unittest.TestResult()
     test_name = args and args[0] or None
     if skip_net_tests:
         test.use_network = False
 
     # Run the entry tests first ,since these need to be the first to import the
     # 'entry' module.
-    test_util.run_test_suites(
-        result, False, verboose, False, None, test_name, [],
+    result = test_util.run_test_suites(
+        'buildman', False, verboose, False, None, test_name, [],
         [test.TestBuild, func_test.TestFunctional,
          'buildman.toolchain', 'patman.gitutil'])
 
-    return test_util.report_result('buildman', test_name, result)
+    return (0 if result.wasSuccessful() else 1)
 
 options, args = cmdline.ParseArgs()
 
diff --git a/tools/dtoc/main.py b/tools/dtoc/main.py
index fac9db9c786..5508759d4d5 100755
--- a/tools/dtoc/main.py
+++ b/tools/dtoc/main.py
@@ -24,7 +24,6 @@ see doc/driver-model/of-plat.rst
 from argparse import ArgumentParser
 import os
 import sys
-import unittest
 
 # Bring in the patman libraries
 our_path = os.path.dirname(os.path.realpath(__file__))
@@ -49,18 +48,18 @@ def run_tests(processes, args):
     from dtoc import test_src_scan
     from dtoc import test_dtoc
 
-    result = unittest.TestResult()
     sys.argv = [sys.argv[0]]
     test_name = args.files and args.files[0] or None
 
     test_dtoc.setup()
 
-    test_util.run_test_suites(
-        result, debug=True, verbosity=1, test_preserve_dirs=False,
+    result = test_util.run_test_suites(
+        toolname='dtoc', debug=True, verbosity=1, test_preserve_dirs=False,
         processes=processes, test_name=test_name, toolpath=[],
         class_and_module_list=[test_dtoc.TestDtoc,test_src_scan.TestSrcScan])
 
-    return test_util.report_result('binman', test_name, result)
+    return (0 if result.wasSuccessful() else 1)
+
 
 def RunTestCoverage():
     """Run the tests and check that we get 100% coverage"""
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index 3859af8d032..3baf4437cdd 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -784,13 +784,13 @@ def RunTests(args):
     Returns:
         Return code, 0 on success
     """
-    result = unittest.TestResult()
     test_name = args and args[0] or None
-    test_util.run_test_suites(
-        result, False, False, False, None, test_name, None,
+    result = test_util.run_test_suites(
+        'test_fdt', False, False, False, None, test_name, None,
         [TestFdt, TestNode, TestProp, TestFdtUtil])
 
-    return test_util.report_result('fdt', test_name, result)
+    return (0 if result.wasSuccessful() else 1)
+
 
 if __name__ != '__main__':
     sys.exit(1)
diff --git a/tools/patman/main.py b/tools/patman/main.py
index 2a2ac457093..66d4806c8d8 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -12,7 +12,6 @@ import re
 import shutil
 import sys
 import traceback
-import unittest
 
 if __name__ == "__main__":
     # Allow 'from patman import xxx to work'
@@ -134,13 +133,12 @@ if args.cmd == 'test':
     import doctest
     from patman import func_test
 
-    result = unittest.TestResult()
-    test_util.run_test_suites(
-        result, False, False, False, None, None, None,
+    result = test_util.run_test_suites(
+        'patman', False, False, False, None, None, None,
         [test_checkpatch.TestPatch, func_test.TestFunctional,
          'gitutil', 'settings', 'terminal'])
 
-    sys.exit(test_util.report_result('patman', args.testname, result))
+    sys.exit(0 if result.wasSuccessful() else 1)
 
 # Process commits, produce patches files, check them, email them
 elif args.cmd == 'send':
diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index a4c2a2c3c0b..ba8f87f75f0 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -102,36 +102,12 @@ def capture_sys_output():
         sys.stdout, sys.stderr = old_out, old_err
 
 
-def report_result(toolname:str, test_name: str, result: unittest.TestResult):
-    """Report the results from a suite of tests
-
-    Args:
-        toolname: Name of the tool that ran the tests
-        test_name: Name of test that was run, or None for all
-        result: A unittest.TestResult object containing the results
-    """
-    print(result)
-    for test, err in result.errors:
-        print(test.id(), err)
-    for test, err in result.failures:
-        print(test.id(), err)
-    if result.skipped:
-        print('%d %s test%s SKIPPED:' % (len(result.skipped), toolname,
-            's' if len(result.skipped) > 1 else ''))
-        for skip_info in result.skipped:
-            print('%s: %s' % (skip_info[0], skip_info[1]))
-    if result.errors or result.failures:
-        print('%s tests FAILED' % toolname)
-        return 1
-    return 0
-
-
-def run_test_suites(result, debug, verbosity, test_preserve_dirs, processes,
+def run_test_suites(toolname, debug, verbosity, test_preserve_dirs, processes,
                     test_name, toolpath, class_and_module_list):
     """Run a series of test suites and collect the results
 
     Args:
-        result: A unittest.TestResult object to add the results to
+        toolname: Name of the tool that ran the tests
         debug: True to enable debugging, which shows a full stack trace on error
         verbosity: Verbosity level to use (0-4)
         test_preserve_dirs: True to preserve the input directory used by tests
@@ -145,11 +121,6 @@ def run_test_suites(result, debug, verbosity, test_preserve_dirs, processes,
         class_and_module_list: List of test classes (type class) and module
            names (type str) to run
     """
-    for module in class_and_module_list:
-        if isinstance(module, str) and (not test_name or test_name == module):
-            suite = doctest.DocTestSuite(module)
-            suite.run(result)
-
     sys.argv = [sys.argv[0]]
     if debug:
         sys.argv.append('-D')
@@ -161,6 +132,19 @@ def run_test_suites(result, debug, verbosity, test_preserve_dirs, processes,
 
     suite = unittest.TestSuite()
     loader = unittest.TestLoader()
+    runner = unittest.TextTestRunner(
+        stream=sys.stdout,
+        verbosity=(1 if verbosity is None else verbosity),
+    )
+
+    if use_concurrent and processes != 1:
+        suite = ConcurrentTestSuite(suite,
+                fork_for_tests(processes or multiprocessing.cpu_count()))
+
+    for module in class_and_module_list:
+        if isinstance(module, str) and (not test_name or test_name == module):
+            suite.addTests(doctest.DocTestSuite(module))
+
     for module in class_and_module_list:
         if isinstance(module, str):
             continue
@@ -179,9 +163,9 @@ def run_test_suites(result, debug, verbosity, test_preserve_dirs, processes,
                 suite.addTests(loader.loadTestsFromName(test_name, module))
         else:
             suite.addTests(loader.loadTestsFromTestCase(module))
-    if use_concurrent and processes != 1:
-        concurrent_suite = ConcurrentTestSuite(suite,
-                fork_for_tests(processes or multiprocessing.cpu_count()))
-        concurrent_suite.run(result)
-    else:
-        suite.run(result)
+
+    print(f" Running {toolname} tests ".center(70, "="))
+    result = runner.run(suite)
+    print()
+
+    return result

From dd6b92b0b9532bb4ba0ad8ac3620b1f3b81adf5b Mon Sep 17 00:00:00 2001
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Date: Sat, 2 Apr 2022 20:06:07 +0300
Subject: [PATCH 09/34] patman: test_util: Customize unittest test results for
 more info

By default, unittest test summaries only print extended info about tests
that failed or couldn't run due to an error. Use a custom text result
class to print info about more cases: skipped tests, expected failures
and unexpected successes.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---
 tools/patman/test_util.py | 46 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index ba8f87f75f0..130d9140914 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -102,6 +102,51 @@ def capture_sys_output():
         sys.stdout, sys.stderr = old_out, old_err
 
 
+class FullTextTestResult(unittest.TextTestResult):
+    """A test result class that can print extended text results to a stream
+
+    This is meant to be used by a TestRunner as a result class. Like
+    TextTestResult, this prints out the names of tests as they are run,
+    errors as they occur, and a summary of the results at the end of the
+    test run. Beyond those, this prints information about skipped tests,
+    expected failures and unexpected successes.
+
+    Args:
+        stream: A file-like object to write results to
+        descriptions (bool): True to print descriptions with test names
+        verbosity (int): Detail of printed output per test as they run
+            Test stdout and stderr always get printed when buffering
+            them is disabled by the test runner. In addition to that,
+            0: Print nothing
+            1: Print a dot per test
+            2: Print test names
+    """
+    def __init__(self, stream, descriptions, verbosity):
+        self.verbosity = verbosity
+        super().__init__(stream, descriptions, verbosity)
+
+    def printErrors(self):
+        "Called by TestRunner after test run to summarize the tests"
+        # The parent class doesn't keep unexpected successes in the same
+        # format as the rest. Adapt it to what printErrorList expects.
+        unexpected_successes = [
+            (test, 'Test was expected to fail, but succeeded.\n')
+            for test in self.unexpectedSuccesses
+        ]
+
+        super().printErrors()  # FAIL and ERROR
+        self.printErrorList('SKIP', self.skipped)
+        self.printErrorList('XFAIL', self.expectedFailures)
+        self.printErrorList('XPASS', unexpected_successes)
+
+    def addSkip(self, test, reason):
+        """Called when a test is skipped."""
+        # Add empty line to keep spacing consistent with other results
+        if not reason.endswith('\n'):
+            reason += '\n'
+        super().addSkip(test, reason)
+
+
 def run_test_suites(toolname, debug, verbosity, test_preserve_dirs, processes,
                     test_name, toolpath, class_and_module_list):
     """Run a series of test suites and collect the results
@@ -135,6 +180,7 @@ def run_test_suites(toolname, debug, verbosity, test_preserve_dirs, processes,
     runner = unittest.TextTestRunner(
         stream=sys.stdout,
         verbosity=(1 if verbosity is None else verbosity),
+        resultclass=FullTextTestResult,
     )
 
     if use_concurrent and processes != 1:

From ebcaafcded40da8ae6cb4234c2ba9901c7bee644 Mon Sep 17 00:00:00 2001
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Date: Sat, 2 Apr 2022 20:06:08 +0300
Subject: [PATCH 10/34] patman: test_util: Print test stdout/stderr within test
 summaries

While running tests for a python tool, the tests' outputs get printed in
whatever order they happen to run, without any indication as to which
output belongs to which test. Unittest supports capturing these outputs
and printing them as part of the test summaries, but when a failure or
error occurs it switches back to printing as the tests run. Testtools
and subunit tests can do the same as their parts inherit from unittest,
but they don't outright expose this functionality.

On the unittest side, enable output buffering for the custom test result
class. Try to avoid ugly outputs by not printing stdout/stderr before
the test summary for low verbosity levels and for successful tests.

On the subunit side, implement a custom TestProtocolClient that enables
the same underlying functionality and injects the captured streams as
additional test details. This causes them to be merged into their test's
error traceback message, which is later rebuilt into an exception and
passed to our unittest report class.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 tools/concurrencytest/concurrencytest.py | 83 +++++++++++++++++++++++-
 tools/patman/test_util.py                | 33 +++++++++-
 2 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/tools/concurrencytest/concurrencytest.py b/tools/concurrencytest/concurrencytest.py
index 5e88b94f415..1c4f03f37e5 100644
--- a/tools/concurrencytest/concurrencytest.py
+++ b/tools/concurrencytest/concurrencytest.py
@@ -31,6 +31,7 @@ from subunit import ProtocolTestCase, TestProtocolClient
 from subunit.test_results import AutoTimingTestResultDecorator
 
 from testtools import ConcurrentTestSuite, iterate_tests
+from testtools.content import TracebackContent, text_content
 
 
 _all__ = [
@@ -43,11 +44,81 @@ _all__ = [
 CPU_COUNT = cpu_count()
 
 
-def fork_for_tests(concurrency_num=CPU_COUNT):
+class BufferingTestProtocolClient(TestProtocolClient):
+    """A TestProtocolClient which can buffer the test outputs
+
+    This class captures the stdout and stderr output streams of the
+    tests as it runs them, and includes the output texts in the subunit
+    stream as additional details.
+
+    Args:
+        stream: A file-like object to write a subunit stream to
+        buffer (bool): True to capture test stdout/stderr outputs and
+            include them in the test details
+    """
+    def __init__(self, stream, buffer=True):
+        super().__init__(stream)
+        self.buffer = buffer
+
+    def _addOutcome(self, outcome, test, error=None, details=None,
+            error_permitted=True):
+        """Report a test outcome to the subunit stream
+
+        The parent class uses this function as a common implementation
+        for various methods that report successes, errors, failures, etc.
+
+        This version automatically upgrades the error tracebacks to the
+        new 'details' format by wrapping them in a Content object, so
+        that we can include the captured test output in the test result
+        details.
+
+        Args:
+            outcome: A string describing the outcome - used as the
+                event name in the subunit stream.
+            test: The test case whose outcome is to be reported
+            error: Standard unittest positional argument form - an
+                exc_info tuple.
+            details: New Testing-in-python drafted API; a dict from
+                string to subunit.Content objects.
+            error_permitted: If True then one and only one of error or
+                details must be supplied. If False then error must not
+                be supplied and details is still optional.
+        """
+        if details is None:
+            details = {}
+
+        # Parent will raise an exception if error_permitted is False but
+        # error is not None. We want that exception in that case, so
+        # don't touch error when error_permitted is explicitly False.
+        if error_permitted and error is not None:
+            # Parent class prefers error over details
+            details['traceback'] = TracebackContent(error, test)
+            error_permitted = False
+            error = None
+
+        if self.buffer:
+            stdout = sys.stdout.getvalue()
+            if stdout:
+                details['stdout'] = text_content(stdout)
+
+            stderr = sys.stderr.getvalue()
+            if stderr:
+                details['stderr'] = text_content(stderr)
+
+        return super()._addOutcome(outcome, test, error=error,
+                details=details, error_permitted=error_permitted)
+
+
+def fork_for_tests(concurrency_num=CPU_COUNT, buffer=False):
     """Implementation of `make_tests` used to construct `ConcurrentTestSuite`.
 
     :param concurrency_num: number of processes to use.
     """
+    if buffer:
+        test_protocol_client_class = BufferingTestProtocolClient
+    else:
+        test_protocol_client_class = TestProtocolClient
+
     def do_fork(suite):
         """Take suite and start up multiple runners by forking (Unix only).
 
@@ -76,7 +147,7 @@ def fork_for_tests(concurrency_num=CPU_COUNT):
                     # child actually gets keystrokes for pdb etc).
                     sys.stdin.close()
                     subunit_result = AutoTimingTestResultDecorator(
-                        TestProtocolClient(stream)
+                        test_protocol_client_class(stream)
                     )
                     process_suite.run(subunit_result)
                 except:
@@ -93,7 +164,13 @@ def fork_for_tests(concurrency_num=CPU_COUNT):
             else:
                 os.close(c2pwrite)
                 stream = os.fdopen(c2pread, 'rb')
-                test = ProtocolTestCase(stream)
+                # If we don't pass the second argument here, it defaults
+                # to sys.stdout.buffer down the line. But if we don't
+                # pass it *now*, it may be resolved after sys.stdout is
+                # replaced with a StringIO (to capture tests' outputs)
+                # which doesn't have a buffer attribute and can end up
+                # occasionally causing a 'broken-runner' error.
+                test = ProtocolTestCase(stream, sys.stdout.buffer)
                 result.append(test)
         return result
     return do_fork
diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index 130d9140914..c27e0b39e5f 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -15,6 +15,7 @@ from patman import command
 
 from io import StringIO
 
+buffer_outputs = True
 use_concurrent = True
 try:
     from concurrencytest.concurrencytest import ConcurrentTestSuite
@@ -120,6 +121,7 @@ class FullTextTestResult(unittest.TextTestResult):
             0: Print nothing
             1: Print a dot per test
             2: Print test names
+            3: Print test names, and buffered outputs for failing tests
     """
     def __init__(self, stream, descriptions, verbosity):
         self.verbosity = verbosity
@@ -139,12 +141,39 @@ class FullTextTestResult(unittest.TextTestResult):
         self.printErrorList('XFAIL', self.expectedFailures)
         self.printErrorList('XPASS', unexpected_successes)
 
+    def addError(self, test, err):
+        """Called when an error has occurred."""
+        super().addError(test, err)
+        self._mirrorOutput &= self.verbosity >= 3
+
+    def addFailure(self, test, err):
+        """Called when a test has failed."""
+        super().addFailure(test, err)
+        self._mirrorOutput &= self.verbosity >= 3
+
+    def addSubTest(self, test, subtest, err):
+        """Called at the end of a subtest."""
+        super().addSubTest(test, subtest, err)
+        self._mirrorOutput &= self.verbosity >= 3
+
+    def addSuccess(self, test):
+        """Called when a test has completed successfully"""
+        super().addSuccess(test)
+        # Don't print stdout/stderr for successful tests
+        self._mirrorOutput = False
+
     def addSkip(self, test, reason):
         """Called when a test is skipped."""
         # Add empty line to keep spacing consistent with other results
         if not reason.endswith('\n'):
             reason += '\n'
         super().addSkip(test, reason)
+        self._mirrorOutput &= self.verbosity >= 3
+
+    def addExpectedFailure(self, test, err):
+        """Called when an expected failure/error occurred."""
+        super().addExpectedFailure(test, err)
+        self._mirrorOutput &= self.verbosity >= 3
 
 
 def run_test_suites(toolname, debug, verbosity, test_preserve_dirs, processes,
@@ -180,12 +209,14 @@ def run_test_suites(toolname, debug, verbosity, test_preserve_dirs, processes,
     runner = unittest.TextTestRunner(
         stream=sys.stdout,
         verbosity=(1 if verbosity is None else verbosity),
+        buffer=buffer_outputs,
         resultclass=FullTextTestResult,
     )
 
     if use_concurrent and processes != 1:
         suite = ConcurrentTestSuite(suite,
-                fork_for_tests(processes or multiprocessing.cpu_count()))
+                fork_for_tests(processes or multiprocessing.cpu_count(),
+                               buffer=buffer_outputs))
 
     for module in class_and_module_list:
         if isinstance(module, str) and (not test_name or test_name == module):

From 7750ee45a62c7834f170f817232e432b8d2a14d3 Mon Sep 17 00:00:00 2001
From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Date: Mon, 4 Apr 2022 22:45:03 +0200
Subject: [PATCH 11/34] sandbox: add function os_printf()

Before setting up the devices U-Boot's printf() function cannot be used
for console output. Provide function os_printf() to print to stderr.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 arch/sandbox/cpu/os.c | 13 +++++++++++++
 include/os.h          |  7 +++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index 3b230606a97..f937991139c 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -12,6 +12,7 @@
 #include <getopt.h>
 #include <setjmp.h>
 #include <signal.h>
+#include <stdarg.h>
 #include <stdio.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -54,6 +55,18 @@ ssize_t os_write(int fd, const void *buf, size_t count)
 	return write(fd, buf, count);
 }
 
+int os_printf(const char *fmt, ...)
+{
+	va_list args;
+	int i;
+
+	va_start(args, fmt);
+	i = vfprintf(stdout, fmt, args);
+	va_end(args);
+
+	return i;
+}
+
 off_t os_lseek(int fd, off_t offset, int whence)
 {
 	if (whence == OS_SEEK_SET)
diff --git a/include/os.h b/include/os.h
index 10e198cf503..148178787bc 100644
--- a/include/os.h
+++ b/include/os.h
@@ -16,6 +16,13 @@
 struct rtc_time;
 struct sandbox_state;
 
+/**
+ * os_printf() - print directly to OS console
+ *
+ * @format: format string
+ */
+int os_printf(const char *format, ...);
+
 /**
  * Access to the OS read() system call
  *

From 66995164ddbedd3449673052a10fc35917bf3d78 Mon Sep 17 00:00:00 2001
From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Date: Mon, 4 Apr 2022 22:45:04 +0200
Subject: [PATCH 12/34] sandbox: show error if the device-tree cannot be loaded

U-Boot's printf() used before setting up U-Boot's serial driver does not
create any output. Use os_printf() for error messages related to loading
the device-tree.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 arch/sandbox/cpu/cpu.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
index 7a82798c36d..d077948dd7b 100644
--- a/arch/sandbox/cpu/cpu.c
+++ b/arch/sandbox/cpu/cpu.c
@@ -331,27 +331,27 @@ void *board_fdt_blob_setup(int *ret)
 		err = setup_auto_tree(blob);
 		if (!err)
 			goto done;
-		printf("Unable to create empty FDT: %s\n", fdt_strerror(err));
+		os_printf("Unable to create empty FDT: %s\n", fdt_strerror(err));
 		*ret = -EINVAL;
 		goto fail;
 	}
 
 	err = os_get_filesize(fname, &size);
 	if (err < 0) {
-		printf("Failed to find FDT file '%s'\n", fname);
+		os_printf("Failed to find FDT file '%s'\n", fname);
 		*ret = err;
 		goto fail;
 	}
 	fd = os_open(fname, OS_O_RDONLY);
 	if (fd < 0) {
-		printf("Failed to open FDT file '%s'\n", fname);
+		os_printf("Failed to open FDT file '%s'\n", fname);
 		*ret = -EACCES;
 		goto fail;
 	}
 
 	if (os_read(fd, blob, size) != size) {
 		os_close(fd);
-		printf("Failed to read FDT file '%s'\n", fname);
+		os_printf("Failed to read FDT file '%s'\n", fname);
 		*ret =  -EIO;
 		goto fail;
 	}

From d3eb1bf7cf242440c966ff20114abbe7f94c4a46 Mon Sep 17 00:00:00 2001
From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Date: Tue, 19 Apr 2022 18:46:36 +0200
Subject: [PATCH 13/34] dm: fix formatting of uclass dump

Insert an empty line after each uclass independent of whether it has
devices or not.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 drivers/core/dump.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/core/dump.c b/drivers/core/dump.c
index fe97dca954d..21d9e7a91f7 100644
--- a/drivers/core/dump.c
+++ b/drivers/core/dump.c
@@ -89,8 +89,6 @@ void dm_dump_uclass(void)
 			continue;
 
 		printf("uclass %d: %s\n", id, uc->uc_drv->name);
-		if (list_empty(&uc->dev_head))
-			continue;
 		uclass_foreach_dev(dev, uc) {
 			dm_display_line(dev, i);
 			i++;

From 4780f7d8a6b2f479884d4e6068a73d1a69f82d4d Mon Sep 17 00:00:00 2001
From: Sean Anderson <sean.anderson@seco.com>
Date: Fri, 29 Apr 2022 10:53:34 -0400
Subject: [PATCH 14/34] patman: Fix defaults not propagating to subparsers

On python 3.8.10 (and 3.10), subparsers are not updated with defaults. I
suspect this is related to [1]. Fix this by explicitly updating
subparsers with settings.

[1] https://github.com/python/cpython/issues/89398

Fixes: 3145b63513 ("patman: Update defaults in subparsers")
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
Reviewed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Tested-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---
 tools/patman/settings.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/patman/settings.py b/tools/patman/settings.py
index 7c2b5c196c0..4c847fe88fd 100644
--- a/tools/patman/settings.py
+++ b/tools/patman/settings.py
@@ -246,8 +246,10 @@ def _UpdateDefaults(main_parser, config):
 
     # Collect the defaults from each parser
     defaults = {}
+    parser_defaults = []
     for parser in parsers:
         pdefs = parser.parse_known_args()[0]
+        parser_defaults.append(pdefs)
         defaults.update(vars(pdefs))
 
     # Go through the settings and collect defaults
@@ -264,8 +266,11 @@ def _UpdateDefaults(main_parser, config):
         else:
             print("WARNING: Unknown setting %s" % name)
 
-    # Set all the defaults (this propagates through all subparsers)
+    # Set all the defaults and manually propagate them to subparsers
     main_parser.set_defaults(**defaults)
+    for parser, pdefs in zip(parsers, parser_defaults):
+        parser.set_defaults(**{ k: v for k, v in defaults.items()
+                                    if k in pdefs })
 
 def _ReadAliasFile(fname):
     """Read in the U-Boot git alias file if it exists.

From 2be964d29f11ead9c82ba1a19fbfeceb63e3f62d Mon Sep 17 00:00:00 2001
From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Date: Sat, 30 Apr 2022 07:55:53 +0200
Subject: [PATCH 15/34] sandbox: raise SANDBOX_RAM_SIZE_MB default to 256

The UEFI Self Certification Test (SCT) cannot run on 128 MiB.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 arch/sandbox/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sandbox/Kconfig b/arch/sandbox/Kconfig
index 5f55c7f28e1..852a7c8bf2c 100644
--- a/arch/sandbox/Kconfig
+++ b/arch/sandbox/Kconfig
@@ -17,11 +17,11 @@ config SANDBOX64
 
 config SANDBOX_RAM_SIZE_MB
 	int "RAM size in MiB"
-	default 128
+	default 256
 	range 64 4095 if !SANDBOX64
 	range 64 268435456 if SANDBOX64
 	help
-	  Memory size of the sandbox in MiB. The default value is 128 MiB.
+	  Memory size of the sandbox in MiB. The default value is 256 MiB.
 	  The minimum value is 64 MiB. The maximum value is 4095 MiB for the
 	  32bit sandbox.
 

From 06d590844f3560facdaeba728148e8db0fdbb023 Mon Sep 17 00:00:00 2001
From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Date: Sat, 30 Apr 2022 11:26:23 +0200
Subject: [PATCH 16/34] test: fix some pylint errors in test_bind.py

* Use spaces not tabs
* Limit lines to 100 spaces
* Remove an unused import
* Sort imports correctly
* Add a module description

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 test/py/tests/test_bind.py | 291 +++++++++++++++++++------------------
 1 file changed, 148 insertions(+), 143 deletions(-)

diff --git a/test/py/tests/test_bind.py b/test/py/tests/test_bind.py
index 8ad277da190..d7e6626d45f 100644
--- a/test/py/tests/test_bind.py
+++ b/test/py/tests/test_bind.py
@@ -1,186 +1,191 @@
 # SPDX-License-Identifier: GPL-2.0
 # Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
 
-import os.path
-import pytest
+""" Test for bind command """
+
 import re
+import pytest
 
 def in_tree(response, name, uclass, drv, depth, last_child):
-	lines = [x.strip() for x in response.splitlines()]
-	leaf = ''
-	if depth != 0:
-		leaf = '   ' + '    ' * (depth - 1) ;
-		if not last_child:
-			leaf = leaf + r'\|'
-		else:
-                        leaf = leaf + '`'
+    lines = [x.strip() for x in response.splitlines()]
+    leaf = ''
+    if depth != 0:
+        leaf = '   ' + '    ' * (depth - 1)
+        if not last_child:
+            leaf = leaf + r'\|'
+        else:
+            leaf = leaf + '`'
 
-	leaf = leaf + '-- ' + name
-	line = (r' *{:10.10} *[0-9]*  \[ [ +] \]   {:20.20}  [` |]{}$'
-	        .format(uclass, drv, leaf))
-	prog = re.compile(line)
-	for l in lines:
-		if prog.match(l):
-			return True
-	return False
+    leaf = leaf + '-- ' + name
+    line = (r' *{:10.10} *[0-9]*  \[ [ +] \]   {:20.20}  [` |]{}$'
+            .format(uclass, drv, leaf))
+    prog = re.compile(line)
+    for l in lines:
+        if prog.match(l):
+            return True
+    return False
 
 
 @pytest.mark.buildconfigspec('cmd_bind')
 def test_bind_unbind_with_node(u_boot_console):
 
-	tree = u_boot_console.run_command('dm tree')
-	assert in_tree(tree, 'bind-test', 'simple_bus', 'simple_bus', 0, True)
-	assert in_tree(tree, 'bind-test-child1', 'phy', 'phy_sandbox', 1, False)
-	assert in_tree(tree, 'bind-test-child2', 'simple_bus', 'simple_bus', 1, True)
+    tree = u_boot_console.run_command('dm tree')
+    assert in_tree(tree, 'bind-test', 'simple_bus', 'simple_bus', 0, True)
+    assert in_tree(tree, 'bind-test-child1', 'phy', 'phy_sandbox', 1, False)
+    assert in_tree(tree, 'bind-test-child2', 'simple_bus', 'simple_bus', 1, True)
 
-	#bind usb_ether driver (which has no compatible) to usb@1 node.
-	##New entry usb_ether should appear in the dm tree
-	response = u_boot_console.run_command('bind  /usb@1 usb_ether')
-	assert response == ''
-	tree = u_boot_console.run_command('dm tree')
-	assert in_tree(tree, 'usb@1', 'ethernet', 'usb_ether', 1, True)
+    #bind usb_ether driver (which has no compatible) to usb@1 node.
+    ##New entry usb_ether should appear in the dm tree
+    response = u_boot_console.run_command('bind  /usb@1 usb_ether')
+    assert response == ''
+    tree = u_boot_console.run_command('dm tree')
+    assert in_tree(tree, 'usb@1', 'ethernet', 'usb_ether', 1, True)
 
-	#Unbind child #1. No error expected and all devices should be there except for bind-test-child1
-	response = u_boot_console.run_command('unbind  /bind-test/bind-test-child1')
-	assert response == ''
-	tree = u_boot_console.run_command('dm tree')
-	assert in_tree(tree, 'bind-test', 'simple_bus', 'simple_bus', 0, True)
-	assert 'bind-test-child1' not in tree
-	assert in_tree(tree, 'bind-test-child2', 'simple_bus', 'simple_bus', 1, True)
+    #Unbind child #1. No error expected and all devices should be there except for bind-test-child1
+    response = u_boot_console.run_command('unbind  /bind-test/bind-test-child1')
+    assert response == ''
+    tree = u_boot_console.run_command('dm tree')
+    assert in_tree(tree, 'bind-test', 'simple_bus', 'simple_bus', 0, True)
+    assert 'bind-test-child1' not in tree
+    assert in_tree(tree, 'bind-test-child2', 'simple_bus', 'simple_bus', 1, True)
 
-	#bind child #1. No error expected and all devices should be there
-	response = u_boot_console.run_command('bind  /bind-test/bind-test-child1 phy_sandbox')
-	assert response == ''
-	tree = u_boot_console.run_command('dm tree')
-	assert in_tree(tree, 'bind-test', 'simple_bus', 'simple_bus', 0, True)
-	assert in_tree(tree, 'bind-test-child1', 'phy', 'phy_sandbox', 1, True)
-	assert in_tree(tree, 'bind-test-child2', 'simple_bus', 'simple_bus', 1, False)
+    #bind child #1. No error expected and all devices should be there
+    response = u_boot_console.run_command('bind  /bind-test/bind-test-child1 phy_sandbox')
+    assert response == ''
+    tree = u_boot_console.run_command('dm tree')
+    assert in_tree(tree, 'bind-test', 'simple_bus', 'simple_bus', 0, True)
+    assert in_tree(tree, 'bind-test-child1', 'phy', 'phy_sandbox', 1, True)
+    assert in_tree(tree, 'bind-test-child2', 'simple_bus', 'simple_bus', 1, False)
 
-	#Unbind child #2. No error expected and all devices should be there except for bind-test-child2
-	response = u_boot_console.run_command('unbind  /bind-test/bind-test-child2')
-	assert response == ''
-	tree = u_boot_console.run_command('dm tree')
-	assert in_tree(tree, 'bind-test', 'simple_bus', 'simple_bus', 0, True)
-	assert in_tree(tree, 'bind-test-child1', 'phy', 'phy_sandbox', 1, True)
-	assert 'bind-test-child2' not in tree
+    #Unbind child #2. No error expected and all devices should be there except for bind-test-child2
+    response = u_boot_console.run_command('unbind  /bind-test/bind-test-child2')
+    assert response == ''
+    tree = u_boot_console.run_command('dm tree')
+    assert in_tree(tree, 'bind-test', 'simple_bus', 'simple_bus', 0, True)
+    assert in_tree(tree, 'bind-test-child1', 'phy', 'phy_sandbox', 1, True)
+    assert 'bind-test-child2' not in tree
 
 
-	#Bind child #2. No error expected and all devices should be there
-	response = u_boot_console.run_command('bind /bind-test/bind-test-child2 simple_bus')
-	assert response == ''
-	tree = u_boot_console.run_command('dm tree')
-	assert in_tree(tree, 'bind-test', 'simple_bus', 'simple_bus', 0, True)
-	assert in_tree(tree, 'bind-test-child1', 'phy', 'phy_sandbox', 1, False)
-	assert in_tree(tree, 'bind-test-child2', 'simple_bus', 'simple_bus', 1, True)
+    #Bind child #2. No error expected and all devices should be there
+    response = u_boot_console.run_command('bind /bind-test/bind-test-child2 simple_bus')
+    assert response == ''
+    tree = u_boot_console.run_command('dm tree')
+    assert in_tree(tree, 'bind-test', 'simple_bus', 'simple_bus', 0, True)
+    assert in_tree(tree, 'bind-test-child1', 'phy', 'phy_sandbox', 1, False)
+    assert in_tree(tree, 'bind-test-child2', 'simple_bus', 'simple_bus', 1, True)
 
-	#Unbind parent. No error expected. All devices should be removed and unbound
-	response = u_boot_console.run_command('unbind  /bind-test')
-	assert response == ''
-	tree = u_boot_console.run_command('dm tree')
-	assert 'bind-test' not in tree
-	assert 'bind-test-child1' not in tree
-	assert 'bind-test-child2' not in tree
+    #Unbind parent. No error expected. All devices should be removed and unbound
+    response = u_boot_console.run_command('unbind  /bind-test')
+    assert response == ''
+    tree = u_boot_console.run_command('dm tree')
+    assert 'bind-test' not in tree
+    assert 'bind-test-child1' not in tree
+    assert 'bind-test-child2' not in tree
 
-	#try binding invalid node with valid driver
-	response = u_boot_console.run_command('bind  /not-a-valid-node simple_bus')
-	assert response != ''
-	tree = u_boot_console.run_command('dm tree')
-	assert 'not-a-valid-node' not in tree
+    #try binding invalid node with valid driver
+    response = u_boot_console.run_command('bind  /not-a-valid-node simple_bus')
+    assert response != ''
+    tree = u_boot_console.run_command('dm tree')
+    assert 'not-a-valid-node' not in tree
 
-	#try binding valid node with invalid driver
-	response = u_boot_console.run_command('bind  /bind-test not_a_driver')
-	assert response != ''
-	tree = u_boot_console.run_command('dm tree')
-	assert 'bind-test' not in tree
+    #try binding valid node with invalid driver
+    response = u_boot_console.run_command('bind  /bind-test not_a_driver')
+    assert response != ''
+    tree = u_boot_console.run_command('dm tree')
+    assert 'bind-test' not in tree
 
-	#bind /bind-test. Device should come up as well as its children
-	response = u_boot_console.run_command('bind  /bind-test simple_bus')
-	assert response == ''
-	tree = u_boot_console.run_command('dm tree')
-	assert in_tree(tree, 'bind-test', 'simple_bus', 'simple_bus', 0, True)
-	assert in_tree(tree, 'bind-test-child1', 'phy', 'phy_sandbox', 1, False)
-	assert in_tree(tree, 'bind-test-child2', 'simple_bus', 'simple_bus', 1, True)
+    #bind /bind-test. Device should come up as well as its children
+    response = u_boot_console.run_command('bind  /bind-test simple_bus')
+    assert response == ''
+    tree = u_boot_console.run_command('dm tree')
+    assert in_tree(tree, 'bind-test', 'simple_bus', 'simple_bus', 0, True)
+    assert in_tree(tree, 'bind-test-child1', 'phy', 'phy_sandbox', 1, False)
+    assert in_tree(tree, 'bind-test-child2', 'simple_bus', 'simple_bus', 1, True)
 
-	response = u_boot_console.run_command('unbind  /bind-test')
-	assert response == ''
+    response = u_boot_console.run_command('unbind  /bind-test')
+    assert response == ''
 
 def get_next_line(tree, name):
-	treelines = [x.strip() for x in tree.splitlines() if x.strip()]
-	child_line = ''
-	for idx, line in enumerate(treelines):
-		if ('-- ' + name) in line:
-			try:
-				child_line = treelines[idx+1]
-			except:
-				pass
-			break
-	return child_line
+    treelines = [x.strip() for x in tree.splitlines() if x.strip()]
+    child_line = ''
+    for idx, line in enumerate(treelines):
+        if '-- ' + name in line:
+            try:
+                child_line = treelines[idx+1]
+            except:
+                pass
+            break
+    return child_line
 
 @pytest.mark.buildconfigspec('cmd_bind')
 def test_bind_unbind_with_uclass(u_boot_console):
-	#bind /bind-test
-	response = u_boot_console.run_command('bind  /bind-test simple_bus')
-	assert response == ''
+    #bind /bind-test
+    response = u_boot_console.run_command('bind  /bind-test simple_bus')
+    assert response == ''
 
-	#make sure bind-test-child2 is there and get its uclass/index pair
-	tree = u_boot_console.run_command('dm tree')
-	child2_line = [x.strip() for x in tree.splitlines() if '-- bind-test-child2' in x]
-	assert len(child2_line) == 1
+    #make sure bind-test-child2 is there and get its uclass/index pair
+    tree = u_boot_console.run_command('dm tree')
+    child2_line = [x.strip() for x in tree.splitlines() if '-- bind-test-child2' in x]
+    assert len(child2_line) == 1
 
-	child2_uclass = child2_line[0].split()[0]
-	child2_index = int(child2_line[0].split()[1])
+    child2_uclass = child2_line[0].split()[0]
+    child2_index = int(child2_line[0].split()[1])
 
-	#bind simple_bus as a child of bind-test-child2
-	response = u_boot_console.run_command('bind  {} {} simple_bus'.format(child2_uclass, child2_index))
+    #bind simple_bus as a child of bind-test-child2
+    response = u_boot_console.run_command(
+                    'bind  {} {} simple_bus'.format(child2_uclass, child2_index))
 
-	#check that the child is there and its uclass/index pair is right
-	tree = u_boot_console.run_command('dm tree')
+    #check that the child is there and its uclass/index pair is right
+    tree = u_boot_console.run_command('dm tree')
 
-	child_of_child2_line = get_next_line(tree, 'bind-test-child2')
-	assert child_of_child2_line
-	child_of_child2_index = int(child_of_child2_line.split()[1])
-	assert in_tree(tree, 'simple_bus', 'simple_bus', 'simple_bus', 2, True)
-	assert child_of_child2_index == child2_index + 1
+    child_of_child2_line = get_next_line(tree, 'bind-test-child2')
+    assert child_of_child2_line
+    child_of_child2_index = int(child_of_child2_line.split()[1])
+    assert in_tree(tree, 'simple_bus', 'simple_bus', 'simple_bus', 2, True)
+    assert child_of_child2_index == child2_index + 1
 
-	#unbind the child and check it has been removed
-	response = u_boot_console.run_command('unbind  simple_bus {}'.format(child_of_child2_index))
-	assert response == ''
-	tree = u_boot_console.run_command('dm tree')
-	assert in_tree(tree, 'bind-test-child2', 'simple_bus', 'simple_bus', 1, True)
-	assert not in_tree(tree, 'simple_bus', 'simple_bus', 'simple_bus', 2, True)
-	child_of_child2_line = get_next_line(tree, 'bind-test-child2')
-	assert child_of_child2_line == ''
+    #unbind the child and check it has been removed
+    response = u_boot_console.run_command('unbind  simple_bus {}'.format(child_of_child2_index))
+    assert response == ''
+    tree = u_boot_console.run_command('dm tree')
+    assert in_tree(tree, 'bind-test-child2', 'simple_bus', 'simple_bus', 1, True)
+    assert not in_tree(tree, 'simple_bus', 'simple_bus', 'simple_bus', 2, True)
+    child_of_child2_line = get_next_line(tree, 'bind-test-child2')
+    assert child_of_child2_line == ''
 
-	#bind simple_bus as a child of bind-test-child2
-	response = u_boot_console.run_command('bind  {} {} simple_bus'.format(child2_uclass, child2_index))
+    #bind simple_bus as a child of bind-test-child2
+    response = u_boot_console.run_command(
+                    'bind  {} {} simple_bus'.format(child2_uclass, child2_index))
 
-	#check that the child is there and its uclass/index pair is right
-	tree = u_boot_console.run_command('dm tree')
-	treelines = [x.strip() for x in tree.splitlines() if x.strip()]
+    #check that the child is there and its uclass/index pair is right
+    tree = u_boot_console.run_command('dm tree')
+    treelines = [x.strip() for x in tree.splitlines() if x.strip()]
 
-	child_of_child2_line = get_next_line(tree, 'bind-test-child2')
-	assert child_of_child2_line
-	child_of_child2_index = int(child_of_child2_line.split()[1])
-	assert in_tree(tree, 'simple_bus', 'simple_bus', 'simple_bus', 2, True)
-	assert child_of_child2_index == child2_index + 1
+    child_of_child2_line = get_next_line(tree, 'bind-test-child2')
+    assert child_of_child2_line
+    child_of_child2_index = int(child_of_child2_line.split()[1])
+    assert in_tree(tree, 'simple_bus', 'simple_bus', 'simple_bus', 2, True)
+    assert child_of_child2_index == child2_index + 1
 
-	#unbind the child and check it has been removed
-	response = u_boot_console.run_command('unbind  {} {} simple_bus'.format(child2_uclass, child2_index))
-	assert response == ''
+    #unbind the child and check it has been removed
+    response = u_boot_console.run_command(
+                    'unbind  {} {} simple_bus'.format(child2_uclass, child2_index))
+    assert response == ''
 
-	tree = u_boot_console.run_command('dm tree')
-	assert in_tree(tree, 'bind-test-child2', 'simple_bus', 'simple_bus', 1, True)
+    tree = u_boot_console.run_command('dm tree')
+    assert in_tree(tree, 'bind-test-child2', 'simple_bus', 'simple_bus', 1, True)
 
-	child_of_child2_line = get_next_line(tree, 'bind-test-child2')
-	assert child_of_child2_line == ''
+    child_of_child2_line = get_next_line(tree, 'bind-test-child2')
+    assert child_of_child2_line == ''
 
-	#unbind the child again and check it doesn't change the tree
-	tree_old = u_boot_console.run_command('dm tree')
-	response = u_boot_console.run_command('unbind  {} {} simple_bus'.format(child2_uclass, child2_index))
-	tree_new = u_boot_console.run_command('dm tree')
+    #unbind the child again and check it doesn't change the tree
+    tree_old = u_boot_console.run_command('dm tree')
+    response = u_boot_console.run_command(
+                    'unbind  {} {} simple_bus'.format(child2_uclass, child2_index))
+    tree_new = u_boot_console.run_command('dm tree')
 
-	assert response == ''
-	assert tree_old == tree_new
+    assert response == ''
+    assert tree_old == tree_new
 
-	response = u_boot_console.run_command('unbind  /bind-test')
-	assert response == ''
+    response = u_boot_console.run_command('unbind  /bind-test')
+    assert response == ''

From 1452870404804210db1d797ec046e24a99c101bf Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sun, 8 May 2022 04:39:19 -0600
Subject: [PATCH 17/34] dm: core: Rename dm_dump_all()

This is not a good name anymore as it does not dump everything. Rename it
to dm_dump_tree() to avoid confusion.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 cmd/dm.c            | 8 ++++----
 drivers/core/dump.c | 2 +-
 include/dm/util.h   | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/cmd/dm.c b/cmd/dm.c
index ca609224f55..03674cd0864 100644
--- a/cmd/dm.c
+++ b/cmd/dm.c
@@ -10,10 +10,10 @@
 #include <command.h>
 #include <dm/util.h>
 
-static int do_dm_dump_all(struct cmd_tbl *cmdtp, int flag, int argc,
-			  char *const argv[])
+static int do_dm_dump_tree(struct cmd_tbl *cmdtp, int flag, int argc,
+			   char *const argv[])
 {
-	dm_dump_all();
+	dm_dump_tree();
 
 	return 0;
 }
@@ -70,7 +70,7 @@ static char dm_help_text[] =
 #endif
 
 U_BOOT_CMD_WITH_SUBCMDS(dm, "Driver model low level access", dm_help_text,
-	U_BOOT_SUBCMD_MKENT(tree, 1, 1, do_dm_dump_all),
+	U_BOOT_SUBCMD_MKENT(tree, 1, 1, do_dm_dump_tree),
 	U_BOOT_SUBCMD_MKENT(uclass, 1, 1, do_dm_dump_uclass),
 	U_BOOT_SUBCMD_MKENT(devres, 1, 1, do_dm_dump_devres),
 	U_BOOT_SUBCMD_MKENT(drivers, 1, 1, do_dm_dump_drivers),
diff --git a/drivers/core/dump.c b/drivers/core/dump.c
index 21d9e7a91f7..994e308f057 100644
--- a/drivers/core/dump.c
+++ b/drivers/core/dump.c
@@ -45,7 +45,7 @@ static void show_devices(struct udevice *dev, int depth, int last_flag)
 	}
 }
 
-void dm_dump_all(void)
+void dm_dump_tree(void)
 {
 	struct udevice *root;
 
diff --git a/include/dm/util.h b/include/dm/util.h
index 4428f045b72..c52daa87ef3 100644
--- a/include/dm/util.h
+++ b/include/dm/util.h
@@ -25,7 +25,7 @@ struct list_head;
 int list_count_items(struct list_head *head);
 
 /* Dump out a tree of all devices */
-void dm_dump_all(void);
+void dm_dump_tree(void);
 
 /* Dump out a list of uclasses and their devices */
 void dm_dump_uclass(void);

From dee2f5ae5cde41a5f9da7f154b47bfe92f531957 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sun, 8 May 2022 04:39:20 -0600
Subject: [PATCH 18/34] dm: core: Sort dm subcommands

Put these in alphabetic order, both in the help and in the implementation,
as there are quite a few subcommands now. Tweak the help for 'dm tree' to
better explain what it does.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 cmd/dm.c | 58 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/cmd/dm.c b/cmd/dm.c
index 03674cd0864..0c7554a1b1d 100644
--- a/cmd/dm.c
+++ b/cmd/dm.c
@@ -10,18 +10,10 @@
 #include <command.h>
 #include <dm/util.h>
 
-static int do_dm_dump_tree(struct cmd_tbl *cmdtp, int flag, int argc,
-			   char *const argv[])
+static int do_dm_dump_driver_compat(struct cmd_tbl *cmdtp, int flag, int argc,
+				    char * const argv[])
 {
-	dm_dump_tree();
-
-	return 0;
-}
-
-static int do_dm_dump_uclass(struct cmd_tbl *cmdtp, int flag, int argc,
-			     char *const argv[])
-{
-	dm_dump_uclass();
+	dm_dump_driver_compat();
 
 	return 0;
 }
@@ -42,37 +34,45 @@ static int do_dm_dump_drivers(struct cmd_tbl *cmdtp, int flag, int argc,
 	return 0;
 }
 
-static int do_dm_dump_driver_compat(struct cmd_tbl *cmdtp, int flag, int argc,
-				    char * const argv[])
-{
-	dm_dump_driver_compat();
-
-	return 0;
-}
-
-static int do_dm_dump_static_driver_info(struct cmd_tbl *cmdtp, int flag, int argc,
-					 char * const argv[])
+static int do_dm_dump_static_driver_info(struct cmd_tbl *cmdtp, int flag,
+					 int argc, char * const argv[])
 {
 	dm_dump_static_driver_info();
 
 	return 0;
 }
 
+static int do_dm_dump_tree(struct cmd_tbl *cmdtp, int flag, int argc,
+			   char *const argv[])
+{
+	dm_dump_tree();
+
+	return 0;
+}
+
+static int do_dm_dump_uclass(struct cmd_tbl *cmdtp, int flag, int argc,
+			     char *const argv[])
+{
+	dm_dump_uclass();
+
+	return 0;
+}
+
 #if CONFIG_IS_ENABLED(SYS_LONGHELP)
 static char dm_help_text[] =
-	"tree          Dump driver model tree ('*' = activated)\n"
-	"dm uclass        Dump list of instances for each uclass\n"
+	"compat        Dump list of drivers with compatibility strings\n"
 	"dm devres        Dump list of device resources for each device\n"
 	"dm drivers       Dump list of drivers with uclass and instances\n"
-	"dm compat        Dump list of drivers with compatibility strings\n"
-	"dm static        Dump list of drivers with static platform data"
+	"dm static        Dump list of drivers with static platform data\n"
+	"dn tree          Dump tree of driver model devices ('*' = activated)\n"
+	"dm uclass        Dump list of instances for each uclass"
 	;
 #endif
 
 U_BOOT_CMD_WITH_SUBCMDS(dm, "Driver model low level access", dm_help_text,
-	U_BOOT_SUBCMD_MKENT(tree, 1, 1, do_dm_dump_tree),
-	U_BOOT_SUBCMD_MKENT(uclass, 1, 1, do_dm_dump_uclass),
+	U_BOOT_SUBCMD_MKENT(compat, 1, 1, do_dm_dump_driver_compat),
 	U_BOOT_SUBCMD_MKENT(devres, 1, 1, do_dm_dump_devres),
 	U_BOOT_SUBCMD_MKENT(drivers, 1, 1, do_dm_dump_drivers),
-	U_BOOT_SUBCMD_MKENT(compat, 1, 1, do_dm_dump_driver_compat),
-	U_BOOT_SUBCMD_MKENT(static, 1, 1, do_dm_dump_static_driver_info));
+	U_BOOT_SUBCMD_MKENT(static, 1, 1, do_dm_dump_static_driver_info),
+	U_BOOT_SUBCMD_MKENT(tree, 1, 1, do_dm_dump_tree),
+	U_BOOT_SUBCMD_MKENT(uclass, 1, 1, do_dm_dump_uclass));

From c625666ea10d88141546807f1b720703ba710eea Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sun, 8 May 2022 04:39:21 -0600
Subject: [PATCH 19/34] dm: core: Fix addresses in the dm static command

This command converts pointers to addresses, but the pointers being
converted are in the image's rodata region. For sandbox this means it
is not in DRAM so it does not make sense to do this conversion.

Fix this by showing a simple pointer instead. Drop the unnecessary
@ and hex prefixes.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 drivers/core/dump.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/core/dump.c b/drivers/core/dump.c
index 994e308f057..e434fe04728 100644
--- a/drivers/core/dump.c
+++ b/drivers/core/dump.c
@@ -169,8 +169,6 @@ void dm_dump_static_driver_info(void)
 
 	puts("Driver                    Address\n");
 	puts("---------------------------------\n");
-	for (entry = drv; entry != drv + n_ents; entry++) {
-		printf("%-25.25s @%08lx\n", entry->name,
-		       (ulong)map_to_sysmem(entry->plat));
-	}
+	for (entry = drv; entry != drv + n_ents; entry++)
+		printf("%-25.25s %p\n", entry->name, entry->plat);
 }

From d32f62f49d338e68c2d793a9e2412677a740c89f Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sun, 8 May 2022 04:39:22 -0600
Subject: [PATCH 20/34] dm: core: Add documentation for the dm command

Add a description and examples for the dm subcommands.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 doc/usage/cmd/dm.rst | 487 +++++++++++++++++++++++++++++++++++++++++++
 doc/usage/index.rst  |   1 +
 2 files changed, 488 insertions(+)
 create mode 100644 doc/usage/cmd/dm.rst

diff --git a/doc/usage/cmd/dm.rst b/doc/usage/cmd/dm.rst
new file mode 100644
index 00000000000..7bc1962a754
--- /dev/null
+++ b/doc/usage/cmd/dm.rst
@@ -0,0 +1,487 @@
+.. SPDX-License-Identifier: GPL-2.0+:
+
+dm command
+==========
+
+Synopis
+-------
+
+::
+
+    dm compat
+    dm devres
+    dm drivers
+    dm static
+    dm tree
+    dm uclass
+
+Description
+-----------
+
+The *dm* command allows viewing information about driver model, including the
+tree of devices and list of available uclasses.
+
+
+dm compat
+~~~~~~~~~
+
+This shows the compatible strings associated with each driver. Often there
+is only one, but multiple strings are shown on their own line. These strings
+can be looked up in the device tree files for each board, to see which driver is
+used for each node.
+
+dm devres
+~~~~~~~~~
+
+This shows a list of a `devres` (device resource) records for a device. Some
+drivers use the devres API to allocate memory, so that it can be freed
+automatically (without any code needed in the driver's remove() method) when the
+device is removed.
+
+This feature is controlled by CONFIG_DEVRES so no useful output is obtained if
+this option is disabled.
+
+dm drivers
+~~~~~~~~~~
+
+This shows all the available drivers, their uclass and a list of devices that
+use that driver, each on its own line. Drivers with no devices are shown with
+`<none>` as the driver name.
+
+
+dm mem
+~~~~~~
+
+This subcommand is really just for debugging and exploration. It can be enabled
+with the `CONFIG_DM_STATS` option.
+
+All output is in hex except that in brackets which is decimal.
+
+The output consists of a header shows the size of the main device model
+structures (struct udevice, struct driver, struct uclass and struct uc_driver)
+and the count and memory used by each (number of devices, memory used by
+devices, memory used by device names, number of uclasses, memory used by
+uclasses).
+
+After that is a table of information about each type of data that can be
+attached to a device, showing the number that have non-null data for that type,
+the total size of all that data, the amount of memory used in total, the
+amount that would be used if this type uses tags instead and the amount that
+would be thus saved.
+
+The `driver_data` line shows the number of devices which have non-NULL driver
+data.
+
+The `tags` line shows the number of tags and the memory used by those.
+
+At the bottom is an indication of the total memory usage obtained by undertaking
+various changes, none of which is currently implemented in U-Boot:
+
+With tags
+    Using tags instead of all attached types
+
+Singly linked
+    Using a singly linked list
+
+driver index
+    Using a driver index instead of a pointer
+
+uclass index
+    Using a uclass index instead of a pointer
+
+Drop device name
+    Using empty device names
+
+
+dm static
+~~~~~~~~~
+
+This shows devices bound by platform data, i.e. not from the device tree. There
+are normally none of these, but some boards may use static devices for space
+reasons.
+
+
+dm tree
+~~~~~~~
+
+This shows the full tree of devices including the following fields:
+
+uclass
+    Shows the name of the uclass for the device
+
+Index
+    Shows the index number of the device, within the uclass. This shows the
+    ordering within the uclass, but not the sequence number.
+
+Probed
+    Shows `+` if the device is active
+
+Driver
+    Shows the name of the driver that this device uses
+
+Name
+    Shows the device name as well as the tree structure, since child devices are
+    shown attached to their parent.
+
+
+dm uclass
+~~~~~~~~~
+
+This shows each uclass along with a list of devices in that uclass. The uclass
+ID is shown (e.g. uclass 7) and its name.
+
+For each device, the format is::
+
+    n    name @ a, seq s
+
+where `n` is the index within the uclass, `a` is the address of the device in
+memory and `s` is the sequence number of the device.
+
+
+Examples
+--------
+
+dm compat
+~~~~~~~~~
+
+This example shows an abridged version of the sandbox output::
+
+    => dm compat
+    Driver                Compatible
+    --------------------------------
+    act8846_reg
+    sandbox_adder         sandbox,adder
+    axi_sandbox_bus       sandbox,axi
+    blk_partition
+    bootcount-rtc         u-boot,bootcount-rtc
+    ...
+    rockchip_rk805        rockchip,rk805
+                          rockchip,rk808
+                          rockchip,rk809
+                          rockchip,rk816
+                          rockchip,rk817
+                          rockchip,rk818
+    root_driver
+    rtc-rv8803            microcrystal,rv8803
+                          epson,rx8803
+                          epson,rx8900
+    ...
+    wdt_gpio              linux,wdt-gpio
+    wdt_sandbox           sandbox,wdt
+
+
+dm devres
+~~~~~~~~~
+
+This example shows an abridged version of the sandbox test output (running
+U-Boot with the -T flag)::
+
+    => dm devres
+    - root_driver
+    - demo_shape_drv
+    - demo_simple_drv
+    - demo_shape_drv
+    ...
+    - h-test
+    - devres-test
+        00000000130194e0 (100 byte) devm_kmalloc_release  BIND
+    - another-test
+    ...
+    - syscon@3
+    - a-mux-controller
+        0000000013025e60 (96 byte) devm_kmalloc_release  PROBE
+        0000000013025f00 (24 byte) devm_kmalloc_release  PROBE
+        0000000013026010 (24 byte) devm_kmalloc_release  PROBE
+        0000000013026070 (24 byte) devm_kmalloc_release  PROBE
+        00000000130260d0 (24 byte) devm_kmalloc_release  PROBE
+    - syscon@3
+    - a-mux-controller
+        0000000013026150 (96 byte) devm_kmalloc_release  PROBE
+        00000000130261f0 (24 byte) devm_kmalloc_release  PROBE
+        0000000013026300 (24 byte) devm_kmalloc_release  PROBE
+        0000000013026360 (24 byte) devm_kmalloc_release  PROBE
+        00000000130263c0 (24 byte) devm_kmalloc_release  PROBE
+    - emul-mux-controller
+        0000000013025fa0 (32 byte) devm_kmalloc_release  PROBE
+    - testfdtm0
+    - testfdtm1
+    ...
+    - pinmux_spi0_pins
+    - pinmux_uart0_pins
+    - pinctrl-single-bits
+        0000000013229180 (320 byte) devm_kmalloc_release  PROBE
+        0000000013229300 (40 byte) devm_kmalloc_release  PROBE
+        0000000013229370 (160 byte) devm_kmalloc_release  PROBE
+        000000001322c190 (40 byte) devm_kmalloc_release  PROBE
+        000000001322c200 (32 byte) devm_kmalloc_release  PROBE
+    - pinmux_i2c0_pins
+    ...
+    - reg@0
+    - reg@1
+
+
+dm drivers
+~~~~~~~~~~
+
+This example shows an abridged version of the sandbox output::
+
+    => dm drivers
+    Driver                    uid uclass               Devices
+    ----------------------------------------------------------
+    act8846_reg               087 regulator            <none>
+    sandbox_adder             021 axi                  adder
+                                                    adder
+    axi_sandbox_bus           021 axi                  axi@0
+    ...
+    da7219                    061 misc                 <none>
+    demo_shape_drv            001 demo                 demo_shape_drv
+                                                    demo_shape_drv
+                                                    demo_shape_drv
+    demo_simple_drv           001 demo                 demo_simple_drv
+                                                    demo_simple_drv
+    testfdt_drv               003 testfdt              a-test
+                                                    b-test
+                                                    d-test
+                                                    e-test
+                                                    f-test
+                                                    g-test
+                                                    another-test
+                                                    chosen-test
+    testbus_drv               005 testbus              some-bus
+                                                    mmio-bus@0
+                                                    mmio-bus@1
+    dsa-port                  039 ethernet             lan0
+                                                    lan1
+    dsa_sandbox               035 dsa                  dsa-test
+    eep_sandbox               121 w1_eeprom            <none>
+    ...
+    pfuze100_regulator        087 regulator            <none>
+    phy_sandbox               077 phy                  bind-test-child1
+                                                    gen_phy@0
+                                                    gen_phy@1
+                                                    gen_phy@2
+    pinconfig                 078 pinconfig            gpios
+                                                    gpio0
+                                                    gpio1
+                                                    gpio2
+                                                    gpio3
+                                                    i2c
+                                                    groups
+                                                    pins
+                                                    i2s
+                                                    spi
+                                                    cs
+                                                    pinmux_pwm_pins
+                                                    pinmux_spi0_pins
+                                                    pinmux_uart0_pins
+                                                    pinmux_i2c0_pins
+                                                    pinmux_lcd_pins
+    pmc_sandbox               017 power-mgr            pci@1e,0
+    act8846 pmic              080 pmic                 <none>
+    max77686_pmic             080 pmic                 <none>
+    mc34708_pmic              080 pmic                 pmic@41
+    ...
+    wdt_gpio                  122 watchdog             gpio-wdt
+    wdt_sandbox               122 watchdog             wdt@0
+    =>
+
+
+dm mem
+~~~~~~
+
+This example shows the sandbox output::
+
+    > dm mem
+    Struct sizes: udevice b0, driver 80, uclass 30, uc_driver 78
+    Memory: device fe:aea0, device names a16, uclass 5e:11a0
+
+    Attached type    Count   Size    Cur   Tags   Save
+    ---------------  -----  -----  -----  -----  -----
+    plat                45    a8f   aea0   a7c4    6dc (1756)
+    parent_plat         1a    3b8   aea0   a718    788 (1928)
+    uclass_plat         3d    6b4   aea0   a7a4    6fc (1788)
+    priv                8a   68f3   aea0   a8d8    5c8 (1480)
+    parent_priv          8   38a0   aea0   a6d0    7d0 (2000)
+    uclass_priv         4e   14a6   aea0   a7e8    6b8 (1720)
+    driver_data          f      0   aea0   a6ec    7b4 (1972)
+    uclass               6     20
+    Attached total     191   cb54                  3164 (12644)
+    tags                 0      0
+
+    Total size: 18b94 (101268)
+
+    With tags:       15a30 (88624)
+    - singly-linked: 14260 (82528)
+    - driver index:  13b6e (80750)
+    - uclass index:  1347c (78972)
+    Drop device name (not SRAM): a16 (2582)
+    =>
+
+
+dm static
+~~~~~~~~~
+
+This example shows the sandbox output::
+
+    => dm static
+    Driver                    Address
+    ---------------------------------
+    demo_shape_drv            0000562edab8dca0
+    demo_simple_drv           0000562edab8dca0
+    demo_shape_drv            0000562edab8dc90
+    demo_simple_drv           0000562edab8dc80
+    demo_shape_drv            0000562edab8dc80
+    test_drv                  0000562edaae8840
+    test_drv                  0000562edaae8848
+    test_drv                  0000562edaae8850
+    sandbox_gpio              0000000000000000
+    mod_exp_sw                0000000000000000
+    sandbox_test_proc         0000562edabb5330
+    qfw_sandbox               0000000000000000
+    sandbox_timer             0000000000000000
+    sandbox_serial            0000562edaa8ed00
+    sysreset_sandbox          0000000000000000
+
+
+dm tree
+-------
+
+This example shows the abridged sandbox output::
+
+    => dm tree
+    Class     Index  Probed  Driver                Name
+    -----------------------------------------------------------
+    root          0  [ + ]   root_driver           root_driver
+    demo          0  [   ]   demo_shape_drv        |-- demo_shape_drv
+    demo          1  [   ]   demo_simple_drv       |-- demo_simple_drv
+    demo          2  [   ]   demo_shape_drv        |-- demo_shape_drv
+    demo          3  [   ]   demo_simple_drv       |-- demo_simple_drv
+    demo          4  [   ]   demo_shape_drv        |-- demo_shape_drv
+    test          0  [   ]   test_drv              |-- test_drv
+    test          1  [   ]   test_drv              |-- test_drv
+    test          2  [   ]   test_drv              |-- test_drv
+    ..
+    sysreset      0  [   ]   sysreset_sandbox      |-- sysreset_sandbox
+    bootstd       0  [   ]   bootstd_drv           |-- bootstd
+    bootmeth      0  [   ]   bootmeth_distro       |   |-- syslinux
+    bootmeth      1  [   ]   bootmeth_efi          |   `-- efi
+    reboot-mod    0  [   ]   reboot-mode-gpio      |-- reboot-mode0
+    reboot-mod    1  [   ]   reboot-mode-rtc       |-- reboot-mode@14
+    ...
+    ethernet      7  [ + ]   dsa-port              |   `-- lan1
+    pinctrl       0  [ + ]   sandbox_pinctrl_gpio  |-- pinctrl-gpio
+    gpio          1  [ + ]   sandbox_gpio          |   |-- base-gpios
+    nop           0  [ + ]   gpio_hog              |   |   |-- hog_input_active_low
+    nop           1  [ + ]   gpio_hog              |   |   |-- hog_input_active_high
+    nop           2  [ + ]   gpio_hog              |   |   |-- hog_output_low
+    nop           3  [ + ]   gpio_hog              |   |   `-- hog_output_high
+    gpio          2  [   ]   sandbox_gpio          |   |-- extra-gpios
+    gpio          3  [   ]   sandbox_gpio          |   `-- pinmux-gpios
+    i2c           0  [ + ]   sandbox_i2c           |-- i2c@0
+    i2c_eeprom    0  [   ]   i2c_eeprom            |   |-- eeprom@2c
+    i2c_eeprom    1  [   ]   i2c_eeprom_partition  |   |   `-- bootcount@10
+    rtc           0  [   ]   sandbox_rtc           |   |-- rtc@43
+    rtc           1  [ + ]   sandbox_rtc           |   |-- rtc@61
+    i2c_emul_p    0  [ + ]   sandbox_i2c_emul_par  |   |-- emul
+    i2c_emul      0  [   ]   sandbox_i2c_eeprom_e  |   |   |-- emul-eeprom
+    i2c_emul      1  [   ]   sandbox_i2c_rtc_emul  |   |   |-- emul0
+    i2c_emul      2  [ + ]   sandbox_i2c_rtc_emul  |   |   |-- emull
+    i2c_emul      3  [   ]   sandbox_i2c_pmic_emu  |   |   |-- pmic-emul0
+    i2c_emul      4  [   ]   sandbox_i2c_pmic_emu  |   |   `-- pmic-emul1
+    pmic          0  [   ]   sandbox_pmic          |   |-- sandbox_pmic
+    regulator     0  [   ]   sandbox_buck          |   |   |-- buck1
+    regulator     1  [   ]   sandbox_buck          |   |   |-- buck2
+    regulator     2  [   ]   sandbox_ldo           |   |   |-- ldo1
+    regulator     3  [   ]   sandbox_ldo           |   |   |-- ldo2
+    regulator     4  [   ]   sandbox_buck          |   |   `-- no_match_by_nodename
+    pmic          1  [   ]   mc34708_pmic          |   `-- pmic@41
+    bootcount     0  [ + ]   bootcount-rtc         |-- bootcount@0
+    bootcount     1  [   ]   bootcount-i2c-eeprom  |-- bootcount
+    ...
+    clk           4  [   ]   fixed_clock           |-- osc
+    firmware      0  [   ]   sandbox_firmware      |-- sandbox-firmware
+    scmi_agent    0  [   ]   sandbox-scmi_agent    `-- scmi
+    clk           5  [   ]   scmi_clk                  |-- protocol@14
+    reset         2  [   ]   scmi_reset_domain         |-- protocol@16
+    nop           8  [   ]   scmi_voltage_domain       `-- regulators
+    regulator     5  [   ]   scmi_regulator                |-- reg@0
+    regulator     6  [   ]   scmi_regulator                `-- reg@1
+    =>
+
+
+dm uclass
+~~~~~~~~~
+
+This example shows the abridged sandbox output::
+
+    => dm uclass
+    uclass 0: root
+    0   * root_driver @ 03015460, seq 0
+
+    uclass 1: demo
+    0     demo_shape_drv @ 03015560, seq 0
+    1     demo_simple_drv @ 03015620, seq 1
+    2     demo_shape_drv @ 030156e0, seq 2
+    3     demo_simple_drv @ 030157a0, seq 3
+    4     demo_shape_drv @ 03015860, seq 4
+
+    uclass 2: test
+    0     test_drv @ 03015980, seq 0
+    1     test_drv @ 03015a60, seq 1
+    2     test_drv @ 03015b40, seq 2
+    ...
+    uclass 20: audio-codec
+    0     audio-codec @ 030168e0, seq 0
+
+    uclass 21: axi
+    0     adder @ 0301db60, seq 1
+    1     adder @ 0301dc40, seq 2
+    2     axi@0 @ 030217d0, seq 0
+
+    uclass 22: blk
+    0     mmc2.blk @ 0301ca00, seq 0
+    1     mmc1.blk @ 0301cee0, seq 1
+    2     mmc0.blk @ 0301d380, seq 2
+
+    uclass 23: bootcount
+    0   * bootcount@0 @ 0301b3f0, seq 0
+    1     bootcount @ 0301b4b0, seq 1
+    2     bootcount_4@0 @ 0301b570, seq 2
+    3     bootcount_2@0 @ 0301b630, seq 3
+
+    uclass 24: bootdev
+    0     mmc2.bootdev @ 0301cbb0, seq 0
+    1     mmc1.bootdev @ 0301d050, seq 1
+    2     mmc0.bootdev @ 0301d4f0, seq 2
+
+    ...
+    uclass 78: pinconfig
+    0     gpios @ 03022410, seq 0
+    1     gpio0 @ 030224d0, seq 1
+    2     gpio1 @ 03022590, seq 2
+    3     gpio2 @ 03022650, seq 3
+    4     gpio3 @ 03022710, seq 4
+    5     i2c @ 030227d0, seq 5
+    6     groups @ 03022890, seq 6
+    7     pins @ 03022950, seq 7
+    8     i2s @ 03022a10, seq 8
+    9     spi @ 03022ad0, seq 9
+    10    cs @ 03022b90, seq 10
+    11    pinmux_pwm_pins @ 03022e10, seq 11
+    12    pinmux_spi0_pins @ 03022ed0, seq 12
+    13    pinmux_uart0_pins @ 03022f90, seq 13
+    14  * pinmux_i2c0_pins @ 03023130, seq 14
+    15  * pinmux_lcd_pins @ 030231f0, seq 15
+
+    ...
+    uclass 119: virtio
+    0     sandbox_virtio1 @ 030220d0, seq 0
+    1     sandbox_virtio2 @ 03022190, seq 1
+
+    uclass 120: w1
+    uclass 121: w1_eeprom
+    uclass 122: watchdog
+    0   * gpio-wdt @ 0301c070, seq 0
+    1   * wdt@0 @ 03021710, seq 1
+
+    =>
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index 8d08ea14b00..8b98629d6bf 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -33,6 +33,7 @@ Shell commands
    cmd/bootz
    cmd/cbsysinfo
    cmd/conitrace
+   cmd/dm
    cmd/echo
    cmd/env
    cmd/event

From 53c20bebb2215caaadc58b2eee2c80c61456b93d Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sun, 8 May 2022 04:39:23 -0600
Subject: [PATCH 21/34] dm: core: Switch the testbus driver to use a new struct

At present this driver uses 'priv' struct to hold 'plat' data, which is
confusing. The contents of the strct don't matter, since only dtoc is
using it. Create a new struct with the correct name.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 drivers/misc/test_drv.c | 2 +-
 include/dm/test.h       | 7 +++++++
 tools/dtoc/test_dtoc.py | 2 +-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/test_drv.c b/drivers/misc/test_drv.c
index 5d72982f258..b6df1189032 100644
--- a/drivers/misc/test_drv.c
+++ b/drivers/misc/test_drv.c
@@ -109,7 +109,7 @@ UCLASS_DRIVER(testbus) = {
 	.child_post_probe = testbus_child_post_probe_uclass,
 
 	/* This is for dtoc testing only */
-	.per_device_plat_auto   = sizeof(struct dm_test_uclass_priv),
+	.per_device_plat_auto   = sizeof(struct dm_test_uclass_plat),
 };
 
 static int testfdt_drv_ping(struct udevice *dev, int pingval, int *pingret)
diff --git a/include/dm/test.h b/include/dm/test.h
index 4919064cc02..b5937509212 100644
--- a/include/dm/test.h
+++ b/include/dm/test.h
@@ -92,6 +92,13 @@ struct dm_test_uclass_priv {
 	int total_add;
 };
 
+/**
+ * struct dm_test_uclass_plat - private plat data for test uclass
+ */
+struct dm_test_uclass_plat {
+	char dummy[32];
+};
+
 /**
  * struct dm_test_parent_data - parent's information on each child
  *
diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
index c81bcc9c32f..8bac2076214 100755
--- a/tools/dtoc/test_dtoc.py
+++ b/tools/dtoc/test_dtoc.py
@@ -616,7 +616,7 @@ struct dm_test_pdata __attribute__ ((section (".priv_data")))
 u8 _denx_u_boot_test_bus_priv_some_bus[sizeof(struct dm_test_priv)]
 \t__attribute__ ((section (".priv_data")));
 #include <dm/test.h>
-u8 _denx_u_boot_test_bus_ucplat_some_bus[sizeof(struct dm_test_uclass_priv)]
+u8 _denx_u_boot_test_bus_ucplat_some_bus[sizeof(struct dm_test_uclass_plat)]
 \t__attribute__ ((section (".priv_data")));
 #include <test.h>
 

From 930a3ddadebf3660cc3163081671de189300afdd Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sun, 8 May 2022 04:39:24 -0600
Subject: [PATCH 22/34] dm: core: Support accessing core tags

At present tag numbers are only allocated for non-core data, meaning that
the 'core' data, like priv and plat, are accessed through dedicated
functions.

For debugging and consistency it is convenient to use tags for this 'core'
data too. Add support for this, with new tag numbers and functions to
access the pointer and size for each.

Update one of the test drivers so that the uclass-private data can be
tested here.

There is some code duplication with functions like device_alloc_priv() but
this is not addressed for now. At some point, some rationalisation may
help to reduce code size, but more thought it needed on that.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 drivers/core/device.c   | 65 +++++++++++++++++++++++++++++++++
 drivers/misc/test_drv.c |  4 ++-
 include/dm/device.h     | 25 +++++++++++++
 include/dm/tag.h        | 13 ++++++-
 test/dm/core.c          | 80 +++++++++++++++++++++++++++++++++++++++++
 tools/dtoc/test_dtoc.py |  4 +++
 6 files changed, 189 insertions(+), 2 deletions(-)

diff --git a/drivers/core/device.c b/drivers/core/device.c
index 7f71570a940..d9ce546c0c4 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -674,6 +674,71 @@ void *dev_get_parent_priv(const struct udevice *dev)
 	return dm_priv_to_rw(dev->parent_priv_);
 }
 
+void *dev_get_attach_ptr(const struct udevice *dev, enum dm_tag_t tag)
+{
+	switch (tag) {
+	case DM_TAG_PLAT:
+		return dev_get_plat(dev);
+	case DM_TAG_PARENT_PLAT:
+		return dev_get_parent_plat(dev);
+	case DM_TAG_UC_PLAT:
+		return dev_get_uclass_plat(dev);
+	case DM_TAG_PRIV:
+		return dev_get_priv(dev);
+	case DM_TAG_PARENT_PRIV:
+		return dev_get_parent_priv(dev);
+	case DM_TAG_UC_PRIV:
+		return dev_get_uclass_priv(dev);
+	default:
+		return NULL;
+	}
+}
+
+int dev_get_attach_size(const struct udevice *dev, enum dm_tag_t tag)
+{
+	const struct udevice *parent = dev_get_parent(dev);
+	const struct uclass *uc = dev->uclass;
+	const struct uclass_driver *uc_drv = uc->uc_drv;
+	const struct driver *parent_drv = NULL;
+	int size = 0;
+
+	if (parent)
+		parent_drv = parent->driver;
+
+	switch (tag) {
+	case DM_TAG_PLAT:
+		size = dev->driver->plat_auto;
+		break;
+	case DM_TAG_PARENT_PLAT:
+		if (parent) {
+			size = parent_drv->per_child_plat_auto;
+			if (!size)
+				size = parent->uclass->uc_drv->per_child_plat_auto;
+		}
+		break;
+	case DM_TAG_UC_PLAT:
+		size = uc_drv->per_device_plat_auto;
+		break;
+	case DM_TAG_PRIV:
+		size = dev->driver->priv_auto;
+		break;
+	case DM_TAG_PARENT_PRIV:
+		if (parent) {
+			size = parent_drv->per_child_auto;
+			if (!size)
+				size = parent->uclass->uc_drv->per_child_auto;
+		}
+		break;
+	case DM_TAG_UC_PRIV:
+		size = uc_drv->per_device_auto;
+		break;
+	default:
+		break;
+	}
+
+	return size;
+}
+
 static int device_get_device_tail(struct udevice *dev, int ret,
 				  struct udevice **devp)
 {
diff --git a/drivers/misc/test_drv.c b/drivers/misc/test_drv.c
index b6df1189032..927618256f0 100644
--- a/drivers/misc/test_drv.c
+++ b/drivers/misc/test_drv.c
@@ -108,7 +108,9 @@ UCLASS_DRIVER(testbus) = {
 	.child_pre_probe = testbus_child_pre_probe_uclass,
 	.child_post_probe = testbus_child_post_probe_uclass,
 
-	/* This is for dtoc testing only */
+	.per_device_auto   = sizeof(struct dm_test_uclass_priv),
+
+	/* Note: this is for dtoc testing as well as tags*/
 	.per_device_plat_auto   = sizeof(struct dm_test_uclass_plat),
 };
 
diff --git a/include/dm/device.h b/include/dm/device.h
index 5bdb10653f8..12c6ba37ff3 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -11,6 +11,7 @@
 #define _DM_DEVICE_H
 
 #include <dm/ofnode.h>
+#include <dm/tag.h>
 #include <dm/uclass-id.h>
 #include <fdtdec.h>
 #include <linker_lists.h>
@@ -546,6 +547,30 @@ void *dev_get_parent_priv(const struct udevice *dev);
  */
 void *dev_get_uclass_priv(const struct udevice *dev);
 
+/**
+ * dev_get_attach_ptr() - Get the value of an attached pointed tag
+ *
+ * The tag is assumed to hold a pointer, if it exists
+ *
+ * @dev: Device to look at
+ * @tag: Tag to access
+ * @return value of tag, or NULL if there is no tag of this type
+ */
+void *dev_get_attach_ptr(const struct udevice *dev, enum dm_tag_t tag);
+
+/**
+ * dev_get_attach_size() - Get the size of an attached tag
+ *
+ * Core tags have an automatic-allocation mechanism where the allocated size is
+ * defined by the device, parent or uclass. This returns the size associated
+ * with a particular tag
+ *
+ * @dev: Device to look at
+ * @tag: Tag to access
+ * @return size of auto-allocated data, 0 if none
+ */
+int dev_get_attach_size(const struct udevice *dev, enum dm_tag_t tag);
+
 /**
  * dev_get_parent() - Get the parent of a device
  *
diff --git a/include/dm/tag.h b/include/dm/tag.h
index 54fc31eb153..9cb5d68f0a3 100644
--- a/include/dm/tag.h
+++ b/include/dm/tag.h
@@ -13,8 +13,19 @@
 struct udevice;
 
 enum dm_tag_t {
+	/* Types of core tags that can be attached to devices */
+	DM_TAG_PLAT,
+	DM_TAG_PARENT_PLAT,
+	DM_TAG_UC_PLAT,
+
+	DM_TAG_PRIV,
+	DM_TAG_PARENT_PRIV,
+	DM_TAG_UC_PRIV,
+	DM_TAG_DRIVER_DATA,
+	DM_TAG_ATTACH_COUNT,
+
 	/* EFI_LOADER */
-	DM_TAG_EFI = 0,
+	DM_TAG_EFI = DM_TAG_ATTACH_COUNT,
 
 	DM_TAG_COUNT,
 };
diff --git a/test/dm/core.c b/test/dm/core.c
index ebd504427d1..26e2fd56619 100644
--- a/test/dm/core.c
+++ b/test/dm/core.c
@@ -1275,3 +1275,83 @@ static int dm_test_uclass_find_device(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_uclass_find_device, UT_TESTF_SCAN_FDT);
+
+/* Test getting information about tags attached to devices */
+static int dm_test_dev_get_attach(struct unit_test_state *uts)
+{
+	struct udevice *dev;
+
+	ut_assertok(uclass_first_device_err(UCLASS_TEST_FDT, &dev));
+	ut_asserteq_str("a-test", dev->name);
+
+	ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PLAT));
+	ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PRIV));
+	ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_UC_PRIV));
+	ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_UC_PLAT));
+	ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PLAT));
+	ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PRIV));
+
+	ut_asserteq(sizeof(struct dm_test_pdata),
+		    dev_get_attach_size(dev, DM_TAG_PLAT));
+	ut_asserteq(sizeof(struct dm_test_priv),
+		    dev_get_attach_size(dev, DM_TAG_PRIV));
+	ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_UC_PRIV));
+	ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_UC_PLAT));
+	ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PLAT));
+	ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PRIV));
+
+	return 0;
+}
+DM_TEST(dm_test_dev_get_attach, UT_TESTF_SCAN_FDT);
+
+/* Test getting information about tags attached to bus devices */
+static int dm_test_dev_get_attach_bus(struct unit_test_state *uts)
+{
+	struct udevice *dev, *child;
+
+	ut_assertok(uclass_first_device_err(UCLASS_TEST_BUS, &dev));
+	ut_asserteq_str("some-bus", dev->name);
+
+	ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PLAT));
+	ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PRIV));
+	ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_UC_PRIV));
+	ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_UC_PLAT));
+	ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PLAT));
+	ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PRIV));
+
+	ut_asserteq(sizeof(struct dm_test_pdata),
+		    dev_get_attach_size(dev, DM_TAG_PLAT));
+	ut_asserteq(sizeof(struct dm_test_priv),
+		    dev_get_attach_size(dev, DM_TAG_PRIV));
+	ut_asserteq(sizeof(struct dm_test_uclass_priv),
+		    dev_get_attach_size(dev, DM_TAG_UC_PRIV));
+	ut_asserteq(sizeof(struct dm_test_uclass_plat),
+		    dev_get_attach_size(dev, DM_TAG_UC_PLAT));
+	ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PLAT));
+	ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PRIV));
+
+	/* Now try the child of the bus */
+	ut_assertok(device_first_child_err(dev, &child));
+	ut_asserteq_str("c-test@5", child->name);
+
+	ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PLAT));
+	ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PRIV));
+	ut_assertnull(dev_get_attach_ptr(child, DM_TAG_UC_PRIV));
+	ut_assertnull(dev_get_attach_ptr(child, DM_TAG_UC_PLAT));
+	ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PARENT_PLAT));
+	ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PARENT_PRIV));
+
+	ut_asserteq(sizeof(struct dm_test_pdata),
+		    dev_get_attach_size(child, DM_TAG_PLAT));
+	ut_asserteq(sizeof(struct dm_test_priv),
+		    dev_get_attach_size(child, DM_TAG_PRIV));
+	ut_asserteq(0, dev_get_attach_size(child, DM_TAG_UC_PRIV));
+	ut_asserteq(0, dev_get_attach_size(child, DM_TAG_UC_PLAT));
+	ut_asserteq(sizeof(struct dm_test_parent_plat),
+		    dev_get_attach_size(child, DM_TAG_PARENT_PLAT));
+	ut_asserteq(sizeof(struct dm_test_parent_data),
+		    dev_get_attach_size(child, DM_TAG_PARENT_PRIV));
+
+	return 0;
+}
+DM_TEST(dm_test_dev_get_attach_bus, UT_TESTF_SCAN_FDT);
diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
index 8bac2076214..879ca2ab2bf 100755
--- a/tools/dtoc/test_dtoc.py
+++ b/tools/dtoc/test_dtoc.py
@@ -618,6 +618,9 @@ u8 _denx_u_boot_test_bus_priv_some_bus[sizeof(struct dm_test_priv)]
 #include <dm/test.h>
 u8 _denx_u_boot_test_bus_ucplat_some_bus[sizeof(struct dm_test_uclass_plat)]
 \t__attribute__ ((section (".priv_data")));
+#include <dm/test.h>
+u8 _denx_u_boot_test_bus_uc_priv_some_bus[sizeof(struct dm_test_uclass_priv)]
+	__attribute__ ((section (".priv_data")));
 #include <test.h>
 
 DM_DEVICE_INST(some_bus) = {
@@ -628,6 +631,7 @@ DM_DEVICE_INST(some_bus) = {
 \t.driver_data\t= DM_TEST_TYPE_FIRST,
 \t.priv_\t\t= _denx_u_boot_test_bus_priv_some_bus,
 \t.uclass\t\t= DM_UCLASS_REF(testbus),
+\t.uclass_priv_ = _denx_u_boot_test_bus_uc_priv_some_bus,
 \t.uclass_node\t= {
 \t\t.prev = &DM_UCLASS_REF(testbus)->dev_head,
 \t\t.next = &DM_UCLASS_REF(testbus)->dev_head,

From 0dfda34ca594c701955cfcb71711a7599f97bae3 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sun, 8 May 2022 04:39:25 -0600
Subject: [PATCH 23/34] dm: core: Add a way to collect memory usage

Add a function for collecting the amount of memory used by driver model,
including devices, uclasses and attached data and tags.

This information can provide insights into how to reduce the memory
required by driver model. Future work may look at execution speed also.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 drivers/core/root.c | 53 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/core/tag.c  | 11 ++++++++++
 include/dm/root.h   | 45 ++++++++++++++++++++++++++++++++++++++
 include/dm/tag.h    | 11 ++++++++++
 test/dm/core.c      | 11 ++++++++++
 5 files changed, 131 insertions(+)

diff --git a/drivers/core/root.c b/drivers/core/root.c
index 17dd1205a32..f24ddfa5218 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -449,6 +449,59 @@ void dm_get_stats(int *device_countp, int *uclass_countp)
 	*uclass_countp = uclass_get_count();
 }
 
+void dev_collect_stats(struct dm_stats *stats, const struct udevice *parent)
+{
+	const struct udevice *dev;
+	int i;
+
+	stats->dev_count++;
+	stats->dev_size += sizeof(struct udevice);
+	stats->dev_name_size += strlen(parent->name) + 1;
+	for (i = 0; i < DM_TAG_ATTACH_COUNT; i++) {
+		int size = dev_get_attach_size(parent, i);
+
+		if (size ||
+		    (i == DM_TAG_DRIVER_DATA && parent->driver_data)) {
+			stats->attach_count[i]++;
+			stats->attach_size[i] += size;
+			stats->attach_count_total++;
+			stats->attach_size_total += size;
+		}
+	}
+
+	list_for_each_entry(dev, &parent->child_head, sibling_node)
+		dev_collect_stats(stats, dev);
+}
+
+void uclass_collect_stats(struct dm_stats *stats)
+{
+	struct uclass *uc;
+
+	list_for_each_entry(uc, gd->uclass_root, sibling_node) {
+		int size;
+
+		stats->uc_count++;
+		stats->uc_size += sizeof(struct uclass);
+		size = uc->uc_drv->priv_auto;
+		if (size) {
+			stats->uc_attach_count++;
+			stats->uc_attach_size += size;
+		}
+	}
+}
+
+void dm_get_mem(struct dm_stats *stats)
+{
+	memset(stats, '\0', sizeof(*stats));
+	dev_collect_stats(stats, gd->dm_root);
+	uclass_collect_stats(stats);
+	dev_tag_collect_stats(stats);
+
+	stats->total_size = stats->dev_size + stats->uc_size +
+		stats->attach_size_total + stats->uc_attach_size +
+		stats->tag_size;
+}
+
 #ifdef CONFIG_ACPIGEN
 static int root_acpi_get_name(const struct udevice *dev, char *out_name)
 {
diff --git a/drivers/core/tag.c b/drivers/core/tag.c
index 22999193a5a..2961725b658 100644
--- a/drivers/core/tag.c
+++ b/drivers/core/tag.c
@@ -6,6 +6,7 @@
 
 #include <malloc.h>
 #include <asm/global_data.h>
+#include <dm/root.h>
 #include <dm/tag.h>
 #include <linux/err.h>
 #include <linux/list.h>
@@ -137,3 +138,13 @@ int dev_tag_del_all(struct udevice *dev)
 
 	return -ENOENT;
 }
+
+void dev_tag_collect_stats(struct dm_stats *stats)
+{
+	struct dmtag_node *node;
+
+	list_for_each_entry(node, &gd->dmtag_list, sibling) {
+		stats->tag_count++;
+		stats->tag_size += sizeof(struct dmtag_node);
+	}
+}
diff --git a/include/dm/root.h b/include/dm/root.h
index e888fb993c0..382f83c7f5b 100644
--- a/include/dm/root.h
+++ b/include/dm/root.h
@@ -9,11 +9,49 @@
 #ifndef _DM_ROOT_H_
 #define _DM_ROOT_H_
 
+#include <dm/tag.h>
+
 struct udevice;
 
 /* Head of the uclass list if CONFIG_OF_PLATDATA_INST is enabled */
 extern struct list_head uclass_head;
 
+/**
+ * struct dm_stats - Information about driver model memory usage
+ *
+ * @total_size: All data
+ * @dev_count: Number of devices
+ * @dev_size: Size of all devices (just the struct udevice)
+ * @dev_name_size: Bytes used by device names
+ * @uc_count: Number of uclasses
+ * @uc_size: Size of all uclasses (just the struct uclass)
+ * @tag_count: Number of tags
+ * @tag_size: Bytes used by all tags
+ * @uc_attach_count: Number of uclasses with attached data (priv)
+ * @uc_attach_size: Total size of that attached data
+ * @attach_count_total: Total number of attached data items for all udevices and
+ *	uclasses
+ * @attach_size_total: Total number of bytes of attached data
+ * @attach_count: Number of devices with attached, for each type
+ * @attach_size: Total number of bytes of attached data, for each type
+ */
+struct dm_stats {
+	int total_size;
+	int dev_count;
+	int dev_size;
+	int dev_name_size;
+	int uc_count;
+	int uc_size;
+	int tag_count;
+	int tag_size;
+	int uc_attach_count;
+	int uc_attach_size;
+	int attach_count_total;
+	int attach_size_total;
+	int attach_count[DM_TAG_ATTACH_COUNT];
+	int attach_size[DM_TAG_ATTACH_COUNT];
+};
+
 /**
  * dm_root() - Return pointer to the top of the driver tree
  *
@@ -141,4 +179,11 @@ static inline int dm_remove_devices_flags(uint flags) { return 0; }
  */
 void dm_get_stats(int *device_countp, int *uclass_countp);
 
+/**
+ * dm_get_mem() - Get stats on memory usage in driver model
+ *
+ * @mem: Place to put the information
+ */
+void dm_get_mem(struct dm_stats *stats);
+
 #endif
diff --git a/include/dm/tag.h b/include/dm/tag.h
index 9cb5d68f0a3..1ea3c9f7af3 100644
--- a/include/dm/tag.h
+++ b/include/dm/tag.h
@@ -10,6 +10,7 @@
 #include <linux/list.h>
 #include <linux/types.h>
 
+struct dm_stats;
 struct udevice;
 
 enum dm_tag_t {
@@ -118,4 +119,14 @@ int dev_tag_del(struct udevice *dev, enum dm_tag_t tag);
  */
 int dev_tag_del_all(struct udevice *dev);
 
+/**
+ * dev_tag_collect_stats() - Collect information on driver model performance
+ *
+ * This collects information on how driver model is performing. For now it only
+ * includes memory usage
+ *
+ * @stats: Place to put the collected information
+ */
+void dev_tag_collect_stats(struct dm_stats *stats);
+
 #endif /* _DM_TAG_H */
diff --git a/test/dm/core.c b/test/dm/core.c
index 26e2fd56619..fd4d7569728 100644
--- a/test/dm/core.c
+++ b/test/dm/core.c
@@ -1355,3 +1355,14 @@ static int dm_test_dev_get_attach_bus(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_dev_get_attach_bus, UT_TESTF_SCAN_FDT);
+
+/* Test getting information about tags attached to bus devices */
+static int dm_test_dev_get_mem(struct unit_test_state *uts)
+{
+	struct dm_stats stats;
+
+	dm_get_mem(&stats);
+
+	return 0;
+}
+DM_TEST(dm_test_dev_get_mem, UT_TESTF_SCAN_FDT);

From 2cb4ddb91ec9fcb77c895e4a1192a15aece700c6 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sun, 8 May 2022 04:39:26 -0600
Subject: [PATCH 24/34] dm: core: Add a command to show driver model statistics

This command shows the memory used by driver model along with various
hints as to what it might be if some 'core' tags were moved to use the
tag list instead of a core (i.e. always-there) pointer.

This may help with future work to reduce memory usage.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 cmd/dm.c             | 24 +++++++++++++++
 drivers/core/Kconfig | 11 +++++++
 drivers/core/dump.c  | 73 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/core/tag.c   | 18 +++++++++++
 include/dm/root.h    |  2 +-
 include/dm/tag.h     |  8 +++++
 include/dm/util.h    |  9 ++++++
 7 files changed, 144 insertions(+), 1 deletion(-)

diff --git a/cmd/dm.c b/cmd/dm.c
index 0c7554a1b1d..eb40f0865fe 100644
--- a/cmd/dm.c
+++ b/cmd/dm.c
@@ -8,6 +8,7 @@
 
 #include <common.h>
 #include <command.h>
+#include <dm/root.h>
 #include <dm/util.h>
 
 static int do_dm_dump_driver_compat(struct cmd_tbl *cmdtp, int flag, int argc,
@@ -34,6 +35,19 @@ static int do_dm_dump_drivers(struct cmd_tbl *cmdtp, int flag, int argc,
 	return 0;
 }
 
+#if CONFIG_IS_ENABLED(DM_STATS)
+static int do_dm_dump_mem(struct cmd_tbl *cmdtp, int flag, int argc,
+			  char *const argv[])
+{
+	struct dm_stats mem;
+
+	dm_get_mem(&mem);
+	dm_dump_mem(&mem);
+
+	return 0;
+}
+#endif /* DM_STATS */
+
 static int do_dm_dump_static_driver_info(struct cmd_tbl *cmdtp, int flag,
 					 int argc, char * const argv[])
 {
@@ -58,11 +72,20 @@ static int do_dm_dump_uclass(struct cmd_tbl *cmdtp, int flag, int argc,
 	return 0;
 }
 
+#if CONFIG_IS_ENABLED(DM_STATS)
+#define DM_MEM_HELP	"dm mem           Provide a summary of memory usage\n"
+#define DM_MEM		U_BOOT_SUBCMD_MKENT(mem, 1, 1, do_dm_dump_mem),
+#else
+#define DM_MEM_HELP
+#define DM_MEM
+#endif
+
 #if CONFIG_IS_ENABLED(SYS_LONGHELP)
 static char dm_help_text[] =
 	"compat        Dump list of drivers with compatibility strings\n"
 	"dm devres        Dump list of device resources for each device\n"
 	"dm drivers       Dump list of drivers with uclass and instances\n"
+	DM_MEM_HELP
 	"dm static        Dump list of drivers with static platform data\n"
 	"dn tree          Dump tree of driver model devices ('*' = activated)\n"
 	"dm uclass        Dump list of instances for each uclass"
@@ -73,6 +96,7 @@ U_BOOT_CMD_WITH_SUBCMDS(dm, "Driver model low level access", dm_help_text,
 	U_BOOT_SUBCMD_MKENT(compat, 1, 1, do_dm_dump_driver_compat),
 	U_BOOT_SUBCMD_MKENT(devres, 1, 1, do_dm_dump_devres),
 	U_BOOT_SUBCMD_MKENT(drivers, 1, 1, do_dm_dump_drivers),
+	DM_MEM
 	U_BOOT_SUBCMD_MKENT(static, 1, 1, do_dm_dump_static_driver_info),
 	U_BOOT_SUBCMD_MKENT(tree, 1, 1, do_dm_dump_tree),
 	U_BOOT_SUBCMD_MKENT(uclass, 1, 1, do_dm_dump_uclass));
diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index 9b9a7148a1a..97dc699e969 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -75,6 +75,17 @@ config DM_DEBUG
 	help
 	  Say Y here if you want to compile in debug messages in DM core.
 
+config DM_STATS
+	bool "Collect and show driver model stats"
+	depends on DM
+	default y if SANDBOX
+	help
+	  Enable this to collect and display memory statistics about driver
+	  model. This can help to figure out where all the memory is going and
+	  to find optimisations.
+
+	  To display the memory stats, use the 'dm mem' command.
+
 config DM_DEVICE_REMOVE
 	bool "Support device removal"
 	depends on DM
diff --git a/drivers/core/dump.c b/drivers/core/dump.c
index e434fe04728..1c1f7e4d308 100644
--- a/drivers/core/dump.c
+++ b/drivers/core/dump.c
@@ -172,3 +172,76 @@ void dm_dump_static_driver_info(void)
 	for (entry = drv; entry != drv + n_ents; entry++)
 		printf("%-25.25s %p\n", entry->name, entry->plat);
 }
+
+void dm_dump_mem(struct dm_stats *stats)
+{
+	int total, total_delta;
+	int i;
+
+	/* Support SPL printf() */
+	printf("Struct sizes: udevice %x, driver %x, uclass %x, uc_driver %x\n",
+	       (int)sizeof(struct udevice), (int)sizeof(struct driver),
+	       (int)sizeof(struct uclass), (int)sizeof(struct uclass_driver));
+	printf("Memory: device %x:%x, device names %x, uclass %x:%x\n",
+	       stats->dev_count, stats->dev_size, stats->dev_name_size,
+	       stats->uc_count, stats->uc_size);
+	printf("\n");
+	printf("%-15s  %5s  %5s  %5s  %5s  %5s\n", "Attached type", "Count",
+	       "Size", "Cur", "Tags", "Save");
+	printf("%-15s  %5s  %5s  %5s  %5s  %5s\n", "---------------", "-----",
+	       "-----", "-----", "-----", "-----");
+	total_delta = 0;
+	for (i = 0; i < DM_TAG_ATTACH_COUNT; i++) {
+		int cur_size, new_size, delta;
+
+		cur_size = stats->dev_count * sizeof(struct udevice);
+		new_size = stats->dev_count * (sizeof(struct udevice) -
+			sizeof(void *));
+		/*
+		 * Let's assume we can fit each dmtag_node into 32 bits. We can
+		 * limit the 'tiny tags' feature to SPL with
+		 * CONFIG_SPL_SYS_MALLOC_F_LEN <= 64KB, so needing 14 bits to
+		 * point to anything in that region (with 4-byte alignment).
+		 * So:
+		 *    4 bits for tag
+		 *    14 bits for offset of dev
+		 *    14 bits for offset of data
+		 */
+		new_size += stats->attach_count[i] * sizeof(u32);
+		delta = cur_size - new_size;
+		total_delta += delta;
+		printf("%-16s %5x %6x %6x %6x %6x (%d)\n", tag_get_name(i),
+		       stats->attach_count[i], stats->attach_size[i],
+		       cur_size, new_size, delta > 0 ? delta : 0, delta);
+	}
+	printf("%-16s %5x %6x\n", "uclass", stats->uc_attach_count,
+	       stats->uc_attach_size);
+	printf("%-16s %5x %6x  %5s  %5s  %6x (%d)\n", "Attached total",
+	       stats->attach_count_total + stats->uc_attach_count,
+	       stats->attach_size_total + stats->uc_attach_size, "", "",
+	       total_delta > 0 ? total_delta : 0, total_delta);
+	printf("%-16s %5x %6x\n", "tags", stats->tag_count, stats->tag_size);
+	printf("\n");
+	printf("Total size: %x (%d)\n", stats->total_size, stats->total_size);
+	printf("\n");
+
+	total = stats->total_size;
+	total -= total_delta;
+	printf("With tags:       %x (%d)\n", total, total);
+
+	/* Use singly linked lists in struct udevice (3 nodes in each) */
+	total -= sizeof(void *) * 3 * stats->dev_count;
+	printf("- singly-linked: %x (%d)\n", total, total);
+
+	/* Use an index into the struct_driver list instead of a pointer */
+	total = total + stats->dev_count * (1 - sizeof(void *));
+	printf("- driver index:  %x (%d)\n", total, total);
+
+	/* Same with the uclass */
+	total = total + stats->dev_count * (1 - sizeof(void *));
+	printf("- uclass index:  %x (%d)\n", total, total);
+
+	/* Drop the device name */
+	printf("Drop device name (not SRAM): %x (%d)\n", stats->dev_name_size,
+	       stats->dev_name_size);
+}
diff --git a/drivers/core/tag.c b/drivers/core/tag.c
index 2961725b658..a3c5cb7e57c 100644
--- a/drivers/core/tag.c
+++ b/drivers/core/tag.c
@@ -16,6 +16,24 @@ struct udevice;
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static const char *const tag_name[] = {
+	[DM_TAG_PLAT]		= "plat",
+	[DM_TAG_PARENT_PLAT]	= "parent_plat",
+	[DM_TAG_UC_PLAT]	= "uclass_plat",
+
+	[DM_TAG_PRIV]		= "priv",
+	[DM_TAG_PARENT_PRIV]	= "parent_priv",
+	[DM_TAG_UC_PRIV]	= "uclass_priv",
+	[DM_TAG_DRIVER_DATA]	= "driver_data",
+
+	[DM_TAG_EFI]		= "efi",
+};
+
+const char *tag_get_name(enum dm_tag_t tag)
+{
+	return tag_name[tag];
+}
+
 int dev_tag_set_ptr(struct udevice *dev, enum dm_tag_t tag, void *ptr)
 {
 	struct dmtag_node *node;
diff --git a/include/dm/root.h b/include/dm/root.h
index 382f83c7f5b..b2f30a842f5 100644
--- a/include/dm/root.h
+++ b/include/dm/root.h
@@ -182,7 +182,7 @@ void dm_get_stats(int *device_countp, int *uclass_countp);
 /**
  * dm_get_mem() - Get stats on memory usage in driver model
  *
- * @mem: Place to put the information
+ * @stats: Place to put the information
  */
 void dm_get_mem(struct dm_stats *stats);
 
diff --git a/include/dm/tag.h b/include/dm/tag.h
index 1ea3c9f7af3..745088ffcff 100644
--- a/include/dm/tag.h
+++ b/include/dm/tag.h
@@ -129,4 +129,12 @@ int dev_tag_del_all(struct udevice *dev);
  */
 void dev_tag_collect_stats(struct dm_stats *stats);
 
+/**
+ * tag_get_name() - Get the name of a tag
+ *
+ * @tag: Tag to look up, which must be valid
+ * Returns: Name of tag
+ */
+const char *tag_get_name(enum dm_tag_t tag);
+
 #endif /* _DM_TAG_H */
diff --git a/include/dm/util.h b/include/dm/util.h
index c52daa87ef3..e10c6060ce0 100644
--- a/include/dm/util.h
+++ b/include/dm/util.h
@@ -6,6 +6,8 @@
 #ifndef __DM_UTIL_H
 #define __DM_UTIL_H
 
+struct dm_stats;
+
 #if CONFIG_IS_ENABLED(DM_WARN)
 #define dm_warn(fmt...) log(LOGC_DM, LOGL_WARNING, ##fmt)
 #else
@@ -48,6 +50,13 @@ void dm_dump_driver_compat(void);
 /* Dump out a list of drivers with static platform data */
 void dm_dump_static_driver_info(void);
 
+/**
+ * dm_dump_mem() - Dump stats on memory usage in driver model
+ *
+ * @mem: Stats to dump
+ */
+void dm_dump_mem(struct dm_stats *stats);
+
 #if CONFIG_IS_ENABLED(OF_PLATDATA_INST) && CONFIG_IS_ENABLED(READ_ONLY)
 void *dm_priv_to_rw(void *priv);
 #else

From 4f6500aa1a62a80e8df2ffdf16fe4c3eabd99f1c Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sun, 8 May 2022 04:39:27 -0600
Subject: [PATCH 25/34] dm: spl: Allow SPL to show memory usage

Add an option to tell SPL to show memory usage for driver model just
before it boots into the next phase.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 common/spl/spl.c     |  9 +++++++++
 drivers/core/Kconfig | 10 ++++++++++
 2 files changed, 19 insertions(+)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 2a69a7c9324..dff4eef7071 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -34,6 +34,7 @@
 #include <malloc.h>
 #include <mapmem.h>
 #include <dm/root.h>
+#include <dm/util.h>
 #include <linux/compiler.h>
 #include <fdt_support.h>
 #include <bootcount.h>
@@ -780,6 +781,14 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 
 	bootcount_inc();
 
+	/* Dump driver model states to aid analysis */
+	if (CONFIG_IS_ENABLED(DM_STATS)) {
+		struct dm_stats mem;
+
+		dm_get_mem(&mem);
+		dm_dump_mem(&mem);
+	}
+
 	memset(&spl_image, '\0', sizeof(spl_image));
 #ifdef CONFIG_SYS_SPL_ARGS_ADDR
 	spl_image.arg = (void *)CONFIG_SYS_SPL_ARGS_ADDR;
diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index 97dc699e969..99e28713f9b 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -86,6 +86,16 @@ config DM_STATS
 
 	  To display the memory stats, use the 'dm mem' command.
 
+config SPL_DM_STATS
+	bool "Collect and show driver model stats in SPL"
+	depends on DM_SPL
+	help
+	  Enable this to collect and display memory statistics about driver
+	  model. This can help to figure out where all the memory is going and
+	  to find optimisations.
+
+	  The stats are displayed just before SPL boots to the next phase.
+
 config DM_DEVICE_REMOVE
 	bool "Support device removal"
 	depends on DM

From d4462ba0443d4ea7de290b2ccf90b2a1c0122b93 Mon Sep 17 00:00:00 2001
From: Heinrich Schuchardt <xypron.glpk@gmx.de>
Date: Sun, 12 Jun 2022 11:25:22 +0000
Subject: [PATCH 26/34] sandbox: cast to pointer from integer of different size

Building sandbox_defconfig on ARMv7 with HOST_32BIT=y results in:

drivers/misc/qfw_sandbox.c:51:25: warning:
cast to pointer from integer of different size [-Wint-to-pointer-cast]
   51 |         void *address = (void *)be64_to_cpu(dma->address);

Add the missing type conversion.

Fixes: 69512551aa84 ("test: qemu: add qfw sandbox driver, dm tests, qemu tests")
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
 drivers/misc/qfw_sandbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/qfw_sandbox.c b/drivers/misc/qfw_sandbox.c
index b09974d33bd..1002df75339 100644
--- a/drivers/misc/qfw_sandbox.c
+++ b/drivers/misc/qfw_sandbox.c
@@ -48,7 +48,7 @@ static void qfw_sandbox_read_entry_dma(struct udevice *dev, struct qfw_dma *dma)
 {
 	u16 entry;
 	u32 control = be32_to_cpu(dma->control);
-	void *address = (void *)be64_to_cpu(dma->address);
+	void *address = (void *)(uintptr_t)be64_to_cpu(dma->address);
 	u32 length = be32_to_cpu(dma->length);
 	struct qfw_sandbox_plat *plat = dev_get_plat(dev);
 	struct fw_cfg_file *file;

From d7f071725287a5462c57242bf04a752cd5ddfff1 Mon Sep 17 00:00:00 2001
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Date: Sat, 18 Jun 2022 15:13:06 +0300
Subject: [PATCH 27/34] spl: binman: Fix use of undeclared u_boot_any symbols

Some SPL functions directly use the binman 'u_boot_any' symbols to get
U-Boot's binman image position. These symbols are declared by the
SPL/TPL_BINMAN_SYMBOLS configs, but they are accessed by macros defined
by just CONFIG_BINMAN. So when BINMAN is enabled and BINMAN_SYMBOLS is
disabled, the code tries to use undeclared symbols and we get an error.

Therefore, any use of 'u_boot_any' symbols in the code is an implicit
dependency on SPL/TPL_BINMAN_SYMBOLS. However, in the current uses
they are meant to be the next phase's values, where that happens to be
U-Boot. In the meantime, helper funcions spl_get_image_pos/size() were
introduced to get these values.

Convert all uses of u_boot_any symbols to these functions, so we only
access these symbols at one place. Make sure they will not use these
symbols when the BINMAN_SYMBOLS configs are disabled, by returning early
in those cases.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---
 common/spl/spl.c     | 10 +++++++---
 common/spl/spl_ram.c |  2 +-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index dff4eef7071..75051656249 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -150,9 +150,11 @@ void spl_fixup_fdt(void *fdt_blob)
 #endif
 }
 
-#if CONFIG_IS_ENABLED(BINMAN_SYMBOLS)
 ulong spl_get_image_pos(void)
 {
+	if (!CONFIG_IS_ENABLED(BINMAN_SYMBOLS))
+		return BINMAN_SYM_MISSING;
+
 #ifdef CONFIG_VPL
 	if (spl_next_phase() == PHASE_VPL)
 		return binman_sym(ulong, u_boot_vpl, image_pos);
@@ -164,6 +166,9 @@ ulong spl_get_image_pos(void)
 
 ulong spl_get_image_size(void)
 {
+	if (!CONFIG_IS_ENABLED(BINMAN_SYMBOLS))
+		return BINMAN_SYM_MISSING;
+
 #ifdef CONFIG_VPL
 	if (spl_next_phase() == PHASE_VPL)
 		return binman_sym(ulong, u_boot_vpl, size);
@@ -172,7 +177,6 @@ ulong spl_get_image_size(void)
 		binman_sym(ulong, u_boot_spl, size) :
 		binman_sym(ulong, u_boot_any, size);
 }
-#endif /* BINMAN_SYMBOLS */
 
 ulong spl_get_image_text_base(void)
 {
@@ -223,7 +227,7 @@ __weak struct image_header *spl_get_load_buffer(ssize_t offset, size_t size)
 
 void spl_set_header_raw_uboot(struct spl_image_info *spl_image)
 {
-	ulong u_boot_pos = binman_sym(ulong, u_boot_any, image_pos);
+	ulong u_boot_pos = spl_get_image_pos();
 
 	spl_image->size = CONFIG_SYS_MONITOR_LEN;
 
diff --git a/common/spl/spl_ram.c b/common/spl/spl_ram.c
index 82964592571..d64710878cf 100644
--- a/common/spl/spl_ram.c
+++ b/common/spl/spl_ram.c
@@ -70,7 +70,7 @@ static int spl_ram_load_image(struct spl_image_info *spl_image,
 		load.read = spl_ram_load_read;
 		spl_load_simple_fit(spl_image, &load, 0, header);
 	} else {
-		ulong u_boot_pos = binman_sym(ulong, u_boot_any, image_pos);
+		ulong u_boot_pos = spl_get_image_pos();
 
 		debug("Legacy image\n");
 		/*

From e09a8b9f1b7d16052f5b56303411bdb5388a8044 Mon Sep 17 00:00:00 2001
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Date: Sat, 18 Jun 2022 15:13:07 +0300
Subject: [PATCH 28/34] spl: binman: Make TPL_BINMAN_SYMBOLS depend on
 TPL_FRAMEWORK

TPL_BINMAN_SYMBOLS depends on SPL_FRAMEWORK. The code this enables is
compiled by checking CONFIG_$(SPL_TPL_)FRAMEWORK, so it should depend on
TPL_FRAMEWORK instead (which in turn depends on SPL_FRAMEWORK). This was
most likely a typo due to copy-pasting the config's SPL version, fix it.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---
 common/spl/Kconfig.tpl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/spl/Kconfig.tpl b/common/spl/Kconfig.tpl
index 9a0e719cf94..834cb6b6dd8 100644
--- a/common/spl/Kconfig.tpl
+++ b/common/spl/Kconfig.tpl
@@ -10,7 +10,7 @@ config TPL_SIZE_LIMIT
 
 config TPL_BINMAN_SYMBOLS
 	bool "Declare binman symbols in TPL"
-	depends on SPL_FRAMEWORK && BINMAN
+	depends on TPL_FRAMEWORK && BINMAN
 	default y
 	help
 	  This enables use of symbols in TPL which refer to U-Boot, enabling TPL

From 5204823c81c2be4ef3abfcaae351401f8dd5f058 Mon Sep 17 00:00:00 2001
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Date: Sat, 18 Jun 2022 15:13:08 +0300
Subject: [PATCH 29/34] spl: binman: Declare extern symbols for VPL as well

The binman extern symbol declarations in spl.h are missing the VPL
symbols recently added to spl.c, add them like the others.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---
 include/spl.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/spl.h b/include/spl.h
index 83ac583e0b4..1778e0f5368 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -288,6 +288,8 @@ binman_sym_extern(ulong, u_boot_any, image_pos);
 binman_sym_extern(ulong, u_boot_any, size);
 binman_sym_extern(ulong, u_boot_spl, image_pos);
 binman_sym_extern(ulong, u_boot_spl, size);
+binman_sym_extern(ulong, u_boot_vpl, image_pos);
+binman_sym_extern(ulong, u_boot_vpl, size);
 
 /**
  * spl_get_image_pos() - get the image position of the next phase

From d8830cf84035120562bb490be276fab9e43d6414 Mon Sep 17 00:00:00 2001
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Date: Sat, 18 Jun 2022 15:13:09 +0300
Subject: [PATCH 30/34] spl: binman: Split binman symbols support from enabling
 binman

Enabling CONFIG_BINMAN makes binman run after a build to package any
images specified in the device-tree. It also enables a mechanism for
SPL/TPL to declare and use special linker symbols that refer to other
entries in the same binman image. A similar feature that gets this info
from the device-tree exists for U-Boot proper, but it is gated behind a
CONFIG_BINMAN_FDT unlike the symbols.

Confusingly, CONFIG_SPL/TPL_BINMAN_SYMBOLS also exist. These configs
don't actually enable/disable the symbols mechanism as one would expect,
but declare some symbols for U-Boot using this mechanism.

Reuse the BINMAN_SYMBOLS configs to make them toggle the symbols
mechanism, and declare symbols for the U-Boot phases in a dependent
BINMAN_UBOOT_SYMBOLS config. Extend it to cover symbols of all phases.
Update the config prompt and help message to make it clearer about this.
Fix binman test binaries to work with CONFIG_IS_ENABLED(BINMAN_SYMBOLS).

Co-developed-by: Peng Fan <peng.fan@nxp.com>
[Alper: New config for phase symbols, update Kconfigs, commit message]
Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---
 common/spl/Kconfig                          | 22 ++++++++++++++-----
 common/spl/Kconfig.tpl                      | 24 +++++++++++++++------
 common/spl/spl.c                            |  9 ++++----
 include/binman_sym.h                        |  6 +++---
 tools/binman/test/Makefile                  |  2 +-
 tools/binman/test/generated/autoconf.h      |  3 +++
 tools/binman/test/u_boot_binman_syms.c      |  2 +-
 tools/binman/test/u_boot_binman_syms_size.c |  2 +-
 8 files changed, 49 insertions(+), 21 deletions(-)
 create mode 100644 tools/binman/test/generated/autoconf.h

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 2ad2351c6eb..46d9be73bb1 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -190,12 +190,24 @@ config SPL_BINMAN_SYMBOLS
 	depends on SPL_FRAMEWORK && BINMAN
 	default y
 	help
-	  This enables use of symbols in SPL which refer to U-Boot, enabling SPL
-	  to obtain the location of U-Boot simply by calling spl_get_image_pos()
-	  and spl_get_image_size().
+	  This enables use of symbols in SPL which refer to other entries in
+	  the same binman image as the SPL. These can be declared with the
+	  binman_sym_declare(type, entry, prop) macro and accessed by the
+	  binman_sym(type, entry, prop) macro defined in binman_sym.h.
 
-	  For this to work, you must have a U-Boot image in the binman image, so
-	  binman can update SPL with the location of it.
+	  See tools/binman/binman.rst for a detailed explanation.
+
+config SPL_BINMAN_UBOOT_SYMBOLS
+	bool "Declare binman symbols for U-Boot phases in SPL"
+	depends on SPL_BINMAN_SYMBOLS
+	default y
+	help
+	  This enables use of symbols in SPL which refer to U-Boot phases,
+	  enabling SPL to obtain the location and size of its next phase simply
+	  by calling spl_get_image_pos() and spl_get_image_size().
+
+	  For this to work, you must have all U-Boot phases in the same binman
+	  image, so binman can update SPL with the locations of everything.
 
 source "common/spl/Kconfig.nxp"
 
diff --git a/common/spl/Kconfig.tpl b/common/spl/Kconfig.tpl
index 834cb6b6dd8..8c59c767302 100644
--- a/common/spl/Kconfig.tpl
+++ b/common/spl/Kconfig.tpl
@@ -9,16 +9,28 @@ config TPL_SIZE_LIMIT
 	  If this value is zero, it is ignored.
 
 config TPL_BINMAN_SYMBOLS
-	bool "Declare binman symbols in TPL"
+	bool "Support binman symbols in TPL"
 	depends on TPL_FRAMEWORK && BINMAN
 	default y
 	help
-	  This enables use of symbols in TPL which refer to U-Boot, enabling TPL
-	  to obtain the location of U-Boot simply by calling spl_get_image_pos()
-	  and spl_get_image_size().
+	  This enables use of symbols in TPL which refer to other entries in
+	  the same binman image as the TPL. These can be declared with the
+	  binman_sym_declare(type, entry, prop) macro and accessed by the
+	  binman_sym(type, entry, prop) macro defined in binman_sym.h.
 
-	  For this to work, you must have a U-Boot image in the binman image, so
-	  binman can update TPL with the location of it.
+	  See tools/binman/binman.rst for a detailed explanation.
+
+config TPL_BINMAN_UBOOT_SYMBOLS
+	bool "Declare binman symbols for U-Boot phases in TPL"
+	depends on TPL_BINMAN_SYMBOLS
+	default y
+	help
+	  This enables use of symbols in TPL which refer to U-Boot phases,
+	  enabling TPL to obtain the location and size of its next phase simply
+	  by calling spl_get_image_pos() and spl_get_image_size().
+
+	  For this to work, you must have all U-Boot phases in the same binman
+	  image, so binman can update TPL with the locations of everything.
 
 config TPL_FRAMEWORK
 	bool "Support TPL based upon the common SPL framework"
diff --git a/common/spl/spl.c b/common/spl/spl.c
index 75051656249..5dd122611c2 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -52,11 +52,10 @@ DECLARE_GLOBAL_DATA_PTR;
 
 u32 *boot_params_ptr = NULL;
 
-#if CONFIG_IS_ENABLED(BINMAN_SYMBOLS)
+#if CONFIG_IS_ENABLED(BINMAN_UBOOT_SYMBOLS)
 /* See spl.h for information about this */
 binman_sym_declare(ulong, u_boot_any, image_pos);
 binman_sym_declare(ulong, u_boot_any, size);
-#endif
 
 #ifdef CONFIG_TPL
 binman_sym_declare(ulong, u_boot_spl, image_pos);
@@ -68,6 +67,8 @@ binman_sym_declare(ulong, u_boot_vpl, image_pos);
 binman_sym_declare(ulong, u_boot_vpl, size);
 #endif
 
+#endif /* BINMAN_UBOOT_SYMBOLS */
+
 /* Define board data structure */
 static struct bd_info bdata __attribute__ ((section(".data")));
 
@@ -152,7 +153,7 @@ void spl_fixup_fdt(void *fdt_blob)
 
 ulong spl_get_image_pos(void)
 {
-	if (!CONFIG_IS_ENABLED(BINMAN_SYMBOLS))
+	if (!CONFIG_IS_ENABLED(BINMAN_UBOOT_SYMBOLS))
 		return BINMAN_SYM_MISSING;
 
 #ifdef CONFIG_VPL
@@ -166,7 +167,7 @@ ulong spl_get_image_pos(void)
 
 ulong spl_get_image_size(void)
 {
-	if (!CONFIG_IS_ENABLED(BINMAN_SYMBOLS))
+	if (!CONFIG_IS_ENABLED(BINMAN_UBOOT_SYMBOLS))
 		return BINMAN_SYM_MISSING;
 
 #ifdef CONFIG_VPL
diff --git a/include/binman_sym.h b/include/binman_sym.h
index 72e6765fe52..8586ef8731b 100644
--- a/include/binman_sym.h
+++ b/include/binman_sym.h
@@ -13,7 +13,7 @@
 
 #define BINMAN_SYM_MISSING	(-1UL)
 
-#ifdef CONFIG_BINMAN
+#if CONFIG_IS_ENABLED(BINMAN_SYMBOLS)
 
 /**
  * binman_symname() - Internal function to get a binman symbol name
@@ -77,7 +77,7 @@
 #define binman_sym(_type, _entry_name, _prop_name) \
 	(*(_type *)&binman_symname(_entry_name, _prop_name))
 
-#else /* !BINMAN */
+#else /* !CONFIG_IS_ENABLED(BINMAN_SYMBOLS) */
 
 #define binman_sym_declare(_type, _entry_name, _prop_name)
 
@@ -87,6 +87,6 @@
 
 #define binman_sym(_type, _entry_name, _prop_name) BINMAN_SYM_MISSING
 
-#endif /* BINMAN */
+#endif /* CONFIG_IS_ENABLED(BINMAN_SYMBOLS) */
 
 #endif
diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile
index 57057e2d588..bea8567c9b4 100644
--- a/tools/binman/test/Makefile
+++ b/tools/binman/test/Makefile
@@ -21,7 +21,7 @@ CC		= $(CROSS_COMPILE)gcc
 OBJCOPY		= $(CROSS_COMPILE)objcopy
 
 VPATH := $(SRC)
-CFLAGS := -march=i386 -m32 -nostdlib -I $(SRC)../../../include \
+CFLAGS := -march=i386 -m32 -nostdlib -I $(SRC)../../../include -I $(SRC) \
 	-Wl,--no-dynamic-linker
 
 LDS_UCODE := -T $(SRC)u_boot_ucode_ptr.lds
diff --git a/tools/binman/test/generated/autoconf.h b/tools/binman/test/generated/autoconf.h
new file mode 100644
index 00000000000..6a23039f469
--- /dev/null
+++ b/tools/binman/test/generated/autoconf.h
@@ -0,0 +1,3 @@
+#define CONFIG_BINMAN 1
+#define CONFIG_SPL_BUILD 1
+#define CONFIG_SPL_BINMAN_SYMBOLS 1
diff --git a/tools/binman/test/u_boot_binman_syms.c b/tools/binman/test/u_boot_binman_syms.c
index 37fc339ce84..89fee5567e1 100644
--- a/tools/binman/test/u_boot_binman_syms.c
+++ b/tools/binman/test/u_boot_binman_syms.c
@@ -5,7 +5,7 @@
  * Simple program to create some binman symbols. This is used by binman tests.
  */
 
-#define CONFIG_BINMAN
+#include <linux/kconfig.h>
 #include <binman_sym.h>
 
 binman_sym_declare(unsigned long, u_boot_spl_any, offset);
diff --git a/tools/binman/test/u_boot_binman_syms_size.c b/tools/binman/test/u_boot_binman_syms_size.c
index 7224bc1863c..c4a053f96f1 100644
--- a/tools/binman/test/u_boot_binman_syms_size.c
+++ b/tools/binman/test/u_boot_binman_syms_size.c
@@ -5,7 +5,7 @@
  * Simple program to create some binman symbols. This is used by binman tests.
  */
 
-#define CONFIG_BINMAN
+#include <linux/kconfig.h>
 #include <binman_sym.h>
 
 binman_sym_declare(char, u_boot_spl, pos);

From 3a7d32787602a54486856e8392ba202b56ede3c7 Mon Sep 17 00:00:00 2001
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Date: Sat, 18 Jun 2022 15:13:10 +0300
Subject: [PATCH 31/34] spl: binman: Add config options for binman symbols in
 VPL

The SPL code declares binman symbols for U-Boot phases depending on
CONFIG_IS_ENABLED(BINMAN_UBOOT_SYMBOLS). This config exists for SPL and
TPL, also add a version for VPL.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---
 common/spl/Kconfig.vpl | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/common/spl/Kconfig.vpl b/common/spl/Kconfig.vpl
index ba1ea6075b9..daec0bb7bff 100644
--- a/common/spl/Kconfig.vpl
+++ b/common/spl/Kconfig.vpl
@@ -198,4 +198,28 @@ config VPL_TEXT_BASE
 	help
 	  The address in memory that VPL will be running from.
 
+config VPL_BINMAN_SYMBOLS
+	bool "Declare binman symbols in VPL"
+	depends on VPL_FRAMEWORK && BINMAN
+	default y
+	help
+	  This enables use of symbols in VPL which refer to other entries in
+	  the same binman image as the VPL. These can be declared with the
+	  binman_sym_declare(type, entry, prop) macro and accessed by the
+	  binman_sym(type, entry, prop) macro defined in binman_sym.h.
+
+	  See tools/binman/binman.rst for a detailed explanation.
+
+config VPL_BINMAN_UBOOT_SYMBOLS
+	bool "Declare binman symbols for U-Boot phases in VPL"
+	depends on VPL_BINMAN_SYMBOLS
+	default y
+	help
+	  This enables use of symbols in VPL which refer to U-Boot phases,
+	  enabling VPL to obtain the location and size of its next phase simply
+	  by calling spl_get_image_pos() and spl_get_image_size().
+
+	  For this to work, you must have all U-Boot phases in the same binman
+	  image, so binman can update VPL with the locations of everything.
+
 endmenu

From 367ecbf2d3b1c16a3b98b9f6430b8197d2bddbf9 Mon Sep 17 00:00:00 2001
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Date: Sat, 18 Jun 2022 15:13:11 +0300
Subject: [PATCH 32/34] spl: binman: Check at runtime if binman symbols were
 filled in

Binman lets us declare symbols in SPL/TPL that refer to other entries in
the same binman image as them. These symbols are filled in with the
correct values while binman assembles the images, but this is done
in-memory only. Symbols marked as optional can be filled with
BINMAN_SYM_MISSING as an error value if their referred entry is missing.

However, the unmodified SPL/TPL binaries are still available on disk,
and can be used by people. For these files, nothing ensures that the
symbols are set to this error value, and they will be considered valid
when they are not.

Empirically, all symbols show up as zero in a sandbox_vpl build when we
run e.g. tpl/u-boot-tpl directly. On the other hand, zero is a perfectly
fine value for a binman-written symbol, so we cannot say the symbols
have wrong values based on that.

Declare a magic symbol that binman always fills in with a fixed value.
Check this value as an indicator that symbols were filled in correctly.
Return the error value for all symbols when this magic symbol has the
wrong value.

For binman tests, we need to make room for the new symbol in the mocked
SPL/TPL data by extending them by four bytes. This messes up some test
image layouts. Fix the affected values, and check the magic symbol
wherever it makes sense.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---
 common/spl/spl.c                            |  1 +
 include/binman_sym.h                        | 45 ++++++++++++++++++++-
 tools/binman/elf.py                         | 12 ++++--
 tools/binman/elf_test.py                    | 12 +++---
 tools/binman/ftest.py                       | 33 +++++++--------
 tools/binman/test/021_image_pad.dts         |  2 +-
 tools/binman/test/024_sorted.dts            |  2 +-
 tools/binman/test/028_pack_4gb_outside.dts  |  2 +-
 tools/binman/test/029_x86_rom.dts           |  6 +--
 tools/binman/test/053_symbols.dts           |  2 +-
 tools/binman/test/149_symbols_tpl.dts       |  4 +-
 tools/binman/test/155_symbols_tpl_x86.dts   |  4 +-
 tools/binman/test/187_symbols_sub.dts       |  2 +-
 tools/binman/test/u_boot_binman_syms.c      |  4 ++
 tools/binman/test/u_boot_binman_syms_size.c |  4 ++
 15 files changed, 97 insertions(+), 38 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 5dd122611c2..29e0898f03d 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -41,6 +41,7 @@
 #include <wdt.h>
 
 DECLARE_GLOBAL_DATA_PTR;
+DECLARE_BINMAN_MAGIC_SYM;
 
 #ifndef CONFIG_SYS_UBOOT_START
 #define CONFIG_SYS_UBOOT_START	CONFIG_SYS_TEXT_BASE
diff --git a/include/binman_sym.h b/include/binman_sym.h
index 8586ef8731b..528d7e4e90e 100644
--- a/include/binman_sym.h
+++ b/include/binman_sym.h
@@ -11,6 +11,8 @@
 #ifndef __BINMAN_SYM_H
 #define __BINMAN_SYM_H
 
+/* BSYM in little endian, keep in sync with tools/binman/elf.py */
+#define BINMAN_SYM_MAGIC_VALUE	(0x4d595342UL)
 #define BINMAN_SYM_MISSING	(-1UL)
 
 #if CONFIG_IS_ENABLED(BINMAN_SYMBOLS)
@@ -62,6 +64,37 @@
 		__attribute__((aligned(4), weak, unused, \
 		section(".binman_sym")))
 
+/**
+ * _binman_sym_magic - Internal magic symbol for validity checks
+ *
+ * When building images, binman fills in this symbol with the magic
+ * value #defined above. This is used to check at runtime if the
+ * symbol values were filled in and are OK to use.
+ */
+extern ulong _binman_sym_magic;
+
+/**
+ * DECLARE_BINMAN_MAGIC_SYM - Declare the internal magic symbol
+ *
+ * This macro declares the _binman_sym_magic symbol so that it exists.
+ * Declaring it here would cause errors during linking due to multiple
+ * definitions of the symbol.
+ */
+#define DECLARE_BINMAN_MAGIC_SYM \
+	ulong _binman_sym_magic \
+		__attribute__((aligned(4), section(".binman_sym")))
+
+/**
+ * BINMAN_SYMS_OK - Check if the symbol values are valid
+ *
+ * This macro checks if the magic symbol's value is filled properly,
+ * which indicates that other symbols are OK to use as well.
+ *
+ * Return: 1 if binman symbol values are usable, 0 if not
+ */
+#define BINMAN_SYMS_OK \
+	(*(ulong *)&_binman_sym_magic == BINMAN_SYM_MAGIC_VALUE)
+
 /**
  * binman_sym() - Access a previously declared symbol
  *
@@ -72,10 +105,14 @@
  * @_type: Type f the symbol (e.g. unsigned long)
  * @entry_name: Name of the entry to look for (e.g. 'u_boot_spl')
  * @_prop_name: Property value to get from that entry (e.g. 'pos')
- * @returns value of that property (filled in by binman)
+ *
+ * Return: value of that property (filled in by binman), or
+ *	   BINMAN_SYM_MISSING if the value is unavailable
  */
 #define binman_sym(_type, _entry_name, _prop_name) \
-	(*(_type *)&binman_symname(_entry_name, _prop_name))
+	(BINMAN_SYMS_OK ? \
+	 (*(_type *)&binman_symname(_entry_name, _prop_name)) : \
+	 BINMAN_SYM_MISSING)
 
 #else /* !CONFIG_IS_ENABLED(BINMAN_SYMBOLS) */
 
@@ -85,6 +122,10 @@
 
 #define binman_sym_extern(_type, _entry_name, _prop_name)
 
+#define DECLARE_BINMAN_MAGIC_SYM
+
+#define BINMAN_SYMS_OK (0)
+
 #define binman_sym(_type, _entry_name, _prop_name) BINMAN_SYM_MISSING
 
 #endif /* CONFIG_IS_ENABLED(BINMAN_SYMBOLS) */
diff --git a/tools/binman/elf.py b/tools/binman/elf.py
index afa05e58fdd..6d440ddf21d 100644
--- a/tools/binman/elf.py
+++ b/tools/binman/elf.py
@@ -25,6 +25,9 @@ try:
 except:  # pragma: no cover
     ELF_TOOLS = False
 
+# BSYM in little endian, keep in sync with include/binman_sym.h
+BINMAN_SYM_MAGIC_VALUE = 0x4d595342
+
 # Information about an EFL symbol:
 # section (str): Name of the section containing this symbol
 # address (int): Address of the symbol (its value)
@@ -223,9 +226,12 @@ def LookupAndWriteSymbols(elf_fname, entry, section):
                 raise ValueError('%s has size %d: only 4 and 8 are supported' %
                                  (msg, sym.size))
 
-            # Look up the symbol in our entry tables.
-            value = section.GetImage().LookupImageSymbol(name, sym.weak, msg,
-                                                         base.address)
+            if name == '_binman_sym_magic':
+                value = BINMAN_SYM_MAGIC_VALUE
+            else:
+                # Look up the symbol in our entry tables.
+                value = section.GetImage().LookupImageSymbol(name, sym.weak,
+                                                             msg, base.address)
             if value is None:
                 value = -1
                 pack_string = pack_string.lower()
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py
index 02bc1083749..5a51c64cfee 100644
--- a/tools/binman/elf_test.py
+++ b/tools/binman/elf_test.py
@@ -127,7 +127,7 @@ class TestElf(unittest.TestCase):
         elf_fname = self.ElfTestFile('u_boot_binman_syms')
         with self.assertRaises(ValueError) as e:
             elf.LookupAndWriteSymbols(elf_fname, entry, section)
-        self.assertIn('entry_path has offset 4 (size 8) but the contents size '
+        self.assertIn('entry_path has offset 8 (size 8) but the contents size '
                       'is a', str(e.exception))
 
     def testMissingImageStart(self):
@@ -161,18 +161,20 @@ class TestElf(unittest.TestCase):
         This should produce -1 values for all thress symbols, taking up the
         first 16 bytes of the image.
         """
-        entry = FakeEntry(24)
+        entry = FakeEntry(28)
         section = FakeSection(sym_value=None)
         elf_fname = self.ElfTestFile('u_boot_binman_syms')
         elf.LookupAndWriteSymbols(elf_fname, entry, section)
-        self.assertEqual(tools.get_bytes(255, 20) + tools.get_bytes(ord('a'), 4),
-                                                                  entry.data)
+        expected = (struct.pack('<L', elf.BINMAN_SYM_MAGIC_VALUE) +
+                    tools.get_bytes(255, 20) +
+                    tools.get_bytes(ord('a'), 4))
+        self.assertEqual(expected, entry.data)
 
     def testDebug(self):
         """Check that enabling debug in the elf module produced debug output"""
         try:
             tout.init(tout.DEBUG)
-            entry = FakeEntry(20)
+            entry = FakeEntry(24)
             section = FakeSection()
             elf_fname = self.ElfTestFile('u_boot_binman_syms')
             with test_util.capture_sys_output() as (stdout, stderr):
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index b5cf549703a..fa1f421c052 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -43,8 +43,8 @@ from patman import tout
 # Contents of test files, corresponding to different entry types
 U_BOOT_DATA           = b'1234'
 U_BOOT_IMG_DATA       = b'img'
-U_BOOT_SPL_DATA       = b'56780123456789abcdefghi'
-U_BOOT_TPL_DATA       = b'tpl9876543210fedcbazyw'
+U_BOOT_SPL_DATA       = b'56780123456789abcdefghijklm'
+U_BOOT_TPL_DATA       = b'tpl9876543210fedcbazywvuts'
 BLOB_DATA             = b'89'
 ME_DATA               = b'0abcd'
 VGA_DATA              = b'vga'
@@ -1406,8 +1406,9 @@ class TestFunctional(unittest.TestCase):
         elf_fname = self.ElfTestFile('u_boot_binman_syms')
         syms = elf.GetSymbols(elf_fname, ['binman', 'image'])
         addr = elf.GetSymbolAddress(elf_fname, '__image_copy_start')
+        self.assertEqual(syms['_binman_sym_magic'].address, addr)
         self.assertEqual(syms['_binman_u_boot_spl_any_prop_offset'].address,
-                         addr)
+                         addr + 4)
 
         self._SetupSplElf('u_boot_binman_syms')
         data = self._DoReadFileDtb(dts, entry_args=entry_args,
@@ -1415,17 +1416,17 @@ class TestFunctional(unittest.TestCase):
         # The image should contain the symbols from u_boot_binman_syms.c
         # Note that image_pos is adjusted by the base address of the image,
         # which is 0x10 in our test image
-        sym_values = struct.pack('<LQLL', 0x00,
-                                 u_boot_offset + len(U_BOOT_DATA),
+        sym_values = struct.pack('<LLQLL', elf.BINMAN_SYM_MAGIC_VALUE,
+                                 0x00, u_boot_offset + len(U_BOOT_DATA),
                                  0x10 + u_boot_offset, 0x04)
-        expected = (sym_values + base_data[20:] +
+        expected = (sym_values + base_data[24:] +
                     tools.get_bytes(0xff, 1) + U_BOOT_DATA + sym_values +
-                    base_data[20:])
+                    base_data[24:])
         self.assertEqual(expected, data)
 
     def testSymbols(self):
         """Test binman can assign symbols embedded in U-Boot"""
-        self.checkSymbols('053_symbols.dts', U_BOOT_SPL_DATA, 0x18)
+        self.checkSymbols('053_symbols.dts', U_BOOT_SPL_DATA, 0x1c)
 
     def testSymbolsNoDtb(self):
         """Test binman can assign symbols embedded in U-Boot SPL"""
@@ -3610,20 +3611,20 @@ class TestFunctional(unittest.TestCase):
 
     def _CheckSymbolsTplSection(self, dts, expected_vals):
         data = self._DoReadFile(dts)
-        sym_values = struct.pack('<LQLL', *expected_vals)
+        sym_values = struct.pack('<LLQLL', elf.BINMAN_SYM_MAGIC_VALUE, *expected_vals)
         upto1 = 4 + len(U_BOOT_SPL_DATA)
-        expected1 = tools.get_bytes(0xff, 4) + sym_values + U_BOOT_SPL_DATA[20:]
+        expected1 = tools.get_bytes(0xff, 4) + sym_values + U_BOOT_SPL_DATA[24:]
         self.assertEqual(expected1, data[:upto1])
 
         upto2 = upto1 + 1 + len(U_BOOT_SPL_DATA)
-        expected2 = tools.get_bytes(0xff, 1) + sym_values + U_BOOT_SPL_DATA[20:]
+        expected2 = tools.get_bytes(0xff, 1) + sym_values + U_BOOT_SPL_DATA[24:]
         self.assertEqual(expected2, data[upto1:upto2])
 
-        upto3 = 0x34 + len(U_BOOT_DATA)
+        upto3 = 0x3c + len(U_BOOT_DATA)
         expected3 = tools.get_bytes(0xff, 1) + U_BOOT_DATA
         self.assertEqual(expected3, data[upto2:upto3])
 
-        expected4 = sym_values + U_BOOT_TPL_DATA[20:]
+        expected4 = sym_values + U_BOOT_TPL_DATA[24:]
         self.assertEqual(expected4, data[upto3:upto3 + len(U_BOOT_TPL_DATA)])
 
     def testSymbolsTplSection(self):
@@ -3631,14 +3632,14 @@ class TestFunctional(unittest.TestCase):
         self._SetupSplElf('u_boot_binman_syms')
         self._SetupTplElf('u_boot_binman_syms')
         self._CheckSymbolsTplSection('149_symbols_tpl.dts',
-                                     [0x04, 0x1c, 0x10 + 0x34, 0x04])
+                                     [0x04, 0x20, 0x10 + 0x3c, 0x04])
 
     def testSymbolsTplSectionX86(self):
         """Test binman can assign symbols in a section with end-at-4gb"""
         self._SetupSplElf('u_boot_binman_syms_x86')
         self._SetupTplElf('u_boot_binman_syms_x86')
         self._CheckSymbolsTplSection('155_symbols_tpl_x86.dts',
-                                     [0xffffff04, 0xffffff1c, 0xffffff34,
+                                     [0xffffff04, 0xffffff20, 0xffffff3c,
                                       0x04])
 
     def testPackX86RomIfwiSectiom(self):
@@ -4488,7 +4489,7 @@ class TestFunctional(unittest.TestCase):
 
     def testSymbolsSubsection(self):
         """Test binman can assign symbols from a subsection"""
-        self.checkSymbols('187_symbols_sub.dts', U_BOOT_SPL_DATA, 0x18)
+        self.checkSymbols('187_symbols_sub.dts', U_BOOT_SPL_DATA, 0x1c)
 
     def testReadImageEntryArg(self):
         """Test reading an image that would need an entry arg to generate"""
diff --git a/tools/binman/test/021_image_pad.dts b/tools/binman/test/021_image_pad.dts
index 1ff8dab296f..c5abbbcdd6a 100644
--- a/tools/binman/test/021_image_pad.dts
+++ b/tools/binman/test/021_image_pad.dts
@@ -10,7 +10,7 @@
 		};
 
 		u-boot {
-			offset = <24>;
+			offset = <28>;
 		};
 	};
 };
diff --git a/tools/binman/test/024_sorted.dts b/tools/binman/test/024_sorted.dts
index b79d9adf682..b54f9b14191 100644
--- a/tools/binman/test/024_sorted.dts
+++ b/tools/binman/test/024_sorted.dts
@@ -7,7 +7,7 @@
 	binman {
 		sort-by-offset;
 		u-boot {
-			offset = <26>;
+			offset = <30>;
 		};
 
 		u-boot-spl {
diff --git a/tools/binman/test/028_pack_4gb_outside.dts b/tools/binman/test/028_pack_4gb_outside.dts
index 11a1f6059e2..b6ad7fb56a5 100644
--- a/tools/binman/test/028_pack_4gb_outside.dts
+++ b/tools/binman/test/028_pack_4gb_outside.dts
@@ -13,7 +13,7 @@
 		};
 
 		u-boot-spl {
-			offset = <0xffffffe7>;
+			offset = <0xffffffe3>;
 		};
 	};
 };
diff --git a/tools/binman/test/029_x86_rom.dts b/tools/binman/test/029_x86_rom.dts
index 88aa007bbae..ad8f9d6e1bd 100644
--- a/tools/binman/test/029_x86_rom.dts
+++ b/tools/binman/test/029_x86_rom.dts
@@ -7,13 +7,13 @@
 	binman {
 		sort-by-offset;
 		end-at-4gb;
-		size = <32>;
+		size = <36>;
 		u-boot {
-			offset = <0xffffffe0>;
+			offset = <0xffffffdc>;
 		};
 
 		u-boot-spl {
-			offset = <0xffffffe7>;
+			offset = <0xffffffe3>;
 		};
 	};
 };
diff --git a/tools/binman/test/053_symbols.dts b/tools/binman/test/053_symbols.dts
index 29658092764..b28f34a72fa 100644
--- a/tools/binman/test/053_symbols.dts
+++ b/tools/binman/test/053_symbols.dts
@@ -10,7 +10,7 @@
 		};
 
 		u-boot {
-			offset = <0x18>;
+			offset = <0x1c>;
 		};
 
 		u-boot-spl2 {
diff --git a/tools/binman/test/149_symbols_tpl.dts b/tools/binman/test/149_symbols_tpl.dts
index 0a4ab3f1fab..4e649c45978 100644
--- a/tools/binman/test/149_symbols_tpl.dts
+++ b/tools/binman/test/149_symbols_tpl.dts
@@ -11,12 +11,12 @@
 		};
 
 		u-boot-spl2 {
-			offset = <0x1c>;
+			offset = <0x20>;
 			type = "u-boot-spl";
 		};
 
 		u-boot {
-			offset = <0x34>;
+			offset = <0x3c>;
 		};
 
 		section {
diff --git a/tools/binman/test/155_symbols_tpl_x86.dts b/tools/binman/test/155_symbols_tpl_x86.dts
index 9d7dc51b3d9..e1ce33e67fb 100644
--- a/tools/binman/test/155_symbols_tpl_x86.dts
+++ b/tools/binman/test/155_symbols_tpl_x86.dts
@@ -14,12 +14,12 @@
 		};
 
 		u-boot-spl2 {
-			offset = <0xffffff1c>;
+			offset = <0xffffff20>;
 			type = "u-boot-spl";
 		};
 
 		u-boot {
-			offset = <0xffffff34>;
+			offset = <0xffffff3c>;
 		};
 
 		section {
diff --git a/tools/binman/test/187_symbols_sub.dts b/tools/binman/test/187_symbols_sub.dts
index 54511a73711..3ab62d37215 100644
--- a/tools/binman/test/187_symbols_sub.dts
+++ b/tools/binman/test/187_symbols_sub.dts
@@ -11,7 +11,7 @@
 			};
 
 			u-boot {
-				offset = <24>;
+				offset = <28>;
 			};
 		};
 
diff --git a/tools/binman/test/u_boot_binman_syms.c b/tools/binman/test/u_boot_binman_syms.c
index 89fee5567e1..ed761246aec 100644
--- a/tools/binman/test/u_boot_binman_syms.c
+++ b/tools/binman/test/u_boot_binman_syms.c
@@ -5,9 +5,13 @@
  * Simple program to create some binman symbols. This is used by binman tests.
  */
 
+typedef unsigned long ulong;
+
 #include <linux/kconfig.h>
 #include <binman_sym.h>
 
+DECLARE_BINMAN_MAGIC_SYM;
+
 binman_sym_declare(unsigned long, u_boot_spl_any, offset);
 binman_sym_declare(unsigned long long, u_boot_spl2, offset);
 binman_sym_declare(unsigned long, u_boot_any, image_pos);
diff --git a/tools/binman/test/u_boot_binman_syms_size.c b/tools/binman/test/u_boot_binman_syms_size.c
index c4a053f96f1..fa41b3d9a33 100644
--- a/tools/binman/test/u_boot_binman_syms_size.c
+++ b/tools/binman/test/u_boot_binman_syms_size.c
@@ -5,7 +5,11 @@
  * Simple program to create some binman symbols. This is used by binman tests.
  */
 
+typedef unsigned long ulong;
+
 #include <linux/kconfig.h>
 #include <binman_sym.h>
 
+DECLARE_BINMAN_MAGIC_SYM;
+
 binman_sym_declare(char, u_boot_spl, pos);

From 6516c9b349b3272c6c9cb7a4bdcfdef617d9f4ee Mon Sep 17 00:00:00 2001
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Date: Sat, 18 Jun 2022 15:13:12 +0300
Subject: [PATCH 33/34] spl: binman: Disable u_boot_any symbols for i.MX8M
 boards

The i.MX8M boards use partially specified binman images which have an
SPL entry without a U-Boot entry. This would normally cause an error due
to the 'u_boot_any' binman symbols declared by BINMAN_UBOOT_SYMBOLS
requiring a U-Boot-like entry in the same image as the SPL.

However, a problem in the ARMv8 __image_copy_start symbol definition
effectively disables binman from attempting to write any symbols at all,
so everything appears to work fine until runtime. A future patch fixes
the issue in the linker scripts, which lets binman fill in the symbols,
which would result in the build error described above.

Explicitly disable the 'u_boot_any' symbols for i.MX8M boards. They are
already effectively unusable, and they are incompatible with the boards'
current binman image descriptions.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---
 common/spl/Kconfig     | 1 +
 common/spl/Kconfig.tpl | 1 +
 common/spl/Kconfig.vpl | 1 +
 3 files changed, 3 insertions(+)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 46d9be73bb1..152569ee435 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -200,6 +200,7 @@ config SPL_BINMAN_SYMBOLS
 config SPL_BINMAN_UBOOT_SYMBOLS
 	bool "Declare binman symbols for U-Boot phases in SPL"
 	depends on SPL_BINMAN_SYMBOLS
+	default n if ARCH_IMX8M
 	default y
 	help
 	  This enables use of symbols in SPL which refer to U-Boot phases,
diff --git a/common/spl/Kconfig.tpl b/common/spl/Kconfig.tpl
index 8c59c767302..e314b793a2e 100644
--- a/common/spl/Kconfig.tpl
+++ b/common/spl/Kconfig.tpl
@@ -23,6 +23,7 @@ config TPL_BINMAN_SYMBOLS
 config TPL_BINMAN_UBOOT_SYMBOLS
 	bool "Declare binman symbols for U-Boot phases in TPL"
 	depends on TPL_BINMAN_SYMBOLS
+	default n if ARCH_IMX8M
 	default y
 	help
 	  This enables use of symbols in TPL which refer to U-Boot phases,
diff --git a/common/spl/Kconfig.vpl b/common/spl/Kconfig.vpl
index daec0bb7bff..ba4b2e4f99e 100644
--- a/common/spl/Kconfig.vpl
+++ b/common/spl/Kconfig.vpl
@@ -213,6 +213,7 @@ config VPL_BINMAN_SYMBOLS
 config VPL_BINMAN_UBOOT_SYMBOLS
 	bool "Declare binman symbols for U-Boot phases in VPL"
 	depends on VPL_BINMAN_SYMBOLS
+	default n if ARCH_IMX8M
 	default y
 	help
 	  This enables use of symbols in VPL which refer to U-Boot phases,

From e87da5704ffa6fc782d93d137fa30a37a5df3566 Mon Sep 17 00:00:00 2001
From: Peng Fan <peng.fan@nxp.com>
Date: Sat, 18 Jun 2022 15:13:13 +0300
Subject: [PATCH 34/34] armv8: u-boot-spl.lds: mark __image_copy_start as
 symbol

In arch/arm/lib/sections.c there is below code:
char __image_copy_start[0] __section(".__image_copy_start");
But actually 'objdump -t spl/u-boot-spl' not able to find out
symbol '__image_copy_start' for binman update image-pos/size.

So update link file

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
Reviewed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---
 arch/arm/cpu/armv8/u-boot-spl.lds | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds
index d02b788e608..7cb9d731246 100644
--- a/arch/arm/cpu/armv8/u-boot-spl.lds
+++ b/arch/arm/cpu/armv8/u-boot-spl.lds
@@ -23,7 +23,7 @@ SECTIONS
 {
 	.text : {
 		. = ALIGN(8);
-		*(.__image_copy_start)
+		__image_copy_start = .;
 		CPUDIR/start.o (.text*)
 		*(.text*)
 	} >.sram