Skip to contentSkip to conversation

arm64: dts: allwinner: Add Anbernic RG35XX (Plus/H/2024) support

PATCH Superseded Ryan Walklin 5 participants 23 messages Apr 20, 2024 – Apr 22, 2024

Subsystems (2)

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v2 cover arm64: dts: allwinner: Add Anbernic RG35XX (Plus/H/2024) support

Revised patchset based on review with many changes including improved regulators and formatting. Changelog inline, original cover below.

--

The Anbernic RG35XX is a family of handheld gaming devices. There are 4 variants, of which 3 using the Allwinner H700 chip are covered by this patchset. The fourth (released first and named simply RG35XX) uses an Actions Semiconductor ATM7039s which is a 32-bit Cortex-A9 chip with no mainline support and is not covered.

Common features (RG35XX-2024):
- Allwinner H700 @ 1.5GHz (H616 variant exposing RGB LCD pins, with 4x Cortex-A53 Cores and a Mali G31 GPU)
- 1 GB LPDDR4 DRAM
- AXP717 PMIC (patches accepted in mfd-next -
https://kernel.googlesource.com/pub/scm/linux/kernel/git/lee/mfd/+/d2ac3df75c3a995064cfac0171e082a30d8c4c66)
- 3.5" 640x480 RGB LCD
- Mini-HDMI, 3.5mm audio jack, mono speaker, two microSD slots and USB-C (USB 2.0) for power.

RG35XX-Plus adds:
- RTL8821CS SDIO Wifi/BT chip

RG35XX-H (Horizontal form-factor) adds:
- RTL8821CS SDIO Wifi/BT chip
- Two analog thumbsticks
- Second USB-C port
- Stereo speaker

Patch 1 adds the DT bindings for the board names, Patch 2 adds the -2024 device as a common base, Patch 3 adds Wifi/BT support for the -Plus (and -H), and Patch 3 adds the second USB and thumbsticks for the -H. The -H is a strict superset of the -Plus, which is in turn a strict superset of the -2024, so this translates quite neatly. Alternatively a single DTS for the three devices could be considered.

LCD, HDMI, audio and GPU support are not yet ready and relying on out-of-tree patches currently, so will be added once these drivers are mainlined.

Ryan

Ryan Walklin (5):
dt-bindings: arm: sunxi: document Anbernic RG35XX handheld gaming
device variants
arm64: dts: allwinner: h700: Add RG35XX 2024 DTS
arm64: dts: allwinner: h700: Add RG35XX-Plus DTS
arm64: dts: allwinner: h700: Add RG35XX-H DTS
arm64: dts: allwinner: h700: Add RG35XX DTBs to Makefile

 .../devicetree/bindings/arm/sunxi.yaml        |  15 +
 arch/arm64/boot/dts/allwinner/Makefile        |   3 +
 .../sun50i-h700-anbernic-rg35xx-2024.dts      | 375 ++++++++++++++++++
 .../sun50i-h700-anbernic-rg35xx-h.dts         | 126 ++++++
 .../sun50i-h700-anbernic-rg35xx-plus.dts      |  53 +++
 5 files changed, 572 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-plus.dts
Signature · 1 line
2.44.0
#1 lore raw

PATCH v2 1/5 dt-bindings: arm: sunxi: document Anbernic RG35XX handheld gaming device variants

RG35XX 2024: Base version with Allwinner H700
RG35XX Plus: Adds Wifi/BT
RG35XX H: Adds second USB port and analog sticks to -Plus in horizontal form factor

---
 Documentation/devicetree/bindings/arm/sunxi.yaml | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
Documentation/devicetree/bindings/arm/sunxi.yaml +15 -0
index 09d835db6db5..fc10f54561c9 100644
@@ -56,6 +56,21 @@ properties:
           - const: anbernic,rg-nano
           - const: allwinner,sun8i-v3s
 
+      - description: Anbernic RG35XX (2024)
+      - items:
+          - const: anbernic,rg35xx-2024
+          - const: allwinner,sun50i-h700
+
+      - description: Anbernic RG35XX Plus
+      - items:
+          - const: anbernic,rg35xx-plus
+          - const: allwinner,sun50i-h700
+
+      - description: Anbernic RG35XX H
+      - items:
+          - const: anbernic,rg35xx-h
+          - const: allwinner,sun50i-h700
+
       - description: Amarula A64 Relic
         items:
           - const: amarula,a64-relic
Signature · 1 line
2.44.0
#2 lore raw

PATCH v2 2/5 arm64: dts: allwinner: h700: Add RG35XX 2024 DTS

In reply to Ryan Walklin

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

Changelog v1..v2:
- Update copyright
- Spaces -> Tabs
- Add cpufreq support [1]
- Remove MMC aliases
- Fix GPIO button and regulator node names
- Note unused AXP717 regulators
- Update regulator for SD slots
- Remove unused I2C3 device
- Update NMI interrupt controller for AXP717 [2]
- Add chassis-type
- Address USB EHCI/OHCI0 correctly and add usb vbus supply
- Add PIO vcc-pg-supply
- Correct boost regulator voltage and name

[1]: https://lore.kernel.org/linux-sunxi/20240419071450.7aiheeoyq35yjoo7@vireshk-i7/T/#t
[2]: https://lore.kernel.org/linux-sunxi/[email protected]/T/#t

---
 .../sun50i-h700-anbernic-rg35xx-2024.dts      | 375 ++++++++++++++++++
 1 file changed, 375 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +375 -0
new file mode 100644
index 000000000000..d8081273677d
@@ -0,0 +1,375 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) 2024 Ryan Walklin <[email protected]>.
+ */
+
+/dts-v1/;
+
+#include "sun50i-h616.dtsi"
+#include "sun50i-h616-cpu-opp.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";
+	chassis-type = "handset";
+	compatible = "anbernic,rg35xx-2024", "allwinner,sun50i-h700";
+
+	aliases {
+		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";
+
+	   button-up {
+		   label = "D-Pad Up";
+		   gpios = <&pio 0 6 GPIO_ACTIVE_LOW>; /* PA6 */
+		   linux,input-type = <EV_KEY>;
+		   linux,code = <BTN_DPAD_UP>;
+		};
+
+		button-down {
+			label = "D-Pad Down";
+			gpios = <&pio 4 0 GPIO_ACTIVE_LOW>; /* PE0 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_DOWN>;
+		};
+
+		button-left {
+			label = "D-Pad left";
+			gpios = <&pio 0 8 GPIO_ACTIVE_LOW>; /* PA8 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_LEFT>;
+		};
+
+		button-right {
+			label = "D-Pad Right";
+			gpios = <&pio 0 9 GPIO_ACTIVE_LOW>; /* PA9 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_RIGHT>;
+		};
+
+		button-a {
+			label = "Action-Pad A";
+			gpios = <&pio 0 0 GPIO_ACTIVE_LOW>; /* PA0 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_EAST>;
+		};
+
+		button-b {
+			label = "Action-Pad B";
+			gpios = <&pio 0 1 GPIO_ACTIVE_LOW>; /* PA1 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_SOUTH>;
+		};
+
+		button-x {
+			label = "Action-Pad X";
+			gpios = <&pio 0 3 GPIO_ACTIVE_LOW>; /* PA3 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_NORTH>;
+		};
+
+		button-y {
+			label = "Action Pad Y";
+			gpios = <&pio 0 2 GPIO_ACTIVE_LOW>; /* PA2 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_WEST>;
+		};
+
+		button-start {
+			label = "Key Start";
+			gpios = <&pio 0 4 GPIO_ACTIVE_LOW>; /* PA4 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_START>;
+		};
+
+		button-select {
+			label = "Key Select";
+			gpios = <&pio 0 5 GPIO_ACTIVE_LOW>; /* PA5 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_SELECT>;
+		};
+
+		button-l1 {
+			label = "Key L1";
+			gpios = <&pio 0 10 GPIO_ACTIVE_LOW>; /* PA10 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TL>;
+		};
+
+		button-l2 {
+			label = "Key L2";
+			gpios = <&pio 0 11 GPIO_ACTIVE_LOW>; /* PA11 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TL2>;
+		};
+
+		button-r1 {
+			label = "Key R1";
+			gpios = <&pio 0 12 GPIO_ACTIVE_LOW>; /* PA12 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TR>;
+		};
+
+		button-r2 {
+			label = "Key R2";
+			gpios = <&pio 0 7 GPIO_ACTIVE_LOW>; /* PA7 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TR2>;
+		};
+
+		button-menu {
+			label = "Key Menu";
+			gpios = <&pio 4 3 GPIO_ACTIVE_LOW>; /* PE3 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_MODE>;
+		};
+
+		button-vol-up {
+			label = "Key Volume Up";
+			gpios = <&pio 4 1 GPIO_ACTIVE_LOW>; /* PE1 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <KEY_VOLUMEUP>;
+		};
+
+		button-vol-down {
+			label = "Key Volume Down";
+			gpios = <&pio 4 2 GPIO_ACTIVE_LOW>; /* PE2 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <KEY_VOLUMEDOWN>;
+		};
+	};
+
+	reg_vcc_sd2: regulator-vcc3v3 {
+		compatible = "regulator-fixed";
+		gpio = <&pio 4 4 GPIO_ACTIVE_HIGH>; /* PE4 */
+		regulator-name = "vcc_3v3_sd2";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+	};
+
+	reg_vcc_usb: regulator-vcc-5v0-usb {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&pio 8 7 GPIO_ACTIVE_HIGH>; /* PI7 */
+		regulator-name = "vcc_5v0_usb";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vcc_3v3_usb>;
+	};
+
+	vcc_3v3_usb: vcc-3v3-usb {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&pio 4 5 GPIO_ACTIVE_HIGH>; /* PE5 */
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-name = "vcc_3v3_usb";
+		regulator-max-microvolt = <3300000>;
+		regulator-min-microvolt = <3300000>;
+	};
+
+	reg_vcc5v: regulator-vcc5v { /* USB-C power input */
+		compatible = "regulator-fixed";
+		regulator-name = "vcc-5v";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+	};
+
+	reg_pll_dcx0: regulator-pll-dcx0 {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-name = "vcc-pll";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+	};
+};
+
+&cpu0 {
+	cpu-supply = <&reg_dcdc1>;
+};
+
+&mmc0 {
+	vmmc-supply = <&reg_vcc_sd2>;
+	disable-wp;
+	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>;  /* PF6 */
+	bus-width = <4>;
+	status = "okay";
+};
+
+&mmc2 {
+	vmmc-supply = <&reg_vcc_sd2>;
+	vqmmc-supply = <&reg_aldo1>;
+	cd-gpios = <&pio 4 22 GPIO_ACTIVE_LOW>; /* PE22 */
+	bus-width = <4>;
+	status = "okay";
+};
+
+&ohci0 {
+	status = "okay";
+};
+
+&ehci0 {
+	status = "okay";
+};
+
+&r_rsb {
+   status = "okay";
+
+   axp717: pmic@3a3 {
+		compatible = "x-powers,axp717";
+		reg = <0x3a3>;
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		interrupt-parent = <&nmi_intc>;
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+		x-powers,drive-vbus-en;
+
+		vin1-supply = <&reg_vcc5v>;
+		vin2-supply = <&reg_vcc5v>;
+		vin3-supply = <&reg_vcc5v>;
+		vin4-supply = <&reg_vcc5v>;
+
+		regulators {
+			reg_dcdc1: dcdc1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <810000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-name = "vdd-cpu";
+			};
+
+			reg_dcdc2: dcdc2 {
+				regulator-always-on;
+				regulator-min-microvolt = <940000>;
+				regulator-max-microvolt = <940000>;
+				regulator-name = "vdd-sys";
+			};
+
+			reg_dcdc3: dcdc3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1100000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-name = "vdd-dram";
+			};
+
+			reg_aldo1: aldo1 {
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcc-sd2";
+			};
+
+			reg_aldo2: aldo2 {
+				/* unused */
+			};
+
+			reg_aldo3: aldo3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;
+				regulator-name = "axp717-aldo3";
+			};
+
+			reg_aldo4: aldo4 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;
+				regulator-name = "axp717-aldo4";
+			};
+
+			reg_bldo1: bldo1 {
+				/* unused */
+			};
+
+			reg_bldo2: bldo2 {
+				regulator-always-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-name = "vcc-pll";
+			};
+
+			reg_bldo3: bldo3 {
+				/* unused */
+			};
+
+			reg_bldo4: bldo4 {
+				/* unused */
+			};
+
+			reg_cldo1: cldo1 {
+				/* unused */
+			};
+
+			reg_cldo2: cldo2 {
+				/* unused */
+			};
+
+			reg_cldo3: cldo3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;
+				regulator-name = "axp717-cldo3";
+			};
+
+			reg_cldo4: cldo4 {
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcc-wifi";
+			};
+
+			reg_boost: boost {
+				regulator-min-microvolt = <5126000>;
+				regulator-max-microvolt = <5126000>;
+				regulator-name = "boost";
+			};
+
+			reg_cpusldo: cpusldo {
+				/* unused */
+			};
+		};
+	};
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_ph_pins>;
+	status = "okay";
+};
+
+&pio {
+	vcc-pg-supply = <&reg_pll_dcx0>;
+};
+
+/* the AXP717 has USB type-C role switch functionality, to be implemented */
+&usbotg {
+	dr_mode = "host";   /* USB type-C receptable */
+	status = "okay";
+};
+
+&usbphy {
+	usb0_vbus-supply = <&reg_vcc_usb>;
+	status = "okay";
+};
Signature · 1 line
2.44.0
#3 lore raw

Re: [PATCH v2 2/5] arm64: dts: allwinner: h700: Add RG35XX 2024 DTS

