Repository navigation
novatek: add U-Boot for NT98562 and NT98566 - #53
Merged
Merged
Conversation
NETDEVICES selected DM_ETH, so with CONFIG_DM off a board with a network driver failed to link on uclass and dev_get_priv references. Select it only when DM is on, as openipc-hisilicon and openipc-xm do, so the legacy designware_initialize() path still builds.
The firmware scan, recovery and boot-limit failsafe of the openipc-hisilicon branch, for the Anyka AK39 SoCs: common/openipc.c and include/openipc.h as on openipc-xm and openipc-gk710x (7208b96, itself from openipc-hisilicon 655d32f), unchanged. CONFIG_OPENIPC builds it; PRODUCT_SOC and PRODUCT_SOCMODEL name the target in the environment.
With MMC on, firmware_recovery() runs at every boot, and on NOR it did
not recover anything:
- Every fs_*() call closes the filesystem when it returns, but
recovery_mount() selected the card once: the fs_exists() worked and the
fs_size()/fs_read() after it failed ("Recovery: failed to stat
script.txt"). The medium found is now remembered and selected again
before each call.
- recovery_write() wrote a file to the flash offset in its uImage
header's load field, header included. A wrapper made for that
(`mkimage -T firmware -a 0x250000`) left its header in front of the
squashfs, so the scan found no rootfs; a kernel uImage as built went to
its RAM load address. Now a wrapper's payload goes to its offset without
the header, a kernel uImage or FIT as built goes to kernaddr, and a
squashfs as built to rootaddr; the kernel must end before rootaddr and
the rootfs within the flash.
- Erase and write errors were ignored and the file reported written.
The same code is on openipc-hisilicon, openipc-xm and openipc-gk710x,
where it has the same faults.
Under qemu-hisilicon's ak3918ev300, from an empty flash with only U-Boot:
a card with uImage.ak3918ev300.img and rootfs.squashfs.ak3918ev300.img,
as wrappers or as built, is written to 0x50000/0x250000 and boots to
login; script.txt runs on ak3918ev200 and ak3919ev330.
The rule is the uImage's: its payload goes to the flash offset in its load field, header stripped (mkimage -T firmware -a <offset>), whatever that offset is -- the card maker decides. The fallbacks of the previous commit (a kernel uImage as built to kernaddr, a raw squashfs to rootaddr) are dropped; a FIT kernel still goes to kernaddr. A card left in no longer reflashes NOR at every boot: an image the flash already holds is compared chunk by chunk and skipped (Qodo, #33). Under qemu-hisilicon's ak3918ev300, from an empty flash: the card's uImage.ak3918ev300.img (-a 0x50000) and rootfs.squashfs.ak3918ev300.img (-a 0x250000) are written and boot to login; the next boot with the card still in reports both "already in flash" and writes nothing.
The loader of Novatek's NT9856x Linux SDK v1.01.007 (nt9856x_loader_release_v1.01.007), as the SDK ships it: the boot flow sources (LibExt/LIBExt_src/Ctrl_Flow, Project/Model/Src), the headers, the make system, the DRAM parameter tables (Tools/ConfigRam/560), and what it links against and post-processes with, which Novatek ships as binaries: the driver and DRAM libraries (ARC/Lib/Release) and the Linux host tools that lay out, pack and sum the image and write the DRAM parameters into it (Tools/Bin). The mask ROM loads it into SRAM; it trains the DDR, reads the device tree at 0x10000, loads U-Boot as the tree says and starts it. Left out: the documentation, the Windows tools and the in-house build scripts. .gitattributes keeps the vendor bytes as they are (CRLF sources, host binaries); .gitignore lets the libraries in.
What the SDK loader needs to build with the Arm toolchains of a Linux
distribution (arm-none-eabi, or arm-linux-gnueabi as U-Boot's), not the
SDK's own Linaro 6.4 arm-eabi:
- target("thumb2") is not a GCC target attribute: target("thumb");
- bin_info.h's header guard defined a different name than it tested;
- -fshort-enums: the vendor libraries are built with short enums, the
bare-metal ABI, which a Linux toolchain does not default to;
- the link does not ask for libstdc++, which nothing uses;
- a raise() for libgcc's division by zero handler, which a Linux
toolchain's libgcc calls;
- the model defaults to the NT98562 with the SPI NOR, as Xiongmai's
NT98562 loaders are built (DDR 666 MHz, 512 Mbit); build.sh sets CHIP
and DRAM1_CLK for each SoC.
The NA51089 camera SoCs: one Cortex-A9, DDR in the package (64 MiB on
the NT98562, 128 MiB on the NT98566), Novatek's peripherals from
0xF0000000. After the NT9856x SDK v1.01.007's U-Boot 2019.04 (board
nvt-na51089) and the loader's sources.
- The SDK's loader trains the DDR, then starts U-Boot where its device
tree's /nvt_memory_cfg/uboot says, once the HEADINFO 0x300 into the
image names that address as its CodeEntry and the image sums up.
board/novatek/na51089/u-boot.lds (mainline's, with the HEADINFO at
0x300) and soc.c place it; the image builder fills in the length and
the sum.
- Console: UART0, a 16550 at 24 MHz, mainline's NS16550. Timer: timer 0
of the timer block, which the loader leaves running at 1 MHz.
Reset: the watchdog, as the vendor U-Boot.
- The loader leaves its device tree and SHMINFO in DDR. The board copies
the tree out of the way of the kernel and gives its address in
${fdtaddr}; ft_board_setup() splits the DDR at ${osmem} between
/memory and the media pool (/hdal-memory/media), as the vendor
U-Boot's nvt_boot does, and board_lmb_reserve() keeps the tree below
${osmem}.
- include/configs/openipc-common.h: openipc-anyka's, with the Novatek
parameters (osmem 32M, load address 0x01000000, ttyS0, MTD spi_nor.0).
Verified under qemu-hisilicon nt98562 and nt98566.
OpenIPC's NT9856x kernels are uImages without a device tree, booted by
bootflash's "bootm ${baseaddr}"; the board's tree is the one the loader
read from flash, which the board keeps at ${fdtaddr}. With ARCH_NOVATEK,
a legacy image given no device tree boots with that one, as an Android
image already does.
Verified under qemu-hisilicon nt98562: the OpenIPC nt98562 kernel boots
to login with the loader's tree.
The controller at 0xF0400000 runs one operation at a time: a bare command, a command with a status byte back, the JEDEC ID, a command with an address, a block erase that polls the status itself, an automatic status poll, N bytes in or out. Reads and programs hold the chip selected by hand across the command-and-address operation and the data one. After the vendor U-Boot's nvt_flash_spi and spinor.c: its clock, reset, pins, timing and PHY set-up, READ 0x03 at 48 MHz single lane, whole cache lines by DMA and the rest through the 64-byte FIFO, 256-byte pages by DMA and shorter programs by PIO, 64 KiB erases. Like ak_sflash, it provides spi_flash_probe() itself, for the sf command, the environment and common/openipc.c, without the SPI and spi-nor layers. Block protection a vendor loader left is cleared. Verified under qemu-hisilicon nt98562: sf erase/write/read with an unaligned offset, length and buffer against crc32, saveenv and the environment back after a reset, the OpenIPC firmware read and booted.
The Synopsys Ethernet QoS MAC at 0xF02B0000 with the embedded PHY at 0xF02B3800, non-DM: drivers/net/nvt_eqos.c, after the vendor U-Boot's eth_na51089.c (a cut-down early dwc_eth_qos): one TX and one RX ring, MDIO clause 22 through the MAC, the PHY by phylib. board_eth_init() takes the wiring from the loader's device tree (/top@f0010000/eth pinmux; Xiongmai boards: the embedded PHY, its LEDs on the second pads) and brings up the clocks, the SRAM, the PHY PLL and the PHY's power as the vendor does, before the MAC: its DMA reset only completes once the PHY gives it an RX clock. An RMII PHY is wired the vendor's way too, untested. Verified under qemu-hisilicon nt98562: dhcp, ping, a 3 MB tftpboot against crc32, Linux's driver up after U-Boot used the MAC.
Novatek's own SD host at 0xF0420000, non-DM, after the vendor U-Boot's nvt_ivot_mmc.c: its pins, clock and pad drive (the Xiongmai device tree's table), 4-bit, PIO through the 16-word FIFO, 400 kHz then up to 24 MHz, no card detect: with no card the first command times out in about 10 ms. Verified under qemu-hisilicon nt98562: mmc info, fatls, fatload and mmc read/write against the host's CRCs, no delay without a card, and common/openipc.c's SD recovery of a changed kernel.
build.sh builds, for each SoC, U-Boot, the loader (in a copy, with the DDR clock of Xiongmai's loaders: NT98562 666 MHz, NT98566 933 MHz) and the device tree, and openipc/nvt-image.py lays them out as the 256 KiB image a programmer writes at offset 0: the loader, the tree at 0x10000, U-Boot at 0x20000 in an LZMA BCL1 container with its HEADINFO length and sum filled in. The environment is at 0x40000. The device tree is the one a Xiongmai NT98562 camera's loader carries (IPC_NT98562_80N40_S38), which the vendor kernels need whole, with the OpenIPC boot chain in /nor and its nvtpack index, through which the loader finds U-Boot; nt98566.dts makes the changes Xiongmai's NT98566 tree makes. The config fragments follow openipc-anyka's: the OpenIPC layer and boot limit, the SPI NOR environment, the SD card, the Ethernet. Verified under qemu-hisilicon nt98562 and nt98566: two boots of an image holding only U-Boot (the second loads the environment the first saved), the OpenIPC lite firmware to login, TFTP recovery from a flash holding only the image, the boot limit's failsafe boot.
ci.yml only calls build-novatek.yml on the openipc branch, as on the other families' branches. CLAUDE.md covers the loader, the image layout, the hand-off to the kernel, the drivers and what was verified where.
PR Summary by QodoAdd OpenIPC U-Boot support for Novatek NT98562 and NT98566
AI Description
Diagram
High-Level Assessment
Files changed (158)
|
Code Review by Qodo
1. Neither loader build finds its DRAM profile
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| #define UBI_MAGIC_ASCII "UBI#" | ||
| #define FDT_MAGIC 0xd00dfeed | ||
|
|
||
| #define SQSH_SIZE_OFF 0x58 |
There was a problem hiding this comment.
2. Rootfs scans assign the wrong partition size 🐞 Bug ≡ Correctness
get_squashfs_size() reads the SquashFS size at offset 0x58, although the bytes_used field is at 0x28. When a rootfs is found, the unrelated value is rounded, clamped, and used in the generated rootfs and firmware partition layout.
Agent Prompt
## Issue description
Automatic NOR scanning reads rootfs size from the wrong SquashFS superblock offset.
## Fix Focus Areas
- common/openipc.c[39-41]
- common/openipc.c[96-106]
## Recommended Fix
Read the `bytes_used` field at offset `0x28`, preferably through a shared superblock definition, and reject sizes that exceed the remaining flash.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+398
to
+401
| if (image_check_magic(&hdr)) { | ||
| addr = image_get_load(&hdr); | ||
| offset = sizeof(hdr); | ||
| size = image_get_data_size(&hdr); |
There was a problem hiding this comment.
3. Recovery images can overwrite boot flash 🐞 Bug ☼ Reliability
NOR recovery_write() trusts a uImage's load address and payload length as a flash destination and read size without checking the available RAM, flash capacity, or partition boundaries. If an oversized or incorrectly addressed .img is present on the recovery card, the read can exceed its load buffer or the erase and write can reach the rootfs or reserved boot area.
Agent Prompt
## Issue description
NOR SD recovery accepts image-controlled sizes and destinations without bounding memory or flash writes.
## Fix Focus Areas
- common/openipc.c[398-426]
- include/configs/openipc-common.h[36-48]
## Recommended Fix
Validate the complete image size against the available load buffer and validate the destination, erase range, and payload against flash capacity and the appropriate kernel or rootfs region before reading or erasing.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
…, DMA cache From the Qodo review of #53: - sf_unprotect() checks that the block protection went: a program or erase of a protected block is accepted and dropped by the flash. If clearing it fails, the probe says so and writes and erases return -EROFS instead of passing for success. - Erases set erase_info.state (MTD_ERASING, then MTD_ERASE_DONE, or MTD_ERASE_FAILED with fail_addr). - A capacity byte past 16 MiB is no longer taken for 8 MiB: the driver uses the first 16 MiB (3-byte addresses) and says so; an unknown one is taken for 8 MiB with a warning. Both are printed once. - A DMA read invalidates the buffer's cache lines before the transfer too, so no dirty line lands on the data afterwards. (D-cache is off on these targets; this keeps the driver right if it is turned on.) Verified under qemu-hisilicon nt98562: sf erase/write/read with an unaligned offset, length and buffer against crc32, saveenv, the OpenIPC firmware to login; a 32 MiB flash image probes as its first 16 MiB.
MakeConfig.txt chose SECURE_DECRYPT_OPTEE_OS from SECUREBOOT_DECRYPT_UBOOT in its secure branch. A vendor typo found by the Qodo review of #53; these builds leave both at Normal, so their loaders do not change.
The loader's make system names ConfigRam/562/... and ConfigRam/566/..., which do not exist, as Qodo noted on #53. For these in-package parts Configram560 writes the DDR parameters from its own tables (-package=562 or 566) and does not read the profile: built with the default path, a real 933 MHz 1 Gbit profile and a 400 MHz 8 Gbit one, the NT98566 loader differs only in its build time stamp and the sums over it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
U-Boot for Novatek's NA51089 camera SoCs, NT98562 (64 MiB) and NT98566 (128 MiB): mainline v2022.10 with the same OpenIPC boot layer as
openipc-anyka(identicalcommon/openipc.c).One image for a programmer, written at offset 0 (
u-boot-<soc>-nor.bin, 256 KiB):0x00000board/novatek/na51089/loader0x100000x200000x400000x50000, as on the other families-fshort-enums, araise()stub, two GCC 13 fixes). It still links Novatek's closed libraries (the DDR training) and runs their Linux host tools fromTools/Bin.${fdtaddr}, a smallboot/image-fdt.cchange). Like the vendor'snvt_boot, it gives Linux the firstosmemof memory and the vendor media pool the rest.nvt_sflash), Ethernet with the embedded PHY (nvt_eqos), SD on SDIO1 (nvt_mmc).Verified under qemu-hisilicon
nt98562/nt98566:fw_printenvworks.sfwrite and read at unaligned offsets.saveenvandreset.Not verified on silicon yet. qemu does not run the loader, so DDR, pins, clocks, the PHY and SD timing are untested. Keep a flash backup and a programmer at hand.
CI (
build-novatek.yml) comes in the PR againstopenipc. Its smoke test needs a qemu-hisilicon whose loader stand-in unpacks LZMA BCL1 (that change is in its own PR).