install: improve NOR install reliability and rootfs_data handling

CLAUDE.md

@@ -221,6 +221,12 @@ Safety properties already built in — do not re-derive or undo them:
   stage. Generic wipes can lose `ethaddr`; registered stock-U-Boot migrations
   capture and restore the factory MAC instead of falling back to OpenIPC's
   compiled-in `00:00:23:34:45:66`.
+- Generic NOR installs preserve the persistent `rootfs_data` overlay unless
+  `--wipe-rootfs-data` is explicitly requested. The wipe is destructive and
+  must be CRC-verified after erase. Exact `--stage` plans must include
+  `rootfs-data` when the wipe flag is used, and `--skip-stage rootfs-data`
+  conflicts with an explicit wipe. Registered stock-U-Boot migrations retain
+  their existing cleanup behavior; do not silently change either default.
 - `install` defaults to the complete production stage plan. Development runs may
   use repeated `--stage` for an exact subset or repeated `--skip-stage` to
   subtract stages. The stage names are `uboot`, `kernel`, `rootfs`,

README.md

@@ -96,6 +96,14 @@ The `env` stage on a stock-U-Boot NOR migration requires explicit
 and restores the captured factory `ethaddr`. Other selected stages can be run
 without wiping the environment.
 
+Generic NOR installs preserve the existing `rootfs_data` overlay by default.
+Add `--wipe-rootfs-data` when a clean persistent overlay is desired; Defib
+erases the region and verifies the erased contents by CRC before continuing.
+When an exact `--stage` plan is used, that plan must include
+`--stage rootfs-data`; `--skip-stage rootfs-data` conflicts with an explicit
+wipe. Registered stock-U-Boot migrations retain their existing `rootfs_data`
+cleanup behavior.
+
 The release U-Boot owns boot-critical hardware initialization such as DDR
 cold-init and RAM probing limits. Defib owns the layout it actually flashes: it
 detects NOR capacity, selects the standard OpenIPC 8/16/32 MiB layout, and
@@ -126,6 +134,11 @@ defib install -c hi3518ev100:hiwatch-ds-i203 \
   --uboot u-boot-hi3518ev100-ddr3-256m-universal.bin \
   --wipe-env --stage uboot --stage env -p /dev/ttyUSB0 -d
 
+# Generic NOR install with a clean persistent overlay.
+defib install -c hi3516ev200 \
+  --firmware openipc.hi3516ev200-nor-lite.tgz \
+  --wipe-rootfs-data -p /dev/ttyUSB0
+
 # Full production plan except kernel/rootfs writes.
 defib install -c hi3518ev100:hiwatch-ds-i203 \
   --firmware hi3518ev100_lite_hiwatch-ds-i203-nor.tgz \

src/defib/cli/app.py

@@ -2179,6 +2179,16 @@ def install(
             "their captured factory ethaddr is restored."
         ),
     ),
