Skip to content

Conversation

@pyavitz
Copy link
Collaborator

@pyavitz pyavitz commented Dec 12, 2025

SpacemiT MusePi Pro (https://paste.armbian.com/cijebezabo.yaml)

  • Update linux-6.6.y patching
  • Move edge to 6.18.y
  • Fixup overlay support in both linux patching directories

NOTE: Needs investigation.

  • LCD/DSI support has been disabled in the k1-musepi-pro.dts as it causes the unit to lock up during boot.
  • CAM driver support has been disabled on EDGE. It fails to build.
# Disable K1X Cam support
# CONFIG_SPACEMIT_K1X_CAMERA_V2=y
# CONFIG_SPACEMIT_K1X_VI_V2=y
# CONFIG_SPACEMIT_K1X_VI_IOMMU=y
# CONFIG_SPACEMIT_K1X_ISP_V2=y
# CONFIG_SPACEMIT_K1X_CPP_V2=y
# CONFIG_SPACEMIT_K1X_SENSOR_V2=y

TODO

  • Replace EDGE with mainline linux
  • Add legacy branch that pulls kernel source from Bianbu

To boot via eMMC/NVMe SPI needs to be updated.
https://bianbu-linux.spacemit.com/en/device/boot/#norblk-device-flashing

cat <<EOF > "partition_2M.json"
{
  "version": "1.0",
  "format": "mtd",
  "partitions": [
    {
      "name": "bootinfo",
      "offset": "0",
      "size": "128K",
      "image": "bootinfo_spinor.bin"
    },
    {
      "name": "fsbl",
      "offset": "128K",
      "size": "256K",
      "image": "FSBL.bin"
    },
    {
      "name": "env",
      "offset": "384K",
      "size": "64K"
    },
    {
      "name": "opensbi",
      "offset": "448K",
      "size": "192K",
      "image": "fw_dynamic.itb"
    },
    {
      "name": "uboot",
      "offset": "640K",
      "size": "-",
      "image": "u-boot.itb"
    }
  ]
}
EOF

RUN:

fastboot stage FSBL.bin
fastboot continue
sleep .75
fastboot stage u-boot.bin
fastboot continue
sleep .75
fastboot flash mtd partition_2M.json
sleep .75
fastboot flash bootinfo bootinfo_spinor.bin
sleep .75
fastboot flash fsbl FSBL.bin
sleep .75
fastboot flash env u-boot-env-default.bin
sleep .75
fastboot flash opensbi fw_dynamic.itb
sleep .75
fastboot flash uboot u-boot.itb
sleep .75
fastboot reboot

OUTPUT:

Output:
Sending 'FSBL.bin' (183 KB)                        OKAY [  0.031s]
Finished. Total time: 0.038s
Resuming boot                                      OKAY [  0.007s]
Finished. Total time: 0.007s
Sending 'u-boot.bin' (2254 KB)                     OKAY [  0.072s]
Finished. Total time: 0.072s
Resuming boot                                      OKAY [  0.000s]
Finished. Total time: 0.000s
< waiting for any device >
Warning: skip copying mtd image avb footer (mtd partition size: 0, mtd image size: 591).
Sending 'mtd' (0 KB)                               OKAY [  0.010s]
Writing 'mtd'                                      OKAY [  0.220s]
Finished. Total time: 0.317s
Sending 'bootinfo' (0 KB)                          OKAY [  0.009s]
Writing 'bootinfo'                                 OKAY [  0.130s]
Finished. Total time: 0.155s
Sending 'fsbl' (183 KB)                            OKAY [  0.014s]
Writing 'fsbl'                                     OKAY [  2.531s]
Finished. Total time: 2.560s
Sending 'env' (16 KB)                              OKAY [  0.010s]
Writing 'env'                                      OKAY [  0.242s]
Finished. Total time: 0.267s
Sending 'opensbi' (133 KB)                         OKAY [  0.012s]
Writing 'opensbi'                                  OKAY [  1.896s]
Finished. Total time: 1.924s
Sending 'uboot' (2254 KB)                          OKAY [  0.086s]
Writing 'uboot'                                    OKAY [ 29.520s]
Finished. Total time: 29.622s
Rebooting                                          OKAY [  0.000s]
Finished. Total time: 0.151s

Summary by CodeRabbit

  • New Features

    • MusePi Pro board support with full hardware configuration and firmware install prompts
    • Device tree overlays to enable/disable QSPI and add I2C4/PWM7/SPI3/UART5
    • UEFI, Syslinux and script-based boot methods added
    • New LCD panel support (TC358762XBG)
  • Kernel Improvements

    • Upgraded to Linux 6.18 with expanded sensor, video, SPI and I/O driver coverage
    • K1 CPU max frequency increased to 1.8 GHz

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

Signed-off-by: Patrick Yavitz <pyavitz@gmail.com>
Signed-off-by: Patrick Yavitz <pyavitz@gmail.com>
Signed-off-by: Patrick Yavitz <pyavitz@gmail.com>
Signed-off-by: Patrick Yavitz <pyavitz@gmail.com>
Signed-off-by: Patrick Yavitz <pyavitz@gmail.com>
Signed-off-by: Patrick Yavitz <pyavitz@gmail.com>
Signed-off-by: Patrick Yavitz <pyavitz@gmail.com>
Signed-off-by: Patrick Yavitz <pyavitz@gmail.com>
Signed-off-by: Patrick Yavitz <pyavitz@gmail.com>
@github-actions github-actions bot added the 02 Milestone: First quarter release label Dec 12, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 12, 2025

Walkthrough

Adds a MusePi Pro board config and device tree, upgrades SpacemiT kernel targets (6.17→6.18), expands kernel drivers/overlay build support, applies multiple spacemit kernel and U‑Boot patches, and introduces board-specific U‑Boot and BSP post‑install hooks and firmware handling.

Changes

Cohort / File(s) Summary
Board config: MusePi Pro
config/boards/musepipro.conf
New board definition: BOARD_* variables, U-Boot target/boot settings, package list, plus two post_* hooks to enable extra U-Boot configs and install board firmware/modules at BSP tweak time.
Kernel defconfig & family sources
config/kernel/linux-spacemit-edge.config
config/sources/families/spacemit.conf
Edge kernel bumped to 6.18, large defconfig changes (IIO/sensors, display/TFT, regulators, modularization, ext4 enabled, netfilter adjustments) and UBOOT_TARGET_MAP / kernel branch/version updates.
New board device trees & panel DTIs
patch/kernel/archive/spacemit-6.18/dt/k1-musepi-pro.dts
.../dt/lcd_tc358762xbg_dpi_800x480.dtsi
patch/kernel/archive/spacemit-6.6/dt/k1-musepi-pro.dts
Adds comprehensive MusePi Pro DTS (power, memory, peripherals, pinctrl, displays, cameras) and a new LCD panel dtsi with DSI timings/commands.
DTB/overlay build system & overlay files
patch/kernel/archive/spacemit-6.18/006-general-spacemit-overlays.patch
patch/kernel/archive/spacemit-6.18/007-device-tree-overlay-support.patch
patch/kernel/archive/spacemit-6.18/dt/Makefile
patch/kernel/archive/spacemit-6.18/overlay/*.dts
Adds overlay build/flattening support, DTB install changes, DTC flags, .scr asset support, and overlay DTBOs (disable-qspi, i2c4/pwm7/spi3/uart5) plus overlay Makefile entries.
Kernel patching metadata
patch/kernel/archive/spacemit-6.18/0000.patching_config.yaml
patch/kernel/archive/spacemit-6.6/0000.patching_config.yaml
Updates patching_config.yaml entries to new spacemit kernel baselines (6.18 and 6.6 archives respectively).
Kernel patches: rtl8852bs / cpufreq / spidev
patch/kernel/archive/spacemit-6.16/001-rtl8852bs-...patch
.../002-Max-freq-limitation-1.8GHz.patch
.../003-Add-spacemit-k1x-spi-support.patch
Disables an RTL8852BS debug flag and log, raises K1 max freq limit (1.6→1.8 GHz) and simplifies cpufreq verification, and adds k1x-spi entries to spidev driver tables.
Kernel patches: SHA256 guards & overlay support
patch/kernel/archive/spacemit-6.18/005-Fixup-rtl8852bs-sha256.patch
Adds kernel-version guards around SHA256/HMAC functions for compatibility with older kernels.
Kernel dt removals/forward ports
patch/kernel/archive/spacemit-6.17/004-SpacemiT-K1-X-Forward-port-6.6-DTSI.patch
Deletes a forward-ported k1-x.dtsi DTSI (removes many DT changes previously applied).
Additional board DTs (BananaPi F3) and overlays
patch/kernel/archive/spacemit-6.6/dt/...
patch/kernel/archive/spacemit-6.6/overlay/...
Adds BananaPi F3 DTS and overlay infra; also parallels MusePi Pro DT/overlay additions in 6.6 archive.
U-Boot legacy patches (K1-X)
patch/u-boot/legacy/u-boot-spacemit-k1/001-MBR-support.patch
.../003-SpacemiT-K1X-Fixups.patch
.../005-Add-uefi-syslinux-and-script-support.patch
MBR partition handling/logging, rename env vars (bootfs_part→distro_bootpart, boot_device/boot_devnum→devtype/devnum), add address macros (KERNEL_ADDR_R, RAMDISK_ADDR_R, FDT_ADDR_R, FDTOVERLAY_ADDR_R), enable FDT overlay support and add UEFI/SYSLINUX/script boot paths.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Pay extra attention to:
    • config/kernel/linux-spacemit-edge.config — many defconfig edits across subsystems.
    • k1-musepi-pro.dts and related pinctrl/regulator/clock wiring.
    • Overlay build system changes in scripts/Makefile.* and dt/Makefile for build regressions.
    • U-Boot env/address macro changes and boot flow renames for backwards compatibility.

Possibly related PRs

Suggested reviewers

  • igorpecovnik
  • brentr
  • EvilOlaf
  • rpardini

"I nibble bugs and bits with care and cheer,
New DTs and overlays spring up here,
From boot to kernel, wires and pins in tune,
I hop — MusePi Pro will boot by noon! 🐇✨"

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add board SpacemiT MusePi Pro and update linux patching' directly describes the main changes: adding a new board (MusePi Pro) and updating kernel patching. It is concise and specific, clearly indicating the primary objectives without vague language.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • SPACEMIT-001: Request failed with status code 404

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/large PR with 250 lines or more Needs review Seeking for review Hardware Hardware related like kernel, U-Boot, ... Patches Patches related to kernel, U-Boot, ... labels Dec 12, 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: 11

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
patch/kernel/archive/spacemit-6.6/0000.patching_config.yaml (1)

1-1: Update stale file comment to match actual file path.

Line 1 references "spacemit-legacy-6.1" but the actual file is for "spacemit-6.6". This appears to be a copy-paste artifact from a template that should be corrected for clarity.

-config: # This is file 'patch/kernel/spacemit-legacy-6.1/0000.patching_config.yaml'
+config: # This is file 'patch/kernel/archive/spacemit-6.6/0000.patching_config.yaml'
patch/u-boot/legacy/u-boot-spacemit-k1/005-Add-uefi-syslinux-and-script-support.patch (1)

337-359: Critical: Lines 348-359 look like invalid .env syntax (not var=value).
After the autoboot=...fi; assignment (ending Line 347), the subsequent nvme ... lines (Line 348+) are not prefixed by an environment variable name. If this file is parsed as a standard env text file, this will break env generation/import.

-autoboot=if test -e ${devtype} ${devnum}:${distro_bootpart} /extlinux/extlinux.conf; then \
+autoboot=if test -e ${devtype} ${devnum}:${distro_bootpart} /extlinux/extlinux.conf; then \
 		sysboot ${devtype} ${devnum}:${distro_bootpart} any 0x2000000 /extlinux/extlinux.conf; \
@@
-	fi;
-	nvme part; \
-	if test -e nvme 0:1 /extlinux/extlinux.conf; then \
+	fi; \
+	if test -e nvme 0:1 /extlinux/extlinux.conf; then \
 		sysboot nvme 0:1 any 0x2000000 /extlinux/extlinux.conf; \
@@
-	fi;
+	fi
🧹 Nitpick comments (12)
config/sources/families/spacemit.conf (1)

64-80: Consider quoting variables in write_uboot_platform for robustness.

While the current implementation works, several variables are used unquoted (e.g., ${2}boot0, $2) which could cause issues if device paths contain spaces. This is a minor robustness concern.

 write_uboot_platform() {
 	local device=${2}
-	if [ -b ${2}boot0 ]; then
+	if [ -b "${2}boot0" ]; then
 		echo "eMMC"
-		DEVICE=`ls /dev/mmcblk*boot0 | sed 's/^.....//'`
-		echo 0 > /sys/block/${DEVICE}/force_ro
+		DEVICE=$(ls /dev/mmcblk*boot0 | sed 's/^.....//')
+		echo 0 > "/sys/block/${DEVICE}/force_ro"
 		sleep .50
-		dd if="$1/bootinfo_emmc.bin" of="/dev/${DEVICE}" bs=512 conv=notrunc
-		dd if="$1/FSBL.bin" of="/dev/${DEVICE}" bs=512 seek=1 conv=notrunc
+		dd if="$1/bootinfo_emmc.bin" of="/dev/${DEVICE}" bs=512 conv=notrunc status=none
+		dd if="$1/FSBL.bin" of="/dev/${DEVICE}" bs=512 seek=1 conv=notrunc status=none
 	else
 		echo "SDCARD"
-		dd if="$1/bootinfo_emmc.bin" of=$2 bs=512 conv=notrunc
-		dd if="$1/FSBL.bin" of="$2" bs=512 seek=1 conv=notrunc
+		dd if="$1/bootinfo_emmc.bin" of="$2" bs=512 conv=notrunc status=none
+		dd if="$1/FSBL.bin" of="$2" bs=512 seek=1 conv=notrunc status=none
 	fi
-	dd if="$1/fw_dynamic.itb" of="$2" bs=512 seek=1280 conv=notrunc
-	dd if="$1/u-boot.itb" of="$2" bs=512 seek=2048 conv=notrunc
+	dd if="$1/fw_dynamic.itb" of="$2" bs=512 seek=1280 conv=notrunc status=none
+	dd if="$1/u-boot.itb" of="$2" bs=512 seek=2048 conv=notrunc status=none
 	sync
 }
patch/kernel/archive/spacemit-6.6/dt/k1-musepi-pro.dts (1)

280-284: Use "okay" instead of "ok" for status property.

While both values work functionally, "okay" is the standard spelling used throughout the kernel device tree bindings. Line 281 uses "ok".

 	dpi_panel: panel@0 {
-			status = "ok";
+			status = "okay";
 			compatible = "raspberrypi,dpi-panel";
 	};
patch/kernel/archive/spacemit-6.6/dt/Makefile (1)

14-15: Consider consolidating DTB entries for maintainability.

The k1-bananapi-f3.dtb and k1-musepi-pro.dtb entries are added separately from the main dtb-$(CONFIG_SOC_SPACEMIT_K1X) list (lines 3-11). While functional, consolidating them into the main list would improve readability.

 dtb-$(CONFIG_SOC_SPACEMIT_K1X) += k1-x_fpga.dtb k1-x_fpga_1x4.dtb k1-x_fpga_2x2.dtb k1-x_evb.dtb \
 				  k1-x_deb2.dtb k1-x_deb1.dtb k1-x_hs450.dtb k1-x_kx312.dtb \
 				  k1-x_MINI-PC.dtb k1-x_MUSE-N1.dtb k1-x_mingo.dtb \
 				  k1-x_MUSE-Pi.dtb k1-x_milkv-jupiter.dtb m1-x_milkv-jupiter.dtb \
 				  k1-x_MUSE-Book.dtb k1-x_lpi3a.dtb k1-x_MUSE-Card.dtb \
 				  k1-x_MUSE-Paper.dtb k1-x_MUSE-Paper-mini-4g.dtb \
 				  k1-x_baton-camera.dtb k1-x_FusionOne.dtb k1-x_orangepi-rv2.dtb \
 				  k1-x_ZT001H.dtb k1-x_uav.dtb k1-x_MUSE-Paper2.dtb \
-				  k1-x_bit-brick.dtb
+				  k1-x_bit-brick.dtb k1-bananapi-f3.dtb k1-musepi-pro.dtb
 obj-$(CONFIG_BUILTIN_DTB) += $(addsuffix .o, $(dtb-y))
 
-dtb-$(CONFIG_SOC_SPACEMIT_K1X) += k1-bananapi-f3.dtb
-dtb-$(CONFIG_SOC_SPACEMIT_K1X) += k1-musepi-pro.dtb
-
 subdir-y += overlay
patch/kernel/archive/spacemit-6.18/overlay/Makefile (1)

6-7: Minor: Inconsistent indentation.

Line 7 uses spaces for indentation while line 3-4 use tabs. Kernel Makefiles typically use tabs consistently.

 scr-$(CONFIG_SOC_SPACEMIT_K1X) += \
-       spacemit-fixup.scr
+	spacemit-fixup.scr
patch/u-boot/legacy/u-boot-spacemit-k1/001-MBR-support.patch (2)

21-21: Debug print statements should be removed or converted to debug().

Multiple printf("BPI: ...") statements are added throughout this patch (lines 21, 44, 51, 63, 71, 93, 101). These will output to the console during every boot, adding noise. Consider removing them or converting to debug() or pr_debug() macros which are only active when DEBUG is defined.

-	printf("BPI: :%s\n", "_load_env_from_blk");
+	debug("BPI: :%s\n", "_load_env_from_blk");

80-88: Hardcoded sector addresses lack documentation.

The fallback sector addresses 0x500 for opensbi and 0x800 for uboot are magic numbers. Consider adding comments explaining these values and their origin (e.g., partition table layout, vendor specification).

+		/* Fallback sector addresses for MBR layout without GPT partition table:
+		 * opensbi at sector 0x500 (2560), uboot at sector 0x800 (4096)
+		 */
 		if (!strcmp(part_name, "opensbi")){
 			info.start = 0x500;
patch/kernel/archive/spacemit-6.18/dt/k1-musepi-pro.dts (2)

281-281: Use "okay" instead of "ok" for status.

The device tree specification prefers "okay" over "ok" for the status property. While both may work, "okay" is the standard form.

 	dpi_panel: panel@0 {
-			status = "ok";
+			status = "okay";
 			compatible = "raspberrypi,dpi-panel";

280-283: Inconsistent indentation in panel node.

The status and compatible properties inside panel@0 have extra indentation compared to sibling nodes.

 	dpi_panel: panel@0 {
-			status = "ok";
-			compatible = "raspberrypi,dpi-panel";
+		status = "okay";
+		compatible = "raspberrypi,dpi-panel";
 	};
patch/kernel/archive/spacemit-6.18/dt/lcd_tc358762xbg_dpi_800x480.dtsi (2)

3-61: Potentially ambiguous/duplicated lane configuration (dsi-lane-number vs lane-number).
You set both dsi-lane-number = <1>; (Line 6) and lane-number = <1>; (Line 24). If different consumers interpret these, it’s easy to drift into mismatched settings—especially when debugging DSI lockups. Consider consolidating to the single property the actual panel/bridge driver consumes (or add a short comment clarifying which is used).


3-61: Missing/implicit binding cues (e.g., compatible)—verify this is intended for your overlay framework.
If this node is meant to be matched/validated by a standard panel binding, it typically needs a compatible and consistent property names. If it’s purely a SpacemiT-private “lcds” database consumed by a vendor driver, it’s fine—but please confirm it won’t trip dt-schema checks or confuse downstream integrators.

config/boards/musepipro.conf (1)

21-26: Quote + check the actual firmware file; also verify whether rootfs install is needed.
[[ -d ... ]] (Line 22) doesn’t ensure esos.elf exists, and cp -fv $SRC/... (Line 25) is unquoted. Also, per prior SpacemiT flow, esos.elf is embedded into the kernel image during build; please confirm you still need it in /lib/firmware at runtime (otherwise this is extra surface area / potential packaging drift). Based on learnings, the kernel embedding is already handled via custom_kernel_config__spacemit_k1_firmware().

-	if [[ -d "$SRC/packages/blobs/riscv64/spacemit" ]]; then
+	if [[ -f "$SRC/packages/blobs/riscv64/spacemit/esos.elf" ]]; then
 		run_host_command_logged mkdir -pv "${destination}"/lib/firmware
 		display_alert "$BOARD" "Installing boot firmware" "info"
-		run_host_command_logged cp -fv $SRC/packages/blobs/riscv64/spacemit/esos.elf "${destination}"/lib/firmware
+		run_host_command_logged cp -fv "$SRC/packages/blobs/riscv64/spacemit/esos.elf" "${destination}/lib/firmware/"
 	fi
patch/u-boot/legacy/u-boot-spacemit-k1/005-Add-uefi-syslinux-and-script-support.patch (1)

337-366: Prefer a single load address variable instead of hardcoding 0x2000000 repeatedly.
Using ${scriptaddr} (already present in the prior env and common across U-Boot) or a dedicated env var (e.g., loadaddr) makes this easier to maintain and reduces overlap risk when memory layout changes.

Comment on lines +53 to +54
# CONFIG_MODULE_SIG= is not set
# CONFIG_MODULE_SIG_SHA256= is not set
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: malformed Kconfig “not set” lines won’t disable module signing.
These lines (Line 53-54) are not in Kconfig’s recognized format (# CONFIG_FOO is not set). As written, they’re just comments, so CONFIG_MODULE_SIG / CONFIG_MODULE_SIG_SHA256 may end up enabled depending on defaults/oldconfig.

-# CONFIG_MODULE_SIG= is not set
-# CONFIG_MODULE_SIG_SHA256= is not set
+# CONFIG_MODULE_SIG is not set
+# CONFIG_MODULE_SIG_SHA256 is not set
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# CONFIG_MODULE_SIG= is not set
# CONFIG_MODULE_SIG_SHA256= is not set
# CONFIG_MODULE_SIG is not set
# CONFIG_MODULE_SIG_SHA256 is not set
🤖 Prompt for AI Agents
In config/kernel/linux-spacemit-edge.config around lines 53 to 54, the Kconfig
“not set” lines are malformed ("# CONFIG_MODULE_SIG= is not set" and "#
CONFIG_MODULE_SIG_SHA256= is not set"); replace them with the exact Kconfig
syntax "# CONFIG_MODULE_SIG is not set" and "# CONFIG_MODULE_SIG_SHA256 is not
set" so the options are actually disabled, then regenerate or re-run kernel
config (e.g., make oldconfig) to ensure the changes take effect.

Comment on lines 39 to 45
edge)
declare -g KERNELSOURCE="https://github.com/jmontleon/linux-bianbu.git"
declare -g KERNELBRANCH="branch:linux-6.17.y"
declare -g KERNELBRANCH="branch:linux-6.18.y"
declare -g EXTRAWIFI="no" # WiFi drivers are already included in the kernel
declare -g KERNEL_MAJOR_MINOR="6.17"
declare -g KERNEL_MAJOR_MINOR="6.18"
declare -g LINUXCONFIG="linux-${LINUXFAMILY}-edge"
;;
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 12, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify linux-6.18.y branch exists in linux-bianbu.git
git ls-remote --heads https://github.com/jmontleon/linux-bianbu.git | grep -E 'linux-6\.18\.y'

Repository: armbian/build

Length of output: 121


🏁 Script executed:

#!/bin/bash
# Check if linux-bianbu is a vendor fork and examine spacemit.conf structure
cd /tmp
git clone --depth 1 https://github.com/jmontleon/linux-bianbu.git 2>&1 | head -20
echo "---"
# Check available branches
git -C linux-bianbu branch -a 2>&1 | head -20

Repository: armbian/build

Length of output: 3750


🏁 Script executed:

#!/bin/bash
# Examine the full spacemit.conf to understand naming patterns
cat config/sources/families/spacemit.conf

Repository: armbian/build

Length of output: 3022


🏁 Script executed:

#!/bin/bash
# Check git history to see if spacemit.conf was using "edge" before
git log --oneline -n 20 config/sources/families/spacemit.conf

Repository: armbian/build

Length of output: 110


🏁 Script executed:

#!/bin/bash
# Compare with other vendor fork configs to see naming patterns
# Check if other vendor forks use "vendor"/"vendor-rt" or "current"/"edge"
rg -l "vendor.*fork\|bianbu\|jmontleon" config/sources/families/ | head -5
rg "KERNELSOURCE.*gitee\|github.*jmontleon" config/sources/families/ -A 5 | head -40

Repository: armbian/build

Length of output: 39


Update spacemit.conf to use vendor-appropriate branch naming.

The linux-6.18.y branch exists in the source repository and KERNEL_MAJOR_MINOR correctly matches the version. However, the "edge" branch designation for a vendor fork (linux-bianbu.git) violates Armbian conventions. Vendor kernel branches should use "vendor" or "vendor-rt" naming; "current" and "edge" are reserved for mainline kernel branches. Both the "current" and "edge" sections here reference the same vendor source and should follow this pattern.

🤖 Prompt for AI Agents
In config/sources/families/spacemit.conf around lines 39 to 45, the vendor
kernel branch is named "edge" which violates Armbian conventions; update the
branch naming and section labels to use vendor-appropriate names. Change the
branch designation to "vendor" (or "vendor-rt" if real-time) and ensure both the
current and edge sections that reference the same linux-bianbu.git vendor fork
use the vendor naming consistently; also update any related LINUXCONFIG or
comments to reflect the vendor branch naming so the file follows Armbian branch
conventions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we have a precedent from TI k3 to use vendor-edge

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
# Check the k3 family configuration for branch naming patterns
echo "=== Checking k3 family config ==="
cat config/sources/families/k3.conf 2>/dev/null || echo "k3.conf not found"

echo -e "\n=== Checking k3-beagle family config ==="
cat config/sources/families/k3-beagle.conf 2>/dev/null || echo "k3-beagle.conf not found"

echo -e "\n=== Checking for vendor-edge pattern in k3 related files ==="
rg -n "vendor-edge|vendor.*edge" config/sources/families/k3*.conf config/sources/families/include/k3*.inc 2>/dev/null || echo "No vendor-edge pattern found"

echo -e "\n=== Checking case statements in k3 files ==="
rg -n "^\s*(vendor|edge|current)" config/sources/families/k3*.conf config/sources/families/include/k3*.inc 2>/dev/null | head -30

Length of output: 3950


@tabrisnet, you're absolutely correct! I can confirm the k3 family uses vendor-edge for bleeding-edge vendor kernels. Looking at config/sources/families/k3.conf:

  • Line 17: vendor | vendor-rt) — for stable TI Processor SDK releases
  • Line 32: vendor-edge) — for latest pre-released/bleeding-edge TI SDK

So the proper naming pattern for spacemit.conf should be:

  • vendor (or vendor | vendor-rt) — for the stable linux-6.6.y branch (currently labeled "current")
  • vendor-edge — for the bleeding-edge linux-6.18.y branch (currently labeled "edge")

Both branches reference the same linux-bianbu.git vendor fork, so both should use vendor-prefixed naming. Thank you for catching this!

    (\(\
    (-.-)
    o_(")(")  Learned something new!

✏️ Learnings added
Learnt from: tabrisnet
Repo: armbian/build PR: 9058
File: config/sources/families/spacemit.conf:39-45
Timestamp: 2025-12-12T23:09:56.813Z
Learning: In the Armbian build system, vendor kernel forks can use "vendor-edge" branch naming for bleeding-edge or pre-release vendor kernels, as demonstrated in config/sources/families/k3.conf. The typical vendor branch naming pattern is: "vendor" or "vendor-rt" for stable vendor releases, and "vendor-edge" for bleeding-edge/pre-release vendor versions. The "edge" naming without "vendor-" prefix is reserved for mainline kernel branches.

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: 8330
File: config/sources/families/sun55iw3.conf:32-36
Timestamp: 2025-06-25T03:42:09.086Z
Learning: In Armbian build system configuration files like config/sources/families/*.conf, KERNELSOURCE is explicitly declared when using unofficial or 3rd party kernel repositories (like the "dev" branch using https://github.com/apritzel/linux), but can be omitted when using the standard mainline kernel (like the "edge" branch) since it will fall back to the default mainline source.

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.

Learnt from: EvilOlaf
Repo: armbian/build PR: 8330
File: config/sources/families/sun55iw3.conf:32-36
Timestamp: 2025-06-25T03:40:52.109Z
Learning: In Armbian build system configuration files like config/sources/families/*.conf, when KERNELSOURCE is not explicitly declared in a case branch, the system falls back to a default KERNELSOURCE value, so missing KERNELSOURCE declarations are not critical issues.

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: 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.

Learnt from: tabrisnet
Repo: armbian/build PR: 8746
File: config/sources/families/filogic.conf:61-68
Timestamp: 2025-10-13T02:26:18.249Z
Learning: In the Armbian build system, the "edge" kernel branch is intended to be bleeding-edge and experimental. It does not require hardware testing or stability verification before merging, as it's expected to contain potentially unstable or untested code for early adopters and developers.

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: The-going
Repo: armbian/build PR: 8147
File: config/sources/families/include/sunxi64_common.inc:38-39
Timestamp: 2025-04-30T16:16:47.150Z
Learning: The Armbian build system references Linux kernel versions in the form "tag:v6.14.4" in the KERNELBRANCH variable, even when point release tags might not be directly visible in the upstream repository in the same form.

Learnt from: Grippy98
Repo: armbian/build PR: 8152
File: lib/functions/configuration/interactive.sh:209-266
Timestamp: 2025-05-05T12:35:07.143Z
Learning: For the interactive kernel selection in Armbian, KERNEL_MAJOR_MINOR and KERNEL_DESCRIPTION are parsed from family.conf but deliberately not set as environment variables to avoid potential interference with other parts of the build system.

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.

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.

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: 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.

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.

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: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.

Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.

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: 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: rpardini
Repo: armbian/build PR: 8879
File: config/sources/families/uefi-x86.conf:0-0
Timestamp: 2025-11-06T15:36:04.682Z
Learning: As of PR #8879, the uefi-x86 family in the Armbian build system now includes kernel patches for the first time. The current and edge branches for uefi-x86 are specifically configured for Apple T2-based x86 machines, including T2-specific patches from the linux-t2 project and custom kernel configuration options for Apple hardware drivers.

Learnt from: leggewie
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-08-29T18:44:47.732Z
Learning: When creating GitHub issues for code improvements in the Armbian build project, include the relevant patches/diffs in the issue description or open a PR directly instead of just describing the changes. This makes it more efficient for the assignee to handle the improvements.

Comment on lines +69 to +75
*/
+#if (LINUX_VERSION_CODE < KERNEL_VERSION(5, 8, 0))
int hmac_sha256(const u8 *key, size_t key_len, const u8 *data,
size_t data_len, u8 *mac)
{
return hmac_sha256_vector(key, key_len, 1, &data, &data_len, mac);
}
Copy link
Contributor

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:

cat patch/kernel/archive/spacemit-6.18/005-Fixup-rtl8852bs-sha256.patch

Repository: armbian/build

Length of output: 3920


🌐 Web query:

When was hmac_sha256_vector function added to Linux kernel crypto subsystem

💡 Result:

The hmac_sha256_vector implementation was added in the SHA‑256 library patch series published June 30, 2025. [1][2]

Sources:

  • Patchwork / Eric Biggers — "lib/crypto: sha256: Add HMAC‑SHA224 and HMAC‑SHA256 support" (Jun 30, 2025). [1]
  • LWN summary of the SHA‑256 library improvements (covering the same series, Jun 30, 2025). [2]

🌐 Web query:

When was hmac_sha256 function added to Linux kernel main include files

💡 Result:

The hmac/HMAC-SHA256 support (including hmac_sha256 APIs in include/crypto/sha2.h) was added in late June 2025 — the patch series introducing HMAC‑SHA224/HMAC‑SHA256 was posted June 25–30, 2025 (patch messages: Jun 25 and Jun 30, 2025). [1][2]

Sources: Linux kernel patch mailing-list posts (Eric Biggers) and LWN summary. [1] Jun 25, 2025 patch; [2] Jun 30, 2025 LWN article.


🏁 Script executed:

# Search for kernel version when hmac_sha256 and hmac_sha256_vector were added to mainline
rg -i "hmac_sha256" --type h /dev/null 2>/dev/null || echo "Searching kernel history..."
# Check if there are other patches addressing this in the build system
fd "rtl8852bs" patch/

Repository: armbian/build

Length of output: 780


🌐 Web query:

Linux kernel 5.8 SHA256 hmac_sha256_vector crypto functions availability

💡 Result:

Short answer: Kernel 5.8 provides HMAC‑SHA256 via the kernel Crypto API (AHASH template "hmac(sha256)" and the shash/ahash interfaces such as crypto_shash / crypto_ahash), but the newer in‑kernel SHA‑256 library HMAC convenience helpers (higher‑level hmac_sha256_* library functions) were not part of the vintage 5.8 library and were added later (see the SHA‑256 library / HMAC work merged in mid‑2025). [1][2]

Details / pointers:

  • The Crypto API docs (applicable to the 5.x series) describe keyed digests and the HMAC template (e.g. hmac(sha256)) and the shash/ahash APIs you should use from kernel code. [1]
  • A recent patch series (2025) adds explicit SHA‑256 library HMAC helpers and updates callers to use them (i.e. those convenience library functions are a later addition, not original to 5.8). See the SHA‑256 library / HMAC patch discussion and coverage in LWN. [2]

If you want, I can:

  • show a minimal kernel‑module example using crypto_alloc_ahash("hmac(sha256)") / crypto_ahash_* calls, or
  • search your tree for a specific symbol name (e.g. hmac_sha256_vector) to confirm whether it exists in a particular kernel source tree.

🌐 Web query:

rtl8852bs driver sha256 backport kernel version compatibility

💡 Result:

Short answer:

  • The vendor out-of-tree rtl8852bs (SDIO) backport used by Armbian is marked "only been tested on Linux 6.1". Use that backport for kernels around 6.1; if your kernel already includes the in-tree rtw89 driver prefer rtw89 instead. [1][2]

  • The in‑kernel RTW89 driver (supports RTL8852x family) has been present in mainline kernels since ~5.16 and is the recommended path for newer kernels (rtw89/CONFIG_RTW89 appears across 5.16→6.x). Third‑party rtw89 forks note different supported ranges (example: morrownr targets 6.6+, other forks target 5.15–6.5). Match the driver fork/backport to your exact kernel version. [3][4][5]

  • SHA256: there is no single canonical "rtl8852bs backport sha256" published universally — check the specific backport tarball/release you downloaded (GitHub release or distro package) and verify its sha256 there, or compute it locally with:
    sha256sum
    If you point me to the exact backport release URL or package file name you have, I can fetch its published checksum (if available) and/or compute and report the SHA‑256. [1][3]

Sources:
[1] armbian/wifi-rtl8852bs README (tested on 6.1).
[2] LKDDb / CONFIG_RTW89 kernel coverage (rtw89 in 5.16–6.x).
[3] morrownr rtw89 README (compatibility notes; 6.6+).
[4] a5a5aa555oo rtw89 README (notes: supports ~5.15–6.5).
[5] rtw89 driver info / wiki pages.


🏁 Script executed:

# Check if there are other rtl8852bs patches with version information
rg "KERNEL_VERSION" patch/kernel/archive/spacemit-6.18/ -A 2 -B 2

Repository: armbian/build

Length of output: 3220


🌐 Web query:

spacemit kernel 6.18 rtl8852bs wifi driver support

💡 Result:

Short answer: Yes — the SpacemiT/Bianbu kernel tree includes RTL8852BS support (SDIO) and the upstream rtw89 driver covers RTL8852* PCI variants; for SDIO RTL8852BS you’ll usually rely on the vendor/SpacemiT SDIO driver in their kernel tree rather than only the generic rtw89 PCI code. [1][2][3]

Notes / pointers:

  • SpacemiT / Bianbu docs list rtl8852bs under drivers/net/wireless/realtek in their device/tree and show platform (spacemit-rf) support for rtl8852bs (SDIO). [1][2]
  • Upstream CONFIG_RTW89 (rtw89 driver) is present in 6.18-rc+ and provides in-kernel Realtek 802.11ax support (mostly PCI devices such as 8852BE/8852AE); it does not by itself guarantee SDIO rtl8852bs support. [3]
  • If you run vanilla mainline 6.18 on SpacemiT hardware, use the SpacemiT/Bianbu kernel branch (their patches include the SDIO rtl8852bs and platform glue). Also ensure matching Realtek firmware (linux-firmware) and the SpacemiT platform power/GPIO driver are enabled. [1][2]

Sources:
[1] Bianbu / SpacemiT Wi‑Fi peripheral docs showing rtl8852bs and spacemit-rf platform code. (bianbu-linux.spacemit.com)
[2] Bianbu release notes / kernel branches mentioning rtl8852bs fixes in their kernel tree. (bianbu-linux.spacemit.com)
[3] LKDDb / CONFIG_RTW89 info showing rtw89 present in 6.18-rc+ (Realtek 802.11ax support). (cateee.net)

If you want, tell me which distro/kernel build you’re using and I’ll give exact steps (install patch/firmware or enable the SpacemiT kernel branch).

Citations:


🌐 Web query:

Linux kernel hmac_sha256_vector when added mainline kernel version

💡 Result:

There is no mainline Linux kernel symbol named hmac_sha256_vector — that function is a userspace helper (e.g. in wpa_supplicant). The kernel exposes HMAC via the Crypto API (ahash/shash), not a hmac_sha256_vector wrapper. [1][2][3]

Sources:

  • Kernel Crypto API (HMAC/SHA256 explanation). [1]
  • hmac_sha256_vector in wpa_supplicant source. [2]
  • Notes showing a backwards-compat/compat wrapper (hmac_sha256/hmac_sha256_vector) was dropped by drivers/compat work (Linux 6.18 compat mention). [3]

🌐 Web query:

Linux kernel hmac_sha256 function include crypto when added mainline

💡 Result:

The HMAC‑SHA256 library functions (added to include/crypto/sha2.h and lib/crypto/sha256.c and exposed to crypto/sha256) were introduced in mid‑2025 by Eric Biggers’ patch series—initial posting 25 June 2025 with followups (v2 series around 30 Jun–1 Jul 2025). [1][2][3]


🌐 Web query:

kernel crypto lib hmac_sha256 sha2.h version 6.18

💡 Result:

Short answer: Linux kernel 6.18 adds first‑class HMAC‑SHA224/HMAC‑SHA256 support to the crypto library — include/crypto/sha2.h was extended with HMAC key/ctx structs and HMAC APIs (e.g. __hmac_sha256_init and the hmac_sha*_init_usingrawkey helpers). See the patch series adding HMAC‑SHA224/256 and the crypto/sha256 wrapping changes. [1][2][3]

Sources

  • Patch: "lib/crypto: sha256: Add HMAC‑SHA224 and HMAC‑SHA256 support" (showing changes to include/crypto/sha2.h). [1]
  • Mailing‑list archive of the same patch (diff + explanation). [2]
  • Crypto library/6.17–6.18 pull summary listing the sha2 HMAC additions. [3]

Version guard mismatch between declaration and implementation will cause linker errors for kernels 5.8.0–6.17.x.

In sha256.h, hmac_sha256 is declared with < KERNEL_VERSION(6, 18, 0), but in sha256.c, the implementation is guarded by < KERNEL_VERSION(5, 8, 0). This mismatch means the declaration exists for kernels 5.8.0–6.17.x, but the implementation does not—resulting in an "undefined reference" linker error.

Additionally, the hmac_sha256 implementation calls hmac_sha256_vector, which is only compiled for kernels < 5.8.0. Both guards should be aligned to < KERNEL_VERSION(6, 18, 0) to ensure consistent availability across the kernel range where the driver provides these functions (before the kernel itself provides them in 6.18.0).

Change the implementation guard in sha256.c (line 102) from < KERNEL_VERSION(5, 8, 0) to < KERNEL_VERSION(6, 18, 0), and verify that hmac_sha256_vector is similarly available for the same kernel range.

🤖 Prompt for AI Agents
In patch/kernel/archive/spacemit-6.18/005-Fixup-rtl8852bs-sha256.patch around
lines 69 to 75, the implementation of hmac_sha256 is guarded with "#if
(LINUX_VERSION_CODE < KERNEL_VERSION(5, 8, 0))" which mismatches the declaration
guard in sha256.h and causes undefined reference on kernels 5.8.0–6.17.x; change
the implementation guard to "#if (LINUX_VERSION_CODE < KERNEL_VERSION(6, 18,
0))" so it matches the declaration, and verify that hmac_sha256_vector is also
compiled under the same "< KERNEL_VERSION(6, 18, 0)" range (adjust its guard if
necessary) to ensure both declaration and implementation are available for the
same kernel versions.

Comment on lines +19 to +21
+# Overlay targets
+extra-y += $(dtbo-y) $(scr-y) $(dtbotxt-y)
+
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential build dependency hazard: .scr artifacts will require mkimage once scr-y is set.
Since extra-y now pulls $(scr-y) into the build, missing mkimage in the toolchain will fail the kernel build for configs enabling scripts. Consider using an overridable variable and avoiding a hard-coded binary name.

Example tweak:

+MKIMAGE ?= mkimage
 quiet_cmd_scr = MKIMAGE $@
-cmd_scr = mkimage -C none -A $(ARCH) -T script -d $< $@
+cmd_scr = $(MKIMAGE) -C none -A $(ARCH) -T script -d $< $@

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In patch/kernel/archive/spacemit-6.18/006-general-spacemit-overlays.patch around
lines 19-21, adding scr-y to extra-y will pull .scr artifacts into the build and
create a hard dependency on the mkimage binary; change this by introducing an
overridable variable for the mkimage tool (e.g. MKIMAGE ?= mkimage) and use that
variable wherever the mkimage binary is required, and guard any explicit
make-time dependency on mkimage with a conditional so builds that do not set
scr-y or that provide a different tool do not fail; ensure the Makefile/patch
documents the new variable and does not reference a hard-coded "mkimage" name.

Comment on lines +76 to +87
+quiet_cmd_scr = MKIMAGE $@
+cmd_scr = mkimage -C none -A $(ARCH) -T script -d $< $@
+
$(obj)/%.dtb: $(obj)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
$(call if_changed_dep,dtc)

$(obj)/%.dtbo: $(src)/%.dtso $(DTC) FORCE
$(call if_changed_dep,dtc)

+$(obj)/%.scr: $(src)/%.scr-cmd FORCE
+ $(call if_changed,scr)
+
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

.scr rule should use a tool variable and ideally validate tool presence when enabled.
Right now it shells out to mkimage unconditionally when .scr targets are built. If the build environment is missing u-boot-tools, this will be a hard failure; using MKIMAGE ?= mkimage (and documenting the dependency) makes this easier to handle.

🤖 Prompt for AI Agents
In patch/kernel/archive/spacemit-6.18/006-general-spacemit-overlays.patch around
lines 76–87, the .scr rule calls mkimage directly which will hard-fail if
u-boot-tools is not installed; replace the hard-coded command with a tool
variable (e.g. MKIMAGE ?= mkimage) and update quiet_cmd_scr/cmd_scr to use
$(MKIMAGE) instead of mkimage, and add a minimal presence/validation check so
the build either emits a clear error when MKIMAGE is missing or skips/enables
the rule conditionally (for example by checking the variable origin or testing
for the binary at configure time and failing with a helpful message). Ensure the
dependency on u-boot-tools is documented in comments.

scr-$(CONFIG_SOC_SPACEMIT_K1X) += \
spacemit-fixup.scr

dtb-y += $(dtbo-y) $(scr-y) $(dtbotxt-y)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for dtbotxt-y definitions in spacemit-6.18 directory
rg -n 'dtbotxt-y' patch/kernel/archive/spacemit-6.18/ --type-list | head -20
echo "---"
rg -n 'dtbotxt-y' patch/kernel/archive/spacemit-6.18/

Repository: armbian/build

Length of output: 680


🏁 Script executed:

#!/bin/bash
# Search for dtbotxt-y definition patterns (assignment, not just usage)
rg 'dtbotxt-y\s*[:+]?=' patch/kernel/archive/spacemit-6.18/ -n

echo "---"
# Check what Makefiles exist in spacemit-6.18 directory
find patch/kernel/archive/spacemit-6.18 -name "Makefile*" -type f

echo "---"
# Check the patch file for context
rg -B 5 -A 5 'dtbotxt-y' patch/kernel/archive/spacemit-6.18/006-general-spacemit-overlays.patch

Repository: armbian/build

Length of output: 464


🏁 Script executed:

#!/bin/bash
# Read the overlay/Makefile entirely to see variable definitions
echo "=== overlay/Makefile ==="
cat -n patch/kernel/archive/spacemit-6.18/overlay/Makefile

echo ""
echo "=== Search for dtbo-y and scr-y definitions ==="
rg 'dtbo-y|scr-y' patch/kernel/archive/spacemit-6.18/ -n | grep -v 'Binary'

Repository: armbian/build

Length of output: 701


🏁 Script executed:

#!/bin/bash
# Check dt/Makefile
echo "=== dt/Makefile ==="
cat -n patch/kernel/archive/spacemit-6.18/dt/Makefile

echo ""
echo "=== Search for dtbotxt in entire spacemit-6.18 ==="
rg 'dtbotxt' patch/kernel/archive/spacemit-6.18/ -n

echo ""
echo "=== Check if spacemit-6.18 has a parent Makefile or patch that defines it ==="
ls -la patch/kernel/archive/spacemit-6.18/ | grep -i make

Repository: armbian/build

Length of output: 1406


🏁 Script executed:

#!/bin/bash
# Search for dtbotxt in entire repository
echo "=== Search for dtbotxt in all kernel patches ==="
rg 'dtbotxt' patch/kernel/ -n

echo ""
echo "=== Search in entire repo ==="
rg 'dtbotxt' . -n --max-count=20

Repository: armbian/build

Length of output: 25122


Add missing dtbotxt-$(CONFIG_SOC_SPACEMIT_K1X) definition.

The dtbotxt-y variable is aggregated on line 9 but never defined. Compare with similar architectures (rockchip64-6.18, meson64-6.18) which define dtbotxt-$(CONFIG_ARCH_*) assignments before aggregating into dtb-y. Add the missing definition to ensure overlay documentation files are properly included.

🤖 Prompt for AI Agents
In patch/kernel/archive/spacemit-6.18/overlay/Makefile around line 9, dtbotxt-y
is aggregated but dtbotxt-$(CONFIG_SOC_SPACEMIT_K1X) is not defined; add a
definition before the dtb-y aggregation such as: define
dtbotxt-$(CONFIG_SOC_SPACEMIT_K1X) to list the overlay documentation file(s) for
the K1X SOC (for example dtbotxt-$(CONFIG_SOC_SPACEMIT_K1X) :=
spacemit-k1x.dtbo.txt or dtbotxt-$(CONFIG_SOC_SPACEMIT_K1X) :=
overlay/spacemit-k1x.dtbo.txt depending on repo layout) so the dtbotxt-y
variable includes the proper files when aggregated on line 9.

Comment on lines +30 to +46
cpu_0: cpu@0 {
cpu-ai = "true";
};

cpu_1: cpu@1 {
cpu-ai = "true";
};

cpu_2: cpu@2 {
reg = <2>;
cpu-ai = "true";
};

cpu_3: cpu@3 {
reg = <3>;
cpu-ai = "true";
};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing reg property on cpu_0 and cpu_1.

cpu_0 (Line 30-32) and cpu_1 (Line 34-36) are missing the reg property, while cpu_2 and cpu_3 have it. This inconsistency may cause boot issues or incorrect CPU topology detection.

 		cpu_0: cpu@0 {
+			reg = <0>;
 			cpu-ai = "true";
 		};

 		cpu_1: cpu@1 {
+			reg = <1>;
 			cpu-ai = "true";
 		};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cpu_0: cpu@0 {
cpu-ai = "true";
};
cpu_1: cpu@1 {
cpu-ai = "true";
};
cpu_2: cpu@2 {
reg = <2>;
cpu-ai = "true";
};
cpu_3: cpu@3 {
reg = <3>;
cpu-ai = "true";
};
cpu_0: cpu@0 {
reg = <0>;
cpu-ai = "true";
};
cpu_1: cpu@1 {
reg = <1>;
cpu-ai = "true";
};
cpu_2: cpu@2 {
reg = <2>;
cpu-ai = "true";
};
cpu_3: cpu@3 {
reg = <3>;
cpu-ai = "true";
};
🤖 Prompt for AI Agents
In patch/kernel/archive/spacemit-6.6/dt/k1-musepi-pro.dts around lines 30 to 46,
cpu_0 and cpu_1 are missing the required reg properties (cpu_2 and cpu_3 have
reg <2> and <3>), so add reg = <0>; to cpu_0 and reg = <1>; to cpu_1 so all CPU
nodes consistently declare their CPU index; keep the existing cpu-ai = "true"
entries and formatting consistent with the other CPU nodes.

Comment on lines +3 to +61
/ { lcds: lcds {
lcd_tc358762xbg_dpi_800x480: lcd_tc358762xbg_dpi_800x480 {
dsi-work-mode = <1>; /* video burst mode*/
dsi-lane-number = <1>;
dsi-color-format = "rgb888";
width-mm = <95>;
height-mm = <53>;
use-dcs-write;

/*mipi info*/
height = <480>;
width = <800>;
hfp = <1>;
hbp = <46>;
hsync = <2>;
vfp = <7>;
vbp = <21>;
vsync = <2>;
fps = <55>;
work-mode = <0>;
rgb-mode = <3>;
lane-number = <1>;
phy-bit-clock = <700000000>;
split-enable = <0>;
eotp-enable = <0>;
burst-mode = <2>;
esd-check-enable = <0>;

/* DSI_CMD, DSI_MODE, timeout, len, cmd */
initial-command = [
39 01 F0 01 11
39 01 28 01 29
];
sleep-in-command = [
39 01 78 01 28
39 01 78 01 10
];
sleep-out-command = [
39 01 96 01 11
39 01 32 01 29
];

display-timings {
timing0 {
clock-frequency = <25000000>;
hactive = <800>;
hfront-porch = <1>;
hback-porch = <46>;
hsync-len = <2>;
vactive = <480>;
vfront-porch = <7>;
vback-porch = <21>;
vsync-len = <2>;
vsync-active = <1>;
hsync-active = <1>;
};
};
};
};}; No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Search for references to this LCD dtsi file
rg "lcd_tc358762xbg" --type device-tree

Repository: armbian/build

Length of output: 92


🏁 Script executed:

# Look for the main device tree source and check if DSI/LCD is referenced or disabled
fd -e dtsi -e dts | head -20

Repository: armbian/build

Length of output: 1191


🏁 Script executed:

# Check the actual file content to verify braces syntax
cat -n "patch/kernel/archive/spacemit-6.6/dt/lcd_tc358762xbg_dpi_800x480.dtsi"

Repository: armbian/build

Length of output: 1636


🏁 Script executed:

# Search for comments about DSI or LCD lockups in DT files
rg -i "lockup|dsi.*lock|lcd.*lock" --type device-tree -A 2 -B 2

Repository: armbian/build

Length of output: 92


🏁 Script executed:

# Search for references to this LCD file without file type restriction
rg "lcd_tc358762xbg" -A 2 -B 2

Repository: armbian/build

Length of output: 2662


🏁 Script executed:

# Search for includes of this specific file path
rg "lcd_tc358762xbg_dpi_800x480\.dtsi"

Repository: armbian/build

Length of output: 255


🏁 Script executed:

# Look for the main k1-musepi-pro device tree to see if/how this is included
fd "k1-musepi-pro" | head -20

Repository: armbian/build

Length of output: 167


🏁 Script executed:

# Search for DSI or LCD related disable/status comments in spacemit DT files
rg -i "dsi|lcd" "patch/kernel/archive/spacemit-6.6/dt/" | head -30

Repository: armbian/build

Length of output: 2177


🏁 Script executed:

# Get context around the DSI status in k1-musepi-pro.dts for 6.6
cat -n "patch/kernel/archive/spacemit-6.6/dt/k1-musepi-pro.dts" | grep -A 10 -B 2 "&dsi2"

Repository: armbian/build

Length of output: 336


🏁 Script executed:

# Check if there are any PR notes, comments, or documentation about LCD/DSI lockups
rg -i "lockup|boot.*hang|dsi.*issue" "patch/kernel/archive/spacemit-6.6/" -A 3 -B 3

Repository: armbian/build

Length of output: 8816


🏁 Script executed:

# Look at the PR/commit message context if available
git log --oneline -20 -- "patch/kernel/archive/spacemit-6.6/dt/lcd_tc358762xbg_dpi_800x480.dtsi" 2>/dev/null | head -10

Repository: armbian/build

Length of output: 110


Keep DSI disabled until lane/bitclock parameters are validated against the actual TC358762XBG controller limits.

This .dtsi is already included in the default k1-musepi-pro build, but DSI is safely disabled (status = "disabled") at the controller level. Before enabling DSI, confirm that dsi-lane-number = <1> and phy-bit-clock = <700000000> match the panel and controller datasheet specifications.

Minor: expand the final };}; into readable multi-line braces to improve clarity.

