Skip to content

mt7601u-openipc: pin the source to a commit instead of HEAD - #2246

Open
phedoreanu wants to merge 1 commit into
OpenIPC:masterfrom
phedoreanu:hi3518ev200-ultimate-mt7601u
Open

mt7601u-openipc: pin the source to a commit instead of HEAD#2246
phedoreanu wants to merge 1 commit into
OpenIPC:masterfrom
phedoreanu:hi3518ev200-ultimate-mt7601u

Conversation

@phedoreanu

@phedoreanu phedoreanu commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

What

Pin MT7601U_OPENIPC_VERSION to a commit SHA (0ac4655, current tip of openipc/mt7601u) instead of HEAD.

Rescoped after review: the original PR also enabled BR2_PACKAGE_MT7601U_OPENIPC in hi3518ev200_ultimate_defconfig. Per @flyrouter, firmware images do not ship WiFi drivers — only WiFi utilities — and per-device customisation belongs in the Builder repository, so that change is dropped. The device profile for this camera will go through Builder instead.

Why

VERSION = HEAD is a moving target: anyone enabling this package gets whatever openipc/mt7601u happens to be at build time, so the same tree can produce different artifacts over time and can break with no change in this repo. Pinning to an immutable ref makes builds reproducible.

0ac4655 is the lineage that was runtime-verified on a Hi3518EV200 camera: WPA2 join, RTSP streaming, 15–17 Mbps TX iperf3.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

hi3518ev200_ultimate: enable MT7601U Wi‑Fi driver alongside its firmware

🐞 Bug fix ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

AI Description

• Enable the MT7601U kernel driver in hi3518ev200_ultimate to match the already-shipped firmware
 blob.
• Prevent boot-time Wi‑Fi failures where mt7601u.bin exists but no module can load it.
• Align this defconfig’s Wi‑Fi packaging with existing “ship drivers in defconfig” practice.
Diagram

graph TD
  A["hi3518ev200_ultimate_defconfig"] --> B["Buildroot build"] --> C[("Rootfs image") ] --> D["mt7601u module loads"] --> E["wlan0 up"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Make MT7601U driver package select its firmware automatically
  • ➕ Eliminates defconfig drift (firmware enabled without driver, or vice versa)
  • ➕ Centralizes correctness in package metadata rather than per-board configs
  • ➖ Requires changing package Config.in/dependency rules and verifying no unintended selections across targets
2. Remove MT7601U firmware from this defconfig
  • ➕ Reduces image size if MT7601U support is not desired for this target
  • ➖ Does not solve the reported failure mode for users with MT7601U dongles
  • ➖ Regresses functionality compared to current intent (firmware already included)
3. Build MT7601U driver into the kernel instead of a module
  • ➕ Avoids module-loading issues during boot and simplifies runtime bring-up
  • ➖ Kernel rebuild/config changes may be more intrusive than a Buildroot package enable
  • ➖ Potentially increases kernel image size permanently

Recommendation: Proceed with this PR as the minimal, target-scoped fix: it corrects the concrete mismatch (firmware without driver) with negligible risk. Consider a follow-up to make the MT7601U driver package select its firmware to prevent similar misconfigurations across other defconfigs.

Files changed (1) +1 / -0

Other (1) +1 / -0
hi3518ev200_ultimate_defconfigEnable MT7601U driver package for hi3518ev200_ultimate +1/-0

Enable MT7601U driver package for hi3518ev200_ultimate

• Adds BR2_PACKAGE_MT7601U_OPENIPC=y so the image ships the MT7601U kernel driver module in addition to the already-enabled mt7601u firmware blob.

br-ext-chip-hisilicon/configs/hi3518ev200_ultimate_defconfig

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 9, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Moving driver version ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
Enabling BR2_PACKAGE_MT7601U_OPENIPC makes this defconfig depend on the mt7601u-openipc package,
which is configured to fetch source at VERSION=HEAD. This makes future hi3518ev200_ultimate builds
non-reproducible and can break unexpectedly when upstream changes without any version bump in this
repo.
Code

br-ext-chip-hisilicon/configs/hi3518ev200_ultimate_defconfig[76]

+BR2_PACKAGE_MT7601U_OPENIPC=y
Evidence
The PR enables the MT7601U driver package in the defconfig, and that package is explicitly
configured to fetch from a moving HEAD ref, which can change independently of this repository.

br-ext-chip-hisilicon/configs/hi3518ev200_ultimate_defconfig[68-80]
general/package/mt7601u-openipc/mt7601u-openipc.mk[7-9]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`hi3518ev200_ultimate_defconfig` now enables `BR2_PACKAGE_MT7601U_OPENIPC`, but the package recipe fetches the driver source at `MT7601U_OPENIPC_VERSION = HEAD`, which is a moving target.
## Issue Context
This introduces non-reproducible builds (the same firmware target can produce different artifacts over time) and increases the chance of sudden build/runtime regressions when upstream `openipc/mt7601u` changes.
## Fix Focus Areas
- general/package/mt7601u-openipc/mt7601u-openipc.mk[7-9]
## Suggested fix
- Change `MT7601U_OPENIPC_VERSION` from `HEAD` to an immutable reference (a specific commit SHA or a release tag).
- (If your Buildroot flow supports it) add/update a corresponding `.hash` file for the tarball to enforce integrity and reproducibility.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread br-ext-chip-hisilicon/configs/hi3518ev200_ultimate_defconfig Outdated

@flyrouter flyrouter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The concept of creating firmwares doesn't allow any external drivers from packages. Inside the firmware there will only be WiFi utilities but no drivers, that's the rule. Also, note that we are gradually moving away from Ultimate firmwares in favour of Lite ones. And also pay attention to the Builder repository where firmwares are customised for specific devices. If you remove everything unnecessary, everything even the WiFi driver fits into the Lite firmware.

@phedoreanu
phedoreanu force-pushed the hi3518ev200-ultimate-mt7601u branch from 195fc2f to 03c92fe Compare August 12, 2026 08:20
@phedoreanu phedoreanu changed the title hi3518ev200_ultimate: ship the MT7601U driver, not only its firmware blob mt7601u-openipc: pin the source to a commit instead of HEAD Aug 12, 2026
@phedoreanu

Copy link
Copy Markdown
Contributor Author

Understood, and thanks for spelling out the rule — that also explains why the defconfigs carry the firmware blobs but not the drivers. I've rescoped the PR: the defconfig change is dropped, and what remains is only the mt7601u-openipc.mk change pinning VERSION = HEAD to a commit SHA, which is worth having for reproducibility regardless of where the package is enabled. I'll take the device-specific integration (this camera's WiFi driver selection) to the Builder repository instead.

VERSION = HEAD is a moving target: anyone enabling this package gets
whatever openipc/mt7601u happens to be at build time, so the same tree
can produce different artifacts over time and break without any change
in this repo. Pin the current tip (0ac4655, 'Adjust configuration
path'), which is the lineage that was runtime-verified on a
Hi3518EV200 camera (WPA2 join, RTSP streaming, 15-17 Mbps TX iperf3).
@phedoreanu
phedoreanu force-pushed the hi3518ev200-ultimate-mt7601u branch from 03c92fe to e1f373a Compare August 12, 2026 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants