Re: [PATCH v3 00/19] binman: Simple templating feature and mkimage conversion

[Jan Kiszka]

On 10.07.23 04:40, Simon Glass wrote:
> This series converts the mkimage entry type to be a section, i.e. based on
> the entry_Section class. This makes it more consistent in its behaviour,
> e.g. allowing symbol writing and expanded entries.
> 
> A simple templating feature is also introduced, to reduce duplication
> when a set of entries must be used in multiple images.
> 
> In this version the nodes from the template are placed before any other
> nodes, meaning that the template sets the node order. This seems more
> consistent than other mechanisms.
> 
> Changes in v3:
> - Add new patch for elf to return number of written symbols
> - Add new patch with more detail on how ObtainContents() works
> - Fix up some tests which now need SPL and TPL
> - Avoid calling ObtainContents() when not needed
> - Fix 'specific' typo
> - Add a new devicetree file especially for node copying
> - Correct logic for merging nodes in order
> - Tidy up some comments
> - Adjust to use the new example file
> - Drop duplicate dts-v1 header
> - Add new test case for templating in a FIT
> - Add new patch to support writing symbols inside a mkimage image
> 
> Changes in v2:
> - Drop now-unnecessary methods in mkimage etype
> - Correct ordering of template nodes
> - Fix 'preseverd' and 'inserter' typos
> 
> Marek Vasut (1):
>   binman: Convert mkimage to Entry_section
> 
> Simon Glass (18):
>   binman: Correct coverage gap in control
>   binman: Init align_default in entry_Section
>   binman: Use GetEntries() to obtain section contents
>   binman: Read _multiple_data_files in the correct place
>   binman: Allow disabling symbol writing
>   stm32mp15: Avoid writing symbols in SPL
>   binman: Update elf to return number of written symbols
>   binman: Add more detail on how ObtainContents() works
>   binman: Provide a way to specify the fdt-list directly
>   binman: Drop __bss_size variable in bss_data.c
>   binman: Correct handling of zero bss size
>   dtoc: Support copying the contents of a node into another
>   dtoc: Allow inserting a list of nodes into another
>   binman: Support simple templates
>   binman: Support templating with multiple images
>   binman: Add a test for templating in a FIT
>   binman: Support templates at any level
>   binman: Support writing symbols inside a mkimage image
> 
>  arch/arm/dts/stm32mp15-u-boot.dtsi |   1 +
>  tools/binman/binman.rst|  89 +
>  tools/binman/control.py|  34 +++-
>  tools/binman/elf.py|  13 +-
>  tools/binman/elf_test.py   |  13 +-
>  tools/binman/entries.rst   |   6 +
>  tools/binman/entry.py  |  11 +-
>  tools/binman/etype/blob_phase.py   |   5 +
>  tools/binman/etype/fit.py  |   9 +
>  tools/binman/etype/mkimage.py  | 110 ---
>  tools/binman/etype/section.py  |  54 +++--
>  tools/binman/etype/u_boot_spl_bss_pad.py   |   2 +-
>  tools/binman/etype/u_boot_tpl_bss_pad.py   |   2 +-
>  tools/binman/etype/u_boot_vpl_bss_pad.py   |   2 +-
>  tools/binman/ftest.py  | 218 -
>  tools/binman/test/282_symbols_disable.dts  |  25 +++
>  tools/binman/test/283_mkimage_special.dts  |  24 +++
>  tools/binman/test/284_fit_fdt_list.dts |  58 ++
>  tools/binman/test/285_spl_expand.dts   |  13 ++
>  tools/binman/test/286_template.dts |  42 
>  tools/binman/test/287_template_multi.dts   |  27 +++
>  tools/binman/test/288_template_fit.dts |  37 
>  tools/binman/test/289_template_section.dts |  52 +
>  tools/binman/test/290_mkimage_sym.dts  |  27 +++
>  tools/binman/test/Makefile |   5 +-
>  tools/binman/test/bss_data.c   |   3 +-
>  tools/binman/test/bss_data_zero.c  |  16 ++
>  tools/binman/test/bss_data_zero.lds|  15 ++
>  tools/binman/test/embed_data.lds   |   1 +
>  tools/dtoc/fdt.py  | 122 +++-
>  tools/dtoc/test/dtoc_test_copy.dts |  86 
>  tools/dtoc/test_fdt.py |  93 +
>  32 files changed, 1147 insertions(+), 68 deletions(-)
>  create mode 100644 tools/binman/test/282_symbols_disable.dts
>  create mode 100644 tools/binman/test/283_mkimage_special.dts
>  create mode 100644 tools/binman/test/284_fit_fdt_list.dts
>  create mode 100644 tools/binman/test/285_spl_expand.dts
>  create mode 100644 tools/binman/test/286_template.dts
>  create mode 100644 tools/binman/test/287_template_multi.dts
>  create mode 100644 tools/binman/test/288_template_fit.dts
>  create mode 100644 tools/binman/test/289_template_section.dts
>  create mode 100644 tools/binman/test/290_mkimage_sym.dts
>  create mode 100644 tools/binman/test/bss_data_zero.c
>  create mode 100644 tools/binman/test/bss_data_zero.lds
>  create mode 100644 tools/dtoc/test/dtoc_test_copy.dts
> 

Works much better!

What does not work yet:

/dts-v1/;

/ {

[PATCH v3 19/19] binman: Support writing symbols inside a mkimage image

[Simon Glass]

Add support for writing symbols and determining the assumed position of
binaries inside a mkimage image. This is useful as an example for other
entry types which might want to do the same thing.

Signed-off-by: Simon Glass 
---

Changes in v3:
- Add new patch to support writing symbols inside a mkimage image

 tools/binman/entry.py |  2 -
 tools/binman/etype/mkimage.py | 36 +++
 tools/binman/ftest.py | 64 +++
 tools/binman/test/290_mkimage_sym.dts | 27 +++
 4 files changed, 127 insertions(+), 2 deletions(-)
 create mode 100644 tools/binman/test/290_mkimage_sym.dts

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 0d4cb94f7002..42e0b7b91450 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -1314,8 +1314,6 @@ features to produce new behaviours.
 """
 data = b''
 for entry in entries:
-# First get the input data and put it in a file
-entry.ObtainContents(fake_size=fake_size)
 data += entry.GetData()
 uniq = self.GetUniqueName()
 fname = tools.get_output_filename(f'{prefix}.{uniq}')
diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
index 493761da59f5..9d60c85722b3 100644
--- a/tools/binman/etype/mkimage.py
+++ b/tools/binman/etype/mkimage.py
@@ -268,3 +268,39 @@ class Entry_mkimage(Entry_section):
 
 def CheckEntries(self):
 pass
+
+def ProcessContents(self):
+# The blob may have changed due to WriteSymbols()
+ok = super().ProcessContents()
+data = self.BuildSectionData(True)
+ok2 = self.ProcessContentsUpdate(data)
+return ok and ok2
+
+def SetImagePos(self, image_pos):
+"""Set the position in the image
+
+This sets each subentry's offsets, sizes and positions-in-image
+according to where they ended up in the packed mkimage file.
+
+NOTE: This assumes a legacy mkimage and assumes that the images are
+written to the output in order. SoC-specific mkimage handling may not
+conform to this, in which case these values may be wrong.
+
+Args:
+image_pos (int): Position of this entry in the image
+"""
+# The mkimage header consists of 0x40 bytes, following by a table of
+# offsets for each file
+upto = 0x40
+
+# Skip the 0-terminated list of offsets (assume a single image)
+upto += 4 + 4
+for entry in self.GetEntries().values():
+entry.SetOffsetSize(upto, None)
+
+# Give up if any entries lack a size
+if entry.size is None:
+return
+upto += entry.size
+
+super().SetImagePos(image_pos)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index e96223cbd892..e53181afb78a 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -6820,6 +6820,70 @@ fdt fdtmapExtract the devicetree 
blob from the fdtmap
 second = U_BOOT_DATA + b'#' + VGA_DATA + U_BOOT_DTB_DATA
 self.assertEqual(U_BOOT_IMG_DATA + first + second + first, data)
 
+def testMkimageSymbols(self):
+"""Test using mkimage to build an image with symbols in it"""
+self._SetupSplElf('u_boot_binman_syms')
+data = self._DoReadFile('290_mkimage_sym.dts')
+
+image = control.images['image']
+entries = image.GetEntries()
+self.assertIn('u-boot', entries)
+u_boot = entries['u-boot']
+
+mkim = entries['mkimage']
+mkim_entries = mkim.GetEntries()
+self.assertIn('u-boot-spl', mkim_entries)
+spl = mkim_entries['u-boot-spl']
+self.assertIn('u-boot-spl2', mkim_entries)
+spl2 = mkim_entries['u-boot-spl2']
+
+# skip the mkimage header and the area sizes
+mk_data = data[mkim.offset + 0x40:]
+size, term = struct.unpack('>LL', mk_data[:8])
+
+# There should be only one image, so check that the zero terminator is
+# present
+self.assertEqual(0, term)
+
+content = mk_data[8:8 + size]
+
+# 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
+spl_data = content[:0x18]
+content = content[0x1b:]
+
+# After the header is a table of offsets for each image. There should
+# only be one image, then a 0 terminator, so figure out the real start
+# of the image data
+base = 0x40 + 8
+
+# Check symbols in both u-boot-spl and u-boot-spl2
+for i in range(2):
+vals = struct.unpack(';
+   #size-cells = <1>;
+
+   binman {
+   u-boot-dtb {
+   };
+
+   mkimage {
+   args = "-n test -T script";
+
+   u-boot-spl {
+

[PATCH v3 18/19] binman: Support templates at any level

[Simon Glass]

Allow templates to be used inside a section, not just in the top-level
/binman node.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 tools/binman/control.py|  5 ++-
 tools/binman/ftest.py  |  8 
 tools/binman/test/289_template_section.dts | 52 ++
 3 files changed, 63 insertions(+), 2 deletions(-)
 create mode 100644 tools/binman/test/289_template_section.dts

diff --git a/tools/binman/control.py b/tools/binman/control.py
index f92c152285ce..25e66814837d 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -493,8 +493,8 @@ def _ProcessTemplates(parent):
 Processing involves copying each subnode of the template node into the
 target node.
 
-For now this is not done recursively, so templates must be at the top level
-of the binman image.
+This is done recursively, so templates can be at any level of the binman
+image, e.g. inside a section.
 
 See 'Templates' in the Binman documnentation for details.
 """
@@ -502,6 +502,7 @@ def _ProcessTemplates(parent):
 tmpl = fdt_util.GetPhandleList(node, 'insert-template')
 if tmpl:
 node.copy_subnodes_from_phandles(tmpl)
+_ProcessTemplates(node)
 
 def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded):
 """Prepare the images to be processed and select the device tree
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index dfca6316624c..e96223cbd892 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -6812,6 +6812,14 @@ fdt fdtmapExtract the devicetree 
blob from the fdtmap
 tools.write_file(fname, fit_data)
 out = tools.run('dumpimage', '-l', fname)
 
+def testTemplateSection(self):
+"""Test using a template in a section (not at top level)"""
+TestFunctional._MakeInputFile('vga2.bin', b'#' + VGA_DATA)
+data = self._DoReadFile('289_template_section.dts')
+first = U_BOOT_DATA + VGA_DATA + U_BOOT_DTB_DATA
+second = U_BOOT_DATA + b'#' + VGA_DATA + U_BOOT_DTB_DATA
+self.assertEqual(U_BOOT_IMG_DATA + first + second + first, data)
+
 
 if __name__ == "__main__":
 unittest.main()
diff --git a/tools/binman/test/289_template_section.dts 
b/tools/binman/test/289_template_section.dts
new file mode 100644
index ..8a744a0cf686
--- /dev/null
+++ b/tools/binman/test/289_template_section.dts
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   binman {
+   u-boot-img {
+   };
+
+   common_part: template {
+   u-boot {
+   };
+
+   intel-vga {
+   filename = "vga.bin";
+   };
+   };
+
+   first {
+   type = "section";
+   insert-template = <_part>;
+
+   u-boot-dtb {
+   };
+   };
+
+   section {
+   second {
+   type = "section";
+   insert-template = <_part>;
+
+   u-boot-dtb {
+   };
+
+   intel-vga {
+   filename = "vga2.bin";
+   };
+   };
+   };
+
+   second {
+   type = "section";
+   insert-template = <_part>;
+
+   u-boot-dtb {
+   };
+   };
+   };
+};
-- 
2.41.0.390.g38632f3daf-goog

[PATCH v3 17/19] binman: Add a test for templating in a FIT

[Simon Glass]

Add this as a separate test case.

Signed-off-by: Simon Glass 
---

Changes in v3:
- Add new test case for templating in a FIT

 tools/binman/ftest.py  |  7 +
 tools/binman/test/288_template_fit.dts | 37 ++
 2 files changed, 44 insertions(+)
 create mode 100644 tools/binman/test/288_template_fit.dts

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index dd6075b871d4..dfca6316624c 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -6805,6 +6805,13 @@ fdt fdtmapExtract the devicetree 
blob from the fdtmap
 data = tools.read_file(image_fname)
 self.assertEqual(b'blobother', data)
 
+def testTemplateFit(self):
+"""Test using a template in a FIT"""
+fit_data = self._DoReadFile('288_template_fit.dts')
+fname = os.path.join(self._indir, 'fit_data.fit')
+tools.write_file(fname, fit_data)
+out = tools.run('dumpimage', '-l', fname)
+
 
 if __name__ == "__main__":
 unittest.main()
diff --git a/tools/binman/test/288_template_fit.dts 
b/tools/binman/test/288_template_fit.dts
new file mode 100644
index ..d84dca4ea41e
--- /dev/null
+++ b/tools/binman/test/288_template_fit.dts
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+   binman: binman {
+   multiple-images;
+
+   my_template: template {
+   fit@0 {
+   images {
+   kernel-1 {
+   };
+   kernel-2 {
+   };
+   };
+   };
+   };
+
+   image {
+   filename = "image.bin";
+   insert-template = <_template>;
+
+   fit@0 {
+   description = "desc";
+   configurations {
+   };
+   images {
+   kernel-3 {
+   };
+   kernel-4 {
+   };
+   };
+   };
+   };
+   };
+};
-- 
2.41.0.390.g38632f3daf-goog

[PATCH v3 16/19] binman: Support templating with multiple images

[Simon Glass]

Allow a template to appear in the top level description when using
multiple images.

Signed-off-by: Simon Glass 
---

Changes in v3:
- Drop duplicate dts-v1 header

 tools/binman/control.py  |  5 +++--
 tools/binman/ftest.py| 12 +++
 tools/binman/test/287_template_multi.dts | 27 
 3 files changed, 42 insertions(+), 2 deletions(-)
 create mode 100644 tools/binman/test/287_template_multi.dts

diff --git a/tools/binman/control.py b/tools/binman/control.py
index e9c4a65a75a1..f92c152285ce 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -57,8 +57,9 @@ def _ReadImageDesc(binman_node, use_expanded):
 images = OrderedDict()
 if 'multiple-images' in binman_node.props:
 for node in binman_node.subnodes:
-images[node.name] = Image(node.name, node,
-  use_expanded=use_expanded)
+if 'template' not in node.name:
+images[node.name] = Image(node.name, node,
+  use_expanded=use_expanded)
 else:
 images['image'] = Image('image', binman_node, 
use_expanded=use_expanded)
 return images
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index fc5d8a839e6b..dd6075b871d4 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -6793,6 +6793,18 @@ fdt fdtmapExtract the devicetree 
blob from the fdtmap
 second = U_BOOT_DATA + b'#' + VGA_DATA + U_BOOT_DTB_DATA
 self.assertEqual(U_BOOT_IMG_DATA + first + second, data)
 
+def testTemplateBlobMulti(self):
+"""Test using a template with 'multiple-images' enabled"""
+TestFunctional._MakeInputFile('my-blob.bin', b'blob')
+TestFunctional._MakeInputFile('my-blob2.bin', b'other')
+retcode = self._DoTestFile('287_template_multi.dts')
+
+self.assertEqual(0, retcode)
+image = control.images['image']
+image_fname = tools.get_output_filename('my-image.bin')
+data = tools.read_file(image_fname)
+self.assertEqual(b'blobother', data)
+
 
 if __name__ == "__main__":
 unittest.main()
diff --git a/tools/binman/test/287_template_multi.dts 
b/tools/binman/test/287_template_multi.dts
new file mode 100644
index ..122bfccd5652
--- /dev/null
+++ b/tools/binman/test/287_template_multi.dts
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+/ {
+   binman: binman {
+   multiple-images;
+
+   my_template: template {
+   blob-ext@0 {
+   filename = "my-blob.bin";
+   offset = <0>;
+   };
+   blob-ext@8 {
+   offset = <8>;
+   };
+   };
+
+   image {
+   pad-byte = <0x40>;
+   filename = "my-image.bin";
+   insert-template = <_template>;
+   blob-ext@8 {
+   filename = "my-blob2.bin";
+   };
+   };
+   };
+};
-- 
2.41.0.390.g38632f3daf-goog

[PATCH v3 15/19] binman: Support simple templates

[Simon Glass]

Collections can used to collect the contents of other entries into a
single entry, but they result in a single entry, with the original entries
'left behind' in their old place.

It is useful to be able to specific a set of entries ones and have it used
in multiple images, or parts of an image.

Implement this mechanism.

Signed-off-by: Simon Glass 
---

(no changes since v2)

Changes in v2:
- Correct ordering of template nodes
- Fix 'preseverd' and 'inserter' typos

 tools/binman/binman.rst| 82 ++
 tools/binman/control.py| 26 ++
 tools/binman/etype/section.py  |  3 +-
 tools/binman/ftest.py  |  8 +++
 tools/binman/test/286_template.dts | 42 +++
 5 files changed, 160 insertions(+), 1 deletion(-)
 create mode 100644 tools/binman/test/286_template.dts

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index a4b31fe5397b..73d38e6b64bb 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -727,6 +727,13 @@ optional:
 Note that missing, optional blobs do not produce a non-zero exit code from
 binman, although it does show a warning about the missing external blob.
 
+insert-template:
+This is not strictly speaking an entry property, since it is processed 
early
+in Binman before the entries are read. It is a list of phandles of nodes to
+include in the current (target) node. For each node, its subnodes and their
+properties are brought into the target node. See Templates_ below for
+more information.
+
 The attributes supported for images and sections are described below. Several
 are similar to those for entries.
 
@@ -1172,6 +1179,81 @@ If you are having trouble figuring out what is going on, 
you can use
  arch/arm/dts/u-boot.dtsi ... found: "arch/arm/dts/juno-r2-u-boot.dtsi"
 
 
+Templates
+=
+
+Sometimes multiple images need to be created which have all have a common
+part. For example, a board may generate SPI and eMMC images which both include
+a FIT. Since the FIT includes many entries, it is tedious to repeat them twice
+in the image description.
+
+Templates provide a simple way to handle this::
+
+binman {
+multiple-images;
+common_part: template-1 {
+fit {
+... lots of entries in here
+};
+
+text {
+text = "base image";
+};
+};
+
+spi-image {
+filename = "image-spi.bin";
+insert-template = <>;
+
+/* things specific to SPI follow */
+footer {
+];
+
+text {
+text = "SPI image";
+};
+};
+
+mmc-image {
+filename = "image-mmc.bin";
+insert-template = <>;
+
+/* things specific to MMC follow */
+footer {
+];
+
+text {
+text = "MMC image";
+};
+};
+};
+
+The template node name must start with 'template', so it is not considered to 
be
+an image itself.
+
+The mechanism is very simple. For each phandle in the 'insert-templates'
+property, the source node is looked up. Then the subnodes of that source node
+are copied into the target node, i.e. the one containing the `insert-template`
+property.
+
+If the target node has a node with the same name as a template, its properties
+override corresponding properties in the template. This allows the template to
+be uses as a base, with the node providing updates to the properties as needed.
+The overriding happens recursively.
+
+Template nodes appear first in each node that they are inserted into and
+ordering of template nodes is preserved. Other nodes come afterwards. If a
+template node also appears in the target node, then the template node sets the
+order. Thus the template can be used to set the ordering, even if the target
+node provides all the properties. In the above example, `fit` and `text` appear
+first in the `spi-image` and `mmc-image` images, followed by `footer`.
+
+Where there are multiple template nodes, they are inserted in that order. so
+the first template node appears first, then the second.
+
+Note that template nodes are not removed from the binman description at 
present.
+
+
 Updating an ELF file
 
 
diff --git a/tools/binman/control.py b/tools/binman/control.py
index 7e2dd3541b96..e9c4a65a75a1 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -22,6 +22,7 @@ from binman import bintool
 from binman import cbfs_util
 from binman import elf
 from binman import entry
+from dtoc import fdt_util
 from u_boot_pylib import command
 from u_boot_pylib import tools
 from u_boot_pylib import tout
@@ -478,6 +479,29 @@ def SignEntries(image_fname, input_fname, 
privatekey_fname, algo, entry_paths,
 
 AfterReplace(image, allow_resize=True, write_map=write_map)
 
+def _ProcessTemplates(parent):
+"""Handle any 

[PATCH v3 14/19] dtoc: Allow inserting a list of nodes into another

[Simon Glass]

Provide a way to specify a phandle list of nodes which are to be inserted
into an existing node.

Signed-off-by: Simon Glass 
---

Changes in v3:
- Adjust to use the new example file

 tools/dtoc/fdt.py  | 19 +++
 tools/dtoc/test/dtoc_test_copy.dts | 13 -
 tools/dtoc/test_fdt.py | 25 -
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index dcd78b9e5fbf..4375d3923fbf 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -732,6 +732,25 @@ class Node:
 for node in reversed(src.subnodes):
 dst.copy_node(node)
 
+def copy_subnodes_from_phandles(self, phandle_list):
+"""Copy subnodes of a list of nodes into another node
+
+Args:
+phandle_list (list of int): List of phandles of nodes to copy
+
+For each node in the phandle list, its subnodes and their properties 
are
+copied recursively. Note that it does not copy the node itself, nor its
+properties.
+"""
+# Process in reverse order, since new nodes are inserted at the start 
of
+# the destination's node list. We want them to appear in order of the
+# phandle list
+for phandle in phandle_list.__reversed__():
+parent = self.GetFdt().LookupPhandle(phandle)
+tout.debug(f'adding template {parent.path} to node {self.path}')
+for node in parent.subnodes.__reversed__():
+self.copy_node(node)
+
 
 class Fdt:
 """Provides simple access to a flat device tree blob using libfdts.
diff --git a/tools/dtoc/test/dtoc_test_copy.dts 
b/tools/dtoc/test/dtoc_test_copy.dts
index bf2b50e92d71..1939256349fe 100644
--- a/tools/dtoc/test/dtoc_test_copy.dts
+++ b/tools/dtoc/test/dtoc_test_copy.dts
@@ -10,6 +10,7 @@
 / {
#address-cells = <1>;
#size-cells = <1>;
+   copy-list = < >;
 
dest {
bootph-all;
@@ -45,7 +46,7 @@
};
};
 
-   base {
+   base: base {
compatible = "sandbox,i2c";
bootph-all;
#address-cells = <1>;
@@ -72,4 +73,14 @@
};
};
};
+
+   another: another {
+   earlier {
+   wibble = <2>;
+   };
+
+   later {
+   fibble = <3>;
+   };
+   };
 };
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index b3593cbee8b2..0ee4956591f9 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -316,7 +316,8 @@ class TestNode(unittest.TestCase):
 chk = dtb.GetNode('/dest/base')
 self.assertTrue(chk)
 self.assertEqual(
-{'compatible', 'bootph-all', '#address-cells', '#size-cells'},
+{'compatible', 'bootph-all', '#address-cells', '#size-cells',
+ 'phandle'},
 chk.props.keys())
 
 # Check the first property
@@ -376,6 +377,28 @@ class TestNode(unittest.TestCase):
 dst = new_dtb.GetNode('/dest')
 do_copy_checks(new_dtb, dst, expect_none=False)
 
+def test_copy_subnodes_from_phandles(self):
+"""Test copy_node() function"""
+dtb = fdt.FdtScan(find_dtb_file('dtoc_test_copy.dts'))
+
+orig = dtb.GetNode('/')
+node_list = fdt_util.GetPhandleList(orig, 'copy-list')
+
+dst = dtb.GetNode('/dest')
+dst.copy_subnodes_from_phandles(node_list)
+
+pmic = dtb.GetNode('/dest/over')
+self.assertTrue(pmic)
+
+subn = dtb.GetNode('/dest/first@0')
+self.assertTrue(subn)
+self.assertEqual({'a-prop', 'b-prop', 'reg'}, subn.props.keys())
+
+self.assertEqual(
+['/dest/earlier', '/dest/later', '/dest/over', '/dest/first@0',
+ '/dest/second', '/dest/existing', '/dest/base'],
+[n.path for n in dst.subnodes])
+
 
 class TestProp(unittest.TestCase):
 """Test operation of the Prop class"""
-- 
2.41.0.390.g38632f3daf-goog

[PATCH v3 13/19] dtoc: Support copying the contents of a node into another

[Simon Glass]

This permits implementation of a simple templating system, where a node
can be reused as a base for others.

For now this adds new subnodes after any existing ones.

Signed-off-by: Simon Glass 
---

Changes in v3:
- Add a new devicetree file especially for node copying
- Correct logic for merging nodes in order
- Tidy up some comments

 tools/dtoc/fdt.py  | 103 -
 tools/dtoc/test/dtoc_test_copy.dts |  75 +
 tools/dtoc/test_fdt.py |  70 
 3 files changed, 245 insertions(+), 3 deletions(-)
 create mode 100644 tools/dtoc/test/dtoc_test_copy.dts

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index a8e05349a720..dcd78b9e5fbf 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -13,6 +13,7 @@ from dtoc import fdt_util
 import libfdt
 from libfdt import QUIET_NOTFOUND
 from u_boot_pylib import tools
+from u_boot_pylib import tout
 
 # This deals with a device tree, presenting it as an assortment of Node and
 # Prop objects, representing nodes and properties, respectively. This file
@@ -264,6 +265,13 @@ class Prop:
 fdt_obj.setprop(node.Offset(), self.name, self.bytes)
 self.dirty = False
 
+def purge(self):
+"""Set a property offset to None
+
+The property remains in the tree structure and will be recreated when
+the FDT is synced
+"""
+self._offset = None
 
 class Node:
 """A device tree node
