devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] arm64: dts: allwinner: Support Orange Pi 3 LTS board
@ 2025-04-13 13:42 Jernej Skrabec
  2025-04-13 13:42 ` [PATCH 1/2] dt-bindings: arm: sunxi: Add " Jernej Skrabec
  2025-04-13 13:42 ` [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS Jernej Skrabec
  0 siblings, 2 replies; 18+ messages in thread
From: Jernej Skrabec @ 2025-04-13 13:42 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt
  Cc: wens, samuel, devicetree, linux-arm-kernel, linux-sunxi,
	linux-kernel, Jernej Skrabec

Orange Pi 3 LTS has similar functionality to Orange Pi 3 with slightly
different hardware. 

OrangePi 3 LTS has following features:
- Allwinner H6 quad-core 64-bit ARM Cortex-A53
- GPU Mali-T720
- 2 GB LPDDR3 RAM
- AXP805 PMIC
- AW859A Wifi/BT 5.0
- 2x USB 2.0 host port (A)
- USB 3.0 Host
- Gigabit Ethernet (Motorcomm YT8531C phy)
- HDMI 2.0 port
- soldered 8 GB eMMC
- 2x LED
- microphone
- audio jack

Best regards,
Jernej

Jernej Skrabec (2):
  dt-bindings: arm: sunxi: Add Orange Pi 3 LTS board
  arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS

 .../devicetree/bindings/arm/sunxi.yaml        |   5 +
 arch/arm64/boot/dts/allwinner/Makefile        |   1 +
 .../allwinner/sun50i-h6-orangepi-3-lts.dts    | 351 ++++++++++++++++++
 3 files changed, 357 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts

-- 
2.49.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/2] dt-bindings: arm: sunxi: Add Orange Pi 3 LTS board
  2025-04-13 13:42 [PATCH 0/2] arm64: dts: allwinner: Support Orange Pi 3 LTS board Jernej Skrabec
@ 2025-04-13 13:42 ` Jernej Skrabec
  2025-04-15 21:56   ` Rob Herring (Arm)
  2025-04-13 13:42 ` [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS Jernej Skrabec
  1 sibling, 1 reply; 18+ messages in thread
From: Jernej Skrabec @ 2025-04-13 13:42 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt
  Cc: wens, samuel, devicetree, linux-arm-kernel, linux-sunxi,
	linux-kernel, Jernej Skrabec

Orange Pi 3 LTS board is similar to Orange Pi 3, with slightly different
hardware but mostly same functionality. It has less options than
original variant. eMMC was optional before, now it's always included.
2 GB RAM is now standard, previous variant also has 1 GB RAM version.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 Documentation/devicetree/bindings/arm/sunxi.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml
index f536cdd2c1a6..f417745c799e 100644
--- a/Documentation/devicetree/bindings/arm/sunxi.yaml
+++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
@@ -981,6 +981,11 @@ properties:
           - const: xunlong,orangepi-3
           - const: allwinner,sun50i-h6
 
+      - description: Xunlong OrangePi 3 LTS
+        items:
+          - const: xunlong,orangepi-3-lts
+          - const: allwinner,sun50i-h6
+
       - description: Xunlong OrangePi Lite
         items:
           - const: xunlong,orangepi-lite
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-13 13:42 [PATCH 0/2] arm64: dts: allwinner: Support Orange Pi 3 LTS board Jernej Skrabec
  2025-04-13 13:42 ` [PATCH 1/2] dt-bindings: arm: sunxi: Add " Jernej Skrabec
@ 2025-04-13 13:42 ` Jernej Skrabec
  2025-04-25  0:57   ` Andre Przywara
  2025-04-25 12:54   ` Andre Przywara
  1 sibling, 2 replies; 18+ messages in thread
From: Jernej Skrabec @ 2025-04-13 13:42 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt
  Cc: wens, samuel, devicetree, linux-arm-kernel, linux-sunxi,
	linux-kernel, Jernej Skrabec

OrangePi 3 LTS is quite similar to original OrangePi 3, but it has a lot
small changes that makes DT sharing unpractical with it.

OrangePi 3 LTS has following features:
- Allwinner H6 quad-core 64-bit ARM Cortex-A53
- GPU Mali-T720
- 2 GB LPDDR3 RAM
- AXP805 PMIC
- AW859A Wifi/BT 5.0
- 2x USB 2.0 host port (A)
- USB 3.0 Host
- Gigabit Ethernet (Motorcomm YT8531C phy)
- HDMI 2.0 port
- soldered 8 GB eMMC
- 2x LED
- microphone
- audio jack

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 arch/arm64/boot/dts/allwinner/Makefile        |   1 +
 .../allwinner/sun50i-h6-orangepi-3-lts.dts    | 351 ++++++++++++++++++
 2 files changed, 352 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts

diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
index 00bed412ee31..72c43bd0e2ab 100644
--- a/arch/arm64/boot/dts/allwinner/Makefile
+++ b/arch/arm64/boot/dts/allwinner/Makefile
@@ -33,6 +33,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus2.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-beelink-gs1.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3.dtb
+dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3-lts.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-lite2.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-one-plus.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64.dtb
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts
new file mode 100644
index 000000000000..c8830d5c2f09
--- /dev/null
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts
@@ -0,0 +1,351 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+// Copyright (C) 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
+// Based on sun50i-h6-orangepi-3.dts, which is:
+// Copyright (C) 2019 Ondřej Jirman <megous@megous.com>
+
+/dts-v1/;
+
+#include "sun50i-h6.dtsi"
+#include "sun50i-h6-cpu-opp.dtsi"
+#include "sun50i-h6-gpu-opp.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+
+/ {
+	model = "OrangePi 3 LTS";
+	compatible = "xunlong,orangepi-3-lts", "allwinner,sun50i-h6";
+
+	aliases {
+		ethernet0 = &emac;
+		ethernet1 = &aw859a;
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	connector {
+		compatible = "hdmi-connector";
+		ddc-en-gpios = <&pio 7 2 GPIO_ACTIVE_HIGH>; /* PH2 */
+		type = "a";
+
+		port {
+			hdmi_con_in: endpoint {
+				remote-endpoint = <&hdmi_out_con>;
+			};
+		};
+	};
+
+	ext_osc32k: ext_osc32k_clk {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <32768>;
+		clock-output-names = "ext_osc32k";
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		led-0 {
+			function = LED_FUNCTION_POWER;
+			color = <LED_COLOR_ID_RED>;
+			gpios = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */
+			default-state = "on";
+		};
+
+		led-1 {
+			function = LED_FUNCTION_STATUS;
+			color = <LED_COLOR_ID_GREEN>;
+			gpios = <&r_pio 0 7 GPIO_ACTIVE_HIGH>; /* PL7 */
+		};
+	};
+
+	reg_gmac_3v3: gmac-3v3 {
+		compatible = "regulator-fixed";
+		regulator-name = "gmac-3v3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		startup-delay-us = <150000>;
+		enable-active-high;
+		gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
+	};
+
+	reg_vcc5v: vcc5v {
+		/* board wide 5V supply directly from the DC jack */
+		compatible = "regulator-fixed";
+		regulator-name = "vcc-5v";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
+	};
+
+	reg_wifi_3v3: wifi-3v3 {
+		/* 3.3V regulator for WiFi and BT */
+		compatible = "regulator-fixed";
+		regulator-name = "wifi-3v3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		enable-active-high;
+		gpio = <&pio 7 7 GPIO_ACTIVE_HIGH>; /* PH7 */
+	};
+
+	wifi_pwrseq: wifi-pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		clocks = <&rtc 1>;
+		clock-names = "ext_clock";
+		reset-gpios = <&r_pio 1 3 GPIO_ACTIVE_LOW>; /* PM3 */
+		post-power-on-delay-ms = <200>;
+	};
+};
+
+&cpu0 {
+	cpu-supply = <&reg_dcdca>;
+};
+
+&de {
+	status = "okay";
+};
+
+&dwc3 {
+	status = "okay";
+};
+
+&ehci0 {
+	status = "okay";
+};
+
+&ehci3 {
+	status = "okay";
+};
+
+&emac {
+	pinctrl-names = "default";
+	pinctrl-0 = <&ext_rgmii_pins>;
+	phy-mode = "rgmii-rxid";
+	phy-handle = <&ext_rgmii_phy>;
+	phy-supply = <&reg_gmac_3v3>;
+	allwinner,rx-delay-ps = <0>;
+	allwinner,tx-delay-ps = <700>;
+	status = "okay";
+};
+
+&gpu {
+	mali-supply = <&reg_dcdcc>;
+	status = "okay";
+};
+
+&hdmi {
+	hvcc-supply = <&reg_bldo2>;
+	status = "okay";
+};
+
+&hdmi_out {
+	hdmi_out_con: endpoint {
+		remote-endpoint = <&hdmi_con_in>;
+	};
+};
+
+&mdio {
+	ext_rgmii_phy: ethernet-phy@1 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <1>;
+
+		motorcomm,clk-out-frequency-hz = <125000000>;
+
+		reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* PD14 */
+		reset-assert-us = <15000>;
+		reset-deassert-us = <100000>;
+	};
+};
+
+&mmc0 {
+	vmmc-supply = <&reg_cldo1>;
+	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
+	disable-wp;
+	bus-width = <4>;
+	status = "okay";
+};
+
+&mmc1 {
+	vmmc-supply = <&reg_wifi_3v3>;
+	vqmmc-supply = <&reg_bldo3>;
+	mmc-pwrseq = <&wifi_pwrseq>;
+	bus-width = <4>;
+	non-removable;
+	status = "okay";
+
+	aw859a: wifi@1 {
+		reg = <1>;
+	};
+};
+
+&mmc2 {
+	vmmc-supply = <&reg_cldo1>;
+	vqmmc-supply = <&reg_bldo2>;
+	cap-mmc-hw-reset;
+	non-removable;
+	bus-width = <8>;
+	status = "okay";
+};
+
+&ohci0 {
+	status = "okay";
+};
+
+&ohci3 {
+	status = "okay";
+};
+
+&pio {
+	vcc-pc-supply = <&reg_bldo2>;
+	vcc-pd-supply = <&reg_cldo1>;
+	vcc-pg-supply = <&reg_bldo3>;
+};
+
+&r_ir {
+	status = "okay";
+};
+
+&r_i2c {
+	status = "okay";
+
+	axp805: pmic@36 {
+		compatible = "x-powers,axp805", "x-powers,axp806";
+		reg = <0x36>;
+		interrupt-parent = <&r_intc>;
+		interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		x-powers,self-working-mode;
+		vina-supply = <&reg_vcc5v>;
+		vinb-supply = <&reg_vcc5v>;
+		vinc-supply = <&reg_vcc5v>;
+		vind-supply = <&reg_vcc5v>;
+		vine-supply = <&reg_vcc5v>;
+		aldoin-supply = <&reg_vcc5v>;
+		bldoin-supply = <&reg_vcc5v>;
+		cldoin-supply = <&reg_vcc5v>;
+
+		regulators {
+			reg_aldo1: aldo1 {
+				regulator-always-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcc-pl-led-ir";
+			};
+
+			reg_aldo2: aldo2 {
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcc33-audio-tv-ephy-mac";
+			};
+
+			/* ALDO3 is shorted to CLDO1 */
+			reg_aldo3: aldo3 {
+				regulator-always-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-1";
+			};
+
+			reg_bldo1: bldo1 {
+				regulator-always-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-name = "vcc18-dram-bias-pll";
+			};
+
+			reg_bldo2: bldo2 {
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-name = "vcc-efuse-pcie-hdmi-pc";
+			};
+
+			reg_bldo3: bldo3 {
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-name = "vcc-pm-pg-dcxoio-wifi";
+			};
+
+			bldo4 {
+				/* unused */
+			};
+
+			reg_cldo1: cldo1 {
+				regulator-always-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-2";
+			};
+
+			cldo2 {
+				/* unused */
+			};
+
+			cldo3 {
+				/* unused */
+			};
+
+			reg_dcdca: dcdca {
+				regulator-always-on;
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <1160000>;
+				regulator-ramp-delay = <2500>;
+				regulator-name = "vdd-cpu";
+			};
+
+			reg_dcdcc: dcdcc {
+				regulator-enable-ramp-delay = <32000>;
+				regulator-min-microvolt = <810000>;
+				regulator-max-microvolt = <1080000>;
+				regulator-ramp-delay = <2500>;
+				regulator-name = "vdd-gpu";
+			};
+
+			reg_dcdcd: dcdcd {
+				regulator-always-on;
+				regulator-min-microvolt = <960000>;
+				regulator-max-microvolt = <960000>;
+				regulator-name = "vdd-sys";
+			};
+
+			reg_dcdce: dcdce {
+				regulator-always-on;
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-name = "vcc-dram";
+			};
+
+			sw {
+				/* unused */
+			};
+		};
+	};
+};
+
+&rtc {
+	clocks = <&ext_osc32k>;
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_ph_pins>;
+	status = "okay";
+};
+
+&usb2otg {
+	dr_mode = "host";
+	status = "okay";
+};
+
+&usb2phy {
+	usb0_id_det-gpios = <&pio 2 15 GPIO_ACTIVE_HIGH>; /* PC15 */
+	usb0_vbus-supply = <&reg_vcc5v>;
+	usb3_vbus-supply = <&reg_vcc5v>;
+	status = "okay";
+};
+
+&usb3phy {
+	status = "okay";
+};
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] dt-bindings: arm: sunxi: Add Orange Pi 3 LTS board
  2025-04-13 13:42 ` [PATCH 1/2] dt-bindings: arm: sunxi: Add " Jernej Skrabec
