Re: [PATCH v2 3/3] arm64: dts: qcom: Add Vicharak Axon Mini

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The PCIe endpoints lack their power supply definitions (`vddpe-3v3-supply`), relying instead on `regulator-always-on`.
- [Medium] The USB 2.0 host controller lacks its VBUS power supply definition (`vbus-supply`), relying instead on `regulator-always-on`.
--

commit 9bf522377749eb6ea3477116679a7d9817103a9f
Author: Ajit Singh <blfizzyy@gmail.com>

arm64: dts: qcom: Add Vicharak Axon Mini

Add DTS for the Vicharak Axon Mini board based on the Qualcomm QCS6490 SoC.
This adds debug UART, eMMC, UFS, SDIO WLAN, USB 2.0 host, PCIe support along
with regulators.

> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-vicharak-axon-mini.dts b/arch/arm64/boot/dts/qcom/qcs6490-vicharak-axon-mini.dts
> new file mode 100644
> index 0000000000000..e9d54804ade1d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-vicharak-axon-mini.dts

[ ... ]

> +	vcc_5v0_usb2_0: regulator-vcc-5v0-usb2-0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc_5v0_usb2_0";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +
> +		vin-supply = <&vcc_5v0>;
> +
> +		gpio = <&tlmm 117 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +		pinctrl-0 = <&vcc5v0_usb2_0_en>;
> +		pinctrl-names = "default";
> +
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};
> +
> +	vcc_pcie1_3v3: regulator-vcc-pcie1-3v3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc_pcie1_3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +
> +		vin-supply = <&vcc_5v0>;
> +
> +		gpio = <&tlmm 115 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +		pinctrl-0 = <&vcc_pcie1_3v3_en>;
> +		pinctrl-names = "default";
> +
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};
> +
> +	vcc_pcie0_dsi_3v3: regulator-vcc-pcie0-dsi-3v3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc_pcie0_dsi_3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +
> +		vin-supply = <&vcc_5v0>;
> +
> +		gpio = <&tlmm 114 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +		pinctrl-0 = <&vcc_pcie0_dsi_3v3_en>;
> +		pinctrl-names = "default";
> +
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};

[ ... ]

> +&pcie0 {
> +	perst-gpios = <&tlmm 87 GPIO_ACTIVE_LOW>;
> +	wake-gpios = <&tlmm 89 GPIO_ACTIVE_HIGH>;
> +
> +	pinctrl-0 = <&pcie0_clkreq_n>, <&pcie0_reset_n>, <&pcie0_wake_n>;
> +	pinctrl-names = "default";
> +
> +	status = "okay";
> +};

Can the &pcie0 node properly power-manage its endpoint without a
vddpe-3v3-supply property?

The device tree defines vcc_pcie0_dsi_3v3 with regulator-always-on and doesn't
link it here. This breaks proper PCIe power management, as the driver cannot
turn power off during suspend or reset sequences.

[ ... ]

> +&pcie1 {
> +	perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> +	wake-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;
> +
> +	pinctrl-0 = <&pcie1_reset_n>, <&pcie1_wake_n>, <&pcie1_clkreq_n>;
> +	pinctrl-names = "default";
> +
> +	status = "okay";
> +};

Similarly here, should vcc_pcie1_3v3 be referenced as vddpe-3v3-supply for
the &pcie1 node instead of relying on regulator-always-on?

[ ... ]

> +&usb_2 {
> +	dr_mode = "host";
> +
> +	status = "okay";
> +};

Does the USB 2.0 host controller need a vbus-supply reference here?

The device tree defines vcc_5v0_usb2_0 as a 5V regulator but relies on
regulator-always-on instead of referencing it as vbus-supply (typically
via a usb-a-connector child node). Without this linkage, the USB driver
cannot control VBUS power when the port is inactive or suspended.

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