SMART AM40 improve#9735
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRemaps the RK3399 RTC alias to Changes
Sequence Diagram(s)sequenceDiagram
participant DT as Device Tree
participant Driver as VPD Platform Driver
participant GPIO as GPIO Controller
participant IRQ as IRQ Subsystem
participant ExtCon as Extcon Core
DT->>Driver: Probe (det-gpios, vpd-data-role, booleans)
Driver->>GPIO: devm_gpiod_get(..., GPIOD_IN)
GPIO-->>Driver: GPIO descriptor
Driver->>IRQ: request threaded IRQ for det GPIO
IRQ-->>Driver: IRQ invoked on state change
Driver->>Driver: schedule_delayed_work
Driver->>GPIO: gpiod_get_value(det)
GPIO-->>Driver: value (0/1)
Driver->>ExtCon: set EXTCON states (USB / USB_HOST / DISP_DP) and properties (polarity, superspeed)
ExtCon-->>ExtCon: notify userspace / subsystem
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
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 (1)
patch/kernel/archive/rockchip64-7.0/general-add-miniDP-virtual-extcon.patch (1)
244-255:⚠️ Potential issue | 🟠 MajorEnforce the
det-gpiosbinding requirement in code.Line 244 uses
devm_gpiod_get_optional(), but the device tree binding explicitly marksdet-gpiosas required. Line 251 then callsgpiod_to_irq()unconditionally without checking ifvpd->det_gpiois NULL. All in-tree nodes already providedet-gpios, so change todevm_gpiod_get()to match the binding contract, or add a null check and early return after line 244. Apply the same fix to the 6.18 and 6.12 copies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-7.0/general-add-miniDP-virtual-extcon.patch` around lines 244 - 255, The code incorrectly uses devm_gpiod_get_optional() to obtain vpd->det_gpio despite the device-tree binding marking det-gpios as required, then calls gpiod_to_irq() unconditionally; replace devm_gpiod_get_optional() with devm_gpiod_get() when populating vpd->det_gpio (or alternatively add an explicit NULL check and early return if vpd->det_gpio is NULL) so that gpiod_to_irq(vpd->det_gpio) is only called with a valid GPIO; apply the same change to the 6.18 and 6.12 backported copies to keep bindings consistent.
🤖 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-7.0/general-add-miniDP-virtual-extcon.patch`:
- Around line 244-255: The code incorrectly uses devm_gpiod_get_optional() to
obtain vpd->det_gpio despite the device-tree binding marking det-gpios as
required, then calls gpiod_to_irq() unconditionally; replace
devm_gpiod_get_optional() with devm_gpiod_get() when populating vpd->det_gpio
(or alternatively add an explicit NULL check and early return if vpd->det_gpio
is NULL) so that gpiod_to_irq(vpd->det_gpio) is only called with a valid GPIO;
apply the same change to the 6.18 and 6.12 backported copies to keep bindings
consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8e5fd9d8-2ef4-48ae-bd7e-9a3e33bef959
📒 Files selected for processing (9)
patch/kernel/archive/rockchip64-6.12/dt/rk3399-am40.dtspatch/kernel/archive/rockchip64-6.12/general-add-miniDP-virtual-extcon.patchpatch/kernel/archive/rockchip64-6.18/dt/rk3399-am40.dtspatch/kernel/archive/rockchip64-6.18/general-add-miniDP-virtual-extcon.patchpatch/kernel/archive/rockchip64-6.6/general-add-miniDP-dt-doc.patchpatch/kernel/archive/rockchip64-6.6/general-add-miniDP-virtual-extcon.patchpatch/kernel/archive/rockchip64-7.0/dt/rk3399-am40.dtspatch/kernel/archive/rockchip64-7.0/general-add-miniDP-virtual-extcon.patchpatch/u-boot/v2026.01/board_smart-am40/add-board-smart-am40.patch
💤 Files with no reviewable changes (2)
- patch/kernel/archive/rockchip64-6.6/general-add-miniDP-virtual-extcon.patch
- patch/kernel/archive/rockchip64-6.6/general-add-miniDP-dt-doc.patch
738a579 to
4faf0b2
Compare
Fixed |
|
✅ This PR has been reviewed and approved — all set for merge! |
Description
Optimizing SMART AM40 DTS
Change the rk808 alias from rtc99 to rtc1. rk808 RTC does not have actual functionality, so avoid letting it register as /dev/rtc0, because most kernel configurations restore time from /dev/rtc0 at boot. Registering the useless rk808 as rtc1 is better (as seen in most SBC DTS implementations); rtc99 is too large.
Change the naming of some nodes from specific IC models to IC types. This better conforms to Linux DTS naming conventions.
Add I2C speed configurations for the HDMI DDC, matching the implementation of other RK3399-based devices.
Adjust the PCIe to 2 lanes, as the M.2 Key-E slot hardware limit is 2 lanes.
Update DP HPD GPIO to active-high. The HPD pin goes high when HDMI is connected and low when disconnected. The previous incorrect description did not break HDMI (Type-C DP Alt Mode) because the Type-C Virtual PD extcon driver used
gpiod_get_raw_valueinstead ofgpiod_get_value, causing it to ignore theGPIO_ACTIVE_XXXflags defined in the DTS.Optimizing Type-C Virtual PD Extcon Driver
Change the det-gpio request flag from
GPIOD_ASIStoGPIOD_IN, as this pin is strictly an input.Switch from
gpiod_get_raw_valuetogpiod_get_valuefor reading det-gpio. This allows the driver to support devices with active-low HPD logic purely via DTS declarations without requiring driver code modifications.Update the driver patches for kernel versions 6.12, 6.18, and 7.0, as they can share the same patch file. The version 6.6 patch has been removed after confirming no DTS relies on it for that specific kernel.
How Has This Been Tested?
Build the 6.18 image for SMART AM40 and verified successful booting from an SD card. Following initial validation, the 7.0 kernel .deb packages were installed and tested after a reboot.
Build and test the NORCO EMB-3531 image to verify display functionality, as it also need the Type-C Virtual PD extcon driver.
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Improvements