Comment on lines +17 to +25
@@ -529,8 +529,8 @@ void _load_env_from_blk(struct blk_desc *dev_desc, const char *dev_name, int dev
#endif
}

- env_set("bootfs_part", simple_itoa(part));
- env_set("bootfs_devname", dev_name);
+ env_set("distro_bootpart", simple_itoa(part));
+ env_set("devnum", dev_name);

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: devnum is set to dev_name (string) in _load_env_from_blk().
env_set("devnum", dev_name); (Line 24) looks incorrect: devnum should be the device index (e.g., 0), while dev_name is typically mmc/nvme/etc. This will break any env logic that assumes ${devnum} is numeric.

-	env_set("distro_bootpart", simple_itoa(part));
-	env_set("devnum", dev_name);
+	env_set("distro_bootpart", simple_itoa(part));
+	env_set("devtype", dev_name);
+	env_set("devnum", simple_itoa(dev));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@@ -529,8 +529,8 @@ void _load_env_from_blk(struct blk_desc *dev_desc, const char *dev_name, int dev
#endif
}
- env_set("bootfs_part", simple_itoa(part));
- env_set("bootfs_devname", dev_name);
+ env_set("distro_bootpart", simple_itoa(part));
+ env_set("devnum", dev_name);
}
env_set("distro_bootpart", simple_itoa(part));
env_set("devtype", dev_name);
env_set("devnum", simple_itoa(dev));
🤖 Prompt for AI Agents
In patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch around
lines 17 to 25, env_set("devnum", dev_name) is wrong because devnum must be the
numeric device index; change it to set the numeric index string instead (e.g.,
env_set("devnum", simple_itoa(dev))). Ensure you use the function's device index
parameter (dev) and convert it to a string with simple_itoa before calling
env_set.

