-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
BPI-R4/filogic - kernel drivers into modules #8528
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
Conversation
WalkthroughThe PR updates the linux-filogic-current kernel config file, converting many previously built-in (y) options to modules (m) and adding several new module entries. Changes affect Bluetooth, RFKILL, block devices (loop, nbd, ram, sd), ATA, multiple PHY drivers, filesystems (XFS, BTRFS, F2FS, ISO9660, UDF, NTFS, NTFS3), network/filesystem servers (NFS, NFSv4, NFSD, CIFS, SMB server), FUSE/CCUSE, and networking utilities (WireGuard, IFB, NetConsole, TUN, VXLAN). Some lines were reformatted without semantic change. No source code or public API changes—only CONFIG symbol edits in a single kernel config file. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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 (4)
config/kernel/linux-filogic-current.config (4)
254-261: Modularizing Bluetooth and RFKILL looks fine; confirm early userspace expectations.Moving CONFIG_BT, CONFIG_RFKILL_FULL, and CONFIG_RFKILL_GPIO to modules aligns with the goal of shrinking the kernel. Just ensure nothing in the initramfs or very-early boot expects rfkill to be available before modules load (typical Armbian images are fine, but double-check custom images).
If you ever need rfkill instantly after pivot (or in initramfs), add rfkill to the initramfs module list or to modules-load.d.
317-324: WG/IFB/TUN as modules are good; netconsole as a module can’t do true early-boot logging.
- WireGuard, IFB, TUN to =m: Good call; reduces image size without impacting normal operation.
- NETCONSOLE=m (Line 322): If you want kernel messages before userspace (or before initramfs module loading), netconsole must be built-in. As a module, it only starts after module load.
Apply one of the following based on your intent:
-CONFIG_NETCONSOLE=m +CONFIG_NETCONSOLE=yOr keep it modular and document/initramfs-include it for early load.
Confirm whether early-boot netconsole is desired on R4. If yes, switch to =y or ensure initramfs loads it before cmdline processing.
544-547: PR objective vs. config: Btrfs remains built-in.The PR description mentions modularizing Btrfs, but CONFIG_BTRFS_FS is still =y (Line 545). If that’s intentional, clarify; otherwise switch to =m to align with the stated goal.
-CONFIG_BTRFS_FS=y +CONFIG_BTRFS_FS=mConfirm whether Btrfs should be modular on R4.
Also applies to: 563-566
120-121: Heads-up: ZBUD is deprecated in newer kernels; let Kconfig pick a modern zpool.Not part of this PR’s changes, but CONFIG_ZSWAP_ZPOOL_DEFAULT_ZBUD (Line 120) has been deprecated since 6.13 and slated for removal around 6.15. On newer kernels, defconfig tends to auto-correct. Consider switching the default to zsmalloc or accepting Kconfig’s default.
-CONFIG_ZSWAP_ZPOOL_DEFAULT_ZBUD=y +# Prefer zsmalloc (implicit via Kconfig) or set explicitly if available: +# CONFIG_ZSWAP_ZPOOL_DEFAULT_ZBUD is not setWhich kernel version does linux-filogic-current track right now? If ≥ 6.13, expect this to be dropped automatically; otherwise, consider pre-emptive cleanup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
config/kernel/linux-filogic-current.config(5 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: rafayahmed317
PR: armbian/build#8484
File: config/kernel/linux-rk35xx-vendor.config:904-906
Timestamp: 2025-08-11T22:00:13.411Z
Learning: In the linux-rk35xx-vendor kernel, the LTE driver options (CONFIG_LTE, CONFIG_LTE_RM310, CONFIG_LTE_EM05) are defined as boolean in drivers/net/lte/Kconfig and cannot be built as modules - they must be either built-in (=y) or disabled.
📚 Learning: 2025-08-11T22:00:13.411Z
Learnt from: rafayahmed317
PR: armbian/build#8484
File: config/kernel/linux-rk35xx-vendor.config:904-906
Timestamp: 2025-08-11T22:00:13.411Z
Learning: In the linux-rk35xx-vendor kernel, the LTE driver options (CONFIG_LTE, CONFIG_LTE_RM310, CONFIG_LTE_EM05) are defined as boolean in drivers/net/lte/Kconfig and cannot be built as modules - they must be either built-in (=y) or disabled.
Applied to files:
config/kernel/linux-filogic-current.config
📚 Learning: 2025-07-26T11:14:41.697Z
Learnt from: pyavitz
PR: armbian/build#8421
File: config/kernel/linux-sunxi64-edge.config:82-83
Timestamp: 2025-07-26T11:14:41.697Z
Learning: In Linux kernel 6.13 and later, CONFIG_ZBUD was deprecated and is scheduled for removal in kernel 6.15. The zbud compressed page allocator was found to consume more memory than alternatives like zsmalloc. Therefore, CONFIG_ZSWAP_ZPOOL_DEFAULT_ZBUD becomes obsolete in current kernels, and make defconfig will auto-correct by removing such deprecated options. This demonstrates how kernel defconfigs can be backwards compatible but not forward compatible.
Applied to files:
config/kernel/linux-filogic-current.config
📚 Learning: 2025-07-27T13:03:53.146Z
Learnt from: pyavitz
PR: armbian/build#8421
File: config/kernel/linux-sunxi64-edge.config:2664-2671
Timestamp: 2025-07-27T13:03:53.146Z
Learning: In Linux kernel configuration for the Armbian build system, when CONFIG_EXT2_FS and CONFIG_EXT3_FS are both set to =y (built-in), CONFIG_EXT4_FS is automatically enabled/hard-coded and does not need to be explicitly specified in the configuration file. This dependency relationship means that EXT4 support is implicitly included when EXT2 and EXT3 are built into the kernel.
Applied to files:
config/kernel/linux-filogic-current.config
📚 Learning: 2025-07-27T13:03:53.146Z
Learnt from: pyavitz
PR: armbian/build#8421
File: config/kernel/linux-sunxi64-edge.config:2664-2671
Timestamp: 2025-07-27T13:03:53.146Z
Learning: In the Armbian build system's kernel configuration, when CONFIG_EXT2_FS=y and CONFIG_EXT3_FS=y are both set to built-in, EXT4 filesystem support becomes automatically available/hard-coded without requiring an explicit CONFIG_EXT4_FS line in the configuration file. This is specific to how Armbian handles kernel configuration and differs from typical standalone Linux kernel configuration behavior.
Applied to files:
config/kernel/linux-filogic-current.config
🔇 Additional comments (5)
config/kernel/linux-filogic-current.config (5)
305-313: Block device drivers as modules: check boot and rescue workflows.
- Loop/NBD/RAM to =m (Lines 305–307): OK for runtime use, not required for boot. If you rely on ramdisk for rescue or on NBD for netboot in some scenarios, these must be present in the initramfs.
- BLK_DEV_SD to =m (Line 310): Safe for eMMC/NVMe boot; required only for USB/SCSI disks. Keep in mind it’s needed for USB rootfs.
- ATA to =m (Line 312): R4 has no SATA; module is appropriate.
Do any supported BPI-R4 images boot from USB or NBD? If yes, add the relevant modules to initramfs for those variants.
544-555: XFS, F2FS, FUSE/CUSE modularization: OK; ensure autoload for userland FUSE.
- XFS=m, F2FS=m: Good tradeoff for non-root filesystems.
- FUSE/ CUSE=m: Fine; /dev/fuse typically triggers autoload. Make sure userspace has fuse3 installed and udev module autoloading is intact.
If you expect to mount F2FS/XFS from initramfs (rare), add them to the initramfs module list.
558-559: ISO9660/UDF to modules: sensible unless ISO/UDF root or very early mounts are used.Looks good; reduces kernel size while keeping removable media support available post-boot.
Confirm no installer or recovery flow requires ISO/UDF during initramfs.
587-591: Network filesystems and servers as modules: good; validate netboot scenarios.
- NFS client/server (NFS, NFS_V4, NFSD) and CIFS/SMB server to =m: Matches the “nonessential for boot” rationale. Keep in mind NFS root requires client built-in or present in initramfs.
If any deployment uses NFS root or early CIFS mounts, add these to initramfs.
341-347: No change needed: HWMON is a bool, not a module
The review’s suggestion to changeCONFIG_REALTEK_PHY_HWMON=yto=mis invalid becauseREALTEK_PHY_HWMONis defined as a Boolean (built-in only) symbol, not a tristate. def_bool in the Kconfig enforces it can only be ‘y’ or ‘n’—it cannot be ‘m’ (patchwork.kernel.org, cateee.net).Likely an incorrect or invalid review comment.
540d41c to
c04e5fe
Compare
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
♻️ Duplicate comments (1)
config/kernel/linux-filogic-current.config (1)
563-565: Still carrying both NTFS3 and legacy NTFS; consider disabling legacy NTFS to reduce footprint.You already converted NTFS3 to a module—good. Keeping legacy NTFS in parallel is usually unnecessary unless you have specific read-only/compat scenarios. Prior review suggested removing it; repeating here for convenience.
Suggested diff:
CONFIG_NTFS3_FS=m CONFIG_NTFS3_FS_POSIX_ACL=y -CONFIG_NTFS_FS=m +# CONFIG_NTFS_FS is not set
🧹 Nitpick comments (2)
config/kernel/linux-filogic-current.config (2)
254-254: Bluetooth/RFKILL modularization is reasonable; consider disabling entirely to save more space if unused on R4.If the BPI-R4 platform/DTBs in this family have no BT controller and you don’t plan to support BT dongles out of the box, you can disable BT and RFKILL to shave a bit more kernel and initramfs size. Otherwise, keeping them as modules is fine.
Apply if you want to disable them:
-CONFIG_BT=m -CONFIG_RFKILL_FULL=m -CONFIG_RFKILL_GPIO=m +# CONFIG_BT is not set +# CONFIG_RFKILL_FULL is not set +# CONFIG_RFKILL_GPIO is not setAlso applies to: 260-261
119-121: ZSWAP default zpool is zbud; prefer zsmalloc for efficiency and forward-compat.Based on recent learnings, zbud is deprecated in newer kernels and is less memory-efficient than zsmalloc. Switching now improves behavior and future-proofs configs as kernels advance.
Apply:
CONFIG_ZSWAP=y -CONFIG_ZSWAP_ZPOOL_DEFAULT_ZBUD=y +# CONFIG_ZSWAP_ZPOOL_DEFAULT_ZBUD is not set +CONFIG_ZSWAP_ZPOOL_DEFAULT_ZSMALLOC=yContext: Retrieved learning notes indicate CONFIG_ZBUD deprecation in newer kernels, and defconfig auto-correct behavior as kernels evolve.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
config/kernel/linux-filogic-current.config(5 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: rafayahmed317
PR: armbian/build#8484
File: config/kernel/linux-rk35xx-vendor.config:904-906
Timestamp: 2025-08-11T22:00:13.411Z
Learning: In the linux-rk35xx-vendor kernel, the LTE driver options (CONFIG_LTE, CONFIG_LTE_RM310, CONFIG_LTE_EM05) are defined as boolean in drivers/net/lte/Kconfig and cannot be built as modules - they must be either built-in (=y) or disabled.
📚 Learning: 2025-08-11T22:00:13.411Z
Learnt from: rafayahmed317
PR: armbian/build#8484
File: config/kernel/linux-rk35xx-vendor.config:904-906
Timestamp: 2025-08-11T22:00:13.411Z
Learning: In the linux-rk35xx-vendor kernel, the LTE driver options (CONFIG_LTE, CONFIG_LTE_RM310, CONFIG_LTE_EM05) are defined as boolean in drivers/net/lte/Kconfig and cannot be built as modules - they must be either built-in (=y) or disabled.
Applied to files:
config/kernel/linux-filogic-current.config
📚 Learning: 2025-07-26T11:14:41.697Z
Learnt from: pyavitz
PR: armbian/build#8421
File: config/kernel/linux-sunxi64-edge.config:82-83
Timestamp: 2025-07-26T11:14:41.697Z
Learning: In Linux kernel 6.13 and later, CONFIG_ZBUD was deprecated and is scheduled for removal in kernel 6.15. The zbud compressed page allocator was found to consume more memory than alternatives like zsmalloc. Therefore, CONFIG_ZSWAP_ZPOOL_DEFAULT_ZBUD becomes obsolete in current kernels, and make defconfig will auto-correct by removing such deprecated options. This demonstrates how kernel defconfigs can be backwards compatible but not forward compatible.
Applied to files:
config/kernel/linux-filogic-current.config
📚 Learning: 2025-07-27T13:03:53.146Z
Learnt from: pyavitz
PR: armbian/build#8421
File: config/kernel/linux-sunxi64-edge.config:2664-2671
Timestamp: 2025-07-27T13:03:53.146Z
Learning: In Linux kernel configuration for the Armbian build system, when CONFIG_EXT2_FS and CONFIG_EXT3_FS are both set to =y (built-in), CONFIG_EXT4_FS is automatically enabled/hard-coded and does not need to be explicitly specified in the configuration file. This dependency relationship means that EXT4 support is implicitly included when EXT2 and EXT3 are built into the kernel.
Applied to files:
config/kernel/linux-filogic-current.config
📚 Learning: 2025-07-27T13:03:53.146Z
Learnt from: pyavitz
PR: armbian/build#8421
File: config/kernel/linux-sunxi64-edge.config:2664-2671
Timestamp: 2025-07-27T13:03:53.146Z
Learning: In the Armbian build system's kernel configuration, when CONFIG_EXT2_FS=y and CONFIG_EXT3_FS=y are both set to built-in, EXT4 filesystem support becomes automatically available/hard-coded without requiring an explicit CONFIG_EXT4_FS line in the configuration file. This is specific to how Armbian handles kernel configuration and differs from typical standalone Linux kernel configuration behavior.
Applied to files:
config/kernel/linux-filogic-current.config
🧬 Code graph analysis (1)
config/kernel/linux-filogic-current.config (1)
lib/functions/compilation/armbian-kernel.sh (3)
armbian_kernel_config__disable_various_options(70-100)kernel_config_set_m(368-378)armbian_kernel_config_apply_opts_from_arrays(418-448)
🔇 Additional comments (4)
config/kernel/linux-filogic-current.config (4)
341-347: PHY drivers modularized while keeping PCS_MTK_USXGMII=y: LGTM.Shifting ICPlus/MaxLinear/Realtek PHYs to modules is consistent with the “modularize nonessential” goal, and keeping PCS_MTK_USXGMII built-in is prudent for early phylink bring-up on filogic. Realtek PHY HWMON=y with REALTEK_PHY=m is fine (the bool compiles into the module).
544-559: XFS/Btrfs/F2FS/ISO9660/UDF to modules: aligns with ext4-root default.This saves vmlinuz space without impacting the common ext4-root path. Make sure the installer/initramfs pulls these modules when needed for removable media.
317-323: Confirm early-boot netconsole requirementsThe
rgsearch returned onlyCONFIG_NETCONSOLEentries in various kernel config fragments (mostly=mwithCONFIG_NETCONSOLE_DYNAMIC=y, and a few=y), but no occurrences of anetconsole=kernel-cmdline invocation in any boot or init scripts. As noted, building Netconsole as a module will not capture very-early boot or panic messages.• Location:
config/kernel/linux-filogic-current.configlines 317–323 –CONFIG_NETCONSOLE=m
• Repository-wide grep: nonetconsole=parameters found in bootloader or init scriptsIf you do require Netconsole to start tracing before module loading (e.g., for panic dumps), you must:
- Build it-in (
CONFIG_NETCONSOLE=y), not as=m.- Add the appropriate
netconsole=argument to your kernel command line (in your bootloader or initramfs).Please confirm whether early-kernel Netconsole logging is a necessity or if the current module-based setup meets your requirements.
305-307: No initramfs tweaks required: storage modules are already included by defaultArmbian does not override the default initramfs-tools policy (no
initramfs.confin the repo), so mkinitramfs falls back to the Debian default ofMODULES=mostwhich “adds most file system, all ata, sata, scsi and usb drivers” — this implicitly covers
sd_mod,usb_storage,libata,ahci,xhcianduas(manpages.debian.org).Since there are no board-specific lists under
etc/initramfs-tools/modulesfor the filogic images, and no customMODULES=depoverride, all necessary storage drivers will be packaged into the initramfs.
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.
AI: "Modulize" is a less common variant of "modularize," which is the standard term used to describe the process of designing or producing something in separate sections or modules for flexibility and efficiency.
Thank you for your continued work, @tabrisnet. May I kindly ask you to separate your changes into atomic commits so they can be more easily cherry-picked and reverted if necessary? I suggest "git rebase -i origin/main" to do that. Then you can force-push to your branch after that.
87ee0a3 to
c5491ee
Compare
c5491ee to
960a3c9
Compare
how small is small? |
atomic and small are related but not the same. things that belong together ought to be committed together and stuff that doesn't should not. |
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.
excellent work, thank you very much
for anybody who needs btrfs support, for example, they need to make sure now that the module will be included in the initramfs
see armbian#8528 forcing BTRFS to be built/available is a policy choice. Forcing it to be built-in may not be a valid policy choice.
Modulize [modularize?] a bunch of inessential drivers/filesystems, and
add the
IFBdriver along withTUNandWIREGUARD. The goal is NOT to take away functionality, merely shrink the size of the kernel image.a) the
IFBdriver is used in traffic shaping to allow shaping of downstream traffic. see http://shorewall.org/traffic_shaping.htm for an example of how/why. If the rest of this PR is rejected, I'll split this into a separate one withTUNandWIREGUARD[see below].b) Armbian uses
ext4by default for the root FS.XFS,F2FS,BtrFScould be loaded from/lib/modulesor added to the initrd. You wouldn't have to twist my arm to do the same forMSDOS,VFAT&exFAT. possible excuses to keep them built-in include: EFI or boot partitions. Not yet a thing forfilogichowever. Note that I do useF2FSon some of my systems for eMMC/microSD, andXFSon NVMe, so I'm not just doing this for FSs I think nobody uses.c)
NFS,SMB/CIFSaren't niche, but also are inessential on this platform. I'm sure somebody will use them, but there's no need for them to be built-in that I'm aware of.d)
NTFS&FUSEare IMO unlikely to be used on this platform, maybeSSHFS? regardless, there is little probable harm for making them modules.e)
loopis mostly a development & backup tool.nbdis no big deal, it's mostly used for diskless boot. and the simple ramdisk is mostly obsoleted byzramandtmpfs. regardless, I'm not taking any of them away.f)
ATAandSDare SATA & PATA/IDE and SCSI, respectively. the R4 has no native SATA, PATA/IDE is obsolete, and SCSI is unlikely on this platform short of USB storage [which I don't consider unlikely, but not essential at boot]. Note thatxhci_mtk_hcdis the USB host-controller driver and already a module.g)
TUNis a VPN thing. so isWIREGUARD. Both are likely for this platform, given it's a router. But again, inessential at boot and it's new functionality.h)
NETCONSOLEmight be interesting, but it's new [was not part of the default config], so I'm not taking anything away that was available at boot.i) various network PHY drivers: they don't look essential [somebody tell me if they are?]. I did see something related to filogic in an OpenWRT forum post saying that the realtek PHY might be needed for some SFPs, but it was assumed they'd be modules.
j)
BT&RFKILL. afaict this platform has no native Bluetooth.RFKILLis probably useful for WiFI, but that is optional too.Description
see above commit-msg
see PR #8517 for some more of how this came up.
Documentation summary for feature / change
no documentation changes needed.
How Has This Been Tested?
built/install/booted on a BPI-R4 board, verified that network interfaces still work.
Checklist:
Please delete options that are not relevant.