Rockchip: youyeetoo yy3568: enable pci-e ssd#9876
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 updates device tree files for the RK3568 YY3568 board across kernel and U-Boot bootloader. Changes add PCIe controller wiring, Ethernet RGMII configuration, and PMIC power-control support while simplifying regulator suspend-state declarations in U-Boot. ChangesKernel Device Tree PCIe and Subsystem Configuration
U-Boot Device Tree Board Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 |
github-actions
Bot
added
size/large
github-actions
Bot
added
Hardware
hqnicolas
requested review from
EvilOlaf
and removed request for
ColorfulRhino,
SuperKali,
Tonymac32,
brentr,
catalinii,
chainsx,
efectn,
ginkage,
krachlatte,
schwar3kat and
sputnik2019
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
patch/kernel/archive/rockchip64-6.18/dt/rk3568-yy3568.dts (1)
587-589: 💤 Low valueClarify the purpose of enabling I2S0.
The PR objectives focus on enabling PCIe SSD support, but this change enables the
i2s0_8chaudio interface. Is this change intentional and related to the board bring-up, or is it outside the scope of this PR?🤖 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/rockchip64-6.18/dt/rk3568-yy3568.dts` around lines 587 - 589, The i2s0_8ch device node was enabled but the PR is about PCIe SSD support; confirm whether enabling i2s0_8ch is intentional or accidental and either remove the status = "okay"; change from the diff or add a clarifying comment and matching commit message explaining why i2s0_8ch is required for this board bring-up; locate the i2s0_8ch node in the DTS (symbol i2s0_8ch) and either revert that change or add a one-line comment above it describing its purpose and relevance to this PR.
🤖 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/kernel/archive/rockchip64-6.18/dt/rk3568-yy3568.dts`:
- Around line 126-137: Duplicate regulator-name values cause conflicts; rename
the new regulator block vcc3v3_pcie (regulator-name) to a unique identifier
(e.g., "vcc3v3_pcie3x2") so it no longer collides with vcc3v3_pi6c_05. Update
the regulator-name property inside the vcc3v3_pcie node and keep the node label
(vcc3v3_pcie:) or change it consistently if you prefer, ensuring any references
to that regulator (gpio/pinctrl usage) remain correct.
- Around line 9-14: The DTS file includes unused dt-bindings headers; remove the
two unnecessary include lines for dt-bindings/leds/common.h and
dt-bindings/input/input.h from rk3568-yy3568.dts so only the actually used
bindings remain (keep dt-bindings/clock/rk3568-cru.h since CLK_HDMI_CEC is
referenced); update the includes block to eliminate those two headers and run a
DT compile/check to ensure no references to COLOR_/FUNCTION_/LED_* or
KEY_/BTN_/... remain.
- Line 117: The DT node for the vcc3v3_pi6c_05 fixed-regulator currently uses
the deprecated singular property "gpio" (gpio = <&gpio0 RK_PC7
GPIO_ACTIVE_HIGH>); update it to the correct plural property "gpios" to match
fixed-regulator.yaml expectations. Locate the vcc3v3_pi6c_05 regulator node and
replace the "gpio" property name with "gpios" (keeping the same phandle and
tuple <&gpio0 RK_PC7 GPIO_ACTIVE_HIGH>) so the enable GPIO is expressed as
gpios, not gpio.
In `@patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3568-yy3568.dts`:
- Around line 126-144: The &gmac0 node currently sets phy-mode="rgmii" but lacks
explicit tx_delay/rx_delay which means Rockchip dwmac binding defaults will
apply; confirm whether the PHY provides internal RGMII delays or the MAC should
(compare to rk3568-hinlink-h68k.dts and the &gmac1 node), then add explicit
tx_delay and rx_delay properties to the &gmac0 device node (or change phy-mode
to the appropriate variant that indicates internal PHY delay) to match the
board-tuned values used in the kernel DT and ensure ethernet0 timing matches;
update the rk3568-yy3568.dts &gmac0 node to include the chosen tx_delay and
rx_delay properties or adjust phy-mode accordingly.
---
Nitpick comments:
In `@patch/kernel/archive/rockchip64-6.18/dt/rk3568-yy3568.dts`:
- Around line 587-589: The i2s0_8ch device node was enabled but the PR is about
PCIe SSD support; confirm whether enabling i2s0_8ch is intentional or accidental
and either remove the status = "okay"; change from the diff or add a clarifying
comment and matching commit message explaining why i2s0_8ch is required for this
board bring-up; locate the i2s0_8ch node in the DTS (symbol i2s0_8ch) and either
revert that change or add a one-line comment above it describing its purpose and
relevance to this PR.
🪄 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: 35a4dd33-3003-4196-8d6a-72c935187ef7
📒 Files selected for processing (2)
patch/kernel/archive/rockchip64-6.18/dt/rk3568-yy3568.dtspatch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3568-yy3568.dts
| #include <dt-bindings/leds/common.h> | ||
| #include <dt-bindings/pwm/pwm.h> | ||
| #include <dt-bindings/input/input.h> | ||
| #include <dt-bindings/pinctrl/rockchip.h> | ||
| #include <dt-bindings/soc/rockchip,vop2.h> | ||
| #include <dt-bindings/clock/rk3568-cru.h> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify usage of LED and input constants in the device tree
# Check for LED common constants (COLOR_, FUNCTION_, etc.)
echo "=== Checking for LED common.h constants ==="
rg -n '\b(COLOR_|FUNCTION_|LED_)\w+' patch/kernel/archive/rockchip64-6.18/dt/rk3568-yy3568.dts
# Check for input constants (KEY_, BTN_, etc.)
echo "=== Checking for input.h constants ==="
rg -n '\b(KEY_|BTN_|SW_|REL_|ABS_)\w+' patch/kernel/archive/rockchip64-6.18/dt/rk3568-yy3568.dts
# Check for RK3568 clock constants
echo "=== Checking for rk3568-cru.h constants ==="
rg -n '\bCLK_\w+' patch/kernel/archive/rockchip64-6.18/dt/rk3568-yy3568.dtsRepository: armbian/build
Length of output: 228
Drop unused LED/input dt-bindings headers in rk3568-yy3568.dts (lines 9-14)
dt-bindings/leds/common.h and dt-bindings/input/input.h aren’t referenced anywhere in this DTS (no COLOR_/FUNCTION_/LED_* or KEY_/BTN_/SW_/REL_/ABS_* usage found), while dt-bindings/clock/rk3568-cru.h is used via CLK_HDMI_CEC.
🤖 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/rockchip64-6.18/dt/rk3568-yy3568.dts` around lines 9 -
14, The DTS file includes unused dt-bindings headers; remove the two unnecessary
include lines for dt-bindings/leds/common.h and dt-bindings/input/input.h from
rk3568-yy3568.dts so only the actually used bindings remain (keep
dt-bindings/clock/rk3568-cru.h since CLK_HDMI_CEC is referenced); update the
includes block to eliminate those two headers and run a DT compile/check to
ensure no references to COLOR_/FUNCTION_/LED_* or KEY_/BTN_/... remain.
| compatible = "regulator-fixed"; | ||
| enable-active-high; | ||
| gpios = <&gpio0 RK_PC7 GPIO_ACTIVE_HIGH>; | ||
| gpio = <&gpio0 RK_PC7 GPIO_ACTIVE_HIGH>; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the correct property name in fixed-regulator binding
# Check the fixed-regulator binding documentation
fd -t f 'regulator.*\.yaml|fixed-regulator.*' --exec cat {} \; | grep -A5 -B5 "gpio:"Repository: armbian/build
Length of output: 39
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Locate fixed-regulator binding YAMLs
echo "== fixed-regulator binding candidates =="
fd -t f -i 'fixed-regulator.*\.ya?ml' . | head -n 50
# 2) Specifically search for 'fixed-regulator' under regulator bindings
echo
echo "== regulator binding files containing 'fixed-regulator' =="
fd -t f 'regulator*.yaml' . | while read -r f; do
if rg -n --fixed-string 'fixed-regulator' "$f" >/dev/null 2>&1; then
echo "$f"
fi
done | head -n 50
# 3) If we find any candidate yaml, grep for 'gpio' and 'gpios' keys within them
echo
echo "== Inspect keys in located fixed-regulator YAMLs =="
files="$(fd -t f -i 'fixed-regulator.*\.ya?ml' . | head -n 20 || true)"
if [ -z "$files" ]; then
echo "No fixed-regulator YAML found by name pattern."
exit 0
fi
echo "$files" | while read -r f; do
echo "--- $f ---"
rg -n 'gpios|gpio' "$f" | head -n 80 || true
doneRepository: armbian/build
Length of output: 258
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f="patch/kernel/archive/rockchip64-6.18/dt/rk3568-yy3568.dts"
echo "== Context around line 117 in $f =="
nl -ba "$f" | sed -n '95,135p'
echo
echo "== Repo-wide search for fixed-regulator bindings/schema =="
rg -n --hidden --no-ignore-vcs "fixed-regulator" . || true
echo
echo "== Repo-wide search for fixed regulator enable gpio wording =="
rg -n --hidden --no-ignore-vcs "fixed.*regulator.*gpio|enable.*(gpio|gpios)" patch . || true
echo
echo "== Repo-wide search for 'gpios:' key in any yaml schema =="
fd -t f -e yaml -e yml . | head -n 2000 >/dev/null
rg -n --hidden --no-ignore-vcs "gpios\s*:" . --glob '*.yaml' --glob '*.yml' || true
echo
echo "== Repo-wide search for 'gpio:' key in regulator bindings yaml =="
rg -n --hidden --no-ignore-vcs "^\s*gpio\s*:" . --glob '*regulator*' --glob '*.yaml' --glob '*.yml' || trueRepository: armbian/build
Length of output: 209
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f="patch/kernel/archive/rockchip64-6.18/dt/rk3568-yy3568.dts"
echo "== Context around line 117 in $f =="
awk 'NR>=110 && NR<=125 {printf "%5d\t%s\n", NR, $0}' "$f"
echo
echo "== Repo search: fixed-regulator in YAML/YML =="
rg -n --hidden --no-ignore-vcs "fixed[- ]regulator" . --glob '*.yaml' --glob '*.yml' || true
echo
echo "== Repo search: regulator fixed binding filenames =="
fd -t f --hidden --no-ignore-vcs -e yaml -e yml . | rg -n -i "fixed[-_]?regulator|regulator[/_-]?fixed" || true
echo
echo "== Repo search: enable gpios/gpio keys in YAML/YML =="
rg -n --hidden --no-ignore-vcs "enable.*(gpios|gpio)\s*:" . --glob '*.yaml' --glob '*.yml' || true
echo
echo "== Repo search: gpios: key anywhere in YAML/YML =="
rg -n --hidden --no-ignore-vcs "gpios\s*:" . --glob '*.yaml' --glob '*.yml' || trueRepository: armbian/build
Length of output: 878
🌐 Web query:
Linux kernel fixed-regulator.yaml enable GPIO property name gpio vs gpios
💡 Result:
In Linux kernel fixed-regulator device tree bindings, the GPIO property for enable control is gpios (plural). The singular gpio property is supported only for compatibility and is deprecated. Evidence: - fixed-regulator.yaml defines both gpio and gpios as properties with maxItems: 1, and the binding explicitly rejects having both at once in the fixed-regulator.yaml schema (“not: required: [ gpio, gpios ]”). [1] - kernel.org’s fixed-regulator.yaml copy shows gpio: “gpio to use for enable control” and gpios: “maxItems: 1”, and indicates the gpio/gpios distinction via binding logic. [1] - A Linux-Kernel Archive patch message states the intent: fixed regulator originally used “gpio” (singular) but standard GPIO bindings recommend “gpios” (plural); Linux was modified to handle both, and it recommends the best practice and deprecates the singular variant. [2] - The GPIO binding documentation states that GPIO properties should be named “[ -]gpios”; gpio is valid only for compatibility reasons and is deprecated for newer bindings. [3] So, for new DTs you should use: - gpios = <&gpioX >; and avoid: - gpio = <&gpioX >; (use only if you’re maintaining an older DT / need compatibility) [1][3][2]
Citations:
- 1: https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
- 2: https://lkml.iu.edu/1810.0/03504.html
- 3: https://www.kernel.org/doc/Documentation/devicetree/bindings/gpio/gpio.txt
DT fixed-regulator enable GPIO property should use gpios, not gpio.
In patch/kernel/archive/rockchip64-6.18/dt/rk3568-yy3568.dts (line 117), the vcc3v3_pi6c_05 fixed-regulator uses gpio = <&gpio0 RK_PC7 GPIO_ACTIVE_HIGH>;, but upstream fixed-regulator.yaml expects gpios (plural) for enable control; gpio (singular) is only a deprecated compatibility alias. If the change was from gpios → gpio, restore gpios.
🤖 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/rockchip64-6.18/dt/rk3568-yy3568.dts` at line 117, The
DT node for the vcc3v3_pi6c_05 fixed-regulator currently uses the deprecated
singular property "gpio" (gpio = <&gpio0 RK_PC7 GPIO_ACTIVE_HIGH>); update it to
the correct plural property "gpios" to match fixed-regulator.yaml expectations.
Locate the vcc3v3_pi6c_05 regulator node and replace the "gpio" property name
with "gpios" (keeping the same phandle and tuple <&gpio0 RK_PC7
GPIO_ACTIVE_HIGH>) so the enable GPIO is expressed as gpios, not gpio.
| vcc3v3_pcie: vcc3v3-pcie-regulator { | ||
| compatible = "regulator-fixed"; | ||
| regulator-name = "vcc3v3_pcie"; | ||
| regulator-min-microvolt = <3300000>; | ||
| regulator-max-microvolt = <3300000>; | ||
| enable-active-high; | ||
| gpio = <&gpio3 RK_PC3 GPIO_ACTIVE_HIGH>; | ||
| pinctrl-names = "default"; | ||
| pinctrl-0 = <&pcie30_pwr>; | ||
| startup-delay-us = <5000>; | ||
| vin-supply = <&vcc5v0_sys>; | ||
| }; |
There was a problem hiding this comment.
Duplicate regulator name detected.
The new vcc3v3_pcie regulator uses regulator-name = "vcc3v3_pcie" (line 128), but this name is already used by the vcc3v3_pi6c_05 regulator (line 120). Duplicate regulator names can cause conflicts in the regulator framework and make debugging difficult.
Consider renaming one of these regulators to ensure unique names. For example, the new regulator could be named "vcc3v3_pcie3x2" to reflect its use with the pcie3x2 controller.
🔧 Proposed fix to rename the new regulator
vcc3v3_pcie: vcc3v3-pcie-regulator {
compatible = "regulator-fixed";
- regulator-name = "vcc3v3_pcie";
+ regulator-name = "vcc3v3_pcie3x2";
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3300000>;🤖 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/rockchip64-6.18/dt/rk3568-yy3568.dts` around lines 126 -
137, Duplicate regulator-name values cause conflicts; rename the new regulator
block vcc3v3_pcie (regulator-name) to a unique identifier (e.g.,
"vcc3v3_pcie3x2") so it no longer collides with vcc3v3_pi6c_05. Update the
regulator-name property inside the vcc3v3_pcie node and keep the node label
(vcc3v3_pcie:) or change it consistently if you prefer, ensuring any references
to that regulator (gpio/pinctrl usage) remain correct.
| &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
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Looking for inherited gmac0 timing properties in included DTS/DTSI files..."
fd -a 'rk3568(\-u-boot)?\.dtsi$|rk3568-yy3568\.dts$' . \
| while read -r f; do
echo
echo "== $f =="
rg -n -C2 '(^&gmac0\b|tx_delay|rx_delay|phy-mode)' "$f" || true
doneRepository: armbian/build
Length of output: 2898
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TARGET="patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3568-yy3568.dts"
echo "== Includes in $TARGET =="
sed -n '1,220p' "$TARGET" | nl -ba | sed -n '1,140p' | sed -n '1,140p'
# specifically show include lines near top
echo
echo "== Include directives =="
rg -n '^\s*(`#include`\s+|/include/)' "$TARGET" || true
echo
echo "== Search for tx_delay/rx_delay anywhere in u-boot dt subtree =="
rg -n --hidden --no-ignore -S 'tx_delay|rx_delay' patch/u-boot/legacy/u-boot-radxa-rk35xx/dt || true
echo
echo "== Search specifically for gmac0 delay properties in any dts/dtsi =="
# Find all RK3568-related dt files under u-boot tree and search for &gmac0 with tx_delay/rx_delay nearby
fd -e dts -e dtsi rk3568 patch/u-boot/legacy/u-boot-radxa-rk35xx/dt \
| while read -r f; do
echo
echo "== $f =="
rg -n -C2 '(&gmac0\b.*\{|\btx_delay\b|\brx_delay\b|phy-mode\s*=\s*")' "$f" || true
done
echo
echo "== Search for tx_delay/rx_delay for gmac0 in kernel archive rk3568 dt files =="
fd -e dts -e dtsi rk3568 patch/kernel/archive/rockchip64-*/dt \
| while read -r f; do
# Only print if gmac0 or delays appear
if rg -n '(&gmac0\b|tx_delay|rx_delay)' "$f" >/dev/null; then
echo
echo "== $f =="
rg -n -C2 '(^\s*&gmac0\b|tx_delay|rx_delay)' "$f" || true
fi
doneRepository: armbian/build
Length of output: 197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TARGET="patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3568-yy3568.dts"
echo "== First 220 lines of $TARGET (no nl) =="
sed -n '1,220p' "$TARGET" | cat
echo
echo "== Include directives in $TARGET =="
rg -n '^\s*(`#include`\s+|/include/)' "$TARGET" || true
echo
echo "== Search tx_delay/rx_delay anywhere under u-boot DT tree =="
rg -n -S 'tx_delay|rx_delay' patch/u-boot/legacy/u-boot-radxa-rk35xx/dt || true
echo
echo "== Search for &gmac0 blocks (and nearby delay/phy-mode) within u-boot DT subtree =="
fd -e dts -e dtsi rk3568 patch/u-boot/legacy/u-boot-radxa-rk35xx/dt \
| while read -r f; do
if rg -n '(&gmac0\b|tx_delay|rx_delay|phy-mode\s*=\s*")' "$f" >/dev/null 2>&1; then
echo
echo "== $f =="
rg -n -C2 '(^\s*&gmac0\b|tx_delay|rx_delay|phy-mode\s*=\s*")' "$f" || true
fi
done
echo
echo "== Search for tx_delay/rx_delay under kernel archive RK3568 DT files =="
fd -e dts -e dtsi rk3568 patch/kernel/archive/rockchip64-*/dt \
| while read -r f; do
if rg -n 'tx_delay|rx_delay' "$f" >/dev/null 2>&1; then
echo
echo "== $f =="
rg -n -C2 '(&gmac0\b|tx_delay|rx_delay)' "$f" || true
fi
doneRepository: armbian/build
Length of output: 11317
🌐 Web query:
kernel device tree binding rgmii tx_delay rx_delay properties
💡 Result:
In Linux device-tree bindings for RGMII, the common delay-related properties you’ll see are the MAC-side pico-second properties: 1) Properties on the MAC/controller node - rx-internal-delay-ps: RGMII receive clock delay in pico seconds; if present, the MAC applies the RX delay [1]. - tx-internal-delay-ps: RGMII transmit clock delay in pico seconds; if present, the MAC applies the TX delay [1]. These are used with the RGMII phy-mode sub-modes where delays are expected to be handled internally by either side of the link (MAC vs PCB vs PHY). The binding documentation describes how rgmii/rgmii-id/rgmii-rxid/rgmii-txid relate to where delays come from [1]. 2) Example of a controller-specific binding using different property names Some older or vendor-specific controller bindings use tx-delay and rx-delay (in small integer steps) rather than *_internal-delay-ps. For example, the APM X-Gene ENET binding documents: - tx-delay: maps valid values 0..7 to specific picosecond delay values; default value is 4 (1611 ps) [2]. - rx-delay: maps valid values 0..7 to specific picosecond delay values; default value is 2 (899 ps) [2]. This is board/controller-specific, not the generic kernel-wide property naming. 3) Naming note: “rgmii tx_delay rx_delay properties” vs “tx-internal-delay-ps rx-internal-delay-ps” - If you are looking for the kernel’s generic, binding-documented properties: use rx-internal-delay-ps and tx-internal-delay-ps [1]. - If your DTS/SoC binding uses tx-delay and rx-delay, that’s likely a specific vendor binding where the semantics/ranges are defined in that device’s binding text (e.g., X-Gene ENET) [2]. If you share your specific Ethernet controller (compatible string) or the DTS snippet / binding file path, I can map exactly which property names and units apply to your device.
Citations:
- 1: https://github.com/Xilinx/linux-xlnx/blob/master/Documentation/devicetree/bindings/net/ethernet-controller.yaml
- 2: https://mjmwired.net/kernel/Documentation/devicetree/bindings/net/apm-xgene-enet.txt
🌐 Web query:
rockchip gmac rgmii tx_delay rx_delay device tree binding
💡 Result:
For Rockchip GMAC (dwmac) in device tree using RGMII, the binding expects the optional properties tx_delay and rx_delay on the Ethernet MAC node. 1) DT binding properties - tx_delay: “Delay value for TXD timing.” Range 0..0x7F, default 0x30 [1]. - rx_delay: “Delay value for RXD timing.” Range 0..0x7F, default 0x10 [1]. These are described in the Rockchip dwmac device-tree binding documentation (dwmac.yaml / legacy dwmac.txt). [1][2][3] 2) Example DT usage (RGMII) A typical Rockchip dwmac node sets phy-mode = "rgmii" and provides tx_delay/rx_delay values, e.g. tx_delay = <0x30>; rx_delay = <0x10>; [2]. 3) Notes about phy-mode and delay usage (why this matters) - In RGMII-id modes (rgmii-id / rgmii-txid / rgmii-rxid), the delays are typically applied by the PHY; Linux may not read/apply tx_delay/rx_delay from DT in those modes. One LKML thread describes a warning where tx_delay/rx_delay properties are missing/ignored depending on phy-mode, and explains the relationship between controller/PHY clock delays in the various rgmii-id modes. [4] - Another LKML thread notes that selecting rgmii-id on ROCK 4D led to no delays being configured in the MAC (since delays were expected to be provided elsewhere), and changing DT to configure MAC delays (via tx_delay/rx_delay) fixed link/data issues. [5] 4) Practical takeaway - If you want the Rockchip GMAC MAC to apply RGMII delays, set phy-mode to a mode where controller-side delays are used (commonly phy-mode = "rgmii") and set tx_delay and rx_delay on the MAC node per the binding defaults/ranges. [1][4][2] If you tell me your specific SoC (e.g., RK3568/RK3588/RK3576) and kernel version, I can point to the exact upstream DTS snippet/commit for that chip’s dwmac node and show what phy-mode it uses with tx_delay/rx_delay.
Citations:
- 1: https://sbexr.rabexc.org/latest/sources/28/73013145d50d97.html
- 2: https://gem5.googlesource.com/arm/linux/+/512cf465ee01eb23936a9e6ed0b6414eccb00853/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
- 3: https://android.googlesource.com/kernel/common.git/+/android-trusty-4.4/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
- 4: https://lkml.rescloud.iu.edu/2211.2/01549.html
- 5: https://lkml.org/lkml/2025/7/24/1087
Add/confirm RGMII delay tuning for &gmac0
patch/u-boot/legacy/u-boot-radxa-rk35xx/dt/rk3568-yy3568.dtssets&gmac0 { phy-mode = "rgmii"; }but omitstx_delay/rx_delay, unlike&gmac1.- A search across the u-boot RK3568 DT files (including
rk3568.dtsi/rk3568-u-boot.dtsiunder this DTS directory) shows notx_delay/rx_delayfor&gmac0, so the Rockchip dwmac binding defaults will apply (tx_delay default0x30, rx_delay default0x10), not board-tuned values. - Kernel RK3568 reference DTS (
rk3568-hinlink-h68k.dts) setstx_delay/rx_delayfor&gmac0, so u-boot likely needs equivalent delay properties unless PHY-side internal delay is intentionally providing the timing. - Confirm the intended delay source (PHY internal delay vs MAC delays) and add
tx_delay/rx_delayto&gmac0(or adjustphy-modeaccordingly) to avoid Ethernet timing mismatch onethernet0.
🤖 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 - 144, The &gmac0 node currently sets phy-mode="rgmii" but lacks explicit
tx_delay/rx_delay which means Rockchip dwmac binding defaults will apply;
confirm whether the PHY provides internal RGMII delays or the MAC should
(compare to rk3568-hinlink-h68k.dts and the &gmac1 node), then add explicit
tx_delay and rx_delay properties to the &gmac0 device node (or change phy-mode
to the appropriate variant that indicates internal PHY delay) to match the
board-tuned values used in the kernel DT and ensure ethernet0 timing matches;
update the rk3568-yy3568.dts &gmac0 node to include the chosen tx_delay and
rx_delay properties or adjust phy-mode accordingly.
|
This changes are already in #9832 |
1 participant
Description
enable PCI-e SSD on Machine model: Youyeetoo YY3568
How Has This Been Tested?
6.18.32-current-rockchip64 (build@armbian)
Forum Post
https://paste.armbian.com/ekupotacut
Checklist:
Summary by CodeRabbit
Release Notes