Signed-off-by: Patrick Yavitz <pyavitz@gmail.com>
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: 1

♻️ Duplicate comments (1)
patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch (1)

17-25: Fix devnum/devtype assignment in _load_env_from_blk() (currently broken).
Line 24 sets devnum to dev_name (a string like mmc/nvme) which will break any distro-boot logic expecting numeric ${devnum}. This matches the previously reported issue.

Suggested patch adjustment:

-	env_set("distro_bootpart", simple_itoa(part));
-	env_set("devnum", dev_name);
+	env_set("distro_bootpart", simple_itoa(part));
+	env_set("devtype", dev_name);
+	env_set("devnum", simple_itoa(dev));
🧹 Nitpick comments (1)
patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch (1)

81-101: Address macros: good direction, but FDTOVERLAY_ADDR_R=0x01000000 needs target validation.
Hard-coding overlay load/application buffers that low can collide with U-Boot/firmware/runtime allocations depending on the K1X RAM map.

Please validate on hardware that:

  • overlay load + apply does not corrupt U-Boot/stack/malloc region
  • overlay size worst-case still fits safely at the chosen address
📜 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 91461e8 and 87fc0f5.

📒 Files selected for processing (1)
  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch (1 hunks)
🧰 Additional context used
🧠 Learnings (21)
📓 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: rpardini
Repo: armbian/build PR: 8879
File: config/sources/families/uefi-x86.conf:0-0
Timestamp: 2025-11-06T15:36:04.682Z
Learning: As of PR #8879, the uefi-x86 family in the Armbian build system now includes kernel patches for the first time. The current and edge branches for uefi-x86 are specifically configured for Apple T2-based x86 machines, including T2-specific patches from the linux-t2 project and custom kernel configuration options for Apple hardware drivers.
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.
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.
Learnt from: leggewie
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-08-29T18:44:47.732Z
Learning: When creating GitHub issues for code improvements in the Armbian build project, include the relevant patches/diffs in the issue description or open a PR directly instead of just describing the changes. This makes it more efficient for the assignee to handle the improvements.
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.
📚 Learning: 2025-03-31T22:20:41.849Z
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.