Quoted 15 lines · Ryan Walklin

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.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +11 -0
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		led-0 {
+			function = LED_FUNCTION_POWER;
+			color = <LED_COLOR_ID_GREEN>;

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 43

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+			gpios = <&pio 8 12 GPIO_ACTIVE_HIGH>; /* PI12 */
+			default-state = "on";
+		};
+	};
+
+	gpio-keys {
+	   compatible = "gpio-keys";
+
+	   button-up {
+		   label = "D-Pad Up";

Please fix your indentation to match kernel/DTS coding style.

Best regards,
Krzysztof

#4 lore raw

Re: [PATCH v2 2/5] arm64: dts: allwinner: h700: Add RG35XX 2024 DTS

Quoted 6 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 43

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +2 -0
+	   button-up {
+		   label = "D-Pad Up";

Please fix your indentation to match kernel/DTS coding style.

Good spot, thanks!

Quoted 3 lines

Best regards,
Krzysztof

#5 lore raw

Re: [PATCH v2 2/5] arm64: dts: allwinner: h700: Add RG35XX 2024 DTS

In reply to Ryan Walklin

On Sat, 20 Apr 2024 22:43:56 +1200
Ryan Walklin <[email protected]> wrote:

Hi Ryan,

many thanks for the respin and the changes! We are getting there ...

Quoted 35 lines

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

Changelog v1..v2:
- Update copyright
- Spaces -> Tabs
- Add cpufreq support [1]
- Remove MMC aliases
- Fix GPIO button and regulator node names
- Note unused AXP717 regulators
- Update regulator for SD slots
- Remove unused I2C3 device
- Update NMI interrupt controller for AXP717 [2]
- Add chassis-type
- Address USB EHCI/OHCI0 correctly and add usb vbus supply
- Add PIO vcc-pg-supply
- Correct boost regulator voltage and name

Quoted 4 lines

[1]: https://lore.kernel.org/linux-sunxi/20240419071450.7aiheeoyq35yjoo7@vireshk-i7/T/#t
[2]: https://lore.kernel.org/linux-sunxi/[email protected]/T/#t

As those are dependencies, but WIP, this gets a bit tricky:
- cpufreq is pretty far, but comes through a different tree. I suggest you drop the cpu-opp.dtsi include, to not complicate things. We can add this later as a fix, once this OPP file has reached the master tree.
- The NMI binding and DT node seem important, but have not been merged yet. I suggest to mention them as a requirement. The patches (binding plus H616 .dtsi change) should go through the sunxi tree as well, so the maintainers can order this appropriately when merging.
- The GPADC (in the later patch) is similar, but it is not as critical as the NMI. Not sure how the maintainers want to handle this, but we might add those bits as an (optional) patch on top, so this can be handled more independently from the GPADC series.

Quoted 13 lines
---
 .../sun50i-h700-anbernic-rg35xx-2024.dts      | 375 ++++++++++++++++++
 1 file changed, 375 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +0 -0
new file mode 100644
index 000000000000..d8081273677d

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 9

Quoted 9 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +9 -0
@@ -0,0 +1,375 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) 2024 Ryan Walklin <[email protected]>.
+ */
+
+/dts-v1/;
+
+#include "sun50i-h616.dtsi"
+#include "sun50i-h616-cpu-opp.dtsi"

As mentioned, please drop this line for now.

Quoted 23 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +23 -0
+#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";
+	chassis-type = "handset";
+	compatible = "anbernic,rg35xx-2024", "allwinner,sun50i-h700";
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		led-0 {
+			function = LED_FUNCTION_POWER;

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 42

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+			color = <LED_COLOR_ID_GREEN>;
+			gpios = <&pio 8 12 GPIO_ACTIVE_HIGH>; /* PI12 */
+			default-state = "on";
+		};
+	};
+
+	gpio-keys {
+	   compatible = "gpio-keys";
+
+	   button-up {

indentation

Quoted 125 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +125 -0
+		   label = "D-Pad Up";
+		   gpios = <&pio 0 6 GPIO_ACTIVE_LOW>; /* PA6 */
+		   linux,input-type = <EV_KEY>;
+		   linux,code = <BTN_DPAD_UP>;
+		};
+
+		button-down {
+			label = "D-Pad Down";
+			gpios = <&pio 4 0 GPIO_ACTIVE_LOW>; /* PE0 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_DOWN>;
+		};
+
+		button-left {
+			label = "D-Pad left";
+			gpios = <&pio 0 8 GPIO_ACTIVE_LOW>; /* PA8 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_LEFT>;
+		};
+
+		button-right {
+			label = "D-Pad Right";
+			gpios = <&pio 0 9 GPIO_ACTIVE_LOW>; /* PA9 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_RIGHT>;
+		};
+
+		button-a {
+			label = "Action-Pad A";
+			gpios = <&pio 0 0 GPIO_ACTIVE_LOW>; /* PA0 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_EAST>;
+		};
+
+		button-b {
+			label = "Action-Pad B";
+			gpios = <&pio 0 1 GPIO_ACTIVE_LOW>; /* PA1 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_SOUTH>;
+		};
+
+		button-x {
+			label = "Action-Pad X";
+			gpios = <&pio 0 3 GPIO_ACTIVE_LOW>; /* PA3 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_NORTH>;
+		};
+
+		button-y {
+			label = "Action Pad Y";
+			gpios = <&pio 0 2 GPIO_ACTIVE_LOW>; /* PA2 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_WEST>;
+		};
+
+		button-start {
+			label = "Key Start";
+			gpios = <&pio 0 4 GPIO_ACTIVE_LOW>; /* PA4 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_START>;
+		};
+
+		button-select {
+			label = "Key Select";
+			gpios = <&pio 0 5 GPIO_ACTIVE_LOW>; /* PA5 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_SELECT>;
+		};
+
+		button-l1 {
+			label = "Key L1";
+			gpios = <&pio 0 10 GPIO_ACTIVE_LOW>; /* PA10 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TL>;
+		};
+
+		button-l2 {
+			label = "Key L2";
+			gpios = <&pio 0 11 GPIO_ACTIVE_LOW>; /* PA11 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TL2>;
+		};
+
+		button-r1 {
+			label = "Key R1";
+			gpios = <&pio 0 12 GPIO_ACTIVE_LOW>; /* PA12 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TR>;
+		};
+
+		button-r2 {
+			label = "Key R2";
+			gpios = <&pio 0 7 GPIO_ACTIVE_LOW>; /* PA7 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TR2>;
+		};
+
+		button-menu {
+			label = "Key Menu";
+			gpios = <&pio 4 3 GPIO_ACTIVE_LOW>; /* PE3 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_MODE>;
+		};
+
+		button-vol-up {
+			label = "Key Volume Up";
+			gpios = <&pio 4 1 GPIO_ACTIVE_LOW>; /* PE1 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <KEY_VOLUMEUP>;
+		};
+
+		button-vol-down {
+			label = "Key Volume Down";
+			gpios = <&pio 4 2 GPIO_ACTIVE_LOW>; /* PE2 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <KEY_VOLUMEDOWN>;
+		};
+	};
+
+	reg_vcc_sd2: regulator-vcc3v3 {
+		compatible = "regulator-fixed";
+		gpio = <&pio 4 4 GPIO_ACTIVE_HIGH>; /* PE4 */
+		regulator-name = "vcc_3v3_sd2";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 177

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+	};
+
+	reg_vcc_usb: regulator-vcc-5v0-usb {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&pio 8 7 GPIO_ACTIVE_HIGH>; /* PI7 */
+		regulator-name = "vcc_5v0_usb";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vcc_3v3_usb>;

This looks wrong, see below.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 185

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+	};
+
+	vcc_3v3_usb: vcc-3v3-usb {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&pio 4 5 GPIO_ACTIVE_HIGH>; /* PE5 */
+		regulator-always-on;
+		regulator-boot-on;

There seems to be something off with this one. First, it seems odd that an always-on regulator is GPIO controlled, as this surely means it's not enabled at reset time (because the GPIO is initially HighZ and thus not enabled). So who turns this on? Most likely it's the kernel? What happens if we turn this off (or leave it off)?

Secondly, why is this 3.3V (both by name and voltage), but then supplies the 5.0V USB VBUS?
And given that this is chained with reg_vcc_usb above, are those really two regulators, controlled by two GPIOs?

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 198

Quoted 13 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +13 -0
+		regulator-name = "vcc_3v3_usb";
+		regulator-max-microvolt = <3300000>;
+		regulator-min-microvolt = <3300000>;
+	};
+
+	reg_vcc5v: regulator-vcc5v { /* USB-C power input */
+		compatible = "regulator-fixed";
+		regulator-name = "vcc-5v";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+	};
+
+	reg_pll_dcx0: regulator-pll-dcx0 {

It's DCXO (letter "oh", not zero): digitally controlled/compensated crystal *o*scillator.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 204

Quoted 6 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +6 -0
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-name = "vcc-pll";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+	};

That one looks odd as well. While there might be a discrete regulator (what this node is describing), I doubt it, since the AXP717 has plenty of rails. In particular I am not sure if that fixed one would supply PortG (mainly WiFi), which seems unneeded for the boot process, and surely we want that switchable to save power if the WiFi is not needed.

You also have a VCC-PLL regulator below (BLDO2).
So please try to drop this regulator, I am pretty sure there is an AXP rail that powers the WiFi.

See below for more on this.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 212

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+};
+
+&cpu0 {
+	cpu-supply = <&reg_dcdc1>;
+};
+
+&mmc0 {
+	vmmc-supply = <&reg_vcc_sd2>;

I don't think this GPIO controlled regulator provides the supply voltage for the first SD card, since it would be disabled on reset, and the BROM couldn't boot from the SD card. So it must be some other 3.3V source, either a discrete regulator (fixed regulator), or some default-on 3.3V AXP rail.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 225

Quoted 13 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +13 -0
+	disable-wp;
+	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>;  /* PF6 */
+	bus-width = <4>;
+	status = "okay";
+};
+
+&mmc2 {
+	vmmc-supply = <&reg_vcc_sd2>;
+	vqmmc-supply = <&reg_aldo1>;
+	cd-gpios = <&pio 4 22 GPIO_ACTIVE_LOW>; /* PE22 */
+	bus-width = <4>;
+	status = "okay";
+};

This one seems more plausible. To confirm this, you could not use reg_vcc_sd2 anywhere, and use some other 3.3V supply for vmmc, which should break operation on the second SD card. Then just swap reg_vcc_sd2 back in, and if it starts working again, we have confirmation.

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+
+&ohci0 {
+	status = "okay";
+};
+
+&ehci0 {
+	status = "okay";
+};
+
+&r_rsb {

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 245

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+   status = "okay";
+
+   axp717: pmic@3a3 {
+		compatible = "x-powers,axp717";
+		reg = <0x3a3>;
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		interrupt-parent = <&nmi_intc>;
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+		x-powers,drive-vbus-en;

Remove this last line, the AXP717 binding does not support this, and the Linux driver doesn't implement this, as the AXP717 does not seem to have this functionality.

Quoted 33 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +33 -0
+
+		vin1-supply = <&reg_vcc5v>;
+		vin2-supply = <&reg_vcc5v>;
+		vin3-supply = <&reg_vcc5v>;
+		vin4-supply = <&reg_vcc5v>;
+
+		regulators {
+			reg_dcdc1: dcdc1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <810000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-name = "vdd-cpu";
+			};
+
+			reg_dcdc2: dcdc2 {
+				regulator-always-on;
+				regulator-min-microvolt = <940000>;
+				regulator-max-microvolt = <940000>;
+				regulator-name = "vdd-sys";
+			};
+
+			reg_dcdc3: dcdc3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1100000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-name = "vdd-dram";
+			};
+
+			reg_aldo1: aldo1 {
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 288

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+				regulator-name = "vcc-sd2";
+			};
+
+			reg_aldo2: aldo2 {
+				/* unused */
+			};
+
+			reg_aldo3: aldo3 {
+				regulator-always-on;
+				regulator-boot-on;

Please remove this last line, it doesn't make sense in this context. Cf. Documentation//devicetree/bindings/regulator/regulator.yaml. I think the same applies to the other uses throughout this file.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 290

Quoted 2 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +2 -0
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;

This is not right. What is the voltage of this rail? The kernel should tell you what was set in the register, through regulator_summary, or you check what's the voltage on a BSP system.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 291

Quoted 1 line
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +1 -0
+				regulator-name = "axp717-aldo3";

If the system dies when you remove always-on, you might have found some essential supply. Candidates for consumers are listed in the H616 or T5 series datasheet. Matching the voltage might give you an idea. Then use this consumer as the name.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 294

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+			};
+
+			reg_aldo4: aldo4 {

Same for this one: Please remove regulator-boot-on, fix the voltage, and provide some rationale why this needs to be on.

Quoted 7 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +7 -0
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;
+				regulator-name = "axp717-aldo4";
+			};
+

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 311

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+			reg_bldo1: bldo1 {
+				/* unused */
+			};
+
+			reg_bldo2: bldo2 {
+				regulator-always-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-name = "vcc-pll";
+			};

This one is a good example: fixed voltage, no boot-on, and regulator name provides rationale why it must be always-on. The others should look similar.

Quoted 13 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +13 -0
+
+			reg_bldo3: bldo3 {
+				/* unused */
+			};
+
+			reg_bldo4: bldo4 {
+				/* unused */
+			};
+
+			reg_cldo1: cldo1 {
+				/* unused */
+			};
+

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 334

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+			reg_cldo2: cldo2 {
+				/* unused */
+			};
+
+			reg_cldo3: cldo3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;
+				regulator-name = "axp717-cldo3";

Same here as ALDO3/4: what voltage is it, and what does it probably supply?

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 345

Quoted 11 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +11 -0
+			};
+
+			reg_cldo4: cldo4 {
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcc-wifi";
+			};
+
+			reg_boost: boost {
+				regulator-min-microvolt = <5126000>;
+				regulator-max-microvolt = <5126000>;

It might be better to use a range, say 5.0V till 5.2V. The kernel will then just continue using the default, which seems to be this 5.126V (5440mV + 9 * 64mV).

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 350

Quoted 5 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+				regulator-name = "boost";
+			};
+
+			reg_cpusldo: cpusldo {
+				/* unused */

Is that so? I thought you mentioned on IRC this is required as well?

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 363

Quoted 13 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +13 -0
+			};
+		};
+	};
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_ph_pins>;
+	status = "okay";
+};
+
+&pio {
+	vcc-pg-supply = <&reg_pll_dcx0>;

As mentioned above, it seems unlikely to be a fixed regulator. If in doubt, leave them out, they are not essential for the operation.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 366

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+};
+
+/* the AXP717 has USB type-C role switch functionality, to be implemented */

Replace "to be implemented" with "not yet described by the binding". This is DT land, so we don't care about implementations or the Linux kernel, it's all about DTs and DT bindings.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 368

Quoted 2 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +2 -0
+&usbotg {
+	dr_mode = "host";   /* USB type-C receptable */

So does this really work? It seems wrong to make this unconditional, given this is the only way to charge the device. When power is supplied through the USB-C port, surely driving VBUS from the board sounds wrong. Unless you have a killer feature for a host port, I think without working role switching, "peripheral" would be the safer choice.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 373

Quoted 5 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+	status = "okay";
+};
+
+&usbphy {
+	usb0_vbus-supply = <&reg_vcc_usb>;

When you stick to "peripheral" above, you should drop this line for now, especially since this regulator chain looks quite suspicious still.

Cheers,
Andre

Quoted 2 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +2 -0
+	status = "okay";
+};
#6 lore raw

Re: [PATCH v2 2/5] arm64: dts: allwinner: h700: Add RG35XX 2024 DTS

Quoted 6 lines · Andre Przywara

On Sat, 20 Apr 2024 22:43:56 +1200
Ryan Walklin <[email protected]> wrote:

Hi Ryan,

many thanks for the respin and the changes! We are getting there ...

Thanks for the review!

Quoted 9 lines
Quoted 3 lines

[1]: https://lore.kernel.org/linux-sunxi/20240419071450.7aiheeoyq35yjoo7@vireshk-i7/T/#t
[2]: https://lore.kernel.org/linux-sunxi/[email protected]/T/#t

As those are dependencies, but WIP, this gets a bit tricky:
- cpufreq is pretty far, but comes through a different tree. I suggest you drop the cpu-opp.dtsi include, to not complicate things. We can add this later as a fix, once this OPP file has reached the master tree.

Done, thanks. Have also not increased the DCDC1 voltage to 1.16v for 1.5GHz as it's not yet working (and technically out of spec for the SoC) but will relook at the vendor BSP once this set is merged.

Quoted 2 lines

- The NMI binding and DT node seem important, but have not been merged yet. I suggest to mention them as a requirement.

Done, thanks.

Quoted 2 lines

- The GPADC (in the later patch) is similar, but it is not as critical as the NMI.

Will pull this out separately, as you say its only required for the joysticks on the -H.

Quoted 11 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 177

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+	reg_vcc_usb: regulator-vcc-5v0-usb {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&pio 8 7 GPIO_ACTIVE_HIGH>; /* PI7 */
+		regulator-name = "vcc_5v0_usb";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vcc_3v3_usb>;

This looks wrong, see below.

Quoted 14 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 185

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+	};
+
+	vcc_3v3_usb: vcc-3v3-usb {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&pio 4 5 GPIO_ACTIVE_HIGH>; /* PE5 */
+		regulator-always-on;
+		regulator-boot-on;

There seems to be something off with this one. First, it seems odd that an always-on regulator is GPIO controlled, as this surely means it's not enabled at reset time (because the GPIO is initially HighZ and thus not enabled). So who turns this on? Most likely it's the kernel? What happens if we turn this off (or leave it off)?

This makes more sense with that explanation, thanks. Taking a while to get my head round how the power distribution is done in the embedded space.

Quoted 6 lines

Secondly, why is this 3.3V (both by name and voltage), but then supplies the 5.0V USB VBUS?
And given that this is chained with reg_vcc_usb above, are those really two regulators, controlled by two GPIOs?

Quoted 11 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 196

Quoted 11 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +11 -0
+		regulator-name = "vcc_3v3_usb";
+		regulator-max-microvolt = <3300000>;
+		regulator-min-microvolt = <3300000>;
+	};
+
+	reg_vcc5v: regulator-vcc5v { /* USB-C power input */
+		compatible = "regulator-fixed";
+		regulator-name = "vcc-5v";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+	};

Will do some more work on this and try and figure it out. The USB and second SD are not working well currently, probably this is why.

Quoted 5 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+	reg_pll_dcx0: regulator-pll-dcx0 {

It's DCXO (letter "oh", not zero): digitally controlled/compensated crystal *o*scillator.

D'oh! Had to google PLL, should have googled this too.

Quoted 14 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 204

Quoted 7 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +6 -0
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-name = "vcc-pll";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+	};

That one looks odd as well. While there might be a discrete regulator (what this node is describing), I doubt it, since the AXP717 has plenty of rails. In particular I am not sure if that fixed one would supply PortG (mainly WiFi), which seems unneeded for the boot process, and surely we want that switchable to save power if the WiFi is not needed.

Quoted 3 lines

You also have a VCC-PLL regulator below (BLDO2).
So please try to drop this regulator, I am pretty sure there is an AXP rail that powers the WiFi.

Makes sense, will dig into the vendor BSP.

Quoted 4 lines

See below for more on this.

Quoted 14 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 225

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+
+&mmc2 {
+	vmmc-supply = <&reg_vcc_sd2>;
+	vqmmc-supply = <&reg_aldo1>;
+	cd-gpios = <&pio 4 22 GPIO_ACTIVE_LOW>; /* PE22 */
+	bus-width = <4>;
+	status = "okay";
+};

This one seems more plausible. To confirm this, you could not use reg_vcc_sd2 anywhere, and use some other 3.3V supply for vmmc, which should break operation on the second SD card. Then just swap reg_vcc_sd2 back in, and if it starts working again, we have confirmation.

In hindsight this is taken from the vendor BSP, so I think is correct but will do some testing.

Quoted 5 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 245

Quoted 1 line
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +1 -0
+		x-powers,drive-vbus-en;

Remove this last line, the AXP717 binding does not support this, and the Linux driver doesn't implement this, as the AXP717 does not seem to have this functionality.

Thanks, fixed.

Quoted 23 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 288

