From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vladimir Svoboda Date: Wed, 2 May 2018 11:06:38 +0200 Subject: [U-Boot] [PATCH] arm: Add support for Trenz TE0820 (zynqmp) In-Reply-To: <528e701f-d3de-828d-47e7-421a19bf302d@xilinx.com> References: <20180430141003.21416-1-ze.vlad@gmail.com> <528e701f-d3de-828d-47e7-421a19bf302d@xilinx.com> Message-ID: <1820d397-6637-b45e-b74d-a2de625691c4@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Thanks for the review, On 02/05/18 10:00, Michal Simek wrote: > Hi, + Trenz support. > > On 30.4.2018 16:10, Vladimir Svoboda wrote: >> Add support for Trenz TE0820 revision 2 MPSoC module. >> >> Signed-off-by: Vladimir Svoboda > I can't see your name in git log that's why I expect this is maybe your > the first submission. How can I trust that you will test this board > regularly that make sense to have it in the tree? > > I don't have this board that's why it can be broken quickly. This is indeed my first patch submission to U-boot. I work at HIPPEROS S.A. and we develop a RTOS. We are part of a european-funded project, called Tulipp, where partners use that board. That's the guarantee I can give you. > >> --- >> >> arch/arm/dts/Makefile | 1 + >> arch/arm/dts/zynqmp-te0820-rev2.dts | 669 ++++++++++++++++++ >> .../zynqmp/zynqmp-te0820-rev2/psu_init_gpl.c | 624 ++++++++++++++++ > There are a lot of versions with different chips on it. It will be good > to exactly write what version this is going to support. > Also there is different amount of flash on 1EL version based on > https://shop.trenz-electronic.de/en/Products/Trenz-Electronic/TE08XX-Zynq-UltraScale/TE0820-Zynq-UltraScale/ > > Also this is SOM and it requires Carrier board to know wiring. What > carrier board is this configuration for? Indeed, there are several versions. I can only test with the module TE0820-02-03EG-1EA as this is the only one we have. The carrier board we use is the Sundance EMC2, they will sell it with the TE0820 module under the name EMC2-ZU3EG. > >> configs/xilinx_zynqmp_te0820_rev2_defconfig | 105 +++ >> include/configs/xilinx_zynqmp_te0820.h | 49 ++ >> 5 files changed, 1448 insertions(+) >> create mode 100644 arch/arm/dts/zynqmp-te0820-rev2.dts >> create mode 100644 board/xilinx/zynqmp/zynqmp-te0820-rev2/psu_init_gpl.c >> create mode 100644 configs/xilinx_zynqmp_te0820_rev2_defconfig >> create mode 100644 include/configs/xilinx_zynqmp_te0820.h >> >> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile >> index ac7667b1e8..ec4e7cc206 100644 >> --- a/arch/arm/dts/Makefile >> +++ b/arch/arm/dts/Makefile >> @@ -148,6 +148,7 @@ dtb-$(CONFIG_ARCH_ZYNQ) += \ >> dtb-$(CONFIG_ARCH_ZYNQMP) += \ >> zynqmp-mini-emmc.dtb \ >> zynqmp-mini-nand.dtb \ >> + zynqmp-te0820-rev2.dtb \ > This is Trenz that's why it you should use trenz instead of zynqmp. Just for my information, what does zynqmp refers to then? I thought it was the technical name for UltraScale+ MPSoC. > >> zynqmp-zcu100-revC.dtb \ >> zynqmp-zcu102-revA.dtb \ >> zynqmp-zcu102-revB.dtb \ >> diff --git a/arch/arm/dts/zynqmp-te0820-rev2.dts b/arch/arm/dts/zynqmp-te0820-rev2.dts >> new file mode 100644 >> index 0000000000..db23dfe45c >> --- /dev/null >> +++ b/arch/arm/dts/zynqmp-te0820-rev2.dts >> @@ -0,0 +1,669 @@ >> +/* >> + * dts file for Xilinx ZynqMP TE0820 Rev2 > Trenz again. So you expect "Xilinx Trenz TE0820 Rev2" or "Trenz TE0820 Rev2"? > >> + * >> + * (C) Copyright 2018, HIPPEROS. >> + * >> + * Vladimir Svoboda >> + * >> + * SPDX-License-Identifier: GPL-2.0+ > Please use kernel format where this line comes first. > >> + */ >> + >> +/dts-v1/; >> + >> +#include "zynqmp.dtsi" >> +#include "zynqmp-clk-ccf.dtsi" >> +#include >> +#include >> +#include >> + >> +/ { >> + model = "ZynqMP TE0820 Rev2"; > Trenz, > >> + compatible = "xlnx,zynqmp-te0820-rev2", "xlnx,zynqmp-te0820", "xlnx,zynqmp"; > Add trenz to vendor-prefixes.txt in the Linux kernel and use it here. > >> + >> + aliases { >> + ethernet0 = &gem3; >> + gpio0 = &gpio; > Based on Rob Herring this shouldn't be here. > > >> + i2c0 = &i2c0; >> + i2c1 = &i2c1; >> + mmc0 = &sdhci1; >> + rtc0 = &rtc; >> + serial0 = &uart0; >> + serial1 = &uart1; >> + serial2 = &dcc; >> + spi0 = &qspi; >> + usb0 = &usb0; > The same for gpio. > >> + }; >> + >> + chosen { >> + bootargs = "earlycon"; >> + stdout-path = "serial0:115200n8"; >> + }; >> + >> + memory at 0 { >> + device_type = "memory"; >> + reg = <0x0 0x0 0x0 0x40000000>; >> + }; >> + >> + gpio-keys { >> + compatible = "gpio-keys"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + autorepeat; >> + sw19 { >> + label = "sw19"; >> + gpios = <&gpio 22 GPIO_ACTIVE_HIGH>; >> + linux,code = ; >> + gpio-key,wakeup; >> + autorepeat; >> + }; >> + }; >> + >> + leds { >> + compatible = "gpio-leds"; >> + heartbeat_led { >> + label = "heartbeat"; >> + gpios = <&gpio 23 GPIO_ACTIVE_HIGH>; >> + linux,default-trigger = "heartbeat"; >> + }; >> + }; > This and stuff below looks like c&p from zcu102 version which is incorrect. This is indeed a c&p from the ZCU 102 where I modified a couple of things (memory size, SDIO...). I did not check the validity of all the devices information and there are probably errors regarding the routing of GPIO. > > I am going to skip the rest of this patch because questions above needs > to be answered first. I hope that this answer will help to continue the review. Vladimir > > Thanks, > Michal