Applied to files:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 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:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 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:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 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:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 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:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 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:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 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:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 Learning: 2025-09-14T11:37:35.089Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:65-72
Timestamp: 2025-09-14T11:37:35.089Z
Learning: In the Armbian build system, patch directories referenced in BOOTPATCHDIR and KERNELPATCHDIR configurations can be non-existent without causing build failures. Empty patch directories are also ignored by git, so missing patch directories should not be flagged as errors during code review.

Applied to files:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 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:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 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:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 Learning: 2025-06-12T21:03:39.686Z
Learnt from: djurny
Repo: armbian/build PR: 8287
File: config/bootscripts/boot-sunxi.cmd:148-161
Timestamp: 2025-06-12T21:03:39.686Z
Learning: In Armbian U-Boot bootscripts, the `${cpu}` environment variable is always pre-set by U-Boot (or defaults to a valid value), so additional guards against it being unset are unnecessary.

Applied to files:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 Learning: 2025-09-14T05:23:42.991Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8632
File: config/sources/families/include/rockchip64_common.inc:327-334
Timestamp: 2025-09-14T05:23:42.991Z
Learning: In the Armbian build system for Rockchip boards, SPI flash consistently maps to /dev/mtd0, so hard-coding this device path in flashcp commands is acceptable and based on hardware conventions rather than being a bug.