Quoted 4 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +4 -0
+
+			reg_aldo3: aldo3 {
+				regulator-always-on;
+				regulator-boot-on;

Please remove this last line, it doesn't make sense in this context. Cf. Documentation//devicetree/bindings/regulator/regulator.yaml. I think the same applies to the other uses throughout this file.

Review

Quoted 2 lines
(quoted diff) +2 -0
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;

This is not right. What is the voltage of this rail? The kernel should tell you what was set in the register, through regulator_summary, or you check what's the voltage on a BSP system.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 291

Quoted 1 line
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +1 -0
+				regulator-name = "axp717-aldo3";

If the system dies when you remove always-on, you might have found some essential supply. Candidates for consumers are listed in the H616 or T5 series datasheet. Matching the voltage might give you an idea. Then use this consumer as the name.

Quoted 6 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 294

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+			};
+
+			reg_aldo4: aldo4 {

Same for this one: Please remove regulator-boot-on, fix the voltage, and provide some rationale why this needs to be on.

Thanks, will get the correct voltages. It turns out ALDO3/4 are both needed, but CPUSLDO is not. Will try and figure out what they are supplying.

Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 345

Quoted 4 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +4 -0
+
+			reg_boost: boost {
+				regulator-min-microvolt = <5126000>;
+				regulator-max-microvolt = <5126000>;

It might be better to use a range, say 5.0V till 5.2V. The kernel will then just continue using the default, which seems to be this 5.126V (5440mV + 9 * 64mV).

Thanks, fixed.

Quoted 7 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 363

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+
+&pio {
+	vcc-pg-supply = <&reg_pll_dcx0>;

As mentioned above, it seems unlikely to be a fixed regulator. If in doubt, leave them out, they are not essential for the operation.

Will do for now, thanks.

Quoted 18 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 366

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+};
+
+/* the AXP717 has USB type-C role switch functionality, to be implemented */

Replace "to be implemented" with "not yet described by the binding". This is DT land, so we don't care about implementations or the Linux kernel, it's all about DTs and DT bindings.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 368

Quoted 2 lines

+&usbotg {

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +1 -0
+	dr_mode = "host";   /* USB type-C receptable */

So does this really work? It seems wrong to make this unconditional, given this is the only way to charge the device. When power is supplied through the USB-C port, surely driving VBUS from the board sounds wrong. Unless you have a killer feature for a host port, I think without working role switching, "peripheral" would be the safer choice.

Quoted 9 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 373

Quoted 5 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+	status = "okay";
+};
+
+&usbphy {
+	usb0_vbus-supply = <&reg_vcc_usb>;

When you stick to "peripheral" above, you should drop this line for now, especially since this regulator chain looks quite suspicious still.

Thanks, fixed.

Quoted 2 lines

Cheers,
Andre

Thanks again for the review! Will post up a v3 after some more thought about the regulators.

Ryan

#7 lore raw

Re: [PATCH v2 2/5] arm64: dts: allwinner: h700: Add RG35XX 2024 DTS

On Sun, 21 Apr 2024 14:28:22 +1200
"Ryan Walklin" <[email protected]> wrote:

Hi Ryan,

(please also see my reply to Chris' email, it was easier to reply there.)

Quoted 14 lines · Andre Przywara
Quoted 6 lines

On Sat, 20 Apr 2024 22:43:56 +1200
Ryan Walklin <[email protected]> wrote:

Hi Ryan,

many thanks for the respin and the changes! We are getting there ...

Thanks for the review!

Quoted 4 lines
Quoted 3 lines

[1]: https://lore.kernel.org/linux-sunxi/20240419071450.7aiheeoyq35yjoo7@vireshk-i7/T/#t
[2]: https://lore.kernel.org/linux-sunxi/[email protected]/T/#t

Quoted 7 lines
Quoted 5 lines

As those are dependencies, but WIP, this gets a bit tricky:
- cpufreq is pretty far, but comes through a different tree. I suggest you drop the cpu-opp.dtsi include, to not complicate things. We can add this later as a fix, once this OPP file has reached the master tree.

Done, thanks. Have also not increased the DCDC1 voltage to 1.16v for 1.5GHz as it's not yet working (and technically out of spec for the SoC) but will relook at the vendor BSP once this set is merged.

Yes, that sounds good to me, thanks!

Quoted 102 lines
Quoted 2 lines

- The NMI binding and DT node seem important, but have not been merged yet. I suggest to mention them as a requirement.

Done, thanks.

Quoted 2 lines

- The GPADC (in the later patch) is similar, but it is not as critical as the NMI.

Will pull this out separately, as you say its only required for the joysticks on the -H.

Quoted 11 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 177

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+	reg_vcc_usb: regulator-vcc-5v0-usb {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&pio 8 7 GPIO_ACTIVE_HIGH>; /* PI7 */
+		regulator-name = "vcc_5v0_usb";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vcc_3v3_usb>;  

This looks wrong, see below.

Quoted 14 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 185

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+	};
+
+	vcc_3v3_usb: vcc-3v3-usb {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&pio 4 5 GPIO_ACTIVE_HIGH>; /* PE5 */
+		regulator-always-on;
+		regulator-boot-on;  

There seems to be something off with this one. First, it seems odd that an always-on regulator is GPIO controlled, as this surely means it's not enabled at reset time (because the GPIO is initially HighZ and thus not enabled). So who turns this on? Most likely it's the kernel? What happens if we turn this off (or leave it off)?

This makes more sense with that explanation, thanks. Taking a while to get my head round how the power distribution is done in the embedded space.

Quoted 6 lines

Secondly, why is this 3.3V (both by name and voltage), but then supplies the 5.0V USB VBUS?
And given that this is chained with reg_vcc_usb above, are those really two regulators, controlled by two GPIOs?

Quoted 11 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 196

Quoted 11 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +11 -0
+		regulator-name = "vcc_3v3_usb";
+		regulator-max-microvolt = <3300000>;
+		regulator-min-microvolt = <3300000>;
+	};
+
+	reg_vcc5v: regulator-vcc5v { /* USB-C power input */
+		compatible = "regulator-fixed";
+		regulator-name = "vcc-5v";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+	};  

Will do some more work on this and try and figure it out. The USB and second SD are not working well currently, probably this is why.

Quoted 5 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+	reg_pll_dcx0: regulator-pll-dcx0 {  

It's DCXO (letter "oh", not zero): digitally controlled/compensated crystal *o*scillator.

D'oh! Had to google PLL, should have googled this too.

Quoted 14 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 204

Quoted 7 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +6 -0
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-name = "vcc-pll";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+	};  

That one looks odd as well. While there might be a discrete regulator (what this node is describing), I doubt it, since the AXP717 has plenty of rails. In particular I am not sure if that fixed one would supply PortG (mainly WiFi), which seems unneeded for the boot process, and surely we want that switchable to save power if the WiFi is not needed.

Quoted 3 lines

You also have a VCC-PLL regulator below (BLDO2).
So please try to drop this regulator, I am pretty sure there is an AXP rail that powers the WiFi.

Makes sense, will dig into the vendor BSP.

Quoted 4 lines

See below for more on this.

Quoted 14 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 225

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+
+&mmc2 {
+	vmmc-supply = <&reg_vcc_sd2>;
+	vqmmc-supply = <&reg_aldo1>;
+	cd-gpios = <&pio 4 22 GPIO_ACTIVE_LOW>; /* PE22 */
+	bus-width = <4>;
+	status = "okay";
+};  

This one seems more plausible. To confirm this, you could not use reg_vcc_sd2 anywhere, and use some other 3.3V supply for vmmc, which should break operation on the second SD card. Then just swap reg_vcc_sd2 back in, and if it starts working again, we have confirmation.

Quoted 1 line

In hindsight this is taken from the vendor BSP, so I think is correct but will do some testing.

Don't sweat it, if that's working, it's fine. If it's not entirely accurate, we can fix that later.

Quoted 50 lines
Quoted 6 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 245

Quoted 2 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +1 -0
+		x-powers,drive-vbus-en;  

Remove this last line, the AXP717 binding does not support this, and the Linux driver doesn't implement this, as the AXP717 does not seem to have this functionality.

Thanks, fixed.

Quoted 23 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 288

Quoted 4 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +4 -0
+
+			reg_aldo3: aldo3 {
+				regulator-always-on;
+				regulator-boot-on;  

Please remove this last line, it doesn't make sense in this context. Cf. Documentation//devicetree/bindings/regulator/regulator.yaml. I think the same applies to the other uses throughout this file.

Review

Quoted 2 lines
(quoted diff) +2 -0
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;  

This is not right. What is the voltage of this rail? The kernel should tell you what was set in the register, through regulator_summary, or you check what's the voltage on a BSP system.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 291

Quoted 1 line
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +1 -0
+				regulator-name = "axp717-aldo3";  

If the system dies when you remove always-on, you might have found some essential supply. Candidates for consumers are listed in the H616 or T5 series datasheet. Matching the voltage might give you an idea. Then use this consumer as the name.

Quoted 6 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 294

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+			};
+
+			reg_aldo4: aldo4 {  

Same for this one: Please remove regulator-boot-on, fix the voltage, and provide some rationale why this needs to be on.

Thanks, will get the correct voltages. It turns out ALDO3/4 are both needed, but CPUSLDO is not. Will try and figure out what they are supplying.

Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 345

Quoted 4 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +4 -0
+
+			reg_boost: boost {
+				regulator-min-microvolt = <5126000>;
+				regulator-max-microvolt = <5126000>;  

It might be better to use a range, say 5.0V till 5.2V. The kernel will then just continue using the default, which seems to be this 5.126V (5440mV + 9 * 64mV).

Quoted 1 line

Thanks, fixed.

I saw in your git tree you kept the max at 5.126V. I think it would make more sense to put 5.2V there, since this is the typical accepted high voltage for VBUS, as issued by many power supplies, for instance. Also it somewhat detaches that entry from the limitations of the AXP, and more states what we *want*.
It wouldn't immediately make a difference anyways, since the kernel will see that the voltage already programmed (5.126V) is within range, and will just keep it.

Cheers,
Andre

Quoted 42 lines
Quoted 7 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 363

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+
+&pio {
+	vcc-pg-supply = <&reg_pll_dcx0>;  

As mentioned above, it seems unlikely to be a fixed regulator. If in doubt, leave them out, they are not essential for the operation.

Will do for now, thanks.

Quoted 18 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 366

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+};
+
+/* the AXP717 has USB type-C role switch functionality, to be implemented */  

Replace "to be implemented" with "not yet described by the binding". This is DT land, so we don't care about implementations or the Linux kernel, it's all about DTs and DT bindings.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 368

Quoted 2 lines

+&usbotg {

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +1 -0
+	dr_mode = "host";   /* USB type-C receptable */  

So does this really work? It seems wrong to make this unconditional, given this is the only way to charge the device. When power is supplied through the USB-C port, surely driving VBUS from the board sounds wrong. Unless you have a killer feature for a host port, I think without working role switching, "peripheral" would be the safer choice.

Quoted 9 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 373

Quoted 5 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+	status = "okay";
+};
+
+&usbphy {
+	usb0_vbus-supply = <&reg_vcc_usb>;  

When you stick to "peripheral" above, you should drop this line for now, especially since this regulator chain looks quite suspicious still.

Thanks, fixed.

Quoted 2 lines

Cheers,
Andre

Thanks again for the review! Will post up a v3 after some more thought about the regulators.

Ryan

#8 lore raw

Re: [PATCH v2 2/5] arm64: dts: allwinner: h700: Add RG35XX 2024 DTS

In reply to Andre Przywara
Quoted 86 lines · Andre Przywara

On Sat, 20 Apr 2024 22:43:56 +1200
Ryan Walklin <[email protected]> wrote:

Hi Ryan,

many thanks for the respin and the changes! We are getting there ...

Quoted 35 lines

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

Changelog v1..v2:
- Update copyright
- Spaces -> Tabs
- Add cpufreq support [1]
- Remove MMC aliases
- Fix GPIO button and regulator node names
- Note unused AXP717 regulators
- Update regulator for SD slots
- Remove unused I2C3 device
- Update NMI interrupt controller for AXP717 [2]
- Add chassis-type
- Address USB EHCI/OHCI0 correctly and add usb vbus supply
- Add PIO vcc-pg-supply
- Correct boost regulator voltage and name

Quoted 4 lines

[1]: https://lore.kernel.org/linux-sunxi/20240419071450.7aiheeoyq35yjoo7@vireshk-i7/T/#t
[2]: https://lore.kernel.org/linux-sunxi/[email protected]/T/#t

As those are dependencies, but WIP, this gets a bit tricky:
- cpufreq is pretty far, but comes through a different tree. I suggest you drop the cpu-opp.dtsi include, to not complicate things. We can add this later as a fix, once this OPP file has reached the master tree.
- The NMI binding and DT node seem important, but have not been merged yet. I suggest to mention them as a requirement. The patches (binding plus H616 .dtsi change) should go through the sunxi tree as well, so the maintainers can order this appropriately when merging.
- The GPADC (in the later patch) is similar, but it is not as critical as the NMI. Not sure how the maintainers want to handle this, but we might add those bits as an (optional) patch on top, so this can be handled more independently from the GPADC series.

Quoted 13 lines
---
 .../sun50i-h700-anbernic-rg35xx-2024.dts      | 375 ++++++++++++++++++
 1 file changed, 375 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +0 -0
new file mode 100644
index 000000000000..d8081273677d

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 9

Quoted 9 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +9 -0
@@ -0,0 +1,375 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) 2024 Ryan Walklin <[email protected]>.
+ */
+
+/dts-v1/;
+
+#include "sun50i-h616.dtsi"
+#include "sun50i-h616-cpu-opp.dtsi"

As mentioned, please drop this line for now.

Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 17

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+#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";
+	chassis-type = "handset";

I've done my extensive testing on the 35XXH only so far, but based on past experience I strongly doubt Anbernic did much different between boards. Superficially the main differences besides form factor are gpadc joysticks and an extra USB host port on the 35XXH, so anything I have to say about USB look at with skepticsm.

Quoted 11 lines
Quoted 11 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +11 -0
+	compatible = "anbernic,rg35xx-2024", "allwinner,sun50i-h700";
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	leds {
Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 36

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+		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";
+		};

FYI - PI12 can run as a PWM so when we get PWM output I intend to request this be run as a PWM led (so we can adjust the brightness). I'll handle that when it's ready though so don't worry for now.

Quoted 105 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 42

Quoted 6 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +6 -0
+	};
+
+	gpio-keys {
+	   compatible = "gpio-keys";
+
+	   button-up {

indentation

Quoted 96 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +96 -0
+		   label = "D-Pad Up";
+		   gpios = <&pio 0 6 GPIO_ACTIVE_LOW>; /* PA6 */
+		   linux,input-type = <EV_KEY>;
+		   linux,code = <BTN_DPAD_UP>;
+		};
+
+		button-down {
+			label = "D-Pad Down";
+			gpios = <&pio 4 0 GPIO_ACTIVE_LOW>; /* PE0 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_DOWN>;
+		};
+
+		button-left {
+			label = "D-Pad left";
+			gpios = <&pio 0 8 GPIO_ACTIVE_LOW>; /* PA8 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_LEFT>;
+		};
+
+		button-right {
+			label = "D-Pad Right";
+			gpios = <&pio 0 9 GPIO_ACTIVE_LOW>; /* PA9 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_RIGHT>;
+		};
+
+		button-a {
+			label = "Action-Pad A";
+			gpios = <&pio 0 0 GPIO_ACTIVE_LOW>; /* PA0 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_EAST>;
+		};
+
+		button-b {
+			label = "Action-Pad B";
+			gpios = <&pio 0 1 GPIO_ACTIVE_LOW>; /* PA1 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_SOUTH>;
+		};
+
+		button-x {
+			label = "Action-Pad X";
+			gpios = <&pio 0 3 GPIO_ACTIVE_LOW>; /* PA3 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_NORTH>;
+		};
+
+		button-y {
+			label = "Action Pad Y";
+			gpios = <&pio 0 2 GPIO_ACTIVE_LOW>; /* PA2 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_WEST>;
+		};
+
+		button-start {
+			label = "Key Start";
+			gpios = <&pio 0 4 GPIO_ACTIVE_LOW>; /* PA4 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_START>;
+		};
+
+		button-select {
+			label = "Key Select";
+			gpios = <&pio 0 5 GPIO_ACTIVE_LOW>; /* PA5 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_SELECT>;
+		};
+
+		button-l1 {
+			label = "Key L1";
+			gpios = <&pio 0 10 GPIO_ACTIVE_LOW>; /* PA10 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TL>;
+		};
+
+		button-l2 {
+			label = "Key L2";
+			gpios = <&pio 0 11 GPIO_ACTIVE_LOW>; /* PA11 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TL2>;
+		};
+
+		button-r1 {
+			label = "Key R1";
+			gpios = <&pio 0 12 GPIO_ACTIVE_LOW>; /* PA12 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TR>;
+		};
+
+		button-r2 {
+			label = "Key R2";
+			gpios = <&pio 0 7 GPIO_ACTIVE_LOW>; /* PA7 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TR2>;
+		};
Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 146

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+
+		button-menu {
+			label = "Key Menu";
+			gpios = <&pio 4 3 GPIO_ACTIVE_LOW>; /* PE3 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_MODE>;
+		};
+

Your preference, but in the past I've had the volume up/down done as a seperate node so we can enable key repeat. That way holding volume up or down has the effect of continuously raising or lowering the volume. It's desirable for volume buttons, but not for gamepads, hence the separation. Also I usually alphabetize node names, I don't remember if I'm just that anal or if I was told to at one point though.

Quoted 49 lines
Quoted 21 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +21 -0
+		button-vol-up {
+			label = "Key Volume Up";
+			gpios = <&pio 4 1 GPIO_ACTIVE_LOW>; /* PE1 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <KEY_VOLUMEUP>;
+		};
+
+		button-vol-down {
+			label = "Key Volume Down";
+			gpios = <&pio 4 2 GPIO_ACTIVE_LOW>; /* PE2 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <KEY_VOLUMEDOWN>;
+		};
+	};
+
+	reg_vcc_sd2: regulator-vcc3v3 {
+		compatible = "regulator-fixed";
+		gpio = <&pio 4 4 GPIO_ACTIVE_HIGH>; /* PE4 */
+		regulator-name = "vcc_3v3_sd2";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 177

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+	};
+
+	reg_vcc_usb: regulator-vcc-5v0-usb {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&pio 8 7 GPIO_ACTIVE_HIGH>; /* PI7 */
+		regulator-name = "vcc_5v0_usb";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vcc_3v3_usb>;

This looks wrong, see below.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 185

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+	};
+
+	vcc_3v3_usb: vcc-3v3-usb {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&pio 4 5 GPIO_ACTIVE_HIGH>; /* PE5 */
+		regulator-always-on;
+		regulator-boot-on;

There seems to be something off with this one. First, it seems odd that an always-on regulator is GPIO controlled, as this surely means it's not enabled at reset time (because the GPIO is initially HighZ and thus not enabled). So who turns this on? Most likely it's the kernel? What happens if we turn this off (or leave it off)?

Quoted 5 lines

Secondly, why is this 3.3V (both by name and voltage), but then supplies the 5.0V USB VBUS?
And given that this is chained with reg_vcc_usb above, are those really two regulators, controlled by two GPIOs?

It's described wrong, but based on the behaviour I've seen. The specific seems to be 2 GPIO controlled regulators; one of them enables the logic voltage for the USB (the 3.3v regulator) and the other enables the VBUS for the USB (the 5v regulator). If you only enable the 5v regulator the bus otherwise lays dormant. If you only enable the 3v3 regulator the USB bus powers on and intermittently enumerates devices at 3.3v. Further enabling the 5v regulator drives the USB at 4.6v (I'm guessing something is still wrong, we have more to poke at).

To my knowledge this only applies for the USB host port which this device lacks.

Quoted 30 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 198

Quoted 13 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +13 -0
+		regulator-name = "vcc_3v3_usb";
+		regulator-max-microvolt = <3300000>;
+		regulator-min-microvolt = <3300000>;
+	};
+
+	reg_vcc5v: regulator-vcc5v { /* USB-C power input */
+		compatible = "regulator-fixed";
+		regulator-name = "vcc-5v";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+	};
+
+	reg_pll_dcx0: regulator-pll-dcx0 {

It's DCXO (letter "oh", not zero): digitally controlled/compensated crystal *o*scillator.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 204

Quoted 6 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +6 -0
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-name = "vcc-pll";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+	};

That one looks odd as well. While there might be a discrete regulator (what this node is describing), I doubt it, since the AXP717 has plenty of rails. In particular I am not sure if that fixed one would supply PortG (mainly WiFi), which seems unneeded for the boot process, and surely we want that switchable to save power if the WiFi is not needed.

Quoted 5 lines

You also have a VCC-PLL regulator below (BLDO2).
So please try to drop this regulator, I am pretty sure there is an AXP rail that powers the WiFi.

See below for more on this.

After much digging I'm certain the rails powering the wifi are reg_cldo4 for the 3.3v and reg_aldo4 for the IO bits at 1.8v. The curious thing about reg_cldo4 is it appears to feed into another regulator that regulates at precisely 3.3v. If I under or overvolt cldo4 I still read exactly 3.3v at the wifi module, but as soon as I turn off reg_cldo4 the wifi reads 0v. So while I can't directly control the power of the wifi's 3.3v rail, as long as I get cldo4 close to 3.3v it powers correctly.

Quoted 15 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 212

Quoted 9 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+};
+
+&cpu0 {
+	cpu-supply = <&reg_dcdc1>;
+};
+
+&mmc0 {
+	vmmc-supply = <&reg_vcc_sd2>;

I don't think this GPIO controlled regulator provides the supply voltage for the first SD card, since it would be disabled on reset, and the BROM couldn't boot from the SD card. So it must be some other 3.3V source, either a discrete regulator (fixed regulator), or some default-on 3.3V AXP rail.

Both the vcc and the logic for the mmc0 appear to be handled by the cldo3 regulator at 3.3v.

Quoted 11 lines
Quoted 11 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+	disable-wp;
+	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>;  /* PF6 */
+	bus-width = <4>;
+	status = "okay";
+};
+
+&mmc2 {
+	vmmc-supply = <&reg_vcc_sd2>;
+	vqmmc-supply = <&reg_aldo1>;
+	cd-gpios = <&pio 4 22 GPIO_ACTIVE_LOW>; /* PE22 */
Quoted 9 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 217

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+	bus-width = <4>;
+	status = "okay";
+};

This one seems more plausible. To confirm this, you could not use reg_vcc_sd2 anywhere, and use some other 3.3V supply for vmmc, which should break operation on the second SD card. Then just swap reg_vcc_sd2 back in, and if it starts working again, we have confirmation.

cldo3 and the gpio controlled regulator are the 2 regulators used for mmc2 on my board. My notes have the vmmc supply as cldo3 and the vqmmc supply as the GPIO controlled one, but that feels wrong. That said the IO pins measured 1.1v when the GPIO regulator was off and 3.3v when the GPIO regulator was on. The 1.1v didn't seem to come from the PMIC, as the only rail I had running 1.1v at the time was the RAM and I tested and confirmed that wasn't it.

Quoted 9 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 234

Quoted 9 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +9 -0
+
+&ohci0 {
+	status = "okay";
+};
+
+&ehci0 {
+	status = "okay";
+};
+

I haven't confirmed on my board we need ohci0 and ehci0 for the OTG port.

Quoted 3 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 237

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+&r_rsb {
+   status = "okay";
+

Any advantage to doing this on the rsb over i2c? That's how I have mine wired. Both are fine with me, I just didn't know what was better.

Quoted 13 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 245

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+   axp717: pmic@3a3 {
+		compatible = "x-powers,axp717";
+		reg = <0x3a3>;
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		interrupt-parent = <&nmi_intc>;
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+		x-powers,drive-vbus-en;

Remove this last line, the AXP717 binding does not support this, and the Linux driver doesn't implement this, as the AXP717 does not seem to have this functionality.

Quoted 5 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 250

Quoted 5 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+
+		vin1-supply = <&reg_vcc5v>;
+		vin2-supply = <&reg_vcc5v>;
+		vin3-supply = <&reg_vcc5v>;
+		vin4-supply = <&reg_vcc5v>;

FYI - I never actually confirmed the vin supply of the PMIC, I just put these in here to shut up some errors or warnings. If these are based on something I did it couldn't hurt to recheck.

Quoted 7 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 257

Quoted 7 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +7 -0
+
+		regulators {
+			reg_dcdc1: dcdc1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <810000>;
+				regulator-max-microvolt = <1100000>;

The CPU opp table in the BSP device tree had this between 900000 and 1120000. Though like most things, take anything in the BSP device tree with a grain of salt (lime and tequila help too).

Quoted 3 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 260

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+				regulator-name = "vdd-cpu";
+			};
+

I'm just speculating, but I strongly *guess* that this dcdc2 is used for the GPU. SoC datasheet says max should be 900000 for the GPU, but I don't have an opp table to go off of sadly.

Quoted 47 lines
Quoted 18 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +18 -0
+			reg_dcdc2: dcdc2 {
+				regulator-always-on;
+				regulator-min-microvolt = <940000>;
+				regulator-max-microvolt = <940000>;
+				regulator-name = "vdd-sys";
+			};
+
+			reg_dcdc3: dcdc3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1100000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-name = "vdd-dram";
+			};
+
+			reg_aldo1: aldo1 {
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 288

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+				regulator-name = "vcc-sd2";
+			};
+
+			reg_aldo2: aldo2 {
+				/* unused */
+			};
+
+			reg_aldo3: aldo3 {
+				regulator-always-on;
+				regulator-boot-on;

Please remove this last line, it doesn't make sense in this context. Cf. Documentation//devicetree/bindings/regulator/regulator.yaml. I think the same applies to the other uses throughout this file.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 290

Quoted 2 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +2 -0
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;

This is not right. What is the voltage of this rail? The kernel should tell you what was set in the register, through regulator_summary, or you check what's the voltage on a BSP system.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 291

Quoted 1 line
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +1 -0
+				regulator-name = "axp717-aldo3";

If the system dies when you remove always-on, you might have found some essential supply. Candidates for consumers are listed in the H616 or T5 series datasheet. Matching the voltage might give you an idea. Then use this consumer as the name.

Quoted 6 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 294

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+			};
+
+			reg_aldo4: aldo4 {

Same for this one: Please remove regulator-boot-on, fix the voltage, and provide some rationale why this needs to be on.

aldo4 is a critical regulator that should run at 1.8v based on my testing.

Quoted 41 lines
Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +7 -0
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;
+				regulator-name = "axp717-aldo4";
+			};
+

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 311

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+			reg_bldo1: bldo1 {
+				/* unused */
+			};
+
+			reg_bldo2: bldo2 {
+				regulator-always-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-name = "vcc-pll";
+			};

This one is a good example: fixed voltage, no boot-on, and regulator name provides rationale why it must be always-on. The others should look similar.

Quoted 18 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +18 -0
+
+			reg_bldo3: bldo3 {
+				/* unused */
+			};
+
+			reg_bldo4: bldo4 {
+				/* unused */
+			};
+
+			reg_cldo1: cldo1 {
+				/* unused */
+			};
+
+			reg_cldo2: cldo2 {
+				/* unused */
+			};
+
+			reg_cldo3: cldo3 {
Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 334

Quoted 5 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;
+				regulator-name = "axp717-cldo3";

Same here as ALDO3/4: what voltage is it, and what does it probably supply?

cldo3 is a critical regulator run at 3.3v based on my testing. It supplies power to the majority of the system's pins.

Quoted 3 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 336

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +2 -0
+			};
+

cldo4 feeds the wifi, but through another fixed voltage regulator that seems to pull up or down to 3.3v if we happen to raise or lower cldo4 a bit above or below 3.3v.

Quoted 14 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 345

Quoted 9 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +9 -0
+			reg_cldo4: cldo4 {
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcc-wifi";
+			};
+
+			reg_boost: boost {
+				regulator-min-microvolt = <5126000>;
+				regulator-max-microvolt = <5126000>;

It might be better to use a range, say 5.0V till 5.2V. The kernel will then just continue using the default, which seems to be this 5.126V (5440mV + 9 * 64mV).

Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 350

Quoted 5 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+				regulator-name = "boost";
+			};
+
+			reg_cpusldo: cpusldo {
+				/* unused */

Is that so? I thought you mentioned on IRC this is required as well?

I still haven't completely finished the regulator analysis, but I actually think it is the case that this one isn't used.

Quoted 8 lines
Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+			};
+		};
+	};
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_ph_pins>;
Quoted 9 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 363

Quoted 5 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+	status = "okay";
+};
+
+&pio {
+	vcc-pg-supply = <&reg_pll_dcx0>;

As mentioned above, it seems unlikely to be a fixed regulator. If in doubt, leave them out, they are not essential for the operation.

On my 35XXH I can confirm PG is supplied by reg_aldo4 at 1.8v and the rest are supplied by reg_cldo3 at 3.3v.

        vcc-pa-supply = <&reg_cldo3>;
        vcc-pc-supply = <&reg_cldo3>;
        vcc-pe-supply = <&reg_cldo3>;
        vcc-pf-supply = <&reg_cldo3>;
        vcc-pg-supply = <&reg_aldo4>;
        vcc-ph-supply = <&reg_cldo3>;
        vcc-pi-supply = <&reg_cldo3>;

This is what my 35XXH looks like after manually raising or lowering the PMIC voltage values a tad, observing the GPIO out voltages, adjusting again, measuring the voltages again, etc etc. By checking at least 1 pin from each bank and confirming voltage changes via PMIC changes I get this mapping.

Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 366

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+};
+
+/* the AXP717 has USB type-C role switch functionality, to be implemented */

Replace "to be implemented" with "not yet described by the binding". This is DT land, so we don't care about implementations or the Linux kernel, it's all about DTs and DT bindings.

Quoted 9 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 368

Quoted 2 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +2 -0
+&usbotg {
+	dr_mode = "host";   /* USB type-C receptable */

So does this really work? It seems wrong to make this unconditional, given this is the only way to charge the device. When power is supplied through the USB-C port, surely driving VBUS from the board sounds wrong. Unless you have a killer feature for a host port, I think without working role switching, "peripheral" would be the safer choice.

Again making me wish I'd have used a different device for my mainline efforts, but on the 35XXH the OTG port I know does work for me as a peripherial port. As for the Type-C stuff, I don't expect any future AXP717 modifications to matter for this device, as manually fiddling the USB-C bits in the PMIC didn't register my otg port as anything other than a standard USB port (none of the USB-C specific registers seemed to register anything).

Quoted 10 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 373

Quoted 6 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+	status = "okay";
+};
+
+&usbphy {
+	usb0_vbus-supply = <&reg_vcc_usb>;

When you stick to "peripheral" above, you should drop this line for now, especially since this regulator chain looks quite suspicious still.

Concur, especially since now when I look at my 35XX+ 2024 model I only see an OTG port and not a host port.

Quoted 6 lines

Cheers,
Andre

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 375

Quoted 2 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +2 -0
+	status = "okay";
+};

Thank you for all your hard work on this series. I'm going to continue to try and identify the remaining regulators on my board and what they do/how they're used. I expect at least one or two of the ones we've flagged for removal will need to be added back once we get the panel code working.

Chris

#9 lore raw

Re: [PATCH v2 2/5] arm64: dts: allwinner: h700: Add RG35XX 2024 DTS

Quoted 3 lines · Chris Morgan

FYI - PI12 can run as a PWM so when we get PWM output I intend to request this be run as a PWM led (so we can adjust the brightness). I'll handle that when it's ready though so don't worry for now.

Sounds good.

Quoted 2 lines

Your preference, but in the past I've had the volume up/down done as a seperate node so we can enable key repeat.

Done as mentioned.

Quoted 2 lines

separation. Also I usually alphabetize node names, I don't remember if I'm just that anal or if I was told to at one point though.

I'm easy, the order there seemed logical based on the physical layout on the -Plus but I don't have a real preference.

Quoted 10 lines

It's described wrong, but based on the behaviour I've seen. The specific seems to be 2 GPIO controlled regulators; one of them enables the logic voltage for the USB (the 3.3v regulator) and the other enables the VBUS for the USB (the 5v regulator). If you only enable the 5v regulator the bus otherwise lays dormant. If you only enable the 3v3 regulator the USB bus powers on and intermittently enumerates devices at 3.3v. Further enabling the 5v regulator drives the USB at 4.6v (I'm guessing something is still wrong, we have more to poke at).

Quoted 3 lines

To my knowledge this only applies for the USB host port which this device lacks.

Will await your work on this, I'm not at all familiar with USB to this depth, conceptually makes sense though. This will also rely on the boost working well, which is at least now being enabled with the WIP AXP717 driver.

Quoted 9 lines

After much digging I'm certain the rails powering the wifi are reg_cldo4 for the 3.3v and reg_aldo4 for the IO bits at 1.8v. The curious thing about reg_cldo4 is it appears to feed into another regulator that regulates at precisely 3.3v. If I under or overvolt cldo4 I still read exactly 3.3v at the wifi module, but as soon as I turn off reg_cldo4 the wifi reads 0v. So while I can't directly control the power of the wifi's 3.3v rail, as long as I get cldo4 close to 3.3v it powers correctly.

Nice, have reworked the SD and Wifi regulators as described, and power management for the Wifi in particular seems to be working much better after moving it off BLDO2 (which seems to be VCC-PLL) to CLDO4.

Quoted 9 lines
Quoted 8 lines

Review

Quoted 2 lines

+&mmc0 {

(quoted diff) +1 -0
+	vmmc-supply = <&reg_vcc_sd2>;

I don't think this GPIO controlled regulator provides the supply voltage for the first SD card, since it would be disabled on reset, and the BROM couldn't boot from the SD card. So it must be some other 3.3V source, either a discrete regulator (fixed regulator), or some default-on 3.3V AXP rail.

Quoted 3 lines

Both the vcc and the logic for the mmc0 appear to be handled by the cldo3 regulator at 3.3v.

Thanks, makes sense. I've tentatively renamed CLDO3 to VCC-IO, looking at the H616 datasheet.

Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +11 -0
+	disable-wp;
+	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>;  /* PF6 */
+	bus-width = <4>;
+	status = "okay";
+};
+
+&mmc2 {
+	vmmc-supply = <&reg_vcc_sd2>;
+	vqmmc-supply = <&reg_aldo1>;
+	cd-gpios = <&pio 4 22 GPIO_ACTIVE_LOW>; /* PE22 */
+	bus-width = <4>;
Quoted 8 lines
Quoted 8 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+	status = "okay";
+};

This one seems more plausible. To confirm this, you could not use reg_vcc_sd2 anywhere, and use some other 3.3V supply for vmmc, which should break operation on the second SD card. Then just swap reg_vcc_sd2 back in, and if it starts working again, we have confirmation.

Swapped in CLDO4 for MMC2 and the card read fine, will swap it back and see what happens.

Quoted 9 lines

cldo3 and the gpio controlled regulator are the 2 regulators used for mmc2 on my board. My notes have the vmmc supply as cldo3 and the vqmmc supply as the GPIO controlled one, but that feels wrong. That said the IO pins measured 1.1v when the GPIO regulator was off and 3.3v when the GPIO regulator was on. The 1.1v didn't seem to come from the PMIC, as the only rail I had running 1.1v at the time was the RAM and I tested and confirmed that wasn't it.

Quoted 13 lines
Quoted 9 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 234

Quoted 9 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +9 -0
+
+&ohci0 {
+	status = "okay";
+};
+
+&ehci0 {
+	status = "okay";
+};
+

I haven't confirmed on my board we need ohci0 and ehci0 for the OTG port.

OK, shall I remove them or will it do no harm?

Quoted 7 lines
Quoted 3 lines

Review

Quoted 3 lines

+&r_rsb {

(quoted diff) +2 -0
+   status = "okay";
+

Any advantage to doing this on the rsb over i2c? That's how I have mine wired. Both are fine with me, I just didn't know what was better.

I don't think so, this was Andre's first suggestion when he sent the AXP717's kernel driver so have stuck with it, but either should be fine.

Quoted 10 lines
Quoted 6 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 250

Quoted 6 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+
+		vin1-supply = <&reg_vcc5v>;
+		vin2-supply = <&reg_vcc5v>;
+		vin3-supply = <&reg_vcc5v>;
+		vin4-supply = <&reg_vcc5v>;

FYI - I never actually confirmed the vin supply of the PMIC, I just put these in here to shut up some errors or warnings. If these are based on something I did it couldn't hurt to recheck.

Ta, will see if I can find anything out.

Quoted 13 lines
Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 257

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +7 -0
+
+		regulators {
+			reg_dcdc1: dcdc1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <810000>;
+				regulator-max-microvolt = <1100000>;

The CPU opp table in the BSP device tree had this between 900000 and 1120000. Though like most things, take anything in the BSP device tree with a grain of salt (lime and tequila help too).

Quite! That's the spec from the H616 datasheet, I may need to push it up to 1.16v max to hit 1.5Ghz, but will relook at it once the cpufreq patches land.

Quoted 10 lines

I'm just speculating, but I strongly *guess* that this dcdc2 is used for the GPU. SoC datasheet says max should be 900000 for the GPU, but I don't have an opp table to go off of sadly.

Quoted 6 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 266

Quoted 6 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +6 -0
+			reg_dcdc2: dcdc2 {
+				regulator-always-on;
+				regulator-min-microvolt = <940000>;
+				regulator-max-microvolt = <940000>;
+				regulator-name = "vdd-sys";
+			};

Thanks, I think I did know that from somewhere. My reading of the datasheet is 0.81-0.99v though, so will put those in.

Quoted 4 lines

aldo4 is a critical regulator that should run at 1.8v based on my testing.

Yup, noted.

Quoted 4 lines

cldo3 is a critical regulator run at 3.3v based on my testing. It supplies power to the majority of the system's pins.

Have called this VCC-IO for now.

Quoted 11 lines

On my 35XXH I can confirm PG is supplied by reg_aldo4 at 1.8v and the rest are supplied by reg_cldo3 at 3.3v.

        vcc-pa-supply = <&reg_cldo3>;
        vcc-pc-supply = <&reg_cldo3>;
        vcc-pe-supply = <&reg_cldo3>;
        vcc-pf-supply = <&reg_cldo3>;
        vcc-pg-supply = <&reg_aldo4>;
        vcc-ph-supply = <&reg_cldo3>;
        vcc-pi-supply = <&reg_cldo3>;
Quoted 6 lines

This is what my 35XXH looks like after manually raising or lowering the PMIC voltage values a tad, observing the GPIO out voltages, adjusting again, measuring the voltages again, etc etc. By checking at least 1 pin from each bank and confirming voltage changes via PMIC changes I get this mapping.

Great, have added these in. From the vendor DT looks like the audio codec is powered from the G pin bank.

Quoted 7 lines

Thank you for all your hard work on this series. I'm going to continue to try and identify the remaining regulators on my board and what they do/how they're used. I expect at least one or two of the ones we've flagged for removal will need to be added back once we get the panel code working.

Chris

No worries, thanks for the feedback! Much improved now, I think the last big issue here is the USB ports. Have been testing on my -H for now, but will have to look at the -Plus too given the single port.

Ryan

#10 lore raw

Re: [PATCH v2 2/5] arm64: dts: allwinner: h700: Add RG35XX 2024 DTS

On Sun, 21 Apr 2024 20:43:26 +1200
"Ryan Walklin" <[email protected]> wrote:

Hi,

Quoted 102 lines · Chris Morgan
Quoted 4 lines

FYI - PI12 can run as a PWM so when we get PWM output I intend to request this be run as a PWM led (so we can adjust the brightness). I'll handle that when it's ready though so don't worry for now.

Sounds good.

Quoted 2 lines

Your preference, but in the past I've had the volume up/down done as a seperate node so we can enable key repeat.

Done as mentioned.

Quoted 2 lines

separation. Also I usually alphabetize node names, I don't remember if I'm just that anal or if I was told to at one point though.

I'm easy, the order there seemed logical based on the physical layout on the -Plus but I don't have a real preference.

Quoted 10 lines

It's described wrong, but based on the behaviour I've seen. The specific seems to be 2 GPIO controlled regulators; one of them enables the logic voltage for the USB (the 3.3v regulator) and the other enables the VBUS for the USB (the 5v regulator). If you only enable the 5v regulator the bus otherwise lays dormant. If you only enable the 3v3 regulator the USB bus powers on and intermittently enumerates devices at 3.3v. Further enabling the 5v regulator drives the USB at 4.6v (I'm guessing something is still wrong, we have more to poke at).

Quoted 3 lines

To my knowledge this only applies for the USB host port which this device lacks.

Will await your work on this, I'm not at all familiar with USB to this depth, conceptually makes sense though. This will also rely on the boost working well, which is at least now being enabled with the WIP AXP717 driver.

Quoted 9 lines

After much digging I'm certain the rails powering the wifi are reg_cldo4 for the 3.3v and reg_aldo4 for the IO bits at 1.8v. The curious thing about reg_cldo4 is it appears to feed into another regulator that regulates at precisely 3.3v. If I under or overvolt cldo4 I still read exactly 3.3v at the wifi module, but as soon as I turn off reg_cldo4 the wifi reads 0v. So while I can't directly control the power of the wifi's 3.3v rail, as long as I get cldo4 close to 3.3v it powers correctly.

Nice, have reworked the SD and Wifi regulators as described, and power management for the Wifi in particular seems to be working much better after moving it off BLDO2 (which seems to be VCC-PLL) to CLDO4.

Quoted 9 lines
Quoted 8 lines

Review

Quoted 2 lines

+&mmc0 {

(quoted diff) +1 -0
+	vmmc-supply = <&reg_vcc_sd2>;  

I don't think this GPIO controlled regulator provides the supply voltage for the first SD card, since it would be disabled on reset, and the BROM couldn't boot from the SD card. So it must be some other 3.3V source, either a discrete regulator (fixed regulator), or some default-on 3.3V AXP rail.

Quoted 3 lines

Both the vcc and the logic for the mmc0 appear to be handled by the cldo3 regulator at 3.3v.

Thanks, makes sense. I've tentatively renamed CLDO3 to VCC-IO, looking at the H616 datasheet.

Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +11 -0
+	disable-wp;
+	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>;  /* PF6 */
+	bus-width = <4>;
+	status = "okay";
+};
+
+&mmc2 {
+	vmmc-supply = <&reg_vcc_sd2>;
+	vqmmc-supply = <&reg_aldo1>;
+	cd-gpios = <&pio 4 22 GPIO_ACTIVE_LOW>; /* PE22 */
+	bus-width = <4>;
Quoted 8 lines
Quoted 8 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+	status = "okay";
+};  

This one seems more plausible. To confirm this, you could not use reg_vcc_sd2 anywhere, and use some other 3.3V supply for vmmc, which should break operation on the second SD card. Then just swap reg_vcc_sd2 back in, and if it starts working again, we have confirmation.

Swapped in CLDO4 for MMC2 and the card read fine, will swap it back and see what happens.

Quoted 9 lines

cldo3 and the gpio controlled regulator are the 2 regulators used for mmc2 on my board. My notes have the vmmc supply as cldo3 and the vqmmc supply as the GPIO controlled one, but that feels wrong. That said the IO pins measured 1.1v when the GPIO regulator was off and 3.3v when the GPIO regulator was on. The 1.1v didn't seem to come from the PMIC, as the only rail I had running 1.1v at the time was the RAM and I tested and confirmed that wasn't it.

Quoted 13 lines
Quoted 9 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 234

Quoted 9 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +9 -0
+
+&ohci0 {
+	status = "okay";
+};
+
+&ehci0 {
+	status = "okay";
+};
+  

I haven't confirmed on my board we need ohci0 and ehci0 for the OTG port.

Quoted 1 line

OK, shall I remove them or will it do no harm?

For pure peripheral mode we don't need them, but it wouldn't harm to have them anyway, and later on we will want to have them.

Quoted 8 lines
Quoted 7 lines
Quoted 3 lines

Review

Quoted 3 lines

+&r_rsb {

(quoted diff) +2 -0
+   status = "okay";
+  

Any advantage to doing this on the rsb over i2c? That's how I have mine wired. Both are fine with me, I just didn't know what was better.

I don't think so, this was Andre's first suggestion when he sent the AXP717's kernel driver so have stuck with it, but either should be fine.

As mentioned in the other mail, please stick to RSB.

Quoted 32 lines
Quoted 9 lines
Quoted 5 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 250

Quoted 5 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+
+		vin1-supply = <&reg_vcc5v>;
+		vin2-supply = <&reg_vcc5v>;
+		vin3-supply = <&reg_vcc5v>;
+		vin4-supply = <&reg_vcc5v>;  

FYI - I never actually confirmed the vin supply of the PMIC, I just put these in here to shut up some errors or warnings. If these are based on something I did it couldn't hurt to recheck.

Ta, will see if I can find anything out.

Quoted 13 lines
Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 257

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +7 -0
+
+		regulators {
+			reg_dcdc1: dcdc1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <810000>;
+				regulator-max-microvolt = <1100000>;  

The CPU opp table in the BSP device tree had this between 900000 and 1120000. Though like most things, take anything in the BSP device tree with a grain of salt (lime and tequila help too).

Quite! That's the spec from the H616 datasheet, I may need to push it up to 1.16v max to hit 1.5Ghz, but will relook at it once the cpufreq patches land.

Quoted 4 lines

I'm just speculating, but I strongly *guess* that this dcdc2 is used for the GPU. SoC datasheet says max should be 900000 for the GPU, but I don't have an opp table to go off of sadly.

Quoted 8 lines
Quoted 6 lines
Quoted 6 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 266

Quoted 6 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +6 -0
+			reg_dcdc2: dcdc2 {
+				regulator-always-on;
+				regulator-min-microvolt = <940000>;
+				regulator-max-microvolt = <940000>;
+				regulator-name = "vdd-sys";
+			};  

Thanks, I think I did know that from somewhere. My reading of the datasheet is 0.81-0.99v though, so will put those in.

Please try to be as precise as possible. The BSP used 0.94V, if I remember the dumps correctly, so it's better to stick with that single value. *Changing* such a critical voltage during runtime sounds like asking for trouble, and given the recent experience with GPU DVFS on the A64, I'd say we stick with a single voltage.
Also a range only makes sense if the consumer knows about that and supports a change, examples are the SD card I/O voltage, or the CPU supply, for DVFS. But most devices just *enable* their regulator. So this would leave the voltage at whatever was set before, which does not sound helpful if this was wrong or slightly off for whatever reason.

Quoted 10 lines
Quoted 3 lines

aldo4 is a critical regulator that should run at 1.8v based on my testing.

Yup, noted.

Quoted 4 lines

cldo3 is a critical regulator run at 3.3v based on my testing. It supplies power to the majority of the system's pins.

Have called this VCC-IO for now.

Yeah, that's a good name.

Cheers,
Andre

Quoted 31 lines
Quoted 11 lines

On my 35XXH I can confirm PG is supplied by reg_aldo4 at 1.8v and the rest are supplied by reg_cldo3 at 3.3v.

        vcc-pa-supply = <&reg_cldo3>;
        vcc-pc-supply = <&reg_cldo3>;
        vcc-pe-supply = <&reg_cldo3>;
        vcc-pf-supply = <&reg_cldo3>;
        vcc-pg-supply = <&reg_aldo4>;
        vcc-ph-supply = <&reg_cldo3>;
        vcc-pi-supply = <&reg_cldo3>;
Quoted 6 lines

This is what my 35XXH looks like after manually raising or lowering the PMIC voltage values a tad, observing the GPIO out voltages, adjusting again, measuring the voltages again, etc etc. By checking at least 1 pin from each bank and confirming voltage changes via PMIC changes I get this mapping.

Great, have added these in. From the vendor DT looks like the audio codec is powered from the G pin bank.

Quoted 7 lines

Thank you for all your hard work on this series. I'm going to continue to try and identify the remaining regulators on my board and what they do/how they're used. I expect at least one or two of the ones we've flagged for removal will need to be added back once we get the panel code working.

Chris

No worries, thanks for the feedback! Much improved now, I think the last big issue here is the USB ports. Have been testing on my -H for now, but will have to look at the -Plus too given the single port.

Ryan

#11 lore raw

Re: [PATCH v2 2/5] arm64: dts: allwinner: h700: Add RG35XX 2024 DTS

In reply to Chris Morgan

On Sat, 20 Apr 2024 23:00:15 -0500
Chris Morgan <[email protected]> wrote:

Hi,

Quoted 117 lines · Andre Przywara
Quoted 86 lines

On Sat, 20 Apr 2024 22:43:56 +1200
Ryan Walklin <[email protected]> wrote:

Hi Ryan,

many thanks for the respin and the changes! We are getting there ...

Quoted 35 lines

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

Changelog v1..v2:
- Update copyright
- Spaces -> Tabs
- Add cpufreq support [1]
- Remove MMC aliases
- Fix GPIO button and regulator node names
- Note unused AXP717 regulators
- Update regulator for SD slots
- Remove unused I2C3 device
- Update NMI interrupt controller for AXP717 [2]
- Add chassis-type
- Address USB EHCI/OHCI0 correctly and add usb vbus supply
- Add PIO vcc-pg-supply
- Correct boost regulator voltage and name

Quoted 4 lines

[1]: https://lore.kernel.org/linux-sunxi/20240419071450.7aiheeoyq35yjoo7@vireshk-i7/T/#t
[2]: https://lore.kernel.org/linux-sunxi/[email protected]/T/#t

As those are dependencies, but WIP, this gets a bit tricky:
- cpufreq is pretty far, but comes through a different tree. I suggest you drop the cpu-opp.dtsi include, to not complicate things. We can add this later as a fix, once this OPP file has reached the master tree.
- The NMI binding and DT node seem important, but have not been merged yet. I suggest to mention them as a requirement. The patches (binding plus H616 .dtsi change) should go through the sunxi tree as well, so the maintainers can order this appropriately when merging.
- The GPADC (in the later patch) is similar, but it is not as critical as the NMI. Not sure how the maintainers want to handle this, but we might add those bits as an (optional) patch on top, so this can be handled more independently from the GPADC series.

Quoted 13 lines
---
 .../sun50i-h700-anbernic-rg35xx-2024.dts      | 375 ++++++++++++++++++
 1 file changed, 375 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +0 -0
new file mode 100644
index 000000000000..d8081273677d

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 9

Quoted 9 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +9 -0
@@ -0,0 +1,375 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) 2024 Ryan Walklin <[email protected]>.
+ */
+
+/dts-v1/;
+
+#include "sun50i-h616.dtsi"
+#include "sun50i-h616-cpu-opp.dtsi"  

As mentioned, please drop this line for now.

Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 17

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+#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";
+	chassis-type = "handset";  

I've done my extensive testing on the 35XXH only so far, but based on past experience I strongly doubt Anbernic did much different between boards. Superficially the main differences besides form factor are gpadc joysticks and an extra USB host port on the 35XXH, so anything I have to say about USB look at with skepticsm.

Quoted 15 lines
Quoted 15 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +15 -0
+	compatible = "anbernic,rg35xx-2024", "allwinner,sun50i-h700";
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		led-0 {
+			function = LED_FUNCTION_POWER;
Quoted 8 lines
Quoted 4 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 36

Quoted 4 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +4 -0
+			color = <LED_COLOR_ID_GREEN>;
+			gpios = <&pio 8 12 GPIO_ACTIVE_HIGH>; /* PI12 */
+			default-state = "on";
+		};  

FYI - PI12 can run as a PWM so when we get PWM output I intend to request this be run as a PWM led (so we can adjust the brightness). I'll handle that when it's ready though so don't worry for now.

Well, this has the bitter taste of requiring the PWM driver, so it wouldn't work with current kernels. As the DTs are independent from the kernel, this is some kind of regression: with the current DT it works on every kernel, but using an updated DT (using PWM) the LED would stay off. So it needs to be seen whether it's worth it. Maybe it works when U-Boot powers this on, and Linux wouldn't touch it, when there is no PWM support. Anyway that's indeed something to consider later, just wanted to mention it.

Quoted 112 lines
Quoted 112 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 42

Quoted 6 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +6 -0
+	};
+
+	gpio-keys {
+	   compatible = "gpio-keys";
+
+	   button-up {  

indentation

Quoted 103 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +103 -0
+		   label = "D-Pad Up";
+		   gpios = <&pio 0 6 GPIO_ACTIVE_LOW>; /* PA6 */
+		   linux,input-type = <EV_KEY>;
+		   linux,code = <BTN_DPAD_UP>;
+		};
+
+		button-down {
+			label = "D-Pad Down";
+			gpios = <&pio 4 0 GPIO_ACTIVE_LOW>; /* PE0 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_DOWN>;
+		};
+
+		button-left {
+			label = "D-Pad left";
+			gpios = <&pio 0 8 GPIO_ACTIVE_LOW>; /* PA8 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_LEFT>;
+		};
+
+		button-right {
+			label = "D-Pad Right";
+			gpios = <&pio 0 9 GPIO_ACTIVE_LOW>; /* PA9 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_RIGHT>;
+		};
+
+		button-a {
+			label = "Action-Pad A";
+			gpios = <&pio 0 0 GPIO_ACTIVE_LOW>; /* PA0 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_EAST>;
+		};
+
+		button-b {
+			label = "Action-Pad B";
+			gpios = <&pio 0 1 GPIO_ACTIVE_LOW>; /* PA1 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_SOUTH>;
+		};
+
+		button-x {
+			label = "Action-Pad X";
+			gpios = <&pio 0 3 GPIO_ACTIVE_LOW>; /* PA3 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_NORTH>;
+		};
+
+		button-y {
+			label = "Action Pad Y";
+			gpios = <&pio 0 2 GPIO_ACTIVE_LOW>; /* PA2 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_WEST>;
+		};
+
+		button-start {
+			label = "Key Start";
+			gpios = <&pio 0 4 GPIO_ACTIVE_LOW>; /* PA4 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_START>;
+		};
+
+		button-select {
+			label = "Key Select";
+			gpios = <&pio 0 5 GPIO_ACTIVE_LOW>; /* PA5 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_SELECT>;
+		};
+
+		button-l1 {
+			label = "Key L1";
+			gpios = <&pio 0 10 GPIO_ACTIVE_LOW>; /* PA10 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TL>;
+		};
+
+		button-l2 {
+			label = "Key L2";
+			gpios = <&pio 0 11 GPIO_ACTIVE_LOW>; /* PA11 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TL2>;
+		};
+
+		button-r1 {
+			label = "Key R1";
+			gpios = <&pio 0 12 GPIO_ACTIVE_LOW>; /* PA12 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TR>;
+		};
+
+		button-r2 {
+			label = "Key R2";
+			gpios = <&pio 0 7 GPIO_ACTIVE_LOW>; /* PA7 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TR2>;
+		};
+
+		button-menu {
+			label = "Key Menu";
+			gpios = <&pio 4 3 GPIO_ACTIVE_LOW>; /* PE3 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_MODE>;
+		};
Quoted 8 lines
Quoted 1 line

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 146

Quoted 1 line
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +1 -0
+  

Your preference, but in the past I've had the volume up/down done as a seperate node so we can enable key repeat. That way holding volume up or down has the effect of continuously raising or lowering the volume. It's desirable for volume buttons, but not for gamepads, hence the separation. Also I usually alphabetize node names, I don't remember if I'm just that anal or if I was told to at one point though.

Typically some kind of ordering is preferred, to avoid confusion as to where put new nodes. And if in doubt, this is indeed alphabetical.

Quoted 56 lines
Quoted 50 lines
Quoted 22 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +21 -0
+		button-vol-up {
+			label = "Key Volume Up";
+			gpios = <&pio 4 1 GPIO_ACTIVE_LOW>; /* PE1 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <KEY_VOLUMEUP>;
+		};
+
+		button-vol-down {
+			label = "Key Volume Down";
+			gpios = <&pio 4 2 GPIO_ACTIVE_LOW>; /* PE2 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <KEY_VOLUMEDOWN>;
+		};
+	};
+
+	reg_vcc_sd2: regulator-vcc3v3 {
+		compatible = "regulator-fixed";
+		gpio = <&pio 4 4 GPIO_ACTIVE_HIGH>; /* PE4 */
+		regulator-name = "vcc_3v3_sd2";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 177

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+	};
+
+	reg_vcc_usb: regulator-vcc-5v0-usb {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&pio 8 7 GPIO_ACTIVE_HIGH>; /* PI7 */
+		regulator-name = "vcc_5v0_usb";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vcc_3v3_usb>;  

This looks wrong, see below.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 185

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+	};
+
+	vcc_3v3_usb: vcc-3v3-usb {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&pio 4 5 GPIO_ACTIVE_HIGH>; /* PE5 */
+		regulator-always-on;
+		regulator-boot-on;  

There seems to be something off with this one. First, it seems odd that an always-on regulator is GPIO controlled, as this surely means it's not enabled at reset time (because the GPIO is initially HighZ and thus not enabled). So who turns this on? Most likely it's the kernel? What happens if we turn this off (or leave it off)?

Quoted 5 lines

Secondly, why is this 3.3V (both by name and voltage), but then supplies the 5.0V USB VBUS?
And given that this is chained with reg_vcc_usb above, are those really two regulators, controlled by two GPIOs?

Quoted 3 lines

It's described wrong, but based on the behaviour I've seen. The specific seems to be 2 GPIO controlled regulators; one of them enables the logic voltage for the USB (the 3.3v regulator)

As mentioned in the other mail, there are VCC-USB and VCC-USB2 pins on the SoC, requiring 3.3V for the internal USB logic/PHY.
But it would be odd to see that on a GPIO controlled regulator, since IIUC this is essential for USB operation, so would affect the FEL mode as well, which is driven early by the BROM, knowing nothing about the board specifics.
So I would expect any AXP rails (except those being on at reset) or GPIO regulators only being responsible for *host mode* related power - FEL mode does not need that, it's just peripheral mode. But since the AXP has reset defaults, it might be an AXP rails that drives VCC-USB.

Quoted 6 lines

and the other enables the VBUS
for the USB (the 5v regulator). If you only enable the 5v regulator the bus otherwise lays dormant. If you only enable the 3v3 regulator the USB bus powers on and intermittently enumerates devices at 3.3v. Further enabling the 5v regulator drives the USB at 4.6v (I'm guessing something is still wrong, we have more to poke at).

As mentioned, 4.6V would make sense if that's the pass-through from the input power. It's probably already not 5.0V anymore when it comes in, and might drop further under load.

Quoted 40 lines

To my knowledge this only applies for the USB host port which this device lacks.

Quoted 30 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 198

Quoted 13 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +13 -0
+		regulator-name = "vcc_3v3_usb";
+		regulator-max-microvolt = <3300000>;
+		regulator-min-microvolt = <3300000>;
+	};
+
+	reg_vcc5v: regulator-vcc5v { /* USB-C power input */
+		compatible = "regulator-fixed";
+		regulator-name = "vcc-5v";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+	};
+
+	reg_pll_dcx0: regulator-pll-dcx0 {  

It's DCXO (letter "oh", not zero): digitally controlled/compensated crystal *o*scillator.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 204

Quoted 6 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +6 -0
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-name = "vcc-pll";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+	};  

That one looks odd as well. While there might be a discrete regulator (what this node is describing), I doubt it, since the AXP717 has plenty of rails. In particular I am not sure if that fixed one would supply PortG (mainly WiFi), which seems unneeded for the boot process, and surely we want that switchable to save power if the WiFi is not needed.

Quoted 5 lines

You also have a VCC-PLL regulator below (BLDO2).
So please try to drop this regulator, I am pretty sure there is an AXP rail that powers the WiFi.

See below for more on this.

Quoted 8 lines

After much digging I'm certain the rails powering the wifi are reg_cldo4 for the 3.3v and reg_aldo4 for the IO bits at 1.8v. The curious thing about reg_cldo4 is it appears to feed into another regulator that regulates at precisely 3.3v. If I under or overvolt cldo4 I still read exactly 3.3v at the wifi module, but as soon as I turn off reg_cldo4 the wifi reads 0v. So while I can't directly control the power of the wifi's 3.3v rail, as long as I get cldo4 close to 3.3v it powers correctly.

That's interesting, but I highly doubt that Anbernic would slap an expensive buck/boost converter to something uncritical as the WiFi supply. So there might be something different at play here? Are you able to get any hints from looking at the PCB and the traces? Those regulators are typically easy to spot.

Quoted 15 lines
Quoted 6 lines
Quoted 6 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +6 -0
+};
+
+&cpu0 {
+	cpu-supply = <&reg_dcdc1>;
+};
+
Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 212

Quoted 2 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +2 -0
+&mmc0 {
+	vmmc-supply = <&reg_vcc_sd2>;  

I don't think this GPIO controlled regulator provides the supply voltage for the first SD card, since it would be disabled on reset, and the BROM couldn't boot from the SD card. So it must be some other 3.3V source, either a discrete regulator (fixed regulator), or some default-on 3.3V AXP rail.

Quoted 2 lines

Both the vcc and the logic for the mmc0 appear to be handled by the cldo3 regulator at 3.3v.

That makes sense, and would mean that CLDO3 is enabled on AXP reset and set to 3.3V. The AXP datasheet is not helpful, and typically just refers to "EFUSE" when it comes to the reset values.

Would you be able to poke the AXP early? Maybe drop the AXP driver from U-Boot proper, and read the registers from U-Boot via the i2c command? To see what the default values and enabled state is for all rails?

Quoted 20 lines
Quoted 10 lines
Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+	disable-wp;
+	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>;  /* PF6 */
+	bus-width = <4>;
+	status = "okay";
+};
+
+&mmc2 {
+	vmmc-supply = <&reg_vcc_sd2>;
+	vqmmc-supply = <&reg_aldo1>;
+	cd-gpios = <&pio 4 22 GPIO_ACTIVE_LOW>; /* PE22 */
Quoted 9 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 217

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+	bus-width = <4>;
+	status = "okay";
+};  

This one seems more plausible. To confirm this, you could not use reg_vcc_sd2 anywhere, and use some other 3.3V supply for vmmc, which should break operation on the second SD card. Then just swap reg_vcc_sd2 back in, and if it starts working again, we have confirmation.

Quoted 7 lines

cldo3 and the gpio controlled regulator are the 2 regulators used for mmc2 on my board. My notes have the vmmc supply as cldo3 and the vqmmc supply as the GPIO controlled one, but that feels wrong. That said the IO pins measured 1.1v when the GPIO regulator was off and 3.3v when the GPIO regulator was on. The 1.1v didn't seem to come from the PMIC, as the only rail I had running 1.1v at the time was the RAM and I tested and confirmed that wasn't it.

The easiest solution would just wire the PortC pins directly to the SD card socket, and whatever VCC-PC is, would be used.
There is the possibility to have switchable level shifters between the SoC and the SD card slot, to be able to switch between 1.8V and 3.3V, for UHS-1 operation. But I doubt that Anbernic would go this way, since this sounds more expensive, for little benefit.

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +4 -0
+
+&ohci0 {
+	status = "okay";
+};
Quoted 8 lines
Quoted 5 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 234

Quoted 5 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+
+&ehci0 {
+	status = "okay";
+};
+  

I haven't confirmed on my board we need ohci0 and ehci0 for the OTG port.

So we don't need them for pure peripheral mode, but for host mode they are needed. Technically the MUSB controller also supports host mode, but IIUC Allwinner doesn't use this setup, and recommends the proper HCIs for host mode.

Quoted 6 lines
Quoted 3 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 237

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+&r_rsb {
+   status = "okay";
+  

Any advantage to doing this on the rsb over i2c? That's how I have mine wired. Both are fine with me, I just didn't know what was better.

In general we prefer RSB. If nothing else: it's faster (3MHz instead of 100/400 KHz), which gives a slight advantage for fast DVFS switching. Also it's easier to program - each register write is a single fire-and-forget setup in the RSB registers, as opposed to much more driver hand-holding for I2C. That doesn't really matter for Linux (which supports both), but helps TF-A, for instance.

Quoted 13 lines
Quoted 13 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 245

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +8 -0
+   axp717: pmic@3a3 {
+		compatible = "x-powers,axp717";
+		reg = <0x3a3>;
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		interrupt-parent = <&nmi_intc>;
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+		x-powers,drive-vbus-en;  

Remove this last line, the AXP717 binding does not support this, and the Linux driver doesn't implement this, as the AXP717 does not seem to have this functionality.

Quoted 9 lines
Quoted 5 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 250

Quoted 5 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+
+		vin1-supply = <&reg_vcc5v>;
+		vin2-supply = <&reg_vcc5v>;
+		vin3-supply = <&reg_vcc5v>;
+		vin4-supply = <&reg_vcc5v>;  

FYI - I never actually confirmed the vin supply of the PMIC, I just put these in here to shut up some errors or warnings. If these are based on something I did it couldn't hurt to recheck.

I would assume the buck converters use the input power, since this is the least current limited of all available, and the outputs promise quite some current capability.
For the LDOs it might make sense to use a lower, closer matching input voltage, so it only has to drop from say 3.3V to 1.8V, but for the DCDCs using reg_vcc5v would make sense.

Though on second thought I wonder how this works when the device runs on battery. Need to check the manual on how it handles the battery switch, but I think for now it's safe to go with that above.

Quoted 11 lines
Quoted 7 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 257

Quoted 7 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +7 -0
+
+		regulators {
+			reg_dcdc1: dcdc1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <810000>;
+				regulator-max-microvolt = <1100000>;  

The CPU opp table in the BSP device tree had this between 900000 and 1120000. Though like most things, take anything in the BSP device tree with a grain of salt (lime and tequila help too).

You need lots of booze for anything BSP related ;-)

But indeed the lowest OPP voltage is 900mV, so it would make sense to go with that.

Quoted 7 lines
Quoted 3 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 260

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+				regulator-name = "vdd-cpu";
+			};
+  

I'm just speculating, but I strongly *guess* that this dcdc2 is used for the GPU. SoC datasheet says max should be 900000 for the GPU, but I don't have an opp table to go off of sadly.

It's probably used for both: on other H616 boards we call this regulator "vdd-gpu-sys". The datasheet describes both voltages with the same constraints, and since you need quite some oomph for both, and it's an odd voltage, it makes sense to use one DCDC for this.

Quoted 54 lines
Quoted 47 lines
Quoted 18 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +18 -0
+			reg_dcdc2: dcdc2 {
+				regulator-always-on;
+				regulator-min-microvolt = <940000>;
+				regulator-max-microvolt = <940000>;
+				regulator-name = "vdd-sys";
+			};
+
+			reg_dcdc3: dcdc3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1100000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-name = "vdd-dram";
+			};
+
+			reg_aldo1: aldo1 {
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 288

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+				regulator-name = "vcc-sd2";
+			};
+
+			reg_aldo2: aldo2 {
+				/* unused */
+			};
+
+			reg_aldo3: aldo3 {
+				regulator-always-on;
+				regulator-boot-on;  

Please remove this last line, it doesn't make sense in this context. Cf. Documentation//devicetree/bindings/regulator/regulator.yaml. I think the same applies to the other uses throughout this file.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 290

Quoted 2 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +2 -0
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;  

This is not right. What is the voltage of this rail? The kernel should tell you what was set in the register, through regulator_summary, or you check what's the voltage on a BSP system.

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 291

Quoted 1 line
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +1 -0
+				regulator-name = "axp717-aldo3";  

If the system dies when you remove always-on, you might have found some essential supply. Candidates for consumers are listed in the H616 or T5 series datasheet. Matching the voltage might give you an idea. Then use this consumer as the name.

Quoted 6 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 294

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+			};
+
+			reg_aldo4: aldo4 {  

Same for this one: Please remove regulator-boot-on, fix the voltage, and provide some rationale why this needs to be on.

Quoted 2 lines

aldo4 is a critical regulator that should run at 1.8v based on my testing.

Thanks, that makes sense, since 1.8V is needed quite a lot.

Quoted 83 lines
Quoted 40 lines
Quoted 7 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +7 -0
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;
+				regulator-name = "axp717-aldo4";
+			};
+

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 311

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+			reg_bldo1: bldo1 {
+				/* unused */
+			};
+
+			reg_bldo2: bldo2 {
+				regulator-always-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-name = "vcc-pll";
+			};  

This one is a good example: fixed voltage, no boot-on, and regulator name provides rationale why it must be always-on. The others should look similar.

Quoted 18 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +18 -0
+
+			reg_bldo3: bldo3 {
+				/* unused */
+			};
+
+			reg_bldo4: bldo4 {
+				/* unused */
+			};
+
+			reg_cldo1: cldo1 {
+				/* unused */
+			};
+
+			reg_cldo2: cldo2 {
+				/* unused */
+			};
+
+			reg_cldo3: cldo3 {
Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 334

Quoted 5 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;
+				regulator-name = "axp717-cldo3";  

Same here as ALDO3/4: what voltage is it, and what does it probably supply?

cldo3 is a critical regulator run at 3.3v based on my testing. It supplies power to the majority of the system's pins.

Quoted 3 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 336

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +2 -0
+			};
+  

cldo4 feeds the wifi, but through another fixed voltage regulator that seems to pull up or down to 3.3v if we happen to raise or lower cldo4 a bit above or below 3.3v.

Quoted 14 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 345

Quoted 9 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +9 -0
+			reg_cldo4: cldo4 {
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcc-wifi";
+			};
+
+			reg_boost: boost {
+				regulator-min-microvolt = <5126000>;
+				regulator-max-microvolt = <5126000>;  

It might be better to use a range, say 5.0V till 5.2V. The kernel will then just continue using the default, which seems to be this 5.126V (5440mV + 9 * 64mV).

Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 350

Quoted 5 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+				regulator-name = "boost";
+			};
+
+			reg_cpusldo: cpusldo {
+				/* unused */  

Is that so? I thought you mentioned on IRC this is required as well?

Quoted 2 lines

I still haven't completely finished the regulator analysis, but I actually think it is the case that this one isn't used.

Right, fair enough: the H616/H700 does not have a management processor, so the SoC probably does not need some kind of standby voltage.

Quoted 30 lines
Quoted 18 lines
Quoted 4 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+			};
+		};
+	};

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 363

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +10 -0
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_ph_pins>;
+	status = "okay";
+};
+
+&pio {
+	vcc-pg-supply = <&reg_pll_dcx0>;  

As mentioned above, it seems unlikely to be a fixed regulator. If in doubt, leave them out, they are not essential for the operation.

On my 35XXH I can confirm PG is supplied by reg_aldo4 at 1.8v and the rest are supplied by reg_cldo3 at 3.3v.

        vcc-pa-supply = <&reg_cldo3>;
        vcc-pc-supply = <&reg_cldo3>;
        vcc-pe-supply = <&reg_cldo3>;
        vcc-pf-supply = <&reg_cldo3>;
        vcc-pg-supply = <&reg_aldo4>;
        vcc-ph-supply = <&reg_cldo3>;
        vcc-pi-supply = <&reg_cldo3>;
Quoted 5 lines

This is what my 35XXH looks like after manually raising or lowering the PMIC voltage values a tad, observing the GPIO out voltages, adjusting again, measuring the voltages again, etc etc. By checking at least 1 pin from each bank and confirming voltage changes via PMIC changes I get this mapping.

Ah, many thanks for doing this - and sounds also fun, actually ;-)

Quoted 18 lines
Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 366

Quoted 3 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +3 -0
+};
+
+/* the AXP717 has USB type-C role switch functionality, to be implemented */  

Replace "to be implemented" with "not yet described by the binding". This is DT land, so we don't care about implementations or the Linux kernel, it's all about DTs and DT bindings.

Quoted 9 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 368

Quoted 2 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +2 -0
+&usbotg {
+	dr_mode = "host";   /* USB type-C receptable */  

So does this really work? It seems wrong to make this unconditional, given this is the only way to charge the device. When power is supplied through the USB-C port, surely driving VBUS from the board sounds wrong. Unless you have a killer feature for a host port, I think without working role switching, "peripheral" would be the safer choice.

Quoted 7 lines

Again making me wish I'd have used a different device for my mainline efforts, but on the 35XXH the OTG port I know does work for me as a peripherial port. As for the Type-C stuff, I don't expect any future AXP717 modifications to matter for this device, as manually fiddling the USB-C bits in the PMIC didn't register my otg port as anything other than a standard USB port (none of the USB-C specific registers seemed to register anything).

Mmmh, interesting, though this might mean we don't understand this very well yet. Because it looks like easy on the board side: just connect the CC pins on the AXP to the CC pins on the connector, and the rest falls in place?

Cheers,
Andre

Quoted 27 lines
Quoted 9 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 373

Quoted 5 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +5 -0
+	status = "okay";
+};
+
+&usbphy {
+	usb0_vbus-supply = <&reg_vcc_usb>;  

When you stick to "peripheral" above, you should drop this line for now, especially since this regulator chain looks quite suspicious still.

Concur, especially since now when I look at my 35XX+ 2024 model I only see an OTG port and not a host port.

Quoted 6 lines

Cheers,
Andre

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts line 375

Quoted 2 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +2 -0
+	status = "okay";
+};  

Thank you for all your hard work on this series. I'm going to continue to try and identify the remaining regulators on my board and what they do/how they're used. I expect at least one or two of the ones we've flagged for removal will need to be added back once we get the panel code working.

Chris

maintainer
#12 lore raw

Re: [PATCH v2 2/5] arm64: dts: allwinner: h700: Add RG35XX 2024 DTS

In reply to Ryan Walklin

Dne sobota, 20. april 2024 ob 12:43:56 GMT +2 je Ryan Walklin napisal(a):

Quoted 35 lines

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

Changelog v1..v2:
- Update copyright
- Spaces -> Tabs
- Add cpufreq support [1]
- Remove MMC aliases
- Fix GPIO button and regulator node names
- Note unused AXP717 regulators
- Update regulator for SD slots
- Remove unused I2C3 device
- Update NMI interrupt controller for AXP717 [2]
- Add chassis-type
- Address USB EHCI/OHCI0 correctly and add usb vbus supply
- Add PIO vcc-pg-supply
- Correct boost regulator voltage and name

Quoted 4 lines

[1]: https://lore.kernel.org/linux-sunxi/20240419071450.7aiheeoyq35yjoo7@vireshk-i7/T/#t
[2]: https://lore.kernel.org/linux-sunxi/[email protected]/T/#t

Put changelog and links in this case below --- line, so it won't be part of the commit.

Best regards,
Jernej

Quoted 389 lines
---
 .../sun50i-h700-anbernic-rg35xx-2024.dts      | 375 ++++++++++++++++++
 1 file changed, 375 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts new file +375 -0
new file mode 100644
index 000000000000..d8081273677d
@@ -0,0 +1,375 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) 2024 Ryan Walklin <[email protected]>.
+ */
+
+/dts-v1/;
+
+#include "sun50i-h616.dtsi"
+#include "sun50i-h616-cpu-opp.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";
+	chassis-type = "handset";
+	compatible = "anbernic,rg35xx-2024", "allwinner,sun50i-h700";
+
+	aliases {
+		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";
+
+	   button-up {
+		   label = "D-Pad Up";
+		   gpios = <&pio 0 6 GPIO_ACTIVE_LOW>; /* PA6 */
+		   linux,input-type = <EV_KEY>;
+		   linux,code = <BTN_DPAD_UP>;
+		};
+
+		button-down {
+			label = "D-Pad Down";
+			gpios = <&pio 4 0 GPIO_ACTIVE_LOW>; /* PE0 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_DOWN>;
+		};
+
+		button-left {
+			label = "D-Pad left";
+			gpios = <&pio 0 8 GPIO_ACTIVE_LOW>; /* PA8 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_LEFT>;
+		};
+
+		button-right {
+			label = "D-Pad Right";
+			gpios = <&pio 0 9 GPIO_ACTIVE_LOW>; /* PA9 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_DPAD_RIGHT>;
+		};
+
+		button-a {
+			label = "Action-Pad A";
+			gpios = <&pio 0 0 GPIO_ACTIVE_LOW>; /* PA0 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_EAST>;
+		};
+
+		button-b {
+			label = "Action-Pad B";
+			gpios = <&pio 0 1 GPIO_ACTIVE_LOW>; /* PA1 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_SOUTH>;
+		};
+
+		button-x {
+			label = "Action-Pad X";
+			gpios = <&pio 0 3 GPIO_ACTIVE_LOW>; /* PA3 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_NORTH>;
+		};
+
+		button-y {
+			label = "Action Pad Y";
+			gpios = <&pio 0 2 GPIO_ACTIVE_LOW>; /* PA2 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_WEST>;
+		};
+
+		button-start {
+			label = "Key Start";
+			gpios = <&pio 0 4 GPIO_ACTIVE_LOW>; /* PA4 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_START>;
+		};
+
+		button-select {
+			label = "Key Select";
+			gpios = <&pio 0 5 GPIO_ACTIVE_LOW>; /* PA5 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_SELECT>;
+		};
+
+		button-l1 {
+			label = "Key L1";
+			gpios = <&pio 0 10 GPIO_ACTIVE_LOW>; /* PA10 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TL>;
+		};
+
+		button-l2 {
+			label = "Key L2";
+			gpios = <&pio 0 11 GPIO_ACTIVE_LOW>; /* PA11 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TL2>;
+		};
+
+		button-r1 {
+			label = "Key R1";
+			gpios = <&pio 0 12 GPIO_ACTIVE_LOW>; /* PA12 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TR>;
+		};
+
+		button-r2 {
+			label = "Key R2";
+			gpios = <&pio 0 7 GPIO_ACTIVE_LOW>; /* PA7 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_TR2>;
+		};
+
+		button-menu {
+			label = "Key Menu";
+			gpios = <&pio 4 3 GPIO_ACTIVE_LOW>; /* PE3 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_MODE>;
+		};
+
+		button-vol-up {
+			label = "Key Volume Up";
+			gpios = <&pio 4 1 GPIO_ACTIVE_LOW>; /* PE1 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <KEY_VOLUMEUP>;
+		};
+
+		button-vol-down {
+			label = "Key Volume Down";
+			gpios = <&pio 4 2 GPIO_ACTIVE_LOW>; /* PE2 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <KEY_VOLUMEDOWN>;
+		};
+	};
+
+	reg_vcc_sd2: regulator-vcc3v3 {
+		compatible = "regulator-fixed";
+		gpio = <&pio 4 4 GPIO_ACTIVE_HIGH>; /* PE4 */
+		regulator-name = "vcc_3v3_sd2";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+	};
+
+	reg_vcc_usb: regulator-vcc-5v0-usb {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&pio 8 7 GPIO_ACTIVE_HIGH>; /* PI7 */
+		regulator-name = "vcc_5v0_usb";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vcc_3v3_usb>;
+	};
+
+	vcc_3v3_usb: vcc-3v3-usb {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&pio 4 5 GPIO_ACTIVE_HIGH>; /* PE5 */
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-name = "vcc_3v3_usb";
+		regulator-max-microvolt = <3300000>;
+		regulator-min-microvolt = <3300000>;
+	};
+
+	reg_vcc5v: regulator-vcc5v { /* USB-C power input */
+		compatible = "regulator-fixed";
+		regulator-name = "vcc-5v";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+	};
+
+	reg_pll_dcx0: regulator-pll-dcx0 {
+		compatible = "regulator-fixed";
+		regulator-always-on;
+		regulator-name = "vcc-pll";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+	};
+};
+
+&cpu0 {
+	cpu-supply = <&reg_dcdc1>;
+};
+
+&mmc0 {
+	vmmc-supply = <&reg_vcc_sd2>;
+	disable-wp;
+	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>;  /* PF6 */
+	bus-width = <4>;
+	status = "okay";
+};
+
+&mmc2 {
+	vmmc-supply = <&reg_vcc_sd2>;
+	vqmmc-supply = <&reg_aldo1>;
+	cd-gpios = <&pio 4 22 GPIO_ACTIVE_LOW>; /* PE22 */
+	bus-width = <4>;
+	status = "okay";
+};
+
+&ohci0 {
+	status = "okay";
+};
+
+&ehci0 {
+	status = "okay";
+};
+
+&r_rsb {
+   status = "okay";
+
+   axp717: pmic@3a3 {
+		compatible = "x-powers,axp717";
+		reg = <0x3a3>;
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		interrupt-parent = <&nmi_intc>;
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+		x-powers,drive-vbus-en;
+
+		vin1-supply = <&reg_vcc5v>;
+		vin2-supply = <&reg_vcc5v>;
+		vin3-supply = <&reg_vcc5v>;
+		vin4-supply = <&reg_vcc5v>;
+
+		regulators {
+			reg_dcdc1: dcdc1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <810000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-name = "vdd-cpu";
+			};
+
+			reg_dcdc2: dcdc2 {
+				regulator-always-on;
+				regulator-min-microvolt = <940000>;
+				regulator-max-microvolt = <940000>;
+				regulator-name = "vdd-sys";
+			};
+
+			reg_dcdc3: dcdc3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1100000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-name = "vdd-dram";
+			};
+
+			reg_aldo1: aldo1 {
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcc-sd2";
+			};
+
+			reg_aldo2: aldo2 {
+				/* unused */
+			};
+
+			reg_aldo3: aldo3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;
+				regulator-name = "axp717-aldo3";
+			};
+
+			reg_aldo4: aldo4 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;
+				regulator-name = "axp717-aldo4";
+			};
+
+			reg_bldo1: bldo1 {
+				/* unused */
+			};
+
+			reg_bldo2: bldo2 {
+				regulator-always-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-name = "vcc-pll";
+			};
+
+			reg_bldo3: bldo3 {
+				/* unused */
+			};
+
+			reg_bldo4: bldo4 {
+				/* unused */
+			};
+
+			reg_cldo1: cldo1 {
+				/* unused */
+			};
+
+			reg_cldo2: cldo2 {
+				/* unused */
+			};
+
+			reg_cldo3: cldo3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <3500000>;
+				regulator-name = "axp717-cldo3";
+			};
+
+			reg_cldo4: cldo4 {
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcc-wifi";
+			};
+
+			reg_boost: boost {
+				regulator-min-microvolt = <5126000>;
+				regulator-max-microvolt = <5126000>;
+				regulator-name = "boost";
+			};
+
+			reg_cpusldo: cpusldo {
+				/* unused */
+			};
+		};
+	};
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_ph_pins>;
+	status = "okay";
+};
+
+&pio {
+	vcc-pg-supply = <&reg_pll_dcx0>;
+};
+
+/* the AXP717 has USB type-C role switch functionality, to be implemented */
+&usbotg {
+	dr_mode = "host";   /* USB type-C receptable */
+	status = "okay";
+};
+
+&usbphy {
+	usb0_vbus-supply = <&reg_vcc_usb>;
+	status = "okay";
+};
#13 lore raw

