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).
[snip]
> >>+ uart5: serial@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.
> >>+&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.
[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!
--
Tom
signature.asc
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
