Re: [U-Boot] [PATCH v2 3/3] arm64: zynqmp: add support for Avnet UltraZed-EV Starter Kit

2019-06-11 Thread Luca Ceresoli
Hi Michal,

On 28/05/19 09:45, Michal Simek wrote:
[...]
>> +static unsigned long psu_ddr_phybringup_data(void)
>> +{
>> +unsigned int regval = 0;
>> +unsigned int pll_retry = 10;
>> +unsigned int pll_locked = 0;
>> +
>> +while ((pll_retry > 0) && (!pll_locked)) {
>> +Xil_Out32(0xFD080004, 0x00040010);
>> +Xil_Out32(0xFD080004, 0x00040011);
>> +
>> +while ((Xil_In32(0xFD080030) & 0x1) != 1)
>> +;
>> +pll_locked = (Xil_In32(0xFD080030) & 0x8000)
>> +>> 31;
>> +pll_locked &= (Xil_In32(0xFD0807E0) & 0x1)
>> +>> 16;
>> +pll_locked &= (Xil_In32(0xFD0809E0) & 0x1)
>> +>> 16;
>> +pll_locked &= (Xil_In32(0xFD080BE0) & 0x1)
>> +>> 16;
>> +pll_locked &= (Xil_In32(0xFD080DE0) & 0x1)
>> +>> 16;
>> +pll_retry--;
>> +}
>> +Xil_Out32(0xFD0800C4, Xil_In32(0xFD0800C4) | (pll_retry << 16));
>> +if (!pll_locked)
>> +return (0);
> 
> nit: return 0;

In v3 I'm sending a new patch to let tools/zynqmp_psu_init_minimize.sh
do it automatically.

Ok for the other change requests too, will be in v3.

-- 
Luca
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v2 3/3] arm64: zynqmp: add support for Avnet UltraZed-EV Starter Kit

2019-05-28 Thread Michal Simek
On 24. 05. 19 15:40, Luca Ceresoli wrote:
> Avnet UltraZed-EV Starter Kit is composed by the UltraZed-EV SoM and the
> only publicly-available compatible carrier card. The SoM is based on the EV
> version of the Xilinx ZynqMP SoC+FPGA.
> 
> The psu_init_gpl.c file has been generated from the board definition files
> at [0] using Vivado 2018.3 and then minimized by
> tools/zynqmp_psu_init_minimize.sh. Manually removed serdes init code since
> it is not mentioned in device tree and fixed a checkpatch error.
> 
> [0] 
> https://github.com/Avnet/bdf/tree/3686c9ff7d2f0467fb4fcf39f861b8d6ff183b12/ultrazed_7ev_cc/1.1
> 
> Signed-off-by: Luca Ceresoli 
> 
> ---
> 
> Changes v1 -> v2:
>  - remove serdes code from psu_init_gpl.c since no serdes is enabled in DT
>(Michal)
>  - split DT in two files: SOM and carrier card (Michal)
>  - improved DT comments, added product URLs
>  - DT: add missing phy-handle under ethernet node (Michal)
> 
> Whole patchset tested on:
>  - current u-boot/master, as is
>  - current u-boot-microblaze/master
>  - current u-boot-microblaze/master with the addtion of
>commit f89d6133eef2 ("configs: move CONFIG_SPL_TEXT_BASE to Kconfig")
> ---
>  arch/arm/dts/Makefile |   1 +
>  arch/arm/dts/avnet-ultrazedev-cc.dts  |  58 ++
>  arch/arm/dts/avnet-ultrazedev-som.dtsi|  54 ++
>  .../zynqmp/avnet-ultrazedev-cc/psu_init_gpl.c | 663 ++
>  configs/avnet_ultrazedev_defconfig|  64 ++
>  5 files changed, 840 insertions(+)
>  create mode 100644 arch/arm/dts/avnet-ultrazedev-cc.dts
>  create mode 100644 arch/arm/dts/avnet-ultrazedev-som.dtsi
>  create mode 100644 board/xilinx/zynqmp/avnet-ultrazedev-cc/psu_init_gpl.c
>  create mode 100644 configs/avnet_ultrazedev_defconfig

