hi3536dv100: drop the nbd8010s variant now that OpenIPC U-Boot handles the E

agent/README.md

@@ -89,11 +89,11 @@ gk7205v500 also serves gk7205v510/v530. The V500 bootrom has no SPL stage:
 u-boot-xmedia image (downloaded, or `-f` for your own), whose DDR-init code
 runs first; the bootrom then executes the slot in place at `0x41007000`.
 
-hi3536dv100 is the DVR/NVR part Xiongmai sells as "Hi3536E". Its UART runs
-off a fixed 24 MHz clock, so `SET_BAUD` reaches 921600 with no clock
-switching. The OpenIPC SPL for it doesn't bring up DDR on the Xiongmai
-NBD8010S-KL-V2 board; use `-c hi3536dv100:nbd8010s` there, which uploads the
-board's own SPL instead.
+hi3536dv100 also covers the "Hi3536E" that Xiongmai puts in its NVRs, a
+variant of the same die. Its UART runs off a fixed 24 MHz clock, so
+`SET_BAUD` reaches 921600 with no clock switching. The E variant needs its
+own DDR settings; OpenIPC's u-boot-hi3536dv100 applies them when
+SYSCTRL+0xEE0 reads 0x02000000, so plain `-c hi3536dv100` works on both.
 
 Addresses from [qemu-hisilicon](https://github.com/OpenIPC/LoTool) hardware definitions.
 

src/defib/firmware.py

@@ -214,6 +214,28 @@ def _cached_file(name: str) -> Path | None:
     return None
 
 
+def _has_hi3536e_ddr_config(data: bytes) -> bool:
+    """Built after OpenIPC/u-boot-hi3536dv100#1, whose SPL programs the
+    Hi3536E's DDR. Its DMC timing values sit in the SPL's literal pool
+    whichever registers the compiler picks; older builds lack them, and on
+    a Hi3536E their DDR training resets the chip."""
+    spl = data[:0x5000]
+    return all(v.to_bytes(4, "little") in spl for v in (0x34421A12, 0x4144B48D))
+
+
+# Cache entries that a later published build fixed in a way recovery depends
+# on. One that fails its check is downloaded again; offline it is still used
+# (it works on the boards the fix is not about), with a warning.
+_CACHE_CHECKS: dict[str, Callable[[bytes], bool]] = {
+    "u-boot-hi3536dv100-universal.bin": _has_hi3536e_ddr_config,
+}
+
+
+def _is_stale(path: Path) -> bool:
+    check = _CACHE_CHECKS.get(path.name)
+    return check is not None and not check(path.read_bytes())
+
+
 def get_cached_path(chip: str, flash_type: str | None = None) -> Path | None:
     """Get the path to cached firmware, or None if not cached.
 
@@ -236,7 +258,7 @@ def get_cached_path(chip: str, flash_type: str | None = None) -> Path | None:
         if not candidate:
             continue
         path = _cached_file(candidate)
-        if path is not None:
+        if path is not None and not _is_stale(path):
             return path
     return None
 
@@ -321,9 +343,22 @@ def download_firmware(
     # Download
     name = asset_name(chip, flash_type)
     assert name is not None  # firmware_url() is None otherwise
+    dest = get_cache_dir() / name
+    # A stale entry is about to be replaced, and _download() deletes dest
+    # when it fails; keep its bytes to put back if we turn out to be offline.
+    stale = _cached_file(name)
+    stale_data = stale.read_bytes() if stale is not None else None
     try:
-        return _download(url, get_cache_dir() / name, on_progress)
+        return _download(url, dest, on_progress)
     except ConnectionError:
+        if stale_data is not None:
+            dest.write_bytes(stale_data)
+            logger.warning(
+                "Download of %s failed; using the cached copy, which predates "
+                "a fix this chip may need",
+                name,
+            )
+            return dest
         # A universal image cached before the per-flash-type split still boots
         # a NOR board, so keep it usable offline. It is never downloaded, and
         # never used for NAND, whose split layout it carries is retired.

src/defib/profiles/data/hi3536dv100.json

@@ -1 +1 @@
-{"name": "hi3536dv100", "PRESTEP0": [4, 224, 45, 229, 36, 0, 159, 229, 36, 16, 159, 229, 0, 16, 128, 229, 32, 0, 159, 229, 32, 16, 159, 229, 4, 16, 128, 228, 0, 224, 128, 229, 4, 240, 157, 228, 239, 190, 173, 222, 239, 190, 173, 222, 239, 190, 173, 222, 60, 1, 5, 18, 78, 87, 79, 68, 64, 1, 5, 18, 117, 106, 105, 122], "DDRSTEP0": [4, 224, 45, 229, 36, 0, 159, 229, 36, 16, 159, 229, 0, 16, 128, 229, 32, 0, 159, 229, 32, 16, 159, 229, 4, 16, 128, 228, 0, 224, 128, 229, 4, 240, 157, 228, 239, 190, 173, 222, 239, 190, 173, 222, 239, 190, 173, 222, 60, 1, 5, 18, 120, 86, 52, 18, 64, 1, 5, 18, 117, 106, 105, 122], "ADDRESS": ["0x04012000", "0x04010500", "0x81000000"], "FILELEN": ["0x0040", "0x5000"], "STEPLEN": ["0x0040", "0x0050"], "variants": {"nbd8010s": {"SPL_BLOB": "hi3536dv100-nbd8010s-spl.bin", "FILELEN": ["0x0040", "0x5840"]}}}
+{"name": "hi3536dv100", "PRESTEP0": [4, 224, 45, 229, 36, 0, 159, 229, 36, 16, 159, 229, 0, 16, 128, 229, 32, 0, 159, 229, 32, 16, 159, 229, 4, 16, 128, 228, 0, 224, 128, 229, 4, 240, 157, 228, 239, 190, 173, 222, 239, 190, 173, 222, 239, 190, 173, 222, 60, 1, 5, 18, 78, 87, 79, 68, 64, 1, 5, 18, 117, 106, 105, 122], "DDRSTEP0": [4, 224, 45, 229, 36, 0, 159, 229, 36, 16, 159, 229, 0, 16, 128, 229, 32, 0, 159, 229, 32, 16, 159, 229, 4, 16, 128, 228, 0, 224, 128, 229, 4, 240, 157, 228, 239, 190, 173, 222, 239, 190, 173, 222, 239, 190, 173, 222, 60, 1, 5, 18, 120, 86, 52, 18, 64, 1, 5, 18, 117, 106, 105, 122], "ADDRESS": ["0x04012000", "0x04010500", "0x81000000"], "FILELEN": ["0x0040", "0x5000"], "STEPLEN": ["0x0040", "0x0050"]}

tests/test_agent_binary_lookup.py

@@ -43,9 +43,6 @@ def test_a_variant_suffix_still_resolves(self, monkeypatch, tmp_path):
         (tmp_path / "agent-hi3516cv500.bin").write_bytes(b"\x00" * 16)
         assert get_agent_binary("hi3516av300:emmc") is not None
 
-    def test_a_board_variant_uses_the_chip_build(self):
-        assert agent_binary_for("hi3536dv100:nbd8010s") == "hi3536dv100"
-
     def test_every_path_is_absolute(self):
         assert all(Path(p).is_absolute() for p in _agent_search_path("hi3516cv300"))
 

tests/test_firmware.py

@@ -359,3 +359,52 @@ def test_gk7205v5xx_shares_the_v500_donor_cache_entry(self, tmp_path, monkeypatc
         donor.write_bytes(b"D" * 4096)
         assert get_cached_path("gk7205v510") == donor
         assert get_cached_path("gk7205v510", "nand") is None
+
+
+class TestStaleCacheRefresh:
+    """A cached u-boot-hi3536dv100-universal.bin from before
+    OpenIPC/u-boot-hi3536dv100#1 resets a Hi3536E during DDR training, so
+    the cache must not keep serving it once the fixed build is published."""
+
+    FIXED = b"\x00" * 0x100 + (0x34421A12).to_bytes(4, "little") \
+        + (0x4144B48D).to_bytes(4, "little") + b"\x00" * 0x4000
+    OLD = b"\x00" * 0x4108
+
+    def _seed(self, tmp_path, monkeypatch, data):
+        monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path))
+        monkeypatch.setattr("sys.platform", "linux")
+        path = get_cache_dir() / "u-boot-hi3536dv100-universal.bin"
+        path.write_bytes(data)
+        return path
+
+    def test_a_fixed_build_is_used_from_cache(self, tmp_path, monkeypatch):
+        path = self._seed(tmp_path, monkeypatch, self.FIXED)
+        assert get_cached_path("hi3536dv100") == path
+
+    def test_an_old_build_is_downloaded_again(self, tmp_path, monkeypatch):
+        from defib import firmware
+
+        path = self._seed(tmp_path, monkeypatch, self.OLD)
+        assert get_cached_path("hi3536dv100") is None
+
+        def fake_download(url, dest, on_progress=None):
+            dest.write_bytes(self.FIXED)
+            return dest
+
+        monkeypatch.setattr(firmware, "_download", fake_download)
+        assert download_firmware("hi3536dv100") == path
+        assert path.read_bytes() == self.FIXED
+
+    def test_offline_the_old_build_is_still_used(self, tmp_path, monkeypatch):
+        from defib import firmware
+
+        path = self._seed(tmp_path, monkeypatch, self.OLD)
+
+        def offline(url, dest, on_progress=None):
+            if dest.exists():
+                dest.unlink()  # what _download() does on failure
+            raise ConnectionError("offline")
+
+        monkeypatch.setattr(firmware, "_download", offline)
+        assert download_firmware("hi3536dv100") == path
+        assert path.read_bytes() == self.OLD

tests/test_profiles.py

@@ -220,19 +220,6 @@ def test_real_av300_emmc_variant_loads_spl_blob(self):
         # SPL extracted at 0x5000 (above the gzip boundary at ~0x5220)
         assert len(p.spl_data) == 0x5000
 
-    def test_real_hi3536dv100_nbd8010s_variant_loads_spl_blob(self):
-        """The Xiongmai NBD8010S-KL-V2 NVR's DDR differs from the board the
-        OpenIPC hi3536dv100 SPL was built for, so its variant ships the SPL
-        from the board's own U-Boot. The blob runs right up to the LZMA
-        payload at 0x5840, and FILELEN grows to match so none of it is cut."""
-        p = load_profile("hi3536dv100:nbd8010s", PROFILES_DIR)
-        assert p.spl_blob == "hi3536dv100-nbd8010s-spl.bin"
-        assert p.spl_data is not None
-        assert p.spl_data[:4] == bytes.fromhex("170800ea")
-        assert len(p.spl_data) == 0x5840
-        assert p.spl_max_size == 0x5840
-        assert load_profile("hi3536dv100", PROFILES_DIR).spl_data is None
-
 
 class TestSplBlob:
     """SPL_BLOB lets a profile (typically a board variant) ship pre-built

tests/test_recovery_session.py

@@ -57,7 +57,6 @@ async def test_full_standard_session(self):
 
     @pytest.mark.asyncio
     @pytest.mark.parametrize("chip, spl_len", [
-        ("hi3536dv100:nbd8010s", 0x5840),
         # 0x5000 blob under an inherited FILELEN of 0x6000: must not be
         # padded out to the profile maximum.
         ("hi3516av300:emmc", 0x5000),