Skip to content

Conversation

EvilOlaf
Copy link
Member

Description

  • Bump uboot for both current and edge to v2025.10.
  • consoldidate/simplify board config
  • re-enable previously broken stuff

Note: It might be possible to get rid of vendor uboot altogether. Needs further testing though.

How Has This Been Tested?

  • build uboot package
  • boot from microSD with edge kernel, but w/o crypto options yet
  • boot from SPI with vendor kernel on nvme, but w/o crypto options yet

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

@github-actions github-actions bot added size/small PR with less then 50 lines 11 Milestone: Fourth quarter release Needs review Seeking for review Hardware Hardware related like kernel, U-Boot, ... labels Oct 21, 2025
Copy link
Contributor

coderabbitai bot commented Oct 21, 2025

Walkthrough

Consolidates branch-specific U-Boot handlers for Orange Pi 5 Plus into a single function post_family_config__orangepi5plus_use_mainline_uboot() with an early return for the vendor branch, removes the old edge/current branch-specific handlers, bumps mainline U-Boot defaults (BOOTBRANCH from tag:v2024.10-rc3tag:v2025.10, BOOTPATCHDIR v2024.10v2025.10), changes gating logic to skip on vendor instead of non-edge/current, and enables ARM crypto CE flags (ARMV8_CRYPTO, ARMV8_CE_SHA1, ARMV8_CE_SHA256) in the new path.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

Patches

Suggested reviewers

  • rpardini
  • amazingfate
  • igorpecovnik
  • paolosabatino
  • pyavitz

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title "opi5plus - board config maintenance" is clearly related to the changeset and refers to genuine aspects of the changes being made to the Orange Pi 5 Plus board configuration. The title identifies both the board (opi5plus) and the type of work being performed (board config maintenance), which accurately reflects the modifications to the config/boards/orangepi5-plus.conf file. While the title is somewhat broad and doesn't specify all the details (such as the U-Boot version bump or the consolidation of functions), it adequately conveys that this is maintenance work on the board configuration, making it a partially-to-fully related title that a teammate can understand.
Description Check ✅ Passed The pull request description is clearly related to the changeset. It explicitly outlines the main objectives (bumping U-Boot to v2025.10, consolidating/simplifying board config, and re-enabling previously broken functionality), which directly correspond to the changes summarized in the raw_summary. The description also provides specific testing evidence and includes a completed checklist, demonstrating concrete work performed. The description is neither vague nor generic; it contains actionable details and specific version numbers that accurately reflect what has been changed.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent 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 9e10f74 and a38f5bb.

📒 Files selected for processing (1)
  • config/boards/orangepi5-plus.conf (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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-plus.conf
📚 Learning: 2025-10-14T05:08:11.785Z
Learnt from: EvilOlaf
PR: armbian/build#8754
File: config/boards/bestv-r3300-l.csc:14-16
Timestamp: 2025-10-14T05:08:11.785Z
Learning: In the Armbian build system, BOOTBRANCH_BOARD is a valid framework variable used as a fallback when BOOTBRANCH is unset. The framework checks BOOTBRANCH_BOARD before applying the default bootloader branch value (see config/sources/common.conf). Board configuration files can use BOOTBRANCH_BOARD to specify the bootloader branch.

Applied to files:

  • config/boards/orangepi5-plus.conf
🔇 Additional comments (4)
config/boards/orangepi5-plus.conf (4)

31-32: Good consolidation of branch-specific handlers.

The approach of using a single function with an early return for the vendor branch is cleaner and more maintainable than separate branch-specific handlers.


56-56: Improved gating logic.

The change from checking for specific branches (edge/current) to simply skipping vendor is more maintainable and clearer.


39-40: U-Boot v2025.10 patches verified as available and compatible.

The patch directory patch/u-boot/v2025.10/ exists and is properly configured for the orangepi5-plus board, which uses the RK3588 SoC. The same patch version is already in use by radxa-zero3 (also RK3588-based). The version bump is consistent with the U-Boot branch tag and has been tested per the PR description.


60-62: Incorrect review comment—these crypto flags were not modified by this PR.

The crypto flags at lines 60-62 (ARMV8_CRYPTO, ARMV8_CE_SHA1, ARMV8_CE_SHA256 set to "y") are pre-existing code. The git diff shows they remain unchanged before and after this PR. The only actual change in this file is the removal of a comment on line 30.

The review incorrectly analyzes static existing code rather than the actual changes introduced by the PR.

Likely an incorrect or invalid review comment.


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.

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 106ba53 and 9e10f74.

📒 Files selected for processing (1)
  • config/boards/orangepi5-plus.conf (2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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: 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.
📚 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-plus.conf
📚 Learning: 2025-10-14T05:08:11.785Z
Learnt from: EvilOlaf
PR: armbian/build#8754
File: config/boards/bestv-r3300-l.csc:14-16
Timestamp: 2025-10-14T05:08:11.785Z
Learning: In the Armbian build system, BOOTBRANCH_BOARD is a valid framework variable used as a fallback when BOOTBRANCH is unset. The framework checks BOOTBRANCH_BOARD before applying the default bootloader branch value (see config/sources/common.conf). Board configuration files can use BOOTBRANCH_BOARD to specify the bootloader branch.

Applied to files:

  • config/boards/orangepi5-plus.conf
🔇 Additional comments (3)
config/boards/orangepi5-plus.conf (3)

56-56: Good consolidation of gating logic.

The updated gating logic now consistently skips vendor branch in both functions, which is cleaner and more maintainable than the previous approach.


60-62: Verify that ARM crypto options work correctly.

The PR testing notes indicate that boots were performed "without crypto options," but these changes enable ARMV8_CRYPTO, ARMV8_CE_SHA1, and ARMV8_CE_SHA256. Please confirm that these crypto acceleration features work correctly with the new U-Boot version, or clarify whether additional testing is planned before merge.


39-40: No issues found — v2025.10 patch directory exists and is ready.

Verification confirms the patch/u-boot/v2025.10/ directory contains 33 patches, including board-specific patches and general fixes. The version bump in lines 39-40 is properly supported.

actually self-explaining I think
@github-actions github-actions bot added the Ready to merge Reviewed, tested and ready for merge label Oct 21, 2025
Copy link
Contributor

✅ This PR has been reviewed and approved — all set for merge!

@github-actions github-actions bot removed the Needs review Seeking for review label Oct 21, 2025
@EvilOlaf
Copy link
Member Author

Do not merge yet. I want to test with crypto enabled later today unless @alexl83 is faster.

@igorpecovnik igorpecovnik added Work in progress Unfinished / work in progress and removed Ready to merge Reviewed, tested and ready for merge labels Oct 21, 2025
@EvilOlaf
Copy link
Member Author

https://paste.armbian.de/unohaqutuv.yaml seems fine

Copy link
Contributor

✅ This PR has been reviewed and approved — all set for merge!

@github-actions github-actions bot added Ready to merge Reviewed, tested and ready for merge and removed Work in progress Unfinished / work in progress labels Oct 21, 2025
@EvilOlaf EvilOlaf merged commit 7be3ce0 into armbian:main Oct 21, 2025
12 checks passed
@EvilOlaf EvilOlaf deleted the 5plusuboot branch October 21, 2025 16:43
@EvilOlaf
Copy link
Member Author

next step would be eliminating vendor uboot altogether. I have a vendor kernel on nvme and mainline uboot on SPI. Works just fine. @efectn what do you think? Anything that might be missing in mainline uboot which is there in stone-age vendor?

@rpardini
Copy link
Member

If I remember, some vendor kernel DTs assumed that vendor u-boot will have had brought up some regulators (npu?) so those things might fail. Unsure if it applies to OPi5's...

@efectn
Copy link
Member

efectn commented Oct 21, 2025

If I remember, some vendor kernel DTs assumed that vendor u-boot will have had brought up some regulators (npu?) so those things might fail. Unsure if it applies to OPi5's...

Is it specific to npu or also affecting vpu etc.? I suppose there might be also some problems related to mac addresses, but i don't think it is also static in vendor uboot too

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, ... Ready to merge Reviewed, tested and ready for merge size/small PR with less then 50 lines

Development

Successfully merging this pull request may close these issues.

4 participants