PATCH v2 3/5 arm64: dts: allwinner: h700: Add RG35XX-Plus DTS

In reply to Ryan Walklin

The RG35XX-Plus adds a RTL8221CS SDIO Wifi/BT chip to the RG35XX (2024).

Enabled in this DTS:
- WiFi
- Bluetooth
- Supporting power sequence and GPIOs

Changelog v1..v2:
- Update copyright
- Spaces -> Tabs
- Remove redundant &uart0 node and DTS version tag
- Add GPIO bank comments
- Use generic pwrseq name

---
 .../sun50i-h700-anbernic-rg35xx-plus.dts      | 53 +++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-plus.dts
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-plus.dts new file +53 -0
new file mode 100644
index 000000000000..67ba1c7bb3ca
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) 2024 Ryan Walklin <[email protected]>.
+ */
+
+#include "sun50i-h700-anbernic-rg35xx-2024.dts"
+
+/ {
+	model = "Anbernic RG35XX Plus";
+	compatible = "anbernic,rg35xx-plus", "allwinner,sun50i-h700";
+
+	wifi_pwrseq: pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		clocks = <&rtc CLK_OSC32K_FANOUT>;
+		clock-names = "ext_clock";
+		pinctrl-0 = <&x32clk_fanout_pin>;
+		pinctrl-names = "default";
+		post-power-on-delay-ms = <200>;
+		reset-gpios = <&pio 6 18 GPIO_ACTIVE_LOW>; /* PG18 */
+	};
+};
+
+/* SDIO WiFi RTL8821CS */
+&mmc1 {
+	vmmc-supply = <&reg_cldo4>;
+	vqmmc-supply = <&reg_pll_dcx0>;
+	mmc-pwrseq = <&wifi_pwrseq>;
+	bus-width = <4>;
+	non-removable;
+	status = "okay";
+
+	sdio_wifi: wifi@1 {
+	   reg = <1>;
+	   interrupt-parent = <&pio>;
+	   interrupts = <6 15 IRQ_TYPE_LEVEL_LOW>; /* PG15 */
+	   interrupt-names = "host-wake";
+	};
+};
+
+/* Bluetooth RTL8821CS */
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
+	uart-has-rtscts;
+	status = "okay";
+
+	bluetooth {
+		compatible = "realtek,rtl8821cs-bt", "realtek,rtl8723bs-bt";
+		device-wake-gpios = <&pio 6 17 GPIO_ACTIVE_HIGH>; /* PG17 */
+		enable-gpios = <&pio 6 19 GPIO_ACTIVE_HIGH>; /* PG19 */
+		host-wake-gpios = <&pio 6 16 GPIO_ACTIVE_HIGH>; /* PG16 */
+	};
+};
Signature · 1 line
2.44.0
#14 lore raw

