Rockchip: youyeetoo yy3568: enable pci-e NVMe ssd#9877
Conversation
hqnicolas
requested review from
CodeChenL,
ColorfulRhino,
HeyMeco,
JohnTheCoolingFan,
SuperKali,
TheSnowfield,
Tonymac32,
ahoneybun,
amazingfate,
brentr,
catalinii,
chainsx,
efectn,
ginkage,
iav,
igorpecovnik,
joekhoobyar,
krachlatte,
lanefu,
paolosabatino,
prahal,
pyavitz,
rpardini,
schwar3kat and
sputnik2019
as code owners
📝 WalkthroughWalkthroughThis PR synchronizes device tree configurations for the Rockchip RK3568 Yooyeetoo YY3568 board across kernel versions (6.18, 7.0, 7.1) and u-boot. Changes add clock bindings, restructure PCIe regulator/PHY wiring, enable audio support, and extend u-boot board peripheral initialization including storage, networking, PMIC, and crypto support. ChangesKernel Device Tree Synchronization (6.18, 7.0, 7.1)
U-Boot Board Device Tree Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
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)
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 |
hqnicolas
requested review from
EvilOlaf
and removed request for
CodeChenL,
ColorfulRhino,
JohnTheCoolingFan,
efectn,
pyavitz,
schwar3kat and
sputnik2019
github-actions
Bot
added
size/large
hqnicolas
requested review from
SuperKali,
TheSnowfield,
amazingfate,
brentr,
catalinii,
ginkage,
iav,
igorpecovnik,
joekhoobyar,
krachlatte,
lanefu,
paolosabatino,
prahal and
rpardini
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3568-yy3568.dts (1)
220-221:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
pmic-resetnever selects the reset mux.
pinctrl-3points at&soc_slppin_gpio, whilesoc_slppin_rstis defined below and otherwise unused. That leaves thepmic-resetstate in GPIO mode instead ofRK_FUNC_2, so PMIC reset requests will not drive the intended reset function.🛠️ Proposed fix
- pinctrl-3 = <&soc_slppin_gpio>, <&rk817_slppin_rst>; + pinctrl-3 = <&soc_slppin_rst>, <&rk817_slppin_rst>;Also applies to: 553-557
🤖 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/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3568-yy3568.dts` around lines 220 - 221, pinctrl-3 currently references &soc_slppin_gpio instead of the reset mux, so the pmic-reset state remains in GPIO mode; update the device-tree entries that define the pmic-reset pin state (pinctrl-3) to reference &rk817_slppin_rst (which selects RK_FUNC_2) instead of &soc_slppin_gpio so PMIC reset requests use the reset mux; apply the same correction to the duplicate block around lines 553-557 where the same mistaken reference appears (look for symbols pinctrl-3, &soc_slppin_gpio, &rk817_slppin_rst, and pmic-reset).
🤖 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 `@patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3568-yy3568.dts`:
- Around line 126-143: The &gmac0 node currently uses phy-mode = "rgmii" with
clock_in_out = "output" but does not specify controller-side RGMII delays;
update the &gmac0 device tree node to either add appropriate tx_delay and
rx_delay properties (e.g., tx_delay = <...>; rx_delay = <...>) so the dwmac
controller applies the timing, or change phy-mode to the correct
rgmii-id/rgmii-rxid/rgmii-txid variant that indicates the PHY handles delays;
modify the properties in the &gmac0 node (phy-mode, add tx_delay/rx_delay)
accordingly to match where delays are implemented.
---
Outside diff comments:
In `@patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3568-yy3568.dts`:
- Around line 220-221: pinctrl-3 currently references &soc_slppin_gpio instead
of the reset mux, so the pmic-reset state remains in GPIO mode; update the
device-tree entries that define the pmic-reset pin state (pinctrl-3) to
reference &rk817_slppin_rst (which selects RK_FUNC_2) instead of
&soc_slppin_gpio so PMIC reset requests use the reset mux; apply the same
correction to the duplicate block around lines 553-557 where the same mistaken
reference appears (look for symbols pinctrl-3, &soc_slppin_gpio,
&rk817_slppin_rst, and pmic-reset).
🪄 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: c55526ae-aeeb-430d-87b4-ad1cd4bbfd36
📒 Files selected for processing (4)
patch/kernel/archive/rockchip64-6.18/dt/rk3568-yy3568.dtspatch/kernel/archive/rockchip64-7.0/dt/rk3568-yy3568.dtspatch/kernel/archive/rockchip64-7.1/dt/rk3568-yy3568.dtspatch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3568-yy3568.dts
| vcc3v3_pi6c_05: vcc3v3-pi6c-05 { | ||
| u-boot,dm-pre-reloc; | ||
| compatible = "regulator-fixed"; | ||
| regulator-name = "led_sys"; | ||
| enable-active-high; | ||
| gpio = <&gpio0 RK_PC1 GPIO_ACTIVE_HIGH>; | ||
| regulator-boot-on; | ||
| gpio = <&gpio0 RK_PC7 GPIO_ACTIVE_HIGH>; | ||
| pinctrl-names = "default"; | ||
| pinctrl-0 = <&pcie_enable_h>; |
There was a problem hiding this comment.
U-Boot still uses the stale PCIe REFCLK/OE wiring.
The board fix described in this PR moves the PCIe clock-buffer enable onto an active-low pcie_oe path, but this DTS still drives an active-high pcie_enable_h on GPIO0_C7 and never attaches that OE rail to &pcie30phy. As written, the Linux-side NVMe fix is not actually mirrored in U-Boot, so PCIe3/NVMe discovery can still fail before handoff.
Also applies to: 495-517, 521-534
There was a problem hiding this comment.
@Arthur97172 feel free to include the PCIe3/NVMe uboot fixes, to enable the device also to boot from PCI-e, there is also an OTA feature that I fail to apply into this board, feel free to make it boot
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| &gmac0 { | ||
| u-boot,dm-pre-reloc; | ||
| phy-mode = "rgmii"; | ||
| clock_in_out = "output"; | ||
| snps,reset-gpio = <&gpio2 RK_PD3 GPIO_ACTIVE_LOW>; | ||
| snps,reset-active-low; | ||
| snps,reset-delays-us = <0 20000 100000>; | ||
| assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>; | ||
| assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>; | ||
| assigned-clock-rates = <0>, <125000000>; | ||
| pinctrl-names = "default"; | ||
| pinctrl-0 = <&gmac0_miim | ||
| &gmac0_tx_bus2 | ||
| &gmac0_rx_bus2 | ||
| &gmac0_rgmii_clk | ||
| &gmac0_rgmii_bus>; | ||
| phy-handle = <&rgmii_phy0>; | ||
| status = "okay"; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Rockchip RK3568 dwmac device-tree configuration, when a node uses phy-mode = "rgmii"andclock_in_out = "output", should tx_delayandrx_delaybe provided unless the PHY mode isrgmii-id, rgmii-rxid, or rgmii-txid?
💡 Result:
Yes—provide tx_delay and rx_delay for phy-mode = "rgmii" (including when clock_in_out = "output"). Do not provide them for phy-mode = "rgmii-id", "rgmii-rxid", or "rgmii-txid". Evidence: In the Rockchip dwmac DT binding, tx_delay and rx_delay are defined as controller-provided RGMII delay settings (TXD timing / RXD timing) with defaults 0x30 and 0x10 [1][2]. For rgmii-id operation, Linux notes that “In rgmii-id mode these are disabled, no matter what they are set to in the dt”, because the rgmii-id variants “apply the delays in the phy” [3][4]. So the practical rule you’re asking about is: - phy-mode = "rgmii" (not rgmii-id/rxid/txid): tx_delay and rx_delay should be provided (or you’ll fall back to the binding defaults) [1][2]. - phy-mode = "rgmii-id" / "rgmii-rxid" / "rgmii-txid": tx_delay and rx_delay should not be provided/are ignored [3][4].
Citations:
- 1: https://gem5.googlesource.com/arm/linux/+/512cf465ee01eb23936a9e6ed0b6414eccb00853/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
- 2: https://sbexr.rabexc.org/latest/sources/28/73013145d50d97.html
- 3: https://lists.infradead.org/pipermail/linux-arm-kernel/2022-November/790888.html
- 4: https://lkml.rescloud.iu.edu/2211.2/01549.html
Fix GMAC0 RGMII timing: add tx_delay/rx_delay or use an rgmii-*-id PHY mode
&gmac0 sets phy-mode = "rgmii" with clock_in_out = "output" but omits tx_delay/rx_delay; for plain rgmii the dwmac binding expects controller-side delay settings (the rgmii-id/rgmii-rxid/rgmii-txid variants are the ones where delays are handled in the PHY and DT delays are not used). Add tx_delay/rx_delay for &gmac0, or switch phy-mode to the correct *-id variant if the PHY is responsible for the RGMII delays.
🤖 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/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3568-yy3568.dts` around lines
126 - 143, The &gmac0 node currently uses phy-mode = "rgmii" with clock_in_out =
"output" but does not specify controller-side RGMII delays; update the &gmac0
device tree node to either add appropriate tx_delay and rx_delay properties
(e.g., tx_delay = <...>; rx_delay = <...>) so the dwmac controller applies the
timing, or change phy-mode to the correct rgmii-id/rgmii-rxid/rgmii-txid variant
that indicates the PHY handles delays; modify the properties in the &gmac0 node
(phy-mode, add tx_delay/rx_delay) accordingly to match where delays are
implemented.
There was a problem hiding this comment.
Dear robot, I cannot invent gmac0 delay values for YY3568 kernel DTS if the vendor doesn’t provide them, so leaving gmac0 unset is safer than guessing.
There was a problem hiding this comment.
@hqnicolas, that's a fair point — using wrong delay values would be worse than relying on the binding defaults (0x30 tx / 0x10 rx). A few non-guessing alternatives if you want to revisit this later:
- Check the vendor BSP — Rockchip's BSP tree or Radxa's downstream kernel often carries board-specific tuned delay values; that would be the authoritative source.
- Check similar RK3568 boards — e.g.,
rk3568-evb*.dtsorrk3568-rock-3*.dtsin upstream may give calibrated ballpark values for the same RTL8211F PHY. - Switch
phy-modetorgmii-id— if the RTL8211F is configured to provide internal RX/TX delay (the common default for that PHY), usingrgmii-idmakes the intent explicit and skips the need for controller-side delays entirely.
If you'd like to track this as a follow-up issue rather than blocking this PR, I'm happy to open one.
🐇 (understood, safety first over speculation)
✏️ Learnings added
Learnt from: hqnicolas
Repo: armbian/build PR: 9877
File: patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3568-yy3568.dts:126-143
Timestamp: 2026-05-22T16:23:45.422Z
Learning: On the Youyeetoo YY3568 board (RK3568), the `gmac0` DT node uses `phy-mode = "rgmii"` with `clock_in_out = "output"` but no `tx_delay`/`rx_delay` values are provided by the vendor, so they are intentionally left unset (falling back to dwmac binding defaults 0x30/0x10). Do not flag the absence of these properties as a definite error for this board; instead suggest checking vendor BSP or switching to `rgmii-id` as alternatives. The `gmac1` node does have explicit delays (`tx_delay = <0x44>; rx_delay = <0x26>`).
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: EvilOlaf
Repo: armbian/build PR: 9067
File: patch/kernel/archive/rockchip64-6.19/dt/rk3399-orangepi-4-lts.dts:869-875
Timestamp: 2025-12-23T04:44:25.212Z
Learning: During code reviews of device tree changes (any .dts file), verify node properties against the official kernel device tree binding documentation (*.yaml) rather than relying solely on patterns observed in existing DTS files. This helps catch copy-paste or drift errors and ensures properties align with authoritative bindings for each device node type.
Learnt from: paolosabatino
Repo: armbian/build PR: 9662
File: patch/u-boot/v2026.04/dt_upstream_rockchip/rk3318-box.dts:519-525
Timestamp: 2026-04-13T20:52:36.022Z
Learning: When reviewing Armbian U-Boot device-tree files (e.g., under `patch/u-boot/**`), some peripherals may have DT nodes that are not actually consumed by U-Boot (example: a Bluetooth child node under `&uart0` for BCM43341-bt). In such cases, DT issues that would only affect the unsupported U-Boot runtime (e.g., typos in the Bluetooth node, malformed/incorrect interrupt properties inside that Bluetooth subnode) should not be treated as a U-Boot runtime bug. Still correct obvious DT syntax/structural problems so the node can be safely reused in a kernel DTS context if applicable.
1 participant
Description
This PR replaces #9832 , applying the changes requested by https://github.com/Arthur97172
I keep this dedication here in recognition of Arthur's effort.
To the author, we can close the PR.
#9832
Fixed NVMe SSD detection issue on Youyeetoo YY3568.
tested on 7.0.9-edge-rockchip64 by @hqnicolas
https://paste.armbian.com/amimofemog
tested on 6.18.32-current-rockchip64 by @hqnicolas
https://paste.armbian.com/ekupotacut
Problem
rockchip_p3phy_rk3568_init: lock failed)Phy link never came up)Root Cause
Missing / incorrect configuration for:
Solution
pcie30_pwrandpcie_oepinctrlvcc3v3_pcie+pcie_oe_regulatorwith active-low)&combphy1,&pcie30phyand&pcie3x2with correct clocks, phy-supply and reset polaritystartup-delay-usfor power stabilityThis makes the M.2 NVMe SSD work reliably on YY3568.
Documentation summary for feature / change
How Has This Been Tested?
lspcishows NVMe device andlsblk/nvme listworksTest Configuration:
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Chores