zephyr: drop programmatic generation of binman device-tree input

Move the binman device-tree input into the Zephyr build system's DTS,
which allows overriding parts of the binman settings on a
board/project specific basis using device-tree.

In the process, we need to move the flash offset and size to
device-tree instead of Kconfig, because Kconfig is not including
during device-tree preprocessing.  This is overall, a net-good change,
as this should never been Kconfig in the first place.  This assumes
the existing Kconfig defaults of 512 KB of flash with 256 KB sections,
however can still be overridden via device-tree.

BUG=b:184388037
BRANCH=none
TEST=zmake testall
     manually investigate the flash output for lazor using dump_fmap

Signed-off-by: Jack Rosenthal <jrosenth@chromium.org>
Change-Id: I424ea15454d5cd02947642116673bebcad3406cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2860987
Reviewed-by: Simon Glass <sjg@chromium.org>

9 files changed, 120 insertions, 137 deletions

diff --git a/zephyr/Kconfig.flash b/zephyr/Kconfig.flash
index e79732d928..89f8c10341 100644
--- a/zephyr/Kconfig.flash
+++ b/zephyr/Kconfig.flash
@@ -14,36 +14,6 @@ config PLATFORM_EC_SPI_FLASH_REGS
 	  registers, and (2) the inverse function to set the status registers
 	  based on the desired protection offset/length.
 
-config PLATFORM_EC_PROTECTED_STORAGE_OFF
-	hex "Position of the RO image in Flash memory"
-	default 0x0
-	help
-	  Sets the position in flash memory where the RO image begins. This is
-	  the address that will be used to start copying the image into RAM.
-
-config PLATFORM_EC_PROTECTED_STORAGE_SIZE
-	hex "Size of the RO image in Flash memory"
-	default 0x40000
-	help
-	  The total size of the RO image in flash memory. This will dictate the
-	  ending position of the RO image being copied into RAM when combined
-	  with PLATFORM_EC_PROTECTED_STORAGE_OFF.
-
-config PLATFORM_EC_WRITABLE_STORAGE_OFF
-	hex "Position of the RW image in Flash memory"
-	default 0x40000
-	help
-	  Sets the position in flash memory where the RW image begins. This is
-	  the address that will be used to start copying the image into RAM.
-
-config PLATFORM_EC_WRITABLE_STORAGE_SIZE
-	hex "Size of the RW image in Flash memory"
-	default 0x40000
-	help
-	  The total size of the RW image in flash memory. This will dictate the
-	  ending position of the RW image being copied into RAM when combined
-	  with PLATFORM_EC_WRITABLE_STORAGE_OFF.
-
 config PLATFORM_EC_CONSOLE_CMD_CHARGEN
 	bool "Console command: chargen"
 	depends on UART_INTERRUPT_DRIVEN
diff --git a/zephyr/dts/bindings/cros_flash/cros-ec,flash-layout.yaml b/zephyr/dts/bindings/cros_flash/cros-ec,flash-layout.yaml
new file mode 100644
index 0000000000..991f3c71cf
--- /dev/null
+++ b/zephyr/dts/bindings/cros_flash/cros-ec,flash-layout.yaml
@@ -0,0 +1,39 @@
+# Copyright 2021 The Chromium OS Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+description: Describes the size and offset of the RO and RW regions
+
+compatible: "cros-ec,flash-layout"
+
+include: base.yaml
+
+properties:
+  type:
+    type: string
+    required: true
+    description: >-
+      A property required by the binman utility, defined here only to
+      prevent Zephyr's binding generator from erroring about
+      additional properties.
+
+      The property should be set to the value "section" (done by
+      binman.dtsi) and should not be used anywhere in C code.
+
+      See README.entries in binman for a description of what binman
+      uses this for.
+  offset:
+    type: int
+    required: true
+    description: >-
+      The offset of the flash region from the base of the flash.
+  size:
+    type: int
+    required: true
+    description: >-
+      The size of the flash region, in bytes.
+  read-only:
+    type: boolean
+    required: false
+    description: >-
+      Set if the section should be read-only.
diff --git a/zephyr/include/cros/binman.dtsi b/zephyr/include/cros/binman.dtsi
new file mode 100644
index 0000000000..82bb801e6c
--- /dev/null
+++ b/zephyr/include/cros/binman.dtsi
@@ -0,0 +1,51 @@
+/* Copyright 2021 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	binman {
+		filename = "zephyr.bin";
+		pad-byte = <0x1d>;
+		wp-ro {
+			compatible = "cros-ec,flash-layout";
+			type = "section";
+			offset = <0x0>;
+			size = <0x40000>;
+			read-only;
+			ec-ro {
+				type = "section";
+				ro-fw {
+					type = "blob";
+					filename = "zephyr_ro.bin";
+				};
+				fmap {
+				};
+				ro-frid {
+					type = "text";
+					size = <32>;
+					text-label = "version";
+				};
+			};
+		};
+		ec-rw {
+			compatible = "cros-ec,flash-layout";
+			type = "section";
+			offset = <0x40000>;
+			size = <0x40000>;
+			rw-fw {
+				type = "blob";
+				filename = "zephyr_rw.bin";
+			};
+			rw-fwid {
+				type = "text";
+				size = <32>;
+				text-label = "version";
+			};
+		};
+	};
+};
diff --git a/zephyr/include/cros/ite/it8xxx2.dtsi b/zephyr/include/cros/ite/it8xxx2.dtsi
index 9072b36424..12ba51388d 100644
--- a/zephyr/include/cros/ite/it8xxx2.dtsi
+++ b/zephyr/include/cros/ite/it8xxx2.dtsi
@@ -3,6 +3,8 @@
  * found in the LICENSE file.
  */
 
