From ba25f13997b53306a586fc58d2dc41f64d9d35b2 Mon Sep 17 00:00:00 2001 From: Vladimir Smitka Date: Fri, 4 Sep 2026 05:11:07 +0000 Subject: [PATCH] zephyr-cp: check that a board's layout matches its non-Zephyr build Boards that CircuitPython also builds from ports/raspberrypi or ports/nordic must keep nvm and the CIRCUITPY drive exactly where that build has them, or switching firmware between the two loses the user's data. Until now the only check was a person with the board in hand; #11272 found a 4 KB nvm offset error that way. check_partitions.py now reads a `counterpart = "/"` key from the Zephyr board's circuitpython.toml, derives the expected placement from the counterpart's own mpconfigboard.mk/.h and mpconfigport.h the way those files derive it (raspberrypi: nvm after CIRCUITPY_FIRMWARE_SIZE, the drive after nvm and the saves partition; nordic: bootloader at the top of flash, internal filesystem and nvm below it, or the whole external chip as the drive) and compares it with the resolved devicetree. The eight boards with a counterpart declare it. On the current tree seven match; feather_nrf52840_sense_zephyr puts nvm at 0xf2000+0x2000 while ports/nordic/feather_bluefruit_sense has it at 0xf3000+0x1000 (CIRCUITPY_INTERNAL_NVM_SIZE 4096). That is fixed separately. Also adds --board for build directories not named build-, and tests for the reference derivation against the real board files. --- .../clue_nrf52840_zephyr/circuitpython.toml | 3 + .../circuitpython.toml | 3 + .../circuitpython.toml | 3 + .../feather_rp2040_zephyr/circuitpython.toml | 3 + .../rpi_pico2_w_zephyr/circuitpython.toml | 3 + .../rpi_pico2_zephyr/circuitpython.toml | 3 + .../rpi_pico_w_zephyr/circuitpython.toml | 3 + .../rpi_pico_zephyr/circuitpython.toml | 3 + ports/zephyr-cp/cptools/check_partitions.py | 218 +++++++++++++++++- .../cptools/tests/test_check_partitions.py | 106 ++++++++- 10 files changed, 346 insertions(+), 2 deletions(-) diff --git a/ports/zephyr-cp/boards/adafruit/clue_nrf52840_zephyr/circuitpython.toml b/ports/zephyr-cp/boards/adafruit/clue_nrf52840_zephyr/circuitpython.toml index 2d7b8c3db48..4af0c99d878 100644 --- a/ports/zephyr-cp/boards/adafruit/clue_nrf52840_zephyr/circuitpython.toml +++ b/ports/zephyr-cp/boards/adafruit/clue_nrf52840_zephyr/circuitpython.toml @@ -2,3 +2,6 @@ CIRCUITPY_BUILD_EXTENSIONS = ["hex", "uf2"] USB_VID=0x239A USB_PID=0x8072 NAME="CLUE nRF52840 Express" + +# Non-Zephyr build of the same board; nvm and CIRCUITPY must sit where it puts them. +counterpart = "nordic/clue_nrf52840_express" diff --git a/ports/zephyr-cp/boards/adafruit/feather_nrf52840_sense_zephyr/circuitpython.toml b/ports/zephyr-cp/boards/adafruit/feather_nrf52840_sense_zephyr/circuitpython.toml index eacb0f9607d..5dfa12f79df 100644 --- a/ports/zephyr-cp/boards/adafruit/feather_nrf52840_sense_zephyr/circuitpython.toml +++ b/ports/zephyr-cp/boards/adafruit/feather_nrf52840_sense_zephyr/circuitpython.toml @@ -2,3 +2,6 @@ CIRCUITPY_BUILD_EXTENSIONS = ["elf", "uf2"] USB_VID=0x239A USB_PID=0x8088 NAME="Feather Bluefruit Sense" + +# Non-Zephyr build of the same board; nvm and CIRCUITPY must sit where it puts them. +counterpart = "nordic/feather_bluefruit_sense" diff --git a/ports/zephyr-cp/boards/adafruit/feather_nrf52840_zephyr/circuitpython.toml b/ports/zephyr-cp/boards/adafruit/feather_nrf52840_zephyr/circuitpython.toml index c4d1099a77e..8f97aca1d3d 100644 --- a/ports/zephyr-cp/boards/adafruit/feather_nrf52840_zephyr/circuitpython.toml +++ b/ports/zephyr-cp/boards/adafruit/feather_nrf52840_zephyr/circuitpython.toml @@ -2,3 +2,6 @@ CIRCUITPY_BUILD_EXTENSIONS = ["elf", "uf2"] USB_VID=0x239A USB_PID=0x802A NAME="Feather nRF52840 Express" + +# Non-Zephyr build of the same board; nvm and CIRCUITPY must sit where it puts them. +counterpart = "nordic/feather_nrf52840_express" diff --git a/ports/zephyr-cp/boards/adafruit/feather_rp2040_zephyr/circuitpython.toml b/ports/zephyr-cp/boards/adafruit/feather_rp2040_zephyr/circuitpython.toml index 9d3c229ed1b..4e992a212f4 100644 --- a/ports/zephyr-cp/boards/adafruit/feather_rp2040_zephyr/circuitpython.toml +++ b/ports/zephyr-cp/boards/adafruit/feather_rp2040_zephyr/circuitpython.toml @@ -1 +1,4 @@ CIRCUITPY_BUILD_EXTENSIONS = ["elf", "uf2"] + +# Non-Zephyr build of the same board; nvm and CIRCUITPY must sit where it puts them. +counterpart = "raspberrypi/adafruit_feather_rp2040" diff --git a/ports/zephyr-cp/boards/raspberrypi/rpi_pico2_w_zephyr/circuitpython.toml b/ports/zephyr-cp/boards/raspberrypi/rpi_pico2_w_zephyr/circuitpython.toml index 0f901d1149e..c042e1581c0 100644 --- a/ports/zephyr-cp/boards/raspberrypi/rpi_pico2_w_zephyr/circuitpython.toml +++ b/ports/zephyr-cp/boards/raspberrypi/rpi_pico2_w_zephyr/circuitpython.toml @@ -1,2 +1,5 @@ CIRCUITPY_BUILD_EXTENSIONS = ["elf", "uf2"] BLOBS=["hal_infineon"] + +# Non-Zephyr build of the same board; nvm and CIRCUITPY must sit where it puts them. +counterpart = "raspberrypi/raspberry_pi_pico2_w" diff --git a/ports/zephyr-cp/boards/raspberrypi/rpi_pico2_zephyr/circuitpython.toml b/ports/zephyr-cp/boards/raspberrypi/rpi_pico2_zephyr/circuitpython.toml index 9d3c229ed1b..75ceb6ba60d 100644 --- a/ports/zephyr-cp/boards/raspberrypi/rpi_pico2_zephyr/circuitpython.toml +++ b/ports/zephyr-cp/boards/raspberrypi/rpi_pico2_zephyr/circuitpython.toml @@ -1 +1,4 @@ CIRCUITPY_BUILD_EXTENSIONS = ["elf", "uf2"] + +# Non-Zephyr build of the same board; nvm and CIRCUITPY must sit where it puts them. +counterpart = "raspberrypi/raspberry_pi_pico2" diff --git a/ports/zephyr-cp/boards/raspberrypi/rpi_pico_w_zephyr/circuitpython.toml b/ports/zephyr-cp/boards/raspberrypi/rpi_pico_w_zephyr/circuitpython.toml index 0f901d1149e..deb99e3effa 100644 --- a/ports/zephyr-cp/boards/raspberrypi/rpi_pico_w_zephyr/circuitpython.toml +++ b/ports/zephyr-cp/boards/raspberrypi/rpi_pico_w_zephyr/circuitpython.toml @@ -1,2 +1,5 @@ CIRCUITPY_BUILD_EXTENSIONS = ["elf", "uf2"] BLOBS=["hal_infineon"] + +# Non-Zephyr build of the same board; nvm and CIRCUITPY must sit where it puts them. +counterpart = "raspberrypi/raspberry_pi_pico_w" diff --git a/ports/zephyr-cp/boards/raspberrypi/rpi_pico_zephyr/circuitpython.toml b/ports/zephyr-cp/boards/raspberrypi/rpi_pico_zephyr/circuitpython.toml index 9d3c229ed1b..fae9877df08 100644 --- a/ports/zephyr-cp/boards/raspberrypi/rpi_pico_zephyr/circuitpython.toml +++ b/ports/zephyr-cp/boards/raspberrypi/rpi_pico_zephyr/circuitpython.toml @@ -1 +1,4 @@ CIRCUITPY_BUILD_EXTENSIONS = ["elf", "uf2"] + +# Non-Zephyr build of the same board; nvm and CIRCUITPY must sit where it puts them. +counterpart = "raspberrypi/raspberry_pi_pico" diff --git a/ports/zephyr-cp/cptools/check_partitions.py b/ports/zephyr-cp/cptools/check_partitions.py index 4d4769ec95f..05fb982f4a1 100644 --- a/ports/zephyr-cp/cptools/check_partitions.py +++ b/ports/zephyr-cp/cptools/check_partitions.py @@ -11,12 +11,20 @@ 0x10000100, so a partition left at 0x100 silently turns it off and the UF2 is built for the wrong address with no second stage bootloader linked in. +A board that CircuitPython also builds from another port (``counterpart`` in its +``circuitpython.toml``) is additionally compared with that build: nvm and the +CIRCUITPY drive must sit exactly where ``ports/`` puts them, or switching +firmware between the two builds loses the user's data. The expected placement +is derived from the counterpart's configuration files the same way its +``mpconfigport.h`` derives it. + This reads the ``edt.pickle`` a build already produced, so it costs no build time -- point it at build directories after building, or run it with no arguments to check every build directory in the port. python cptools/check_partitions.py # every build-* dir python cptools/check_partitions.py build-raspberrypi_rpi_pico_zephyr + python cptools/check_partitions.py --board raspberrypi_rpi_pico_zephyr build-x Exits non-zero when a layout has problems. """ @@ -24,11 +32,31 @@ import argparse import pathlib import pickle +import re import sys +import tomllib + +import board_tools PORT_DIR = pathlib.Path(__file__).resolve().parent.parent +TOP = PORT_DIR.parent.parent EDT_MODULE = PORT_DIR / "zephyr" / "scripts" / "dts" / "python-devicetree" / "src" +# Parity with the non-Zephyr build. A board that CircuitPython also builds from another +# port (ports/raspberrypi, ports/nordic) names it in circuitpython.toml: +# +# counterpart = "raspberrypi/raspberry_pi_pico_w" +# +# The nvm and CIRCUITPY drive of the Zephyr build then have to sit exactly where that +# build puts them, so that switching firmware between the two never loses user data. +# The expected placement is derived from the counterpart's own configuration files, the +# same way its mpconfigport.h derives it, rather than copied into the overlay by hand. +DEFINE_RE = re.compile(r"^\s*#\s*define\s+([A-Za-z_]\w*)\s+(.+?)\s*(?://.*|/\*.*)?$", re.MULTILINE) +CFLAG_DEFINE_RE = re.compile(r"-D([A-Za-z_]\w*)=(?:'([^']*)'|\"([^\"]*)\"|(\S+))") +MK_ASSIGN_RE = re.compile(r"^\s*([A-Za-z_]\w*)\s*[?:+]?=\s*(.*?)\s*$", re.MULTILINE) +IDENTIFIER_RE = re.compile(r"(?]+$") + def load_edt(build_dir): """Load the pickled devicetree a build produced, or None if there is none.""" @@ -165,6 +193,183 @@ def check_layout(edt): return problems +def read_defines(paths): + """Collect ``#define``s, ``-D`` flags and make assignments from configuration files. + + The first definition of a name wins, and files earlier in ``paths`` take + precedence, so list the board's files before the port's defaults. + """ + defines = {} + for path in paths: + if not path.is_file(): + continue + text = path.read_text() + if path.suffix == ".mk": + for match in CFLAG_DEFINE_RE.finditer(text): + value = next(v for v in match.groups()[1:] if v is not None) + defines.setdefault(match.group(1), value) + for match in MK_ASSIGN_RE.finditer(text): + defines.setdefault(match.group(1), match.group(2)) + else: + for match in DEFINE_RE.finditer(text): + defines.setdefault(match.group(1), match.group(2)) + return defines + + +def evaluate(defines, name, default=None): + """Integer value of a define, following references to other defines. + + Returns ``default`` when the name is undefined and None when it cannot be + evaluated, so that a layout is never declared correct on a guess. + """ + expr = defines.get(name) + if expr is None: + return default + for ident in sorted(set(IDENTIFIER_RE.findall(expr)), key=len, reverse=True): + value = evaluate(defines, ident) + if value is None: + return None + expr = re.sub(rf"(?/`` under ``ports/`` and ``flash_size`` the + size of the internal flash. Returns ``{"nvm": (offset, size), "circuitpy": + (offset, size)}``; ``"circuitpy"`` is the string ``"external"`` when that + build uses a whole external flash chip as the drive. Raises ValueError when + the port's rules are unknown or a value cannot be derived. + """ + port, _, board = counterpart.partition("/") + port_dir = TOP / "ports" / port + board_dir = port_dir / "boards" / board + if not board_dir.is_dir(): + raise ValueError(f"{counterpart}: no such board under ports/") + defines = read_defines( + [ + board_dir / "mpconfigboard.mk", + board_dir / "mpconfigboard.h", + port_dir / "mpconfigport.mk", + port_dir / "mpconfigport.h", + ] + ) + + def value(name, default=None): + result = evaluate(defines, name, default) + if result is None: + raise ValueError(f"{counterpart}: cannot evaluate {name}") + return result + + if port == "raspberrypi": + # ports/raspberrypi/mpconfigport.h: nvm directly follows the firmware region, + # the drive follows nvm (and the saves partition on boards that have one). + firmware = value("CIRCUITPY_FIRMWARE_SIZE") + nvm_size = value("CIRCUITPY_INTERNAL_NVM_SIZE") + drive = firmware + nvm_size + value("CIRCUITPY_SAVES_PARTITION_SIZE", 0) + return {"nvm": (firmware, nvm_size), "circuitpy": (drive, flash_size - drive)} + if port == "nordic": + # ports/nordic/mpconfigport.h: the bootloader sits at the top of flash, an + # internal filesystem (when there is one) directly below it and nvm below that. + bootloader = ( + flash_size + - value("BOOTLOADER_SIZE") + - value("BOOTLOADER_SETTINGS_SIZE") + - value("BOOTLOADER_MBR_SIZE") + ) + external = any( + defines.get(flag, "0").strip() == "1" + for flag in ("QSPI_FLASH_FILESYSTEM", "SPI_FLASH_FILESYSTEM") + ) + fs_size = 0 if external else value("CIRCUITPY_INTERNAL_FLASH_FILESYSTEM_SIZE") + fs_start = bootloader - fs_size + nvm_size = value("CIRCUITPY_INTERNAL_NVM_SIZE") + return { + "nvm": (fs_start - nvm_size, nvm_size), + "circuitpy": "external" if external else (fs_start, fs_size), + } + raise ValueError(f"{counterpart}: no layout rules known for port {port}") + + +def is_internal_flash(node): + return "soc-nv-flash" in getattr(node, "compats", []) + + +def is_external_flash(node): + return node.props.get("size") is not None and any( + "nor" in compat for compat in getattr(node, "compats", []) + ) + + +def check_parity(edt, counterpart): + """Return problems where the layout differs from the non-Zephyr ``counterpart``.""" + internal = [node for node in edt.nodes if is_internal_flash(node)] + if not internal: + return [f"no soc-nv-flash device to compare with {counterpart}"] + try: + expected = reference_layout(counterpart, device_size(internal[0])) + except ValueError as e: + return [str(e)] + + partitions = {} + for node, dev, offset, size, mapped in iter_partitions(edt): + label = node.labels[0] if node.labels else node.name + partitions[label] = (dev, offset, size) + + problems = [] + + def compare(label, want): + found = partitions.get(label) + if found is None: + problems.append( + f"{label}: missing, {counterpart} has it at 0x{want[0]:x}+0x{want[1]:x}" + ) + return + dev, offset, size = found + if not is_internal_flash(dev): + problems.append(f"{label}: not on the internal flash, unlike {counterpart}") + if (offset, size) != want: + problems.append( + f"{label}: 0x{offset:x}+0x{size:x}, {counterpart} has it at " + f"0x{want[0]:x}+0x{want[1]:x}" + ) + + compare("nvm_partition", expected["nvm"]) + if expected["circuitpy"] == "external": + # supervisor/flash.c uses the first flash device no partition covers as the + # drive, which is the whole chip only while the chip carries no partitions. + if "circuitpy_partition" in partitions: + problems.append( + f"circuitpy_partition: declared, but {counterpart} uses the whole " + f"external flash as the drive" + ) + external = [node for node in edt.nodes if is_external_flash(node)] + if not external: + problems.append(f"no external flash, but {counterpart} keeps the drive on one") + for dev, offset, size in partitions.values(): + if is_external_flash(dev): + problems.append( + f"{dev.labels[0] if dev.labels else dev.name}: carries partitions, " + f"but {counterpart} uses the whole chip as the drive" + ) + break + else: + compare("circuitpy_partition", expected["circuitpy"]) + return problems + + +def counterpart_of(board_id): + """The ``counterpart`` a board declares in its circuitpython.toml, if any.""" + toml_path = board_tools.find_mpconfigboard(PORT_DIR, board_id) if board_id else None + if toml_path is None: + return None + with toml_path.open("rb") as f: + return tomllib.load(f).get("counterpart") + + def main(): parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) parser.add_argument( @@ -172,6 +377,10 @@ def main(): nargs="*", help="Build directories to check (default: every build-* in the port)", ) + parser.add_argument( + "--board", + help="Board built in the (single) build directory; default: taken from its name", + ) args = parser.parse_args() dirs = [pathlib.Path(d) for d in args.build_dirs] @@ -180,6 +389,8 @@ def main(): if not dirs: print("No build directories found; build a board first.", file=sys.stderr) return 1 + if args.board and len(dirs) != 1: + parser.error("--board applies to exactly one build directory") failed = [] checked = 0 @@ -190,6 +401,10 @@ def main(): checked += 1 n_parts = sum(1 for _ in iter_partitions(edt)) problems = check_layout(edt) + board = args.board or build_dir.name.removeprefix("build-") + counterpart = counterpart_of(board) + if counterpart: + problems += check_parity(edt, counterpart) if problems: failed.append(build_dir.name) print(f"{build_dir.name}:") @@ -199,7 +414,8 @@ def main(): # Say how much was inspected: a board whose overlay defines no # partitions at all would otherwise be indistinguishable from a # verified-good one. - print(f"{build_dir.name}: ok ({n_parts} partitions checked)") + parity = f", matches ports/{counterpart}" if counterpart else "" + print(f"{build_dir.name}: ok ({n_parts} partitions checked{parity})") if not checked: print("No build directory held an edt.pickle; build a board first.", file=sys.stderr) diff --git a/ports/zephyr-cp/cptools/tests/test_check_partitions.py b/ports/zephyr-cp/cptools/tests/test_check_partitions.py index 12a35ba6ea9..e8f9d470662 100644 --- a/ports/zephyr-cp/cptools/tests/test_check_partitions.py +++ b/ports/zephyr-cp/cptools/tests/test_check_partitions.py @@ -18,7 +18,15 @@ sys.path.insert(0, str(pathlib.Path(__file__).parent.parent)) -from check_partitions import check_layout, device_size, iter_partitions # noqa: E402 +from check_partitions import ( # noqa: E402 + check_layout, + check_parity, + device_size, + evaluate, + iter_partitions, + read_defines, + reference_layout, +) BINDINGS = [str(portdir / "zephyr/dts/bindings")] @@ -292,3 +300,99 @@ def test_no_partitions_yields_nothing_checked(self): if __name__ == "__main__": sys.exit(pytest.main([__file__])) + + +class TestParity: + """Agreement with the non-Zephyr build of the same board. + + The reference values are what ports/raspberrypi/mpconfigport.h and + ports/nordic/mpconfigport.h derive for these boards; the tests read the real + board files so a change there is caught here. + """ + + def test_evaluate_follows_references(self): + defines = {"A": "(2 * B)", "B": "(512 * 1024)", "C": "A + UNKNOWN"} + assert evaluate(defines, "A") == 1024 * 1024 + assert evaluate(defines, "B") == 512 * 1024 + assert evaluate(defines, "C") is None + assert evaluate(defines, "MISSING", 7) == 7 + + def test_read_defines_precedence(self, tmp_path): + mk = tmp_path / "mpconfigboard.mk" + mk.write_text( + "CFLAGS += -DCIRCUITPY_FIRMWARE_SIZE='(1536 * 1024)'\nQSPI_FLASH_FILESYSTEM = 1\n" + ) + header = tmp_path / "mpconfigport.h" + header.write_text( + "#ifndef CIRCUITPY_FIRMWARE_SIZE\n#define CIRCUITPY_FIRMWARE_SIZE (1020 * 1024)\n#endif\n" + ) + defines = read_defines([mk, header]) + assert evaluate(defines, "CIRCUITPY_FIRMWARE_SIZE") == 1536 * 1024 + assert defines["QSPI_FLASH_FILESYSTEM"] == "1" + + def test_raspberrypi_default_firmware_size(self): + layout = reference_layout("raspberrypi/raspberry_pi_pico", 0x200000) + assert layout["nvm"] == (0xFF000, 0x1000) + assert layout["circuitpy"] == (0x100000, 0x100000) + + def test_raspberrypi_board_firmware_size(self): + layout = reference_layout("raspberrypi/raspberry_pi_pico_w", 0x200000) + assert layout["nvm"] == (0x180000, 0x1000) + assert layout["circuitpy"] == (0x181000, 0x7F000) + + def test_nordic_external_drive(self): + layout = reference_layout("nordic/feather_nrf52840_express", 0x100000) + assert layout["nvm"] == (0xF2000, 0x2000) + assert layout["circuitpy"] == "external" + + def test_unknown_port_and_board_rejected(self): + with pytest.raises(ValueError): + reference_layout("espressif/adafruit_feather_esp32s3", 0x400000) + with pytest.raises(ValueError): + reference_layout("raspberrypi/no_such_board", 0x200000) + + def rp2040_layout(self, nvm_offset): + return xip_flash( + f""" + code_partition: partition@100 {{ + compatible = "zephyr,mapped-partition"; + reg = <0x100 0xfdf00>; + }}; + nvm_partition: partition@{nvm_offset:x} {{ + compatible = "zephyr,mapped-partition"; + label = "nvm"; + reg = <0x{nvm_offset:x} 0x1000>; + }}; + circuitpy_partition: partition@100000 {{ + compatible = "zephyr,mapped-partition"; + label = "circuitpy"; + reg = <0x100000 0x100000>; + }}; + """ + ) + + def test_parity_matches(self): + edt = parse_dts_string(self.rp2040_layout(0xFF000)) + assert check_parity(edt, "raspberrypi/raspberry_pi_pico") == [] + + def test_parity_reports_moved_nvm(self): + edt = parse_dts_string(self.rp2040_layout(0xFE000)) + problems = check_parity(edt, "raspberrypi/raspberry_pi_pico") + assert len(problems) == 1 + assert "nvm_partition" in problems[0] + assert "0xfe000" in problems[0] and "0xff000" in problems[0] + + def test_parity_reports_missing_partition(self): + edt = parse_dts_string( + xip_flash( + """ + code_partition: partition@100 { + compatible = "zephyr,mapped-partition"; + reg = <0x100 0xfdf00>; + }; + """ + ) + ) + problems = check_parity(edt, "raspberrypi/raspberry_pi_pico") + assert any(p.startswith("nvm_partition: missing") for p in problems) + assert any(p.startswith("circuitpy_partition: missing") for p in problems)