edge: bump to 6.18 stable and rewrite patches #9016
Conversation
EvilOlaf
requested review from
ColorfulRhino,
HeyMeco,
JohnTheCoolingFan,
SuperKali,
TheSnowfield,
Tonymac32,
ahoneybun,
amazingfate,
brentr,
catalinii,
chainsx,
efectn,
igorpecovnik,
joekhoobyar,
krachlatte,
lanefu,
paolosabatino,
prahal,
pyavitz,
rpardini and
schwar3kat
as code owners
github-actions
bot
added
02
WalkthroughUpdate kernel version target to 6.19-rc1, add device tree LED configuration support for RTL8169 Ethernet driver, introduce USB3 PHY power state tracking and disconnected device state handling for RK3399 USB3 controller. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
✅ This PR has been reviewed and approved — all set for merge! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
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.18/rk3399-usbc-phy-rockchip-naneng-Add-fallback-for-old-DTs.patch (1)
70-76: Fix error handling logic and remove unprofessional comment.Two issues:
Critical: Line 74 has broken error handling. The expression
ret >= 0evaluates totruefor success andfalsefor error, but this is backwards from the intended flag semantics. This meansusb3_phy_poweredwill be set tofalseon success andtrueon error.Minor: The
//XXX: blehcomment at line 74 is unprofessional and should be replaced with a meaningful explanation or removed.Apply this diff to fix the error handling:
if (dwc->usb3_phy_reset_quirk) { 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) + dev_err(dwc->dev, "failed to power on USB3 PHY: %d\n", ret); } + dwc->usb3_phy_powered = (ret >= 0); }Alternatively, track success more robustly by checking all PHY power-on results rather than just the last one.
🧹 Nitpick comments (1)
patch/kernel/archive/rockchip64-6.18/rk3399-usbc-usb-dwc3-Track-the-power-state-of-usb3_generic_phy.patch (1)
29-41: Verify unconditional clearing of usb3_phy_powered flag.The
usb3_phy_poweredflag is set tofalseat line 38 even when the power-off loop doesn't execute (whendwc->usb3_phy_poweredis initiallyfalse). While this provides defensive safety, it may mask logic errors where the flag state becomes inconsistent.Consider whether the flag should only be cleared after actually powering off the PHYs:
if (dwc->usb3_phy_powered) - for (i = 0; i < dwc->num_usb3_ports; i++) + for (i = 0; i < dwc->num_usb3_ports; i++) { phy_power_off(dwc->usb3_generic_phy[i]); - dwc->usb3_phy_powered = false; + } + dwc->usb3_phy_powered = false; + }Alternatively, if unconditional clearing is intentional for defensive programming (e.g., to handle cases where the flag may be set incorrectly), document this behavior in a comment.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
config/sources/mainline-kernel.conf.sh(1 hunks)patch/kernel/archive/rockchip64-6.18/net-ethernet-realtek-add-r8169-LED-configuration-from-OF.patch(2 hunks)patch/kernel/archive/rockchip64-6.18/rk3399-usbc-phy-rockchip-naneng-Add-fallback-for-old-DTs.patch(5 hunks)patch/kernel/archive/rockchip64-6.18/rk3399-usbc-usb-dwc3-Track-the-power-state-of-usb3_generic_phy.patch(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8968
File: patch/u-boot/u-boot-sunxi/arm64-dts-sun50i-h6-orangepi.dtsi-Rollback-r_rsb-to-r_i2c.patch:36-36
Timestamp: 2025-11-20T18:20:11.985Z
Learning: The rewrite-patches tool (REWRITE_PATCHES=yes) in the Armbian build system can inadvertently introduce semantic changes when the u-boot/kernel git base revision differs from expected state. The tool applies patches, commits them, and re-exports them using git format-patch, which can cause the re-exported patch to reflect the base revision's state rather than preserving the original patch intent. This is particularly problematic for device tree changes like interrupt specifications. The tool currently lacks validation mechanisms to detect such semantic drift, and affected patches must be manually corrected after rewriting.
Learnt from: rpardini
Repo: armbian/build PR: 8879
File: config/sources/families/uefi-x86.conf:0-0
Timestamp: 2025-11-06T15:36:04.682Z
Learning: As of PR #8879, the uefi-x86 family in the Armbian build system now includes kernel patches for the first time. The current and edge branches for uefi-x86 are specifically configured for Apple T2-based x86 machines, including T2-specific patches from the linux-t2 project and custom kernel configuration options for Apple hardware drivers.
Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-07-21T04:12:02.439Z
Learning: In the Armbian build system, for recurring maintenance tasks like kernel version bumping, TODO comments should use generic version formats (e.g., "MAJOR.MINOR-rc1") rather than specific version numbers (e.g., "6.17-rc1") to avoid the need for frequent comment updates that would create unnecessary maintenance overhead.
Learnt from: tabrisnet
Repo: armbian/build PR: 8746
File: config/sources/families/filogic.conf:61-68
Timestamp: 2025-10-13T02:26:18.249Z
Learning: In the Armbian build system, the "edge" kernel branch is intended to be bleeding-edge and experimental. It does not require hardware testing or stability verification before merging, as it's expected to contain potentially unstable or untested code for early adopters and developers.
Learnt from: The-going
Repo: armbian/build PR: 8147
File: config/sources/families/include/sunxi64_common.inc:38-39
Timestamp: 2025-04-30T16:16:47.150Z
Learning: The Armbian build system references Linux kernel versions in the form "tag:v6.14.4" in the KERNELBRANCH variable, even when point release tags might not be directly visible in the upstream repository in the same form.
Learnt from: amazingfate
Repo: armbian/build PR: 8453
File: lib/functions/compilation/patch/drivers_network.sh:213-216
Timestamp: 2025-08-02T14:59:28.145Z
Learning: The wireless driver patches wireless-rtl8812au-Fix-6.16.patch, wireless-rtl8811cu-Fix-6.16.patch, and wireless-rtl88x2bu-Fix-6.16.patch in the Armbian build system are backward compatible and can be applied unconditionally without version checks, even though they're named for 6.16+ fixes.
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
Learnt from: tabrisnet
Repo: armbian/build PR: 8746
File: config/kernel/linux-filogic-edge.config:344-470
Timestamp: 2025-10-11T19:52:54.648Z
Learning: The chainsx/linux-filogic kernel (legacy branch) for BPI-R4 uses out-of-tree proprietary drivers (CONFIG_NET_DSA_AN8855, CONFIG_AIROHA_EN8801SC_PHY, CONFIG_AIR_AN8855_PHY, CONFIG_MDIO_AN8855, CONFIG_MFD_AIROHA_AN8855). The frank-w/BPI-Router-Linux kernel (current and edge branches) uses mainline kernel drivers instead: CONFIG_NET_DSA_MT7530 for the DSA switch and CONFIG_MEDIATEK_2P5GE_PHY for PHY support. These provide equivalent functionality for BPI-R4 networking.
📚 Learning: 2025-04-30T16:16:47.150Z
Learnt from: The-going
Repo: armbian/build PR: 8147
File: config/sources/families/include/sunxi64_common.inc:38-39
Timestamp: 2025-04-30T16:16:47.150Z
Learning: The Armbian build system references Linux kernel versions in the form "tag:v6.14.4" in the KERNELBRANCH variable, even when point release tags might not be directly visible in the upstream repository in the same form.
Applied to files:
config/sources/mainline-kernel.conf.sh
📚 Learning: 2025-07-21T04:12:02.439Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-07-21T04:12:02.439Z
Learning: In the Armbian build system, for recurring maintenance tasks like kernel version bumping, TODO comments should use generic version formats (e.g., "MAJOR.MINOR-rc1") rather than specific version numbers (e.g., "6.17-rc1") to avoid the need for frequent comment updates that would create unnecessary maintenance overhead.
Applied to files:
config/sources/mainline-kernel.conf.sh
📚 Learning: 2025-11-10T22:05:40.490Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8913
File: config/sources/families/k3-beagle.conf:16-16
Timestamp: 2025-11-10T22:05:40.490Z
Learning: In the Armbian build system, kernel branches using non-mainline/vendor forks (like BeagleBoard's linux repository) should be named "vendor" or "vendor-rt" rather than "current" or "edge". The "current" and "edge" naming is reserved for mainline kernel branches. This affects both the case statement in family config files (e.g., `vendor | vendor-rt)` instead of `current | current-rt)`) and the corresponding KERNEL_TARGET declarations in board config files.
Applied to files:
config/sources/mainline-kernel.conf.sh
📚 Learning: 2025-09-09T07:30:22.419Z
Learnt from: amazingfate
Repo: armbian/build PR: 8595
File: lib/functions/compilation/patch/drivers_network.sh:0-0
Timestamp: 2025-09-09T07:30:22.419Z
Learning: RTW_WARN_LMT sed workaround in driver_rtl8852bs() function in lib/functions/compilation/patch/drivers_network.sh is necessary for kernel 6.1 compatibility, even when the upstream wifi-rtl8852bs driver has the macro defined for newer kernels. The workaround ensures cross-kernel compatibility from 6.1 to 6.16+.
Applied to files:
config/sources/mainline-kernel.conf.shpatch/kernel/archive/rockchip64-6.18/net-ethernet-realtek-add-r8169-LED-configuration-from-OF.patch
📚 Learning: 2025-06-25T03:42:09.086Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8330
File: config/sources/families/sun55iw3.conf:32-36
Timestamp: 2025-06-25T03:42:09.086Z
Learning: In Armbian build system configuration files like config/sources/families/*.conf, KERNELSOURCE is explicitly declared when using unofficial or 3rd party kernel repositories (like the "dev" branch using https://github.com/apritzel/linux), but can be omitted when using the standard mainline kernel (like the "edge" branch) since it will fall back to the default mainline source.
Applied to files:
config/sources/mainline-kernel.conf.sh
📚 Learning: 2025-09-11T06:06:52.328Z
Learnt from: SuperKali
Repo: armbian/build PR: 8608
File: lib/functions/compilation/patch/drivers_network.sh:361-363
Timestamp: 2025-09-11T06:06:52.328Z
Learning: For the RTW88 SDIO RF path detection fix patch (004-rtw88-sdio-rf-path-detection-fix.patch), SuperKali prefers using exact version matching (eq 6.1 || eq 6.16) rather than range checks, as the patch has only been tested on those specific kernel versions and may be mainstreamed soon.
Applied to files:
config/sources/mainline-kernel.conf.sh
📚 Learning: 2025-11-02T20:49:56.719Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.719Z
Learning: In Armbian board configuration files (config/boards/*.conf, *.csc, etc.), do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__* functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Board-specific kernel modifications cause inconsistency in kernel packages published to the apt repository because boards within a family share the same kernel packages. Kernel configuration changes must be made in the appropriate kernel config file (e.g., config/kernel/linux-*-*.config) or in family configuration files (config/sources/families/*.conf, *.inc) instead.
Applied to files:
config/sources/mainline-kernel.conf.sh
📚 Learning: 2025-05-05T12:35:07.143Z
Learnt from: Grippy98
Repo: armbian/build PR: 8152
File: lib/functions/configuration/interactive.sh:209-266
Timestamp: 2025-05-05T12:35:07.143Z
Learning: For the interactive kernel selection in Armbian, KERNEL_MAJOR_MINOR and KERNEL_DESCRIPTION are parsed from family.conf but deliberately not set as environment variables to avoid potential interference with other parts of the build system.
Applied to files:
config/sources/mainline-kernel.conf.sh
📚 Learning: 2025-06-24T10:08:40.313Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8328
File: lib/functions/compilation/patch/drivers_network.sh:542-545
Timestamp: 2025-06-24T10:08:40.313Z
Learning: In the Armbian build system, when a PR removes build support for a specific kernel version, version check issues for that removed version become practically irrelevant even if they appear incorrect in isolation. Context about which kernel versions are being deprecated/removed is important for understanding the impact of version-related code changes.
Applied to files:
config/sources/mainline-kernel.conf.sh
📚 Learning: 2025-10-13T02:26:18.249Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8746
File: config/sources/families/filogic.conf:61-68
Timestamp: 2025-10-13T02:26:18.249Z
Learning: In the Armbian build system, the "edge" kernel branch is intended to be bleeding-edge and experimental. It does not require hardware testing or stability verification before merging, as it's expected to contain potentially unstable or untested code for early adopters and developers.
Applied to files:
config/sources/mainline-kernel.conf.sh
📚 Learning: 2025-03-31T22:20:48.475Z
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.
Applied to files:
config/sources/mainline-kernel.conf.sh
📚 Learning: 2025-11-20T18:20:11.985Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8968
File: patch/u-boot/u-boot-sunxi/arm64-dts-sun50i-h6-orangepi.dtsi-Rollback-r_rsb-to-r_i2c.patch:36-36
Timestamp: 2025-11-20T18:20:11.985Z
Learning: The rewrite-patches tool (REWRITE_PATCHES=yes) in the Armbian build system can inadvertently introduce semantic changes when the u-boot/kernel git base revision differs from expected state. The tool applies patches, commits them, and re-exports them using git format-patch, which can cause the re-exported patch to reflect the base revision's state rather than preserving the original patch intent. This is particularly problematic for device tree changes like interrupt specifications. The tool currently lacks validation mechanisms to detect such semantic drift, and affected patches must be manually corrected after rewriting.
Applied to files:
patch/kernel/archive/rockchip64-6.18/rk3399-usbc-phy-rockchip-naneng-Add-fallback-for-old-DTs.patch
🧬 Code graph analysis (1)
config/sources/mainline-kernel.conf.sh (2)
config/sources/vendors/microsoft/wsl2.hooks.sh (2)
post_family_config_branch_current__wsl2(14-19)post_family_config_branch_edge__wsl2(21-26)lib/functions/configuration/interactive.sh (1)
get_kernel_info_for_branch(180-208)
🔇 Additional comments (8)
patch/kernel/archive/rockchip64-6.18/net-ethernet-realtek-add-r8169-LED-configuration-from-OF.patch (2)
27-34: LGTM with minor observation.The LED configuration function correctly reads the device tree property and writes to the LED_CTRL register only when the property is present. The implementation uses
of_property_read_u32which handles missing properties gracefully by returning non-zero.Note: This function is only called for RTL8168H variants. If other RTL8168 variants also support LED configuration from device tree, consider calling this function in their initialization paths as well.
1-49: This concern may not apply to the Armbian build workflow.In the Armbian build system, patch directories are not strictly version-locked to their directory names. According to documented patterns, patch directories referenced in kernel patch configurations can be applied across multiple kernel versions without issue. The placement of this patch in the
rockchip64-6.18directory does not necessarily prevent it from being applied when building for 6.19-rc1, as the build system's patch application logic is more flexible than a strict version-matching requirement. Verify the PR's kernel configuration to confirm whether 6.18 patches are intentionally included in the 6.19-rc1 build, rather than assuming a directory mismatch indicates an error.patch/kernel/archive/rockchip64-6.18/rk3399-usbc-usb-dwc3-Track-the-power-state-of-usb3_generic_phy.patch (1)
21-28: Consider error handling in power-on path.The flag
usb3_phy_powered = trueis set after the USB2 PHY power-on at line 25, which happens after all USB3 PHYs are powered on. If USB2 PHY power-on fails (line 21-22), the function goes toerr_power_off_usb3_phy, but the flag remainsfalse.Verify that this is the intended behavior: the flag should track whether USB3 PHYs were successfully powered on, and setting it after USB2 power-on appears correct since error paths will power off USB3 PHYs without the flag being set.
patch/kernel/archive/rockchip64-6.18/rk3399-usbc-phy-rockchip-naneng-Add-fallback-for-old-DTs.patch (4)
157-157: LGTM! Clever use of masking for DISCONNECTED state.The
DWC3_GCTL_PRTCAP_DEVICE_DISCONNECTEDvalue (6) is designed to mask down toDWC3_GCTL_PRTCAP_DEVICE(2) when masked withDWC3_GCTL_PRTCAP_OTG(3), as shown in line 24. This allows the disconnected state to be tracked incurrent_dr_rolewhile writing the correct register value.The comment at lines 152-157 clearly explains this design choice.
236-241: Verify impact of broader extcon notifier registration.The change from
extcon_register_notifier(dwc->edev, EXTCON_USB_HOST, &dwc->edev_nb)toextcon_register_notifier_all(dwc->edev, &dwc->edev_nb)means the notifier will now fire for all extcon events (includingEXTCON_USB), not justEXTCON_USB_HOST.This is necessary for the quirk logic (lines 210-212) which checks
EXTCON_USBstate, but ensure:
- The
dwc3_drd_notifiercallback can handle all event types efficiently- There are no performance implications from additional notifications
- All extcon event types are handled correctly in
dwc3_drd_updateSince the quirk-specific logic relies on checking multiple extcon states, verify this change is required for correct operation.
62-65: Verify PHY power-off loop matches power-on loop structure.The USB3 PHY power-off loop (lines 62-64) executes when
dwc->usb3_phy_powered && dwc->usb3_phy_reset_quirk, but the subsequent power-on loop (lines 71-76) executes when justdwc->usb3_phy_reset_quirkis set.This asymmetry means the power-on loop will execute even if the power-off loop didn't run (when
usb3_phy_poweredwasfalse). Verify this is intentional—it could result in callingphy_power_onon already-powered PHYs or PHYs that were never powered off.Consider whether both loops should have matching conditions:
- if (dwc->usb3_phy_powered && dwc->usb3_phy_reset_quirk) + if (dwc->usb3_phy_reset_quirk && dwc->usb3_phy_powered) for (int j = 0; j < dwc->num_usb3_ports; j++) phy_power_off(dwc->usb3_generic_phy[j]); reg = dwc3_readl(dwc->regs, DWC3_GCTL); reg |= DWC3_GCTL_CORESOFTRESET; dwc3_writel(dwc->regs, DWC3_GCTL, reg); - if (dwc->usb3_phy_reset_quirk) { + if (dwc->usb3_phy_reset_quirk && dwc->usb3_phy_powered) {Or document why unconditional power-on is safe in this context.
189-218: Verify extcon state logic handles mode transitions safely.The code correctly uses
extcon_get_state()with proper return value checking (ret > 0) and callsdwc3_set_mode()via the driver's helper rather than direct register access. However, confirm that:
- The
dwc3_set_mode()call properly serializes mode changes with interrupt handlers, as dwc3 race conditions between PRTCAP switches and running gadget/host event handling are known to exist in the kernel driver.- When
usb3_phy_reset_quirkis enabled, the logic correctly handles edge cases where bothEXTCON_USB_HOSTandEXTCON_USBstates might be sampled across multipleextcon_get_state()calls, or where the extcon state changes between the two checks.- Runtime-PM state is correct before calling
dwc3_set_mode(), as the controller may be runtime-suspended during mode transitions on some platforms.config/sources/mainline-kernel.conf.sh (1)
10-13: LGTM! The kernel version bump from 6.18-rc7 to 6.19-rc1 follows the expected progression, and the TODO comment appropriately uses generic version format as per project conventions.
|
For edge let’s check if qcs6490 family / radxa dragon-q6a also compiles fine. Else let’s pin that family config to 6.18 stable |
5 participants
Description
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.