IPCHW_HISI takes its default from IPCHW_VENDORS

.github/workflows/pr-build-check.yml

@@ -208,3 +208,78 @@ jobs:
             fi
           done
           exit $status
+
+      - name: IPCHW_HISI's default follows IPCHW_VENDORS
+        # A default is the kind of thing that drifts without anyone noticing,
+        # and the two spellings that must NOT move are the two ipcinfo is
+        # built as: its own "all", and the firmware package's "none", which
+        # means only the always-in vendors and so IS a HiSilicon build. A
+        # silently HiSi-blind ipcinfo would be worse than a large one.
+        #
+        # Configure only -- this asserts the cache value, not a binary.
+        run: |
+          status=0
+          check() { # <vendors|-> <hisi|-> <expected>
+            d=$(mktemp -d)
+            args=(-H. -B"$d" -DSKIP_VERSION=ON -DONLY_LIBRARY=ON)
+            [ "$1" != "-" ] && args+=(-DIPCHW_VENDORS="$1")
+            [ "$2" != "-" ] && args+=(-DIPCHW_HISI="$2")
+            if ! cmake "${args[@]}" > "$d/log" 2>&1; then
+              echo "::error::configure failed for VENDORS=$1 HISI=$2"
+              tail -20 "$d/log"; status=1; rm -rf "$d"; return
+            fi
+            got=$(grep '^IPCHW_HISI:STRING=' "$d/CMakeCache.txt" | cut -d= -f2-)
+            if [ "$got" = "$3" ]; then
+              printf 'ok    VENDORS=%-16s HISI=%-8s -> %s\n' "$1" "$2" "$got"
+            else
+              echo "::error::VENDORS=$1 HISI=$2 gave IPCHW_HISI=$got, wanted $3"
+              status=1
+            fi
+            rm -rf "$d"
+          }
+          check -              -      all   # ipctool's own default
+          check all            -      all   # ipcinfo's default: must not move
+          check none           -      all   # firmware's ipcinfo: a HiSi build
+          check ingenic        -      none  # a narrowing implies no HiSilicon
+          check sstar          -      none
+          check "ingenic;sstar" -     none
+          check ingenic        all    all   # an explicit value always wins
+          check ingenic        v4     v4
+          check all            none   none
+
+          # And the same question asked of ONE build directory, reconfigured.
+          # This is where the first cut of this was wrong: the derived value
+          # lands in the cache, and on the next configure it is
+          # indistinguishable from one the user typed -- so a tree first
+          # configured with a narrowed vendor kept IPCHW_HISI=none when it was
+          # later broadened to `all`, and that build could not identify a
+          # HiSilicon camera. Build directories are reused across boards.
+          d=$(mktemp -d)
+          step() { # <cmake args...> <expected>
+            exp="${!#}"; set -- "${@:1:$#-1}"
+            if ! cmake -H. -B"$d" -DSKIP_VERSION=ON -DONLY_LIBRARY=ON "$@" \
+                 > "$d/log" 2>&1; then
+              echo "::error::reconfigure failed: $*"; tail -20 "$d/log"
+              status=1; return
+            fi
+            got=$(grep '^IPCHW_HISI:' "$d/CMakeCache.txt" | cut -d= -f2-)
+            if [ "$got" = "$exp" ]; then
+              printf 'ok    reconfigure %-38s -> %s\n' "$*" "$got"
+            else
+              echo "::error::reconfigure $* gave IPCHW_HISI=$got, wanted $exp"
+              status=1
+            fi
+          }
+          step -DIPCHW_VENDORS=ingenic none   # narrowed
+          step -DIPCHW_VENDORS=all     all    # broadened: must follow
+          step -DIPCHW_VENDORS=sstar   none   # narrowed again
+          step -DIPCHW_VENDORS=none    all    # a HiSilicon build
+          rm -rf "$d"; d=$(mktemp -d)
+          step -DIPCHW_VENDORS=all -DIPCHW_HISI=v4 v4  # explicit
+          step -DIPCHW_VENDORS=ingenic v4     # and it survives a vendor change
+          rm -rf "$d"; d=$(mktemp -d)
+          step -DIPCHW_VENDORS=ingenic none
+          step -DIPCHW_HISI=all        all    # explicit on a LATER configure
+          step -DIPCHW_VENDORS=sstar   all    # still theirs
+          rm -rf "$d"
+          exit $status

CLAUDE.md

@@ -52,8 +52,29 @@ CMake knobs worth knowing:
   dynamic/ASAN build (use a glibc cross toolchain that ships libasan; musl
   toolchains do not).
 - `-DIPCHW_VENDORS=all|none|"sstar;ingenic"` selects which vendor HALs go into
-  `libipchw`. HiSilicon is always in. The `ipctool` executable always carries
-  every vendor.
+  `libipchw`. HiSilicon is always in — it is not in this knob's vocabulary,
+  because `chipid.c` reaches it directly rather than through the vendor table.
+  The `ipctool` executable always carries every vendor.
+- `-DIPCHW_HISI=all|none|"v4"` selects which HiSilicon *generations* `libipchw`
+  can identify — the chip-ID table, a sensor-bus back-end per generation, and
+  the temperature and die-ID readers. Roughly 8 KB on arm32 for the lot.
+
+  **Its default follows `IPCHW_VENDORS`**, because naming your silicon should
+  not have to be done twice. A narrowing — `-DIPCHW_VENDORS=ingenic` — says the
+  binary will never boot on a HiSilicon part, so `IPCHW_HISI` defaults to
+  `none`. The two spellings that are not a narrowing keep every generation:
+  `all` means "identify any camera this is dropped on", and `none` means only
+  the always-in vendors, which *is* HiSilicon. Both are how `ipcinfo` is built
+  (its own default and the firmware package's `none`), so neither moved when
+  this was added; a narrowed consumer lost 8.3 KB. An explicit `-DIPCHW_HISI`
+  always wins.
+
+  Do not reach for weak symbols or a section registry to replace this knob: it
+  was tried, and it works by *not* pulling `hal_hisi.c.o` from the archive,
+  which deletes HiSilicon detection from every static consumer including the
+  ones that want it. "Which silicon can this build identify" is a runtime
+  question the linker cannot answer — nothing here is dead code as far as it
+  knows.
 - `-DIPCHW_PADMUX=all|none|"v1;v4;sstar"` selects which SoC families' pad-mux
   tables go into `libipchw`. `sstar` and `ingenic` are families here too: the
   vendor knob answers "can this build detect the SoC", this one answers "does

CMakeLists.txt

@@ -114,20 +114,96 @@ foreach(_f sstar ingenic)
 endforeach()
 set(IPCHW_PADMUX_BACKENDS_ALL src/hal/sstar_padmux.c src/hal/ingenic_padmux.c)
 
-# Which HiSilicon generations libipchw can identify. IPCHW_VENDORS cannot
-# reach any of this: chipid.c dispatches to the HiSilicon HAL directly rather
-# than through the vendor table, so hal_hisi.c has always been compiled in
-# whole -- the chip-ID table for all ten generations, a sensor-bus back-end
-# per generation, and the temperature and die-ID readers for each. A consumer
+# Which HiSilicon generations libipchw can identify. IPCHW_VENDORS does not
+# reach the CODE: chipid.c dispatches to the HiSilicon HAL directly rather
+# than through the vendor table, so hal_hisi.c is compiled in whole whenever
+# any generation is selected -- the chip-ID table, a sensor-bus back-end per
+# generation, and the temperature and die-ID readers for each. A consumer
 # built against one vendor SDK runs on one generation of one family.
 #
+# It does reach the DEFAULT, though, and that is new. See below.
+#
 # Same shape and vocabulary as IPCHW_PADMUX above, and "none" is meaningful
 # here too: a SigmaStar or Ingenic consumer drops the HiSilicon HAL entirely.
 # The ipctool executable always carries every generation.
 set(IPCHW_HISI_GENERATIONS v1 v2 v2a v3 v3a v4 v4a v5 3536c 3536d)
-set(IPCHW_HISI "all" CACHE STRING
+
+# The default follows IPCHW_VENDORS, because a consumer that has named its
+# silicon has already answered this question and should not have to answer it
+# twice.
+#
+# HiSilicon is not in IPCHW_VENDORS' vocabulary -- it is the always-in vendor
+# -- so `-DIPCHW_VENDORS=ingenic` says, unambiguously, that this binary will
+# never boot on a HiSilicon part. It used to keep all ten generations anyway,
+# and majestic paid about 7 KB per board on every Ingenic, SigmaStar,
+# Rockchip, Novatek, Fullhan, GrainMedia, Xiongmai, Allwinner and v4l2 build
+# for it, purely for not knowing a second knob existed.
+#
+# The two spellings that are NOT a narrowing keep every generation:
+#
+#   all   "identify any camera this is dropped on" -- which includes HiSi.
+#   none  only the always-in vendors, which IS HiSilicon. A HiSilicon build.
+#
+# Both are what ipcinfo is built as (its own default, and the firmware
+# package's `IPCHW_VENDORS=none`), so neither moves. That matters: ipcinfo
+# exists to name an unknown camera, and a silently HiSi-blind ipcinfo would be
+# worse than a large one.
+#
+# An explicit -DIPCHW_HISI wins -- this only fills in a value nobody gave.
+#
+# "Nobody gave" has to survive a SECOND configure of the same build directory,
+# which is the whole difficulty. A derived value goes in the cache, and on the
+# next run it is indistinguishable from one the user typed -- so a plain
+# `if(NOT DEFINED IPCHW_HISI)` would derive `none` for
+# `-DIPCHW_VENDORS=ingenic`, and then keep it when that same tree was
+# reconfigured with `-DIPCHW_VENDORS=all`. A build asking to identify any
+# camera would quietly fail to identify a HiSilicon one. Build directories are
+# reused across boards; this is not hypothetical.
+#
+# So remember what the derivation last wrote. If the cache still holds exactly
+# that, nobody has overridden it and it is ours to update; if it holds anything
+# else, the user said so and we leave it alone.
+#
+# The one case this cannot tell apart is an explicit -D that happens to match
+# what would have been derived anyway. CMake gives no way to distinguish that
+# from the cached derivation, and the cost is that changing IPCHW_VENDORS in
+# that tree also moves IPCHW_HISI -- re-pass -DIPCHW_HISI to pin it.
+set(_hisi_default "all")
+if(NOT IPCHW_VENDORS STREQUAL "all" AND NOT IPCHW_VENDORS STREQUAL "none")
+  set(_hisi_default "none")
+endif()
+
+if(NOT DEFINED IPCHW_HISI)
+  # Nobody has said anything yet.
+  set(_hisi_value ${_hisi_default})
+  set(_hisi_derived TRUE)
+elseif(DEFINED _IPCHW_HISI_AUTO AND IPCHW_HISI STREQUAL "${_IPCHW_HISI_AUTO}")
+  # Still carrying exactly what we derived last time, so it is ours to update
+  # and IPCHW_VENDORS may have moved under it.
+  set(_hisi_value ${_hisi_default})
+  set(_hisi_derived TRUE)
+else()
+  set(_hisi_value ${IPCHW_HISI})
+endif()
+
+# Unconditionally, and FORCE: in the explicit case the value is already the
+# user's, and this is what gives the entry its type and docstring. A bare -D
+# creates it UNINITIALIZED, and leaving it that way hides it from ccmake and
+# from anything that reads the cache by type.
+set(IPCHW_HISI ${_hisi_value} CACHE STRING
     "HiSilicon generations to compile into libipchw: 'all', 'none', or a \
-;-separated subset of: ${IPCHW_HISI_GENERATIONS}")
+;-separated subset of: ${IPCHW_HISI_GENERATIONS}" FORCE)
+
+if(_hisi_derived)
+  set(_IPCHW_HISI_AUTO ${_hisi_default} CACHE INTERNAL
+      "The IPCHW_HISI this project derived, so a later configure can tell it \
+from one the user set")
+else()
+  # Explicitly set: stop tracking, so it is never rewritten from under them.
+  unset(_IPCHW_HISI_AUTO CACHE)
+endif()
+unset(_hisi_value)
+unset(_hisi_default)
 
 if(IPCHW_HISI STREQUAL "all")
   set(_hisi_selected ${IPCHW_HISI_GENERATIONS})
@@ -163,7 +239,14 @@ if(_hisi_selected)
   list(APPEND IPCHW_HISI_DEFS IPCHW_HISI_ANY)
 endif()
 list(APPEND IPCHW_HISI_ALL_DEFS IPCHW_HISI_ANY)
-message(STATUS "libipchw HiSilicon generations: ${_hisi_selected}")
+if(_hisi_derived AND IPCHW_HISI STREQUAL "none")
+  message(STATUS "libipchw HiSilicon generations: none "
+                 "(derived from IPCHW_VENDORS=${IPCHW_VENDORS}; "
+                 "set IPCHW_HISI explicitly to override)")
+else()
+  message(STATUS "libipchw HiSilicon generations: ${_hisi_selected}")
+endif()
+unset(_hisi_derived)
 
 # Split either side of where the HiSilicon HAL sits so that a build carrying
 # it hands the archiver the same member order as before: --gc-sections keeps