ipctool: build ipcinfo for the board it ships on - #2432
Conversation
PR Summary by QodoBuild board-specific ipcinfo binaries
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1. Non-HiSilicon firmware fails to build
|
|
Parked as draft behind #2433. This PR's CI is red on #2433 takes all three boards back under their caps. Once it lands I will rebase this onto the new master, where its own saving stacks on top. |
ipcinfo links libipchw, which knows every SoC vendor and every HiSilicon generation ipctool has ever learned: a detection table for eleven vendor HALs, and a chip-ID table, a sensor-bus back-end, a temperature formula and a die-ID reader for each of ten HiSilicon generations. A camera is one SoC. The rest is code that cannot execute on it, and it ships on all but three of this tree's board configs, onto boards whose rootfs cap leaves tens of KB spare -- gk7205v300_lite sits at 5080KB of 5120KB. Upstream takes both as build options defaulting to "all" (ipctool #178 and #211), and majestic already derives them the same way from its VENDOR and SDK code. This derives them from OPENIPC_SOC_VENDOR and OPENIPC_SOC_FAMILY. The family names are not a coincidence: ipctool's getchipfamily() returns the same strings, so `ipcinfo -f` on a board prints the key the table is indexed by. A vendor that is not HiSilicon reaches hal_hisi through nothing -- the only route is a HiSilicon UART0 base in chipid.c's dispatch -- so those boards drop hal_hisi.c and ispreg.c from the library altogether. Checked rather than assumed, on the two non-HiSilicon lab boards: ssc30kq has no uart line in /proc/iomem at all, and t31 reports 0x10031000, which is not one of the six bases that dispatch (and is one digit from xm510's 0x10030000 without being it). Two families are deliberately unmapped. The 3520DV200 ID sets no chip_generation, and no ID in ipctool's table matches a GK7101 or GK7102, so neither has a generation to name -- they keep every one rather than be trimmed on a guess. Unmapped means nothing is passed and upstream's default applies, so ambarella, anyka and ti keep every HAL too, and a new SoC is never silently trimmed to the wrong thing. Nothing derives IPCHW_SENSORS. A mainline image is per-family and gets flashed onto whatever camera someone bought; `ipcinfo --short-sensor` exists to name a sensor nobody catalogued, and it runs once, on an unprovisioned camera, with the answer written to U-Boot env. Compiling a probe family out turns that into "SENSOR is not detected, aborting". IPCHW_PADMUX is left alone too: --gc-sections already drops it from ipcinfo, and libipchw carries its selection as a PUBLIC compile definition, so setting it would break config_tool in sigmastar-osdrv-infinity6 on ssc325_lite and ssc325de_lite. CONF_OPTS only, never DEPENDENCIES: a _DEPENDENCIES line would add an edge to the graph ci-matrix.py walks and move its frozen board counts. Measured on gk7205v300_lite, same tree and commit, only this file differing: ipcinfo 34916 -> 22036 bytes -12880 (-37%) rootfs.squashfs 5080 -> 5076 KB -4KB, headroom 40KB -> 44KB Verified on a hi3516ev200 (V4, SC2315E, musl armv7), which takes the same options this file gives the Goke family: -c -f -v -s -l -F -i -S -x, every long form, the combined -ci that extutils uses, and -t all answer exactly as the stock 35124-byte binary does. ci-matrix --self-test passes and the frozen counts are unchanged; touching this package builds every board, which is the coverage this wants.
8b692f4 to
5a92725
Compare
|
Rebased onto master now that #2433 has landed (ce3cbef). Git dropped the three size commits as already-upstream, so this is back to a single The size picture it lands into, from #2433's own CI:
That last row is why this one still matters beyond its own merits: gk7205v300_lite is the tightest board of the three and is the only one still tripping the 32KB headroom warning. Its gadget lever was spent in #2421 and it has no staging r8188eu to drop, so the ipcinfo trim here is the next increment available to it. |
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
Code review by qodo was updated up to the latest commit 5a92725 |
|
Code review by qodo was updated up to the latest commit 5a92725 |
1 participant
ipcinfois 34,916 bytes on agk7205v300_liteimage whose rootfs sits at 5080 KB of a 5120 KB cap. It linkslibipchw, which knows every SoC vendor and every HiSilicon generation ipctool has ever learned — a detection table for eleven vendor HALs, plus a chip-ID table, a sensor-bus back-end, a temperature formula and a die-ID reader for each of ten HiSilicon generations. A camera is one SoC.Upstream takes both as build options defaulting to
all(ipctool#178, #211); majestic already derives them the same way from itsVENDORand SDK code. This derives them fromOPENIPC_SOC_VENDORandOPENIPC_SOC_FAMILY.The family names are not a coincidence — ipctool's
getchipfamily()returns the same strings, soipcinfo -fon a board prints the key the table is indexed by.Measured
gk7205v300_lite, same tree and commit, only this file differing:Hardware
On a hi3516ev200 (V4, SC2315E, musl armv7), which takes the same options this file gives the Goke family —
-c -f -v -s -l -F -i -S -x, every long form, the combined-cithatextutilsuses, and-t, all against the stock 35,124-byte binary on the same box:What is dropped, and how that was checked rather than assumed
A vendor that is not HiSilicon reaches
hal_hisithrough nothing — the only route is a HiSilicon UART0 base inchipid.c's dispatch — so those boards drophal_hisi.candispreg.cfrom the library altogether. Verified on the two non-HiSilicon lab boards:/proc/iomemuartgeneric_detect_cpu→ sstar0x100310000x10030000, and not it)Worth recording the third: hi3516av300 reports
0x120a0000— a base ipctool comments as "hi3516ev200" — on a board that is V4A. That is real-hardware confirmation of why ipctool#211 gates those arms all-or-nothing rather than per generation.Two families deliberately unmapped
The
3520DV200ID sets nochip_generationat all, and no ID in ipctool's table matches a GK7101/GK7102 — neither has a generation to name, so they keep every one rather than be trimmed on a guess. Unmapped means nothing is passed and upstream's default applies, so ambarella, anyka and ti keep every HAL too, and a new SoC is never silently trimmed to the wrong thing.Two knobs deliberately not used
IPCHW_SENSORSexists upstream and is not touched. A mainline image is per-family and gets flashed onto whatever camera someone bought;ipcinfo --short-sensorexists to name a sensor nobody catalogued, and it runs once, on an unprovisioned camera, with the answer written to U-Boot env. Compiling a probe family out turns that intoSENSOR is not detected, aborting. The gk7205v200 driver set needs eight of the eleven families anyway (os02g10is found by the SuperPix probe, not OmniVision), so the whole knob was worth 536 bytes there.IPCHW_PADMUXis left alone:--gc-sectionsalready drops it fromipcinfo, andlibipchwcarries its selection as aPUBLICcompile definition, so setting it would breakconfig_toolinsigmastar-osdrv-infinity6onssc325_liteandssc325de_lite.Notes for review
CONF_OPTSonly, neverDEPENDENCIES— a_DEPENDENCIESline would add an edge to the graphci-matrix.pywalks and move its frozen board counts.--self-testpasses unchanged (99 boards, 136 packages, 56 cases), and touching this package builds every board, which is the coverage this wants.If you build locally and see no change, check the download cache.
IPCTOOL_VERSION = HEADand buildroot names the tarballipctool-HEAD.tar.gz, caching it by filename with no.hash, so a warmdl/keeps whatever HEAD was when that machine first built it — cmake then saysManually-specified variables were not used by the project: IPCHW_HISIand the binary comes out 26424 instead of 22036. It bit me mid-measurement. CI is unaffected: the "Refresh moving-ref package downloads" step added in #2352 deletes*-HEAD.tar.gzevery run.