Allow boot from USB and NVMe on ODROID-M1 #3852
Conversation
Use devtype instead of hardcoding device type to mmc in U-Boot script Signed-off-by: Ricardo Pardini <[email protected]>
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
home-assistant
bot
added
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
📝 WalkthroughWalkthroughThe pull request modifies the Changes
Sequence DiagramsequenceDiagram
participant U as U-Boot
participant S as Boot Script
participant D as Device Storage
U->>S: Execute boot script
S->>S: Determine device type
S->>D: Read partition info using ${devtype}
S->>D: Load configuration files
S->>D: Load kernel and device trees
S->>U: Boot system
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
buildroot-external/board/hardkernel/odroid-m1/uboot-boot.ush (2)
67-77: LGTM! Consider documenting storage type detection.The kernel loading implementation correctly supports different storage types while maintaining the A/B slot system. However, it would be helpful to document how U-Boot detects and sets the
${devtype}variable.Consider adding a comment explaining how the storage type is detected and how
${devtype}gets its value (e.g., 'mmc', 'usb', 'nvme').
Line range hint
1-77: Excellent implementation of multi-storage boot support!The changes consistently implement device-agnostic boot support throughout the script, enabling USB and NVMe boot while maintaining all existing functionality. The implementation is clean and follows a systematic approach to replace storage-specific commands with flexible alternatives.
This implementation provides a good foundation for adding support for other storage types in the future, as it abstracts the storage-specific commands behind the
${devtype}variable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
buildroot-external/board/hardkernel/odroid-m1/uboot-boot.ush(3 hunks)
🔇 Additional comments (3)
buildroot-external/board/hardkernel/odroid-m1/uboot-boot.ush (3)
1-12: LGTM! Device-agnostic boot state management.The replacement of hardcoded 'mmc' commands with '${devtype}' successfully makes the boot state management work across different storage types (MMC, USB, NVMe).
38-56: LGTM! Robust device tree handling across storage types.The device tree and overlay handling has been correctly adapted to work with different storage types while maintaining proper error handling.
Verify that the overlays path remains consistent across storage types:
✅ Verification successful
Overlay path structure verified and consistent
The "overlays/" directory path is consistently used across all board configurations, and the implementation correctly follows the established pattern while supporting flexible storage types.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if overlay paths are consistently defined # Look for overlay path definitions or mount points rg -A 3 "overlays.*dtbo|overlay.*path"Length of output: 4257
29-33: Verify config loading across storage types.The changes correctly implement device-agnostic config loading. However, we should verify the behavior across different storage types.
✅ Verification successful
Implementation follows established device-agnostic pattern
The changes align with the device-agnostic approach used in other modern board configurations (green, m1s, vim3). This pattern has been proven to work across different storage types.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if haos-config.txt loading is tested for different storage types # Look for test cases or documentation covering USB/NVMe config loading # Search for test files fd -t f -e sh -e py -e txt "test.*odroid.*m1" --exec rg -l "haos-config" # Search for documentation fd -t f -e md -e rst "odroid.*m1|readme|doc" --exec rg -l "haos-config.*usb|haos-config.*nvme"Length of output: 169
Script:
#!/bin/bash # Search for U-Boot test framework and boot scripts fd -t f "uboot-boot.ush" --exec rg -l "haos-config" # Look for similar patterns in other board configs rg -p "fatload.*haos-config" buildroot-external/board/ # Check for general boot documentation fd -t f -e md -e rst . --exec rg -l "boot.*configuration|haos.*config"Length of output: 1706
|
This rabbit's not too knowledgeable; |
Better said, those are "standard boot" variables, as "standard boot" is the name of the feature that provides them. It fairly new (since ~2022) as a built-in thing in U-Boot, maybe Coderabbit will pick it at some point in the future :) |
|
Nice. I'm working with a colleague on the Armbian uboot to get it tested and merged there (it's just setexpr and squashfs support, and a small patch to add fileenv, but otherwise is just mainline 2025.01 uboot) and then I'll come back for docs and updating the HAOS-shipped uboot the same way. Unsure if we want to go all in into supporting writing uboot to SPI as well. |
In similar vein to @viraniac's #3784, but for the ODROID-M1.
I've adapted Armbian's mainline u-boot (which is a decent way to get rid of Petitboot) with support for
setexprandfileenvandsquashfs: see armbian/build#7769. With those we can now boot HAOS using mainline u-boot in SPI, and thus run HAOS completely from NVMe, drastically enhancing performance and reliability.Summary by CodeRabbit