Add HDD park at shutdown for Odroid HC4 #9214
Conversation
Odroid HC4 is a NAS. Needs to park HDD heads prior to reboot or shutdown.
📝 WalkthroughWalkthroughA new shell function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
Hey @ean365! 👋Thanks for submitting your first pull request to the Armbian project — we're excited to have you contributing! 🧡 If you'd like to stay informed about project updates or collaborate more closely with the team, Also, don’t forget to ⭐ star the repo if you haven’t already — and welcome aboard! 🚀 |
github-actions
bot
added
size/small
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
config/sources/families/meson-sm1.conf (1)
20-26: Excellent approach: reusing existing shutdown script for HC4.The implementation correctly gates HDD parking functionality to the odroidhc4 board and follows proper systemd shutdown hook conventions. The permissions (0755) and ownership (root:root) are appropriate. The
family_tweaks_bsp()function is automatically invoked during BSP package creation,$destinationis properly initialized in the build context, and theodroid.shutdownscript exists and is compatible with HC4's SATA hardware.Optional: Add defensive quoting for path variables
While the current implementation works correctly, quoting variables is a defensive coding best practice for shell scripts:
Suggested improvement
family_tweaks_bsp() { if [[ $BOARD == odroidhc4 ]]; then # park HDD heads on shutdown - mkdir -p $destination/lib/systemd/system-shutdown - install -o root -g root -m 0755 $SRC/packages/bsp/odroid/odroid.shutdown $destination/lib/systemd/system-shutdown/odroid.shutdown + mkdir -p "$destination/lib/systemd/system-shutdown" + install -o root -g root -m 0755 "$SRC/packages/bsp/odroid/odroid.shutdown" "$destination/lib/systemd/system-shutdown/odroid.shutdown" fi }
📜 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.
📒 Files selected for processing (1)
config/sources/families/meson-sm1.conf
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
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: igorpecovnik
Repo: armbian/build PR: 8720
File: lib/functions/rootfs/distro-specific.sh:38-47
Timestamp: 2025-11-09T22:30:27.163Z
Learning: In lib/functions/rootfs/distro-specific.sh, the systemd sleep.conf.d override that disables suspend/hibernation is intentionally applied system-wide to all Armbian images (desktop, CLI, and minimal), not gated to desktop-only builds, because suspend/resume is fragile on most boards.
📚 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/sources/families/meson-sm1.conf
📚 Learning: 2025-11-09T22:30:27.163Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8720
File: lib/functions/rootfs/distro-specific.sh:38-47
Timestamp: 2025-11-09T22:30:27.163Z
Learning: In lib/functions/rootfs/distro-specific.sh, the systemd sleep.conf.d override that disables suspend/hibernation is intentionally applied system-wide to all Armbian images (desktop, CLI, and minimal), not gated to desktop-only builds, because suspend/resume is fragile on most boards.
Applied to files:
config/sources/families/meson-sm1.conf
📚 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:
config/sources/families/meson-sm1.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/sources/families/meson-sm1.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/sources/families/meson-sm1.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/sources/families/meson-sm1.conf
📚 Learning: 2025-08-03T15:21:20.148Z
Learnt from: pyavitz
Repo: armbian/build PR: 8455
File: config/sources/families/sun50iw1.conf:19-24
Timestamp: 2025-08-03T15:21:20.148Z
Learning: In the Armbian build system, when copying firmware files during family_tweaks_s(), use /lib/firmware/updates/ instead of /lib/firmware/ to avoid conflicts with the Armbian firmware package. The /lib/firmware/updates directory takes precedence in Linux firmware loading hierarchy and is the proper location for user-installed firmware files.
Applied to files:
config/sources/families/meson-sm1.conf
📚 Learning: 2025-12-16T12:22:20.156Z
Learnt from: tabrisnet
Repo: armbian/build PR: 9085
File: lib/functions/rootfs/rootfs-create.sh:303-306
Timestamp: 2025-12-16T12:22:20.156Z
Learning: The post_debootstrap_customize hook concept in lib/functions/rootfs/rootfs-create.sh has been tested in PR #9000. The hook placement after package installations (including desktop packages) and before cleanup operations (autoremove, qemu undeploy) is validated as appropriate for rootfs customization.
Applied to files:
config/sources/families/meson-sm1.conf
📚 Learning: 2025-09-07T17:39:32.272Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8586
File: config/boards/nanopi-r76s.conf:15-21
Timestamp: 2025-09-07T17:39:32.272Z
Learning: In the Armbian build system, the variables $BOARD and $SDCARD are always set by the build framework, so guard checks for these variables are unnecessary in board configuration files and hook functions.
Applied to files:
config/sources/families/meson-sm1.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/sources/families/meson-sm1.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/meson-sm1.conf
📚 Learning: 2025-12-12T23:09:56.813Z
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 Armbian build configs for vendor kernel sources, prefer the following branch naming conventions: use 'vendor' or 'vendor-rt' for stable vendor releases, and 'vendor-edge' for bleeding-edge/pre-release vendor versions. The 'edge' naming without the 'vendor-' prefix is reserved for mainline kernel branches. Apply this pattern to family config files under config/sources/families (e.g., spacemit.conf) to ensure consistent vendor kernel sourcing naming across the repository.
Applied to files:
config/sources/families/meson-sm1.conf
rpardini
left a comment
rpardini
left a comment
There was a problem hiding this comment.
Nice, but please do board-specific stuff in the board file, not the family file.
You can use a hook called post_family_tweaks_bsp.
|
Also perhaps replacing the script with something like this (needs testing): |
|
Thanks. @rpardini, are you saying that it should be added to odroidhc4.csc instead? I put it in meson-sm1.conf because I'm not totally familiar with Armbian practice, thought it needed to be in a conf file, and because I already saw multiple device specific stuff in there. I can switch it into the .csc, if that is what you are recommending?? @igorpecovnik, I was trying to reuse the existing odroid.shutdown script, since it is used by the XU4 as well, and it is working. Do you see a big difference between them? If so, and you prefer the new one above, then I can put in a separate PR to update the script as you suggest. I have been chasing/fixing this issue all night into the wee hours. I'm going to bed, but will check your responses tomorrow (well later today). |
Correct. The board config file can also contain hooks to modify aspects. Hooks in the board family file (meson-sm1.conf in this case) would affect all boards depending on this family config (like odroid n2 for example). |
less dependencies. |
|
Hmmm. I hadn't realized it, but it looks like It's strange that the kernel driver for the HDD doesn't have this built-in -- total miss on that. Also, I'm not quite sure what you meant by moving to extensions, you have way more to say in how you want to structure the project, but it seems like it is part of bsp to me, where it has been for XU4. |
|
Hey, it's me again. As you noted, there's two parts to this: one script laying around somewhere, and the board-related code (legacy "tweaks", which is usually family code) that activates it. "Extensions" are a mechanism where we can define both in a single place (using heredocs, or Ref "minimal images don't have xxx": then don't do xxx for minimals. With an extension you can test for BUILD_MINIMAL (or whatever) and complain/warn/break/avoid-it etc. |
Yes. So the script looks for its existence and only then proceed. mdadm is thus not a dependency and nor require package if you don't intend to have raid setup.
This is just an idea as parking HDD is not something that is only tied to those boards. Extension is clean(er) way for sorting out those things. As with next board that might need this kind of functionality its only one line of code: Example: IMO if this part is getting attention, lets do it better? |
|
Such an improvement would benefit both official NAS — Helios4, Helios64, ODroidHC4, and other pets that use USB rotational drives. Will it work for USB drives? |
If by "USB drives" you mean USB connected HDDs, then yes I think it should work just the same to command them to park the heads (idle-immediate), but of course this implementation would need to be tested to be certain. |
There's only so much one can do as what SATA command is emitted by the USB controller is up to the USB controller. But yes, one can send |
|
As part of testing the other PR, I reworked the odroid.shutdown script into some #!/bin/bash
set -euo pipefail
declare LOG_PREFIX="sata.shutdown"
declare -i DRY_RUN=0
log_kmsg() {
declare msg="$1"
echo "${LOG_PREFIX}: ${msg}" > /dev/kmsg
}
sleep_and_log() {
declare -i wait="$1"
if ((wait > 0)); then
log_kmsg "Sleeping for ${wait} seconds."
fi
((DRY_RUN)) || sleep "${wait}"
return 0
}
[[ "${1:-}" == "--dry-run" ]] && DRY_RUN=1 && log_kmsg "Dry run mode enabled."
log_kmsg "Starting shutdown script (DRY_RUN==${DRY_RUN})."
export LANG=C LC_ALL=C
log_kmsg "Syncing filesystems."
((DRY_RUN)) && log_kmsg "[dry-run] Would sync filesystems." || sync || log_kmsg "Warning: sync failed."
if command -v mdadm &> /dev/null; then
log_kmsg "Waiting for mdadm arrays to clean."
((DRY_RUN)) && log_kmsg "[dry-run] Would run: mdadm --wait-clean --scan." || mdadm --wait-clean --scan || log_kmsg "Warning: mdadm --wait-clean failed."
else
log_kmsg "mdadm not found, skipping RAID clean."
fi
if ! command -v hdparm &> /dev/null; then
log_kmsg "hdparm not found, skipping disk parking."
exit 1
fi
log_kmsg "Parking SATA disks."
for dev in /sys/block/sd*; do
declare -i wait=0
if [[ ! -e "${dev}" ]]; then
log_kmsg "Could not find ${dev}, skipping."
continue
fi
declare dev_name="${dev##*/}"
declare dev_path="/dev/${dev_name}"
declare mounted=0 # Check if device or any partition is mounted using findmnt
if findmnt -rn -S "${dev_path}" > /dev/null; then
log_kmsg "Device ${dev_path} is mounted (findmnt). Will skip kernel delete."
mounted=1
else
for part in "/dev/${dev_name}"[0-9]*; do
[[ -b "${part}" ]] || continue
if findmnt -rn -S "${part}" > /dev/null; then
log_kmsg "Partition ${part} is mounted (findmnt). Will skip kernel delete for ${dev_path}."
mounted=1
break
fi
done
fi
if ((DRY_RUN)); then
log_kmsg "[dry-run] Would run: hdparm -y ${dev_path}"
log_kmsg "[dry-run] Would set wait=2."
wait=2
else
if hdparm -y "${dev_path}"; then
log_kmsg "Disk ${dev_path} parked."
wait=2
else
log_kmsg "Warning: hdparm failed for ${dev_path}"
fi
fi
if ((mounted)); then
log_kmsg "Mounted filesystems detected on ${dev_path} or its partitions. Skipping kernel delete."
elif ((DRY_RUN)); then
log_kmsg "[dry-run] Would run: echo 1 > /sys/class/block/${dev_name}/device/delete"
log_kmsg "[dry-run] Would log: Disk ${dev_path} deleted from kernel."
else
sleep_and_log "${wait}"
log_kmsg "Deleting disk ${dev_path} from kernel /sys/class/block/${dev_name}/device/delete ..."
if echo 1 > "/sys/class/block/${dev_name}/device/delete"; then
log_kmsg "Disk ${dev_path} deleted from kernel."
else
log_kmsg "Warning: failed to delete ${dev_path} from kernel."
fi
fi
sleep_and_log "${wait}"
done
log_kmsg "Shutdown script completed."
exit 0This produces serial console output similar to |
|
It's so strange to learn about the need to park disk heads, when the last time I saw this was before the Windows '95 era... Although back then it was common knowledge, and everyone always gave the command to park disks before turning off their PCs. |
|
I remember. Old HDDs in 80s-early-90s absolutely needed to be parked. Later, the automatic/emergency parking feature was added using stored energy. Even though, in modern times, it works fairly reliably and has been optimized, it is still nonetheless considered an emergency, somewhat "violent", and something to be avoided -- give it a warning and time to do it nicely, if possible... Linux ATA driver (and most modern OSs) still does send So, I think the issue in this case might just be that after sending the UPDATE: Actually, it is starting to look like Linux ATA driver only sends a |
| if [[ $BOARD == odroidhc4 ]]; then | ||
| # park HDD heads on shutdown | ||
| mkdir -p $destination/lib/systemd/system-shutdown | ||
| install -o root -g root -m 0755 $SRC/packages/bsp/odroid/odroid.shutdown $destination/lib/systemd/system-shutdown/odroid.shutdown |
There was a problem hiding this comment.
this feels like it should be a more generic BSP thing, possibly as an extension.
Not so much this function [although yes] as much as the packages/bsp/odroid/odroid.shutdown script. it should be something like packages/bsp/common/park-hdds.shutdown
It can't be that HC4 is the only candidate. e.g. odroidm1 has SATA. or rock-5-itx, or well over a dozen others.
tabris@brunnt:~/build/armbian-build$ egrep -l '^#.*SATA' config/boards/*.{csc,conf}|wc -l
35
That's not exactly a reason to not do this PR as much as to consider another potentially larger PR.
@coderabbitai
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Read the odroid.shutdown script to understand what it does
echo "=== Contents of packages/bsp/odroid/odroid.shutdown ==="
cat packages/bsp/odroid/odroid.shutdown
echo -e "\n\n=== Boards with SATA mentioned in comments ==="
# Find boards with SATA mentioned
grep -l 'SATA' config/boards/*.{csc,conf} 2>/dev/null | head -20
echo -e "\n\n=== Check for other shutdown scripts ==="
# Look for other shutdown scripts in BSP packages
find packages/bsp -name "*.shutdown" -type f
echo -e "\n\n=== Check if any other families install shutdown scripts ==="
# Search for system-shutdown installations in family configs
rg -n 'system-shutdown' config/sources/families/ -C 2
Length of output: 2582
@tabrisnet Excellent architectural observation! After examining the script, I can confirm your intuition is absolutely correct.
The odroid.shutdown script is completely generic — it contains zero Odroid-specific logic:
- Syncs filesystems
- Waits for mdadm arrays to reach clean state
- Parks all SATA disks via
/sys/block/sd*pattern using hdparm - Cleans up block device references
The "odroid" naming is misleading and historical. This script could benefit any of the 35+ SATA-capable boards you identified.
Suggested refactoring approach:
-
Rename and relocate:
packages/bsp/odroid/odroid.shutdown→packages/bsp/common/park-sata-disks.shutdown -
Opt-in mechanism via board/family configs:
ENABLE_SATA_PARKING="yes" # Set in board .csc or family .conf
-
Generic BSP installation in a common hook that checks the flag and installs for any board requesting it
-
Update existing boards: odroidxu4, odroidhc4, and potentially odroidm1s, rock-5-itx, helios64, and others
Would you like me to create a GitHub issue to track this refactoring work? The current PR is fine as a tactical fix, but this deserves a proper architectural solution for the broader ecosystem.
🧠 Learnings used
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: igorpecovnik
Repo: armbian/build PR: 8720
File: lib/functions/rootfs/distro-specific.sh:38-47
Timestamp: 2025-11-09T22:30:27.163Z
Learning: In lib/functions/rootfs/distro-specific.sh, the systemd sleep.conf.d override that disables suspend/hibernation is intentionally applied system-wide to all Armbian images (desktop, CLI, and minimal), not gated to desktop-only builds, because suspend/resume is fragile on most boards.
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: pyavitz
Repo: armbian/build PR: 8455
File: config/sources/families/sun50iw1.conf:19-24
Timestamp: 2025-08-03T15:21:20.148Z
Learning: In the Armbian build system, when copying firmware files during family_tweaks_s(), use /lib/firmware/updates/ instead of /lib/firmware/ to avoid conflicts with the Armbian firmware package. The /lib/firmware/updates directory takes precedence in Linux firmware loading hierarchy and is the proper location for user-installed firmware files.
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: 0
File: :0-0
Timestamp: 2025-11-10T23:44:53.363Z
Learning: For the RV1106 family in Armbian (config/sources/families/rockchip-rv1106.conf), udev.children-max=1 is set in the kernel boot parameters to prevent early-boot OOM errors. The RV1106 has a single-core CPU, so parallelism in udev child processes provides no practical benefit. Testing higher values is impractical due to the difficulty of monitoring at this early boot stage.
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: 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: 0
File: :0-0
Timestamp: 2025-12-19T13:56:45.124Z
Learning: When reviewing kernel or u-boot version bump PRs in the Armbian build system, check if patches existed in previous kernel version directories (e.g., sunxi-6.12, sunxi-6.13) before describing them as new features. If a patch and the majority of its contents existed previously with no major functionality changes, focus the review on the actual changes: the version bump itself and patch compatibility adjustments. Don't describe existing patches being ported/maintained across versions as new features or drivers—this is misleading. The patches are existing code being re-aligned to work with the new upstream version.
Learnt from: rpardini
Repo: armbian/build PR: 9159
File: patch/u-boot/u-boot-genio/0026-dts-configs-add-Grinn-GenioSBC-510.patch:161-161
Timestamp: 2026-01-03T20:46:29.189Z
Learning: For the Armbian genio family (config/sources/families/genio.conf and patch/u-boot/u-boot-genio/), when reviewing PRs that include vendor U-Boot patches from Collabora, avoid flagging potential issues in board configurations that are out of scope for the PR's primary focus (e.g., don't flag Genio 510/700 board issues when the PR is focused on radxa-nio-12l/Genio 1200). The maintainer prioritizes keeping vendor patches close to upstream for easier re-copying and maintenance, even if secondary board configs have potential mismatches.
<!-- </add_learning>
There was a problem hiding this comment.
I agree. It probably belongs as a generic extension for any system that needs or wants it, and not just limited to the Odroid. I can update this PR to make it generic. We were already going to be moving it to an extension. @rpardini, @igorpecovnik, thoughts?
Also, as a better long-term fix, I am looking at patching Linux shutdown/restart and device driver -- after I confirm the issues I outlined above... Whereby it looks like Linux does send a standby-immediate to HDDs in order to park their heads, but only during shutdown and not during a restart. (Probably add something like ata_standby_on_restart feature.) Since Linux was written mostly for the big iron machines, I think they assumed that the power control hardware and boot logic would always be sophisticated enough to leave everything powered on and spinning through a restart -- makes perfect sense in a perfect world -- but is unfortunately not always the case, especially with some of these smaller/cheaper SBCs.
There was a problem hiding this comment.
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!
6 participants
Odroid HC4 is a NAS. Needs to park HDD heads prior to reboot or shutdown. This simple PR adds the already existing odroid.shutdown bsp script (originally used for the XU4) to the meson-sm1.conf file for the HC4.
I have successfully tested the odroid.shutdown script on the HC4 with HDDs, and it properly parks the heads. Without this, the heads are forced to do an emergency retract, which is a violent action that degrades the HDD over time.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.