review: eleven rules a board-support PR walked straight through

.pr_agent.toml

@@ -49,6 +49,16 @@ Prioritise, in order:
    family. A sensor, I2C address, GPIO number, or resolution that is true only of the
    contributor's bench must not become the default for everyone else. This breaks already
    deployed cameras at upgrade time, which is the worst failure mode this project has.
+   "Additive, so it changes no existing board" is the right exemption for a load_<vendor>
+   sensor case arm and is not a licence for the shared overlay: etc/wireless/usb has
+   accumulated 46 arms keyed on retail model names while 18 builder devices override the
+   whole file, and those 46 are grandfathered, not precedent.
+   Question a devmem write in a shipped script. It used to be the only way to tell a
+   camera something is soldered to a pad; majestic now owns the pads through its pins
+   configuration and /api/v1/pinmux, which put the choice back at startup and after a
+   reload, and drive a PWM lamp without anyone computing a duty cycle. A devmem line
+   under general/overlay/ has to say why configuration cannot express it and - since
+   rc.local is shared - why every other camera should execute it.
 
 2. Toolchain-wide flags added without measurement. BR2_TARGET_OPTIMIZATION is appended to
    TOOLCHAIN_WRAPPER_OPTS in Buildroot's toolchain/toolchain-wrapper.mk, so it is baked
@@ -65,14 +75,40 @@ Prioritise, in order:
    URL the .mk actually fetches. Binaries lifted out of a camera's factory firmware are
    not a source of supply: there is no source, no vendor SDK, and no way to rebuild them
    for the next kernel.
+   Detect a binary by the DIFF MARKER, never the extension. A hunk rendered as
+   "Binary files a/... and b/... differ" is a blob whatever it is named - an executable
+   with no extension at all reads to a text-diff summariser as "contains no textual
+   changes", and that is how one shipped past review into general/overlay/, which is
+   copied verbatim into every image. Report it, and say you cannot see its contents.
+   A register initialisation table copied out of a camera vendor's own driver is the same
+   provenance failure written in C: unverifiable against a datasheet, and replacing a
+   whole table rather than adding the deltas a board needs changes every other board
+   using that sensor.
+   A new *.patch is fine against a third-party upstream (every patch in the tree today is
+   - ffmpeg, mbedtls, vtund, libre, siproxd, zerotier, libwebsockets, f2fs-tools, the
+   Realtek drivers) and is a finding when the package's *_SITE resolves to an openipc
+   repository, because there the fix has somewhere better to go and a *_VERSION bump
+   silently drops it.
 
 4. Changes that belong to a different repository. Kernel code and kernel patches belong to
    OpenIPC/linux. Support for one retail camera model belongs to OpenIPC/builder, under
    devices/common/br-ext-chip-<vendor>/ - general/overlay/usr/sbin/sysupgrade already
    encodes this split, routing lite|ultimate|neo to firmware and everything else to
-   builder. Hardware probing and bring-up tools belong to OpenIPC/ipctool. Bugs in the
-   majestic streamer belong to majestic's maintainers. Redirecting a contributor is a
+   builder. Hardware probing and bring-up tools belong to OpenIPC/ipctool. HiSilicon
+   sensor drivers and open SDK code belong to OpenIPC/openhisilicon, which is what
+   hisilicon-opensdk fetches; Sigmastar sensor drivers belong to OpenIPC/sensors. Bugs in
+   the majestic streamer belong to majestic's maintainers. Redirecting a contributor is a
    normal, useful review outcome; say which repo and why.
+   Say which SEAM too, not only which repo. The tree already has one for every
+   board-specific thing, and a contributor who has not been shown it writes a new init
+   script with the pin numbers typed in: /usr/share/openipc/customizer.sh and
+   /usr/share/openipc/muxes.sh (both run by S30customizer), the declarative pin map
+   /usr/share/openipc/gpio.conf, late-overlays.list, and
+   general/scripts/excludes/<model>_<variant>.list. Builder ships all of them per device
+   at those same paths under devices/<board>/. An excludes list added to THIS repo is not
+   per-board - the key is <model>_<variant>, which the generic board answers to as well.
+   A comment explaining why a workaround is needed has usually already named the
+   repository that owns the fix. Read it as a redirect and quote it back.
 
 5. Monkey-patching in place of a fix. Dynamic-linker injection shims - a library forced
    ahead of the real one through the linker's LD_* environment knob, spelled out in
@@ -109,6 +145,28 @@ Prioritise, in order:
    builds is not compiled by CI, so nothing proves it even builds. And a diff must do what
    its title says: a PR adding one sensor has no business repointing a SITE that every
    board of that vendor consumes.
