On 14/04/2024 10:33, Ryan Walklin wrote:
> From: Ryan Walklin <ryan@xxxxxxxxxxxxx>
>
> The base model RG35XX (2024) is a handheld gaming device based on an Allwinner
> H700 chip.
>
> The H700 is a H616 variant (4x ARM Cortex-A53 cores @ 1.5Ghz with Mali G31 GPU)
> which exposes RGB LCD and NMI pins.
>
> Device features:
> - Allwinner H700 @ 1.5GHz
> - 1GB LPDDR4 DRAM
> - X-Powers AXP717 PMIC
> - 3.5" 640x480 RGB LCD
> - Two microSD slots
> - Mini-HDMI out
> - GPIO keypad
> - 3.5mm headphone jack
>
> Enabled in this DTS:
> - AXP717 PMIC with regulators (interrupt controller TBC/TBD)
> - Power LED (charge LED on device controlled directly by PMIC)
> - Serial UART (accessible from PIN headers on the board)
> - MMC slots
>
> Signed-off-by: Ryan Walklin <ryan@xxxxxxxxxxxxx>
> ---
> .../sun50i-h700-anbernic-rg35xx-2024.dts | 405 ++++++++++++++++++
> 1 file changed, 405 insertions(+)
> create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts b/arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
> new file mode 100644
> index 000000000000..b4140d450687
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
> @@ -0,0 +1,405 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/*
> + * Copyright (C) 2024 Andre Przywara <andre.przywara@xxxxxxx>.
> + * Copyright (C) 2024 Ryan Walklin <ryan@xxxxxxxxxxxxx>.
> + * Copyright (C) 2024 Chris Morgan <macroalpha82@xxxxxxxxx>.
> + */
> +
> +/dts-v1/;
> +
> +#include "sun50i-h616.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/linux-event-codes.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/leds/common.h>
> +
> +/ {
> + model = "Anbernic RG35XX 2024";
> + compatible = "anbernic,rg35xx-2024", "allwinner,sun50i-h700";
> +
> + aliases {
> + mmc0 = &mmc0;
> + mmc1 = &mmc2;
> + mmc2 = &mmc1;
> + serial0 = &uart0;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> +
> + led-0 {
> + function = LED_FUNCTION_POWER;
> + color = <LED_COLOR_ID_GREEN>;
> + gpios = <&pio 8 12 GPIO_ACTIVE_HIGH>; /* PI12 */
> + default-state = "on";
> + };
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> +
> + keyUp {
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
No uppercase letters are allowed as node names.
...
> +
> + keyVol+ {
Neither this
> + label = "Key Volume Up";
> + gpios = <&pio 4 1 GPIO_ACTIVE_LOW>; /* PE1 */
> + linux,input-type = <EV_KEY>;
> + linux,code = <KEY_VOLUMEUP>;
> + };
> +
> + keyVol- {
Although it is allowed but not consistent and readable.
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + label = "Key Volume Down";
> + gpios = <&pio 4 2 GPIO_ACTIVE_LOW>; /* PE2 */
> + linux,input-type = <EV_KEY>;
> + linux,code = <KEY_VOLUMEDOWN>;
> + };
> + };
> +
> + reg_vcc5v: vcc5v {
Use some common reasonable prefix, e.g. regulator
> + compatible = "regulator-fixed";
> + regulator-name = "vcc-5v";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + };
> +
> + vcc_5v0_usb: vcc-5v0-usb { /* needs gpios */
> + compatible = "regulator-fixed";
> + regulator-name = "vcc_5v0_usb";
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + reg_vcc3v3: vcc3v3 {
> + gpio = <&pio 4 4 GPIO_ACTIVE_HIGH>;
> + compatible = "regulator-fixed";
> + regulator-name = "vcc-3v3";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> +
> + };
> +
> + reg_vcc1v8: vcc1v8 {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc-1v8";
> + regulator-always-on;
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + };
> +};
> +
> +&cpu0 {
> + cpu-supply = <®_dcdc1>;
> +};
> +
> +&mmc0 {
> + vmmc-supply = <®_vcc3v3>;
> + disable-wp;
> + cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
> + bus-width = <4>;
> + status = "okay";
> +};
> +
> +&mmc2 {
> + vmmc-supply = <®_vcc3v3>;
> + vqmmc-supply = <®_aldo1>;
> + cd-gpios = <&pio 4 22 GPIO_ACTIVE_LOW>; /* PE 22 */
> + //cd-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* PE16 */
Please do not add dead code without explanation.
> + bus-width = <4>;
> + status = "okay";
> +};
> +
> +&ohci0 {
> + status = "okay";
> +};
> +
> +&ehci1 {
> + status = "okay";
> +};
> +
> +&i2c3 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c3_ph_pins>;
> + status = "okay";
> +};
> +
> +&r_rsb {
> + status = "okay";
> +
> + axp717: pmic@3a3 {
> + compatible = "x-powers,axp717";
> + //interrupt-controller;
> + //#interrupt-cells = <1>;
> + reg = <0x3a3>;
> + //interrupt-parent = <&r_intc>; /* test */
> + //interrupts = <0 IRQ_TYPE_LEVEL_LOW>; /* test */
Keep test and dead code outside upstream submission.
Best regards,
Krzysztof