Re: [PATCH v2 3/5] arm64: dts: allwinner: h700: Add RG35XX-Plus DTS

On Sat, 20 Apr 2024 22:43:57 +1200
Ryan Walklin <[email protected]> wrote:

Hi,

Quoted 42 lines

The RG35XX-Plus adds a RTL8221CS SDIO Wifi/BT chip to the RG35XX (2024).

Enabled in this DTS:
- WiFi
- Bluetooth
- Supporting power sequence and GPIOs

Changelog v1..v2:
- Update copyright
- Spaces -> Tabs
- Remove redundant &uart0 node and DTS version tag
- Add GPIO bank comments
- Use generic pwrseq name

---
 .../sun50i-h700-anbernic-rg35xx-plus.dts      | 53 +++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-plus.dts
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-plus.dts new file +16 -0
new file mode 100644
index 000000000000..67ba1c7bb3ca
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) 2024 Ryan Walklin <[email protected]>.
+ */
+
+#include "sun50i-h700-anbernic-rg35xx-2024.dts"
+
+/ {
+	model = "Anbernic RG35XX Plus";
+	compatible = "anbernic,rg35xx-plus", "allwinner,sun50i-h700";
+
+	wifi_pwrseq: pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		clocks = <&rtc CLK_OSC32K_FANOUT>;
+		clock-names = "ext_clock";
+		pinctrl-0 = <&x32clk_fanout_pin>;

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-plus.dts line 26

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-plus.dts new file +10 -0
+		pinctrl-names = "default";
+		post-power-on-delay-ms = <200>;
+		reset-gpios = <&pio 6 18 GPIO_ACTIVE_LOW>; /* PG18 */
+	};
+};
+
+/* SDIO WiFi RTL8821CS */
+&mmc1 {
+	vmmc-supply = <&reg_cldo4>;
+	vqmmc-supply = <&reg_pll_dcx0>;

It would be great to confirm what the I/O voltage for MMC1/WiFi really is, 1.8V or 3.3V? Is someone with a board able to measure this?

The rest looks good to me, thanks for the changes.

Cheers,
Andre

Quoted 27 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-plus.dts new file +27 -0
+	mmc-pwrseq = <&wifi_pwrseq>;
+	bus-width = <4>;
+	non-removable;
+	status = "okay";
+
+	sdio_wifi: wifi@1 {
+	   reg = <1>;
+	   interrupt-parent = <&pio>;
+	   interrupts = <6 15 IRQ_TYPE_LEVEL_LOW>; /* PG15 */
+	   interrupt-names = "host-wake";
+	};
+};
+
+/* Bluetooth RTL8821CS */
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
+	uart-has-rtscts;
+	status = "okay";
+
+	bluetooth {
+		compatible = "realtek,rtl8821cs-bt", "realtek,rtl8723bs-bt";
+		device-wake-gpios = <&pio 6 17 GPIO_ACTIVE_HIGH>; /* PG17 */
+		enable-gpios = <&pio 6 19 GPIO_ACTIVE_HIGH>; /* PG19 */
+		host-wake-gpios = <&pio 6 16 GPIO_ACTIVE_HIGH>; /* PG16 */
+	};
+};
#15 lore raw

