Add support for qidi x-* boards#9564
Conversation
Makerbase power control adaptions due to voltage regulator limitations
Makerbase power control adaptions due to voltage limitations
🚫 Missing required board assetsThis PR adds new board configuration(s). Required assets must already exist in github/armbian/armbian.github.io.
Missing items
Once the missing files are added (or a PR is opened in armbian/armbian.github.io), re-run this check. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
patch/kernel/archive/rockchip64-7.0/dt/rk3328-qidi-x6.dts (2)
206-206: Minor whitespace inconsistencies — leading spaces before tabs.Several lines have a leading space character before the tab indentation, which creates inconsistent formatting:
- Line 206:
&gmac2phystatus line- Lines 450-451, 456-457: pinctrl closing braces
- Line 487:
&spdif_soundstatus line- Line 510:
&uart0status line✨ Proposed whitespace fix (example)
&gmac2phy { assigned-clocks = <&cru SCLK_MAC2PHY>; assigned-clock-rate = <50000000>; assigned-clock-parents = <&cru SCLK_MAC2PHY_SRC>; phy-supply = <&vcc_phy>; - status = "okay"; + status = "okay"; };Also applies to: 450-451, 456-457, 487-487, 510-510
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-7.0/dt/rk3328-qidi-x6.dts` at line 206, Several device-tree lines contain a stray leading space before the tab indentation causing inconsistent whitespace; remove the extra leading space so indentation is only tabs for the affected nodes—fix the status line under &gmac2phy, the pinctrl closing brace lines, the status line under &spdif_sound, and the status line under &uart0 by deleting the single space character before the tab on each of those lines to make indentation uniform.
453-457: GPIO1_PC2 is used by bothpinctrl_spi0_cs2andgmac2ioreset.This pin (GPIO1_RK_PC2) is defined here for SPI0 CS2, but also used at line 259 as
snps,reset-gpiofor&gmac2io. Currentlygmac2iois disabled so there's no runtime conflict, but these are mutually exclusive configurations.Consider adding a comment to document this mutual exclusivity, preventing future misconfiguration if someone enables gmac2io.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-7.0/dt/rk3328-qidi-x6.dts` around lines 453 - 457, pinctrl_spi0_cs2 currently assigns GPIO1_PC2 (RK_PC2) for SPI0 CS2 while the node &gmac2io also uses the same pin as snps,reset-gpio; add a clear inline comment near the pinctrl_spi0_cs2 block (and optionally near the &gmac2io reset-gpio) stating that GPIO1_PC2 / RK_PC2 is shared and these configurations are mutually exclusive so enabling gmac2io requires removing or repurposing pinctrl_spi0_cs2, referencing the symbols pinctrl_spi0_cs2, RK_PC2 (GPIO1_PC2) and &gmac2io to make the constraint explicit for future maintainers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/boards/qidi-x6.csc`:
- Around line 14-15: The config sets HAS_VIDEO_OUTPUT="no" while still enabling
BOOT_LOGO="desktop", which is semantically inconsistent; update the board config
by removing BOOT_LOGO="desktop" or changing it to BOOT_LOGO="none" and ensure
PLYMOUTH remains disabled when HAS_VIDEO_OUTPUT is "no", i.e., edit the entries
for HAS_VIDEO_OUTPUT and BOOT_LOGO in this file so they are consistent (either
keep HAS_VIDEO_OUTPUT="no" and set BOOT_LOGO="none" or remove BOOT_LOGO).
---
Nitpick comments:
In `@patch/kernel/archive/rockchip64-7.0/dt/rk3328-qidi-x6.dts`:
- Line 206: Several device-tree lines contain a stray leading space before the
tab indentation causing inconsistent whitespace; remove the extra leading space
so indentation is only tabs for the affected nodes—fix the status line under
&gmac2phy, the pinctrl closing brace lines, the status line under &spdif_sound,
and the status line under &uart0 by deleting the single space character before
the tab on each of those lines to make indentation uniform.
- Around line 453-457: pinctrl_spi0_cs2 currently assigns GPIO1_PC2 (RK_PC2) for
SPI0 CS2 while the node &gmac2io also uses the same pin as snps,reset-gpio; add
a clear inline comment near the pinctrl_spi0_cs2 block (and optionally near the
&gmac2io reset-gpio) stating that GPIO1_PC2 / RK_PC2 is shared and these
configurations are mutually exclusive so enabling gmac2io requires removing or
repurposing pinctrl_spi0_cs2, referencing the symbols pinctrl_spi0_cs2, RK_PC2
(GPIO1_PC2) and &gmac2io to make the constraint explicit for future maintainers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 56cf3fc5-8de6-462e-96c4-7ce358c56831
📒 Files selected for processing (9)
.github/CODEOWNERSconfig/boards/qidi-x6.cscpatch/kernel/archive/rockchip64-6.18/dt/rk3328-mkspi.dtspatch/kernel/archive/rockchip64-6.18/dt/rk3328-qidi-x6.dtspatch/kernel/archive/rockchip64-7.0/dt/rk3328-mkspi.dtspatch/kernel/archive/rockchip64-7.0/dt/rk3328-qidi-x6.dtspatch/u-boot/u-boot-rockchip64/board_qidi-x6/01-qidi-x6-rk3328-add-defconfig.patchpatch/u-boot/u-boot-rockchip64/board_qidi-x6/02-qidi-x6-rk3328-add-dts.patchpatch/u-boot/u-boot-rockchip64/board_qidi-x6/02-qidi-x6-rk3328-add-mkspi-changes.patch
🚫 Missing required board assetsThis PR adds new board configuration(s). Required assets must already exist in github/armbian/armbian.github.io.
Missing items
Once the missing files are added (or a PR is opened in armbian/armbian.github.io), re-run this check. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
patch/kernel/archive/rockchip64-6.18/dt/rk3328-qidi-x6.dts (1)
12-14: Consider using a consistent vendor prefix in the compatible string.The compatible string uses
"mks,rk3328-qidi-x6"with the "mks" vendor prefix, but this DTS is for a Qidi board. If this is intentionally based on MKS hardware, the current naming is acceptable. Otherwise, consider using a Qidi-specific vendor prefix like"qidi,rk3328-qidi-x6"for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-6.18/dt/rk3328-qidi-x6.dts` around lines 12 - 14, Update the device tree's compatible string to use a Qidi-specific vendor prefix for consistency: change the compatible entry that currently contains "mks,rk3328-qidi-x6" (near the root node where model = "Qidi X-6") to "qidi,rk3328-qidi-x6" unless the MKS prefix was intentionally used; ensure the compatible list remains ["qidi,rk3328-qidi-x6", "rockchip,rk3328"] so downstream matching stays correct.patch/kernel/archive/rockchip64-7.0/dt/rk3328-qidi-x6.dts (1)
12-14: Vendor prefix inconsistency in compatible string.The compatible string uses
"mks,rk3328-qidi-x6"but the board is branded as Qidi. If this is because the Qidi boards are MKS derivatives/OEM products, this is fine. Otherwise, consider using"qidi,rk3328-x6"for proper vendor identification.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patch/kernel/archive/rockchip64-7.0/dt/rk3328-qidi-x6.dts` around lines 12 - 14, The compatible property currently lists "mks,rk3328-qidi-x6" which mismatches the model "Qidi X-6"; update the compatible string to use the correct vendor prefix (e.g., "qidi,rk3328-x6") unless this board is intentionally an MKS OEM variant; modify the compatible entry in the root node (/ { compatible = ... }) to reflect the chosen vendor prefix and ensure the ordering and secondary fallback "rockchip,rk3328" remain intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@patch/kernel/archive/rockchip64-6.18/dt/rk3328-qidi-x6.dts`:
- Around line 29-49: The opp-microvolt entries for the DMC OPP nodes (e.g.,
opp-786000000, opp-798000000, opp-840000000, opp-924000000, opp-1068000000)
contain an obvious typo where the third value is 12000000 (12V); change those
third values to 1200000 (1.2V) so the opp-microvolt tuples reflect realistic DDR
rail voltages; update each opp-* node's opp-microvolt array accordingly while
leaving the first two values and any status fields unchanged.
In `@patch/kernel/archive/rockchip64-7.0/dt/rk3328-qidi-x6.dts`:
- Around line 1-14: Update the U-Boot defconfig so the default DTB matches this
kernel DTS: change the CONFIG_DEFAULT_DEVICE_TREE value from "rk3328-roc-cc" to
"rk3328-qidi-x6" in the defconfig patch that sets CONFIG_DEFAULT_DEVICE_TREE;
ensure the string exactly matches the DTS filename base (rk3328-qidi-x6) so
U-Boot will pass the correct DTB to the kernel at boot.
- Around line 29-49: The OPP node entries (e.g., opp-786000000, opp-798000000,
opp-840000000, opp-924000000, opp-1068000000) have an incorrect third value in
their opp-microvolt arrays set to 12000000 (12V); update each opp-microvolt
third value to 1200000 (1.2V) to reflect the correct DDR max voltage, ensuring
you edit the opp-microvolt properties for all listed OPP nodes so the max
voltage is 1200000 µV.
---
Nitpick comments:
In `@patch/kernel/archive/rockchip64-6.18/dt/rk3328-qidi-x6.dts`:
- Around line 12-14: Update the device tree's compatible string to use a
Qidi-specific vendor prefix for consistency: change the compatible entry that
currently contains "mks,rk3328-qidi-x6" (near the root node where model = "Qidi
X-6") to "qidi,rk3328-qidi-x6" unless the MKS prefix was intentionally used;
ensure the compatible list remains ["qidi,rk3328-qidi-x6", "rockchip,rk3328"] so
downstream matching stays correct.
In `@patch/kernel/archive/rockchip64-7.0/dt/rk3328-qidi-x6.dts`:
- Around line 12-14: The compatible property currently lists
"mks,rk3328-qidi-x6" which mismatches the model "Qidi X-6"; update the
compatible string to use the correct vendor prefix (e.g., "qidi,rk3328-x6")
unless this board is intentionally an MKS OEM variant; modify the compatible
entry in the root node (/ { compatible = ... }) to reflect the chosen vendor
prefix and ensure the ordering and secondary fallback "rockchip,rk3328" remain
intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9650b737-2143-4d1a-ab33-68f572e00b83
📒 Files selected for processing (3)
config/boards/qidi-x6.cscpatch/kernel/archive/rockchip64-6.18/dt/rk3328-qidi-x6.dtspatch/kernel/archive/rockchip64-7.0/dt/rk3328-qidi-x6.dts
✅ Files skipped from review due to trivial changes (1)
- config/boards/qidi-x6.csc
🚫 Missing required board assetsThis PR adds new board configuration(s). Required assets must already exist in github/armbian/armbian.github.io.
Missing items
Once the missing files are added (or a PR is opened in armbian/armbian.github.io), re-run this check. |
|
image pull request added to armbian.github.io repo |
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
Added reusable.yml config change. Same goes for others. |
|
✅ This PR has been reviewed and approved — all set for merge! |
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Added a new board configuration for Qidi X-* 3D printer motherboards. In the past the MKS Pi image was used what made some problems due to different hardware configurations. The qidi baords lack sone of the hardware and interfaces. This config disables some device tree entries for better hardware support and less error messages during boot.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.
Checklist:
Please delete options that are not relevant.
Summary by CodeRabbit
New Features
Chores