Update boot-mvebu.cmd #8166
Update boot-mvebu.cmd #8166
Conversation
Fix for armbian#8165 Determine the load address for kernel and initial ramdisk based on DT and kernel image size.
|
""" WalkthroughThe boot script in Suggested labels
Suggested reviewers
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)config/bootscripts/boot-mvebu.cmd (1)🔇 Additional comments (11)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 1
🧹 Nitpick comments (5)
config/bootscripts/boot-mvebu.cmd (5)
18-20: Make alignment & extra-size parameters self-consistent and documented
Great to seefdt_extrasizeandaddr_alignpromoted to variables – this makes the script far more portable.
Two small improvements would tighten things up:
- Document in a short comment that both values must be hexadecimal (or decimal) and that
addr_alignmust be a power-of-two.- Re-use
${addr_align}wherever0x1000is hard-coded later (lines 42-44) so that a single variable truly controls alignment.
20-20: Local scratch variablemodulocan leak into the persistent U-Boot environment
setexpr modulo …stores the result in the global environment.
If the board re-boots without power-cycling, the previous value stays around and could pollute other scripts.
Prependlocal.to the variable name (U-Boot 2020.04+) orsetexpr _mod …to minimise the risk:-setexpr modulo ${addr_next} % ${addr_align} -if itest $modulo -gt 0 ; then +setexpr _mod ${addr_next} % ${addr_align} +if itest $_mod -gt 0 ; then
47-49: Comment does not match implementation
The note “add 'extrasize' before aligning” is misleading – you actually add it after the first alignment and before the finaladdr_nextalignment.
Either update the comment or move the addition above the alignment step.
104-113: Guard against missing kernel / ramdisk and propagate their sizes
load ${devtype} … ${prefix}zImageand…uInitrdcan fail silently (e.g., wrong partition).
Consider surrounding eachloadwithifand aborting on failure, otherwisebootzwill jump into undefined memory.
Second, after loading the ramdisk you may want to advance and alignaddr_nextagain so scripts that chain-load additional blobs (e.g., firmware) can continue to reuse the helper.
115-116: Minor: echo source address range for easier debugging
Right beforebootz, printing the computed${kernel_addr_r}-${addr_next}and${ramdisk_addr_r}-${filesize}ranges greatly simplifies debugging overlapping issues on constrained boards.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
config/bootscripts/boot-mvebu.cmd(2 hunks)
🔇 Additional comments (1)
config/bootscripts/boot-mvebu.cmd (1)
72-75: Potential under-allocation after overlay failure
When overlays fail you reload the smaller original DT and callfdt resize ${fdt_extrasize}, but you do not re-computefdt_totalsize.
If the over-laid DT had grown beyondoriginal + extrasize,kernel_addr_rnow points further away than necessary – harmless but wastes RAM.
Conversely, iffdt resizegrew the blob more than the first calculation (unlikely but possible with a different${fdt_extrasize}), kernel/ramdisk could overlap.
At minimum, please add a safety note or recomputefdt_totalsizein this branch.
|
Testing by @igorpecovnik showed that U-Boot from a freshly built image does not have the Notes to self:
|
- Updated names to reflect function better. - Followed coding style of rest of the file, e.g. `test` instead of `itest` and `if ... ; then`. - If `setexpr` fails, use configured addresses - either the ones defined in boot.cmd or set by armbianEnv.txt.
Changed the DT size back to 256KiB as there is no reason to trim it actually.
|
Changes for 17329c1:
Testing:
|
|
Changes for 17329c1:
|
Follow-up for: - armbian#8165 - armbian#8166 Enable the `setexpr` command on the U-Boot monitor commandline, to enable calculating the load addresses based on the sizes of the DT, kernel, initial ramdisk.
Fix for #8165
Determine the load address for kernel and initial ramdisk based on DT and kernel image size.
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
${fdt_extrasize}variable tofdt resize ${fdt_extrasize}at every point afdt resizeis done.fdt applyandfdt resizefrom the SPI -SATA workaround as fdt already applied and resized.fdt resize ${fdt_extrasize}at fallback to original DT in case any overlay fails to apply.loads.totalsizeof the DT with a fallback to calculating it based on DT filesize + size increases due tofdt resize.${addr_align}.GitHub issue reference: 8165
Jira reference number AR-2676
Documentation summary for feature / change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.
Helios4 with U-Boot 2018.11 and
linux-image-current-mvebu25.5.0-trunk.435.As
fdt header get <var> totalsizeis not working on this version of U-Boot, the fallback correctly determines the DT size.Box boots without issue.
Orangepi zero with U-Boot 2021.04-armbian (
linux-u-boot-orangepizero-current25.5.0-trunk.496) andlinux-image-current-sunxi25.5.0-trunk.496.This was a test of the concept itself, to see if the calculation methods would work. On this U-Boot the
fdt header get <var> totalsizeis working.SBC boots without issue.
Checklist:
Please delete options that are not relevant.