Re: [PATCH v2 3/5] arm64: dts: allwinner: h700: Add RG35XX-Plus DTS

Quoted 61 lines · Andre Przywara

On Sat, 20 Apr 2024 22:43:57 +1200
Ryan Walklin <[email protected]> wrote:

Hi,

Quoted 42 lines

The RG35XX-Plus adds a RTL8221CS SDIO Wifi/BT chip to the RG35XX (2024).

Enabled in this DTS:
- WiFi
- Bluetooth
- Supporting power sequence and GPIOs

Changelog v1..v2:
- Update copyright
- Spaces -> Tabs
- Remove redundant &uart0 node and DTS version tag
- Add GPIO bank comments
- Use generic pwrseq name

---
 .../sun50i-h700-anbernic-rg35xx-plus.dts      | 53 +++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-plus.dts
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-plus.dts new file +16 -0
new file mode 100644
index 000000000000..67ba1c7bb3ca
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) 2024 Ryan Walklin <[email protected]>.
+ */
+
+#include "sun50i-h700-anbernic-rg35xx-2024.dts"
+
+/ {
+	model = "Anbernic RG35XX Plus";
+	compatible = "anbernic,rg35xx-plus", "allwinner,sun50i-h700";
+
+	wifi_pwrseq: pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		clocks = <&rtc CLK_OSC32K_FANOUT>;
+		clock-names = "ext_clock";
+		pinctrl-0 = <&x32clk_fanout_pin>;

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-plus.dts line 26

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-plus.dts new file +10 -0
+		pinctrl-names = "default";
+		post-power-on-delay-ms = <200>;
+		reset-gpios = <&pio 6 18 GPIO_ACTIVE_LOW>; /* PG18 */
+	};
+};
+
+/* SDIO WiFi RTL8821CS */
+&mmc1 {
+	vmmc-supply = <&reg_cldo4>;
+	vqmmc-supply = <&reg_pll_dcx0>;

It would be great to confirm what the I/O voltage for MMC1/WiFi really is, 1.8V or 3.3V? Is someone with a board able to measure this?

Quoted 1 line

The rest looks good to me, thanks for the changes.

I have measured this on my 35XXH and can confirm both the mmc1 IO voltage and the uart1 IO voltage is 1.8v. It's supplied by aldo4.

Thank you.
Chris

Quoted 32 lines

Cheers,
Andre

Quoted 27 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-plus.dts new file +27 -0
+	mmc-pwrseq = <&wifi_pwrseq>;
+	bus-width = <4>;
+	non-removable;
+	status = "okay";
+
+	sdio_wifi: wifi@1 {
+	   reg = <1>;
+	   interrupt-parent = <&pio>;
+	   interrupts = <6 15 IRQ_TYPE_LEVEL_LOW>; /* PG15 */
+	   interrupt-names = "host-wake";
+	};
+};
+
+/* Bluetooth RTL8821CS */
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
+	uart-has-rtscts;
+	status = "okay";
+
+	bluetooth {
+		compatible = "realtek,rtl8821cs-bt", "realtek,rtl8723bs-bt";
+		device-wake-gpios = <&pio 6 17 GPIO_ACTIVE_HIGH>; /* PG17 */
+		enable-gpios = <&pio 6 19 GPIO_ACTIVE_HIGH>; /* PG19 */
+		host-wake-gpios = <&pio 6 16 GPIO_ACTIVE_HIGH>; /* PG16 */
+	};
+};
#16 lore raw

