Allow boot from USB and NVMe on Khadas VIM3 #3784
Conversation
Signed-off-by: Gunjan Gupta <gunjan@wesion.com>
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
📝 WalkthroughWalkthroughThe pull request modifies the Changes
Sequence DiagramsequenceDiagram
participant Script as Boot Script
participant DevType as ${devtype}
participant Storage as Storage Device
Script->>DevType: Determine device type
DevType->>Storage: Select storage device
Storage-->>Script: Prepare for boot operations
Script->>Storage: Perform boot actions (load kernel, etc.)
The sequence diagram illustrates how the script now uses a dynamic 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
buildroot-external/board/khadas/vim3/uboot-boot.ush (1)
Line range hint
1-108: Consider documenting supported device types and testing requirements.The changes successfully implement dynamic device type support, making the boot process more flexible. To ensure long-term maintainability:
- Document the supported device types (MMC, USB, NVMe) in the board's documentation
- Add test cases to verify boot functionality across different storage devices
- Consider adding a comment in the script header explaining the device type handling
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
buildroot-external/board/khadas/vim3/uboot-boot.ush(3 hunks)
🔇 Additional comments (5)
buildroot-external/board/khadas/vim3/uboot-boot.ush (5)
4-14: LGTM! Verify environment storage compatibility.The replacement of hardcoded
mmccommands with${devtype}maintains the same functionality while adding support for different storage devices. The environment storage mechanism remains consistent.Please ensure that the environment storage works correctly with different device types by testing:
- Reading/writing environment from MMC
- Reading/writing environment from USB
- Reading/writing environment from NVMe
32-33: LGTM! Configuration loading properly adapted.The configuration loading mechanism has been correctly updated to use the dynamic device type while maintaining the same functionality.
38-38: LGTM! Command line loading properly adapted.The command line loading mechanism has been correctly updated to use the dynamic device type.
43-56: LGTM! Verify DTB loading across device types.The device tree and overlay loading mechanisms have been correctly updated to use the dynamic device type.
Please verify that the device tree and overlays load correctly from different storage devices, particularly testing:
- Base DTB loading
- Overlay loading and application
- Error handling and DTB restoration
77-85: LGTM! Ensure comprehensive boot testing.The kernel loading mechanism has been correctly updated to use the dynamic device type while maintaining the A/B slot management functionality.
Please ensure comprehensive testing of the boot process:
- Successful boot from both A and B slots
- Proper fallback behavior when a boot fails
- Boot counter decrement working correctly
- Testing across all supported device types (MMC, USB, NVMe)
Currently its hardcoded in the script to use mmc command. This means the image can not be flashed to usb or nvme drive and booted from the same. Making it flexible to allow booting in such scenarios.
Summary by CodeRabbit