| Message ID | 20190409002452.14551-3-megous@megous.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for Orange Pi 3 | expand |
Hi, On Tue, Apr 09, 2019 at 02:24:41AM +0200, megous@megous.com wrote: > +&mmc0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&mmc0_pins>; Since 5 minutes ago, that's now the default. > +&usb2otg { > + /* > + * Beware that this board will not automatically disconnect > + * VBUS from DCIN, when self-powered and used as a peripheral. > + */ > + dr_mode = "otg"; > + status = "okay"; > +}; As we were discussing, I guess leaving it as host is the safest option. I can fix both issues while applying if that's ok for you. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Based on the conversation about using common dtsi from this thread[1], I'm commenting here to make show the diff directly on the nodes, giving comments on each node so-that we can see the diff globally. On Tue, Apr 9, 2019 at 5:55 AM megous via linux-sunxi <linux-sunxi@googlegroups.com> wrote: > > From: Ondrej Jirman <megous@megous.com> > > Orange Pi 3 is a H6 based SBC made by Xulong, released in January 2019. It > has the following features: > > - Allwinner H6 quad-core 64-bit ARM Cortex-A53 > - GPU Mali-T720 > - 1GB or 2GB LPDDR3 RAM > - AXP805 PMIC > - AP6256 Wifi/BT 5.0 > - USB 2.0 host port (A) > - USB 2.0 micro usb, OTG > - USB 3.0 Host + 4 port USB hub (GL3510) > - Gigabit Ethernet (Realtek RTL8211E phy) > - HDMI 2.0 port > - soldered eMMC (optional) > - 3x LED (one is on the bottom) > - microphone > - audio jack > - PCIe > > Add basic support for the board. > > Signed-off-by: Ondrej Jirman <megous@megous.com> > --- > arch/arm64/boot/dts/allwinner/Makefile | 1 + > .../dts/allwinner/sun50i-h6-orangepi-3.dts | 216 ++++++++++++++++++ > 2 files changed, 217 insertions(+) > create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > > diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile > index e4dce2f6fa3a..285a7cb5135b 100644 > --- a/arch/arm64/boot/dts/allwinner/Makefile > +++ b/arch/arm64/boot/dts/allwinner/Makefile > @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-pc2.dtb > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-prime.dtb > 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-orangepi-3.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.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > new file mode 100644 > index 000000000000..5fbc5e410883 > --- /dev/null > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > @@ -0,0 +1,216 @@ > +// SPDX-License-Identifier: (GPL-2.0+ or MIT) > +/* > + * Copyright (C) 2019 Ondřej Jirman <megous@megous.com> > + */ > + > +/dts-v1/; > + > +#include "sun50i-h6.dtsi" > + > +#include <dt-bindings/gpio/gpio.h> > + > +/ { > + model = "OrangePi 3"; > + compatible = "xunlong,orangepi-3", "allwinner,sun50i-h6"; > + > + aliases { > + serial0 = &uart0; > + }; > + > + chosen { > + stdout-path = "serial0:115200n8"; > + }; > + > + leds { > + compatible = "gpio-leds"; > + > + power { > + label = "orangepi:red:power"; > + gpios = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */ > + default-state = "on"; > + }; > + > + status { > + label = "orangepi:green:status"; > + gpios = <&r_pio 0 7 GPIO_ACTIVE_HIGH>; /* PL7 */ > + }; > + }; > + > + 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; > + }; > +}; all above nodes are common to all h6 opi boards. > + > +&cpu0 { > + cpu-supply = <®_dcdca>; > +}; > + > +&ehci0 { > + status = "okay"; > +}; > + > +&ehci3 { > + status = "okay"; > +}; common to all h6 opi boards. > + > +&mmc0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&mmc0_pins>; > + vmmc-supply = <®_cldo1>; > + cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */ > + bus-width = <4>; > + status = "okay"; > +}; common to all h6 opi boards. > + > +&ohci0 { > + status = "okay"; > +}; > + > +&ohci3 { > + status = "okay"; > +}; common to all h6 opi boards. > + > +&pio { > + vcc-pc-supply = <®_bldo2>; > + vcc-pd-supply = <®_cldo1>; > +}; > + > +&r_i2c { > + status = "okay"; > + > + axp805: pmic@36 { > + compatible = "x-powers,axp805", "x-powers,axp806"; > + reg = <0x36>; > + interrupt-parent = <&r_intc>; > + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; > + interrupt-controller; > + #interrupt-cells = <1>; > + x-powers,self-working-mode; > + vina-supply = <®_vcc5v>; > + vinb-supply = <®_vcc5v>; > + vinc-supply = <®_vcc5v>; > + vind-supply = <®_vcc5v>; > + vine-supply = <®_vcc5v>; > + aldoin-supply = <®_vcc5v>; > + bldoin-supply = <®_vcc5v>; > + cldoin-supply = <®_vcc5v>; all these supply voltage regulators are common to h6 opi boards. > + > + regulators { > + reg_aldo1: aldo1 { > + regulator-always-on; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + regulator-name = "vcc-pl-led-ir"; > + }; same like other h6 boards. > + > + reg_aldo2: aldo2 { > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + regulator-name = "vcc33-audio-tv-ephy-mac"; > + }; rest of h6 opi boards used it for vcc-ac200, we may append ac200 and make it common since the voltage is same. > + > + /* 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"; > + }; same like above regulator. > + > + reg_bldo1: bldo1 { > + regulator-always-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-name = "vcc18-dram-bias-pll"; same like other h6 boards. > + }; > + > + reg_bldo2: bldo2 { > + regulator-always-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-name = "vcc-efuse-pcie-hdmi-pc"; > + }; same like other h6 boards. > + > + bldo3 { > + /* unused */ > + }; This is used in other h6 opi boards so we may attach status or re-enable it on board dts file. > + > + bldo4 { > + /* unused */ > + }; same like other h6 boards. > + > + reg_cldo1: cldo1 { > + regulator-always-on; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-2"; > + }; same like other h6 boards. > + > + cldo2 { > + /* unused */ > + }; > + > + cldo3 { > + /* unused */ > + }; These two used for wifi, so we may define on board dts or attach status property. > + > + reg_dcdca: dcdca { > + regulator-always-on; > + regulator-min-microvolt = <800000>; > + regulator-max-microvolt = <1160000>; > + regulator-name = "vdd-cpu"; > + }; > + > + reg_dcdcc: dcdcc { > + regulator-min-microvolt = <810000>; > + regulator-max-microvolt = <1080000>; > + 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 */ > + }; all above regulators are common to h6 boards. > + }; > + }; > +}; > + > +&uart0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&uart0_ph_pins>; > + status = "okay"; > +}; > + > +&usb2otg { > + /* > + * Beware that this board will not automatically disconnect > + * VBUS from DCIN, when self-powered and used as a peripheral. > + */ > + dr_mode = "otg"; > + status = "okay"; > +}; above nodes uart0, usb2otg are common to h6 opi boards. > + > +&usb2phy { > + usb0_id_det-gpios = <&pio 2 15 GPIO_ACTIVE_HIGH>; /* PC15 */ > + usb0_vbus-supply = <®_vcc5v>; > + usb3_vbus-supply = <®_vcc5v>; > + status = "okay"; > +}; > -- id detect pin can be differ, so we may move this on board specific dts. Note: the emac node is also common between h6 opi boards. [1] https://lkml.org/lkml/2019/4/5/857
On Tue, Apr 09, 2019 at 10:12:30AM +0200, Maxime Ripard wrote: > Hi, > > On Tue, Apr 09, 2019 at 02:24:41AM +0200, megous@megous.com wrote: > > +&mmc0 { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&mmc0_pins>; > > Since 5 minutes ago, that's now the default. Ah. :) > > +&usb2otg { > > + /* > > + * Beware that this board will not automatically disconnect > > + * VBUS from DCIN, when self-powered and used as a peripheral. > > + */ > > + dr_mode = "otg"; > > + status = "okay"; > > +}; > > As we were discussing, I guess leaving it as host is the safest > option. > > I can fix both issues while applying if that's ok for you. It's ok with me. Thank you. o. > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
Hi Jagan, On Tue, Apr 09, 2019 at 02:08:18PM +0530, Jagan Teki wrote: > Based on the conversation about using common dtsi from this thread[1], > I'm commenting here to make show the diff directly on the nodes, > giving comments on each node so-that we can see the diff globally. Thanks for the suggestions below. It mostly repeats the differences and commonalities I already stated in the previous discussion. I don't have much to add to what I already said previously, though, because you didn't address my concerns there. But I can restate and expand on those concerns. Previously I already agreed it's possible to base orangepi-3.dts on orangepi.dtsi, and proposed a maintainable way forward, and why to follow it (to quote myself): Schematics allow for high amount of variability in the power tree (see all the NC (not connected) / 0R resistors) in the schematic around AXP805. Every board based on this Xunlong design can be subtly different. I already suggested a maintainable solution, below. Where base dtsi has empty config for regulators and every board based on that just defines it completely for itself. A few regulators (for CPU/GPU) will most probably have the same meaning on every derived board, so these can probably be kept in dtsi without causing too much annoyance. It's unpleasant to have wrong regulator setup defined in an underlying dtsi, and then trying to override it by removing/adding random properties in the board dts for the new boards based on that, so that it fits. The rest of the current HW descriptions in the sun50i-h6-orangepi.dtsi can be shared (as of now). My suggestion was this: So to base Orange Pi 3 dts on top of existing sun50i-h6-orangepi.dtsi I'd have to first move some things out of the base dtsi to the OrangePi Lite2 and One Plus board dts files, in order to have sun50i-h6-orangepi.dtsi only describe HW that is *really* shared by these 2 boards and Orange Pi 3. If I do that, I'd undefine all the axp805 regulator nodes and move the configurations to each of the 3 board files. That will probably end up being the least confusing and most maintainable. See axp81x.dtsi lines 86-144 for what I mean. You seem to be suggesting a solution where every time we add an OrangePi H6 based board, the person adding it will have to go through the base dtsi and all the other boards based on it, status disable or otherwise change regulators in the base dtsi, patch all the other boards to re-enable it. It would be already unpleasant just adding a third board based on this approach. And when the fourth board is added, with another small differences in the regulator use/meanings, the person will be looking at patching 4 dts files + adding one for his own board. For what benefit, to save some bytes right now? I think maintainability, ease of adding new boards is more important, than having a dtsi that tries to maximally cover all the commonalities between the existing boards right now, without regards for the future. That's why I suggested an approach like in axp81x.dtsi lines 86-144. thank you and regards, o. > On Tue, Apr 9, 2019 at 5:55 AM megous via linux-sunxi > <linux-sunxi@googlegroups.com> wrote: > > > > From: Ondrej Jirman <megous@megous.com> > > > > Orange Pi 3 is a H6 based SBC made by Xulong, released in January 2019. It > > has the following features: > > > > - Allwinner H6 quad-core 64-bit ARM Cortex-A53 > > - GPU Mali-T720 > > - 1GB or 2GB LPDDR3 RAM > > - AXP805 PMIC > > - AP6256 Wifi/BT 5.0 > > - USB 2.0 host port (A) > > - USB 2.0 micro usb, OTG > > - USB 3.0 Host + 4 port USB hub (GL3510) > > - Gigabit Ethernet (Realtek RTL8211E phy) > > - HDMI 2.0 port > > - soldered eMMC (optional) > > - 3x LED (one is on the bottom) > > - microphone > > - audio jack > > - PCIe > > > > Add basic support for the board. > > > > Signed-off-by: Ondrej Jirman <megous@megous.com> > > --- > > arch/arm64/boot/dts/allwinner/Makefile | 1 + > > .../dts/allwinner/sun50i-h6-orangepi-3.dts | 216 ++++++++++++++++++ > > 2 files changed, 217 insertions(+) > > create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > > > > diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile > > index e4dce2f6fa3a..285a7cb5135b 100644 > > --- a/arch/arm64/boot/dts/allwinner/Makefile > > +++ b/arch/arm64/boot/dts/allwinner/Makefile > > @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-pc2.dtb > > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-prime.dtb > > 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-orangepi-3.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.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > > new file mode 100644 > > index 000000000000..5fbc5e410883 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > > @@ -0,0 +1,216 @@ > > +// SPDX-License-Identifier: (GPL-2.0+ or MIT) > > +/* > > + * Copyright (C) 2019 Ondřej Jirman <megous@megous.com> > > + */ > > + > > +/dts-v1/; > > + > > +#include "sun50i-h6.dtsi" > > + > > +#include <dt-bindings/gpio/gpio.h> > > + > > +/ { > > + model = "OrangePi 3"; > > + compatible = "xunlong,orangepi-3", "allwinner,sun50i-h6"; > > + > > + aliases { > > + serial0 = &uart0; > > + }; > > + > > + chosen { > > + stdout-path = "serial0:115200n8"; > > + }; > > + > > + leds { > > + compatible = "gpio-leds"; > > + > > + power { > > + label = "orangepi:red:power"; > > + gpios = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */ > > + default-state = "on"; > > + }; > > + > > + status { > > + label = "orangepi:green:status"; > > + gpios = <&r_pio 0 7 GPIO_ACTIVE_HIGH>; /* PL7 */ > > + }; > > + }; > > + > > + 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; > > + }; > > +}; > > all above nodes are common to all h6 opi boards. > > > + > > +&cpu0 { > > + cpu-supply = <®_dcdca>; > > +}; > > + > > +&ehci0 { > > + status = "okay"; > > +}; > > + > > +&ehci3 { > > + status = "okay"; > > +}; > > common to all h6 opi boards. > > > + > > +&mmc0 { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&mmc0_pins>; > > + vmmc-supply = <®_cldo1>; > > + cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */ > > + bus-width = <4>; > > + status = "okay"; > > +}; > > common to all h6 opi boards. > > > + > > +&ohci0 { > > + status = "okay"; > > +}; > > + > > +&ohci3 { > > + status = "okay"; > > +}; > > common to all h6 opi boards. > > > + > > +&pio { > > + vcc-pc-supply = <®_bldo2>; > > + vcc-pd-supply = <®_cldo1>; > > +}; > > + > > +&r_i2c { > > + status = "okay"; > > + > > + axp805: pmic@36 { > > + compatible = "x-powers,axp805", "x-powers,axp806"; > > + reg = <0x36>; > > + interrupt-parent = <&r_intc>; > > + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; > > + interrupt-controller; > > + #interrupt-cells = <1>; > > + x-powers,self-working-mode; > > + vina-supply = <®_vcc5v>; > > + vinb-supply = <®_vcc5v>; > > + vinc-supply = <®_vcc5v>; > > + vind-supply = <®_vcc5v>; > > + vine-supply = <®_vcc5v>; > > + aldoin-supply = <®_vcc5v>; > > + bldoin-supply = <®_vcc5v>; > > + cldoin-supply = <®_vcc5v>; > > all these supply voltage regulators are common to h6 opi boards. > > > + > > + regulators { > > + reg_aldo1: aldo1 { > > + regulator-always-on; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + regulator-name = "vcc-pl-led-ir"; > > + }; > > same like other h6 boards. > > > + > > + reg_aldo2: aldo2 { > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + regulator-name = "vcc33-audio-tv-ephy-mac"; > > + }; > > rest of h6 opi boards used it for vcc-ac200, we may append ac200 and > make it common since the voltage is same. > > > + > > + /* 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"; > > + }; > > same like above regulator. > > > + > > + reg_bldo1: bldo1 { > > + regulator-always-on; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <1800000>; > > + regulator-name = "vcc18-dram-bias-pll"; > > same like other h6 boards. > > > + }; > > + > > + reg_bldo2: bldo2 { > > + regulator-always-on; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <1800000>; > > + regulator-name = "vcc-efuse-pcie-hdmi-pc"; > > + }; > > same like other h6 boards. > > > + > > + bldo3 { > > + /* unused */ > > + }; > > This is used in other h6 opi boards so we may attach status or > re-enable it on board dts file. > > > + > > + bldo4 { > > + /* unused */ > > + }; > > same like other h6 boards. > > > + > > + reg_cldo1: cldo1 { > > + regulator-always-on; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + regulator-name = "vcc33-io-pd-emmc-sd-usb-uart-2"; > > + }; > > same like other h6 boards. > > > + > > + cldo2 { > > + /* unused */ > > + }; > > + > > + cldo3 { > > + /* unused */ > > + }; > > These two used for wifi, so we may define on board dts or attach > status property. > > > + > > + reg_dcdca: dcdca { > > + regulator-always-on; > > + regulator-min-microvolt = <800000>; > > + regulator-max-microvolt = <1160000>; > > + regulator-name = "vdd-cpu"; > > + }; > > + > > + reg_dcdcc: dcdcc { > > + regulator-min-microvolt = <810000>; > > + regulator-max-microvolt = <1080000>; > > + 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 */ > > + }; > > all above regulators are common to h6 boards. > > > + }; > > + }; > > +}; > > + > > +&uart0 { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&uart0_ph_pins>; > > + status = "okay"; > > +}; > > + > > +&usb2otg { > > + /* > > + * Beware that this board will not automatically disconnect > > + * VBUS from DCIN, when self-powered and used as a peripheral. > > + */ > > + dr_mode = "otg"; > > + status = "okay"; > > +}; > > above nodes uart0, usb2otg are common to h6 opi boards. > > > + > > +&usb2phy { > > + usb0_id_det-gpios = <&pio 2 15 GPIO_ACTIVE_HIGH>; /* PC15 */ > > + usb0_vbus-supply = <®_vcc5v>; > > + usb3_vbus-supply = <®_vcc5v>; > > + status = "okay"; > > +}; > > -- > > id detect pin can be differ, so we may move this on board specific dts. > > Note: the emac node is also common between h6 opi boards. > > [1] https://lkml.org/lkml/2019/4/5/857
On Tue, Apr 09, 2019 at 01:31:57PM +0200, Ondřej Jirman wrote: > Hi Jagan, > > On Tue, Apr 09, 2019 at 02:08:18PM +0530, Jagan Teki wrote: > > Based on the conversation about using common dtsi from this thread[1], > > I'm commenting here to make show the diff directly on the nodes, > > giving comments on each node so-that we can see the diff globally. > > Thanks for the suggestions below. It mostly repeats the differences and > commonalities I already stated in the previous discussion. > > I don't have much to add to what I already said previously, though, because you > didn't address my concerns there. But I can restate and expand on those > concerns. > > Previously I already agreed it's possible to base orangepi-3.dts on > orangepi.dtsi, and proposed a maintainable way forward, and why to follow it (to > quote myself): > > Schematics allow for high amount of variability in the power tree (see all the > NC (not connected) / 0R resistors) in the schematic around AXP805. Every board > based on this Xunlong design can be subtly different. > > I already suggested a maintainable solution, below. Where base dtsi has empty > config for regulators and every board based on that just defines it completely > for itself. > > A few regulators (for CPU/GPU) will most probably have the same meaning on > every derived board, so these can probably be kept in dtsi without causing too > much annoyance. > > It's unpleasant to have wrong regulator setup defined in an underlying dtsi, > and then trying to override it by removing/adding random properties in the > board dts for the new boards based on that, so that it fits. > > The rest of the current HW descriptions in the sun50i-h6-orangepi.dtsi can be > shared (as of now). > > My suggestion was this: > > So to base Orange Pi 3 dts on top of existing sun50i-h6-orangepi.dtsi I'd have > to first move some things out of the base dtsi to the OrangePi Lite2 and One > Plus board dts files, in order to have sun50i-h6-orangepi.dtsi only describe > HW that is *really* shared by these 2 boards and Orange Pi 3. > > If I do that, I'd undefine all the axp805 regulator nodes and move the > configurations to each of the 3 board files. That will probably end up being > the least confusing and most maintainable. See axp81x.dtsi lines 86-144 for > what I mean. > > You seem to be suggesting a solution where every time we add an OrangePi H6 > based board, the person adding it will have to go through the base dtsi and all > the other boards based on it, status disable or otherwise change regulators in > the base dtsi, patch all the other boards to re-enable it. > > It would be already unpleasant just adding a third board based on this approach. > And when the fourth board is added, with another small differences in the > regulator use/meanings, the person will be looking at patching 4 dts files > + adding one for his own board. For what benefit, to save some bytes right now? > > I think maintainability, ease of adding new boards is more important, than > having a dtsi that tries to maximally cover all the commonalities between the > existing boards right now, without regards for the future. That's why > I suggested an approach like in axp81x.dtsi lines 86-144. I agree. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Tue, Apr 09, 2019 at 11:33:04AM +0200, Ondřej Jirman wrote: > On Tue, Apr 09, 2019 at 10:12:30AM +0200, Maxime Ripard wrote: > > Hi, > > > > On Tue, Apr 09, 2019 at 02:24:41AM +0200, megous@megous.com wrote: > > > +&mmc0 { > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <&mmc0_pins>; > > > > Since 5 minutes ago, that's now the default. > > Ah. :) > > > > +&usb2otg { > > > + /* > > > + * Beware that this board will not automatically disconnect > > > + * VBUS from DCIN, when self-powered and used as a peripheral. > > > + */ > > > + dr_mode = "otg"; > > > + status = "okay"; > > > +}; > > > > As we were discussing, I guess leaving it as host is the safest > > option. > > > > I can fix both issues while applying if that's ok for you. > > It's ok with me. Thank you. Done, thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Hi Ondřej Jirman, On Tue, Apr 9, 2019 at 5:01 PM Ondřej Jirman <megous@megous.com> wrote: > > Hi Jagan, > > On Tue, Apr 09, 2019 at 02:08:18PM +0530, Jagan Teki wrote: > > Based on the conversation about using common dtsi from this thread[1], > > I'm commenting here to make show the diff directly on the nodes, > > giving comments on each node so-that we can see the diff globally. > > Thanks for the suggestions below. It mostly repeats the differences and > commonalities I already stated in the previous discussion. > > I don't have much to add to what I already said previously, though, because you > didn't address my concerns there. But I can restate and expand on those > concerns. > > Previously I already agreed it's possible to base orangepi-3.dts on > orangepi.dtsi, and proposed a maintainable way forward, and why to follow it (to > quote myself): > > Schematics allow for high amount of variability in the power tree (see all the > NC (not connected) / 0R resistors) in the schematic around AXP805. Every board > based on this Xunlong design can be subtly different. Well, how about defining them in required dts file and group it common regulators in dtsi? The idea of all these 3 H6 OPI boards are evaluated in the same design consideration by adding new IP's in new boards design by keeping the core things common like lite2 has wifi, one plus has emac and 3 has PCIe etc. This is what I got from Steven(OrangePI). > > I already suggested a maintainable solution, below. Where base dtsi has empty > config for regulators and every board based on that just defines it completely > for itself. > > A few regulators (for CPU/GPU) will most probably have the same meaning on > every derived board, so these can probably be kept in dtsi without causing too > much annoyance. > > It's unpleasant to have wrong regulator setup defined in an underlying dtsi, > and then trying to override it by removing/adding random properties in the > board dts for the new boards based on that, so that it fits. If we manage them properly by moving common things in dtsi and rest in dts, we may avoid these underlying issues. > > The rest of the current HW descriptions in the sun50i-h6-orangepi.dtsi can be > shared (as of now). > > My suggestion was this: > > So to base Orange Pi 3 dts on top of existing sun50i-h6-orangepi.dtsi I'd have > to first move some things out of the base dtsi to the OrangePi Lite2 and One > Plus board dts files, in order to have sun50i-h6-orangepi.dtsi only describe > HW that is *really* shared by these 2 boards and Orange Pi 3. > > If I do that, I'd undefine all the axp805 regulator nodes and move the > configurations to each of the 3 board files. That will probably end up being > the least confusing and most maintainable. See axp81x.dtsi lines 86-144 for > what I mean. > > You seem to be suggesting a solution where every time we add an OrangePi H6 > based board, the person adding it will have to go through the base dtsi and all > the other boards based on it, status disable or otherwise change regulators in > the base dtsi, patch all the other boards to re-enable it. > > It would be already unpleasant just adding a third board based on this approach. > And when the fourth board is added, with another small differences in the > regulator use/meanings, the person will be looking at patching 4 dts files > + adding one for his own board. For what benefit, to save some bytes right now? > > I think maintainability, ease of adding new boards is more important, than > having a dtsi that tries to maximally cover all the commonalities between the > existing boards right now, without regards for the future. That's why > I suggested an approach like in axp81x.dtsi lines 86-144. True, if the boards indeed are different but we can maintain easily if the boards are from same family of design based my experience and few boards do share common things in dtsi already. Anyway, thanks for inputs. Seems like patch is applied already may be we can move into dtsi if require in future. Thanks, Jagan.
diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile index e4dce2f6fa3a..285a7cb5135b 100644 --- a/arch/arm64/boot/dts/allwinner/Makefile +++ b/arch/arm64/boot/dts/allwinner/Makefile @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-pc2.dtb dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-prime.dtb 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-orangepi-3.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.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts new file mode 100644 index 000000000000..5fbc5e410883 --- /dev/null +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts @@ -0,0 +1,216 @@ +// SPDX-License-Identifier: (GPL-2.0+ or MIT) +/* + * Copyright (C) 2019 Ondřej Jirman <megous@megous.com> + */ + +/dts-v1/; + +#include "sun50i-h6.dtsi" + +#include <dt-bindings/gpio/gpio.h> + +/ { + model = "OrangePi 3"; + compatible = "xunlong,orangepi-3", "allwinner,sun50i-h6"; + + aliases { + serial0 = &uart0; + }; + + chosen { + stdout-path = "serial0:115200n8"; + }; + + leds { + compatible = "gpio-leds"; + + power { + label = "orangepi:red:power"; + gpios = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */ + default-state = "on"; + }; + + status { + label = "orangepi:green:status"; + gpios = <&r_pio 0 7 GPIO_ACTIVE_HIGH>; /* PL7 */ + }; + }; + + 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; + }; +}; + +&cpu0 { + cpu-supply = <®_dcdca>; +}; + +&ehci0 { + status = "okay"; +}; + +&ehci3 { + status = "okay"; +}; + +&mmc0 { + pinctrl-names = "default"; + pinctrl-0 = <&mmc0_pins>; + vmmc-supply = <®_cldo1>; + cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */ + bus-width = <4>; + status = "okay"; +}; + +&ohci0 { + status = "okay"; +}; + +&ohci3 { + status = "okay"; +}; + +&pio { + vcc-pc-supply = <®_bldo2>; + vcc-pd-supply = <®_cldo1>; +}; + +&r_i2c { + status = "okay"; + + axp805: pmic@36 { + compatible = "x-powers,axp805", "x-powers,axp806"; + reg = <0x36>; + interrupt-parent = <&r_intc>; + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; + interrupt-controller; + #interrupt-cells = <1>; + x-powers,self-working-mode; + vina-supply = <®_vcc5v>; + vinb-supply = <®_vcc5v>; + vinc-supply = <®_vcc5v>; + vind-supply = <®_vcc5v>; + vine-supply = <®_vcc5v>; + aldoin-supply = <®_vcc5v>; + bldoin-supply = <®_vcc5v>; + cldoin-supply = <®_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-always-on; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-name = "vcc-efuse-pcie-hdmi-pc"; + }; + + bldo3 { + /* unused */ + }; + + 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-name = "vdd-cpu"; + }; + + reg_dcdcc: dcdcc { + regulator-min-microvolt = <810000>; + regulator-max-microvolt = <1080000>; + 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 */ + }; + }; + }; +}; + +&uart0 { + pinctrl-names = "default"; + pinctrl-0 = <&uart0_ph_pins>; + status = "okay"; +}; + +&usb2otg { + /* + * Beware that this board will not automatically disconnect + * VBUS from DCIN, when self-powered and used as a peripheral. + */ + dr_mode = "otg"; + status = "okay"; +}; + +&usb2phy { + usb0_id_det-gpios = <&pio 2 15 GPIO_ACTIVE_HIGH>; /* PC15 */ + usb0_vbus-supply = <®_vcc5v>; + usb3_vbus-supply = <®_vcc5v>; + status = "okay"; +};