PATCH v2 4/5 arm64: dts: allwinner: h700: Add RG35XX-H DTS

In reply to Ryan Walklin

The RG35XX-H adds thumbsticks, a stereo speaker, and a second USB port to the RG35XX-Plus, and has a horizontal form factor.

Enabled in this DTS:
- Thumbsticks
- Second USB port

Changelog v1..v2:
- Update copyright
- Spaces -> Tabs
- Add GP ADC joystick axes and mux [1]
- Add EHCI/OHCI1 for second USB port and add vbus supply

[1]: https://lore.kernel.org/linux-sunxi/[email protected]/T/#t

---
 .../sun50i-h700-anbernic-rg35xx-h.dts         | 126 ++++++++++++++++++
 1 file changed, 126 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +126 -0
new file mode 100644
index 000000000000..d62cf5cd9d9b
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) 2024 Ryan Walklin <[email protected]>.
+ * Copyright (C) 2024 Chris Morgan <[email protected]>.
+ */
+
+#include "sun50i-h700-anbernic-rg35xx-plus.dts"
+
+/ {
+	model = "Anbernic RG35XX H";
+	compatible = "anbernic,rg35xx-h", "allwinner,sun50i-h700";
+
+	adc-joystick {
+		compatible = "adc-joystick";
+		io-channels = <&adc_mux 0>,
+				  <&adc_mux 1>,
+				  <&adc_mux 2>,
+				  <&adc_mux 3>;
+		pinctrl-0 = <&joy_mux_pin>;
+		pinctrl-names = "default";
+		poll-interval = <60>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		axis@0 {
+			reg = <0>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <4096 0>;
+			linux,code = <ABS_X>;
+		};
+
+		axis@1 {
+			reg = <1>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <0 4096>;
+			linux,code = <ABS_Y>;
+		};
+
+		axis@2 {
+			reg = <2>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <0 4096>;
+			linux,code = <ABS_RX>;
+		};
+
+		axis@3 {
+			reg = <3>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <4096 0>;
+			linux,code = <ABS_RY>;
+		};
+	};
+
+	adc_mux: adc-mux {
+		compatible = "io-channel-mux";
+		channels = "left_x", "left_y", "right_x", "right_y";
+		#io-channel-cells = <1>;
+		io-channels = <&gpadc 0>;
+		io-channel-names = "parent";
+		mux-controls = <&gpio_mux>;
+		settle-time-us = <100>;
+	};
+
+	gpio_keys: gpio-keys-thumb {
+		compatible = "gpio-keys";
+
+		button-thumbl {
+			label = "GPIO Thumb Left";
+			gpios = <&pio 4 8 GPIO_ACTIVE_LOW>; /* PE8 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_THUMBL>;
+		};
+
+		button-thumbr {
+			label = "GPIO Thumb Right";
+			gpios = <&pio 4 9 GPIO_ACTIVE_LOW>; /* PE9 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_THUMBR>;
+		};
+	};
+
+	gpio_mux: mux-controller {
+		compatible = "gpio-mux";
+		mux-gpios = <&pio 8 1 GPIO_ACTIVE_LOW>,
+				<&pio 8 2 GPIO_ACTIVE_LOW>; /* PI1, PI2 */
+		#mux-control-cells = <0>;
+	};
+};
+
+&gpadc {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	status = "okay";
+
+	channel@0 {
+		reg = <0>;
+	};
+};
+
+&pio {
+	joy_mux_pin: joy-mux-pin {
+		pins = "PI0";
+		function = "gpio_out";
+	};
+};
+
+&ehci1 {
+	status = "okay";
+};
+
+&ohci1 {
+	status = "okay";
+};
+
+&usbotg {
+	dr_mode = "peripheral";
+	status = "okay";
+};
+
+&usbphy {
+	usb1_vbus-supply = <&reg_vcc_usb>;
+};
Signature · 1 line
2.44.0
#17 lore raw

Re: [PATCH v2 4/5] arm64: dts: allwinner: h700: Add RG35XX-H DTS

On Sat, 20 Apr 2024 22:43:58 +1200
Ryan Walklin <[email protected]> wrote:

Hi,

Quoted 12 lines

The RG35XX-H adds thumbsticks, a stereo speaker, and a second USB port to the RG35XX-Plus, and has a horizontal form factor.

Enabled in this DTS:
- Thumbsticks
- Second USB port

Changelog v1..v2:
- Update copyright
- Spaces -> Tabs
- Add GP ADC joystick axes and mux [1]
- Add EHCI/OHCI1 for second USB port and add vbus supply

Quoted 3 lines

[1]: https://lore.kernel.org/linux-sunxi/[email protected]/T/#t

As mention on patch 2/5, this might be better an optional dependency, so the GPADC part might be a separate patch.

Quoted 71 lines
---
 .../sun50i-h700-anbernic-rg35xx-h.dts         | 126 ++++++++++++++++++
 1 file changed, 126 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +58 -0
new file mode 100644
index 000000000000..d62cf5cd9d9b
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) 2024 Ryan Walklin <[email protected]>.
+ * Copyright (C) 2024 Chris Morgan <[email protected]>.
+ */
+
+#include "sun50i-h700-anbernic-rg35xx-plus.dts"
+
+/ {
+	model = "Anbernic RG35XX H";
+	compatible = "anbernic,rg35xx-h", "allwinner,sun50i-h700";
+
+	adc-joystick {
+		compatible = "adc-joystick";
+		io-channels = <&adc_mux 0>,
+				  <&adc_mux 1>,
+				  <&adc_mux 2>,
+				  <&adc_mux 3>;
+		pinctrl-0 = <&joy_mux_pin>;
+		pinctrl-names = "default";
+		poll-interval = <60>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		axis@0 {
+			reg = <0>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <4096 0>;
+			linux,code = <ABS_X>;
+		};
+
+		axis@1 {
+			reg = <1>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <0 4096>;
+			linux,code = <ABS_Y>;
+		};
+
+		axis@2 {
+			reg = <2>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <0 4096>;
+			linux,code = <ABS_RX>;
+		};
+
+		axis@3 {
+			reg = <3>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <4096 0>;
+			linux,code = <ABS_RY>;
+		};
+	};
+
+	adc_mux: adc-mux {

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts line 68

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +10 -0
+		compatible = "io-channel-mux";
+		channels = "left_x", "left_y", "right_x", "right_y";
+		#io-channel-cells = <1>;
+		io-channels = <&gpadc 0>;
+		io-channel-names = "parent";
+		mux-controls = <&gpio_mux>;
+		settle-time-us = <100>;
+	};
+
+	gpio_keys: gpio-keys-thumb {

Is there any reason to not just use the existing gpio-keys node? Either put a label on it in patch 2/5, and reference that below, outside of the root node, or use an absolute path reference.

Quoted 47 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +47 -0
+		compatible = "gpio-keys";
+
+		button-thumbl {
+			label = "GPIO Thumb Left";
+			gpios = <&pio 4 8 GPIO_ACTIVE_LOW>; /* PE8 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_THUMBL>;
+		};
+
+		button-thumbr {
+			label = "GPIO Thumb Right";
+			gpios = <&pio 4 9 GPIO_ACTIVE_LOW>; /* PE9 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_THUMBR>;
+		};
+	};
+
+	gpio_mux: mux-controller {
+		compatible = "gpio-mux";
+		mux-gpios = <&pio 8 1 GPIO_ACTIVE_LOW>,
+				<&pio 8 2 GPIO_ACTIVE_LOW>; /* PI1, PI2 */
+		#mux-control-cells = <0>;
+	};
+};
+
+&gpadc {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	status = "okay";
+
+	channel@0 {
+		reg = <0>;
+	};
+};
+
+&pio {
+	joy_mux_pin: joy-mux-pin {
+		pins = "PI0";
+		function = "gpio_out";
+	};
+};
+
+&ehci1 {
+	status = "okay";
+};
+
+&ohci1 {

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts line 125

Quoted 10 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +10 -0
+	status = "okay";
+};
+
+&usbotg {
+	dr_mode = "peripheral";
+	status = "okay";
+};
+
+&usbphy {
+	usb1_vbus-supply = <&reg_vcc_usb>;

This is the dodgy USB supply chain. Any chance we can narrow this down, to maybe one GPIO controlled regulator? Also, where does the boost controller come into play here?

Cheers,
Andre

Quoted 1 line
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +1 -0
+};
#18 lore raw

Re: [PATCH v2 4/5] arm64: dts: allwinner: h700: Add RG35XX-H DTS

Quoted 101 lines · Andre Przywara

On Sat, 20 Apr 2024 22:43:58 +1200
Ryan Walklin <[email protected]> wrote:

Hi,

Quoted 12 lines

The RG35XX-H adds thumbsticks, a stereo speaker, and a second USB port to the RG35XX-Plus, and has a horizontal form factor.

Enabled in this DTS:
- Thumbsticks
- Second USB port

Changelog v1..v2:
- Update copyright
- Spaces -> Tabs
- Add GP ADC joystick axes and mux [1]
- Add EHCI/OHCI1 for second USB port and add vbus supply

Quoted 3 lines

[1]: https://lore.kernel.org/linux-sunxi/[email protected]/T/#t

As mention on patch 2/5, this might be better an optional dependency, so the GPADC part might be a separate patch.

Quoted 77 lines
---
 .../sun50i-h700-anbernic-rg35xx-h.dts         | 126 ++++++++++++++++++
 1 file changed, 126 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +64 -0
new file mode 100644
index 000000000000..d62cf5cd9d9b
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) 2024 Ryan Walklin <[email protected]>.
+ * Copyright (C) 2024 Chris Morgan <[email protected]>.
+ */
+
+#include "sun50i-h700-anbernic-rg35xx-plus.dts"
+
+/ {
+	model = "Anbernic RG35XX H";
+	compatible = "anbernic,rg35xx-h", "allwinner,sun50i-h700";
+
+	adc-joystick {
+		compatible = "adc-joystick";
+		io-channels = <&adc_mux 0>,
+				  <&adc_mux 1>,
+				  <&adc_mux 2>,
+				  <&adc_mux 3>;
+		pinctrl-0 = <&joy_mux_pin>;
+		pinctrl-names = "default";
+		poll-interval = <60>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		axis@0 {
+			reg = <0>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <4096 0>;
+			linux,code = <ABS_X>;
+		};
+
+		axis@1 {
+			reg = <1>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <0 4096>;
+			linux,code = <ABS_Y>;
+		};
+
+		axis@2 {
+			reg = <2>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <0 4096>;
+			linux,code = <ABS_RX>;
+		};
+
+		axis@3 {
+			reg = <3>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <4096 0>;
+			linux,code = <ABS_RY>;
+		};
+	};
+
+	adc_mux: adc-mux {
+		compatible = "io-channel-mux";
+		channels = "left_x", "left_y", "right_x", "right_y";
+		#io-channel-cells = <1>;
+		io-channels = <&gpadc 0>;
+		io-channel-names = "parent";
+		mux-controls = <&gpio_mux>;
Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts line 68

Quoted 4 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +4 -0
+		settle-time-us = <100>;
+	};
+
+	gpio_keys: gpio-keys-thumb {

Is there any reason to not just use the existing gpio-keys node? Either put a label on it in patch 2/5, and reference that below, outside of the root node, or use an absolute path reference.

I would also second just putting an alias and adding these to it. I myself as a preference tend to set the GPIO volume buttons as a seperate node only so I can enable key repeat on them, otherwise one node is best.

Quoted 29 lines
Quoted 29 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +28 -0
+		compatible = "gpio-keys";
+
+		button-thumbl {
+			label = "GPIO Thumb Left";
+			gpios = <&pio 4 8 GPIO_ACTIVE_LOW>; /* PE8 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_THUMBL>;
+		};
+
+		button-thumbr {
+			label = "GPIO Thumb Right";
+			gpios = <&pio 4 9 GPIO_ACTIVE_LOW>; /* PE9 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_THUMBR>;
+		};
+	};
+
+	gpio_mux: mux-controller {
+		compatible = "gpio-mux";
+		mux-gpios = <&pio 8 1 GPIO_ACTIVE_LOW>,
+				<&pio 8 2 GPIO_ACTIVE_LOW>; /* PI1, PI2 */
+		#mux-control-cells = <0>;
+	};
+};
+
+&gpadc {
+	#address-cells = <1>;
+	#size-cells = <0>;
Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts line 104

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +8 -0
+	status = "okay";
+
+	channel@0 {
+		reg = <0>;
+	};
+};
+
+&pio {

After extensive testing with a multimeter and fudging the regulator voltages up or down, I've been able to figure out the regulator assignments for each of the different power domains. Schematics would have helped, but sadly this had to be done the hard way. Based on past experience with Anbernic I would strongly suspect all devices have this assignment, but I know for sure my 35XXH does.

        vcc-pa-supply = <&reg_cldo3>;
        vcc-pc-supply = <&reg_cldo3>;
        vcc-pe-supply = <&reg_cldo3>;
        vcc-pf-supply = <&reg_cldo3>;
        vcc-pg-supply = <&reg_aldo4>;
        vcc-ph-supply = <&reg_cldo3>;
        vcc-pi-supply = <&reg_cldo3>;

On my board the reg_cldo3 is a constant 3.3v output, and the reg_aldo4 is a constant 1.8v output.

Quoted 17 lines
Quoted 17 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +17 -0
+	joy_mux_pin: joy-mux-pin {
+		pins = "PI0";
+		function = "gpio_out";
+	};
+};
+
+&ehci1 {
+	status = "okay";
+};
+
+&ohci1 {
+	status = "okay";
+};
+
+&usbotg {
+	dr_mode = "peripheral";
+	status = "okay";
Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts line 125

Quoted 4 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +4 -0
+};
+
+&usbphy {
+	usb1_vbus-supply = <&reg_vcc_usb>;

This is the dodgy USB supply chain. Any chance we can narrow this down, to maybe one GPIO controlled regulator? Also, where does the boost controller come into play here?

I haven't figured out the boost regulator yet, but for the host port I've been able to ascertain there's no less than 2 GPIO controlled regulators in play. PE5 must be driven high or the USB host port will not power on at all. If PE5 is driven high then the port kicks on, but at 3.3v. Once I also enable PI7 the port then reaches 4.6v. I'm not sure how to get it to a proper 5v yet, I'm still working that part out.

Maybe PE5 is a reset of some kind that's active low, I don't know. I just know so far/for sure that if PE5 is low then nothing registers on the USB host port; if PE5 is high but PI7 is low the port sort-of works at 3.3v, and if both PE5 and PI7 are high the port works consistently and at 4.6v.

Quoted 6 lines

Cheers,
Andre

Quoted 1 line
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +1 -0
+};

Thank you,
Chris

#19 lore raw

Re: [PATCH v2 4/5] arm64: dts: allwinner: h700: Add RG35XX-H DTS

On Sun, 21 Apr 2024, at 3:09 PM, Chris Morgan wrote:

Thanks for the review and all the work on the regulators!

Quoted 10 lines
Quoted 4 lines

Is there any reason to not just use the existing gpio-keys node? Either put a label on it in patch 2/5, and reference that below, outside of the root node, or use an absolute path reference.

I would also second just putting an alias and adding these to it. I myself as a preference tend to set the GPIO volume buttons as a seperate node only so I can enable key repeat on them, otherwise one node is best.

Thanks, have split the volume keys out and merged the thumbsticks into the existing node.

Quoted 16 lines

After extensive testing with a multimeter and fudging the regulator voltages up or down, I've been able to figure out the regulator assignments for each of the different power domains. Schematics would have helped, but sadly this had to be done the hard way. Based on past experience with Anbernic I would strongly suspect all devices have this assignment, but I know for sure my 35XXH does.

        vcc-pa-supply = <&reg_cldo3>;
        vcc-pc-supply = <&reg_cldo3>;
        vcc-pe-supply = <&reg_cldo3>;
        vcc-pf-supply = <&reg_cldo3>;
        vcc-pg-supply = <&reg_aldo4>;
        vcc-ph-supply = <&reg_cldo3>;
        vcc-pi-supply = <&reg_cldo3>;
Quoted 3 lines

On my board the reg_cldo3 is a constant 3.3v output, and the reg_aldo4 is a constant 1.8v output.

Nice work! These work well in my testing.

Quoted 8 lines

I haven't figured out the boost regulator yet, but for the host port I've been able to ascertain there's no less than 2 GPIO controlled regulators in play. PE5 must be driven high or the USB host port will not power on at all. If PE5 is driven high then the port kicks on, but at 3.3v. Once I also enable PI7 the port then reaches 4.6v. I'm not sure how to get it to a proper 5v yet, I'm still working that part out.

Thanks for working all that out, will hold off any other changes for now.

Ryan

#20 lore raw

Re: [PATCH v2 4/5] arm64: dts: allwinner: h700: Add RG35XX-H DTS

In reply to Chris Morgan

On Sat, 20 Apr 2024 22:09:40 -0500
Chris Morgan <[email protected]> wrote:

Hi Chris,

many thanks for chiming in and doing all those experiments!

Quoted 169 lines · Andre Przywara
Quoted 101 lines

On Sat, 20 Apr 2024 22:43:58 +1200
Ryan Walklin <[email protected]> wrote:

Hi,

Quoted 12 lines

The RG35XX-H adds thumbsticks, a stereo speaker, and a second USB port to the RG35XX-Plus, and has a horizontal form factor.

Enabled in this DTS:
- Thumbsticks
- Second USB port

Changelog v1..v2:
- Update copyright
- Spaces -> Tabs
- Add GP ADC joystick axes and mux [1]
- Add EHCI/OHCI1 for second USB port and add vbus supply

Quoted 3 lines

[1]: https://lore.kernel.org/linux-sunxi/[email protected]/T/#t

As mention on patch 2/5, this might be better an optional dependency, so the GPADC part might be a separate patch.

Quoted 77 lines
---
 .../sun50i-h700-anbernic-rg35xx-h.dts         | 126 ++++++++++++++++++
 1 file changed, 126 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +64 -0
new file mode 100644
index 000000000000..d62cf5cd9d9b
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Copyright (C) 2024 Ryan Walklin <[email protected]>.
+ * Copyright (C) 2024 Chris Morgan <[email protected]>.
+ */
+
+#include "sun50i-h700-anbernic-rg35xx-plus.dts"
+
+/ {
+	model = "Anbernic RG35XX H";
+	compatible = "anbernic,rg35xx-h", "allwinner,sun50i-h700";
+
+	adc-joystick {
+		compatible = "adc-joystick";
+		io-channels = <&adc_mux 0>,
+				  <&adc_mux 1>,
+				  <&adc_mux 2>,
+				  <&adc_mux 3>;
+		pinctrl-0 = <&joy_mux_pin>;
+		pinctrl-names = "default";
+		poll-interval = <60>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		axis@0 {
+			reg = <0>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <4096 0>;
+			linux,code = <ABS_X>;
+		};
+
+		axis@1 {
+			reg = <1>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <0 4096>;
+			linux,code = <ABS_Y>;
+		};
+
+		axis@2 {
+			reg = <2>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <0 4096>;
+			linux,code = <ABS_RX>;
+		};
+
+		axis@3 {
+			reg = <3>;
+			abs-flat = <32>;
+			abs-fuzz = <32>;
+			abs-range = <4096 0>;
+			linux,code = <ABS_RY>;
+		};
+	};
+
+	adc_mux: adc-mux {
+		compatible = "io-channel-mux";
+		channels = "left_x", "left_y", "right_x", "right_y";
+		#io-channel-cells = <1>;
+		io-channels = <&gpadc 0>;
+		io-channel-names = "parent";
+		mux-controls = <&gpio_mux>;
Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts line 68

Quoted 4 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +4 -0
+		settle-time-us = <100>;
+	};
+
+	gpio_keys: gpio-keys-thumb {  

Is there any reason to not just use the existing gpio-keys node? Either put a label on it in patch 2/5, and reference that below, outside of the root node, or use an absolute path reference.

I would also second just putting an alias and adding these to it. I myself as a preference tend to set the GPIO volume buttons as a seperate node only so I can enable key repeat on them, otherwise one node is best.

Quoted 29 lines
Quoted 29 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +28 -0
+		compatible = "gpio-keys";
+
+		button-thumbl {
+			label = "GPIO Thumb Left";
+			gpios = <&pio 4 8 GPIO_ACTIVE_LOW>; /* PE8 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_THUMBL>;
+		};
+
+		button-thumbr {
+			label = "GPIO Thumb Right";
+			gpios = <&pio 4 9 GPIO_ACTIVE_LOW>; /* PE9 */
+			linux,input-type = <EV_KEY>;
+			linux,code = <BTN_THUMBR>;
+		};
+	};
+
+	gpio_mux: mux-controller {
+		compatible = "gpio-mux";
+		mux-gpios = <&pio 8 1 GPIO_ACTIVE_LOW>,
+				<&pio 8 2 GPIO_ACTIVE_LOW>; /* PI1, PI2 */
+		#mux-control-cells = <0>;
+	};
+};
+
+&gpadc {
+	#address-cells = <1>;
+	#size-cells = <0>;
Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts line 104

Quoted 8 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +8 -0
+	status = "okay";
+
+	channel@0 {
+		reg = <0>;
+	};
+};
+
+&pio {  

After extensive testing with a multimeter and fudging the regulator voltages up or down, I've been able to figure out the regulator assignments for each of the different power domains. Schematics would have helped, but sadly this had to be done the hard way. Based on past experience with Anbernic I would strongly suspect all devices have this assignment, but I know for sure my 35XXH does.

        vcc-pa-supply = <&reg_cldo3>;
        vcc-pc-supply = <&reg_cldo3>;
        vcc-pe-supply = <&reg_cldo3>;
        vcc-pf-supply = <&reg_cldo3>;
        vcc-pg-supply = <&reg_aldo4>;
        vcc-ph-supply = <&reg_cldo3>;
        vcc-pi-supply = <&reg_cldo3>;
Quoted 2 lines

On my board the reg_cldo3 is a constant 3.3v output, and the reg_aldo4 is a constant 1.8v output.

Ah, many thanks for doing this, this is indeed very helpful! There are more power input pins on the SoC, some are essential, like the already mentioned VCC-PLL, but also AVCC, I think. The datasheet should give the voltages required for each. So if CLDO3 is 3.3V, and ALDO4 is 1.8V, both right from reset, then there is a chance that those rails are (re-)used for those essential lines as well.

Also there are per-subsystem power supplies, like for HDMI and USB. For HDMI the binding supports a -supply property, but for the USB-PHY we do not (which should be fixed).
So there must be some 3.3V rail supplying VCC-USB, and that's NOT the VBUS bus power, but needed for the PHY, as USB 1.x uses 3.3V on the data lines. And even though this line is needed for USB only, we must mark it as always-on, as there is no way to specify the consumer, as just mentioned.

Quoted 33 lines
Quoted 17 lines
Quoted 17 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +17 -0
+	joy_mux_pin: joy-mux-pin {
+		pins = "PI0";
+		function = "gpio_out";
+	};
+};
+
+&ehci1 {
+	status = "okay";
+};
+
+&ohci1 {
+	status = "okay";
+};
+
+&usbotg {
+	dr_mode = "peripheral";
+	status = "okay";
Quoted 8 lines

arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts line 125

Quoted 4 lines
arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts new file +4 -0
+};
+
+&usbphy {
+	usb1_vbus-supply = <&reg_vcc_usb>;  

This is the dodgy USB supply chain. Any chance we can narrow this down, to maybe one GPIO controlled regulator? Also, where does the boost controller come into play here?

I haven't figured out the boost regulator yet, but for the host port I've been able to ascertain there's no less than 2 GPIO controlled regulators in play. PE5 must be driven high or the USB host port will not power on at all. If PE5 is driven high then the port kicks on, but at 3.3v. Once I also enable PI7 the port then reaches 4.6v. I'm not sure how to get it to a proper 5v yet, I'm still working that part out.

Quoted 5 lines

Maybe PE5 is a reset of some kind that's active low, I don't know. I just know so far/for sure that if PE5 is low then nothing registers on the USB host port; if PE5 is high but PI7 is low the port sort-of works at 3.3v, and if both PE5 and PI7 are high the port works consistently and at 4.6v.

Is that with some power provided through the other USB port? Then 4.6V sound plausible.
Also I would assume there is some regulator or switch that toggles VBUS between the PMIC input voltage and the boost regulator. The former would be used when the device is powered through a USB cable, the latter when it's on battery. I don't think it makes much sense to always use the boost regulator, even when there is some "native" 5.0V available. That being said I have no idea how to model this switch in the DT atm, but maybe this helps with the understanding of the role of those two GPIOs?

Should PE5 really drive VCC-USB and VCC-USB2 on the SoC (I somewhat doubt that, though), then we must mark this regulator as always-on, see above.

Cheers,
Andre

#21 lore raw

PATCH v2 5/5 arm64: dts: allwinner: h700: Add RG35XX DTBs to Makefile

In reply to Ryan Walklin
---
 arch/arm64/boot/dts/allwinner/Makefile | 3 +++
 1 file changed, 3 insertions(+)
arch/arm64/boot/dts/allwinner/Makefile +3 -0
index 21149b346a60..c2c871d8b71e 100644
@@ -47,3 +47,6 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-longanpi-3h.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-orangepi-zero2w.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-orangepi-zero3.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-transpeed-8k618-t.dtb
+dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h700-anbernic-rg35xx-2024.dtb
+dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h700-anbernic-rg35xx-plus.dtb
+dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h700-anbernic-rg35xx-h.dtb
Signature · 1 line
2.44.0
#22 lore raw

Re: [PATCH v2 5/5] arm64: dts: allwinner: h700: Add RG35XX DTBs to Makefile

On Sat, 20 Apr 2024 22:43:59 +1200
Ryan Walklin <[email protected]> wrote:

Hi,

it looks unusual to have this as a separate patch. Please squash the respective line into the patch that introduces each file.

Cheers,
Andre

Quoted 16 lines
---
 arch/arm64/boot/dts/allwinner/Makefile | 3 +++
 1 file changed, 3 insertions(+)
arch/arm64/boot/dts/allwinner/Makefile +3 -0
index 21149b346a60..c2c871d8b71e 100644
@@ -47,3 +47,6 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-longanpi-3h.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-orangepi-zero2w.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-orangepi-zero3.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-transpeed-8k618-t.dtb
+dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h700-anbernic-rg35xx-2024.dtb
+dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h700-anbernic-rg35xx-plus.dtb
+dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h700-anbernic-rg35xx-h.dtb