Ask which chip it is, and which layout goes on it - #130
Conversation
A camera with a ruined overlay could not be revived by a full reflash. Its flash was 16MB, the 8MB entry had been chosen in the wizard, and the instructions erased 0x0..0x800000 and stopped. Every OpenIPC mtdparts ends `-(rootfs_data)`, meaning to the end of the device, so the overlay ran to 0x1000000 and the half above the erase survived. /init mounts a jffs2 it finds and only reformats when that mount fails, so the old, broken overlay came back every boot. The reporter got out of it by generating the 8MB page for its image, the 16MB page for its `sf erase 0x0 0x1000000`, and editing the filename by hand. The menu was one field doing two jobs: how big the chip is, and how it is carved up. Those are not always the same answer -- nor32m was already the exception, rewritten to the nor16m command set because no bootloader upstream defines mtdpartsnor32m. So split them. The flash type stays the chip and keeps its permanent-link key; a second menu picks the layout and defaults to the chip's own, which is what every combination the form could produce before this already had. The assembled image is the size of the chip whatever layout goes inside it, so the erase spans the whole part and there is nowhere for a stale overlay to survive. Everything that used to key on the chip and meant the layout now says so: FlashLayout's offsets, the uk/ur/set macro names, and the rule that keeps Ultimate off a 5120KB rootfs partition -- which is the layout's doing and not the chip's, so it now applies to the 8MB layout on a 32MB part too. Images already cached keep their names: the layout only enters the filename when it is not the one the chip would have had anyway.
PR Summary by QodoSeparate flash chip size from partition layout
AI Description
Diagram
High-Level Assessment
Files changed (19)
|
Code Review by Qodo
1.
|
Qodo found three, and driving the wizard script through a DOM stub found a fourth that was mine. **The layout stopped following the chip.** The form opens on nor8m, and nor8m is a legal layout on a larger part, so `checkPartitionLayout` never took it away: picking NOR 16M left 8MB partitions on a 16MB camera and the edition limiter took Ultimate off with them. It now follows the chip until the visitor settles it themselves -- by using the menu, or by arriving on a permanent link that carries a `part` that is not the chip's own, which is as deliberate as clicking. **The rootfs was bounded by the image, not by its partition.** `layout` gave the rootfs part the whole chip-sized image as its limit. That was near enough while the layout was the chip -- an 8MB image ends 0xb0000 past the rootfs partition -- and is not now: a 16MB image laid out the 8MB way ends 0x8b0000 past it, and `download_full_image` takes the edition and the layout straight from the query string. `?fw_release=ultimate&flash_size=16&layout=8` would have written a 7MB rootfs from 0x250000 clean through rootfs_data. It is bounded by the overlay offset now, which is what the mtdparts says the partition is. **"This SoC needs a larger chip" reached chips that are large enough.** The Ultimate guard became the layout's, so the no-Lite branch under it now sees 16MB and 32MB parts, where the advice is not to buy a chip but to choose the other layout. It tells the two apart, like the warning beside it already did. **NAND lost Ultimate.** The layout menu keeps its value while it is hidden, so `checkPartitionLayout` handed back nor8m on a NAND part and the 8MB rootfs rule disabled an edition that eleven of the sixteen NAND boards are published as and nothing else. It answers with no layout where there is none to choose.
* Give SigmaStar and Ingenic the layout their bootloader has `FlashLayout::EIGHT_MEG_LAYOUT_VENDORS = %w[SigmaStar Ingenic]` was removed in #78 on the grounds that their bootloaders define mtdpartsnor16m and the uknor16m/urnor16m pair "down to the byte", the same as the HiSilicon and Goke ones. The repositories that carry those definitions -- u-boot-msc313e, u-boot-t20, u-boot-t40 -- are real, and no released binary is built from any of them, so no camera runs them. OpenIPC/firmware's .github/workflows/uboot.yml is what builds the bootloaders this site links. It clones openipc/u-boot-sigmastar and openipc/u-boot-ingenic and runs their build.sh; the SigmaStar one's spinor loop is `ssc377 ssc377d ssc377de ssc377qe ssc378de ssc378qe` against include/configs/infinity6c.h, which includes configs/sstar-common.h. That header is the whole environment every SSC3xx NOR camera boots with: kernaddr=0x50000 kernsize=0x200000 rootaddr=0x250000 rootsize=0x500000 rootmtd=5120k uknor / urnor / ubnor CONFIG_BOOTARGS "... mtdparts=NOR_FLASH:256k(boot),64k(env),2048k(kernel), ${rootmtd}(rootfs),-(rootfs_data) ..." A repo-wide grep for uknor8m|uknor16m|urnor8m|urnor16m|setnor8m|setnor16m| mtdpartsnor returns nothing in either repo. u-boot-ingenic's isvp_common.h carries the same unsuffixed uknor/urnor. **The 16MB image cannot boot.** One mtdparts string, a 2048k kernel inside it, and only ${rootmtd} varying: the rootfs starts at 0x250000 at every chip size. Handing these two NOR[16] put it at 0x350000. The image openipc.org serves for ssc377qe today has "hsqs" at 0x350000 and 0xff at 0x250000, and its env region at 0x40000 is blank -- a full-image flash erases the env, so the camera comes up on the compiled-in bootargs, looks for the rootfs at 0x250000, finds erased flash and panics on root mount. The 8MB-layout image of the same page has it at 0x250000 and boots. Since #130 the 16MB layout is what a 16MB chip selects by default, so this is what a SigmaStar visitor gets by pressing the button. **None of the macros exist.** `run setnor16m`, `run uknor16m; run urnor16m` and the printenv hint under them name variables no SigmaStar or Ingenic camera has. U-Boot answers `## Error: "uknor16m" not defined` and flashes nothing, with no failure the reader can see. This predates #78 -- the expert section has emitted suffixed macros for these vendors since it existed. So the pin was right for these two, even though its own comment gave a wrong reason for it. What replaces it is the same fact stated properly: they have one mtdparts, so 16MB means rootmtd=10240k at the same offset rather than a rootfs 1MB further up, and their macros are `uknor`/`urnor` with nothing after them. That keeps Ultimate reachable, which a flat pin to the 8MB numbers would not: its rootfs is 7832KB on ssc338q, 7252KB on ssc30kq and 6772KB on t31 against the 5120KB rootmtd defaults to. rootmtd is a plain variable, saved unexpanded and expanded at boot by `cmdnor`, so the step that a macro does elsewhere is a setenv here: setenv rootmtd 10240k; setenv rootsize 0xA00000 saveenv reset and where the layout is already the bootloader's own, that step is dropped rather than rendered as a `run setnor8m` under a heading calling it required. @flash_type_command is gone with it. It was the layout name, which is the macro suffix only for the bootloaders that name their macros after the layout, and the view read it in four places -- so the offsets and the commands could disagree. Camera answers both now, off the same suffix. * Assert the 16MB agreement per vendor rather than once for everybody The test that pinned every vendor to 0x350000 was written when every vendor had the same answer. It still checks the thing it was for -- that the image and the page cannot describe different layouts -- but reads the offset each bootloader is actually on. A model of its own per vendor with it: the cache filename carries the model and not the vendor, so one model built under three vendors has the second and third read back the first's image. Nothing in production can hit that, a model belongs to one vendor, but a test that did would pass on a stale file.
The report
A user could not revive a camera with a ruined overlay, however many times they
flashed the full image. It took them five minutes to work out why, and they
noted that a less experienced person would simply be stuck.
Their flash was 16MB and they had chosen the 8MB entry in the wizard. The
instructions that came out erase
0x0..0x800000and stop. Every OpenIPCmtdparts ends
-(rootfs_data)— to the end of the device — so on a 16MB partthe overlay runs to
0x1000000, and the half above the erase was never touched.general/overlay/initmounts a jffs2 it finds and only callsflash_eraseallwhen that mount fails, so the old, broken overlay came back on every boot.
They got out of it by generating the 8MB page for its image, the 16MB page for
its
sf erase 0x0 0x1000000, and editing16to8in the filename by hand.The cause
One menu was answering two questions: how big the chip is, and how it is carved
up. Those are not always the same answer.
nor32mwas already the exceptionthat proved it — the controller rewrote it to the
nor16mcommand set becauseno bootloader upstream defines
mtdpartsnor32m— so 32MB chip + 16MB layoutwas expressible while 16MB chip + 8MB layout was not.
The change
The flash-type menu still means the chip and keeps its
?rom=permanent-linkkey. A second menu picks the partition layout and defaults to the chip's own, so
every configuration the form could produce before this renders identically.
The assembled image is the size of the chip whatever layout goes inside it, so
the erase spans the whole part and a stale overlay has nowhere to survive.
Three things that were already wrong fell out of the split:
nor32m → nor16mrewrite is gone. It only ever meant "a 32MB chip wearsthe 16MB layout", which
Camera#partition_layoutnow says directly.That is the layout's doing, so it now catches the 8MB layout on a 32MB part
too, and the message names whichever of the two is actually the limit.
Images already cached keep their names: the layout only enters the filename when
it is not the one the chip would have had anyway
(
openipc-<soc>-nor-lite-16mb-parts8m.bin).Verification
the chip-sized erase under a smaller layout, the filename rule, and
Firmware's validation of a layout larger than the chip.baseline. The test files came out four under.
i18n-tasks missingclean; en/ru/zh all carry the new strings.U-Boot commands read by hand, including NAND (layout menu hidden,
uknand/urnandunchanged) and both refusal paths.Noted, not fixed here
Camera#flash_size_blocksanswers0x16000fornor32m— decimal 65536 with ahex prefix, where it should be
0x10000. It would make the SD-card backupmmc write44MB from a 32MB read. It renders for nobody today, because the twoselects that reach that path are commented out of the form.