+   The overlay's mirror image of this needs saying separately, because general/overlay/
+   needs no Config.in and no .mk - anything dropped there ships, so a file nothing on the
+   camera ever opens passes every dead-code check while costing every board of every
+   vendor, several of which sit within 32 KB of their squashfs cap. Grep the installed
+   path of an added overlay file; if nothing reads it, that is a finding.
+
+8. Commands a shipped script calls must exist on the image. A camera has no package
+   manager and often no network, and a missing command does not fail loudly - sh prints
+   "not found" to a console nobody reads and carries on, so the script reports success
+   having done nothing. Busybox applets and binaries installed by a package the board's
+   defconfig selects are the whole set. ipctool is the trap that looks like an exception:
+   general/package/ipctool/ipctool.mk installs ipcinfo and nothing else, so
+   BR2_PACKAGE_IPCTOOL=y puts no "ipctool" on the image; /usr/sbin/ipctool is a symlink to
+   extutils, whose ipctool arm downloads the tool from the ipctool repository's "latest"
+   release into /tmp on first use. That is a convenience for a person at a shell, and in
+   a boot script it is a network fetch during boot, from a floating tag, repeated every
+   boot because /tmp is tmpfs, and a silent no-op until the network is up.
+   Relatedly, a shipped script bounces the streamer with /etc/init.d/S95majestic restart.
+   A hand-rolled killall-and-relaunch loses the -s argument, the start-stop-daemon -x
+   lookup, the ten-second wait for the old process to release the sensor HAL, and the
+   trap '' HUP that S95majestic carries because the disposition survives exec() where a
+   handler does not.
 
 Do not report: speculation about whether a change was written with AI assistance - judge
 the diff on blast radius, provenance, evidence, and repo ownership, never on writing

CLAUDE.md

@@ -35,6 +35,8 @@ else. Check this first — it costs a minute and can save an evening.
 | Kernel source and kernel patches | [OpenIPC/linux][linux] |
 | Support for one specific retail camera model | [OpenIPC/builder][builder] |
 | Probing, identification, bring-up tooling | [OpenIPC/ipctool][ipctool] |
+| HiSilicon sensor drivers and open SDK code | [OpenIPC/openhisilicon][openhisilicon] |
+| Sigmastar sensor drivers | [OpenIPC/sensors][sensors] |
 | A bug in the video stream itself | majestic's maintainers, not a shim here |
 | Documentation, device notes, how-tos | [OpenIPC/wiki][wiki], [docs][docs] |
 
@@ -148,15 +150,31 @@ Summarised from `pr_compliance_checklist.yaml`; the reasoning is in `best_practi
   every caller, is invisible to anyone debugging the process, and freezes the real bug in place.
 - **No binaries without buildable source.** Nothing lifted out of a factory image, nothing a
   script in the same pull request generated. A `PROVENANCE.md` documents the problem; it does not
-  solve it.
+  solve it. The test is the diff marker — anything rendered `Binary files ... differ` — not the
+  extension; an executable with no extension is still a blob, and `general/overlay/` is never the
+  place for one. A register table copied out of a camera vendor's driver is the same problem
+  written in C.
+- **No patches against OpenIPC's own packages.** Every `*.patch` in the tree targets a third-party
+  upstream. If a package's `*_SITE` is an `openipc` repository, the fix is a pull request there
+  and a `*_VERSION` bump here.
+- **Nothing ships that nothing runs.** An overlay file needs no `Config.in`, so nothing catches a
+  file the camera never opens. Grep the installed path before adding one.
+- **Shipped scripts call only what the image contains.** Busybox applets, or binaries a package in
+  the board's defconfig installs. `ipctool` looks installed and is not: the package ships
+  `ipcinfo`, and `/usr/sbin/ipctool` is an `extutils` arm that downloads the tool at first use.
 - **No runtime patching of vendor blob memory**, and no `kallsyms` address hooking.
 - **`*_SITE` points at an OpenIPC-org repository or a documented upstream**, never a personal
   fork, and a `*_VERSION` bump never becomes less specific than the pin it replaces — a full
   40-character SHA for anything new. `Config.in` help text must name the URL the `.mk` fetches.
 - **No board-specific value in shared files.** A sensor name, I2C address, GPIO number,
   resolution, MAC prefix or IP literal does not belong in `general/overlay/` or in a shared
-  `load_<vendor>` default. Extending a case arm, or adding a sensor to a package's list, is
-  additive and fine.
+  `load_<vendor>` default. Extending a `load_<vendor>` case arm, or adding a sensor to a
+  package's list, is additive and fine; that exemption does not extend to the overlay.
+  Per-board values have their own seams — `/usr/share/openipc/customizer.sh` and
+  `/usr/share/openipc/muxes.sh`, both run by `S30customizer`; the declarative pin map
+  `/usr/share/openipc/gpio.conf`; `late-overlays.list`; and
+  `general/scripts/excludes/<model>_<variant>.list`. All of them live per-device in
+  OpenIPC/builder under `devices/<board>/`, at the same paths.
 
 ## Verifying a change
 
@@ -341,4 +359,6 @@ Maintainers append the pull request number at merge time; you do not need to.
 [docs]: https://docs.openipc.org
 [ipctool]: https://github.com/OpenIPC/ipctool
 [linux]: https://github.com/OpenIPC/linux
+[openhisilicon]: https://github.com/OpenIPC/openhisilicon
+[sensors]: https://github.com/OpenIPC/sensors
 [wiki]: https://github.com/OpenIPC/wiki