orangepizero2w: fix ethernet using sunxi-gmac driver#9815
Conversation
It is patched into Armbian kernel sources, and was enabled in Linux 6.12, but has not been enabled anymore with Linux 6.18 and above. Additionally, a possible attempt to split the AC200 related patch ended up with half of sunxi-gmac dependencies missing, including such which are explicitly enabled in defconfs, including e.g. CONFIG_AC200_PHY_SUNXI. The mainline driver dwmac-sun8i, that might have been seen as possible replacement for sunxi-gmac, does not support the Ethernet chip of the Orange Pi Zero 2W expansion board yet. Signed-off-by: MichaIng <micha@dietpi.com>
EvilOlaf
requested review from
AGM1968,
JohnTheCoolingFan,
PanderMusubi,
TheSnowfield,
Tonymac32,
chraac,
igorpecovnik,
janprunk,
krachlatte,
pyavitz,
schwar3kat,
sgjava and
teknoid
as code owners
📝 WalkthroughWalkthroughThis pull request integrates H618 GMAC Ethernet and AC200 PHY/MFD hardware support for Sunxi-based systems. It adds kernel module configurations to enable the drivers, registers the corresponding patches in kernel series, and ensures proper build dependencies are declared in Kconfig. ChangesH618 GMAC and AC200 Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
github-actions
Bot
added
05
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
Tested the patch on an OrangePi Zero 2W. Unfortunately, with kernel 7.0+ the board hangs after loading bcachefs during the filesystem check stage. On kernel 6.18.29 the network works fully, but I had to enable the following config options manually, otherwise the GMAC module would not compile (Not entirely sure why this was required) |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/kernel/linux-sunxi64-edge.config`:
- Line 763: Replace the manual addition of the CONFIG_AC200_PHY_SUNXI entry by
regenerating the kernel config with Armbian’s tool: run the Armbian kernel
config rewrite command (compile.sh rewrite-kernel-config) so the
CONFIG_AC200_PHY_SUNXI symbol is added via the official toolchain and dependency
resolution rather than hardcoding "CONFIG_AC200_PHY_SUNXI=m" in the config file;
verify the regenerated config contains CONFIG_AC200_PHY_SUNXI and commit the
regenerated config instead of the manual change.
- Line 1469: Regenerate the kernel config instead of manually adding
CONFIG_MFD_AC200_SUNXI by running the kernel-config rewrite step (./compile.sh
rewrite-kernel-config BOARD=orangepizero2w BRANCH=edge) so the MFD_AC200_SUNXI
Kconfig is picked up as an overlay; then verify the symbol exists in the kernel
patches and references by searching for "config MFD_AC200_SUNXI" and
"MFD_AC200_SUNXI" (e.g., with rg/fd across patch/kernel/archive/sunxi-7.0 and
sunxi-6.18) and confirm the patch that adds the AC200 MFD driver (0201.*ac200)
defines the symbol, ensuring armbian/build kernel config overlays remain minimal
and regenerated.
- Line 696: The patch manually adds CONFIG_SUNXI_GMAC=m which violates the repo
policy that kernel configs under config/kernel/* must be minimal overlays
generated by the official tool; regenerate the overlay instead by running the
rewrite command (./compile.sh rewrite-kernel-config BOARD=orangepizero2w
BRANCH=edge) so Kconfig resolves dependencies automatically, and verify the
SUNXI_GMAC Kconfig symbol exists in the kernel sources/patches (search for
"config SUNXI_GMAC" or "SUNXI_GMAC") before committing the regenerated overlay.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 34d0c290-ff92-4e2d-a352-f44de4a4e33f
📒 Files selected for processing (2)
config/kernel/linux-sunxi64-current.configconfig/kernel/linux-sunxi64-edge.config
✅ Files skipped from review due to trivial changes (1)
- config/kernel/linux-sunxi64-current.config
|
Test images for various sunxi64 boards, randomly selected edge or current: https://testing.armbian.de/ |
|
No network on Inovatto quadra. https://paste.armbian.com/fupuralaga |
|
Do you have a armbianmonitor report handy without the changes of this pr? |
github-actions
Bot
added
size/small
There was a problem hiding this comment.
🧹 Nitpick comments (1)
patch/kernel/archive/sunxi-7.0/patches.armbian/drv-net-stmmac-sun8i-add-h618-emac.patch (1)
81-88: ⚡ Quick winKconfig design concern: hidden dependency on AC200_PHY_SUNXI not expressed in config rules.
The driver unconditionally calls
gmac_ephy_shutdown()andephy_is_enable()(wrapped in#if 1 // defined(CONFIG_SUNXI_EPHY)guards that always evaluate true) — both are EXPORT_SYMBOL_GPL from sunxi-ephy, which depends onAC200_PHY_SUNXI/MFD_AC200_SUNXI. The Kconfig now onlyselectsPHYLIB/CRC32/MII, with no explicit dependency on those AC200 symbols.Current defconfigs (
linux-sunxi64-current.configandlinux-sunxi64-edge.config) do enable bothAC200_PHY_SUNXI=mandMFD_AC200_SUNXI=mwhereverSUNXI_GMAC=mis set, so in practice there is no immediate load-failure risk. However, the Kconfig does not express this dependency, creating a latent maintainability issue: a developer unaware of this hidden coupling could misconfigureSUNXI_GMACwithout the required AC200 modules and hit unresolved symbols at module load time.Consider adding
depends on AC200_PHY_SUNXIto the SUNXI_GMAC config block to make the dependency explicit. This preserves the "relaxed" spirit of the change (no longer hard-failing the build of the rest of the kernel) while preventing accidentally unloadable configurations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patch/kernel/archive/sunxi-7.0/patches.armbian/drv-net-stmmac-sun8i-add-h618-emac.patch` around lines 81 - 88, The SUNXI_GMAC Kconfig introduces a hidden runtime dependency on symbols from sunxi-ephy (gmac_ephy_shutdown, ephy_is_enable) which come from AC200_PHY_SUNXI / MFD_AC200_SUNXI; make this dependency explicit by adding a Kconfig dependency (e.g., "depends on AC200_PHY_SUNXI") to the SUNXI_GMAC stanza so the config system prevents selecting SUNXI_GMAC without the AC200 PHY module while retaining the module-style behavior (keep tristate/selects as-is).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@patch/kernel/archive/sunxi-7.0/patches.armbian/drv-net-stmmac-sun8i-add-h618-emac.patch`:
- Around line 81-88: The SUNXI_GMAC Kconfig introduces a hidden runtime
dependency on symbols from sunxi-ephy (gmac_ephy_shutdown, ephy_is_enable) which
come from AC200_PHY_SUNXI / MFD_AC200_SUNXI; make this dependency explicit by
adding a Kconfig dependency (e.g., "depends on AC200_PHY_SUNXI") to the
SUNXI_GMAC stanza so the config system prevents selecting SUNXI_GMAC without the
AC200 PHY module while retaining the module-style behavior (keep
tristate/selects as-is).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8f24c655-c4e0-4980-b601-adc65f9b3783
📒 Files selected for processing (4)
patch/kernel/archive/sunxi-6.18/patches.armbian/drv-net-stmmac-sun8i-add-h618-emac.patchpatch/kernel/archive/sunxi-6.18/series.confpatch/kernel/archive/sunxi-7.0/patches.armbian/drv-net-stmmac-sun8i-add-h618-emac.patchpatch/kernel/archive/sunxi-7.0/series.conf
|
Let's put this aside for now, I don't see (yet) how to combine both worlds. AI has some idea but without hw on hand testing this is quite annoying for multiple parties.... |
It works. https://paste.armbian.com/kifiyegago |
4 participants
Description
Micha did some cleanup after my mess with the sunxi patchset. Well, it is what it is when no professional wants to tackle an issue but leave it to an amateur like myself to do it 😁 . Anyway below is the commit message.
How Has This Been Tested?
- [x] boot orangepizero2 (H616) edge, ethernet works https://paste.armbian.com/ducapenibu- [x] boot orangepizero (H2+) current, ethernet works https://paste.armbian.com/umarimutuv (not because it is even affected, just because I can)- [x] boot orangepioneplus (H6) current, ethernet works https://paste.armbian.com/benupejuliChecklist:
Summary by CodeRabbit
Release Notes