Re: [PATCH V4 6/6] arm64: dts: rockchip: Add Anbernic RG Vita-Pro

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Hardware Description Mismatch for Bluetooth Controller
- [Medium] Incomplete Device Tree description for SPI Joysticks
- [Medium] Contradictory Suspend States for PMIC Regulators
--

commit 79a0a3133515f64c6cd7697921d2b225d5dc8aa8
Author: Chris Morgan <macromorgan@hotmail.com>
Subject: arm64: dts: rockchip: Add Anbernic RG Vita-Pro

This commit adds the device tree for the Anbernic RG Vita-Pro handset,
based on the Rockchip RK3576 SoC. It includes initial hardware descriptions
for the display, audio, buttons, networking, and power management.

>  - RTL8852BE WiFi/Bluetooth combo

[Severity: High]
The commit message mentions an RTL8852BE combo chip, but the device tree
configures the Bluetooth controller using the realtek,rtl8822cs-bt compatible
over a UART bus below.

Is this compatible string correct for this hardware? The RTL8852BE typically
interfaces its Bluetooth module via USB rather than UART. If the hardware is
indeed RTL8822CS, should the commit message be updated to reflect the correct
chipset?

> diff --git a/arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts b/arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts
> new file mode 100644
> index 0000000000000..344ba18705338
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts
> @@ -0,0 +1,1327 @@

[ ... ]

> +			vdd_cpu_lit_s0: dcdc-reg3 {
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <550000>;
> +				regulator-max-microvolt = <950000>;
> +				regulator-name = "vdd_cpu_lit_s0";
> +				regulator-ramp-delay = <12500>;
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +					regulator-suspend-microvolt = <750000>;
> +				};
> +			};

[Severity: Medium]
Is it intentional to set both regulator-off-in-suspend and
regulator-suspend-microvolt here?

These properties appear to contradict each other, as turning the regulator off
would make the target suspend voltage meaningless. Would it be better to use
regulator-on-in-suspend if the voltage needs to be maintained during sleep?

[ ... ]

> +			vdd_gpu_s0: dcdc-reg5 {
> +				regulator-boot-on;
> +				regulator-enable-ramp-delay = <400>;
> +				regulator-min-microvolt = <550000>;
> +				regulator-max-microvolt = <900000>;
> +				regulator-name = "vdd_gpu_s0";
> +				regulator-ramp-delay = <12500>;
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +					regulator-suspend-microvolt = <850000>;
> +				};
> +			};

[Severity: Medium]
Does this GPU regulator have the same contradictory suspend state issue as
vdd_cpu_lit_s0 above?

[ ... ]

> +&spi1 {
> +	num-cs = <1>;
> +	pinctrl-0 = <&spi1m2_pins>, <&spi1m2_csn0>;
> +	status = "okay";
> +
> +	/* SPI controlled MCU for joystick and joystick LEDs. */
> +};

[Severity: Medium]
The commit message notes the analog joysticks are controlled via userspace.
Since there is no child node defined on this SPI bus, how does the kernel
instantiate the userspace interface?

Should there be a spidev node or another specific child device defined here
to allow userspace to communicate with the MCU?

[ ... ]

> +&uart6 {
> +	pinctrl-0 = <&uart6m1_xfer>, <&uart6m1_ctsn>, <&uart6m1_rtsn>;
> +	pinctrl-names = "default";
> +	uart-has-rtscts;
> +	status = "okay";
> +
> +	bluetooth {
> +		compatible = "realtek,rtl8822cs-bt";
> +		device-wake-gpios = <&gpio2 RK_PB6 GPIO_ACTIVE_HIGH>;
> +		enable-gpios = <&gpio2 RK_PB5 GPIO_ACTIVE_HIGH>;
> +		host-wake-gpios = <&gpio2 RK_PC1 GPIO_ACTIVE_HIGH>;
> +		pinctrl-0 = <&bt_reset_gpio>, <&bt_wake_gpio>, <&bt_wake_host_irq>;
> +		pinctrl-names = "default";
> +	};
> +};

[Severity: High]
This is the UART bus configuration mentioned earlier where the bluetooth
controller is defined. Could you clarify if the RTL8852BE mentioned in the
commit message is accurate, or if this rtl8822cs-bt compatible string is
the correct hardware description?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602212512.418166-1-macroalpha82@gmail.com?part=6