[U-Boot] [PATCH v2] board/BuR/zynq/brsmarc2: initial commit
Hannes Schmelzer
hannes at schmelzer.or.at
Fri May 3 11:34:24 UTC 2019
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?
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.
> [snip]
>>>> + uart5: serial at 40200A00 {
>>>> + status = "disabled";
>>>> + compatible = "ns16550a", "bur,DdVxSf16x5xIO";
>>>> + bur,hwtree = "IF21";
>>>> + reg = <0x40200A00 0x40>;
>>>> + interrupt-parent = <&intc>;
>>>> + interrupts = <0 90 4>;
>>>> + };
>>> all these PL stuff can't go to mainline.
>> Please explain me the reason why this PL stuff cannot go mainline?
>> Maybe a solution cold be to drop those in the mainline dts and then
>> patch it again into it on the local branch. But that would be a way which
>> i don't prefer.
> So, now that we're clear this is for vxWorks, "PL stuff" ? The only
> thing I see here that's not spelled out (but based on cursory
> examination of the kernel, implied as normal) is interrupt-parent as a
> property.
Right. The spelling about interrupt is the same as the other
peripherals within the SoC (zynq-7000.dtsi).
>
>>>> +&gem0 {
>>>> + status = "okay";
>>>> + phy-mode = "rgmii-id";
>>>> + phy-handle = <ðernet_phy0>;
>>>> + mac-address = [ 00 00 00 00 00 00 ];
>>> 0 mac is wrong.
>> No, this zeros are placeholder.
>> The real MAC-Adresses are get into the dts during u-boot's fdt_fixup(...).
>> They come from environment, and th environment is setup from some
>> u-boot script.
> But we shouldn't need the node here to set the property is I think the
> point.
OK ... i will take here "real" or at least formal correct MAC-addresses
into for having a fallback if nothing is programmed at all and bring up
the interface initially.
> [snip]
>>>> +"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.
>
thanks for your time,
cheers,
Hannes
More information about the U-Boot
mailing list