[draft2 PATCH] ARM: dts: sun8i: r40: add devicetree for FETA40i-C/OKA40i-C

[draft2 PATCH] ARM: dts: sun8i: r40: add devicetree for FETA40i-C/OKA40i-C

[Ivan Uvarov]

[-- Attachment #1.1.1: Type: text/plain, Size: 8463 bytes --]

From: Ivan Uvarov <i.uvarov@cognitivepilot.com>

This patch adds support for the Forlinx FETA40i-C SoM and OKA40i-C
devboard[1] that is based on it. The devicetree is split into a .dtsi 
which (hopefully) corresponds to the functions of the SoM itself and 
a .dts for the devboard.

[1]:https://linux-sunxi.org/Forlinx_OKA40i-C

Signed-off-by: Ivan Uvarov <i.uvarov@cognitivepilot.com>
---
 arch/arm/boot/dts/Makefile               |   1 +
 arch/arm/boot/dts/sun8i-r40-feta40i.dtsi |  68 +++++++
 arch/arm/boot/dts/sun8i-r40-oka40i-c.dts | 238 +++++++++++++++++++++++
 3 files changed, 307 insertions(+)
 create mode 100644 arch/arm/boot/dts/sun8i-r40-feta40i.dtsi
 create mode 100644 arch/arm/boot/dts/sun8i-r40-oka40i-c.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 8e5d4ab4e7..88aae9de95 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1222,6 +1222,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
 	sun8i-r16-nintendo-super-nes-classic.dtb \
 	sun8i-r16-parrot.dtb \
 	sun8i-r40-bananapi-m2-ultra.dtb \
+	sun8i-r40-oka40i-c.dtb \
 	sun8i-s3-elimo-initium.dtb \
 	sun8i-s3-lichee-zero-plus.dtb \
 	sun8i-s3-pinecube.dtb \
diff --git a/arch/arm/boot/dts/sun8i-r40-feta40i.dtsi b/arch/arm/boot/dts/sun8i-r40-feta40i.dtsi
new file mode 100644
index 0000000000..edfb846db1
--- /dev/null
+++ b/arch/arm/boot/dts/sun8i-r40-feta40i.dtsi
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0+ OR MIT
+// Copyright (C) 2021 Ivan Uvarov <i.uvarov@cognitivepilot.com>
+// Based on the sun8i-r40-bananapi-m2-ultra.dts, which is:
+//  Copyright (C) 2017 Chen-Yu Tsai <wens@csie.org>
+//  Copyright (C) 2017 Icenowy Zheng <icenowy@aosc.io>
+
+#include "sun8i-r40.dtsi"
+
+
+&i2c0 {
+	status =3D "okay";
+
+	axp22x: pmic@34 {
+		compatible =3D "x-powers,axp221";
+		reg =3D <0x34>;
+		interrupt-parent =3D <&nmi_intc>;
+		interrupts =3D <0 IRQ_TYPE_LEVEL_LOW>;
+	};
+};
+#include "axp22x.dtsi"
+
+&mmc2 {
+	vmmc-supply =3D <&reg_dcdc1>;
+	vqmmc-supply =3D <&reg_aldo2>;
+	bus-width =3D <8>;
+  non-removable;
+	status =3D "okay";
+};
+
+
+&pio {
+	pinctrl-names =3D "default";
+	pinctrl-0 =3D <&clk_out_a_pin>;
+	vcc-pa-supply =3D <&reg_dcdc1>;
+	vcc-pc-supply =3D <&reg_aldo2>;
+	vcc-pd-supply =3D <&reg_dcdc1>;
+	vcc-pf-supply =3D <&reg_dldo4>;
+	vcc-pg-supply =3D <&reg_dldo1>;
+};
+
+&reg_aldo2 {
+	regulator-always-on;
+	regulator-min-microvolt =3D <1800000>;
+	regulator-max-microvolt =3D <2500000>;
+	regulator-name =3D "vcc-pa";
+};//2500000uV reported by kernel
+
+&reg_dcdc1 {
+	regulator-always-on;
+	regulator-min-microvolt =3D <3300000>;
+	regulator-max-microvolt =3D <3300000>;
+	regulator-name =3D "vcc-3v3";
+};
+
+
+//I don't know whether these really belong here
+&reg_dldo1 {
+	regulator-always-on;
+	regulator-min-microvolt =3D <3300000>;
+	regulator-max-microvolt =3D <3300000>;
+	regulator-name =3D "vcc-wifi-io";
+};
+
+&reg_dldo4 {
+	regulator-always-on;
+	regulator-min-microvolt =3D <2500000>;
+	regulator-max-microvolt =3D <2500000>;
+	regulator-name =3D "vdd2v5-sata";
diff --git a/arch/arm/boot/dts/sun8i-r40-oka40i-c.dts b/arch/arm/boot/dts/sun8i-r40-oka40i-c.dts
new file mode 100644
index 0000000000..7e47cf633e
--- /dev/null
+++ b/arch/arm/boot/dts/sun8i-r40-oka40i-c.dts
@@ -0,0 +1,238 @@
+// SPDX-License-Identifier: GPL-2.0+ OR MIT
+// Copyright (C) 2021 Ivan Uvarov <i.uvarov@cognitivepilot.com>
+// Based on the sun8i-r40-bananapi-m2-ultra.dts, which is:
+//  Copyright (C) 2017 Chen-Yu Tsai <wens@csie.org>
+//  Copyright (C) 2017 Icenowy Zheng <icenowy@aosc.io>
+
+/dts-v1/;
+#include "sun8i-r40-feta40i.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+	model =3D "Forlinx OKA40i-C";
+	compatible =3D "forlinx,oka40i-c", "allwinner,sun8i-r40";
+
+	aliases {
+		ethernet0 =3D &gmac;
+		serial0 =3D &uart0;
+	};
+
+	chosen {
+		stdout-path =3D "serial0:115200n8";
+	};
+
+	connector {
+		compatible =3D "hdmi-connector";
+		type =3D "a";
+
+		port {
+			hdmi_con_in: endpoint {
+				remote-endpoint =3D <&hdmi_out_con>;
+			};
+		};
+	};
+
+	leds {
+		compatible =3D "gpio-leds";
+
+		user-led-5 {
+			label =3D "oka40i:led5:user";
+			gpios =3D <&pio 7 26 GPIO_ACTIVE_LOW>;
+		};
+
+		user-led-6 {
+			label =3D "oka40i:led6:user";
+			gpios =3D <&pio 8 15 GPIO_ACTIVE_LOW>;
+		};
+
+	};
+
+	reg_vcc5v0: vcc5v0 {
+		compatible =3D "regulator-fixed";
+		regulator-name =3D "vcc5v0";
+		regulator-min-microvolt =3D <5000000>;
+		regulator-max-microvolt =3D <5000000>;
+		//gpio =3D <&pio 7 23 GPIO_ACTIVE_HIGH>; // PH23
+		//enable-active-high;
+	};
+
+	wifi_pwrseq: wifi_pwrseq {
+		compatible =3D "mmc-pwrseq-simple";
+		reset-gpios =3D <&pio 1 10 GPIO_ACTIVE_LOW>; // PB10 WIFI_EN
+		clocks =3D <&ccu CLK_OUTA>;
+		clock-names =3D "ext_clock";
+	};
+};
+
+&ahci {
+	ahci-supply =3D <&reg_dldo4>;
+	phy-supply =3D <&reg_eldo2>;
+	status =3D "okay";
+};
+
+&de {
+	status =3D "okay";
+};
+
+&ehci1 {
+	status =3D "okay";
+};
+
+&ehci2 {
+	status =3D "okay";
+};
+
+&gmac {
+	pinctrl-names =3D "default";
+	pinctrl-0 =3D <&gmac_rgmii_pins>;
+	phy-handle =3D <&phy1>;
+	phy-mode =3D "rgmii-id";
+	phy-supply =3D <&reg_dcdc1>;
+	status =3D "okay";
+};
+
+&gmac_mdio {
+	phy1: ethernet-phy@1 {
+		compatible =3D "ethernet-phy-ieee802.3-c22";
+		reg =3D <1>;
+	};
+};
+
+&hdmi {
+	status =3D "okay";
+};
+
+&hdmi_out {
+	hdmi_out_con: endpoint {
+		remote-endpoint =3D <&hdmi_con_in>;
+	};
+};
+
+
+&i2c2 {
+	status =3D "okay";
+};
+
+
+&mmc0 {
+	vmmc-supply =3D <&reg_dcdc1>;
+	vqmmc-supply =3D <&reg_dcdc1>;
+	bus-width =3D <4>;
+	cd-gpios =3D <&pio 8 11 GPIO_ACTIVE_LOW>; // PI11
+	status =3D "okay";
+};
+
+&mmc1 {
+	vmmc-supply =3D <&reg_dcdc1>;
+	vqmmc-supply =3D <&reg_dcdc1>;
+	mmc-pwrseq =3D <&wifi_pwrseq>;
+	bus-width =3D <4>;
+	status =3D "okay";
+};
+
+&ohci1 {
+	status =3D "okay";
+};
+
+&ohci2 {
+	status =3D "okay";
+};
+
+&reg_aldo3 {
+	regulator-always-on;
+	regulator-min-microvolt =3D <3000000>;
+	regulator-max-microvolt =3D <3000000>;
+	regulator-name =3D "avcc";
+};
+
+&reg_dc1sw {
+	regulator-min-microvolt =3D <3300000>;
+	regulator-max-microvolt =3D <3300000>;
+	regulator-name =3D "vcc-lcd";
+};
+
+&reg_dcdc2 {
+	regulator-always-on;
+	regulator-min-microvolt =3D <1100000>;
+	regulator-max-microvolt =3D <1160000>;
+	regulator-name =3D "vdd-cpu";
+};//1100000uV reported by kernel
+
+&reg_dcdc3 {
+	regulator-always-on;
+	regulator-min-microvolt =3D <1100000>;
+	regulator-max-microvolt =3D <1200000>;
+	regulator-name =3D "vdd-sys";
+};//1100000uV reported by kernel
+
+
+&reg_dcdc5 {
+	regulator-always-on;
+	regulator-min-microvolt =3D <1500000>;
+	regulator-max-microvolt =3D <1500000>;
+	regulator-name =3D "vcc-dram";
+};
+
+&reg_dldo2 {
+	// regulator-always-on;
+	regulator-min-microvolt =3D <3300000>;
+	regulator-max-microvolt =3D <3300000>;
+	regulator-name =3D "vcc-wifi";
+};
+
+&reg_dldo3 { // possibly unneeded
+	// regulator-always-on;
+	regulator-min-microvolt =3D <3300000>;
+	regulator-max-microvolt =3D <3300000>;
+	regulator-name =3D "vcc-wifi-2";
+};
+
+&reg_eldo2 {
+	regulator-always-on;
+	regulator-min-microvolt =3D <1200000>;
+	regulator-max-microvolt =3D <1200000>;
+	regulator-name =3D "vdd1v2-sata";
+};
+
+&reg_eldo3 {
+	regulator-always-on;
+	regulator-min-microvolt =3D <2800000>;
+	regulator-max-microvolt =3D <2800000>;
+	regulator-name =3D "vcc-pe";
+};
+
+&tcon_tv0 {
+	status =3D "okay";
+};
+
+&uart0 {
+	pinctrl-names =3D "default";
+	pinctrl-0 =3D <&uart0_pb_pins>;
+	status =3D "okay";
+};
+
+&uart3 {
+	pinctrl-names =3D "default";
+	pinctrl-0 =3D <&uart3_pg_pins>, <&uart3_rts_cts_pg_pins>;
+	uart-has-rtscts;
+	status =3D "okay";
+
+	bluetooth {
+		compatible =3D "brcm,bcm43438-bt";
+		clocks =3D <&ccu CLK_OUTA>;
+		clock-names =3D "lpo";
+		vbat-supply =3D <&reg_dldo2>;
+		vddio-supply =3D <&reg_dldo1>;
+		device-wakeup-gpios =3D <&pio 6 11 GPIO_ACTIVE_HIGH>; /* PG11 */
+		/* TODO host wake line connected to PMIC GPIO pins */
+		shutdown-gpios =3D <&pio 7 12 GPIO_ACTIVE_HIGH>; /* PH12 */
+		max-speed =3D <1500000>;
+	};
+};
+
+&usbphy {
+	usb1_vbus-supply =3D <&reg_vcc5v0>;
+	usb2_vbus-supply =3D <&reg_vcc5v0>;
+	status =3D "okay";
+};
-- 
2.25.1




[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [draft2 PATCH] ARM: dts: sun8i: r40: add devicetree for FETA40i-C/OKA40i-C

[Andre Przywara]

On Tue, 2 Mar 2021 13:54:15 +0300
Ivan Uvarov <i.uvarov@cognitivepilot.com> wrote:

Hi Ivan,

many thanks for spending the time in piecing this together and caring
about upstreaming!

> From: Ivan Uvarov <i.uvarov@cognitivepilot.com>
> 
> This patch adds support for the Forlinx FETA40i-C SoM and OKA40i-C
> devboard[1] that is based on it. The devicetree is split into a .dtsi 
> which (hopefully) corresponds to the functions of the SoM itself and 
> a .dts for the devboard.
> 
> [1]:https://linux-sunxi.org/Forlinx_OKA40i-C
> 
> Signed-off-by: Ivan Uvarov <i.uvarov@cognitivepilot.com>
> ---
>  arch/arm/boot/dts/Makefile               |   1 +
>  arch/arm/boot/dts/sun8i-r40-feta40i.dtsi |  68 +++++++
>  arch/arm/boot/dts/sun8i-r40-oka40i-c.dts | 238 +++++++++++++++++++++++
>  3 files changed, 307 insertions(+)
>  create mode 100644 arch/arm/boot/dts/sun8i-r40-feta40i.dtsi
>  create mode 100644 arch/arm/boot/dts/sun8i-r40-oka40i-c.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 8e5d4ab4e7..88aae9de95 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1222,6 +1222,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
>  	sun8i-r16-nintendo-super-nes-classic.dtb \
>  	sun8i-r16-parrot.dtb \
>  	sun8i-r40-bananapi-m2-ultra.dtb \
> +	sun8i-r40-oka40i-c.dtb \
>  	sun8i-s3-elimo-initium.dtb \
>  	sun8i-s3-lichee-zero-plus.dtb \
>  	sun8i-s3-pinecube.dtb \
> diff --git a/arch/arm/boot/dts/sun8i-r40-feta40i.dtsi b/arch/arm/boot/dts/sun8i-r40-feta40i.dtsi
> new file mode 100644
> index 0000000000..edfb846db1
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-r40-feta40i.dtsi
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0+ OR MIT
> +// Copyright (C) 2021 Ivan Uvarov <i.uvarov@cognitivepilot.com>
> +// Based on the sun8i-r40-bananapi-m2-ultra.dts, which is:
> +//  Copyright (C) 2017 Chen-Yu Tsai <wens@csie.org>
> +//  Copyright (C) 2017 Icenowy Zheng <icenowy@aosc.io>
> +
> +#include "sun8i-r40.dtsi"
> +
> +
> +&i2c0 {
> +	status =3D "okay";
> +
> +	axp22x: pmic@34 {
> +		compatible =3D "x-powers,axp221";
> +		reg =3D <0x34>;
> +		interrupt-parent =3D <&nmi_intc>;
> +		interrupts =3D <0 IRQ_TYPE_LEVEL_LOW>;
> +	};
> +};
> +#include "axp22x.dtsi"
> +
> +&mmc2 {
> +	vmmc-supply =3D <&reg_dcdc1>;
> +	vqmmc-supply =3D <&reg_aldo2>;
> +	bus-width =3D <8>;
> +  non-removable;

Should be a TAB here.

And I don't know if this is just my client, but someone mangled equal
signs into "=3D" everywhere :-(
I am trying to ignore this for now ...
Can you force pure text email in Thunderbird? Or use git send-email?

> +	status =3D "okay";
> +};
> +
> +
> +&pio {
> +	pinctrl-names =3D "default";
> +	pinctrl-0 =3D <&clk_out_a_pin>;
> +	vcc-pa-supply =3D <&reg_dcdc1>;
> +	vcc-pc-supply =3D <&reg_aldo2>;
> +	vcc-pd-supply =3D <&reg_dcdc1>;
> +	vcc-pf-supply =3D <&reg_dldo4>;
> +	vcc-pg-supply =3D <&reg_dldo1>;
> +};
> +
> +&reg_aldo2 {
> +	regulator-always-on;
> +	regulator-min-microvolt =3D <1800000>;
> +	regulator-max-microvolt =3D <2500000>;
> +	regulator-name =3D "vcc-pa";
> +};//2500000uV reported by kernel

I think min and max should be the same in those cases. Do you know what
the recommended voltage is? Does the BSP report something?
PortA is used for Ethernet, is that using 2.5V as the signalling
voltage?

> +
> +&reg_dcdc1 {
> +	regulator-always-on;
> +	regulator-min-microvolt =3D <3300000>;
> +	regulator-max-microvolt =3D <3300000>;
> +	regulator-name =3D "vcc-3v3";
> +};
> +
> +
> +//I don't know whether these really belong here

Since it seems to be used above, for VCC_PG: yes.

> +&reg_dldo1 {
> +	regulator-always-on;
> +	regulator-min-microvolt =3D <3300000>;
> +	regulator-max-microvolt =3D <3300000>;
> +	regulator-name =3D "vcc-wifi-io";
> +};
> +
> +&reg_dldo4 {
> +	regulator-always-on;

Does this really need to be always on? If it's just for SATA, I'd
expect this to be enabled by the driver when needed? As you correctly do
in the ahci node below.

> +	regulator-min-microvolt =3D <2500000>;
> +	regulator-max-microvolt =3D <2500000>;
> +	regulator-name =3D "vdd2v5-sata";

Does this end prematurely here?

> diff --git a/arch/arm/boot/dts/sun8i-r40-oka40i-c.dts b/arch/arm/boot/dts/sun8i-r40-oka40i-c.dts
> new file mode 100644
> index 0000000000..7e47cf633e
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-r40-oka40i-c.dts
> @@ -0,0 +1,238 @@
> +// SPDX-License-Identifier: GPL-2.0+ OR MIT
> +// Copyright (C) 2021 Ivan Uvarov <i.uvarov@cognitivepilot.com>
> +// Based on the sun8i-r40-bananapi-m2-ultra.dts, which is:
> +//  Copyright (C) 2017 Chen-Yu Tsai <wens@csie.org>
> +//  Copyright (C) 2017 Icenowy Zheng <icenowy@aosc.io>
> +
> +/dts-v1/;
> +#include "sun8i-r40-feta40i.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> +	model =3D "Forlinx OKA40i-C";
> +	compatible =3D "forlinx,oka40i-c", "allwinner,sun8i-r40";
> +
> +	aliases {
> +		ethernet0 =3D &gmac;
> +		serial0 =3D &uart0;
> +	};
> +
> +	chosen {
> +		stdout-path =3D "serial0:115200n8";
> +	};
> +
> +	connector {
> +		compatible =3D "hdmi-connector";
> +		type =3D "a";
> +
> +		port {
> +			hdmi_con_in: endpoint {
> +				remote-endpoint =3D <&hdmi_out_con>;
> +			};
> +		};
> +	};
> +
> +	leds {
> +		compatible =3D "gpio-leds";
> +
> +		user-led-5 {
> +			label =3D "oka40i:led5:user";

The current binding uses different properties:
		function = LED_FUNCTION_xxx;
		color = <LED_COLOR_ID_RED>;

> +			gpios =3D <&pio 7 26 GPIO_ACTIVE_LOW>;

I think it's customary to add the GPIO pin spelled out as a comment, so
		gpios = <&pio 7 26 GPIO_ACTIVE_LOW>; /* PH27 */

> +		};
> +
> +		user-led-6 {
> +			label =3D "oka40i:led6:user";
> +			gpios =3D <&pio 8 15 GPIO_ACTIVE_LOW>;
> +		};
> +
> +	};
> +
> +	reg_vcc5v0: vcc5v0 {
> +		compatible =3D "regulator-fixed";
> +		regulator-name =3D "vcc5v0";
> +		regulator-min-microvolt =3D <5000000>;
> +		regulator-max-microvolt =3D <5000000>;
> +		//gpio =3D <&pio 7 23 GPIO_ACTIVE_HIGH>; // PH23

Is that regulator switchable? If not, no need for commented lines, just
remove them. I don't see PH23 connected to a regulator on the board.

> +		//enable-active-high;

This is redundant anyway this the above GPIO_ACTIVE_HIGH.

> +	};
> +
> +	wifi_pwrseq: wifi_pwrseq {
> +		compatible =3D "mmc-pwrseq-simple";
> +		reset-gpios =3D <&pio 1 10 GPIO_ACTIVE_LOW>; // PB10 WIFI_EN
> +		clocks =3D <&ccu CLK_OUTA>;
> +		clock-names =3D "ext_clock";
> +	};
> +};
> +
> +&ahci {
> +	ahci-supply =3D <&reg_dldo4>;
> +	phy-supply =3D <&reg_eldo2>;
> +	status =3D "okay";
> +};
> +
> +&de {
> +	status =3D "okay";
> +};
> +
> +&ehci1 {
> +	status =3D "okay";
> +};
> +
> +&ehci2 {
> +	status =3D "okay";
> +};
> +
> +&gmac {
> +	pinctrl-names =3D "default";
> +	pinctrl-0 =3D <&gmac_rgmii_pins>;
> +	phy-handle =3D <&phy1>;
> +	phy-mode =3D "rgmii-id";
> +	phy-supply =3D <&reg_dcdc1>;
> +	status =3D "okay";
> +};
> +
> +&gmac_mdio {
> +	phy1: ethernet-phy@1 {
> +		compatible =3D "ethernet-phy-ieee802.3-c22";
> +		reg =3D <1>;
> +	};
> +};
> +
> +&hdmi {
> +	status =3D "okay";
> +};
> +
> +&hdmi_out {
> +	hdmi_out_con: endpoint {
> +		remote-endpoint =3D <&hdmi_con_in>;
> +	};
> +};
> +
> +
> +&i2c2 {
> +	status =3D "okay";
> +};
> +
> +
> +&mmc0 {
> +	vmmc-supply =3D <&reg_dcdc1>;
> +	vqmmc-supply =3D <&reg_dcdc1>;
> +	bus-width =3D <4>;
> +	cd-gpios =3D <&pio 8 11 GPIO_ACTIVE_LOW>; // PI11
> +	status =3D "okay";
> +};
> +
> +&mmc1 {

So this is the SDIO connector on the board, right? Which is just a set
of header pins? Not sure we should have it in here, then.

> +	vmmc-supply =3D <&reg_dcdc1>;
> +	vqmmc-supply =3D <&reg_dcdc1>;
> +	mmc-pwrseq =3D <&wifi_pwrseq>;
> +	bus-width =3D <4>;
> +	status =3D "okay";
> +};

What about MMC3, which is apparently connected to a microSD slot (TF
card in the schematic)?

> +
> +&ohci1 {
> +	status =3D "okay";
> +};
> +
> +&ohci2 {
> +	status =3D "okay";
> +};
> +
> +&reg_aldo3 {
> +	regulator-always-on;
> +	regulator-min-microvolt =3D <3000000>;
> +	regulator-max-microvolt =3D <3000000>;
> +	regulator-name =3D "avcc";
> +};
> +
> +&reg_dc1sw {
> +	regulator-min-microvolt =3D <3300000>;
> +	regulator-max-microvolt =3D <3300000>;
> +	regulator-name =3D "vcc-lcd";
> +};
> +
> +&reg_dcdc2 {
> +	regulator-always-on;
> +	regulator-min-microvolt =3D <1100000>;
> +	regulator-max-microvolt =3D <1160000>;
> +	regulator-name =3D "vdd-cpu";
> +};//1100000uV reported by kernel

Again, this should be the one value then, for both min and max.

> +
> +&reg_dcdc3 {
> +	regulator-always-on;
> +	regulator-min-microvolt =3D <1100000>;
> +	regulator-max-microvolt =3D <1200000>;
> +	regulator-name =3D "vdd-sys";
> +};//1100000uV reported by kernel

Same here.

> +
> +
> +&reg_dcdc5 {
> +	regulator-always-on;
> +	regulator-min-microvolt =3D <1500000>;
> +	regulator-max-microvolt =3D <1500000>;
> +	regulator-name =3D "vcc-dram";
> +};
> +
> +&reg_dldo2 {
> +	// regulator-always-on;

Please, no commented properties.

> +	regulator-min-microvolt =3D <3300000>;
> +	regulator-max-microvolt =3D <3300000>;
> +	regulator-name =3D "vcc-wifi";
> +};
> +
> +&reg_dldo3 { // possibly unneeded

If it's unneeded, you can drop it. If it turns out be needed later on,
we can always add it. Did the board work without it?
Did you try Wifi? We should not add untested features.

> +	// regulator-always-on;
> +	regulator-min-microvolt =3D <3300000>;
> +	regulator-max-microvolt =3D <3300000>;
> +	regulator-name =3D "vcc-wifi-2";
> +};
> +
> +&reg_eldo2 {
> +	regulator-always-on;

This shouldn't be always on. I guess this AXP pin is connected to the
VDD-SATA pin on the SoC, so it just drives the integrated SATA PHY.
Which probably means that this node belong into the SoM .dtsi, as the
connection is on the SoM. You just reference it from the ahci node here.

> +	regulator-min-microvolt =3D <1200000>;
> +	regulator-max-microvolt =3D <1200000>;
> +	regulator-name =3D "vdd1v2-sata";
> +};
> +
> +&reg_eldo3 {
> +	regulator-always-on;

Again, why always on? Do you know where this is connected to?
If this is VCC_PE on the SoC, then I wonder if this should be moved to
the SoM .dtsi, with a range between 1.8V and 3.3V, since those are
valid input voltages for the VCC_PE pin. And then here you overwrite
this with 2.8V, as this is apparently used for the CSI connector on the
board, which seems to work on 2.8V.
But I am not sure if this is too much, and just having this node here
is sufficient.

> +	regulator-min-microvolt =3D <2800000>;
> +	regulator-max-microvolt =3D <2800000>;
> +	regulator-name =3D "vcc-pe";
> +};
> +
> +&tcon_tv0 {
> +	status =3D "okay";
> +};
> +
> +&uart0 {
> +	pinctrl-names =3D "default";
> +	pinctrl-0 =3D <&uart0_pb_pins>;
> +	status =3D "okay";
> +};
> +
> +&uart3 {

Please add uart3 to the aliases section then.

Cheers,
Andre

> +	pinctrl-names =3D "default";
> +	pinctrl-0 =3D <&uart3_pg_pins>, <&uart3_rts_cts_pg_pins>;
> +	uart-has-rtscts;
> +	status =3D "okay";
> +
> +	bluetooth {
> +		compatible =3D "brcm,bcm43438-bt";
> +		clocks =3D <&ccu CLK_OUTA>;
> +		clock-names =3D "lpo";
> +		vbat-supply =3D <&reg_dldo2>;
> +		vddio-supply =3D <&reg_dldo1>;
> +		device-wakeup-gpios =3D <&pio 6 11 GPIO_ACTIVE_HIGH>; /* PG11 */
> +		/* TODO host wake line connected to PMIC GPIO pins */
> +		shutdown-gpios =3D <&pio 7 12 GPIO_ACTIVE_HIGH>; /* PH12 */
> +		max-speed =3D <1500000>;
> +	};
> +};
> +
> +&usbphy {
> +	usb1_vbus-supply =3D <&reg_vcc5v0>;
> +	usb2_vbus-supply =3D <&reg_vcc5v0>;
> +	status =3D "okay";
> +};


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [draft2 PATCH] ARM: dts: sun8i: r40: add devicetree for FETA40i-C/OKA40i-C

[Ivan Uvarov]

On Wed, 3 Mar 2021 13:08:34 +0000
Andre Przywara <andre.przywara@arm.com> wrote:

> On Tue, 2 Mar 2021 13:54:15 +0300
> Ivan Uvarov <i.uvarov@cognitivepilot.com> wrote:
> 
> Hi Ivan,
> 
> many thanks for spending the time in piecing this together and caring
> about upstreaming!
> 

Many more thanks to you for helping me out with the process. I wouldn't
have gotten anywhere near this far without your help.

> 
> Should be a TAB here.
> 
> And I don't know if this is just my client, but someone mangled equal
> signs into "=3D" everywhere :-(
> I am trying to ignore this for now ...
> Can you force pure text email in Thunderbird? Or use git send-email?
> 

I was using Thunderbird's plain text mode when composing this; looks
like it imposes its own modifications anyway. I've switched to
Claws now, hopefully it'll handle the next iteration more
gracefully.


> > +&reg_dldo1 {
> > +	regulator-always-on;
> > +	regulator-min-microvolt =3D <3300000>;
> > +	regulator-max-microvolt =3D <3300000>;
> > +	regulator-name =3D "vcc-wifi-io";
> > +};
> > +
> > +&reg_dldo4 {
> > +	regulator-always-on;  
> 
> Does this really need to be always on? If it's just for SATA, I'd
> expect this to be enabled by the driver when needed? As you correctly
> do in the ahci node below.
I'm not sure, to be honest. I haven't tested without it. I should
probably try to.

> > +	regulator-min-microvolt =3D <2500000>;
> > +	regulator-max-microvolt =3D <2500000>;
> > +	regulator-name =3D "vdd2v5-sata";  
> 
> Does this end prematurely here?

It does. Looks like I deleted the last line of the file somehow while
creating the patch. 


> > +
> > +&mmc0 {
> > +	vmmc-supply =3D <&reg_dcdc1>;
> > +	vqmmc-supply =3D <&reg_dcdc1>;
> > +	bus-width =3D <4>;
> > +	cd-gpios =3D <&pio 8 11 GPIO_ACTIVE_LOW>; // PI11
> > +	status =3D "okay";
> > +};
> > +
> > +&mmc1 {  
> 
> So this is the SDIO connector on the board, right? Which is just a set
> of header pins? 
Yes, that's it.

> Not sure we should have it in here, then.
OK.
 
> What about MMC3, which is apparently connected to a microSD slot (TF
> card in the schematic)?

I never needed it, so never added it. Probably should.

> 
> > +	regulator-min-microvolt =3D <3300000>;
> > +	regulator-max-microvolt =3D <3300000>;
> > +	regulator-name =3D "vcc-wifi";
> > +};
> > +
> > +&reg_dldo3 { // possibly unneeded  
> 
> If it's unneeded, you can drop it. If it turns out be needed later on,
> we can always add it. Did the board work without it?
> Did you try Wifi? We should not add untested features.

WiFi does work without any hiccups on my board with this DTS. I haven't
tested with the regulator disabled.

> > +	// regulator-always-on;
> > +	regulator-min-microvolt =3D <3300000>;
> > +	regulator-max-microvolt =3D <3300000>;
> > +	regulator-name =3D "vcc-wifi-2";
> > +};
> > +
> > +&reg_eldo2 {
> > +	regulator-always-on;  
> 
> This shouldn't be always on. I guess this AXP pin is connected to the
> VDD-SATA pin on the SoC, so it just drives the integrated SATA PHY.
> Which probably means that this node belong into the SoM .dtsi, as the
> connection is on the SoM. You just reference it from the ahci node
> here.
> 
> > +	regulator-min-microvolt =3D <1200000>;
> > +	regulator-max-microvolt =3D <1200000>;
> > +	regulator-name =3D "vdd1v2-sata";
> > +};
> > +
> > +&reg_eldo3 {
> > +	regulator-always-on;  
> 
> Again, why always on? Do you know where this is connected to?
> If this is VCC_PE on the SoC, then I wonder if this should be moved to
> the SoM .dtsi, with a range between 1.8V and 3.3V, since those are
> valid input voltages for the VCC_PE pin. And then here you overwrite
> this with 2.8V, as this is apparently used for the CSI connector on
> the board, which seems to work on 2.8V.
> But I am not sure if this is too much, and just having this node here
> is sufficient.

I'll look into both of those.

> > +	regulator-min-microvolt =3D <2800000>;
> > +	regulator-max-microvolt =3D <2800000>;
> > +	regulator-name =3D "vcc-pe";
> > +};
> > +
> > +&tcon_tv0 {
> > +	status =3D "okay";
> > +};
> > +
> > +&uart0 {
> > +	pinctrl-names =3D "default";
> > +	pinctrl-0 =3D <&uart0_pb_pins>;
> > +	status =3D "okay";
> > +};
> > +
> > +&uart3 {  
> 
> Please add uart3 to the aliases section then.

Sure. 

I think I'll post a new version of the patch tomorrow, which should fix
formatting and address the issues you've raised that can be fixed
immediately without much further testing. It will also include the
other uarts that are available on the devboard (2,4 and 7).


Thank you for your assistance once more, you've been tremendously
helpful. 



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel