[U-Boot] [PATCH v3 0/6] Run-time configuration of U-Boot via a flat device tree (fdt)

[U-Boot] [PATCH v3 0/6] Run-time configuration of U-Boot via a flat device tree (fdt)

[Simon Glass]

At present in U-Boot configuration is mostly done using CONFIG options in the
board file. This patch set makes it possible for a single U-Boot binary to
support multiple boards, with the exact configuration of each board
controlled by a flat device tree (fdt). This is the approach recently taken
by the ARM Linux kernel and has been used by PowerPC for some time.

In other words, manufacturers can potentially use a single U-Boot binary
across an entire product line, with one fdt per model.

The fdt is a convenient vehicle for implementing run-time configuration for
three reasons. Firstly it is easy to use, being a simple text file. It is
extensible since it consists of nodes and properties in a nice hierarchical
format.

Finally, there is already excellent infrastructure for the fdt: a compiler
checks the text file and converts it to a compact binary format, and a library
is already available in U-Boot (libfdt) for handling this format.

To read about fdts, take a look at the specification here:

https://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf

You also might find this section of the Linux kernel documentation useful:
(access this in the Linux kernel source code)

Documentation/devicetree/booting-without-of.txt

To use this patch set you will need to get the device tree compiler here:

git://jdl.com/software/dtc.git

and add some defines to your board (only ARM is currently supported):

 #define CONFIG_OF_CONTROL       (to enable run-time config control via fdt)
 #define CONFIG_OF_EMBED or CONFIG_OF_SEPARATE
 (either build the fdt blob into U-Boot, or create a separate u-boot.dtb
  and u-boot-dtb.bin)
 #define CONFIG_DEFAULT_DEVICE_TREE	"<your name>"
 (to specify the name of the device tree file is
  board/<vendor>/dts/<your name>.dts)

Typically a CPU device tree include file is provided which defines all the
devices available on that CPU/SOC, with each set to 'status = "disabled"'.
Board device tree files should adjust only the devices they use, setting
'status = 'ok"' in each case, and leaving the existing devices alone and
disabled. To facilitate this, a CPU/SOC device tree header is supported in
arch/<arch>/dts. The name of this is defined by CONFIG_ARCH_DEVICE_TREE,
typically defined in arch/<arch>/cpu/.../config.mk. You can use the
following line within the board device tree file to include this header:

	/include/ ARCH_CPU_DTS

For example, for Tegra2 we might have in arch/arm/cpu/armv7/tegra2/config.mk
these lines:

CONFIG_ARCH_DEVICE_TREE := tegra250

This means that ARCH_CPU_DTS will point to arch/arm/dts/tegra250.dtsi.

This patch set does not include any drivers which actually use the fdt, but
there is a basic fdt decode library (fdtdec) to simplify this process. I
have provided an example i2c implementation previously:

http://patchwork.ozlabs.org/patch/114425/

It is important to understand that the fdt only selects options available in
the platform / drivers. It cannot add new drivers (yet). So you must still
have the CONFIG option to enable the driver. For example, you need to define
CONFIG_SYS_NS16550 to bring in the NS16550 driver, but can use the fdt to
specific the UART clock, peripheral address, etc. In very broad terms, the
CONFIG options in general control *what* driver files are pulled in, and the
fdt controls *how* those files work.

While only ARM is supported in this patch series, it should be easy enough to
add support for other architectures.

Changes in v2:
- Add example proposed decode helper library
- Modify Makefile to create combined U-boot/fdt (u-boot-dtb.bin)
- Add note about CONFIG_ARCH_DEVICE_TREE

Changes in v3:
- Simplify decode library to remove provide only primitive functions
- Remove i2c decode function
- Rename fdt_decode to fdtdec, since it will be used a lot
- Moved fdt_decode.c to /lib
- Export almost all functions from fdtdec, to allow widespread use
- Remove use of FDT_ERR_MISSING which is not strictly needed now
- Add fdtcontroladdr environment variable
- Add note about CONFIG_ARCH_DEVICE_TREE
- Move fdt files into board/<vendor>/dts
- Provide access to ARCH_CPU_DTS which is the CPU's base device tree
- Rename gd->blob to gd->fdt_blob

Simon Glass (6):
  fdt: ARM: Add device tree control of U-Boot (CONFIG_OF_CONTROL)
  fdt: Add support for embedded device tree (CONFIG_OF_EMBED)
  fdt: Add support for a separate device tree (CONFIG_OF_SEPARATE)
  fdt: ARM: Implement embedded and separate device tree
  fdt: ARM: Add fdtcontroladdr to set device tree address in
    environment
  fdt: add decode helper library

 .gitignore                         |    1 +
 Makefile                           |   12 +++
 README                             |   34 +++++++
 arch/arm/include/asm/global_data.h |    1 +
 arch/arm/lib/board.c               |   52 +++++++++--
 config.mk                          |    1 +
 doc/README.fdt-control             |  184 ++++++++++++++++++++++++++++++++++++
 dts/Makefile                       |  109 +++++++++++++++++++++
 include/common.h                   |    1 +
 include/fdtdec.h                   |  122 ++++++++++++++++++++++++
 lib/Makefile                       |    1 +
 lib/fdtdec.c                       |  131 +++++++++++++++++++++++++
 12 files changed, 642 insertions(+), 7 deletions(-)
 create mode 100644 doc/README.fdt-control
 create mode 100644 dts/Makefile
 create mode 100644 include/fdtdec.h
 create mode 100644 lib/fdtdec.c

-- 
1.7.3.1

[U-Boot] [PATCH v3 1/6] fdt: ARM: Add device tree control of U-Boot (CONFIG_OF_CONTROL)

[Simon Glass]

This adds a device tree pointer to the global data. It can be set by
board code. A later commit will add support for embedding it in U-Boot.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v3:
- Rename gd->blob to gd->fdt_blob

 README                             |   11 +++++++++++
 arch/arm/include/asm/global_data.h |    1 +
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/README b/README
index 3a8938f..1175960 100644
--- a/README
+++ b/README
@@ -810,6 +810,17 @@ The following options need to be configured:
 
 		XXX - this list needs to get updated!
 
+- Device tree:
+		CONFIG_OF_CONTROL
+		If this variable is defined, U-Boot will use a device tree
+		to configure its devices, instead of relying on statically
+		compiled #defines in the board file. This option is
+		experimental and only available on a few boards. The device
+		tree is available in the global data as gd->blob.
+
+		U-Boot needs to get its device tree from somewhere. This will
+		be enabled in a future patch.
+
 - Watchdog:
 		CONFIG_WATCHDOG
 		If this variable is defined, it enables watchdog
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
index fac98d5..c3ff789 100644
--- a/arch/arm/include/asm/global_data.h
+++ b/arch/arm/include/asm/global_data.h
@@ -76,6 +76,7 @@ typedef	struct	global_data {
 #if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
 	unsigned long	tlb_addr;
 #endif
+	const void	*fdt_blob;	/* Our device tree, NULL if none */
 	void		**jt;		/* jump table */
 	char		env_buf[32];	/* buffer for getenv() before reloc. */
 #if defined(CONFIG_POST) || defined(CONFIG_LOGBUFFER)
-- 
1.7.3.1

[U-Boot] [PATCH v3 2/6] fdt: Add support for embedded device tree (CONFIG_OF_EMBED)

[Simon Glass]

This new option allows U-Boot to embed a binary device tree into its image
to allow run-time control of peripherals. This device tree is for U-Boot's
own use and is not necessarily the same one as is passed to the kernel.

The device tree compiler output should be placed in the $(obj)
rooted tree. Since $(OBJCOPY) insists on adding the path to the
generated symbol names, to ensure consistency it should be
invoked from the directory where the .dtb file is located and
given the input file name without the path.

This commit contains my entry for the ugliest Makefile / shell interaction
competition.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v3:
- Move fdt files into board/<vendor>/dts
- Provide access to ARCH_CPU_DTS which is the CPU's base device tree

 Makefile         |    4 ++
 README           |   11 ++++-
 config.mk        |    1 +
 dts/Makefile     |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/common.h |    1 +
 5 files changed, 124 insertions(+), 2 deletions(-)
 create mode 100644 dts/Makefile

diff --git a/Makefile b/Makefile
index 5db2e0e..52a6348 100644
--- a/Makefile
+++ b/Makefile
@@ -224,6 +224,9 @@ endif
 ifeq ($(CPU),ixp)
 LIBS += arch/arm/cpu/ixp/npe/libnpe.o
 endif
+ifeq ($(CONFIG_OF_EMBED),y)
+LIBS += dts/libdts.o
+endif
 LIBS += arch/$(ARCH)/lib/lib$(ARCH).o
 LIBS += fs/cramfs/libcramfs.o fs/fat/libfat.o fs/fdos/libfdos.o fs/jffs2/libjffs2.o \
 	fs/reiserfs/libreiserfs.o fs/ext2/libext2fs.o fs/yaffs2/libyaffs2.o \
@@ -962,6 +965,7 @@ clobber:	clean
 	@rm -f $(obj)u-boot.kwb
 	@rm -f $(obj)u-boot.imx
 	@rm -f $(obj)u-boot.ubl
+	@rm -f $(obj)u-boot.dtb
 	@rm -f $(obj)tools/{env/crc32.c,inca-swap-bytes}
 	@rm -f $(obj)arch/powerpc/cpu/mpc824x/bedbug_603e.c
 	@rm -fr $(obj)include/asm/proc $(obj)include/asm/arch $(obj)include/asm
diff --git a/README b/README
index 1175960..8678d78 100644
--- a/README
+++ b/README
@@ -818,8 +818,15 @@ The following options need to be configured:
 		experimental and only available on a few boards. The device
 		tree is available in the global data as gd->blob.
 
-		U-Boot needs to get its device tree from somewhere. This will
-		be enabled in a future patch.
+		U-Boot needs to get its device tree from somewhere. At present
+		the only way is to embed it in the image with CONFIG_OF_EMBED.
+
+		CONFIG_OF_EMBED
+		If this variable is defined, U-Boot will embed a device tree
+		binary in its image. This device tree file should be in the
+		board directory and called <soc>-<board>.dts. The binary file
+		is then picked up in board_init_f() and made available through
+		the global data structure as gd->blob.
 
 - Watchdog:
 		CONFIG_WATCHDOG
diff --git a/config.mk b/config.mk
index e2b440d..6e61eb6 100644
--- a/config.mk
+++ b/config.mk
@@ -124,6 +124,7 @@ STRIP	= $(CROSS_COMPILE)strip
 OBJCOPY = $(CROSS_COMPILE)objcopy
 OBJDUMP = $(CROSS_COMPILE)objdump
 RANLIB	= $(CROSS_COMPILE)RANLIB
+DTC	= dtc
 
 #########################################################################
 
diff --git a/dts/Makefile b/dts/Makefile
new file mode 100644
index 0000000..06b7603
--- /dev/null
+++ b/dts/Makefile
@@ -0,0 +1,109 @@
+#
+# Copyright (c) 2011 The Chromium OS Authors.
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundatio; either version 2 of
+# the License, or (at your option) any later version.
+#
+# 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.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+# This Makefile builds the internal U-Boot fdt if CONFIG_OF_CONTROL is
+# enabled. See doc/README.fdt-control for more details.
+
+include $(TOPDIR)/config.mk
+
+LIB	= $(obj)libdts.o
+
+$(if $(CONFIG_DEFAULT_DEVICE_TREE),,\
+$(error Please define CONFIG_DEFAULT_DEVICE_TREE in your board header file))
+DEVICE_TREE = $(subst ",,$(CONFIG_DEFAULT_DEVICE_TREE))
+
+$(if $(CONFIG_ARCH_DEVICE_TREE),,\
+$(error Your architecture does not have device tree support enabled. \
+Please define CONFIG_ARCH_DEVICE_TREE))
+
+# We preprocess the device tree file provide a useful define
+DTS_CPPFLAGS := -DARCH_CPU_DTS=\"../arch/$(ARCH)/dts/$(CONFIG_ARCH_DEVICE_TREE).dtsi\"
+
+all:	$(obj).depend $(LIB)
+
+# Use a constant name for this so we can access it from C code.
+# objcopy doesn't seem to allow us to set the symbol name independently of
+# the filename.
+DT_BIN	:= $(obj)dt.dtb
+
+$(DT_BIN): $(TOPDIR)/board/$(VENDOR)/dts/$(DEVICE_TREE).dts
+	cat $< | $(CPP) -P $(DTS_CPPFLAGS) - >$@.tmp
+	$(DTC) -R 4 -p 0x1000 -O dtb -o ${DT_BIN} $@.tmp
+	rm $@.tmp
+
+process_lds = \
+	$(1) | sed -r -n 's/^OUTPUT_$(2)[ ("]*([^")]*).*/\1/p'
+
+# Run the compiler and get the link script from the linker
+GET_LDS = $(CC) $(CFLAGS) $(LDFLAGS) -Wl,--verbose 2>&1
+
+$(obj)dt.o: $(DT_BIN)
+	# We want the output format and arch.
+	# We also hope to win a prize for ugliest Makefile / shell interaction
+	# We look in the LDSCRIPT first.
+	# Then try the linker which should give us the answer.
+	# Then check it worked.
+	oformat=`$(call process_lds,cat $(LDSCRIPT),FORMAT)` ;\
+	oarch=`$(call process_lds,cat $(LDSCRIPT),ARCH)` ;\
+	\
+	[ -z $${oformat} ] && \
+		oformat=`$(call process_lds,$(GET_LDS),FORMAT)` ;\
+	[ -z $${oarch} ] && \
+		oarch=`$(call process_lds,$(GET_LDS),ARCH)` ;\
+	\
+	[ -z $${oformat} ] && \
+		echo "Cannot read OUTPUT_FORMAT from lds file $(LDSCRIPT)" && \
+		exit 1 || true ;\
+	[ -z $${oarch} ] && \
+		echo "Cannot read OUTPUT_ARCH from lds file $(LDSCRIPT)" && \
+		exit 1 || true ;\
+	\
+	cd $(dir ${DT_BIN}) && \
+	$(OBJCOPY) -I binary -O $${oformat} -B $${oarch} \
+		$(notdir ${DT_BIN}) $@
+	rm $(DT_BIN)
+
+OBJS-$(CONFIG_OF_EMBED)	:= dt.o
+
+COBJS	:= $(OBJS-y)
+
+OBJS	:= $(addprefix $(obj),$(COBJS))
+
+binary:	$(DT_BIN)
+
+$(LIB):	$(OBJS) $(DTB)
+	$(call cmd_link_o_target, $(OBJS))
+
+clean:
+	rm -f $(OBJS) $(LIB)
+	rm -f $(DT_BIN)
+
+distclean:	clean
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
diff --git a/include/common.h b/include/common.h
index eb19a44..1feeaf4 100644
--- a/include/common.h
+++ b/include/common.h
@@ -270,6 +270,7 @@ int	checkdram     (void);
 int	last_stage_init(void);
 extern ulong monitor_flash_len;
 int mac_read_from_eeprom(void);
+extern u8 _binary_dt_dtb_start[];	/* embedded device tree blob */
 
 /* common/flash.c */
 void flash_perror (int);
-- 
1.7.3.1

[U-Boot] [PATCH v3 3/6] fdt: Add support for a separate device tree (CONFIG_OF_SEPARATE)

[Simon Glass]

This adds support for an FDT to be build as a separate binary file called
u-boot.dtb. This can be concatenated with the U-Boot binary to provide a
device tree located at run-time by U-Boot. The Makefile is modified to
provide this file in u-boot-dtb.bin.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Modify Makefile to create combined U-boot/fdt (u-boot-dtb.bin)
- Add note about CONFIG_ARCH_DEVICE_TREE

 .gitignore             |    1 +
 Makefile               |    8 ++
 README                 |   16 ++++-
 doc/README.fdt-control |  172 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 195 insertions(+), 2 deletions(-)
 create mode 100644 doc/README.fdt-control

diff --git a/.gitignore b/.gitignore
index 0a9dc0c..a367082 100644
--- a/.gitignore
+++ b/.gitignore
@@ -35,6 +35,7 @@
 /u-boot.dis
 /u-boot.lds
 /u-boot.ubl
+/u-boot.dtb
 
 #
 # Generated files
diff --git a/Makefile b/Makefile
index 52a6348..c28c8bf 100644
--- a/Makefile
+++ b/Makefile
@@ -354,9 +354,17 @@ ALL-$(CONFIG_ONENAND_U_BOOT) += $(obj)u-boot-onenand.bin
 ONENAND_BIN ?= $(obj)onenand_ipl/onenand-ipl-2k.bin
 ALL-$(CONFIG_MMC_U_BOOT) += $(obj)mmc_spl/u-boot-mmc-spl.bin
 ALL-$(CONFIG_SPL) += $(obj)spl/u-boot-spl.bin
+ALL-$(CONFIG_OF_SEPARATE) += $(obj)u-boot.dtb $(obj)u-boot-dtb.bin
 
 all:		$(ALL-y)
 
+$(obj)u-boot.dtb:	$(obj)u-boot
+		$(MAKE) -C dts binary
+		mv $(obj)dts/dt.dtb $@
+
+$(obj)u-boot-dtb.bin:	$(obj)u-boot.bin $(obj)u-boot.dtb
+		cat $^ >$@
+
 $(obj)u-boot.hex:	$(obj)u-boot
 		$(OBJCOPY) ${OBJCFLAGS} -O ihex $< $@
 
diff --git a/README b/README
index 8678d78..8ab2b0c 100644
--- a/README
+++ b/README
@@ -818,8 +818,8 @@ The following options need to be configured:
 		experimental and only available on a few boards. The device
 		tree is available in the global data as gd->blob.
 
-		U-Boot needs to get its device tree from somewhere. At present
-		the only way is to embed it in the image with CONFIG_OF_EMBED.
+		U-Boot needs to get its device tree from somewhere. This can
+		be done using one of the two options below:
 
 		CONFIG_OF_EMBED
 		If this variable is defined, U-Boot will embed a device tree
@@ -828,6 +828,18 @@ The following options need to be configured:
 		is then picked up in board_init_f() and made available through
 		the global data structure as gd->blob.
 
+		CONFIG_OF_SEPARATE
+		If this variable is defined, U-Boot will build a device tree
+		binary. It will be called u-boot.dtb. Architecture-specific
+		code will locate it at run-time. Generally this works by:
+
+			cat u-boot.bin u-boot.dtb >image.bin
+
+		and in fact, U-Boot does this for you, creating a file called
+		u-boot-dtb.bin which is useful in the common case. You can
+		still use the individual files if you need something more
+		exotic.
+
 - Watchdog:
 		CONFIG_WATCHDOG
 		If this variable is defined, it enables watchdog
diff --git a/doc/README.fdt-control b/doc/README.fdt-control
new file mode 100644
index 0000000..3f8bb5a
--- /dev/null
+++ b/doc/README.fdt-control
@@ -0,0 +1,172 @@
+#
+# Copyright (c) 2011 The Chromium OS Authors.
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundatio; either version 2 of
+# the License, or (at your option) any later version.
+#
+# 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.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+Device Tree Control in U-Boot
+=============================
+
+This feature provides for run-time configuration of U-Boot via a flat
+device tree (fdt). U-Boot configuration has traditionally been done
+using CONFIG options in the board config file. This feature aims to
+make it possible for a single U-Boot binary to support multiple boards,
+with the exact configuration of each board controlled by a flat device
+tree (fdt). This is the approach recently taken by the ARM Linux kernel
+and has been used by PowerPC for some time.
+
+The fdt is a convenient vehicle for implementing run-time configuration
+for three reasons. Firstly it is easy to use, being a simple text file.
+It is extensible since it consists of nodes and properties in a nice
+hierarchical format.
+
+Finally, there is already excellent infrastructure for the fdt: a
+compiler checks the text file and converts it to a compact binary
+format, and a library is already available in U-Boot (libfdt) for
+handling this format.
+
+The dts directory contains a Makefile for building the device tree blob
+and embedding it in your U-Boot image. This is useful since it allows
+U-Boot to configure itself according to what it finds there. If you have
+a number of similar boards with different peripherals, you can describe
+the features of each board in the device tree file, and have a single
+generic source base.
+
+To enable this feature, add CONFIG_OF_CONTROL to your board config file.
+
+
+What is a Flat Device Tree?
+---------------------------
+
+An fdt can be specified in source format as a text file. To read about
+the fdt syntax, take a look at the specification here:
+
+https://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf
+
+You also might find this section of the Linux kernel documentation
+useful: (access this in the Linux kernel source code)
+
+	Documentation/devicetree/booting-without-of.txt
+
+There is also a mailing list:
+
+	http://lists.ozlabs.org/listinfo/devicetree-discuss
+
+In case you are wondering, OF stands for Open Firmware.
+
+
+Tools
+-----
+
+To use this feature you will need to get the device tree compiler here:
+
+	git://jdl.com/software/dtc.git
+
+For example:
+
+	$ git clone git://jdl.com/software/dtc.git
+	$ cd dtc
+	$ make
+	$ sudo make install
+
+Then run the compiler (your version will vary):
+
+	$ dtc -v
+	Version: DTC 1.2.0-g2cb4b51f
+	$ make tests
+	$ cd tests
+	$ ./run_tests.sh
+	********** TEST SUMMARY
+	*     Total testcases:	1371
+	*                PASS:	1371
+	*                FAIL:	0
+	*   Bad configuration:	0
+	* Strange test result:	0
+
+You will also find a useful ftdump utility for decoding a binary file.
+
+
+Where do I get an fdt file for my board?
+----------------------------------------
+
+You may find that the Linux kernel has a suitable file. Look in the
+kernel source in arch/<arch>/boot/dts.
+
+If not you might find other boards with suitable files that you can
+modify to your needs. Look in the board directories for files with a
+.dts extension.
+
+Failing that, you could write one from scratch yourself!
+
+
+Configuration
+-------------
+
+Use:
+
+#define CONFIG_DEFAULT_DEVICE_TREE	"<name>"
+
+to set the filename of the device tree source. Then put your device tree
+file into
+
+	board/<vendor>/dts/<name>.dts
+
+This should include your CPU or SOC's device tree file, placed in
+arch/<arch>/dts, and then make any adjustments required. The name of this
+is CONFIG_ARCH_DEVICE_TREE.dts.
+
+If CONFIG_OF_EMBED is defined, then it will be picked up and built into
+the U-Boot image (including u-boot.bin).
+
+If CONFIG_OF_SEPARATE is defined, then it will be built and placed in
+a u-boot.dtb file alongside u-boot.bin. A common approach is then to
+join the two:
+
+	cat u-boot.bin u-boot.dtb >image.bin
+
+and then flash image.bin onto your board.
+
+You cannot use both of these options at the same time.
+
+
+Limitations
+-----------
+
+U-Boot is designed to build with a single architecture type and CPU
+type. So for example it is not possible to build a single ARM binary
+which runs on your AT91 and OMAP boards, relying on an fdt to configure
+the various features. This is because you must select one of
+the CPU families within arch/arm/cpu/arm926ejs (omap or at91) at build
+time. Similarly you cannot build for multiple cpu types or
+architectures.
+
+That said the complexity reduction by using fdt to support variants of
+boards which use the same SOC / CPU can be substantial.
+
+It is important to understand that the fdt only selects options
+available in the platform / drivers. It cannot add new drivers (yet). So
+you must still have the CONFIG option to enable the driver. For example,
+you need to define CONFIG_SYS_NS16550 to bring in the NS16550 driver,
+but can use the fdt to specific the UART clock, peripheral address, etc.
+In very broad terms, the CONFIG options in general control *what* driver
+files are pulled in, and the fdt controls *how* those files work.
+
+--
+Simon Glass <sjg@chromium.org>
+1-Sep-11
-- 
1.7.3.1

[U-Boot] [PATCH v3 4/6] fdt: ARM: Implement embedded and separate device tree

[Simon Glass]

This locates the device tree either embedded within U-Boot or attached to the
end as a separate binary.

When CONFIG_OF_CONTROL is defined, U-Boot requires a valid fdt. A check is
provided for this early in initialisation.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v3:
- Add note about CONFIG_ARCH_DEVICE_TREE

 arch/arm/lib/board.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 1fe3751..b0f3162 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -48,6 +48,7 @@
 #include <nand.h>
 #include <onenand_uboot.h>
 #include <mmc.h>
+#include <libfdt.h>
 #include <post.h>
 #include <logbuff.h>
 
@@ -197,6 +198,17 @@ static int arm_pci_init(void)
 }
 #endif /* CONFIG_CMD_PCI || CONFIG_PCI */
 
+#ifdef CONFIG_OF_CONTROL
+static int check_fdt(void)
+{
+	/* We must have an fdt */
+	if (fdt_check_header(gd->fdt_blob))
+		panic("No valid fdt found - please append one to U-Boot\n"
+			"binary or define CONFIG_OF_EMBED\n");
+	return 0;
+}
+#endif
+
 /*
  * Breathe some life into the board...
  *
@@ -239,6 +251,9 @@ init_fnc_t *init_sequence[] = {
 #if defined(CONFIG_BOARD_EARLY_INIT_F)
 	board_early_init_f,
 #endif
+#ifdef CONFIG_OF_CONTROL
+	check_fdt,
+#endif
 	timer_init,		/* initialize timer */
 #ifdef CONFIG_FSL_ESDHC
 	get_clocks,
@@ -276,6 +291,13 @@ void board_init_f(ulong bootflag)
 	memset((void *)gd, 0, sizeof(gd_t));
 
 	gd->mon_len = _bss_end_ofs;
+#ifdef CONFIG_OF_EMBED
+	/* Get a pointer to the FDT */
+	gd->fdt_blob = _binary_dt_dtb_start;
+#elif defined CONFIG_OF_SEPARATE
+	/* FDT is at end of image */
+	gd->fdt_blob = (void *)(_end_ofs + _TEXT_BASE);
+#endif
 
 	for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
 		if ((*init_fnc_ptr)() != 0) {
-- 
1.7.3.1

[U-Boot] [PATCH v3 5/6] fdt: ARM: Add fdtcontroladdr to set device tree address in environment

[Simon Glass]

This adds support for a new environment variable called 'fdtcontroladdr'. If
defined, the hex address is used as the address of the control fdt for U-Boot.

Note: I have not changed CONFIG_PRAM section as I already have an
outstanding patch on that.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v3:
- Add fdtcontroladdr environment variable

 README                 |    4 ++++
 arch/arm/lib/board.c   |   30 +++++++++++++++++++++++-------
 doc/README.fdt-control |   12 ++++++++++++
 3 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/README b/README
index 8ab2b0c..f121d4d 100644
--- a/README
+++ b/README
@@ -3489,6 +3489,10 @@ List of environment variables (most likely not complete):
 		  add the information it needs into it, and the memory
 		  must be accessible by the kernel.
 
+  fdtcontroladdr- if set this is the address of the control flattened
+		  device tree used by U-Boot when CONFIG_OF_CONTROL is
+		  defined.
+
   i2cfast	- (PPC405GP|PPC405EP only)
 		  if set to 'y' configures Linux I2C driver for fast
 		  mode (400kHZ). This environment variable is used in
diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index b0f3162..f26d77a 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -116,18 +116,31 @@ void blue_led_off(void) __attribute__((weak, alias("__blue_led_off")));
  * but let's get it working (again) first...
  */
 
+/**
+ * Decode the value of an environment variable and return it.
+ *
+ * @param name		Name of environemnt variable
+ * @param base		Number base to use (normally 10, or 16 for hex)
+ * @param default_val	Default value to return if the variable is not
+ *			found
+ * @return the decoded value, or default_val if not found
+ */
+static int getenv_int(const char *name, int base, int default_val)
+{
+	char tmp[64];	/* long enough for environment variables */
+	int i = getenv_f(name, tmp, sizeof(tmp));
+
+	return (i > 0)
+		? (int) simple_strtoul(tmp, NULL, base)
+		: default_val;
+}
+
 #if defined(CONFIG_ARM_DCC) && !defined(CONFIG_BAUDRATE)
 #define CONFIG_BAUDRATE 115200
 #endif
 static int init_baudrate(void)
 {
-	char tmp[64];	/* long enough for environment variables */
-	int i = getenv_f("baudrate", tmp, sizeof(tmp));
-
-	gd->baudrate = (i > 0)
-			? (int) simple_strtoul(tmp, NULL, 10)
-			: CONFIG_BAUDRATE;
-
+	gd->baudrate = getenv_int("baudrate", 10, CONFIG_BAUDRATE);
 	return (0);
 }
 
@@ -298,6 +311,9 @@ void board_init_f(ulong bootflag)
 	/* FDT is at end of image */
 	gd->fdt_blob = (void *)(_end_ofs + _TEXT_BASE);
 #endif
+	/* Allow the early environment to override the fdt address */
+	gd->fdt_blob = (void *)getenv_int("fdtcontroladdr", 16,
+						(uintptr_t)gd->fdt_blob);
 
 	for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
 		if ((*init_fnc_ptr)() != 0) {
diff --git a/doc/README.fdt-control b/doc/README.fdt-control
index 3f8bb5a..85bda03 100644
--- a/doc/README.fdt-control
+++ b/doc/README.fdt-control
@@ -144,6 +144,18 @@ and then flash image.bin onto your board.
 
 You cannot use both of these options at the same time.
 
+If you wish to put the fdt at a different address in memory, you can
+define the "fdtcontroladdr" environment variable. This is the hex
+address of the fdt binary blob, and will override either of the options.
+Be aware that this environment variable is checked prior to relocation,
+when only the compiled-in environment is available. Therefore it is not
+possible to define this variable in the saved SPI/NAND flash
+environment, for example (it will be ignored).
+
+To use this, put something like this in your board header file:
+
+#define CONFIG_EXTRA_ENV_SETTINGS	"fdtcontroladdr=10000\0"
+
 
 Limitations
 -----------
-- 
1.7.3.1

[U-Boot] [PATCH v3 6/6] fdt: add decode helper library

[Simon Glass]

This library provides useful functions to drivers which want to use
the fdt to control their operation. Functions are provided to:

- look up and enumerate a device type (for example assigning i2c bus 0,
 i2c bus 1, etc.)
- decode basic types from the fdt, like addresses and integers

While this library is not strictly necessary, it helps to minimise the
changes to a driver, in order to make it work under fdt control. Less
code is required, and so the barrier to switch drivers over is lower.

Additional functions to read arrays and GPIOs could be made available
here also.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Add example proposed decode helper library

Changes in v3:
- Simplify decode library to remove provide only primitive functions
- Remove i2c decode function
- Rename fdt_decode to fdtdec, since it will be used a lot
- Moved fdt_decode.c to /lib
- Export almost all functions from fdtdec, to allow widespread use
- Remove use of FDT_ERR_MISSING which is not strictly needed now

 include/fdtdec.h |  122 ++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/Makefile     |    1 +
 lib/fdtdec.c     |  131 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 254 insertions(+), 0 deletions(-)
 create mode 100644 include/fdtdec.h
 create mode 100644 lib/fdtdec.c

diff --git a/include/fdtdec.h b/include/fdtdec.h
new file mode 100644
index 0000000..b38f48c
--- /dev/null
+++ b/include/fdtdec.h
@@ -0,0 +1,122 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+
+/*
+ * This file contains convenience functions for decoding useful and
+ * enlightening information from FDTs. It is intended to be used by device
+ * drivers and board-specific code within U-Boot. It aims to reduce the
+ * amount of FDT munging required within U-Boot itself, so that driver code
+ * changes to support FDT are minimized.
+ */
+
+#include <libfdt.h>
+
+/*
+ * A typedef for a physical address. Note that fdt data is always big
+ * endian even on a litle endian machine.
+ */
+#ifdef CONFIG_PHYS_64BIT
+typedef u64 addr_t;
+#define ADDR_T_NONE (-1ULL)
+#define addr_to_cpu(reg) be64_to_cpu(reg)
+#else
+typedef u32 addr_t;
+#define ADDR_T_NONE (-1U)
+#define addr_to_cpu(reg) be32_to_cpu(reg)
+#endif
+
+/* Information obtained about memory from the FDT */
+struct fdt_memory {
+	addr_t start;
+	addr_t end;
+};
+
+/**
+ * Compat types that we know about and for which we might have drivers.
+ * Each is named COMPAT_<dir>_<filename> where <dir> is the directory
+ * within drivers.
+ */
+enum fdt_compat_id {
+	COMPAT_UNKNOWN,
+
+	COMPAT_COUNT,
+};
+
+/**
+ * Find the next numbered alias for a peripheral. This is used to enumerate
+ * all the peripherals of a certain type.
+ *
+ * Do the first call with *upto = 0. Assuming /aliases/<name>0 exists then
+ * this function will return a pointer to the node the alias points to, and
+ * then update *upto to 1. Next time you call this function, the next node
+ * will be returned.
+ *
+ * All nodes returned will match the compatible ID, as it is assumed that
+ * all peripherals use the same driver.
+ *
+ * @param blob		FDT blob to use
+ * @param name		Root name of alias to search for
+ * @param id		Compatible ID to look for
+ * @return offset of next compatible node, or -FDT_ERR_NOTFOUND if no more
+ */
+int fdtdec_next_alias(const void *blob, const char *name,
+		enum fdt_compat_id id, int *upto);
+
+/**
+ * Look up an address property in a node and return it as an address.
+ * The property must hold either one address with no trailing data or
+ * one address with a length. This is only tested on 32-bit machines.
+ *
+ * @param blob	FDT blob
+ * @param node	node to examine
+ * @param prop_name	name of property to find
+ * @return address, if found, or ADDR_T_NONE if not
+ */
+addr_t fdtdec_get_addr(const void *blob, int node,
+		const char *prop_name);
+
+/**
+ * Look up a 32-bit integer property in a node and return it. The property
+ * must have at least 4 bytes of data. The value of the first cell is
+ * returned.
+ *
+ * @param blob	FDT blob
+ * @param node	node to examine
+ * @param prop_name	name of property to find
+ * @param default_val	default value to return if the property is not found
+ * @return integer value, if found, or default_val if not
+ */
+s32 fdtdec_get_int(const void *blob, int node, const char *prop_name,
+		s32 default_val);
+
+/**
+ * Checks whether a node is enabled.
+ * This looks for a 'status' property. If this exists, then returns 1 if
+ * the status is 'ok' and 0 otherwise. If there is no status property,
+ * it returns the default value.
+ *
+ * @param blob	FDT blob
+ * @param node	node to examine
+ * @param default_val	default value to return if no 'status' property exists
+ * @return integer value 0/1, if found, or default_val if not
+ */
+int fdtdec_get_is_enabled(const void *blob, int node, int default_val);
diff --git a/lib/Makefile b/lib/Makefile
index 884f64c..3b901c9 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -38,6 +38,7 @@ COBJS-y += crc16.o
 COBJS-y += crc32.o
 COBJS-y += display_options.o
 COBJS-y += errno.o
+COBJS-$(CONFIG_OF_CONTROL) += fdtdec.o
 COBJS-$(CONFIG_GZIP) += gunzip.o
 COBJS-y += hashtable.o
 COBJS-$(CONFIG_LMB) += lmb.o
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
new file mode 100644
index 0000000..c33b3ed
--- /dev/null
+++ b/lib/fdtdec.c
@@ -0,0 +1,131 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <serial.h>
+#include <libfdt.h>
+#include <fdtdec.h>
+
+/* we need a generic GPIO interface here */
+#include <asm/arch/gpio.h>
+
+/*
+ * Here are the type we know about. One day we might allow drivers to
+ * register. For now we just put them here. The COMPAT macro allows us to
+ * turn this into a sparse list later, and keeps the ID with the name.
+ */
+#define COMPAT(id, name) name
+static const char *compat_names[COMPAT_COUNT] = {
+};
+
+/**
+ * Look in the FDT for an alias with the given name and return its node.
+ *
+ * @param blob	FDT blob
+ * @param name	alias name to look up
+ * @return node offset if found, or an error code < 0 otherwise
+ */
+static int find_alias_node(const void *blob, const char *name)
+{
+	const char *path;
+	int alias_node;
+
+	debug("find_alias_node: %s\n", name);
+	alias_node = fdt_path_offset(blob, "/aliases");
+	if (alias_node < 0)
+		return alias_node;
+	path = fdt_getprop(blob, alias_node, name, NULL);
+	if (!path)
+		return -FDT_ERR_NOTFOUND;
+	return fdt_path_offset(blob, path);
+}
+
+addr_t fdtdec_get_addr(const void *blob, int node,
+		const char *prop_name)
+{
+	const addr_t *cell;
+	int len;
+
+	debug("get_addr: %s\n", prop_name);
+	cell = fdt_getprop(blob, node, prop_name, &len);
+	if (cell && (len == sizeof(addr_t) || len == sizeof(addr_t) * 2))
+		return addr_to_cpu(*cell);
+	return ADDR_T_NONE;
+}
+
+s32 fdtdec_get_int(const void *blob, int node, const char *prop_name,
+		s32 default_val)
+{
+	const s32 *cell;
+	int len;
+
+	debug("get_size: %s\n", prop_name);
+	cell = fdt_getprop(blob, node, prop_name, &len);
+	if (cell && len >= sizeof(s32))
+		return fdt32_to_cpu(cell[0]);
+	return default_val;
+}
+
+int fdtdec_get_is_enabled(const void *blob, int node, int default_val)
+{
+	const char *cell;
+
+	cell = fdt_getprop(blob, node, "status", NULL);
+	if (cell)
+		return 0 == strcmp(cell, "ok");
+	return default_val;
+}
+
+enum fdt_compat_id fd_dec_lookup(const void *blob, int node)
+{
+	enum fdt_compat_id id;
+
+	/* Search our drivers */
+	for (id = COMPAT_UNKNOWN; id < COMPAT_COUNT; id++)
+		if (0 == fdt_node_check_compatible(blob, node,
+				compat_names[id]))
+			return id;
+	return COMPAT_UNKNOWN;
+}
+
+int fdtdec_next_compatible(const void *blob, int node,
+		enum fdt_compat_id id)
+{
+	return fdt_node_offset_by_compatible(blob, node, compat_names[id]);
+}
+
+int fdtdec_next_alias(const void *blob, const char *name,
+		enum fdt_compat_id id, int *upto)
+{
+#define MAX_STR_LEN 20
+	char str[MAX_STR_LEN + 20];
+	int node, err;
+
+	sprintf(str, "%.*s%d", MAX_STR_LEN, name, *upto);
+	(*upto)++;
+	node = find_alias_node(blob, str);
+	if (node < 0)
+		return node;
+	err = fdt_node_check_compatible(blob, node, compat_names[id]);
+	if (err < 0)
+		return err;
+	return err ? -FDT_ERR_NOTFOUND : node;
+}
-- 
1.7.3.1

[U-Boot] [PATCH v3 0/6] Run-time configuration of U-Boot via a flat device tree (fdt)

[Stephen Warren]

Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
...
> and add some defines to your board (only ARM is currently supported):
> 
>  #define CONFIG_OF_CONTROL       (to enable run-time config control via fdt)
>  #define CONFIG_OF_EMBED or CONFIG_OF_SEPARATE
>  (either build the fdt blob into U-Boot, or create a separate u-boot.dtb
>   and u-boot-dtb.bin)
>  #define CONFIG_DEFAULT_DEVICE_TREE	"<your name>"
>  (to specify the name of the device tree file is
>   board/<vendor>/dts/<your name>.dts)
> 
> Typically a CPU device tree include file is provided which defines all the
> devices available on that CPU/SOC, with each set to 'status = "disabled"'.
> Board device tree files should adjust only the devices they use, setting
> 'status = 'ok"' in each case, and leaving the existing devices alone and
> disabled.

s/existing/other/ in the second-to-last line? Well, I guess given this is
patch 0, that's not important.

In the Linux kernel, things used to work exactly as described above, but
the kernel has switched to having no "status" properties in the SoC base
file (and hence everything defaults to enabled), with the per-board .dts
files set 'status = "disabled"' where desired. I imagine U-Boot should
follow the same practice.

I forget the exact reason for this in the kernel; it may simply have been
due to precedent on PowerPC. Grant Likely would know the details.

...
> For example, for Tegra2 we might have in arch/arm/cpu/armv7/tegra2/config.mk
> these lines:
> 
> CONFIG_ARCH_DEVICE_TREE := tegra250
> 
> This means that ARCH_CPU_DTS will point to arch/arm/dts/tegra250.dtsi.

In the kernel, we renamed this to tegra20; "Tegra 250" is a marketing name,
whereas "Tegra 20" is an engineering name. Engineering names should be more
stable, and probably fewer in number, and so are more suitable for device-
tree.

(I guess that's not particularly important for this patchset, since it
doesn't include tegra250.dtsi, but it'll be relevant soon I'm sure:-)

-- 
nvpublic

[U-Boot] [PATCH v3 1/6] fdt: ARM: Add device tree control of U-Boot (CONFIG_OF_CONTROL)

[Stephen Warren]

Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
> This adds a device tree pointer to the global data. It can be set by
> board code. A later commit will add support for embedding it in U-Boot.

s/it/the device-tree/?

...
> diff --git a/README b/README
...
> +- Device tree:
> +		CONFIG_OF_CONTROL
> +		If this variable is defined, U-Boot will use a device tree
> +		to configure its devices, instead of relying on statically
> +		compiled #defines in the board file. This option is
> +		experimental and only available on a few boards. The device
> +		tree is available in the global data as gd->blob.

gd->fdt_blob

-- 
nvpublic

[U-Boot] [PATCH v3 0/6] Run-time configuration of U-Boot via a flat device tree (fdt)

[Simon Glass]

Hi Stephen,

On Tue, Oct 11, 2011 at 4:37 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
> ...
>> and add some defines to your board (only ARM is currently supported):
>>
>> ?#define CONFIG_OF_CONTROL ? ? ? (to enable run-time config control via fdt)
>> ?#define CONFIG_OF_EMBED or CONFIG_OF_SEPARATE
>> ?(either build the fdt blob into U-Boot, or create a separate u-boot.dtb
>> ? and u-boot-dtb.bin)
>> ?#define CONFIG_DEFAULT_DEVICE_TREE ? "<your name>"
>> ?(to specify the name of the device tree file is
>> ? board/<vendor>/dts/<your name>.dts)
>>
>> Typically a CPU device tree include file is provided which defines all the
>> devices available on that CPU/SOC, with each set to 'status = "disabled"'.
>> Board device tree files should adjust only the devices they use, setting
>> 'status = 'ok"' in each case, and leaving the existing devices alone and
>> disabled.
>
> s/existing/other/ in the second-to-last line? Well, I guess given this is
> patch 0, that's not important.

Will fix.

>
> In the Linux kernel, things used to work exactly as described above, but
> the kernel has switched to having no "status" properties in the SoC base
> file (and hence everything defaults to enabled), with the per-board .dts
> files set 'status = "disabled"' where desired. I imagine U-Boot should
> follow the same practice.
>
> I forget the exact reason for this in the kernel; it may simply have been
> due to precedent on PowerPC. Grant Likely would know the details.

Well it's going to create a bit of work for boards that use only a
subset of these 30-peripheral SOCs, but perhaps the expectation is
that you would be buying a simpler SOC if you didn't need all the
features.

>
> ...
>> For example, for Tegra2 we might have in arch/arm/cpu/armv7/tegra2/config.mk
>> these lines:
>>
>> CONFIG_ARCH_DEVICE_TREE := tegra250
>>
>> This means that ARCH_CPU_DTS will point to arch/arm/dts/tegra250.dtsi.
>
> In the kernel, we renamed this to tegra20; "Tegra 250" is a marketing name,
> whereas "Tegra 20" is an engineering name. Engineering names should be more
> stable, and probably fewer in number, and so are more suitable for device-
> tree.
>
> (I guess that's not particularly important for this patchset, since it
> doesn't include tegra250.dtsi, but it'll be relevant soon I'm sure:-)

Yes I did see that on the list. I will change the message in this
patch set, and try to remember when we get to 'soon' :-)

Regards,
Simon

>
> --
> nvpublic
>
>

[U-Boot] [PATCH v3 0/6] Run-time configuration of U-Boot via a flat device tree (fdt)

[Jason]

Simon, Stephen,

On Tue, Oct 11, 2011 at 04:54:27PM -0700, Simon Glass wrote:
> Hi Stephen,
> 
> On Tue, Oct 11, 2011 at 4:37 PM, Stephen Warren <swarren@nvidia.com> wrote:
> > Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
> > ...
> >> and add some defines to your board (only ARM is currently supported):
> >>
> >> ?#define CONFIG_OF_CONTROL ? ? ? (to enable run-time config control via fdt)
> >> ?#define CONFIG_OF_EMBED or CONFIG_OF_SEPARATE
> >> ?(either build the fdt blob into U-Boot, or create a separate u-boot.dtb
> >> ? and u-boot-dtb.bin)
> >> ?#define CONFIG_DEFAULT_DEVICE_TREE ? "<your name>"
> >> ?(to specify the name of the device tree file is
> >> ? board/<vendor>/dts/<your name>.dts)
> >>
> >> Typically a CPU device tree include file is provided which defines all the
> >> devices available on that CPU/SOC, with each set to 'status = "disabled"'.
> >> Board device tree files should adjust only the devices they use, setting
> >> 'status = 'ok"' in each case, and leaving the existing devices alone and
> >> disabled.
...
> >
> > In the Linux kernel, things used to work exactly as described above, but
> > the kernel has switched to having no "status" properties in the SoC base
> > file (and hence everything defaults to enabled), with the per-board .dts
> > files set 'status = "disabled"' where desired. I imagine U-Boot should
> > follow the same practice.
> >
> > I forget the exact reason for this in the kernel; it may simply have been
> > due to precedent on PowerPC. Grant Likely would know the details.
> 
> Well it's going to create a bit of work for boards that use only a
> subset of these 30-peripheral SOCs, but perhaps the expectation is
> that you would be buying a simpler SOC if you didn't need all the
> features.

I imagine u-boot's methodology of loading drivers only as needed will
prevent the unnecessary initialization of 30 drivers which are default
'status = "enabled";'.  Or, am I missing something?

thx,

Jason.

[U-Boot] [PATCH v3 0/6] Run-time configuration of U-Boot via a flat device tree (fdt)

[Simon Glass]

Hi Jason,

On Tue, Oct 11, 2011 at 6:00 PM, Jason <u-boot@lakedaemon.net> wrote:
> Simon, Stephen,
>
> On Tue, Oct 11, 2011 at 04:54:27PM -0700, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Tue, Oct 11, 2011 at 4:37 PM, Stephen Warren <swarren@nvidia.com> wrote:
>> > Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
>> > ...
>> >> and add some defines to your board (only ARM is currently supported):
>> >>
>> >> ?#define CONFIG_OF_CONTROL ? ? ? (to enable run-time config control via fdt)
>> >> ?#define CONFIG_OF_EMBED or CONFIG_OF_SEPARATE
>> >> ?(either build the fdt blob into U-Boot, or create a separate u-boot.dtb
>> >> ? and u-boot-dtb.bin)
>> >> ?#define CONFIG_DEFAULT_DEVICE_TREE ? "<your name>"
>> >> ?(to specify the name of the device tree file is
>> >> ? board/<vendor>/dts/<your name>.dts)
>> >>
>> >> Typically a CPU device tree include file is provided which defines all the
>> >> devices available on that CPU/SOC, with each set to 'status = "disabled"'.
>> >> Board device tree files should adjust only the devices they use, setting
>> >> 'status = 'ok"' in each case, and leaving the existing devices alone and
>> >> disabled.
> ...
>> >
>> > In the Linux kernel, things used to work exactly as described above, but
>> > the kernel has switched to having no "status" properties in the SoC base
>> > file (and hence everything defaults to enabled), with the per-board .dts
>> > files set 'status = "disabled"' where desired. I imagine U-Boot should
>> > follow the same practice.
>> >
>> > I forget the exact reason for this in the kernel; it may simply have been
>> > due to precedent on PowerPC. Grant Likely would know the details.
>>
>> Well it's going to create a bit of work for boards that use only a
>> subset of these 30-peripheral SOCs, but perhaps the expectation is
>> that you would be buying a simpler SOC if you didn't need all the
>> features.
>
> I imagine u-boot's methodology of loading drivers only as needed will
> prevent the unnecessary initialization of 30 drivers which are default
> 'status = "enabled";'. ?Or, am I missing something?

The glorious U-Boot driver refactor of 2015? :-) For now I don't know
how this will turn out, and I will change the commit message in the
next patch set.

Regards,
Simon

>
> thx,
>
> Jason.
>

[U-Boot] [PATCH v3 5/6] fdt: ARM: Add fdtcontroladdr to set device tree address in environment

[Mike Frysinger]

On Tuesday 11 October 2011 18:26:10 Simon Glass wrote:
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> 
> +/**
> + * Decode the value of an environment variable and return it.
> + *
> + * @param name		Name of environemnt variable
> + * @param base		Number base to use (normally 10, or 16 for hex)
> + * @param default_val	Default value to return if the variable is not
> + *			found
> + * @return the decoded value, or default_val if not found
> + */
> +static int getenv_int(const char *name, int base, int default_val)
> +{
> +	char tmp[64];	/* long enough for environment variables */
> +	int i = getenv_f(name, tmp, sizeof(tmp));
> +
> +	return (i > 0)
> +		? (int) simple_strtoul(tmp, NULL, base)
> +		: default_val;
> +}

pretty much everyone does this with gd->baudrate.  would make sense to put 
this into common/cmd_nvedit.c and convert all arches.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111013/6b35acb2/attachment.pgp 

[U-Boot] [PATCH v3 6/6] fdt: add decode helper library

[Mike Frysinger]

On Tuesday 11 October 2011 18:26:11 Simon Glass wrote:
> --- /dev/null
> +++ b/include/fdtdec.h
>
> +/*
> + * A typedef for a physical address. Note that fdt data is always big
> + * endian even on a litle endian machine.
> + */
> +#ifdef CONFIG_PHYS_64BIT
> +typedef u64 addr_t;
> +#define ADDR_T_NONE (-1ULL)
> +#define addr_to_cpu(reg) be64_to_cpu(reg)
> +#else
> +typedef u32 addr_t;
> +#define ADDR_T_NONE (-1U)
> +#define addr_to_cpu(reg) be32_to_cpu(reg)
> +#endif

"addr" is fairly generic.  how about "fdt_addr" instead ?

> --- /dev/null
> +++ b/lib/fdtdec.c
>
> +/* we need a generic GPIO interface here */
> +#include <asm/arch/gpio.h>

we have asm/gpio.h now, although i don't see this code using anything from the 
gpio header to need this include ...

> +static const char *compat_names[COMPAT_COUNT] = {

static const char * const compat_names[COMPAT_COUNT] = {

> +int fdtdec_next_alias(const void *blob, const char *name,
> +		enum fdt_compat_id id, int *upto)
> +{
> +#define MAX_STR_LEN 20
> +	char str[MAX_STR_LEN + 20];
> +	int node, err;
> +
> +	sprintf(str, "%.*s%d", MAX_STR_LEN, name, *upto);

where's that "20" coming from ?  just arbitrarily defined ?  might want to add 
an assert(strlen(name) <= MAX_STR_LEN).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111013/c6b28a7b/attachment.pgp 

[U-Boot] [PATCH v3 5/6] fdt: ARM: Add fdtcontroladdr to set device tree address in environment

[Simon Glass]

Hi Mike,

On Thu, Oct 13, 2011 at 1:27 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 11 October 2011 18:26:10 Simon Glass wrote:
>> --- a/arch/arm/lib/board.c
>> +++ b/arch/arm/lib/board.c
>>
>> +/**
>> + * Decode the value of an environment variable and return it.
>> + *
>> + * @param name ? ? ? ? ? ? ? Name of environemnt variable
>> + * @param base ? ? ? ? ? ? ? Number base to use (normally 10, or 16 for hex)
>> + * @param default_val ? ? ? ?Default value to return if the variable is not
>> + * ? ? ? ? ? ? ? ? ? found
>> + * @return the decoded value, or default_val if not found
>> + */
>> +static int getenv_int(const char *name, int base, int default_val)
>> +{
>> + ? ? char tmp[64]; ? /* long enough for environment variables */
>> + ? ? int i = getenv_f(name, tmp, sizeof(tmp));
>> +
>> + ? ? return (i > 0)
>> + ? ? ? ? ? ? ? (int) simple_strtoul(tmp, NULL, base)
>> + ? ? ? ? ? ? : default_val;
>> +}
>
> pretty much everyone does this with gd->baudrate. ?would make sense to put
> this into common/cmd_nvedit.c and convert all arches.
> -mike
>

I was worried someone might say that. I will take a look.

Regards,
Simon

[U-Boot] [PATCH v3 2/6] fdt: Add support for embedded device tree (CONFIG_OF_EMBED)

[Stephen Warren]

Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
> This new option allows U-Boot to embed a binary device tree into its image
> to allow run-time control of peripherals. This device tree is for U-Boot's
> own use and is not necessarily the same one as is passed to the kernel.
...
> diff --git a/dts/Makefile b/dts/Makefile
...
> +$(DT_BIN): $(TOPDIR)/board/$(VENDOR)/dts/$(DEVICE_TREE).dts
> +	cat $< | $(CPP) -P $(DTS_CPPFLAGS) - >$@.tmp

Couldn't that just be:

	$(CPP) -P $(DTS_CPPFLAGS) $< >$@.tmp

> +	$(DTC) -R 4 -p 0x1000 -O dtb -o ${DT_BIN} $@.tmp
> +	rm $@.tmp

-- 
nvpublic

[U-Boot] [PATCH v3 2/6] fdt: Add support for embedded device tree (CONFIG_OF_EMBED)

[Stephen Warren]

Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
> This new option allows U-Boot to embed a binary device tree into its image
> to allow run-time control of peripherals. This device tree is for U-Boot's
> own use and is not necessarily the same one as is passed to the kernel.
> 
> The device tree compiler output should be placed in the $(obj)
> rooted tree. Since $(OBJCOPY) insists on adding the path to the
> generated symbol names, to ensure consistency it should be
> invoked from the directory where the .dtb file is located and
> given the input file name without the path.
...
> +process_lds = \
> +	$(1) | sed -r -n 's/^OUTPUT_$(2)[ ("]*([^")]*).*/\1/p'
> +
> +# Run the compiler and get the link script from the linker
> +GET_LDS = $(CC) $(CFLAGS) $(LDFLAGS) -Wl,--verbose 2>&1
> +
> +$(obj)dt.o: $(DT_BIN)
> +	# We want the output format and arch.
> +	# We also hope to win a prize for ugliest Makefile / shell interaction
> +	# We look in the LDSCRIPT first.
> +	# Then try the linker which should give us the answer.
> +	# Then check it worked.
> +	oformat=`$(call process_lds,cat $(LDSCRIPT),FORMAT)` ;\
> +	oarch=`$(call process_lds,cat $(LDSCRIPT),ARCH)` ;\
> +	\
> +	[ -z $${oformat} ] && \
> +		oformat=`$(call process_lds,$(GET_LDS),FORMAT)` ;\
> +	[ -z $${oarch} ] && \
> +		oarch=`$(call process_lds,$(GET_LDS),ARCH)` ;\
> +	\
> +	[ -z $${oformat} ] && \
> +		echo "Cannot read OUTPUT_FORMAT from lds file $(LDSCRIPT)" && \
> +		exit 1 || true ;\
> +	[ -z $${oarch} ] && \
> +		echo "Cannot read OUTPUT_ARCH from lds file $(LDSCRIPT)" && \
> +		exit 1 || true ;\
> +	\
> +	cd $(dir ${DT_BIN}) && \
> +	$(OBJCOPY) -I binary -O $${oformat} -B $${oarch} \
> +		$(notdir ${DT_BIN}) $@
> +	rm $(DT_BIN)

Instead of all that, can't you just run a trivial script to generate a .c
file containing the data from DTB_BIN, and then use the compiler to compile
that, i.e. spit out something like:

const unsigned char dtb[] = {
  0xaa, 0x55, ......
};

That'd certainly drastically simplify the makefile, although waste a little
more time and temp disk space.

-- 
nvpublic

[U-Boot] [PATCH v3 2/6] fdt: Add support for embedded device tree (CONFIG_OF_EMBED)

[Simon Glass]

Hi Stephen,

On Thu, Oct 13, 2011 at 2:13 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
>> This new option allows U-Boot to embed a binary device tree into its image
>> to allow run-time control of peripherals. This device tree is for U-Boot's
>> own use and is not necessarily the same one as is passed to the kernel.
> ...
>> diff --git a/dts/Makefile b/dts/Makefile
> ...
>> +$(DT_BIN): $(TOPDIR)/board/$(VENDOR)/dts/$(DEVICE_TREE).dts
>> + ? ? cat $< | $(CPP) -P $(DTS_CPPFLAGS) - >$@.tmp
>
> Couldn't that just be:
>
> ? ? ? ?$(CPP) -P $(DTS_CPPFLAGS) $< >$@.tmp
>
>> + ? ? $(DTC) -R 4 -p 0x1000 -O dtb -o ${DT_BIN} $@.tmp
>> + ? ? rm $@.tmp

I did have that originally, but got this warning:

armv7a-cros-linux-gnueabi-gcc.real: warning:
/home/sjg/trunk/src/third_party/u-boot/files/board/nvidia/dts/seaboard.dts:
linker input file unused because linking not done

so I fell back to doing as I have done. I think it might be because
gcc normally assumes that anything it cannot understand must be a link
script.

Regards,
Simon

>
> --
> nvpublic
>
>

[U-Boot] [PATCH v3 2/6] fdt: Add support for embedded device tree (CONFIG_OF_EMBED)

[Simon Glass]

Hi Stephen,

On Thu, Oct 13, 2011 at 2:21 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
>> This new option allows U-Boot to embed a binary device tree into its image
>> to allow run-time control of peripherals. This device tree is for U-Boot's
>> own use and is not necessarily the same one as is passed to the kernel.
>>
>> The device tree compiler output should be placed in the $(obj)
>> rooted tree. Since $(OBJCOPY) insists on adding the path to the
>> generated symbol names, to ensure consistency it should be
>> invoked from the directory where the .dtb file is located and
>> given the input file name without the path.
> ...
>> +process_lds = \
>> + ? ? $(1) | sed -r -n 's/^OUTPUT_$(2)[ ("]*([^")]*).*/\1/p'
>> +
>> +# Run the compiler and get the link script from the linker
>> +GET_LDS = $(CC) $(CFLAGS) $(LDFLAGS) -Wl,--verbose 2>&1
>> +
>> +$(obj)dt.o: $(DT_BIN)
>> + ? ? # We want the output format and arch.
>> + ? ? # We also hope to win a prize for ugliest Makefile / shell interaction
>> + ? ? # We look in the LDSCRIPT first.
>> + ? ? # Then try the linker which should give us the answer.
>> + ? ? # Then check it worked.
>> + ? ? oformat=`$(call process_lds,cat $(LDSCRIPT),FORMAT)` ;\
>> + ? ? oarch=`$(call process_lds,cat $(LDSCRIPT),ARCH)` ;\
>> + ? ? \
>> + ? ? [ -z $${oformat} ] && \
>> + ? ? ? ? ? ? oformat=`$(call process_lds,$(GET_LDS),FORMAT)` ;\
>> + ? ? [ -z $${oarch} ] && \
>> + ? ? ? ? ? ? oarch=`$(call process_lds,$(GET_LDS),ARCH)` ;\
>> + ? ? \
>> + ? ? [ -z $${oformat} ] && \
>> + ? ? ? ? ? ? echo "Cannot read OUTPUT_FORMAT from lds file $(LDSCRIPT)" && \
>> + ? ? ? ? ? ? exit 1 || true ;\
>> + ? ? [ -z $${oarch} ] && \
>> + ? ? ? ? ? ? echo "Cannot read OUTPUT_ARCH from lds file $(LDSCRIPT)" && \
>> + ? ? ? ? ? ? exit 1 || true ;\
>> + ? ? \
>> + ? ? cd $(dir ${DT_BIN}) && \
>> + ? ? $(OBJCOPY) -I binary -O $${oformat} -B $${oarch} \
>> + ? ? ? ? ? ? $(notdir ${DT_BIN}) $@
>> + ? ? rm $(DT_BIN)
>
> Instead of all that, can't you just run a trivial script to generate a .c
> file containing the data from DTB_BIN, and then use the compiler to compile
> that, i.e. spit out something like:
>
> const unsigned char dtb[] = {
> ?0xaa, 0x55, ......
> };
>
> That'd certainly drastically simplify the makefile, although waste a little
> more time and temp disk space.

What, and withdraw my Makefile contest entry? :-)

I feel that objcopy is designed to do exactly this, and generating C
code is a roundabout way of producing an object file with data in it.
The difficulty of finding out the output format/architecture is
something we might clean up in U-Boot generally at some point (e.g.
figure it out as part of the original 'make ..._config') in which case
this would all go away.

Thoughts?

Regards,
Simon

>
> --
> nvpublic
>
>

[U-Boot] [PATCH v3 6/6] fdt: add decode helper library

[Simon Glass]

Hi Mike,

On Thu, Oct 13, 2011 at 1:33 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 11 October 2011 18:26:11 Simon Glass wrote:
>> --- /dev/null
>> +++ b/include/fdtdec.h
>>
>> +/*
>> + * A typedef for a physical address. Note that fdt data is always big
>> + * endian even on a litle endian machine.
>> + */
>> +#ifdef CONFIG_PHYS_64BIT
>> +typedef u64 addr_t;
>> +#define ADDR_T_NONE (-1ULL)
>> +#define addr_to_cpu(reg) be64_to_cpu(reg)
>> +#else
>> +typedef u32 addr_t;
>> +#define ADDR_T_NONE (-1U)
>> +#define addr_to_cpu(reg) be32_to_cpu(reg)
>> +#endif
>
> "addr" is fairly generic. ?how about "fdt_addr" instead ?

OK

>
>> --- /dev/null
>> +++ b/lib/fdtdec.c
>>
>> +/* we need a generic GPIO interface here */
>> +#include <asm/arch/gpio.h>
>
> we have asm/gpio.h now, although i don't see this code using anything from the
> gpio header to need this include ...

Will remove it.

>
>> +static const char *compat_names[COMPAT_COUNT] = {
>
> static const char * const compat_names[COMPAT_COUNT] = {
>
>> +int fdtdec_next_alias(const void *blob, const char *name,
>> + ? ? ? ? ? ? enum fdt_compat_id id, int *upto)
>> +{
>> +#define MAX_STR_LEN 20
>> + ? ? char str[MAX_STR_LEN + 20];
>> + ? ? int node, err;
>> +
>> + ? ? sprintf(str, "%.*s%d", MAX_STR_LEN, name, *upto);
>
> where's that "20" coming from ? ?just arbitrarily defined ? ?might want to add
> an assert(strlen(name) <= MAX_STR_LEN).
> -mike
>

OK. Of course I would like to use snprintf()...

Regards,
Simon

[U-Boot] [PATCH v3 3/6] fdt: Add support for a separate device tree (CONFIG_OF_SEPARATE)

[Stephen Warren]

Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
> This adds support for an FDT to be build as a separate binary file called
> u-boot.dtb. This can be concatenated with the U-Boot binary to provide a
> device tree located at run-time by U-Boot. The Makefile is modified to
> provide this file in u-boot-dtb.bin.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Modify Makefile to create combined U-boot/fdt (u-boot-dtb.bin)
> - Add note about CONFIG_ARCH_DEVICE_TREE
> 
>  .gitignore             |    1 +
>  Makefile               |    8 ++
>  README                 |   16 ++++-
>  doc/README.fdt-control |  172 ++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 195 insertions(+), 2 deletions(-)
>  create mode 100644 doc/README.fdt-control

Should the documentation be in the first patch; it seems a little random
to include it in one of the two somewhat equivalent patches that add
build support for a DTB transport mechanism.

-- 
nvpublic

[U-Boot] [PATCH v3 4/6] fdt: ARM: Implement embedded and separate device tree

[Stephen Warren]

Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
> This locates the device tree either embedded within U-Boot or attached to the
> end as a separate binary.
> 
> When CONFIG_OF_CONTROL is defined, U-Boot requires a valid fdt. A check is
> provided for this early in initialisation.

The subject of this patch seems a little misleading; something more like
"fdt: ARM: Check that a control fdt is present" would be more accurate.

Sorry for the trivial comments!

-- 
nvpublic

[U-Boot] [PATCH v3 2/6] fdt: Add support for embedded device tree (CONFIG_OF_EMBED)

[Stephen Warren]

Simon Glass wrote at Thursday, October 13, 2011 3:25 PM:
> Hi Stephen,
> 
> On Thu, Oct 13, 2011 at 2:21 PM, Stephen Warren <swarren@nvidia.com> wrote:
> > Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
> >> This new option allows U-Boot to embed a binary device tree into its image
> >> to allow run-time control of peripherals. This device tree is for U-Boot's
> >> own use and is not necessarily the same one as is passed to the kernel.
> >>
> >> The device tree compiler output should be placed in the $(obj)
> >> rooted tree. Since $(OBJCOPY) insists on adding the path to the
> >> generated symbol names, to ensure consistency it should be
> >> invoked from the directory where the .dtb file is located and
> >> given the input file name without the path.
> > ...
> >> +process_lds = \
> >> + ? ? $(1) | sed -r -n 's/^OUTPUT_$(2)[ ("]*([^")]*).*/\1/p'
> >> +
> >> +# Run the compiler and get the link script from the linker
> >> +GET_LDS = $(CC) $(CFLAGS) $(LDFLAGS) -Wl,--verbose 2>&1
> >> +
> >> +$(obj)dt.o: $(DT_BIN)
> >> + ? ? # We want the output format and arch.
> >> + ? ? # We also hope to win a prize for ugliest Makefile / shell interaction
> >> + ? ? # We look in the LDSCRIPT first.
> >> + ? ? # Then try the linker which should give us the answer.
> >> + ? ? # Then check it worked.
> >> + ? ? oformat=`$(call process_lds,cat $(LDSCRIPT),FORMAT)` ;\
> >> + ? ? oarch=`$(call process_lds,cat $(LDSCRIPT),ARCH)` ;\
> >> + ? ? \
> >> + ? ? [ -z $${oformat} ] && \
> >> + ? ? ? ? ? ? oformat=`$(call process_lds,$(GET_LDS),FORMAT)` ;\
> >> + ? ? [ -z $${oarch} ] && \
> >> + ? ? ? ? ? ? oarch=`$(call process_lds,$(GET_LDS),ARCH)` ;\
> >> + ? ? \
> >> + ? ? [ -z $${oformat} ] && \
> >> + ? ? ? ? ? ? echo "Cannot read OUTPUT_FORMAT from lds file $(LDSCRIPT)" && \
> >> + ? ? ? ? ? ? exit 1 || true ;\
> >> + ? ? [ -z $${oarch} ] && \
> >> + ? ? ? ? ? ? echo "Cannot read OUTPUT_ARCH from lds file $(LDSCRIPT)" && \
> >> + ? ? ? ? ? ? exit 1 || true ;\
> >> + ? ? \
> >> + ? ? cd $(dir ${DT_BIN}) && \
> >> + ? ? $(OBJCOPY) -I binary -O $${oformat} -B $${oarch} \
> >> + ? ? ? ? ? ? $(notdir ${DT_BIN}) $@
> >> + ? ? rm $(DT_BIN)
> >
> > Instead of all that, can't you just run a trivial script to generate a .c
> > file containing the data from DTB_BIN, and then use the compiler to compile
> > that, i.e. spit out something like:
> >
> > const unsigned char dtb[] = {
> > ?0xaa, 0x55, ......
> > };
> >
> > That'd certainly drastically simplify the makefile, although waste a little
> > more time and temp disk space.
> 
> What, and withdraw my Makefile contest entry? :-)

:-)

> I feel that objcopy is designed to do exactly this, and generating C
> code is a roundabout way of producing an object file with data in it.
> The difficulty of finding out the output format/architecture is
> something we might clean up in U-Boot generally at some point (e.g.
> figure it out as part of the original 'make ..._config') in which case
> this would all go away.
> 
> Thoughts?

Looking some more, dtc has option "-O asm" which writes directly to a text
file that can be assembled; there'd be no extra temp files or conversions
if you used that.

-- 
nvpublic

[U-Boot] [PATCH v3 2/6] fdt: Add support for embedded device tree (CONFIG_OF_EMBED)

[Mike Frysinger]

On Tuesday 11 October 2011 18:26:07 Simon Glass wrote:
> --- /dev/null
> +++ b/dts/Makefile
>
> +clean:
> +	rm -f $(OBJS) $(LIB)
> +	rm -f $(DT_BIN)
> +
> +distclean:	clean

i don't think this actually gets used, so punt it
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111013/ec529ae4/attachment.pgp 

[U-Boot] [PATCH v3 6/6] fdt: add decode helper library

[Mike Frysinger]

On Thursday 13 October 2011 17:28:10 Simon Glass wrote:
> On Thu, Oct 13, 2011 at 1:33 PM, Mike Frysinger wrote:
> > On Tuesday 11 October 2011 18:26:11 Simon Glass wrote:
> >> +int fdtdec_next_alias(const void *blob, const char *name,
> >> +             enum fdt_compat_id id, int *upto)
> >> +{
> >> +#define MAX_STR_LEN 20
> >> +     char str[MAX_STR_LEN + 20];
> >> +     int node, err;
> >> +
> >> +     sprintf(str, "%.*s%d", MAX_STR_LEN, name, *upto);
> > 
> > where's that "20" coming from ?  just arbitrarily defined ?  might want
> > to add an assert(strlen(name) <= MAX_STR_LEN).
> 
> OK. Of course I would like to use snprintf()...

i think that might be nicer anyways than %.*s

	char str[40];	/* an arbitrary len */
	snprintf(str, sizeof(str), "%s%d", name, *upto);
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111013/0f112de4/attachment.pgp 

[U-Boot] [PATCH v3 6/6] fdt: add decode helper library

[Simon Glass]

Hi Mike,

On Thu, Oct 13, 2011 at 4:01 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thursday 13 October 2011 17:28:10 Simon Glass wrote:
>> On Thu, Oct 13, 2011 at 1:33 PM, Mike Frysinger wrote:
>> > On Tuesday 11 October 2011 18:26:11 Simon Glass wrote:
>> >> +int fdtdec_next_alias(const void *blob, const char *name,
>> >> + ? ? ? ? ? ? enum fdt_compat_id id, int *upto)
>> >> +{
>> >> +#define MAX_STR_LEN 20
>> >> + ? ? char str[MAX_STR_LEN + 20];
>> >> + ? ? int node, err;
>> >> +
>> >> + ? ? sprintf(str, "%.*s%d", MAX_STR_LEN, name, *upto);
>> >
>> > where's that "20" coming from ? ?just arbitrarily defined ? ?might want
>> > to add an assert(strlen(name) <= MAX_STR_LEN).
>>
>> OK. Of course I would like to use snprintf()...
>
> i think that might be nicer anyways than %.*s
>
> ? ? ? ?char str[40]; ? /* an arbitrary len */
> ? ? ? ?snprintf(str, sizeof(str), "%s%d", name, *upto);
> -mike
>

Yes I agree, but we have the minor issue that snprintf() is not merged yet :-(

Regards,
Simon

[U-Boot] [PATCH v3 6/6] fdt: add decode helper library

[Mike Frysinger]

On Thursday 13 October 2011 19:16:52 Simon Glass wrote:
> On Thu, Oct 13, 2011 at 4:01 PM, Mike Frysinger wrote:
> >        char str[40];   /* an arbitrary len */
> >        snprintf(str, sizeof(str), "%s%d", name, *upto);
> 
> Yes I agree, but we have the minor issue that snprintf() is not merged yet

ah, good point.  obviously didn't realize :).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111013/334ba38b/attachment.pgp 

[U-Boot] [PATCH v3 6/6] fdt: add decode helper library

[Kumar Gala]

On Oct 11, 2011, at 5:26 PM, Simon Glass wrote:

> This library provides useful functions to drivers which want to use
> the fdt to control their operation. Functions are provided to:
> 
> - look up and enumerate a device type (for example assigning i2c bus 0,
> i2c bus 1, etc.)
> - decode basic types from the fdt, like addresses and integers
> 
> While this library is not strictly necessary, it helps to minimise the
> changes to a driver, in order to make it work under fdt control. Less
> code is required, and so the barrier to switch drivers over is lower.
> 
> Additional functions to read arrays and GPIOs could be made available
> here also.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Add example proposed decode helper library
> 
> Changes in v3:
> - Simplify decode library to remove provide only primitive functions
> - Remove i2c decode function
> - Rename fdt_decode to fdtdec, since it will be used a lot
> - Moved fdt_decode.c to /lib
> - Export almost all functions from fdtdec, to allow widespread use
> - Remove use of FDT_ERR_MISSING which is not strictly needed now
> 
> include/fdtdec.h |  122 ++++++++++++++++++++++++++++++++++++++++++++++++++
> lib/Makefile     |    1 +
> lib/fdtdec.c     |  131 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 254 insertions(+), 0 deletions(-)
> create mode 100644 include/fdtdec.h
> create mode 100644 lib/fdtdec.c

I think this is the wrong approach.  If we intend to go down the path of embedded a device tree we should look at produce a live tree structure that can be used like Linux has.

There are a lot of places we are manipulate device tree blobs that would be more efficient if we had a live tree structure in place.

I think that will only get worse with the addition of embedding a tree.

- k

[U-Boot] [PATCH v3 6/6] fdt: add decode helper library

[Simon Glass]

Hi Kumar,

On Fri, Oct 14, 2011 at 11:12 AM, Kumar Gala <galak@kernel.crashing.org> wrote:
>
> On Oct 11, 2011, at 5:26 PM, Simon Glass wrote:
>
>> This library provides useful functions to drivers which want to use
>> the fdt to control their operation. Functions are provided to:
>>
>> - look up and enumerate a device type (for example assigning i2c bus 0,
>> i2c bus 1, etc.)
>> - decode basic types from the fdt, like addresses and integers
>>
>> While this library is not strictly necessary, it helps to minimise the
>> changes to a driver, in order to make it work under fdt control. Less
>> code is required, and so the barrier to switch drivers over is lower.
>>
>> Additional functions to read arrays and GPIOs could be made available
>> here also.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>> Changes in v2:
>> - Add example proposed decode helper library
>>
>> Changes in v3:
>> - Simplify decode library to remove provide only primitive functions
>> - Remove i2c decode function
>> - Rename fdt_decode to fdtdec, since it will be used a lot
>> - Moved fdt_decode.c to /lib
>> - Export almost all functions from fdtdec, to allow widespread use
>> - Remove use of FDT_ERR_MISSING which is not strictly needed now
>>
>> include/fdtdec.h | ?122 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> lib/Makefile ? ? | ? ?1 +
>> lib/fdtdec.c ? ? | ?131 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 254 insertions(+), 0 deletions(-)
>> create mode 100644 include/fdtdec.h
>> create mode 100644 lib/fdtdec.c
>
> I think this is the wrong approach. ?If we intend to go down the path of embedded a device tree we should look at produce a live tree structure that can be used like Linux has.

The intent of this patch set is to provide U-Boot with a simple
run-time configuration system.

Can you please explain why you want a live tree for use by U-Boot?
U-Boot already has one that it passes on to Linux, and this is under
control of scripts, etc., quite separate from this.

This fdt is for configuration of devices used by U-Boot itself.

What can U-Boot find out that it would want to put into this private
fdt? Are you thinking of probing for devices early on and then using
them later in U-Boot? If so, surely no code can exist which does this
now, since this patch set is new.

>
> There are a lot of places we are manipulate device tree blobs that would be more efficient if we had a live tree structure in place.

Please can you point to these? Are you sure you are not referring to
the working_fdt? As I understand it this is already a live structure.

>
> I think that will only get worse with the addition of embedding a tree.

I need a bit more info / understanding before I can respond to this
email properly.

Regards,
Simon

>
> - k
>
>

[U-Boot] [PATCH v3 6/6] fdt: add decode helper library

[Aaron Williams]

Hi,

On Friday, October 14, 2011 11:21:54 AM Simon Glass wrote:
> Hi Kumar,
> 
> On Fri, Oct 14, 2011 at 11:12 AM, Kumar Gala <galak@kernel.crashing.org> 
wrote:
> > On Oct 11, 2011, at 5:26 PM, Simon Glass wrote:
> >> This library provides useful functions to drivers which want to use
> >> the fdt to control their operation. Functions are provided to:
> >> 
> >> - look up and enumerate a device type (for example assigning i2c bus 0,
> >> i2c bus 1, etc.)
> >> - decode basic types from the fdt, like addresses and integers
> >> 
> >> While this library is not strictly necessary, it helps to minimise the
> >> changes to a driver, in order to make it work under fdt control. Less
> >> code is required, and so the barrier to switch drivers over is lower.
> >> 
> >> Additional functions to read arrays and GPIOs could be made available
> >> here also.
> >> 
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >> ---
> >> Changes in v2:
> >> - Add example proposed decode helper library
> >> 
> >> Changes in v3:
> >> - Simplify decode library to remove provide only primitive functions
> >> - Remove i2c decode function
> >> - Rename fdt_decode to fdtdec, since it will be used a lot
> >> - Moved fdt_decode.c to /lib
> >> - Export almost all functions from fdtdec, to allow widespread use
> >> - Remove use of FDT_ERR_MISSING which is not strictly needed now
> >> 
> >> include/fdtdec.h |  122
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++ lib/Makefile     |  
> >>  1 +
> >> lib/fdtdec.c     |  131
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed,
> >> 254 insertions(+), 0 deletions(-)
> >> create mode 100644 include/fdtdec.h
> >> create mode 100644 lib/fdtdec.c
> > 
> > I think this is the wrong approach.  If we intend to go down the path of
> > embedded a device tree we should look at produce a live tree structure
> > that can be used like Linux has.
> 
> The intent of this patch set is to provide U-Boot with a simple
> run-time configuration system.
> 
> Can you please explain why you want a live tree for use by U-Boot?
> U-Boot already has one that it passes on to Linux, and this is under
> control of scripts, etc., quite separate from this.
> 
> This fdt is for configuration of devices used by U-Boot itself.
> 
> What can U-Boot find out that it would want to put into this private
> fdt? Are you thinking of probing for devices early on and then using
> them later in U-Boot? If so, surely no code can exist which does this
> now, since this patch set is new.
> 
> > There are a lot of places we are manipulate device tree blobs that would
> > be more efficient if we had a live tree structure in place.
> 
> Please can you point to these? Are you sure you are not referring to
> the working_fdt? As I understand it this is already a live structure.
> 
> > I think that will only get worse with the addition of embedding a tree.
> 
> I need a bit more info / understanding before I can respond to this
> email properly.
> 
> Regards,
> Simon
> 
> > - k
> 
This would actually be helpful for us. We have switched our bootloader to make 
use of the FDT and use it both within our common SOC code which is linked to 
our U-Boot image as well as what we pass to the Linux kernel.

We use a single FDT image for both.

We currently have our FDT for each board contain all possible options and then 
at runtime trim the parts that are not detected at runtime. Many of our boards 
have pluggable networking modules for example which are supported by our U-
Boot image.

-Aaron

-- 
Aaron Williams <Aaron.Williams@cavium.com>
 (408) 943-7198

[U-Boot] [PATCH v3 6/6] fdt: add decode helper library

[Simon Glass]

Hi Aaron,

On Fri, Oct 14, 2011 at 6:42 PM, Aaron Williams
<Aaron.Williams@cavium.com> wrote:
> Hi,
>
> On Friday, October 14, 2011 11:21:54 AM Simon Glass wrote:
>> Hi Kumar,
>>
>> On Fri, Oct 14, 2011 at 11:12 AM, Kumar Gala <galak@kernel.crashing.org>
> wrote:
>> > On Oct 11, 2011, at 5:26 PM, Simon Glass wrote:
>> >> This library provides useful functions to drivers which want to use
>> >> the fdt to control their operation. Functions are provided to:
>> >>
>> >> - look up and enumerate a device type (for example assigning i2c bus 0,
>> >> i2c bus 1, etc.)
>> >> - decode basic types from the fdt, like addresses and integers
>> >>
>> >> While this library is not strictly necessary, it helps to minimise the
>> >> changes to a driver, in order to make it work under fdt control. Less
>> >> code is required, and so the barrier to switch drivers over is lower.
>> >>
>> >> Additional functions to read arrays and GPIOs could be made available
>> >> here also.
>> >>
>> >> Signed-off-by: Simon Glass <sjg@chromium.org>
>> >> ---
>> >> Changes in v2:
>> >> - Add example proposed decode helper library
>> >>
>> >> Changes in v3:
>> >> - Simplify decode library to remove provide only primitive functions
>> >> - Remove i2c decode function
>> >> - Rename fdt_decode to fdtdec, since it will be used a lot
>> >> - Moved fdt_decode.c to /lib
>> >> - Export almost all functions from fdtdec, to allow widespread use
>> >> - Remove use of FDT_ERR_MISSING which is not strictly needed now
>> >>
>> >> include/fdtdec.h | ?122
>> >> ++++++++++++++++++++++++++++++++++++++++++++++++++ lib/Makefile ? ? |
>> >> ?1 +
>> >> lib/fdtdec.c ? ? | ?131
>> >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed,
>> >> 254 insertions(+), 0 deletions(-)
>> >> create mode 100644 include/fdtdec.h
>> >> create mode 100644 lib/fdtdec.c
>> >
>> > I think this is the wrong approach. ?If we intend to go down the path of
>> > embedded a device tree we should look at produce a live tree structure
>> > that can be used like Linux has.
>>
>> The intent of this patch set is to provide U-Boot with a simple
>> run-time configuration system.
>>
>> Can you please explain why you want a live tree for use by U-Boot?
>> U-Boot already has one that it passes on to Linux, and this is under
>> control of scripts, etc., quite separate from this.
>>
>> This fdt is for configuration of devices used by U-Boot itself.
>>
>> What can U-Boot find out that it would want to put into this private
>> fdt? Are you thinking of probing for devices early on and then using
>> them later in U-Boot? If so, surely no code can exist which does this
>> now, since this patch set is new.
>>
>> > There are a lot of places we are manipulate device tree blobs that would
>> > be more efficient if we had a live tree structure in place.
>>
>> Please can you point to these? Are you sure you are not referring to
>> the working_fdt? As I understand it this is already a live structure.
>>
>> > I think that will only get worse with the addition of embedding a tree.
>>
>> I need a bit more info / understanding before I can respond to this
>> email properly.
>>
>> Regards,
>> Simon
>>
>> > - k
>>
> This would actually be helpful for us. We have switched our bootloader to make
> use of the FDT and use it both within our common SOC code which is linked to
> our U-Boot image as well as what we pass to the Linux kernel.

Do you like the fdt with U-Boot or find it in flash somewhere?

>
> We use a single FDT image for both.

This is easy enough with the current patch set in that the build
system can provide the same fdt to both U-Boot and the kernel. It
would also be possible to make a copy of the U-Boot one (through some
new 'fdt' command), update it and pass it to the kernel. It might be a
little early to do that though - at least on the ARM side many fdt
things in the kernel are still in flux and there might be a desire to
limit the U-Boot changes (dealing with changed fdt definitions for
peripherals, etc.) forced by changes in the kernel.

>
> We currently have our FDT for each board contain all possible options and then
> at runtime trim the parts that are not detected at runtime. Many of our boards
> have pluggable networking modules for example which are supported by our U-
> Boot image.

Sounds great. On ARM SOCs there is often not much interest in probing
since it either isn't possible or isn't useful (since things are known
at build time anyway). So far my patch set does not deal with a
dynamic fdt - it is just a basic implementation to get started. I'm
sure it can expand as need dictates.

Regards,
Simon

>
> -Aaron
>
> --
> Aaron Williams <Aaron.Williams@cavium.com>
> ?(408) 943-7198
>

[U-Boot] [PATCH v3 2/6] fdt: Add support for embedded device tree (CONFIG_OF_EMBED)

[Simon Glass]

Hi Stephen,

On Thu, Oct 13, 2011 at 2:50 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Simon Glass wrote at Thursday, October 13, 2011 3:25 PM:
>> Hi Stephen,
>>
>> On Thu, Oct 13, 2011 at 2:21 PM, Stephen Warren <swarren@nvidia.com> wrote:
>> > Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
>> >> This new option allows U-Boot to embed a binary device tree into its image
>> >> to allow run-time control of peripherals. This device tree is for U-Boot's
>> >> own use and is not necessarily the same one as is passed to the kernel.
>> >>
>> >> The device tree compiler output should be placed in the $(obj)
>> >> rooted tree. Since $(OBJCOPY) insists on adding the path to the
>> >> generated symbol names, to ensure consistency it should be
>> >> invoked from the directory where the .dtb file is located and
>> >> given the input file name without the path.
>> > ...
>> >> +process_lds = \
>> >> + ? ? $(1) | sed -r -n 's/^OUTPUT_$(2)[ ("]*([^")]*).*/\1/p'
>> >> +
>> >> +# Run the compiler and get the link script from the linker
>> >> +GET_LDS = $(CC) $(CFLAGS) $(LDFLAGS) -Wl,--verbose 2>&1
>> >> +
>> >> +$(obj)dt.o: $(DT_BIN)
>> >> + ? ? # We want the output format and arch.
>> >> + ? ? # We also hope to win a prize for ugliest Makefile / shell interaction
>> >> + ? ? # We look in the LDSCRIPT first.
>> >> + ? ? # Then try the linker which should give us the answer.
>> >> + ? ? # Then check it worked.
>> >> + ? ? oformat=`$(call process_lds,cat $(LDSCRIPT),FORMAT)` ;\
>> >> + ? ? oarch=`$(call process_lds,cat $(LDSCRIPT),ARCH)` ;\
>> >> + ? ? \
>> >> + ? ? [ -z $${oformat} ] && \
>> >> + ? ? ? ? ? ? oformat=`$(call process_lds,$(GET_LDS),FORMAT)` ;\
>> >> + ? ? [ -z $${oarch} ] && \
>> >> + ? ? ? ? ? ? oarch=`$(call process_lds,$(GET_LDS),ARCH)` ;\
>> >> + ? ? \
>> >> + ? ? [ -z $${oformat} ] && \
>> >> + ? ? ? ? ? ? echo "Cannot read OUTPUT_FORMAT from lds file $(LDSCRIPT)" && \
>> >> + ? ? ? ? ? ? exit 1 || true ;\
>> >> + ? ? [ -z $${oarch} ] && \
>> >> + ? ? ? ? ? ? echo "Cannot read OUTPUT_ARCH from lds file $(LDSCRIPT)" && \
>> >> + ? ? ? ? ? ? exit 1 || true ;\
>> >> + ? ? \
>> >> + ? ? cd $(dir ${DT_BIN}) && \
>> >> + ? ? $(OBJCOPY) -I binary -O $${oformat} -B $${oarch} \
>> >> + ? ? ? ? ? ? $(notdir ${DT_BIN}) $@
>> >> + ? ? rm $(DT_BIN)
>> >
>> > Instead of all that, can't you just run a trivial script to generate a .c
>> > file containing the data from DTB_BIN, and then use the compiler to compile
>> > that, i.e. spit out something like:
>> >
>> > const unsigned char dtb[] = {
>> > ?0xaa, 0x55, ......
>> > };
>> >
>> > That'd certainly drastically simplify the makefile, although waste a little
>> > more time and temp disk space.
>>
>> What, and withdraw my Makefile contest entry? :-)
>
> :-)
>
>> I feel that objcopy is designed to do exactly this, and generating C
>> code is a roundabout way of producing an object file with data in it.
>> The difficulty of finding out the output format/architecture is
>> something we might clean up in U-Boot generally at some point (e.g.
>> figure it out as part of the original 'make ..._config') in which case
>> this would all go away.
>>
>> Thoughts?
>
> Looking some more, dtc has option "-O asm" which writes directly to a text
> file that can be assembled; there'd be no extra temp files or conversions
> if you used that.

Yes! That's much better - I will use that. I think I should still
create the temp files just so that people can see what went wrong with
a failed build. For example, if there is an error in the device tree
file it is nice to see the exact file that was compiled, including
preprocessing.

But it gets rid of the architecture stuff. I do feel that objcopy is
there partly to support putting binary bobs into object files, but the
reality is that it is just too ugly.

Thanks very much for looking into this and for the suggestion.

Regards,
Simon

>
> --
> nvpublic
>
>

[U-Boot] [PATCH v3 2/6] fdt: Add support for embedded device tree (CONFIG_OF_EMBED)

[Grant Likely]

On Thu, Oct 13, 2011 at 3:50 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Simon Glass wrote at Thursday, October 13, 2011 3:25 PM:
>> Hi Stephen,
>>
>> On Thu, Oct 13, 2011 at 2:21 PM, Stephen Warren <swarren@nvidia.com> wrote:
>> > Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
>> >> This new option allows U-Boot to embed a binary device tree into its image
>> >> to allow run-time control of peripherals. This device tree is for U-Boot's
>> >> own use and is not necessarily the same one as is passed to the kernel.
>> >>
>> >> The device tree compiler output should be placed in the $(obj)
>> >> rooted tree. Since $(OBJCOPY) insists on adding the path to the
>> >> generated symbol names, to ensure consistency it should be
>> >> invoked from the directory where the .dtb file is located and
>> >> given the input file name without the path.
>> > ...
>> >> +process_lds = \
>> >> + ? ? $(1) | sed -r -n 's/^OUTPUT_$(2)[ ("]*([^")]*).*/\1/p'
>> >> +
>> >> +# Run the compiler and get the link script from the linker
>> >> +GET_LDS = $(CC) $(CFLAGS) $(LDFLAGS) -Wl,--verbose 2>&1
>> >> +
>> >> +$(obj)dt.o: $(DT_BIN)
>> >> + ? ? # We want the output format and arch.
>> >> + ? ? # We also hope to win a prize for ugliest Makefile / shell interaction
>> >> + ? ? # We look in the LDSCRIPT first.
>> >> + ? ? # Then try the linker which should give us the answer.
>> >> + ? ? # Then check it worked.
>> >> + ? ? oformat=`$(call process_lds,cat $(LDSCRIPT),FORMAT)` ;\
>> >> + ? ? oarch=`$(call process_lds,cat $(LDSCRIPT),ARCH)` ;\
>> >> + ? ? \
>> >> + ? ? [ -z $${oformat} ] && \
>> >> + ? ? ? ? ? ? oformat=`$(call process_lds,$(GET_LDS),FORMAT)` ;\
>> >> + ? ? [ -z $${oarch} ] && \
>> >> + ? ? ? ? ? ? oarch=`$(call process_lds,$(GET_LDS),ARCH)` ;\
>> >> + ? ? \
>> >> + ? ? [ -z $${oformat} ] && \
>> >> + ? ? ? ? ? ? echo "Cannot read OUTPUT_FORMAT from lds file $(LDSCRIPT)" && \
>> >> + ? ? ? ? ? ? exit 1 || true ;\
>> >> + ? ? [ -z $${oarch} ] && \
>> >> + ? ? ? ? ? ? echo "Cannot read OUTPUT_ARCH from lds file $(LDSCRIPT)" && \
>> >> + ? ? ? ? ? ? exit 1 || true ;\
>> >> + ? ? \
>> >> + ? ? cd $(dir ${DT_BIN}) && \
>> >> + ? ? $(OBJCOPY) -I binary -O $${oformat} -B $${oarch} \
>> >> + ? ? ? ? ? ? $(notdir ${DT_BIN}) $@
>> >> + ? ? rm $(DT_BIN)
>> >
>> > Instead of all that, can't you just run a trivial script to generate a .c
>> > file containing the data from DTB_BIN, and then use the compiler to compile
>> > that, i.e. spit out something like:
>> >
>> > const unsigned char dtb[] = {
>> > ?0xaa, 0x55, ......
>> > };
>> >
>> > That'd certainly drastically simplify the makefile, although waste a little
>> > more time and temp disk space.
>>
>> What, and withdraw my Makefile contest entry? :-)
>
> :-)
>
>> I feel that objcopy is designed to do exactly this, and generating C
>> code is a roundabout way of producing an object file with data in it.
>> The difficulty of finding out the output format/architecture is
>> something we might clean up in U-Boot generally at some point (e.g.
>> figure it out as part of the original 'make ..._config') in which case
>> this would all go away.
>>
>> Thoughts?
>
> Looking some more, dtc has option "-O asm" which writes directly to a text
> file that can be assembled; there'd be no extra temp files or conversions
> if you used that.

I recommend *not* using the asm output option.  It's not nearly as
well tested and it is likely to have some big-endian-isms in it that
don't work well.  I prefer the objcopy approach myself.  That's what
it is for.

g.

[U-Boot] [PATCH v3 2/6] fdt: Add support for embedded device tree (CONFIG_OF_EMBED)

[Simon Glass]

Hi Grant,

On Fri, Oct 14, 2011 at 10:46 PM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> On Thu, Oct 13, 2011 at 3:50 PM, Stephen Warren <swarren@nvidia.com> wrote:
>> Simon Glass wrote at Thursday, October 13, 2011 3:25 PM:
>>> Hi Stephen,
>>>
>>> On Thu, Oct 13, 2011 at 2:21 PM, Stephen Warren <swarren@nvidia.com> wrote:
>>> > Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
>>> >> This new option allows U-Boot to embed a binary device tree into its image
>>> >> to allow run-time control of peripherals. This device tree is for U-Boot's
>>> >> own use and is not necessarily the same one as is passed to the kernel.
>>> >>
>>> >> The device tree compiler output should be placed in the $(obj)
>>> >> rooted tree. Since $(OBJCOPY) insists on adding the path to the
>>> >> generated symbol names, to ensure consistency it should be
>>> >> invoked from the directory where the .dtb file is located and
>>> >> given the input file name without the path.
>>> > ...
>>> >> +process_lds = \
>>> >> + ? ? $(1) | sed -r -n 's/^OUTPUT_$(2)[ ("]*([^")]*).*/\1/p'
>>> >> +
>>> >> +# Run the compiler and get the link script from the linker
>>> >> +GET_LDS = $(CC) $(CFLAGS) $(LDFLAGS) -Wl,--verbose 2>&1
>>> >> +
>>> >> +$(obj)dt.o: $(DT_BIN)
>>> >> + ? ? # We want the output format and arch.
>>> >> + ? ? # We also hope to win a prize for ugliest Makefile / shell interaction
>>> >> + ? ? # We look in the LDSCRIPT first.
>>> >> + ? ? # Then try the linker which should give us the answer.
>>> >> + ? ? # Then check it worked.
>>> >> + ? ? oformat=`$(call process_lds,cat $(LDSCRIPT),FORMAT)` ;\
>>> >> + ? ? oarch=`$(call process_lds,cat $(LDSCRIPT),ARCH)` ;\
>>> >> + ? ? \
>>> >> + ? ? [ -z $${oformat} ] && \
>>> >> + ? ? ? ? ? ? oformat=`$(call process_lds,$(GET_LDS),FORMAT)` ;\
>>> >> + ? ? [ -z $${oarch} ] && \
>>> >> + ? ? ? ? ? ? oarch=`$(call process_lds,$(GET_LDS),ARCH)` ;\
>>> >> + ? ? \
>>> >> + ? ? [ -z $${oformat} ] && \
>>> >> + ? ? ? ? ? ? echo "Cannot read OUTPUT_FORMAT from lds file $(LDSCRIPT)" && \
>>> >> + ? ? ? ? ? ? exit 1 || true ;\
>>> >> + ? ? [ -z $${oarch} ] && \
>>> >> + ? ? ? ? ? ? echo "Cannot read OUTPUT_ARCH from lds file $(LDSCRIPT)" && \
>>> >> + ? ? ? ? ? ? exit 1 || true ;\
>>> >> + ? ? \
>>> >> + ? ? cd $(dir ${DT_BIN}) && \
>>> >> + ? ? $(OBJCOPY) -I binary -O $${oformat} -B $${oarch} \
>>> >> + ? ? ? ? ? ? $(notdir ${DT_BIN}) $@
>>> >> + ? ? rm $(DT_BIN)
>>> >
>>> > Instead of all that, can't you just run a trivial script to generate a .c
>>> > file containing the data from DTB_BIN, and then use the compiler to compile
>>> > that, i.e. spit out something like:
>>> >
>>> > const unsigned char dtb[] = {
>>> > ?0xaa, 0x55, ......
>>> > };
>>> >
>>> > That'd certainly drastically simplify the makefile, although waste a little
>>> > more time and temp disk space.
>>>
>>> What, and withdraw my Makefile contest entry? :-)
>>
>> :-)
>>
>>> I feel that objcopy is designed to do exactly this, and generating C
>>> code is a roundabout way of producing an object file with data in it.
>>> The difficulty of finding out the output format/architecture is
>>> something we might clean up in U-Boot generally at some point (e.g.
>>> figure it out as part of the original 'make ..._config') in which case
>>> this would all go away.
>>>
>>> Thoughts?
>>
>> Looking some more, dtc has option "-O asm" which writes directly to a text
>> file that can be assembled; there'd be no extra temp files or conversions
>> if you used that.
>
> I recommend *not* using the asm output option. ?It's not nearly as
> well tested and it is likely to have some big-endian-isms in it that
> don't work well. ?I prefer the objcopy approach myself. ?That's what
> it is for.

Oh dear that sounds bad. The difficultly is in figuring out the
arguments to objcopy (architecture and oformat). But we do need it to
work reliably, so I will move back to the original approach for now.

Regards,
Simon

>
> g.
>

[U-Boot] [PATCH v3 3/6] fdt: Add support for a separate device tree (CONFIG_OF_SEPARATE)

[Simon Glass]

Hi Stephen,

On Thu, Oct 13, 2011 at 2:36 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
>> This adds support for an FDT to be build as a separate binary file called
>> u-boot.dtb. This can be concatenated with the U-Boot binary to provide a
>> device tree located at run-time by U-Boot. The Makefile is modified to
>> provide this file in u-boot-dtb.bin.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>> Changes in v2:
>> - Modify Makefile to create combined U-boot/fdt (u-boot-dtb.bin)
>> - Add note about CONFIG_ARCH_DEVICE_TREE
>>
>> ?.gitignore ? ? ? ? ? ? | ? ?1 +
>> ?Makefile ? ? ? ? ? ? ? | ? ?8 ++
>> ?README ? ? ? ? ? ? ? ? | ? 16 ++++-
>> ?doc/README.fdt-control | ?172 ++++++++++++++++++++++++++++++++++++++++++++++++
>> ?4 files changed, 195 insertions(+), 2 deletions(-)
>> ?create mode 100644 doc/README.fdt-control
>
> Should the documentation be in the first patch; it seems a little random
> to include it in one of the two somewhat equivalent patches that add
> build support for a DTB transport mechanism.

Unfortunately the first patch is ARM-specific so I probably should not
put it there (maybe this is just a minor issue, I'm not sure). I have
not done patches to enable fdt in all architectures so far and I'm not
sure this is wanted / required. I will move the README up one patch to
the first non-ARM-specific patch.

Regards,
Simon

>
> --
> nvpublic
>
>

[U-Boot] [PATCH v3 4/6] fdt: ARM: Implement embedded and separate device tree

[Simon Glass]

Hi Stephen,

On Thu, Oct 13, 2011 at 2:39 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Simon Glass wrote at Tuesday, October 11, 2011 4:26 PM:
>> This locates the device tree either embedded within U-Boot or attached to the
>> end as a separate binary.
>>
>> When CONFIG_OF_CONTROL is defined, U-Boot requires a valid fdt. A check is
>> provided for this early in initialisation.
>
> The subject of this patch seems a little misleading; something more like
> "fdt: ARM: Check that a control fdt is present" would be more accurate.

It does also implement it. Before this, there is no code to find the
device tree on ARM. But yes it also checks that it gets something
valid. I will change the name to something like 'fdt: ARM: Implement
and verify embedded and separate device tree'

>
> Sorry for the trivial comments!

Thanks for looking at it.

Regards,
Simon

>
> --
> nvpublic
>
>

[U-Boot] [PATCH v3 5/6] fdt: ARM: Add fdtcontroladdr to set device tree address in environment

[Simon Glass]

Hi Mike,

On Thu, Oct 13, 2011 at 2:08 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Mike,
>
> On Thu, Oct 13, 2011 at 1:27 PM, Mike Frysinger <vapier@gentoo.org> wrote:
>> On Tuesday 11 October 2011 18:26:10 Simon Glass wrote:
>>> --- a/arch/arm/lib/board.c
>>> +++ b/arch/arm/lib/board.c
>>>
>>> +/**
>>> + * Decode the value of an environment variable and return it.
>>> + *
>>> + * @param name ? ? ? ? ? ? ? Name of environemnt variable
>>> + * @param base ? ? ? ? ? ? ? Number base to use (normally 10, or 16 for hex)
>>> + * @param default_val ? ? ? ?Default value to return if the variable is not
>>> + * ? ? ? ? ? ? ? ? ? found
>>> + * @return the decoded value, or default_val if not found
>>> + */
>>> +static int getenv_int(const char *name, int base, int default_val)
>>> +{
>>> + ? ? char tmp[64]; ? /* long enough for environment variables */
>>> + ? ? int i = getenv_f(name, tmp, sizeof(tmp));
>>> +
>>> + ? ? return (i > 0)
>>> + ? ? ? ? ? ? ? (int) simple_strtoul(tmp, NULL, base)
>>> + ? ? ? ? ? ? : default_val;
>>> +}
>>
>> pretty much everyone does this with gd->baudrate. ?would make sense to put
>> this into common/cmd_nvedit.c and convert all arches.
>> -mike
>>
>
> I was worried someone might say that. I will take a look.

That patch series has been posted, so if it is accepted then we can
remove the definition of this function from the patch series. I am
going to send the fdt series against upstream/master but will happily
adjust this particular patch if getenv_ulong() is accepted.

Regards,
Simon