@ 2025-04-15 21:56   ` Rob Herring (Arm)
  0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring (Arm) @ 2025-04-15 21:56 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: samuel, linux-sunxi, conor+dt, devicetree, linux-kernel, krzk+dt,
	wens, linux-arm-kernel


On Sun, 13 Apr 2025 15:42:56 +0200, Jernej Skrabec wrote:
> Orange Pi 3 LTS board is similar to Orange Pi 3, with slightly different
> hardware but mostly same functionality. It has less options than
> original variant. eMMC was optional before, now it's always included.
> 2 GB RAM is now standard, previous variant also has 1 GB RAM version.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  Documentation/devicetree/bindings/arm/sunxi.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-13 13:42 ` [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS Jernej Skrabec
@ 2025-04-25  0:57   ` Andre Przywara
  2025-04-25 12:54   ` Andre Przywara
  1 sibling, 0 replies; 18+ messages in thread
From: Andre Przywara @ 2025-04-25  0:57 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: robh, krzk+dt, conor+dt, wens, samuel, devicetree,
	linux-arm-kernel, linux-sunxi, linux-kernel

On Sun, 13 Apr 2025 15:42:57 +0200
Jernej Skrabec <jernej.skrabec@gmail.com> wrote:

> OrangePi 3 LTS is quite similar to original OrangePi 3, but it has a lot
> small changes that makes DT sharing unpractical with it.
> 
> OrangePi 3 LTS has following features:
> - Allwinner H6 quad-core 64-bit ARM Cortex-A53
> - GPU Mali-T720
> - 2 GB LPDDR3 RAM
> - AXP805 PMIC
> - AW859A Wifi/BT 5.0
> - 2x USB 2.0 host port (A)
> - USB 3.0 Host
> - Gigabit Ethernet (Motorcomm YT8531C phy)
> - HDMI 2.0 port
> - soldered 8 GB eMMC
> - 2x LED
> - microphone
> - audio jack
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

I guess you tried to use a common orangepi-3.dtsi, but that didn't
work out well?

> ---
>  arch/arm64/boot/dts/allwinner/Makefile        |   1 +
>  .../allwinner/sun50i-h6-orangepi-3-lts.dts    | 351 ++++++++++++++++++
>  2 files changed, 352 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts
> 
> diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> index 00bed412ee31..72c43bd0e2ab 100644
> --- a/arch/arm64/boot/dts/allwinner/Makefile
> +++ b/arch/arm64/boot/dts/allwinner/Makefile
> @@ -33,6 +33,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus2.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-beelink-gs1.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3.dtb
> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3-lts.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-lite2.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-one-plus.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64.dtb
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts
> new file mode 100644
> index 000000000000..c8830d5c2f09
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts
> @@ -0,0 +1,351 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +// Copyright (C) 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
> +// Based on sun50i-h6-orangepi-3.dts, which is:
> +// Copyright (C) 2019 Ondřej Jirman <megous@megous.com>
> +
> +/dts-v1/;
> +
> +#include "sun50i-h6.dtsi"
> +#include "sun50i-h6-cpu-opp.dtsi"
> +#include "sun50i-h6-gpu-opp.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +
> +/ {
> +	model = "OrangePi 3 LTS";
> +	compatible = "xunlong,orangepi-3-lts", "allwinner,sun50i-h6";
> +
> +	aliases {
> +		ethernet0 = &emac;
> +		ethernet1 = &aw859a;
> +		serial0 = &uart0;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	connector {
> +		compatible = "hdmi-connector";
> +		ddc-en-gpios = <&pio 7 2 GPIO_ACTIVE_HIGH>; /* PH2 */
> +		type = "a";
> +
> +		port {
> +			hdmi_con_in: endpoint {
> +				remote-endpoint = <&hdmi_out_con>;
> +			};
> +		};
> +	};
> +
> +	ext_osc32k: ext_osc32k_clk {

Shouldn't the node name contain dashes instead of underscores?

> +		#clock-cells = <0>;
> +		compatible = "fixed-clock";
> +		clock-frequency = <32768>;
> +		clock-output-names = "ext_osc32k";
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		led-0 {
> +			function = LED_FUNCTION_POWER;
> +			color = <LED_COLOR_ID_RED>;
> +			gpios = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */
> +			default-state = "on";
> +		};
> +
> +		led-1 {
> +			function = LED_FUNCTION_STATUS;
> +			color = <LED_COLOR_ID_GREEN>;
> +			gpios = <&r_pio 0 7 GPIO_ACTIVE_HIGH>; /* PL7 */
> +		};
> +	};
> +
> +	reg_gmac_3v3: gmac-3v3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "gmac-3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		startup-delay-us = <150000>;
> +		enable-active-high;
> +		gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
> +	};
> +
> +	reg_vcc5v: vcc5v {
> +		/* board wide 5V supply directly from the DC jack */
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc-5v";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_wifi_3v3: wifi-3v3 {
> +		/* 3.3V regulator for WiFi and BT */
> +		compatible = "regulator-fixed";
> +		regulator-name = "wifi-3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		enable-active-high;
> +		gpio = <&pio 7 7 GPIO_ACTIVE_HIGH>; /* PH7 */
> +	};
> +
> +	wifi_pwrseq: wifi-pwrseq {
> +		compatible = "mmc-pwrseq-simple";
> +		clocks = <&rtc 1>;

Shouldn't that be <&rtc CLK_OSC32K_FANOUT>?

Cheers,
Andre

> +		clock-names = "ext_clock";
> +		reset-gpios = <&r_pio 1 3 GPIO_ACTIVE_LOW>; /* PM3 */
> +		post-power-on-delay-ms = <200>;
> +	};
> +};
> +
> +&cpu0 {
> +	cpu-supply = <&reg_dcdca>;
> +};
> +
> +&de {
> +	status = "okay";
> +};
> +
> +&dwc3 {
> +	status = "okay";
> +};
> +
> +&ehci0 {
> +	status = "okay";
> +};
> +
> +&ehci3 {
> +	status = "okay";
> +};
> +
> +&emac {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&ext_rgmii_pins>;
> +	phy-mode = "rgmii-rxid";
> +	phy-handle = <&ext_rgmii_phy>;
> +	phy-supply = <&reg_gmac_3v3>;
> +	allwinner,rx-delay-ps = <0>;
> +	allwinner,tx-delay-ps = <700>;
> +	status = "okay";
> +};
> +
> +&gpu {
> +	mali-supply = <&reg_dcdcc>;
> +	status = "okay";
> +};
> +
> +&hdmi {
> +	hvcc-supply = <&reg_bldo2>;
> +	status = "okay";
> +};
> +
> +&hdmi_out {
> +	hdmi_out_con: endpoint {
> +		remote-endpoint = <&hdmi_con_in>;
> +	};
> +};
> +
> +&mdio {
> +	ext_rgmii_phy: ethernet-phy@1 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <1>;
> +
> +		motorcomm,clk-out-frequency-hz = <125000000>;
> +
> +		reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* PD14 */
> +		reset-assert-us = <15000>;
> +		reset-deassert-us = <100000>;
> +	};
> +};
> +
> +&mmc0 {
> +	vmmc-supply = <&reg_cldo1>;
> +	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
> +	disable-wp;
> +	bus-width = <4>;
> +	status = "okay";
> +};
> +
> +&mmc1 {
> +	vmmc-supply = <&reg_wifi_3v3>;
> +	vqmmc-supply = <&reg_bldo3>;
> +	mmc-pwrseq = <&wifi_pwrseq>;
> +	bus-width = <4>;
> +	non-removable;
> +	status = "okay";
> +
> +	aw859a: wifi@1 {
> +		reg = <1>;
> +	};
> +};
> +
> +&mmc2 {
> +	vmmc-supply = <&reg_cldo1>;
> +	vqmmc-supply = <&reg_bldo2>;
> +	cap-mmc-hw-reset;
> +	non-removable;
> +	bus-width = <8>;
> +	status = "okay";
> +};
> +
> +&ohci0 {
> +	status = "okay";
> +};
> +
> +&ohci3 {
> +	status = "okay";
> +};
> +
> +&pio {
> +	vcc-pc-supply = <&reg_bldo2>;
> +	vcc-pd-supply = <&reg_cldo1>;
> +	vcc-pg-supply = <&reg_bldo3>;
> +};
> +
> +&r_ir {
> +	status = "okay";
> +};
> +
> +&r_i2c {
> +	status = "okay";
> +
> +	axp805: pmic@36 {
> +		compatible = "x-powers,axp805", "x-powers,axp806";
> +		reg = <0x36>;
> +		interrupt-parent = <&r_intc>;
> +		interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_LOW>;
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +		x-powers,self-working-mode;
> +		vina-supply = <&reg_vcc5v>;
> +		vinb-supply = <&reg_vcc5v>;
> +		vinc-supply = <&reg_vcc5v>;
> +		vind-supply = <&reg_vcc5v>;
> +		vine-supply = <&reg_vcc5v>;
> +		aldoin-supply = <&reg_vcc5v>;
> +		bldoin-supply = <&reg_vcc5v>;
> +		cldoin-supply = <&reg_vcc5v>;
> +
> +		regulators {
> +			reg_aldo1: aldo1 {
> +				regulator-always-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vcc-pl-led-ir";
> +			};
> +
> +			reg_aldo2: aldo2 {
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vcc33-audio-tv-ephy-mac";
> +			};
> +
> +			/* ALDO3 is shorted to CLDO1 */
> +			reg_aldo3: aldo3 {
> +				regulator-always-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-1";
> +			};
> +
> +			reg_bldo1: bldo1 {
> +				regulator-always-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-name = "vcc18-dram-bias-pll";
> +			};
> +
> +			reg_bldo2: bldo2 {
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-name = "vcc-efuse-pcie-hdmi-pc";
> +			};
> +
> +			reg_bldo3: bldo3 {
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-name = "vcc-pm-pg-dcxoio-wifi";
> +			};
> +
> +			bldo4 {
> +				/* unused */
> +			};
> +
> +			reg_cldo1: cldo1 {
> +				regulator-always-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-2";
> +			};
> +
> +			cldo2 {
> +				/* unused */
> +			};
> +
> +			cldo3 {
> +				/* unused */
> +			};
> +
> +			reg_dcdca: dcdca {
> +				regulator-always-on;
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <1160000>;
> +				regulator-ramp-delay = <2500>;
> +				regulator-name = "vdd-cpu";
> +			};
> +
> +			reg_dcdcc: dcdcc {
> +				regulator-enable-ramp-delay = <32000>;
> +				regulator-min-microvolt = <810000>;
> +				regulator-max-microvolt = <1080000>;
> +				regulator-ramp-delay = <2500>;
> +				regulator-name = "vdd-gpu";
> +			};
> +
> +			reg_dcdcd: dcdcd {
> +				regulator-always-on;
> +				regulator-min-microvolt = <960000>;
> +				regulator-max-microvolt = <960000>;
> +				regulator-name = "vdd-sys";
> +			};
> +
> +			reg_dcdce: dcdce {
> +				regulator-always-on;
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-name = "vcc-dram";
> +			};
> +
> +			sw {
> +				/* unused */
> +			};
> +		};
> +	};
> +};
> +
> +&rtc {
> +	clocks = <&ext_osc32k>;
> +};
> +
> +&uart0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart0_ph_pins>;
> +	status = "okay";
> +};
> +
> +&usb2otg {
> +	dr_mode = "host";
> +	status = "okay";
> +};
> +
> +&usb2phy {
> +	usb0_id_det-gpios = <&pio 2 15 GPIO_ACTIVE_HIGH>; /* PC15 */
> +	usb0_vbus-supply = <&reg_vcc5v>;
> +	usb3_vbus-supply = <&reg_vcc5v>;
> +	status = "okay";
> +};
> +
> +&usb3phy {
> +	status = "okay";
> +};


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-13 13:42 ` [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS Jernej Skrabec
  2025-04-25  0:57   ` Andre Przywara