+    wipe_rootfs_data: bool = typer.Option(
+        False,
+        "--wipe-rootfs-data",
+        help=(
+            "Erase and CRC-verify the persistent rootfs_data overlay on NOR. "
+            "Generic installs preserve it unless this flag is given; with exact "
+            "--stage selection include --stage rootfs-data. Registered stock-U-Boot "
+            "migrations keep their existing migration behavior."
+        ),
+    ),
     final_reset: bool = typer.Option(
         True,
         "--final-reset/--no-final-reset",
@@ -2234,6 +2244,7 @@ def install(
         nor_size=nor_size,
         nand=nand,
         wipe_env=wipe_env,
+        wipe_rootfs_data=wipe_rootfs_data,
         final_reset=final_reset,
         stages=tuple(stage or ()),
         skip_stages=tuple(skip_stage or ()),

src/defib/flashdump.py

@@ -23,6 +23,7 @@
 from typing import Callable
 
 from defib.transport.base import Transport, TransportTimeout
+from defib.uboot_tftp import run_uboot_tftp
 
 logger = logging.getLogger(__name__)
 
@@ -269,13 +270,19 @@ async def send_command(
     timeout: float = 5.0,
     wait_for: str | None = None,
     verify_echo: bool = False,
+    require_prompt: bool = False,
 ) -> str:
     """Send a command to U-Boot and collect the response.
 
     ``verify_echo`` is intended for fragile legacy UART consoles.  When set,
     the command is entered one character at a time and U-Boot must echo every
     byte correctly before the final carriage return is sent.  This prevents a
     corrupted ``sf erase/write/read`` line from ever being executed.
+
+    Historically callers using ``wait_for`` received the partial buffer when the
+    deadline expired. Keep that behavior by default because dump/restore probes
+    use timeout as capability information. Installer writes opt into strict prompt
+    completion with ``require_prompt=True``.
     """
     # Clear any pending input.
     try:
@@ -316,7 +323,14 @@ async def send_command(
                 return buf.decode("ascii", errors="replace")
             continue
 
-    return buf.decode("ascii", errors="replace")
+    response = buf.decode("ascii", errors="replace")
+    if wait_for and require_prompt:
+        partial = response.strip()[-200:] or "<no response>"
+        raise TransportTimeout(
+            f"Timed out waiting for {wait_for!r} after U-Boot command {cmd!r}; "
+            f"partial response: {partial}"
+        )
+    return response
 
 
 async def tftp_to_ram(
@@ -325,22 +339,24 @@ async def tftp_to_ram(
     filename: str,
     timeout: float = 120.0,
 ) -> str:
-    """Download a file via TFTP into RAM.
-
-    Tries ``tftpboot`` first; falls back to ``tftp`` if the U-Boot
-    build doesn't have the ``tftpboot`` alias.
-
-    Returns the command response text.  Raises RuntimeError on failure.
-    """
-    cmd = f"tftpboot 0x{addr:x} {filename}"
-    resp = await send_command(transport, cmd, timeout=timeout, wait_for="# ")
-    if "unknown command" in resp.lower():
-        logger.debug("tftpboot not available, falling back to tftp")
-        cmd = f"tftp 0x{addr:x} {filename}"
-        resp = await send_command(transport, cmd, timeout=timeout, wait_for="# ")
-    if "done" not in resp.lower() and "bytes transferred" not in resp.lower():
-        raise RuntimeError(f"TFTP download failed: {resp.strip()[-200:]}")
-    return resp
+    """Download a file via TFTP into RAM using legacy lenient prompt handling."""
+
+    async def run_command(command: str, command_timeout: float) -> tuple[bool, str]:
+        response = await send_command(
+            transport,
+            command,
+            timeout=command_timeout,
+            wait_for="# ",
+        )
+        return True, response
+
+    return await run_uboot_tftp(
+        run_command,
+        filename,
+        addr,
+        use_loadaddr=False,
+        timeout=timeout,
+    )
 
 
 def detect_flash_from_text(text: str) -> int | None:
@@ -421,11 +437,18 @@ async def detect_flash(
 
 
 async def _detect_crc32(transport: Transport) -> bool:
-    """Check if U-Boot has the crc32 command."""
+    """Check if U-Boot has the crc32 command without making backup depend on it."""
     resp = await send_command(transport, "crc32 0 0", timeout=3.0, wait_for="# ")
-    # If crc32 exists, it will output a CRC value or usage.
-    # If not, it will say "Unknown command"
-    return "unknown command" not in resp.lower()
+    text = resp.lower()
+    if "unknown command" in text:
+        return False
+    # A supported crc32 command prints either a checksum or usage/help text.
+    # No/partial response is inconclusive, so keep dump-flash usable without
+    # per-block CRC verification rather than treating silence as support.
+    return bool(
+        re.search(r"==>\s*[0-9a-f]{8}|crc32 for", text)
+        or ("usage:" in text and "crc32" in text)
+    )
 
 
 async def _get_device_crc32(

src/defib/install/firmware.py

@@ -7,6 +7,8 @@
 from dataclasses import dataclass
 from pathlib import Path
 
+from defib.uboot_tftp import uboot_tftp_commands as uboot_tftp_commands
+
 
 @dataclass(frozen=True)
 class FirmwareBundle:
@@ -18,25 +20,6 @@ class FirmwareBundle:
     rootfs: bytes
 
 
-def uboot_tftp_commands(
-    filename: str,
-    ram_addr: int,
-    *,
-    use_loadaddr: bool,
-) -> tuple[str, str]:
-    """Return primary/fallback U-Boot TFTP commands for one staged file.
-
-    Vendor-U-Boot migrations deliberately use ``loadaddr`` so the command
-    line stays short on fragile legacy UART consoles.  Generic boot-ROM and
-    download-command installs retain the historical explicit RAM address and
-    therefore do not depend on environment read-back formatting.
-    """
-    if use_loadaddr:
-        return f"tftpboot {filename}", f"tftp {filename}"
-    address = f"0x{ram_addr:x}"
-    return f"tftpboot {address} {filename}", f"tftp {address} {filename}"
-
-
 def load_firmware_bundle(path: str | Path) -> FirmwareBundle:
     """Read kernel/rootfs and verify any matching md5sum entries in one pass."""
     kernel_name = ""

src/defib/install/layout.py

@@ -88,7 +88,9 @@ def uboot_flash_command_error(response: str) -> str | None:
         if not line:
             continue
         text = line.lower()
-        if text.startswith(("error:", "unknown command", "usage: sf", "usage: nand")):
+        if text.startswith(("error:", "unknown command", "usage:")):
+            return line
+        if re.match(r"failed\b", text):
             return line
         if "no spi flash selected" in text:
             return line
@@ -101,6 +103,33 @@ def uboot_flash_command_error(response: str) -> str | None:
     return None
 
 
+def uboot_sf_lock_unsupported(response: str) -> bool:
+    """Return True only when the U-Boot build lacks a usable sf lock command."""
+
+    lines = [line.strip().lower() for line in response.splitlines() if line.strip()]
+    for line in lines:
+        mentions_sf_lock = "sf" in line or "lock" in line
+        if mentions_sf_lock and "unknown command" in line:
+            return True
+        if mentions_sf_lock and ("not supported" in line or "unsupported" in line):
+            return True
+
+    # Older U-Boot variants often print the whole sf usage table for an unknown
+    # subcommand. That is compatibility information only when the table itself
+    # contains no sf lock entry. If sf lock is listed, a Usage response means our
+    # invocation was rejected and must remain a hard failure.
+    usage_seen = any(line.startswith("usage:") for line in lines)
+    sf_usage: list[str] = []
+    for line in lines:
+        candidate = line.removeprefix("usage:").strip() if line.startswith("usage:") else line
+        if re.match(r"^sf(?:\s|$)", candidate):
+            sf_usage.append(candidate)
+    if usage_seen and sf_usage:
+        return not any(re.match(r"^sf\s+lock(?:\s|$)", line) for line in sf_usage)
+
+    return False
+
+
 async def set_uboot_env_verified(
     cmd: Command,
     key: str,

src/defib/install/model.py

@@ -75,6 +75,7 @@ class InstallRequest:
     nor_size: int = 0
     nand: bool = False
     wipe_env: bool = False
+    wipe_rootfs_data: bool = False
     final_reset: bool = True
     stages: tuple[str, ...] = ()
     skip_stages: tuple[str, ...] = ()