Skip to content

Conversation

igorpecovnik
Copy link
Member

Description

Replacement of: #8632

How Has This Been Tested?

  • Tested sata spi image boot.
  • Tested sata doesn't boot with normal spi image.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

Copy link
Contributor

coderabbitai bot commented Sep 28, 2025

Walkthrough

This PR updates Orange Pi 5 board config and shared Rockchip64 SPI flashing logic. It adds Bluetooth HCI attach params and enables the bluetooth-hciattach extension. It refactors the mainline U-Boot override hook, updates U-Boot to 2025.07, extends UBOOT_TARGET_MAP with SATA targets, and introduces SATA-specific bootconfig patching, retention, and packaging steps with vendor-branch guards. Several branch checks expand to cover current and edge. The Rockchip64 common script broadens SPI image discovery, adds non-interactive selection with automatic deployment based on file type (rkspi_loader\*.img via dd, others via flashcp), and refactors interactive selection to resolve full paths.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

05, Needs review, size/large, Hardware, Patches

Suggested reviewers

  • pyavitz
  • efectn
  • rpardini

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly captures the primary changes by indicating the overhaul of the Orange Pi 5 board configuration and the update to mainline U-Boot, which align with the modifications detailed in the pull request.
Description Check ✅ Passed The description directly relates to the updates in U-Boot usage for current and edge builds, the addition of the SATA SPI image logic, and links to prior related pull requests, reflecting the actual contents and objectives of the changeset.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch opi5-mainline-uboot-update2

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added size/large PR with 250 lines or more 11 Milestone: Fourth quarter release Needs review Seeking for review Hardware Hardware related like kernel, U-Boot, ... Patches Patches related to kernel, U-Boot, ... labels Sep 28, 2025
@igorpecovnik igorpecovnik added Ready to merge Reviewed, tested and ready for merge and removed Needs review Seeking for review labels Sep 28, 2025
@coderabbitai coderabbitai bot requested a review from rpardini September 28, 2025 04:40
@igorpecovnik igorpecovnik merged commit a96702d into main Sep 28, 2025
11 of 12 checks passed
@igorpecovnik igorpecovnik deleted the opi5-mainline-uboot-update2 branch September 28, 2025 04:45
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between eebef46 and 7b1c174.

