On Thu, 6 Apr 2023 14:32:18 +0100 Andre Przywara <andre.przyw...@arm.com> wrote:
Hi, briefly forgot about sunxi-common-regulators.dtsi, so: > On Thu, 6 Apr 2023 14:35:13 +0200 > Ludwig Kormann <ludwig.korm...@in-circuit.de> wrote: > > Hi Ludwig, > > > Add board support for ICnova A20 SomPi compute module on > > ICnova ADB4006 development board. > > thanks for sending this! > For new boards we need to get the DT reviewed on the Linux list first, so > please send the DT there (To: Samuel, Jernej, Chen-Yu, Rob, Krzysztof , Cc: > linux-sunxi, linux-arm-kernel). This is more a process thing, since the DT > review knowledge and also compliance testing is more prominent on the > Linux side. > > Once it has been approved and queued, we pick it up from there. > > I will give you some feedback on the DT anyway: > > > Specification: > > SoM > > - Processor: Allwinner A20 Cortex-A7 Dual Core at 1GHz > > - 512MB DDR3 RAM > > - Fast Ethernet (Phy: Realtek RTL8201CP) > > So if you have a SoM/host board setup, we typically describe both > components in separate files: a SoM .dtsi, and a devboard .dts. > The devboard includes the SoM then. See the SoPine [1] for an example. > > [1] arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts > > I also see that the SoM has NAND flash, does that work for you with > mainline U-Boot and/or Linux? Is it SLC, by any chance? > > > ADB4006 > > - I2C > > - 2x USB 2.0 > > - 1x Fast Ethernet port > > - 1x SATA > > - 2x buttons > > Are those buttons "power" and "FEL"? If not (connected to GPIOs), you > should describe them in the DT, see "gpio-keys" in > sun50i-h5-orangepi-pc2.dts for an example. > > > - 2x LEDS > > - serial console > > - HDMI > > - µSD-Card slot > > - Audio Line-In / Line-Out > > - GPIO pinheaders > > > > https://wiki.in-circuit.de/index.php5?title=ICnova_ADB4006 > > https://wiki.in-circuit.de/index.php5?title=ICnova_A20_SODIMM > > Signed-off-by: Ludwig Kormann <ludwig.korm...@in-circuit.de> > > --- > > arch/arm/dts/Makefile | 1 + > > arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts | 248 ++++++++++++++++++ > > board/sunxi/MAINTAINERS | 5 + > > configs/icnova-a20-adb4006_defconfig | 25 ++ > > 4 files changed, 279 insertions(+) > > create mode 100644 arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts > > create mode 100644 configs/icnova-a20-adb4006_defconfig > > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > > index 0a9b1f7749..47dcaff780 100644 > > --- a/arch/arm/dts/Makefile > > +++ b/arch/arm/dts/Makefile > > @@ -623,6 +623,7 @@ dtb-$(CONFIG_MACH_SUN7I) += \ > > sun7i-a20-haoyu-marsboard.dtb \ > > sun7i-a20-hummingbird.dtb \ > > sun7i-a20-i12-tvbox.dtb \ > > + sun7i-a20-icnova-a20-adb4006.dtb \ > > sun7i-a20-icnova-swac.dtb \ > > Out of interest, how does this SoM/board relate to this "swac" thing here? > Is that an older, integrated board? > > > sun7i-a20-itead-ibox.dtb \ > > sun7i-a20-lamobo-r1.dtb \ > > diff --git a/arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts > > b/arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts > > new file mode 100644 > > index 0000000000..e43df838ec > > --- /dev/null > > +++ b/arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts > > @@ -0,0 +1,248 @@ > > +/* > > + * Copyright 2023 Ludwig Kormann <ludwig.korm...@in-circuit.de> > > + * > > + * This file is dual-licensed: you can use it either under the terms > > Please use the shorter SPDX header for licensing: > // SPDX-License-Identifier: (GPL-2.0 OR MIT) > > > + * of the GPL or the X11 license, at your option. Note that this dual > > + * licensing only applies to this file, and not this project as a > > + * whole. > > + * > > + * a) This file is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License as > > + * published by the Free Software Foundation; either version 2 of the > > + * License, or (at your option) any later version. > > + * > > + * This file is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * Or, alternatively, > > + * > > + * b) Permission is hereby granted, free of charge, to any person > > + * obtaining a copy of this software and associated documentation > > + * files (the "Software"), to deal in the Software without > > + * restriction, including without limitation the rights to use, > > + * copy, modify, merge, publish, distribute, sublicense, and/or > > + * sell copies of the Software, and to permit persons to whom the > > + * Software is furnished to do so, subject to the following > > + * conditions: > > + * > > + * The above copyright notice and this permission notice shall be > > + * included in all copies or substantial portions of the Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES > > + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT > > + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, > > + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > > + * OTHER DEALINGS IN THE SOFTWARE. > > + */ > > + > > +/dts-v1/; > > +#include "sun7i-a20.dtsi" > > +#include "sunxi-common-regulators.dtsi" > > + > > +#include <dt-bindings/gpio/gpio.h> > > +#include <dt-bindings/interrupt-controller/irq.h> > > + > > +/ { > > + model = "In-Circuit ICnova A20 ADB4006"; > > + compatible = "in-circuit,icnova-a20-adb4006", "incircuit,icnova-a20", > > + "allwinner,sun7i-a20"; > > + > > + aliases { > > + serial0 = &uart0; > > + }; > > + > > + chosen { > > + stdout-path = "serial0:115200n8"; > > + }; > > + > > + hdmi-connector { > > + compatible = "hdmi-connector"; > > + type = "a"; > > + > > + port { > > + hdmi_con_in: endpoint { > > + remote-endpoint = <&hdmi_out_con>; > > + }; > > + }; > > + }; > > + > > + leds { > > + compatible = "gpio-leds"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&led_pins_icnova_a20_adb4006>; > > Those two properties (and the pinctrl child node it refers to) are not > needed, the "gpios" property takes care of that. > > > + > > + led-0 { > > + label = "icnova_a20_adb4006:yellow:usr"; > > The "label" property is deprecated, please use "function" and "color" > instead. Compare to sun50i-h616-orangepi-zero2.dts. > > > + gpios = <&pio 7 21 GPIO_ACTIVE_HIGH>; > > Please add the pin name ("PH21") as a comment, behind the semicolon. > > > + }; > > + > > + led-1 { > > + label = "icnova_a20_adb4006:red:usr"; > > + gpios = <&pio 7 20 GPIO_ACTIVE_HIGH>; > > + linux,default-trigger = "heartbeat"; > > + }; > > + }; > > +}; > > + > > +&ahci { > > + target-supply = <®_ahci_5v>; > > + status = "okay"; > > +}; > > + > > +&codec { > > + status = "okay"; > > +}; > > + > > +&cpu0 { > > + cpu-supply = <®_dcdc2>; > > +}; > > + > > +&de { > > + status = "okay"; > > +}; > > + > > +&ehci0 { > > + status = "okay"; > > +}; > > + > > +&ehci1 { > > + status = "okay"; > > +}; > > + > > +&gmac { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&gmac_mii_pins>; > > + phy-handle = <&phy1>; > > + phy-mode = "mii"; > > + status = "okay"; > > +}; > > + > > +&hdmi { > > + status = "okay"; > > +}; > > + > > +&hdmi_out { > > + hdmi_out_con: endpoint { > > + remote-endpoint = <&hdmi_con_in>; > > + }; > > +}; > > + > > +&i2c0 { > > + status = "okay"; > > + > > + axp209: pmic@34 { > > + reg = <0x34>; > > + interrupt-parent = <&nmi_intc>; > > + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; > > + }; > > +}; > > + > > +&i2c1 { > > + status = "okay"; > > +}; > > + > > +&gmac_mdio { > > + phy1: ethernet-phy@1 { > > + reg = <1>; > > + }; > > +}; > > + > > +&mmc0 { > > + vmmc-supply = <®_vcc3v3>; > > + bus-width = <4>; > > + cd-gpios = <&pio 7 1 GPIO_ACTIVE_LOW>; /* PH1 */ > > + status = "okay"; > > +}; > > + > > +&ohci0 { > > + status = "okay"; > > +}; > > + > > +&ohci1 { > > + status = "okay"; > > +}; > > + > > +&otg_sram { > > + status = "okay"; > > +}; > > + > > +&pio { > > + led_pins_icnova_a20_adb4006: led-pins { > > + pins = "PH20", "PH21"; > > + function = "gpio_out"; > > + drive-strength = <20>; > > + }; > > +}; > > As mentioned above, this whole pio node is not needed. > > > + > > +®_ahci_5v { > > + status = "okay"; > > +}; > > + > > +#include "axp209.dtsi" > > + > > +&ac_power_supply { > > + status = "okay"; > > +}; > > + > > +®_dcdc2 { > > + regulator-always-on; > > + regulator-min-microvolt = <1000000>; > > + regulator-max-microvolt = <1400000>; > > + regulator-name = "vdd-cpu"; > > +}; > > + > > +®_dcdc3 { > > + regulator-always-on; > > + regulator-min-microvolt = <1000000>; > > + regulator-max-microvolt = <1400000>; > > + regulator-name = "vdd-int-dll"; > > +}; > > + > > +®_ldo1 { > > + regulator-name = "vdd-rtc"; > > +}; > > + > > +®_ldo2 { > > + regulator-always-on; > > + regulator-min-microvolt = <3000000>; > > + regulator-max-microvolt = <3000000>; > > + regulator-name = "avcc"; > > +}; > > + > > +®_ldo4 { > > + regulator-always-on; > > + regulator-min-microvolt = <2800000>; > > + regulator-max-microvolt = <2800000>; > > + regulator-name = "csi-io"; > > Why is this always on? Is there a camera connected that's not described in > the DT? > > > +}; > > + > > +®_usb1_vbus { > > + status = "okay"; > > +}; > > + > > +®_usb2_vbus { > > + status = "okay"; > > +}; > > + > > +&uart0 { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&uart0_pb_pins>; > > + status = "okay"; > > +}; > > + > > +&usb_otg { > > + dr_mode = "otg"; > > + status = "okay"; > > +}; > > + > > +&usbphy { > > + usb0_id_det-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH | GPIO_PULL_UP>; /* PH4 > > */ > > + usb1_vbus-supply = <®_usb1_vbus>; > > + usb2_vbus-supply = <®_usb2_vbus>; > > So looking at the defconfig below, PH3, PH5 and PH6 also seem to play a > role for USB. Please describe them in the DT here. We are in the process > of removing the U-Boot Kconfig hacks for those GPIOs, in favour of using > DT. Not sure if VBUS is really provided by the AXP then, are there separate > GPIO controlled regulators, by any chance? Scratch the part about the regulators, that's what is already in sunxi-common-regulators.dtsi. You should add a usb0_vbus_det-gpios property for PH5, though. > > > + status = "okay"; > > +}; > > diff --git a/board/sunxi/MAINTAINERS b/board/sunxi/MAINTAINERS > > index 80e3f4be4b..2bbf0188d7 100644 > > --- a/board/sunxi/MAINTAINERS > > +++ b/board/sunxi/MAINTAINERS > > @@ -231,6 +231,11 @@ M: Stefan Roese <s...@denx.de> > > S: Maintained > > F: configs/icnova-a20-swac_defconfig > > > > +ICnova-A20-ADB4006 BOARD > > +M: Ludwig Kormann <ludwig.korm...@in-circuit.de> > > +S: Maintained > > +F: configs/icnova-a20-adb4006_defconfig > > + > > ITEAD IBOX BOARD > > M: Marcus Cooper <codekip...@gmail.com> > > S: Maintained > > diff --git a/configs/icnova-a20-adb4006_defconfig > > b/configs/icnova-a20-adb4006_defconfig > > new file mode 100644 > > index 0000000000..c327410ca7 > > --- /dev/null > > +++ b/configs/icnova-a20-adb4006_defconfig > > @@ -0,0 +1,25 @@ > > +CONFIG_ARM=y > > +CONFIG_ARCH_SUNXI=y > > +CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-icnova-a20-adb4006" > > +CONFIG_SPL=y > > +CONFIG_MACH_SUN7I=y > > +CONFIG_DRAM_CLK=384 > > +CONFIG_USB0_VBUS_DET="PH5" > > +CONFIG_USB0_ID_DET="PH4" > > +CONFIG_USB1_VBUS_PIN="PH6" > > +CONFIG_USB2_VBUS_PIN="PH3" > > These four pins should all have a DT equivalent, then they could go away > (eventually). > > > +CONFIG_SATAPWR="PB8" > > SATAPWR is going to go away. Is that regulator controlling power for a > SATA *disk*? Via this white 2pin header? > There is no really perfect DT replacement for this, but you could copy > this solution here: > https://lore.kernel.org/linux-arm-kernel/20230120012616.30960-1-andre.przyw...@arm.com/ Also scratch that, PB8 is mentioned via target-supply already, so that should be fine. Sorry for the confusion about those! Cheers, Andre > > Cheers (und Grüße an die Elbe!), > Andre > > > +CONFIG_AHCI=y > > +# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set > > +CONFIG_SPL_I2C=y > > +CONFIG_SCSI_AHCI=y > > +CONFIG_SYS_I2C_MVTWSI=y > > +CONFIG_SYS_I2C_SLAVE=0x7f > > +CONFIG_SYS_I2C_SPEED=400000 > > +CONFIG_ETH_DESIGNWARE=y > > +CONFIG_MII=y > > +CONFIG_SUN7I_GMAC=y > > +CONFIG_AXP_ALDO4_VOLT=2800 > > +CONFIG_SCSI=y > > +CONFIG_USB_EHCI_HCD=y > > +CONFIG_USB_OHCI_HCD=y >