@@ -534,8 +542,8 @@ class Node:
 """
 return self.AddData(prop_name, struct.pack('>I', val))
 
-def AddSubnode(self, name):
-"""Add a new subnode to the node
+def Subnode(self, name):
+"""Create new subnode for the node
 
 Args:
 name: name of node to add
@@ -544,10 +552,72 @@ class Node:
 New subnode that was created
 """
 path = self.path + '/' + name
-subnode = Node(self._fdt, self, None, name, path)
+return Node(self._fdt, self, None, name, path)
+
+def AddSubnode(self, name):
+"""Add a new subnode to the node, after all other subnodes
+
+Args:
+name: name of node to add
+
+Returns:
+New subnode that was created
+"""
+subnode = self.Subnode(name)
 self.subnodes.append(subnode)
 return subnode
 
+def insert_subnode(self, name):
+"""Add a new subnode to the node, before all other subnodes
+
+This deletes other subnodes and sets their offset to None, so that they
+will be recreated after this one.
+
+Args:
+name: name of node to add
+
+Returns:
+New subnode that was created
+"""
+# Deleting a node invalidates the offsets of all following nodes, so
+# process in reverse order so that the offset of each node remains 
valid
+# until deletion.
+for subnode in reversed(self.subnodes):
+subnode.purge(True)
+subnode = self.Subnode(name)
+self.subnodes.insert(0, subnode)
+return subnode
+
+def purge(self, delete_it=False):
+"""Purge this node, setting offset to None and deleting from FDT"""
+if self._offset is not None:
+if delete_it:
+CheckErr(self._fdt._fdt_obj.del_node(self.Offset()),
+ "Node '%s': delete" % self.path)
+self._offset = None
+self._fdt.Invalidate()
+
+for prop in self.props.values():
+prop.purge()
+
+for subnode in self.subnodes:
+subnode.purge(False)
+
+def move_to_first(self):
+"""Move the current node to first in its parent's node list"""
+parent = self.parent
+if parent.subnodes and parent.subnodes[0] == self:
+return
+for subnode in reversed(parent.subnodes):
+subnode.purge(True)
+
+new_subnodes = [self]
+for subnode in parent.subnodes:
+#subnode.purge(False)
+if subnode != self:
+new_subnodes.append(subnode)
+parent.subnodes = new_subnodes
+
 def Delete(self):
 """Delete a node
 
@@ -635,6 +705,33 @@ class Node:
 prop.Sync(auto_resize)
 return added
 
+def copy_node(self, src):
+"""Copy a node and all its subnodes into this node
+
+Args:
+src (Node): Node to copy
+
+This works recursively.
+
+The new node is put before all other nodes. If the node already
+exists, just its subnodes and properties are copied, placing them 
before
+any existing subnodes. Properties which exist in the destination node
+already are not copied.
+"""
+dst = self.FindNode(src.name)
+if dst:
+dst.move_to_first()
+else:
+dst = self.insert_subnode(src.name)
+for name, src_prop in src.props.items():
+

[PATCH v3 12/19] binman: Correct handling of zero bss size

[Simon Glass]

Fix the check for the __bss_size symbol, since it may be 0. Unfortunately
there was no test coverage for this.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 tools/binman/elf_test.py |  5 +
 tools/binman/etype/u_boot_spl_bss_pad.py |  2 +-
 tools/binman/etype/u_boot_tpl_bss_pad.py |  2 +-
 tools/binman/etype/u_boot_vpl_bss_pad.py |  2 +-
 tools/binman/ftest.py| 12 
 tools/binman/test/285_spl_expand.dts | 13 +
 tools/binman/test/Makefile   |  5 -
 tools/binman/test/bss_data_zero.c| 16 
 tools/binman/test/bss_data_zero.lds  | 15 +++
 tools/binman/test/embed_data.lds |  1 +
 10 files changed, 69 insertions(+), 4 deletions(-)
 create mode 100644 tools/binman/test/285_spl_expand.dts
 create mode 100644 tools/binman/test/bss_data_zero.c
 create mode 100644 tools/binman/test/bss_data_zero.lds

diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py
index 2fb3f6f28ff1..cc95b424b338 100644
--- a/tools/binman/elf_test.py
+++ b/tools/binman/elf_test.py
@@ -371,6 +371,11 @@ class TestElf(unittest.TestCase):
 elf.GetSymbolOffset(fname, 'embed')
 self.assertIn('__image_copy_start', str(e.exception))
 
+def test_get_symbol_address(self):
+fname = self.ElfTestFile('embed_data')
+addr = elf.GetSymbolAddress(fname, 'region_size')
+self.assertEqual(0, addr)
+
 
 if __name__ == '__main__':
 unittest.main()
diff --git a/tools/binman/etype/u_boot_spl_bss_pad.py 
b/tools/binman/etype/u_boot_spl_bss_pad.py
index 1ffeb3911fd8..4af4045d3702 100644
--- a/tools/binman/etype/u_boot_spl_bss_pad.py
+++ b/tools/binman/etype/u_boot_spl_bss_pad.py
@@ -38,7 +38,7 @@ class Entry_u_boot_spl_bss_pad(Entry_blob):
 def ObtainContents(self):
 fname = tools.get_input_filename('spl/u-boot-spl')
 bss_size = elf.GetSymbolAddress(fname, '__bss_size')
-if not bss_size:
+if bss_size is None:
 self.Raise('Expected __bss_size symbol in spl/u-boot-spl')
 self.SetContents(tools.get_bytes(0, bss_size))
 return True
diff --git a/tools/binman/etype/u_boot_tpl_bss_pad.py 
b/tools/binman/etype/u_boot_tpl_bss_pad.py
index 29c6a9541296..46d2cd58f7e2 100644
--- a/tools/binman/etype/u_boot_tpl_bss_pad.py
+++ b/tools/binman/etype/u_boot_tpl_bss_pad.py
@@ -38,7 +38,7 @@ class Entry_u_boot_tpl_bss_pad(Entry_blob):
 def ObtainContents(self):
 fname = tools.get_input_filename('tpl/u-boot-tpl')
 bss_size = elf.GetSymbolAddress(fname, '__bss_size')
-if not bss_size:
+if bss_size is None:
 self.Raise('Expected __bss_size symbol in tpl/u-boot-tpl')
 self.SetContents(tools.get_bytes(0, bss_size))
 return True
diff --git a/tools/binman/etype/u_boot_vpl_bss_pad.py 
b/tools/binman/etype/u_boot_vpl_bss_pad.py
index bba38ccf9e93..12b286a71987 100644
--- a/tools/binman/etype/u_boot_vpl_bss_pad.py
+++ b/tools/binman/etype/u_boot_vpl_bss_pad.py
@@ -38,7 +38,7 @@ class Entry_u_boot_vpl_bss_pad(Entry_blob):
 def ObtainContents(self):
 fname = tools.get_input_filename('vpl/u-boot-vpl')
 bss_size = elf.GetSymbolAddress(fname, '__bss_size')
-if not bss_size:
+if bss_size is None:
 self.Raise('Expected __bss_size symbol in vpl/u-boot-vpl')
 self.SetContents(tools.get_bytes(0, bss_size))
 return True
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index ed1940ed9f71..dc9a95d341e5 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -6773,6 +6773,18 @@ fdt fdtmapExtract the devicetree 
blob from the fdtmap
 self.assertEqual(U_BOOT_NODTB_DATA, data[-len(U_BOOT_NODTB_DATA):])
 fit_data = data[len(U_BOOT_DATA):-len(U_BOOT_NODTB_DATA)]
 
+def testSplEmptyBss(self):
+"""Test an expanded SPL with a zero-size BSS"""
+# ELF file with a '__bss_size' symbol
+self._SetupSplElf(src_fname='bss_data_zero')
+
+entry_args = {
+'spl-bss-pad': 'y',
+'spl-dtb': 'y',
+}
+data = self._DoReadFileDtb('285_spl_expand.dts',
+   use_expanded=True, entry_args=entry_args)[0]
+
 
 if __name__ == "__main__":
 unittest.main()
diff --git a/tools/binman/test/285_spl_expand.dts 
b/tools/binman/test/285_spl_expand.dts
new file mode 100644
index ..9c88ccb287b1
--- /dev/null
+++ b/tools/binman/test/285_spl_expand.dts
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   binman {
+   u-boot-spl {
+   };
+   };
+};
diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile
index cd66a3038be2..4d152eee9c09 100644
--- a/tools/binman/test/Makefile
+++ b/tools/binman/test/Makefile
@@ -32,7 +32,7 @@ LDS_BINMAN_EMBED := -T 

[PATCH v3 11/19] binman: Drop __bss_size variable in bss_data.c

[Simon Glass]

This is not needed since the linker script sets it up. Drop the variable
to avoid confusion.

Fix the prototype for main() while we are here.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 tools/binman/test/bss_data.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/binman/test/bss_data.c b/tools/binman/test/bss_data.c
index 4f9b64cef9ef..7047a3bb014d 100644
--- a/tools/binman/test/bss_data.c
+++ b/tools/binman/test/bss_data.c
@@ -7,9 +7,8 @@
  */
 
 int bss_data[10];
-int __bss_size = sizeof(bss_data);
 
-int main()
+int main(void)
 {
bss_data[2] = 2;
 
-- 
2.41.0.390.g38632f3daf-goog

[PATCH v3 09/19] binman: Convert mkimage to Entry_section

[Simon Glass]

From: Marek Vasut 

This is needed to handle mkimage with inner section located itself in a
section.

Signed-off-by: Marek Vasut 
Use BuildSectionData() instead of ObtainContents(), add tests and a few
other minor fixes:
Signed-off-by: Simon Glass 
---

Changes in v3:
- Fix up some tests which now need SPL and TPL
- Avoid calling ObtainContents() when not needed

Changes in v2:
- Drop now-unnecessary methods in mkimage etype

 tools/binman/entry.py |  6 +-
 tools/binman/etype/mkimage.py | 71 ++-
 tools/binman/ftest.py | 69 +-
 tools/binman/test/283_mkimage_special.dts | 24 
 4 files changed, 135 insertions(+), 35 deletions(-)
 create mode 100644 tools/binman/test/283_mkimage_special.dts

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index f20f32213a9b..0d4cb94f7002 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -1314,10 +1314,8 @@ features to produce new behaviours.
 """
 data = b''
 for entry in entries:
-# First get the input data and put it in a file. If not available,
-# try later.
-if not entry.ObtainContents(fake_size=fake_size):
-return None, None, None
+# First get the input data and put it in a file
+entry.ObtainContents(fake_size=fake_size)
 data += entry.GetData()
 uniq = self.GetUniqueName()
 fname = tools.get_output_filename(f'{prefix}.{uniq}')
diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
index cb3e10672ad7..493761da59f5 100644
--- a/tools/binman/etype/mkimage.py
+++ b/tools/binman/etype/mkimage.py
@@ -8,10 +8,11 @@
 from collections import OrderedDict
 
 from binman.entry import Entry
+from binman.etype.section import Entry_section
 from dtoc import fdt_util
 from u_boot_pylib import tools
 
-class Entry_mkimage(Entry):
+class Entry_mkimage(Entry_section):
 """Binary produced by mkimage
 
 Properties / Entry arguments:
@@ -121,10 +122,8 @@ class Entry_mkimage(Entry):
 """
 def __init__(self, section, etype, node):
 super().__init__(section, etype, node)
-self._mkimage_entries = OrderedDict()
 self._imagename = None
-self._filename = fdt_util.GetString(self._node, 'filename')
-self.align_default = None
+self._multiple_data_files = False
 
 def ReadNode(self):
 super().ReadNode()
@@ -135,41 +134,55 @@ class Entry_mkimage(Entry):
'data-to-imagename')
 if self._data_to_imagename and self._node.FindNode('imagename'):
 self.Raise('Cannot use both imagename node and data-to-imagename')
-self.ReadEntries()
 
 def ReadEntries(self):
 """Read the subnodes to find out what should go in this image"""
 for node in self._node.subnodes:
-entry = Entry.Create(self, node)
+if self.IsSpecialSubnode(node):
+continue
+entry = Entry.Create(self, node,
+ expanded=self.GetImage().use_expanded,
+ missing_etype=self.GetImage().missing_etype)
 entry.ReadNode()
+entry.SetPrefix(self._name_prefix)
 if entry.name == 'imagename':
 self._imagename = entry
 else:
-self._mkimage_entries[entry.name] = entry
+self._entries[entry.name] = entry
 
-def ObtainContents(self):
+def BuildSectionData(self, required):
+"""Build mkimage entry contents
+
+Runs mkimage to build the entry contents
+
+Args:
+required (bool): True if the data must be present, False if it is 
OK
+to return None
+
+Returns:
+bytes: Contents of the section
+"""
 # Use a non-zero size for any fake files to keep mkimage happy
 # Note that testMkimageImagename() relies on this 'mkimage' parameter
 fake_size = 1024
 if self._multiple_data_files:
 fnames = []
 uniq = self.GetUniqueName()
-for entry in self._mkimage_entries.values():
-if not entry.ObtainContents(fake_size=fake_size):
-return False
-if entry._pathname:
-fnames.append(entry._pathname)
+for entry in self._entries.values():
+# Put the contents in a temporary file
+ename = f'mkimage-in-{uniq}-{entry.name}'
+fname = tools.get_output_filename(ename)
+data = entry.GetData(required)
+tools.write_file(fname, data)
+fnames.append(fname)
 input_fname = ":".join(fnames)
+data = b''
 else:
 data, input_fname, uniq = self.collect_contents_to_file(
-

[PATCH v3 10/19] binman: Provide a way to specify the fdt-list directly

[Simon Glass]

Sometimes multiple boards are built with binman and it is useful to
specify a different FDT list for each. At present this is not possible
without providing multiple values of the of-list entryarg (which is not
supported in the U-Boot build system).

Allow a fit,fdt-list-val string-list property to be used instead.

Signed-off-by: Simon Glass 
---

Changes in v3:
- Fix 'specific' typo

 tools/binman/entries.rst   |  6 +++
 tools/binman/etype/fit.py  |  9 
 tools/binman/ftest.py  | 14 ++-
 tools/binman/test/284_fit_fdt_list.dts | 58 ++
 4 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 tools/binman/test/284_fit_fdt_list.dts

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index b71af801fdad..b55f424620a3 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -615,6 +615,12 @@ The top-level 'fit' node supports the following special 
properties:
 `of-list` meaning that `-a of-list="dtb1 dtb2..."` should be passed
 to binman.
 
+fit,fdt-list-val
+As an alternative to fit,fdt-list the list of device tree files
+can be provided in this property as a string list, e.g.::
+
+fit,fdt-list-val = "dtb1", "dtb2";
+
 Substitutions
 ~
 
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
index c395706ece5f..ef4d0667578d 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -81,6 +81,12 @@ class Entry_fit(Entry_section):
 `of-list` meaning that `-a of-list="dtb1 dtb2..."` should be passed
 to binman.
 
+fit,fdt-list-val
+As an alternative to fit,fdt-list the list of device tree files
+can be provided in this property as a string list, e.g.::
+
+fit,fdt-list-val = "dtb1", "dtb2";
+
 Substitutions
 ~
 
@@ -361,6 +367,9 @@ class Entry_fit(Entry_section):
 [EntryArg(self._fit_list_prop.value, str)])
 if fdts is not None:
 self._fdts = fdts.split()
+else:
+self._fdts = fdt_util.GetStringList(self._node, 'fit,fdt-list-val')
+
 self._fit_default_dt = self.GetEntryArgsOrProps([EntryArg('default-dt',
   str)])[0]
 
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 60e69443c400..ed1940ed9f71 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -6761,6 +6761,18 @@ fdt fdtmapExtract the devicetree 
blob from the fdtmap
 # Just check that the data appears in the file somewhere
 self.assertIn(U_BOOT_DATA, data)
 
+def testFitFdtList(self):
+"""Test an image with an FIT with the fit,fdt-list-val option"""
+entry_args = {
+'default-dt': 'test-fdt2',
+}
+data = self._DoReadFileDtb(
+'284_fit_fdt_list.dts',
+entry_args=entry_args,
+extra_indirs=[os.path.join(self._indir, TEST_FDT_SUBDIR)])[0]
+self.assertEqual(U_BOOT_NODTB_DATA, data[-len(U_BOOT_NODTB_DATA):])
+fit_data = data[len(U_BOOT_DATA):-len(U_BOOT_NODTB_DATA)]
+
 
-if __name__ == "_s_main__":
+if __name__ == "__main__":
 unittest.main()
diff --git a/tools/binman/test/284_fit_fdt_list.dts 
b/tools/binman/test/284_fit_fdt_list.dts
new file mode 100644
index ..8885313f5b88
--- /dev/null
+++ b/tools/binman/test/284_fit_fdt_list.dts
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   binman {
+   u-boot {
+   };
+   fit {
+   description = "test-desc";
+   #address-cells = <1>;
+   fit,fdt-list-val = "test-fdt1", "test-fdt2";
+
+   images {
+   kernel {
+   description = "Vanilla Linux kernel";
+   type = "kernel";
+   arch = "ppc";
+   os = "linux";
+   compression = "gzip";
+   load = <>;
+   entry = <>;
+   hash-1 {
+   algo = "crc32";
+   };
+   hash-2 {
+   algo = "sha1";
+   };
+   u-boot {
+   };
+   };
+   @fdt-SEQ {
+   description = "fdt-NAME.dtb";
+   

[PATCH v3 07/19] binman: Update elf to return number of written symbols

[Simon Glass]

Update the LookupAndWriteSymbols() function to return the number of
symbols written. Also add some logging for when debugging is not
enabled.

Signed-off-by: Simon Glass 
---

Changes in v3:
- Add new patch for elf to return number of written symbols

 tools/binman/elf.py  | 13 +++--
 tools/binman/elf_test.py |  8 +---
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/tools/binman/elf.py b/tools/binman/elf.py
index 5816284c32a2..4219001feac3 100644
--- a/tools/binman/elf.py
+++ b/tools/binman/elf.py
@@ -248,6 +248,9 @@ def LookupAndWriteSymbols(elf_fname, entry, section, 
is_elf=False,
 entry: Entry to process
 section: Section which can be used to lookup symbol values
 base_sym: Base symbol marking the start of the image
+
+Returns:
+int: Number of symbols written
 """
 if not base_sym:
 base_sym = '__image_copy_start'
@@ -269,12 +272,13 @@ def LookupAndWriteSymbols(elf_fname, entry, section, 
is_elf=False,
 
 if not syms:
 tout.debug('LookupAndWriteSymbols: no syms')
-return
+return 0
 base = syms.get(base_sym)
 if not base and not is_elf:
 tout.debug('LookupAndWriteSymbols: no base')
-return
+return 0
 base_addr = 0 if is_elf else base.address
+count = 0
 for name, sym in syms.items():
 if name.startswith('_binman'):
 msg = ("Section '%s': Symbol '%s'\n   in entry '%s'" %
@@ -307,6 +311,11 @@ def LookupAndWriteSymbols(elf_fname, entry, section, 
is_elf=False,
(msg, name, offset, value, len(value_bytes)))
 entry.data = (entry.data[:offset] + value_bytes +
 entry.data[offset + sym.size:])
+count += 1
+if count:
+tout.detail(
+f"Section '{section.GetPath()}': entry '{entry.GetPath()}' : 
{count} symbols")
+return count
 
 def GetSymbolValue(sym, data, msg):
 """Get the value of a symbol
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py
index c98083961b53..2fb3f6f28ff1 100644
--- a/tools/binman/elf_test.py
+++ b/tools/binman/elf_test.py
@@ -141,7 +141,8 @@ class TestElf(unittest.TestCase):
 entry = FakeEntry(10)
 section = FakeSection()
 elf_fname = self.ElfTestFile('u_boot_binman_syms_bad')
-elf.LookupAndWriteSymbols(elf_fname, entry, section)
+count = elf.LookupAndWriteSymbols(elf_fname, entry, section)
+self.assertEqual(0, count)
 
 def testBadSymbolSize(self):
 """Test that an attempt to use an 8-bit symbol are detected
@@ -162,7 +163,7 @@ class TestElf(unittest.TestCase):
 def testNoValue(self):
 """Test the case where we have no value for the symbol
 
-This should produce -1 values for all thress symbols, taking up the
+This should produce -1 values for all three symbols, taking up the
 first 16 bytes of the image.
 """
 if not elf.ELF_TOOLS:
@@ -170,7 +171,8 @@ class TestElf(unittest.TestCase):
 entry = FakeEntry(28)
 section = FakeSection(sym_value=None)
 elf_fname = self.ElfTestFile('u_boot_binman_syms')
-elf.LookupAndWriteSymbols(elf_fname, entry, section)
+count = elf.LookupAndWriteSymbols(elf_fname, entry, section)
+self.assertEqual(5, count)
 expected = (struct.pack('

[PATCH v3 08/19] binman: Add more detail on how ObtainContents() works

[Simon Glass]

This area of binman can be a bit confusing. Add some more comments to
help.

Signed-off-by: Simon Glass 
---

Changes in v3:
- Add new patch with more detail on how ObtainContents() works

 tools/binman/entry.py |  3 +++
 tools/binman/etype/section.py | 32 +++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 328b5bc568a9..f20f32213a9b 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -474,6 +474,9 @@ class Entry(object):
 def ObtainContents(self, skip_entry=None, fake_size=0):
 """Figure out the contents of an entry.
 
+For missing blobs (where allow-missing is enabled), the contents are 
set
+to b'' and self.missing is set to True.
+
 Args:
 skip_entry (Entry): Entry to skip when obtaining section contents
 fake_size (int): Size of fake file to create if needed
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index d56cc11d1023..d9b9e428024a 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -316,12 +316,15 @@ class Entry_section(Entry):
 This should be overridden by subclasses which want to build their own
 data structure for the section.
 
+Missing entries will have be given empty (or fake) data, so are
+processed normally here.
+
 Args:
 required: True if the data must be present, False if it is OK to
 return None
 
 Returns:
-Contents of the section (bytes)
+Contents of the section (bytes), None if not available
 """
 section_data = bytearray()
 
@@ -711,6 +714,33 @@ class Entry_section(Entry):
 def GetEntryContents(self, skip_entry=None):
 """Call ObtainContents() for each entry in the section
 
+The overall goal of this function is to read in any available data in
+this entry and any subentries. This includes reading in blobs, setting
+up objects which have predefined contents, etc.
+
+Since entry types which contain entries call ObtainContents() on all
+those entries too, the result is that ObtainContents() is called
+recursively for the whole tree below this one.
+
+Entries with subentries are generally not *themselves& processed here,
+i.e. their ObtainContents() implementation simply obtains contents of
+their subentries, skipping their own contents. For example, the
+implementation here (for entry_Section) does not attempt to pack the
+entries into a final result. That is handled later.
+
+Generally, calling this results in SetContents() being called for each
+entry, so that the 'data' and 'contents_size; properties are set, and
+subsequent calls to GetData() will return value data.
+
+Where 'allow_missing' is set, this can result in the 'missing' property
+being set to True if there is no data. This is handled by setting the
+data to b''. This function will still return success. Future calls to
+GetData() for this entry will return b'', or in the case where the data
+is faked, GetData() will return that fake data.
+
+Args:
+skip_entry: (single) Entry to skip, or None to process all entries
+
 Note that this may set entry.absent to True if the entry is not
 actually needed
 """
-- 
2.41.0.390.g38632f3daf-goog

[PATCH v3 06/19] stm32mp15: Avoid writing symbols in SPL

[Simon Glass]

These boards use SPL in a mkimage entry and apparently access the symbol
containing the image position of U-Boot, but put U-Boot in another
image. This means that binman is unable to fill in the symbol correctly
in the SPL binary.

This doesn't matter at present since mkimage doesn't support symbol
writing. But with the upcoming conversion to a section, it will. So add
a property to disable symbol writing.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 arch/arm/dts/stm32mp15-u-boot.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/dts/stm32mp15-u-boot.dtsi 
b/arch/arm/dts/stm32mp15-u-boot.dtsi
index d872c6fc5679..573dd4d3ed56 100644
--- a/arch/arm/dts/stm32mp15-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp15-u-boot.dtsi
@@ -226,6 +226,7 @@
mkimage {
args = "-T stm32image -a 0x2ffc2500 -e 0x2ffc2500";
u-boot-spl {
+   no-write-symbols;
};
};
};
-- 
2.41.0.390.g38632f3daf-goog

[PATCH v3 05/19] binman: Allow disabling symbol writing

[Simon Glass]

Some boards don't use symbol writing but do access the symbols in SPL.
Provide an option to work around this.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 tools/binman/binman.rst   |  7 ++
 tools/binman/entry.py |  4 +++-
 tools/binman/etype/blob_phase.py  |  5 
 tools/binman/ftest.py | 28 +++
 tools/binman/test/282_symbols_disable.dts | 25 
 5 files changed, 64 insertions(+), 5 deletions(-)
 create mode 100644 tools/binman/test/282_symbols_disable.dts

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index 23cbb99b4b0b..a4b31fe5397b 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -831,6 +831,13 @@ write-symbols:
 binman. This is automatic for certain entry types, e.g. `u-boot-spl`. See
 binman_syms_ for more information.
 
+no-write-symbols:
+Disables symbol writing for this entry. This can be used in entry types
+where symbol writing is automatic. For example, if `u-boot-spl` refers to
+the `u_boot_any_image_pos` symbol but U-Boot is not available in the image
+containing SPL, this can be used to disable the writing. Quite likely this
+indicates a bug in your setup.
+
 elf-filename:
 Sets the file name of a blob's associated ELF file. For example, if the
 blob is `zephyr.bin` then the ELF file may be `zephyr.elf`. This allows
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 39456906a477..328b5bc568a9 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -158,6 +158,7 @@ class Entry(object):
 self.offset_from_elf = None
 self.preserve = False
 self.build_done = False
+self.no_write_symbols = False
 
 @staticmethod
 def FindEntryClass(etype, expanded):
@@ -321,6 +322,7 @@ class Entry(object):
  'offset-from-elf')
 
 self.preserve = fdt_util.GetBool(self._node, 'preserve')
+self.no_write_symbols = fdt_util.GetBool(self._node, 
'no-write-symbols')
 
 def GetDefaultFilename(self):
 return None
@@ -695,7 +697,7 @@ class Entry(object):
 Args:
   section: Section containing the entry
 """
-if self.auto_write_symbols:
+if self.auto_write_symbols and not self.no_write_symbols:
 # Check if we are writing symbols into an ELF file
 is_elf = self.GetDefaultFilename() == self.elf_fname
 elf.LookupAndWriteSymbols(self.elf_fname, self, section.GetImage(),
diff --git a/tools/binman/etype/blob_phase.py b/tools/binman/etype/blob_phase.py
index b937158756fd..951d9934050e 100644
--- a/tools/binman/etype/blob_phase.py
+++ b/tools/binman/etype/blob_phase.py
@@ -52,3 +52,8 @@ class Entry_blob_phase(Entry_section):
 
 # Read entries again, now that we have some
 self.ReadEntries()
+
+# Propagate the no-write-symbols property
+if self.no_write_symbols:
+for entry in self._entries.values():
+entry.no_write_symbols = True
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 43b4f850a691..dabb3f689fdb 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -1452,7 +1452,7 @@ class TestFunctional(unittest.TestCase):
 self.assertEqual(U_BOOT_SPL_NODTB_DATA, 
data[:len(U_BOOT_SPL_NODTB_DATA)])
 
 def checkSymbols(self, dts, base_data, u_boot_offset, entry_args=None,
- use_expanded=False):
+ use_expanded=False, no_write_symbols=False):
 """Check the image contains the expected symbol values
 
 Args:
@@ -1481,9 +1481,14 @@ class TestFunctional(unittest.TestCase):
 sym_values = struct.pack(';
+   #size-cells = <1>;
+
+   binman {
+   pad-byte = <0xff>;
+   u-boot-spl {
+   no-write-symbols;
+   };
+
+   u-boot {
+   offset = <0x38>;
+   no-expanded;
+   };
+
+   u-boot-spl2 {
+   type = "u-boot-spl";
+   no-write-symbols;
+   };
+   };
+};
-- 
2.41.0.390.g38632f3daf-goog

[PATCH v3 03/19] binman: Use GetEntries() to obtain section contents

[Simon Glass]

Some section types don't have a simple _entries list. Use the GetEntries()
method in GetEntryContents() and other places to handle this.

This makes the behaviour more consistent.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 tools/binman/etype/section.py | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 77250a7525c6..d56cc11d1023 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -720,7 +720,7 @@ class Entry_section(Entry):
 next_todo.append(entry)
 return entry
 
-todo = self._entries.values()
+todo = self.GetEntries().values()
 for passnum in range(3):
 threads = state.GetThreads()
 next_todo = []
@@ -893,7 +893,7 @@ class Entry_section(Entry):
 allow_missing: True if allowed, False if not allowed
 """
 self.allow_missing = allow_missing
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.SetAllowMissing(allow_missing)
 
 def SetAllowFakeBlob(self, allow_fake):
@@ -903,7 +903,7 @@ class Entry_section(Entry):
 allow_fake: True if allowed, False if not allowed
 """
 super().SetAllowFakeBlob(allow_fake)
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.SetAllowFakeBlob(allow_fake)
 
 def CheckMissing(self, missing_list):
@@ -915,7 +915,7 @@ class Entry_section(Entry):
 Args:
 missing_list: List of Entry objects to be added to
 """
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.CheckMissing(missing_list)
 
 def CheckFakedBlobs(self, faked_blobs_list):
@@ -926,7 +926,7 @@ class Entry_section(Entry):
 Args:
 faked_blobs_list: List of Entry objects to be added to
 """
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.CheckFakedBlobs(faked_blobs_list)
 
 def CheckOptional(self, optional_list):
@@ -937,7 +937,7 @@ class Entry_section(Entry):
 Args:
 optional_list (list): List of Entry objects to be added to
 """
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.CheckOptional(optional_list)
 
 def check_missing_bintools(self, missing_list):
@@ -949,7 +949,7 @@ class Entry_section(Entry):
 missing_list: List of Bintool objects to be added to
 """
 super().check_missing_bintools(missing_list)
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.check_missing_bintools(missing_list)
 
 def _CollectEntries(self, entries, entries_by_name, add_entry):
@@ -999,12 +999,12 @@ class Entry_section(Entry):
 entry.Raise(f'Missing required properties/entry args: {missing}')
 
 def CheckAltFormats(self, alt_formats):
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.CheckAltFormats(alt_formats)
 
 def AddBintools(self, btools):
 super().AddBintools(btools)
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.AddBintools(btools)
 
 def read_elf_segments(self):
-- 
2.41.0.390.g38632f3daf-goog

[PATCH v3 04/19] binman: Read _multiple_data_files in the correct place

[Simon Glass]

Move this to the ReadEntries() function where it belongs.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 tools/binman/etype/mkimage.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
index e028c4407081..cb3e10672ad7 100644
--- a/tools/binman/etype/mkimage.py
+++ b/tools/binman/etype/mkimage.py
@@ -121,7 +121,6 @@ class Entry_mkimage(Entry):
 """
 def __init__(self, section, etype, node):
 super().__init__(section, etype, node)
-self._multiple_data_files = fdt_util.GetBool(self._node, 
'multiple-data-files')
 self._mkimage_entries = OrderedDict()
 self._imagename = None
 self._filename = fdt_util.GetString(self._node, 'filename')
@@ -129,6 +128,8 @@ class Entry_mkimage(Entry):
 
 def ReadNode(self):
 super().ReadNode()
+self._multiple_data_files = fdt_util.GetBool(self._node,
+ 'multiple-data-files')
 self._args = fdt_util.GetArgs(self._node, 'args')
 self._data_to_imagename = fdt_util.GetBool(self._node,
'data-to-imagename')
-- 
2.41.0.390.g38632f3daf-goog

[PATCH v3 01/19] binman: Correct coverage gap in control

[Simon Glass]

Add a pragma to deal with the code-coverage gap which drops binman down to
90% coverage.

Fixes: de65b122a25 (tools: Fall back to importlib_resources on Python 3.6)

Signed-off-by: Simon Glass 
---

(no changes since v1)

 tools/binman/control.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py
index 68597c4e7792..7e2dd3541b96 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -9,7 +9,7 @@ from collections import OrderedDict
 import glob
 try:
 import importlib.resources
-except ImportError:
+except ImportError:  # pragma: no cover
 # for Python 3.6
 import importlib_resources
 import os
-- 
2.41.0.390.g38632f3daf-goog

[PATCH v3 02/19] binman: Init align_default in entry_Section

[Simon Glass]

This should be set up in the init function, to avoid a warning about a
property not set up there. Fix it.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 tools/binman/etype/section.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index c36edd13508b..77250a7525c6 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -168,6 +168,7 @@ class Entry_section(Entry):
 self._end_4gb = False
 self._ignore_missing = False
 self._filename = None
+self.align_default = 0
 
 def IsSpecialSubnode(self, node):
 """Check if a node is a special one used by the section itself
-- 
2.41.0.390.g38632f3daf-goog

[PATCH v3 00/19] binman: Simple templating feature and mkimage conversion

[Simon Glass]

This series converts the mkimage entry type to be a section, i.e. based on
the entry_Section class. This makes it more consistent in its behaviour,
e.g. allowing symbol writing and expanded entries.

A simple templating feature is also introduced, to reduce duplication
when a set of entries must be used in multiple images.

In this version the nodes from the template are placed before any other
nodes, meaning that the template sets the node order. This seems more
consistent than other mechanisms.

Changes in v3:
- Add new patch for elf to return number of written symbols
- Add new patch with more detail on how ObtainContents() works
- Fix up some tests which now need SPL and TPL
- Avoid calling ObtainContents() when not needed
- Fix 'specific' typo
- Add a new devicetree file especially for node copying
- Correct logic for merging nodes in order
- Tidy up some comments
- Adjust to use the new example file
- Drop duplicate dts-v1 header
- Add new test case for templating in a FIT
- Add new patch to support writing symbols inside a mkimage image

Changes in v2:
- Drop now-unnecessary methods in mkimage etype
- Correct ordering of template nodes
- Fix 'preseverd' and 'inserter' typos

Marek Vasut (1):
  binman: Convert mkimage to Entry_section

Simon Glass (18):
  binman: Correct coverage gap in control
  binman: Init align_default in entry_Section
  binman: Use GetEntries() to obtain section contents
  binman: Read _multiple_data_files in the correct place
  binman: Allow disabling symbol writing
  stm32mp15: Avoid writing symbols in SPL
  binman: Update elf to return number of written symbols
  binman: Add more detail on how ObtainContents() works
  binman: Provide a way to specify the fdt-list directly
  binman: Drop __bss_size variable in bss_data.c
  binman: Correct handling of zero bss size
  dtoc: Support copying the contents of a node into another
  dtoc: Allow inserting a list of nodes into another
  binman: Support simple templates
  binman: Support templating with multiple images
  binman: Add a test for templating in a FIT
  binman: Support templates at any level
  binman: Support writing symbols inside a mkimage image

 arch/arm/dts/stm32mp15-u-boot.dtsi |   1 +
 tools/binman/binman.rst|  89 +
 tools/binman/control.py|  34 +++-
 tools/binman/elf.py|  13 +-
 tools/binman/elf_test.py   |  13 +-
 tools/binman/entries.rst   |   6 +
 tools/binman/entry.py  |  11 +-
 tools/binman/etype/blob_phase.py   |   5 +
 tools/binman/etype/fit.py  |   9 +
 tools/binman/etype/mkimage.py  | 110 ---
 tools/binman/etype/section.py  |  54 +++--
 tools/binman/etype/u_boot_spl_bss_pad.py   |   2 +-
 tools/binman/etype/u_boot_tpl_bss_pad.py   |   2 +-
 tools/binman/etype/u_boot_vpl_bss_pad.py   |   2 +-
 tools/binman/ftest.py  | 218 -
 tools/binman/test/282_symbols_disable.dts  |  25 +++
 tools/binman/test/283_mkimage_special.dts  |  24 +++
 tools/binman/test/284_fit_fdt_list.dts |  58 ++
 tools/binman/test/285_spl_expand.dts   |  13 ++
 tools/binman/test/286_template.dts |  42 
 tools/binman/test/287_template_multi.dts   |  27 +++
 tools/binman/test/288_template_fit.dts |  37 
 tools/binman/test/289_template_section.dts |  52 +
 tools/binman/test/290_mkimage_sym.dts  |  27 +++
 tools/binman/test/Makefile |   5 +-
 tools/binman/test/bss_data.c   |   3 +-
 tools/binman/test/bss_data_zero.c  |  16 ++
 tools/binman/test/bss_data_zero.lds|  15 ++
 tools/binman/test/embed_data.lds   |   1 +
 tools/dtoc/fdt.py  | 122 +++-
 tools/dtoc/test/dtoc_test_copy.dts |  86 
 tools/dtoc/test_fdt.py |  93 +
 32 files changed, 1147 insertions(+), 68 deletions(-)
 create mode 100644 tools/binman/test/282_symbols_disable.dts
 create mode 100644 tools/binman/test/283_mkimage_special.dts
 create mode 100644 tools/binman/test/284_fit_fdt_list.dts
 create mode 100644 tools/binman/test/285_spl_expand.dts
 create mode 100644 tools/binman/test/286_template.dts
 create mode 100644 tools/binman/test/287_template_multi.dts
 create mode 100644 tools/binman/test/288_template_fit.dts
 create mode 100644 tools/binman/test/289_template_section.dts
 create mode 100644 tools/binman/test/290_mkimage_sym.dts
 create mode 100644 tools/binman/test/bss_data_zero.c
 create mode 100644 tools/binman/test/bss_data_zero.lds
 create mode 100644 tools/dtoc/test/dtoc_test_copy.dts

-- 
2.41.0.390.g38632f3daf-goog

Re: [PATCH 0/4] introduce EFI_RAM_DISK_PROTOCOL

[AKASHI Takahiro]

On Mon, Jul 10, 2023 at 11:13:12AM +0900, Masahisa Kojima wrote:
> On Fri, 7 Jul 2023 at 18:12, AKASHI Takahiro  
> wrote:
> >
> > On Fri, Jul 07, 2023 at 05:19:33PM +0900, Masahisa Kojima wrote:
> > > Hi Akashi-san,
> > >
> > > On Fri, 7 Jul 2023 at 16:16, AKASHI Takahiro  
> > > wrote:
> > > >
> > > > On Fri, Jul 07, 2023 at 08:29:12AM +0200, Heinrich Schuchardt wrote:
> > > > > On 7/7/23 06:00, Masahisa Kojima wrote:
> > > > > > This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
> > > > > > The major purpose of this series is a preparation for EFI HTTP(S) 
> > > > > > boot.
> > > > > >
> > > > > > Now U-Boot can download the distro installer ISO image
> > > > > > via wget or tftpboot commands, but U-Boot can not mount
> > > > > > the downloaded ISO image.
> > > > > > By calling EFI_RAM_DISK_PROTOCOL->register API, user can
> > > > > > mount the ISO image and boot the distro installer.
> > > > >
> > > > > If I understand you correctly, with your design RAM disks will only
> > > > > exist in the EFI sub-system.
> > > >
> > > > Probably, not. As Kojima-san's "dm tree" shows, there is a U-Boot
> > > > block device (and associated partition devices) thanks to your
> > > > efi_driver framework.
> > >
> > > Read/Write the RAM disk is expected to be called from the EFI context, so
> >
> > An associated U-Boot block device should work as well on top of your
> > RAW_DISK_PROTOCOL via a generated RAM disk object (efi_disk).
> > That is why SYMPLE_FILE_SYSTEM_PROTOCOL, which solely relies on U-Boot's 
> > proper
> > filesystem subsystem, is installed to the RAM disk object.
> 
> I now realize that my RAM_DISK_PROTOCOL implementation happens to work
> thanks to the block cache.
> When I disable CONFIG_BLOCK_CACHE and load some EFI application(it
> does set  'app_gd')
> before calling RAM_DISK_PROTOCOL service, RAM_DISK_PROTOCOL does not work.
> ConnectController fails.
> 
> >
> > > native U-Boot can not access the RAM disk.
> >
> > I believe that, in theory, "fatls efidisk 0 1" (or efi_disk as an 
> > interface?)
> > should work even under your current implementation.
> 
> "fatls efiloader 0:2" does not work.
> I think "efiloader" device is designed to be accessed from EFI application 
> only
> even if it is listed by "dm tree".

I don't believe so.
(the interface name may be "efi_blk" or "efiblk", though.)

If the command fails, it's a bug. Must be fixed.

> I understand that ramdisk as UCLASS_BLK driver is required as Heinrich said.

That said, I agree to starting with UCLASS_BLK from the viewpoint of
U-Boot driver model/UEFI integration.

-Takahiro Akashi

> 
> Thanks,
> Masahisa Kojima
> 
> >
> > -Takahiro Akashi
> >
> > >  # Honestly speaking, I'm not sure how U-Boot prohibits the access to
> > > the EFI RAM disk
> > > because the udevices are created for the RAM disk.
> > >
> > > Thanks,
> > > Masahisa Kojima
> > >
> > > >
> > > > > We strive to synchronize what is happening on the driver model level 
> > > > > and
> > > > > on the UEFI level. I would prefer a design where we have a UCLASS_BLK
> > > > > driver ram disks and the EFI_RAM_DISK_PROTOCOL is a one method of
> > > > > managing ram disk devices.
> > > >
> > > > That said, I agree to starting with U-Boot block device implementation,
> > > > UEFI_DISK comes automatically. It benefits both U-Boot proper and
> > > > UEFI subsystem equally as well.
> > > > (That is also what I meant to say in my first response.)
> > > >
> > > > > A big issue we have is RAM management. malloc() can only assign 
> > > > > limited
> > > > > amount of memory which is probably too small for the RAM disk you are
> > > > > looking at. We manage page sized memory in the EFI sub-system but this
> > > > > is not integrated with the LMB memory checks.
> > > >
> > > > Not sure, is it enough simply to add some restrictions on the start size
> > > > and the size when a memory region is specified for a raw disk?
> > > >
> > > > -Takahiro Akashi
> > > >
> > > > > The necessary sequence of development is probably:
> > > > >
> > > > > * Rework memory management
> > > > > * Implement ramdisks as UCLASS_BLK driver
> > > > > * Implement the EFI_RAM_DISK_PROTOCOL based on the UCLASS_BLK driver.
> > > > >
> > > > > Best regards
> > > > >
> > > > > Heinrich
> > > > >
> > > > > >
> > > > > > Note that the installation process may not proceed
> > > > > > after the distro installer calls ExitBootServices()
> > > > > > since there is no hand-off process for the block device of
> > > > > > memory mapped ISO image.
> > > > > >
> > > > > > The EFI_RAM_DISK_PROTOCOL was tested with the
> > > > > > debian network installer[1] on qemu_arm64 platform.
> > > > > > The example procedure is as follows.
> > > > > >   => tftpboot 4100 mini.iso
> > > > > >   => efidebug disk load 4100 $filesize
> > > > > >
> > > > > > After these commands, ISO image is mounted like:
> > > > > >
> > > > > >   => efidebug dh
> > > > > >
> > > > > >  7eec5910 (efiblk#0)
> > > > > >
> > 

Re: [PATCH 0/4] introduce EFI_RAM_DISK_PROTOCOL

[Masahisa Kojima]

On Fri, 7 Jul 2023 at 18:12, AKASHI Takahiro  wrote:
>
> On Fri, Jul 07, 2023 at 05:19:33PM +0900, Masahisa Kojima wrote:
> > Hi Akashi-san,
> >
> > On Fri, 7 Jul 2023 at 16:16, AKASHI Takahiro  
> > wrote:
> > >
> > > On Fri, Jul 07, 2023 at 08:29:12AM +0200, Heinrich Schuchardt wrote:
> > > > On 7/7/23 06:00, Masahisa Kojima wrote:
> > > > > This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
> > > > > The major purpose of this series is a preparation for EFI HTTP(S) 
> > > > > boot.
> > > > >
> > > > > Now U-Boot can download the distro installer ISO image
> > > > > via wget or tftpboot commands, but U-Boot can not mount
> > > > > the downloaded ISO image.
> > > > > By calling EFI_RAM_DISK_PROTOCOL->register API, user can
> > > > > mount the ISO image and boot the distro installer.
> > > >
> > > > If I understand you correctly, with your design RAM disks will only
> > > > exist in the EFI sub-system.
> > >
> > > Probably, not. As Kojima-san's "dm tree" shows, there is a U-Boot
> > > block device (and associated partition devices) thanks to your
> > > efi_driver framework.
> >
> > Read/Write the RAM disk is expected to be called from the EFI context, so
>
> An associated U-Boot block device should work as well on top of your
> RAW_DISK_PROTOCOL via a generated RAM disk object (efi_disk).
> That is why SYMPLE_FILE_SYSTEM_PROTOCOL, which solely relies on U-Boot's 
> proper
> filesystem subsystem, is installed to the RAM disk object.

I now realize that my RAM_DISK_PROTOCOL implementation happens to work
thanks to the block cache.
When I disable CONFIG_BLOCK_CACHE and load some EFI application(it
does set  'app_gd')
before calling RAM_DISK_PROTOCOL service, RAM_DISK_PROTOCOL does not work.
ConnectController fails.

>
> > native U-Boot can not access the RAM disk.
>
> I believe that, in theory, "fatls efidisk 0 1" (or efi_disk as an interface?)
> should work even under your current implementation.

"fatls efiloader 0:2" does not work.
I think "efiloader" device is designed to be accessed from EFI application only
even if it is listed by "dm tree".

I understand that ramdisk as UCLASS_BLK driver is required as Heinrich said.

Thanks,
Masahisa Kojima

>
> -Takahiro Akashi
>
> >  # Honestly speaking, I'm not sure how U-Boot prohibits the access to
> > the EFI RAM disk
> > because the udevices are created for the RAM disk.
> >
> > Thanks,
> > Masahisa Kojima
> >
> > >
> > > > We strive to synchronize what is happening on the driver model level and
> > > > on the UEFI level. I would prefer a design where we have a UCLASS_BLK
> > > > driver ram disks and the EFI_RAM_DISK_PROTOCOL is a one method of
> > > > managing ram disk devices.
> > >
> > > That said, I agree to starting with U-Boot block device implementation,
> > > UEFI_DISK comes automatically. It benefits both U-Boot proper and
> > > UEFI subsystem equally as well.
> > > (That is also what I meant to say in my first response.)
> > >
> > > > A big issue we have is RAM management. malloc() can only assign limited
> > > > amount of memory which is probably too small for the RAM disk you are
> > > > looking at. We manage page sized memory in the EFI sub-system but this
> > > > is not integrated with the LMB memory checks.
> > >
> > > Not sure, is it enough simply to add some restrictions on the start size
> > > and the size when a memory region is specified for a raw disk?
> > >
> > > -Takahiro Akashi
> > >
> > > > The necessary sequence of development is probably:
> > > >
> > > > * Rework memory management
> > > > * Implement ramdisks as UCLASS_BLK driver
> > > > * Implement the EFI_RAM_DISK_PROTOCOL based on the UCLASS_BLK driver.
> > > >
> > > > Best regards
> > > >
> > > > Heinrich
> > > >
> > > > >
> > > > > Note that the installation process may not proceed
> > > > > after the distro installer calls ExitBootServices()
> > > > > since there is no hand-off process for the block device of
> > > > > memory mapped ISO image.
> > > > >
> > > > > The EFI_RAM_DISK_PROTOCOL was tested with the
> > > > > debian network installer[1] on qemu_arm64 platform.
> > > > > The example procedure is as follows.
> > > > >   => tftpboot 4100 mini.iso
> > > > >   => efidebug disk load 4100 $filesize
> > > > >
> > > > > After these commands, ISO image is mounted like:
> > > > >
> > > > >   => efidebug dh
> > > > >
> > > > >  7eec5910 (efiblk#0)
> > > > >
> > > > > /RamDisk(0x4100,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)
> > > > >Block IO
> > > > >
> > > > >  7eec6140 (efiblk#0:1)
> > > > >
> > > > > /RamDisk(0x4100,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(1,0x01,0,0x0,0x33800)
> > > > >Block IO
> > > > >
> > > > >  7eec62b0 (efiblk#0:2)
> > > > >
> > > > > /RamDisk(0x4100,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(2,0x01,0,0x33800,0x1)
> > > > >Block IO
> > > > >System Partition
> > > > >Simple File 

Re: [PATCH 07/10] test: dm: add SCMI base protocol test

[AKASHI Takahiro]

On Fri, Jul 07, 2023 at 11:35:49AM -0600, Simon Glass wrote:
> Hi,
> 
> On Tue, 4 Jul 2023 at 03:35, AKASHI Takahiro  
> wrote:
> >
> > Hi Simon,
> >
> > On Mon, Jul 03, 2023 at 02:30:57PM +0100, Simon Glass wrote:
> > > Hi,
> > >
> > > On Mon, 3 Jul 2023 at 01:57, AKASHI Takahiro  
> > > wrote:
> > > >
> > > > On Thu, Jun 29, 2023 at 08:09:58PM +0100, Simon Glass wrote:
> > > > > Hi AKASHI,
> > > > >
> > > > > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
> > > > >  wrote:
> > > > > >
> > > > > > Added is a new unit test for SCMI base protocol, which will 
> > > > > > exercise all
> > > > > > the commands provided by the protocol, except 
> > > > > > SCMI_BASE_NOTIFY_ERRORS.
> > > > > >   $ ut dm scmi_base
> > > > > > It is assumed that test.dtb is used as sandbox's device tree.
> > > > > >
> > > > > > Signed-off-by: AKASHI Takahiro 
> > > > > > ---
> > > > > >  test/dm/scmi.c | 112 
> > > > > > +
> > > > > >  1 file changed, 112 insertions(+)
> > > > > >
> > > > > > diff --git a/test/dm/scmi.c b/test/dm/scmi.c
> > > > > > index 881be3171b7c..563017bb63e0 100644
> > > > > > --- a/test/dm/scmi.c
> > > > > > +++ b/test/dm/scmi.c
> > > > > > @@ -16,6 +16,9 @@
> > > > > >  #include 
> > > > > >  #include 
> > > > > >  #include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > >  #include 
> > > > > >  #include 
> > > > > >  #include 
> > > > > > @@ -95,6 +98,115 @@ static int dm_test_scmi_sandbox_agent(struct 
> > > > > > unit_test_state *uts)
> > > > > >  }
> > > > > >  DM_TEST(dm_test_scmi_sandbox_agent, UT_TESTF_SCAN_FDT);
> > > > > >
> > > > > > +static int dm_test_scmi_base(struct unit_test_state *uts)
> > > > > > +{
> > > > > > +   struct udevice *agent_dev, *base;
> > > > > > +   struct scmi_agent_priv *priv;
> > > > > > +   const struct scmi_base_ops *ops;
> > > > > > +   u32 version, num_agents, num_protocols, impl_version;
> > > > > > +   u32 attributes, agent_id;
> > > > > > +   char vendor[SCMI_BASE_NAME_LENGTH_MAX],
> > > > > > +agent_name[SCMI_BASE_NAME_LENGTH_MAX];
> > > > > > +   u8 *protocols;
> > > > > > +   int ret;
> > > > > > +
> > > > > > +   /* preparation */
> > > > > > +   ut_assertok(uclass_get_device_by_name(UCLASS_SCMI_AGENT, 
> > > > > > "scmi",
> > > > > > + _dev));
> > > > > > +   ut_assertnonnull(agent_dev);
> > > > > > +   ut_assertnonnull(priv = dev_get_uclass_plat(agent_dev));
> > > > > > +   ut_assertnonnull(base = scmi_get_protocol(agent_dev,
> > > > > > + 
> > > > > > SCMI_PROTOCOL_ID_BASE));
> > > > > > +   ut_assertnonnull(ops = dev_get_driver_ops(base));
> > > > > > +
> > > > > > +   /* version */
> > > > > > +   ret = (*ops->protocol_version)(base, );
> > > > >
> > > > > Can you add uclass helpers to call each of the methods? That is how it
> > > > > is commonly done. You should not be calling ops->xxx directly here.
> > > >
> > > > Yes, I will add inline functions instead.
> > >
> > > I don't mean inline...see all the other uclasses which define a
> >
> > Okay, I will *real* functions.
> >
> > > function which is implemented in the uclass. It is confusing when one
> > > uclass does something different. People might copy this style and then
> > > the code base diverges. Did you not notice this when looking around
> > > the source tree?
> >
> > But one concern came up in my mind.
> > Contrary to ordinary "device controllers", there exists only a single
> > implementation of driver for each of "udevice"'s associated with SCMI
> > protocols including the base protocol.
> >
> > So if I follow your suggestion, the code (base.c) might look like:
> > ===
> > static int __scmi_base_discover_vendor(struct udevice *dev, u8 *vendor)
> > {
> >   ...
> > }
> >
> > struct scmi_base_ops scmi_base_ops = {
> >
> >   .base_discover_vendor = __scmi_base_discover_vendor,
> >
> > }
> >
> > int scmi_base_discover_vendor(struct udevice *dev, u8 *vendor)
> > {
> >   struct scmi_base_ops *ops;
> >
> >   ops = scmi_base_dev_ops(dev);
> >
> >   return ops->base_discover_vendor(dev, vendor);
> > }
> > ===
> >
> > We will have to have similar definitions for every operation in ops.
> > It looks quite weird to me as there are always pairs of functions,
> > like __scmi_base_discover_vendor() and scmi_base_discover_vendor().
> 
> Yes I understand that you only have one driver at present. Is there
> not a sandbox driver?

No.
Please remember that SCMI protocol drivers on U-Boot are nothing but
stubs that makes a call to SCMI servers, supporting common communication
channel interfaces for different transports (either OP-TEE, SMCCC or mailbox).

Sandbox driver, if is properly named, is also implemented as a sort of
transport layer, where a invocation is replaced with a function call which
mimicks one of specific commands in SCMI protocol on behalf of a real 

[GIT PULL] Please pull fsl-qoirq-2023-7-6 for next

[Peng Fan]

Hi Tom,

Please pull fsl-qoriq-2023-7-6 for next

-
Enable DM Serial for ls1043ardb and ls1046ardb/afrwy
Fixed secure boot on LS-CH2 platforms
-
https://source.denx.de/u-boot/custodians/u-boot-fsl-qoriq/-/pipelines/16795

Thanks,
Peng.

The following changes since commit e80f4079b3a3db0961b73fa7a96e6c90242d8d25:

  Merge tag 'v2023.07-rc6' into next (2023-07-05 11:28:55 -0400)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-fsl-qoriq.git 
tags/fsl-qoriq-2023-7-6

for you to fetch changes up to fcf75435c8fd443547f5d4192d1cf70fb8a84034:

  LFU-544: Kconfig.nxp: Fixed secure boot on LS-CH2 platforms (2023-07-06 
13:04:56 +0800)


Camelia Groza (7):
  arch: arm: dts: ls1043a: sync serial nodes with Linux
  arch: arm: dts: ls1043a: tag serial nodes with bootph-all
  configs: ls1043ardb: enable DM_SERIAL
  arch: arm: dts: ls1046a: sync serial nodes with Linux
  arch: arm: dts: ls1046a: tag serial nodes with bootph-all
  configs: ls1046ardb: enable DM_SERIAL
  configs: ls1046afrwy: enable DM_SERIAL

Kshitiz Varshney (1):
  LFU-544: Kconfig.nxp: Fixed secure boot on LS-CH2 platforms

 arch/Kconfig.nxp|  2 +-
 arch/arm/dts/fsl-ls1043a-qds.dtsi   |  2 +-
 arch/arm/dts/fsl-ls1043a-rdb-u-boot.dtsi|  5 +
 arch/arm/dts/fsl-ls1043a-rdb.dts|  6 +-
 arch/arm/dts/fsl-ls1043a-u-boot.dtsi| 19 +++
 arch/arm/dts/fsl-ls1043a.dtsi   | 16 +++-
 arch/arm/dts/fsl-ls1046a-frwy-u-boot.dtsi   |  5 +
 arch/arm/dts/fsl-ls1046a-frwy.dts   | 22 +-
 arch/arm/dts/fsl-ls1046a-qds.dtsi   |  2 +-
 arch/arm/dts/fsl-ls1046a-rdb-u-boot.dtsi|  5 +
 arch/arm/dts/fsl-ls1046a-rdb.dts| 14 +-
 arch/arm/dts/fsl-ls1046a-u-boot.dtsi| 19 +++
 arch/arm/dts/fsl-ls1046a.dtsi   | 28 
+++-
 configs/ls1043ardb_SECURE_BOOT_defconfig|  4 +++-
 configs/ls1043ardb_defconfig|  4 +++-
 configs/ls1043ardb_nand_SECURE_BOOT_defconfig   |  4 +++-
 configs/ls1043ardb_nand_defconfig   |  3 ++-
 configs/ls1043ardb_sdcard_SECURE_BOOT_defconfig |  4 +++-
 configs/ls1043ardb_sdcard_defconfig |  3 ++-
 configs/ls1043ardb_tfa_SECURE_BOOT_defconfig|  4 +++-
 configs/ls1043ardb_tfa_defconfig|  4 +++-
 configs/ls1046afrwy_tfa_SECURE_BOOT_defconfig   |  4 +++-
 configs/ls1046afrwy_tfa_defconfig   |  4 +++-
 configs/ls1046ardb_emmc_defconfig   |  3 ++-
 configs/ls1046ardb_qspi_SECURE_BOOT_defconfig   |  4 +++-
 configs/ls1046ardb_qspi_defconfig   |  4 +++-
 configs/ls1046ardb_qspi_spl_defconfig   |  3 ++-
 configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig |  4 +++-
 configs/ls1046ardb_sdcard_defconfig |  3 ++-
 configs/ls1046ardb_tfa_SECURE_BOOT_defconfig|  4 +++-
 configs/ls1046ardb_tfa_defconfig|  4 +++-
 31 files changed, 174 insertions(+), 38 deletions(-)
 create mode 100644 arch/arm/dts/fsl-ls1043a-rdb-u-boot.dtsi
 create mode 100644 arch/arm/dts/fsl-ls1043a-u-boot.dtsi
 create mode 100644 arch/arm/dts/fsl-ls1046a-frwy-u-boot.dtsi
 create mode 100644 arch/arm/dts/fsl-ls1046a-rdb-u-boot.dtsi
 create mode 100644 arch/arm/dts/fsl-ls1046a-u-boot.dtsi

Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware

[AKASHI Takahiro]

On Fri, Jul 07, 2023 at 11:35:52AM -0600, Simon Glass wrote:
> Hi,
> 
> On Tue, 4 Jul 2023 at 02:26, AKASHI Takahiro  
> wrote:
> >
> > On Mon, Jul 03, 2023 at 02:30:54PM +0100, Simon Glass wrote:
> > > Hi,
> > >
> > > On Mon, 3 Jul 2023 at 01:55, AKASHI Takahiro  
> > > wrote:
> > > >
> > > > On Thu, Jun 29, 2023 at 08:10:00PM +0100, Simon Glass wrote:
> > > > > Hi AKASHI,
> > > > >
> > > > > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
> > > > >  wrote:
> > > > > >
> > > > > > This command, "scmi", provides a command line interface to various 
> > > > > > SCMI
> > > > > > protocols. It supports at least initially SCMI base protocol with 
> > > > > > the sub
> > > > > > command, "base", and is intended mainly for debug purpose.
> > > > > >
> > > > > > Signed-off-by: AKASHI Takahiro 
> > > > > > ---
> > > > > >  cmd/Kconfig  |   8 ++
> > > > > >  cmd/Makefile |   1 +
> > > > > >  cmd/scmi.c   | 373 
> > > > > > +++
> > > > > >  3 files changed, 382 insertions(+)
> > > > > >  create mode 100644 cmd/scmi.c
> > > > > >
> > > > > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > > > > index 02e54f1e50fe..065470a76295 100644
> > > > > > --- a/cmd/Kconfig
> > > > > > +++ b/cmd/Kconfig
> > > > > > @@ -2504,6 +2504,14 @@ config CMD_CROS_EC
> > > > > >   a number of sub-commands for performing EC tasks such as
> > > > > >   updating its flash, accessing a small saved context area
> > > > > >   and talking to the I2C bus behind the EC (if there is 
> > > > > > one).
> > > > > > +
> > > > > > +config CMD_SCMI
> > > > > > +   bool "Enable scmi command"
> > > > > > +   depends on SCMI_FIRMWARE
> > > > > > +   default n
> > > > > > +   help
> > > > > > + This command provides user interfaces to several SCMI 
> > > > > > protocols,
> > > > > > + including Base protocol.
> > > > >
> > > > > Please mention what is SCMI
> > > >
> > > > I will give a full spelling.
> > > >
> > > > > >  endmenu
> > > > > >
> > > > > >  menu "Filesystem commands"
> > > > > > diff --git a/cmd/Makefile b/cmd/Makefile
> > > > > > index 6c37521b4e2b..826e0b74b587 100644
> > > > > > --- a/cmd/Makefile
> > > > > > +++ b/cmd/Makefile
> > > > > > @@ -156,6 +156,7 @@ obj-$(CONFIG_CMD_SATA) += sata.o
> > > > > >  obj-$(CONFIG_CMD_NVME) += nvme.o
> > > > > >  obj-$(CONFIG_SANDBOX) += sb.o
> > > > > >  obj-$(CONFIG_CMD_SF) += sf.o
> > > > > > +obj-$(CONFIG_CMD_SCMI) += scmi.o
> > > > > >  obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o
> > > > > >  obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o
> > > > > >  obj-$(CONFIG_CMD_SEAMA) += seama.o
> > > > > > diff --git a/cmd/scmi.c b/cmd/scmi.c
> > > > > > new file mode 100644
> > > > > > index ..c97f77e97d95
> > > > > > --- /dev/null
> > > > > > +++ b/cmd/scmi.c
> > > > > > @@ -0,0 +1,373 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > +/*
> > > > > > + *  SCMI utility command
> > > > > > + *
> > > > > > + *  Copyright (c) 2023 Linaro Limited
> > > > > > + * Author: AKASHI Takahiro
> > > > > > + */
> > > > > > +
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include  /* uclass_get_device */
> > > > >
> > > > > Goes before linux/bitfield.h
> > > >
> > > > Can you tell me why?
> > >
> > > It is just a convention:
> > > https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files
> >
> > Okay.
> >
> > > >
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +
> > > > > > +static struct udevice *agent;
> > > > > > +static struct udevice *base_proto;
> > > > > > +static const struct scmi_base_ops *ops;
> > > > > > +
> > > > > > +struct {
> > > > > > +   enum scmi_std_protocol id;
> > > > > > +   const char *name;
> > > > > > +} protocol_name[] = {
> > > > > > +   {SCMI_PROTOCOL_ID_BASE, "Base"},
> > > > > > +   {SCMI_PROTOCOL_ID_POWER_DOMAIN, "Power domain management"},
> > > > > > +   {SCMI_PROTOCOL_ID_SYSTEM, "System power management"},
> > > > > > +   {SCMI_PROTOCOL_ID_PERF, "Performance domain management"},
> > > > > > +   {SCMI_PROTOCOL_ID_CLOCK, "Clock management"},
> > > > > > +   {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"},
> > > > > > +   {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"},
> > > > > > +   {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain 
> > > > > > management"},
> > > > > > +};
> > > > > > +
> > > > > > +/**
> > > > > > + * scmi_convert_id_to_string() - get the name of SCMI protocol
> > > > > > + *
> > > > > > + * @id:Protocol ID
> > > > > > + *
> > > > > > + * Get the printable name of the protocol, @id
> > > > > > + *
> > > > > > + * Return: Name string on success, NULL on failure
> > > > > > + */
> > > > > > +static const char *scmi_convert_id_to_string(enum 
> > > > > > scmi_std_protocol id)
> > > > >
> > > > > scmi_lookup_id?
> > > >
> > 

[BUG] fdt_pack_reg in common/fdt_support.c can cause crash from unaligned access

[David Virag]

Hi,

I'm trying to port U-Boot to a new board (Samsung JACKPOTLTE, ARMv8,
Exynos7885) but when CONFIG_ARCH_FIXUP_FDT_MEMORY is enabled, the bootm
command leads to an unaligned memory access, which results in a
synchronous abort.

After a long debugging session, I concluded that fdt_pack_reg in
common/fdt_support.c writes to unaligned addresses in its for loop.
In the case of address_cells being 2, and size_cells being 1, the
buffer pointer gets incremented by 12 in each loop, making the second
iteration (i=1) write a 64bit value to a non 64bit aligned address.

Turning the alignment check enable bit (A) off in SCTLR makes the
function work as intended. I couldn't find code that touches this bit,
but I may have missed something. I don't think writing in two parts
should be the fix, but something should be done about this. As far as I
understand, any arm64 board that has this bit turned on, either from
previous code or just the initial status of the bit after power on,
could crash here.

This is on top of the latest commit as of now
(0beb649053b86b2cfd5cf55a0fc68bc2fe91a430)

What should be done here?

Best regards,
David

[PATCH] common: Kconfig: Fix CMD_BMP/BMP dependency

[Samuel Dionne-Riel]

Using `default y` will not select BMP when CMD_BMP has been enabled, if
it was already configured.

By using `select`, if `CMD_BMP` is turned on, it will force the presence
of `BMP`.

Fixes: 072b0e16c482114d242580dd7a3197db5966705f
Signed-off-by: Samuel Dionne-Riel 
---
 cmd/Kconfig| 1 +
 common/Kconfig | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 02e54f1e50..94c54b2359 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1988,6 +1988,7 @@ config CMD_2048
 config CMD_BMP
bool "Enable 'bmp' command"
depends on VIDEO
+   select BMP
help
  This provides a way to obtain information about a BMP-format image
  and to display it. BMP (which presumably stands for BitMaP) is a
diff --git a/common/Kconfig b/common/Kconfig
index bbabadb35e..d0117e3dc8 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -1157,7 +1157,7 @@ config IO_TRACE
 
 config BMP
bool "Enable bmp image display"
-   default y if CMD_BMP
+   default n
help
  Enable bmp functions to display bmp image and get bmp info.
 
-- 
2.41.0

[PATCH v2 6/6] arm: mvebu: Remove unused alias from RC AC5X dts

[Chris Packham]

The sar-reg0 alias was left over from an earlier iteration of the
patches adding support for this board. Remove the unused alias.

Fixes: 6cc8b5db40 ("arm: mvebu: Add RD-AC5X board")
Signed-off-by: Chris Packham 
---
 arch/arm/dts/ac5-98dx35xx-rd.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/dts/ac5-98dx35xx-rd.dts b/arch/arm/dts/ac5-98dx35xx-rd.dts
index d9f217cd4a5f..1dc85bb7ef24 100644
--- a/arch/arm/dts/ac5-98dx35xx-rd.dts
+++ b/arch/arm/dts/ac5-98dx35xx-rd.dts
@@ -31,7 +31,6 @@
usb0 = 
usb1 = 
pinctrl0 = 
-   sar-reg0 = "/config-space/sar-reg";
};
 
usb1phy: usb-phy {
-- 
2.41.0

[PATCH v2 5/6] arm: mvebu: Add Allied Telesis x240 board

[Chris Packham]

The x240 and SE240 are a series of L2+ switches from Allied Telesis.
There are a number of them in the range but as far as U-Boot is
concerned all the CPU block components are the same so there's only one
board defined.

Signed-off-by: Chris Packham 
---

Notes:
Changes in v2:
- drop CONFIG_DEBUG_UART

 arch/arm/dts/Makefile  |   3 +-
 arch/arm/dts/ac5-98dx35xx-atl-x240.dts | 212 +
 arch/arm/mach-mvebu/Kconfig|   7 +
 board/alliedtelesis/x240/MAINTAINERS   |   7 +
 board/alliedtelesis/x240/Makefile  |   6 +
 board/alliedtelesis/x240/x240.c|  13 ++
 configs/x240_defconfig |  86 ++
 include/configs/x240.h |  37 +
 8 files changed, 370 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/dts/ac5-98dx35xx-atl-x240.dts
 create mode 100644 board/alliedtelesis/x240/MAINTAINERS
 create mode 100644 board/alliedtelesis/x240/Makefile
 create mode 100644 board/alliedtelesis/x240/x240.c
 create mode 100644 configs/x240_defconfig
 create mode 100644 include/configs/x240.h

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 480269fa6065..38d878a0f853 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -303,7 +303,8 @@ dtb-$(CONFIG_ARCH_MVEBU) += \
cn9132-db-B.dtb \
cn9130-crb-A.dtb\
cn9130-crb-B.dtb\
-   ac5-98dx35xx-rd.dtb
+   ac5-98dx35xx-rd.dtb \
+   ac5-98dx35xx-atl-x240.dtb
 endif
 
 dtb-$(CONFIG_ARCH_SYNQUACER) += synquacer-sc2a11-developerbox.dtb
diff --git a/arch/arm/dts/ac5-98dx35xx-atl-x240.dts 
b/arch/arm/dts/ac5-98dx35xx-atl-x240.dts
new file mode 100644
index ..820ec18b4355
--- /dev/null
+++ b/arch/arm/dts/ac5-98dx35xx-atl-x240.dts
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+/dts-v1/;
+
+#include 
+#include 
+#include "ac5-98dx35xx.dtsi"
+
+/ {
+   model = "Allied Telesis x240";
+   compatible = "alliedtelesis,x240", "marvell,ac5x", "marvell,ac5";
+
+   aliases {
+   serial0 = 
+   spiflash0 = 
+   gpio0 = 
+   gpio1 = 
+   spi0 = 
+   i2c0 = 
+   usb0 = 
+   pinctrl0 = 
+   };
+
+   chosen {
+   stdout-path = "serial0:115200n8";
+   };
+
+   boot-board {
+   compatible = "atl,boot-board";
+   present-gpio = < 6 GPIO_ACTIVE_HIGH>;
+   override-gpio = < 2 GPIO_ACTIVE_HIGH>;
+   };
+
+   gpio-leds {
+   compatible = "gpio-leds";
+
+   fault {
+   label = "fault:red";
+   gpios = <_gpio 11 GPIO_ACTIVE_LOW>;
+   default-state = "on";
+   };
+   };
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+
+   nand-ecc-strength = <4>;
+   nand-ecc-step-size = <512>;
+   status = "okay";
+
+   partitions {
+   compatible = "fixed-partitions";
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   partition@user {
+   reg = <0x 0x1000>;
+   label = "user";
+   };
+   };
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   status = "okay";
+
+   mux@71 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   compatible = "nxp,pca9546";
+   reg = <0x71>;
+   i2c-mux-idle-disconnect;
+   reset-gpios = < 4 GPIO_ACTIVE_LOW>;   /* 
MPP36 */
+   status = "okay";
+
+   i2c@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <1>;
+
+   hwmon@2e {
+   compatible = "adi,adt7476";
+   reg = <0x2e>;
+   };
+
+   rtc@68 {
+   compatible = "adi,max31331";
+   reg = <0x68>;
+   };
+
+   system_gpio: gpio@27 {
+   compatible = "nxp,pca9555";
+   gpio-controller;
+   #gpio-cells= <2>;
+   reg = <0x27>;
+   interrupt-parent = <>;
+   interrupts = <25 IRQ_TYPE_LEVEL_LOW>;   /* 
MPP25 */
+   };
+   };
+   };
+};
+
+ {
+   status = "okay";
+   spiflash0: flash@0 {
+   compatible = "jedec,spi-nor";
+   spi-max-frequency = <5000>;
+   spi-tx-bus-width = <1>; /* 1-single, 2-dual, 4-quad */
+   spi-rx-bus-width = <1>; 

[PATCH v2 4/6] mtd: nand: pxa3xx: Enable devbus/nand arbiter on Armada 8K

[Chris Packham]

The CN9130 SoC (an ARMADA 8K type) has both a NAND Flash Controller and
a generic local bus controller (Device Bus Controller) that share common
pins.

With a board design that incorporates both a NAND flash and uses
the Device Bus (in our case for an SRAM) accessing the Device Bus device
fails unless the NfArbiterEn bit is set. Setting the bit enables
arbitration between the Device Bus and the NAND flash.

Since there is no obvious downside in enabling this for designs that
don't require arbitration, we always enable it.

Signed-off-by: Chris Packham 
---
 drivers/mtd/nand/raw/pxa3xx_nand.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/pxa3xx_nand.c 
b/drivers/mtd/nand/raw/pxa3xx_nand.c
index 9dee580ab9c2..d502e967f92c 100644
--- a/drivers/mtd/nand/raw/pxa3xx_nand.c
+++ b/drivers/mtd/nand/raw/pxa3xx_nand.c
@@ -125,6 +125,7 @@ DECLARE_GLOBAL_DATA_PTR;
 /* System control register and bit to enable NAND on some SoCs */
 #define GENCONF_SOC_DEVICE_MUX 0x208
 #define GENCONF_SOC_DEVICE_MUX_NFC_EN BIT(0)
+#define GENCONF_SOC_DEVICE_MUX_NFC_DEVBUS_ARB_EN BIT(27)
 
 /*
  * This should be large enough to read 'ONFI' and 'JEDEC'.
@@ -1739,7 +1740,7 @@ static int alloc_nand_resource(struct udevice *dev, 
struct pxa3xx_nand_info *inf
return PTR_ERR(sysctrl_base);
 
regmap_read(sysctrl_base, GENCONF_SOC_DEVICE_MUX, );
-   reg |= GENCONF_SOC_DEVICE_MUX_NFC_EN;
+   reg |= GENCONF_SOC_DEVICE_MUX_NFC_EN | 
GENCONF_SOC_DEVICE_MUX_NFC_DEVBUS_ARB_EN;
regmap_write(sysctrl_base, GENCONF_SOC_DEVICE_MUX, reg);
}
 
-- 
2.41.0

[PATCH v2 3/6] mtd: nand: pxa3xx: Add support for the Marvell AC5 SoC

[Chris Packham]

The NAND flash controller (NFC) on the AC5/AC5X SoC is the same as
the NFC used on other Marvell SoCs. It does have the additional
restriction of only supporting SDR timing modes up to 3.

Signed-off-by: Chris Packham 
---
 drivers/mtd/nand/raw/pxa3xx_nand.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/pxa3xx_nand.c 
b/drivers/mtd/nand/raw/pxa3xx_nand.c
index fcd1b9c63614..9dee580ab9c2 100644
--- a/drivers/mtd/nand/raw/pxa3xx_nand.c
+++ b/drivers/mtd/nand/raw/pxa3xx_nand.c
@@ -167,6 +167,7 @@ enum pxa3xx_nand_variant {
PXA3XX_NAND_VARIANT_PXA,
PXA3XX_NAND_VARIANT_ARMADA370,
PXA3XX_NAND_VARIANT_ARMADA_8K,
+   PXA3XX_NAND_VARIANT_AC5,
 };
 
 struct pxa3xx_nand_host {
@@ -391,6 +392,10 @@ static const struct udevice_id pxa3xx_nand_dt_ids[] = {
.compatible = "marvell,armada-8k-nand-controller",
.data = PXA3XX_NAND_VARIANT_ARMADA_8K,
},
+   {
+   .compatible = "marvell,mvebu-ac5-pxa3xx-nand",
+   .data = PXA3XX_NAND_VARIANT_AC5,
+   },
{}
 };
 
@@ -505,6 +510,9 @@ static int pxa3xx_nand_init_timings(struct pxa3xx_nand_host 
*host)
if (mode < 0)
mode = 0;
 
+   if (info->variant == PXA3XX_NAND_VARIANT_AC5)
+   mode = min(mode, 3);
+
timings = onfi_async_timing_mode_to_sdr_timings(mode);
if (IS_ERR(timings))
return PTR_ERR(timings);
@@ -730,7 +738,8 @@ static irqreturn_t pxa3xx_nand_irq(struct pxa3xx_nand_info 
*info)
 
/* NDCB3 register is available in NFCv2 (Armada 370/XP SoC) */
if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370 ||
-   info->variant == PXA3XX_NAND_VARIANT_ARMADA_8K)
+   info->variant == PXA3XX_NAND_VARIANT_ARMADA_8K ||
+   info->variant == PXA3XX_NAND_VARIANT_AC5)
nand_writel(info, NDCB0, info->ndcb3);
}
 
@@ -1579,7 +1588,8 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 
/* Device detection must be done with ECC disabled */
if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370 ||
-   info->variant == PXA3XX_NAND_VARIANT_ARMADA_8K)
+   info->variant == PXA3XX_NAND_VARIANT_ARMADA_8K ||
+   info->variant == PXA3XX_NAND_VARIANT_AC5)
nand_writel(info, NDECCCTRL, 0x0);
 
if (nand_scan_ident(mtd, 1, NULL))
@@ -1630,7 +1640,8 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 */
if (mtd->writesize > info->chunk_size) {
if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370 ||
-   info->variant == PXA3XX_NAND_VARIANT_ARMADA_8K) {
+   info->variant == PXA3XX_NAND_VARIANT_ARMADA_8K ||
+   info->variant == PXA3XX_NAND_VARIANT_AC5) {
chip->cmdfunc = nand_cmdfunc_extended;
} else {
dev_err(mtd->dev,
-- 
2.41.0

[PATCH v2 2/6] arm: mvebu: ac5: Define mvebu_get_nand_clock()

[Chris Packham]

The NF_CLK for the AC5 SoC runs at 400MHz. There's no strapping
or gating require so just add a mvebu_get_nand_clock() that
returns this value.

Signed-off-by: Chris Packham 
---
 arch/arm/mach-mvebu/alleycat5/soc.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/mach-mvebu/alleycat5/soc.c 
b/arch/arm/mach-mvebu/alleycat5/soc.c
index dc69f46eedb2..734b0a87dd49 100644
--- a/arch/arm/mach-mvebu/alleycat5/soc.c
+++ b/arch/arm/mach-mvebu/alleycat5/soc.c
@@ -255,6 +255,12 @@ void soc_print_clock_info(void)
printf("\tMSS %4d MHz\n", 200);
 }
 
+/* Return NAND clock in Hz */
+u32 mvebu_get_nand_clock(void)
+{
+   return 400 * 100;
+}
+
 /*
  * Override of __weak int mach_cpu_init(void) :
  * SoC/machine dependent CPU setup
-- 
2.41.0

[PATCH v2 1/6] arm: mvebu: ac5: Add nand-controller node

[Chris Packham]

The AC5/AC5X SoC has a NAND flash controller. Add this to the
SoC device tree.

Signed-off-by: Chris Packham 
---
 arch/arm/dts/ac5-98dx25xx.dtsi | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/dts/ac5-98dx25xx.dtsi b/arch/arm/dts/ac5-98dx25xx.dtsi
index 3c68355f323a..f53b4781d7fd 100644
--- a/arch/arm/dts/ac5-98dx25xx.dtsi
+++ b/arch/arm/dts/ac5-98dx25xx.dtsi
@@ -251,6 +251,15 @@
status = "disabled";
};
 
+   nand: nand-controller@805b {
+   compatible = "marvell,mvebu-ac5-pxa3xx-nand";
+   reg = <0x0 0x805b 0x0 0x54>;
+   #address-cells = <0x0001>;
+   marvell,nand-enable-arbiter;
+   num-cs = <0x0001>;
+   status = "disabled";
+   };
+
gic: interrupt-controller@8060 {
compatible = "arm,gic-v3";
#interrupt-cells = <3>;
-- 
2.41.0

[PATCH v2 0/6] Support for AC5X NAND and AT-x240 board

[Chris Packham]

This series adds support for the NAND flash controller on the AC5X SoC
and adds support for the Allied Telesis x240 board.

I've also included 2 unrelated changes. "arm: mvebu: Remove unused alias
from RC AC5X dts" removes an unused alias from the dts. This was in the
neighborhood of the x240 so I included it.

"mtd: nand: pxa3xx: Enable devbus/nand arbiter on Armada 8K" allows
using the NFC and device bus at the same time. This is needed for
another board (CN9130 based) that I'll hopefully get upstream in the
near future. I figured I'd include it now since I was touching the NAND
driver.

Chris Packham (6):
  arm: mvebu: ac5: Add nand-controller node
  arm: mvebu: ac5: Define mvebu_get_nand_clock()
  mtd: nand: pxa3xx: Add support for the Marvell AC5 SoC
  mtd: nand: pxa3xx: Enable devbus/nand arbiter on Armada 8K
  arm: mvebu: Add Allied Telesis x240 board
  arm: mvebu: Remove unused alias from RC AC5X dts

 arch/arm/dts/Makefile  |   3 +-
 arch/arm/dts/ac5-98dx25xx.dtsi |   9 ++
 arch/arm/dts/ac5-98dx35xx-atl-x240.dts | 212 +
 arch/arm/dts/ac5-98dx35xx-rd.dts   |   1 -
 arch/arm/mach-mvebu/Kconfig|   7 +
 arch/arm/mach-mvebu/alleycat5/soc.c|   6 +
 board/alliedtelesis/x240/MAINTAINERS   |   7 +
 board/alliedtelesis/x240/Makefile  |   6 +
 board/alliedtelesis/x240/x240.c|  13 ++
 configs/x240_defconfig |  86 ++
 drivers/mtd/nand/raw/pxa3xx_nand.c |  20 ++-
 include/configs/x240.h |  37 +
 12 files changed, 401 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm/dts/ac5-98dx35xx-atl-x240.dts
 create mode 100644 board/alliedtelesis/x240/MAINTAINERS
 create mode 100644 board/alliedtelesis/x240/Makefile
 create mode 100644 board/alliedtelesis/x240/x240.c
 create mode 100644 configs/x240_defconfig
 create mode 100644 include/configs/x240.h

-- 
2.41.0

Re: [bug report] sunxi: H6: no ethernet on Orange Pi One Plus

[Anne Macedo]

On Sun, Jul 09, 2023 at 04:32:22PM +, Anne Macedo wrote:
> On Tue, Jul 04, 2023 at 11:22:29PM +, Anne Macedo wrote:
> > Hey!
> > 
> > I'm trying to bake Linux images for the Orange Pi One Plus using Yocto.
> > Everything works fine, except for Ethernet.
> > 
> > On the u-boot prompt:
> > 
> > => dhcp
> > No ethernet found.
> > 
> > After adding:
> > 
> > CONFIG_SPL_SPI_SUNXI=y
> > CONFIG_SUN8I_EMAC=y
> > 
> > to configs/orangepi_one_plus_defconfig, I started seeing this error:
> > 
> > => dhcp
> > sun8i_emac_eth_start: Timeout
> > 
> > I saw this other bug report but I couldn't really understand what has
> > been made to fix this issue [1].
> > 
> > More context here [2].
> > 
> > [1] https://lists.denx.de/pipermail/u-boot/2021-June/451357.html
> > [2] https://github.com/linux-sunxi/meta-sunxi/issues/387
> > 
> > Regards,
> > Anne
Quick update
> 
> Just wanted to share a summary of my findings about the ethernet on the
> Orange Pi One Plus (Allwinner H6).
> 
> 1. PMIC should not be disabled. I tested and if I disable PMIC, MAC
> never turns on, even if I force the gpio PD6 pin to be on. When building
> tfa, use SUNXI_SETUP_REGULATORS=1 or just don't pass it.
> 
> 2. These configs are needed on configs/orangepi_one_plus_defconfig
> 
> CONFIG_SPL_SPI_SUNXI=y
CONFIG_SPL_SPI_SUNXI may not be needed in this context, I don't know why
I mentioned it in the first place.
> CONFIG_SUN8I_EMAC=y
> 
> 3. With this config, there's this strange behavior where ethernet is
> only detected after a crash:
> 
> # Fresh boot
> Net:   Could not get PHY for ethernet@502: addr 1
> No ethernet found.
> 
> # Forcing the board to crash
> => mii dump
> "Synchronous Abort" handler, esr 0x9644
> 
> Code: 3221 d5033fbf 91408013 f9481a60 (b9004801) 
> Resetting CPU ...
> 
> resetting ...
> 
> # Reboot
> Net:   eth0: ethernet@502
> 
> 4. I'm testing CONFIG_MACPWR="PD6" and that successfully enabled MAC on
> u-boot (I see the LEDs turning on). However, I see the behaviour from #3
> but crash-rebooting doesn't seem to enable ethernet...

According to this patch [1], we should use DM driver, so I believe
CONFIG_MACPWR is deprecated. 

I then added these configs:

# Required by regulator
CONFIG_DM=y
# Required by regulator fixed
CONFIG_DM_REGULATOR=y
# Required by the ethernet's definition on the dts
CONFIG_DM_REGULATOR_FIXED=y
# This adds a handy regulator cmd on u-boot
CONFIG_CMD_REGULATOR=y

With the regulator command, I turned it on and saw the LEDs turning on
and dhcp "trying" to work (it doesn't get an IP though).

=> regulator list
| Device  | regulator-name  | Parent
| vcc5v   | vcc-5v  | root_driver
| gmac-3v3| vcc-gmac-3v3| root_driver
=> regulator dev vcc-gmac-3v3
dev: vcc-gmac-3v3 @ gmac-3v3
=> regulator enable
=> dhcp
BOOTP broadcast 1
BOOTP broadcast 2

For some reason, vcc-gmac-3v3 is disabled by default, and it only seems
to start manually for me. It sometimes start when I try a mii dump or a
dhcp, but there's a lot of phy errors that make it unusable.

[1] https://lists.denx.de/pipermail/u-boot/2022-December/501397.html
> 
> Regards,
> Anne

Re: [bug report] sunxi: H6: no ethernet on Orange Pi One Plus

[Anne Macedo]

On Tue, Jul 04, 2023 at 11:22:29PM +, Anne Macedo wrote:
> Hey!
> 
> I'm trying to bake Linux images for the Orange Pi One Plus using Yocto.
> Everything works fine, except for Ethernet.
> 
> On the u-boot prompt:
> 
> => dhcp
> No ethernet found.
> 
> After adding:
> 
> CONFIG_SPL_SPI_SUNXI=y
> CONFIG_SUN8I_EMAC=y
> 
> to configs/orangepi_one_plus_defconfig, I started seeing this error:
> 
> => dhcp
> sun8i_emac_eth_start: Timeout
> 
> I saw this other bug report but I couldn't really understand what has
> been made to fix this issue [1].
> 
> More context here [2].
> 
> [1] https://lists.denx.de/pipermail/u-boot/2021-June/451357.html
> [2] https://github.com/linux-sunxi/meta-sunxi/issues/387
> 
> Regards,
> Anne

Just wanted to share a summary of my findings about the ethernet on the
Orange Pi One Plus (Allwinner H6).

1. PMIC should not be disabled. I tested and if I disable PMIC, MAC
never turns on, even if I force the gpio PD6 pin to be on. When building
tfa, use SUNXI_SETUP_REGULATORS=1 or just don't pass it.

2. These configs are needed on configs/orangepi_one_plus_defconfig

CONFIG_SPL_SPI_SUNXI=y
CONFIG_SUN8I_EMAC=y

3. With this config, there's this strange behavior where ethernet is
only detected after a crash:

# Fresh boot
Net:   Could not get PHY for ethernet@502: addr 1
No ethernet found.

# Forcing the board to crash
=> mii dump
"Synchronous Abort" handler, esr 0x9644

Code: 3221 d5033fbf 91408013 f9481a60 (b9004801) 
Resetting CPU ...

resetting ...

# Reboot
Net:   eth0: ethernet@502

4. I'm testing CONFIG_MACPWR="PD6" and that successfully enabled MAC on
u-boot (I see the LEDs turning on). However, I see the behaviour from #3
but crash-rebooting doesn't seem to enable ethernet...

Regards,
Anne

Re: Pull request efi-2023-07-rc7

[Tom Rini]

On Sun, Jul 09, 2023 at 12:38:46PM +0200, Heinrich Schuchardt wrote:
> Dear Tom,
> 
> The following changes since commit 0beb649053b86b2cfd5cf55a0fc68bc2fe91a430:
> 
>   MAINTAINERS: correct at91 tree link (2023-07-07 11:37:09 -0400)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-efi.git
> tags/efi-2023-07-rc7
> 
> for you to fetch changes up to e05a4b12a056fd7fe22e85715c8f5e5e811fb551:
> 
>   mkeficapsule: fix efi_firmware_management_capsule_header data type
> (2023-07-09 10:32:28 +0200)
> 
> Gitlab CI showed no issues:
> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/16815
> 
> 
> Pull request efi-2023-07-rc7
> 
> Documentation:
> 
> * Fix links to Linux kernel documentation
> 
> UEFI:
> 
> * Fix memory leak in efidebug dh subcommand
> * Fix underflow when calculating remaining variable store size
> * Increase default variable store size to 64 KiB
> * mkeficapsule: fix efi_firmware_management_capsule_header data type

None of these look like boot regression fixes, nor security fixes that
we should have already applied earlier this cycle, so I'm going to hold
this off for v2023.07 unless you strongly object.

-- 
Tom


signature.asc
Description: PGP signature

RE: [PATCH 1/2] cmd: thordown: Add proper dependency for CMD_THOR_DOWNLOAD

[Soma, Ashok Reddy]

Hi Heinrich,

> -Original Message-
> From: Heinrich Schuchardt 
> Sent: Sunday, July 9, 2023 7:09 PM
> To: Soma, Ashok Reddy ; u-
> b...@lists.denx.de
> Cc: s...@chromium.org; ilias.apalodi...@linaro.org; rfried@gmail.com;
> seanedm...@microsoft.com; tob...@waldekranz.com; s...@denx.de;
> j...@metanate.com; Simek, Michal ; git (AMD-
> Xilinx) 
> Subject: Re: [PATCH 1/2] cmd: thordown: Add proper dependency for
> CMD_THOR_DOWNLOAD
> 
> 
> 
> Am 9. Juli 2023 15:09:57 MESZ schrieb Ashok Reddy Soma
> :
> >When CONFIG_CMD_USB and CONFIG_USB are disabled some compilation
> errors
> >are seen as below.
> 
> Thanks for your patch.
> 
> Currently we have no documentation for the thordown command.  We
> should create a man page in /docs/usage/cmd/.
> 
> Do you have any description of the usage of the command?

No, I was not working with thor download command
I was disabling CONFIG_CMD_USB and CONFIG_USB and saw some compilation errors 
from cmd/thordown.c.
So, added dependency and sent patch.

Thanks,
Ashok

> 
> Best regards
> 
> Heinrich
> 
> 
> 
> >
> >cmd/thordown.o: in function `usb_gadget_initialize':
> >include/linux/usb/gadget.h:981: undefined reference to `board_usb_init'
> >cmd/thordown.o: in function `do_thor_down':
> >cmd/thordown.c:68: undefined reference to `g_dnl_unregister'
> >cmd/thordown.o: in function `usb_gadget_release':
> >include/linux/usb/gadget.h:986: undefined reference to
> `board_usb_cleanup'
> >cmd/thordown.o: in function `do_thor_down':
> >cmd/thordown.c:41: undefined reference to `g_dnl_register'
> >cmd/thordown.c:48: undefined reference to `thor_init'
> >cmd/thordown.c:56: undefined reference to `thor_handle'
> >gnu/aarch64/lin/aarch64-linux/bin/aarch64-linux-gnu-ld.bfd: line 4:  8485
> >Segmentation fault  (core dumped) $CC --sysroot=$LIBC
> >--no-warn-rwx-segment "$@"
> >Makefile:1779: recipe for target 'u-boot' failed
> >make: *** [u-boot] Error 139
> >make: *** Deleting file 'u-boot'
> >
> >Add dependency of CMD_USB for CONFIG_CMD_THOR_DOWNLOAD to fix
> the errors.
> >
> >Signed-off-by: Ashok Reddy Soma 
> >---
> >
> > cmd/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> >diff --git a/cmd/Kconfig b/cmd/Kconfig
> >index 02e54f1e50..b44df9d67a 100644
> >--- a/cmd/Kconfig
> >+++ b/cmd/Kconfig
> >@@ -526,6 +526,7 @@ config CMD_SPL_WRITE_SIZE
> >
> > config CMD_THOR_DOWNLOAD
> > bool "thor - TIZEN 'thor' download"
> >+depends on CMD_USB
> > select DFU
> > help
> >   Implements the 'thor' download protocol. This is a way of

Re: [PATCH v3 02/11] capsule: authenticate: Add capsule public key in platform's dtb

[Heinrich Schuchardt]

Am 9. Juli 2023 15:33:17 MESZ schrieb Sughosh Ganu :
>The EFI capsule authentication logic in u-boot expects the public key
>in the form of an EFI Signature List(ESL) to be provided as part of
>the platform's dtb. Currently, the embedding of the ESL file into the
>dtb needs to be done manually.
>
>Add a signature node in the u-boot dtsi file and include the public
>key through the capsule-key property. This file is per architecture,
>and is currently being added for sandbox and arm architectures. It

The device-tree compiler can pick up files from /include/. If the dtsi file is 
not architecture specific, we should avoid code duplication.

We should treat all EFI architectures the same.

Best regards

Heinrich

>will have to be added for other architectures which need to enable
>capsule authentication support.
>
>The path to the ESL file is specified through the
>CONFIG_EFI_CAPSULE_ESL_FILE symbol.
>
>Signed-off-by: Sughosh Ganu 
>---
>Changes since V2:
>* Add the public key ESL file through the u-boot.dtsi.
>* Add the dtsi files for sandbox and arm architectures.
>* Add a check in the Makefile that the ESL file path is not empty.
>
> arch/arm/dts/u-boot.dtsi | 17 +
> arch/sandbox/dts/u-boot.dtsi | 17 +
> lib/efi_loader/Kconfig   | 11 +++
> lib/efi_loader/Makefile  |  7 +++
> 4 files changed, 52 insertions(+)
> create mode 100644 arch/arm/dts/u-boot.dtsi
> create mode 100644 arch/sandbox/dts/u-boot.dtsi
>
>diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
>new file mode 100644
>index 00..60bd004937
>--- /dev/null
>+++ b/arch/arm/dts/u-boot.dtsi
>@@ -0,0 +1,17 @@
>+// SPDX-License-Identifier: GPL-2.0+
>+/*
>+ * Devicetree file with miscellaneous nodes that will be included
>+ * at build time into the DTB. Currently being used for including
>+ * capsule related information.
>+ *
>+ */
>+
>+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
>+/ {
>+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
>+  signature {
>+  capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
>+  };
>+#endif
>+};
>+#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
>diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
>new file mode 100644
>index 00..60bd004937
>--- /dev/null
>+++ b/arch/sandbox/dts/u-boot.dtsi
>@@ -0,0 +1,17 @@
>+// SPDX-License-Identifier: GPL-2.0+
>+/*
>+ * Devicetree file with miscellaneous nodes that will be included
>+ * at build time into the DTB. Currently being used for including
>+ * capsule related information.
>+ *
>+ */
>+
>+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
>+/ {
>+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
>+  signature {
>+  capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
>+  };
>+#endif
>+};
>+#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
>diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>index c5835e6ef6..1326a1d109 100644
>--- a/lib/efi_loader/Kconfig
>+++ b/lib/efi_loader/Kconfig
>@@ -234,6 +234,17 @@ config EFI_CAPSULE_MAX
> Select the max capsule index value used for capsule report
> variables. This value is used to create CapsuleMax variable.
> 
>+config EFI_CAPSULE_ESL_FILE
>+  string "Path to the EFI Signature List File"
>+  default ""
>+  depends on EFI_CAPSULE_AUTHENTICATE
>+  help
>+Provides the absolute path to the EFI Signature List
>+file which will be embedded in the platform's device
>+tree and used for capsule authentication at the time
>+of capsule update.
>+
>+
> config EFI_DEVICE_PATH_TO_TEXT
>   bool "Device path to text protocol"
>   default y
>diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>index 13a35eae6c..9fb04720d9 100644
>--- a/lib/efi_loader/Makefile
>+++ b/lib/efi_loader/Makefile
>@@ -86,3 +86,10 @@ obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
> 
> EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
> $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
>+
>+ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
>+EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_ESL_FILE))
>+ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","")
>+$(error .esl cerificate not found. Configure your CONFIG_EFI_CAPSULE_ESL_FILE)
>+endif
>+endif

Re: [PATCH 1/2] cmd: thordown: Add proper dependency for CMD_THOR_DOWNLOAD

[Heinrich Schuchardt]

Am 9. Juli 2023 15:09:57 MESZ schrieb Ashok Reddy Soma 
:
>When CONFIG_CMD_USB and CONFIG_USB are disabled some compilation errors
>are seen as below.

Thanks for your patch.

Currently we have no documentation for the thordown command.  We should create 
a man page in /docs/usage/cmd/.

Do you have any description of the usage of the command?

Best regards

Heinrich



>
>cmd/thordown.o: in function `usb_gadget_initialize':
>include/linux/usb/gadget.h:981: undefined reference to `board_usb_init'
>cmd/thordown.o: in function `do_thor_down':
>cmd/thordown.c:68: undefined reference to `g_dnl_unregister'
>cmd/thordown.o: in function `usb_gadget_release':
>include/linux/usb/gadget.h:986: undefined reference to `board_usb_cleanup'
>cmd/thordown.o: in function `do_thor_down':
>cmd/thordown.c:41: undefined reference to `g_dnl_register'
>cmd/thordown.c:48: undefined reference to `thor_init'
>cmd/thordown.c:56: undefined reference to `thor_handle'
>gnu/aarch64/lin/aarch64-linux/bin/aarch64-linux-gnu-ld.bfd: line 4:  8485
>Segmentation fault  (core dumped) $CC --sysroot=$LIBC
>--no-warn-rwx-segment "$@"
>Makefile:1779: recipe for target 'u-boot' failed
>make: *** [u-boot] Error 139
>make: *** Deleting file 'u-boot'
>
>Add dependency of CMD_USB for CONFIG_CMD_THOR_DOWNLOAD to fix the errors.
>
>Signed-off-by: Ashok Reddy Soma 
>---
>
> cmd/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/cmd/Kconfig b/cmd/Kconfig
>index 02e54f1e50..b44df9d67a 100644
>--- a/cmd/Kconfig
>+++ b/cmd/Kconfig
>@@ -526,6 +526,7 @@ config CMD_SPL_WRITE_SIZE
> 
> config CMD_THOR_DOWNLOAD
>   bool "thor - TIZEN 'thor' download"
>+  depends on CMD_USB
>   select DFU
>   help
> Implements the 'thor' download protocol. This is a way of

[PATCH v3 11/11] sandbox: capsule: Generate capsule related files through binman

[Sughosh Ganu]

The EFI capsule files can now be generated as part of u-boot
build. This is done through binman. Add capsule entry nodes in the
u-boot.dtsi for the sandbox architecture for generating the
capsules. Remove the corresponding generation of capsules from the
capsule update conftest file.

The capsules are generated through the config file for the sandbox
variant, and through explicit parameters for the sandbox_flattree
variant.

Also generate the FIT image used for testing the capsule update
feature on the sandbox_flattree variant through binman. Remove the now
superfluous its file which was used for generating this FIT image.

Signed-off-by: Sughosh Ganu 
---
Changes since V2:
* New patch for generating the capsules and capsule input files
  through binman.

 arch/sandbox/dts/u-boot.dtsi  | 143 ++
 test/py/tests/test_efi_capsule/conftest.py|  62 
 .../tests/test_efi_capsule/uboot_bin_env.its  |  36 -
 3 files changed, 143 insertions(+), 98 deletions(-)
 delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its

diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
index 60bd004937..292fb86a50 100644
--- a/arch/sandbox/dts/u-boot.dtsi
+++ b/arch/sandbox/dts/u-boot.dtsi
@@ -13,5 +13,148 @@
capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
};
 #endif
+
+   binman: binman {
+   multiple-images;
+   };
+};
+
+ {
+   itb {
+   filename = "/tmp/capsules/uboot_bin_env.itb";
+
+   fit {
+   description = "Automatic U-Boot environment update";
+   #address-cells = <2>;
+
+   images {
+   u-boot-bin {
+   description = "U-Boot binary on SPI 
Flash";
+   data = 
/incbin/("/tmp/capsules/u-boot.bin.new");
+   compression = "none";
+   type = "firmware";
+   arch = "sandbox";
+   load = <0>;
+   hash-1 {
+   algo = "sha1";
+   };
+   };
+   u-boot-env {
+   description = "U-Boot environment on 
SPI Flash";
+   data = 
/incbin/("/tmp/capsules/u-boot.env.new");
+   compression = "none";
+   type = "firmware";
+   arch = "sandbox";
+   load = <0>;
+   hash-1 {
+   algo = "sha1";
+   };
+   };
+   };
+   };
+   };
+
+#ifdef CONFIG_EFI_USE_CAPSULE_CFG_FILE
+   capsule1 {
+   capsule {
+   cfg-file = CONFIG_EFI_CAPSULE_CFG_FILE;
+   };
+   };
+#else
+   capsule2 {
+   capsule {
+   image-index = <0x1>;
+   image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
+   filename = "/tmp/capsules/u-boot.bin.new";
+   capsule = "/tmp/capsules/Test01";
+   };
+   };
+
+   capsule3 {
+   capsule {
+   image-index = <0x2>;
+   image-type-id = "5A7021F5-FEF2-48B4-AABA-832E777418C0";
+   filename = "/tmp/capsules/u-boot.env.new";
+   capsule = "/tmp/capsules/Test02";
+   };
+   };
+
+   capsule4 {
+   capsule {
+   image-index = <0x1>;
+   image-type-id = "058B7D83-50D5-4C47-A195-60D86AD341C4";
+   filename = "/tmp/capsules/u-boot.bin.new";
+   capsule = "/tmp/capsules/Test03";
+   };
+   };
+
+   capsule5 {
+   capsule {
+   image-index = <0x1>;
+   image-type-id = "3673B45D-6A7C-46F3-9E60-ADABB03F7937";
+   filename = "/tmp/capsules/uboot_bin_env.itb";
+   capsule = "/tmp/capsules/Test04";
+   };
+   };
+
+   capsule6 {
+   capsule {
+   image-index = <0x1>;
+   image-type-id = "058B7D83-50D5-4C47-A195-60D86AD341C4";
+   filename = "/tmp/capsules/uboot_bin_env.itb";
+   capsule = "/tmp/capsules/Test05";
+   };
+   };
+
+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
+   capsule7 {
+   capsule {
+   

[PATCH v3 10/11] sandbox: capsule: Add a config file for generating capsules

[Sughosh Ganu]

Support has been added to the mkeficapsule tool to generate capsules
by parsing the capsule parameters through a config file. Add a config
file for generating capsules. These capsules will be used for testing
the capsule update feature on sandbox platform.

Enable generation of capsules through the config file on the sandbox
variant.

Signed-off-by: Sughosh Ganu 
---
Changes since V2:
* New patch to add the capsule generation config file for sandbox.

 .azure-pipelines.yml  |  1 +
 .gitlab-ci.yml|  1 +
 configs/sandbox_defconfig |  2 +
 test/py/conftest.py   |  5 ++
 .../test_efi_capsule/sandbox_capsule_cfg.txt  | 75 +++
 5 files changed, 84 insertions(+)
 create mode 100644 test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt

diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 75075bbd07..cc196bf98c 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -403,6 +403,7 @@ stages:
   echo -n "u-boot:New" >/tmp/capsules/u-boot.bin.new;
   echo -n "u-boot-env:Old" >/tmp/capsules/u-boot.env.old;
   echo -n "u-boot-env:New" >/tmp/capsules/u-boot.env.new;
+  cp test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt 
/tmp/capsules/;
   if [[ "${TEST_PY_BD}" == "sandbox" ]] || [[ "${TEST_PY_BD}" == 
"sandbox_flattree" ]]; then
   openssl req -x509 -sha256 -newkey rsa:2048 -subj 
/CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER.key -out /tmp/capsules/SIGNER.crt 
-nodes -days 365;
   openssl req -x509 -sha256 -newkey rsa:2048 -subj 
/CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER2.key -out 
/tmp/capsules/SIGNER2.crt -nodes -days 365;
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 577eebd678..614bf61962 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -42,6 +42,7 @@ stages:
 - echo -n "u-boot:New" >/tmp/capsules/u-boot.bin.new;
 - echo -n "u-boot-env:Old" >/tmp/capsules/u-boot.env.old;
 - echo -n "u-boot-env:New" >/tmp/capsules/u-boot.env.new;
+- cp test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt /tmp/capsules/;
 - if [[ "${TEST_PY_BD}" == "sandbox" ]] || [[ "${TEST_PY_BD}" == 
"sandbox_flattree" ]]; then
openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ 
-keyout /tmp/capsules/SIGNER.key -out /tmp/capsules/SIGNER.crt -nodes -days 365;
openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ 
-keyout /tmp/capsules/SIGNER2.key -out /tmp/capsules/SIGNER2.crt -nodes -days 
365;
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index d8a2386bb0..0f4c59e1a8 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -340,6 +340,8 @@ CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
 CONFIG_EFI_CAPSULE_AUTHENTICATE=y
 CONFIG_EFI_CAPSULE_ESL_FILE="/tmp/capsules/SIGNER.esl"
+CONFIG_EFI_CAPSULE_CFG_FILE="/tmp/capsules/sandbox_capsule_cfg.txt"
+CONFIG_EFI_USE_CAPSULE_CFG_FILE=y
 CONFIG_EFI_SECURE_BOOT=y
 CONFIG_TEST_FDTDEC=y
 CONFIG_UNIT_TEST=y
diff --git a/test/py/conftest.py b/test/py/conftest.py
index 661ed74fae..f32ab1a70c 100644
--- a/test/py/conftest.py
+++ b/test/py/conftest.py
@@ -161,6 +161,11 @@ def setup_capsule_build(source_dir, build_dir, board_type, 
log):
)
 run_command(name, cmd, source_dir)
 
+capsule_cfg_file = 'test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt'
+name = 'cp'
+cmd = ( ' cp %s %s' % (capsule_cfg_file, capsule_sig_dir))
+run_command(name, cmd, source_dir)
+
 gen_capsule_payloads(capsule_sig_dir)
 
 def run_build(config, source_dir, build_dir, board_type, log):
diff --git a/test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt 
b/test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt
new file mode 100644
index 00..4e5065d538
--- /dev/null
+++ b/test/py/tests/test_efi_capsule/sandbox_capsule_cfg.txt
@@ -0,0 +1,75 @@
+{
+   image-index: 1
+   image-guid: 09D7CF52-0720-4710-91D1-08469B7FE9C8
+   payload: /tmp/capsules/u-boot.bin.new
+   capsule: /tmp/capsules/Test01
+}
+{
+   image-index: 2
+   image-guid: 5A7021F5-FEF2-48B4-AABA-832E777418C0
+   payload: /tmp/capsules/u-boot.env.new
+   capsule: /tmp/capsules/Test02
+}
+{
+   image-index: 1
+   image-guid: 058B7D83-50D5-4C47-A195-60D86AD341C4
+   payload: /tmp/capsules/u-boot.bin.new
+   capsule: /tmp/capsules/Test03
+
+}
+{
+   image-index: 1
+   image-guid: 3673B45D-6A7C-46F3-9E60-ADABB03F7937
+   payload: /tmp/capsules/uboot_bin_env.itb
+   capsule: /tmp/capsules/Test04
+
+}
+{
+   image-index: 1
+   image-guid: 058B7D83-50D5-4C47-A195-60D86AD341C4
+   payload: /tmp/capsules/uboot_bin_env.itb
+   capsule: /tmp/capsules/Test05
+
+}
+{
+   image-index: 1
+   image-guid: 058B7D83-50D5-4C47-A195-60D86AD341C4
+   payload: /tmp/capsules/uboot_bin_env.itb
+   capsule: /tmp/capsules/Test05
+}
+{
+   

[PATCH v3 09/11] test: capsule: Remove public key embed logic from capsule update test

[Sughosh Ganu]

The embedding of the public key EFI Signature List(ESL) file into the
platform's DTB is now done at the time of u-boot build. Remove this
logic from the capsule update test' configuration.

Include the public key for the sandbox and sandbox_flattree variant
as part of the build.

Signed-off-by: Sughosh Ganu 
---
Changes since V2:
* New patch for removing the capsule key and ESL generation logic from
  the capsule test config file.

 configs/sandbox_defconfig|  1 +
 configs/sandbox_flattree_defconfig   |  1 +
 test/py/tests/test_efi_capsule/conftest.py   | 30 +++-
 test/py/tests/test_efi_capsule/signature.dts | 10 ---
 4 files changed, 6 insertions(+), 36 deletions(-)
 delete mode 100644 test/py/tests/test_efi_capsule/signature.dts

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 1ec44d5b33..d8a2386bb0 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -339,6 +339,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
 CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
 CONFIG_EFI_CAPSULE_AUTHENTICATE=y
+CONFIG_EFI_CAPSULE_ESL_FILE="/tmp/capsules/SIGNER.esl"
 CONFIG_EFI_SECURE_BOOT=y
 CONFIG_TEST_FDTDEC=y
 CONFIG_UNIT_TEST=y
diff --git a/configs/sandbox_flattree_defconfig 
b/configs/sandbox_flattree_defconfig
index e7657d40dc..8d60744771 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -226,6 +226,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
 CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
 CONFIG_EFI_CAPSULE_AUTHENTICATE=y
+CONFIG_EFI_CAPSULE_ESL_FILE="/tmp/capsules/SIGNER.esl"
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
diff --git a/test/py/tests/test_efi_capsule/conftest.py 
b/test/py/tests/test_efi_capsule/conftest.py
index a337e62936..9b0f7e635d 100644
--- a/test/py/tests/test_efi_capsule/conftest.py
+++ b/test/py/tests/test_efi_capsule/conftest.py
@@ -25,42 +25,20 @@ def efi_capsule_data(request, u_boot_config):
 image_path = u_boot_config.persistent_data_dir + '/test_efi_capsule.img'
 
 try:
+capsules_path_dir = '/tmp/capsules/'
 # Create a target device
 check_call('dd if=/dev/zero of=./spi.bin bs=1MiB count=16', shell=True)
 
 check_call('rm -rf %s' % mnt_point, shell=True)
 check_call('mkdir -p %s' % data_dir, shell=True)
 check_call('mkdir -p %s' % install_dir, shell=True)
+check_call('cp %s/* %s ' % (capsules_path_dir, data_dir), shell=True)
 
 capsule_auth_enabled = u_boot_config.buildconfig.get(
 'config_efi_capsule_authenticate')
 if capsule_auth_enabled:
-# Create private key (SIGNER.key) and certificate (SIGNER.crt)
-check_call('cd %s; '
-   'openssl req -x509 -sha256 -newkey rsa:2048 '
-'-subj /CN=TEST_SIGNER/ -keyout SIGNER.key '
-'-out SIGNER.crt -nodes -days 365'
-   % data_dir, shell=True)
-check_call('cd %s; %scert-to-efi-sig-list SIGNER.crt SIGNER.esl'
-   % (data_dir, EFITOOLS_PATH), shell=True)
-
-# Update dtb adding capsule certificate
-check_call('cd %s; '
-   'cp %s/test/py/tests/test_efi_capsule/signature.dts .'
-   % (data_dir, u_boot_config.source_dir), shell=True)
-check_call('cd %s; '
-   'dtc -@ -I dts -O dtb -o signature.dtbo signature.dts; '
-   'fdtoverlay -i %s/arch/sandbox/dts/test.dtb '
-'-o test_sig.dtb signature.dtbo'
-   % (data_dir, u_boot_config.build_dir), shell=True)
-
-# Create *malicious* private key (SIGNER2.key) and certificate
-# (SIGNER2.crt)
-check_call('cd %s; '
-   'openssl req -x509 -sha256 -newkey rsa:2048 '
-'-subj /CN=TEST_SIGNER/ -keyout SIGNER2.key '
-'-out SIGNER2.crt -nodes -days 365'
-   % data_dir, shell=True)
+check_call('cp %s/arch/sandbox/dts/test.dtb %s/test_sig.dtb' %
+   (u_boot_config.build_dir, data_dir), shell=True)
 
 # Create capsule files
 # two regions: one for u-boot.bin and the other for u-boot.env
diff --git a/test/py/tests/test_efi_capsule/signature.dts 
b/test/py/tests/test_efi_capsule/signature.dts
deleted file mode 100644
index 078cfc76c9..00
--- a/test/py/tests/test_efi_capsule/signature.dts
+++ /dev/null
@@ -1,10 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-
-/dts-v1/;
-/plugin/;
-
-&{/} {
-   signature {
-   capsule-key = /incbin/("SIGNER.esl");
-   };
-};
-- 
2.34.1

[PATCH v3 08/11] test: py: Setup capsule files for testing

[Sughosh Ganu]

Support has being added through earlier commits to build capsules and
embed the public key needed for capsule authentication as part of
u-boot build.

>From the testing point-of-view, this means the input files needed for
the above have to be setup before invoking the build. Set this up in
the pytest configuration file for testing the capsule update feature.

Signed-off-by: Sughosh Ganu 
---
Changes since V2:
* New patch for setting up the capsule files in the pytest setup
  before initiation of u-boot build.

 test/py/conftest.py | 87 +
 1 file changed, 87 insertions(+)

diff --git a/test/py/conftest.py b/test/py/conftest.py
index fc9dd3a83f..661ed74fae 100644
--- a/test/py/conftest.py
+++ b/test/py/conftest.py
@@ -80,6 +80,89 @@ def pytest_addoption(parser):
 help='Run sandbox under gdbserver. The argument is the channel '+
 'over which gdbserver should communicate, e.g. localhost:1234')
 
+def setup_capsule_build(source_dir, build_dir, board_type, log):
+"""Setup the platform's build for testing capsule updates
+
+This generates the payload/input files needed for testing the
+capsule update functionality, along with the keys for signing
+the capsules. An EFI Signature List(ESL) file, which houses the
+public key for capsule authentication is generated as
+well.
+
+The ESL file is subsequently embedded into the platform's
+dtb during the u-boot build, to be used for capsule
+authentication.
+
+Two sets of keys are generated, namely SIGNER and SIGNER2.
+The SIGNER2 key pair is used as a malicious key for testing the
+the capsule authentication functionality.
+
+All the generated files are placed under the /tmp/capsules/
+directory.
+
+Args:
+soruce_dir (str): Directory containing source code
+build_dir (str): Directory to build in
+board_type (str): board_type parameter (e.g. 'sandbox')
+log (Logfile): Log file to use
+
+Returns:
+Nothing.
+"""
+def run_command(name, cmd, source_dir):
+with log.section(name):
+if isinstance(cmd, str):
+cmd = cmd.split()
+runner = log.get_runner(name, None)
+runner.run(cmd, cwd=source_dir)
+runner.close()
+log.status_pass('OK')
+
+def gen_capsule_payloads(capsule_dir):
+fname = '%su-boot.bin.old' % capsule_dir
+with open(fname, 'w') as fd:
+fd.write('u-boot:Old')
+
+fname = '%su-boot.bin.new' % capsule_dir
+with open(fname, 'w') as fd:
+fd.write('u-boot:New')
+
+fname = '%su-boot.env.old' % capsule_dir
+with open(fname, 'w') as fd:
+fd.write('u-boot-env:Old')
+
+fname = '%su-boot.env.new' % capsule_dir
+with open(fname, 'w') as fd:
+fd.write('u-boot-env:New')
+
+capsule_sig_dir = '/tmp/capsules/'
+sig_name = 'SIGNER'
+mkdir_p(capsule_sig_dir)
+name = 'openssl'
+cmd = ( 'openssl req -x509 -sha256 -newkey rsa:2048 '
+'-subj /CN=TEST_SIGNER/ -keyout %s%s.key '
+'-out %s%s.crt -nodes -days 365'
+% (capsule_sig_dir, sig_name, capsule_sig_dir, sig_name)
+   )
+run_command(name, cmd, source_dir)
+
+name = 'cert-to-efi-sig-list'
+cmd = ( 'cert-to-efi-sig-list %s%s.crt %s%s.esl'
+% (capsule_sig_dir, sig_name, capsule_sig_dir, sig_name)
+   )
+run_command(name, cmd, source_dir)
+
+sig_name = 'SIGNER2'
+name = 'openssl'
+cmd = ( 'openssl req -x509 -sha256 -newkey rsa:2048 '
+'-subj /CN=TEST_SIGNER/ -keyout %s%s.key '
+'-out %s%s.crt -nodes -days 365'
+% (capsule_sig_dir, sig_name, capsule_sig_dir, sig_name)
+   )
+run_command(name, cmd, source_dir)
+
+gen_capsule_payloads(capsule_sig_dir)
+
 def run_build(config, source_dir, build_dir, board_type, log):
 """run_build: Build U-Boot
 
@@ -90,6 +173,10 @@ def run_build(config, source_dir, build_dir, board_type, 
log):
 board_type (str): board_type parameter (e.g. 'sandbox')
 log (Logfile): Log file to use
 """
+capsule_boards = ( 'sandbox', 'sandbox64', 'sandbox_flattree' )
+if board_type in capsule_boards:
+setup_capsule_build(source_dir, build_dir, board_type, log)
+
 if config.getoption('buildman'):
 if build_dir != source_dir:
 dest_args = ['-o', build_dir, '-w']
-- 
2.34.1

[PATCH v3 07/11] CI: capsule: Setup the files needed for capsule update testing

[Sughosh Ganu]

Support has being added through earlier commits to build capsules
and embed the public key needed for capsule authentication as part of
u-boot build.

>From the testing point-of-view, this means the input files needed for
generating the above have to be setup before invoking the build. Set
this up in the CI configuration files for testing the capsule update
feature.

Signed-off-by: Sughosh Ganu 
---
Changes since V2:
* New patch setting up the capsule files needed for CI run

 .azure-pipelines.yml | 21 +
 .gitlab-ci.yml   | 19 +++
 2 files changed, 40 insertions(+)

diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 96b2ab4d75..75075bbd07 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -398,6 +398,17 @@ stages:
   wget -O - 
https://github.com/riscv/opensbi/releases/download/v0.9/opensbi-0.9-rv-bin.tar.xz
 | tar -C /tmp -xJ;
   export 
OPENSBI=/tmp/opensbi-0.9-rv-bin/share/opensbi/lp64/generic/firmware/fw_dynamic.bin;
   fi
+  mkdir -p /tmp/capsules/;
+  echo -n "u-boot:Old" >/tmp/capsules/u-boot.bin.old;
+  echo -n "u-boot:New" >/tmp/capsules/u-boot.bin.new;
+  echo -n "u-boot-env:Old" >/tmp/capsules/u-boot.env.old;
+  echo -n "u-boot-env:New" >/tmp/capsules/u-boot.env.new;
+  if [[ "${TEST_PY_BD}" == "sandbox" ]] || [[ "${TEST_PY_BD}" == 
"sandbox_flattree" ]]; then
+  openssl req -x509 -sha256 -newkey rsa:2048 -subj 
/CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER.key -out /tmp/capsules/SIGNER.crt 
-nodes -days 365;
+  openssl req -x509 -sha256 -newkey rsa:2048 -subj 
/CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER2.key -out 
/tmp/capsules/SIGNER2.crt -nodes -days 365;
+  cert-to-efi-sig-list /tmp/capsules/SIGNER.crt 
/tmp/capsules/SIGNER.esl;
+  fi
+
   # the below corresponds to .gitlab-ci.yml "script"
   cd ${WORK_DIR}
   export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD};
@@ -582,6 +593,16 @@ stages:
   cd ${WORK_DIR}
   # make environment variables available as tests are running inside a 
container
   export BUILDMAN="${BUILDMAN}"
+  if [[ "${BUILDMAN}" == "sandbox" ]] || [[ "${BUILDMAN}" == "sandbox 
x86" ]]; then
+  if [ ! -d "/tmp/capsules/" ]; then
+  mkdir -p /tmp/capsules/;
+  openssl req -x509 -sha256 -newkey rsa:2048 -subj 
/CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER.key -out /tmp/capsules/SIGNER.crt 
-n
+odes -days 365;
+  openssl req -x509 -sha256 -newkey rsa:2048 -subj 
/CN=TEST_SIGNER/ -keyout /tmp/capsules/SIGNER2.key -out 
/tmp/capsules/SIGNER2.crt
+-nodes -days 365;
+  cert-to-efi-sig-list /tmp/capsules/SIGNER.crt 
/tmp/capsules/SIGNER.esl;
+  fi
+  fi
   git config --global --add safe.directory ${WORK_DIR}
   EOF
   cat << "EOF" >> build.sh
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index e6c6ab3586..577eebd678 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -37,6 +37,17 @@ stages:
 export 
OPENSBI=/tmp/opensbi-0.9-rv-bin/share/opensbi/lp64/generic/firmware/fw_dynamic.bin;
   fi
 
+- mkdir -p /tmp/capsules/;
+- echo -n "u-boot:Old" >/tmp/capsules/u-boot.bin.old;
+- echo -n "u-boot:New" >/tmp/capsules/u-boot.bin.new;
+- echo -n "u-boot-env:Old" >/tmp/capsules/u-boot.env.old;
+- echo -n "u-boot-env:New" >/tmp/capsules/u-boot.env.new;
+- if [[ "${TEST_PY_BD}" == "sandbox" ]] || [[ "${TEST_PY_BD}" == 
"sandbox_flattree" ]]; then
+   openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ 
-keyout /tmp/capsules/SIGNER.key -out /tmp/capsules/SIGNER.crt -nodes -days 365;
+   openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ 
-keyout /tmp/capsules/SIGNER2.key -out /tmp/capsules/SIGNER2.crt -nodes -days 
365;
+   cert-to-efi-sig-list /tmp/capsules/SIGNER.crt /tmp/capsules/SIGNER.esl;
+  fi
+
   after_script:
 - cp -v /tmp/${TEST_PY_BD}/*.{html,css} .
 - rm -rf /tmp/uboot-test-hooks /tmp/venv
@@ -131,6 +142,14 @@ build all other platforms:
   stage: world build
   script:
 - ret=0;
+  if [ ! -d "/tmp/capsules/" ]; then
+mkdir -p /tmp/capsules/;
+openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ 
-keyout /tmp/capsules/SIGNER.key -out /tmp/capsules/SIGNER.crt -nodes -days
+ 365;
+openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_SIGNER/ 
-keyout /tmp/capsules/SIGNER2.key -out /tmp/capsules/SIGNER2.crt -nodes -da
+ys 365;
+cert-to-efi-sig-list /tmp/capsules/SIGNER.crt /tmp/capsules/SIGNER.esl;
+  fi
   git config --global --add safe.directory "${CI_PROJECT_DIR}";
   ./tools/buildman/buildman -o /tmp -PEWM -x arm,powerpc || ret=$?;
   if [[ $ret -ne 0 ]]; then
-- 
2.34.1

[PATCH v3 06/11] binman: capsule: Add support for generating capsules

[Sughosh Ganu]

Add support in binman for generating capsules. The capsule parameters
can be specified either through a config file or through the capsule
binman entry.

Signed-off-by: Sughosh Ganu 
---
Changes since V2:
* New patch which generates capsules through binman replacing the
  earlier make target.

 tools/binman/btool/mkeficapsule.py |  91 +
 tools/binman/entries.rst   |  27 
 tools/binman/etype/capsule.py  | 102 +
 3 files changed, 220 insertions(+)
 create mode 100644 tools/binman/btool/mkeficapsule.py
 create mode 100644 tools/binman/etype/capsule.py

diff --git a/tools/binman/btool/mkeficapsule.py 
b/tools/binman/btool/mkeficapsule.py
new file mode 100644
index 00..9f656c12cf
--- /dev/null
+++ b/tools/binman/btool/mkeficapsule.py
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2023 Linaro Limited
+#
+"""Bintool implementation for mkeficapsule tool
+
+mkeficapsule is a tool used for generating EFI capsules.
+
+The following are the command-line options to be provided
+to the tool
+Usage: mkeficapsule [options]  
+Options:
+   -g, --guid guid for image blob type
+   -i, --index  update image index
+   -I, --instanceupdate hardware instance
+   -p, --private-key   private key file
+   -c, --certificate  signer's certificate file
+   -m, --monotonic-count  monotonic count
+   -d, --dump_sig  dump signature (*.p7)
+   -A, --fw-accept  firmware accept capsule, requires GUID, no image blob
+   -R, --fw-revert  firmware revert capsule, takes no GUID, no image blob
+   -o, --capoemflag Capsule OEM Flag, an integer between 0x and 0x
+   -f, --cfg-file  config file with capsule parameters
+   -h, --help  print a help message
+
+"""
+
+from binman import bintool
+
+class Bintoolmkeficapsule(bintool.Bintool):
+"""Handles the 'mkeficapsule' tool
+
+This bintool is used for generating the EFI capsules. The
+capsule generation parameters can either be specified through
+command-line, or through a config file.
+
+"""
+def __init__(self, name):
+super().__init__(name, 'mkeficapsule tool for generating capsules')
+
+def capsule_cfg_file(self, cfg_file):
+
+args = [
+f'--cfg-file={cfg_file}'
+]
+self.run_cmd(*args)
+
+def cmdline_capsule(self, image_index, image_guid, hardware_instance,
+payload, output_fname):
+
+args = [
+f'--index={image_index}',
+f'--guid={image_guid}',
+f'--instance={hardware_instance}',
+payload,
+output_fname
+]
+self.run_cmd(*args)
+
+def cmdline_auth_capsule(self, image_index, image_guid, hardware_instance,
+ monotonic_count, priv_key, pub_key,
+ payload, output_fname):
+
+args = [
+f'--index={image_index}',
+f'--guid={image_guid}',
+f'--instance={hardware_instance}',
+f'--monotonic-count={monotonic_count}',
+f'--private-key={priv_key}',
+f'--certificate={pub_key}',
+payload,
+output_fname
+]
+self.run_cmd(*args)
+
+def fetch(self, method):
+"""Fetch handler for mkeficapsule
+
+This builds the tool from source
+
+Returns:
+tuple:
+str: Filename of fetched file to copy to a suitable directory
+str: Name of temp directory to remove, or None
+"""
+if method != bintool.FETCH_BUILD:
+return None
+result = self.build_from_git(
+'https://source.denx.de/u-boot/u-boot.git',
+'tools',
+'tools/mkeficapsule')
+return result
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index b71af801fd..9a263e8691 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -283,6 +283,33 @@ entry; similarly for SPL.
 
 
 
+.. _etype_capsule:
+
+Entry: capsule: Entry for generating EFI Capsule files
+--
+
+This is an entry for generating EFI capsules.
+
+The parameters needed for generation of the capsules can either be
+provided separately, or through a config file.
+
+Properties / Entry arguments:
+- cfg-file: Config file for providing capsule
+  parameters.
+- image-index: Unique number for identifying
+  corresponding payload image.
+- image-type-id: Image GUID which will be used
+  for identifying the image.
+- hardware-instance: Optional number for identifying
+  unique hardware instance of a device in the system.
+- monotomic-count: Count used when signing an image.
+- private-key: Path to private key file.
+- pub-key-cert: Path to public key certificate file.
+- filename: Path to the 

[PATCH v3 05/11] doc: Add documentation to describe capsule config file format

[Sughosh Ganu]

The UEFI capsule can be generated either through command-line
parameters, or, by specifying those in a config file. Add
documentation to describe the format of the config file.

Signed-off-by: Sughosh Ganu 
---
Changes since V2: None

 doc/develop/uefi/uefi.rst | 64 +++
 1 file changed, 64 insertions(+)

diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index c04e62f3a5..ddf8e20cb0 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -442,6 +442,70 @@ following command can be issued
   --guid c1b629f1-ce0e-4894-82bf-f0a38387e630 \
   optee.bin optee.capsule
 
+Or alternatively, the capsule can be generated through a make target
+
+.. code-block:: bash
+
+$ make capsule
+
+Issuing the above make command requires specifying the capsule
+parameters through a config file instead. The Kconfig symbol
+CONFIG_EFI_CAPSULE_CFG_FILE is to be used for specifying the path to
+the config file.
+
+The config file describes the parameters that are used for generating
+one or more capsules. The parameters for a given capsule file are
+specified within curly braces, in the form of "key:value" pairs. All
+the parameters that are currently supported by the mkeficapsule tool
+can be specified through the config file.
+
+The following are some example payload parameters specified through
+the config file.
+
+.. code-block:: none
+
+   {
+   image-guid: 02f4d760-cfd5-43bd-8e2d-a42acb33c660
+   hardware-instance: 0
+   monotonic-count: 1
+   payload: u-boot.bin
+   image-index: 1
+   private-key: /path/to/priv/key
+   pub-key-cert: /path/to/pub/key
+   capsule: u-boot.capsule
+   }
+   {
+   image-guid: 4ce292da-1dd8-428d-a1c2-77743ef8b96e
+   hardware-instance: 0
+   payload: u-boot.itb
+   image-index: 2
+   oemflags: 0x8000
+   capsule: fit.capsule
+   }
+   {
+   capsule-type: accept
+   image-guid: 4ce292da-1dd8-428d-a1c2-77743ef8b96e
+   capsule: accept.capsule
+   }
+   {
+   capsule-type: revert
+   capsule: revert.capsule
+   }
+
+The following are the keys that specify the capsule parameters
+
+..code-block:: none
+
+image-guid: Image GUID
+image-index: Image index value
+private-key: Path to the private key file used for capsule signing
+pub-key-cert: Path to the public key crt file used for capsule signing
+payload: Path to the capsule payload file
+capsule: Path to the output capsule file that is generated
+hardware-instance: Hardware Instance value
+monotonic-count: Monotonic count value
+capsule-type: Specifies capsule type. normal(default), accept or revert
+oemflags: 16bit Oemflags value to be used(populated in capsule header)
 
 Enabling Capsule Authentication
 ***
-- 
2.34.1

[PATCH v3 04/11] tools: mkeficapsule: Add support for parsing capsule params from config file

[Sughosh Ganu]

Add support for specifying the parameters needed for capsule
generation through a config file, instead of passing them through
command-line. Parameters for more than a single capsule file can be
specified, resulting in generation of multiple capsules through a
single invocation of the command.

This path is to be used for generating capsules through a make target,
with the parameters being parsed from the config file.

Signed-off-by: Sughosh Ganu 
---
Changes since V2:
* Add a Kconfig boolean symbol CONFIG_EFI_USE_CAPSULE_CFG_FILE which
  can be used to generate capsules through config file or parameters.

 tools/Kconfig  |  16 ++
 tools/Makefile |   1 +
 tools/eficapsule.h | 110 
 tools/mkeficapsule.c   |  84 +
 tools/mkeficapsule_parse.c | 345 +
 5 files changed, 526 insertions(+), 30 deletions(-)
 create mode 100644 tools/mkeficapsule_parse.c

diff --git a/tools/Kconfig b/tools/Kconfig
index 539708f277..9b744aba31 100644
--- a/tools/Kconfig
+++ b/tools/Kconfig
@@ -98,6 +98,22 @@ config TOOLS_MKEFICAPSULE
  optionally sign that file. If you want to enable UEFI capsule
  update feature on your target, you certainly need this.
 
+config EFI_CAPSULE_CFG_FILE
+   string "Path to the EFI Capsule Config File"
+   default ""
+   help
+ Path to the EFI capsule config file which provides the
+ parameters needed to build capsule(s). Parameters can be
+ provided for multiple payloads resulting in corresponding
+ capsule images being generated.
+
+config EFI_USE_CAPSULE_CFG_FILE
+   bool "Use the config file for generating capsules"
+   help
+ Boolean option used to specify if the EFI capsules are to
+ be generated through parameters specified via the config
+ file or through command line.
+
 menuconfig FSPI_CONF_HEADER
bool "FlexSPI Header Configuration"
help
diff --git a/tools/Makefile b/tools/Makefile
index d793cf3bec..ef366f3d61 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -250,6 +250,7 @@ HOSTLDLIBS_mkeficapsule += \
 HOSTLDLIBS_mkeficapsule += \
$(shell pkg-config --libs uuid 2> /dev/null || echo "-luuid")
 hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
+mkeficapsule-objs := mkeficapsule.o mkeficapsule_parse.o
 
 # We build some files with extra pedantic flags to try to minimize things
 # that won't build on some weird host compiler -- though there are lots of
diff --git a/tools/eficapsule.h b/tools/eficapsule.h
index 072a4b5598..42e66c6d6a 100644
--- a/tools/eficapsule.h
+++ b/tools/eficapsule.h
@@ -52,6 +52,38 @@ typedef struct {
 /* flags */
 #define CAPSULE_FLAGS_PERSIST_ACROSS_RESET  0x0001
 
+enum capsule_type {
+   CAPSULE_NORMAL_BLOB = 0,
+   CAPSULE_ACCEPT,
+   CAPSULE_REVERT,
+};
+
+/**
+ * struct efi_capsule_params - Capsule parameters
+ * @image_guid: Guid value of the payload input image
+ * @image_index: Image index value
+ * @hardware_instance: Hardware instance to be used for the image
+ * @monotonic_count: Monotonic count value to be used for signed capsule
+ * @privkey_file: Path to private key used in capsule signing
+ * @cert_file: Path to public key certificate used in capsule signing
+ * @input_file: Path to payload input image
+ * @capsule_file: Path to the output capsule file
+ * @oemflags: Oemflags to be populated in the capsule header
+ * @capsule: Capsule Type, normal or accept or revert
+ */
+struct efi_capsule_params {
+   efi_guid_t *image_guid;
+   unsigned long image_index;
+   unsigned long hardware_instance;
+   uint64_t monotonic_count;
+   char *privkey_file;
+   char *cert_file;
+   char *input_file;
+   char *capsule_file;
+   unsigned long oemflags;
+   enum capsule_type capsule;
+};
+
 struct efi_capsule_header {
efi_guid_t capsule_guid;
uint32_t header_size;
@@ -113,4 +145,82 @@ struct efi_firmware_image_authentication {
struct win_certificate_uefi_guid auth_info;
 } __packed;
 
+/**
+ * capsule_with_cfg_file() - Generate capsule from config file
+ * @cfg_file: Path to the config file
+ *
+ * Parse the capsule parameters from the config file and use the
+ * parameters for generating one or more capsules.
+ *
+ * Return: None
+ *
+ */
+void capsule_with_cfg_file(const char *cfg_file);
+
+/**
+ * convert_uuid_to_guid() - convert UUID to GUID
+ * @buf:   UUID binary
+ *
+ * UUID and GUID have the same data structure, but their binary
+ * formats are different due to the endianness. See lib/uuid.c.
+ * Since uuid_parse() can handle only UUID, this function must
+ * be called to get correct data for GUID when parsing a string.
+ *
+ * The correct data will be returned in @buf.
+ */
+void convert_uuid_to_guid(unsigned char *buf);
+
+/**
+ * create_empty_capsule() - Generate an empty capsule
+ * @path: Path to the empty capsule file to be generated
+ * @guid: Guid value 

[PATCH v3 02/11] capsule: authenticate: Add capsule public key in platform's dtb

[Sughosh Ganu]

The EFI capsule authentication logic in u-boot expects the public key
in the form of an EFI Signature List(ESL) to be provided as part of
the platform's dtb. Currently, the embedding of the ESL file into the
dtb needs to be done manually.

Add a signature node in the u-boot dtsi file and include the public
key through the capsule-key property. This file is per architecture,
and is currently being added for sandbox and arm architectures. It
will have to be added for other architectures which need to enable
capsule authentication support.

The path to the ESL file is specified through the
CONFIG_EFI_CAPSULE_ESL_FILE symbol.

Signed-off-by: Sughosh Ganu 
---
Changes since V2:
* Add the public key ESL file through the u-boot.dtsi.
* Add the dtsi files for sandbox and arm architectures.
* Add a check in the Makefile that the ESL file path is not empty.

 arch/arm/dts/u-boot.dtsi | 17 +
 arch/sandbox/dts/u-boot.dtsi | 17 +
 lib/efi_loader/Kconfig   | 11 +++
 lib/efi_loader/Makefile  |  7 +++
 4 files changed, 52 insertions(+)
 create mode 100644 arch/arm/dts/u-boot.dtsi
 create mode 100644 arch/sandbox/dts/u-boot.dtsi

diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
new file mode 100644
index 00..60bd004937
--- /dev/null
+++ b/arch/arm/dts/u-boot.dtsi
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Devicetree file with miscellaneous nodes that will be included
+ * at build time into the DTB. Currently being used for including
+ * capsule related information.
+ *
+ */
+
+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
+/ {
+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
+   signature {
+   capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
+   };
+#endif
+};
+#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
new file mode 100644
index 00..60bd004937
--- /dev/null
+++ b/arch/sandbox/dts/u-boot.dtsi
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Devicetree file with miscellaneous nodes that will be included
+ * at build time into the DTB. Currently being used for including
+ * capsule related information.
+ *
+ */
+
+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
+/ {
+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
+   signature {
+   capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
+   };
+#endif
+};
+#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index c5835e6ef6..1326a1d109 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -234,6 +234,17 @@ config EFI_CAPSULE_MAX
  Select the max capsule index value used for capsule report
  variables. This value is used to create CapsuleMax variable.
 
+config EFI_CAPSULE_ESL_FILE
+   string "Path to the EFI Signature List File"
+   default ""
+   depends on EFI_CAPSULE_AUTHENTICATE
+   help
+ Provides the absolute path to the EFI Signature List
+ file which will be embedded in the platform's device
+ tree and used for capsule authentication at the time
+ of capsule update.
+
+
 config EFI_DEVICE_PATH_TO_TEXT
bool "Device path to text protocol"
default y
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 13a35eae6c..9fb04720d9 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -86,3 +86,10 @@ obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
 
 EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
 $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
+
+ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
+EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_ESL_FILE))
+ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","")
+$(error .esl cerificate not found. Configure your CONFIG_EFI_CAPSULE_ESL_FILE)
+endif
+endif
-- 
2.34.1

[PATCH v3 03/11] doc: capsule: Document the new mechanism to embed ESL file into dtb

[Sughosh Ganu]

Update the document to specify how the EFI Signature List(ESL) file
can be embedded into the platform's dtb as part of the u-boot build.

Signed-off-by: Sughosh Ganu 
---
Changes since V2:
* Highlight the need to use the u-boot.dtsi file for embedding the
  public key ESL into the DTB.

 doc/develop/uefi/uefi.rst | 22 +-
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index ffe25ca231..c04e62f3a5 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -495,20 +495,16 @@ and used by the steps highlighted below.
 ...
 }
 
-You can do step-4 manually with
+You can perform step-4 by defining the Kconfig symbol
+CONFIG_EFI_CAPSULE_ESL_FILE. Once this has been done, the signature
+node can be added to the u-boot.dtsi file. For reference, check the
+u-boot.dtsi file for the sandbox architecture. If this node has not
+been added to the architecture's u-boot.dtsi file, this needs to be
+done. The node has currently been added for the sandbox and arm
+architectures' in the u-boot.dtsi file. Once the u-boot.dtsi file has
+been added with the signature node, the esl file will automatically
+get embedded into the platform's dtb as part of u-boot build.
 
-.. code-block:: console
-
-$ dtc -@ -I dts -O dtb -o signature.dtbo signature.dts
-$ fdtoverlay -i orig.dtb -o new.dtb -v signature.dtbo
-
-where signature.dts looks like::
-
-&{/} {
-signature {
-capsule-key = /incbin/("CRT.esl");
-};
-};
 
 Executing the boot manager
 ~~
-- 
2.34.1

[PATCH v3 01/11] nuvoton: npcm845-evb: Add a newline at the end of file

[Sughosh Ganu]

Add a newline at the end of the dts, without which the build fails
when including the u-boot.dtsi file.

Signed-off-by: Sughosh Ganu 
---
Changes since V2:
* New patch

 arch/arm/dts/nuvoton-npcm845-evb.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/dts/nuvoton-npcm845-evb.dts 
b/arch/arm/dts/nuvoton-npcm845-evb.dts
index 3cab7807e3..a93666cb41 100644
--- a/arch/arm/dts/nuvoton-npcm845-evb.dts
+++ b/arch/arm/dts/nuvoton-npcm845-evb.dts
@@ -354,4 +354,4 @@
_pins
_pins
>;
-};
\ No newline at end of file
+};
-- 
2.34.1

[PATCH v3 00/11] Integrate EFI capsule tasks into u-boot's build flow

[Sughosh Ganu]

This patchset aims to bring two capsule related tasks under the u-boot
build flow.

One is the embedding of the public key into the platform's dtb. The
public key is in the form of an EFI Signature List(ESL) file and is
used for capsule authentication. This is being achieved by adding the
signature node containing the capsule public key in the architecture's
u-boot.dtsi file. Currently, the u-boot.dtsi file has been added for
the sandbox and arm architectures. The path to the ESL file is being
provided through a Kconfig symbol(CONFIG_EFI_CAPSULE_ESL_FILE).

Changes have also been made to the test flow so that the keys used for
signing the capsule, and the ESL file, are generated prior to invoking
the u-boot's build, which enables embedding the ESL file into the dtb
as part of the u-boot build.

The other task is related to generation of capsules. Support is being
added to generate capsules by specifying the capsule parameters in a
config file. Calling the mkeficapsule tool then results in generation
of the corresponding capsule files. The capsules can be generated as
part of u-boot build, and this is being achieved through binman, by
adding a capsule entry type. The capsules can be generated either by
specifying the capsule parameters in a config file, or through
specifying them as properties under the capsule entry node. If using
the config file, the path to the config file is to be specified
through a Kconfig symbol(CONFIG_EFI_CAPSULE_CFG_FILE).

Changes have also been made to the efi capsule update feature testing
setup on the sandbox variants. Currently, the capsule files and the
public key ESL file are generated after u-boot has been built. This
logic has been changed so that the capsule input files along with the
keys needed for capsule signing and authentication are generated prior
to initiation of the u-boot build. The placement of all the files
needed for generation of capsules, along with the generated capsule
files is under the /tmp/capsules/ directory.

Currently, the capsule update feature is tested on the sandbox
and sandbox_flattree variants in CI. The capsule generation through
config file is enabled for the sandbox variant, with the
sandbox_flattree variant generating capsules through the command-line
parameters.

The document has been updated to reflect the above changes.

Changes since V2:
This version embeds the capsule auth related public key through the
u-boot.dtsi file. The capsule generation has been moved to binman. The
changes in the test setup have been split into multiple patches,
instead of a single monolithic patch.

* Add the public key ESL file through the u-boot.dtsi
* Add the dtsi files for sandbox and arm architectures
* Add a check in the Makefile that the ESL file path is not empty.
* Highlight the need to use the u-boot.dtsi file for embedding the
  public key ESL into the DTB.
* Add a Kconfig boolean symbol CONFIG_EFI_USE_CAPSULE_CFG_FILE which
  can be used to generate capsules through config file or parameters.
* New patch which generates capsules through binman replacing the
  earlier make target.
* New patch setting up the capsule files needed for CI run
* New patch for setting up the capsule files in the pytest setup
  before initiation of u-boot build.
* New patch for removing the capsule key and ESL generation logic from
  the capsule test config file.
* New patch to add the capsule generation config file for sandbox.
* New patch for generating the capsules and capsule input files
  through binman.


Sughosh Ganu (11):
  nuvoton: npcm845-evb: Add a newline at the end of file
  capsule: authenticate: Add capsule public key in platform's dtb
  doc: capsule: Document the new mechanism to embed ESL file into dtb
  tools: mkeficapsule: Add support for parsing capsule params from
config file
  doc: Add documentation to describe capsule config file format
  binman: capsule: Add support for generating capsules
  CI: capsule: Setup the files needed for capsule update testing
  test: py: Setup capsule files for testing
  test: capsule: Remove public key embed logic from capsule update test
  sandbox: capsule: Add a config file for generating capsules
  sandbox: capsule: Generate capsule related files through binman

 .azure-pipelines.yml  |  22 ++
 .gitlab-ci.yml|  20 +
 arch/arm/dts/nuvoton-npcm845-evb.dts  |   2 +-
 arch/arm/dts/u-boot.dtsi  |  17 +
 arch/sandbox/dts/u-boot.dtsi  | 160 
 configs/sandbox_defconfig |   3 +
 configs/sandbox_flattree_defconfig|   1 +
 doc/develop/uefi/uefi.rst |  86 -
 lib/efi_loader/Kconfig|  11 +
 lib/efi_loader/Makefile   |   7 +
 test/py/conftest.py   |  92 +
 test/py/tests/test_efi_capsule/conftest.py|  92 +
 .../test_efi_capsule/sandbox_capsule_cfg.txt  |  75 
 

[PATCH 1/2] cmd: thordown: Add proper dependency for CMD_THOR_DOWNLOAD

[Ashok Reddy Soma]

When CONFIG_CMD_USB and CONFIG_USB are disabled some compilation errors
are seen as below.

cmd/thordown.o: in function `usb_gadget_initialize':
include/linux/usb/gadget.h:981: undefined reference to `board_usb_init'
cmd/thordown.o: in function `do_thor_down':
cmd/thordown.c:68: undefined reference to `g_dnl_unregister'
cmd/thordown.o: in function `usb_gadget_release':
include/linux/usb/gadget.h:986: undefined reference to `board_usb_cleanup'
cmd/thordown.o: in function `do_thor_down':
cmd/thordown.c:41: undefined reference to `g_dnl_register'
cmd/thordown.c:48: undefined reference to `thor_init'
cmd/thordown.c:56: undefined reference to `thor_handle'
gnu/aarch64/lin/aarch64-linux/bin/aarch64-linux-gnu-ld.bfd: line 4:  8485
Segmentation fault  (core dumped) $CC --sysroot=$LIBC
--no-warn-rwx-segment "$@"
Makefile:1779: recipe for target 'u-boot' failed
make: *** [u-boot] Error 139
make: *** Deleting file 'u-boot'

Add dependency of CMD_USB for CONFIG_CMD_THOR_DOWNLOAD to fix the errors.

Signed-off-by: Ashok Reddy Soma 
---

 cmd/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 02e54f1e50..b44df9d67a 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -526,6 +526,7 @@ config CMD_SPL_WRITE_SIZE
 
 config CMD_THOR_DOWNLOAD
bool "thor - TIZEN 'thor' download"
+   depends on CMD_USB
select DFU
help
  Implements the 'thor' download protocol. This is a way of
-- 
2.17.1

[PATCH 2/2] zynqmp: config: Add proper dependencies for USB

[Ashok Reddy Soma]

When CONFIG_CMD_USB and CONFIG_USB are disabled, still some compilation
errors are seen as below.

In file included from include/configs/xilinx_zynqmp.h:173,
 from include/config.h:3,
 from include/common.h:16,
 from env/common.c:10:
include/config_distro_bootcmd.h:302:9: error: expected '}' before 
'BOOT_TARGET_DEVICES_references_USB_without_CONFIG_CMD_USB'
  302 | BOOT_TARGET_DEVICES_references_USB_without_CONFIG_CMD_USB
  | ^
include/config_distro_bootcmd.h:302:9: note: in definition of macro
'BOOTENV_DEV_NAME_USB'
  302 | BOOT_TARGET_DEVICES_references_USB_without_CONFIG_CMD_USB
  | ^
include/configs/xilinx_zynqmp.h:77:41: note: in expansion of macro
'BOOTENV_DEV_NAME'
   77 | # define BOOT_TARGET_DEVICES_USB(func)  func(USB, usb, 0)
   func(USB, usb, 1)
  | ^~~~
include/configs/xilinx_zynqmp.h:168:9: note: in expansion of macro
'BOOT_TARGET_DEVICES_USB'
  168 | BOOT_TARGET_DEVICES_USB(func) \
  | ^~~
include/config_distro_bootcmd.h:454:25: note: in expansion of macro
'BOOT_TARGET_DEVICES'
  454 | "boot_targets=" BOOT_TARGET_DEVICES(BOOTENV_DEV_NAME) "\0"
  | ^~~
include/config_distro_bootcmd.h:474:9: note: in expansion of macro
'BOOTENV_BOOT_TARGETS'
  474 | BOOTENV_BOOT_TARGETS \
  | ^~~~
include/configs/xilinx_zynqmp.h:179:9: note: in expansion of macro
'BOOTENV'
  179 | BOOTENV
  | ^~~
include/env_default.h:120:9: note: in expansion of macro
'CFG_EXTRA_ENV_SETTINGS'
  120 | CFG_EXTRA_ENV_SETTINGS
  | ^~
In file included from env/common.c:32:
include/env_default.h:27:36: note: to match this '{'
   27 | const char default_environment[] = {
  |^
scripts/Makefile.build:256: recipe for target 'env/common.o' failed
make[1]: *** [env/common.o] Error 1
Makefile:1853: recipe for target 'env' failed
make: *** [env] Error 2
make: *** Waiting for unfinished jobs

Add CONFIG_USB_STORAGE as dependency for USB related macro's such as
BOOT_TARGET_DEVICES_USB() and DFU_DEFAULT_POLL_TIMEOUT and
CONFIG_THOR_RESET_OFF.

Remove CONFIG_ZYNQMP_USB from Kconfig and also from defconfig since it
is not used anywhere else.

Signed-off-by: Ashok Reddy Soma 
---

 arch/arm/mach-zynqmp/Kconfig | 3 ---
 configs/xilinx_zynqmp_virt_defconfig | 1 -
 include/configs/xilinx_zynqmp.h  | 4 ++--
 3 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
index fd6f07715a..26b80b7882 100644
--- a/arch/arm/mach-zynqmp/Kconfig
+++ b/arch/arm/mach-zynqmp/Kconfig
@@ -84,9 +84,6 @@ config ZYNQMP_SPL_PM_CFG_OBJ_FILE
  Leave this option empty if your PMU firmware has a hard-coded
  configuration object or you are loading it by any other means.
 
-config ZYNQMP_USB
-   bool "Configure ZynqMP USB"
-
 config ZYNQMP_NO_DDR
bool "Disable DDR MMU mapping"
help
diff --git a/configs/xilinx_zynqmp_virt_defconfig 
b/configs/xilinx_zynqmp_virt_defconfig
index c4bbde2206..6bda4f8453 100644
--- a/configs/xilinx_zynqmp_virt_defconfig
+++ b/configs/xilinx_zynqmp_virt_defconfig
@@ -17,7 +17,6 @@ CONFIG_ENV_OFFSET_REDUND=0x1E8
 CONFIG_SPL_SPI_FLASH_SUPPORT=y
 CONFIG_SPL_SPI=y
 CONFIG_CMD_FRU=y
-CONFIG_ZYNQMP_USB=y
 CONFIG_SYS_LOAD_ADDR=0x800
 CONFIG_AHCI=y
 CONFIG_SYS_MEMTEST_START=0x
diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h
index 011f0034c5..44f8914b80 100644
--- a/include/configs/xilinx_zynqmp.h
+++ b/include/configs/xilinx_zynqmp.h
@@ -29,7 +29,7 @@
 
 /* Miscellaneous configurable options */
 
-#if defined(CONFIG_ZYNQMP_USB)
+#if defined(CONFIG_USB_STORAGE)
 #define DFU_DEFAULT_POLL_TIMEOUT   300
 
 # define PARTS_DEFAULT \
@@ -73,7 +73,7 @@
 # define BOOT_TARGET_DEVICES_SCSI(func)
 #endif
 
-#if defined(CONFIG_ZYNQMP_USB)
+#if defined(CONFIG_USB_STORAGE)
 # define BOOT_TARGET_DEVICES_USB(func) func(USB, usb, 0) func(USB, usb, 1)
 #else
 # define BOOT_TARGET_DEVICES_USB(func)
-- 
2.17.1

[PATCH 0/2] Fix dependencies of USB Kconfig options

[Ashok Reddy Soma]

When USB device driver CONFIG_USB and CONFIG_CMD_USB are disabled, some
compilation issues are seen. Also CMD_THOR_DOWNLOAD should depend on
CONFIG_CMD_USB. Add dependencies to resolve those issues and compile
properly. Also remove unused config CONFIG_ZYNQMP_USB.


Ashok Reddy Soma (2):
  cmd: thordown: Add proper dependency for CMD_THOR_DOWNLOAD
  zynqmp: config: Add proper dependencies for USB

 arch/arm/mach-zynqmp/Kconfig | 3 ---
 cmd/Kconfig  | 1 +
 configs/xilinx_zynqmp_virt_defconfig | 1 -
 include/configs/xilinx_zynqmp.h  | 4 ++--
 4 files changed, 3 insertions(+), 6 deletions(-)

-- 
2.17.1

Re: [PATCH v3 u-boot] mmc: spl: Make partition choice in default_spl_mmc_emmc_boot_partition() more explicit

[Pali Rohár]

On Saturday 08 July 2023 11:28:34 Tom Rini wrote:
> On Sat, Jul 08, 2023 at 10:30:47AM +0200, Pali Rohár wrote:
> > On Friday 07 July 2023 19:43:34 Tom Rini wrote:
> > > On Sat, Jul 08, 2023 at 12:54:38AM +0200, Pali Rohár wrote:
> > > 
> > > > To make eMMC partition choosing in default_spl_mmc_emmc_boot_partition()
> > > > function better understandable, rewrite it via explicit switch-case code
> > > > pattern.
> > > > 
> > > > Also indicate an error when eMMC EXT_CSD[179] register is configured by
> > > > user to value which is not suitable for eMMC booting and SPL do not know
> > > > how to interpret it.
> > > > 
> > > > Signed-off-by: Pali Rohár 
> > > 
> > > I did some quick local testing to check on the size impact and this is
> > > indeed quite manageable, thanks for reworking things!
> > > 
> > > Reviewed-by: Tom Rini 
> > 
> > Unfortunately no, sama5d2_xplained_emmc, sama5d2_xplained_mmc and
> > sama5d2_xplained_qspiflash are failing with v3; but not with v2.
> > 
> > Seems that these boards have their SPL at limit and adding any new
> > puts() increase size over the limit.
> > 
> > So I think that there is no other way than just adding a new config
> > option to hide this new puts().
> 
> We should enable LTO on those platforms then and buy us some space.
> 
> -- 
> Tom


Ok, but then this is really out of what I have a free time to do...

Re: imx8mp: issues getting dtb with pcie enabled to boot

[Fabio Estevam]

On Fri, Jul 7, 2023 at 8:48 PM Sahaj Sarup  wrote:

> For now the fix seems to be `clocks = <_blk_ctrl 0>;`

Please submit the fix to Linux.

> this gets pcie working under linux but not under u-boot, i'm guessing pcie 
> for imx8 is not yet implemented?

That's correct.

[PATCH v2] x86: Update docs link in bootparam header

[Paul Barker]

After Linux commit ff61f0791ce9, x86 documentation was moved to
arch/x86 and the link in bootparam.h was broken.

Signed-off-by: Paul Barker 
---
 arch/x86/include/asm/bootparam.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/bootparam.h b/arch/x86/include/asm/bootparam.h
index ea816ca74698..ac4865300f1b 100644
--- a/arch/x86/include/asm/bootparam.h
+++ b/arch/x86/include/asm/bootparam.h
@@ -62,7 +62,7 @@ struct setup_indirect {
 /**
  * struct setup_header - Information needed by Linux to boot
  *
- * See https://www.kernel.org/doc/html/latest/x86/boot.html
+ * See https://www.kernel.org/doc/html/latest/arch/x86/boot.html
  */
 struct setup_header {
__u8setup_sects;

base-commit: 0beb649053b86b2cfd5cf55a0fc68bc2fe91a430
-- 
2.34.1

Re: [PATCH] x86: Update docs link in bootparam header

[Paul Barker]

On 09/07/2023 02:47, Heinrich Schuchardt wrote:

On 7/7/23 17:41, Tom Rini wrote:

On Fri, Jul 07, 2023 at 07:51:42AM +0100, Paul Barker wrote:


After Linux commit ff61f0791ce9, x86 documentation was moved to
arch/x86 and the link in bootparam.h was broken.

Signed-off-by: Paul Barker 
---
  arch/x86/include/asm/bootparam.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/bootparam.h 
b/arch/x86/include/asm/bootparam.h

index ea816ca74698..1f8ca569b880 100644
--- a/arch/x86/include/asm/bootparam.h
+++ b/arch/x86/include/asm/bootparam.h
@@ -62,7 +62,7 @@ struct setup_indirect {
  /**
   * struct setup_header - Information needed by Linux to boot
   *
- * See https://www.kernel.org/doc/html/latest/x86/boot.html
+ * See https://docs.kernel.org/arch/x86/boot.html


This is also now:
https://www.kernel.org/doc/html/latest/arch/x86/boot.html
And tree-wide we have two examples of docs.kernel.org and the rest are
www.kernel.org/doc/html/latest for the base.  We should be consistent
here, so I'm deferring to Heinrich.



In Linux' /Documentation/ they always uses
https://www.kernel.org/doc/html/latest. So let's stick to this.


Ok, I'll send a v2.

Thanks,
Paul

Pull request efi-2023-07-rc7

[Heinrich Schuchardt]

Dear Tom,

The following changes since commit 0beb649053b86b2cfd5cf55a0fc68bc2fe91a430:

  MAINTAINERS: correct at91 tree link (2023-07-07 11:37:09 -0400)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-efi.git
tags/efi-2023-07-rc7

for you to fetch changes up to e05a4b12a056fd7fe22e85715c8f5e5e811fb551:

  mkeficapsule: fix efi_firmware_management_capsule_header data type
(2023-07-09 10:32:28 +0200)

Gitlab CI showed no issues:
https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/16815


Pull request efi-2023-07-rc7

Documentation:

* Fix links to Linux kernel documentation

UEFI:

* Fix memory leak in efidebug dh subcommand
* Fix underflow when calculating remaining variable store size
* Increase default variable store size to 64 KiB
* mkeficapsule: fix efi_firmware_management_capsule_header data type


Alper Nebi Yasak (2):
  efi_loader: Avoid underflow when calculating remaining var store size
  efi_loader: Increase default variable store size to 64KiB

Heinrich Schuchardt (1):
  doc: harmonize Linux kernel documentation links

Malte Schmidt (1):
  mkeficapsule: fix efi_firmware_management_capsule_header data type

Masahisa Kojima (1):
  cmd: efidebug: add missing efi_free_pool for dh subcommand

Paul Barker (1):
  x86: Update docs link in bootparam header

 arch/x86/include/asm/bootparam.h | 2 +-
 cmd/efidebug.c   | 1 +
 doc/develop/docstyle.rst | 2 +-
 doc/usage/blkmap.rst | 2 +-
 lib/efi_loader/Kconfig   | 5 +++--
 lib/efi_loader/efi_var_mem.c | 4 
 tools/eficapsule.h   | 2 +-
 7 files changed, 12 insertions(+), 6 deletions(-)

Re: [PATCH v2] x86: Update docs link in bootparam header

[Heinrich Schuchardt]

On 7/9/23 11:44, Paul Barker wrote:

After Linux commit ff61f0791ce9, x86 documentation was moved to
arch/x86 and the link in bootparam.h was broken.

Signed-off-by: Paul Barker 


Reviewed-by: Heinrich Schuchardt 


---
  arch/x86/include/asm/bootparam.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/bootparam.h b/arch/x86/include/asm/bootparam.h
index ea816ca74698..ac4865300f1b 100644
--- a/arch/x86/include/asm/bootparam.h
+++ b/arch/x86/include/asm/bootparam.h
@@ -62,7 +62,7 @@ struct setup_indirect {
  /**
   * struct setup_header - Information needed by Linux to boot
   *
- * See https://www.kernel.org/doc/html/latest/x86/boot.html
+ * See https://www.kernel.org/doc/html/latest/arch/x86/boot.html
   */
  struct setup_header {
__u8setup_sects;

base-commit: 0beb649053b86b2cfd5cf55a0fc68bc2fe91a430

Re: [PATCH] efi_loader: make efi_delete_handle() follow the EFI spec

[Heinrich Schuchardt]

On 6/26/23 12:04, Ilias Apalodimas wrote:

The EFI doesn't allow removal of handles, unless all hosted protocols
are cleanly removed.  Our efi_delete_handle() is a bit intrusive.
Although it does try to delete protocols before removing a handle,
it doesn't care if that fails.  Instead it only returns an error if the
handle is invalid. On top of that none of the callers of that function
check the return code.

So let's rewrite this in a way that fits the EFI spec better.  Instead
of forcing the handle removal, gracefully uninstall all the handle
protocols.  According to the EFI spec when the last protocol is removed
the handle will be deleted.  Also switch all the callers and check the
return code. Some callers can't do anything useful apart from reporting
an error.  The disk related functions on the other hand, can prevent a
medium that is being used by EFI from removal.

The only function that doesn't check the result is efi_delete_image().
But that function needs a bigger rework anyway, so we can clean it up in
the future

Signed-off-by: Ilias Apalodimas 
---
Heinrich this needs to be applied on top of
https://lore.kernel.org/u-boot/20230620061932.113292-1-ilias.apalodi...@linaro.org/

  cmd/bootefi.c | 19 ---
  include/efi_loader.h  |  2 +-
  lib/efi_driver/efi_uclass.c   | 13 ++---
  lib/efi_loader/efi_boottime.c | 26 ++
  lib/efi_loader/efi_disk.c | 17 +
  5 files changed, 42 insertions(+), 35 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 8aa15a64c836..60b9e950ffdc 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -606,20 +606,6 @@ failure:
return ret;
  }

-/**
- * bootefi_run_finish() - finish up after running an EFI test
- *
- * @loaded_image_info: Pointer to a struct which holds the loaded image info
- * @image_obj: Pointer to a struct which holds the loaded image object
- */
-static void bootefi_run_finish(struct efi_loaded_image_obj *image_obj,
-  struct efi_loaded_image *loaded_image_info)
-{
-   efi_restore_gd();
-   free(loaded_image_info->load_options);
-   efi_delete_handle(_obj->header);
-}
-
  /**
   * do_efi_selftest() - execute EFI selftest
   *
@@ -638,7 +624,10 @@ static int do_efi_selftest(void)

/* Execute the test */
ret = EFI_CALL(efi_selftest(_obj->header, ));
-   bootefi_run_finish(image_obj, loaded_image_info);
+   efi_restore_gd();
+   free(loaded_image_info->load_options);
+   if (efi_delete_handle(_obj->header) != EFI_SUCCESS)
+   log_err("Failed to delete loaded image handle\n");

return ret != EFI_SUCCESS;
  }
diff --git a/include/efi_loader.h b/include/efi_loader.h
index e530bcf15f76..43ccf49abec4 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -618,7 +618,7 @@ void efi_add_handle(efi_handle_t obj);
  /* Create handle */
  efi_status_t efi_create_handle(efi_handle_t *handle);
  /* Delete handle */
-void efi_delete_handle(efi_handle_t obj);
+efi_status_t efi_delete_handle(efi_handle_t obj);
  /* Call this to validate a handle and find the EFI object for it */
  struct efi_object *efi_search_obj(const efi_handle_t handle);
  /* Locate device_path handle */
diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
index 45f935198874..d8755541fc14 100644
--- a/lib/efi_driver/efi_uclass.c
+++ b/lib/efi_driver/efi_uclass.c
@@ -285,10 +285,8 @@ static efi_status_t efi_add_driver(struct driver *drv)
bp->ops = ops;

ret = efi_create_handle(>bp.driver_binding_handle);
-   if (ret != EFI_SUCCESS) {
-   free(bp);
-   goto out;
-   }
+   if (ret != EFI_SUCCESS)
+   goto err;
bp->bp.image_handle = bp->bp.driver_binding_handle;
ret = efi_add_protocol(bp->bp.driver_binding_handle,
   _guid_driver_binding_protocol, bp);
@@ -299,11 +297,12 @@ static efi_status_t efi_add_driver(struct driver *drv)
if (ret != EFI_SUCCESS)
goto err;
}
-out:
-   return ret;

+   return ret;
  err:
-   efi_delete_handle(bp->bp.driver_binding_handle);
+   if (bp->bp.driver_binding_handle &&
+   efi_delete_handle(bp->bp.driver_binding_handle) != EFI_SUCCESS)
+   log_err("Failed to delete handle\n");


You already log errors in efi_delete_handle(), please, avoid  duplicate
log messages increasing code size (applies to several locations in this
patch).

Best regards

Heinrich


free(bp);
return ret;
  }
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index d75a3336e3f1..5123055d7f5d 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -59,6 +59,10 @@ static efi_handle_t current_image;
  static volatile gd_t *efi_gd, *app_gd;
  #endif

+static efi_status_t efi_uninstall_protocol
+   (efi_handle_t 

Re: [PATCH 2/4 v2] efi_loader: check the status of disconnected drivers

[Heinrich Schuchardt]

On 6/20/23 08:19, Ilias Apalodimas wrote:

efi_uninstall_protocol() calls efi_disconnect_all_drivers() but never
checks the return value.  Instead it tries to identify protocols that
are still open after closing the ones that were opened with
EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL, EFI_OPEN_PROTOCOL_GET_PROTOCOL
and EFI_OPEN_PROTOCOL_TEST_PROTOCOL.

Instead of doing that,  check the return value early and exit if
disconnecting the drivers failed.  Also reconnect all the drivers of
a handle if protocols are still found on the handle after disconnecting
controllers and closing the remaining protocols.

While at it fix a memory leak and properly free the opened protocol
information when closing a protocol.

Signed-off-by: Ilias Apalodimas 


Reviewed-by: Heinrich Schuchardt 

Re: [PATCH 1/4 v2] efi_loader: reconnect drivers on failure

[Heinrich Schuchardt]

On 6/20/23 08:19, Ilias Apalodimas wrote:

efi_disconnect_controller() doesn't reconnect drivers in case of
failure.  Reconnect the disconnected drivers properly

Signed-off-by: Ilias Apalodimas 


Reviewed-by: Heinrich Schuchardt 

Re: [PATCH v10 3/4] Boot var automatic management for removable medias

[Heinrich Schuchardt]

On 6/19/23 23:23, Raymond Mao wrote:

Changes for complying to EFI spec §3.5.1.1
'Removable Media Boot Behavior'.
Boot variables can be automatically generated during a removable
media is probed. At the same time, unused boot variables will be
detected and removed.

Please note that currently the function 'efi_disk_remove' has no
ability to distinguish below two scenarios
a) Unplugging of a removable media under U-Boot
b) U-Boot exiting and booting an OS
Thus currently the boot variables management is not added into
'efi_disk_remove' to avoid boot options being added/erased
repeatedly under scenario b) during power cycles
See TODO comments under function 'efi_disk_remove' for more details

Signed-off-by: Raymond Mao 
---
Changes in v3
- Split the patch into moving and renaming functions and
   individual patches for each changed functionality
Changes in v5
- Move function call of efi_bootmgr_update_media_device_boot_option()
   from efi_init_variables() to efi_init_obj_list()
Changes in v6
- Revert unrelated changes
Changes in v7
- adapt the return code of function
   efi_bootmgr_update_media_device_boot_option()
Changes in v8
- add a note in the commit message for future reference
Changes in v9
- amend the note text in the commit message
- add a TODO comment in 'efi_disk_remove'
Changes in v10
- fix typo and build failures with 'CONFIG_CMD_BOOTEFI_BOOTMGR=n'

  lib/efi_loader/efi_disk.c  | 18 ++
  lib/efi_loader/efi_setup.c |  7 +++
  2 files changed, 25 insertions(+)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index d2256713a8..911a4adfb1 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -687,6 +687,13 @@ int efi_disk_probe(void *ctx, struct event *event)
return -1;
}

+   /* only do the boot option management when UEFI sub-system is 
initialized */
+   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) && efi_obj_list_initialized 
== EFI_SUCCESS) {
+   ret = efi_bootmgr_update_media_device_boot_option();
+   if (ret != EFI_SUCCESS)
+   return -1;
+   }
+
return 0;
  }

@@ -773,6 +780,17 @@ int efi_disk_remove(void *ctx, struct event *event)
return efi_disk_delete_part(dev);
else
return 0;
+
+   /*
+* TODO A flag to distinguish below 2 different scenarios of this
+* function call is needed:
+* a) Unplugging of a removable media under U-Boot
+* b) U-Boot exiting and booting an OS
+* In case of scenario a), efi_bootmgr_update_media_device_boot_option()
+* needs to be invoked here to update the boot options and remove the
+* unnecessary ones.
+*/


As in future we want to integrate more EFI drivers with the driver model
such a status should be in a global variable. It will have to be set in
ExitBootServices() before invoking dm_remove_devices_flags().

Reviewed-by: Heinrich Schuchardt 


+
  }

  /**
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 58d4e13402..69c8b27730 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -245,6 +245,13 @@ efi_status_t efi_init_obj_list(void)
if (ret != EFI_SUCCESS)
goto out;

+   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
+   /* update boot option after variable service initialized */
+   ret = efi_bootmgr_update_media_device_boot_option();
+   if (ret != EFI_SUCCESS)
+   goto out;
+   }
+
/* Define supported languages */
ret = efi_init_platform_lang();
if (ret != EFI_SUCCESS)

Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type

[Heinrich Schuchardt]

On 6/16/23 10:28, Stefan Herbrechtsmeier wrote:

From: Malte Schmidt 

The data type of item_offset_list shall be UINT64 according to the UEFI [1]
specifications.

In include/efi_api.h the correct data type is used. The bug was probably
never noticed because of little endianness.

[1] https://uefi.org/specs/UEFI/2.10/index.html

Signed-off-by: Malte Schmidt 

Signed-off-by: Stefan Herbrechtsmeier 
---

  tools/eficapsule.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/eficapsule.h b/tools/eficapsule.h
index 753fb73313..2099a2e9b8 100644
--- a/tools/eficapsule.h
+++ b/tools/eficapsule.h
@@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header {
uint32_t version;
uint16_t embedded_driver_count;
uint16_t payload_item_count;
-   uint32_t item_offset_list[];
+   uint64_t item_offset_list[];


Defining the same structure in two places it bad practice.
https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/11

Reviewed-by: Heinrich Schuchardt 


  } __packed;

  /* image_capsule_support */