Applied to files:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 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:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 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:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 Learning: 2025-09-14T06:32:29.806Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.

Applied to files:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 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:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 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:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 Learning: 2025-08-11T12:39:22.861Z
Learnt from: pyavitz
Repo: armbian/build PR: 8481
File: config/kernel/linux-spacemit-edge.config:492-494
Timestamp: 2025-08-11T12:39:22.861Z
Learning: The SpacemiT platform requires esos.elf firmware to be embedded in the kernel image for boot. The Armbian build system handles this by copying esos.elf from $SRC/packages/blobs/riscv64/spacemit/ to ${kernel_work_dir}/firmware/ via the custom_kernel_config__spacemit_k1_firmware() function in config/sources/families/spacemit.conf, ensuring CONFIG_EXTRA_FIRMWARE="esos.elf" can successfully embed it during kernel build.

Applied to files:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 Learning: 2025-11-06T15:36:04.682Z
Learnt from: rpardini
Repo: armbian/build PR: 8879
File: config/sources/families/uefi-x86.conf:0-0
Timestamp: 2025-11-06T15:36:04.682Z
Learning: As of PR #8879, the uefi-x86 family in the Armbian build system now includes kernel patches for the first time. The current and edge branches for uefi-x86 are specifically configured for Apple T2-based x86 machines, including T2-specific patches from the linux-t2 project and custom kernel configuration options for Apple hardware drivers.