@ 2025-04-25 12:54   ` Andre Przywara
  2025-04-25 14:37     ` Chen-Yu Tsai
                       ` (2 more replies)
  1 sibling, 3 replies; 18+ messages in thread
From: Andre Przywara @ 2025-04-25 12:54 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: robh, krzk+dt, conor+dt, wens, samuel, devicetree,
	linux-arm-kernel, linux-sunxi, linux-kernel

On Sun, 13 Apr 2025 15:42:57 +0200
Jernej Skrabec <jernej.skrabec@gmail.com> wrote:

Hi Jernej,

thanks for sending this, I now went through this in more detail and
compared against the schematic, so some more nits below.
I added the two comments from my other email before, so you can ignore that
one now.

> OrangePi 3 LTS is quite similar to original OrangePi 3, but it has a lot
> small changes that makes DT sharing unpractical with it.
> 
> OrangePi 3 LTS has following features:
> - Allwinner H6 quad-core 64-bit ARM Cortex-A53
> - GPU Mali-T720
> - 2 GB LPDDR3 RAM
> - AXP805 PMIC
> - AW859A Wifi/BT 5.0
> - 2x USB 2.0 host port (A)
> - USB 3.0 Host
> - Gigabit Ethernet (Motorcomm YT8531C phy)
> - HDMI 2.0 port
> - soldered 8 GB eMMC
> - 2x LED
> - microphone
> - audio jack
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  arch/arm64/boot/dts/allwinner/Makefile        |   1 +
>  .../allwinner/sun50i-h6-orangepi-3-lts.dts    | 351 ++++++++++++++++++
>  2 files changed, 352 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts
> 
> diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> index 00bed412ee31..72c43bd0e2ab 100644
> --- a/arch/arm64/boot/dts/allwinner/Makefile
> +++ b/arch/arm64/boot/dts/allwinner/Makefile
> @@ -33,6 +33,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus2.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-beelink-gs1.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3.dtb
> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3-lts.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-lite2.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-one-plus.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64.dtb
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts
> new file mode 100644
> index 000000000000..c8830d5c2f09
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts
> @@ -0,0 +1,351 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +// Copyright (C) 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
> +// Based on sun50i-h6-orangepi-3.dts, which is:
> +// Copyright (C) 2019 Ondřej Jirman <megous@megous.com>
> +
> +/dts-v1/;
> +
> +#include "sun50i-h6.dtsi"
> +#include "sun50i-h6-cpu-opp.dtsi"
> +#include "sun50i-h6-gpu-opp.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +
> +/ {
> +	model = "OrangePi 3 LTS";
> +	compatible = "xunlong,orangepi-3-lts", "allwinner,sun50i-h6";
> +
> +	aliases {
> +		ethernet0 = &emac;
> +		ethernet1 = &aw859a;
> +		serial0 = &uart0;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	connector {
> +		compatible = "hdmi-connector";
> +		ddc-en-gpios = <&pio 7 2 GPIO_ACTIVE_HIGH>; /* PH2 */
> +		type = "a";
> +
> +		port {
> +			hdmi_con_in: endpoint {
> +				remote-endpoint = <&hdmi_out_con>;
> +			};
> +		};
> +	};
> +
> +	ext_osc32k: ext_osc32k_clk {

For the sake of completeness, as mentioned in the other mail, I think we
want dashes in the node name.

> +		#clock-cells = <0>;
> +		compatible = "fixed-clock";
> +		clock-frequency = <32768>;
> +		clock-output-names = "ext_osc32k";

Should the output name also contain dashes instead?

> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		led-0 {
> +			function = LED_FUNCTION_POWER;
> +			color = <LED_COLOR_ID_RED>;
> +			gpios = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */
> +			default-state = "on";

Maybe something for a follow up patch, but I noticed that the schematic
does not show current limiting resistors for the LEDs. This probably works
because the default drive strength is 0b01, so level 1 (in a range from 0
to 3, which we map to 10, 20, 30, 40 mA). The exact LED is not mentioned,
but I would imagine that more than 20 mA would not be healthy, and even
this might be a stretch over longer times.

So should we force the drive-strength to <10> or <20> somewhere? How does
this even work for those gpios references?

> +		};
> +
> +		led-1 {
> +			function = LED_FUNCTION_STATUS;
> +			color = <LED_COLOR_ID_GREEN>;
> +			gpios = <&r_pio 0 7 GPIO_ACTIVE_HIGH>; /* PL7 */
> +		};
> +	};
> +
> +	reg_gmac_3v3: gmac-3v3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "gmac-3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		startup-delay-us = <150000>;
> +		enable-active-high;
> +		gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */

Can you please add a "vin-supply = <&reg_vcc5v>;" line here, to chain them
up nicely?

> +	};
> +
> +	reg_vcc5v: vcc5v {
> +		/* board wide 5V supply directly from the DC jack */
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc-5v";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_wifi_3v3: wifi-3v3 {
> +		/* 3.3V regulator for WiFi and BT */
> +		compatible = "regulator-fixed";
> +		regulator-name = "wifi-3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		enable-active-high;
> +		gpio = <&pio 7 7 GPIO_ACTIVE_HIGH>; /* PH7 */

Same vin-supply here, please, this discrete regulator is fed by DCIN 5V.

> +	};
> +
> +	wifi_pwrseq: wifi-pwrseq {
> +		compatible = "mmc-pwrseq-simple";
> +		clocks = <&rtc 1>;

As mentioned yesterday, please use CLK_OSC32K_FANOUT.

> +		clock-names = "ext_clock";
> +		reset-gpios = <&r_pio 1 3 GPIO_ACTIVE_LOW>; /* PM3 */
> +		post-power-on-delay-ms = <200>;
> +	};
> +};
> +
> +&cpu0 {
> +	cpu-supply = <&reg_dcdca>;
> +};
> +
> +&de {
> +	status = "okay";
> +};
> +
> +&dwc3 {
> +	status = "okay";
> +};
> +
> +&ehci0 {
> +	status = "okay";
> +};
> +
> +&ehci3 {
> +	status = "okay";
> +};
> +
> +&emac {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&ext_rgmii_pins>;
> +	phy-mode = "rgmii-rxid";

So relating to what Andrew said earlier today, should this read rgmii-id
instead? Since the strap resistors just set some boot-up value, but we
want the PHY driver to enable both RX and TX delay programmatically?

> +	phy-handle = <&ext_rgmii_phy>;
> +	phy-supply = <&reg_gmac_3v3>;
> +	allwinner,rx-delay-ps = <0>;
> +	allwinner,tx-delay-ps = <700>;
> +	status = "okay";
> +};
> +
> +&gpu {
> +	mali-supply = <&reg_dcdcc>;
> +	status = "okay";
> +};
> +
> +&hdmi {
> +	hvcc-supply = <&reg_bldo2>;
> +	status = "okay";
> +};
> +
> +&hdmi_out {
> +	hdmi_out_con: endpoint {
> +		remote-endpoint = <&hdmi_con_in>;
> +	};
> +};
> +
> +&mdio {
> +	ext_rgmii_phy: ethernet-phy@1 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <1>;
> +
> +		motorcomm,clk-out-frequency-hz = <125000000>;
> +
> +		reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* PD14 */
> +		reset-assert-us = <15000>;
> +		reset-deassert-us = <100000>;
> +	};
> +};
> +
> +&mmc0 {
> +	vmmc-supply = <&reg_cldo1>;
> +	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
> +	disable-wp;
> +	bus-width = <4>;
> +	status = "okay";
> +};
> +
> +&mmc1 {
> +	vmmc-supply = <&reg_wifi_3v3>;
> +	vqmmc-supply = <&reg_bldo3>;
> +	mmc-pwrseq = <&wifi_pwrseq>;
> +	bus-width = <4>;
> +	non-removable;
> +	status = "okay";
> +
> +	aw859a: wifi@1 {
> +		reg = <1>;
> +	};
> +};
> +
> +&mmc2 {
> +	vmmc-supply = <&reg_cldo1>;
> +	vqmmc-supply = <&reg_bldo2>;
> +	cap-mmc-hw-reset;
> +	non-removable;
> +	bus-width = <8>;
> +	status = "okay";

Given that it's 1.8V on the I/O lines, I think we would need the
mmc-ddr-1_8v and mmc-hs200-1_8v properties, for higher speed modes? Or
does that not work?

> +};
> +
> +&ohci0 {
> +	status = "okay";
> +};
> +
> +&ohci3 {
> +	status = "okay";
> +};
> +
> +&pio {
> +	vcc-pc-supply = <&reg_bldo2>;
> +	vcc-pd-supply = <&reg_cldo1>;
> +	vcc-pg-supply = <&reg_bldo3>;
> +};
> +
> +&r_ir {
> +	status = "okay";
> +};
> +
> +&r_i2c {
> +	status = "okay";
> +
> +	axp805: pmic@36 {
> +		compatible = "x-powers,axp805", "x-powers,axp806";
> +		reg = <0x36>;
> +		interrupt-parent = <&r_intc>;
> +		interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_LOW>;
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +		x-powers,self-working-mode;
> +		vina-supply = <&reg_vcc5v>;
> +		vinb-supply = <&reg_vcc5v>;
> +		vinc-supply = <&reg_vcc5v>;
> +		vind-supply = <&reg_vcc5v>;
> +		vine-supply = <&reg_vcc5v>;
> +		aldoin-supply = <&reg_vcc5v>;
> +		bldoin-supply = <&reg_vcc5v>;
> +		cldoin-supply = <&reg_vcc5v>;
> +
> +		regulators {
> +			reg_aldo1: aldo1 {
> +				regulator-always-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vcc-pl-led-ir";
> +			};
> +
> +			reg_aldo2: aldo2 {
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vcc33-audio-tv-ephy-mac";
> +			};
> +
> +			/* ALDO3 is shorted to CLDO1 */
> +			reg_aldo3: aldo3 {
> +				regulator-always-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;cl
> +				regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-1";
> +			};
> +
> +			reg_bldo1: bldo1 {
> +				regulator-always-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-name = "vcc18-dram-bias-pll";
> +			};
> +
> +			reg_bldo2: bldo2 {
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-name = "vcc-efuse-pcie-hdmi-pc";
> +			};
> +
> +			reg_bldo3: bldo3 {
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-name = "vcc-pm-pg-dcxoio-wifi";

As you mention in the name, this rail is connected to VCC_DCXO, which IIUC
is an essential supply, for the crystal oscillator circuit. So I think this
needs to be always on?

> +			};
> +
> +			bldo4 {
> +				/* unused */
> +			};
> +
> +			reg_cldo1: cldo1 {
> +				regulator-always-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-2";
> +			};
> +
> +			cldo2 {
> +				/* unused */
> +			};
> +
> +			cldo3 {
> +				/* unused */
> +			};
> +
> +			reg_dcdca: dcdca {
> +				regulator-always-on;
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <1160000>;
> +				regulator-ramp-delay = <2500>;
> +				regulator-name = "vdd-cpu";
> +			};

Can you maybe add a comment here to say that DCDCB is polyphased to DCDCA?

I went through the whole rest and compared against the schematic
(looking at GPIOs and power rails), and that looks OK from what I can see.

Thanks,
Andre


> +
> +			reg_dcdcc: dcdcc {
> +				regulator-enable-ramp-delay = <32000>;
> +				regulator-min-microvolt = <810000>;
> +				regulator-max-microvolt = <1080000>;
> +				regulator-ramp-delay = <2500>;
> +				regulator-name = "vdd-gpu";
> +			};
> +
> +			reg_dcdcd: dcdcd {
> +				regulator-always-on;
> +				regulator-min-microvolt = <960000>;
> +				regulator-max-microvolt = <960000>;
> +				regulator-name = "vdd-sys";
> +			};
> +
> +			reg_dcdce: dcdce {
> +				regulator-always-on;
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-name = "vcc-dram";
> +			};
> +
> +			sw {
> +				/* unused */
> +			};
> +		};
> +	};
> +};
> +
> +&rtc {
> +	clocks = <&ext_osc32k>;
> +};
> +
> +&uart0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart0_ph_pins>;
> +	status = "okay";
> +};
> +
> +&usb2otg {
> +	dr_mode = "host";
> +	status = "okay";
> +};
> +
> +&usb2phy {
> +	usb0_id_det-gpios = <&pio 2 15 GPIO_ACTIVE_HIGH>; /* PC15 */
> +	usb0_vbus-supply = <&reg_vcc5v>;
> +	usb3_vbus-supply = <&reg_vcc5v>;
> +	status = "okay";
> +};
> +
> +&usb3phy {
> +	status = "okay";
> +};


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-25 12:54   ` Andre Przywara
@ 2025-04-25 14:37     ` Chen-Yu Tsai
  2025-04-25 15:34     ` Andrew Lunn
  2025-04-26 18:26     ` Jernej Škrabec
  2 siblings, 0 replies; 18+ messages in thread
From: Chen-Yu Tsai @ 2025-04-25 14:37 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Jernej Skrabec, robh, krzk+dt, conor+dt, samuel, devicetree,
	linux-arm-kernel, linux-sunxi, linux-kernel

On Fri, Apr 25, 2025 at 8:54 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Sun, 13 Apr 2025 15:42:57 +0200
> Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
>
> Hi Jernej,
>
> thanks for sending this, I now went through this in more detail and
> compared against the schematic, so some more nits below.
> I added the two comments from my other email before, so you can ignore that
> one now.
>
> > OrangePi 3 LTS is quite similar to original OrangePi 3, but it has a lot
> > small changes that makes DT sharing unpractical with it.
> >
> > OrangePi 3 LTS has following features:
> > - Allwinner H6 quad-core 64-bit ARM Cortex-A53
> > - GPU Mali-T720
> > - 2 GB LPDDR3 RAM
> > - AXP805 PMIC
> > - AW859A Wifi/BT 5.0
> > - 2x USB 2.0 host port (A)
> > - USB 3.0 Host
> > - Gigabit Ethernet (Motorcomm YT8531C phy)
> > - HDMI 2.0 port
> > - soldered 8 GB eMMC
> > - 2x LED
> > - microphone
> > - audio jack
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > ---
> >  arch/arm64/boot/dts/allwinner/Makefile        |   1 +
> >  .../allwinner/sun50i-h6-orangepi-3-lts.dts    | 351 ++++++++++++++++++
> >  2 files changed, 352 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> > index 00bed412ee31..72c43bd0e2ab 100644
> > --- a/arch/arm64/boot/dts/allwinner/Makefile
> > +++ b/arch/arm64/boot/dts/allwinner/Makefile
> > @@ -33,6 +33,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus2.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-beelink-gs1.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3.dtb
> > +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3-lts.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-lite2.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-one-plus.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64.dtb
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts
> > new file mode 100644
> > index 000000000000..c8830d5c2f09
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts
> > @@ -0,0 +1,351 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +// Copyright (C) 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
> > +// Based on sun50i-h6-orangepi-3.dts, which is:
> > +// Copyright (C) 2019 Ondřej Jirman <megous@megous.com>
> > +
> > +/dts-v1/;
> > +
> > +#include "sun50i-h6.dtsi"
> > +#include "sun50i-h6-cpu-opp.dtsi"
> > +#include "sun50i-h6-gpu-opp.dtsi"
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/leds/common.h>
> > +
> > +/ {
> > +     model = "OrangePi 3 LTS";
> > +     compatible = "xunlong,orangepi-3-lts", "allwinner,sun50i-h6";
> > +
> > +     aliases {
> > +             ethernet0 = &emac;
> > +             ethernet1 = &aw859a;
> > +             serial0 = &uart0;
> > +     };
> > +
> > +     chosen {
> > +             stdout-path = "serial0:115200n8";
> > +     };
> > +
> > +     connector {
> > +             compatible = "hdmi-connector";
> > +             ddc-en-gpios = <&pio 7 2 GPIO_ACTIVE_HIGH>; /* PH2 */
> > +             type = "a";
> > +
> > +             port {
> > +                     hdmi_con_in: endpoint {
> > +                             remote-endpoint = <&hdmi_out_con>;
> > +                     };
> > +             };
> > +     };
> > +
> > +     ext_osc32k: ext_osc32k_clk {
>
> For the sake of completeness, as mentioned in the other mail, I think we
> want dashes in the node name.
>
> > +             #clock-cells = <0>;
> > +             compatible = "fixed-clock";
> > +             clock-frequency = <32768>;
> > +             clock-output-names = "ext_osc32k";
>
> Should the output name also contain dashes instead?
>
> > +     };
> > +
> > +     leds {
> > +             compatible = "gpio-leds";
> > +
> > +             led-0 {
> > +                     function = LED_FUNCTION_POWER;
> > +                     color = <LED_COLOR_ID_RED>;
> > +                     gpios = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */
> > +                     default-state = "on";
>
> Maybe something for a follow up patch, but I noticed that the schematic
> does not show current limiting resistors for the LEDs. This probably works
> because the default drive strength is 0b01, so level 1 (in a range from 0
> to 3, which we map to 10, 20, 30, 40 mA). The exact LED is not mentioned,
> but I would imagine that more than 20 mA would not be healthy, and even
> this might be a stretch over longer times.
>
> So should we force the drive-strength to <10> or <20> somewhere? How does
> this even work for those gpios references?

I think it's possible to have a pinctrl setting node without the
"function" property? That way the pin config settings are applied,
but no function is muxed, thereby not conflicting with GPIO usage.

I've not actually tried it though.

This is also why I didn't want new pinctrl bindings that "force"
one to mux the pin.

ChenYu

> > +             };
> > +
> > +             led-1 {
> > +                     function = LED_FUNCTION_STATUS;
> > +                     color = <LED_COLOR_ID_GREEN>;
> > +                     gpios = <&r_pio 0 7 GPIO_ACTIVE_HIGH>; /* PL7 */
> > +             };
> > +     };
> > +
> > +     reg_gmac_3v3: gmac-3v3 {
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "gmac-3v3";
> > +             regulator-min-microvolt = <3300000>;
> > +             regulator-max-microvolt = <3300000>;
> > +             startup-delay-us = <150000>;
> > +             enable-active-high;
> > +             gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
>
> Can you please add a "vin-supply = <&reg_vcc5v>;" line here, to chain them
> up nicely?
>
> > +     };
> > +
> > +     reg_vcc5v: vcc5v {
> > +             /* board wide 5V supply directly from the DC jack */
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "vcc-5v";
> > +             regulator-min-microvolt = <5000000>;
> > +             regulator-max-microvolt = <5000000>;
> > +             regulator-always-on;
> > +     };
> > +
> > +     reg_wifi_3v3: wifi-3v3 {
> > +             /* 3.3V regulator for WiFi and BT */
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "wifi-3v3";
> > +             regulator-min-microvolt = <3300000>;
> > +             regulator-max-microvolt = <3300000>;
> > +             enable-active-high;
> > +             gpio = <&pio 7 7 GPIO_ACTIVE_HIGH>; /* PH7 */
>
> Same vin-supply here, please, this discrete regulator is fed by DCIN 5V.
>
> > +     };
> > +
> > +     wifi_pwrseq: wifi-pwrseq {
> > +             compatible = "mmc-pwrseq-simple";
> > +             clocks = <&rtc 1>;
>
> As mentioned yesterday, please use CLK_OSC32K_FANOUT.
>
> > +             clock-names = "ext_clock";
> > +             reset-gpios = <&r_pio 1 3 GPIO_ACTIVE_LOW>; /* PM3 */
> > +             post-power-on-delay-ms = <200>;
> > +     };
> > +};
> > +
> > +&cpu0 {
> > +     cpu-supply = <&reg_dcdca>;
> > +};
> > +
> > +&de {
> > +     status = "okay";
> > +};
> > +
> > +&dwc3 {
> > +     status = "okay";
> > +};
> > +
> > +&ehci0 {
> > +     status = "okay";
> > +};
> > +
> > +&ehci3 {
> > +     status = "okay";
> > +};
> > +
> > +&emac {
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&ext_rgmii_pins>;
> > +     phy-mode = "rgmii-rxid";
>
> So relating to what Andrew said earlier today, should this read rgmii-id
> instead? Since the strap resistors just set some boot-up value, but we
> want the PHY driver to enable both RX and TX delay programmatically?
>
> > +     phy-handle = <&ext_rgmii_phy>;
> > +     phy-supply = <&reg_gmac_3v3>;
> > +     allwinner,rx-delay-ps = <0>;
> > +     allwinner,tx-delay-ps = <700>;
> > +     status = "okay";
> > +};
> > +
> > +&gpu {
> > +     mali-supply = <&reg_dcdcc>;
> > +     status = "okay";
> > +};
> > +
> > +&hdmi {
> > +     hvcc-supply = <&reg_bldo2>;
> > +     status = "okay";
> > +};
> > +
> > +&hdmi_out {
> > +     hdmi_out_con: endpoint {
> > +             remote-endpoint = <&hdmi_con_in>;
> > +     };
> > +};
> > +
> > +&mdio {
> > +     ext_rgmii_phy: ethernet-phy@1 {
> > +             compatible = "ethernet-phy-ieee802.3-c22";
> > +             reg = <1>;
> > +
> > +             motorcomm,clk-out-frequency-hz = <125000000>;
> > +
> > +             reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* PD14 */
> > +             reset-assert-us = <15000>;
> > +             reset-deassert-us = <100000>;
> > +     };
> > +};
> > +
> > +&mmc0 {
> > +     vmmc-supply = <&reg_cldo1>;
> > +     cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
> > +     disable-wp;
> > +     bus-width = <4>;
> > +     status = "okay";
> > +};
> > +
> > +&mmc1 {
> > +     vmmc-supply = <&reg_wifi_3v3>;
> > +     vqmmc-supply = <&reg_bldo3>;
> > +     mmc-pwrseq = <&wifi_pwrseq>;
> > +     bus-width = <4>;
> > +     non-removable;
> > +     status = "okay";
> > +
> > +     aw859a: wifi@1 {
> > +             reg = <1>;
> > +     };
> > +};
> > +
> > +&mmc2 {
> > +     vmmc-supply = <&reg_cldo1>;
> > +     vqmmc-supply = <&reg_bldo2>;
> > +     cap-mmc-hw-reset;
> > +     non-removable;
> > +     bus-width = <8>;
> > +     status = "okay";
>
> Given that it's 1.8V on the I/O lines, I think we would need the
> mmc-ddr-1_8v and mmc-hs200-1_8v properties, for higher speed modes? Or
> does that not work?
>
> > +};
> > +
> > +&ohci0 {
> > +     status = "okay";
> > +};
> > +
> > +&ohci3 {
> > +     status = "okay";
> > +};
> > +
> > +&pio {
> > +     vcc-pc-supply = <&reg_bldo2>;
> > +     vcc-pd-supply = <&reg_cldo1>;
> > +     vcc-pg-supply = <&reg_bldo3>;
> > +};
> > +
> > +&r_ir {
> > +     status = "okay";
> > +};
> > +
> > +&r_i2c {
> > +     status = "okay";
> > +
> > +     axp805: pmic@36 {
> > +             compatible = "x-powers,axp805", "x-powers,axp806";
> > +             reg = <0x36>;
> > +             interrupt-parent = <&r_intc>;
> > +             interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_LOW>;
> > +             interrupt-controller;
> > +             #interrupt-cells = <1>;
> > +             x-powers,self-working-mode;
> > +             vina-supply = <&reg_vcc5v>;
> > +             vinb-supply = <&reg_vcc5v>;
> > +             vinc-supply = <&reg_vcc5v>;
> > +             vind-supply = <&reg_vcc5v>;
> > +             vine-supply = <&reg_vcc5v>;
> > +             aldoin-supply = <&reg_vcc5v>;
> > +             bldoin-supply = <&reg_vcc5v>;
> > +             cldoin-supply = <&reg_vcc5v>;
> > +
> > +             regulators {
> > +                     reg_aldo1: aldo1 {
> > +                             regulator-always-on;
> > +                             regulator-min-microvolt = <3300000>;
> > +                             regulator-max-microvolt = <3300000>;
> > +                             regulator-name = "vcc-pl-led-ir";
> > +                     };
> > +
> > +                     reg_aldo2: aldo2 {
> > +                             regulator-min-microvolt = <3300000>;
> > +                             regulator-max-microvolt = <3300000>;
> > +                             regulator-name = "vcc33-audio-tv-ephy-mac";
> > +                     };
> > +
> > +                     /* ALDO3 is shorted to CLDO1 */
> > +                     reg_aldo3: aldo3 {
> > +                             regulator-always-on;
> > +                             regulator-min-microvolt = <3300000>;
> > +                             regulator-max-microvolt = <3300000>;cl
> > +                             regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-1";
> > +                     };
> > +
> > +                     reg_bldo1: bldo1 {
> > +                             regulator-always-on;
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +                             regulator-name = "vcc18-dram-bias-pll";
> > +                     };
> > +
> > +                     reg_bldo2: bldo2 {
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +                             regulator-name = "vcc-efuse-pcie-hdmi-pc";
> > +                     };
> > +
> > +                     reg_bldo3: bldo3 {
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +                             regulator-name = "vcc-pm-pg-dcxoio-wifi";
>
> As you mention in the name, this rail is connected to VCC_DCXO, which IIUC
> is an essential supply, for the crystal oscillator circuit. So I think this
> needs to be always on?
>
> > +                     };
> > +
> > +                     bldo4 {
> > +                             /* unused */
> > +                     };
> > +
> > +                     reg_cldo1: cldo1 {
> > +                             regulator-always-on;
> > +                             regulator-min-microvolt = <3300000>;
> > +                             regulator-max-microvolt = <3300000>;
> > +                             regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-2";
> > +                     };
> > +
> > +                     cldo2 {
> > +                             /* unused */
> > +                     };
> > +
> > +                     cldo3 {
> > +                             /* unused */
> > +                     };
> > +
> > +                     reg_dcdca: dcdca {
> > +                             regulator-always-on;
> > +                             regulator-min-microvolt = <800000>;
> > +                             regulator-max-microvolt = <1160000>;
> > +                             regulator-ramp-delay = <2500>;
> > +                             regulator-name = "vdd-cpu";
> > +                     };
>
> Can you maybe add a comment here to say that DCDCB is polyphased to DCDCA?
>
> I went through the whole rest and compared against the schematic
> (looking at GPIOs and power rails), and that looks OK from what I can see.
>
> Thanks,
> Andre
>
>
> > +
> > +                     reg_dcdcc: dcdcc {
> > +                             regulator-enable-ramp-delay = <32000>;
> > +                             regulator-min-microvolt = <810000>;
> > +                             regulator-max-microvolt = <1080000>;
> > +                             regulator-ramp-delay = <2500>;
> > +                             regulator-name = "vdd-gpu";
> > +                     };
> > +
> > +                     reg_dcdcd: dcdcd {
> > +                             regulator-always-on;
> > +                             regulator-min-microvolt = <960000>;
> > +                             regulator-max-microvolt = <960000>;
> > +                             regulator-name = "vdd-sys";
> > +                     };
> > +
> > +                     reg_dcdce: dcdce {
> > +                             regulator-always-on;
> > +                             regulator-min-microvolt = <1200000>;
> > +                             regulator-max-microvolt = <1200000>;
> > +                             regulator-name = "vcc-dram";
> > +                     };
> > +
> > +                     sw {
> > +                             /* unused */
> > +                     };
> > +             };
> > +     };
> > +};
> > +
> > +&rtc {
> > +     clocks = <&ext_osc32k>;
> > +};
> > +
> > +&uart0 {
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&uart0_ph_pins>;
> > +     status = "okay";
> > +};
> > +
> > +&usb2otg {
> > +     dr_mode = "host";
> > +     status = "okay";
> > +};
> > +
> > +&usb2phy {
> > +     usb0_id_det-gpios = <&pio 2 15 GPIO_ACTIVE_HIGH>; /* PC15 */
> > +     usb0_vbus-supply = <&reg_vcc5v>;
> > +     usb3_vbus-supply = <&reg_vcc5v>;
> > +     status = "okay";
> > +};
> > +
> > +&usb3phy {
> > +     status = "okay";
> > +};
>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-25 12:54   ` Andre Przywara
  2025-04-25 14:37     ` Chen-Yu Tsai
@ 2025-04-25 15:34     ` Andrew Lunn
  2025-04-26 18:00       ` Jernej Škrabec
  2025-04-26 18:26     ` Jernej Škrabec
  2 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2025-04-25 15:34 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Jernej Skrabec, robh, krzk+dt, conor+dt, wens, samuel, devicetree,
	linux-arm-kernel, linux-sunxi, linux-kernel

