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)