Applied to files:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
📚 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:

  • patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch
🔇 Additional comments (2)
patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch (2)

68-76: Enabling CONFIG_OF_LIBFDT_OVERLAY looks aligned with the overlay work—please ensure it’s supported for this U-Boot base.

Please verify the underlying U-Boot version used by u-boot-spacemit-k1 supports CONFIG_OF_LIBFDT_OVERLAY=y and that the defconfig still builds.


104-123: Distro-boot migration is correct; splash/LED removal is intentional and not a regression.

The *_addr_r naming convention is the distro-boot standard, not just a stylistic rename. The removed environment variables (splashimage, splashpos, splashfile, led0_gpio) are board-specific optional features that are not part of the distro-boot specification. No bootscripts in the codebase reference these removed variables, so there is no boot regression.

The fdtfile=spacemit/CONFIG_DEFAULT_DEVICE_TREE.dtb path format correctly follows the vendor-prefixed device tree directory convention used by other boards (e.g., amlogic/). The patch properly standardizes SpacemiT K1 boot environment to use distro-boot variables (devtype, devnum, distro_bootpart) and address layout variables, which is consistent with the inclusion of <config_distro_bootcmd.h> and improves boot compatibility.

Comment on lines +28 to +67
@@ -747,7 +747,7 @@ void setenv_boot_mode(void)
u32 boot_mode = get_boot_mode();
switch (boot_mode) {
case BOOT_MODE_NAND:
- env_set("boot_device", "nand");
+ env_set("devtype", "nand");
break;
case BOOT_MODE_NOR:
char *blk_name;
@@ -758,23 +758,23 @@ void setenv_boot_mode(void)
return;
}

