[U-Boot] [PATCH v2] board/BuR/zynq/brsmarc2: initial commit
Hannes Schmelzer
hannes at schmelzer.or.at
Fri May 10 05:49:33 UTC 2019
On 5/9/19 11:49 PM, Michal Simek wrote:
> On 08. 05. 19 5:37, Hannes Schmelzer wrote:
>>
>> On 5/3/19 6:29 PM, Michal Simek wrote:
>>> On 03. 05. 19 6:18, Tom Rini wrote:
>>>> On Fri, May 03, 2019 at 01:34:24PM +0200, Hannes Schmelzer wrote:
>>>>> On 5/2/19 9:03 PM, Tom Rini wrote:
>>>>>> On Thu, May 02, 2019 at 08:34:32PM +0200, Hannes Schmelzer wrote:
>>>>>>> On 5/2/19 6:06 PM, Michal Simek wrote:
>>>>>>>> Hi,
>>>>>>> Hi Michal,
>>>>>>>> On 02. 05. 19 5:14, Hannes Schmelzer wrote:
>>>>>>>>> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
>>>>>>>>> index dfa5b02..2b00129 100644
>>>>>>>>> --- a/arch/arm/dts/Makefile
>>>>>>>>> +++ b/arch/arm/dts/Makefile
>>>>>>>>> @@ -208,6 +208,8 @@ dtb-$(CONFIG_ARCH_ZYNQ) += \
>>>>>>>>> zynq-zc770-xm011-x16.dtb \
>>>>>>>>> zynq-zc770-xm012.dtb \
>>>>>>>>> zynq-zc770-xm013.dtb \
>>>>>>>>> + zynq-brsmarc2.dtb \
>>>>>>>>> + zynq-brsmarc2_r512.dtb \
>>>>>>>> Can't you detect it if you have 512M version?
>>>>>>>> u-boot itself has code for these kind of detection.
>>>>>>>>
>>>>>>>> long get_ram_size(long *base, long maxsize)
>>>>>>> I actually think not,
>>>>>>> because i need different ps7_init stuff for the two different RAM
>>>>>>> chips.
>>>>>>> (timing, adress lines, ...) But i will check if i even can drop
>>>>>>> the two
>>>>>>> different dts files.
>>>>>>>>> +/ {
>>>>>>>>> + model = "BRSMARC2 Zynq SoM";
>>>>>>>>> + compatible = "xlnx,zynq-7000";
>>>>>>>>> +
>>>>>>>>> + fset: factory-settings {
>>>>>>>>> + 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>;
>>>>>>>>> + };
>>>>>>>> What's this? No compatible string. This looks quite hacky.
>>>>>>> This are factory settings, used by the OS (in this case vxWorks),
>>>>>>> to identify on which hardware it runs, and have per device unique
>>>>>>> stuff
>>>>>>> (serial number).
>>>>>>> But you're right, it would be nice to have here some compatible
>>>>>>> string,
>>>>>>> i will change this. Today we just search for the node
>>>>>>> "factory-setting".
>>>>>>> A more comfortable ways would be vxFdtNodeOffsetByCompatible(....)
>>>>>>>>> +
>>>>>>>>> + aliases {
>>>>>>>>> + ethernet0 = &gem0;
>>>>>>>>> + ethernet1 = &gem1;
>>>>>>>>> + i2c0 = &i2c0;
>>>>>>>>> + serial0 = &uart0;
>>>>>>>>> + spi0 = &qspi;
>>>>>>>>> + mmc0 = &sdhci0;
>>>>>>>>> + fset = &fset;
>>>>>>>>> + can0 = &can0;
>>>>>>>>> + can1 = &can1;
>>>>>>>>> + };
>>>>>>>>> +
>>>>>>>>> + memory {
>>>>>>>>> + device_type = "memory";
>>>>>>>>> + reg = <0x0 0x10000000>;
>>>>>>>>> + };
>>>>>>>>> +
>>>>>>>>> + 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>;
>>>>>>>>> + };
>>>>>>>> Where is mainline dt binding for this?
>>>>>>> Nowhere, because u-boot nor linux does use this,
>>>>>>> this is only for the vxWorks OS.
>>>>>> This is what I kinda figured was the case. We now have some
>>>>>> interesting
>>>>>> times ahead of us as yes, we normally think about DTS reviews in terms
>>>>>> of Linux. But this is all for a board that uses vxWorks. Perhaps the
>>>>>> best thing to do here is note (and for all of the other boards too,
>>>>>> but
>>>>>> you can wait for general feedback before v3'ing them all) in the dts
>>>>>> comment that it's for vxWorks as people tend to assume a DTS file
>>>>>> is for
>>>>>> Linux (even with many counter examples).
>>>>> OK. Thanks.
>>>>>
>>>>> would it be fine having it like this ?
>>>>>
>>>>> /* factory-settings for the vxWorks target */
>>>>> fset: factory-settings {
>>>>> compatible = "bur,fsetv1";
>>>>> bl-version = " ";
>>>>> .....
>>>>> };
>>>>>
>>>>> /* misc. peripheral, used in vxWorks */
>>>>> board {
>>>>> status = "okay";
>>>>> ...
>>>>> };
>>>>>
>>>>> Or might be a general description on top would be more helpful?
>>>> I think a general comment at the top saying that this tree is only valid
>>>> for vxWorks and U-Boot is enough detail.
>> Okay, i will do so like this:
>> /*
>> * This devicetree is only valid for u-boot and the primary OS (vxWorks
>> 6.9.4.x)
>> * of this board.
>> */
>>> It would be much better to simply put vxworks stuff to one dtsi file and
>>> common stuff to another. That it will be clear what it is vxworks part
>>> and what it is common.
>>>
>>>
>>>>> In general i pay attention to describe devices as generic as possible,
>>>>> meaning that a) syntax is correct, b) some linux or even u-boot isn't
>>>>> disturbed by this descriptions.
>>>>>
>>>>> With a look to the paragraph below, the implemented UART in FPGA,
>>>>> is 16550 compatible, so there might be a chance that it would work
>>>>> with Linux as well.
>>>> wrt the UART, are there specified bindings in vxWorks? That seems like
>>>> a case where the DTS needs to be valid for what U-Boot says the binding
>>>> is (and in turn, what Linux does). And perhaps there's room for
>>>> clarifications to those bindings.
>> actually i think the UART is described valid in the fdt and can be used
>> by u-boot or linux as well even i've not tested it yet.
>>
>>> I think we need to get more clarity what exactly vxworks expects and
>>> what are just your "hacks" to get it work.
>>> If vxworks deviates existing dt binding, or create completely new one.
>> One thing to say is, that vxWorks 6.9.4.x doesn't support fdt
>> out of the box. All the support and usage of fdt within 6.9.4.x is
>> introduced or backported from vxWorks 7 (which is based on fdt,
>> even with their own and mostly different bindings) by myself -
>> so called my "hacks" - because i don't want have thousand of different
>> vxWorks images for one board in various only very little different
>> hw-variants.
>> Excactly for this purpose is the fdt.
>>
>> My thinking of the dts file for a board is that it should be good
>> readable and
>> easy maintainable by the board maintainer - in this case it's me. So
>> splitting
>> it up into one more file does only degrade readability.
> I don't agree with this. It is not only for you. We are doing all
> reviews to have codebase nice and clean. And adding hacky code(C/asm/dt)
> to u-boot because it is used only one person is simply bad thing to do.
OK. i will split them up. have a look to V3.
>
>>>>>>>>> +"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"
>>>>>>>> we are trying to get rid of these variables. Please enable distro
>>>>>>>> boot
>>>>>>>> and put all of these to scripts and run them.
>>>>>>> No, i don't want some distro defaults ... because we've our own boot
>>>>>>> strategy.
>>>>>> How much of this strategy is common to all the BuR platforms? Perhaps
>>>>>> the answer is that you should make include/environment/bur/ and put
>>>>>> the
>>>>>> common script in there. Thanks!
>>>>> The strategy itself (having b_mode and then try several targets) is
>>>>> always
>>>>> the
>>>>> same but the boot-targets itself are individual. I will start a
>>>>> project for
>>>>> catching the "core" into 'include/environment/bur/' over all BuR
>>>>> boards.
>>>>> But this will take more time than a simple rework of this patch,
>>>>> meaning that this will follow later on.
>>>> I'm OK with that, thanks!
>>>>
>>> Zynq/ZynqMP code prioritize bootmode with distro boot.
>>> I personally want to have less include/configs/* files for the same type
>>> of boards. It means do whatever you want but wire that via scripts.
>>> Let distro boot to load your scripts and do that stuff there.
>>> And I also have no problem to have these scripts in u-boot to show
>>> others what you do.
>> I think that the existing code doesn't fit to my needs.
> Then fix it.
No. The existing code will not be modified to fit for my specific hardware.
It doesn't make sense at all and i have absolutely no chance to test boards
which may be affected from my changes.
>
>> I have here some resetcontroller and/or gpios below which tell me how to
>> boot.
>> This is really specific to b&r boards. So i will walk the above.
> Then do it properly and it can be useful for others too
It is done well for my point of view!
I have here some specific hardware to support.
Nobody else, except those who have this B&R specific hardware, will find
this useful.
Maybe somebody is looking at it and can fork an idea from the b&r stuff,
that would be ok.
>> But i will take up the idea for having different scripts which
>> run on different boot targets. As told already i have too look
>> here over all "my" b&r boards and not only the zynq board
>> for having a standard how b&r boards are working.
> Ok. Looking forward for v3.
>
> Thanks,
> Michal
cheers,
Hannes
More information about the U-Boot
mailing list