ARM: dts: sun8i: r40: add dts for Forlinx FETA40i-C & OKA40i-C
Subsystems (3)
PATCH 1/1 ARM: dts: sun8i: r40: add dts for Forlinx FETA40i-C & OKA40i-C
Add support for the Forlinx FETA40i-C SoM and OKA40i-C devboard[1] based on it. The DT 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
---
draft2 -> v1:
* fix indentation from spaces to tabs;
* remove commented properties and other extraneous comments;
* enable uarts 2,4,5&7 and alias uart3;
* update the user-LED bindings;
* remove mmc1 & add mmc3 binding;
* bring together the max/min microvolts on three regulators;
* The 2.5v in ALDO2 came from U-Boot, where apparently this is the KConfig default for this regulator. The correct voltage is 1.8v.
* remove extraneous always-ons from regulators dldo1 and eldo2/3;
* remove reg_dldo3 node entirely.
arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/sun8i-r40-feta40i.dtsi | 68 +++++++ arch/arm/boot/dts/sun8i-r40-oka40i-c.dts | 245 +++++++++++++++++++++++ arch/arm/boot/dts/sun8i-r40.dtsi | 35 ++++ 4 files changed, 349 insertions(+) create mode 100644 arch/arm/boot/dts/sun8i-r40-feta40i.dtsi create mode 100644 arch/arm/boot/dts/sun8i-r40-oka40i-c.dts
arch/arm/boot/dts/Makefile +1 -0
@@ -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 \
(quoted diff) +0 -0
b/arch/arm/boot/dts/sun8i-r40-feta40i.dtsi new file mode 100644 index 0000000000..4764661849
arch/arm/boot/dts/sun8i-r40-feta40i.dtsi new file +68 -0
68 lines not on this page · raw message
(quoted diff) +0 -0
b/arch/arm/boot/dts/sun8i-r40-oka40i-c.dts new file mode 100644 index 0000000000..ae06dd72c9
arch/arm/boot/dts/sun8i-r40-oka40i-c.dts new file +67 -0
67 lines not on this page · raw message
WIFI_EN
+ clocks = <&ccu CLK_OUTA>;
+ clock-names = "ext_clock";
+ };
+};
+
+&ahci {
+ ahci-supply = <®_dldo4>;
+ phy-supply = <®_eldo2>;
+ status = "okay";
+};
+
+&de {
+ status = "okay";
+};
+
+&ehci1 {
+ status = "okay";
+};
+
+&ehci2 {
+ status = "okay";
+};
+
+&gmac {
+ pinctrl-names = "default";
+ pinctrl-0 = <&gmac_rgmii_pins>;
+ phy-handle = <&phy1>;
+ phy-mode = "rgmii-id";
+ phy-supply = <®_dcdc1>;
+ status = "okay";
+};
+
+&gmac_mdio {
+ phy1: ethernet-phy@1 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
+ };
+};
+
+&hdmi {
+ status = "okay";
+};
+
+&hdmi_out {
+ hdmi_out_con: endpoint {
+ remote-endpoint = <&hdmi_con_in>;
+ };
+};
+
+
+&i2c2 {
+ status = "okay";
+};
+
+
+&mmc0 {
+ vmmc-supply = <®_dcdc1>;
+ vqmmc-supply = <®_dcdc1>;
+ bus-width = <4>;
+ cd-gpios = <&pio 8 11 GPIO_ACTIVE_LOW>; // PI11
+ status = "okay";
+};
+
+&mmc3 {
+ vmmc-supply = <®_dcdc1>;
+ vqmmc-supply = <®_dcdc1>;
+ bus-width = <4>;
+ cd-gpios = <&pio 8 10 GPIO_ACTIVE_LOW>; // PI10
+ status = "okay";
+};
+
+&ohci1 {
+ status = "okay";
+};
+
+&ohci2 {
+ status = "okay";
+};
+
+®_aldo3 {
+ regulator-always-on;
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <3000000>;
+ regulator-name = "avcc";
+};
+
+®_dc1sw {
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "vcc-lcd";
+};
+
+®_dcdc2 {
+ regulator-always-on;
+ regulator-min-microvolt = <1100000>;
+ regulator-max-microvolt = <1100000>;
+ regulator-name = "vdd-cpu";
+};
+
+®_dcdc3 {
+ regulator-always-on;
+ regulator-min-microvolt = <1100000>;
+ regulator-max-microvolt = <1100000>;
+ regulator-name = "vdd-sys";
+};
+
+®_dcdc5 {
+ regulator-always-on;
+ regulator-min-microvolt = <1500000>;
+ regulator-max-microvolt = <1500000>;
+ regulator-name = "vcc-dram";
+};
+
+®_dldo2 {
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "vcc-wifi";
+};
+
+®_eldo2 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-name = "vdd1v2-sata";
+};
+
+®_eldo3 {
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ regulator-name = "vcc-pe";
+};
+
+&tcon_tv0 {
+ status = "okay";
+};
+
+&uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart0_pb_pins>;
+ status = "okay";
+};
+
+&uart2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart2_pi_pins>, <&uart2_rts_cts_pi_pins>;
+ uart-has-rtscts;
+ status = "okay";
+};
+
+&uart3 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart3_pg_pins>, <&uart3_rts_cts_pg_pins>;
+ uart-has-rtscts;
+ status = "okay";
+};
+
+&uart4 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart4_pg_pins>;
+ status = "okay";
+};
+
+&uart5 { /* RS485 */
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart5_ph_pins>;
+ status = "okay";
+};
+
+&uart7 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart7_pi_pins>;
+ status = "okay";
+};
+
+&usbphy {
+ usb1_vbus-supply = <®_vcc5v0>;
+ usb2_vbus-supply = <®_vcc5v0>;
+ status = "okay";
+};
(quoted diff) +0 -0
b/arch/arm/boot/dts/sun8i-r40.dtsi index d5ad3b9efd..0fe3a8fcd4 100644
arch/arm/boot/dts/sun8i-r40.dtsi +35 -0
@@ -357,6 +357,8 @@ mmc3: mmc@1c12000 { clock-names = "ahb", "mmc"; resets = <&ccu RST_BUS_MMC3>; reset-names = "ahb"; + pinctrl-0 = <&mmc3_pins>; + pinctrl-names = "default"; interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>; status = "disabled"; #address-cells = <1>;
@@ -601,6 +603,14 @@ mmc2_pins: mmc2-pins { bias-pull-up; }; + mmc3_pins: mmc3-pins { + pins = "PI4", "PI5", "PI6", + "PI7", "PI8", "PI9"; + function = "mmc3"; + drive-strength = <30>; + bias-pull-up; + }; + /omit-if-no-ref/ spi0_pc_pins: spi0-pc-pins { pins = "PC0", "PC1", "PC2";
@@ -636,6 +646,16 @@ uart0_pb_pins: uart0-pb-pins { function = "uart0"; }; + uart2_pi_pins: uart2-pi-pins { + pins = "PI18", "PI19"; + function = "uart2"; + }; + + uart2_rts_cts_pi_pins: uart2-rts-cts-pi-pins{ + pins = "PI16", "PI17"; + function = "uart2"; + }; + uart3_pg_pins: uart3-pg-pins { pins = "PG6", "PG7"; function = "uart3";
@@ -645,6 +665,21 @@ uart3_rts_cts_pg_pins: uart3-rts-cts-pg-pins { pins = "PG8", "PG9"; function = "uart3"; }; + + uart4_pg_pins: uart4-pg-pins { + pins = "PG10", "PG11"; + function = "uart4"; + }; + + uart5_ph_pins: uart5-ph-pins { + pins = "PH6", "PH7"; + function = "uart5"; + }; + + uart7_pi_pins: uart7-pi-pins { + pins = "PI20", "PI21"; + function = "uart7"; + }; }; wdt: watchdog@1c20c90 {
Signature · 1 line
2.25.1
Hi,
You seem to have some issues with your mailer that mangles the mail, you should try using git-send-email instead.
There's also a bunch of checkpatch --strict issues you want to get rid of.
Quoted 5 lines · Ivan Uvarov
Add support for the Forlinx FETA40i-C SoM and OKA40i-C devboard[1] based on it. The DT is split into a .dtsi, which (hopefully) corresponds to the functions of the SoM itself, and a .dts for the devboard.
Instead of a URL that might be dead at some point, it would be better to have a proper commit log explaining why you did your patch that way
Quoted 377 lines
Signed-off-by Ivan Uvarov <[email protected]>---
draft2 -> v1:
* fix indentation from spaces to tabs;
* remove commented properties and other extraneous comments;
* enable uarts 2,4,5&7 and alias uart3;
* update the user-LED bindings;
* remove mmc1 & add mmc3 binding;
* bring together the max/min microvolts on three regulators;
* The 2.5v in ALDO2 came from U-Boot, where apparently this is the KConfig default for this regulator. The correct voltage is 1.8v.
* remove extraneousalways-ons from regulators dldo1 and eldo2/3;
* removereg_dldo3node entirely.arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/sun8i-r40-feta40i.dtsi | 68 +++++++ arch/arm/boot/dts/sun8i-r40-oka40i-c.dts | 245 +++++++++++++++++++++++ arch/arm/boot/dts/sun8i-r40.dtsi | 35 ++++ 4 files changed, 349 insertions(+) create mode 100644 arch/arm/boot/dts/sun8i-r40-feta40i.dtsi create mode 100644 arch/arm/boot/dts/sun8i-r40-oka40i-c.dtsarch/arm/boot/dts/Makefile +1
-0@@ -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 \(quoted diff) +0
-0
b/arch/arm/boot/dts/sun8i-r40-feta40i.dtsinew file mode 100644 index 0000000000..4764661849arch/arm/boot/dts/sun8i-r40-feta40i.dtsi new file +68
-0@@ -0,0 +1,68 @@ +// SPDX-License-Identifier: GPL-2.0+ OR MIT +// Copyright (C) 2021 Ivan Uvarov <[email protected]> +// Based on the sun8i-r40-bananapi-m2-ultra.dts, which is: +// Copyright (C) 2017 Chen-Yu Tsai <[email protected]> +// Copyright (C) 2017 Icenowy Zheng <[email protected]> + + +#include "sun8i-r40.dtsi" + + +&i2c0 { + status = "okay"; + + axp22x: pmic@34 { + compatible = "x-powers,axp221"; + reg = <0x34>; + interrupt-parent = <&nmi_intc>; + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; + }; +}; +#include "axp22x.dtsi" + +&mmc2 { + vmmc-supply = <®_dcdc1>; + vqmmc-supply = <®_aldo2>; + bus-width = <8>; + non-removable; + status = "okay"; +}; + + +&pio { + pinctrl-names = "default"; + pinctrl-0 = <&clk_out_a_pin>; + vcc-pa-supply = <®_dcdc1>; + vcc-pc-supply = <®_aldo2>; + vcc-pd-supply = <®_dcdc1>; + vcc-pf-supply = <®_dldo4>; + vcc-pg-supply = <®_dldo1>; +}; + +®_aldo2 { + regulator-always-on; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-name = "vcc-pa"; +}; + +®_dcdc1 { + regulator-always-on; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-name = "vcc-3v3"; +}; + +®_dldo1 { + regulator-always-on; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-name = "vcc-wifi-io"; +}; + +®_dldo4 { + regulator-always-on; + regulator-min-microvolt = <2500000>; + regulator-max-microvolt = <2500000>; + regulator-name = "vdd2v5-sata"; +};(quoted diff) +0
-0
b/arch/arm/boot/dts/sun8i-r40-oka40i-c.dtsnew file mode 100644 index 0000000000..ae06dd72c9arch/arm/boot/dts/sun8i-r40-oka40i-c.dts new file +67
-0@@ -0,0 +1,245 @@ +// SPDX-License-Identifier: GPL-2.0+ OR MIT +// Copyright (C) 2021 Ivan Uvarov <[email protected]> +// Based on the sun8i-r40-bananapi-m2-ultra.dts, which is: +// Copyright (C) 2017 Chen-Yu Tsai <[email protected]> +// Copyright (C) 2017 Icenowy Zheng <[email protected]> + +/dts-v1/; +#include "sun8i-r40-feta40i.dtsi" + +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/leds/common.h> + +/ { + model = "Forlinx OKA40i-C"; + compatible = "forlinx,oka40i-c", "allwinner,sun8i-r40"; + + aliases { + ethernet0 = &gmac; + serial0 = &uart0; + serial2 = &uart2; + serial3 = &uart3; + serial4 = &uart4; + serial5 = &uart5; /* RS485 */ + serial7 = &uart7; + }; + + chosen { + stdout-path = "serial0:115200n8"; + }; + + connector { + compatible = "hdmi-connector"; + type = "a"; + + port { + hdmi_con_in: endpoint { + remote-endpoint = <&hdmi_out_con>; + }; + }; + }; + + leds { + compatible = "gpio-leds"; + + led-5 { + gpios = <&pio 7 26 GPIO_ACTIVE_LOW>; /* PH26 */ + color = <LED_COLOR_ID_BLUE>; + function = LED_FUNCTION_STATUS; + }; + + led-6 { + gpios = <&pio 8 15 GPIO_ACTIVE_LOW>; /* PI15 */ + color = <LED_COLOR_ID_BLUE>; + function = LED_FUNCTION_STATUS; + }; + }; + + reg_vcc5v0: vcc5v0 { + compatible = "regulator-fixed"; + regulator-name = "vcc5v0"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + }; + + wifi_pwrseq: wifi_pwrseq { + compatible = "mmc-pwrseq-simple"; + reset-gpios = <&pio 1 10 GPIO_ACTIVE_LOW>; // PB10
WIFI_ENarch/arm/boot/dts/sun8i-r40-oka40i-c.dts new file +178
-0+ clocks = <&ccu CLK_OUTA>; + clock-names = "ext_clock"; + }; +}; + +&ahci { + ahci-supply = <®_dldo4>; + phy-supply = <®_eldo2>; + status = "okay"; +}; + +&de { + status = "okay"; +}; + +&ehci1 { + status = "okay"; +}; + +&ehci2 { + status = "okay"; +}; + +&gmac { + pinctrl-names = "default"; + pinctrl-0 = <&gmac_rgmii_pins>; + phy-handle = <&phy1>; + phy-mode = "rgmii-id"; + phy-supply = <®_dcdc1>; + status = "okay"; +}; + +&gmac_mdio { + phy1: ethernet-phy@1 { + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <1>; + }; +}; + +&hdmi { + status = "okay"; +}; + +&hdmi_out { + hdmi_out_con: endpoint { + remote-endpoint = <&hdmi_con_in>; + }; +}; + + +&i2c2 { + status = "okay"; +}; + + +&mmc0 { + vmmc-supply = <®_dcdc1>; + vqmmc-supply = <®_dcdc1>; + bus-width = <4>; + cd-gpios = <&pio 8 11 GPIO_ACTIVE_LOW>; // PI11 + status = "okay"; +}; + +&mmc3 { + vmmc-supply = <®_dcdc1>; + vqmmc-supply = <®_dcdc1>; + bus-width = <4>; + cd-gpios = <&pio 8 10 GPIO_ACTIVE_LOW>; // PI10 + status = "okay"; +}; + +&ohci1 { + status = "okay"; +}; + +&ohci2 { + status = "okay"; +}; + +®_aldo3 { + regulator-always-on; + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3000000>; + regulator-name = "avcc"; +}; + +®_dc1sw { + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-name = "vcc-lcd"; +}; + +®_dcdc2 { + regulator-always-on; + regulator-min-microvolt = <1100000>; + regulator-max-microvolt = <1100000>; + regulator-name = "vdd-cpu"; +}; + +®_dcdc3 { + regulator-always-on; + regulator-min-microvolt = <1100000>; + regulator-max-microvolt = <1100000>; + regulator-name = "vdd-sys"; +}; + +®_dcdc5 { + regulator-always-on; + regulator-min-microvolt = <1500000>; + regulator-max-microvolt = <1500000>; + regulator-name = "vcc-dram"; +}; + +®_dldo2 { + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-name = "vcc-wifi"; +}; + +®_eldo2 { + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-name = "vdd1v2-sata"; +}; + +®_eldo3 { + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + regulator-name = "vcc-pe"; +}; + +&tcon_tv0 { + status = "okay"; +}; + +&uart0 { + pinctrl-names = "default"; + pinctrl-0 = <&uart0_pb_pins>; + status = "okay"; +}; + +&uart2 { + pinctrl-names = "default"; + pinctrl-0 = <&uart2_pi_pins>, <&uart2_rts_cts_pi_pins>; + uart-has-rtscts; + status = "okay"; +}; + +&uart3 { + pinctrl-names = "default"; + pinctrl-0 = <&uart3_pg_pins>, <&uart3_rts_cts_pg_pins>; + uart-has-rtscts; + status = "okay"; +}; + +&uart4 { + pinctrl-names = "default"; + pinctrl-0 = <&uart4_pg_pins>; + status = "okay"; +}; + +&uart5 { /* RS485 */ + pinctrl-names = "default"; + pinctrl-0 = <&uart5_ph_pins>; + status = "okay"; +}; + +&uart7 { + pinctrl-names = "default"; + pinctrl-0 = <&uart7_pi_pins>; + status = "okay"; +}; + +&usbphy { + usb1_vbus-supply = <®_vcc5v0>; + usb2_vbus-supply = <®_vcc5v0>; + status = "okay"; +};(quoted diff) +0
-0
b/arch/arm/boot/dts/sun8i-r40.dtsiindexd5ad3b9efd..0fe3a8fcd4100644arch/arm/boot/dts/sun8i-r40.dtsi +2
-0@@ -357,6 +357,8 @@ mmc3: mmc@1c12000 { clock-names = "ahb", "mmc"; resets = <&ccu RST_BUS_MMC3>; reset-names = "ahb"; + pinctrl-0 = <&mmc3_pins>; + pinctrl-names = "default"; interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>; status = "disabled"; #address-cells = <1>;@@ -601,6 +603,14 @@ mmc2_pins: mmc2-pins { bias-pull-up;
arch/arm/boot/dts/sun8i-r40.dtsi mmc2_pins: mmc2-pins { · line 613
Quoted 10 lines
arch/arm/boot/dts/sun8i-r40.dtsi +8
-0}; + mmc3_pins: mmc3-pins { + pins = "PI4", "PI5", "PI6", + "PI7", "PI8", "PI9"; + function = "mmc3"; + drive-strength = <30>; + bias-pull-up; + }; +
This should be in a separate patch
Quoted 7 lines
arch/arm/boot/dts/sun8i-r40.dtsi +0
-0/omit-if-no-ref/ spi0_pc_pins: spi0-pc-pins { pins = "PC0", "PC1", "PC2";@@ -636,6 +646,16 @@ uart0_pb_pins: uart0-pb-pins { function = "uart0"; };
arch/arm/boot/dts/sun8i-r40.dtsi uart0_pb_pins: uart0-pb-pins { · line 658
Quoted 10 lines
arch/arm/boot/dts/sun8i-r40.dtsi +10
-0+ uart2_pi_pins: uart2-pi-pins { + pins = "PI18", "PI19"; + function = "uart2"; + }; + + uart2_rts_cts_pi_pins: uart2-rts-cts-pi-pins{ + pins = "PI16", "PI17"; + function = "uart2"; + }; +
Ditto, and it should have /omit-if-no-ref/
Quoted 12 lines
arch/arm/boot/dts/sun8i-r40.dtsi +5
-0uart3_pg_pins: uart3-pg-pins { pins = "PG6", "PG7"; function = "uart3";@@ -645,6 +665,21 @@ uart3_rts_cts_pg_pins: uart3-rts-cts-pg-pins { pins = "PG8", "PG9"; function = "uart3"; }; + + uart4_pg_pins: uart4-pg-pins { + pins = "PG10", "PG11"; + function = "uart4"; + };
arch/arm/boot/dts/sun8i-r40.dtsi uart3_rts_cts_pg_pins: uart3-rts-cts-pg-pins { · line 682
Quoted 10 lines
arch/arm/boot/dts/sun8i-r40.dtsi +10
-0+ + uart5_ph_pins: uart5-ph-pins { + pins = "PH6", "PH7"; + function = "uart5"; + }; + + uart7_pi_pins: uart7-pi-pins { + pins = "PI20", "PI21"; + function = "uart7"; + };
Ditto
maxime
Hi, Maxime,
Thanks for your comments.
Quoted 4 lines · On Mon, 15 Mar 2021 16:29:04 +0100 Maxime Ripard <[email protected]> wrote:
Hi,
You seem to have some issues with your mailer that mangles the mail, you should try using git-send-email instead.
Will do.
Quoted 2 lines
There's also a bunch of checkpatch --strict issues you want to get rid of.
Thanks for pointing this out.
Besides the style warnings, which I've fixed, there are also warnings pertaining to documentation. Should I add the compatible strings to Documentation/devicetree/bindings/, and/or myself to the MAINTAINERS file? If so, would you suggest that I do so in a separate patch, or as part of another patch that adds the relevant files?
Quoted 10 lines · Ivan Uvarov
Quoted 6 lines
Add support for the Forlinx FETA40i-C SoM and OKA40i-C devboard[1] based on it. The DT is split into a .dtsi, which (hopefully) corresponds to the functions of the SoM itself, and a .dts for the devboard.
Instead of a URL that might be dead at some point, it would be better to have a proper commit log explaining why you did your patch that way
I'm not sure I understand.
By "that way" do you mean that I should explain why I've split up the devicetree, or something else?
The linked page is just a collection of information about the device; should I describe the SoM and/or devboard in my commit log instead?
Quoted 24 lines
Quoted 4 lines
...
arch/arm/boot/dts/sun8i-r40.dtsi +0
-0@@ -601,6 +603,14 @@ mmc2_pins: mmc2-pins { bias-pull-up;arch/arm/boot/dts/sun8i-r40.dtsi mmc2_pins: mmc2-pins { · line 613
Quoted 10 lines
arch/arm/boot/dts/sun8i-r40.dtsi +8
-0}; + mmc3_pins: mmc3-pins { + pins = "PI4", "PI5", "PI6", + "PI7", "PI8", "PI9"; + function = "mmc3"; + drive-strength = <30>; + bias-pull-up; + }; +This should be in a separate patch
Quoted 7 lines
arch/arm/boot/dts/sun8i-r40.dtsi +0
-0/omit-if-no-ref/ spi0_pc_pins: spi0-pc-pins { pins = "PC0", "PC1", "PC2";@@ -636,6 +646,16 @@ uart0_pb_pins: uart0-pb-pins { function = "uart0"; };
Quoted 13 lines
arch/arm/boot/dts/sun8i-r40.dtsi uart0_pb_pins: uart0-pb-pins { · line 658
Quoted 11 lines
arch/arm/boot/dts/sun8i-r40.dtsi +10
-0+ uart2_pi_pins: uart2-pi-pins { + pins = "PI18", "PI19"; + function = "uart2"; + }; + + uart2_rts_cts_pi_pins: uart2-rts-cts-pi-pins{ + pins = "PI16", "PI17"; + function = "uart2"; + }; +Ditto, and it should have /omit-if-no-ref/
Should the MMC pins be in the same patch as the UART pins?
Also, would it be a good idea to add /omit-if-no-ref/ to the uart3 pins as well while I'm here?
Quoted 25 lines
Quoted 12 lines
arch/arm/boot/dts/sun8i-r40.dtsi +0
-0uart3_pg_pins: uart3-pg-pins { pins = "PG6", "PG7";arch/arm/boot/dts/sun8i-r40.dtsi +5
-0function = "uart3";@@ -645,6 +665,21 @@ uart3_rts_cts_pg_pins: uart3-rts-cts-pg-pins { pins = "PG8", "PG9"; function = "uart3"; }; + + uart4_pg_pins: uart4-pg-pins { + pins = "PG10", "PG11"; + function = "uart4"; + };arch/arm/boot/dts/sun8i-r40.dtsi uart3_rts_cts_pg_pins: uart3-rts-cts-pg-pins { · line 682
Quoted 10 lines
arch/arm/boot/dts/sun8i-r40.dtsi +10
-0+ + uart5_ph_pins: uart5-ph-pins { + pins = "PH6", "PH7"; + function = "uart5"; + }; + + uart7_pi_pins: uart7-pi-pins { + pins = "PI20", "PI21"; + function = "uart7"; + };Ditto
Quoted 1 line
maxime
Since this won't be a monolithic patch anymore, would it make more sense to make this a three+-patch series:
1. add board+SOM DT without mmc3 or extra uarts;
2+. add pins to r40.dtsi;
3. add mmc3&uarts to the devboard DT,
or to make only two(+) patches:
1+. add pins to r40;
2. add board+SOM DT?
--
Regards,
Ivan Uvarov
Hi,
Quoted 7 lines · Ivan Uvarov
Quoted 2 lines
There's also a bunch of checkpatch --strict issues you want to get rid of.
Thanks for pointing this out.
Besides the style warnings, which I've fixed, there are also warnings pertaining to documentation. Should I add the compatible strings toDocumentation/devicetree/bindings/, and/or myself to theMAINTAINERSfile?
You can ignore the one about the MAINTAINERS file, but you should address the rest
Quoted 2 lines
If so, would you suggest that I do so in a separate patch, or as part of another patch that adds the relevant files?
In general, bindings patches should be separate indeed
Quoted 11 lines
Quoted 10 lines · Ivan Uvarov
Quoted 6 lines
Add support for the Forlinx FETA40i-C SoM and OKA40i-C devboard[1] based on it. The DT is split into a .dtsi, which (hopefully) corresponds to the functions of the SoM itself, and a .dts for the devboard.
Instead of a URL that might be dead at some point, it would be better to have a proper commit log explaining why you did your patch that way
Quoted 7 lines
I'm not sure I understand.
By "that way" do you mean that I should explain why I've split up the devicetree, or something else?
The linked page is just a collection of information about the device; should I describe the SoM and/or devboard in my commit log instead?
Yep. Essentially the git commit should be freestanding: if we look back at this commit in 5 years, will we able to tell without any context what this is about. Just pointing to an URL is risky, since the link might change or the website gone, and then you have no idea what it's about anymore.
Quoted 36 lines
Quoted 22 lines
arch/arm/boot/dts/sun8i-r40.dtsi mmc2_pins: mmc2-pins { · line 613
Quoted 12 lines
arch/arm/boot/dts/sun8i-r40.dtsi +8
-0@@ -601,6 +603,14 @@ mmc2_pins: mmc2-pins { bias-pull-up; }; + mmc3_pins: mmc3-pins { + pins = "PI4", "PI5", "PI6", + "PI7", "PI8", "PI9"; + function = "mmc3"; + drive-strength = <30>; + bias-pull-up; + }; +This should be in a separate patch
Quoted 7 lines
arch/arm/boot/dts/sun8i-r40.dtsi +0
-0/omit-if-no-ref/ spi0_pc_pins: spi0-pc-pins { pins = "PC0", "PC1", "PC2";@@ -636,6 +646,16 @@ uart0_pb_pins: uart0-pb-pins { function = "uart0"; };Quoted 13 lines
arch/arm/boot/dts/sun8i-r40.dtsi uart0_pb_pins: uart0-pb-pins { · line 658
Quoted 11 lines
arch/arm/boot/dts/sun8i-r40.dtsi +10
-0+ uart2_pi_pins: uart2-pi-pins { + pins = "PI18", "PI19"; + function = "uart2"; + }; + + uart2_rts_cts_pi_pins: uart2-rts-cts-pi-pins{ + pins = "PI16", "PI17"; + function = "uart2"; + }; +Ditto, and it should have /omit-if-no-ref/
Quoted 1 line
Should the MMC pins be in the same patch as the UART pins?
Yep
Quoted 2 lines
Also, would it be a good idea to add /omit-if-no-ref/ to the uart3 pins as well while I'm here?
To all the nodes honestly. If you want to make a preliminary patch doing this, go ahead :)
Quoted 28 lines
Quoted 16 lines
Quoted 16 lines
arch/arm/boot/dts/sun8i-r40.dtsi +0
-0uart3_pg_pins: uart3-pg-pins { pins = "PG6", "PG7";arch/arm/boot/dts/sun8i-r40.dtsi +9
-0function = "uart3";@@ -645,6 +665,21 @@ uart3_rts_cts_pg_pins: uart3-rts-cts-pg-pins { pins = "PG8", "PG9"; function = "uart3"; }; + + uart4_pg_pins: uart4-pg-pins { + pins = "PG10", "PG11"; + function = "uart4"; + }; + + uart5_ph_pins: uart5-ph-pins { + pins = "PH6", "PH7"; + function = "uart5";Quoted 8 lines
arch/arm/boot/dts/sun8i-r40.dtsi uart3_rts_cts_pg_pins: uart3-rts-cts-pg-pins { · line 682
Quoted 6 lines
arch/arm/boot/dts/sun8i-r40.dtsi +6
-0+ }; + + uart7_pi_pins: uart7-pi-pins { + pins = "PI20", "PI21"; + function = "uart7"; + };Ditto
Since this won't be a monolithic patch anymore, would it make more sense to make this a three+-patch series:
Quoted 8 lines
1. add board+SOM DT without mmc3 or extra uarts;
2+. add pins tor40.dtsi;
3. add mmc3&uarts to the devboard DT,or to make only two(+) patches:
1+. add pins to r40;
2. add board+SOM DT?
The latter would be more natural indeed
Maxime