JP7.0/7.1 build - #361
Conversation
There was a problem hiding this comment.
Pull request overview
This PR restructures the JetPack workspace setup/build flow to support JP7.0/7.1 and to consolidate source syncing into a single sync-sources.sh driven by per-version source lists (moving source URLs toward GitLab).
Changes:
- Add JP7 (7.0/7.1) support and normalize JetPack handling via
scripts/setup-common. - Replace per-version
source_sync_*.shusage with a singlescripts/sync-sources.shthat readsscripts/sources_*.x. - Update build/apply scripts and kernel Makefiles to align with the new source layout and toolchains.
Reviewed changes
Copilot reviewed 34 out of 44 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| setup_workspace.sh | Uses setup-common, installs toolchains, and syncs sources via unified sync-sources.sh. |
| scripts/setup-common | Adds JP7 handling, version persistence, and introduces version_lt(). |
| scripts/sync-sources.sh | Reads per-JP source lists and adds GitLab/protocol fallback logic. |
| scripts/sources_7.x | New JP7 source list for unified syncing. |
| scripts/sources_6.x | New JP6 source list for unified syncing. |
| scripts/sources_5.x | New JP5 source list for unified syncing. |
| scripts/sources_4.x | New JP4 source list for unified syncing. |
| build_all.sh | Updates cross-compile selection and build flow for normalized source dirs. |
| apply_patches.sh | Refactors patch application/reset flow for the new versioning/layout. |
| kernel/kernel-noble-src/Makefile | Adds a kernel Makefile for JP7/noble kernel source tree. |
| kernel/kernel-jammy-src/Makefile | Removes explicit ARCH=arm64 usage (now relies on environment). |
| nvidia-oot/Makefile | Adds NV_OOT_IVC_EXT_SKIP_BUILD to the oot build invocation. |
| kernel/realsense/d4xx.c | Adds multiple kernel-version compatibility adjustments. |
| nvidia-oot/7.0/, nvidia-oot/7.1/ | Adds JP7 patch set files (7.1 referencing 7.0 where applicable). |
| kernel/kernel-noble-src/7.0/, kernel/kernel-noble-src/7.1/ | Adds JP7 kernel patch sets. |
| hardware/nvidia/t23x/nv-public/7.x/* | Adds JP7 overlay patch enabling D4XX dtbos. |
| hardware/nvidia/platform/t19x/galen/kernel-dts/* | Adds/adjusts JP4/JP5 DTS patch sets for D4XX. |
Comments suppressed due to low confidence (2)
scripts/sync-sources.sh:190
- The tag-existence check is broken:
if [ -n $(git ... tag -l "^$TAG\$" 2>&1 >/dev/null) ]; thenredirects stdout to/dev/nullinside the command substitution, so the substitution is always empty and the test becomes effectively always-true/always-wrong. This can lead to attempting to checkout a non-existent tag and masking the real failure mode. Useif [ -n "$(git -C "$LDK_SOURCE_DIR" tag -l "^$TAG$" )" ]; thenorgit ... tag -l ... | grep -q .instead.
scripts/sync-sources.sh:65 SOURCE_INFO=$(cat scripts/sources_$JETPACK_VERSION)assumesJETPACK_VERSIONis set and that the corresponding sources file exists; if this script is invoked directly (or from a different working directory), it will fail with a confusing error. Consider validatingJETPACK_VERSIONand the file path up-front (with a clear message), or accept a CLI option to select the sources list instead of relying on an external environment variable.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # set JP4 devicetree | ||
| if [[ "$JETPACK_VERSION" == "4.6.1" ]]; then | ||
| if [[ "$JETPACK_VERSION" == "4.x" ]]; then | ||
| JP5_D4XX_DTSI="tegra194-camera-d4xx.dtsi" | ||
| fi | ||
| if [[ "$JETPACK_VERSION" == "6.x" ]]; then | ||
| D4XX_SRC_DST=nvidia-oot | ||
| else | ||
| if version_lt "$JETPACK_VERSION" "6.0"; then | ||
| D4XX_SRC_DST=kernel/nvidia | ||
| else | ||
| D4XX_SRC_DST=nvidia-oot | ||
| fi |
There was a problem hiding this comment.
The version_lt "$JETPACK_VERSION" "6.0" checks will misbehave because setup-common normalizes JETPACK_VERSION to 4.x/5.x/6.x/7.x. That can break patch destination selection (D4XX_SRC_DST) and later logic (e.g., the nvethernetrm symlink block). Use the numeric version for numeric comparisons, or change these checks to explicit string comparisons against the normalized family value.
|
|
||
| ifdef KERNEL_OUTPUT | ||
| O_OPT := O=$(KERNEL_OUTPUT) | ||
| $(mkdir -p $(KERNEL_OUTPUT)) |
There was a problem hiding this comment.
$(mkdir -p $(KERNEL_OUTPUT)) is a no-op in make (it expands an undefined variable named mkdir -p ...), so the output directory isn't actually created here. If the intent is to ensure $(KERNEL_OUTPUT) exists, use $(shell mkdir -p $(KERNEL_OUTPUT)) or create the directory inside the kernel:/install: recipes before invoking the kernel build.
| $(mkdir -p $(KERNEL_OUTPUT)) | |
| $(shell mkdir -p $(KERNEL_OUTPUT)) |
| function version_lt { | ||
| IFS='.' read -r -a v1 <<< "$1" | ||
| IFS='.' read -r -a v2 <<< "$2" | ||
| for i in 0 1 2; do | ||
| [[ v1[i] -lt v2[i] ]] && return 0 | ||
| [[ v1[i] -gt v2[i] ]] && return 1 | ||
| done | ||
| return 1 | ||
| } |
There was a problem hiding this comment.
version_lt() is currently broken: [[ v1[i] -lt v2[i] ]] / [[ v1[i] -gt v2[i] ]] compares literal strings instead of array elements (missing ${...}), which will lead to test errors and incorrect branching. Also, this helper assumes numeric dot-separated versions, but elsewhere JETPACK_VERSION is normalized to values like 7.x/6.x, which will not parse as integers. Suggest fixing the function to use ${v1[i]:-0}/${v2[i]:-0} (or (( ... ))), and ensure callers pass the numeric version (e.g., 7.1) rather than the normalized JETPACK_VERSION (7.x).
| # copy Makefile for jp6 | ||
| if [[ "$JETPACK_VERSION" == "6.x" ]]; then | ||
| cp ./nvidia-oot/Makefile "sources_$1/" | ||
| cp ./kernel/kernel-jammy-src/Makefile "sources_$1/kernel" | ||
| if ! version_lt "$JETPACK_VERSION" "6.x"; then | ||
| cp ./nvidia-oot/Makefile "sources_$JETPACK_VERSION/" | ||
| cp ./$KERNEL_DIR/Makefile "sources_$JETPACK_VERSION/kernel/" | ||
| fi |
There was a problem hiding this comment.
This uses version_lt "$JETPACK_VERSION" "6.x", but JETPACK_VERSION is normalized to values like 7.x/6.x by setup-common, and version_lt() expects numeric dot-separated versions. As written, this condition will error or behave incorrectly, which will break the JP6/JP7 Makefile copy logic. Consider comparing the numeric version (e.g., 6.0, 7.1) or using explicit string checks against the normalized JETPACK_VERSION (e.g., [[ $JETPACK_VERSION == 6.x || $JETPACK_VERSION == 7.x ]]).
| if version_lt "$JETPACK_VERSION" "6.0"; then | ||
| #JP4/5 | ||
| cd $SRCS/$KERNEL_DIR | ||
| make O=$TEGRA_KERNEL_OUT tegra_defconfig | ||
| if [[ "$DEVDBG" == "1" ]]; then | ||
| scripts/config --file $TEGRA_KERNEL_OUT/.config --enable DYNAMIC_DEBUG | ||
| fi | ||
| make O=$TEGRA_KERNEL_OUT -j${NPROC} | ||
| make O=$TEGRA_KERNEL_OUT modules_install INSTALL_MOD_PATH=$KERNEL_MODULES_OUT |
There was a problem hiding this comment.
version_lt "$JETPACK_VERSION" "6.0" will not work reliably because setup-common rewrites JETPACK_VERSION to 4.x/5.x/6.x/7.x. Passing 7.x (or 6.x) into a numeric comparator will cause test failures and incorrect build paths. Use the numeric version variable from setup-common for numeric comparisons, and reserve the normalized JETPACK_VERSION only for selecting the sources_*.x directory/toolchain.
| make ARCH=arm64 modules | ||
| make ARCH=arm64 dtbs | ||
| make modules | ||
| ! version_lt "$JETPACK_VERSION" "7.0" || make dtbs |
There was a problem hiding this comment.
! version_lt "$JETPACK_VERSION" "7.0" || make dtbs has two issues: (1) it again passes normalized values like 7.x/6.x into a numeric comparator, and (2) the ! ... || ... construct is hard to reason about and can easily invert the intended logic. Recommend switching to a clear conditional using the numeric version (e.g., if version_lt "$version" "7.0"; then make dtbs; fi) or an explicit JETPACK_VERSION family check.
| ! version_lt "$JETPACK_VERSION" "7.0" || make dtbs | |
| if [[ "$JETPACK_VERSION" == 5.* || "$JETPACK_VERSION" == 6.* ]]; then | |
| make dtbs | |
| fi |
One sync-sources.sh script
Source URLs excluded from script
JP version to build given once
Source URLs modified to GitLab (25MiB transfer)
Generic source folders for each JP version