- env_set("boot_device", "nor");
- env_set("boot_devnum", simple_itoa(blk_index));
+ env_set("devtype", "nor");
+ env_set("devnum", simple_itoa(blk_index));
break;
case BOOT_MODE_EMMC:
- env_set("boot_device", "mmc");
- env_set("boot_devnum", simple_itoa(MMC_DEV_EMMC));
+ env_set("devtype", "mmc");
+ env_set("devnum", simple_itoa(MMC_DEV_EMMC));
break;
case BOOT_MODE_SD:
- env_set("boot_device", "mmc");
- env_set("boot_devnum", simple_itoa(MMC_DEV_SD));
+ env_set("devtype", "mmc");
+ env_set("devnum", simple_itoa(MMC_DEV_SD));
break;
case BOOT_MODE_USB:
// for fastboot image download and run test
env_set("bootcmd", CONFIG_BOOTCOMMAND);
break;
default:
- env_set("boot_device", "");
+ env_set("devtype", "");
break;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid stale ${devnum} when boot mode is unknown (and consider NAND semantics).
In the default: case (Line 62-65), devtype is cleared but devnum is left as-is, which can cause surprising device selection if later scripts use ${devnum} unconditionally.

Low-impact hardening:

 	default:
 		env_set("devtype", "");
+		env_set("devnum", "");
 		break;
🤖 Prompt for AI Agents
In patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch around
lines 28 to 67, the code clears devtype in the default case but leaves devnum
unchanged which can lead to stale ${devnum} usage; update the default: case to
also clear devnum (env_set("devnum", "")) and, for completeness, ensure
BOOT_MODE_NAND explicitly sets devnum to the intended value (either an explicit
number or an empty string) to avoid ambiguous NAND semantics.

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 Patches Patches related to kernel, U-Boot, ... size/large PR with 250 lines or more

Development

Successfully merging this pull request may close these issues.

2 participants