[U-Boot] [PATCH v3] board/BuR/zynq/brsmarc2: initial commit

Michal Simek monstr at monstr.eu
Wed May 29 13:34:24 UTC 2019 

On 29. 05. 19 14:31, Hannes Schmelzer wrote:
> On 5/27/19 8:35 AM, Michal Simek wrote:
>> On 10. 05. 19 7:52, Hannes Schmelzer wrote:
>>> This commit adds the first of a few more Xilinx ZYNQ based SoM boards.
>>>
>>> The SoM is based on Xilinx Zynq 7000 SoC.
>>> Mainly vxWorks 6.9.4.x is running on the board,
>>> doing some PLC stuff on various carrier boards.
>>>
>>> Signed-off-by: Hannes Schmelzer <hannes.schmelzer at br-automation.com>
>>>
>>> ---
>>>
>>> Changes in v3:
>>> - drop silicon version 1/2 from ps7_init_gpl.c (they are obsolete)
>>> - board.c: drop obsolete board_eth_init(...)
>>> - board.c: strip down dram_init(..)
>>> - board.c: drop unused #include
>>> - board.c: cleanup comments
>>> - move CONFIG_ZYNQ_SERIAL from boardheader to Kconfig
>>> - move CONFIG_FPGA_ZYNQPL from boardheader to Kconfig
>>> - fix comments in boardheader
>>> - drop *_SYS_LDSCRIPT from boardheader (defined in Kconfig)
>>> - drop CONFIG_SYS_MALLOC_LEN (defined in Kconfig)
>>> - dop CONFIG_ENV_SPI_MAX_HZ from boardheader (defined in Kconfig)
>>> - dts: split vxWorks specific parts away from dts into separate file
>>> - dts: add compatible string to factory-settings node
>>> - dts: drop invalid (zero)= mac-addresses on gem0/gem1
>>> - dts: fixup style on ethernet_phy1 description
>>> - dts: add comments to phy's
>>> - drop helper-scripts for taking vivado handoff files into tree
>>>
>>> Changes in v2:
>>> - fix SDPX tag in Make-files/rules
>>>
>>>   arch/arm/dts/Makefile                       |   2 +
>>>   arch/arm/dts/zynq-brsmarc2-common.dtsi      | 168 +++++++++++++++++
>>>   arch/arm/dts/zynq-brsmarc2-vxworks.dtsi     | 123 +++++++++++++
>>>   arch/arm/dts/zynq-brsmarc2.dts              |  15 ++
>>>   arch/arm/dts/zynq-brsmarc2_r512.dts         |  15 ++
>>>   board/BuR/zynq/.gitignore                   |   1 +
>>>   board/BuR/zynq/MAINTAINERS                  |   6 +
>>>   board/BuR/zynq/Makefile                     |  16 ++
>>>   board/BuR/zynq/brsmarc2/board.c             |  63 +++++++
>>>   board/BuR/zynq/brsmarc2/ps7_init_gpl.c      | 276
>>> ++++++++++++++++++++++++++++
>>>   board/BuR/zynq/brsmarc2_r512/board.c        |   2 +
>>>   board/BuR/zynq/brsmarc2_r512/ps7_init_gpl.c | 276
>>> ++++++++++++++++++++++++++++
>> If these locations are there just because of that ps7_init_gpl wiring
>> because of DT dependency then you should look at XILINX_PS_INIT_FILE
>> option.
> Good point.
> I will have a look at this.
> But on first view into the Makefile i think it will not work.
> This option is catched within "board/xilinx/zynq/Makefile" but this
> Makefile doesn't run for me or my boards.

you should be able to include that Makefile in yours.

> 
>>
>>> +    };
>>> +
>>> +    memory {
>>> +        device_type = "memory";
>>> +        reg = <0x0 0x20000000>;
>>> +    };
>> nit: you have it in dts file that's why this is rewritten all the time.
> ok. will drop it from here.
>>
>>> +};
>>> +
>>> +&gem0 {
>>> +    status = "okay";
>>> +    phy-mode = "rgmii-id";
>>> +    phy-handle = <&ethernet_phy0>;
>>> +
>>> +    /* DP83822 phy */
>>> +    ethernet_phy0: ethernet-phy at 1 {
>>> +        ti,ledcr = <0x0480>;
>>> +        ti,rgmii-rxclk-shift;
>> Are these two your stuff? I can't see it in any binding doc.
> yes,  they are used in vxWorks phy driver.
> I will move them the zynq-brsmarc2-vxworks.dts
>>
>>> +        reg = <3>;
>>> +    };
>>> +};
>>> +
>>> +&i2c0 {
>>> +    u-boot,dm-pre-reloc;
>>> +    status = "okay";
>>> +    clock-frequency = <100000>;
>>> +
>>> +    boardtemp: pct2075_0 at 49 {
>> And based on dt spec you should use this based on names recommendation.
>>
>> boardtemp: temperature-sensor at 49 {
> okay, will change this.
>>
>>> +        #thermal-sensor-cells = <0>;
>>> +        compatible = "nxp,pct2075";
>>> +        reg = <0x49>;
>>> +    };
>>> +
>>> +    temp_core: tmp431_1 at 4C { /* temp. zynq die */
>> you use addresses above as lower case that's why 4c here.
>>
>> temp_core: temperature-sensor at 4c {
>>
>>> +        #thermal-sensor-cells = <1>;
>>> +        compatible = "ti,tmp431";
>>> +        reg = <0x4C>;
>> 4c here.
> okay, will change this.
>>
>>> +    };
>>> +
>>> +    extrtc: rx8571 at 51 {    /* realtime clock */
>> also
>>
>> extrtc: rtc at 51 {
> okay, will change this.
>>> +        compatible = "epson,rx8571";
>>> +        reg = <0x51>;
>>> +    };
>>> +
>>> +    resetc: rststm at 60 {    /* reset controller */
>> and
>>
>> resetc: reset-controller at 60 {
>>
>>> +        compatible = "bur,rststm";
>>> +        reg = <0x60>;
>>> +        hit-gpios = <&gpio0 84 GPIO_ACTIVE_HIGH>;
>>> +        cooling-min-state = <0>;
>>> +        cooling-max-state = <1>;    /* reset gets fired */
>>> +        #cooling-cells = <2>;        /* min followed by max */
>>> +    };
>> This is not approved binding in Linux.
> ok. i will change the name and move the node (and related stuff) to the
> zynq-brsmarc2-vxworks.dts
>>
>>> diff --git a/arch/arm/dts/zynq-brsmarc2-vxworks.dtsi
>>> b/arch/arm/dts/zynq-brsmarc2-vxworks.dtsi
>>> new file mode 100644
>>> index 0000000..a1251c3
>>> --- /dev/null
>>> +++ b/arch/arm/dts/zynq-brsmarc2-vxworks.dtsi
>>> @@ -0,0 +1,123 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * primary OS (vxWorks 6.9.4.x) specific parts for B&R BRSMARC2 boards
>>> + *
>>> + *  Copyright (C) 2019 B&R Industrial Automation GmbH
>>> + */
>>> +
>>> +#include "zynq-brsmarc2-common.dtsi"
>>> +
>>> +/ {
>>> +    fset: factory-settings {
>>> +        compatible = "bur,fsetv1";
>>> +        bl-version    = "                                ";
>>> +        order-no    = "                                ";
>>> +        cpu-order-no    = "                                ";
>>> +        hw-revision    = "                                ";
>>> +        serial-no    = <0>;
>>> +        device-id    = <0x0>;
>>> +        parent-id    = <0x0>;
>>> +        hw-variant    = <0x0>;
>>> +        hw-platform    = <0x0>;
>>> +        fram-offset    = <0x0>;
>>> +        fram-size    = <0x0>;
>>> +        cache-disable    = <0x0>;
>>> +        cpu-clock    = <0x0>;
>>> +    };
>>> +
>>> +    aliases {
>>> +        fset = &fset;
>>> +    };
>>> +
>>> +    board {
>>> +        status = "okay";
>>> +        compatible = "bur,brsmarc2-som";
>>> +        usb0mux-gpios = <&gpio0 68 GPIO_ACTIVE_HIGH>;
>>> +        usb1mux-gpios = <&gpio0 69 GPIO_ACTIVE_HIGH>;
>>> +        powerdown-gpios = <&gpio0 0 GPIO_ACTIVE_LOW>;
>>> +        reset-gpios = <&gpio0 9 GPIO_ACTIVE_LOW>;
>>> +    };
>>> +
>>> +    fpga: fpga at 40000000 {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <1>;
>>> +        status = "disabled";
>> I think that all these PL nodes should go away. We have been discussing
>> this long time ago that we won't push any PL description to mainline
>> because it can change.
>> I would recommend you to switch this to DT overlay which your boot
>> script will apply for you.
>>
>> Also there is no point to add IPs with status disabled to board file.
> the disable/okay can be changed by some factory-script (u-boot-script),
> which runs quite before booting the vxWorks kernel. Here we enable/
> disable features.
> 
> DT overlay is too late for me in the actual state of my product,
> i have running running systems here and will not do such fundamental
> change.
> 
> Can we make the establishment taking my -vxworks.dtsi away from review?

Adding things to mainline is out of any state of product. It shouldn't
be a problem to add stuff which are reviewed and not add things which
are problematic.
I have not a problem with your vxworks binding but I have issue with
describing something what shouldn't be here.


>>> diff --git a/arch/arm/dts/zynq-brsmarc2.dts
>>> b/arch/arm/dts/zynq-brsmarc2.dts
>>> new file mode 100644
>>> index 0000000..52fac02
>>> --- /dev/null
>>> +++ b/arch/arm/dts/zynq-brsmarc2.dts
>>> @@ -0,0 +1,15 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * B&R BRSMARC2 board DTS file (256 MiB variant)
>>> + *
>>> + *  Copyright (C) 2019 B&R Industrial Automation GmbH
>> nit - any particular reason to have two spaces in front of Copyright?
> no reason for that ;-) will change this
>> }
>> +
>> +int dram_init(void)
>> +{
>> +    if (fdtdec_setup_mem_size_base() != 0)
>> +        return -EINVAL;
>> +
>> +    /* simple test if ram_size determined from fdt is plausible */
>> +    gd->ram_size = get_ram_size((void *)gd->ram_base, gd->ram_size);
>> +
>> +    zynq_ddrc_init();
>> +
>> +    return 0;
>> +}
>> you have this function here but still 2 DTs. Is this code working or not?
>>
> 
>>> diff --git a/board/BuR/zynq/brsmarc2_r512/board.c
>>> b/board/BuR/zynq/brsmarc2_r512/board.c
>>> new file mode 100644
>>> index 0000000..a70c2aa
>>> --- /dev/null
>>> +++ b/board/BuR/zynq/brsmarc2_r512/board.c
>>> @@ -0,0 +1,2 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +#include "../brsmarc2/board.c"
>> Any reason for this? You can simply move it to zynq folder and have this
>> common for both boards variants.
> Yes, because i want to use standard build procedure while don't having
> duplicate code.

Just replace this
+obj-y += $(hw-platform-y)/board.o
by
+obj-y += board.o

and you are done.




>>> --- /dev/null
>>> +++ b/include/configs/brsmarc2.h
>>> @@ -0,0 +1,157 @@
>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>> +/*
>>> + * specific parts for B&R brsmarc2 SoM
>>> + *
>>> + * Copyright (C) 2019 Hannes Schmelzer <oe5hpm at oevsv.at> -
>>> + * B&R Industrial Automation GmbH - http://www.br-automation.com
>>> + *
>>> + */
>>> +
>>> +#ifndef __CONFIG_BRSMARC2_H__
>>> +#define __CONFIG_BRSMARC2_H__
>>> +
>>> +#include <configs/bur_cfg_common.h>
>> you should move NETCONSOLE, PREBOOT to Kconfig from this file. .
> okay, moving them to Kconfig.
> But i cannot do this with this run,
> i need here some extra patch because all my boards are affected from this
>>
>>> +
>>> +/* Cache options */
>>> +#define CONFIG_SYS_L2CACHE_OFF
>>> +#ifndef CONFIG_SYS_L2CACHE_OFF
>>> +# define CONFIG_SYS_L2_PL310
>>> +# define CONFIG_SYS_PL310_BASE        0xf8f02000
>>> +#endif
>>> +
>>> +#define ZYNQ_SCUTIMER_BASEADDR        0xF8F00600
>>> +#define CONFIG_SYS_TIMERBASE        ZYNQ_SCUTIMER_BASEADDR
>>> +#define CONFIG_SYS_TIMER_COUNTS_DOWN
>>> +#define CONFIG_SYS_TIMER_COUNTER    (CONFIG_SYS_TIMERBASE + 0x4)
>>> +
>>> +/* Serial drivers */
>>> +#define CONFIG_BAUDRATE        115200
>>> +/* The following table includes the supported baudrates */
>>> +#define CONFIG_SYS_BAUDRATE_TABLE  \
>>> +    {300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200,
>>> 230400}
>>> +
>>> +/* Ethernet driver */
>>> +#ifdef CONFIG_ZYNQ_GEM
>>> +# define CONFIG_MII
>> This is in Kconfig already.
>>
>>> +# define CONFIG_BOOTP_MAY_FAIL
>> you have this in your bur_cfg_common.h already
> so drop it here.
>>
>>> +#endif
>>> +
>>> +/* MMC */
>>> +#define CONFIG_ZYNQ_HISPD_BROKEN
>>> +
>>> +/* USB */
>>> +#define CONFIG_EHCI_IS_TDI
>>> +#define CONFIG_USB_MAX_CONTROLLER_COUNT    1
>>> +
>>> +/* Allow to overwrite serial and ethaddr */
>>> +#define CONFIG_ENV_OVERWRITE
>> you have this in your bur_cfg_common.h already
> drop it also.
>>
>>> +
>>> +/* Environment */
>>> +#define CONFIG_SYS_REDUNDAND_ENVIRONMENT
>>> +#define CONFIG_ENV_OFFSET_REDUND        (CONFIG_ENV_OFFSET + \
>>> +                         CONFIG_ENV_SECT_SIZE)
>>> +#define CONFIG_EXTRA_ENV_SETTINGS    \
>>> +BUR_COMMON_ENV \
>>> +"board_id=0xFF\0" \
>>> +"autoload=0\0" \
>> this is the part of default variables. Can't you just use it from there?
>> CONFIG_SYS_AUTOLOAD
>>
> thanks for that input, will change this.
> 
>>> +"scradr=" __stringify(CONFIG_SYS_LOAD_ADDR) "\0" \
>>> +"dtbaddr=0x4000000\0" \
>>> +"loadaddr=0x2000000\0" \
>>> +"fpgaaddr=fdt get value fpgabase /fpga bur,updaddr;" \
>>> +" fdt get value fpgasize /fpga bur,updsize\0" \
>>> +"fpga=setenv fpgastatus disabled; run fpgaaddr;" \
>>> +" sf read ${loadaddr} ${fpgabase} ${fpgasize} &&" \
>>> +" fpga loadb 0 ${loadaddr} ${fpgasize} && setenv fpgastatus okay\0" \
>>> +"fpgastatus=disabled\0" \
>>> +"fpgaupd=run fpgaaddr && tftp ${loadaddr} X20CP04xx.bit &&" \
>>> +" sf erase ${fpgabase} +${filesize} &&" \
>>> +" sf write ${loadaddr} ${fpgabase} ${filesize}\0" \
>>> +"netupd=tftp ${loadaddr} boot.bin && sf probe &&" \
>>> +" sf erase 0 +${filesize} && sf write ${loadaddr} 0 ${filesize} &&" \
>>> +" tftp ${loadaddr} u-boot-dtb.img &&" \
>>> +" sf erase 0x40000 +${filesize} && sf write ${loadaddr} 0x40000
>>> ${filesize}\0" \
>>> +"cfgscr=mw ${dtbaddr} 0;" \
>>> +" sf probe && sf read ${scradr} 0xC0000 0x10000 && source ${scradr};" \
>>> +" fdt addr ${dtbaddr} || cp ${fdtcontroladdr} ${dtbaddr} 4000\0" \
>>> +"vxargs=setenv bootargs gem(0,0)host:vxWorks h=${serverip}" \
>>> +" e=${ipaddr}:${netmask} g=${gatewayip} u=vxWorksFTP pw=vxWorks\0" \
>>> +"vxfdt=fdt addr ${dtbaddr}; fdt resize 0x10000;" \
>>> +" fdt set /fpga status ${fpgastatus};" \
>>> +" fdt boardsetup\0" \
>>> +"startvx=run vxargs && mw 0x1100 0 && run vxfdt &&" \
>>> +" bootm ${loadaddr} - ${dtbaddr}\0" \
>>> +"b_break=0\0" \
>>> +"b_tgts_std=mmc spi usb0 net0 net1\0" \
>>> +"b_tgts_rcy=spi usb0 net0 net1\0" \
>>> +"b_tgts_pme=usb0 net0 net1 mmc spi\0" \
>>> +"b_deftgts=if test ${b_mode} = 12; then setenv b_tgts ${b_tgts_pme};" \
>>> +" elif test ${b_mode} = 0; then setenv b_tgts ${b_tgts_rcy};" \
>>> +" else setenv b_tgts ${b_tgts_std}; fi\0" \
>>> +"b_mmc=run fpga; mmc dev 0; load mmc 0 ${loadaddr} arimg && run
>>> startvx\0" \
>>> +"b_spi=run fpga; sf read ${loadaddr} 900000 700000 && run startvx\0" \
>>> +"b_net0=tftp ${scradr} netscr-brsmarc2-${board_id}.img && source
>>> ${scradr}\0" \
>>> +"b_net1=tftp ${scradr} netscript.img && source ${scradr}\0" \
>>> +"b_usb0=usb start && load usb 0 ${scradr} bootscr.img && source
>>> ${scradr}\0" \
>>> +"b_default=run b_deftgts; for target in ${b_tgts};"\
>>> +" do run b_${target}; if test ${b_break} = 1; then; exit; fi; done\0"
>>> +
>>> +#define CONFIG_SYS_LOAD_ADDR        0xC0000    /*
>>> +                         * default load and execution
>>> +                         * address for loads / scripts
>>> +                         */
>>> +/* Support both device trees and ATAGs. */
>>> +#define CONFIG_CMDLINE_TAG
>>> +#define CONFIG_SETUP_MEMORY_TAGS
>>> +#define CONFIG_INITRD_TAG
>>> +#define CONFIG_MACH_TYPE        0xFFFFFFFF
>>> +
>>> +/* Miscellaneous configurable options */
>>> +#define CONFIG_CLOCKS
>>> +#define CONFIG_SYS_CONSOLE_INFO_QUIET
>>> +#define CONFIG_SYS_BOOTM_LEN        (32 * 1024 * 1024)
>>> +/* Physical Memory map */
>>> +#define CONFIG_SYS_TEXT_BASE        0x4000000
>>> +
>>> +#define CONFIG_SYS_SDRAM_BASE        0
>>> +
>>> +#define CONFIG_SYS_MEMTEST_START    CONFIG_SYS_SDRAM_BASE
>>> +#define CONFIG_SYS_MEMTEST_END        (CONFIG_SYS_SDRAM_BASE + 0x1000)
>>> +
>>> +#define CONFIG_SYS_INIT_RAM_ADDR    CONFIG_SYS_SDRAM_BASE
>>> +#define CONFIG_SYS_INIT_RAM_SIZE    CONFIG_SYS_MALLOC_LEN
>>> +#define CONFIG_SYS_INIT_SP_ADDR        (CONFIG_SYS_INIT_RAM_ADDR + \
>>> +                    CONFIG_SYS_INIT_RAM_SIZE - \
>>> +                    GENERATED_GBL_DATA_SIZE)
>>> +/* SPL part */
>>> +#ifdef CONFIG_SPL_BUILD
>>> +#define CONFIG_SPL_I2C_SUPPORT
>>> +
>>> +/* Disable dcache for SPL just for sure */
>>> +#define CONFIG_SYS_DCACHE_OFF
>>> +
>>> +#ifdef CONFIG_ZYNQ_QSPI
>>> +# define CONFIG_SPL_SPI_LOAD
>>> +# define CONFIG_SYS_SPI_U_BOOT_OFFS    0x40000
>>> +#endif /* CONFIG_ZYNQ_QSPI */
>>> +
>>> +/* SP location before relocation, must use scratch RAM */
>>> +#define CONFIG_SPL_TEXT_BASE        0x0
>>> +/* 3 * 64kB blocks of OCM - one is on the top because of bootrom */
>>> +#define CONFIG_SPL_MAX_SIZE        0x30000
>>> +/* The highest 64k OCM address */
>>> +#define OCM_HIGH_ADDR            0xffff0000
>>> +/* Just define any reasonable size */
>>> +#define CONFIG_SPL_STACK_SIZE        0x2000
>>> +/* SPL stack position - and stack goes down */
>>> +#define CONFIG_SPL_STACK        (OCM_HIGH_ADDR + CONFIG_SPL_STACK_SIZE)
>>> +/* On the top of OCM space */
>>> +#define CONFIG_SYS_SPL_MALLOC_START    (CONFIG_SPL_STACK + \
>>> +                     GENERATED_GBL_DATA_SIZE)
>>> +
>>> +#define CONFIG_SYS_SPL_MALLOC_SIZE    0x8000
>>> +/* BSS setup */
>>> +#define CONFIG_SPL_BSS_START_ADDR    0x100000
>>> +#define CONFIG_SPL_BSS_MAX_SIZE        0x100000
>>> +#define CONFIG_SYS_UBOOT_START        CONFIG_SYS_TEXT_BASE
>>> +#endif /* CONFIG_SPL_BUILD */
>>> +#endif /* __CONFIG_BRSMARC2_H__ */
>>>
>> I think that this file should be designed differently.
>> You should  #include <configs/zynq-common.h> first and then change
>> whatever you don't like.
>> There are a lot of things which are just c&p from this file and it will
>> be hard to fix all zynq boards by changing one file.
> Yes there are much thing c/p from a verly early stage when zynq was
> introduced into u-boot. I still think that my way fits best to my needs,
> zynq-common as very much stuff which i don't need at all. It will make
> readability of my board worse.
> Of course general fixes will not be applied to my boards, this maybe good
> or not. Often things are changed by somebody, but anybody can have
> all corner cases within her/him viewing angle. With this way i think i have
> most stable conditions for my boards and can take thinks from common
> if think they are useful or maybe taking them not because i have trouble
> with it.

I understand your opinion that it is the most easier way how to maintain
it. On the other hand we need to have setup where with minimal effort we
can update all zynq boards.
Adding board to mainline is also bringing responsibility to test that
boards and keep them alive. There should be some changes in connection
to SPL, variable setup to move to more zynq generic board which is
something I plan to do. Xilinx Versal has done this step, ZynqMP is on
the way and then Zynq should be converted too.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190529/40913096/attachment.sig>

More information about the U-Boot mailing list