-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Modify radxa-e54c board config to fix leds, network and Gnome desktop init. #9111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Modify led setup section to use new led device names Add a keyfile /etc/NetworkManager/conf.d/99-unmanaged-devices.conf to set the internal ethernet, end1, used to connect to the internal switch chip to unmanaged. This interface doesn't accept dhcp IP addresses and Network Manager attempts to connect it via dhcp causing errors. A vendor kernel build with Gnome desktop attempts to use Wayland unsuccessfully causing the desktop to take over 10 minutes to initialise eventually using X11. Modify the board /usr/lib/armbian/armbian-firstlogin file to set WaylandEnable = false if the build is for a vendor kernel with a Gnome desktop.
WalkthroughNormalized Radxa E54C board config: removed Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this 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
🧹 Nitpick comments (1)
config/boards/radxa-e54c.conf (1)
62-72: **Verify the NetworkManager plugin name.**Based on the web search results, the plugin name is 'ifcfg-rh' for NetworkManager. Line 67 contains "ifconfig-rh" which appears to be a typo - it should be "ifcfg-rh".However, upon closer inspection of the configuration structure, the [keyfile] section with unmanaged-devices doesn't require the plugins line at all since the keyfile plugin is always enabled in NetworkManager. The unmanaged-devices directive will work regardless of the plugins specification.
The configuration creates /etc/NetworkManager/conf.d/99-unmanaged-devices.conf to mark the end1 interface as unmanaged, which correctly prevents NetworkManager from attempting DHCP on the internal switch interface.
Potential issue on Line 67: The plugin name "ifconfig-rh" appears to be a typo - the correct NetworkManager plugin name is "ifcfg-rh".
However, this may not cause actual issues because:
- The [keyfile] section and unmanaged-devices directive are handled by the keyfile plugin, which is always enabled by default
- The plugins line in this context may be redundant
Suggested improvement: Either correct the typo to "ifcfg-rh" or simplify by removing the plugins line entirely since the keyfile plugin handles the [keyfile] section by default.
🔎 Suggested fix
Option 1 - Correct the typo:
[main] - plugins=ifconfig-rh,keyfile + plugins=ifcfg-rh,keyfileOption 2 - Simplify (recommended):
[main] - plugins=ifconfig-rh,keyfile - [keyfile] unmanaged-devices=interface-name:end1
📜 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/boards/radxa-e54c.conf(5 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: igorpecovnik
Repo: armbian/build PR: 9087
File: .github/workflows/pr-check-pictures.yml:138-146
Timestamp: 2025-12-16T13:40:07.649Z
Learning: In the Armbian build repository, when introducing new requirements or checks (like the board assets verification workflow), the project prefers an initial educational-only period where violations post helpful PR comments and warnings but don't block merges. This allows contributors to become familiar with new requirements before enforcement is enabled (typically after ~6 months).
📚 Learning: 2025-11-02T20:49:56.719Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.719Z
Learning: In Armbian board configuration files (config/boards/*.conf, *.csc, etc.), do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__* functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Board-specific kernel modifications cause inconsistency in kernel packages published to the apt repository because boards within a family share the same kernel packages. Kernel configuration changes must be made in the appropriate kernel config file (e.g., config/kernel/linux-*-*.config) or in family configuration files (config/sources/families/*.conf, *.inc) instead.
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files, .wip and .conf extensions require BOARD_MAINTAINER to be present and contain at least one maintainer. If no maintainer is present, the board support rules state it does not qualify for standard support and must be moved to community support (.csc extension). This is documented at https://docs.armbian.com/User-Guide_Board-Support-Rules/
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-09-27T21:50:15.915Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-sunxi64-current.config:94-94
Timestamp: 2025-09-27T21:50:15.915Z
Learning: When kernel config files are generated through Armbian's automated process (previous config → armbian-kernel.sh changes → make oldconfig → Armbian machinery processing), manual config file edits are not appropriate since they would be overwritten. Deprecated option handling should be implemented in the automated tooling instead.
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-05-16T15:34:34.672Z
Learnt from: Grippy98
Repo: armbian/build PR: 8202
File: config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh:2-3
Timestamp: 2025-05-16T15:34:34.672Z
Learning: The shell scripts in the Armbian build system's desktop configuration files, such as config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh, rely on environment variables like $SRC and $destination that are expected to be set externally before the script is executed.
Applied to files:
config/boards/radxa-e54c.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/boards/radxa-e54c.conf
🔇 Additional comments (1)
config/boards/radxa-e54c.conf (1)
24-24: LGTM! LED device names updated for new vendor kernel.The device_name changes from
lan1@end1format tolan1format correctly reflect the new LED device naming in the latest vendor kernel, which should restore LED functionality.Also applies to: 33-33, 42-42, 51-51
Tested the suggestion and it works. Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
config/boards/radxa-e54c.conf (1)
63-72: Consider removing the RedHat-specific plugin from NetworkManager config.The NetworkManager configuration correctly marks
end1as unmanaged to prevent DHCP attempts on the internal switch interface. However, thepluginsline includesifconfig-rh, which is specific to RedHat/Fedora systems for reading ifcfg files. Since Armbian is Debian-based, this plugin is unnecessary.🔎 Simplified NetworkManager config
cat <<- EOF > "${destination}"/etc/NetworkManager/conf.d/99-unmanaged-devices.conf [main] - plugins=ifconfig-rh,keyfile + plugins=keyfile [keyfile] unmanaged-devices=interface-name:end1Alternatively, you can omit the
pluginsline entirely, as NetworkManager loads the keyfile plugin by default on Debian-based systems.
📜 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/boards/radxa-e54c.conf(5 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: igorpecovnik
Repo: armbian/build PR: 9087
File: .github/workflows/pr-check-pictures.yml:138-146
Timestamp: 2025-12-16T13:40:07.649Z
Learning: In the Armbian build repository, when introducing new requirements or checks (like the board assets verification workflow), the project prefers an initial educational-only period where violations post helpful PR comments and warnings but don't block merges. This allows contributors to become familiar with new requirements before enforcement is enabled (typically after ~6 months).
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: 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: djurny
Repo: armbian/build PR: 8235
File: packages/bsp/mvebu/helios4/helios4-wol.service:0-0
Timestamp: 2025-05-29T01:56:01.604Z
Learning: For ARM-based devices like Helios4 that use DeviceTree, the expected systemd network interface naming scheme is "end[0-9]+" (DeviceTree alias index), making the regex "^(eth|en[do])[0-9]+" appropriate as it covers traditional "eth", PCI on-board "eno", and DeviceTree "end" interfaces without unnecessary "enp" (PCI slot) support.
Learnt from: pyavitz
Repo: armbian/build PR: 9058
File: patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch:17-25
Timestamp: 2025-12-13T11:45:02.422Z
Learning: In the SpacemiT U-Boot patches for Armbian (patch/u-boot/legacy/u-boot-spacemit-k1/), the environment variable `devnum` is set to the device name string (e.g., "mmc", "nvme") rather than a numeric index, and `distro_bootpart` holds the partition number. This implementation aligns with mainline U-Boot conventions for the SpacemiT platform and has been verified to work correctly by the maintainer.
📚 Learning: 2025-11-02T20:49:56.719Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.719Z
Learning: In Armbian board configuration files (config/boards/*.conf, *.csc, etc.), do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__* functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Board-specific kernel modifications cause inconsistency in kernel packages published to the apt repository because boards within a family share the same kernel packages. Kernel configuration changes must be made in the appropriate kernel config file (e.g., config/kernel/linux-*-*.config) or in family configuration files (config/sources/families/*.conf, *.inc) instead.
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-12-01T02:34:37.451Z
Learnt from: tabrisnet
Repo: armbian/build PR: 9000
File: config/desktop/questing/environments/xfce/debian/postinst:17-18
Timestamp: 2025-12-01T02:34:37.451Z
Learning: In the Armbian build system shell scripts, the sed pattern with -i flag placed after the script argument (e.g., `sed "s/.../.../" -i file`) is intentional and correct. While non-POSIX, this GNU sed extension is used throughout the codebase and works correctly in the Armbian build environment. This pattern should not be flagged as incorrect.
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-09-27T21:50:15.915Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-sunxi64-current.config:94-94
Timestamp: 2025-09-27T21:50:15.915Z
Learning: When kernel config files are generated through Armbian's automated process (previous config → armbian-kernel.sh changes → make oldconfig → Armbian machinery processing), manual config file edits are not appropriate since they would be overwritten. Deprecated option handling should be implemented in the automated tooling instead.
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-11-20T18:20:11.985Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8968
File: patch/u-boot/u-boot-sunxi/arm64-dts-sun50i-h6-orangepi.dtsi-Rollback-r_rsb-to-r_i2c.patch:36-36
Timestamp: 2025-11-20T18:20:11.985Z
Learning: The rewrite-patches tool (REWRITE_PATCHES=yes) in the Armbian build system can inadvertently introduce semantic changes when the u-boot/kernel git base revision differs from expected state. The tool applies patches, commits them, and re-exports them using git format-patch, which can cause the re-exported patch to reflect the base revision's state rather than preserving the original patch intent. This is particularly problematic for device tree changes like interrupt specifications. The tool currently lacks validation mechanisms to detect such semantic drift, and affected patches must be manually corrected after rewriting.
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-05-16T15:34:34.672Z
Learnt from: Grippy98
Repo: armbian/build PR: 8202
File: config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh:2-3
Timestamp: 2025-05-16T15:34:34.672Z
Learning: The shell scripts in the Armbian build system's desktop configuration files, such as config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh, rely on environment variables like $SRC and $destination that are expected to be set externally before the script is executed.
Applied to files:
config/boards/radxa-e54c.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/boards/radxa-e54c.conf
📚 Learning: 2025-07-17T04:12:33.125Z
Learnt from: pyavitz
Repo: armbian/build PR: 8388
File: config/boards/kickpik2b.conf:1-14
Timestamp: 2025-07-17T04:12:33.125Z
Learning: In the Armbian build system, board configuration files (*.conf) follow a project standard of not including shebang lines, even though they contain bash code and are sourced by the build system. This is an established Armbian convention that individual contributors are expected to follow, and changes to this standard would require broader project maintainer approval.
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-12-16T13:40:07.649Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 9087
File: .github/workflows/pr-check-pictures.yml:138-146
Timestamp: 2025-12-16T13:40:07.649Z
Learning: In the Armbian build repository, when introducing new requirements or checks (like the board assets verification workflow), the project prefers an initial educational-only period where violations post helpful PR comments and warnings but don't block merges. This allows contributors to become familiar with new requirements before enforcement is enabled (typically after ~6 months).
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-08-30T06:48:09.091Z
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-08-30T06:48:09.091Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the user prefers flexible grep patterns over anchored ones for BTRFS configuration checks, but agrees to use quiet grep (-q) to avoid polluting build logs.
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-07-25T03:51:50.830Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub or the PR API to get the complete picture of what files are being added or modified.
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-10-03T11:42:46.660Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8705
File: .github/workflows/rewrite-kernel-configs.yml:96-169
Timestamp: 2025-10-03T11:42:46.660Z
Learning: In the Armbian build repository's kernel config rewrite workflow (.github/workflows/rewrite-kernel-configs.yml), deletion of kernel config files does not need to be handled. The workflow intentionally only captures ACMR (added/copied/modified/renamed) changes and does not need to propagate deletions to the aggregator.
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-07-25T03:51:50.830Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-06-24T10:08:40.313Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8328
File: lib/functions/compilation/patch/drivers_network.sh:542-545
Timestamp: 2025-06-24T10:08:40.313Z
Learning: In the Armbian build system, when a PR removes build support for a specific kernel version, version check issues for that removed version become practically irrelevant even if they appear incorrect in isolation. Context about which kernel versions are being deprecated/removed is important for understanding the impact of version-related code changes.
Applied to files:
config/boards/radxa-e54c.conf
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files, .wip and .conf extensions require BOARD_MAINTAINER to be present and contain at least one maintainer. If no maintainer is present, the board support rules state it does not qualify for standard support and must be moved to community support (.csc extension). This is documented at https://docs.armbian.com/User-Guide_Board-Support-Rules/
Applied to files:
config/boards/radxa-e54c.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/boards/radxa-e54c.conf
🔇 Additional comments (2)
config/boards/radxa-e54c.conf (2)
24-24: LED device names correctly updated for new kernel.The removal of the
@end1suffix from the LED device_name entries aligns with the PR objective to support the latest vendor kernel's device naming. For boards with DSA (Distributed Switch Architecture), the interface names lan1, lan2, lan3, and wan are standard.Also applies to: 33-33, 42-42, 51-51
74-76: Wayland disable logic is appropriate for the use case.The conditional correctly gates the Wayland disable to vendor kernel + GNOME desktop builds only. As discussed in previous review comments, this modification is board-specific and addresses the PR objective to prevent long Gnome desktop initialization delays.
| invert=0 | ||
| EOF | ||
|
|
||
| # Internal Ethernet interface can't get an IP address and shouldn't be managed by NetworkManager so make it unmanaged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if systemd-network is used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if systemd-network is used?
It's only NetworkManager that tries to initialize end1 by DHCP and causes boot issues.
The keyfile has no effect on systemd-network, but the images boot okay without network, and all interfaces can then be configured normally. This seems okay for minimal images where we can expect slightly more savvy Linux users anyway.
Description
Modify led setup section of the radxa-e54c board config to use new led device names.
Modify the radxa-e54c board config to add a keyfile /etc/NetworkManager/conf.d/99-unmanaged-devices.conf on the board to set the internal ethernet, end1, used to connect to the internal switch chip to unmanaged. This interface doesn't accept DHCP IP addresses and Network Manager attempts to connect it via DHCP causing errors.
Modify the radxa-e54c board config to modify the boards /usr/lib/armbian/armbian-firstlogin file to set WaylandEnable = false if the build is for a vendor kernel with a Gnome desktop to prevent vendor kernel build with Gnome desktop attempting to use Wayland unsuccessfully causing the desktop to take over 10 minutes to initialize eventually using X11.
Jira reference number AR-2801
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
Bug Fixes
Chores
These changes improve network status clarity and first‑login reliability.
✏️ Tip: You can customize this high-level summary in your review settings.