Refactor patches for NanoPC T6 & LTS #7980
Conversation
SuperKali
requested review from
ColorfulRhino,
JohnTheCoolingFan,
Manouchehri,
TRSx80,
TheSnowfield,
Tonymac32,
ahoneybun,
amazingfate,
brentr,
catalinii,
clee,
efectn,
igorpecovnik,
joekhoobyar,
krachlatte,
lanefu,
paolosabatino,
rpardini and
schwar3kat
as code owners
github-actions
bot
added
size/large
|
I will pick and test on LTS, ASAP. |
|
This is mostly touching @ColorfulRhino's patch for fan and a regulator. Inputs? |
Absolutely not, the fan-related patch I had written here #7393 |
My bad, you're Co-Author on ColorfulRhino's commit, git fooled me. |
SuperKali
force-pushed
the
nanopct6-refactor
branch
from
96198d6 to
4849a0c
Compare
WalkthroughThe changes update the Rockchip RK3588 NanoPC T6 device tree to align the hardware configuration with new functionalities. The updates include adding nodes for USB3.0 power supply, PWM-controlled fan support, and thermal management. Specific modifications introduce a PWM fan node with associated cooling levels, trip points, and cooling maps, as well as a fixed regulator node with GPIO control for enabling power supplies. Adjustments to USB-A functionality add a Suggested labels
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (2)
patch/kernel/archive/rockchip64-6.14/rk3588-1051-board-nanopc-t6-fan-support.patch (1)
19-25: PWM fan node looks good, but fix indentationThe PWM fan implementation correctly defines the necessary properties including compatibility, cooling levels, and PWM parameters. The cooling levels progression (100, 160, 190, 200, 215, 235, 255) provides a good range of fan speeds.
There's an indentation inconsistency in the
fan-supplyline that should be fixed:- fan-supply = <&vcc5v0_sys>; + fan-supply = <&vcc5v0_sys>;patch/kernel/archive/rockchip64-6.14/rk3588-1053-board-nanopc-t6-hdmi1-and-audio-support.patch (1)
3-4: Future date in commit metadata.The commit date is set to March 20, 2025, which is in the future. This might be a typo or a system clock issue.
-Date: Thu, 20 Mar 2025 19:51:46 +0000 +Date: Thu, 20 Mar 2024 19:51:46 +0000
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
patch/kernel/archive/rockchip64-6.14/rk3588-1051-board-nanopc-t6-Add-USB3-psu-and-fan-support.patch(0 hunks)patch/kernel/archive/rockchip64-6.14/rk3588-1051-board-nanopc-t6-fan-support.patch(1 hunks)patch/kernel/archive/rockchip64-6.14/rk3588-1052-board-nanopc-t6-fix-usb3-a.patch(1 hunks)patch/kernel/archive/rockchip64-6.14/rk3588-1053-board-nanopc-t6-fix-usb3-a.patch(0 hunks)patch/kernel/archive/rockchip64-6.14/rk3588-1053-board-nanopc-t6-hdmi1-and-audio-support.patch(4 hunks)
💤 Files with no reviewable changes (2)
- patch/kernel/archive/rockchip64-6.14/rk3588-1053-board-nanopc-t6-fix-usb3-a.patch
- patch/kernel/archive/rockchip64-6.14/rk3588-1051-board-nanopc-t6-Add-USB3-psu-and-fan-support.patch
🔇 Additional comments (11)
patch/kernel/archive/rockchip64-6.14/rk3588-1052-board-nanopc-t6-fix-usb3-a.patch (3)
18-19: Power supply configuration added for USB 2.0 PHY OTG portThis change correctly adds the
phy-supplyproperty to theu2phy0_otgnode, connecting it to thevbus5v0_usbpower supply. This is essential for proper USB OTG functionality as it ensures the PHY has the correct 5V power reference.
25-27: USB role management improvement with extcon propertyAdding the
extconproperty to link the USB host controller (usb_host0_xhci) with the USB PHY (u2phy0) is an appropriate fix. This configuration enables proper external connector state management and helps with USB role switching, which is necessary for the controller that already hasusb-role-switchanddr_mode = "host"properties.
1-10: Well-structured patch with clear purposeThis patch follows proper Git formatting with a descriptive subject line, sign-off, and clear indication of the changes being made. The commit message clearly states the purpose of fixing USB-A 3.0 functionality on the NanoPC T6.
patch/kernel/archive/rockchip64-6.14/rk3588-1051-board-nanopc-t6-fan-support.patch (2)
34-60: Well-structured thermal configurationThe thermal management configuration is properly implemented with appropriate temperature thresholds and hysteresis values. The polling delay of 1000ms is reasonable for thermal monitoring.
The thermal trip points at 55°C and 65°C with the corresponding cooling maps create a two-stage fan control that should provide effective cooling while minimizing noise:
- At 55°C, the fan starts at a low speed (cooling level 1)
- At 65°C, the fan increases to at least cooling level 2 and can scale up as needed
1-6: Update subject and sign-off informationThe patch subject line and sign-off information look good, providing clear information about the purpose of the patch.
patch/kernel/archive/rockchip64-6.14/rk3588-1053-board-nanopc-t6-hdmi1-and-audio-support.patch (6)
19-28: HDMI1 connector configuration looks good.The HDMI Type-A connector is properly defined with the correct endpoint connection to the HDMI1 output.
38-40: HDMI0 audio support enabled correctly.Enabling the hdmi0_sound node by setting status to "okay" properly activates audio support for HDMI0.
49-67: HDMI1 configuration is complete and correct.The configuration properly:
- Enables the HDMI1 controller
- Sets up input from the video processor (VP1)
- Configures output to the physical connector
- Enables audio support for HDMI1
The connections between endpoints are correctly established.
73-75: HDMI1 physical layer enabled correctly.Enabling hdptxphy1 is necessary for HDMI1 to function, as it handles the electrical signaling.
84-90: Audio interfaces enabled correctly.Enabling the I2S5 and I2S6 8-channel interfaces is necessary for proper audio support through HDMI.
101-106: Video processor configuration for HDMI1 looks good.The VP1 endpoint is correctly configured to output to HDMI1, completing the video pipeline from processor to display.
|
Correct on the inconsistent indentation, wrong on the commit date (today is march 25th 2025). |
|
That rabbit is a tiny bit drunk, but otherwise he helps more than hinders. |
SuperKali
force-pushed
the
nanopct6-refactor
branch
from
4849a0c to
113365e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
patch/kernel/archive/rockchip64-6.14/rk3588-1051-board-nanopc-t6-fan-support.patch (4)
24-24: Fix indentation inconsistency.Line 24 uses spaces for indentation while the rest of the file uses tabs, which is inconsistent with the Linux kernel coding style.
- fan-supply = <&vcc5v0_sys>; + fan-supply = <&vcc5v0_sys>;
53-54: Consider finer-grained fan speed control for the first thermal trip point.The current implementation jumps directly to cooling level 1 (160) when temperature reaches 55°C. You might want to consider a more gradual ramp-up for quieter operation under moderate load.
- cooling-device = <&fan0 THERMAL_NO_LIMIT 1>; + cooling-device = <&fan0 0 1>;
1-7: Fix the commit date in the patch header.The commit date (March 20, 2025) is inconsistent with the current date indicated in the PR comments (March 25, 2025).
22-22: Consider documenting cooling level values for better maintainability.Adding a comment explaining the relationship between cooling levels and fan speed percentages would improve code maintainability for future developers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
patch/kernel/archive/rockchip64-6.14/rk3588-1051-board-nanopc-t6-Add-USB3-psu-and-fan-support.patch(0 hunks)patch/kernel/archive/rockchip64-6.14/rk3588-1051-board-nanopc-t6-fan-support.patch(1 hunks)patch/kernel/archive/rockchip64-6.14/rk3588-1052-board-nanopc-t6-fix-usb3-a.patch(1 hunks)patch/kernel/archive/rockchip64-6.14/rk3588-1053-board-nanopc-t6-fix-usb3-a.patch(0 hunks)patch/kernel/archive/rockchip64-6.14/rk3588-1053-board-nanopc-t6-hdmi1-and-audio-support.patch(4 hunks)
💤 Files with no reviewable changes (2)
- patch/kernel/archive/rockchip64-6.14/rk3588-1051-board-nanopc-t6-Add-USB3-psu-and-fan-support.patch
- patch/kernel/archive/rockchip64-6.14/rk3588-1053-board-nanopc-t6-fix-usb3-a.patch
🚧 Files skipped from review as they are similar to previous changes (2)
- patch/kernel/archive/rockchip64-6.14/rk3588-1052-board-nanopc-t6-fix-usb3-a.patch
- patch/kernel/archive/rockchip64-6.14/rk3588-1053-board-nanopc-t6-hdmi1-and-audio-support.patch
🔇 Additional comments (2)
patch/kernel/archive/rockchip64-6.14/rk3588-1051-board-nanopc-t6-fan-support.patch (2)
19-25: Effective fan node configuration with proper compatibility and parameters.The PWM fan implementation looks good, with appropriate cooling levels and PWM configuration. The fan is properly connected to PWM1 and powered by the system 5V supply.
34-60: Well-structured thermal configuration with appropriate trip points.The thermal management implementation is good, with sensible temperature thresholds (55°C and 65°C) and hysteresis values. The cooling maps properly connect the temperature trip points to the fan cooling levels.
Efficiency 🤣
|
SuperKali
added
Ready to merge
|
Tested (clearly a bit late for @igorpecovnik 's merge surge 😜 ) on T6-LTS. It didn't break anything for me, all USB ports still work. but I think that was already there before this. Also |
I waited longer then usual :) |
Description
Regarding PR #7969, I have reworked all the patches for the NanoPC T6 because many things have changed in the mainline. As a result, USB 3.0 on the NanoPC T6 was failing to show up. However, there is an issue in the mainline where USB 3.0 does not work out of the box on the NanoPC T6 non-LTS version. To fix this, I added the correct VBUS supply to the appropriate PHY node and the necessary extcon to connect the
host0node to the correct PHY. With these changes, USB 3.0 now works correctly on the NanoPC T6 non-LTS.Summary:
extcon = "u2phy0"tousb_host0_xhciand updatedvbus-supplyforu2phy0_otgtovcc5v0_usb.How Has This Been Tested?
Checklist: