Re: [PATCH v3 3/4] arm64: dts: qcom: add basic devicetree for Ayaneo Pocket DS gaming console

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Critical] Unresolved references to non-existent nodes `&panel0_in` and `&panel1_in`.
- [High] DisplayPort is not correctly routed to the USB-C connector.
- [High] Missing PCI bridge properties in `pcie@0` root port node.
- [High] Missing `pinctrl-1` sleep state for the `pwm-fan`.
- [High] Thermal zones omit `cooling-maps` to bind the PWM fan and use incorrect trip types.
- [Medium] Empty `panel@0` node lacks a `compatible` string.
- [Low] Undocumented custom DT property `rocknix-u-boot-dt-id`.
--

commit d78e5e056b91e897429aec4c3dcd17b82685dc92
Author: Teguh Sobirin <teguh@sobir.in>

arm64: dts: qcom: add basic devicetree for Ayaneo Pocket DS gaming console

Add initial Device Tree for the Ayaneo Pocket DS gaming console based
on the Qualcomm QCS8550 platform.

> diff --git a/arch/arm64/boot/dts/qcom/qcs8550-ayaneo-pocketds.dts b/arch/arm64/boot/dts/qcom/qcs8550-ayaneo-pocketds.dts
> new file mode 100644
> index 0000000000000..416399a4179b4
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcs8550-ayaneo-pocketds.dts
> @@ -0,0 +1,1883 @@
> +// SPDX-License-Identifier: BSD-3-Clause

[ ... ]

> +/ {
> +	model = "AYANEO Pocket DS";
> +	compatible = "ayaneo,pocketds", "qcom,qcs8550", "qcom,sm8550";
> +	rocknix-u-boot-dt-id = "u-boot-pocket-ds";

This isn't a bug, but is this a documented standard property?
Using custom properties may trigger dtbs_check warnings.

> +
> +	aliases {

[ ... ]

> +	pmic-glink {
> +		compatible = "qcom,sm8550-pmic-glink", "qcom,pmic-glink";
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		orientation-gpios = <&tlmm 11 GPIO_ACTIVE_HIGH>;
> +
> +		connector@0 {
> +			compatible = "usb-c-connector";
> +
> +			reg = <0>;
> +
> +			power-role = "dual";
> +			data-role = "dual";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					reg = <0>;
> +
> +					pmic_glink_hs_in: endpoint {
> +						remote-endpoint = <&usb_1_dwc3_hs>;
> +					};
> +				};
> +
> +				port@1 {
> +					reg = <1>;
> +
> +					pmic_glink_ss_in: endpoint {
> +						remote-endpoint = <&redriver_ss_out>;
> +					};
> +				};
> +			};

The commit message mentions "DisplayPort is muxed over the USB-C connector".
Is the graph topology missing the DP Alt Mode port (port@2) for the
usb-c-connector?

> +		};
> +	};
> +
> +	pwm_fan: pwm-fan {
> +		compatible = "pwm-fan";
> +
> +		pinctrl-0 = <&fan_pwm_active>, <&fan_int>;
> +		pinctrl-names = "default", "sleep";

Does this missing pinctrl-1 property cause the pinctrl core to fail when
transitioning to sleep state?

> +
> +		fan-supply = <&vdd_fan_5v0>;

[ ... ]

> +&cpu7_top_thermal {
> +	polling-delay = <200>;
> +
> +	trips {
> +		cpu7_top_fan0: trip-point2 {
> +			temperature = <70000>;
> +			hysteresis = <3000>;
> +			type = "passive";
> +		};

Should active cooling devices like fans use type = "active" instead of
"passive"?

Also, are cooling-maps subnodes needed to bind these trip points to the
pwm_fan cooling device? Without them, the thermal framework might not
automatically engage the fan.

> +
> +		cpu7_top_fan1: trip-point3 {

[ ... ]

> +&mdss_dp0_out {
> +	remote-endpoint = <&panel0_in>;
> +
> +	data-lanes = <0 1 2 3>;
> +};

Are these endpoints defined anywhere? The commit message mentions display
panel support is deferred, but compiling this might result in a reference
to a non-existent node.

Also, should DisplayPort be routed to the USB QMP PHY's DisplayPort input
instead of a standalone panel, given it is muxed over the USB-C connector?

> +
> +&mdss_dsi0 {
> +	vdda-supply = <&vreg_l3e_1p2>;
> +
> +	status = "okay";
> +
> +	display_panel: panel@0 {
> +		reg = <0>;
> +
> +		pinctrl-0 = <&sde_dsi_active>, <&sde_te_active>;
> +		pinctrl-1 = <&sde_dsi_suspend>, <&sde_te_suspend>;
> +		pinctrl-names = "default", "sleep";
> +	};

Since display panel support is deferred, should this node be removed for now,
or at least include a compatible string to prevent dtbs_check errors?

> +};
> +
> +&mdss_dsi0_phy {

[ ... ]

> +&mdss_dsi1_out {
> +	remote-endpoint = <&panel1_in>;
> +	data-lanes = <0 1 2 3>;
> +};

Will this also cause a compilation failure due to referencing a non-existent
node, as panel support is deferred?

> +
> +&mdss_dsi1_phy {

[ ... ]

> +&pcie1 {
> +	wake-gpios = <&tlmm 99 GPIO_ACTIVE_HIGH>;
> +	perst-gpios = <&tlmm 97 GPIO_ACTIVE_LOW>;
> +
> +	pinctrl-0 = <&pcie1_default_state>;
> +	pinctrl-names = "default";
> +
> +	status = "okay";
> +
> +	pcie@0 {
> +		/* Renesas μPD720201 PCIe USB3.0 HOST CONTROLLER */
> +		usb-controller@0 {
> +			compatible = "pci1912,0014";
> +			reg = <0x10000 0x0 0x0 0x0 0x0>;

Does the pcie@0 bridge node need #address-cells = <3>, #size-cells = <2>, and
a reg property so the child's reg can be properly parsed by the PCI subsystem?

> +
> +			avdd33-supply = <&upd720201_avdd33_reg>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517-pocketds-v3-0-d5910c801756@gmail.com?part=3