Re: [PATCH v3 3/3] arm64: dts: rockchip: add Vicharak Axon board

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Hardware conflict: both `pcie2x1l0` and `sata0` are enabled, but they share the same single-lane PHY (`combphy0_ps`).
- [Medium] The SDIO node lacks UHS capability properties, limiting the Wi-Fi interface to 50MHz.
- [Medium] Incomplete configuration: USB PHYs and `combphy2` are enabled but their respective controllers are disabled.
- [Low] Undocumented Wi-Fi interface configuration.
--

commit 395e8a5adb0ca626a634aafea8c647363459acf1
Author: Hrushiraj Gandhi <hrushirajg23@gmail.com>

arm64: dts: rockchip: add Vicharak Axon board

Add initial support for the Vicharak Axon single-board computer based on
the Rockchip RK3588 SoC.

> The board supports:
> 
> - eMMC storage
> - microSD card
> - Gigabit Ethernet
> - HDMI output (dual HDMI)
> - HDMI input
> - USB 2.0 host ports
> - PCIe 2.0 slots
> - PCIe 3.0 x4 slot
> - SATA
> - RTC
> - Status LEDs

[Severity: Low]
The commit message lists the board's supported features but omits Wi-Fi.

However, the device tree includes an &sdio node configured for a Wi-Fi module,
along with an sdio_pwrseq.

Could the commit message be updated to include Wi-Fi, or is the Wi-Fi
configuration leftover code from a reference design?

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-vicharak-axon.dts b/arch/arm64/boot/dts/rockchip/rk3588-vicharak-axon.dts
> new file mode 100644
> index 0000000000000..9d1e61e1857ae
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-vicharak-axon.dts
> @@ -0,0 +1,1036 @@
[ ... ]
> +&combphy2_psu {
> +	status = "okay";
> +};

[Severity: Medium]
Are the controllers for this PHY supposed to be enabled as well?

combphy2_psu is explicitly enabled here, but all of its potential consumers
(pcie2x1l2, sata2, usb_host2_xhci) appear to be disabled.

Does this leave the PHY initialized but orphaned, potentially wasting power?

[ ... ]
> +&sdio {
> +	max-frequency = <150000000>;
> +	no-sd;
> +	no-mmc;
> +	bus-width = <4>;
> +	disable-wp;
> +	cap-sd-highspeed;
> +	cap-sdio-irq;
> +	keep-power-in-suspend;

[Severity: Medium]
Does this node need UHS capability properties to achieve the requested 150MHz?

Without properties like sd-uhs-sdr104, the Linux MMC core might assume the
host does not support SDR104/UHS speeds and artificially cap the interface to
High-Speed mode (50MHz).

Could this severely throttle the Wi-Fi bandwidth?

[ ... ]
> +&pcie2x1l0 {
> +	reset-gpios = <&gpio4 RK_PA5 GPIO_ACTIVE_HIGH>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie2_0_rst>;
> +	vpcie3v3-supply = <&vcc3v3_pcie20_sata30>;
> +	status = "okay";
> +};

[ ... ]
> +&sata0 {
> +	status = "okay";
> +};

[Severity: High]
Will enabling both pcie2x1l0 and sata0 create a hardware conflict?

In rk3588-base.dtsi, both pcie2x1l0 and sata-port@0 (in sata0) are hardcoded
to use combphy0_ps. combphy0_ps is a single-lane PHY that can only operate in
one mode at a time.

If both controllers are enabled without reassigning the PHY for either, will
one interface fail to link or corrupt the PHY configuration of the other?

[ ... ]
> +&u2phy0 {
> +	status = "okay";
> +};
> +
> +&u2phy0_otg {
> +	status = "okay";
> +};

[Severity: Medium]
Are the USB host controllers for these PHYs intended to be enabled?

u2phy0 and u2phy0_otg (along with u2phy1) are enabled, but their consumers
like usb_host0_xhci remain disabled.

Could this result in non-functional USB ports and wasted power?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601143022.153941-1-hrushirajg23@gmail.com?part=3