feat(boards): Add Seeed Studio reComputer RK3576/RK3588 DevKit support#9719
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
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:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Seeed reComputer RK3576 and RK3588 support: shared vendor provisioning and image hooks, board configs installing Mali/camera packages and audio/system tweaks, U-Boot device trees enabling regulators/PCIe/USB, and a bootscript that selects DTB via EEPROM and applies DT overlays. ChangesSeeed reComputer RK35xx Board Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
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
|
@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. |
|
@EvilOlaf @SteeManMI Thanks both for the detailed explanation — that clears up a lot. @SteeManMI The apt repository deployment angle makes perfect sense now. We hadn't considered the full lifecycle beyond the build cache — the fact that @EvilOlaf The initramfs hook approach for forcing module load is a good idea for PCA953X and RTC. For the Morse wireless bits we'll look at how other extensions handle driver kernel config additions. The direction seems clear: move our config changes to family level (or an extension, for the Morse driver parts), prefer modules over built-in, and use initramfs hooks for anything that needs to be available early. We'll take some time to figure out the right split and come back with a concrete proposal. Thanks again for the guidance. |
Copy linux-rk35xx-vendor.config as base for seeed-rk3576 and seeed-rk3588 families. Board-specific options will be applied in follow-up commits.
Per Armbian guidelines, kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Remove custom_kernel_config__* hooks from board configs and apply all kernel config options directly in per-family LINUXCONFIG files. Each family now has its own LINUXCONFIG derived from the shared rk35xx-vendor base with board-specific overrides: - seeed-rk3576: PCA953X=y, CSF=n (Mali-G52 JM only) - seeed-rk3588: CSF=y (Mali-G610 requires CSF) Also fix unquoted $SDCARD in RK3588 audio naming hook.
- Remove commented-out DT nodes and properties in RK3588 DTS - Replace commented-out register writes in HDMI voltage-swing patch with concise block comment - Fix defconfig: use canonical "# is not set" form, remove trailing whitespace
Merge boot-seeed-rk3576.cmd and boot-seeed-rk3588.cmd into a single boot-seeed-rk35xx.cmd. The scripts only differed in load_addr and the hardcoded fdtfile fallback. The shared script: - Uses load_addr 0x9000000 (safe for both RK3576 and RK3588) - Relies on armbianEnv.txt (BOOT_FDT_FILE) for the default fdtfile instead of hardcoding it, with EEPROM detection as override
Swap mount order so uid=1000,gid=1000,umask=002 is tried first. Plain "mount -o noatime" succeeds on VFAT/exFAT/NTFS but leaves files root-owned, breaking the per-user automount UX.
Correct hunk header count (21 -> 17) and diffstat (11 -> 7 insertions) in the RK3588 HDMI voltage-swing patch. The mismatched counts caused "Hunk is longer than expected" errors during U-Boot patching.
Drop the Debian/Ubuntu branch detection for Seeed APT dist selection. All releases now use "stable" since packages are built on bookworm and forward-compatible.
fcs960k-aic-bluez replaces bluez but does not ship bluetooth.service, which blocks aic-bluetooth.service (Requires=bluetooth.service). Fix by: - Creating a synthetic bluetooth.service unit pointing to fcs960k's bluetoothd when the unit is missing - Softening aic-bluetooth.service dependency from Requires= to Wants= so it can start even without bluetooth.service
Add `|| true` to sed commands that modify aic-bluetooth.service across multiple systemd unit paths, preventing non-zero exit when the file doesn't exist at all locations.
Add build hooks to ensure Mali EGL/GLES/GBM stack works correctly instead of falling back to Mesa/llvmpipe: DMA heap udev rules for GBM, X11 KMS OutputClass, libglvnd bypass for EGL/GLES, and dpkg-divert protected GBM symlink fixup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move shared Mali hooks (DMA heap, X11 KMS, EGL override, GBM fixup) from RK3576/RK3588 board configs to recomputer-rk35xx-common.inc. Both boards now use the same Mali EGL/GBM stack with dpkg-divert protected GBM symlink. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move the inline heredoc in recomputer-rk35xx-common.inc to a standalone script file under packages/bsp/seeed/, matching the project convention of vendor-named BSP directories. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Mali deb packages a 5928-byte GBM stub (2020) that overrides system libgbm via ldconfig and causes incorrect pixel format in DRM framebuffer allocation, resulting in red/blue color swap on all outputs (HDMI, DP, DSI) when the DSI overlay is enabled. Add MALI_USE_SYSTEM_GBM variable — RK3576 sets it to "yes" to opt out of Mali's bundled GBM stub and use the system libgbm instead.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/boards/recomputer-rk3588-devkit.conf`:
- Line 2: BOARD_NAME uses hyphens causing inconsistent display; change the
BOARD_NAME value (the BOARD_NAME variable in this config) from
"reComputer-RK3588-Devkit" to use spaces ("reComputer RK3588 Devkit") so it
matches the RK3576 sibling's display style.
🪄 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: d50cc36d-b7f2-490b-bedd-dc25c422e8c6
📒 Files selected for processing (3)
config/boards/recomputer-rk3576-devkit.confconfig/boards/recomputer-rk3588-devkit.confconfig/bootscripts/boot-seeed-rk35xx.cmd
Match the RK3576 sibling's display style (reComputer RK3576 Devkit) instead of hyphenated form.
|
Thanks for the detailed suggestions. Here's how we've addressed this:
Does this approach look good to you, or do you see a better way to handle this? |
PR was not merged to upstream/main before submitting. @baorepo: Why not merge to upstream, do your QA again, and submit PR after this? Something like: |
|
@sven-ola It’s possible that @mingzhangqun might have made a mistake just now, but he has already corrected it. For now, there’s no such information available. |
|
@HeyMeco Thanks for the guidance! The LED patch has been reworked to use a The remaining patches have been submitted as separate PRs to
|
Move the disable hook from RK3588 board config to recomputer-rk35xx-common.inc so both RK3576 and RK3588 benefit. The service's IRQ affinity is a no-op for seeed-rk35xx families, its cpufreq settings conflict with the BSP interactive governor, and its USB quirks function risks corrupting /boot/armbianEnv.txt.
|
@sven-ola Thanks for the review! This is already merged via the For reference, here's a breakdown of the diff:
~89% of the diff is Rockchip SDK autogenerated content (kernel config + U-Boot patches). The actual hand-written business logic is ~800 lines (board configs + bootscript). |
- bootscript: use command return values instead of rootdev sentinel for corruption detection, add .bak fallback with load/import checks - common.inc: create armbianEnv.txt.bak at build time (no systemd service)
igorpecovnik
left a comment
There was a problem hiding this comment.
I think this should be good to make 1st images, but job needs to continue.
|
✅ This PR has been reviewed and approved — all set for merge! |
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
New Features
Enhancements