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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Hardware resource conflict on combphy0_ps between SATA and PCIe.
- [High] Unsupported `enable-gpios` property will leave HDMI ports unpowered.
- [Medium] Voltage mismatch in the `pcie20_avdd0v85` fixed regulator.
- [Medium] Missing sound card node and disabled I2S controller for the `es8388` audio codec.
- [Low] Missing `vpcie3v3-supply` property in PCIe nodes.
- [Low] `enable-active-high` property used without a corresponding `gpio` property in fixed regulators.
- [Low] Use of obsolete `regulator-compatible` property in I2C regulator nodes.
--

commit f0c2d80408904803253db0f7fedd438665749143
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.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-axon.dts b/arch/arm64/boot/dts/rockchip/rk3588-axon.dts
> new file mode 100644
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-axon.dts
[ ... ]
> +	pcie20_avdd0v85: pcie20-avdd0v85-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "pcie20_avdd0v85";
> +		regulator-boot-on;
> +		regulator-always-on;
> +		regulator-min-microvolt = <850000>;
> +		regulator-max-microvolt = <850000>;
> +		vin-supply = <&vdd_0v75_s0>;

[Severity: Medium]
Will this regulator fail to satisfy constraints during probe? The min and
max microvolt are set to 850mV, but the vin-supply is linked to
vdd_0v75_s0, which provides only 750mV. Since this fixed linear regulator
cannot step up the voltage, should this reference an 850mV supply like
vdd_0v85_s0 instead?

> +	};
[ ... ]
> +	sata_vcc_5v0: sata-vcc-5v0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "sata_vcc_5v0";
> +		startup-delay-us = <5000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +		enable-active-high;

[Severity: Low]
Does this node trigger a dtbs_check schema warning? The enable-active-high
property is used here to denote polarity, but there is no corresponding
gpio or gpios property defined. This same pattern also appears in the
vcc3v3_io_expander and vcc5v0_usb20_host regulator nodes.

> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		vin-supply = <&vcc12v_dcin>;
> +	};
[ ... ]
> +&hdmi0 {
> +	enable-gpios = <&gpio1 RK_PB0 GPIO_ACTIVE_HIGH>;

[Severity: High]
Will these HDMI ports remain unpowered? The Rockchip DesignWare HDMI driver
does not parse or support the enable-gpios property, so these GPIOs will
never be asserted. 

If these GPIOs are required to enable power to the transmitters, should
they be modeled as regulator-fixed nodes and linked properly so the power
domains are actually enabled?

> +	status = "okay";
> +};
[ ... ]
> +&hdmi1 {
> +	enable-gpios = <&gpio1 RK_PA7 GPIO_ACTIVE_HIGH>;

[Severity: High]
Is this subject to the same enable-gpios issue as hdmi0 above?

> +	status = "okay";
> +};
[ ... ]
> +&i2c0 {
> +	pinctrl-0 = <&i2c0m2_xfer>;
> +	pinctrl-names = "default";
> +	status = "okay";
> +
> +	vdd_cpu_big0_s0: vdd_cpu_big0_mem_s0: regulator@42 {
> +		compatible = "rockchip,rk8602";
> +		fcs,suspend-voltage-selector = <1>;
> +		reg = <0x42>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-compatible = "rk860x-reg";

[Severity: Low]
Is the regulator-compatible property needed here? This property is obsolete
and not parsed by the modern regulator driver, which relies exclusively on
the standard compatible string. Leaving it in might generate dtbs_check
warnings. This also appears in the vdd_cpu_big1_s0 node.

> +		regulator-max-microvolt = <1050000>;
[ ... ]
> +&i2c3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c3m0_xfer>;
> +	status = "okay";
> +
> +	es8388: audio-codec@11 {
> +		compatible = "everest,es8388";
> +		reg = <0x11>;
> +		clocks = <&cru I2S0_8CH_MCLKOUT>;

[Severity: Medium]
Is it expected that this audio codec will be unusable? It is instantiated
on the i2c3 bus, but there is no sound card node (such as
simple-audio-card) defined, and the i2s0_8ch controller it connects to
is not enabled. 

Will the Linux audio subsystem fail to create an audio device without
those nodes?

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

[Severity: High]
Will this cause a hardware resource conflict with sata0? In the RK3588 SoC,
both sata0 and pcie2x1l0 are hardwired to use the combphy0_ps Combo PHY by
default. 

Since the device tree enables both of them without remapping the PHY, and
the PHY can only be configured in one mode at a time, won't probing both
cause a failure or overwrite the PHY configuration? Should one of these use
combphy2_psu instead?

[Severity: Low]
Is the vpcie3v3-supply property missing from this PCIe controller node?
The board configures 3.3V power via the vcc3v3_pcie20_sata30 regulator, but
without the supply association, the PCIe framework might not be able to
logically manage slot power during resets or suspend/resume cycles.

[ ... ]
> +&pcie3x4 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie3_reset>;
> +	reset-gpios = <&gpio4 RK_PB6 GPIO_ACTIVE_HIGH>;
> +	status = "okay";
> +};

[Severity: Low]
Is this node also missing its vpcie3v3-supply property for the slot power?

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

[Severity: High]
Is this the SATA controller that will conflict with pcie2x1l0 over
combphy0_ps?

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