Skip to content

Conversation

@jonaswood01
Copy link
Contributor

@jonaswood01 jonaswood01 commented Dec 15, 2025

Description

Add board support for SK-AM62-LP and SK-AM62-SIP boards using AM62x SoC:

  • sk-am62-lp.conf is very similar to sk-am62b.conf, except has its own u-boot defconfigs and kernel device tree
  • sk-am62-sip.conf is also similar to sk-am62b.conf, with manually defined ATF and OPTEE regions due to fixed 512MB RAM packaged on-SoC

Neither board officially supports wifi via CC33XX, though that might come later on.

Documentation summary for feature / change

Will need new boards in Armbian.com. @Grippy98 would love your help here

How Has This Been Tested?

  • sk-am62-lp: build log, boot log
  • sk-am62-sip: build log, boot log
    • Strangely I can only boot sk-am62-sip from builds where I first do sudo rm -rf cache and rm -rf output
    • I haven't extensively debugged this behavior but first noticed when plugged into build automation with many sequential k3 builds without removing cache in between. Might be worth looking into more

Note: the git status changes in the build logs are proxy-related only.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings (as far as I can tell)

Summary by CodeRabbit

  • New Features

    • Added support for TI AM62-LP and AM62-SIP development boards with full boot and platform configurations.
  • Refactor

    • Unified ATF argument handling: replaced a USART-specific setting with a general ATF argument and updated how extra ATF arguments are passed into the build targets.

✏️ Tip: You can customize this high-level summary in your review settings.

@jonaswood01 jonaswood01 requested review from a team and tabrisnet December 15, 2025 22:49
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 15, 2025

Walkthrough

Adds two new TI AM62 board config files and standardizes ATF argument plumbing by replacing per-board USART var with a generic ATF_ARGS and updating the K3 ATF target map to include those args.

Changes

Cohort / File(s) Change Summary
New SK AM62 board configs
config/boards/sk-am62-lp.conf, config/boards/sk-am62-sip.conf
Add declarative board configuration files specifying board metadata, boot/ATF/TIboot3 settings, kernel targets, serial console, and OP-TEE parameters.
K3 ATF build wiring
config/sources/families/include/k3_common.inc
Replace ${ATF_K3_USART_OFFSET} with ${ATF_ARGS} inside ATF_TARGET_MAP, changing how extra ATF build arguments are threaded into the target map.
K3 board variable rename
config/boards/sk-am68.conf, config/boards/sk-am69.conf
Remove ATF_K3_USART_OFFSET and add ATF_ARGS="K3_USART=0x8" (rename/relocate the USART argument into ATF_ARGS).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Attention points:
    • Confirm ATF_ARGS formatting and quoting across boards match expectations.
    • Verify k3_common.inc substitution won't break ATF build targets or makefiles that expect the old variable.
    • Validate newly added board metadata (FDT/file names, OPTEE args) for typos.

Possibly related PRs

Suggested labels

BSP

Suggested reviewers

  • glneo
  • igorpecovnik
  • rpardini

Poem

🐰 I hopped in code, left two boards neat,

ATF_ARGS now skips the old seat.
SK-AM62s in tidy rows,
Build-time carrots in neat little rows.
— a rabbit, pleased with tiny prose 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding board support for two new AM62x SoC boards (SK-AM62-LP and SK-AM62-SIP) to the TI configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

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 d966bc4 and 10f4d10.

📒 Files selected for processing (4)
  • config/boards/sk-am62-sip.conf (1 hunks)
  • config/boards/sk-am68.conf (1 hunks)
  • config/boards/sk-am69.conf (1 hunks)
  • config/sources/families/include/k3_common.inc (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • config/sources/families/include/k3_common.inc
🧰 Additional context used
🧠 Learnings (28)
📓 Common learnings
Learnt from: EvilOlaf
Repo: armbian/build PR: 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: EvilOlaf
Repo: armbian/build PR: 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 API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.
Learnt from: glneo
Repo: armbian/build PR: 8913
File: config/sources/families/include/k3_common.inc:57-60
Timestamp: 2025-11-11T20:56:20.303Z
Learning: In config/sources/families/include/k3_common.inc, the OP-TEE build command at line 59 should be updated in a future PR to explicitly set CROSS_COMPILE64=aarch64-linux-gnu- and CROSS_COMPILE32=arm-linux-gnueabihf- instead of relying on OP-TEE's internal defaults, for better clarity and maintainability. User glneo agreed to address this in a separate PR.
Learnt from: Grippy98
Repo: armbian/build PR: 8624
File: config/boards/sk-am62p.conf:8-8
Timestamp: 2025-09-12T21:44:09.061Z
Learning: For TI K3 family boards in Armbian, BOOT_FDT_FILE uses .dts extension (not .dtb) as the standard convention. The build system handles this correctly by automatically compiling .dts to .dtb during kernel build and using the BOOT_FDT_FILE value directly in bootloader configurations.
📚 Learning: 2025-11-02T20:49:56.719Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.719Z
Learning: In Armbian board configuration files (config/boards/*.conf, *.csc, etc.), do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__* functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Board-specific kernel modifications cause inconsistency in kernel packages published to the apt repository because boards within a family share the same kernel packages. Kernel configuration changes must be made in the appropriate kernel config file (e.g., config/kernel/linux-*-*.config) or in family configuration files (config/sources/families/*.conf, *.inc) instead.

Applied to files:

  • config/boards/sk-am68.conf
  • config/boards/sk-am69.conf
  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-12-15T23:26:34.998Z
Learnt from: bryanbrattlof
Repo: armbian/build PR: 9081
File: config/boards/sk-am62-sip.conf:18-20
Timestamp: 2025-12-15T23:26:34.998Z
Learning: For TI K3 SoC boards with only 512MB of DDR (like SK-AM62-SIP), OP-TEE (BL32) and TFA must be placed at the bottom of DDR (e.g., BL32_BASE=0x80080000, CFG_TZDRAM_START=0x80080000) rather than the standard high-memory layout. This is necessary because U-Boot relocates itself to the highest DDR address (up to 2GB max) during bootup, which would overwrite OP-TEE/TFA if they were placed at the top of the 512MB DDR region. PRELOADED_BL33_BASE is typically set to 0x82000000 in this configuration.

Applied to files:

  • config/boards/sk-am68.conf
  • config/boards/sk-am69.conf
  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-09-12T19:28:38.491Z
Learnt from: Grippy98
Repo: armbian/build PR: 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/sk-am68.conf
  • config/boards/sk-am69.conf
  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-11-11T20:56:20.303Z
Learnt from: glneo
Repo: armbian/build PR: 8913
File: config/sources/families/include/k3_common.inc:57-60
Timestamp: 2025-11-11T20:56:20.303Z
Learning: In config/sources/families/include/k3_common.inc, the OP-TEE build command at line 59 should be updated in a future PR to explicitly set CROSS_COMPILE64=aarch64-linux-gnu- and CROSS_COMPILE32=arm-linux-gnueabihf- instead of relying on OP-TEE's internal defaults, for better clarity and maintainability. User glneo agreed to address this in a separate PR.

Applied to files:

  • config/boards/sk-am68.conf
  • config/boards/sk-am69.conf
  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-09-12T21:44:09.061Z
Learnt from: Grippy98
Repo: armbian/build PR: 8624
File: config/boards/sk-am62p.conf:8-8
Timestamp: 2025-09-12T21:44:09.061Z
Learning: For TI K3 family boards in Armbian, BOOT_FDT_FILE uses .dts extension (not .dtb) as the standard convention. The build system handles this correctly by automatically compiling .dts to .dtb during kernel build and using the BOOT_FDT_FILE value directly in bootloader configurations.

Applied to files:

  • config/boards/sk-am68.conf
  • config/boards/sk-am69.conf
  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-03-31T12:57:13.880Z
Learnt from: libiunc
Repo: armbian/build PR: 8033
File: config/kernel/linux-starfive2-vendor.config:43-43
Timestamp: 2025-03-31T12:57:13.880Z
Learning: For StarFive2 platform kernel configurations, maintain alignment with vendor-provided configurations rather than modifying security settings like SECCOMP. This ensures hardware compatibility as intended by the manufacturer.

Applied to files:

  • config/boards/sk-am68.conf
  • config/boards/sk-am69.conf
  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-06-04T23:45:38.860Z
Learnt from: djurny
Repo: armbian/build PR: 8272
File: config/bootscripts/boot-mvebu.cmd:182-186
Timestamp: 2025-06-04T23:45:38.860Z
Learning: In config/bootscripts/boot-mvebu.cmd, the `fdtfile` variable is mandatory for booting and is pre-set by U-Boot, but can be overridden via armbianEnv.txt. If `fdtfile` is empty, the subsequent device tree file search logic will eventually fail and trigger the critical error "Cannot find DT!" with proper error handling.

Applied to files:

  • config/boards/sk-am69.conf
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files, .wip and .conf extensions require BOARD_MAINTAINER to be present and contain at least one maintainer. If no maintainer is present, the board support rules state it does not qualify for standard support and must be moved to community support (.csc extension). This is documented at https://docs.armbian.com/User-Guide_Board-Support-Rules/

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-07-17T04:12:33.125Z
Learnt from: pyavitz
Repo: armbian/build PR: 8388
File: config/boards/kickpik2b.conf:1-14
Timestamp: 2025-07-17T04:12:33.125Z
Learning: In the Armbian build system, board configuration files (*.conf) follow a project standard of not including shebang lines, even though they contain bash code and are sourced by the build system. This is an established Armbian convention that individual contributors are expected to follow, and changes to this standard would require broader project maintainer approval.

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-06-12T21:14:36.024Z
Learnt from: djurny
Repo: armbian/build PR: 8287
File: config/bootscripts/boot-sunxi.cmd:38-44
Timestamp: 2025-06-12T21:14:36.024Z
Learning: In config/bootscripts/boot-sunxi.cmd the unconditional "+1" increment in func_align_addr_next (when align_overlap_oboe_avoidance="on") is intentional. It compensates for a known off-by-one error in U-Boot where the end address is calculated as start+size instead of start+size-1, so the extra page prevents overlap. This behavior should not be “optimized away”.

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-06-12T21:08:11.318Z
Learnt from: djurny
Repo: armbian/build PR: 8287
File: config/bootscripts/boot-sunxi.cmd:13-23
Timestamp: 2025-06-12T21:08:11.318Z
Learning: In config/bootscripts/boot-sunxi.cmd the variable name `align_overlap_oboe_avoidance` intentionally uses the acronym “OBOE” (Off-By-One Error); it is not a typo and should not be renamed.

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-05-07T20:49:40.969Z
Learnt from: djurny
Repo: armbian/build PR: 8166
File: config/bootscripts/boot-mvebu.cmd:38-50
Timestamp: 2025-05-07T20:49:40.969Z
Learning: The 4KB (0x1000) boundary used for device tree size calculations in U-Boot scripts is specific to how the `fdt resize` command works internally and should remain hardcoded rather than being tied to adjustable alignment variables.

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-12-13T11:39:08.046Z
Learnt from: pyavitz
Repo: armbian/build PR: 9058
File: patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch:28-67
Timestamp: 2025-12-13T11:39:08.046Z
Learning: In the Armbian build system for SpacemiT U-Boot patches (patch/u-boot/legacy/u-boot-spacemit-k1/), alignment with mainline U-Boot behavior is prioritized. For example, in boot mode handling, leaving devnum unchanged in the default case (when devtype is cleared) follows mainline conventions rather than explicitly clearing it to handle edge cases.

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-12-13T11:45:02.422Z
Learnt from: pyavitz
Repo: armbian/build PR: 9058
File: patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch:17-25
Timestamp: 2025-12-13T11:45:02.422Z
Learning: In the SpacemiT U-Boot patches for Armbian (patch/u-boot/legacy/u-boot-spacemit-k1/), the environment variable `devnum` is set to the device name string (e.g., "mmc", "nvme") rather than a numeric index, and `distro_bootpart` holds the partition number. This implementation aligns with mainline U-Boot conventions for the SpacemiT platform and has been verified to work correctly by the maintainer.

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-07-25T03:51:50.830Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 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.

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-07-25T03:51:50.830Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 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 API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-12-16T13:40:01.143Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 9087
File: .github/workflows/pr-check-pictures.yml:138-146
Timestamp: 2025-12-16T13:40:01.143Z
Learning: In the Armbian build repository, when introducing new requirements or checks (like the board assets verification workflow), the project prefers an initial educational-only period where violations post helpful PR comments and warnings but don't block merges. This allows contributors to become familiar with new requirements before enforcement is enabled (typically after ~6 months).

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-03-31T22:20:48.475Z
Learnt from: rpardini
Repo: armbian/build PR: 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/sk-am62-sip.conf
📚 Learning: 2025-11-20T18:20:11.985Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8968
File: patch/u-boot/u-boot-sunxi/arm64-dts-sun50i-h6-orangepi.dtsi-Rollback-r_rsb-to-r_i2c.patch:36-36
Timestamp: 2025-11-20T18:20:11.985Z
Learning: The rewrite-patches tool (REWRITE_PATCHES=yes) in the Armbian build system can inadvertently introduce semantic changes when the u-boot/kernel git base revision differs from expected state. The tool applies patches, commits them, and re-exports them using git format-patch, which can cause the re-exported patch to reflect the base revision's state rather than preserving the original patch intent. This is particularly problematic for device tree changes like interrupt specifications. The tool currently lacks validation mechanisms to detect such semantic drift, and affected patches must be manually corrected after rewriting.

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-10-05T19:31:16.930Z
Learnt from: rpardini
Repo: armbian/build PR: 8718
File: config/boards/thinkpad-x13s.conf:185-185
Timestamp: 2025-10-05T19:31:16.930Z
Learning: In config/boards/thinkpad-x13s.conf, the x13s_obtain_firmware() function is reference-only utility code that never runs. It is kept for documentation purposes. Updates to firmware must be done at the armbian/firmware repository. TODO comments in this function are intentional reminders and should not be flagged.

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-07-21T04:12:02.439Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-07-21T04:12:02.439Z
Learning: In the Armbian build system, for recurring maintenance tasks like kernel version bumping, TODO comments should use generic version formats (e.g., "MAJOR.MINOR-rc1") rather than specific version numbers (e.g., "6.17-rc1") to avoid the need for frequent comment updates that would create unnecessary maintenance overhead.

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-09-25T18:37:00.330Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-10-26T10:41:35.118Z
Learnt from: HackingGate
Repo: armbian/build PR: 8665
File: config/boards/photonicat2.csc:4-4
Timestamp: 2025-10-26T10:41:35.118Z
Learning: In the Armbian build system, rk3576 boards consistently use BOARDFAMILY="rk35xx" for both vendor and edge kernel targets. The rk35xx family configuration sources rockchip64_common.inc, which provides edge and current kernel branch definitions, making these branches available even though they're not defined directly in rk35xx.conf.

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-10-14T05:08:11.785Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 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/sk-am62-sip.conf
📚 Learning: 2025-11-10T22:05:40.490Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8913
File: config/sources/families/k3-beagle.conf:16-16
Timestamp: 2025-11-10T22:05:40.490Z
Learning: In the Armbian build system, kernel branches using non-mainline/vendor forks (like BeagleBoard's linux repository) should be named "vendor" or "vendor-rt" rather than "current" or "edge". The "current" and "edge" naming is reserved for mainline kernel branches. This affects both the case statement in family config files (e.g., `vendor | vendor-rt)` instead of `current | current-rt)`) and the corresponding KERNEL_TARGET declarations in board config files.

Applied to files:

  • config/boards/sk-am62-sip.conf
📚 Learning: 2025-09-14T06:29:18.958Z
Learnt from: amazingfate
Repo: armbian/build PR: 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/sk-am62-sip.conf
🔇 Additional comments (7)
config/boards/sk-am69.conf (1)

18-18: LGTM! Clean refactoring to standardize ATF argument passing.

The rename from ATF_K3_USART_OFFSET to ATF_ARGS with the preserved value makes the configuration more flexible and aligns with the broader standardization across K3 family boards and the updated ATF_TARGET_MAP usage in k3_common.inc.

config/boards/sk-am68.conf (1)

18-18: LGTM! Consistent with the ATF_ARGS standardization.

The refactoring from ATF_K3_USART_OFFSET to ATF_ARGS matches the pattern applied to sk-am69.conf and improves consistency across the K3 board family.

config/boards/sk-am62-sip.conf (5)

1-2: Well-documented hardware specification.

The comment correctly specifies the 512MB fixed RAM, addressing the past review feedback. The format follows Armbian conventions for board configuration headers.


3-6: LGTM! Board identification is properly configured.

The board metadata is complete with a maintainer specified, satisfying the requirements for .conf file extension per Armbian board support rules.


7-12: LGTM! Boot configuration follows K3 family conventions.

The BOOT_FDT_FILE correctly uses the .dts extension per K3 board standards, and the defconfig names are appropriate for the AM6254XXL variant.


13-16: LGTM! Standard console and kernel configuration for K3 boards.

The kernel targets and serial console settings are consistent with other AM62x boards in the K3 family.


17-20: Excellent! Memory layout correctly configured for 512MB DDR with comprehensive documentation.

The memory region definitions are correct for the 512MB fixed RAM configuration:

  • OP-TEE/TFA placed at bottom of DDR (0x80080000) to avoid U-Boot relocation conflicts
  • U-Boot load address at 0x82000000 provides adequate space
  • Inline comments explain the rationale
  • Documentation link provides additional context

This addresses all past review feedback and follows the validated approach for constrained DDR configurations.

Based on learnings, as confirmed by bryanbrattlof in past reviews.


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/small PR with less then 50 lines 02 Milestone: First quarter release Needs review Seeking for review Hardware Hardware related like kernel, U-Boot, ... labels Dec 15, 2025
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: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

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 f2dd38e and d966bc4.

📒 Files selected for processing (3)
  • config/boards/sk-am62-lp.conf (1 hunks)
  • config/boards/sk-am62-sip.conf (1 hunks)
  • config/sources/families/include/k3_common.inc (1 hunks)
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: EvilOlaf
Repo: armbian/build PR: 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: EvilOlaf
Repo: armbian/build PR: 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 API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.
Learnt from: glneo
Repo: armbian/build PR: 8913
File: config/sources/families/include/k3_common.inc:57-60
Timestamp: 2025-11-11T20:56:20.303Z
Learning: In config/sources/families/include/k3_common.inc, the OP-TEE build command at line 59 should be updated in a future PR to explicitly set CROSS_COMPILE64=aarch64-linux-gnu- and CROSS_COMPILE32=arm-linux-gnueabihf- instead of relying on OP-TEE's internal defaults, for better clarity and maintainability. User glneo agreed to address this in a separate PR.
📚 Learning: 2025-11-11T20:56:20.303Z
Learnt from: glneo
Repo: armbian/build PR: 8913
File: config/sources/families/include/k3_common.inc:57-60
Timestamp: 2025-11-11T20:56:20.303Z
Learning: In config/sources/families/include/k3_common.inc, the OP-TEE build command at line 59 should be updated in a future PR to explicitly set CROSS_COMPILE64=aarch64-linux-gnu- and CROSS_COMPILE32=arm-linux-gnueabihf- instead of relying on OP-TEE's internal defaults, for better clarity and maintainability. User glneo agreed to address this in a separate PR.

Applied to files:

  • config/sources/families/include/k3_common.inc
📚 Learning: 2025-09-12T19:28:38.491Z
Learnt from: Grippy98
Repo: armbian/build PR: 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/sources/families/include/k3_common.inc
  • config/boards/sk-am62-sip.conf
  • config/boards/sk-am62-lp.conf
📚 Learning: 2025-12-13T11:39:08.046Z
Learnt from: pyavitz
Repo: armbian/build PR: 9058
File: patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch:28-67
Timestamp: 2025-12-13T11:39:08.046Z
Learning: In the Armbian build system for SpacemiT U-Boot patches (patch/u-boot/legacy/u-boot-spacemit-k1/), alignment with mainline U-Boot behavior is prioritized. For example, in boot mode handling, leaving devnum unchanged in the default case (when devtype is cleared) follows mainline conventions rather than explicitly clearing it to handle edge cases.

Applied to files:

  • config/sources/families/include/k3_common.inc
📚 Learning: 2025-11-02T20:49:56.719Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.719Z
Learning: In Armbian board configuration files (config/boards/*.conf, *.csc, etc.), do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__* functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Board-specific kernel modifications cause inconsistency in kernel packages published to the apt repository because boards within a family share the same kernel packages. Kernel configuration changes must be made in the appropriate kernel config file (e.g., config/kernel/linux-*-*.config) or in family configuration files (config/sources/families/*.conf, *.inc) instead.

Applied to files:

  • config/boards/sk-am62-sip.conf
  • config/boards/sk-am62-lp.conf
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files, .wip and .conf extensions require BOARD_MAINTAINER to be present and contain at least one maintainer. If no maintainer is present, the board support rules state it does not qualify for standard support and must be moved to community support (.csc extension). This is documented at https://docs.armbian.com/User-Guide_Board-Support-Rules/

Applied to files:

  • config/boards/sk-am62-sip.conf
  • config/boards/sk-am62-lp.conf
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).

Applied to files:

  • config/boards/sk-am62-sip.conf
  • config/boards/sk-am62-lp.conf
📚 Learning: 2025-09-12T21:44:09.061Z
Learnt from: Grippy98
Repo: armbian/build PR: 8624
File: config/boards/sk-am62p.conf:8-8
Timestamp: 2025-09-12T21:44:09.061Z
Learning: For TI K3 family boards in Armbian, BOOT_FDT_FILE uses .dts extension (not .dtb) as the standard convention. The build system handles this correctly by automatically compiling .dts to .dtb during kernel build and using the BOOT_FDT_FILE value directly in bootloader configurations.

Applied to files:

  • config/boards/sk-am62-sip.conf
  • config/boards/sk-am62-lp.conf
📚 Learning: 2025-10-26T10:41:35.118Z
Learnt from: HackingGate
Repo: armbian/build PR: 8665
File: config/boards/photonicat2.csc:4-4
Timestamp: 2025-10-26T10:41:35.118Z
Learning: In the Armbian build system, rk3576 boards consistently use BOARDFAMILY="rk35xx" for both vendor and edge kernel targets. The rk35xx family configuration sources rockchip64_common.inc, which provides edge and current kernel branch definitions, making these branches available even though they're not defined directly in rk35xx.conf.

Applied to files:

  • config/boards/sk-am62-sip.conf
  • config/boards/sk-am62-lp.conf
📚 Learning: 2025-11-10T22:05:40.490Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8913
File: config/sources/families/k3-beagle.conf:16-16
Timestamp: 2025-11-10T22:05:40.490Z
Learning: In the Armbian build system, kernel branches using non-mainline/vendor forks (like BeagleBoard's linux repository) should be named "vendor" or "vendor-rt" rather than "current" or "edge". The "current" and "edge" naming is reserved for mainline kernel branches. This affects both the case statement in family config files (e.g., `vendor | vendor-rt)` instead of `current | current-rt)`) and the corresponding KERNEL_TARGET declarations in board config files.

Applied to files:

  • config/boards/sk-am62-sip.conf
  • config/boards/sk-am62-lp.conf
📚 Learning: 2025-10-14T05:08:11.785Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 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/sk-am62-sip.conf
  • config/boards/sk-am62-lp.conf
📚 Learning: 2025-09-14T06:29:18.958Z
Learnt from: amazingfate
Repo: armbian/build PR: 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/sk-am62-sip.conf
  • config/boards/sk-am62-lp.conf
📚 Learning: 2025-07-23T07:30:52.265Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 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/sk-am62-sip.conf
  • config/boards/sk-am62-lp.conf
📚 Learning: 2025-07-17T04:12:33.125Z
Learnt from: pyavitz
Repo: armbian/build PR: 8388
File: config/boards/kickpik2b.conf:1-14
Timestamp: 2025-07-17T04:12:33.125Z
Learning: In the Armbian build system, board configuration files (*.conf) follow a project standard of not including shebang lines, even though they contain bash code and are sourced by the build system. This is an established Armbian convention that individual contributors are expected to follow, and changes to this standard would require broader project maintainer approval.

Applied to files:

  • config/boards/sk-am62-lp.conf
📚 Learning: 2025-08-21T08:10:59.502Z
Learnt from: leggewie
Repo: armbian/build PR: 8524
File: config/boards/orangepi2.csc:6-6
Timestamp: 2025-08-21T08:10:59.502Z
Learning: Not all Armbian boards support all kernel versions (legacy, current, edge). Some boards may only support specific kernel versions due to hardware limitations or lack of mainline support, which is why their KERNEL_TARGET contains only the supported options (e.g., just "legacy").

Applied to files:

  • config/boards/sk-am62-lp.conf
🔇 Additional comments (3)
config/boards/sk-am62-sip.conf (2)

1-20: Board configuration follows standard K3 patterns.

The overall structure is correct and consistent with other K3 board configs, including proper use of .dts extension for BOOT_FDT_FILE, presence of BOARD_MAINTAINER, and appropriate vendor branch naming in KERNEL_TARGET.


8-12: Correct the U-Boot defconfig names for SK-AM62-SIP.

The defconfig names in the code should be am62xsip_evm_a53_defconfig and am62xsip_evm_r5_defconfig (matching the SK-AM62-SIP board), not am6254xxl_evm_a53_defconfig and am6254xxl_evm_r5_defconfig. These defconfigs exist in the U-Boot source tree (not in Armbian's config/ directory) and are documented in the TI Processor SDK U-Boot guide.

Likely an incorrect or invalid review comment.

config/boards/sk-am62-lp.conf (1)

1-19: LGTM! Board configuration follows standard K3 patterns.

The configuration structure is correct and consistent with other K3 board configs (sk-am62p.conf, sk-am62b.conf). The BOOT_FDT_FILE uses .dts extension (standard for K3 family), BOARD_MAINTAINER is present (required for .conf files), and kernel targets use appropriate vendor branch naming. U-Boot defconfigs are sourced from upstream U-Boot during the build process, not stored in the Armbian repository.

Comment on lines 18 to 20
ATF_EXTRA_ARGS="BL32_BASE=0x80080000 PRELOADED_BL33_BASE=0x82000000"
OPTEE_ARGS="CFG_TEE_CORE_LOG_LEVEL=1 CFG_TZDRAM_START=0x80080000"
OPTEE_PLATFORM="k3-am62x"
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 15, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the file and understand the board configuration
cat config/boards/sk-am62-sip.conf

Repository: armbian/build

Length of output: 706


🏁 Script executed:

# Let's also check if there are other AM62 board configs to compare memory layouts
fd "sk-am62" config/boards/

Repository: armbian/build

Length of output: 173


🏁 Script executed:

# Search for any documentation or comments about memory layout in the codebase
rg "0x80080000|0x82000000" --type conf --type txt

Repository: armbian/build

Length of output: 85


🌐 Web query:

TI AM62SIP memory layout OP-TEE ATF BL32 TZDRAM documentation

💡 Result:

Here are authoritative docs and notes about AM62SIP memory layout / OP‑TEE (BL32 / tee-pager) / ATF (BL31) / TZDRAM and related build info:

  • TI Processor SDK — OP‑TEE overview + where TI packages BL32 (tee‑pager_v2.bin) inside tispl.bin and prebuilt bl32.bin location. Useful for OP‑TEE/BL32 placement and build instructions. [1]

  • Processor SDK (Migration / Release notes) — shows AM62SIP platform support and the ATF / OP‑TEE versions used in TI releases (useful to match memory-layout expectations to specific ATF/OP‑TEE versions). [2][3]

  • U‑Boot (TI/K3 build docs) — how BL31/BL32 (ATF/OP‑TEE) are passed into U‑Boot (BINMAN/BINMAN_INDIRS, BL31/TEE params) and build instructions for AM62SIP. Relevant when arranging memory layout and reserved regions. [4]

  • U‑Boot patch / mailing-list discussion for AM62x / AM62SIP — notes that AM62SIP (512 MiB DDR) reserves the top of RAM for TF‑A and OP‑TEE and therefore U‑Boot is restricted to lower RAM (leave ~64 MiB reserved). This documents the practical TZDRAM/BL32 placement behaviour on AM62SIP. [5]

References

  1. TI Processor SDK — OP‑TEE section (bl32/tee‑pager info). [1] https://software-dl.ti.com/processor-sdk-linux/.../Foundational_Components_OPTEE.html
  2. Processor SDK AM62x Release/Migration notes (ATF/OP‑TEE versions; AM62SIP listed). [2] https://software-dl.ti.com/processor-sdk-linux/.../Release_Specific_Migration_Guide.html
  3. Processor SDK AM62Ax OP‑TEE Release notes (OP‑TEE/TF‑A commits used). [3] https://software-dl.ti.com/processor-sdk-linux/.../Release_Specific_Release_Notes.html
  4. TI U‑Boot / build docs for K3 / AM62 (BL31/TEE/BINMAN usage). [4] https://software-dl.ti.com/processor-sdk-linux-rt/.../BG-Build-K3.html
  5. U‑Boot patch / discussion: “Avoid overwriting reserve mem for AM62 SIP” (describes reserved top‑of‑RAM for TF‑A/OP‑TEE). [5] https://patchwork.ozlabs.org/.../board:ti:am62x

If you want, I can:

  • extract the exact BL31/BL32/TZDRAM base and size values from the relevant ATF/OP‑TEE platform files (Makefiles / platform config) and provide the precise addresses used by a specific Processor SDK/ATF/OP‑TEE release — tell me which SDK/ATF/OP‑TEE version you’re targeting.

🌐 Web query:

AM62 SoC 512MB RAM memory map BL32 BL33 OP-TEE ATF addresses

💡 Result:

Summary memory map used on AM62 (512 MiB DDR) — typical addresses used by TF-A (ATF/BL31), OP-TEE (BL32) and BL33 (EL3‑loaded normal world payload / kernel or U‑Boot):

  • DDR range: 0x8000_0000 — 0xA000_0000 (512 MiB). [1][2]
  • TF‑A / ATF (BL31) reserved at the low DDR region; TF‑A region starts at 0x8000_0000 (small reserved area: first 0x80000 / 512 KiB used in some layouts). BL33 preload area often begins at 0x8008_0000. [1]
  • BL33 (preloaded normal‑world payload: kernel / U‑Boot load addr) commonly at 0x8008_0000 / 0x8200_0000 depending on boot mode (see U‑Boot K3 board layout). [1]
  • BL32 (OP‑TEE) typical base for AM62 (512 MiB) is 0x9E80_0000 with ~24 MiB reserved for OP‑TEE; OP‑TEE region ends before 0xA000_0000. [1][3]
  • Many AM62 board trees reserve the top ~64 MiB of a 512 MiB part for TF‑A/OP‑TEE/etc.; U‑Boot limits relocations accordingly to avoid overwriting those regions. [0][1]

Sources:

  • U‑Boot AM62x board memory map (shows DDR start, TF‑A reserved area, PRELOADED_BL33_BASE and BL32_BASE = 0x9E800000, end DDR 0xA0000000). [1]
  • TI MCU+ / AM62x memory map docs (shows TF‑A and OP‑TEE at 0x9E800000 region). [3]
  • Patch/patchwork discussion re: 512 MiB boards reserving top of RAM for TF‑A/OP‑TEE (U‑Boot changes to avoid overwrite). [0]

If you want I can: (a) extract exact Kconfig defaults (CONFIG_K3_ATF_LOAD_ADDR / CONFIG_K3_OPTEE_LOAD_ADDR) from a specific U‑Boot / TF‑A build, or (b) produce a compact visual hex map for your bootloader config — tell me which build/version to target.


🏁 Script executed:

# Check the other AM62 board configs for their memory layout settings
for file in config/boards/sk-am62-lp.conf config/boards/sk-am62b.conf config/boards/sk-am62p.conf; do
  echo "=== $file ==="
  grep -E "ATF_EXTRA_ARGS|OPTEE_ARGS|OPTEE_PLATFORM" "$file" 2>/dev/null || echo "No ATF/OPTEE config found"
done

Repository: armbian/build

Length of output: 358


🏁 Script executed:

# Search for any documentation or comments about AM62SIP memory in the repo
rg "80080000|82000000|9E800000" -A 2 -B 2

Repository: armbian/build

Length of output: 23986


🏁 Script executed:

# Check if there are any build/defconfig files that might reference memory layout
fd "am62.*defconfig" --type f

Repository: armbian/build

Length of output: 39


🌐 Web query:

AM62SIP 512MB DDR memory layout U-Boot ATF OP-TEE addresses 0x80080000 0x82000000

💡 Result:

Summary for AM62SIP (512 MiB DDR) — relevant addresses and roles

  • DDR physical range: 0x8000_0000 — 0x9FFF_FFFF (512 MiB = 0x2000_0000 from 0x8000_0000). [1][2]
  • U-Boot / SPL (A53) text base: 0x8008_0000 (A53 SPL TEXT_BASE / PRELOADED_BL33 region). This is where U‑Boot SPL/BL33 is placed in the lower DDR. [1]
  • R5 / SPL stack/heap boundary and reserved region marker: 0x8200_0000 is used in the U‑Boot SPL memory map (R5 SPL stack/heap / split point in the lower DDR). U‑Boot limits relocation for AM62 SIP to the lower part of RAM to avoid overwriting reserved top-of-RAM areas. [2][3]
  • Trusted Firmware-A / OP‑TEE reserved at top of RAM: On AM62 SIP the top-of-RAM is reserved for TF‑A and OP‑TEE (examples show OP‑TEE load/entry at high addresses such as 0x9E80_0000 in TI examples). Do not place U‑Boot or large relocations into the top-of-RAM reserved region. [3][4]

Practical implication: for AM62SIP with 512 MiB you should place SPL/U-Boot binaries at/around 0x8008_0000 and keep relocation and heap/stack allocations below the 0x8200_0000 region (and avoid the top-of-RAM area reserved for TF‑A/OP‑TEE). References: U‑Boot AM62 docs (A53 SPL map), U‑Boot R5 SPL memory map, and patch notes about AM62 SIP reserved top-of-RAM. [1][2][3][4]

Sources
[1] U‑Boot AM62Px / A53 SPL memory map (TEXT_BASE = 0x80080000).
[2] U‑Boot AM62 R5 SPL memory map (shows 0x82000000 boundary / SPL regions).
[3] U‑Boot patch/notes: "Avoid overwriting reserve mem for AM62 SIP" (512MB, top-of-RAM reserved for TF‑A/OPTEE).
[4] TI/OP‑TEE example device-tree / OP‑TEE load/entry addresses (example 0x9E800000).


🌐 Web query:

U-Boot am62x-hs BL32_BASE TZDRAM correct address documentation

💡 Result:

Short answer — for the AM62x (HS) U-Boot/TFA + OP-TEE layout the documented BL32/TZDRAM placement is at 0x9e800000 (TZDRAM/BL32 start) with a ~24 MiB OP-TEE region up to 0xa0000000 (end of a 512 MiB DDR). [1][2]

References

  • U-Boot AM62x platform memory layout showing BL32_BASE = 0x9e800000 and 24 MiB OP-TEE region. [1]
  • U-Boot/AM62A platform docs (layout diagram including BL32_BASE = 0x9e800000, OP-TEE size and DDR end). [2]

If you want I can fetch the exact .config/Makefile lines or show how to set TFA/OP-TEE build args (e.g. BL32_BASE / CFG_TZDRAM_START) for the am62x-hs defconfig.


🏁 Script executed:

# Check if there's any additional documentation or comments about this in the repo
rg "BL32_BASE|TZDRAM_START" -B 3 -A 3 --type txt --type md

Repository: armbian/build

Length of output: 39


BL32_BASE and CFG_TZDRAM_START are set to incorrect addresses for the 512 MB configuration.

The current values place OP-TEE (BL32) at 0x80080000, which is the U-Boot region, causing a memory conflict. According to TI's U-Boot AM62x memory layout documentation, OP-TEE must be placed at the top of DDR:

  • Correct BL32_BASE: 0x9E800000 (not 0x80080000)
  • Correct CFG_TZDRAM_START: 0x9E800000 (not 0x80080000)
  • OP-TEE size: ~24 MiB, ending at 0xA0000000 (end of 512 MiB DDR)
  • PRELOADED_BL33_BASE at 0x82000000 is correct for U-Boot

Update ATF_EXTRA_ARGS and OPTEE_ARGS to use the correct addresses from TI's reference layout.

🤖 Prompt for AI Agents
In config/boards/sk-am62-sip.conf around lines 18 to 20, BL32_BASE and
CFG_TZDRAM_START are wrongly set to 0x80080000 (conflicting with U-Boot); update
ATF_EXTRA_ARGS to set BL32_BASE=0x9E800000 while keeping
PRELOADED_BL33_BASE=0x82000000, and update OPTEE_ARGS to set
CFG_TZDRAM_START=0x9E800000 (retain CFG_TEE_CORE_LOG_LEVEL and OP-TEE platform
unchanged); ensure these addresses place OP-TEE at the top of 512 MiB DDR (~24
MiB region ending at 0xA0000000) per TI's AM62x layout.

Copy link

@bryanbrattlof bryanbrattlof Dec 15, 2025

Choose a reason for hiding this comment

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

During U-Boot's bootup process it will relocate itself to the highest address backed by DDR (up to a max of 2GB) to maximize the contiguous space for whatever it's booting. Unfortunately where OPTEE and TFA are by default for most K3 SoCs this relocation will overwrite these memory regions if the board only has 512MB of DDR space.

The easiest solution is to move OPTEE and TFA to the bottom of DDR and out of the way of U-Boot during its bootup process.

All that to say, these values are correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bryanbrattlof Thank you for the detailed explanation! I completely misunderstood the memory layout requirements for 512MB DDR boards. I was incorrectly assuming the standard high-memory layout should be used, but you're absolutely right that U-Boot's relocation behavior necessitates placing OP-TEE and TFA at the bottom of DDR on these constrained boards to prevent them from being overwritten.

The values are indeed correct:

  • BL32_BASE=0x80080000 and CFG_TZDRAM_START=0x80080000 for OP-TEE at low memory
  • PRELOADED_BL33_BASE=0x82000000 for U-Boot
  • This arrangement keeps them out of U-Boot's relocation path

I apologize for the incorrect analysis and appreciate you taking the time to clarify this important detail about K3 SoC memory management!


✏️ Learnings added
Learnt from: bryanbrattlof
Repo: armbian/build PR: 9081
File: config/boards/sk-am62-sip.conf:18-20
Timestamp: 2025-12-15T23:26:34.998Z
Learning: For TI K3 SoC boards with only 512MB of DDR (like SK-AM62-SIP), OP-TEE (BL32) and TFA must be placed at the bottom of DDR (e.g., BL32_BASE=0x80080000, CFG_TZDRAM_START=0x80080000) rather than the standard high-memory layout. This is necessary because U-Boot relocates itself to the highest DDR address (up to 2GB max) during bootup, which would overwrite OP-TEE/TFA if they were placed at the top of the 512MB DDR region. PRELOADED_BL33_BASE is typically set to 0x82000000 in this configuration.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).

Learnt from: Grippy98
Repo: armbian/build PR: 8624
File: config/boards/sk-am62p.conf:8-8
Timestamp: 2025-09-12T21:44:09.061Z
Learning: For TI K3 family boards in Armbian, BOOT_FDT_FILE uses .dts extension (not .dtb) as the standard convention. The build system handles this correctly by automatically compiling .dts to .dtb during kernel build and using the BOOT_FDT_FILE value directly in bootloader configurations.

Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.719Z
Learning: In Armbian board configuration files (config/boards/*.conf, *.csc, etc.), do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__* functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Board-specific kernel modifications cause inconsistency in kernel packages published to the apt repository because boards within a family share the same kernel packages. Kernel configuration changes must be made in the appropriate kernel config file (e.g., config/kernel/linux-*-*.config) or in family configuration files (config/sources/families/*.conf, *.inc) instead.

Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files, .wip and .conf extensions require BOARD_MAINTAINER to be present and contain at least one maintainer. If no maintainer is present, the board support rules state it does not qualify for standard support and must be moved to community support (.csc extension). This is documented at https://docs.armbian.com/User-Guide_Board-Support-Rules/

Learnt from: HackingGate
Repo: armbian/build PR: 8665
File: config/boards/photonicat2.csc:4-4
Timestamp: 2025-10-26T10:41:35.118Z
Learning: In the Armbian build system, rk3576 boards consistently use BOARDFAMILY="rk35xx" for both vendor and edge kernel targets. The rk35xx family configuration sources rockchip64_common.inc, which provides edge and current kernel branch definitions, making these branches available even though they're not defined directly in rk35xx.conf.

Learnt from: Grippy98
Repo: armbian/build PR: 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.

Learnt from: pyavitz
Repo: armbian/build PR: 8388
File: config/boards/kickpik2b.conf:1-14
Timestamp: 2025-07-17T04:12:33.125Z
Learning: In the Armbian build system, board configuration files (*.conf) follow a project standard of not including shebang lines, even though they contain bash code and are sourced by the build system. This is an established Armbian convention that individual contributors are expected to follow, and changes to this standard would require broader project maintainer approval.

Learnt from: djurny
Repo: armbian/build PR: 8287
File: config/bootscripts/boot-sunxi.cmd:38-44
Timestamp: 2025-06-12T21:14:36.024Z
Learning: In config/bootscripts/boot-sunxi.cmd the unconditional "+1" increment in func_align_addr_next (when align_overlap_oboe_avoidance="on") is intentional. It compensates for a known off-by-one error in U-Boot where the end address is calculated as start+size instead of start+size-1, so the extra page prevents overlap. This behavior should not be “optimized away”.

Learnt from: djurny
Repo: armbian/build PR: 8166
File: config/bootscripts/boot-mvebu.cmd:38-50
Timestamp: 2025-05-07T20:49:40.969Z
Learning: The 4KB (0x1000) boundary used for device tree size calculations in U-Boot scripts is specific to how the `fdt resize` command works internally and should remain hardcoded rather than being tied to adjustable alignment variables.

Learnt from: glneo
Repo: armbian/build PR: 8913
File: config/sources/families/include/k3_common.inc:57-60
Timestamp: 2025-11-11T20:56:20.303Z
Learning: In config/sources/families/include/k3_common.inc, the OP-TEE build command at line 59 should be updated in a future PR to explicitly set CROSS_COMPILE64=aarch64-linux-gnu- and CROSS_COMPILE32=arm-linux-gnueabihf- instead of relying on OP-TEE's internal defaults, for better clarity and maintainability. User glneo agreed to address this in a separate PR.

Learnt from: pyavitz
Repo: armbian/build PR: 9058
File: patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch:28-67
Timestamp: 2025-12-13T11:39:08.046Z
Learning: In the Armbian build system for SpacemiT U-Boot patches (patch/u-boot/legacy/u-boot-spacemit-k1/), alignment with mainline U-Boot behavior is prioritized. For example, in boot mode handling, leaving devnum unchanged in the default case (when devtype is cleared) follows mainline conventions rather than explicitly clearing it to handle edge cases.

Learnt from: djurny
Repo: armbian/build PR: 8287
File: config/bootscripts/boot-sunxi.cmd:13-23
Timestamp: 2025-06-12T21:08:11.318Z
Learning: In config/bootscripts/boot-sunxi.cmd the variable name `align_overlap_oboe_avoidance` intentionally uses the acronym “OBOE” (Off-By-One Error); it is not a typo and should not be renamed.

Learnt from: libiunc
Repo: armbian/build PR: 8033
File: config/kernel/linux-starfive2-vendor.config:43-43
Timestamp: 2025-03-31T12:57:13.880Z
Learning: For StarFive2 platform kernel configurations, maintain alignment with vendor-provided configurations rather than modifying security settings like SECCOMP. This ensures hardware compatibility as intended by the manufacturer.

Learnt from: pyavitz
Repo: armbian/build PR: 9058
File: patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch:17-25
Timestamp: 2025-12-13T11:45:02.422Z
Learning: In the SpacemiT U-Boot patches for Armbian (patch/u-boot/legacy/u-boot-spacemit-k1/), the environment variable `devnum` is set to the device name string (e.g., "mmc", "nvme") rather than a numeric index, and `distro_bootpart` holds the partition number. This implementation aligns with mainline U-Boot conventions for the SpacemiT platform and has been verified to work correctly by the maintainer.

Learnt from: EvilOlaf
Repo: armbian/build PR: 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: EvilOlaf
Repo: armbian/build PR: 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 API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.

Copy link
Collaborator

Choose a reason for hiding this comment

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

can we document this here with a comment, and/or show the calculations?

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Choose a reason for hiding this comment

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

Yeah +1 with some public facing notes about why we're moving these addresses. There is some notes in the board specific sections in U-Boot so maybe a link to that will help?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comments with that u-boot docs link in 10f4d10

@@ -0,0 +1,20 @@
# Texas Instruments AM62SIP quad core 2xGBE OSPI HDMI
Copy link
Collaborator

Choose a reason for hiding this comment

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

config/boards/sk-am62-lp.conf mentions 2GB, Im assuming this means RAM.
Does this board have variable quantities or some other reason to not specify the quantity on this line?
It may also help to explain the oddness of line 18.
I understand that you have all the documentation and are authoritative, but it might be nice for if somebody comes by here scratching their head.

@coderabbitai

Copy link
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch the full content of both board configuration files
echo "=== sk-am62-sip.conf ==="
cat config/boards/sk-am62-sip.conf
echo ""
echo "=== sk-am62-lp.conf ==="
cat config/boards/sk-am62-lp.conf

Length of output: 1244


@tabrisnet Great catch! You're absolutely right on both points.

According to the PR objectives, the SK-AM62-SIP board has 512 MB of RAM packaged on the SoC (fixed quantity). Per Armbian board configuration standards (see retrieved learning), the first line comment should include RAM specifications. So line 1 should be updated to:

# Texas Instruments AM62SIP quad core 512MB 2xGBE OSPI HDMI

Regarding line 18, the "oddness" is the ATF_EXTRA_ARGS with manually defined memory regions (BL32_BASE=0x80080000 PRELOADED_BL33_BASE=0x82000000). This is unique to sk-am62-sip and not present in sk-am62-lp.conf. According to the PR objectives, this is necessary because the fixed 512 MB RAM requires manually defined ATF and OP-TEE memory regions. An inline comment would help clarify this:

# Manual memory region definitions required for fixed 512MB RAM
ATF_EXTRA_ARGS="BL32_BASE=0x80080000 PRELOADED_BL33_BASE=0x82000000"
OPTEE_ARGS="CFG_TEE_CORE_LOG_LEVEL=1 CFG_TZDRAM_START=0x80080000"

This would make it immediately clear to future maintainers why this board config differs from the similar sk-am62-lp and sk-am62b boards.


🧠 Learnings used
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).

Learnt from: pyavitz
Repo: armbian/build PR: 8388
File: config/boards/kickpik2b.conf:1-14
Timestamp: 2025-07-17T04:12:33.125Z
Learning: In the Armbian build system, board configuration files (*.conf) follow a project standard of not including shebang lines, even though they contain bash code and are sourced by the build system. This is an established Armbian convention that individual contributors are expected to follow, and changes to this standard would require broader project maintainer approval.

Learnt from: djurny
Repo: armbian/build PR: 8287
File: config/bootscripts/boot-sunxi.cmd:13-23
Timestamp: 2025-06-12T21:08:11.318Z
Learning: In config/bootscripts/boot-sunxi.cmd the variable name `align_overlap_oboe_avoidance` intentionally uses the acronym “OBOE” (Off-By-One Error); it is not a typo and should not be renamed.

Learnt from: djurny
Repo: armbian/build PR: 8287
File: config/bootscripts/boot-sunxi.cmd:38-44
Timestamp: 2025-06-12T21:14:36.024Z
Learning: In config/bootscripts/boot-sunxi.cmd the unconditional "+1" increment in func_align_addr_next (when align_overlap_oboe_avoidance="on") is intentional. It compensates for a known off-by-one error in U-Boot where the end address is calculated as start+size instead of start+size-1, so the extra page prevents overlap. This behavior should not be “optimized away”.

Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files, .wip and .conf extensions require BOARD_MAINTAINER to be present and contain at least one maintainer. If no maintainer is present, the board support rules state it does not qualify for standard support and must be moved to community support (.csc extension). This is documented at https://docs.armbian.com/User-Guide_Board-Support-Rules/

Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.719Z
Learning: In Armbian board configuration files (config/boards/*.conf, *.csc, etc.), do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__* functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Board-specific kernel modifications cause inconsistency in kernel packages published to the apt repository because boards within a family share the same kernel packages. Kernel configuration changes must be made in the appropriate kernel config file (e.g., config/kernel/linux-*-*.config) or in family configuration files (config/sources/families/*.conf, *.inc) instead.

Learnt from: rpardini
Repo: armbian/build PR: 8718
File: config/boards/thinkpad-x13s.conf:185-185
Timestamp: 2025-10-05T19:31:16.930Z
Learning: In config/boards/thinkpad-x13s.conf, the x13s_obtain_firmware() function is reference-only utility code that never runs. It is kept for documentation purposes. Updates to firmware must be done at the armbian/firmware repository. TODO comments in this function are intentional reminders and should not be flagged.

Learnt from: EvilOlaf
Repo: armbian/build PR: 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: EvilOlaf
Repo: armbian/build PR: 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 API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.

Learnt from: glneo
Repo: armbian/build PR: 8913
File: config/sources/families/include/k3_common.inc:57-60
Timestamp: 2025-11-11T20:56:20.303Z
Learning: In config/sources/families/include/k3_common.inc, the OP-TEE build command at line 59 should be updated in a future PR to explicitly set CROSS_COMPILE64=aarch64-linux-gnu- and CROSS_COMPILE32=arm-linux-gnueabihf- instead of relying on OP-TEE's internal defaults, for better clarity and maintainability. User glneo agreed to address this in a separate PR.

Choose a reason for hiding this comment

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

I don't think any other DDR densities besides 512MB have been packaged in with the SIP parts today but yeah adding some docs about this will help. This also affects any board with <1GB of DRAM so could be a good note for people trying to bring up a new board.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this board have variable quantities or some other reason to not specify the quantity on this line?

Good catch, that was an oversight.

It may also help to explain the oddness of line 18.

Both points addressed in 10f4d10

@github-actions
Copy link
Contributor

🚫 Missing required board assets

This PR adds new board configuration(s). Required assets must already exist in github/armbian/armbian.github.io.
They are required by Armbian Imager to ensure all boards are displayed with proper images.

  • Board images: board-images/<board>.png (1920x1080 px transparent)
  • Vendor logos: board-vendor-logos/<vendor>-logo.png (512x512 px transparent)

Missing items

  • Board image missing for sk-am62-lp

    • Expected: board-images/sk-am62-lp.png
    • Fix: add the file to armbian/armbian.github.io (folder board-images/)
  • Board image missing for sk-am62-sip

    • Expected: board-images/sk-am62-sip.png
    • Fix: add the file to armbian/armbian.github.io (folder board-images/)

Once the missing files are added (or a PR is opened in armbian/armbian.github.io), re-run this check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

02 Milestone: First quarter release Hardware Hardware related like kernel, U-Boot, ... Needs review Seeking for review size/small PR with less then 50 lines

Development

Successfully merging this pull request may close these issues.

3 participants