maint: rockchip64-current: rewrite kernel patches against 6.18.18#9527
maint: rockchip64-current: rewrite kernel patches against 6.18.18#9527igorpecovnik merged 1 commit intoarmbian:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR includes 18 patches for Rockchip64-6.18 kernel that enable and configure hardware features across multiple boards via device-tree modifications and kernel driver enhancements, including PCIe, HDMI audio, ethernet, USB-C alt-mode, panel displays, SPI, LED control from DT, USB PHY power management, and DMA error handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
patch/kernel/archive/rockchip64-6.18/board-sakurapi-rk3308b-spidev.patch (1)
23-37:⚠️ Potential issue | 🟠 MajorUse
armbian,spi-devinstead ofspidevfor the compatible property.The driver patch registers only
"armbian,spi-dev"in the device tree IDs table (spidev_dt_ids), but the board patch usescompatible = "spidev"at lines 24 and 35. This mismatch will prevent the SPI nodes from binding to the spidev driver.🔧 Proposed fix
spi_dev@0 { - compatible = "spidev"; + compatible = "armbian,spi-dev"; reg = <0>; spi-max-frequency = <0x2faf080>; }; @@ spi_dev@0 { - compatible = "spidev"; + compatible = "armbian,spi-dev"; reg = <0>; spi-max-frequency = <0x2faf080>; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-6.18/board-sakurapi-rk3308b-spidev.patch` around lines 23 - 37, The two device-tree nodes named spi_dev@0 (under the top-level and under &spi1) use compatible = "spidev" but the driver registers only "armbian,spi-dev" (spidev_dt_ids), so update those nodes to use compatible = "armbian,spi-dev"; locate the spi_dev@0 nodes in the patch and replace the compatible string for both occurrences to ensure the spidev driver binds to those SPI nodes.patch/kernel/archive/rockchip64-6.18/rk3399-usbc-phy-rockchip-naneng-Add-fallback-for-old-DTs.patch (1)
185-217:⚠️ Potential issue | 🔴 CriticalLogic bug: HOST mode is always overwritten when quirk is disabled.
The conditional structure in
dwc3_drd_update()has a missingelsethat causes HOST mode to be overwritten:if (ret > 0) mode = DWC3_GCTL_PRTCAP_HOST; if (dwc->usb3_phy_reset_quirk) { // <-- Should be "else if" ... } else { mode = DWC3_GCTL_PRTCAP_DEVICE; // <-- Overwrites HOST when quirk disabled }When
usb3_phy_reset_quirkis false andEXTCON_USB_HOSTis detected, the code setsmode = HOSTthen immediately falls through to theelsebranch settingmode = DEVICE.🐛 Proposed fix
if (dwc->edev) { ret = extcon_get_state(dwc->edev, EXTCON_USB_HOST); if (ret > 0) mode = DWC3_GCTL_PRTCAP_HOST; - - if (dwc->usb3_phy_reset_quirk) { + else if (dwc->usb3_phy_reset_quirk) { /* * With this quirk enabled, we want to pass 0 * to dwc3_set_mode to signal no USB connection * state. */ ret = extcon_get_state(dwc->edev, EXTCON_USB); if (ret > 0) mode = DWC3_GCTL_PRTCAP_DEVICE; } else { mode = DWC3_GCTL_PRTCAP_DEVICE; } dwc3_set_mode(dwc, mode); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-6.18/rk3399-usbc-phy-rockchip-naneng-Add-fallback-for-old-DTs.patch` around lines 185 - 217, In dwc3_drd_update the HOST mode set from extcon_get_state(..., EXTCON_USB_HOST) can be overwritten because the subsequent check uses "if (dwc->usb3_phy_reset_quirk)" rather than "else if"; update the conditional flow so the usb3_phy_reset_quirk branch is only evaluated when HOST was not already chosen—i.e., change the second "if (dwc->usb3_phy_reset_quirk)" to "else if (dwc->usb3_phy_reset_quirk)" (keeping use of extcon_get_state for EXTCON_USB and the final call to dwc3_set_mode with mode) so DWC3_GCTL_PRTCAP_HOST is not clobbered when detected.patch/kernel/archive/rockchip64-6.18/board-station-m2.patch (1)
212-214:⚠️ Potential issue | 🟡 MinorAdd
status = "okay"to enable the sata2 node.The
&sata2node setstarget-supplybut omitsstatus = "okay". All other sata2 nodes in rockchip64-6.18 (rk3566-jp-tvbox, rk3566-h96-tvbox, rk3568-9tripod-x3568-v4) explicitly set status, and there's a dedicated sata2 overlay that enables it. For consistency and to ensure the controller is actually enabled, addstatus = "okay"to the node:Suggested change
+&sata2 { + status = "okay"; + target-supply = <&vcc3v3_pcie>; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-6.18/board-station-m2.patch` around lines 212 - 214, The &sata2 device node currently only sets target-supply = <&vcc3v3_pcie> but omits status, so enable the SATA controller by adding status = "okay" to the &sata2 node (i.e., update the &sata2 block that contains target-supply and vcc3v3_pcie to include status = "okay"). Ensure the exact node name &sata2 and the existing target-supply = <&vcc3v3_pcie> remain unchanged.
🧹 Nitpick comments (4)
patch/kernel/archive/rockchip64-6.18/net-phy-realtek-add-rtl8211x-LED-configuration-from-OF.patch (1)
19-26: Consider checking the return value ofphy_write_paged().The function discards the return value of
phy_write_paged(), which means a write failure would go unnoticed. For LED configuration this is likely acceptable since it's a non-critical feature, but logging a warning on failure could aid debugging.🔧 Optional: Log warning on write failure
static void rtl821x_led_of_init(struct phy_device *phydev) { struct device *dev = &phydev->mdio.dev; u32 val; + int ret; - if (!of_property_read_u32(dev->of_node, "realtek,led-data", &val)) - phy_write_paged(phydev, 0xd04, 0x10, val); + if (!of_property_read_u32(dev->of_node, "realtek,led-data", &val)) { + ret = phy_write_paged(phydev, 0xd04, 0x10, val); + if (ret < 0) + dev_warn(dev, "failed to configure LED: %d\n", ret); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-6.18/net-phy-realtek-add-rtl8211x-LED-configuration-from-OF.patch` around lines 19 - 26, rtl821x_led_of_init currently ignores phy_write_paged()'s return value; change it to capture the return (e.g., int ret = phy_write_paged(phydev, 0xd04, 0x10, val);) and if ret < 0 emit a warning using the device pointer (dev_warn(dev, "rtl821x LED OF write failed: %d\n", ret);) so write failures are logged; keep the of_property_read_u32 flow and only attempt the write when that call succeeds.patch/kernel/archive/rockchip64-6.18/general-possibility-of-disabling-rk808-rtc.patch (1)
41-48: Compatible string format is non-standard but functional.The
.of_compatible = "rk808-rtc"lacks the typical vendor prefix (e.g.,"rockchip,rk808-rtc"). While this works for internal MFD cell matching, standard DT binding convention is"vendor,device". Since this is an existing patch being maintained across kernel versions, this is likely intentional for the specific use case.Consider whether other rk8xx variant arrays (rk808s, rk817s, etc.) might also benefit from this change if those chips are used on boards with alternative RTCs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-6.18/general-possibility-of-disabling-rk808-rtc.patch` around lines 41 - 48, Update the mfd_cell entry in the rk818s array so the .of_compatible value uses the standard vendor,device DT format instead of the bare string; specifically change the .of_compatible for the "rk808-rtc" mfd_cell to the vendor-prefixed form (e.g., "rockchip,rk808-rtc") and apply the same pattern to the other rk8xx variant arrays (rk808s, rk817s, etc.) where appropriate to keep device-tree compatibility consistent across RK8xx RTC entries.patch/kernel/archive/rockchip64-6.18/net-usb-r8152-add-LED-configuration-from-OF.patch (1)
31-45: Trailing whitespace on line 41.There appears to be trailing whitespace (tab character) on the blank line between the
if (ret)block and theocp_write_wordcall. While the patch will apply fine, this may trigger checkpatch warnings.🧹 Suggested fix to remove trailing whitespace
if (ret) return ret; - + ocp_write_word(tp, MCU_TYPE_PLA, PLA_LEDSEL, led_data);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-6.18/net-usb-r8152-add-LED-configuration-from-OF.patch` around lines 31 - 45, Remove the trailing whitespace on the blank line inside r8152_led_configuration: locate the function r8152_led_configuration (which calls of_property_read_u32 and ocp_write_word) and delete the tab/space on the empty line between the "if (ret) return ret;" block and the ocp_write_word(tp, MCU_TYPE_PLA, PLA_LEDSEL, led_data); call so there is a truly empty line (or no blank line) to avoid checkpatch warnings; save and re-run linting.patch/kernel/archive/rockchip64-6.18/rk3399-usbc-phy-rockchip-naneng-Add-fallback-for-old-DTs.patch (1)
70-76: Debug comment should be addressed or removed.The
//XXX: blehcomment indicates the author's dissatisfaction with settingusb3_phy_poweredinside the loop, where only the last port's result is reflected. For RK3399 with a single USB3 port this is functionally acceptable, but consider:
- Moving the flag assignment outside the loop with aggregate success tracking
- Or documenting why this is acceptable (single-port assumption)
♻️ Suggested improvement
if (dwc->usb3_phy_reset_quirk) { + bool all_powered = true; for (int j = 0; j < dwc->num_usb3_ports; j++) { ret = phy_power_on(dwc->usb3_generic_phy[j]); - //XXX: bleh - dwc->usb3_phy_powered = ret >= 0; + if (ret < 0) + all_powered = false; } + dwc->usb3_phy_powered = all_powered; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-6.18/rk3399-usbc-phy-rockchip-naneng-Add-fallback-for-old-DTs.patch` around lines 70 - 76, The inline debug "//XXX: bleh" flags that usb3_phy_powered is being overwritten per-iteration; change the logic in the block guarded by dwc->usb3_phy_reset_quirk so usb3_phy_powered reflects an aggregate result rather than the last port: call phy_power_on(dwc->usb3_generic_phy[j]) for each j in 0..dwc->num_usb3_ports-1, track success across all ports (e.g., set a local bool success and clear it on any negative ret) and assign dwc->usb3_phy_powered = success after the loop; alternatively, if the code truly assumes a single USB3 port, replace the comment with a brief justification mentioning that assumption and leave the single-port assignment as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@patch/kernel/archive/rockchip64-6.18/board-sakurapi-rk3308b-spidev.patch`:
- Around line 23-37: The two device-tree nodes named spi_dev@0 (under the
top-level and under &spi1) use compatible = "spidev" but the driver registers
only "armbian,spi-dev" (spidev_dt_ids), so update those nodes to use compatible
= "armbian,spi-dev"; locate the spi_dev@0 nodes in the patch and replace the
compatible string for both occurrences to ensure the spidev driver binds to
those SPI nodes.
In `@patch/kernel/archive/rockchip64-6.18/board-station-m2.patch`:
- Around line 212-214: The &sata2 device node currently only sets target-supply
= <&vcc3v3_pcie> but omits status, so enable the SATA controller by adding
status = "okay" to the &sata2 node (i.e., update the &sata2 block that contains
target-supply and vcc3v3_pcie to include status = "okay"). Ensure the exact node
name &sata2 and the existing target-supply = <&vcc3v3_pcie> remain unchanged.
In
`@patch/kernel/archive/rockchip64-6.18/rk3399-usbc-phy-rockchip-naneng-Add-fallback-for-old-DTs.patch`:
- Around line 185-217: In dwc3_drd_update the HOST mode set from
extcon_get_state(..., EXTCON_USB_HOST) can be overwritten because the subsequent
check uses "if (dwc->usb3_phy_reset_quirk)" rather than "else if"; update the
conditional flow so the usb3_phy_reset_quirk branch is only evaluated when HOST
was not already chosen—i.e., change the second "if (dwc->usb3_phy_reset_quirk)"
to "else if (dwc->usb3_phy_reset_quirk)" (keeping use of extcon_get_state for
EXTCON_USB and the final call to dwc3_set_mode with mode) so
DWC3_GCTL_PRTCAP_HOST is not clobbered when detected.
---
Nitpick comments:
In
`@patch/kernel/archive/rockchip64-6.18/general-possibility-of-disabling-rk808-rtc.patch`:
- Around line 41-48: Update the mfd_cell entry in the rk818s array so the
.of_compatible value uses the standard vendor,device DT format instead of the
bare string; specifically change the .of_compatible for the "rk808-rtc" mfd_cell
to the vendor-prefixed form (e.g., "rockchip,rk808-rtc") and apply the same
pattern to the other rk8xx variant arrays (rk808s, rk817s, etc.) where
appropriate to keep device-tree compatibility consistent across RK8xx RTC
entries.
In
`@patch/kernel/archive/rockchip64-6.18/net-phy-realtek-add-rtl8211x-LED-configuration-from-OF.patch`:
- Around line 19-26: rtl821x_led_of_init currently ignores phy_write_paged()'s
return value; change it to capture the return (e.g., int ret =
phy_write_paged(phydev, 0xd04, 0x10, val);) and if ret < 0 emit a warning using
the device pointer (dev_warn(dev, "rtl821x LED OF write failed: %d\n", ret);) so
write failures are logged; keep the of_property_read_u32 flow and only attempt
the write when that call succeeds.
In
`@patch/kernel/archive/rockchip64-6.18/net-usb-r8152-add-LED-configuration-from-OF.patch`:
- Around line 31-45: Remove the trailing whitespace on the blank line inside
r8152_led_configuration: locate the function r8152_led_configuration (which
calls of_property_read_u32 and ocp_write_word) and delete the tab/space on the
empty line between the "if (ret) return ret;" block and the ocp_write_word(tp,
MCU_TYPE_PLA, PLA_LEDSEL, led_data); call so there is a truly empty line (or no
blank line) to avoid checkpatch warnings; save and re-run linting.
In
`@patch/kernel/archive/rockchip64-6.18/rk3399-usbc-phy-rockchip-naneng-Add-fallback-for-old-DTs.patch`:
- Around line 70-76: The inline debug "//XXX: bleh" flags that usb3_phy_powered
is being overwritten per-iteration; change the logic in the block guarded by
dwc->usb3_phy_reset_quirk so usb3_phy_powered reflects an aggregate result
rather than the last port: call phy_power_on(dwc->usb3_generic_phy[j]) for each
j in 0..dwc->num_usb3_ports-1, track success across all ports (e.g., set a local
bool success and clear it on any negative ret) and assign dwc->usb3_phy_powered
= success after the loop; alternatively, if the code truly assumes a single USB3
port, replace the comment with a brief justification mentioning that assumption
and leave the single-port assignment as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cdcf96d0-5db8-498f-a702-428da90dedb1
📒 Files selected for processing (17)
patch/kernel/archive/rockchip64-6.18/board-nanopi-zero2-enable-pcie.patchpatch/kernel/archive/rockchip64-6.18/board-odroidm2-enable-hdmi-audio.patchpatch/kernel/archive/rockchip64-6.18/board-odroidm2-fix-for-ethernet.patchpatch/kernel/archive/rockchip64-6.18/board-pbp-add-dp-alt-mode.patchpatch/kernel/archive/rockchip64-6.18/board-sakurapi-rk3308b-fix-vop-panel.patchpatch/kernel/archive/rockchip64-6.18/board-sakurapi-rk3308b-fix-wlan-broken.patchpatch/kernel/archive/rockchip64-6.18/board-sakurapi-rk3308b-spidev.patchpatch/kernel/archive/rockchip64-6.18/board-station-m2.patchpatch/kernel/archive/rockchip64-6.18/drv-spi-spidev-remove-warnings.patchpatch/kernel/archive/rockchip64-6.18/general-disable-mtu-validation.patchpatch/kernel/archive/rockchip64-6.18/general-possibility-of-disabling-rk808-rtc.patchpatch/kernel/archive/rockchip64-6.18/net-phy-realtek-add-rtl8211x-LED-configuration-from-OF.patchpatch/kernel/archive/rockchip64-6.18/net-usb-r8152-add-LED-configuration-from-OF.patchpatch/kernel/archive/rockchip64-6.18/rk3399-usbc-phy-rockchip-naneng-Add-fallback-for-old-DTs.patchpatch/kernel/archive/rockchip64-6.18/rk3399-usbc-usb-dwc3-Track-the-power-state-of-usb3_generic_phy.patchpatch/kernel/archive/rockchip64-6.18/rk356x-add-51.2MHz-PLL-rate-for-HDMI.patchpatch/kernel/archive/rockchip64-6.18/temporary-workaround-dma-reset.patch
|
✅ This PR has been reviewed and approved — all set for merge! |
as per title
Summary by CodeRabbit
New Features
Bug Fixes
Chores