+#include <cros/binman.dtsi>
+
 / {
 	named-bbram-regions {
 		compatible = "named-bbram-regions";
diff --git a/zephyr/include/cros/nuvoton/npcx.dtsi b/zephyr/include/cros/nuvoton/npcx.dtsi
index 8251a34ade..e31b4c1b7c 100644
--- a/zephyr/include/cros/nuvoton/npcx.dtsi
+++ b/zephyr/include/cros/nuvoton/npcx.dtsi
@@ -6,6 +6,7 @@
 
 /dts-v1/;
 
+#include <cros/binman.dtsi>
 #include <nuvoton/npcx7m6fb.dtsi>
 
 / {
diff --git a/zephyr/projects/asurada/boards/riscv/asurada/asurada_defconfig b/zephyr/projects/asurada/boards/riscv/asurada/asurada_defconfig
index 57adfb0400..7c7c732e71 100644
--- a/zephyr/projects/asurada/boards/riscv/asurada/asurada_defconfig
+++ b/zephyr/projects/asurada/boards/riscv/asurada/asurada_defconfig
@@ -43,8 +43,3 @@ CONFIG_CROS_EC_RO_MEM_OFF=0x0
 CONFIG_CROS_EC_RO_SIZE=0x40000
 CONFIG_CROS_EC_RW_MEM_OFF=0x40000
 CONFIG_CROS_EC_RW_SIZE=0x40000
-
-CONFIG_PLATFORM_EC_PROTECTED_STORAGE_OFF=0x0
-CONFIG_PLATFORM_EC_PROTECTED_STORAGE_SIZE=0x40000
-CONFIG_PLATFORM_EC_WRITABLE_STORAGE_OFF=0x40000
-CONFIG_PLATFORM_EC_WRITABLE_STORAGE_SIZE=0x40000
diff --git a/zephyr/projects/volteer/volteer/prj.conf b/zephyr/projects/volteer/volteer/prj.conf
index ed3255049d..0726a58f84 100644
--- a/zephyr/projects/volteer/volteer/prj.conf
+++ b/zephyr/projects/volteer/volteer/prj.conf
@@ -143,10 +143,6 @@ CONFIG_PLATFORM_EC_CONSOLE_CMD_PPC_DUMP=n
 CONFIG_PLATFORM_EC_CONSOLE_CMD_TCPC_DUMP=n
 
 # Flash
-CONFIG_PLATFORM_EC_PROTECTED_STORAGE_OFF=0x0
-CONFIG_PLATFORM_EC_PROTECTED_STORAGE_SIZE=0x40000
-CONFIG_PLATFORM_EC_WRITABLE_STORAGE_OFF=0x40000
-CONFIG_PLATFORM_EC_WRITABLE_STORAGE_SIZE=0x40000
 CONFIG_PLATFORM_EC_CONSOLE_CMD_FLASH=n
 
 # RTC
diff --git a/zephyr/shim/include/config_chip.h b/zephyr/shim/include/config_chip.h
index 8dece3a623..4d732e7331 100644
--- a/zephyr/shim/include/config_chip.h
+++ b/zephyr/shim/include/config_chip.h
@@ -302,11 +302,15 @@
 #define CONFIG_FLASH_WRITE_IDEAL_SIZE	256   /* one page size for write */
 #define CONFIG_FLASH_ERASE_SIZE	0x10000
 #define CONFIG_FLASH_BANK_SIZE		CONFIG_FLASH_ERASE_SIZE
-#define CONFIG_EC_PROTECTED_STORAGE_OFF CONFIG_PLATFORM_EC_PROTECTED_STORAGE_OFF
-#define CONFIG_EC_PROTECTED_STORAGE_SIZE \
-	CONFIG_PLATFORM_EC_PROTECTED_STORAGE_SIZE
-#define CONFIG_EC_WRITABLE_STORAGE_OFF CONFIG_PLATFORM_EC_WRITABLE_STORAGE_OFF
-#define CONFIG_EC_WRITABLE_STORAGE_SIZE CONFIG_PLATFORM_EC_WRITABLE_STORAGE_SIZE
+
+/* Internal, don't use outside this header */
+#define _BINMAN_RO_PATH DT_PATH(binman, wp_ro)
+#define _BINMAN_RW_PATH DT_PATH(binman, ec_rw)
+
+#define CONFIG_EC_PROTECTED_STORAGE_OFF DT_PROP(_BINMAN_RO_PATH, size)
+#define CONFIG_EC_PROTECTED_STORAGE_SIZE DT_PROP(_BINMAN_RO_PATH, offset)
+#define CONFIG_EC_WRITABLE_STORAGE_OFF DT_PROP(_BINMAN_RW_PATH, size)
+#define CONFIG_EC_WRITABLE_STORAGE_SIZE DT_PROP(_BINMAN_RW_PATH, offset)
 
 /* RO image resides at start of protected region, right after header */
 #define CONFIG_RO_STORAGE_OFF	CONFIG_RO_HDR_SIZE
diff --git a/zephyr/zmake/zmake/output_packers.py b/zephyr/zmake/zmake/output_packers.py
index 56d74b03d5..bb3b53a45f 100644
--- a/zephyr/zmake/zmake/output_packers.py
+++ b/zephyr/zmake/zmake/output_packers.py
@@ -3,7 +3,7 @@
 # found in the LICENSE file.
 """Types which provide many builds and composite them into a single binary."""
 import logging
-import pathlib
+import shutil
 import subprocess
 
 import zmake.build_config as build_config
@@ -11,71 +11,6 @@ import zmake.multiproc
 import zmake.util as util
 
 
-def _write_dts_file(dts_file, config_header, output_bin, ro_filename, rw_filename):
-    """Generate the .dts file used for binman.
-
-    Args:
-        dts_file: The dts file to write to.
-        config_header: The full path to the generated autoconf.h header.
-        output_bin: The full path to the binary that binman should output.
-        ro_filename: The RO image file name.
-        rw_filename: The RW image file name.
-
-    Returns:
-        The path to the .dts file that was generated.
-    """
-    dts_file.write("""
-    /dts-v1/;
-    #include "{config_header}"
-    / {{
-      #address-cells = <1>;
-      #size-cells = <1>;
-      binman {{
-        filename = "{output_bin}";
-        pad-byte = <0x1d>;
-        WP_RO {{
-          type = "section";
-          read-only;
-          offset = <CONFIG_PLATFORM_EC_PROTECTED_STORAGE_OFF>;
-          size = <CONFIG_PLATFORM_EC_PROTECTED_STORAGE_SIZE>;
-          EC_RO {{
-            type = "section";
-            RO_FW {{
-              type = "blob";
-              filename = "{ro_filename}";
-            }};
-            fmap {{
-            }};
-            RO_FRID {{
-              type = "text";
-              size = <32>;
-              text-label = "version";
-            }};
-          }};
-        }};
-        EC_RW {{
-          type = "section";
-          offset = <CONFIG_PLATFORM_EC_WRITABLE_STORAGE_OFF>;
-          size = <CONFIG_PLATFORM_EC_WRITABLE_STORAGE_SIZE>;
-          RW_FW {{
-            type = "blob";
-            filename = "{rw_filename}";
-          }};
-          RW_FWID {{
-            type = "text";
-            size = <32>;
-            text-label = "version";
-          }};
-        }};
-      }};
-    }};""".format(
-        output_bin=output_bin,
-        config_header=config_header,
-        ro_filename=ro_filename,
-        rw_filename=rw_filename
-    ))
-
-
 class BasePacker:
     """Abstract base for all packers."""
     def __init__(self, project):
@@ -141,19 +76,10 @@ class NpcxPacker(BasePacker):
         yield 'rw', build_config.BuildConfig(kconfig_defs={'CONFIG_CROS_EC_RW': 'y'})
 
     def pack_firmware(self, work_dir, jobclient, ro, rw, version_string=""):
-        """Pack the 'raw' binary.
-
-        This combines the RO and RW images as specified in the Kconfig file for
-        the project. For this function to work, the following config values must
-        be defined:
-        * CONFIG_CROS_EC_RO_MEM_OFF - The offset in bytes of the RO image from
-          the start of the resulting binary.
-        * CONFIG_CROS_EC_RO_SIZE - The maximum allowed size (in bytes) of the RO
-          image.
-        * CONFIG_CROS_EC_RW_MEM_OFF - The offset in bytes of the RW image from
-          the start of the resulting binary (must be >= RO_MEM_OFF + RO_SIZE).
-        * CONFIG_CROS_EC_RW_SIZE - The maximum allowed size (in bytes) of the RW
-           image.
+        """Pack RO and RW sections using Binman.
+
+        Binman configuration is expected to be found in the RO build
+        device-tree configuration.
 
         Args:
             work_dir: The directory used for packing.
@@ -162,20 +88,18 @@ class NpcxPacker(BasePacker):
             rw: Directory containing the RW image build.
             version_string: The version string to use in FRID/FWID.
 
-        Returns:
-            Tuple mapping the resulting .bin file to the output filename.
+        Yields:
+            2-tuples of the path of each file in the work_dir that
+            should be copied into the output directory, and the output
+            filename.
         """
-        work_dir = pathlib.Path(work_dir).resolve()
-        ro = pathlib.Path(ro).resolve()
-        rw = pathlib.Path(rw).resolve()
-        dts_file_path = work_dir / 'project.dts'
-        with open(dts_file_path, 'w+') as dts_file:
-            _write_dts_file(
-                dts_file=dts_file,
-                config_header=ro / 'zephyr' / 'include' / 'generated' / 'autoconf.h',
-                output_bin=work_dir / 'zephyr.bin',
-                ro_filename=ro / 'zephyr' / 'zephyr.packed.bin',
-                rw_filename=rw / 'zephyr' / 'zephyr.bin')
+        dts_file_path = ro / 'zephyr' / 'zephyr.dts'
+
+        # Copy the inputs into the work directory so that Binman can
+        # find them under a hard-coded name.
+        shutil.copy2(ro / 'zephyr' / 'zephyr.packed.bin',
+                     work_dir / 'zephyr_ro.bin')
+        shutil.copy2(rw / 'zephyr' / 'zephyr.bin', work_dir / 'zephyr_rw.bin')
 
         # Version in FRID/FWID can be at most 31 bytes long (32, minus
         # one for null character).
@@ -185,7 +109,8 @@ class NpcxPacker(BasePacker):
         proc = jobclient.popen(
             ['binman', '-v', '5', 'build',
              '-a', 'version={}'.format(version_string),
-             '-d', dts_file_path, '-m'],
+             '-d', dts_file_path, '-m', '-O', work_dir],
+            cwd=work_dir,
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
             encoding='utf-8')