I am still not happy with this one. Boards have certain revisions which
is not reflected anywhere. Please add it model and also to file name.
I am missing model property - there should be one in SOM. And another
one for carrier card.

I had similar thing recently about how to name board with specific FMC
card.
Name is composed like this
---.dts

And the same style should be used here.
avnet-ultrazedev-cc-revC-ultrazedev-som-revA.dts

It is longer but it describe that configuration fully.
When new som is added to the same base you can put that stuff together.


> 
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 8167cdb4e856..d82dc6529749 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -205,6 +205,7 @@ dtb-$(CONFIG_ARCH_ZYNQ) += \
>   zynq-zybo-z7.dtb
>  dtb-$(CONFIG_ARCH_ZYNQMP) += \
>   avnet-ultra96-rev1.dtb  \
> + avnet-ultrazedev-cc.dtb \
>   zynqmp-mini.dtb \
>   zynqmp-mini-emmc0.dtb   \
>   zynqmp-mini-emmc1.dtb   \
> diff --git a/arch/arm/dts/avnet-ultrazedev-cc.dts 
> b/arch/arm/dts/avnet-ultrazedev-cc.dts
> new file mode 100644
> index ..d94dd4246f7b
> --- /dev/null
> +++ b/arch/arm/dts/avnet-ultrazedev-cc.dts
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0+ OR X11
> +
> +/*
> + * UltraZed-EV Carrier Card (based on the UltraZed-EV SoM)
> + * http://ultrazed.org/product/ultrazed-ev-carrier-card
> + */
> +
> +/dts-v1/;
> +
> +#include "avnet-ultrazedev-som.dtsi"
> +
> +/ {

please add model and compatibility here.

> + chosen {
> + stdout-path = "serial0:115200n8";
> + xlnx,eeprom = 
> + };
> + aliases {
> + ethernet0 = 
> + serial0 = 
> + };
> +};
> +
> + {
> + device_type = "serial";
> + port-number = <0>;

This property shouldn't be needed.

> + status = "okay";
> + u-boot,dm-pre-reloc;
> +};
> +
> +_cc {
> + /* Microchip 24AA025E48T-I/OT: 2K I2C Serial EEPROM with EUI-48 */
> + eeprom: eeprom@51 {
> + compatible = "at,24c02", "i2c-eeprom";

please remove i2c-eeprom - this is just u-boot thing and

at,24c02 should replaced by atmel,24c02. Done in kernel some time ago.


> + reg = <0x51>;
> + };
> +
> + /* IDT Versa Clock 5P49V5935B */
> + vc5: clock-generator@6a {
> + compatible = "idt,5p49v5935";
> + reg = <0x6a>;
> + #clock-cells = <1>;
> + };
> +};
> +
> +/* Ethernet RJ-45 */
> + {
> + status = "okay";
> +};
> +
> +/* microSD card slot */
> + {
> + status = "okay";
> + xlnx,mio_bank = <1>;
> + clock-frequency = <18000>;
> + max-frequency = <5000>;
> + no-1-8-v;
> + disable-wp;
> +};
> diff --git a/arch/arm/dts/avnet-ultrazedev-som.dtsi 
> b/arch/arm/dts/avnet-ultrazedev-som.dtsi
> new file mode 100644
> index ..4ce0a2d31786
> --- /dev/null
> +++ b/arch/arm/dts/avnet-ultrazedev-som.dtsi
> @@ -0,0 +1,54 @@
> +// SPDX-License-Identifier: GPL-2.0+ OR X11
> +
> +/*
> + * UltraZed-EV SoM
> + * http://ultrazed.org/product/ultrazed-ev
> + */
> +
> +/dts-v1/;
> +
>