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

Andre Przywara andre.przywara at arm.com
Thu Apr 6 15:32:18 CEST 2023


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? 

> +	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/

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