family: rockchip: add rv1103/rv1106 + first board 'LuckFox Pico Mini' #8797
Conversation
vidplace7
requested review from
a team,
chainsx,
igorpecovnik,
joekhoobyar and
lanefu
as code owners
github-actions
bot
added
size/large
WalkthroughAdds Rockchip RV1106 support and a new board for Luckfox Pico Mini (RV1103 SoC using RV1106 blobs). Changes include: a new board config Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
Needs review
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/bsp/rv1106/rv1106-firstboot-makeswap.sh (1)
24-32: Consider the partial failure scenario.If
mkswaporswaponfails afterfallocatesucceeds, the script exits (due toset -e) leaving a 256 MiB file that's not properly configured as swap. The service then self-disables at lines 35-36, so it won't retry on the next boot.This is likely acceptable for a first-boot script on a constrained device, but you may want to add cleanup logic in a trap handler to remove the incomplete swapfile on failure.
Example improvement:
+trap 'rm -f "$SWAPFILE"' ERR + # Create swapfile, set permissions, and enable it fallocate -l "${SIZE_MB}M" "$SWAPFILE" chmod 600 "$SWAPFILE" mkswap "$SWAPFILE" # Add to fstab if not present if ! grep -qE '^[^#]*\s+/swapfile\s+swap\s' /etc/fstab; then echo "/swapfile swap swap defaults,nofail,pri=0 0 0" >> /etc/fstab fi swapon -p 0 "$SWAPFILE" +trap - ERRconfig/kernel/linux-rv1106-vendor.config (1)
460-460: Consider disabling DEBUG_INFO for memory-constrained device.
CONFIG_DEBUG_INFO=ysignificantly increases kernel image size (can add 100+ MB). On a device with only 64-256 MB RAM, this impacts available memory and boot time.Unless kernel debugging is essential, consider disabling this for production builds.
-CONFIG_DEBUG_INFO=y +# CONFIG_DEBUG_INFO is not setconfig/boards/luckfox-pico-mini.csc (1)
1-7: Naming inconsistency between RV1103 and RV1106 may cause confusion.The board is RV1103 hardware (lines 1, 7: DTB file is
rv1103g-luckfox-pico-mini.dtb) but uses:
- U-Boot config named
luckfox_rv1106_uboot_defconfig(line 4)- RV1106 blobs (line 11-14)
While lines 10-11 document that "RV1103 but uses RV1106 blobs (doesn't work with RV1103 blobs)", this naming inconsistency could lead to confusion during maintenance or when adding more boards.
Consider whether the U-Boot defconfig or documentation should be clarified to make this unusual configuration more obvious to future maintainers.
config/sources/families/rockchip.conf (3)
268-272: Add explicit return statement for consistency.While bash functions return 0 by default, other functions in this file explicitly return 0 (e.g., lines 265, 317, 326, 345). Add an explicit
return 0for consistency.function pre_config_uboot_target__copy_tee() { # Makefiles expect 'tee.bin' in the uboot build directory display_alert "Copy TEE blob" "info" run_host_command_logged cp ${RKBIN_DIR}/${TEE_BLOB} tee.bin + return 0 }
274-295: Improve logging and error handling for U-Boot post-processing.Several issues with logging and robustness:
- Line 277: Output redirection bypasses
run_host_command_logged, so the command's stderr isn't captured in logs- Line 286:
cpcommand should userun_host_command_loggedfor consistency- Lines 287-292:
sedcommands don't verify pattern matching succeeded; if the INI file format changes, substitutions might silently fail, causingboot_mergerto use wrong pathsConsider wrapping the critical operations in logged commands and verifying sed succeeded:
display_alert "Generate u-boot.its" "info" - run_host_command_logged $SPL_FIT_GENERATOR -t 0x03d00000 -c lzma > u-boot.its + $SPL_FIT_GENERATOR -t 0x03d00000 -c lzma > u-boot.its 2>&1 | tee -a "${DEST}/${LOG_SUBPATH}/output.log" display_alert "Generate u-boot.itb" "info" # Remove uboot-generated itb if exists rm -f u-boot.itb run_host_command_logged tools/mkimage -f u-boot.its -E u-boot.itb display_alert "Generate idblock.img with u-boot SPL" "debug" RKBOOT_INI_FILE=rv1106.ini - cp $RKBIN_DIR/rv11/RV1106MINIALL.ini $RKBOOT_INI_FILE + run_host_command_logged cp $RKBIN_DIR/rv11/RV1106MINIALL.ini $RKBOOT_INI_FILE sed -i "s|Path1=.*rv1106_ddr.*$|Path1=${RKBIN_DIR}/${DDR_BLOB}|g" $RKBOOT_INI_FILE + grep -q "${RKBIN_DIR}/${DDR_BLOB}" $RKBOOT_INI_FILE || exit_with_error "Failed to update DDR_BLOB path in INI file"
320-327: Fix indentation inconsistency.Line 324 uses spaces for indentation while the rest of the file uses tabs. Maintain consistent indentation style.
function post_family_tweaks__rv1106_mkswap() { local service_name="rv1106-firstboot-makeswap" display_alert "Enabling ${service_name}.service" "debug" # This service disables itself after running once - chroot_sdcard systemctl enable "${service_name}.service" + chroot_sdcard systemctl enable "${service_name}.service" return 0 }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (7)
patch/u-boot/legacy/u-boot-rockchip-buildroot/0000.patching_config.yamlis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/defconfig/luckfox_rv1106_uboot_defconfigis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/dt/rv1106-luckfox.dtsis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-malloc_8_hide_optee.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-mem_layout.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-sysboot-extlinux.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/use-armv7a-for-modern-toolchain.patchis excluded by!patch/**
📒 Files selected for processing (5)
config/boards/luckfox-pico-mini.csc(1 hunks)config/kernel/linux-rv1106-vendor.config(1 hunks)config/sources/families/rockchip.conf(2 hunks)packages/bsp/rv1106/rv1106-firstboot-makeswap.service(1 hunks)packages/bsp/rv1106/rv1106-firstboot-makeswap.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-14T05:08:11.785Z
Learnt from: EvilOlaf
PR: armbian/build#8754
File: config/boards/bestv-r3300-l.csc:14-16
Timestamp: 2025-10-14T05:08:11.785Z
Learning: In the Armbian build system, BOOTBRANCH_BOARD is a valid framework variable used as a fallback when BOOTBRANCH is unset. The framework checks BOOTBRANCH_BOARD before applying the default bootloader branch value (see config/sources/common.conf). Board configuration files can use BOOTBRANCH_BOARD to specify the bootloader branch.
Applied to files:
config/sources/families/rockchip.conf
📚 Learning: 2025-09-14T06:29:18.958Z
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:64-70
Timestamp: 2025-09-14T06:29:18.958Z
Learning: In the Armbian build system, vendor branch configurations in family files are designed to be shared across multiple SoCs within the same family that use the same vendor kernel tree. For example, rk35xx and rockchip-rk3588 families both use identical vendor branch settings (same KERNELSOURCE, KERNELBRANCH, and KERNELPATCHDIR), demonstrating that vendor branches are intentionally generic rather than SoC-specific.
Applied to files:
config/sources/families/rockchip.conf
📚 Learning: 2025-09-14T06:32:29.806Z
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.
Applied to files:
config/sources/families/rockchip.conf
🧬 Code graph analysis (4)
config/boards/luckfox-pico-mini.csc (2)
lib/functions/configuration/main-config.sh (1)
write_config_summary_output_file(488-541).github/generate_CODEOWNERS.sh (1)
generate_for_board(18-68)
config/sources/families/rockchip.conf (2)
config/sources/vendors/mekotronics/mekotronics-rk3588.hooks.sh (3)
post_family_config__meko_use_mainline_uboot(22-42)post_family_config__vendor_uboot_mekotronics(8-16)write_uboot_platform(39-41)lib/functions/compilation/uboot.sh (3)
patch_uboot_target(22-48)compile_uboot_target(52-299)uboot_postinst_base(544-562)
config/kernel/linux-rv1106-vendor.config (1)
lib/functions/compilation/armbian-kernel.sh (4)
armbian_kernel_config_apply_opts_from_arrays(577-607)armbian_kernel_config__enable_config_access_in_live_system(508-514)armbian_kernel_config__disable_various_options(71-88)armbian_kernel_config__extrawifi_enable_wifi_opts_80211(24-36)
packages/bsp/rv1106/rv1106-firstboot-makeswap.sh (1)
lib/functions/bsp/armbian-bsp-cli-deb.sh (2)
board_side_bsp_cli_preinst(360-415)board_side_bsp_cli_postinst_finish(448-471)
🔇 Additional comments (9)
packages/bsp/rv1106/rv1106-firstboot-makeswap.sh (1)
35-36: LGTM: Self-disabling service pattern is appropriate.The service correctly disables and masks itself after successful completion, preventing re-execution. The
|| truefallback ensures the script doesn't fail if systemctl commands encounter issues.packages/bsp/rv1106/rv1106-firstboot-makeswap.service (1)
1-14: LGTM: Systemd unit is properly configured.The service correctly:
- Uses
ConditionPathExists=!/swapfileto run only when swapfile doesn't exist- Orders itself after filesystem resize and before basic.target
- Uses oneshot type with RemainAfterExit for one-time execution
- Matches the script installation path in rockchip.conf (line 314)
config/boards/luckfox-pico-mini.csc (1)
10-14: LGTM: RV1106 blob usage is well-documented.The comment at line 10 clearly explains that despite being RV1103 hardware, RV1106 blobs must be used. The
BOOT_SOC="rv1106"setting correctly triggers the appropriate code paths in the rockchip.conf family configuration.config/sources/families/rockchip.conf (4)
62-79: LGTM: RV1106 boot configuration follows established patterns.The RV1106 configuration block properly mirrors the structure used for other Rockchip SoCs (e.g., rk3506). The
LINUXCONFIG="linux-rv1106-${BRANCH}"correctly references the new kernel config file added in this PR.
254-266: GCC 13+ compatibility workaround is correctly implemented.This function appropriately handles the vendor U-Boot incompatibility with GCC 13+ by downgrading
enum-int-mismatcherrors to warnings. The approach of detecting GCC version and conditionally adding CFLAGS is correct and follows the pattern used elsewhere in the build system.
297-304: LGTM: write_uboot_platform follows Rockchip standards.The implementation correctly:
- Detects if running in build environment or live system (line 300)
- Writes idblock.img and u-boot.itb at proper offsets for RV1106
- Uses
status=noneparameter in dd commands per current Rockchip family standardBased on learnings
329-346: LGTM: Memory optimizations are appropriate for constrained device.The function appropriately:
- Disables ramlog to save memory (line 333)
- Disables zram swap in favor of zswap (line 336), aligning with PR objectives
- Increases /run tmpfs to 20M (line 343) to prevent systemd daemon-reload errors on low-memory devices
The fstab append (line 343) is safe since this hook runs once during image creation.
config/kernel/linux-rv1106-vendor.config (2)
113-114: Enable CONFIG_NET_CORE to match CONFIG_NETDEVICES=y.CONFIG_NET_CORE is defined inside the NETDEVICES block and should default to y when NETDEVICES is enabled. The configuration at lines 113-114 explicitly disables it, which contradicts having CONFIG_NETDEVICES=y. Change line 114 to
CONFIG_NET_CORE=y.⛔ Skipped due to learnings
Learnt from: lanefu PR: armbian/build#8377 File: config/kernel/linux-uefi-arm64-cloud.config:165-170 Timestamp: 2025-07-19T00:03:42.995Z Learning: CONFIG_NETKIT was introduced in Linux kernel 4.14.330 (November 2023) and enables BPF-programmable network devices that can operate in Layer 3 or Layer 2 mode. It's a valid configuration option in modern kernels including 6.12.
14-14: CONFIG_BUG is disabled as part of the vendor-provided board configuration, which is standard for this embedded platform.The technical facts in the review are correct: disabling
CONFIG_BUGsaves ~70-100 KB and reduces kernel diagnostics. However, this is a vendor-provided configuration for the RV1106 SoC (a camera/embedded platform whereCONFIG_BUGdisabled is standard practice, as evidenced by OpenIPC's RV1106 configurations).This is not a development choice being proposed in the PR but rather the initial board support configuration for a specific embedded platform. No action is needed unless the board-specific requirements differ from the vendor baseline.
Likely an incorrect or invalid review comment.
vidplace7
force-pushed
the
luckfox-pico-mini
branch
from
d8c6df7 to
508b3a0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
config/sources/families/rockchip.conf (4)
62-79: Allow board-level BOOTBRANCH override (keeps family default, enables per‑board escape hatch).Use BOOTBRANCH_BOARD as a fallback-aware override.
Based on learnings
- BOOTBRANCH="branch:next-dev-buildroot" + BOOTBRANCH="${BOOTBRANCH_BOARD:-branch:next-dev-buildroot}"
268-273: Harden tee.bin copy and standardize logging.Guard missing blobs and use a consistent 3‑arg display_alert.
- function pre_config_uboot_target__copy_tee() { - # Makefiles expect 'tee.bin' in the uboot build directory - display_alert "Copy TEE blob" "info" - run_host_command_logged cp ${RKBIN_DIR}/${TEE_BLOB} tee.bin - } + function pre_config_uboot_target__copy_tee() { + # Makefiles expect 'tee.bin' in the U‑Boot build directory + display_alert "$BOARD" "Copying TEE blob ${TEE_BLOB}" "info" + [[ -f "${RKBIN_DIR}/${TEE_BLOB}" ]] || { display_alert "$BOARD" "TEE blob not found: ${RKBIN_DIR}/${TEE_BLOB}" "err"; return 1; } + run_host_command_logged install -m 0644 "${RKBIN_DIR}/${TEE_BLOB}" tee.bin + }
274-296: Add existence checks around generator/tools for clearer failures.Small robustness boost; no behavioral change.
- run_host_command_logged $SPL_FIT_GENERATOR -t 0x03d00000 -c lzma > u-boot.its + [[ -x "$SPL_FIT_GENERATOR" ]] || { display_alert "$BOARD" "Missing SPL FIT generator: $SPL_FIT_GENERATOR" "err"; return 1; } + run_host_command_logged "$SPL_FIT_GENERATOR" -t 0x03d00000 -c lzma > u-boot.its ... - run_host_command_logged tools/mkimage -f u-boot.its -E u-boot.itb + [[ -x tools/mkimage ]] || { display_alert "$BOARD" "Missing tools/mkimage" "err"; return 1; } + run_host_command_logged tools/mkimage -f u-boot.its -E u-boot.itb ... - run_host_x86_binary_logged $RKBIN_DIR/tools/boot_merger $RKBOOT_INI_FILE + run_host_x86_binary_logged "$RKBIN_DIR/tools/boot_merger" "$RKBOOT_INI_FILE"
306-318: Use install -D for idempotent service/script deployment (ensures dirs exist).Avoids failures when target dirs are missing and removes a plain cp.
- install -m 755 $SRC/packages/bsp/rv1106/${service_name}.sh $destination/usr/bin/${service_name}.sh - cp $SRC/packages/bsp/rv1106/${service_name}.service $destination/lib/systemd/system/${service_name}.service + install -D -m 0755 "$SRC/packages/bsp/rv1106/${service_name}.sh" \ + "$destination/usr/bin/${service_name}.sh" + install -D -m 0644 "$SRC/packages/bsp/rv1106/${service_name}.service" \ + "$destination/lib/systemd/system/${service_name}.service"config/kernel/linux-rv1106-vendor.config (3)
460-465: Trim debug features for low‑RAM boards (optional but recommended).Disable DEBUG_INFO/DEBUG_FS to reduce image size and memory footprint.
-CONFIG_DEBUG_INFO=y -CONFIG_DEBUG_FS=y +# CONFIG_DEBUG_INFO is not set +# CONFIG_DEBUG_FS is not set
339-347: Consider making SND_USB_AUDIO modular or disabling entirely.Saves memory/boot time on headless/embedded use; USB audio can be uncommon on these SBCs.
-CONFIG_SND_USB_AUDIO=y +# CONFIG_SND_USB_AUDIO is not set # or +# CONFIG_SND_USB_AUDIO=m
80-110: Large Bluetooth/Wi‑Fi matrix — consider slimming to tested chipsets only.Many drivers are enabled; pruning to tested ones reduces size/RAM.
Also applies to: 156-247
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (7)
patch/u-boot/legacy/u-boot-rockchip-buildroot/0000.patching_config.yamlis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/defconfig/luckfox_rv1106_uboot_defconfigis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/dt/rv1106-luckfox.dtsis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-malloc_8_hide_optee.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-mem_layout.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-sysboot-extlinux.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/use-armv7a-for-modern-toolchain.patchis excluded by!patch/**
📒 Files selected for processing (5)
config/boards/luckfox-pico-mini.csc(1 hunks)config/kernel/linux-rv1106-vendor.config(1 hunks)config/sources/families/rockchip.conf(2 hunks)packages/bsp/rv1106/rv1106-firstboot-makeswap.service(1 hunks)packages/bsp/rv1106/rv1106-firstboot-makeswap.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- config/boards/luckfox-pico-mini.csc
- packages/bsp/rv1106/rv1106-firstboot-makeswap.service
- packages/bsp/rv1106/rv1106-firstboot-makeswap.sh
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-14T06:32:29.806Z
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.
Applied to files:
config/sources/families/rockchip.conf
📚 Learning: 2025-10-14T05:08:11.785Z
Learnt from: EvilOlaf
PR: armbian/build#8754
File: config/boards/bestv-r3300-l.csc:14-16
Timestamp: 2025-10-14T05:08:11.785Z
Learning: In the Armbian build system, BOOTBRANCH_BOARD is a valid framework variable used as a fallback when BOOTBRANCH is unset. The framework checks BOOTBRANCH_BOARD before applying the default bootloader branch value (see config/sources/common.conf). Board configuration files can use BOOTBRANCH_BOARD to specify the bootloader branch.
Applied to files:
config/sources/families/rockchip.conf
📚 Learning: 2025-09-14T06:29:18.958Z
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:64-70
Timestamp: 2025-09-14T06:29:18.958Z
Learning: In the Armbian build system, vendor branch configurations in family files are designed to be shared across multiple SoCs within the same family that use the same vendor kernel tree. For example, rk35xx and rockchip-rk3588 families both use identical vendor branch settings (same KERNELSOURCE, KERNELBRANCH, and KERNELPATCHDIR), demonstrating that vendor branches are intentionally generic rather than SoC-specific.
Applied to files:
config/sources/families/rockchip.conf
🧬 Code graph analysis (2)
config/kernel/linux-rv1106-vendor.config (2)
lib/functions/compilation/armbian-kernel.sh (1)
armbian_kernel_config__disable_various_options(71-88)lib/functions/compilation/kernel-config.sh (1)
kernel_config_export(132-146)
config/sources/families/rockchip.conf (3)
config/sources/vendors/mekotronics/mekotronics-rk3588.hooks.sh (2)
post_family_config__meko_use_mainline_uboot(22-42)write_uboot_platform(39-41)lib/functions/compilation/uboot.sh (2)
compile_uboot_target(52-299)uboot_postinst_base(544-562)lib/functions/configuration/main-config.sh (1)
write_config_summary_output_file(488-541)
🔇 Additional comments (3)
config/sources/families/rockchip.conf (2)
297-304: dd offsets and status=none look correct.Matches modern standard for Rockchip families; good.
Based on learnings
320-327: Enable first-boot swap service — OK.Service is idempotent and self-disabling; looks fine.
config/kernel/linux-rv1106-vendor.config (1)
491-495: zswap enabled without explicit backend configuration; modern kernels auto-select default.By default, a zpool of type selected in CONFIG_ZSWAP_ZPOOL_DEFAULT Kconfig option is created. The current config specifies
CONFIG_ZSWAP=yandCONFIG_ZSWAP_DEFAULT_ON=ybut lacks an explicit zpool backend flag. zsmalloc is the kernel default in kernels after 6.3 and a zpool of type zsmalloc is created by default.However, the recommendation to add
CONFIG_ZBUD=yis outdated: z3fold was deprecated and disabled by default since 6.13 and zbud was removed in 6.15. For vendor kernels (typically based on 5.10), ZBUD remains functional. SinceCONFIG_ZSWAP_ZPOOL_DEFAULTis not explicitly set in this config, the kernel will auto-select its compiled-in default or use the first available backend at runtime.Set
CONFIG_ZSWAP_ZPOOL_DEFAULT_ZSMALLOC=yto explicitly declare the modern, recommended backend:CONFIG_SWAP=y CONFIG_ZSWAP=y CONFIG_ZSWAP_DEFAULT_ON=y +CONFIG_ZSMALLOC=y +CONFIG_ZSWAP_ZPOOL_DEFAULT_ZSMALLOC=y CONFIG_ZSWAP_COMPRESSOR_DEFAULT_ZSTD=y CONFIG_ZSWAP_COMPRESSOR_DEFAULT="zstd"
| function post_config_uboot_target__downgrade_gcc_errors_to_warnings() { | ||
| declare -i gcc_major_version=0 | ||
| gcc_major_version=$(gcc -dumpversion | cut -d. -f1) | ||
| display_alert "$BOARD" "GCC major version: ${gcc_major_version}" "debug" | ||
|
|
||
| # Rockchip/Radxa U-Boot wont' build with GCC 13 due to enum-int-mismatch | ||
| if [[ ${gcc_major_version} -ge 13 ]]; then | ||
| display_alert "$BOARD" "Adding extra CFLAGS to fix vendor U-Boot building with GCC 13+" "info" | ||
| uboot_cflags_array+=("-Wno-error=enum-int-mismatch") | ||
| fi | ||
|
|
||
| return 0 | ||
| } |
There was a problem hiding this comment.
GCC version check uses host gcc; use target CC/CROSS_COMPILE to avoid false negatives.
With cross compilers, host gcc != target gcc; build may miss -Wno-error on GCC 13+ and fail.
Apply:
- declare -i gcc_major_version=0
- gcc_major_version=$(gcc -dumpversion | cut -d. -f1)
+ declare -i gcc_major_version=0
+ # Prefer target compiler; fall back to host gcc
+ local gcc_cmd="${CC:-${CROSS_COMPILE}gcc}"
+ if command -v "${gcc_cmd}" >/dev/null 2>&1; then
+ gcc_major_version=$("${gcc_cmd}" -dumpfullversion -dumpversion | cut -d. -f1)
+ else
+ gcc_major_version=$(gcc -dumpfullversion -dumpversion | cut -d. -f1)
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function post_config_uboot_target__downgrade_gcc_errors_to_warnings() { | |
| declare -i gcc_major_version=0 | |
| gcc_major_version=$(gcc -dumpversion | cut -d. -f1) | |
| display_alert "$BOARD" "GCC major version: ${gcc_major_version}" "debug" | |
| # Rockchip/Radxa U-Boot wont' build with GCC 13 due to enum-int-mismatch | |
| if [[ ${gcc_major_version} -ge 13 ]]; then | |
| display_alert "$BOARD" "Adding extra CFLAGS to fix vendor U-Boot building with GCC 13+" "info" | |
| uboot_cflags_array+=("-Wno-error=enum-int-mismatch") | |
| fi | |
| return 0 | |
| } | |
| function post_config_uboot_target__downgrade_gcc_errors_to_warnings() { | |
| declare -i gcc_major_version=0 | |
| # Prefer target compiler; fall back to host gcc | |
| local gcc_cmd="${CC:-${CROSS_COMPILE}gcc}" | |
| if command -v "${gcc_cmd}" >/dev/null 2>&1; then | |
| gcc_major_version=$("${gcc_cmd}" -dumpfullversion -dumpversion | cut -d. -f1) | |
| else | |
| gcc_major_version=$(gcc -dumpfullversion -dumpversion | cut -d. -f1) | |
| fi | |
| display_alert "$BOARD" "GCC major version: ${gcc_major_version}" "debug" | |
| # Rockchip/Radxa U-Boot wont' build with GCC 13 due to enum-int-mismatch | |
| if [[ ${gcc_major_version} -ge 13 ]]; then | |
| display_alert "$BOARD" "Adding extra CFLAGS to fix vendor U-Boot building with GCC 13+" "info" | |
| uboot_cflags_array+=("-Wno-error=enum-int-mismatch") | |
| fi | |
| return 0 | |
| } |
There was a problem hiding this comment.
post_config_uboot_target__downgrade_gcc_errors_to_warnings() was copy-pasta'd from rockchip-rk3588.conf and works correctly as written.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the function exists in rockchip-rk3588.conf and show its implementation
rg -A 15 "post_config_uboot_target__downgrade_gcc_errors_to_warnings" config/sources/families/rockchip-rk3588.conf
Length of output: 542
@vidplace7, understood! You're right that it's consistent with the rockchip-rk3588.conf implementation. Since it's working correctly for your builds and follows the established pattern in the Armbian codebase, that makes sense to keep as-is. Thanks for clarifying! 👍
🧠 Learnings used
Learnt from: HackingGate
PR: armbian/build#8665
File: config/boards/photonicat2.csc:4-4
Timestamp: 2025-10-26T10:41:35.107Z
Learning: In the Armbian build system, rk3576 boards consistently use BOARDFAMILY="rk35xx" for both vendor and edge kernel targets. The rk35xx family configuration sources rockchip64_common.inc, which provides edge and current kernel branch definitions, making these branches available even though they're not defined directly in rk35xx.conf.
| function pre_umount_final_image__memory_optimize_defaults() { | ||
| # RV1106 family devices have very limited RAM (64-256MB) | ||
| # Optimize default settings to reduce memory usage | ||
| display_alert "Disabling ramlog by default to save memory" "debug" | ||
| sed -i "s/^ENABLED=.*/ENABLED=false/" "${MOUNT}"/etc/default/armbian-ramlog | ||
|
|
||
| display_alert "Disabling zram swap by default. RV1106 family uses zswap." "debug" | ||
| sed -i "s/^#\?\s*SWAP=.*/SWAP=false/" "${MOUNT}"/etc/default/armbian-zram-config | ||
|
|
||
| # /run is 10% of RAM by default | ||
| # systemd throws errors when <16MB is *free* in this partition | ||
| # during daemon-reload operations. | ||
| # Address with a fixed /run size of 20MB | ||
| display_alert "Increasing /run size in /etc/fstab" "debug" | ||
| echo "tmpfs /run tmpfs rw,nosuid,nodev,noexec,relatime,size=20M,mode=755 0 0" >> "${MOUNT}"/etc/fstab | ||
|
|
||
| return 0 | ||
| } |
There was a problem hiding this comment.
Guard sed targets; make fstab change idempotent. Current code may fail or duplicate entries.
If files are absent, sed -i will fail; echo blindly appends a duplicate /run line on rebuilds.
Apply:
- sed -i "s/^ENABLED=.*/ENABLED=false/" "${MOUNT}"/etc/default/armbian-ramlog
+ if [[ -f "${MOUNT}/etc/default/armbian-ramlog" ]]; then
+ sed -i 's/^ENABLED=.*/ENABLED=false/' "${MOUNT}/etc/default/armbian-ramlog"
+ fi
...
- sed -i "s/^#\?\s*SWAP=.*/SWAP=false/" "${MOUNT}"/etc/default/armbian-zram-config
+ if [[ -f "${MOUNT}/etc/default/armbian-zram-config" ]]; then
+ sed -i 's/^#\?\s*SWAP=.*/SWAP=false/' "${MOUNT}/etc/default/armbian-zram-config"
+ fi
...
- echo "tmpfs /run tmpfs rw,nosuid,nodev,noexec,relatime,size=20M,mode=755 0 0" >> "${MOUNT}"/etc/fstab
+ if ! grep -qE '^[[:space:]]*tmpfs[[:space:]]+/run[[:space:]]+tmpfs' "${MOUNT}/etc/fstab"; then
+ echo 'tmpfs /run tmpfs rw,nosuid,nodev,noexec,relatime,size=20M,mode=755 0 0' >> "${MOUNT}/etc/fstab"
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function pre_umount_final_image__memory_optimize_defaults() { | |
| # RV1106 family devices have very limited RAM (64-256MB) | |
| # Optimize default settings to reduce memory usage | |
| display_alert "Disabling ramlog by default to save memory" "debug" | |
| sed -i "s/^ENABLED=.*/ENABLED=false/" "${MOUNT}"/etc/default/armbian-ramlog | |
| display_alert "Disabling zram swap by default. RV1106 family uses zswap." "debug" | |
| sed -i "s/^#\?\s*SWAP=.*/SWAP=false/" "${MOUNT}"/etc/default/armbian-zram-config | |
| # /run is 10% of RAM by default | |
| # systemd throws errors when <16MB is *free* in this partition | |
| # during daemon-reload operations. | |
| # Address with a fixed /run size of 20MB | |
| display_alert "Increasing /run size in /etc/fstab" "debug" | |
| echo "tmpfs /run tmpfs rw,nosuid,nodev,noexec,relatime,size=20M,mode=755 0 0" >> "${MOUNT}"/etc/fstab | |
| return 0 | |
| } | |
| function pre_umount_final_image__memory_optimize_defaults() { | |
| # RV1106 family devices have very limited RAM (64-256MB) | |
| # Optimize default settings to reduce memory usage | |
| display_alert "Disabling ramlog by default to save memory" "debug" | |
| if [[ -f "${MOUNT}/etc/default/armbian-ramlog" ]]; then | |
| sed -i 's/^ENABLED=.*/ENABLED=false/' "${MOUNT}/etc/default/armbian-ramlog" | |
| fi | |
| display_alert "Disabling zram swap by default. RV1106 family uses zswap." "debug" | |
| if [[ -f "${MOUNT}/etc/default/armbian-zram-config" ]]; then | |
| sed -i 's/^#\?\s*SWAP=.*/SWAP=false/' "${MOUNT}/etc/default/armbian-zram-config" | |
| fi | |
| # /run is 10% of RAM by default | |
| # systemd throws errors when <16MB is *free* in this partition | |
| # during daemon-reload operations. | |
| # Address with a fixed /run size of 20MB | |
| display_alert "Increasing /run size in /etc/fstab" "debug" | |
| if ! grep -qE '^[[:space:]]*tmpfs[[:space:]]+/run[[:space:]]+tmpfs' "${MOUNT}/etc/fstab"; then | |
| echo 'tmpfs /run tmpfs rw,nosuid,nodev,noexec,relatime,size=20M,mode=755 0 0' >> "${MOUNT}/etc/fstab" | |
| fi | |
| return 0 | |
| } |
🤖 Prompt for AI Agents
In config/sources/families/rockchip.conf around lines 329 to 346, the sed calls
can fail if target files are missing and the /etc/fstab change will append
duplicates on repeated runs; guard each sed by first checking the file exists
(e.g. if [ -f "${MOUNT}/etc/default/armbian-ramlog" ]; then sed -i ...; fi),
similarly for armbian-zram-config, and make the /etc/fstab update idempotent by
detecting and replacing any existing tmpfs /run line (or removing/commenting it)
before adding the canonical "tmpfs /run ..." entry, or only append if no
matching tmpfs /run line exists; ensure commented variants are handled by the
match and keep the function returning 0.
There was a problem hiding this comment.
If these files are absent then there is every reason to forcefully fail since then there is something wrong with the framework in general.
There was a problem hiding this comment.
@EvilOlaf, that's a fair point about the file existence checks—failing fast when expected files are missing is a good way to catch framework issues early.
However, there's still the fstab duplication concern: the echo append on Line 343 will add a duplicate /run entry if the image build process runs through this function multiple times (though I'm not certain if that's possible in your build flow). If that's not a concern in practice, then the current implementation is fine.
Let me know if you'd like to keep it as-is or if the fstab duplication scenario is worth guarding against.
🧠 Learnings used
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.
There was a problem hiding this comment.
I highly doubt it will be called more than once. While for generic functions doing various things within the framework this might be true, for custom functions called by hooks this should not be the case.
There was a problem hiding this comment.
@EvilOlaf, understood—thank you for clarifying how the hook system works. If these custom hook functions are only called once per build, then the fstab duplication isn't a concern. I'll leave the implementation as-is. 👍
🧠 Learnings used
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.
508b3a0 to
2bd66da
Compare
|
I would at least try to get it to work without having to rely on Jammy since it won't be there forever. That's more a general task rather than specific to this PR. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
config/sources/families/rockchip.conf (2)
254-266: GCC version check uses host gcc; prefer target compiler.Cross builds may mis-detect gcc version and miss the -Wno-error, breaking vendor U-Boot with GCC 13+. Use ${CC} or ${CROSS_COMPILE}gcc when available. This was raised previously.
- declare -i gcc_major_version=0 - gcc_major_version=$(gcc -dumpversion | cut -d. -f1) + declare -i gcc_major_version=0 + # Prefer target compiler; fall back to host gcc + local gcc_cmd="${CC:-${CROSS_COMPILE}gcc}" + if command -v "${gcc_cmd}" >/dev/null 2>&1; then + gcc_major_version=$("${gcc_cmd}" -dumpfullversion -dumpversion | cut -d. -f1) + else + gcc_major_version=$(gcc -dumpfullversion -dumpversion | cut -d. -f1) + fi
331-348: Guard sed targets and make /run fstab change idempotent.As noted earlier, sed fails if files are absent and echo appends duplicates on rebuilds. Please add file checks and idempotency.
- sed -i "s/^ENABLED=.*/ENABLED=false/" "${MOUNT}"/etc/default/armbian-ramlog + if [[ -f "${MOUNT}/etc/default/armbian-ramlog" ]]; then + sed -i 's/^ENABLED=.*/ENABLED=false/' "${MOUNT}/etc/default/armbian-ramlog" + fi @@ - sed -i "s/^#\?\s*SWAP=.*/SWAP=false/" "${MOUNT}"/etc/default/armbian-zram-config + if [[ -f "${MOUNT}/etc/default/armbian-zram-config" ]]; then + sed -i 's/^#\?\s*SWAP=.*/SWAP=false/' "${MOUNT}/etc/default/armbian-zram-config" + fi @@ - echo "tmpfs /run tmpfs rw,nosuid,nodev,noexec,relatime,size=20M,mode=755 0 0" >> "${MOUNT}"/etc/fstab + if ! grep -qE '^[[:space:]]*tmpfs[[:space:]]+/run[[:space:]]+tmpfs' "${MOUNT}/etc/fstab"; then + echo 'tmpfs /run tmpfs rw,nosuid,nodev,noexec,relatime,size=20M,mode=755 0 0' >> "${MOUNT}/etc/fstab" + fi
🧹 Nitpick comments (4)
config/boards/luckfox-pico-mini.csc (1)
12-14: Avoid per-board blob pinning; rely on family defaults.These rv11 blob paths are already defaulted in the rv1106 family block. Keeping them here increases maintenance burden if blob versions change.
-DDR_BLOB="rv11/rv1106_ddr_924MHz_v1.15.bin" -TEE_BLOB="rv11/rv1106_tee_ta_v1.13.bin" -USBPLUG_BLOB="rv11/rv1106_usbplug_v1.09.bin" +# Use family defaults; override here only if this board needs different blobsconfig/sources/families/rockchip.conf (3)
268-275: Quote and check TEE blob before copying.Guard against missing blob and spaces in paths; keep logging uniform.
- run_host_command_logged cp ${RKBIN_DIR}/${TEE_BLOB} tee.bin + local tee_src="${RKBIN_DIR}/${TEE_BLOB}" + if [[ -f "${tee_src}" ]]; then + run_host_command_logged cp "${tee_src}" tee.bin + else + display_alert "TEE blob not found" "${tee_src}" "err" + return 1 + fi
276-297: Make FIT/ITB/idblock generation fully logged and robust.
- Wrap redirection so output is captured and errors are visible.
- Quote paths; check INI source exists before sed; use run_host_command_logged for rm.
- run_host_command_logged $SPL_FIT_GENERATOR -t 0x03d00000 -c lzma > u-boot.its + run_host_command_logged bash -lc '"$SPL_FIT_GENERATOR" -t 0x03d00000 -c lzma > u-boot.its' @@ - rm -f u-boot.itb + run_host_command_logged rm -f u-boot.itb @@ - RKBOOT_INI_FILE=rv1106.ini - cp $RKBIN_DIR/rv11/RV1106MINIALL.ini $RKBOOT_INI_FILE - sed -i "s|Path1=.*rv1106_ddr.*$|Path1=${RKBIN_DIR}/${DDR_BLOB}|g" $RKBOOT_INI_FILE - sed -i "s|Path1=.*rv1106_usbplug.*$|Path1=${RKBIN_DIR}/${USBPLUG_BLOB}|g" $RKBOOT_INI_FILE - sed -i "s|FlashData=.*$|FlashData=${RKBIN_DIR}/${DDR_BLOB}|g" $RKBOOT_INI_FILE + RKBOOT_INI_FILE="rv1106.ini" + local ini_src="${RKBIN_DIR}/rv11/RV1106MINIALL.ini" + [[ -f "${ini_src}" ]] || { display_alert "Missing INI" "${ini_src}" "err"; return 1; } + cp "${ini_src}" "${RKBOOT_INI_FILE}" + sed -i "s|Path1=.*rv1106_ddr.*$|Path1=${RKBIN_DIR}/${DDR_BLOB}|g" "${RKBOOT_INI_FILE}" + sed -i "s|Path1=.*rv1106_usbplug.*$|Path1=${RKBIN_DIR}/${USBPLUG_BLOB}|g" "${RKBOOT_INI_FILE}" + sed -i "s|FlashData=.*$|FlashData=${RKBIN_DIR}/${DDR_BLOB}|g" "${RKBOOT_INI_FILE}" - sed -i "s|FlashBoot=.*$|FlashBoot=./spl/u-boot-spl.bin|g" $RKBOOT_INI_FILE - sed -i "s|PATH=.*$|PATH=download.bin|g" $RKBOOT_INI_FILE - sed -i "s|IDB_PATH=.*$|IDB_PATH=idblock.img|g" $RKBOOT_INI_FILE - run_host_x86_binary_logged $RKBIN_DIR/tools/boot_merger $RKBOOT_INI_FILE - rm -f $RKBOOT_INI_FILE + sed -i "s|FlashBoot=.*$|FlashBoot=./spl/u-boot-spl.bin|g" "${RKBOOT_INI_FILE}" + sed -i "s|PATH=.*$|PATH=download.bin|g" "${RKBOOT_INI_FILE}" + sed -i "s|IDB_PATH=.*$|IDB_PATH=idblock.img|g" "${RKBOOT_INI_FILE}" + run_host_x86_binary_logged "${RKBIN_DIR}/tools/boot_merger" "${RKBOOT_INI_FILE}" + run_host_command_logged rm -f "${RKBOOT_INI_FILE}"
315-318: Quote destinations and ensure directories exist.Minor hardening for BSP installs.
- install -m 755 $SRC/packages/bsp/rv1106/${service_name}.sh $destination/usr/bin/${service_name}.sh - cp $SRC/packages/bsp/rv1106/${service_name}.service $destination/lib/systemd/system/${service_name}.service + install -d "${destination}/usr/bin" "${destination}/lib/systemd/system" + install -m 755 "${SRC}/packages/bsp/rv1106/${service_name}.sh" "${destination}/usr/bin/${service_name}.sh" + install -m 644 "${SRC}/packages/bsp/rv1106/${service_name}.service" "${destination}/lib/systemd/system/${service_name}.service"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (7)
patch/u-boot/legacy/u-boot-rockchip-buildroot/0000.patching_config.yamlis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/defconfig/luckfox_rv1106_uboot_defconfigis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/dt/rv1106-luckfox.dtsis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-malloc_8_hide_optee.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-mem_layout.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-sysboot-extlinux.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/use-armv7a-for-modern-toolchain.patchis excluded by!patch/**
📒 Files selected for processing (5)
config/boards/luckfox-pico-mini.csc(1 hunks)config/kernel/linux-rv1106-vendor.config(1 hunks)config/sources/families/rockchip.conf(2 hunks)packages/bsp/rv1106/rv1106-firstboot-makeswap.service(1 hunks)packages/bsp/rv1106/rv1106-firstboot-makeswap.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/bsp/rv1106/rv1106-firstboot-makeswap.service
- config/kernel/linux-rv1106-vendor.config
- packages/bsp/rv1106/rv1106-firstboot-makeswap.sh
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-14T06:32:29.806Z
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.
Applied to files:
config/sources/families/rockchip.conf
📚 Learning: 2025-09-11T06:12:54.213Z
Learnt from: SuperKali
PR: armbian/build#8609
File: config/boards/nanopi-r76s.conf:5-5
Timestamp: 2025-09-11T06:12:54.213Z
Learning: In the Armbian build system, board family configuration files (like config/sources/families/rk35xx.conf) can inherit kernel branch definitions from common include files (like config/sources/families/include/rockchip64_common.inc). Even if a branch like "edge" is not defined directly in the family conf file, it may be available through the sourced include file.
Applied to files:
config/sources/families/rockchip.conf
📚 Learning: 2025-10-14T05:08:11.785Z
Learnt from: EvilOlaf
PR: armbian/build#8754
File: config/boards/bestv-r3300-l.csc:14-16
Timestamp: 2025-10-14T05:08:11.785Z
Learning: In the Armbian build system, BOOTBRANCH_BOARD is a valid framework variable used as a fallback when BOOTBRANCH is unset. The framework checks BOOTBRANCH_BOARD before applying the default bootloader branch value (see config/sources/common.conf). Board configuration files can use BOOTBRANCH_BOARD to specify the bootloader branch.
Applied to files:
config/sources/families/rockchip.conf
📚 Learning: 2025-09-14T06:29:18.958Z
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:64-70
Timestamp: 2025-09-14T06:29:18.958Z
Learning: In the Armbian build system, vendor branch configurations in family files are designed to be shared across multiple SoCs within the same family that use the same vendor kernel tree. For example, rk35xx and rockchip-rk3588 families both use identical vendor branch settings (same KERNELSOURCE, KERNELBRANCH, and KERNELPATCHDIR), demonstrating that vendor branches are intentionally generic rather than SoC-specific.
Applied to files:
config/sources/families/rockchip.conf
🧬 Code graph analysis (1)
config/sources/families/rockchip.conf (3)
config/sources/vendors/mekotronics/mekotronics-rk3588.hooks.sh (2)
post_family_config__meko_use_mainline_uboot(22-42)write_uboot_platform(39-41)lib/functions/configuration/main-config.sh (1)
write_config_summary_output_file(488-541)lib/functions/compilation/uboot.sh (4)
compile_uboot_target(52-299)uboot_postinst_base(544-562)patch_uboot_target(22-48)compile_uboot(373-542)
🔇 Additional comments (2)
config/sources/families/rockchip.conf (2)
304-306: dd offsets align with Rockchip layout; status=none matches current standard.Good choice of 0x40/0x440 sector seeks and status=none; postinst sync will flush writes.
If you want extra resilience on slow media, consider adding conv=fsync (optional).
62-79: Kernel config file confirmed to exist for rv1106-vendor branch; no issues found.The file
/home/jailuser/git/config/kernel/linux-rv1106-vendor.configis present, soLINUXCONFIG="linux-rv1106-${BRANCH}"will resolve correctly when BRANCH=vendor and build failures due to missing config are not a concern.
| BOOTBRANCH="branch:next-dev-buildroot" | ||
| BOOTPATCHDIR="legacy/u-boot-rockchip-buildroot" | ||
| BOOTDIR="u-boot-rockchip" | ||
| LINUXCONFIG="linux-rv1106-${BRANCH}" |
There was a problem hiding this comment.
This is effectively a different kernel. Change LINUXFAMILY accordingly.
There was a problem hiding this comment.
I thought about this. Since other rockchip 32bit SoCs are stable on mainline kernel and don't use vendor kernel from ages, another possibility is to keep rv11xx SoCs in rockchip family and revive the vendor kernel and bump it to latest 6.1-rk6.1 to support rv11xx.
The linux-rv1106-vendor kernel config would overwrite the linux-rockchip-vendor config and there won't be a separate rockchip family.
On the other side, I see these rv11xx boards have hard memory contraints and other peculiarities. This could indeed justify a family of its own to keep more control on kernel and OS configuration (ie: optimize for size rather than speed, remove useless kernel modules, prefer built-ins modules, ...)
The current solution is not the best IMHO: at first sight I had the feeling it is "hijacking" the rockchip family to fit rv11xx boards into.
There was a problem hiding this comment.
@amazingfate actually revived the vendor kernel for RK3506 in #8619
RV1106 and RK3506 share a fair bit in common INCLUDING a shared patch set (not my own) that should enable mainline support for both.
See: https://codeberg.org/megi/linux/commits/branch/luckfox-6.18
I plan to add an edge branch for rv1106 + rk3506 based on this as soon as my PR is merged.
With that in mind, could it make sense to combine RV1106 / RK3506 into a single family with rv1106 kernelconfig overriding the rk3506-focused kernelconfig? I think this could be the cleanest option for maintainability going forward.
There was a problem hiding this comment.
@amazingfate actually revived the vendor kernel for RK3506 in #8619 RV1106 and RK3506 share a fair bit in common INCLUDING a shared patch set (not my own) that should enable mainline support for both. See: https://codeberg.org/megi/linux/commits/branch/luckfox-6.18
With that in mind, could it make sense to combine RV1106 / RK3506 into a single family with rv1106 kernelconfig overriding the rk3506-focused kernelconfig? I think this could be the cleanest option for maintainability going forward.
I don't know the rk3506 capabilities, but if you think they are in the same area and have the same or similar constraints, probably it is a good idea to have a separate family to make things cleaner and more maintainable. rockchip have had too so many families in the past we tried to merge things to keep them manageable, but in this case I guess the platform (and contraints) differs enough from rk322x/rk3288 that a separate family could be a wise idea.
As @rpardini also noted, there are some hints in your code a new family is probably the natural choice.
There was a problem hiding this comment.
About the LINUXCONFIG vs LINUXFAMILY, it's not about being similar or not.
If only LINUXCONFIG is changed, a different version (due to different .config hash) of the same .deb package will be produced. It works fine for building images, as each image build gets its own specified parameters and effectively the correct kernel. But it won't work when publishing packages to apt repo, as the package name will be the same, with two different versions.
There was a problem hiding this comment.
Thanks for explaining @rpardini -- I have restructured this PR to create a new family rockchip-rv1106 to prevent this issue 👍
Then defo get your own family. |
| # during daemon-reload operations. | ||
| # Address with a fixed /run size of 20MB | ||
| display_alert "Increasing /run size in /etc/fstab" "debug" | ||
| echo "tmpfs /run tmpfs rw,nosuid,nodev,noexec,relatime,size=20M,mode=755 0 0" >> "${MOUNT}"/etc/fstab |
There was a problem hiding this comment.
given your RAM constraints, how worth it is it to use ext2 plus zram? note ext2 or just be sure to disable the journal on ext3. is there a smaller/more-compact/efficient FS?
There was a problem hiding this comment.
mind you, zram only helps if there are actual files and not just directories or [empty] lock-files, and worst case uses MORE memory for the various empty metadata tables.
| echo "/swapfile swap swap defaults,nofail,pri=0 0 0" >> /etc/fstab | ||
| fi | ||
|
|
||
| swapon -p 0 "$SWAPFILE" |
There was a problem hiding this comment.
might swapon -a be more idiomatic?
There was a problem hiding this comment.
I don't want to activate / otherwise interfere with any other swap devices that users may setup in userpatches. swapon -a (without a priority set) just feels less precise.
vidplace7
force-pushed
the
luckfox-pico-mini
branch
from
2bd66da to
15a7024
Compare
|
This PR has been redone to use it's own family, as suggested by the comments here 👍 |
vidplace7
force-pushed
the
luckfox-pico-mini
branch
from
15a7024 to
bb4adc8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
config/sources/families/rockchip-rv1106.conf (1)
86-107: Consider documenting the FIT load address.Line 89 uses a hardcoded load address
0x03d00000without explanation. Consider adding a comment explaining why this specific address is used for the TEE load address in the FIT image.Example:
+ # TEE load address for RV1106/RV1103 (vendor-specified) run_host_command_logged $SPL_FIT_GENERATOR -t 0x03d00000 -c lzma > u-boot.itsThe INI template manipulation approach is reasonable given the constraints of the Rockchip tooling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (7)
patch/u-boot/legacy/u-boot-rockchip-buildroot/0000.patching_config.yamlis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/defconfig/luckfox_rv1106_uboot_defconfigis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/dt/rv1106-luckfox.dtsis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-malloc_8_hide_optee.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-mem_layout.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-sysboot-extlinux.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/use-armv7a-for-modern-toolchain.patchis excluded by!patch/**
📒 Files selected for processing (5)
config/boards/luckfox-pico-mini.csc(1 hunks)config/kernel/linux-rockchip-rv1106-vendor.config(1 hunks)config/sources/families/rockchip-rv1106.conf(1 hunks)packages/bsp/rv1106/rv1106-firstboot-makeswap.service(1 hunks)packages/bsp/rv1106/rv1106-firstboot-makeswap.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/bsp/rv1106/rv1106-firstboot-makeswap.service
- config/kernel/linux-rockchip-rv1106-vendor.config
- config/boards/luckfox-pico-mini.csc
- packages/bsp/rv1106/rv1106-firstboot-makeswap.sh
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-11T06:12:54.213Z
Learnt from: SuperKali
PR: armbian/build#8609
File: config/boards/nanopi-r76s.conf:5-5
Timestamp: 2025-09-11T06:12:54.213Z
Learning: In the Armbian build system, board family configuration files (like config/sources/families/rk35xx.conf) can inherit kernel branch definitions from common include files (like config/sources/families/include/rockchip64_common.inc). Even if a branch like "edge" is not defined directly in the family conf file, it may be available through the sourced include file.
Applied to files:
config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-09-14T06:29:18.958Z
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:64-70
Timestamp: 2025-09-14T06:29:18.958Z
Learning: In the Armbian build system, vendor branch configurations in family files are designed to be shared across multiple SoCs within the same family that use the same vendor kernel tree. For example, rk35xx and rockchip-rk3588 families both use identical vendor branch settings (same KERNELSOURCE, KERNELBRANCH, and KERNELPATCHDIR), demonstrating that vendor branches are intentionally generic rather than SoC-specific.
Applied to files:
config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-09-14T06:32:29.806Z
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.
Applied to files:
config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-06-25T03:40:52.109Z
Learnt from: EvilOlaf
PR: armbian/build#8330
File: config/sources/families/sun55iw3.conf:32-36
Timestamp: 2025-06-25T03:40:52.109Z
Learning: In Armbian build system configuration files like config/sources/families/*.conf, when KERNELSOURCE is not explicitly declared in a case branch, the system falls back to a default KERNELSOURCE value, so missing KERNELSOURCE declarations are not critical issues.
Applied to files:
config/sources/families/rockchip-rv1106.conf
🧬 Code graph analysis (1)
config/sources/families/rockchip-rv1106.conf (2)
config/sources/vendors/mekotronics/mekotronics-rk3588.hooks.sh (2)
post_family_config__vendor_uboot_mekotronics(8-16)post_family_config__meko_use_mainline_uboot(22-42)lib/functions/configuration/main-config.sh (1)
do_main_configuration(12-358)
🔇 Additional comments (7)
config/sources/families/rockchip-rv1106.conf (7)
1-33: LGTM! Well-structured family initialization.The basic configuration is well-organized with appropriate defaults for the memory-constrained RV1106 family. The BOOT_SOC fallback mechanism and the kernel command-line parameters (cma=0, video=off) align with the PR objectives for optimizing extremely low-memory devices.
35-47: LGTM! Solid BOOT_SOC handling with good error handling.The conditional blob configuration is well-structured with appropriate error handling for unknown SoCs. The use of
:-parameter expansion allows board configs to override these defaults while providing sensible fallbacks.
50-60: Verify whether additional kernel branches should be supported.Currently, only the
vendorbranch is defined. For production use, consider whethercurrentoredgebranches should also be supported, or if vendor-only is intentional for this memory-constrained platform.Based on learnings about Armbian family configurations potentially inheriting branches from common includes.
62-75: LGTM! Appropriate GCC 13+ compatibility fix.This hook correctly handles the known enum-int-mismatch issue with Rockchip vendor U-Boot and GCC 13+. The implementation follows best practices for version detection and flag injection.
109-118: LGTM! Proper dual-context bootloader installation.The write_uboot_platform implementation correctly handles both build-time and live-system upgrade scenarios. The dd commands follow current Rockchip family standards with proper status=none parameters.
Based on learnings about Rockchip family write_uboot_platform patterns.
120-138: LGTM! Appropriate system group and swap service setup.The gpio/i2c group creation with specific GIDs is properly implemented for udev rule support, and the firstboot swap service is a practical solution for the extreme memory constraints of these devices.
140-172: LGTM! Comprehensive BSP asset installation.The BSP tweaks properly install peripheral access rules, sound configuration, and optional firmware hooks. The swap service installation correctly coordinates with the earlier enablement hook.
| function pre_config_uboot_target__copy_tee() { | ||
|
|
||
| # Makefiles expect 'tee.bin' in the uboot build directory | ||
| display_alert "Copy TEE blob" "info" | ||
| run_host_command_logged cp ${RKBIN_DIR}/${TEE_BLOB} tee.bin | ||
|
|
||
| return 0 | ||
| } |
There was a problem hiding this comment.
Fix display_alert parameter order.
The display_alert call is missing the context parameter. It should follow the pattern: display_alert "$BOARD" "Copy TEE blob" "info" to be consistent with other calls in this file.
Apply this diff:
- display_alert "Copy TEE blob" "info"
+ display_alert "$BOARD" "Copy TEE blob" "info"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function pre_config_uboot_target__copy_tee() { | |
| # Makefiles expect 'tee.bin' in the uboot build directory | |
| display_alert "Copy TEE blob" "info" | |
| run_host_command_logged cp ${RKBIN_DIR}/${TEE_BLOB} tee.bin | |
| return 0 | |
| } | |
| function pre_config_uboot_target__copy_tee() { | |
| # Makefiles expect 'tee.bin' in the uboot build directory | |
| display_alert "$BOARD" "Copy TEE blob" "info" | |
| run_host_command_logged cp ${RKBIN_DIR}/${TEE_BLOB} tee.bin | |
| return 0 | |
| } |
🤖 Prompt for AI Agents
In config/sources/families/rockchip-rv1106.conf around lines 77 to 84 the
display_alert call is missing the context/board parameter; change the call to
include the board variable as the first argument so it matches the file pattern,
e.g. call display_alert "$BOARD" "Copy TEE blob" "info" (use quoted variables).
There was a problem hiding this comment.
Good idea, bad details. $BOARD is, for any important purpose, a constant within a run. this adds no actual useful information.
- display_alert "Copy TEE blob" "info"
+ display_alert "pre_config_uboot_target" "Copy TEE blob" "info"
This actually adds info, albeit I'm unclear its the most useful.
There was a problem hiding this comment.
@tabrisnet, you're absolutely right! Using the function/hook name like "pre_config_uboot_target" provides much more useful context about where in the build process the alert occurs, which is indeed more valuable for debugging than the board name.
Thank you for the correction—your suggestion is a clear improvement over my initial recommendation.
🧠 Learnings used
Learnt from: SuperKali
PR: armbian/build#8609
File: config/boards/nanopi-r76s.conf:5-5
Timestamp: 2025-09-11T06:12:54.213Z
Learning: In the Armbian build system, board family configuration files (like config/sources/families/rk35xx.conf) can inherit kernel branch definitions from common include files (like config/sources/families/include/rockchip64_common.inc). Even if a branch like "edge" is not defined directly in the family conf file, it may be available through the sourced include file.
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:64-70
Timestamp: 2025-09-14T06:29:18.958Z
Learning: In the Armbian build system, vendor branch configurations in family files are designed to be shared across multiple SoCs within the same family that use the same vendor kernel tree. For example, rk35xx and rockchip-rk3588 families both use identical vendor branch settings (same KERNELSOURCE, KERNELBRANCH, and KERNELPATCHDIR), demonstrating that vendor branches are intentionally generic rather than SoC-specific.
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.
Learnt from: Grippy98
PR: armbian/build#8622
File: config/sources/families/k3.conf:66-66
Timestamp: 2025-09-12T19:28:38.491Z
Learning: In the Armbian k3 family build system (config/sources/families/k3.conf), builds do not fail when TIBOOT3_BOOTCONFIG is unset, even though tiboot3.bin is still listed in UBOOT_TARGET_MAP. The gating mechanism in pre_config_uboot_target__build_first_stage function works as intended to conditionally build/copy tiboot3.bin only when TIBOOT3_BOOTCONFIG is defined.
- Adds support for "RV1106" family (RV1103 + RV1106) - Adds support for Luckfox Pico Mini (RV1103)
vidplace7
force-pushed
the
luckfox-pico-mini
branch
from
bb4adc8 to
383bf70
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
config/sources/families/rockchip-rv1106.conf (1)
77-84: Fix display_alert call to include context parameter.Line 80 calls
display_alertwith only 2 parameters, which is inconsistent with the 3-parameter pattern used elsewhere in this file (lines 45, 66, 70) and doesn't follow the feedback from the previous review suggesting the function name provides better context than$BOARD.Apply this diff:
- display_alert "Copy TEE blob" "info" + display_alert "pre_config_uboot_target" "Copy TEE blob" "info"
🧹 Nitpick comments (1)
config/sources/families/rockchip-rv1106.conf (1)
174-193: Consider more robust /run fstab handling and RuntimeDirectorySize tuning.The current grep check (line 187) successfully prevents duplicate entries, which improves upon the initial implementation. However, the previous review discussion by tabrisnet suggested two alternative approaches that might be more robust:
Delete-then-add pattern to ensure the entry is always correct:
if grep -q "tmpfs /run tmpfs" "${MOUNT}/etc/fstab"; then sed -i -e 's#tmpfs /run.*##' "${MOUNT}/etc/fstab" fi echo "tmpfs /run tmpfs rw,nosuid,nodev,noexec,relatime,size=20M,mode=755 0 0" >> "${MOUNT}/etc/fstab"Exit with error if an entry already exists (to catch race conditions and identify what else might be adding this entry):
if grep -q "tmpfs /run tmpfs" "${MOUNT}/etc/fstab"; then exit_with_error "/run entry already exists in fstab - investigate source" fiAdditionally, tabrisnet mentioned adjusting
/etc/systemd/logind.conf'sRuntimeDirectorySize(or disabling it entirely) for these extremely memory-constrained devices, which isn't currently addressed.Consider which approach best fits the build system's error-handling philosophy and whether the RuntimeDirectorySize tuning would provide meaningful benefits for 64-256MB RAM devices.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (7)
patch/u-boot/legacy/u-boot-rockchip-buildroot/0000.patching_config.yamlis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/defconfig/luckfox_rv1106_uboot_defconfigis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/dt/rv1106-luckfox.dtsis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-malloc_8_hide_optee.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-mem_layout.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/rv1103-rv1106-sysboot-extlinux.patchis excluded by!patch/**patch/u-boot/legacy/u-boot-rockchip-buildroot/use-armv7a-for-modern-toolchain.patchis excluded by!patch/**
📒 Files selected for processing (5)
config/boards/luckfox-pico-mini.csc(1 hunks)config/kernel/linux-rockchip-rv1106-vendor.config(1 hunks)config/sources/families/rockchip-rv1106.conf(1 hunks)packages/bsp/rv1106/rv1106-firstboot-makeswap.service(1 hunks)packages/bsp/rv1106/rv1106-firstboot-makeswap.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/bsp/rv1106/rv1106-firstboot-makeswap.service
- config/boards/luckfox-pico-mini.csc
- packages/bsp/rv1106/rv1106-firstboot-makeswap.sh
- config/kernel/linux-rockchip-rv1106-vendor.config
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-14T06:29:18.958Z
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:64-70
Timestamp: 2025-09-14T06:29:18.958Z
Learning: In the Armbian build system, vendor branch configurations in family files are designed to be shared across multiple SoCs within the same family that use the same vendor kernel tree. For example, rk35xx and rockchip-rk3588 families both use identical vendor branch settings (same KERNELSOURCE, KERNELBRANCH, and KERNELPATCHDIR), demonstrating that vendor branches are intentionally generic rather than SoC-specific.
Applied to files:
config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-10-26T10:41:35.107Z
Learnt from: HackingGate
PR: armbian/build#8665
File: config/boards/photonicat2.csc:4-4
Timestamp: 2025-10-26T10:41:35.107Z
Learning: In the Armbian build system, rk3576 boards consistently use BOARDFAMILY="rk35xx" for both vendor and edge kernel targets. The rk35xx family configuration sources rockchip64_common.inc, which provides edge and current kernel branch definitions, making these branches available even though they're not defined directly in rk35xx.conf.
Applied to files:
config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-09-14T06:32:29.806Z
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.
Applied to files:
config/sources/families/rockchip-rv1106.conf
🧬 Code graph analysis (1)
config/sources/families/rockchip-rv1106.conf (2)
config/sources/vendors/mekotronics/mekotronics-rk3588.hooks.sh (2)
post_family_config__vendor_uboot_mekotronics(8-16)post_family_config__meko_use_mainline_uboot(22-42)lib/functions/configuration/main-config.sh (2)
write_config_summary_output_file(488-541)do_main_configuration(12-358)
🔇 Additional comments (7)
config/sources/families/rockchip-rv1106.conf (7)
1-34: LGTM! Clean family configuration setup.The header, extension enabling, BOOT_SOC defaulting, and basic configuration variables follow Armbian conventions correctly.
35-47: LGTM! Proper SOC-specific blob configuration.The conditional blob defaults for RV1103 and RV1106 are well-structured with appropriate error handling for unknown BOOT_SOC values. The use of
${VAR:-default}allows board-level overrides while providing sensible defaults.
50-60: LGTM! Vendor kernel configuration follows established patterns.The vendor branch configuration correctly uses shared vendor kernel tree settings (KERNELSOURCE, KERNELBRANCH, KERNELPATCHDIR) and declares KERNEL_MAJOR_MINOR, consistent with how other Rockchip families configure vendor branches. Based on learnings.
62-75: LGTM! Proper GCC 13+ compatibility workaround.The function correctly detects GCC version and conditionally adds compiler flags to work around the enum-int-mismatch error in vendor U-Boot. The implementation is clean and properly documented.
109-118: LGTM! Proper bootloader installation implementation.The
write_uboot_platform()function correctly handles both build-time and board-side execution with conditional logging. The dd commands use appropriate parameters (conv=notrunc,status=none) consistent with current Rockchip family standards. Based on learnings.
120-138: LGTM! Appropriate system group and swap configuration.The family tweaks correctly create gpio and i2c groups that pair with udev rules, and the mkswap firstboot service enablement is a sensible approach for addressing the extremely limited RAM (64-256MB) on these devices.
140-172: LGTM! Proper BSP file installation.Both BSP tweak functions correctly install necessary files (udev rules, ALSA config, firmware hooks, swap service) to appropriate system locations with proper permissions. The conditional firmware installation is a good practice.
| uboot_custom_postprocess() { | ||
|
|
||
| display_alert "Generate u-boot.its" "info" | ||
| run_host_command_logged $SPL_FIT_GENERATOR -t 0x03d00000 -c lzma > u-boot.its | ||
|
|
||
| display_alert "Generate u-boot.itb" "info" | ||
| # Remove uboot-generated itb if exists | ||
| rm -f u-boot.itb | ||
| run_host_command_logged tools/mkimage -f u-boot.its -E u-boot.itb | ||
|
|
||
| display_alert "Generate idblock.img with u-boot SPL" "debug" | ||
| RKBOOT_INI_FILE=rv1106.ini | ||
| cp $RKBIN_DIR/rv11/RV1106MINIALL.ini $RKBOOT_INI_FILE | ||
| sed -i "s|Path1=.*rv1106_ddr.*$|Path1=${RKBIN_DIR}/${DDR_BLOB}|g" $RKBOOT_INI_FILE | ||
| sed -i "s|Path1=.*rv1106_usbplug.*$|Path1=${RKBIN_DIR}/${USBPLUG_BLOB}|g" $RKBOOT_INI_FILE | ||
| sed -i "s|FlashData=.*$|FlashData=${RKBIN_DIR}/${DDR_BLOB}|g" $RKBOOT_INI_FILE | ||
| sed -i "s|FlashBoot=.*$|FlashBoot=./spl/u-boot-spl.bin|g" $RKBOOT_INI_FILE | ||
| sed -i "s|PATH=.*$|PATH=download.bin|g" $RKBOOT_INI_FILE | ||
| sed -i "s|IDB_PATH=.*$|IDB_PATH=idblock.img|g" $RKBOOT_INI_FILE | ||
| run_host_x86_binary_logged $RKBIN_DIR/tools/boot_merger $RKBOOT_INI_FILE | ||
| rm -f $RKBOOT_INI_FILE | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inconsistent display_alert calls missing context parameter.
Lines 88, 91, and 96 use only 2 parameters in display_alert calls, which is inconsistent with the 3-parameter pattern used elsewhere in this file. Following the pattern from previous review feedback, these should include the function name as context for better debugging.
Apply this diff to make the calls consistent:
- display_alert "Generate u-boot.its" "info"
+ display_alert "uboot_custom_postprocess" "Generate u-boot.its" "info"
run_host_command_logged $SPL_FIT_GENERATOR -t 0x03d00000 -c lzma > u-boot.its
- display_alert "Generate u-boot.itb" "info"
+ display_alert "uboot_custom_postprocess" "Generate u-boot.itb" "info"
# Remove uboot-generated itb if exists
rm -f u-boot.itb
run_host_command_logged tools/mkimage -f u-boot.its -E u-boot.itb
- display_alert "Generate idblock.img with u-boot SPL" "debug"
+ display_alert "uboot_custom_postprocess" "Generate idblock.img with u-boot SPL" "debug"
RKBOOT_INI_FILE=rv1106.ini🤖 Prompt for AI Agents
In config/sources/families/rockchip-rv1106.conf around lines 86 to 107, the
display_alert calls on lines 88, 91, and 96 omit the context parameter; update
those three calls to include the current function name
(uboot_custom_postprocess) as the third argument so they follow the file's
3-parameter pattern for consistent logging and easier debugging.
5 participants
Description
Adds new family Rockchip RV1106, which includes the RV1103 and RV1106 SoCs from Rockchip.
RV1106 family includes a single-core
armv7lprocessor with 64-256MB of RAM.Adds first board in "RV1106" family, Luckfox Pico Mini. This is an RV1103 board with 64MB of DRAM 🥔.
Depends on:
I intend to add support for these other boards in this family in the future:
What works:
idblock.img(Rockchipddr.bin; U-Bootspl/u-boot-spl.bin) +u-boot.itb(Rockchiptee.bin; U-Bootu-boot.dtb,u-boot-nodtb.bin)Family optimizations
apt-get.basic.target(available early)linux-rv1106-vendor.configwith optimizations for the platform.video=off cma=0, etc). This is an extremely memory constrained platform, if you need multimedia support, use buildroot/yocto./runtmpfs size to 20M. systemd throws errors when less than 16MB is free in this partition duringdaemon-reload.I'd absolutely welcome critique of any of these optimizations, especially tuning
linux-rv1106-vendor.config❤️Why use this potato? 🥔
⚡ Power Consumption!! These little guys draw ~0.4W at idle. In the Meshtastic LoRa mesh community we're using these for solar-powered BBSs 👴
How Has This Been Tested?
./compile.sh DOCKER_ARMBIAN_BASE_IMAGE="ubuntu:jammy" BOARD=luckfox-pico-mini BRANCH=vendor KERNEL_CONFIGURE=no DEB_COMPRESS=xz KERNEL_BTF=yes KERNEL_GIT=full INSTALL_HEADERS=yes INSTALL_ARMBIAN_FIRMWARE=yes BUILD_DESKTOP=no BUILD_MINIMAL=yes RELEASE=bookworm./compile.sh DOCKER_ARMBIAN_BASE_IMAGE="ubuntu:jammy" BOARD=luckfox-pico-mini BRANCH=vendor KERNEL_CONFIGURE=no DEB_COMPRESS=xz KERNEL_BTF=yes KERNEL_GIT=full INSTALL_HEADERS=yes INSTALL_ARMBIAN_FIRMWARE=yes BUILD_DESKTOP=no BUILD_MINIMAL=yes RELEASE=trixie./compile.sh DOCKER_ARMBIAN_BASE_IMAGE="ubuntu:jammy" BOARD=luckfox-pico-mini BRANCH=vendor KERNEL_CONFIGURE=no DEB_COMPRESS=xz KERNEL_BTF=yes KERNEL_GIT=full INSTALL_HEADERS=yes INSTALL_ARMBIAN_FIRMWARE=yes BUILD_DESKTOP=no BUILD_MINIMAL=yes RELEASE=jammy./compile.sh DOCKER_ARMBIAN_BASE_IMAGE="ubuntu:jammy" BOARD=luckfox-pico-mini BRANCH=vendor KERNEL_CONFIGURE=no DEB_COMPRESS=xz KERNEL_BTF=yes KERNEL_GIT=full INSTALL_HEADERS=yes INSTALL_ARMBIAN_FIRMWARE=yes BUILD_DESKTOP=no BUILD_MINIMAL=yes RELEASE=nobleChecklist: