Re: [PATCH v2 3/3] arm64: dts: rockchip: Add devicetree for the Graperain G3568 v2

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `press-threshold-microvolt` values for the ADC keys are incorrectly set to the midpoints instead of the nominal voltages, breaking the closest-match logic.
- [Medium] The `uart8` pinmux configuration for the AP6356S Bluetooth module omits the RTS pin, breaking hardware flow control.
--

commit fa96250010d1434d5bd9c7fed26ebc77f8c2f9f1
Author: Coia Prant <coiaprant@gmail.com>

arm64: dts: rockchip: Add devicetree for the Graperain G3568 v2

This patch adds support for the Graperain G3568 v2, an RK3568-based
development board. It defines the device tree including CPU, memory, network,
wireless, storage, display, and standard IO interfaces.

Link: https://image.chukouplus.com/upload/C_153/product_file/20211022/6daddec9e400458816dd4c57ba807fc3.pdf
Link: https://blog.gov.cooking/archives/research-graperain-g3568-v2-and-flash.html

> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-graperain-g3568-v2.dts b/arch/arm64/boot/dts/rockchip/rk3568-graperain-g3568-v2.dts
> new file mode 100644
> index 0000000000000..221992d6c6003
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-graperain-g3568-v2.dts
> @@ -0,0 +1,894 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)

[ ... ]

> +	adc-keys {
> +		compatible = "adc-keys";
> +		io-channels = <&saradc 0>;
> +		io-channel-names = "buttons";
> +		keyup-threshold-microvolt = <1800000>;
> +		poll-interval = <100>;
> +
> +		button-esc {
> +			label = "esc";
> +			linux,code = <KEY_ESC>;
> +			press-threshold-microvolt = <1250000>;
> +		};
> +
> +		button-menu {
> +			label = "menu";
> +			linux,code = <KEY_MENU>;
> +			press-threshold-microvolt = <1600000>;
> +		};

[Severity: Medium]
Are these threshold values correct? The commit message declares nominal
voltages (e.g., 1.1V for ESC and 1.4V for MENU), but the values here seem to
be configured to the midpoints.

The adc-keys driver dynamically matches the read voltage to the closest
defined threshold. By configuring midpoints instead of actual nominal
voltages, the distance calculations can become skewed.

For example, when the MENU key (nominal 1.4V) is pressed, its distance to
the configured MENU threshold (1.6V) is 200mV, but its distance to the ESC
threshold (1.25V) is only 150mV.

Won't the driver incorrectly register the MENU key as ESC?

> +		button-vol-down {
> +			label = "volume down";
> +			linux,code = <KEY_VOLUMEDOWN>;
> +			press-threshold-microvolt = <600000>;
> +		};
> +
> +		button-vol-up {
> +			label = "volume up";
> +			linux,code = <KEY_VOLUMEUP>;
> +			press-threshold-microvolt = <100000>;
> +		};
> +	};

[ ... ]

> +/* used for WiFi/BT AP6356S */
> +&uart8 {
> +	pinctrl-0 = <&uart8m0_xfer &uart8m0_ctsn>;
> +	status = "okay";
> +};

[Severity: Medium]
Does this UART configuration omit the RTS pin (&uart8m0_rtsn)?

High-speed Bluetooth modules like the AP6356S require hardware flow control to
prevent packet loss. Without the host RTS pin explicitly muxed, the Bluetooth
module may receive an invalid or floating RTS signal.

Could this prevent it from successfully transmitting data to the host or
cause buffer overflows when hardware flow control is utilized?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630153810.3574714-2-coiaprant@gmail.com?part=3