hisi: read the V5 die ID from the OTP shadow

[johnchia]

ipcinfo -i covers HiSilicon V4 and SigmaStar. Every HISI_OT part — 3516CV608, 3516CV610, 3516CV613, 3516DV500, 3519DV500 — answers with nothing, because hisi_ev300_get_die_id() returns early on any generation but HISI_V4.

On OpenIPC that is not cosmetic. ethaddr_provision() in the firmware's rcS derives the camera's MAC from this value, so a V5 board with no vendor-provisioned ethaddr comes up on the shared placeholder, and a second one collides with it.

Where the V5 die ID lives

V5 has no counterpart to the V4 die-ID block at 0x12020400. The vendor reads its die ID through a bootrom call (otp_get_die_id(), gsl/drivers/share_drivers/share_drivers.c), but every OTP row is also shadowed into a register window at the offset it occupies in OTP, so the 16 bytes of OTP_DIE_ID are readable with an ordinary /dev/mem map. Both halves are in the Hi3516CV610 SDK:

gsl/drivers/otp/otp.h:48    #define OTP_DIE_ID       0xF0   /* 0xF0~0xFF, 16bytes */
gsl/include/platform.h:310  #define SCPU_OTPC_BASE_ADDR   0x101E0000
gsl/include/platform.h:311  #define OTP_SHADOW_BASE       SCPU_OTPC_BASE_ADDR

Despite the SCPU_ (secure CPU) name, the window is reachable from the non-secure side: the OEM's own hwconf.ko is an ordinary kernel module and it ioremaps OTP_SHADOW_BASE + 0x10C (OTP_VERSION_ID_REG), and svb.c reads +0x20, +0x118 and +0x124 with plain readl.

Measured

On a Hi3516CV608 (H4-52POX-S), through /dev/mem:

0x101E00F0 = 0x97090A14      0x101E00F4 = 0x20E41210
0x101E00F8 = 0x39182919      0x101E00FC = 0x04921EAC
0x101E010C = 0x00240180

That last read is the window identifying itself — 0x240180 is exactly OTP_608_VERSION_ID from the SDK's svb.h, so this is the OTP shadow and not an unrelated block that happens to hold entropy.

before:  ipcinfo -i  ->                                    (empty, exit 0)
after:   ipcinfo -i  -> 140a09971012e42019291839ac1e9204

Flashed into an OpenIPC image, the camera provisioned itself on the first boot: eth0 and the saved ethaddr both became 02:77:8e:1c:92:05, the value the firmware's own mac_from_uid derives from that uid.

What the change does

• hisi_ot_get_die_id() reads the four words and emits 32 hex characters in OTP byte order — the shadow words are little-endian, so byte i of what otp_get_die_id() would return is word[i / 4] >> (8 * (i % 4)).
• An unfused or unreadable row reads all-zeroes or all-ones. Both are rejected: a caller that turns the string into a MAC would otherwise hand every board in a fleet the same address.
• hisi_get_die_id() dispatches on chip_generation, V4 to the existing reader and HISI_OT to the new one. hisi_chip_properties() uses it too, so ipctool's JSON gets the id on V5 as well.
• print_serial()'s if (!serial) tested the address of a stack array, which is never null, so a chip with no reader printed whatever the stack held instead of failing. It now honours the reader's return value.

A contract change worth calling out

Because if (!serial) was never true on master, ipcinfo -i always fell through with exit 0. It now exits 1 on a chip with no reader (V4A, INFINITY6C, anything non-HiSi and non-SStar), which also aborts any flags chained after -i on the same command line. That is what ethaddr_provision() wants and it matches how -c, -f and -t already behave, but it will change the outcome for a script that tests the exit code or chains flags.

Separately, ipcinfo never called hal_cleanup() on any exit path, so setup_hal_*()'s side effects -- printk written to 0 0 0 0, and on HISI_OT the sensor clock force-enabled -- outlived the process. Pre-existing, but this PR is what puts ipcinfo -i on every V5 boot's rcS path, so it is fixed here with an atexit() wrapper.

What one part cannot prove

That the value is per-die rather than per-model. This is the field the vendor documents as the die ID, but a second V5 part reading something different is what would settle it — devmem 0x101E00F0 32 is the whole test, and the read is harmless. Only the CV608 was available here; CV610, CV613, DV500 and 3519DV500 take the same path untested.

https://claude.ai/code/session_011qHvUfNDKptXf41shaU1vE

[qodo-free-for-open-source-projects]

PR Summary by Qodo

hisi: read V5 die IDs from the OTP shadow

🐞 Bug fix ✨ Enhancement 🕐 20-40 Minutes

AI Description

• Read HISI_OT die IDs from the V5 OTP shadow in canonical byte order.
• Reject unreadable OTP values to prevent duplicate MAC address provisioning.
• Route CLI and JSON identity output through generation-aware, failure-safe lookup.

Diagram

graph TD
  A["CLI serial"] --> C{"Chip generation"} -->|V4| D["V4 ID reader"]
  B["JSON properties"] --> C
  C -->|HISI_OT| E["V5 OTP reader"] --> F["OTP shadow"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Invoke the boot ROM OTP API

• ➕ Matches the access path used by the vendor implementation.
• ➕ Could abstract the physical OTP shadow layout. Basic hierarchy preserved.
• ➖ Requires a stable, discoverable boot ROM ABI across all targeted parts.
• ➖ Calling undocumented ROM entry points from user space adds platform and safety risk.
• ➖ Does not reuse the project's established memory-register access mechanism.

2. Expose the ID through a kernel nvmem driver

• ➕ Avoids direct physical-memory access from user space.
• ➕ Provides a conventional kernel-mediated OTP interface.
• ➖ Requires kernel and device-tree support absent from existing deployed firmware.
• ➖ Substantially expands the change beyond this user-space library.
• ➖ Would reduce compatibility with legacy OpenIPC images.

Recommendation: Use the PR's direct OTP-shadow read. It reuses the existing mem_reg infrastructure, requires no kernel or undocumented boot ROM integration, and explicitly rejects sentinel values before provisioning can derive duplicate MAC addresses. Follow-up validation across CV610, CV613, DV500, and 3519DV500 is still advisable.

Files changed (3) +74 / -5

Enhancement (2) +65 / -1

hal_hisi.hExpose the generation-aware HiSilicon die-ID API +1/-0

Expose the generation-aware HiSilicon die-ID API

• Declares hisi_get_die_id() for shared use by CLI and chip-property consumers while retaining the existing V4-specific reader.

src/hal/hisi/hal_hisi.h

ispreg.cRead and validate HISI_OT die IDs from OTP shadow +64/-1

Read and validate HISI_OT die IDs from OTP shadow

• Adds a V5 reader for the 16-byte OTP die ID at 0x101E00F0, formats little-endian words in OTP byte order, and rejects all-zero or all-one values. Introduces generation-based dispatch and routes JSON chip properties through it so HISI_OT devices expose their IDs.

src/hal/hisi/ispreg.c

Bug fix (1) +9 / -4

ipcinfo.cMake serial output generation-aware and failure-safe +9/-4

Make serial output generation-aware and failure-safe

• Initializes the serial buffer and uses the new HiSilicon generation-aware die-ID API. It now exits unsuccessfully without writing stdout when no valid identifier is available, replacing the ineffective stack-array null check.

example/ipcinfo.c

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[openipc-ai]

Thanks for this. The investigation is thorough and the CV608 evidence is convincing: the +0x10C word self-identifying the window as the OTP shadow, and the derived MAC matching mac_from_uid. Requesting changes for one gap and two small checks that belong next to it.

1. The OTP base is verified on one CV608 and applied to all five HISI_OT parts, and the read is now unconditional on the plain ipctool path via hisi_chip_properties(). If 0x101E0000 is not the shadow on 3516DV500/3519DV500 the outcome is either a bus error that kills the whole report, or a stable foreign value that gives every board of that model the same MAC. Suggest gating on the parts the CV610 SDK actually covers until a DV500 has been measured (details inline).
2. The all-zero/all-ones rejection only covers the V5 arm. The V4 arm of hisi_get_die_id() returns true on a zero block and prints 48 zeros.
3. The filter combines all four words, so a mix of 0xFFFFFFFF and 0 words passes as an identity.

Non-blocking: ipcinfo -i now exits 1 on chips without a reader and aborts any later flags (worth a line in the description); ipcinfo never calls hal_cleanup(), so the printk and sensor-clock side effects of setup_hal_hisi() persist after the boot-time call; a few nits inline.

I could not verify on hardware. Both the lab 3519DV500 and CV608 were unreachable today. This read-only sequence is the whole test on either part:

for a in 0x101E00F0 0x101E00F4 0x101E00F8 0x101E00FC 0x101E010C; do devmem $a 32; done

0x101E010C should come back as a 0x24xxxx-style OTP version id, and the four die words should differ from the CV608 values in the description. A second CV608 reading different words would also settle per-die versus per-model.

[openipc-ai]

Blocking. This base is verified on one CV608, and the SDK it comes from covers CV608/CV610/CV613, but it runs on every HISI_OT part including 3516DV500 and 3519DV500, and hisi_chip_properties() now performs the read unconditionally on the plain ipctool path.

Two failure modes if 0x101E0000 is not the OTP shadow on one of those:

• Unbacked window: mem_reg() only guards the mmap; the dereference at tools.c is unguarded and there is no SIGBUS handler, so the whole YAML report dies. See 81ccfa4 (the 3536CV100 bus-error fix) for the in-tree precedent.
• Backed by some other stable register: it passes the all-zero/all-ones filter below and every board of that model reports the same id, which is the fleet-wide MAC collision this PR sets out to prevent.

Suggest gating the reader on the chip IDs the CV610 SDK covers until a DV500 or 3519DV500 has been measured. The read is harmless, so this is the whole test:

for a in 0x101E00F0 0x101E00F4 0x101E00F8 0x101E00FC 0x101E010C; do devmem $a 32; done

+0x10C should be a 0x24xxxx-style version id and the four die words should differ from the CV608 values in the description. I tried to run it on the lab 3519DV500 and CV608 today but both were unreachable.

[johnchia]

The DV500 SDK answers this, and it says the base is not CV610-only.

Hi3519DV500 SDK R11 covers both DV500 parts out of one bsp tree — bsp/pub/hi3516dv500_image_glibc/, bsp/tools/pc/uboot_env/env_text/hi3516dv500/, and svb.h's OTP_16D_VERSION_ID alongside OTP_19D_VERSION_ID — and it defines the same window at the same offset:

a55_linux/source/bsp/components/gsl/include/platform.h:335
    #define SCPU_OTPC_BASE_ADDR   0x101E0000
    #define OTP_SHADOW_BASE       SCPU_OTPC_BASE_ADDR
a55_linux/source/bsp/components/gsl/drivers/otp/otp.h:48
    #define OTP_DIE_ID            0xF0   /* 0xF0~0xFF, 16bytes */
a55_linux/source/bsp/components/gsl/drivers/svb/svb.h:80
    #define OTP_BASE_REG          0x101E0000

Identical to the CV610 SDK's platform.h:310 / otp.h:48. So the two vendor SDKs between them cover all five HISI_OT parts and agree on both the base and the row. A chip-ID allowlist would add a table to maintain against a divergence neither SDK shows, so I'd rather cite the second SDK than gate — I've put those citations in the comment above the reader.

One correction to the probe, which matters if you run it: 0x101E010C is the CV6xx family's version-id offset. On DV500 the register moved — svb.h:81 has OTP_VERSION_ID_REG 0x0120, expecting 0x220101 on 3519DV500 and 0x220102 on 3516DV500 (svb.h:85-86). So on a DV500 the sequence is:

for a in 0x101E00F0 0x101E00F4 0x101E00F8 0x101E00FC 0x101E0120; do devmem $a 32; done

As written it would have read a wrong offset on the lab 3519DV500 and looked like a failure.

If you want belt-and-braces anyway after that, the honest version is a runtime self-check rather than a model list: read the version id and require it to look like its family's (0x24xxxx at +0x10C, 0x22xxxx at +0x120). That verifies the window instead of trusting a table. It does not help with the bus-error case, though — the check read would fault first — so it only buys protection against the "backed by some other stable register" half. Say the word and I'll add it.

[openipc-ai]

Thanks, the DV500 SDK citations settle it: two vendor trees, all five HISI_OT parts, same base and same row. Agreed that a chip-ID allowlist would be a table maintained against a divergence neither SDK shows, so citing the second SDK above the reader is the right form.

Thanks also for the offset correction. +0x120 for the DV500 family is now in our lab notes, so the probe on the 3519DV500 will read the right register when that board comes back.

I'd skip the runtime version-id self-check. It cannot help the bus-error half, and it would reintroduce the per-family table by another route.

Per-die versus per-model stays open until a second gen5 board is read. Our lab CV608 and 3519DV500 were unreachable both days; we will run the probe on whichever comes back first and report here.

[openipc-ai]

The V4 arm skips the all-zero/all-ones rejection that the V5 arm gets at line 757. hisi_ev300_get_die_id() returns true on a block of six zero words (the trailing-'0' strip never finds a non-zero digit, so it never breaks), and ipcinfo -i then prints 48 zeros with exit 0. Same for all-ones.

Since the invariant this PR introduces is "callers derive a MAC from this", the check belongs here in the dispatcher so both arms honour it.

[openipc-ai]

This combines all four words, so a shadow that reads e.g. {0xFFFFFFFF, 0xFFFFFFFF, 0, 0} (partially fused or partially locked) passes both tests and is emitted as an identity.

Rejecting a 16-byte value whose bytes are all 0x00 or 0xFF closes that without refusing a legitimate id that happens to contain a zero word. A per-word reject would be too strict for that reason.

[openipc-ai]

Nit: sixteen snprintf calls with running pointer arithmetic lean on the * 8 + 1 precheck above to keep buf + len - ptr non-negative, and a reader has to re-derive that. Copying the words into a uint8_t[16] and hex-encoding in one loop, as bootrom.c does, keeps the endian independence and lets the precheck become 2 * sizeof id + 1.

[openipc-ai]

Two small things now that there is a single entry point:

• hisi_ev300_get_die_id() keeps its own chip_generation != HISI_V4 guard and stays exported, but this dispatcher is now its only caller. Making it static and dropping the inner guard leaves one place encoding the routing. As it stands, a future case HISI_V4A: that reuses it would be silently dead.
• A new API is the moment to take size_t, like sstar_get_die_id(), which is called side by side with it in print_serial().

[openipc-ai]

Worth a line in the description: on master if (!serial) was never true, so -i always fell through with exit 0. Now a chip with no reader (V4A, INFINITY6C, anything non-HiSi/non-SStar) exits 1, and any flags after -i on the command line never run.

That is what ethaddr_provision wants and it matches -c/-f/-t, but it is a contract change for scripts that chain flags or test the exit code.

[openipc-ai]

Nit: the zero-fill and the !*serial test are redundant with found. Every reader writes at least 12 chars before returning true and returns false on every failure path, so if (!found) with a plain char serial[512] is behaviourally identical, and it makes clear which signal is authoritative.

The comment at 111-114 narrates the removed bug. The one sentence worth keeping in code is the last one (provisioning derives a MAC, so a miss must be silent on stdout and non-zero on exit); the rest belongs in the commit message.

[openipc-ai]

Pre-existing, but this PR puts ipcinfo -i on every V5 boot's rcS path, so flagging it: getchipvendor() runs setup_hal_hisi(), which writes 0 0 0 0 to /proc/sys/kernel/printk and on HISI_OT force-enables the sensor clock (CRG8464/CRG8472). ipcinfo never calls hal_cleanup() on any exit path (only sensors.c and i2cspi.c do), so the console stays silent and the CRG stays modified for the rest of boot.

One hal_cleanup() before each exit, or an atexit() registered after getchipname(), fixes it.

[johnchia]

Thanks — that's a careful review. Items 2–8 are done in two commits; item 1 I've pushed back on, with evidence rather than a gate (inline).

1 — the OTP base. The Hi3519DV500 SDK R11 covers both DV500 parts out of one bsp tree and defines the identical OTP_SHADOW_BASE = SCPU_OTPC_BASE_ADDR = 0x101E0000 with OTP_DIE_ID 0xF0, 16 bytes. Two vendor SDKs, all five HISI_OT parts, same window. Citations are now in the comment above the reader. One correction to the probe: +0x10C is the CV6xx version-id offset — on DV500 it is +0x120, expecting 0x220101 / 0x220102, so the sequence as written would have read the wrong register on the lab 3519DV500.

2, 3 — the reject (c64e953). Moved to the dispatcher so both arms honour it, and it now tests the emitted digits rather than whole words, which catches the partially fused {0xFFFFFFFF, 0xFFFFFFFF, 0, 0} mixture too. A legitimate id containing a zero word still passes.

4 — sixteen bytes assembled once, hex-encoded in one indexed loop like bootrom.c; the precheck is 2 * sizeof bytes + 1.

5 — hisi_ev300_get_die_id() is static with the inner guard dropped, so routing lives in one place; both readers take size_t. The V4 arm gained a length precheck, which is what makes its len -= outsz safe under an unsigned type.

7 — dropped the zero-fill and the !*serial retest; found is authoritative. Kept only the sentence about provisioning in the code, moved the rest to the commit message.

8 — separate commit (38ba095), since it is pre-existing and this PR is only what makes it visible. atexit() wrapper rather than a call before each exit: hal_cleanup is only set once a HAL is selected, so the wrapper reads it at exit time and NULL-guards. Both restore paths are idempotent, so it coexists with the explicit calls in sensors.c and i2cspi.c.

6 — noted in the description below.

On verification. The bench CV608 is powered down at the moment, so these two commits are compile-verified only: clean builds on host gcc and arm-openipc-linux-musleabi, plus a host harness that extracts both readers and the filter verbatim from ispreg.c and stubs mem_reg(). The CV608's measured words still encode to 140a09971012e42019291839ac1e9204, byte-identical to the pre-refactor output, and all-zero / all-ones / partially fused are rejected on both arms. I'll re-run on the board and report before you merge.

Per-die versus per-model is still unproven by hardware — the CV608 here is the only gen5 part I have, and re-reading it returns the same words. Your probe on the lab CV608 or a DV500 is the test, with the offset correction above.

[johnchia]

Board came back up — re-ran on the CV608 (H4-52POX-S, HI3516CV610_MPP_V1.0.2.0 B051). Both commits confirmed on hardware.

The refactor is output-preserving. Same string as before, and the raw window still self-identifies:

0x101E00F0 = 0x97090A14      0x101E00F4 = 0x20E41210
0x101E00F8 = 0x39182919      0x101E00FC = 0x04921EAC
0x101E010C = 0x00240180      <- OTP_608_VERSION_ID

ipcinfo -i  -> 140a09971012e42019291839ac1e9204   exit 0

Byte-identical to the pre-refactor build still installed at /usr/bin/ipcinfo on this board, and to what the host harness predicted. The YAML path works too — ipctool reports model: 3516CV608 / id: 140a09971012e42019291839ac1e9204. Chained flags behave: ipcinfo -i -c prints both and exits 0.

The hal_cleanup() fix, A/B against the pre-fix binary. Worth noting the board was already sitting at 0 0 0 0 when I logged in — something on the boot path had run ipcinfo and left it there, which is the bug in the field rather than in theory:

baseline    : 7 4 1 7
after OLD -i: 0 0 0 0
restored    : 7 4 1 7
after NEW -i: 7 4 1 7

The sensor-clock half I could not demonstrate either way: CRG8464=0x00004010 CRG8472=0x0000A010 before and after both binaries, because the streamer is running and the clock was already enabled, so enable_sensor_crg() had nothing to change and the changed flags stayed false. That is the no-op path, not a demonstration of the restore. The printk half is the one this board can show.

Still unproven: per-die versus per-model. This CV608 is the only gen5 part here and re-reading it returns the same four words.

[openipc-ai]

Approving at 38ba095.

The OTP base is now backed by both vendor SDKs for all five HISI_OT parts, the degenerate-id reject sits in the dispatcher and tests the emitted digits so both arms honour it, both readers take size_t with length prechecks, and ipcinfo restores printk and the sensor clock through atexit().

Verified here: host build clean apart from the pre-existing 64-bit warnings, cYAML_test passes, and the PR Build Check is green on arm32, arm64 and mips32. Your CV608 re-run shows byte-identical output and the printk A/B. Per-die versus per-model remains a hardware question, not a blocker.