⛔ Files ignored due to path filters (1)
  • patch/u-boot/v2025.07/board_orangepi5/0001-add-sata-dts-and-defconfig-for-OPi5.patch is excluded by !patch/**
📒 Files selected for processing (2)
  • config/boards/orangepi5.conf (5 hunks)
  • config/sources/families/include/rockchip64_common.inc (1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.
Learnt from: rpardini
PR: armbian/build#8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.
Learnt from: EvilOlaf
PR: armbian/build#8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub or the PR API to get the complete picture of what files are being added or modified.
Learnt from: rpardini
PR: armbian/build#8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
Learnt from: EvilOlaf
PR: armbian/build#8417
File: config/boards/orangepi5pro.csc:57-58
Timestamp: 2025-07-23T07:30:52.265Z
Learning: In the Armbian build system, BOOTPATCHDIR can contain board-specific subdirectories (e.g., board_orangepi5pro) for applying patches to specific boards only. The framework automatically checks if such board-specific subdirectories exist for the board being built and applies those patches accordingly.
📚 Learning: 2025-09-14T06:32:29.806Z
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.

Applied to files:

  • config/sources/families/include/rockchip64_common.inc
  • config/boards/orangepi5.conf
📚 Learning: 2025-09-14T05:23:42.991Z
Learnt from: EvilOlaf
PR: armbian/build#8632
File: config/sources/families/include/rockchip64_common.inc:327-334
Timestamp: 2025-09-14T05:23:42.991Z
Learning: In the Armbian build system for Rockchip boards, SPI flash consistently maps to /dev/mtd0, so hard-coding this device path in flashcp commands is acceptable and based on hardware conventions rather than being a bug.

Applied to files:

  • config/sources/families/include/rockchip64_common.inc
📚 Learning: 2025-03-31T22:20:48.475Z
Learnt from: rpardini
PR: armbian/build#8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.

Applied to files:

  • config/boards/orangepi5.conf
📚 Learning: 2025-09-14T06:29:18.958Z
Learnt from: amazingfate
PR: armbian/build#8619
File: config/sources/families/rockchip.conf:64-70
Timestamp: 2025-09-14T06:29:18.958Z
Learning: In the Armbian build system, vendor branch configurations in family files are designed to be shared across multiple SoCs within the same family that use the same vendor kernel tree. For example, rk35xx and rockchip-rk3588 families both use identical vendor branch settings (same KERNELSOURCE, KERNELBRANCH, and KERNELPATCHDIR), demonstrating that vendor branches are intentionally generic rather than SoC-specific.

Applied to files:

  • config/boards/orangepi5.conf
📚 Learning: 2025-09-11T06:12:54.213Z
Learnt from: SuperKali
PR: armbian/build#8609
File: config/boards/nanopi-r76s.conf:5-5
Timestamp: 2025-09-11T06:12:54.213Z
Learning: In the Armbian build system, board family configuration files (like config/sources/families/rk35xx.conf) can inherit kernel branch definitions from common include files (like config/sources/families/include/rockchip64_common.inc). Even if a branch like "edge" is not defined directly in the family conf file, it may be available through the sourced include file.

Applied to files:

  • config/boards/orangepi5.conf
📚 Learning: 2025-07-23T07:30:52.265Z
Learnt from: EvilOlaf
PR: armbian/build#8417
File: config/boards/orangepi5pro.csc:57-58
Timestamp: 2025-07-23T07:30:52.265Z
Learning: In the Armbian build system, BOOTPATCHDIR can contain board-specific subdirectories (e.g., board_orangepi5pro) for applying patches to specific boards only. The framework automatically checks if such board-specific subdirectories exist for the board being built and applies those patches accordingly.

Applied to files:

  • config/boards/orangepi5.conf
📚 Learning: 2025-09-12T19:28:38.491Z
Learnt from: Grippy98
PR: armbian/build#8622
File: config/sources/families/k3.conf:66-66
Timestamp: 2025-09-12T19:28:38.491Z
Learning: In the Armbian k3 family build system (config/sources/families/k3.conf), builds do not fail when TIBOOT3_BOOTCONFIG is unset, even though tiboot3.bin is still listed in UBOOT_TARGET_MAP. The gating mechanism in pre_config_uboot_target__build_first_stage function works as intended to conditionally build/copy tiboot3.bin only when TIBOOT3_BOOTCONFIG is defined.

Applied to files:

  • config/boards/orangepi5.conf
📚 Learning: 2025-08-02T05:46:10.664Z
Learnt from: EvilOlaf
PR: armbian/build#0
File: :0-0
Timestamp: 2025-08-02T05:46:10.664Z
Learning: In the Armbian build system, the modern recommended approach for kernel configuration is to use the kernel-config command via "./compile.sh BOARD=boardname BRANCH=branchname kernel-config" instead of the deprecated KERNEL_CONFIGURE=yes flag. This provides a two-step workflow: configure using menuconfig, then build, with better transparency and control over configuration changes.

Applied to files:

  • config/boards/orangepi5.conf

Comment on lines 321 to +345
filename=$(basename "$file")
MENU_ITEMS+=("$i" "$filename" "")
FILE_ARRAY+=("$file")
((i++))
done <<< "$FILES"

# If there is only one image, we can skip the dialog
if [[ $i -eq 2 ]]; then
dd if=$1/${MENU_ITEMS[1]} of=$2 conv=notrunc status=none > /dev/null 2>&1
if [[ ! -t 1 || $i -eq 2 ]]; then
first_file="${FILE_ARRAY[0]}"
if [[ "$first_file" == *rkspi_loader*.img ]]; then
dd if="$first_file" of="$2" conv=notrunc status=none > /dev/null 2>&1
else
flashcp -v -p "$first_file" /dev/mtd0
fi
return
fi

[[ -f /etc/armbian-release ]] && source /etc/armbian-release
backtitle="Armbian for $BOARD_NAME install script, https://www.armbian.com"

CHOICE=$(dialog --no-collapse \
--title "armbian-install" \
--backtitle "$backtitle" \
--radiolist "Choose SPI image:" 0 56 4 \
"${MENU_ITEMS[@]}" \
3>&1 1>&2 2>&3)
--title "armbian-install" \
--backtitle "$backtitle" \
--radiolist "Choose SPI image:" 0 56 4 \
"${MENU_ITEMS[@]}" \
3>&1 1>&2 2>&3)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Restore valid dialog state flags

dialog --radiolist requires each entry to end in on or off. Handing it "" makes dialog/whiptail abort with “Expected state to be on/off”, so we drop straight into the “No SPI image chosen.” path and SPI flashing becomes impossible in an interactive session. Please reinstate explicit states (e.g. default the first item to on, the rest to off) before returning.

-		MENU_ITEMS+=("$i" "$filename" "")
+		state="off"
+		[[ $i -eq 1 ]] && state="on"
+		MENU_ITEMS+=("$i" "$filename" "$state")
📝 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.

Suggested change
filename=$(basename "$file")
MENU_ITEMS+=("$i" "$filename" "")
FILE_ARRAY+=("$file")
((i++))
done <<< "$FILES"
# If there is only one image, we can skip the dialog
if [[ $i -eq 2 ]]; then
dd if=$1/${MENU_ITEMS[1]} of=$2 conv=notrunc status=none > /dev/null 2>&1
if [[ ! -t 1 || $i -eq 2 ]]; then
first_file="${FILE_ARRAY[0]}"
if [[ "$first_file" == *rkspi_loader*.img ]]; then
dd if="$first_file" of="$2" conv=notrunc status=none > /dev/null 2>&1
else
flashcp -v -p "$first_file" /dev/mtd0
fi
return
fi
[[ -f /etc/armbian-release ]] && source /etc/armbian-release
backtitle="Armbian for $BOARD_NAME install script, https://www.armbian.com"
CHOICE=$(dialog --no-collapse \
--title "armbian-install" \
--backtitle "$backtitle" \
--radiolist "Choose SPI image:" 0 56 4 \
"${MENU_ITEMS[@]}" \
3>&1 1>&2 2>&3)
--title "armbian-install" \
--backtitle "$backtitle" \
--radiolist "Choose SPI image:" 0 56 4 \
"${MENU_ITEMS[@]}" \
3>&1 1>&2 2>&3)
filename=$(basename "$file")
state="off"
[[ $i -eq 1 ]] && state="on"
MENU_ITEMS+=("$i" "$filename" "$state")
FILE_ARRAY+=("$file")
((i++))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
11 Milestone: Fourth quarter release Hardware Hardware related like kernel, U-Boot, ... Patches Patches related to kernel, U-Boot, ... Ready to merge Reviewed, tested and ready for merge size/large PR with 250 lines or more
Development

Successfully merging this pull request may close these issues.

2 participants