[PATCH 1/1] arm: dts: icnova-a20-adb4006: Add board support

Andre Przywara andre.przywara at arm.com
Thu Apr 6 16:10:28 CEST 2023


On Thu, 6 Apr 2023 14:32:18 +0100
Andre Przywara <andre.przywara at arm.com> wrote:

Hi,

briefly forgot about sunxi-common-regulators.dtsi, so:

> On Thu,  6 Apr 2023 14:35:13 +0200
> Ludwig Kormann <ludwig.kormann at 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.kormann at 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.kormann at 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 = <&reg_ahci_5v>;
> > +	status = "okay";
> > +};
> > +
> > +&codec {
> > +	status = "okay";
> > +};
> > +
> > +&cpu0 {
> > +	cpu-supply = <&reg_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 at 34 {
> > +		reg = <0x34>;
> > +		interrupt-parent = <&nmi_intc>;
> > +		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> > +	};
> > +};
> > +
> > +&i2c1 {
> > +	status = "okay";
> > +};
> > +
> > +&gmac_mdio {
> > +	phy1: ethernet-phy at 1 {
> > +		reg = <1>;
> > +	};
> > +};
> > +
> > +&mmc0 {
> > +	vmmc-supply = <&reg_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.
> 
> > +
> > +&reg_ahci_5v {
> > +	status = "okay";
> > +};
> > +
> > +#include "axp209.dtsi"
> > +
> > +&ac_power_supply {
> > +	status = "okay";
> > +};
> > +
> > +&reg_dcdc2 {
> > +	regulator-always-on;
> > +	regulator-min-microvolt = <1000000>;
> > +	regulator-max-microvolt = <1400000>;
> > +	regulator-name = "vdd-cpu";
> > +};
> > +
> > +&reg_dcdc3 {
> > +	regulator-always-on;
> > +	regulator-min-microvolt = <1000000>;
> > +	regulator-max-microvolt = <1400000>;
> > +	regulator-name = "vdd-int-dll";
> > +};
> > +
> > +&reg_ldo1 {
> > +	regulator-name = "vdd-rtc";
> > +};
> > +
> > +&reg_ldo2 {
> > +	regulator-always-on;
> > +	regulator-min-microvolt = <3000000>;
> > +	regulator-max-microvolt = <3000000>;
> > +	regulator-name = "avcc";
> > +};
> > +
> > +&reg_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?
> 
> > +};
> > +
> > +&reg_usb1_vbus {
> > +	status = "okay";
> > +};
> > +
> > +&reg_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 = <&reg_usb1_vbus>;
> > +	usb2_vbus-supply = <&reg_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 <sr at denx.de>
> >  S:	Maintained
> >  F:	configs/icnova-a20-swac_defconfig
> >  
> > +ICnova-A20-ADB4006 BOARD
> > +M:	Ludwig Kormann <ludwig.kormann at in-circuit.de>
> > +S:	Maintained
> > +F:	configs/icnova-a20-adb4006_defconfig
> > +
> >  ITEAD IBOX BOARD
> >  M:	Marcus Cooper <codekipper at 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.przywara@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
> 



More information about the U-Boot mailing list