Re: [PATCH v3 2/2] arm64: dts: rockchip: add ALIENTEK QuarkPi-CA2

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Medium] arm64: dts: rockchip: Type-C SuperSpeed OF graph bypasses USBDP PHY
- [Medium] arm64: dts: rockchip: Invalid DAPM routing mixes paths and bypasses amplifier
- [Medium] arm64: dts: rockchip: USB Type-C data role mismatch with host controller
- [Low] arm64: dts: rockchip: Makefile entries are not alphabetically sorted
- [Low] arm64: dts: rockchip: Erroneous brace indentation in I2C nodes
--

commit 37d68f4aec1739a1867e904e0086c7d08ca21350
Author: BG9OXA <bg9oxa@163.com>

arm64: dts: rockchip: add ALIENTEK QuarkPi-CA2

This patch adds device tree support for the ALIENTEK QuarkPi-CA2, an
RK3588S-based single-board computer. It introduces nodes for various hardware
components including Ethernet, USB, audio, and display interfaces.

> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index e7936a21c1185..34423e8cfbc97 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -231,6 +231,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-khadas-edge2.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-lubancat-4.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-nanopi-r6s.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-nanopi-r6c.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-quarkpi-ca2.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-odroid-m2.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-orangepi-5.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-orangepi-5b.dtb

[Severity: Low]
Does this insertion break the alphabetical sorting of the Makefile?

It appears quarkpi is placed between nanopi and odroid, which could
lead to future merge conflicts.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-quarkpi-ca2.dts b/arch/arm64/boot/dts/rockchip/rk3588s-quarkpi-ca2.dts
> new file mode 100644
> index 0000000000000..3f971ff8a6f0d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-quarkpi-ca2.dts

[ ... ]

> +		usb_con: connector {
> +			compatible = "usb-c-connector";
> +			label = "USB-C";
> +			power-role = "dual";
> +			data-role = "dual";
> +			try-power-role = "sink";
> +			source-pdos = <0x401912c>;
> +			sink-pdos = <0x4019064>;
> +			op-sink-microwatt = <1000000>;
> +
> +			altmodes {
> +				displayport {
> +					svid = /bits/ 16 <0xff01>;
> +					vdo = <0xffffffff>;
> +				};
> +			};
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					reg = <0>;
> +					usbc0_hs: endpoint {
> +						remote-endpoint = <&usb_host0_hs_ep>;
> +					};
> +				};
> +
> +				port@1 {
> +					reg = <1>;
> +					usbc0_ss: endpoint {
> +						remote-endpoint = <&usb_host0_ss_ep>;
> +					};
> +				};
> +
> +				port@2 {
> +					reg = <2>;
> +					usbc0_sbu: endpoint {
> +						remote-endpoint = <&usbdp_phy0_dp_out>;
> +					};
> +				};
> +			};
> +		};
> +		};
> +		};

[Severity: Low]
Are these closing braces aligned correctly?

It looks like the husb311 and i2c2 nodes end with misaligned and stacked
closing braces, which violates kernel coding style and hurts readability.

[ ... ]

> +&usb_host0_xhci {
> +	dr_mode = "host";
> +	status = "okay";

[Severity: Medium]
Is the dual data-role in the usb_con node compatible with this xHCI
controller configuration?

The usb_con node claims dual data roles, but the DWC3 controller here
is locked to host mode and lacks the usb-role-switch property. Will
this cause role swap requests to fail or break device mode functionality?

[ ... ]

> +&i2c1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c1m4_xfer>;
> +	status = "okay";
> +
> +	es8388: audio-codec@11 {
> +		compatible = "everest,es8388", "everest,es8328";
> +		reg = <0x11>;
> +		clocks = <&cru I2S0_8CH_MCLKOUT_TO_IO>;
> +		assigned-clocks = <&cru I2S0_8CH_MCLKOUT>;
> +		assigned-clock-rates = <12288000>;
> +		AVDD-supply = <&vcc_3v3_s0>;
> +		DVDD-supply = <&vcc_1v8_s0>;
> +		HPVDD-supply = <&vcc_3v3_s0>;
> +		PVDD-supply = <&vcc_3v3_s0>;
> +		#sound-dai-cells = <0>;
> +	};
> +	};

[Severity: Low]
Is this closing brace for the i2c1 node indented correctly?

It appears misaligned, which affects the readability of the device tree file.

[ ... ]

> +		simple-audio-card,routing =
> +			"Headphones", "LOUT1",
> +			"Headphones", "ROUT1",
> +			"Headphones", "Headphone Power OUTL",
> +			"Headphones", "Headphone Power OUTR",
> +			"Headphone Power INL", "LOUT1",
> +			"Headphone Power INR", "ROUT1",
> +			"Headphone Power INL", "LOUT2",
> +			"Headphone Power INR", "ROUT2",

[Severity: Medium]
Does this DAPM routing incorrectly mix paths and bypass the amplifier?

This configuration appears to connect Headphones directly to LOUT1/ROUT1
while also connecting them to the amplifier output, bypassing the headphone
amplifier. It also shorts the codec outputs LOUT1 and LOUT2 into the amplifier
inputs. Could this cause incorrect audio mixing or potential hardware stress?

[ ... ]

> +&usbdp_phy0 {
> +	orientation-switch;
> +	mode-switch;
> +	sbu1-dc-gpios = <&gpio4 RK_PA6 GPIO_ACTIVE_HIGH>;
> +	sbu2-dc-gpios = <&gpio4 RK_PA7 GPIO_ACTIVE_HIGH>;
> +	rockchip,dp-lane-mux = <2 3>;
> +	status = "okay";
> +
> +	port {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		usbdp_phy0_dp_in: endpoint@2 {
> +			reg = <2>;
> +			remote-endpoint = <&dp0_out_con>;
> +		};

[Severity: Medium]
Does this SuperSpeed routing bypass the USBDP PHY?

Earlier in the file, the Type-C connector's SuperSpeed endpoint (usbc0_ss)
is routed directly to usb_host0_ss_ep instead of going through the USBDP PHY.
Here, the usbdp_phy0 port node is missing the SuperSpeed endpoints
(endpoint@0 and endpoint@1) that should sit between the DWC3 and the Type-C
connector.

Will this break hardware orientation switching for SuperSpeed lines since
the PHY handles the orientation muxing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179086059727.25869.4898747284365209003@163.com?part=2