> > +&emac {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&ext_rgmii_pins>;
> > +	phy-mode = "rgmii-rxid";
> 
> So relating to what Andrew said earlier today, should this read rgmii-id
> instead? Since the strap resistors just set some boot-up value, but we
> want the PHY driver to enable both RX and TX delay programmatically?

Yes.

There is a checkpatch.pl patch working its way through the system
which will add warning about any rgmii value other than rgmii-id. Such
values need a comment that the PCB has extra long clock
lines. Hopefully that will make people actually stop and think about
this, rather than just copy broken vendor code.

	Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-25 15:34     ` Andrew Lunn
@ 2025-04-26 18:00       ` Jernej Škrabec
  2025-04-28 12:37         ` Andrew Lunn
  0 siblings, 1 reply; 18+ messages in thread
From: Jernej Škrabec @ 2025-04-26 18:00 UTC (permalink / raw)
  To: Andre Przywara, Andrew Lunn
  Cc: robh, krzk+dt, conor+dt, wens, samuel, devicetree,
	linux-arm-kernel, linux-sunxi, linux-kernel

Dne petek, 25. april 2025 ob 17:34:14 Srednjeevropski poletni čas je Andrew Lunn napisal(a):
> > > +&emac {
> > > +	pinctrl-names = "default";
> > > +	pinctrl-0 = <&ext_rgmii_pins>;
> > > +	phy-mode = "rgmii-rxid";
> > 
> > So relating to what Andrew said earlier today, should this read rgmii-id
> > instead? Since the strap resistors just set some boot-up value, but we
> > want the PHY driver to enable both RX and TX delay programmatically?
> 
> Yes.
> 
> There is a checkpatch.pl patch working its way through the system
> which will add warning about any rgmii value other than rgmii-id. Such
> values need a comment that the PCB has extra long clock
> lines. Hopefully that will make people actually stop and think about
> this, rather than just copy broken vendor code.

I spent quite some time working on ethernet support for this board. Once
I've found PHY datasheet, I confirmed that there is added delay. So this
particular board needs "rgmii-rxid" mode.

Best regards,
Jernej



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-25 12:54   ` Andre Przywara
  2025-04-25 14:37     ` Chen-Yu Tsai
  2025-04-25 15:34     ` Andrew Lunn
@ 2025-04-26 18:26     ` Jernej Škrabec
  2 siblings, 0 replies; 18+ messages in thread
From: Jernej Škrabec @ 2025-04-26 18:26 UTC (permalink / raw)
  To: Andre Przywara
  Cc: robh, krzk+dt, conor+dt, wens, samuel, devicetree,
	linux-arm-kernel, linux-sunxi, linux-kernel

Dne petek, 25. april 2025 ob 14:54:29 Srednjeevropski poletni čas je Andre Przywara napisal(a):
> On Sun, 13 Apr 2025 15:42:57 +0200
> Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
> 
> Hi Jernej,
> 
> thanks for sending this, I now went through this in more detail and
> compared against the schematic, so some more nits below.
> I added the two comments from my other email before, so you can ignore that
> one now.
> 
> > OrangePi 3 LTS is quite similar to original OrangePi 3, but it has a lot
> > small changes that makes DT sharing unpractical with it.
> > 
> > OrangePi 3 LTS has following features:
> > - Allwinner H6 quad-core 64-bit ARM Cortex-A53
> > - GPU Mali-T720
> > - 2 GB LPDDR3 RAM
> > - AXP805 PMIC
> > - AW859A Wifi/BT 5.0
> > - 2x USB 2.0 host port (A)
> > - USB 3.0 Host
> > - Gigabit Ethernet (Motorcomm YT8531C phy)
> > - HDMI 2.0 port
> > - soldered 8 GB eMMC
> > - 2x LED
> > - microphone
> > - audio jack
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > ---
> >  arch/arm64/boot/dts/allwinner/Makefile        |   1 +
> >  .../allwinner/sun50i-h6-orangepi-3-lts.dts    | 351 ++++++++++++++++++
> >  2 files changed, 352 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts
> > 
> > diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> > index 00bed412ee31..72c43bd0e2ab 100644
> > --- a/arch/arm64/boot/dts/allwinner/Makefile
> > +++ b/arch/arm64/boot/dts/allwinner/Makefile
> > @@ -33,6 +33,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus2.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-beelink-gs1.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3.dtb
> > +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3-lts.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-lite2.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-one-plus.dtb
> >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64.dtb
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts
> > new file mode 100644
> > index 000000000000..c8830d5c2f09
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3-lts.dts
> > @@ -0,0 +1,351 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +// Copyright (C) 2025 Jernej Skrabec <jernej.skrabec@gmail.com>
> > +// Based on sun50i-h6-orangepi-3.dts, which is:
> > +// Copyright (C) 2019 Ondřej Jirman <megous@megous.com>
> > +
> > +/dts-v1/;
> > +
> > +#include "sun50i-h6.dtsi"
> > +#include "sun50i-h6-cpu-opp.dtsi"
> > +#include "sun50i-h6-gpu-opp.dtsi"
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/leds/common.h>
> > +
> > +/ {
> > +	model = "OrangePi 3 LTS";
> > +	compatible = "xunlong,orangepi-3-lts", "allwinner,sun50i-h6";
> > +
> > +	aliases {
> > +		ethernet0 = &emac;
> > +		ethernet1 = &aw859a;
> > +		serial0 = &uart0;
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +	};
> > +
> > +	connector {
> > +		compatible = "hdmi-connector";
> > +		ddc-en-gpios = <&pio 7 2 GPIO_ACTIVE_HIGH>; /* PH2 */
> > +		type = "a";
> > +
> > +		port {
> > +			hdmi_con_in: endpoint {
> > +				remote-endpoint = <&hdmi_out_con>;
> > +			};
> > +		};
> > +	};
> > +
> > +	ext_osc32k: ext_osc32k_clk {
> 
> For the sake of completeness, as mentioned in the other mail, I think we
> want dashes in the node name.

Right.

> 
> > +		#clock-cells = <0>;
> > +		compatible = "fixed-clock";
> > +		clock-frequency = <32768>;
> > +		clock-output-names = "ext_osc32k";
> 
> Should the output name also contain dashes instead?
> 
> > +	};
> > +
> > +	leds {
> > +		compatible = "gpio-leds";
> > +
> > +		led-0 {
> > +			function = LED_FUNCTION_POWER;
> > +			color = <LED_COLOR_ID_RED>;
> > +			gpios = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */
> > +			default-state = "on";
> 
> Maybe something for a follow up patch, but I noticed that the schematic
> does not show current limiting resistors for the LEDs. This probably works
> because the default drive strength is 0b01, so level 1 (in a range from 0
> to 3, which we map to 10, 20, 30, 40 mA). The exact LED is not mentioned,
> but I would imagine that more than 20 mA would not be healthy, and even
> this might be a stretch over longer times.
> 
> So should we force the drive-strength to <10> or <20> somewhere? How does
> this even work for those gpios references?

Interestingly, this was already done for "allwinner,auxtek-t003" board and
others with strength 20. I doubt that's 20 mA, since that would certainly
kill LED.

> 
> > +		};
> > +
> > +		led-1 {
> > +			function = LED_FUNCTION_STATUS;
> > +			color = <LED_COLOR_ID_GREEN>;
> > +			gpios = <&r_pio 0 7 GPIO_ACTIVE_HIGH>; /* PL7 */
> > +		};
> > +	};
> > +
> > +	reg_gmac_3v3: gmac-3v3 {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "gmac-3v3";
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		startup-delay-us = <150000>;
> > +		enable-active-high;
> > +		gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
> 
> Can you please add a "vin-supply = <&reg_vcc5v>;" line here, to chain them
> up nicely?

Ok.

> 
> > +	};
> > +
> > +	reg_vcc5v: vcc5v {
> > +		/* board wide 5V supply directly from the DC jack */
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "vcc-5v";
> > +		regulator-min-microvolt = <5000000>;
> > +		regulator-max-microvolt = <5000000>;
> > +		regulator-always-on;
> > +	};
> > +
> > +	reg_wifi_3v3: wifi-3v3 {
> > +		/* 3.3V regulator for WiFi and BT */
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "wifi-3v3";
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		enable-active-high;
> > +		gpio = <&pio 7 7 GPIO_ACTIVE_HIGH>; /* PH7 */
> 
> Same vin-supply here, please, this discrete regulator is fed by DCIN 5V.
> 
> > +	};
> > +
> > +	wifi_pwrseq: wifi-pwrseq {
> > +		compatible = "mmc-pwrseq-simple";
> > +		clocks = <&rtc 1>;
> 
> As mentioned yesterday, please use CLK_OSC32K_FANOUT.

good catch.

> 
> > +		clock-names = "ext_clock";
> > +		reset-gpios = <&r_pio 1 3 GPIO_ACTIVE_LOW>; /* PM3 */
> > +		post-power-on-delay-ms = <200>;
> > +	};
> > +};
> > +
> > +&cpu0 {
> > +	cpu-supply = <&reg_dcdca>;
> > +};
> > +
> > +&de {
> > +	status = "okay";
> > +};
> > +
> > +&dwc3 {
> > +	status = "okay";
> > +};
> > +
> > +&ehci0 {
> > +	status = "okay";
> > +};
> > +
> > +&ehci3 {
> > +	status = "okay";
> > +};
> > +
> > +&emac {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&ext_rgmii_pins>;
> > +	phy-mode = "rgmii-rxid";
> 
> So relating to what Andrew said earlier today, should this read rgmii-id
> instead? Since the strap resistors just set some boot-up value, but we
> want the PHY driver to enable both RX and TX delay programmatically?

As explained in other email, no. This setting reflects strap resistors
and that extra delay is needed. PHY driver sets this delay based on
this mode.

> 
> > +	phy-handle = <&ext_rgmii_phy>;
> > +	phy-supply = <&reg_gmac_3v3>;
> > +	allwinner,rx-delay-ps = <0>;
> > +	allwinner,tx-delay-ps = <700>;
> > +	status = "okay";
> > +};
> > +
> > +&gpu {
> > +	mali-supply = <&reg_dcdcc>;
> > +	status = "okay";
> > +};
> > +
> > +&hdmi {
> > +	hvcc-supply = <&reg_bldo2>;
> > +	status = "okay";
> > +};
> > +
> > +&hdmi_out {
> > +	hdmi_out_con: endpoint {
> > +		remote-endpoint = <&hdmi_con_in>;
> > +	};
> > +};
> > +
> > +&mdio {
> > +	ext_rgmii_phy: ethernet-phy@1 {
> > +		compatible = "ethernet-phy-ieee802.3-c22";
> > +		reg = <1>;
> > +
> > +		motorcomm,clk-out-frequency-hz = <125000000>;
> > +
> > +		reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* PD14 */
> > +		reset-assert-us = <15000>;
> > +		reset-deassert-us = <100000>;
> > +	};
> > +};
> > +
> > +&mmc0 {
> > +	vmmc-supply = <&reg_cldo1>;
> > +	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
> > +	disable-wp;
> > +	bus-width = <4>;
> > +	status = "okay";
> > +};
> > +
> > +&mmc1 {
> > +	vmmc-supply = <&reg_wifi_3v3>;
> > +	vqmmc-supply = <&reg_bldo3>;
> > +	mmc-pwrseq = <&wifi_pwrseq>;
> > +	bus-width = <4>;
> > +	non-removable;
> > +	status = "okay";
> > +
> > +	aw859a: wifi@1 {
> > +		reg = <1>;
> > +	};
> > +};
> > +
> > +&mmc2 {
> > +	vmmc-supply = <&reg_cldo1>;
> > +	vqmmc-supply = <&reg_bldo2>;
> > +	cap-mmc-hw-reset;
> > +	non-removable;
> > +	bus-width = <8>;
> > +	status = "okay";
> 
> Given that it's 1.8V on the I/O lines, I think we would need the
> mmc-ddr-1_8v and mmc-hs200-1_8v properties, for higher speed modes? Or
> does that not work?

It's untested so I wouldn't risk it now.

> 
> > +};
> > +
> > +&ohci0 {
> > +	status = "okay";
> > +};
> > +
> > +&ohci3 {
> > +	status = "okay";
> > +};
> > +
> > +&pio {
> > +	vcc-pc-supply = <&reg_bldo2>;
> > +	vcc-pd-supply = <&reg_cldo1>;
> > +	vcc-pg-supply = <&reg_bldo3>;
> > +};
> > +
> > +&r_ir {
> > +	status = "okay";
> > +};
> > +
> > +&r_i2c {
> > +	status = "okay";
> > +
> > +	axp805: pmic@36 {
> > +		compatible = "x-powers,axp805", "x-powers,axp806";
> > +		reg = <0x36>;
> > +		interrupt-parent = <&r_intc>;
> > +		interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_LOW>;
> > +		interrupt-controller;
> > +		#interrupt-cells = <1>;
> > +		x-powers,self-working-mode;
> > +		vina-supply = <&reg_vcc5v>;
> > +		vinb-supply = <&reg_vcc5v>;
> > +		vinc-supply = <&reg_vcc5v>;
> > +		vind-supply = <&reg_vcc5v>;
> > +		vine-supply = <&reg_vcc5v>;
> > +		aldoin-supply = <&reg_vcc5v>;
> > +		bldoin-supply = <&reg_vcc5v>;
> > +		cldoin-supply = <&reg_vcc5v>;
> > +
> > +		regulators {
> > +			reg_aldo1: aldo1 {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-name = "vcc-pl-led-ir";
> > +			};
> > +
> > +			reg_aldo2: aldo2 {
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-name = "vcc33-audio-tv-ephy-mac";
> > +			};
> > +
> > +			/* ALDO3 is shorted to CLDO1 */
> > +			reg_aldo3: aldo3 {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;cl
> > +				regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-1";
> > +			};
> > +
> > +			reg_bldo1: bldo1 {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +				regulator-name = "vcc18-dram-bias-pll";
> > +			};
> > +
> > +			reg_bldo2: bldo2 {
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +				regulator-name = "vcc-efuse-pcie-hdmi-pc";
> > +			};
> > +
> > +			reg_bldo3: bldo3 {
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +				regulator-name = "vcc-pm-pg-dcxoio-wifi";
> 
> As you mention in the name, this rail is connected to VCC_DCXO, which IIUC
> is an essential supply, for the crystal oscillator circuit. So I think this
> needs to be always on?

It most likely is enabled from boot and since it powers pin banks, it's
always enabled. I'll add it anyway, just to make it more explicit.

> 
> > +			};
> > +
> > +			bldo4 {
> > +				/* unused */
> > +			};
> > +
> > +			reg_cldo1: cldo1 {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +				regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-2";
> > +			};
> > +
> > +			cldo2 {
> > +				/* unused */
> > +			};
> > +
> > +			cldo3 {
> > +				/* unused */
> > +			};
> > +
> > +			reg_dcdca: dcdca {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <800000>;
> > +				regulator-max-microvolt = <1160000>;
> > +				regulator-ramp-delay = <2500>;
> > +				regulator-name = "vdd-cpu";
> > +			};
> 
> Can you maybe add a comment here to say that DCDCB is polyphased to DCDCA?

sure.

> 
> I went through the whole rest and compared against the schematic
> (looking at GPIOs and power rails), and that looks OK from what I can see.

Thanks for checking!

Best regards,
Jernej

> 
> Thanks,
> Andre
> 
> 
> > +
> > +			reg_dcdcc: dcdcc {
> > +				regulator-enable-ramp-delay = <32000>;
> > +				regulator-min-microvolt = <810000>;
> > +				regulator-max-microvolt = <1080000>;
> > +				regulator-ramp-delay = <2500>;
> > +				regulator-name = "vdd-gpu";
> > +			};
> > +
> > +			reg_dcdcd: dcdcd {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <960000>;
> > +				regulator-max-microvolt = <960000>;
> > +				regulator-name = "vdd-sys";
> > +			};
> > +
> > +			reg_dcdce: dcdce {
> > +				regulator-always-on;
> > +				regulator-min-microvolt = <1200000>;
> > +				regulator-max-microvolt = <1200000>;
> > +				regulator-name = "vcc-dram";
> > +			};
> > +
> > +			sw {
> > +				/* unused */
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&rtc {
> > +	clocks = <&ext_osc32k>;
> > +};
> > +
> > +&uart0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart0_ph_pins>;
> > +	status = "okay";
> > +};
> > +
> > +&usb2otg {
> > +	dr_mode = "host";
> > +	status = "okay";
> > +};
> > +
> > +&usb2phy {
> > +	usb0_id_det-gpios = <&pio 2 15 GPIO_ACTIVE_HIGH>; /* PC15 */
> > +	usb0_vbus-supply = <&reg_vcc5v>;
> > +	usb3_vbus-supply = <&reg_vcc5v>;
> > +	status = "okay";
> > +};
> > +
> > +&usb3phy {
> > +	status = "okay";
> > +};
> 
> 





^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-26 18:00       ` Jernej Škrabec
@ 2025-04-28 12:37         ` Andrew Lunn
  2025-04-29 14:51           ` Jernej Škrabec
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2025-04-28 12:37 UTC (permalink / raw)
  To: Jernej Škrabec
  Cc: Andre Przywara, robh, krzk+dt, conor+dt, wens, samuel, devicetree,
	linux-arm-kernel, linux-sunxi, linux-kernel

On Sat, Apr 26, 2025 at 08:00:49PM +0200, Jernej Škrabec wrote:
> Dne petek, 25. april 2025 ob 17:34:14 Srednjeevropski poletni čas je Andrew Lunn napisal(a):
> > > > +&emac {
> > > > +	pinctrl-names = "default";
> > > > +	pinctrl-0 = <&ext_rgmii_pins>;
> > > > +	phy-mode = "rgmii-rxid";
> > > 
> > > So relating to what Andrew said earlier today, should this read rgmii-id
> > > instead? Since the strap resistors just set some boot-up value, but we
> > > want the PHY driver to enable both RX and TX delay programmatically?
> > 
> > Yes.
> > 
> > There is a checkpatch.pl patch working its way through the system
> > which will add warning about any rgmii value other than rgmii-id. Such
> > values need a comment that the PCB has extra long clock
> > lines. Hopefully that will make people actually stop and think about
> > this, rather than just copy broken vendor code.
> 
> I spent quite some time working on ethernet support for this board. Once
> I've found PHY datasheet, I confirmed that there is added delay. So this
> particular board needs "rgmii-rxid" mode.

There have been numerous discussions about what these rgmii modes
mean, because DT developers frequently get them wrong.

Does the PCB have an extra long clock line in the TX direction? That
is what rgmii-rxid means, the PCB is providing the TX delay, the
MAC/PHY pair needs to add the RX delay.

Ignore strapping. That is just a power on default which gets over
ridden once the PHY driver is running.

What PHY is this?

	Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-28 12:37         ` Andrew Lunn
@ 2025-04-29 14:51           ` Jernej Škrabec
  2025-04-29 15:09             ` Andrew Lunn
  0 siblings, 1 reply; 18+ messages in thread
From: Jernej Škrabec @ 2025-04-29 14:51 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Andre Przywara, robh, krzk+dt, conor+dt, wens, samuel, devicetree,
	linux-arm-kernel, linux-sunxi, linux-kernel

Dne ponedeljek, 28. april 2025 ob 14:37:48 Srednjeevropski poletni čas je Andrew Lunn napisal(a):
> On Sat, Apr 26, 2025 at 08:00:49PM +0200, Jernej Škrabec wrote:
> > Dne petek, 25. april 2025 ob 17:34:14 Srednjeevropski poletni čas je Andrew Lunn napisal(a):
> > > > > +&emac {
> > > > > +	pinctrl-names = "default";
> > > > > +	pinctrl-0 = <&ext_rgmii_pins>;
> > > > > +	phy-mode = "rgmii-rxid";
> > > > 
> > > > So relating to what Andrew said earlier today, should this read rgmii-id
> > > > instead? Since the strap resistors just set some boot-up value, but we
> > > > want the PHY driver to enable both RX and TX delay programmatically?
> > > 
> > > Yes.
> > > 
> > > There is a checkpatch.pl patch working its way through the system
> > > which will add warning about any rgmii value other than rgmii-id. Such
> > > values need a comment that the PCB has extra long clock
> > > lines. Hopefully that will make people actually stop and think about
> > > this, rather than just copy broken vendor code.
> > 
> > I spent quite some time working on ethernet support for this board. Once
> > I've found PHY datasheet, I confirmed that there is added delay. So this
> > particular board needs "rgmii-rxid" mode.
> 
> There have been numerous discussions about what these rgmii modes
> mean, because DT developers frequently get them wrong.
> 
> Does the PCB have an extra long clock line in the TX direction? That
> is what rgmii-rxid means, the PCB is providing the TX delay, the
> MAC/PHY pair needs to add the RX delay.

While schematic is accessible, AFAIK PCB/gerbers are not, so I can't really
tell how long it is. But without this extra delay, ethernet doesn't work.

> 
> Ignore strapping. That is just a power on default which gets over
> ridden once the PHY driver is running.
> 
> What PHY is this?

Motorcomm YT8531C.

Best regards,
Jernej

> 
> 	Andrew
> 





^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-29 14:51           ` Jernej Škrabec
@ 2025-04-29 15:09             ` Andrew Lunn
  2025-04-29 15:24               ` Jernej Škrabec
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2025-04-29 15:09 UTC (permalink / raw)
  To: Jernej Škrabec
  Cc: Andre Przywara, robh, krzk+dt, conor+dt, wens, samuel, devicetree,
	linux-arm-kernel, linux-sunxi, linux-kernel

On Tue, Apr 29, 2025 at 04:51:59PM +0200, Jernej Škrabec wrote:
> Dne ponedeljek, 28. april 2025 ob 14:37:48 Srednjeevropski poletni čas je Andrew Lunn napisal(a):
> > On Sat, Apr 26, 2025 at 08:00:49PM +0200, Jernej Škrabec wrote:
> > > Dne petek, 25. april 2025 ob 17:34:14 Srednjeevropski poletni čas je Andrew Lunn napisal(a):
> > > > > > +&emac {
> > > > > > +	pinctrl-names = "default";
> > > > > > +	pinctrl-0 = <&ext_rgmii_pins>;
> > > > > > +	phy-mode = "rgmii-rxid";
> > > > > 
> > > > > So relating to what Andrew said earlier today, should this read rgmii-id
> > > > > instead? Since the strap resistors just set some boot-up value, but we
> > > > > want the PHY driver to enable both RX and TX delay programmatically?
> > > > 
> > > > Yes.
> > > > 
> > > > There is a checkpatch.pl patch working its way through the system
> > > > which will add warning about any rgmii value other than rgmii-id. Such
> > > > values need a comment that the PCB has extra long clock
> > > > lines. Hopefully that will make people actually stop and think about
> > > > this, rather than just copy broken vendor code.
> > > 
> > > I spent quite some time working on ethernet support for this board. Once
> > > I've found PHY datasheet, I confirmed that there is added delay. So this
> > > particular board needs "rgmii-rxid" mode.
> > 
> > There have been numerous discussions about what these rgmii modes
> > mean, because DT developers frequently get them wrong.
> > 
> > Does the PCB have an extra long clock line in the TX direction? That
> > is what rgmii-rxid means, the PCB is providing the TX delay, the
> > MAC/PHY pair needs to add the RX delay.
> 
> While schematic is accessible, AFAIK PCB/gerbers are not, so I can't really
> tell how long it is. But without this extra delay, ethernet doesn't work.

You are not adding an extra delay, you are subtracting a
delay. 'rgmii-rxid' says the TX delay is implemented by the PCB, hence
the PHY does not need to add the delay.

What is normal is that the PCB adds no delays, and the PHY adds the
delay for both the RX and the TX. And you represent this with
'rgmii-id'.

So what you need to find out is, where does the TX delay come from?

	Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-29 15:09             ` Andrew Lunn
@ 2025-04-29 15:24               ` Jernej Škrabec
  2025-04-29 15:27                 ` Jernej Škrabec
  0 siblings, 1 reply; 18+ messages in thread
From: Jernej Škrabec @ 2025-04-29 15:24 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Andre Przywara, robh, krzk+dt, conor+dt, wens, samuel, devicetree,
	linux-arm-kernel, linux-sunxi, linux-kernel

Dne torek, 29. april 2025 ob 17:09:22 Srednjeevropski poletni čas je Andrew Lunn napisal(a):
> On Tue, Apr 29, 2025 at 04:51:59PM +0200, Jernej Škrabec wrote:
> > Dne ponedeljek, 28. april 2025 ob 14:37:48 Srednjeevropski poletni čas je Andrew Lunn napisal(a):
> > > On Sat, Apr 26, 2025 at 08:00:49PM +0200, Jernej Škrabec wrote:
> > > > Dne petek, 25. april 2025 ob 17:34:14 Srednjeevropski poletni čas je Andrew Lunn napisal(a):
> > > > > > > +&emac {
> > > > > > > +	pinctrl-names = "default";
> > > > > > > +	pinctrl-0 = <&ext_rgmii_pins>;
> > > > > > > +	phy-mode = "rgmii-rxid";
> > > > > > 
> > > > > > So relating to what Andrew said earlier today, should this read rgmii-id
> > > > > > instead? Since the strap resistors just set some boot-up value, but we
> > > > > > want the PHY driver to enable both RX and TX delay programmatically?
> > > > > 
> > > > > Yes.
> > > > > 
> > > > > There is a checkpatch.pl patch working its way through the system
> > > > > which will add warning about any rgmii value other than rgmii-id. Such
> > > > > values need a comment that the PCB has extra long clock
> > > > > lines. Hopefully that will make people actually stop and think about
> > > > > this, rather than just copy broken vendor code.
> > > > 
> > > > I spent quite some time working on ethernet support for this board. Once
> > > > I've found PHY datasheet, I confirmed that there is added delay. So this
> > > > particular board needs "rgmii-rxid" mode.
> > > 
> > > There have been numerous discussions about what these rgmii modes
> > > mean, because DT developers frequently get them wrong.
> > > 
> > > Does the PCB have an extra long clock line in the TX direction? That
> > > is what rgmii-rxid means, the PCB is providing the TX delay, the
> > > MAC/PHY pair needs to add the RX delay.
> > 
> > While schematic is accessible, AFAIK PCB/gerbers are not, so I can't really
> > tell how long it is. But without this extra delay, ethernet doesn't work.
> 
> You are not adding an extra delay, you are subtracting a
> delay. 'rgmii-rxid' says the TX delay is implemented by the PCB, hence
> the PHY does not need to add the delay.
> 
> What is normal is that the PCB adds no delays, and the PHY adds the
> delay for both the RX and the TX. And you represent this with
> 'rgmii-id'.

ok, thanks for explanation.

> 
> So what you need to find out is, where does the TX delay come from?

How to do that? Strapping show me this way and testing confirmed it. Not
sure what more I can do? As a hobbyist, I don't have access to anything more
than schematic.

Best regards,
Jernej



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-29 15:24               ` Jernej Škrabec
@ 2025-04-29 15:27                 ` Jernej Škrabec
  2025-04-29 15:45                   ` Andrew Lunn
  0 siblings, 1 reply; 18+ messages in thread
From: Jernej Škrabec @ 2025-04-29 15:27 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Andre Przywara, robh, krzk+dt, conor+dt, wens, samuel, devicetree,
	linux-arm-kernel, linux-sunxi, linux-kernel

Dne torek, 29. april 2025 ob 17:24:18 Srednjeevropski poletni čas je Jernej Škrabec napisal(a):
> Dne torek, 29. april 2025 ob 17:09:22 Srednjeevropski poletni čas je Andrew Lunn napisal(a):
> > On Tue, Apr 29, 2025 at 04:51:59PM +0200, Jernej Škrabec wrote:
> > > Dne ponedeljek, 28. april 2025 ob 14:37:48 Srednjeevropski poletni čas je Andrew Lunn napisal(a):
> > > > On Sat, Apr 26, 2025 at 08:00:49PM +0200, Jernej Škrabec wrote:
> > > > > Dne petek, 25. april 2025 ob 17:34:14 Srednjeevropski poletni čas je Andrew Lunn napisal(a):
> > > > > > > > +&emac {
> > > > > > > > +	pinctrl-names = "default";
> > > > > > > > +	pinctrl-0 = <&ext_rgmii_pins>;
> > > > > > > > +	phy-mode = "rgmii-rxid";
> > > > > > > 
> > > > > > > So relating to what Andrew said earlier today, should this read rgmii-id
> > > > > > > instead? Since the strap resistors just set some boot-up value, but we
> > > > > > > want the PHY driver to enable both RX and TX delay programmatically?
> > > > > > 
> > > > > > Yes.
> > > > > > 
> > > > > > There is a checkpatch.pl patch working its way through the system
> > > > > > which will add warning about any rgmii value other than rgmii-id. Such
> > > > > > values need a comment that the PCB has extra long clock
> > > > > > lines. Hopefully that will make people actually stop and think about
> > > > > > this, rather than just copy broken vendor code.
> > > > > 
> > > > > I spent quite some time working on ethernet support for this board. Once
> > > > > I've found PHY datasheet, I confirmed that there is added delay. So this
> > > > > particular board needs "rgmii-rxid" mode.
> > > > 
> > > > There have been numerous discussions about what these rgmii modes
> > > > mean, because DT developers frequently get them wrong.
> > > > 
> > > > Does the PCB have an extra long clock line in the TX direction? That
> > > > is what rgmii-rxid means, the PCB is providing the TX delay, the
> > > > MAC/PHY pair needs to add the RX delay.
> > > 
> > > While schematic is accessible, AFAIK PCB/gerbers are not, so I can't really
> > > tell how long it is. But without this extra delay, ethernet doesn't work.
> > 
> > You are not adding an extra delay, you are subtracting a
> > delay. 'rgmii-rxid' says the TX delay is implemented by the PCB, hence
> > the PHY does not need to add the delay.
> > 
> > What is normal is that the PCB adds no delays, and the PHY adds the
> > delay for both the RX and the TX. And you represent this with
> > 'rgmii-id'.
> 
> ok, thanks for explanation.
> 
> > 
> > So what you need to find out is, where does the TX delay come from?
> 
> How to do that? Strapping show me this way and testing confirmed it. Not
> sure what more I can do? As a hobbyist, I don't have access to anything more
> than schematic.

I just to be clear, I tested various combinations, including rgmii-id, and it
didn't work, except rgmii-rxid, which matches strapping. Of course Motorcomm
PHY driver took that into account and set registers accordingly.

Best regards,
Jernej






^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-29 15:27                 ` Jernej Škrabec
@ 2025-04-29 15:45                   ` Andrew Lunn
  2025-04-29 15:52                     ` Chen-Yu Tsai
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2025-04-29 15:45 UTC (permalink / raw)
  To: Jernej Škrabec
  Cc: Andre Przywara, robh, krzk+dt, conor+dt, wens, samuel, devicetree,
	linux-arm-kernel, linux-sunxi, linux-kernel

> I just to be clear, I tested various combinations, including rgmii-id, and it
> didn't work, except rgmii-rxid, which matches strapping. Of course Motorcomm
> PHY driver took that into account and set registers accordingly.

So we have:

&emac {
	pinctrl-names = "default";
	pinctrl-0 = <&ext_rgmii_pins>;
	phy-mode = "rgmii-rxid";
	phy-handle = <&ext_rgmii_phy>;
	phy-supply = <&reg_gmac_3v3>;
	allwinner,rx-delay-ps = <0>;
	allwinner,tx-delay-ps = <700>;
	status = "okay";
};

and

&mdio {
	ext_rgmii_phy: ethernet-phy@1 {
		compatible = "ethernet-phy-ieee802.3-c22";
		reg = <1>;

		motorcomm,clk-out-frequency-hz = <125000000>;

		reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* PD14 */
		reset-assert-us = <15000>;
		reset-deassert-us = <100000>;
	};
};

The RX path looks O.K. RGMII-RXID means the PHY should be adding the
2ns delay. The allwinner,rx-delay-ps = <0> should be redundant, that
should be the driver default. And there are no properties in the PHY
node about RX. All good.

TX is the problem. The allwinner,tx-delay-ps = <700> causes the MAC to
add 700ps delay, and 'rgmii-rxid' means the PHY should not add any
delay. But 700ps is too low. It should be around 2000ps. So something
else is adding a delay, or the 700ps is not really 700ps.

You say the PHY is a YT8531C. These PHYs also accept
rx-internal-delay-ps and tx-internal-delay-ps properties in their DT
node.

Try setting 'rgmii-id', allwinner,tx-delay-ps = <0>, and both
rx-internal-delay-ps and tx-internal-delay-ps in the PHY node to 1950.
If that does not work, try other values in the PHY node.

	Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-29 15:45                   ` Andrew Lunn
@ 2025-04-29 15:52                     ` Chen-Yu Tsai
  2025-04-29 16:56                       ` Andrew Lunn
  0 siblings, 1 reply; 18+ messages in thread
From: Chen-Yu Tsai @ 2025-04-29 15:52 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jernej Škrabec, Andre Przywara, robh, krzk+dt, conor+dt,
	samuel, devicetree, linux-arm-kernel, linux-sunxi, linux-kernel

On Tue, Apr 29, 2025 at 11:45 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > I just to be clear, I tested various combinations, including rgmii-id, and it
> > didn't work, except rgmii-rxid, which matches strapping. Of course Motorcomm
> > PHY driver took that into account and set registers accordingly.
>
> So we have:
>
> &emac {
>         pinctrl-names = "default";
>         pinctrl-0 = <&ext_rgmii_pins>;
>         phy-mode = "rgmii-rxid";
>         phy-handle = <&ext_rgmii_phy>;
>         phy-supply = <&reg_gmac_3v3>;
>         allwinner,rx-delay-ps = <0>;
>         allwinner,tx-delay-ps = <700>;
>         status = "okay";
> };
>
> and
>
> &mdio {
>         ext_rgmii_phy: ethernet-phy@1 {
>                 compatible = "ethernet-phy-ieee802.3-c22";
>                 reg = <1>;
>
>                 motorcomm,clk-out-frequency-hz = <125000000>;
>
>                 reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* PD14 */
>                 reset-assert-us = <15000>;
>                 reset-deassert-us = <100000>;
>         };
> };
>
> The RX path looks O.K. RGMII-RXID means the PHY should be adding the
> 2ns delay. The allwinner,rx-delay-ps = <0> should be redundant, that
> should be the driver default. And there are no properties in the PHY
> node about RX. All good.

The default action when the property is missing is to leave the hardware
settings alone. I admit this doesn't match the bindings.

> TX is the problem. The allwinner,tx-delay-ps = <700> causes the MAC to
> add 700ps delay, and 'rgmii-rxid' means the PHY should not add any
> delay. But 700ps is too low. It should be around 2000ps. So something
> else is adding a delay, or the 700ps is not really 700ps.

Anything is possible. As was raised in a previous reply, it's possible
instead of extending the delay range, the decreased the step size and
added more steps. The problem is we don't really know.

> You say the PHY is a YT8531C. These PHYs also accept
> rx-internal-delay-ps and tx-internal-delay-ps properties in their DT
> node.
>
> Try setting 'rgmii-id', allwinner,tx-delay-ps = <0>, and both
> rx-internal-delay-ps and tx-internal-delay-ps in the PHY node to 1950.
> If that does not work, try other values in the PHY node.

I don't get why we should ignore the strappings instead of using them
as reference or even truth. If the strappings worked correctly w/ the
generic PHY driver (that doesn't know how to configure the delay mode
on the PHY side), isn't it working as intended?


ChenYu

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS
  2025-04-29 15:52                     ` Chen-Yu Tsai
@ 2025-04-29 16:56                       ` Andrew Lunn
  0 siblings, 0 replies; 18+ messages in thread
From: Andrew Lunn @ 2025-04-29 16:56 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Jernej Škrabec, Andre Przywara, robh, krzk+dt, conor+dt,
	samuel, devicetree, linux-arm-kernel, linux-sunxi, linux-kernel

> > The RX path looks O.K. RGMII-RXID means the PHY should be adding the
> > 2ns delay. The allwinner,rx-delay-ps = <0> should be redundant, that
> > should be the driver default. And there are no properties in the PHY
> > node about RX. All good.
> 
> The default action when the property is missing is to leave the hardware
> settings alone. I admit this doesn't match the bindings.

Please submit a patch fix the binding.

> > TX is the problem. The allwinner,tx-delay-ps = <700> causes the MAC to
> > add 700ps delay, and 'rgmii-rxid' means the PHY should not add any
> > delay. But 700ps is too low. It should be around 2000ps. So something
> > else is adding a delay, or the 700ps is not really 700ps.
> 
> Anything is possible. As was raised in a previous reply, it's possible
> instead of extending the delay range, the decreased the step size and
> added more steps. The problem is we don't really know.

By poking around with other configuration knobs, i hope we can
determine if this 700ps actually is 700ps.

> > You say the PHY is a YT8531C. These PHYs also accept
> > rx-internal-delay-ps and tx-internal-delay-ps properties in their DT
> > node.
> >
> > Try setting 'rgmii-id', allwinner,tx-delay-ps = <0>, and both
> > rx-internal-delay-ps and tx-internal-delay-ps in the PHY node to 1950.
> > If that does not work, try other values in the PHY node.
> 
> I don't get why we should ignore the strappings instead of using them
> as reference or even truth.

If you don't want the PHY to be reprogrammed pass:

* @PHY_INTERFACE_MODE_NA: Not Applicable - don't touch

rather than one of

 * @PHY_INTERFACE_MODE_RGMII: Reduced gigabit media-independent interface
 * @PHY_INTERFACE_MODE_RGMII_ID: RGMII with Internal RX+TX delay
 * @PHY_INTERFACE_MODE_RGMII_RXID: RGMII with Internal RX delay
 * @PHY_INTERFACE_MODE_RGMII_TXID: RGMII with Internal TX delay

However, if we do pass one of these RGMII modes, i expect the PHY to
follow them.

> If the strappings worked correctly w/ the
> generic PHY driver (that doesn't know how to configure the delay mode
> on the PHY side), isn't it working as intended?

The generic PHY driver is there as a fallback, for when the real PHY
driver is missing. It is nice if it works, but it often does not in
current systems.  What we really care about is that the real PHY
driver works, and the system as a whole follows the DT bindings.

	Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2025-04-29 16:56 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-13 13:42 [PATCH 0/2] arm64: dts: allwinner: Support Orange Pi 3 LTS board Jernej Skrabec
2025-04-13 13:42 ` [PATCH 1/2] dt-bindings: arm: sunxi: Add " Jernej Skrabec
2025-04-15 21:56   ` Rob Herring (Arm)
2025-04-13 13:42 ` [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS Jernej Skrabec
2025-04-25  0:57   ` Andre Przywara
2025-04-25 12:54   ` Andre Przywara
2025-04-25 14:37     ` Chen-Yu Tsai
2025-04-25 15:34     ` Andrew Lunn
2025-04-26 18:00       ` Jernej Škrabec
2025-04-28 12:37         ` Andrew Lunn
2025-04-29 14:51           ` Jernej Škrabec
2025-04-29 15:09             ` Andrew Lunn
2025-04-29 15:24               ` Jernej Škrabec
2025-04-29 15:27                 ` Jernej Škrabec
2025-04-29 15:45                   ` Andrew Lunn
2025-04-29 15:52                     ` Chen-Yu Tsai
2025-04-29 16:56                       ` Andrew Lunn
2025-04-26 18:26     ` Jernej Škrabec

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).