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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Bluetooth `device-wake-gpios` and `enable-gpios` properties are swapped relative to their physical pin assignments and pinctrl definitions.
- [Medium] Contradiction between the commit message's claimed Wi-Fi/BT module and the device tree configuration.
- [Medium] Dangerously high battery charge current limit.
- [Medium] Unrealistic ADC keys press threshold (likely raw ADC value instead of microvolts).
- [Medium] Battery capacity mismatch between commit message and DT.
- [Medium] The SPI joysticks cannot be accessed via userspace because there is no child device node on the SPI bus.
- [Medium] The eMMC (`&sdhci`) node is missing explicit `vmmc-supply` and `vqmmc-supply` properties.
--

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

This patch adds the device tree for the Anbernic RG Vita-Pro handheld,
based on the Rockchip RK3576 SoC. It includes configurations for the
display, buttons, audio, battery, and various peripherals.

> 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..7f703b5c8a298
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts

[ ... ]

> +	adc_keys_home: adc-keys-home {
> +		compatible = "adc-keys";
> +		io-channel-names = "buttons";
> +		io-channels = <&saradc 1>;
> +		keyup-threshold-microvolt = <1800000>;
> +		poll-interval = <60>;
> +
> +		button-home {
> +			label = "HOME";
> +			linux,code = <KEY_HOME>;
> +			press-threshold-microvolt = <1750>;

[Severity: Medium]
Is this threshold correct? 1750 microvolts (1.75mV) is extremely low and
would be highly sensitive to minor resistance or ground bounce. Was this
intended to be the raw 12-bit ADC value (where 1750 represents ~769mV)
rather than microvolts?

> +		};
> +	};
> +
> +	adc_keys_play: adc-keys-play {
> +		compatible = "adc-keys";
> +		io-channel-names = "buttons";
> +		io-channels = <&saradc 3>;
> +		keyup-threshold-microvolt = <1300000>;
> +		poll-interval = <60>;
> +
> +		button-play {
> +			label = "PLAY";
> +			linux,code = <KEY_PLAYPAUSE>;
> +			press-threshold-microvolt = <1750>;

[Severity: Medium]
Does this also use a raw ADC value instead of microvolts?

> +		};
> +	};
> +
> +	/*
> +	 * Values from BSP, except for internal resistance which was set
> +	 * to 80 micro ohms which is more likely 80000 micro ohms.
> +	 */
> +	battery: battery {
> +		compatible = "simple-battery";
> +		charge-full-design-microamp-hours = <8000000>;

[Severity: Medium]
The capacity here is configured to 8000mAh, which contradicts the 5000mAh
mentioned in the commit message. An incorrect capacity can cause battery
fuel gauge drivers to miscalculate the state of charge. Which value is
accurate?

> +		charge-term-current-microamp = <300000>;
> +		constant-charge-current-max-microamp = <10000000>;

[Severity: Medium]
Is the constant charge current intended to be 10A? While the charger IC
might have a hardware limit, 10 Amps is a 2C charge rate for a 5000mAh
battery, which seems dangerously high. Was this intended to be 1A or 2A?

[ ... ]

> +&sdhci {
> +	bus-width = <8>;
> +	full-pwr-cycle-in-suspend;
> +	no-sd;
> +	no-sdio;
> +	non-removable;
> +	mmc-hs400-1_8v;
> +	mmc-hs400-enhanced-strobe;
> +	status = "okay";
> +};

[Severity: Medium]
Does the eMMC node need explicit vmmc-supply and vqmmc-supply properties?
Omitting these forces the MMC subsystem to fall back to dummy regulators,
which triggers kernel warnings and can prevent the driver from executing
necessary power cycles during error recovery.

[ ... ]

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

[Severity: Medium]
How are the analog joysticks accessed from userspace? This node configures
the SPI controller but does not instantiate any child devices (such as a
spidev node). Without a child node, the kernel will not register a device,
and no userspace interface will be created.

[ ... ]

> +	bluetooth {
> +		compatible = "realtek,rtl8822cs-bt";

[Severity: Medium]
This node defines an RTL8822CS module over UART, but the commit message
indicates the hardware uses an RTL8852BE combo module (which typically uses
PCIe for Wi-Fi and USB for Bluetooth). Could you confirm which module is
correct?

> +		device-wake-gpios = <&gpio2 RK_PB6 GPIO_ACTIVE_HIGH>;
> +		enable-gpios = <&gpio2 RK_PB5 GPIO_ACTIVE_HIGH>;

[Severity: High]
Are these GPIOs mapped correctly? The pinctrl defined earlier labels
RK_PB6 as bt_reset_gpio and RK_PB5 as bt_wake_gpio. Driving the wake pin
with the enable/reset logic and vice versa will leave the module in reset.

> +		host-wake-gpios = <&gpio2 RK_PC1 GPIO_ACTIVE_HIGH>;
> +		pinctrl-0 = <&bt_reset_gpio>, <&bt_wake_gpio>, <&bt_wake_host_irq>;
> +		pinctrl-names = "default";
> +	};
> +};

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