mekotronics-r58x-4g: usb fixes + u-boot v2026.04-rc2 + analog audio#9412
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughUpdated Mekotronics R58X-4G board onboarding: bumped U-Boot references to v2026.04, added a UMS DT-patching helper (duplicated in file), introduced a new U-Boot "fileenv" command and Kconfig/Makefile entries, added u-boot patching overlays mapping, and expanded kernel DT with audio, OTG regulator, Type-C, and USB host nodes. Changes
Sequence Diagram(s)mermaid User->>U-Boot: run "fileenv dev:part " Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@igorpecovnik -- follow-up to #9398 "proof" of USB-enablement, all type-a and the type-c port work now. type-c is orientation-sensitive still, and in u-boot is reserved for UMS. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
patch/kernel/archive/rockchip64-6.19/dt/rk3588-blueberry-edge-v12-linux.dts (1)
1008-1014: 🛠️ Refactor suggestion | 🟠 MajorDuplicate
&usb_host0_xhcioverrides — consolidate into one block.There are two
&usb_host0_xhciblocks: lines 1008–1014 setdr_mode = "peripheral"with PHY bindings and speed limit, then lines 1052–1055 overridedr_mode = "host". The later block silently wins ondr_mode, but the staledr_mode = "peripheral"at line 1009 is dead/misleading. This split makes the final effective state hard to reason about.Consolidate into a single block with the intended final state:
♻️ Merge into one coherent block
-&usb_host0_xhci { // usb@fc000000 - dr_mode = "peripheral"; - maximum-speed = "high-speed"; - phys = <&u2phy0_otg>; - phy-names = "usb2-phy"; - status = "okay"; -}; ... -&usb_host0_xhci { - dr_mode = "host"; - status = "okay"; -}; +&usb_host0_xhci { // usb@fc000000 - Type-C port, USB2 only (usbdp_phy0 not wired) + dr_mode = "host"; + maximum-speed = "high-speed"; + phys = <&u2phy0_otg>; + phy-names = "usb2-phy"; + status = "okay"; +};Also applies to: 1052-1055
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-6.19/dt/rk3588-blueberry-edge-v12-linux.dts` around lines 1008 - 1014, Two duplicate &usb_host0_xhci override blocks exist; consolidate them into one coherent override by merging properties from both blocks (maximum-speed, phys, phy-names, status) and keeping the intended final dr_mode ("host" as set in the later block) so there are no conflicting or stale settings—remove the earlier dr_mode="peripheral" and the redundant second &usb_host0_xhci block so a single &usb_host0_xhci contains dr_mode="host", maximum-speed, phys, phy-names and status.
🧹 Nitpick comments (2)
config/boards/mekotronics-r58x-4g.csc (1)
57-69: Inconsistent function name prefix:ekor58x_4gvsmekor58x_4gused elsewhere.Other hook functions in this file use the
mekor58x_4gprefix (e.g.,pre_config_uboot_target__mekor58x_4g_patch_rockchip_common_boot_order), but this one usesekor58x_4g. While functionally harmless (Armbian hooks just need unique names), it makes grepping/discovering board-related hooks harder.♻️ Suggested rename for consistency
-function pre_config_uboot_target__ekor58x_4g_patch_uboot_dtsi_for_ums() { +function pre_config_uboot_target__mekor58x_4g_patch_uboot_dtsi_for_ums() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/boards/mekotronics-r58x-4g.csc` around lines 57 - 69, The function name pre_config_uboot_target__ekor58x_4g_patch_uboot_dtsi_for_ums uses the wrong board prefix; rename it to pre_config_uboot_target__mekor58x_4g_patch_uboot_dtsi_for_ums to match other hooks (e.g., pre_config_uboot_target__mekor58x_4g_patch_rockchip_common_boot_order), and update any internal references or registrations to use the new name so all mekotronics-r58x-4g hooks share the same prefix for discoverability.patch/kernel/archive/rockchip64-6.19/dt/rk3588-blueberry-edge-v12-linux.dts (1)
389-390: Endpoint labels sayes8316but the codec is an ES8388.The label
es8316_p0_0(line 389) and its remote-endpoint reference&es8316_p0_0(line 433) refer to a different Everest codec model (ES8316). While functionally harmless (labels are just internal references), this is confusing for anyone reading or maintaining the DTS.♻️ Rename labels to match the actual codec
port { - es8316_p0_0: endpoint { + es8388_p0_0: endpoint { remote-endpoint = <&i2s0_8ch_p0_0>; };i2s0_8ch_p0_0: endpoint { dai-format = "i2s"; mclk-fs = <256>; - remote-endpoint = <&es8316_p0_0>; + remote-endpoint = <&es8388_p0_0>; };Also applies to: 430-433
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-6.19/dt/rk3588-blueberry-edge-v12-linux.dts` around lines 389 - 390, Rename the misleading ES8316 labels to ES8388 to match the actual codec: change the endpoint label es8316_p0_0 to es8388_p0_0 and update any remote-endpoint references (e.g., &es8316_p0_0) to &es8388_p0_0; ensure both the endpoint declaration (es8316_p0_0: endpoint { ... }) and its consumers (remote-endpoint = <&es8316_p0_0> and any other occurrences around the same block) are updated so the label names are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@patch/kernel/archive/rockchip64-6.19/dt/rk3588-blueberry-edge-v12-linux.dts`:
- Around line 379-381: The device tree node es8388: audio-codec@11 has a
mismatched unit address and reg value (node name uses `@11` while reg = <0x10>);
update the node's unit address to match the reg value (change audio-codec@11 to
audio-codec@10) or adjust the reg to <0x11> so the unit address and the first
reg entry match, ensuring consistency for the node es8388: audio-codec and its
reg property.
---
Outside diff comments:
In `@patch/kernel/archive/rockchip64-6.19/dt/rk3588-blueberry-edge-v12-linux.dts`:
- Around line 1008-1014: Two duplicate &usb_host0_xhci override blocks exist;
consolidate them into one coherent override by merging properties from both
blocks (maximum-speed, phys, phy-names, status) and keeping the intended final
dr_mode ("host" as set in the later block) so there are no conflicting or stale
settings—remove the earlier dr_mode="peripheral" and the redundant second
&usb_host0_xhci block so a single &usb_host0_xhci contains dr_mode="host",
maximum-speed, phys, phy-names and status.
---
Nitpick comments:
In `@config/boards/mekotronics-r58x-4g.csc`:
- Around line 57-69: The function name
pre_config_uboot_target__ekor58x_4g_patch_uboot_dtsi_for_ums uses the wrong
board prefix; rename it to
pre_config_uboot_target__mekor58x_4g_patch_uboot_dtsi_for_ums to match other
hooks (e.g.,
pre_config_uboot_target__mekor58x_4g_patch_rockchip_common_boot_order), and
update any internal references or registrations to use the new name so all
mekotronics-r58x-4g hooks share the same prefix for discoverability.
In `@patch/kernel/archive/rockchip64-6.19/dt/rk3588-blueberry-edge-v12-linux.dts`:
- Around line 389-390: Rename the misleading ES8316 labels to ES8388 to match
the actual codec: change the endpoint label es8316_p0_0 to es8388_p0_0 and
update any remote-endpoint references (e.g., &es8316_p0_0) to &es8388_p0_0;
ensure both the endpoint declaration (es8316_p0_0: endpoint { ... }) and its
consumers (remote-endpoint = <&es8316_p0_0> and any other occurrences around the
same block) are updated so the label names are consistent.
patch/kernel/archive/rockchip64-6.19/dt/rk3588-blueberry-edge-v12-linux.dts
Outdated
Show resolved
Hide resolved
|
✅ This PR has been reviewed and approved — all set for merge! |
- drop invalid and useless enable-gpios on hdmi0 node - was from vendor kernel - does nothing in mainline - is not required in mainline for working HDMI
…_PA7) - probably type-c stuff, but we never know with Meko
- this gets the 3 usb type-A ports in the front working
- also: drop old modem (4G) to-do, as modem already works without it
- fileenv patch (same as v2026.01)
- fdt_fixup_ethernet logging patch (same as v2026.01)
- 0000.patching_config.yaml: defconfig/dt_upstream_rockchip/dt_uboot
- notable in v2026.04:
- dt-rebasing bumped up to v6.18 (which has NPU nodes)
- this allows us to share complete DTs between kernel and u-boot via symlinks
- Kwiboo had a go at LWIP which should be usable now; Kwiboo rocks.
- u-boot v2026.04-rcX has bumped dt-rebasing to v6.18, thus it knows about NPU nodes now and we can simply symlink to kernel DT, reducing duplication by a lot
- works as far as I can tell - both orientations - also wired DP, but can't test (I've no USB-DP hw)
- also `tcpm` cmd for debugging
- saves one .dtsi file in patch/.../dt_uboot - I had the wrong name (missing `-u-boot` suffix) too so this fixes it too
- This reverts commit e94425f. - I am clearly not up to par with type-c/fusb302 stuff yet - sending this for future reference
c0db53c to
f40fb9d
Compare
mekotronics-r58x-4g: usb fixes + u-boot v2026.04-rc2 + analog audio
tcpmcmd for debugging-u-boot.dtsivia hook-u-bootsuffix) too so this fixes it tooSummary by CodeRabbit
Release Notes
New Features
Chores