[U-Boot] [PATCH] arm: Add support for Trenz TE0820 (zynqmp)

[Vladimir Svoboda]

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 <ze.vlad at gmail.com>
> 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 <vladimir.svoboda at hipperos.com>
>> + *
>> + * 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 <dt-bindings/input/input.h>
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/phy/phy.h>
>> +
>> +/ {
>> +	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 = <KEY_DOWN>;
>> +			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