fix BPI-R4/filogic SFP support #8517
Conversation
github-actions
bot
added
08
WalkthroughThis change updates the Filogic kernel configuration file by enabling MediaTek USXGMII PCS support ( Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 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. ✨ 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 (
|
github-actions
bot
added
Needs review
tabrisnet
force-pushed
the
tabrisnet_bpir4_SFP
branch
from
1f9894b to
85d9524
Compare
|
also works with FS.com DAC |
when doing my initial changes [enabled a bunch of PHYs, made NFS/SMB etc modules... random stuff], I somehow enabled some errata fixes, and this one showed up in dmesg. The real star of the show I'm pretty sure is CONFIG_PCS_MTK_USXGMII. Note, it cannot be a module, but on this platform we probably wouldn't want it to be anyway. When I say "it can't", I'm saying it breaks the build during the linker phase for vmlinux. Probably worth fixing, but I'm unclear where to start, other than it's probably a Kconfig dependency problem. CONFIG_I2C_SLAVE_EEPROM came along for the ride b/c it possibly made sense with SFPs. But I haven't tried a test build without it yet. For the record: this works with both my AQS-107-B0C2-CX [an 802.3bz SFP+] and my FS.com SFP-10GSR-85 "generic" SFP+.
tabrisnet
force-pushed
the
tabrisnet_bpir4_SFP
branch
from
85d9524 to
b3cadb7
Compare
coderabbitai
bot
requested review from
AGM1968,
EvilOlaf,
Tonymac32,
chraac and
teknoid
![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/kernel/linux-filogic-current.config (1)
417-419: Drop I2C slave and test/EEPROM emulation; they’re unrelated to SFP and expand attack surface.
SFP cages require an I2C master to read module EEPROM; enabling I2C slave mode and the test/emulation drivers is unnecessary for SFP functionality. These drivers are meant for validation and can expose unintended behaviors on production systems. Also, the SoC’s I2C controller must explicitly support slave role for these to be usable; otherwise they’re dead weight.Apply this diff to revert them:
-CONFIG_I2C_SLAVE=y -CONFIG_I2C_SLAVE_EEPROM=m -CONFIG_I2C_SLAVE_TESTUNIT=m +# CONFIG_I2C_SLAVE is not set +# CONFIG_I2C_SLAVE_EEPROM is not set +# CONFIG_I2C_SLAVE_TESTUNIT is not setIf you have evidence that a particular deployment needs host-as-slave behavior, we can reintroduce them per-profile, but it would be better to keep them off by default.
📜 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(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-26T11:17:56.870Z
Learnt from: pyavitz
PR: armbian/build#8421
File: config/kernel/linux-sunxi64-edge.config:1058-1061
Timestamp: 2025-07-26T11:17:56.870Z
Learning: I2C_DESIGNWARE_SLAVE is a bool configuration option in Linux kernel Kconfig, not tristate. This means it can only be set to 'y' (enabled) or 'n' (disabled), never 'm' (module). When enabled, the slave functionality is compiled into the I2C_DESIGNWARE_CORE driver, regardless of whether the core driver is built-in or as a module.
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 (1)
armbian_kernel_config_apply_opts_from_arrays(418-448)
🔇 Additional comments (2)
config/kernel/linux-filogic-current.config (2)
342-342: PCS_MTK_USXGMII built-in is the right move to avoid link-time failures.
Keeping the PCS built-in aligns with a built-in MAC/SoC (NET_MEDIATEK_SOC=y) and prevents unresolved symbol issues during vmlinux link.Would you mind confirming whether NET_MEDIATEK_SOC and dependent MAC/PHY parts are also built-in on all filogic-current variants? Mixed built-in/module across MAC/PCS often causes the link failure you described. If this turns out to be systemic, a small Kconfig follow-up (e.g., select PCS_MTK_USXGMII when NET_MEDIATEK_SOC=y) in our tree could help prevent regressions.
37-59: Erratum 858921 only affects Cortex-A73
Erratum 858921 is specific to Cortex-A73 cores and does not apply to Cortex-A53. The Kconfig optionCONFIG_ARM64_ERRATUM_858921defaults toyand only enables the workaround at runtime when an A73 is detected.• If your Filogic platform uses Cortex-A73, explicitly enable it in this config:
# CONFIG_ARM64_ERRATUM_858921 is not set +CONFIG_ARM64_ERRATUM_858921=y• If Filogic uses only Cortex-A53 (or other unaffected cores), you can safely leave it unset.
https://www.mediatek.com/products/broadband-wifi/mediatek-filogic-880 |
…robably irrelevant
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
config/kernel/linux-filogic-current.config (1)
37-59: Erratum 858921: consider explicitly setting =y for Cortex‑A73 SoCsGiven Filogic 880 uses Cortex‑A73, explicitly pinning CONFIG_ARM64_ERRATUM_858921=y would make the intent resilient to Kconfig default changes across kernel bumps. Not required if you prefer defaults, but recommended for reproducibility.
If you choose to make it explicit, add this line near the other errata options:
CONFIG_ARM64_ERRATUM_858921=y
📜 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(1 hunks)
🔇 Additional comments (2)
config/kernel/linux-filogic-current.config (2)
330-330: Making CONFIG_PCS_MTK_USXGMII built-in is the right call to avoid link-time issuesBuilding the PCS as built-in aligns with NET_MEDIATEK_SOC=y and prevents vmlinux link failures you saw with =m. Change looks correct for Filogic/BPI‑R4 SFP bring-up.
330-335: Configuration Sanity Check CompleteThe expected options are correctly set in config/kernel/linux-filogic-current.config:
- Line 329: CONFIG_NET_MEDIATEK_SOC=y
- Line 330: CONFIG_PCS_MTK_USXGMII=y
- Line 334: CONFIG_SFP=m
No explicit ARM64 erratum 858921 entry was found (this is optional).
Next steps:
- At runtime, confirm that the SFP module auto-loads and that dmesg shows no “unresolved symbol” warnings.
- If you need SFP functionality before module loading (early-boot), consider changing CONFIG_SFP=m → CONFIG_SFP=y.
igorpecovnik
added
Ready to merge
|
I did initially set it as a module, but as I said in the description, the linker phase failed for an unresolved symbol. It is probably possible IF I change something else [the bot mentioned NET_MEDIATEK_SOC] to a module.
But I'm unclear why we would want it as a module as there is little/no reason on this platform to not load it. If you're not gonna use the 10G SFP cages, then why not get the R2 [that it's 32bit might dissuade you] or R3?
If your argument is that you want to support the R3 in the future... I can see that argument.
Meanwhile, IMO there are much more interesting cases to override as modules like SMB/CIFS, NFS [client or server], XFS [ext4 is the base image], BtrFS, F2FS. Then there is REALTEK_PHY, MAXLINEAR_GPHY, ICPLUS_PHY & maybe the Airoha PHYs [but I'm not sure, they are Mediatek]. And next: CONFIG_ATA, as this platform doesn't natively do SATA [the R2 does], BLK_DEV_LOOP [mostly a development or backup tool], BLK_DEV_NBD [largely a tool for diskless boot].
Then I can't imagine why we have INTEL_MEI built-in...
Sent from myPhone.
|
|
and I forgot to mention we should probably add |
|
if you're looking for my deleted comment... |
|
so I just ran a test build [haven't attempted to run this kernel] with among other changes [SMB, NFS etc, as mentioned previously], and changing |
|
Tested, it works. |
|
Thank you for your work, @tabrisnet
Apologies, I missed that part.
No argument ;-) I am just looking at your code, reviewing. I don't have the board or know too much about it.
Agreed, and please feel free to push more PRs for that, although maybe not for the filesystems (depending how exotic they are and how much they add). |
4 participants
So I'm not completely convinced about
CONFIG_ARM64_ERRATUM_858921, butwhen doing my initial changes [enabled a bunch of PHYs, made NFS/SMB etc modules... random stuff], I somehow enabled some errata fixes, and this one showed up in
dmesg.The real star of the show I'm pretty sure is
CONFIG_PCS_MTK_USXGMII. Note, it cannot be a module, but on this platform we probably wouldn't want it to be anyway. When I say "it can't", I'm saying it breaks the build during the linker phase forvmlinux. Probably worth fixing, but I'm unclear where to start, other than it's probably a Kconfig dependency problem.CONFIG_I2C_SLAVE_EEPROMcame along for the ride b/c it possibly made sense with SFPs. But I haven't tried a test build without it yet.For the record: this works with both my
AQS-107-B0C2-CX[an 802.3bz SFP+] and my FS.comSFP-10GSR-85"generic" SFP+.Description
This is a fix for the second issue found from my forum post. https://forum.armbian.com/topic/54214-bpi-r4-fails-to-build-missing-bl2-firmware/#findComment-223626
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.
linux-{dtb,image}-current-filogicDEBs. reboot.ip link set up dev eth1.ethtool eth1|grep Linkip link setandethtooltcpdump -vv -i eth2 'ether proto 0x88cc'gives positive results.Checklist:
Please delete options that are not relevant.