[U-Boot] [PATCH v3 0/6] Run-time configuration of U-Boot via a flat device tree (fdt)
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
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)
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_CONTROLIf this variable is defined, U-Boot will use a device treeto configure its devices, instead of relying on staticallycompiled #defines in the board file. This option isexperimental and only available on a few boards. The devicetree is available in the global data as gd->blob.
gd->fdt_blob
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);
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
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
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.
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
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.
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
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.
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.
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
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
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.
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
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) {
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!
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
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 -----------
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
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
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
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; +}
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
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
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
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
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
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
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
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
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
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:-)
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
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.
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.
participants (7)
-
Aaron Williams -
Grant Likely -
Jason -
Kumar Gala -
Mike Frysinger -
Simon Glass -
Stephen Warren