Hi, +Matteo Am 30.06.2014 07:15, schrieb Olof Johansson: > On Sun, Jun 29, 2014 at 1:50 PM, Andreas Färber <afaerber@xxxxxxx> wrote: >> This allows to boot the Adapteva Parallella board to serial console. >> >> Cc: Andreas Olofsson <andreas@xxxxxxxxxxxx> >> Signed-off-by: Andreas Färber <afaerber@xxxxxxx> > > Nice and clean DTS, just a couple of comments below. > >> diff --git a/arch/arm/boot/dts/zynq-parallella.dts b/arch/arm/boot/dts/zynq-parallella.dts >> new file mode 100644 >> index 0000000..98df66c >> --- /dev/null >> +++ b/arch/arm/boot/dts/zynq-parallella.dts >> @@ -0,0 +1,63 @@ >> +/* >> + * Copyright (c) 2014 SUSE LINUX Products GmbH >> + * >> + * Derived from zynq-zed.dts: >> + * >> + * Copyright (C) 2011 Xilinx >> + * Copyright (C) 2012 National Instruments Corp. >> + * Copyright (C) 2013 Xilinx >> + * >> + * This software is licensed under the terms of the GNU General Public >> + * License version 2, as published by the Free Software Foundation, and >> + * may be copied, distributed, and modified under those terms. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> +/dts-v1/; >> +/include/ "zynq-7000.dtsi" >> + >> +/ { >> + model = "Parallella Board"; >> + compatible = "xlnx,zynq-7000"; > > This should have a more specific compatible as the first one. Probably > something like: > compatible = "adapteva,parallella", "xlnx,zynq-7000"; Sure, I can add one if desired. As indicated in the file header, I used zynq-zed.dts as template, which doesn't have its own either: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/zynq-zed.dts Andreas, can I assume you would be okay with me assigning Adapteva, Inc. the suggested "adapteva" prefix in the below list? http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/vendor-prefixes.txt I guess it'll come in handy if we ever document the Epiphany coprocessor's registers. The physical chip on the board is connected via some FPGA glue - would that be placed like this? / { amba { epiphany: epiphany@80000000 { compatible = "adapteva,epiphany-iii"; reg = <0x80000000 0x40000000>; }; }; }; It's accessed through /dev/epiphany in a userspace library below, https://github.com/adapteva/epiphany-libs/blob/master/src/e-hal/src/epiphany-hal.c#L215 https://github.com/adapteva/epiphany-libs/commit/80364e8ee4e99e450a632885a62e7a501398c200 but it's not clear to me who is supposed to create that device. No downstream kernel driver nor corresponding node in the device tree: https://github.com/parallella/parallella-linux-adi/blob/d04c39251dc3153b60555642b502dde15f7156a6/arch/arm/boot/dts/zynq-parallella.dts Ben? Andreas? Anyway, given that vf610.dtsi only documents the Cortex-A5 core but not its Cortex-M4 companion core, I'm assuming we should not add the 16 (or 64) Epiphany cores to /cpus node (which would then require to split this file into zynq-parallella.dtsi and zynq-parallella-{e16,e64}.dts). >> + >> + memory { >> + device_type = "memory"; >> + reg = <0 0x20000000>; Err, this should be <0 0x40000000> (1 GiB). Copied from zynq-zed.dts, and downstream has the wrong value, too. > Does the bootloader update this entry, or is it truly static? If it's > updated then it's become recent habit to leave the memory size empty > in the static file. The board uses a downstream U-Boot, displaying 992 MiB on serial. andreas@parallella:/proc/device-tree/memory> hexdump -C reg 00000000 00 00 00 00 3e 00 00 00 |....>...| 00000008 That matches 992 * 1024 * 1024. Shall I change to <0 0> then? Confusing though is that the Epiphany manual says memory at <0x1e000000 0x02000000> is reserved for use by the Epiphany: http://adapteva.com/docs/epiphany_sdk_ref.pdf section 14.1 page 118 I believe that is rather 0x3e000000, which U-Boot seems to confirm: https://github.com/parallella/parallella-uboot/blob/3f4794e9ac524be6a373e6ff39ee9ae4529e0f14/include/configs/parallella.h#L25 >> + }; >> + >> + chosen { >> + bootargs = "console=ttyPS0,115200 earlyprintk root=/dev/mmcblk0p2 rootfstype=ext4 rw rootwait"; >> + }; >> +}; > > A bit more torn on this one, I'm OK with it staying in even if > firmware overrides for all practical purposes since it's good to keep > around for reference w.r.t. console. Note that we're starting to move > over to using /chosen/stdout-path, so you might want to add that now > instead of later. The board boots U-Boot from flash; U-Boot is configured with no bootdelay, and users are instructed to deploy one of two provided devicetree.dtb files, depending on whether they want to run headless (console=ttyPS0,115200) or with HDMI-enabling FPGA bitstream (no console=). No bootargs variable is set in U-Boot environment. U-Boot loads uImage, device tree and bitstream from FAT p1, and device trees currently hardcode an ext4 p2, so I intentionally left that in here, for unmodified .dtb deployment for shipping bootloader. I'll look into /chosen/stdout-path, thanks for the pointer. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html