WIP: Add Seeed Studio reComputer RK3576/RK3588 DevKit support#9719
WIP: Add Seeed Studio reComputer RK3576/RK3588 DevKit support#9719baorepo wants to merge 26 commits intoarmbian:mainfrom
Conversation
Extract duplicated hooks from rk3576 and rk3588 board configs into recomputer-rk35xx-common.inc. Both configs source the shared file and keep only SoC-specific hooks. Common hooks: bluez (FCS960K), USB gadget, Morse FGH100M tools, AIC8800 wireless, security hardening, PATH config, USB auto-mount. Key fixes included: - USB gadget: NM managed via ExecStartPost in usbdevice.service drop-in (not via broken keyfile unmanaged-devices exception) - dnsmasq: After=usbdevice + Restart=on-failure for reliable boot - serial-getty@ttyGS0: ordering via drop-in, not custom service - USB auto-mount: udev SYSTEMD_WANTS triggers usb-automount@.service, mounts under /media/seeed/ with proper ownership - SSH: MaxAuthTries 6 (OpenSSH default) to support multi-key clients Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
When usbdevice fails on first attempt, serial-getty@ttyGS0 tries to start before ttyGS0 exists and stays dead. Add Restart=on-failure so it retries after usbdevice eventually succeeds. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
bind-interfaces fails if usb0 doesn't exist at startup, requiring After=usbdevice + Restart=on-failure workarounds. bind-dynamic tolerates missing interfaces and serves DHCP when usb0 appears, removing the need for the dnsmasq systemd drop-in entirely. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Clear BindsTo=dev-ttyGS0.device from the base unit via drop-in override. ttyGS0 is a USB gadget device not present at boot, so the base unit's BindsTo causes a dependency failure. Also drop Restart=on-failure override to preserve the base unit's Restart=always, preventing the getty from staying dead after a normal logout. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Important Review skippedIgnore keyword(s) in the title. ⛔ Ignored keywords (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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
size/large
The BT UART assignment is per-board, not per-SoC. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Move BT UART selection from BOOT_SOC-based case logic in common.inc to per-board BT_UART variables. RK3576 DevKit uses ttyS4, RK3588 DevKit uses ttyS6. Remove redundant pre_umount_final_image BT UART hook from RK3576 config since common.inc BSP hook now handles it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| AIC8800_TYPE="sdio" | ||
| enable_extension "radxa-aic8800" | ||
|
|
||
| source "${SRC}/config/boards/recomputer-rk35xx-common.inc" |
There was a problem hiding this comment.
Hi, this is not standard. There's no real standard for this, really, except using extensions and/or using family code. Long ago I've tried a "vendor" common dir for the Mekotronics, see sources/vendors/mekotronics and config/boards/mekotronics-r58x.csc? In the end it's just about the file/folder structure.
There was a problem hiding this comment.
Okay, I made revisions based on your two references. Thanks
| BOARD_NAME="reComputer RK3576 Devkit" | ||
| BOARDFAMILY="rk35xx" | ||
| BOOT_SOC="rk3576" | ||
| BOARD_MAINTAINER="Seeed Studio" |
There was a problem hiding this comment.
a named individual as “maintainer” with GitHub ID must be preset in the BOARD_MAINTAINER within Board Configuration File
| "camera-engine-rkaiq" | ||
| } | ||
|
|
||
| # Kernel config customizations for RK3576 |
There was a problem hiding this comment.
IIRC custom kernel configs within board configuration file is a no-go as it breaks cache.
Perhaps consider create an entire new board family?
| } | ||
|
|
||
| # Kernel config customizations for RK3588 | ||
| function custom_kernel_config__recomputer_rk3588_customizations() { |
| #Seeed Studio Rockchip RK3588 octa core 4/8/16GB RAM SoC | ||
| BOARD_NAME="reComputer-RK3588-Devkit" | ||
| BOARDFAMILY="rockchip-rk3588" | ||
| BOARD_MAINTAINER="seeed studio" |
| driver_rtl8723DU | ||
| driver_uwe5622 | ||
| driver_rtl8723cs | ||
| driver_fgh100m |
There was a problem hiding this comment.
For additional out of tree drivers PLEASE create an extension. The driver harness and driver_network is already a huge mess which shall be eventually be moved into one or multiple extensions at some point. Just need to find somebody who is willing to dive into and do that. Until then please don't add no new drivers this way.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (5)
lib/functions/compilation/patch/drivers_network.sh (1)
926-940:⚠️ Potential issue | 🟠 MajorMove FGH100M integration to an extension, and until then scope it to the affected families.
Two concerns on this new harness entry:
Per earlier reviewer feedback on the companion file, out-of-tree drivers should be added as an extension, not by extending
drivers_network.sh/drivers-harness.sh. Because this entire file is hashed into the drivers cache key, any change here invalidates the drivers-patch cache for every kernel/family across the framework — adding a Seeed-specific driver here pays that cost for every build.Unconditional scope — unlike the surrounding out-of-tree drivers (e.g.,
driver_rtl8852bsguards onLINUXFAMILY in spacemit|rk35xx|rockchip64,driver_uwe5622guards onsun*|rockchip64|rk35xx,driver_rtl88x2csguards onmeson64), this one only gates on kernel version ≥6.1. That means the FGH100Mpatch/misc/wireless-fgh100m.patchwill be applied anddrivers/net/wireless/fgh100m/added to the Makefile/Kconfig on every kernel build the driver harness runs against, including families that have nothing to do with the Morse FGH100M HaLow module. If the patch doesn't apply cleanly on some family kernel, it'll break that board's build; if it does apply, you're injecting a driver directory into unrelated images.Until migration to an extension is done, please at minimum add the family guard, matching how the Morse module is actually wired (Seeed RK35xx boards):
🛠️ Minimal interim scoping
driver_fgh100m() { - - if linux-version compare "${version}" ge 6.1; then + if linux-version compare "${version}" ge 6.1 && [[ "${LINUXFAMILY}" == rk35xx || "${LINUXFAMILY}" == rockchip-rk3588 ]]; then display_alert "Adding" "MorseMicro FG-H100M WiFi (MORSE) driver patch for kernel ${version}" "info" process_patch_file "${SRC}/patch/misc/wireless-fgh100m.patch" "applying" - # Add to section Makefile (append, like other drivers) echo "obj-\$(CONFIG_WLAN_VENDOR_MORSE) += fgh100m/" >> "$kerneldir/drivers/net/wireless/Makefile" - # Add to wireless Kconfig (insert after a stable anchor, like other drivers) sed -i '/source "drivers\/net\/wireless\/ti\/Kconfig"/a source "drivers\/net\/wireless\/fgh100m\/Kconfig"' \ "$kerneldir/drivers/net/wireless/Kconfig" fi }Also note the function body is indented with 4 spaces whereas the rest of this file uses tabs — please align style.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/functions/compilation/patch/drivers_network.sh` around lines 926 - 940, The driver_fgh100m function unconditionally injects an out-of-tree driver for all families and uses spaces for indentation; scope it to the affected families and fix indentation: wrap the existing checks so the body only runs when both the kernel version check and a family guard (e.g., LINUXFAMILY matches rk35xx—or the same family set you use for other Morse drivers) are true, keep the call to process_patch_file("${SRC}/patch/misc/wireless-fgh100m.patch"), the Makefile echo and the sed Kconfig insertion, and convert the function body indentation to tabs; also add a TODO comment to move this integration into an extension (so reviewers know it's temporary).config/boards/recomputer-rk3588-devkit.conf (2)
4-4:⚠️ Potential issue | 🟠 Major
BOARD_MAINTAINERmust be a GitHub user ID of a named individual, not a company name.Per the Armbian Board Support Rules linked in the past review,
.conf(standard-supported) boards require a named individual maintainer with a GitHub ID."seeed studio"is neither an individual nor a GitHub handle. Without a qualifying maintainer, the board does not qualify for standard support and should either be renamed to a valid GitHub handle or the file moved to.csc(community supported).As per coding guidelines/learnings: ".wip and .conf extensions require BOARD_MAINTAINER to be present and contain at least one maintainer. If no maintainer is present, ... it does not qualify for standard support and must be moved to community support (.csc extension)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/boards/recomputer-rk3588-devkit.conf` at line 4, The BOARD_MAINTAINER value "seeed studio" is not an individual GitHub user ID as required; update the BOARD_MAINTAINER entry to a real GitHub handle of a named individual (e.g., replace "seeed studio" with the maintainer's GitHub username) so the .conf board qualifies for standard support, or if no individual maintainer exists move the board to community support by changing the file to a .csc variant and/or remove/adjust BOARD_MAINTAINER accordingly.
56-90:⚠️ Potential issue | 🟠 MajorBoard-level kernel config changes break cross-board kernel package consistency.
custom_kernel_config__*andkernel_config_set_*are tied toLINUXFAMILY/BOARDFAMILY, not individual boards. BecauseBOARDFAMILY="rockchip-rk3588"is shared with other boards that will consume the same kernel.debfrom the apt repo, applying these toggles here causes inconsistent kernel packages depending on which board triggered the build. Move these config toggles into a family-level overlay (config/kernel/linux-rockchip-rk3588-*.configorconfig/sources/families/*.inc) instead of the board.conf.As per coding guidelines/learnings: "do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__ functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. ... Kernel configuration changes must be made in the appropriate kernel config file ... or in family configuration files ... instead."*
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/boards/recomputer-rk3588-devkit.conf` around lines 56 - 90, The board-specific function custom_kernel_config__recomputer_rk3588_customizations and its kernel_config_set_*/kernel_config_set_m/y/n calls must be removed from the board .conf and relocated to the Rockchip family-level kernel config (e.g., add the toggles to config/kernel/linux-rockchip-rk3588-*.config or the family include under config/sources/families/*.inc) so the kernel options (CONFIG_RTC_DRV_PCF8563, CONFIG_MORSE_*, CONFIG_VIDEO_*, CONFIG_NVMEM_ROCKCHIP_OTP, CONFIG_DRM_PANFROST/PANTHOR, CONFIG_NET_VENDOR_REALTEK, CONFIG_SENSORS_PWM_FAN, etc.) are applied per LINUXFAMILY/BOARDFAMILY and not per individual BOARD; remove the custom_kernel_config__recomputer_rk3588_customizations function and its kernel_config_set_* invocations from the board file after migrating the settings.config/boards/recomputer-rk3576-devkit.conf (2)
5-5:⚠️ Potential issue | 🟠 Major
BOARD_MAINTAINERshould be a GitHub user ID of a named individual.
.conf(standard-supported) boards must list a named individual maintainer by GitHub handle, not a company name. If no individual maintainer can be committed to long-term support of this board, the file should be moved to.csc(community supported) instead.As per coding guidelines/learnings: ".wip and .conf extensions require BOARD_MAINTAINER to be present and contain at least one maintainer. If no maintainer is present, ... it does not qualify for standard support and must be moved to community support (.csc extension)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/boards/recomputer-rk3576-devkit.conf` at line 5, The BOARD_MAINTAINER entry currently contains a company name; update the BOARD_MAINTAINER value to a real GitHub user handle of a named individual who will maintain this board (e.g., set BOARD_MAINTAINER="githubHandle") so the .conf file meets standard-supported requirements, or if no individual maintainer can be committed, convert this configuration to a community-supported file by renaming/adapting it to the .csc format instead of keeping it as a .conf.
40-67:⚠️ Potential issue | 🟠 MajorBoard-level kernel config changes break cross-board kernel package consistency.
This hook modifies kernel
.configat the board level, but kernel configuration is associated withLINUXFAMILY/BOARDFAMILYin the Armbian build system — other boards in therk35xxfamily will consume the same kernel.debfrom the apt repo, so whichever board triggered the build determines the final kernel content. This yields inconsistent published packages.Move these toggles to
config/kernel/linux-rk35xx-vendor.config(or the appropriate family.inc) so they apply consistently to every board in the family.As per coding guidelines/learnings: "do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__ functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. ... Kernel configuration changes must be made in the appropriate kernel config file ... or in family configuration files ... instead."*
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/boards/recomputer-rk3576-devkit.conf` around lines 40 - 67, The custom_kernel_config__recomputer_rk3576_customizations function is modifying kernel .config at the board level using kernel_config_set_y/kernel_config_set_m/kernel_config_set_val, which must instead live in the rk35xx family/kernel config; move all toggles (CONFIG_GPIO_PCA953X, CONFIG_RTC_DRV_PCF8563, CONFIG_WLAN_VENDOR_MORSE, CONFIG_MORSE_SPI, CONFIG_MORSE_USER_ACCESS, CONFIG_MORSE_VENDOR_COMMAND, CONFIG_MORSE_MONITOR, CONFIG_MAC80211_MESH, CONFIG_MORSE_DEBUG_MASK, CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_V2, CONFIG_VIDEO_IMX708, CONFIG_VIDEO_DW9800W, CONFIG_REGULATOR_AD5398, CONFIG_SENSORS_PWM_FAN) out of custom_kernel_config__recomputer_rk3576_customizations and add equivalent settings to the family-level kernel config (e.g., config/kernel/linux-rk35xx-vendor.config or the rk35xx .inc) so the kernel package is consistent across the rk35xx LINUXFAMILY/BOARDFAMILY builds.
🧹 Nitpick comments (10)
patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts (1)
44-53: Drop commented-out DT blocks before merge.There are several dead commented-out blocks (
vcc5v0_sysnode at lines 44–53,// startup-delay-usat lines 73/85,// vpcie3v3-supplyat lines 158/178). These should be deleted so the final DTS reflects the intended hardware configuration without ambiguity about "is this supposed to be enabled?". If some of them are meant for future enablement, note that in a single/* TODO */comment at the top of the affected node instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts` around lines 44 - 53, Remove all dead commented-out device-tree blocks so the DTS is unambiguous: delete the commented node for vcc5v0_sys (the block containing "vcc5v0_sys" / "vcc5v0-sys"), remove the commented "startup-delay-us" entries, and remove the commented "vpcie3v3-supply" blocks; if any of these are intentionally retained for future use, replace the entire commented block with a single inline TODO comment at the top of the affected node (e.g., "/* TODO: re-enable vcc5v0_sys if needed */") rather than leaving large commented code sections.patch/u-boot/legacy/u-boot-radxa-rk35xx/board_recomputer-rk3588-devkit/0001-change-hdmi-voltage-swing.patch (1)
22-30: Remove the commented-out0x2fregister writes.The previous voltage-swing values are preserved as
//comments next to the new0x20writes, which is effectively dead code in a production HDMI PHY init sequence. Either delete the commented lines, or if the intent is to document the prior tuning, reduce it to a single explanatory comment above the block (e.g., "voltage-swing lowered from 0x2f to 0x20 for RK3588 reComputer HDMI signal integrity"). Makes future rebases against the vendor driver cleaner.♻️ Proposed cleanup
- // voltage-swing - // hdptx_write(hdptx, LANE_REG0303, 0x2f); - // hdptx_write(hdptx, LANE_REG0403, 0x2f); - // hdptx_write(hdptx, LANE_REG0503, 0x2f); - // hdptx_write(hdptx, LANE_REG0603, 0x2f); + /* voltage-swing: lowered from 0x2f to 0x20 for reComputer RK3588 */ hdptx_write(hdptx, LANE_REG0303, 0x20); hdptx_write(hdptx, LANE_REG0403, 0x20); hdptx_write(hdptx, LANE_REG0503, 0x20); hdptx_write(hdptx, LANE_REG0603, 0x20); - // pre-shoot + /* pre-shoot */ hdptx_write(hdptx, LANE_REG0305, 0x03);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/u-boot/legacy/u-boot-radxa-rk35xx/board_recomputer-rk3588-devkit/0001-change-hdmi-voltage-swing.patch` around lines 22 - 30, Remove the dead commented-out register writes and leave only the active hdptx_write calls: delete the four lines with "// hdptx_write(hdptx, LANE_REG0[3-6]03, 0x2f);" and, if desired, replace them with a single concise comment above the block like "voltage-swing lowered from 0x2f to 0x20 for RK3588 reComputer HDMI signal integrity" so the change is documented while keeping the active hdptx_write(hdptx, LANE_REG0303, 0x20), hdptx_write(hdptx, LANE_REG0403, 0x20), hdptx_write(hdptx, LANE_REG0503, 0x20), hdptx_write(hdptx, LANE_REG0603, 0x20) lines intact.patch/u-boot/legacy/u-boot-radxa-rk35xx/board_recomputer-rk3588-devkit/0002-fix-download-devtype-reinit.patch (1)
1-21: Duplicate of the RK3576 counterpart — consider deduplicating.This patch is byte-identical to
patch/u-boot/legacy/u-boot-radxa-rk35xx/board_recomputer-rk3576-devkit/0002-fix-download-devtype-reinit.patch. Since both boards share theu-boot-radxa-rk35xxtree, consider placing shared fixes in a commonpatch/u-boot/legacy/u-boot-radxa-rk35xx/directory (applied to both boards) rather than maintaining two copies. Not blocking, but reduces future drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/u-boot/legacy/u-boot-radxa-rk35xx/board_recomputer-rk3588-devkit/0002-fix-download-devtype-reinit.patch` around lines 1 - 21, The same change to boot_devtype_init (adding need_reinit logic) is duplicated across two board-specific patches; remove the duplicate by extracting the shared edit into a single patch placed under the u-boot-radxa-rk35xx common patch directory and delete the board-specific copy; update the patch series/manifest to reference only the new common patch and ensure the patch header/description mentions both rk3576 and rk3588 boards so the single patch applies to both trees and prevents future drift.config/bootscripts/boot-seeed-rk35xx.cmd (2)
52-52: EEPROM signature check is correct but brittle; consider documenting.The six-byte ASCII magic
"rk35xx"(0x72 0x6b 0x33 0x35 0x78 0x78) is matched byte-by-byte viasetexpr.b— functional, but any future bump to a 7+ byte magic or a different vendor prefix will silently fall through to the fallback path. A short comment above line 52 documenting that these literals correspond to the ASCII bytes of"rk35xx"(and similarly for"00A0"/"00B0"at 63/68) would help future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/bootscripts/boot-seeed-rk35xx.cmd` at line 52, Add a brief inline comment above the if condition that checks ee0..ee5 (the line starting with if test "${ee0}" = "0x72" && test "${ee1}" = "0x6b" && test "${ee2}" = "0x33" && test "${ee3}" = "0x35" && test "${ee4}" = "0x78" && test "${ee5}" = "0x78"; then) documenting that those hex literals are the ASCII bytes for "rk35xx", and note that this byte-by-byte check is brittle (will not handle longer/future magic strings) and point maintainers to the analogous checks for "00A0"/"00B0" (the comparisons around ee* at the later checks) so they know to update comments or logic if the EEPROM signature changes.
1-146: Significant duplication withboot-seeed-rk3576.cmd.This script and
config/bootscripts/boot-seeed-rk3576.cmddiffer only inload_addr(0x9000000vs0x48000000) and the defaultfdtfilefallback. The EEPROM detection logic supports both RK3576 (00A0) and RK3588 (00B0) in both files. Maintaining two near-identical 146-line scripts is a long-term bug magnet — any overlay/bootargs/fixup change has to be kept in sync by hand.Consider either:
- Keeping a single
boot-seeed-rk35xx.cmdand setting the SoC-specificload_addr/fallbackfdtfilevia board-env (orarmbianEnv.txt), and pointing both boards'BOOTSCRIPTat it; or- Generating one from the other via
sedat build time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/bootscripts/boot-seeed-rk35xx.cmd` around lines 1 - 146, There is duplicate boot script logic between boot-seeed-rk35xx.cmd and boot-seeed-rk3576.cmd; consolidate them by creating a single shared boot-seeed-rk35xx.cmd that reads SoC-specific values from board environment (armbianEnv.txt) instead of hardcoding load_addr and fallback fdtfile: keep the EEPROM detection and overlay/bootargs logic as-is (symbols: eeprom_dtb_select, eeprom_dtb_matched, overlays, user_overlays, fdtfile, load_addr, bootargs) and change initialization so load_addr and the default fdtfile are set from environment variables (or fall back to sensible defaults) and ensure BOOTSCRIPT for each board points to the shared script; alternatively implement a build-time generator that produces the board-specific file by replacing load_addr and fallback fdtfile when packaging.config/boards/recomputer-rk3588-devkit.conf (2)
1-1: Expand the first-line hardware description and add a space after#.Per coding guidelines, the first-line comment in a board config is shown during interactive compilation and must list SoC model, core count, RAM options, and key features (connectivity, storage, notable IO). The current line omits storage/connectivity (eMMC, NVMe/PCIe, GbE, USB3, HDMI, WiFi/BT) and uses
#Seeedwithout the customary space after#.✏️ Suggested revision
-#Seeed Studio Rockchip RK3588 octa core 4/8/16GB RAM SoC +# Seeed Studio reComputer RK3588 DevKit octa core 4/8/16GB RAM eMMC NVMe GbE USB3 HDMI WiFi BTAs per coding guidelines/learnings: "the first line comment in Armbian board configuration files ... must specify: SoC model, core count, RAM options, and key features (connectivity options, storage interfaces, special features). This text is used during interactive compilation, so accuracy and completeness matter."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/boards/recomputer-rk3588-devkit.conf` at line 1, Update the first-line comment currently "#Seeed Studio Rockchip RK3588 octa core 4/8/16GB RAM SoC" to include a leading space after "#" and expand it to list SoC model, core count, RAM options and key features (storage and connectivity) — e.g. mention eMMC, NVMe/PCIe, GbE, USB3, HDMI, WiFi/BT — so the board header in recomputer-rk3588-devkit.conf clearly reads like: "# Seeed Studio Rockchip RK3588 octa-core, 4/8/16GB RAM — eMMC, NVMe/PCIe, Gigabit Ethernet, USB3, HDMI, WiFi/BT". Ensure you replace the existing string exactly and keep formatting as a single-line comment.
96-101: Quote$SDCARDexpansions to avoid word-splitting on path changes.Minor robustness nit:
$SDCARDexpansions on these lines are unquoted. While unlikely today, future directory changes with spaces would break the redirections. Prefer"${SDCARD}/etc/udev/rules.d/".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/boards/recomputer-rk3588-devkit.conf` around lines 96 - 101, The mkdir and echo lines use an unquoted $SDCARD variable which can break redirections if the directory contains spaces; update the usages in the mkdir -p and all echo >> redirections that write to 90-naming-audios.rules to use a quoted expansion of the variable (e.g., "${SDCARD}/etc/udev/rules.d/") so the path is safe from word-splitting.patch/u-boot/legacy/u-boot-radxa-rk35xx/defconfig/recomputer-rk3576-devkit_defconfig (2)
237-237: Trailing whitespace.
CONFIG_AVB_LIBAVB_ATX=yhas a trailing space; trivial but worth cleaning up so the line is byte-identical to the equivalent entry in the RK3588 defconfig.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/u-boot/legacy/u-boot-radxa-rk35xx/defconfig/recomputer-rk3576-devkit_defconfig` at line 237, The line containing CONFIG_AVB_LIBAVB_ATX has trailing whitespace; remove the trailing space so the entry is exactly "CONFIG_AVB_LIBAVB_ATX=y" (no trailing whitespace) to match the RK3588 defconfig and ensure byte-identical config entries. Target the line with the symbol CONFIG_AVB_LIBAVB_ATX in the defconfig and delete the extra space at the end of the line.
45-45: Use the canonical "is not set" form instead of=n.U-Boot defconfigs conventionally disable an option with
# CONFIG_FOO is not setrather thanCONFIG_FOO=n. The=nform is not wrong at parse time for bool symbols but will be rewritten bysavedefconfigand is inconsistent with the rest of this file (e.g.,# CONFIG_SPL_SYS_DCACHE_OFF is not set). For easier diffing against upstream and futuresavedefconfigregeneration:✏️ Suggested revision
-CONFIG_SPL_UFS_SUPPORT=n +# CONFIG_SPL_UFS_SUPPORT is not set🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/u-boot/legacy/u-boot-radxa-rk35xx/defconfig/recomputer-rk3576-devkit_defconfig` at line 45, Replace the boolean disablement line for CONFIG_SPL_UFS_SUPPORT so it uses the canonical "is not set" form; locate the CONFIG_SPL_UFS_SUPPORT entry and change the current `CONFIG_SPL_UFS_SUPPORT=n` style to the conventional `# CONFIG_SPL_UFS_SUPPORT is not set` line to match the rest of the defconfig and avoid savedefconfig churn.config/boards/recomputer-rk3576-devkit.conf (1)
1-1: Expand the first-line hardware description and add a space after#.Per coding guidelines, this one-line comment surfaces during interactive compilation and should describe hardware in detail: SoC + cores + RAM options + connectivity + storage + notable IO (eMMC, NVMe/PCIe, GbE, USB3, HDMI, WiFi/BT/HaLow, etc.).
✏️ Suggested revision
-#Seeed Studio Rockchip RK3576 octa core 4/8/16GB RAM SoC +# Seeed Studio reComputer RK3576 DevKit octa core 4/8/16GB RAM eMMC NVMe GbE USB3 HDMI WiFi BT HaLowAs per coding guidelines/learnings: "the first line comment ... must specify: SoC model, core count, RAM options, and key features (connectivity options, storage interfaces, special features). This text is used during interactive compilation, so accuracy and completeness matter."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/boards/recomputer-rk3576-devkit.conf` at line 1, Replace the terse first-line comment "#Seeed Studio Rockchip RK3576 octa core 4/8/16GB RAM SoC" with an expanded hardware description and add a space after the '#'; the new single-line header should state the SoC model (Rockchip RK3576), core count (octa‑core), RAM options (4/8/16GB), and key features such as storage interfaces (eMMC, NVMe/PCIe), networking (GbE, Wi‑Fi/BT/HaLow), USB/HDMI I/O and any notable peripherals present on the Recomputer RK3576 devkit so it can be shown during interactive compilation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/boards/recomputer-rk3588-devkit.conf`:
- Around line 32-53: Update the seeed_recomputer_download_and_install_deb flow
in recomputer-rk35xx-common.inc to verify downloaded .deb integrity and fail
noisily: modify the helper so callers like
pre_install_distribution_specific__recomputer_rk3588_install_libmali,
pre_install_distribution_specific__recomputer_rk3588_install_r8125_dkms, and
pre_install_distribution_specific__recomputer_rk3588_install_camera_engine pass
an expected SHA256 (or the helper looks it up), then after download compute and
compare the SHA256 and abort with a clear error if it mismatches or the download
is not a valid .deb; also handle HTTP errors (non-200/HTML responses) and return
non-zero on failure. Prefer switching the download to Armbian’s fetch_from_repo
/ GIT_FIXED_WORKDIR pattern or otherwise host/pin assets to ensure reproducible
builds and update the helper to use that pattern when a fixed commit/tag is
provided.
In `@config/sources/vendors/seeed-studio/recomputer-rk35xx-common.inc`:
- Around line 44-79: The current
pre_install_distribution_specific__recomputer_install_morse_tools unpacks debs
with dpkg-deb and cp (dpkg-deb -x ... and cp ... "${SDCARD}/"), which leaves
files orphaned outside the dpkg database; instead download the .deb URLs and
install them through the existing package install helpers (e.g.
seeed_recomputer_download_and_install_deb or install_deb_chroot /
install_deb_chroot_via_chroot) so the packages are installed via dpkg in the SD
card chroot, run maintainer scripts, update triggers/ldconfig, and register
files with apt/dpkg; replace the dpkg-deb/cp flow in
pre_install_distribution_specific__recomputer_install_morse_tools with calls to
those helpers and ensure errors are handled similarly to the other deb installs.
- Around line 230-232: The chown call hardcodes seeed:seeed (in the block using
mount -o noatime "$DEV" "$MPOINT" and the subsequent chown seeed:seeed
"$MPOINT"), which may not exist on the image; change this to either create the
seeed user/group before chown or — preferably — detect a real target user (use
SUDO_USER or determine the first non-root user/UID on the system at mount time),
resolve its uid/gid and call chown with that uid:gid for "$MPOINT" (fall back to
creating the seeed user via useradd if no suitable existing user is found), and
remove/suppress the 2>/dev/null on the chown error so failures are visible
during setup.
- Around line 216-243: The script's use of eval on the udev property output (the
usb-automount script line with eval "$(udevadm info ... | grep -E
'^ID_FS_LABEL_ENC=|^ID_MODEL=')" ) allows a malicious device to inject shell
code via ID_MODEL/ID_FS_LABEL_ENC; replace the eval with a safe parser: read
udevadm output line-by-line (e.g., while IFS= read -r line; do case "$line" in
ID_FS_LABEL_ENC=*) value="${line#ID_FS_LABEL_ENC=}" ;; ID_MODEL=*)
value="${line#ID_MODEL=}" ;; esac; done), strip any surrounding quotes, and then
sanitize LABEL (remove/control characters and shell metacharacters like
backticks, $(), ;, \, newlines — e.g., tr -d '\000-\037' and tr -cd '[:alnum:]
_-.' or sed to collapse spaces) before using it to build MPOINT; keep the rest
of usb-automount logic but ensure LABEL is never subject to shell expansion or
eval and always used quoted.
- Around line 6-24: Update the function
seeed_recomputer_download_and_install_deb to (1) accept or determine an expected
SHA256 (e.g., a new fourth argument or sidecar .sha256 file) and after download
verify the checksum with sha256sum before installing; (2) download into a
temporary path (use deb_file + .tmp) with wget using a timeout and retry flags
(e.g., --timeout=30 --tries=3), rm -f the temp file on any wget failure so
partial files aren't left behind, and only mv the temp to deb_file on success;
and (3) only call install_deb_chroot("${deb_file}", ...) after the checksum
passes, logging clear error messages via display_alert on download, checksum
mismatch, or install failures. Ensure references to deb_file,
SRC/cache/download, wget invocation, and install_deb_chroot are updated
accordingly.
- Around line 287-299: The current top-level clone/pull and enable_extension
calls run network I/O during config sourcing; move them into a deferred hook so
fetching happens in the proper build phase. Create a hook function (e.g.,
fetch_sources_tools__seeed_rk_extension_clone or
post_family_config__seeed_rk_extension_clone) that checks
ENABLE_SEEED_RK_EXTENSION and performs the git clone/pull into
${SRC}/extensions/seeed_armbian_extension with proper error handling, and call
enable_extension "seeed_armbian_extension" from inside that hook instead of at
top-level; keep the .inc file free of side-effectful network operations so
config parsing remains deterministic.
In
`@patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3576-recomputer-rk3576-devkit.dts`:
- Around line 20-29: The press-threshold-microvolt on the volumeup-key node is
using the wrong units (1750 µV) so the key never registers; update the property
press-threshold-microvolt in the volumeup-key node to the correct microvolt
value (1750000) to match the keyup-threshold-microvolt pattern (e.g.,
keyup-threshold-microvolt = <1800000>) so the VOL+ press region is around 1.75
V.
In
`@patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts`:
- Around line 13-14: The compatible property contains an invalid string with
spaces; update the compatible entry so it is a single token without whitespace
(e.g., replace the value used in the compatible property with a hyphenated token
like "seeed,recomputer-rk3588-devkit") while keeping the existing secondary
compatible "rockchip,rk3588"; locate the compatible property in the DTS (the
compatible = ... line) and change only that string to the hyphenated form to
match DT conventions and sibling RK3576 DTS.
- Around line 25-30: The press-threshold-microvolt value in the volumeup-key
node is using microvolts but was written as <1750> (1,750 µV) which is incorrect
for a 1.8 V threshold; update the volumeup-key's press-threshold-microvolt
property (node "volumeup-key") to <1750000> so it represents 1,750,000 µV (1.75
V). Also apply the same fix to the corresponding press-threshold-microvolt entry
in rk3576-recomputer-rk3576-devkit.dts to keep both DT files consistent.
- Around line 93-105: The pcie_clk regulator node sets enable-active-low but
passes the GPIO as GPIO_ACTIVE_HIGH, causing a polarity conflict; update the
pcie_clk node (regulator-fixed, regulator-name = "pcie_clk") to match the
vcc3v3_pcie_clk pattern by using enable-active-high with GPIO_ACTIVE_HIGH, or
alternatively change the gpio cell to GPIO_ACTIVE_LOW so the enable-active-low
flag and the gpio cell agree—make the pcie_clk node's enable polarity consistent
with the gpio cell.
---
Duplicate comments:
In `@config/boards/recomputer-rk3576-devkit.conf`:
- Line 5: The BOARD_MAINTAINER entry currently contains a company name; update
the BOARD_MAINTAINER value to a real GitHub user handle of a named individual
who will maintain this board (e.g., set BOARD_MAINTAINER="githubHandle") so the
.conf file meets standard-supported requirements, or if no individual maintainer
can be committed, convert this configuration to a community-supported file by
renaming/adapting it to the .csc format instead of keeping it as a .conf.
- Around line 40-67: The custom_kernel_config__recomputer_rk3576_customizations
function is modifying kernel .config at the board level using
kernel_config_set_y/kernel_config_set_m/kernel_config_set_val, which must
instead live in the rk35xx family/kernel config; move all toggles
(CONFIG_GPIO_PCA953X, CONFIG_RTC_DRV_PCF8563, CONFIG_WLAN_VENDOR_MORSE,
CONFIG_MORSE_SPI, CONFIG_MORSE_USER_ACCESS, CONFIG_MORSE_VENDOR_COMMAND,
CONFIG_MORSE_MONITOR, CONFIG_MAC80211_MESH, CONFIG_MORSE_DEBUG_MASK,
CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_V2, CONFIG_VIDEO_IMX708,
CONFIG_VIDEO_DW9800W, CONFIG_REGULATOR_AD5398, CONFIG_SENSORS_PWM_FAN) out of
custom_kernel_config__recomputer_rk3576_customizations and add equivalent
settings to the family-level kernel config (e.g.,
config/kernel/linux-rk35xx-vendor.config or the rk35xx .inc) so the kernel
package is consistent across the rk35xx LINUXFAMILY/BOARDFAMILY builds.
In `@config/boards/recomputer-rk3588-devkit.conf`:
- Line 4: The BOARD_MAINTAINER value "seeed studio" is not an individual GitHub
user ID as required; update the BOARD_MAINTAINER entry to a real GitHub handle
of a named individual (e.g., replace "seeed studio" with the maintainer's GitHub
username) so the .conf board qualifies for standard support, or if no individual
maintainer exists move the board to community support by changing the file to a
.csc variant and/or remove/adjust BOARD_MAINTAINER accordingly.
- Around line 56-90: The board-specific function
custom_kernel_config__recomputer_rk3588_customizations and its
kernel_config_set_*/kernel_config_set_m/y/n calls must be removed from the board
.conf and relocated to the Rockchip family-level kernel config (e.g., add the
toggles to config/kernel/linux-rockchip-rk3588-*.config or the family include
under config/sources/families/*.inc) so the kernel options
(CONFIG_RTC_DRV_PCF8563, CONFIG_MORSE_*, CONFIG_VIDEO_*,
CONFIG_NVMEM_ROCKCHIP_OTP, CONFIG_DRM_PANFROST/PANTHOR,
CONFIG_NET_VENDOR_REALTEK, CONFIG_SENSORS_PWM_FAN, etc.) are applied per
LINUXFAMILY/BOARDFAMILY and not per individual BOARD; remove the
custom_kernel_config__recomputer_rk3588_customizations function and its
kernel_config_set_* invocations from the board file after migrating the
settings.
In `@lib/functions/compilation/patch/drivers_network.sh`:
- Around line 926-940: The driver_fgh100m function unconditionally injects an
out-of-tree driver for all families and uses spaces for indentation; scope it to
the affected families and fix indentation: wrap the existing checks so the body
only runs when both the kernel version check and a family guard (e.g.,
LINUXFAMILY matches rk35xx—or the same family set you use for other Morse
drivers) are true, keep the call to
process_patch_file("${SRC}/patch/misc/wireless-fgh100m.patch"), the Makefile
echo and the sed Kconfig insertion, and convert the function body indentation to
tabs; also add a TODO comment to move this integration into an extension (so
reviewers know it's temporary).
---
Nitpick comments:
In `@config/boards/recomputer-rk3576-devkit.conf`:
- Line 1: Replace the terse first-line comment "#Seeed Studio Rockchip RK3576
octa core 4/8/16GB RAM SoC" with an expanded hardware description and add a
space after the '#'; the new single-line header should state the SoC model
(Rockchip RK3576), core count (octa‑core), RAM options (4/8/16GB), and key
features such as storage interfaces (eMMC, NVMe/PCIe), networking (GbE,
Wi‑Fi/BT/HaLow), USB/HDMI I/O and any notable peripherals present on the
Recomputer RK3576 devkit so it can be shown during interactive compilation.
In `@config/boards/recomputer-rk3588-devkit.conf`:
- Line 1: Update the first-line comment currently "#Seeed Studio Rockchip RK3588
octa core 4/8/16GB RAM SoC" to include a leading space after "#" and expand it
to list SoC model, core count, RAM options and key features (storage and
connectivity) — e.g. mention eMMC, NVMe/PCIe, GbE, USB3, HDMI, WiFi/BT — so the
board header in recomputer-rk3588-devkit.conf clearly reads like: "# Seeed
Studio Rockchip RK3588 octa-core, 4/8/16GB RAM — eMMC, NVMe/PCIe, Gigabit
Ethernet, USB3, HDMI, WiFi/BT". Ensure you replace the existing string exactly
and keep formatting as a single-line comment.
- Around line 96-101: The mkdir and echo lines use an unquoted $SDCARD variable
which can break redirections if the directory contains spaces; update the usages
in the mkdir -p and all echo >> redirections that write to
90-naming-audios.rules to use a quoted expansion of the variable (e.g.,
"${SDCARD}/etc/udev/rules.d/") so the path is safe from word-splitting.
In `@config/bootscripts/boot-seeed-rk35xx.cmd`:
- Line 52: Add a brief inline comment above the if condition that checks
ee0..ee5 (the line starting with if test "${ee0}" = "0x72" && test "${ee1}" =
"0x6b" && test "${ee2}" = "0x33" && test "${ee3}" = "0x35" && test "${ee4}" =
"0x78" && test "${ee5}" = "0x78"; then) documenting that those hex literals are
the ASCII bytes for "rk35xx", and note that this byte-by-byte check is brittle
(will not handle longer/future magic strings) and point maintainers to the
analogous checks for "00A0"/"00B0" (the comparisons around ee* at the later
checks) so they know to update comments or logic if the EEPROM signature
changes.
- Around line 1-146: There is duplicate boot script logic between
boot-seeed-rk35xx.cmd and boot-seeed-rk3576.cmd; consolidate them by creating a
single shared boot-seeed-rk35xx.cmd that reads SoC-specific values from board
environment (armbianEnv.txt) instead of hardcoding load_addr and fallback
fdtfile: keep the EEPROM detection and overlay/bootargs logic as-is (symbols:
eeprom_dtb_select, eeprom_dtb_matched, overlays, user_overlays, fdtfile,
load_addr, bootargs) and change initialization so load_addr and the default
fdtfile are set from environment variables (or fall back to sensible defaults)
and ensure BOOTSCRIPT for each board points to the shared script; alternatively
implement a build-time generator that produces the board-specific file by
replacing load_addr and fallback fdtfile when packaging.
In
`@patch/u-boot/legacy/u-boot-radxa-rk35xx/board_recomputer-rk3588-devkit/0001-change-hdmi-voltage-swing.patch`:
- Around line 22-30: Remove the dead commented-out register writes and leave
only the active hdptx_write calls: delete the four lines with "//
hdptx_write(hdptx, LANE_REG0[3-6]03, 0x2f);" and, if desired, replace them with
a single concise comment above the block like "voltage-swing lowered from 0x2f
to 0x20 for RK3588 reComputer HDMI signal integrity" so the change is documented
while keeping the active hdptx_write(hdptx, LANE_REG0303, 0x20),
hdptx_write(hdptx, LANE_REG0403, 0x20), hdptx_write(hdptx, LANE_REG0503, 0x20),
hdptx_write(hdptx, LANE_REG0603, 0x20) lines intact.
In
`@patch/u-boot/legacy/u-boot-radxa-rk35xx/board_recomputer-rk3588-devkit/0002-fix-download-devtype-reinit.patch`:
- Around line 1-21: The same change to boot_devtype_init (adding need_reinit
logic) is duplicated across two board-specific patches; remove the duplicate by
extracting the shared edit into a single patch placed under the
u-boot-radxa-rk35xx common patch directory and delete the board-specific copy;
update the patch series/manifest to reference only the new common patch and
ensure the patch header/description mentions both rk3576 and rk3588 boards so
the single patch applies to both trees and prevents future drift.
In
`@patch/u-boot/legacy/u-boot-radxa-rk35xx/defconfig/recomputer-rk3576-devkit_defconfig`:
- Line 237: The line containing CONFIG_AVB_LIBAVB_ATX has trailing whitespace;
remove the trailing space so the entry is exactly "CONFIG_AVB_LIBAVB_ATX=y" (no
trailing whitespace) to match the RK3588 defconfig and ensure byte-identical
config entries. Target the line with the symbol CONFIG_AVB_LIBAVB_ATX in the
defconfig and delete the extra space at the end of the line.
- Line 45: Replace the boolean disablement line for CONFIG_SPL_UFS_SUPPORT so it
uses the canonical "is not set" form; locate the CONFIG_SPL_UFS_SUPPORT entry
and change the current `CONFIG_SPL_UFS_SUPPORT=n` style to the conventional `#
CONFIG_SPL_UFS_SUPPORT is not set` line to match the rest of the defconfig and
avoid savedefconfig churn.
In
`@patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts`:
- Around line 44-53: Remove all dead commented-out device-tree blocks so the DTS
is unambiguous: delete the commented node for vcc5v0_sys (the block containing
"vcc5v0_sys" / "vcc5v0-sys"), remove the commented "startup-delay-us" entries,
and remove the commented "vpcie3v3-supply" blocks; if any of these are
intentionally retained for future use, replace the entire commented block with a
single inline TODO comment at the top of the affected node (e.g., "/* TODO:
re-enable vcc5v0_sys if needed */") rather than leaving large commented code
sections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b0175836-09b1-400b-8dfa-eb9a0f409de2
📒 Files selected for processing (18)
config/boards/recomputer-rk3576-devkit.confconfig/boards/recomputer-rk3588-devkit.confconfig/bootscripts/boot-seeed-rk3576.cmdconfig/bootscripts/boot-seeed-rk35xx.cmdconfig/kernel/linux-rk35xx-vendor.configconfig/sources/vendors/seeed-studio/recomputer-rk35xx-common.inclib/functions/compilation/patch/drivers-harness.shlib/functions/compilation/patch/drivers_network.shpatch/misc/wireless-fgh100m.patchpatch/u-boot/legacy/u-boot-radxa-rk35xx/board_recomputer-rk3576-devkit/0001-update-new-flash-for-rockchip.patchpatch/u-boot/legacy/u-boot-radxa-rk35xx/board_recomputer-rk3576-devkit/0002-fix-download-devtype-reinit.patchpatch/u-boot/legacy/u-boot-radxa-rk35xx/board_recomputer-rk3588-devkit/0001-change-hdmi-voltage-swing.patchpatch/u-boot/legacy/u-boot-radxa-rk35xx/board_recomputer-rk3588-devkit/0001-update-new-flash-for-rockchip.patchpatch/u-boot/legacy/u-boot-radxa-rk35xx/board_recomputer-rk3588-devkit/0002-fix-download-devtype-reinit.patchpatch/u-boot/legacy/u-boot-radxa-rk35xx/defconfig/recomputer-rk3576-devkit_defconfigpatch/u-boot/legacy/u-boot-radxa-rk35xx/defconfig/recomputer-rk3588-devkit_defconfigpatch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3576-recomputer-rk3576-devkit.dtspatch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts
| function pre_install_distribution_specific__recomputer_rk3588_install_libmali() { | ||
| seeed_recomputer_download_and_install_deb \ | ||
| "https://github.com/Seeed-Studio/seeed_armbian_extension/releases/download/v1.0/libmali-valhall-g610-g13p0-x11-wayland-gbm_1.9-1_arm64.deb" \ | ||
| "libmali-valhall-g610-g13p0-x11-wayland-gbm_1.9-1_arm64.deb" \ | ||
| "libmali-g610" | ||
| } | ||
|
|
||
| # Install Realtek r8125 DKMS package | ||
| function pre_install_distribution_specific__recomputer_rk3588_install_r8125_dkms() { | ||
| seeed_recomputer_download_and_install_deb \ | ||
| "https://github.com/Seeed-Studio/seeed_armbian_extension/releases/download/v1.0/realtek-r8125-kmod_9.016.01-1_arm64.deb" \ | ||
| "realtek-r8125-kmod_9.016.01-1_arm64.deb" \ | ||
| "realtek-r8125-dkms" | ||
| } | ||
|
|
||
| # Install RK camera engine package for RK3588 | ||
| function pre_install_distribution_specific__recomputer_rk3588_install_camera_engine() { | ||
| seeed_recomputer_download_and_install_deb \ | ||
| "https://github.com/Seeed-Studio/seeed_armbian_extension/releases/download/v1.0/camera_engine_rkaiq_rk3588_arm64_with_service.deb" \ | ||
| "camera_engine_rkaiq_rk3588_arm64_with_service.deb" \ | ||
| "camera-engine-rkaiq-rk3588" | ||
| } |
There was a problem hiding this comment.
Pin vendor .deb downloads with integrity verification (checksum) and failure handling.
Each pre_install_distribution_specific__* hook fetches a .deb from a GitHub release over the network and installs it directly into the rootfs without any integrity check (SHA256/signature). If the release is later replaced, mirrored by a MITM, or the URL returns HTML (e.g., a 404 page) the build will either fail silently in odd ways or install unintended content into every image. For reproducible/secure builds, please:
- Record a SHA256 for each deb and verify it after download.
- Consider using the Armbian
fetch_from_repo/GIT_FIXED_WORKDIRpattern or hosting assets in a place where you can pin by commit/tag.
This is a general concern about the seeed_recomputer_download_and_install_deb helper in recomputer-rk35xx-common.inc — the fix belongs there.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/boards/recomputer-rk3588-devkit.conf` around lines 32 - 53, Update the
seeed_recomputer_download_and_install_deb flow in recomputer-rk35xx-common.inc
to verify downloaded .deb integrity and fail noisily: modify the helper so
callers like
pre_install_distribution_specific__recomputer_rk3588_install_libmali,
pre_install_distribution_specific__recomputer_rk3588_install_r8125_dkms, and
pre_install_distribution_specific__recomputer_rk3588_install_camera_engine pass
an expected SHA256 (or the helper looks it up), then after download compute and
compare the SHA256 and abort with a clear error if it mismatches or the download
is not a valid .deb; also handle HTTP errors (non-200/HTML responses) and return
non-zero on failure. Prefer switching the download to Armbian’s fetch_from_repo
/ GIT_FIXED_WORKDIR pattern or otherwise host/pin assets to ensure reproducible
builds and update the helper to use that pattern when a fixed commit/tag is
provided.
| function seeed_recomputer_download_and_install_deb() { | ||
| local url="$1" | ||
| local cache_filename="$2" | ||
| local display_name="$3" | ||
|
|
||
| local deb_file="${SRC}/cache/download/${cache_filename}" | ||
| mkdir -p "${SRC}/cache/download" | ||
|
|
||
| if [[ ! -f "${deb_file}" ]]; then | ||
| display_alert "Downloading ${display_name}" "${url}" "info" | ||
| wget "${url}" -O "${deb_file}" || { | ||
| display_alert "Failed to download ${display_name}" "${display_name}" "err" | ||
| return 1 | ||
| } | ||
| fi | ||
|
|
||
| install_deb_chroot "${deb_file}" "" "" "" | ||
| display_alert "${display_name} installed successfully" "${display_name}" "info" | ||
| } |
There was a problem hiding this comment.
Download helper: add integrity check, timeout, and remove partial files on failure.
Three issues with seeed_recomputer_download_and_install_deb:
- No checksum verification —
.debassets are fetched over the network and installed into the rootfs with no SHA256/signature check. A future release asset replacement or MITM silently changes what goes into every image. - Partial file poisoning — on
wgetfailure,-O "${deb_file}"often leaves a truncated file on disk; the next build sees[[ -f "${deb_file}" ]]as true and passes garbage todpkg -i. Write to a.tmpfile andmvon success, orrm -fthe partial on failure. - No timeout / retries — a slow/stuck connection will hang the build indefinitely. Add
--timeout=30 --tries=3(or equivalent).
🛡️ Proposed fix
function seeed_recomputer_download_and_install_deb() {
local url="$1"
local cache_filename="$2"
local display_name="$3"
+ local expected_sha256="${4:-}" # optional; strongly recommended
local deb_file="${SRC}/cache/download/${cache_filename}"
mkdir -p "${SRC}/cache/download"
if [[ ! -f "${deb_file}" ]]; then
display_alert "Downloading ${display_name}" "${url}" "info"
- wget "${url}" -O "${deb_file}" || {
- display_alert "Failed to download ${display_name}" "${display_name}" "err"
- return 1
- }
+ local tmp_file="${deb_file}.tmp"
+ if ! wget --timeout=30 --tries=3 -O "${tmp_file}" "${url}"; then
+ rm -f "${tmp_file}"
+ display_alert "Failed to download ${display_name}" "${display_name}" "err"
+ return 1
+ fi
+ if [[ -n "${expected_sha256}" ]]; then
+ echo "${expected_sha256} ${tmp_file}" | sha256sum -c - || {
+ rm -f "${tmp_file}"
+ display_alert "Checksum mismatch for ${display_name}" "${display_name}" "err"
+ return 1
+ }
+ fi
+ mv "${tmp_file}" "${deb_file}"
fi
install_deb_chroot "${deb_file}" "" "" ""
display_alert "${display_name} installed successfully" "${display_name}" "info"
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/sources/vendors/seeed-studio/recomputer-rk35xx-common.inc` around
lines 6 - 24, Update the function seeed_recomputer_download_and_install_deb to
(1) accept or determine an expected SHA256 (e.g., a new fourth argument or
sidecar .sha256 file) and after download verify the checksum with sha256sum
before installing; (2) download into a temporary path (use deb_file + .tmp) with
wget using a timeout and retry flags (e.g., --timeout=30 --tries=3), rm -f the
temp file on any wget failure so partial files aren't left behind, and only mv
the temp to deb_file on success; and (3) only call
install_deb_chroot("${deb_file}", ...) after the checksum passes, logging clear
error messages via display_alert on download, checksum mismatch, or install
failures. Ensure references to deb_file, SRC/cache/download, wget invocation,
and install_deb_chroot are updated accordingly.
| function pre_install_distribution_specific__recomputer_install_morse_tools() { | ||
| display_alert "Installing Morse wireless tools for FGH100M" "morse-tools" "info" | ||
|
|
||
| local cache_dir="${SRC}/cache/download" | ||
| local extract_dir="${SRC}/cache/morse-tools-extract" | ||
| mkdir -p "${cache_dir}" "${extract_dir}" | ||
|
|
||
| declare -A packages=( | ||
| ["wpa-supplicant-morse-tools"]="https://github.com/Seeed-Studio/seeed_armbian_extension/releases/download/v1.0/wpa-supplicant-morse-tools_1.0-1_arm64.deb" | ||
| ["morsectrl-tools"]="https://github.com/Seeed-Studio/seeed_armbian_extension/releases/download/v1.0/morsectrl-tools_1.0-1_arm64.deb" | ||
| ["hostapd-morse-tools"]="https://github.com/Seeed-Studio/seeed_armbian_extension/releases/download/v1.0/hostapd-morse-tools_1.0-1_arm64.deb" | ||
| ) | ||
|
|
||
| for pkg_name in "${!packages[@]}"; do | ||
| local pkg_url="${packages[$pkg_name]}" | ||
| local pkg_file="${cache_dir}/${pkg_name}_1.0-1_arm64.deb" | ||
|
|
||
| if [[ ! -f "${pkg_file}" ]]; then | ||
| display_alert "Downloading ${pkg_name}" "${pkg_url}" "info" | ||
| wget "${pkg_url}" -O "${pkg_file}" || { | ||
| display_alert "Failed to download ${pkg_name}" "${pkg_name}" "err" | ||
| continue | ||
| } | ||
| fi | ||
|
|
||
| display_alert "Extracting ${pkg_name}" "morse-tools" "info" | ||
| rm -rf "${extract_dir}/${pkg_name}" | ||
| mkdir -p "${extract_dir}/${pkg_name}" | ||
| dpkg-deb -x "${pkg_file}" "${extract_dir}/${pkg_name}" | ||
|
|
||
| display_alert "Copying ${pkg_name} files" "morse-tools" "info" | ||
| cp -r "${extract_dir}/${pkg_name}"/* "${SDCARD}/" | ||
| done | ||
|
|
||
| display_alert "Morse wireless tools installed successfully" "morse-tools" "info" | ||
| } |
There was a problem hiding this comment.
Morse tools are copied around dpkg, producing orphaned files.
dpkg-deb -x "${pkg_file}" "${extract_dir}/${pkg_name}" + cp -r .../* "${SDCARD}/" unpacks three debs into the rootfs without running their maintainer scripts and without registering them in the dpkg database. Downsides:
- No alternatives/triggers/ldconfig run —
postinstside-effects are lost. - Files are unknown to
apt/dpkgand will not be upgraded, removed cleanly, or flagged as conffiles. - Users running
apt-get installof a conflicting package later will have silent overwrites.
Please install these the same way as seeed_recomputer_download_and_install_deb does the other debs (install_deb_chroot) or, better, publish them in a repo and chroot_sdcard_apt_get_install them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/sources/vendors/seeed-studio/recomputer-rk35xx-common.inc` around
lines 44 - 79, The current
pre_install_distribution_specific__recomputer_install_morse_tools unpacks debs
with dpkg-deb and cp (dpkg-deb -x ... and cp ... "${SDCARD}/"), which leaves
files orphaned outside the dpkg database; instead download the .deb URLs and
install them through the existing package install helpers (e.g.
seeed_recomputer_download_and_install_deb or install_deb_chroot /
install_deb_chroot_via_chroot) so the packages are installed via dpkg in the SD
card chroot, run maintainer scripts, update triggers/ldconfig, and register
files with apt/dpkg; replace the dpkg-deb/cp flow in
pre_install_distribution_specific__recomputer_install_morse_tools with calls to
those helpers and ensure errors are handled similarly to the other deb installs.
| cat > "${SDCARD}/usr/local/bin/usb-automount" <<- 'SCRIPT' | ||
| #!/bin/sh | ||
| ACTION="$1" | ||
| DEV="$2" | ||
| BASE="/media/seeed" | ||
|
|
||
| eval "$(udevadm info --query=property --name="$DEV" 2>/dev/null | grep -E '^ID_FS_LABEL_ENC=|^ID_MODEL=')" | ||
| LABEL="${ID_FS_LABEL_ENC:-${ID_MODEL:-$(basename "$DEV")}}" | ||
| LABEL=$(echo "$LABEL" | sed 's/^ *//;s/ *$//' | tr -s ' ') | ||
|
|
||
| case "$ACTION" in | ||
| add) | ||
| MPOINT="$BASE/$LABEL" | ||
| mkdir -p "$MPOINT" | ||
| if mount -o noatime "$DEV" "$MPOINT" 2>/dev/null || \ | ||
| mount -o noatime,uid=1000,gid=1000,umask=002 "$DEV" "$MPOINT" 2>/dev/null; then | ||
| chown seeed:seeed "$MPOINT" 2>/dev/null | ||
| else | ||
| rmdir "$MPOINT" 2>/dev/null | ||
| fi | ||
| ;; | ||
| remove) | ||
| MPOINT="$BASE/$LABEL" | ||
| umount "$DEV" 2>/dev/null || true | ||
| rmdir "$MPOINT" 2>/dev/null || true | ||
| ;; | ||
| esac | ||
| SCRIPT |
There was a problem hiding this comment.
Security: eval on udev properties enables root-level command injection via a USB label.
At line 222 the script runs:
eval "$(udevadm info --query=property --name="$DEV" 2>/dev/null | grep -E '^ID_FS_LABEL_ENC=|^ID_MODEL=')"ID_MODEL is not udev-escaped and is derived from the USB device's USB descriptors (controlled by whoever provides the stick). A crafted device with a model string containing $(…), backticks, or ; will execute arbitrary shell as root when plugged in — this is run from a udev-triggered systemd unit with full root privileges. Even ID_FS_LABEL_ENC, while escaped, is passed through sed into filesystem paths without sanitizing backslash-escape sequences.
Prefer a non-eval property reader. Example:
🛡️ Proposed fix
- eval "$(udevadm info --query=property --name="$DEV" 2>/dev/null | grep -E '^ID_FS_LABEL_ENC=|^ID_MODEL=')"
- LABEL="${ID_FS_LABEL_ENC:-${ID_MODEL:-$(basename "$DEV")}}"
- LABEL=$(echo "$LABEL" | sed 's/^ *//;s/ *$//' | tr -s ' ')
+ ID_FS_LABEL_ENC=$(udevadm info --query=property --name="$DEV" --property=ID_FS_LABEL_ENC --value 2>/dev/null)
+ ID_MODEL=$(udevadm info --query=property --name="$DEV" --property=ID_MODEL --value 2>/dev/null)
+ LABEL="${ID_FS_LABEL_ENC:-${ID_MODEL:-$(basename "$DEV")}}"
+ # Sanitize: keep only a safe character class for path component
+ LABEL=$(printf '%s' "$LABEL" | tr -c 'A-Za-z0-9._-' '_' )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/sources/vendors/seeed-studio/recomputer-rk35xx-common.inc` around
lines 216 - 243, The script's use of eval on the udev property output (the
usb-automount script line with eval "$(udevadm info ... | grep -E
'^ID_FS_LABEL_ENC=|^ID_MODEL=')" ) allows a malicious device to inject shell
code via ID_MODEL/ID_FS_LABEL_ENC; replace the eval with a safe parser: read
udevadm output line-by-line (e.g., while IFS= read -r line; do case "$line" in
ID_FS_LABEL_ENC=*) value="${line#ID_FS_LABEL_ENC=}" ;; ID_MODEL=*)
value="${line#ID_MODEL=}" ;; esac; done), strip any surrounding quotes, and then
sanitize LABEL (remove/control characters and shell metacharacters like
backticks, $(), ;, \, newlines — e.g., tr -d '\000-\037' and tr -cd '[:alnum:]
_-.' or sed to collapse spaces) before using it to build MPOINT; keep the rest
of usb-automount logic but ensure LABEL is never subject to shell expansion or
eval and always used quoted.
| if mount -o noatime "$DEV" "$MPOINT" 2>/dev/null || \ | ||
| mount -o noatime,uid=1000,gid=1000,umask=002 "$DEV" "$MPOINT" 2>/dev/null; then | ||
| chown seeed:seeed "$MPOINT" 2>/dev/null |
There was a problem hiding this comment.
Hardcoded seeed:seeed owner may not exist on the image.
chown seeed:seeed "$MPOINT" assumes a seeed user/group is present in the image. Armbian's default first-boot flow creates a user chosen interactively, and there is no seeed UID created by this PR. If the user is absent, chown silently fails (stderr suppressed by 2>/dev/null) and leaves the mountpoint owned by root, so the documented UX (seeed-owned mounts) won't hold. Either create the seeed user explicitly in a tweak, or fall back to the SUDO_USER / first-user UID detected at mount time.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/sources/vendors/seeed-studio/recomputer-rk35xx-common.inc` around
lines 230 - 232, The chown call hardcodes seeed:seeed (in the block using mount
-o noatime "$DEV" "$MPOINT" and the subsequent chown seeed:seeed "$MPOINT"),
which may not exist on the image; change this to either create the seeed
user/group before chown or — preferably — detect a real target user (use
SUDO_USER or determine the first non-root user/UID on the system at mount time),
resolve its uid/gid and call chown with that uid:gid for "$MPOINT" (fall back to
creating the seeed user via useradd if no suitable existing user is found), and
remove/suppress the 2>/dev/null on the chown error so failures are visible
during setup.
| if [[ "${ENABLE_SEEED_RK_EXTENSION}" == "yes" ]]; then | ||
| if [[ ! -d "${SRC}/extensions/seeed_armbian_extension" ]]; then | ||
| echo "[${BOARD_NAME}] Cloning Seeed extensions from GitHub..." | ||
| git clone https://github.com/Seeed-Studio/seeed_armbian_extension.git "${SRC}/extensions/seeed_armbian_extension" || { | ||
| echo "[${BOARD_NAME}] Warning: Failed to clone Seeed extensions" | ||
| } | ||
| else | ||
| echo "[${BOARD_NAME}] Seeed extensions already exists, pulling updates..." | ||
| git -C "${SRC}/extensions/seeed_armbian_extension" pull || echo "[${BOARD_NAME}] Warning: Failed to pull updates" | ||
| fi | ||
|
|
||
| enable_extension "seeed_armbian_extension" | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm other Armbian board/family confs avoid network operations at source time
rg -nP -C2 '^\s*(git clone|git -C .* pull|wget)\b' config/sources config/boards | head -80Repository: armbian/build
Length of output: 2769
🏁 Script executed:
#!/bin/bash
# Read the full file to understand scope of git clone/pull operations
cat -n config/sources/vendors/seeed-studio/recomputer-rk35xx-common.inc | head -310Repository: armbian/build
Length of output: 13204
🏁 Script executed:
#!/bin/bash
# Find board configs that source this .inc file
rg -nP 'recomputer-rk35xx-common\.inc' config/boards config/sources | head -20Repository: armbian/build
Length of output: 313
🏁 Script executed:
#!/bin/bash
# Check for extension initialization hooks and patterns
rg -nP 'fetch_sources|post_family_config|enable_extension' lib/functions extensions config/sources/families | grep -E '(hook|function|enable_extension)' | head -30Repository: armbian/build
Length of output: 2621
🏁 Script executed:
#!/bin/bash
# Examine post_family_config hook pattern and when it's called
rg -nP 'function (post_family_config|fetch_sources)' lib/functions | head -5Repository: armbian/build
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Find where hooks are called in the build system
rg -nP 'call_extension_method.*post_family_config|call_extension_method.*fetch_sources' lib/functions | head -10Repository: armbian/build
Length of output: 461
🏁 Script executed:
#!/bin/bash
# Find enable_extension definition and understand when it can be called
rg -nP 'function enable_extension|^enable_extension\(\)' lib/functions | head -5Repository: armbian/build
Length of output: 229
🏁 Script executed:
#!/bin/bash
# Read enable_extension function to understand its requirements and timing
sed -n '448,500p' lib/functions/general/extensions.shRepository: armbian/build
Length of output: 2585
🏁 Script executed:
#!/bin/bash
# Find when initialize_extension_manager is called
rg -nP 'initialize_extension_manager' lib/functions | head -15Repository: armbian/build
Length of output: 1166
🏁 Script executed:
#!/bin/bash
# Check timing: when is main-config.sh sourced relative to board config parsing
rg -nP 'source.*main-config|source.*board.*conf' lib/functions lib/cli main.sh | head -20Repository: armbian/build
Length of output: 1168
🏁 Script executed:
#!/bin/bash
# Find the config_source_board_file function and understand when it's called
sed -n '23,50p' lib/functions/main/config-prepare.shRepository: armbian/build
Length of output: 1247
🏁 Script executed:
#!/bin/bash
# Understand the exact flow: find where config_source_board_file sources the board
rg -nP -A 10 'function config_source_board_file' lib/functions/main/config-prepare.sh | head -40Repository: armbian/build
Length of output: 769
🏁 Script executed:
#!/bin/bash
# Check if other extensions do network operations at source time or move them to hooks
rg -nP '(git clone|git pull|wget|curl)' extensions/ | grep -v '^\s*#' | head -20Repository: armbian/build
Length of output: 2395
🏁 Script executed:
#!/bin/bash
# Check if yt6801.sh does curl at top-level or in a hook
head -30 extensions/yt6801.shRepository: armbian/build
Length of output: 1530
🏁 Script executed:
#!/bin/bash
# Verify when enable_extension_recurse_counter is initialized
rg -nP 'enable_extension_recurse_counter' lib/functions/general/extensions.sh | head -10Repository: armbian/build
Length of output: 330
🏁 Script executed:
#!/bin/bash
# Verify the complete flow: when is board config sourced vs when is extension manager initialized
sed -n '20,35p' lib/functions/main/config-prepare.shRepository: armbian/build
Length of output: 789
🏁 Script executed:
#!/bin/bash
# Verify enable_extension behavior when called before initialization
sed -n '448,480p' lib/functions/general/extensions.shRepository: armbian/build
Length of output: 1626
Move git clone/git pull operations to a dedicated hook; avoid network I/O at config-source time.
Lines 287–299 execute when this .inc file is sourced during board config parsing — before the extension manager initializes. This means:
- Network failures during config parse block the build without graceful error handling.
- Mutating
${SRC}/extensions/as a side effect of parsing a config file breaks reproducibility and clean-build assumptions. enable_extensionat top-level works (no fatal error) but is semantically awkward and conflicts with the pattern used throughout the codebase.
Move the clone/pull logic into a hook function (e.g., fetch_sources_tools__seeed_rk_extension_clone or post_family_config__seeed_rk_extension_clone) and call enable_extension from within the hook, gated by ENABLE_SEEED_RK_EXTENSION. This defers network operations to their proper build phase and aligns with how other extensions handle initialization.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/sources/vendors/seeed-studio/recomputer-rk35xx-common.inc` around
lines 287 - 299, The current top-level clone/pull and enable_extension calls run
network I/O during config sourcing; move them into a deferred hook so fetching
happens in the proper build phase. Create a hook function (e.g.,
fetch_sources_tools__seeed_rk_extension_clone or
post_family_config__seeed_rk_extension_clone) that checks
ENABLE_SEEED_RK_EXTENSION and performs the git clone/pull into
${SRC}/extensions/seeed_armbian_extension with proper error handling, and call
enable_extension "seeed_armbian_extension" from inside that hook instead of at
top-level; keep the .inc file free of side-effectful network operations so
config parsing remains deterministic.
| keyup-threshold-microvolt = <1800000>; | ||
| u-boot,dm-pre-reloc; | ||
| status = "okay"; | ||
|
|
||
| volumeup-key { | ||
| u-boot,dm-pre-reloc; | ||
| linux,code = <KEY_VOLUMEUP>; | ||
| label = "volume up"; | ||
| press-threshold-microvolt = <1750>; | ||
| }; |
There was a problem hiding this comment.
press-threshold-microvolt = <1750> is in wrong units — button will never register.
The -microvolt property takes microvolts. With keyup-threshold-microvolt = <1800000> (1.8 V) and press-threshold-microvolt = <1750> (1.75 mV), the press region is essentially 0 V, so the VOL+ key will never trigger — the ADC would need to read ~zero for a hit. This is almost certainly meant to be 1750000 (1.75 V), matching the pattern used elsewhere for ADC-key boards.
🐛 Proposed fix
volumeup-key {
u-boot,dm-pre-reloc;
linux,code = <KEY_VOLUMEUP>;
label = "volume up";
- press-threshold-microvolt = <1750>;
+ press-threshold-microvolt = <1750000>;
};📝 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.
| keyup-threshold-microvolt = <1800000>; | |
| u-boot,dm-pre-reloc; | |
| status = "okay"; | |
| volumeup-key { | |
| u-boot,dm-pre-reloc; | |
| linux,code = <KEY_VOLUMEUP>; | |
| label = "volume up"; | |
| press-threshold-microvolt = <1750>; | |
| }; | |
| keyup-threshold-microvolt = <1800000>; | |
| u-boot,dm-pre-reloc; | |
| status = "okay"; | |
| volumeup-key { | |
| u-boot,dm-pre-reloc; | |
| linux,code = <KEY_VOLUMEUP>; | |
| label = "volume up"; | |
| press-threshold-microvolt = <1750000>; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3576-recomputer-rk3576-devkit.dts`
around lines 20 - 29, The press-threshold-microvolt on the volumeup-key node is
using the wrong units (1750 µV) so the key never registers; update the property
press-threshold-microvolt in the volumeup-key node to the correct microvolt
value (1750000) to match the keyup-threshold-microvolt pattern (e.g.,
keyup-threshold-microvolt = <1800000>) so the VOL+ press region is around 1.75
V.
| model = "Seeed Studio recomputer rk3588 devkit"; | ||
| compatible = "seeed,recomputer rk3588 devkit", "rockchip,rk3588"; |
There was a problem hiding this comment.
compatible string contains spaces — DT matching will break.
"seeed,recomputer rk3588 devkit" is not a valid DT compatible string. Device-tree compatibles are single whitespace-free tokens and drivers match them with exact string comparison; with embedded spaces, nothing that walks compatible (U-Boot board match, kernel OF match, fdt_fixup_* helpers, Armbian's image scripts that key off compatible) will find this board. Use hyphens like the model convention and the sibling RK3576 DTS:
🐛 Proposed fix
- model = "Seeed Studio recomputer rk3588 devkit";
- compatible = "seeed,recomputer rk3588 devkit", "rockchip,rk3588";
+ model = "Seeed Studio reComputer RK3588 DevKit";
+ compatible = "seeed,recomputer-rk3588-devkit", "rockchip,rk3588";📝 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.
| model = "Seeed Studio recomputer rk3588 devkit"; | |
| compatible = "seeed,recomputer rk3588 devkit", "rockchip,rk3588"; | |
| model = "Seeed Studio reComputer RK3588 DevKit"; | |
| compatible = "seeed,recomputer-rk3588-devkit", "rockchip,rk3588"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts`
around lines 13 - 14, The compatible property contains an invalid string with
spaces; update the compatible entry so it is a single token without whitespace
(e.g., replace the value used in the compatible property with a hyphenated token
like "seeed,recomputer-rk3588-devkit") while keeping the existing secondary
compatible "rockchip,rk3588"; locate the compatible property in the DTS (the
compatible = ... line) and change only that string to the hyphenated form to
match DT conventions and sibling RK3576 DTS.
| volumeup-key { | ||
| u-boot,dm-pre-reloc; | ||
| linux,code = <KEY_VOLUMEUP>; | ||
| label = "volume up"; | ||
| press-threshold-microvolt = <1750>; | ||
| }; |
There was a problem hiding this comment.
Same press-threshold-microvolt unit bug as RK3576 DTS.
<1750> µV ≈ 0 V against an 1.8 V keyup threshold — the VOL+ button can't fire. Should be 1750000. (Root cause is the same as flagged on rk3576-recomputer-rk3576-devkit.dts:28; please fix both files together.)
🐛 Proposed fix
volumeup-key {
u-boot,dm-pre-reloc;
linux,code = <KEY_VOLUMEUP>;
label = "volume up";
- press-threshold-microvolt = <1750>;
+ press-threshold-microvolt = <1750000>;
};📝 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.
| volumeup-key { | |
| u-boot,dm-pre-reloc; | |
| linux,code = <KEY_VOLUMEUP>; | |
| label = "volume up"; | |
| press-threshold-microvolt = <1750>; | |
| }; | |
| volumeup-key { | |
| u-boot,dm-pre-reloc; | |
| linux,code = <KEY_VOLUMEUP>; | |
| label = "volume up"; | |
| press-threshold-microvolt = <1750000>; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts`
around lines 25 - 30, The press-threshold-microvolt value in the volumeup-key
node is using microvolts but was written as <1750> (1,750 µV) which is incorrect
for a 1.8 V threshold; update the volumeup-key's press-threshold-microvolt
property (node "volumeup-key") to <1750000> so it represents 1,750,000 µV (1.75
V). Also apply the same fix to the corresponding press-threshold-microvolt entry
in rk3576-recomputer-rk3576-devkit.dts to keep both DT files consistent.
| pcie_clk: pcie-clk-regulator { | ||
| u-boot,dm-pre-reloc; | ||
| enable-active-low; | ||
| gpio = <&gpio4 RK_PB4 GPIO_ACTIVE_HIGH>; | ||
| pinctrl-names = "default"; | ||
| pinctrl-0 = <&pcie30_clk>; | ||
| regulator-boot-on; | ||
| regulator-always-on; | ||
| compatible = "regulator-fixed"; | ||
| regulator-name = "pcie_clk"; | ||
| regulator-min-microvolt = <3300000>; | ||
| regulator-max-microvolt = <3300000>; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and check if it exists
find . -name "rk3588-recomputer-rk3588-devkit.dts" -type f 2>/dev/nullRepository: armbian/build
Length of output: 138
🏁 Script executed:
# Check the actual content of the file around lines 93-105
if [ -f "patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts" ]; then
echo "=== File content around lines 93-105 ==="
sed -n '90,110p' "patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts" | cat -n
else
echo "File not found"
fiRepository: armbian/build
Length of output: 743
🏁 Script executed:
# Search for pcie30_clk pinctrl definition in the same directory
grep -r "pcie30_clk" --include="*.dts" --include="*.dtsi" patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/ 2>/dev/null | head -20Repository: armbian/build
Length of output: 494
🏁 Script executed:
# Look for similar regulator-fixed patterns with enable-active-low and GPIO_ACTIVE_HIGH in RK3588 device trees
grep -B5 -A10 "enable-active-low" patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588*.dts 2>/dev/null | grep -A15 "GPIO_ACTIVE_HIGH" | head -40Repository: armbian/build
Length of output: 2487
🏁 Script executed:
# Search for schematic or hardware documentation in the repository
find . -name "*schematic*" -o -name "*hardware*" -o -name "*recomputer*" -type f 2>/dev/null | head -20Repository: armbian/build
Length of output: 1825
🏁 Script executed:
# Look at the pinctrl definition for pcie30_clk
sed -n '1,400p' "patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts" | grep -A5 "pcie30_clk:"Repository: armbian/build
Length of output: 180
🏁 Script executed:
# Check the entire pinctrl-related section more thoroughly
grep -n "pcie30_clk" "patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts"Repository: armbian/build
Length of output: 192
🏁 Script executed:
# Look for the pinctrl definitions in the file
sed -n '250,350p' "patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts" | cat -nRepository: armbian/build
Length of output: 1814
🏁 Script executed:
# Get full file to find the pinctrl definitions
wc -l "patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts"Repository: armbian/build
Length of output: 140
🏁 Script executed:
# Look for other regulators in this file that use enable-active-low
grep -B3 -A8 "enable-active-low" "patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts"Repository: armbian/build
Length of output: 383
🏁 Script executed:
# Check if there's a corresponding pcie30_clk_en that might clarify the intent
grep -B5 -A10 "pcie30_clk_en" "patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts"Repository: armbian/build
Length of output: 732
🏁 Script executed:
# Search for any documentation references in the repo
find . -name "*.md" -o -name "*.txt" -o -name "*.rst" | xargs grep -l "pcie.*clk\|recomputer" 2>/dev/null | head -10Repository: armbian/build
Length of output: 39
🏁 Script executed:
# Check the vendor configuration files for additional context
cat "config/boards/recomputer-rk3588-devkit.conf" 2>/dev/nullRepository: armbian/build
Length of output: 4988
🏁 Script executed:
# Check if there are other similar boards with pcie_clk regulator to compare
grep -r "pcie.*clk.*regulator" patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/ --include="*.dts" | head -10Repository: armbian/build
Length of output: 293
pcie_clk regulator: enable-active-low with GPIO_ACTIVE_HIGH is inconsistent.
regulator-fixed derives the effective enable polarity from both enable-active-{low,high} and the GPIO cell's active flag. Declaring enable-active-low; while passing the pin as GPIO_ACTIVE_HIGH creates conflicting polarity signals for the driver. The parallel vcc3v3_pcie_clk regulator correctly uses enable-active-high; with GPIO_ACTIVE_HIGH. Please make the two consistent: either drop enable-active-low and keep GPIO_ACTIVE_HIGH, or switch the gpio cell to GPIO_ACTIVE_LOW.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3588-recomputer-rk3588-devkit.dts`
around lines 93 - 105, The pcie_clk regulator node sets enable-active-low but
passes the GPIO as GPIO_ACTIVE_HIGH, causing a polarity conflict; update the
pcie_clk node (regulator-fixed, regulator-name = "pcie_clk") to match the
vcc3v3_pcie_clk pattern by using enable-active-high with GPIO_ACTIVE_HIGH, or
alternatively change the gpio cell to GPIO_ACTIVE_LOW so the enable-active-low
flag and the gpio cell agree—make the pcie_clk node's enable polarity consistent
with the gpio cell.
|
|
||
| if [[ ! -f "${deb_file}" ]]; then | ||
| display_alert "Downloading ${display_name}" "${url}" "info" | ||
| wget "${url}" -O "${deb_file}" || { |
There was a problem hiding this comment.
Use curl. Classic wget isn't even present in Trixie by default anymore. It was replaced with a wrapper that actually calls curl in the background since this tool is actually actively maintained.
- Fix press-threshold-microvolt units: 1750 → 1750000 (µV) in both
RK3576 and RK3588 device trees so VOL+ button actually registers
- Fix RK3588 compatible string: remove spaces, use hyphens
("seeed,recomputer rk3588 devkit" → "seeed,recomputer-rk3588-devkit")
- Fix pcie_clk regulator polarity: change enable-active-low to
enable-active-high to match GPIO_ACTIVE_HIGH flag
Download helper improvements: - Replace wget with curl (--connect-timeout 30 --retry 3) - Download to .tmp file, mv on success (avoid partial file poisoning) - Add optional SHA256 checksum verification (4th parameter) - Apply same curl/tmp pattern to Morse tools downloads USB auto-mount security fix: - Replace eval "$(udevadm info ... | grep ...)" with safe udevadm --property/--value calls to prevent command injection via crafted USB device model strings - Sanitize LABEL to safe characters only (A-Za-z0-9._-)
Use individual GitHub ID as required by Armbian board support rules.
Move git clone/pull and enable_extension from top-level (runs during config file sourcing) to a proper post_family_config hook. This defers network I/O to the appropriate build phase and avoids side effects during config parsing.
Use dpkg --force-depends -i to properly register packages in the dpkg database. Previously used dpkg-deb -x + cp which left files orphaned outside dpkg (no postinst, no triggers, not removable via apt). --force-depends is needed because hostapd/wpa-supplicant tools depend on libssl1.1 which is not available in Noble (ships libssl3). The binaries work at runtime against libssl3.
🚫 Missing required board assetsThis PR adds new board configuration(s). Required assets must already exist in github/armbian/armbian.github.io.
Missing items
Once the missing files are added (or a PR is opened in armbian/armbian.github.io), re-run this check. |
|
@EvilOlaf Thanks for the feedback on What we found: The kernel artifact cache key is We checked: no other board in Options we're considering:
Our preference is option 1 for now — it's the smallest fix that correctly tracks the cache hash, and we can revisit the family structure later if needed. What do you think? |
|
I be totally honest, I don't know exactly how the internals on this work, I just know from previous PRs that on various occasions board-level kernel adjustments are flagged as a bad idea since it breaks apt repo Anyway it might be possible to move all of this to family level and work around this. I just throw in some random thoughts, may be totally wrong though.
If the issue is that, when built as module, it loads (too) late, perhaps create a hook for initramfs generation to force the module into initramfs image? Then it should behave similar to being built-in I guess
If module is sufficient, I'd do the same as above
Without looking deeper, tbh not sure how to exactly handle this. Perhaps there are other extensions already that add some sort of drivers which obviously also come with kernel config settings.
If added as module I don't see a reason not to add to family kernel config and if needed again force move into initramfs. |
At a high level the issue is with the software deployment lifecycle via apt and the Armbian apt repositories. kernel apt packages are produced and populated into the apt repositories keyed by Family-Branch. So after initial image creation when an apt update/upgrade is run the latest kernel package from the repositories will be deployed to the device which will upgrade the kernel to the standard kernel for the Family-Branch. Which will effectively remove any board specific kernel that was originally installed. The Armbian concept of Family is a superset of kernel features that work for all boards within the family. Generally adding additional board feature to the family kernel won't be a problem, but it can sometimes cause conflicts. The other consideration is that this is done to minimize Armbian infrastructure costs. If each board had its own kernel, the storage and infrastructure cost for the build farms and the apt mirrors would be unmaintainable. |
6 participants
Summary
Add board support for Seeed Studio's reComputer RK3576 DevKit and reComputer RK3588 DevKit to the Armbian build framework. These are industrial-grade single-board computers based on Rockchip RK3576 (octa-core A72/A53) and RK3588 (octa-core A76/A55) SoCs, targeting edge AI and IoT applications.
Board Support
config/boards/recomputer-rk3576-devkit.conf): RK3576 SoC,BOARDFAMILY=rk35xx, vendor kernel, custom bootscript, PCA953X GPIO expander, AD5398 DAC for fan control, X11 KMS on card0, ES8311 audio codecconfig/boards/recomputer-rk3588-devkit.conf): RK3588 SoC,BOARDFAMILY=rockchip-rk3588, vendor kernel, custom bootscript, Mali-G610 GPU (libmali), Realtek r8125 Ethernet DKMS, multi-display audio naming (HDMI0/HDMI1/HDMI-In/DP0/ES8311)config/boards/recomputer-rk35xx-common.inc): Extracted ~300 lines of common hooks covering wireless, Bluetooth, USB gadget, security hardening, PATH, and OTA support. BT UART selection uses${BOOT_SOC}case branching (rk3576→ttyS4, rk3588→ttyS6)Hardware Support
security-hardening/recomputer-security.sh/media/seeed/LABELseeed_armbian_extension)RK_AUTO_DECRYP=yes)Boot & Firmware
boot-seeed-rk3576.cmd,boot-seeed-rk35xx.cmd) with SPI flash boot supportrk3576-recomputer-rk3576-devkit.dts,rk3588-recomputer-rk3588-devkit.dtsBuild Framework Integration
lib/functions/compilation/patch/drivers_network.sh: Driver harness integration for Morse wireless FGH100M kernel modulelib/functions/compilation/patch/drivers-harness.sh: Hook for Morse driver buildENABLE_SEEED_RK_EXTENSION=yesclonesseeed_armbian_extensionfrom GitHub for OTA, encryption, and secure boot featuresChanged Files
config/boards/recomputer-rk3576-devkit.confconfig/boards/recomputer-rk3588-devkit.confconfig/boards/recomputer-rk35xx-common.incconfig/bootscripts/boot-seeed-rk3576.cmdconfig/bootscripts/boot-seeed-rk35xx.cmdconfig/kernel/linux-rk35xx-vendor.configpatch/kernel/rockchip-rk35xx-legacy/patch/kernel/rockchip-rk3588-legacy/patch/kernel/rockchip64-7.0/patch/misc/wireless-fgh100m.patchpatch/u-boot/*lib/functions/compilation/patch/How to Test
Serial console:
minicom -b 1500000 -D /dev/ttyUSB0Summary by CodeRabbit
Release Notes
New Features
Bug Fixes