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

[U-Boot] [RFC PATCH v2 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 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.

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)
 #define CONFIG_DEFAULT_DEVICE_TREE	"<your name>"
 (to specify the name of the device tree file is
  board/<vendor>/<board>/<your name>.dts)

This patch set does not include any drivers which actually use the fdt. I have
some concerns about spreading fdt code around the U-Boot code base so am
thinking of having a support file which makes this easier. I can provide a
UART driver modified to use fdt if there is interest.

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 of i2c driver changes required to support fdt control
- Add example fdt fragments for Tegra2 I2C, just for illustration
- Add example proposed decode helper library

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: add decode helper library
  fdt: example modification of i2c driver for fdt control

 .gitignore                                |    1 +
 Makefile                                  |    9 ++
 README                                    |   25 ++++
 arch/arm/include/asm/global_data.h        |    1 +
 arch/arm/lib/board.c                      |   22 ++++
 board/nvidia/seaboard/tegra2-seaboard.dts |   20 ++++
 board/nvidia/seaboard/tegra250.dtsi       |   42 +++++++
 common/Makefile                           |    1 +
 common/fdt_decode.c                       |  177 +++++++++++++++++++++++++++++
 config.mk                                 |    1 +
 doc/README.fdt-control                    |  168 +++++++++++++++++++++++++++
 drivers/i2c/tegra2_i2c.c                  |   91 +++++++++++++++
 dts/Makefile                              |  100 ++++++++++++++++
 include/common.h                          |    1 +
 include/fdt_decode.h                      |   90 +++++++++++++++
 include/libfdt.h                          |    5 +-
 16 files changed, 753 insertions(+), 1 deletions(-)
 create mode 100644 board/nvidia/seaboard/tegra2-seaboard.dts
 create mode 100644 board/nvidia/seaboard/tegra250.dtsi
 create mode 100644 common/fdt_decode.c
 create mode 100644 doc/README.fdt-control
 create mode 100644 dts/Makefile
 create mode 100644 include/fdt_decode.h

-- 
1.7.3.1

[U-Boot] [RFC PATCH v2 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>
---
 README                             |   11 +++++++++++
 arch/arm/include/asm/global_data.h |    1 +
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/README b/README
index 0886987..812805f 100644
--- a/README
+++ b/README
@@ -795,6 +795,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 4fc51fd..818eced 100644
--- a/arch/arm/include/asm/global_data.h
+++ b/arch/arm/include/asm/global_data.h
@@ -73,6 +73,7 @@ typedef	struct	global_data {
 #if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
 	unsigned long	tlb_addr;
 #endif
+	const void	*blob;		/* Our device tree, NULL if none */
 	void		**jt;		/* jump table */
 	char		env_buf[32];	/* buffer for getenv() before reloc. */
 } gd_t;
-- 
1.7.3.1

[U-Boot] [RFC PATCH v2 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>
---
 Makefile         |    4 ++
 README           |   11 +++++-
 config.mk        |    1 +
 dts/Makefile     |  100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/common.h |    1 +
 5 files changed, 115 insertions(+), 2 deletions(-)
 create mode 100644 dts/Makefile

diff --git a/Makefile b/Makefile
index d5a1f0a..658a622 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 812805f..5a2f060 100644
--- a/README
+++ b/README
@@ -803,8 +803,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..e70a16b
--- /dev/null
+++ b/dts/Makefile
@@ -0,0 +1,100 @@
+#
+# 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))
+
+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/$(BOARDDIR)/$(DEVICE_TREE).dts
+	$(DTC) -R 4 -p 0x1000 -O dtb -o ${DT_BIN} $<
+
+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 bd10f31..6cdcc50 100644
--- a/include/common.h
+++ b/include/common.h
@@ -249,6 +249,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] [RFC PATCH v2 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.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 .gitignore             |    1 +
 Makefile               |    5 ++
 README                 |   11 +++-
 doc/README.fdt-control |  168 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 183 insertions(+), 2 deletions(-)
 create mode 100644 doc/README.fdt-control

diff --git a/.gitignore b/.gitignore
index dbf545f..c4ebd34 100644
--- a/.gitignore
+++ b/.gitignore
@@ -34,6 +34,7 @@
 /u-boot.dis
 /u-boot.lds
 /u-boot.ubl
+/u-boot.dtb
 
 #
 # Generated files
diff --git a/Makefile b/Makefile
index 658a622..d73efa1 100644
--- a/Makefile
+++ b/Makefile
@@ -352,9 +352,14 @@ 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
 
 all:		$(ALL-y)
 
+$(obj)u-boot.dtb:	$(obj)u-boot
+		$(MAKE) -C dts binary
+		mv $(obj)dts/dt.dtb $@
+
 $(obj)u-boot.hex:	$(obj)u-boot
 		$(OBJCOPY) ${OBJCFLAGS} -O ihex $< $@
 
diff --git a/README b/README
index 5a2f060..0b8f338 100644
--- a/README
+++ b/README
@@ -803,8 +803,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
@@ -813,6 +813,13 @@ 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
+
 - 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..dfc8f06
--- /dev/null
+++ b/doc/README.fdt-control
@@ -0,0 +1,168 @@
+#
+# 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>/<board>/<name>.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] [RFC PATCH v2 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>
---
 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 a7fb251..6a02001 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>
 
 #ifdef CONFIG_BITBANGMII
 #include <miiphy.h>
@@ -195,6 +196,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->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...
  *
@@ -237,6 +249,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,
@@ -274,6 +289,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->blob = _binary_dt_dtb_start;
+#elif defined CONFIG_OF_SEPARATE
+	/* FDT is at end of image */
+	gd->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] [RFC PATCH v2 5/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
- decode common information for each device class (e.g. i2c has speed,
 controller address, etc.)

While this library is not strictly necessary, it serves two purposes:

1. It allows us 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.

2. It takes advantage of the fact that (just as with CONFIG), each device
class tends to have similar config requirements whether it be Tegra I2C
or Atmel I2C, for example. Additional config can still be provided in the
fdt, if the driver decodes it itself.

Note: We also add FDT_ERR_MISSING in this commit, to record missing
properties. This is a small change to libfdt, which can be dealt with
separately. This error is useful in the case when the correct node exists
but the properties within it do not match our expectations.

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

 common/Makefile      |    1 +
 common/fdt_decode.c  |  177 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/fdt_decode.h |   90 +++++++++++++++++++++++++
 include/libfdt.h     |    5 +-
 4 files changed, 272 insertions(+), 1 deletions(-)
 create mode 100644 common/fdt_decode.c
 create mode 100644 include/fdt_decode.h

diff --git a/common/Makefile b/common/Makefile
index 2edbd71..760073c 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -173,6 +173,7 @@ COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o
 COBJS-$(CONFIG_MODEM_SUPPORT) += modem.o
 COBJS-$(CONFIG_UPDATE_TFTP) += update.o
 COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o
+COBJS-$(CONFIG_OF_CONTROL) += fdt_decode.o
 endif
 
 COBJS-y += console.o
diff --git a/common/fdt_decode.c b/common/fdt_decode.c
new file mode 100644
index 0000000..9e8cf4d
--- /dev/null
+++ b/common/fdt_decode.c
@@ -0,0 +1,177 @@
+/*
+ * 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 <fdt_decode.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] = {
+	COMPAT(NVIDIA_TEGRA250_I2C, "nvidia,tegra250-i2c"),
+};
+
+/**
+ * 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);
+}
+
+/**
+ * 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
+ */
+static addr_t 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;
+}
+
+/**
+ * Look up a 32-bit integer property in a node and return it. The property
+ * must have@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
+ */
+static s32 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;
+}
+
+/**
+ * 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
+ */
+static int 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 fdt_decode_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 fdt_decode_next_compatible(const void *blob, int node,
+		enum fdt_compat_id id)
+{
+	return fdt_node_offset_by_compatible(blob, node, compat_names[id]);
+}
+
+int fdt_decode_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_MISSING : node;
+}
+
+int fdt_decode_i2c(const void *blob, int node, struct fdt_i2c *config)
+{
+	config->reg = (struct i2c_ctlr *)get_addr(blob, node, "reg");
+	config->pinmux = get_int(blob, node, "pinmux", 0);
+	config->speed = get_int(blob, node, "speed", 0);
+	config->periph_id = get_int(blob, node, "periph-id", -1);
+	config->enabled = get_is_enabled(blob, node, 0);
+
+	if (config->periph_id == -1)
+		return -FDT_ERR_MISSING;
+
+	return 0;
+}
diff --git a/include/fdt_decode.h b/include/fdt_decode.h
new file mode 100644
index 0000000..bdcdbba
--- /dev/null
+++ b/include/fdt_decode.h
@@ -0,0 +1,90 @@
+/*
+ * 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 <ns16550.h>
+#include <asm/arch/clock.h>
+
+/* A typedef for a physical address. We should move it to a generic place */
+#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_NVIDIA_TEGRA250_I2C,	/* Tegra 250 i2c */
+
+	COMPAT_COUNT,
+};
+
+/* Information about i2c controller */
+struct fdt_i2c {
+	struct i2c_ctlr *reg;		/* Address of controller registers */
+	int pinmux;			/* Which pin mux setting to use */
+	u32 speed;			/* Speed in KHz */
+	enum periph_id periph_id;	/* Peripheral ID for clock/pinmux */
+	int enabled;			/* 1 if enabled, 0 if disabled */
+};
+
+/**
+ * Returns information from the FDT about an i2c controler. This function reads
+ * out the following attributes:
+ *
+ *	reg
+ *      enabled
+ *	pinmux
+ *	speed
+ *	periph-id
+ *
+ * @param blob		FDT blob to use
+ * @param node		Node to read from
+ * @param config	structure to use to return information
+ * @returns 0 on success, -ve on error, in which case config may or may not be
+ *			unchanged. If the node is present but expected data is
+ *			missing then this will generally return
+ *			-FDT_ERR_MISSING.
+ */
+int fdt_decode_i2c(const void *blob, int node, struct fdt_i2c *config);
diff --git a/include/libfdt.h b/include/libfdt.h
index de82ed5..1c7ac84 100644
--- a/include/libfdt.h
+++ b/include/libfdt.h
@@ -116,7 +116,10 @@
 	 * Should never be returned, if it is, it indicates a bug in
 	 * libfdt itself. */
 
-#define FDT_ERR_MAX		13
+/* Expected data is missing/incomplete while decoding a node */
+#define FDT_ERR_MISSING		14
+
+#define FDT_ERR_MAX		14
 
 /**********************************************************************/
 /* Low-level functions (you probably don't need these)                */
-- 
1.7.3.1

[U-Boot] [RFC PATCH v2 6/6] fdt: example modification of i2c driver for fdt control

[Simon Glass]

This is only an example for comment. It is not a real driver as yet. It
just shows how the config is read from the fdt.

You can see that the main difference (as you might expect) is whether
configuration comes from the fdt or the CONFIG options.

Some drivers will need changing to split their config out in such a clear
way, or to add a structure to hold the config rather than using CONFIG_...
options throughout their code.

This also provides an example fdt fragment to show what the device is
actually reading.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Add example of i2c driver changes required to support fdt control
- Add example fdt fragments for Tegra2 I2C, just for illustration

 board/nvidia/seaboard/tegra2-seaboard.dts |   20 ++++++
 board/nvidia/seaboard/tegra250.dtsi       |   42 +++++++++++++
 drivers/i2c/tegra2_i2c.c                  |   91 +++++++++++++++++++++++++++++
 3 files changed, 153 insertions(+), 0 deletions(-)
 create mode 100644 board/nvidia/seaboard/tegra2-seaboard.dts
 create mode 100644 board/nvidia/seaboard/tegra250.dtsi

diff --git a/board/nvidia/seaboard/tegra2-seaboard.dts b/board/nvidia/seaboard/tegra2-seaboard.dts
new file mode 100644
index 0000000..2c17139
--- /dev/null
+++ b/board/nvidia/seaboard/tegra2-seaboard.dts
@@ -0,0 +1,20 @@
+/dts-v1/;
+
+/memreserve/ 0x1c000000 0x04000000;
+/include/ "tegra250.dtsi"
+
+/ {
+	model = "NVIDIA Seaboard";
+	compatible = "nvidia,seaboard", "nvidia,tegra250";
+
+	/* Enable the ports we want to use, and update the speed if needed */
+	i2c at 0x7000c000 {
+		status = "ok";
+	};
+
+	i2c at 0x7000c400 {
+		speed = <400000>;
+		status = "ok";
+	};
+
+};
diff --git a/board/nvidia/seaboard/tegra250.dtsi b/board/nvidia/seaboard/tegra250.dtsi
new file mode 100644
index 0000000..d6dc19b
--- /dev/null
+++ b/board/nvidia/seaboard/tegra250.dtsi
@@ -0,0 +1,42 @@
+/* This is not a real fdt! It is just for illustration in this RFC */
+
+/ {
+	model = "NVIDIA Tegra 250";
+	compatible = "nvidia,tegra250";
+
+	i2c at 0x7000c000 {
+		compatible = "nvidia,tegra250-i2c";
+		reg = <0x7000c000 0x006c>;
+		pinmux = <1>;
+		speed = <100000>;
+		periph-id = <12>;	// PERIPH_ID_I2C1
+		status = "disabled";
+	};
+
+	i2c at 0x7000c400 {
+		compatible = "nvidia,tegra250-i2c";
+		reg = <0x7000c400 0x006c>;
+		pinmux = <2>;
+		speed = <100000>;
+		periph-id = <54>;	// PERIPH_ID_I2C2
+		status = "disabled";
+	};
+
+	i2c at 0x7000c500 {
+		compatible = "nvidia,tegra250-i2c";
+		reg = <0x7000c500 0x006c>;
+		pinmux = <1>;
+		speed = <100000>;
+		periph-id = <67>;	// PERIPH_ID_I2C3
+		status = "disabled";
+	};
+
+	i2c at 0x7000d000 {
+		compatible = "nvidia,tegra250-i2c";
+		reg = <0x7000d000 0x007c>;
+		pinmux = <1>;
+		speed = <100000>;
+		periph-id = <47>;	// PERIPH_ID_DVC_I2C
+		status = "disabled";
+	};
+};
diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c
index d98519c..9485b56 100644
--- a/drivers/i2c/tegra2_i2c.c
+++ b/drivers/i2c/tegra2_i2c.c
@@ -388,6 +388,97 @@ static int tegra2_i2c_read_data(u32 addr, u8 *data, u32 len)
 	return error;
 }
 
+#ifndef CONFIG_OF_CONTROL
+/* This is the original code, whic uses CONFIG_..., TEGRA2_...,etc. */
+static const enum periph_id i2c_periph_ids[CONFIG_SYS_MAX_I2C_BUS] = {
+	PERIPH_ID_DVC_I2C,
+	PERIPH_ID_I2C1,
+	PERIPH_ID_I2C2,
+	PERIPH_ID_I2C3
+};
+
+static const u32 *i2c_bus_base[CONFIG_SYS_MAX_I2C_BUS] = {
+	(u32 *)TEGRA2_DVC_BASE,
+	(u32 *)TEGRA2_I2C1_BASE,
+	(u32 *)TEGRA2_I2C2_BASE,
+	(u32 *)TEGRA2_I2C3_BASE
+};
+
+/* pinmux_configs based on the pinmux configuration */
+static const int pinmux_configs[CONFIG_SYS_MAX_I2C_BUS] = {
+	CONFIG_I2CP_PIN_MUX,	/* for I2CP (DVC I2C) */
+	CONFIG_I2C1_PIN_MUX,	/* for I2C1 */
+	CONFIG_I2C2_PIN_MUX,	/* for I2C2 */
+	CONFIG_I2C3_PIN_MUX	/* for I2C3 */
+};
+
+static int i2c_get_config(int *index, struct i2c_bus *i2c_bus)
+{
+	int i = *index;
+
+	if (i >= CONFIG_SYS_MAX_I2C_BUS)
+		return -1;
+
+	i2c_bus->periph_id = i2c_periph_ids[i];
+	i2c_bus->pinmux_config = pinmux_configs[i];
+	i2c_bus->regs = (struct i2c_ctlr *)i2c_bus_base[i];
+	i2c_bus->speed = I2CSPEED_KHZ * 1000;
+
+	*index = i + 1;
+
+	return 0;
+}
+#else
+/* This is the fdt code */
+static int i2c_get_config(int *index, struct i2c_bus *i2c_bus)
+{
+	const void *blob = gd->blob;
+	struct fdt_i2c config;
+	int node = fdt_decode_next_alias(blob,
+					 "i2c",
+					 COMPAT_NVIDIA_TEGRA250_I2C,
+					 index);
+	if (node < 0)
+		return -1;
+
+	if (fdt_decode_i2c(blob, node, &config))
+		return -1;
+
+	i2c_bus->periph_id = config.periph_id;
+	i2c_bus->pinmux_config = config.pinmux;
+	i2c_bus->regs = config.reg;
+	i2c_bus->speed = config.speed;
+	i2c_bus->enabled = config.enable;
+
+	return 0;
+}
+#endif
+
+int i2c_init_board(void)
+{
+	struct i2c_bus *i2c_bus;
+	int index = 0;
+	int i;
+
+	/* build the i2c_controllers[] for each controller */
+	for (i = 0; i < CONFIG_SYS_MAX_I2C_BUS; ++i) {
+		i2c_bus = &i2c_controllers[i];
+		i2c_bus->id = i;
+
+		i2c_get_config(&index, i2c_bus);
+
+		if (i2c_bus->periph_id == PERIPH_ID_DVC_I2C)
+			i2c_bus->control =
+				&((struct dvc_ctlr *)i2c_bus->regs)->control;
+		else
+			i2c_bus->control = &i2c_bus->regs->control;
+
+		i2c_init_controller(i2c_bus);
+	}
+
+	return 0;
+}
+
 void i2c_init(int speed, int slaveaddr)
 {
 	debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
-- 
1.7.3.1

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

[Jason]

On Mon, Sep 12, 2011 at 03:04:23PM -0700, Simon Glass wrote:
> This device tree is for U-Boot's own use and is not necessarily the
> same one as is passed to the kernel.

Are there plans to keepup with being able to use a kernel generated fdt?
eg effectively ignoring things we don't care about like sound cards and
wifi?

I can see definite advantages for manufacturers to be able to roll one
kernel and one bootloader across a product line, then one fdt per model.
It almost makes this whole mess make sense. :-)  The kirkwood SoC would
be a good example.

Do you have a git tree up that I could base off of?  git.chromium.org's
version of u-boot doesn't seem to have your code...

thx,

Jason.

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

[Simon Glass]

Hi Jason,

On Mon, Sep 12, 2011 at 4:37 PM, Jason <u-boot@lakedaemon.net> wrote:
> On Mon, Sep 12, 2011 at 03:04:23PM -0700, Simon Glass wrote:
>> This device tree is for U-Boot's own use and is not necessarily the
>> same one as is passed to the kernel.
>
> Are there plans to keepup with being able to use a kernel generated fdt?
> eg effectively ignoring things we don't care about like sound cards and
> wifi?

I would like to use the kernel fdt unmodified, so yes. Unused things
in the fdt bloat U-Boot but shouldn't otherwise matter. It is also
possible to create a tool to filter them out.

>
> I can see definite advantages for manufacturers to be able to roll one
> kernel and one bootloader across a product line, then one fdt per model.
> It almost makes this whole mess make sense. :-) ?The kirkwood SoC would
> be a good example.

Yes that's the intent. You still need CONFIGs for enabling each
feature (i.e. bringing in the code), but configuring it can be done in
common with the kernel.

>
> Do you have a git tree up that I could base off of? ?git.chromium.org's
> version of u-boot doesn't seem to have your code...

You probably need to look at a branch:

http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=shortlog;h=refs/heads/chromeos-v2011.06

This is based on U-Boot 2011.06.

Regards,
Simon

>
> thx,
>
> Jason.
>

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

[Marek Vasut]

On Tuesday, September 13, 2011 12:04:22 AM Simon Glass wrote:
> 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>
> ---
>  README                             |   11 +++++++++++
>  arch/arm/include/asm/global_data.h |    1 +
>  2 files changed, 12 insertions(+), 0 deletions(-)
> 
Hi,

do you actually intend to introduce some kind of a driver model to uboot ?

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

[Simon Glass]

Hi Merek,

On Mon, Sep 12, 2011 at 8:10 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> On Tuesday, September 13, 2011 12:04:22 AM Simon Glass wrote:
>> 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>
>> ---
>> ?README ? ? ? ? ? ? ? ? ? ? ? ? ? ? | ? 11 +++++++++++
>> ?arch/arm/include/asm/global_data.h | ? ?1 +
>> ?2 files changed, 12 insertions(+), 0 deletions(-)
>>
> Hi,
>
> do you actually intend to introduce some kind of a driver model to uboot ?
>

I would love to, yes. To some extent there is a bit of this already,
at least for specific subsystems. Clearly the fdt would work better if
we could just hand U-Boot the fdt and say 'init yourself'. It would
then scan the tree and init all the drivers for all active devices.

However, we can achieve most of the aims using something along the
lines of what I have proposed, where the existing call (say to
nand_init()) can look up the fdt for its node, and then get the
information it needs. The only really difference is the explicit
hard-coded call to nand_init, rather than a general purpose routine to
find a nand node and then locate a driver for it.

To some extent that way of doing things would invert the way U-Boot
currently works. It would also introduce questions about dealing with
multiple devices of the same type (e.g. two different i2c controllers
(not just instances) or driving two displays. These sorts of things
are tricky in U-Boot at the moment.

So overall I think a unified driver model is a separate problem, and
one that we should discuss and perhaps move forward on separately.

Regards.
Simon

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

[Marek Vasut]

On Tuesday, September 13, 2011 06:52:34 AM Simon Glass wrote:
> Hi Merek,
> 
> On Mon, Sep 12, 2011 at 8:10 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > On Tuesday, September 13, 2011 12:04:22 AM Simon Glass wrote:
> >> 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>
> >> ---
> >>  README                             |   11 +++++++++++
> >>  arch/arm/include/asm/global_data.h |    1 +
> >>  2 files changed, 12 insertions(+), 0 deletions(-)
> > 
> > Hi,
> > 
> > do you actually intend to introduce some kind of a driver model to uboot
> > ?
> 
> I would love to, yes. To some extent there is a bit of this already,
> at least for specific subsystems. Clearly the fdt would work better if
> we could just hand U-Boot the fdt and say 'init yourself'. It would
> then scan the tree and init all the drivers for all active devices.
> 
> However, we can achieve most of the aims using something along the
> lines of what I have proposed, where the existing call (say to
> nand_init()) can look up the fdt for its node, and then get the
> information it needs. The only really difference is the explicit
> hard-coded call to nand_init, rather than a general purpose routine to
> find a nand node and then locate a driver for it.
> 
> To some extent that way of doing things would invert the way U-Boot
> currently works. It would also introduce questions about dealing with
> multiple devices of the same type (e.g. two different i2c controllers
> (not just instances) or driving two displays. These sorts of things
> are tricky in U-Boot at the moment.
> 
> So overall I think a unified driver model is a separate problem, and
> one that we should discuss and perhaps move forward on separately.

Well, I have this kind of stuff in mind and I plan to try pushing it as a 
university project in a month or so.

But (!) if you plan to init U-Boot according to FDT and I plan to add driver 
model, we should keep in tight contact so the driver model would be close to the 
FDT.

And yea -- dealing with the "dirty work" like fixing subsystems etc. would be 
part of the driver model stuff.

Cheers
> 
> Regards.
> Simon

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

[Wolfgang Denk]

Dear Simon Glass,

In message <CAPnjgZ3kZJoAGJFkPkxQp+-jnztYECUaEtLZ0nvgzV1f-xUQgQ@mail.gmail.com> you wrote:
> 
> I would love to, yes. To some extent there is a bit of this already,
> at least for specific subsystems. Clearly the fdt would work better if
> we could just hand U-Boot the fdt and say 'init yourself'. It would
> then scan the tree and init all the drivers for all active devices.

No, it would definitely not do that.

U-Boot shall not initialize all possible available devices, but always
only those that it needs itself to perform it's task, which usually is
just to load and start an OS.  It makes no sense to initialize all
network interfaces (and eventually wait for the link to come up), to
initialize all attached disk drives (and wait for them to spin up) or
to scan the whole USB bus and initialize all attached devices when we
don't need any of these to boot the OS.

Suchinitializations shall always be done on demand only, i. e. when a
command is run that accesses any such device.

> However, we can achieve most of the aims using something along the
> lines of what I have proposed, where the existing call (say to
> nand_init()) can look up the fdt for its node, and then get the
> information it needs. The only really difference is the explicit
> hard-coded call to nand_init, rather than a general purpose routine to
> find a nand node and then locate a driver for it.

I can;t parse that.  Why cannot nand_init() do exactly what you
suggest, i. e. find a nand node and then locate a driver for it?
OK, by then it will probably be something like driver_init("nand") or
the like, but that's just a detail.

> To some extent that way of doing things would invert the way U-Boot
> currently works. It would also introduce questions about dealing with
> multiple devices of the same type (e.g. two different i2c controllers
> (not just instances) or driving two displays. These sorts of things
> are tricky in U-Boot at the moment.

Keep in mind that devices are always accessedonly on demand, so you
will have the needed information which device needs to be opened.

> So overall I think a unified driver model is a separate problem, and
> one that we should discuss and perhaps move forward on separately.

Agreed.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If you want strict real-time behavior, run in the real  time  schedu-
ling class.  But there are no seatbelts or airbags;  main(){for(;;);}
can hard hang your system.                          -- Bart Smaalders

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

[Simon Glass]

Hi Wolfgang,

On Tue, Sep 13, 2011 at 2:47 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <CAPnjgZ3kZJoAGJFkPkxQp+-jnztYECUaEtLZ0nvgzV1f-xUQgQ@mail.gmail.com> you wrote:
>>
>> I would love to, yes. To some extent there is a bit of this already,
>> at least for specific subsystems. Clearly the fdt would work better if
>> we could just hand U-Boot the fdt and say 'init yourself'. It would
>> then scan the tree and init all the drivers for all active devices.
>
> No, it would definitely not do that.
>
> U-Boot shall not initialize all possible available devices, but always
> only those that it needs itself to perform it's task, which usually is
> just to load and start an OS. ?It makes no sense to initialize all
> network interfaces (and eventually wait for the link to come up), to
> initialize all attached disk drives (and wait for them to spin up) or
> to scan the whole USB bus and initialize all attached devices when we
> don't need any of these to boot the OS.
>
> Suchinitializations shall always be done on demand only, i. e. when a
> command is run that accesses any such device.

Yes thanks for pointing that out. I am really thinking of the init
sequence in board_init_r() where we init NAND, MMC and the like. So
yes it makes no sense to blindly init everything we can find. The fdt
can provide a list of available options for each device type (although
in practice often it will provide only one option, perhaps with
multiple instances).

>
>> However, we can achieve most of the aims using something along the
>> lines of what I have proposed, where the existing call (say to
>> nand_init()) can look up the fdt for its node, and then get the
>> information it needs. The only really difference is the explicit
>> hard-coded call to nand_init, rather than a general purpose routine to
>> find a nand node and then locate a driver for it.
>
> I can;t parse that. ?Why cannot nand_init() do exactly what you
> suggest, i. e. find a nand node and then locate a driver for it?
> OK, by then it will probably be something like driver_init("nand") or
> the like, but that's just a detail.

That's certainly the current approach, yes. It does work well I think,
and has the virtue of being simple, with minimal changes required to
code.

>
>> To some extent that way of doing things would invert the way U-Boot
>> currently works. It would also introduce questions about dealing with
>> multiple devices of the same type (e.g. two different i2c controllers
>> (not just instances) or driving two displays. These sorts of things
>> are tricky in U-Boot at the moment.
>
> Keep in mind that devices are always accessedonly on demand, so you
> will have the needed information which device needs to be opened.

Yes, that's true and we mustn't do anything which blurs or weakens
that link. When I think of multiple LCD support for example, while
there is hardware support for it, do we have a need for it? Sometimes
this sort of thing is a solution looking for a problem. Yes it would
be easier to implement this feature with a unified driver model,
but...

On the other hand I think serial could benefit from a unified driver
model quite nicely :-)

>
>> So overall I think a unified driver model is a separate problem, and
>> one that we should discuss and perhaps move forward on separately.
>
> Agreed.

OK good. There seem to be a lot of different activities going on, as
Graeme says in the other thread.

Regards,
Simon

>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> If you want strict real-time behavior, run in the real ?time ?schedu-
> ling class. ?But there are no seatbelts or airbags; ?main(){for(;;);}
> can hard hang your system. ? ? ? ? ? ? ? ? ? ? ? ? ?-- Bart Smaalders
>

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

[Wolfgang Denk]

Dear Simon,

In message <CAPnjgZ1y+i0FdawcbBuOi0+SC+Xq3AS=ZZtTi1tVjv8CcNw6Og@mail.gmail.com> you wrote:
> 
> > U-Boot shall not initialize all possible available devices, but always
> > only those that it needs itself to perform it's task, which usually is
> > just to load and start an OS. =A0It makes no sense to initialize all
> > network interfaces (and eventually wait for the link to come up), to
> > initialize all attached disk drives (and wait for them to spin up) or
> > to scan the whole USB bus and initialize all attached devices when we
> > don't need any of these to boot the OS.
> >
> > Suchinitializations shall always be done on demand only, i. e. when a
> > command is run that accesses any such device.
> 
> Yes thanks for pointing that out. I am really thinking of the init
> sequence in board_init_r() where we init NAND, MMC and the like. So

I'm thinking of that, too.  This list initializes a lot of things it
actually should not.  When this code got written, we had only a few
boards, an all init steps took just a few milliseconds, so it appeared
a good idea to print a "complete" system status at boot - always.

Now we don't consider this approach practical any more, especially
when targetting for short boot times.

When introducing a device model, one of the tasks will be to change
this part, too.

> On the other hand I think serial could benefit from a unified driver
> model quite nicely :-)

Agreed, and not only serial.

> OK good. There seem to be a lot of different activities going on, as
> Graeme says in the other thread.

Indeed.  I'm actually amazed how many different things suddenly start
moving :-)

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There are no data that cannot be plotted on a straight  line  if  the
axis are chosen correctly.

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

[Simon Glass]

Hi Wolfgang,

On Tue, Sep 13, 2011 at 4:57 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon,
>
> In message <CAPnjgZ1y+i0FdawcbBuOi0+SC+Xq3AS=ZZtTi1tVjv8CcNw6Og@mail.gmail.com> you wrote:
>>
>> > U-Boot shall not initialize all possible available devices, but always
>> > only those that it needs itself to perform it's task, which usually is
>> > just to load and start an OS. =A0It makes no sense to initialize all
>> > network interfaces (and eventually wait for the link to come up), to
>> > initialize all attached disk drives (and wait for them to spin up) or
>> > to scan the whole USB bus and initialize all attached devices when we
>> > don't need any of these to boot the OS.
>> >
>> > Suchinitializations shall always be done on demand only, i. e. when a
>> > command is run that accesses any such device.
>>
>> Yes thanks for pointing that out. I am really thinking of the init
>> sequence in board_init_r() where we init NAND, MMC and the like. So
>
> I'm thinking of that, too. ?This list initializes a lot of things it
> actually should not. ?When this code got written, we had only a few
> boards, an all init steps took just a few milliseconds, so it appeared
> a good idea to print a "complete" system status at boot - always.
>
> Now we don't consider this approach practical any more, especially
> when targetting for short boot times.

Oh OK, well that's actually quite good then. For one thing it means
that the idea of init order is perhaps not so critical, since we
actually want to remove most of this mandatory init. I mean that if we
are not trying to replicate the init sequence in board_init_r (but in
fact remove most of it) then things are simpler, and perhaps even fall
back to a call to the board code and not much else.

>
> When introducing a device model, one of the tasks will be to change
> this part, too.

Well it seems like it might fit quite nicely. For example cmd_usb can
make a call to get a USB driver pointer/context, which can do
start-of-day init if not already done. There are some complexities
with shared clock/pinmux config, but it shouldn't be too bad. It might
mean that U-Boot takes back control of firing off vendor init code,
currently done at start of day in board_init and the like.

>
>> On the other hand I think serial could benefit from a unified driver
>> model quite nicely :-)
>
> Agreed, and not only serial.

I admit I haven't been through every subsystem...

>
>> OK good. There seem to be a lot of different activities going on, as
>> Graeme says in the other thread.
>
> Indeed. ?I'm actually amazed how many different things suddenly start
> moving :-)

Yes, it is certainly moving. Boot time optimisation, code
clean-up/rationalisation and run-time config should provide plenty of
impetus.

Regards,
Simon

>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> There are no data that cannot be plotted on a straight ?line ?if ?the
> axis are chosen correctly.
>

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

[Wolfgang Denk]

Dear Simon Glass,

In message <CAPnjgZ0FvaM-dM0Af_ZiAT9EKho+zeT4HF_Vx9AqSXALp0pVhA@mail.gmail.com> you wrote:
> 
> Oh OK, well that's actually quite good then. For one thing it means
> that the idea of init order is perhaps not so critical, since we
> actually want to remove most of this mandatory init. I mean that if we
> are not trying to replicate the init sequence in board_init_r (but in
> fact remove most of it) then things are simpler, and perhaps even fall
> back to a call to the board code and not much else.

I think there are at least two parts of it.  One is driver init stuff.
Here it should indeed be sufficient when each command that tries to
open a driver will trigger the initialization of the device if thios
has not been done before (this sounds simple enough, but I see some
complexity because some of these things _will_ be done before
relocation, i. e. without BSS and without writable data segment, so we
only havce the space-limited gd ...).

The other part is board specific dependencies that are not that easy
to detect.

I consider it a pity that Detlev's suggestion went by uncommented:

http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/105824/focus=106328

The "init a driver when it's needed" is some automatic part of such
dependency base intialization.  Eventually we should follow that
method for the remaining parts, too.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A meeting is an event at which the minutes are kept and the hours are
lost.

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

[Jason]

On Mon, Sep 12, 2011 at 05:12:37PM -0700, Simon Glass wrote:
> On Mon, Sep 12, 2011 at 4:37 PM, Jason <u-boot@lakedaemon.net> wrote:
> > Do you have a git tree up that I could base off of? ?git.chromium.org's
> > version of u-boot doesn't seem to have your code...
> 
> You probably need to look at a branch:
> 
> http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=shortlog;h=refs/heads/chromeos-v2011.06
> 
> This is based on U-Boot 2011.06.

Added the remote, thanks.  Once we figure out the mach-types thing, I'll
try using the Marvell integrated RTC driver as a real-world example of a
conversion to fdt.  I'll probably have questions ;-)

thx,

Jason.

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

[Mike Frysinger]

On Monday, September 12, 2011 18:04:22 Simon Glass wrote:
> --- a/arch/arm/include/asm/global_data.h
> +++ b/arch/arm/include/asm/global_data.h
>
> +	const void	*blob;		/* Our device tree, NULL if none */

still "blob" and not "fdt_blob" ?  one man's blob is another man's blub ;)
-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/20110913/92efef47/attachment.pgp 

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

[Simon Glass]

Hi Mike,

On Tue, Sep 13, 2011 at 11:16 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Monday, September 12, 2011 18:04:22 Simon Glass wrote:
>> --- a/arch/arm/include/asm/global_data.h
>> +++ b/arch/arm/include/asm/global_data.h
>>
>> + ? ? const void ? ? ?*blob; ? ? ? ? ?/* Our device tree, NULL if none */
>
> still "blob" and not "fdt_blob" ? ?one man's blob is another man's blub ;)
> -mike
>

Sorry I did say I would change this - will do so before I send out a
real (not RFC) patch.

Regards,
Simon

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

[Simon Glass]

Hi,

On Mon, Sep 12, 2011 at 3:04 PM, Simon Glass <sjg@chromium.org> wrote:
> At present in U-Boot configuration is mostly done using CONFIG options in the
> board file. This patch set 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.
>
[snip]
> ?arch/arm/include/asm/global_data.h ? ? ? ?| ? ?1 +
> ?arch/arm/lib/board.c ? ? ? ? ? ? ? ? ? ? ?| ? 22 ++++

Is anyone interested in trying this out with PowerPC or another architecture?

I don't have the ability to test anything other than ARM, but it would
nice to include support fdt-based run-time config for at least a few
architectures. I am hoping that the changes to the two files above can
be replicated and everything will 'just work', but it is far from
certain.

Regards,
Simon

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

[Simon Glass]

Hi Jason,

On Tue, Sep 13, 2011 at 7:37 AM, Jason <u-boot@lakedaemon.net> wrote:
> On Mon, Sep 12, 2011 at 05:12:37PM -0700, Simon Glass wrote:
>> On Mon, Sep 12, 2011 at 4:37 PM, Jason <u-boot@lakedaemon.net> wrote:
>> > Do you have a git tree up that I could base off of? ?git.chromium.org's
>> > version of u-boot doesn't seem to have your code...
>>
>> You probably need to look at a branch:
>>
>> http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=shortlog;h=refs/heads/chromeos-v2011.06
>>
>> This is based on U-Boot 2011.06.
>
> Added the remote, thanks. ?Once we figure out the mach-types thing, I'll
> try using the Marvell integrated RTC driver as a real-world example of a
> conversion to fdt. ?I'll probably have questions ;-)

That sounds great. I've tried to make it fair straightforward - with
an option for building an fdt into U-Boot itself (i.e. don't define
CONFIG_OF_SEPARATE). This should allow you to use your current dev
flow.

Regards.
Simon

>
> thx,
>
> Jason.
>

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

[Jason]

Simon,

On Tue, Sep 13, 2011 at 02:06:56PM -0700, Simon Glass wrote:
> Hi Jason,
> 
> On Tue, Sep 13, 2011 at 7:37 AM, Jason <u-boot@lakedaemon.net> wrote:
> > On Mon, Sep 12, 2011 at 05:12:37PM -0700, Simon Glass wrote:
> >> On Mon, Sep 12, 2011 at 4:37 PM, Jason <u-boot@lakedaemon.net> wrote:
> >> > Do you have a git tree up that I could base off of? ?git.chromium.org's
> >> > version of u-boot doesn't seem to have your code...
> >>
> >> You probably need to look at a branch:
> >>
> >> http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=shortlog;h=refs/heads/chromeos-v2011.06
> >>
> >> This is based on U-Boot 2011.06.
> >
> > Added the remote, thanks. ?Once we figure out the mach-types thing, I'll
> > try using the Marvell integrated RTC driver as a real-world example of a
> > conversion to fdt. ?I'll probably have questions ;-)
> 
> That sounds great. I've tried to make it fair straightforward - with
> an option for building an fdt into U-Boot itself (i.e. don't define
> CONFIG_OF_SEPARATE). This should allow you to use your current dev
> flow.

Okay, I have an initial version that compiles, but doesn't work.  Which,
surprisingly, is progress.  ;-)  I'll post the patch a little bit later
after I make sure I'm not missing something dumb.

I'll also post some comments to your patch series in a moment.

thx,

Jason.

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

[Simon Glass]

Hi Jason,

On Wed, Sep 14, 2011 at 6:47 AM, Jason <u-boot@lakedaemon.net> wrote:
> Simon,
>
> On Tue, Sep 13, 2011 at 02:06:56PM -0700, Simon Glass wrote:
>> Hi Jason,
>>
>> On Tue, Sep 13, 2011 at 7:37 AM, Jason <u-boot@lakedaemon.net> wrote:
>> > On Mon, Sep 12, 2011 at 05:12:37PM -0700, Simon Glass wrote:
>> >> On Mon, Sep 12, 2011 at 4:37 PM, Jason <u-boot@lakedaemon.net> wrote:
>> >> > Do you have a git tree up that I could base off of? ?git.chromium.org's
>> >> > version of u-boot doesn't seem to have your code...
>> >>
>> >> You probably need to look at a branch:
>> >>
>> >> http://git.chromium.org/gitweb/?p=chromiumos/third_party/u-boot.git;a=shortlog;h=refs/heads/chromeos-v2011.06
>> >>
>> >> This is based on U-Boot 2011.06.
>> >
>> > Added the remote, thanks. ?Once we figure out the mach-types thing, I'll
>> > try using the Marvell integrated RTC driver as a real-world example of a
>> > conversion to fdt. ?I'll probably have questions ;-)
>>
>> That sounds great. I've tried to make it fair straightforward - with
>> an option for building an fdt into U-Boot itself (i.e. don't define
>> CONFIG_OF_SEPARATE). This should allow you to use your current dev
>> flow.
>
> Okay, I have an initial version that compiles, but doesn't work. ?Which,
> surprisingly, is progress. ?;-) ?I'll post the patch a little bit later
> after I make sure I'm not missing something dumb.

A few hints:

- define CONFIG_OF_EMBED to start with, since it will embed the fdt
inside U-Boot which is a good check that all is well. You can move to
the more useful CONFIG_OF_SEPARATE when you get that working
- there is a check in board.c that the fdt is accessible - if it is
dying early then it might be that (early board panic patch is still in
flight)

>
> I'll also post some comments to your patch series in a moment.

OK good.

Regards,
Simon

>
> thx,
>
> Jason.
>

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

[Jason]

Simon,

On Wed, Sep 14, 2011 at 08:47:25AM -0700, Simon Glass wrote:
> On Wed, Sep 14, 2011 at 6:47 AM, Jason <u-boot@lakedaemon.net> wrote:
> > On Tue, Sep 13, 2011 at 02:06:56PM -0700, Simon Glass wrote:
> >> On Tue, Sep 13, 2011 at 7:37 AM, Jason <u-boot@lakedaemon.net> wrote:
> >> > Added the remote, thanks. ?Once we figure out the mach-types thing, I'll
> >> > try using the Marvell integrated RTC driver as a real-world example of a
> >> > conversion to fdt. ?I'll probably have questions ;-)
> >>
> >> That sounds great. I've tried to make it fair straightforward - with
> >> an option for building an fdt into U-Boot itself (i.e. don't define
> >> CONFIG_OF_SEPARATE). This should allow you to use your current dev
> >> flow.
> >
> > Okay, I have an initial version that compiles, but doesn't work. ?Which,
> > surprisingly, is progress. ?;-) ?I'll post the patch a little bit later
> > after I make sure I'm not missing something dumb.
> 
> A few hints:
> 
> - define CONFIG_OF_EMBED to start with, since it will embed the fdt
> inside U-Boot which is a good check that all is well. You can move to
> the more useful CONFIG_OF_SEPARATE when you get that working
> - there is a check in board.c that the fdt is accessible - if it is
> dying early then it might be that (early board panic patch is still in
> flight)

Frustrating morning.  Mainly due to the fact that I've been working with
device trees all of two days.

Here's what I did:
1.) applied your patches against v2011.09-rc1 (to get mvrtc.c)
2.) applied my dreamplug support patch 
3.) modified drivers/rtc/mvrtc.c for OF support
4.) compile
5.) run, then 'date' fails like so:

find_alias_node: rtc0
fdt_decode_next_alias failed.
Error decoding fdt for mvrtc.
## Get date failed

Obviously, I've hacked it up abit to get more error reporting out.
Here's my kirkwood-dreamplug.dts:

##### kirkwood-dreamplug.dts ####
/dts-v1/;

/include/ "kirkwood.dtsi"

/ {
        model = "Marvell Dreamplug";
        compatible = "marvell,dreamplug", "marvell,kirkwood";

        rtc at 0xf1010300 {
                status = "ok";
        };
};
#################################

And the kirkwood.dtsi

#### kirkwood.dtsi ####
/ {
        model = "Marvell Kirkwood";
        compatible = "marvell,kirkwood";
        #address-cells = <1>;
        #size-cells = <1>;

        cpus { 
                #address-cells = <1>;
                #size-cells = <0>;
                cpu at 0 {
                        compatible = "arm,arm926ejs";
                        reg = <0>;
                };
        };

        rtc at 0xf1010300 {
                compatible = "marvell,kirkwood-rtc";
                reg = <0xf1010300 0x02>;
                status = "disabled";
        };
};
#######################

I'd like to make sure my dts files are correct before I get to debugging
code. ;-)

A few notes:

If I compile with '#define DEBUG' in my board config, it builds, but
doesn't run, or at least, there's no output on the serial port.

I had the remove your fdt_decode_i2c() and clock.h include.  The clock.h
include seems to be specific to the tegra2 and doesn't exist for
kirkwood.

thx,

Jason.

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

[Grant Likely]

On Mon, Sep 12, 2011 at 03:04:23PM -0700, Simon Glass wrote:
> 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>

May I suggest an alternate approach?  Rather than hard linking the dtb
into the u-boot image, this would be so much more useful if the dtb
can be concatenated to the u-boot binary so that it can be configured
at install time.  Otherwise, switching to .dtb doesn't seem to
actually buy much when it still requires a recompile of u-boot to
change the dtb configuration data.

The linker script would need to be modified to make sure the end of
the binary image is aligned, and that there is a label indicating the
beginning of the .dtb section.  The init code will also need to read
the .dtb header to get the dtb length so that it can be relocated into
RAM with the rest of u-boot.

Targets could even be added as u-boot.%.bin so that a single build
could spit out several variants as needed, but still produce a bare
u-boot.bin binary that can have the .dtb appended manually.

Using CONFIG_OF_EMBED may actually be harmful if it starts encouraging
developers to put .dts files into the u-boot tree.  Especially when
right now the plan for the kernel is to actually move .dts file out of
the Linux tree and into a separate & neutral repository.

> ---
>  Makefile         |    4 ++
>  README           |   11 +++++-
>  config.mk        |    1 +
>  dts/Makefile     |  100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/common.h |    1 +
>  5 files changed, 115 insertions(+), 2 deletions(-)
>  create mode 100644 dts/Makefile
> 
> diff --git a/Makefile b/Makefile
> index d5a1f0a..658a622 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 812805f..5a2f060 100644
> --- a/README
> +++ b/README
> @@ -803,8 +803,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..e70a16b
> --- /dev/null
> +++ b/dts/Makefile
> @@ -0,0 +1,100 @@
> +#
> +# 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))
> +
> +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/$(BOARDDIR)/$(DEVICE_TREE).dts
> +	$(DTC) -R 4 -p 0x1000 -O dtb -o ${DT_BIN} $<
> +
> +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)

Or, you could use the assembler to do the heavy lifting for you (and
should be more reliable).  Look at the Linux piggy*.S files in
arch/arm/boot/compressed.  (Of course this point is moot if you switch
to appending the dtb).

> +
> +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 bd10f31..6cdcc50 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -249,6 +249,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] [RFC PATCH v2 3/6] fdt: Add support for a separate device tree (CONFIG_OF_SEPARATE)

[Grant Likely]

On Mon, Sep 12, 2011 at 03:04:24PM -0700, Simon Glass wrote:
> 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.

Hahaha, I should the entire thread of patches before commenting.  I
strongly feel that this should be the only method and that
CONFIG_OF_EMBED should be dropped.

> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  .gitignore             |    1 +
>  Makefile               |    5 ++
>  README                 |   11 +++-
>  doc/README.fdt-control |  168 ++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 183 insertions(+), 2 deletions(-)
>  create mode 100644 doc/README.fdt-control
> 
> diff --git a/.gitignore b/.gitignore
> index dbf545f..c4ebd34 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -34,6 +34,7 @@
>  /u-boot.dis
>  /u-boot.lds
>  /u-boot.ubl
> +/u-boot.dtb
>  
>  #
>  # Generated files
> diff --git a/Makefile b/Makefile
> index 658a622..d73efa1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -352,9 +352,14 @@ 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
>  
>  all:		$(ALL-y)
>  
> +$(obj)u-boot.dtb:	$(obj)u-boot
> +		$(MAKE) -C dts binary
> +		mv $(obj)dts/dt.dtb $@
> +
>  $(obj)u-boot.hex:	$(obj)u-boot
>  		$(OBJCOPY) ${OBJCFLAGS} -O ihex $< $@
>  
> diff --git a/README b/README
> index 5a2f060..0b8f338 100644
> --- a/README
> +++ b/README
> @@ -803,8 +803,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
> @@ -813,6 +813,13 @@ 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
> +
>  - 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..dfc8f06
> --- /dev/null
> +++ b/doc/README.fdt-control
> @@ -0,0 +1,168 @@
> +#
> +# 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>/<board>/<name>.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] [RFC PATCH v2 2/6] fdt: Add support for embedded device tree (CONFIG_OF_EMBED)

[Simon Glass]

Hi Jason,

On Wed, Sep 14, 2011 at 9:11 AM, Jason <u-boot@lakedaemon.net> wrote:
> Simon,
>
> On Wed, Sep 14, 2011 at 08:47:25AM -0700, Simon Glass wrote:
>> On Wed, Sep 14, 2011 at 6:47 AM, Jason <u-boot@lakedaemon.net> wrote:
>> > On Tue, Sep 13, 2011 at 02:06:56PM -0700, Simon Glass wrote:
>> >> On Tue, Sep 13, 2011 at 7:37 AM, Jason <u-boot@lakedaemon.net> wrote:
>> >> > Added the remote, thanks. ?Once we figure out the mach-types thing, I'll
>> >> > try using the Marvell integrated RTC driver as a real-world example of a
>> >> > conversion to fdt. ?I'll probably have questions ;-)
>> >>
>> >> That sounds great. I've tried to make it fair straightforward - with
>> >> an option for building an fdt into U-Boot itself (i.e. don't define
>> >> CONFIG_OF_SEPARATE). This should allow you to use your current dev
>> >> flow.
>> >
>> > Okay, I have an initial version that compiles, but doesn't work. ?Which,
>> > surprisingly, is progress. ?;-) ?I'll post the patch a little bit later
>> > after I make sure I'm not missing something dumb.
>>
>> A few hints:
>>
>> - define CONFIG_OF_EMBED to start with, since it will embed the fdt
>> inside U-Boot which is a good check that all is well. You can move to
>> the more useful CONFIG_OF_SEPARATE when you get that working
>> - there is a check in board.c that the fdt is accessible - if it is
>> dying early then it might be that (early board panic patch is still in
>> flight)
>
> Frustrating morning. ?Mainly due to the fact that I've been working with
> device trees all of two days.
>
> Here's what I did:
> 1.) applied your patches against v2011.09-rc1 (to get mvrtc.c)
> 2.) applied my dreamplug support patch
> 3.) modified drivers/rtc/mvrtc.c for OF support
> 4.) compile
> 5.) run, then 'date' fails like so:
>
> find_alias_node: rtc0
> fdt_decode_next_alias failed.
> Error decoding fdt for mvrtc.
> ## Get date failed

I don't actually see an alias in your fdt. And actually I left it out
of mine, so that is understandable. For i2c I have:

...
	aliases {
		i2c0 = "/i2c at 0x7000d000";
		i2c1 = "/i2c at 0x7000c000";
		i2c2 = "/i2c at 0x7000c400";
		i2c3 = "/i2c at 0x7000c500";
	};

find_next_alias is explains this (omited from the patch sorry):

/**
 * 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
 */

So I think you need to add a /alias node and try again. I can submit a
new patch set with this and a couple of other things I want to change,
but it would be good if you can get to the end first, in case you find
other problems.

>
> Obviously, I've hacked it up abit to get more error reporting out.
> Here's my kirkwood-dreamplug.dts:
>
> ##### kirkwood-dreamplug.dts ####
> /dts-v1/;
>
> /include/ "kirkwood.dtsi"
>
> / {
> ? ? ? ?model = "Marvell Dreamplug";
> ? ? ? ?compatible = "marvell,dreamplug", "marvell,kirkwood";
>
> ? ? ? ?rtc at 0xf1010300 {
> ? ? ? ? ? ? ? ?status = "ok";
> ? ? ? ?};
> };
> #################################
>
> And the kirkwood.dtsi
>
> #### kirkwood.dtsi ####
> / {
> ? ? ? ?model = "Marvell Kirkwood";
> ? ? ? ?compatible = "marvell,kirkwood";
> ? ? ? ?#address-cells = <1>;
> ? ? ? ?#size-cells = <1>;
>
> ? ? ? ?cpus {
> ? ? ? ? ? ? ? ?#address-cells = <1>;
> ? ? ? ? ? ? ? ?#size-cells = <0>;
> ? ? ? ? ? ? ? ?cpu at 0 {
> ? ? ? ? ? ? ? ? ? ? ? ?compatible = "arm,arm926ejs";
> ? ? ? ? ? ? ? ? ? ? ? ?reg = <0>;
> ? ? ? ? ? ? ? ?};
> ? ? ? ?};
>
> ? ? ? ?rtc at 0xf1010300 {
> ? ? ? ? ? ? ? ?compatible = "marvell,kirkwood-rtc";
> ? ? ? ? ? ? ? ?reg = <0xf1010300 0x02>;
> ? ? ? ? ? ? ? ?status = "disabled";
> ? ? ? ?};
> };
> #######################
>
> I'd like to make sure my dts files are correct before I get to debugging
> code. ;-)
>
> A few notes:
>
> If I compile with '#define DEBUG' in my board config, it builds, but
> doesn't run, or at least, there's no output on the serial port.

That's because you are getting output prior to relocation. Graeme has
a patch for that, and I also sent a patch for pre-console panic (which
I will update once his patch goes in).

>
> I had the remove your fdt_decode_i2c() and clock.h include. ?The clock.h
> include seems to be specific to the tegra2 and doesn't exist for
> kirkwood.

Yes that's right, it is just an example at this stage, and the idea of
a periph_id is specific to Tegra at present. Patches 5 and 6 are just
an example to show how to use it in code.

Regards,
Simon

>
> thx,
>
> Jason.
>

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

[Simon Glass]

Hi Grant,

On Wed, Sep 14, 2011 at 9:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Mon, Sep 12, 2011 at 03:04:23PM -0700, Simon Glass wrote:
>> 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>
>
> May I suggest an alternate approach? ?Rather than hard linking the dtb
> into the u-boot image, this would be so much more useful if the dtb
> can be concatenated to the u-boot binary so that it can be configured
> at install time. ?Otherwise, switching to .dtb doesn't seem to
> actually buy much when it still requires a recompile of u-boot to
> change the dtb configuration data.

Thanks for your comments.

Yes I agree. This is CONFIG_OF_SEPARATE - see the other patch in the set.

>
> The linker script would need to be modified to make sure the end of
> the binary image is aligned, and that there is a label indicating the
> beginning of the .dtb section. ?The init code will also need to read
> the .dtb header to get the dtb length so that it can be relocated into
> RAM with the rest of u-boot.

The label is _end, and you can just:

   cat u-boot.bin some-fdt.dtb >u-boot.dtb.bin

to make this work. The code in the patch is:

#elif defined CONFIG_OF_SEPARATE
	/* FDT is at end of image */
	gd->fdt_blob = (void *)(_end_ofs + _TEXT_BASE);
#endif

>
> Targets could even be added as u-boot.%.bin so that a single build
> could spit out several variants as needed, but still produce a bare
> u-boot.bin binary that can have the .dtb appended manually.

I did have this as part of the U-Boot Makefile in my testing. If
people don't consider it too intrusive then I can certainly add this
in. It is quite convenient, and just adds an extra target to the
U-Boot Makefile.

>
> Using CONFIG_OF_EMBED may actually be harmful if it starts encouraging
> developers to put .dts files into the u-boot tree. ?Especially when
> right now the plan for the kernel is to actually move .dts file out of
> the Linux tree and into a separate & neutral repository.

It is a dev convenience, but very very useful in development. For
example, when using a debugger it is easy to just load the u-boot ELF
image and get everything there. We do need to make sure people don't
use it in production as it defeats the purpose of run-time config to a
large extent!

if the fdt is not in the U-Boot tree, where does it go? When will the
kernel fdt be set up? That sounds very promising.

Regards.
Simon

>
>> ---
>> ?Makefile ? ? ? ? | ? ?4 ++
>> ?README ? ? ? ? ? | ? 11 +++++-
>> ?config.mk ? ? ? ?| ? ?1 +
>> ?dts/Makefile ? ? | ?100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> ?include/common.h | ? ?1 +
>> ?5 files changed, 115 insertions(+), 2 deletions(-)
>> ?create mode 100644 dts/Makefile
>>
[snip]

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

[Simon Glass]

Hi Grant,

On Wed, Sep 14, 2011 at 9:48 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Mon, Sep 12, 2011 at 03:04:24PM -0700, Simon Glass wrote:
>> 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.
>
> Hahaha, I should the entire thread of patches before commenting. ?I
> strongly feel that this should be the only method and that
> CONFIG_OF_EMBED should be dropped.

:-)

Please see my comments in the other thread.

Regards,
Simon

>
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>> ?.gitignore ? ? ? ? ? ? | ? ?1 +
>> ?Makefile ? ? ? ? ? ? ? | ? ?5 ++
>> ?README ? ? ? ? ? ? ? ? | ? 11 +++-
>> ?doc/README.fdt-control | ?168 ++++++++++++++++++++++++++++++++++++++++++++++++
>> ?4 files changed, 183 insertions(+), 2 deletions(-)
>> ?create mode 100644 doc/README.fdt-control
>>
>> diff --git a/.gitignore b/.gitignore
>> index dbf545f..c4ebd34 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -34,6 +34,7 @@
>> ?/u-boot.dis
>> ?/u-boot.lds
>> ?/u-boot.ubl
>> +/u-boot.dtb
>>
>> ?#
>> ?# Generated files
>> diff --git a/Makefile b/Makefile
>> index 658a622..d73efa1 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -352,9 +352,14 @@ 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
>>
>> ?all: ? ? ? ? $(ALL-y)
>>
>> +$(obj)u-boot.dtb: ? ?$(obj)u-boot
>> + ? ? ? ? ? ? $(MAKE) -C dts binary
>> + ? ? ? ? ? ? mv $(obj)dts/dt.dtb $@
>> +
>> ?$(obj)u-boot.hex: ? ?$(obj)u-boot
>> ? ? ? ? ? ? ? $(OBJCOPY) ${OBJCFLAGS} -O ihex $< $@
>>
>> diff --git a/README b/README
>> index 5a2f060..0b8f338 100644
>> --- a/README
>> +++ b/README
>> @@ -803,8 +803,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
>> @@ -813,6 +813,13 @@ 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
>> +
>> ?- 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..dfc8f06
>> --- /dev/null
>> +++ b/doc/README.fdt-control
>> @@ -0,0 +1,168 @@
>> +#
>> +# 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>/<board>/<name>.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] [RFC PATCH v2 2/6] fdt: Add support for embedded device tree (CONFIG_OF_EMBED)

[Grant Likely]

On Wed, Sep 14, 2011 at 12:03 PM, Simon Glass <sjg@chromium.org> wrote:
> if the fdt is not in the U-Boot tree, where does it go? When will the
> kernel fdt be set up? That sounds very promising.

Into a separate git tree.  Possibly on devicetree.org,
git.secretlab.ca, or git.linaro.org.  I don't really want it on linaro
or kernel.org though because I want to make it clear that it is
absolutely not intended to be Linux-specific.

g.

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

[Simon Glass]

Hi Grant.

On Wed, Sep 14, 2011 at 12:17 PM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> On Wed, Sep 14, 2011 at 12:03 PM, Simon Glass <sjg@chromium.org> wrote:
>> if the fdt is not in the U-Boot tree, where does it go? When will the
>> kernel fdt be set up? That sounds very promising.
>
> Into a separate git tree. ?Possibly on devicetree.org,
> git.secretlab.ca, or git.linaro.org. ?I don't really want it on linaro
> or kernel.org though because I want to make it clear that it is
> absolutely not intended to be Linux-specific.

OK thanks. I think secretlab sounds most exciting :-) In U-Boot,
people can currently do something like:

git clone http://git.denx.de/u-boot.git .
make secretboard_config
make

and get a working u-boot and u-boot.bin. Hopefully we can make just as
easy with fdt.

Regards,
Simon

>
> g.
>

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

[Jason]

Simon,

On Wed, Sep 14, 2011 at 10:45:59AM -0700, Simon Glass wrote:
> On Wed, Sep 14, 2011 at 9:11 AM, Jason <u-boot@lakedaemon.net> wrote:
> > On Wed, Sep 14, 2011 at 08:47:25AM -0700, Simon Glass wrote:
> >> On Wed, Sep 14, 2011 at 6:47 AM, Jason <u-boot@lakedaemon.net> wrote:
> >> > On Tue, Sep 13, 2011 at 02:06:56PM -0700, Simon Glass wrote:
> >> >> On Tue, Sep 13, 2011 at 7:37 AM, Jason <u-boot@lakedaemon.net> wrote:
> >> >> > Added the remote, thanks. ?Once we figure out the mach-types thing, I'll
> >> >> > try using the Marvell integrated RTC driver as a real-world example of a
> >> >> > conversion to fdt. ?I'll probably have questions ;-)
> >> >>
> >> >> That sounds great. I've tried to make it fair straightforward - with
> >> >> an option for building an fdt into U-Boot itself (i.e. don't define
> >> >> CONFIG_OF_SEPARATE). This should allow you to use your current dev
> >> >> flow.
> >> >
> >> > Okay, I have an initial version that compiles, but doesn't work. ?Which,
> >> > surprisingly, is progress. ?;-) ?I'll post the patch a little bit later
> >> > after I make sure I'm not missing something dumb.
> >>
> >> A few hints:
> >>
> >> - define CONFIG_OF_EMBED to start with, since it will embed the fdt
> >> inside U-Boot which is a good check that all is well. You can move to
> >> the more useful CONFIG_OF_SEPARATE when you get that working
> >> - there is a check in board.c that the fdt is accessible - if it is
> >> dying early then it might be that (early board panic patch is still in
> >> flight)
> >
> > Frustrating morning. ?Mainly due to the fact that I've been working with
> > device trees all of two days.
> >
> > Here's what I did:
> > 1.) applied your patches against v2011.09-rc1 (to get mvrtc.c)
> > 2.) applied my dreamplug support patch
> > 3.) modified drivers/rtc/mvrtc.c for OF support
> > 4.) compile
> > 5.) run, then 'date' fails like so:
> >
> > find_alias_node: rtc0
> > fdt_decode_next_alias failed.
> > Error decoding fdt for mvrtc.
> > ## Get date failed
> 
> I don't actually see an alias in your fdt. And actually I left it out
> of mine, so that is understandable. For i2c I have:
> 
> ...
> 	aliases {
> 		i2c0 = "/i2c at 0x7000d000";
> 		i2c1 = "/i2c at 0x7000c000";
> 		i2c2 = "/i2c at 0x7000c400";
> 		i2c3 = "/i2c at 0x7000c500";
> 	};

That worked!  

Marvell>> date
find_alias_node: rtc0
Date: 2011-09-14 (Wednesday)    Time: 14:04:54
Marvell>>

> So I think you need to add a /alias node and try again. I can submit a
> new patch set with this and a couple of other things I want to change,
> but it would be good if you can get to the end first, in case you find
> other problems.

I'll clean up what I have and post it RFC.  

> >
> > Obviously, I've hacked it up abit to get more error reporting out.
> > Here's my kirkwood-dreamplug.dts:
> >
> > ##### kirkwood-dreamplug.dts ####
> > /dts-v1/;
> >
> > /include/ "kirkwood.dtsi"
> >
> > / {
> > ? ? ? ?model = "Marvell Dreamplug";
> > ? ? ? ?compatible = "marvell,dreamplug", "marvell,kirkwood";
> >
> > ? ? ? ?rtc at 0xf1010300 {
> > ? ? ? ? ? ? ? ?status = "ok";
> > ? ? ? ?};
> > };
> > #################################
> >
> > And the kirkwood.dtsi
> >
> > #### kirkwood.dtsi ####
> > / {
> > ? ? ? ?model = "Marvell Kirkwood";
> > ? ? ? ?compatible = "marvell,kirkwood";
> > ? ? ? ?#address-cells = <1>;
> > ? ? ? ?#size-cells = <1>;
> >
> > ? ? ? ?cpus {
> > ? ? ? ? ? ? ? ?#address-cells = <1>;
> > ? ? ? ? ? ? ? ?#size-cells = <0>;
> > ? ? ? ? ? ? ? ?cpu at 0 {
> > ? ? ? ? ? ? ? ? ? ? ? ?compatible = "arm,arm926ejs";
> > ? ? ? ? ? ? ? ? ? ? ? ?reg = <0>;
> > ? ? ? ? ? ? ? ?};
> > ? ? ? ?};
> >
> > ? ? ? ?rtc at 0xf1010300 {
> > ? ? ? ? ? ? ? ?compatible = "marvell,kirkwood-rtc";
> > ? ? ? ? ? ? ? ?reg = <0xf1010300 0x02>;
> > ? ? ? ? ? ? ? ?status = "disabled";
> > ? ? ? ?};
> > };
> > #######################
> >
> > I'd like to make sure my dts files are correct before I get to debugging
> > code. ;-)
> >
> > A few notes:
> >
> > If I compile with '#define DEBUG' in my board config, it builds, but
> > doesn't run, or at least, there's no output on the serial port.
> 
> That's because you are getting output prior to relocation. Graeme has
> a patch for that, and I also sent a patch for pre-console panic (which
> I will update once his patch goes in).

Okay, since I now have some success, I can build off of that.  I'll just
work without debug for now.

> > I had the remove your fdt_decode_i2c() and clock.h include. ?The clock.h
> > include seems to be specific to the tegra2 and doesn't exist for
> > kirkwood.
> 
> Yes that's right, it is just an example at this stage, and the idea of
> a periph_id is specific to Tegra at present. Patches 5 and 6 are just
> an example to show how to use it in code.

Ok, I'll drop those from my branch to make a cleaner example.

thx,

Jason.

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

[Simon Glass]

Hi Jason,

On Wed, Sep 14, 2011 at 12:50 PM, Jason <u-boot@lakedaemon.net> wrote:
> Simon,
>
> On Wed, Sep 14, 2011 at 10:45:59AM -0700, Simon Glass wrote:

[snip]
>> > 5.) run, then 'date' fails like so:
>> >
>> > find_alias_node: rtc0
>> > fdt_decode_next_alias failed.
>> > Error decoding fdt for mvrtc.
>> > ## Get date failed
>>
>> I don't actually see an alias in your fdt. And actually I left it out
>> of mine, so that is understandable. For i2c I have:
>>
>> ...
>> ? ? ? aliases {
>> ? ? ? ? ? ? ? i2c0 = "/i2c at 0x7000d000";
>> ? ? ? ? ? ? ? i2c1 = "/i2c at 0x7000c000";
>> ? ? ? ? ? ? ? i2c2 = "/i2c at 0x7000c400";
>> ? ? ? ? ? ? ? i2c3 = "/i2c at 0x7000c500";
>> ? ? ? };
>
> That worked!
>
> Marvell>> date
> find_alias_node: rtc0
> Date: 2011-09-14 (Wednesday) ? ?Time: 14:04:54
> Marvell>>
>

Great!

>> So I think you need to add a /alias node and try again. I can submit a
>> new patch set with this and a couple of other things I want to change,
>> but it would be good if you can get to the end first, in case you find
>> other problems.
>
> I'll clean up what I have and post it RFC.

OK good

>> > I had the remove your fdt_decode_i2c() and clock.h include. ?The clock.h
>> > include seems to be specific to the tegra2 and doesn't exist for
>> > kirkwood.
>>
>> Yes that's right, it is just an example at this stage, and the idea of
>> a periph_id is specific to Tegra at present. Patches 5 and 6 are just
>> an example to show how to use it in code.
>
> Ok, I'll drop those from my branch to make a cleaner example.

Yes, ideally I would like to keep SOC-specific things out of it at
this stage, but I was asked for an example and had to choose
something! My hope is that we can have the base patch and then a
couple of architecture patches.
>
> thx,
>
> Jason.
>

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

[Wolfgang Denk]

Dear Grant Likely,

In message <20110914164528.GM3134@ponder.secretlab.ca> you wrote:
>
> May I suggest an alternate approach?  Rather than hard linking the dtb
> into the u-boot image, this would be so much more useful if the dtb
> can be concatenated to the u-boot binary so that it can be configured
> at install time.  Otherwise, switching to .dtb doesn't seem to

Actually the DTB address should _always_ be taken from an environment
variable, so we have full flexibility.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In the beginning there was nothing.
And the Lord said "Let There Be Light!"
And still there was nothing, but at least now you could see it.

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

[Jason]

On Wed, Sep 14, 2011 at 01:05:58PM -0700, Simon Glass wrote:
> On Wed, Sep 14, 2011 at 12:50 PM, Jason <u-boot@lakedaemon.net> wrote:
> > On Wed, Sep 14, 2011 at 10:45:59AM -0700, Simon Glass wrote:
> 
> [snip]
> >> > 5.) run, then 'date' fails like so:
> >> >
> >> > find_alias_node: rtc0
> >> > fdt_decode_next_alias failed.
> >> > Error decoding fdt for mvrtc.
> >> > ## Get date failed
> >>
> >> I don't actually see an alias in your fdt. And actually I left it out
> >> of mine, so that is understandable. For i2c I have:
> >>
> >> ...
> >> ? ? ? aliases {
> >> ? ? ? ? ? ? ? i2c0 = "/i2c at 0x7000d000";
> >> ? ? ? ? ? ? ? i2c1 = "/i2c at 0x7000c000";
> >> ? ? ? ? ? ? ? i2c2 = "/i2c at 0x7000c400";
> >> ? ? ? ? ? ? ? i2c3 = "/i2c at 0x7000c500";
> >> ? ? ? };
> >
> > That worked!
> >
> > Marvell>> date
> > find_alias_node: rtc0
> > Date: 2011-09-14 (Wednesday) ? ?Time: 14:04:54
> > Marvell>>
> >
> 
> Great!
> 
> >> So I think you need to add a /alias node and try again. I can submit a
> >> new patch set with this and a couple of other things I want to change,
> >> but it would be good if you can get to the end first, in case you find
> >> other problems.
> >
> > I'll clean up what I have and post it RFC.
> 
> OK good
> 
> >> > I had the remove your fdt_decode_i2c() and clock.h include. ?The clock.h
> >> > include seems to be specific to the tegra2 and doesn't exist for
> >> > kirkwood.
> >>
> >> Yes that's right, it is just an example at this stage, and the idea of
> >> a periph_id is specific to Tegra at present. Patches 5 and 6 are just
> >> an example to show how to use it in code.
> >
> > Ok, I'll drop those from my branch to make a cleaner example.
> 
> Yes, ideally I would like to keep SOC-specific things out of it at
> this stage, but I was asked for an example and had to choose
> something! My hope is that we can have the base patch and then a
> couple of architecture patches.

Yes, I don't like putting fdt_decode_i2c() or fdt_decode_rtc() in
common/fdt_decode.c ...  The current implementation of fdt...i2c() is
arch specific, and fdt...rtc() I know will only work for the simple
integrated rtc with two registers.

Let me rework it to the appropriate place and then I'll post.  I'll also
remove the fdt...i2c().

thx,

Jason.

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

[Simon Glass]

Hi Jason,

On Wed, Sep 14, 2011 at 1:16 PM, Jason <u-boot@lakedaemon.net> wrote:
> On Wed, Sep 14, 2011 at 01:05:58PM -0700, Simon Glass wrote:
>> On Wed, Sep 14, 2011 at 12:50 PM, Jason <u-boot@lakedaemon.net> wrote:
>> > On Wed, Sep 14, 2011 at 10:45:59AM -0700, Simon Glass wrote:
>>
>> [snip]
>> >> > 5.) run, then 'date' fails like so:
>> >> >
>> >> > find_alias_node: rtc0
>> >> > fdt_decode_next_alias failed.
>> >> > Error decoding fdt for mvrtc.
>> >> > ## Get date failed
>> >>
>> >> I don't actually see an alias in your fdt. And actually I left it out
>> >> of mine, so that is understandable. For i2c I have:
>> >>
>> >> ...
>> >> ? ? ? aliases {
>> >> ? ? ? ? ? ? ? i2c0 = "/i2c at 0x7000d000";
>> >> ? ? ? ? ? ? ? i2c1 = "/i2c at 0x7000c000";
>> >> ? ? ? ? ? ? ? i2c2 = "/i2c at 0x7000c400";
>> >> ? ? ? ? ? ? ? i2c3 = "/i2c at 0x7000c500";
>> >> ? ? ? };
>> >
>> > That worked!
>> >
>> > Marvell>> date
>> > find_alias_node: rtc0
>> > Date: 2011-09-14 (Wednesday) ? ?Time: 14:04:54
>> > Marvell>>
>> >
>>
>> Great!
>>
>> >> So I think you need to add a /alias node and try again. I can submit a
>> >> new patch set with this and a couple of other things I want to change,
>> >> but it would be good if you can get to the end first, in case you find
>> >> other problems.
>> >
>> > I'll clean up what I have and post it RFC.
>>
>> OK good
>>
>> >> > I had the remove your fdt_decode_i2c() and clock.h include. ?The clock.h
>> >> > include seems to be specific to the tegra2 and doesn't exist for
>> >> > kirkwood.
>> >>
>> >> Yes that's right, it is just an example at this stage, and the idea of
>> >> a periph_id is specific to Tegra at present. Patches 5 and 6 are just
>> >> an example to show how to use it in code.
>> >
>> > Ok, I'll drop those from my branch to make a cleaner example.
>>
>> Yes, ideally I would like to keep SOC-specific things out of it at
>> this stage, but I was asked for an example and had to choose
>> something! My hope is that we can have the base patch and then a
>> couple of architecture patches.
>
> Yes, I don't like putting fdt_decode_i2c() or fdt_decode_rtc() in
> common/fdt_decode.c ... ?The current implementation of fdt...i2c() is
> arch specific, and fdt...rtc() I know will only work for the simple
> integrated rtc with two registers.

This is one of the things to resolve. I think we need an fdt_decode to
lighten the load of finding aliases, decoding addresses and the like,
but a bigger question is whether we want the various i2c drivers to
share decode logic. It will help make the drivers more similar, but
clearly means that they have to follow the lowest common denominator.
This is a bit like CONFIG_ works at present - and we are replacing the
CONFIG_ items. For i2c the configs might be CONFIG_SYS_I2C_SPEED and
the controller address (and maybe CONFIG_SYS_I2C_SLAVE). But we don't
deal with particular i2c config like pinmux settings etc. which are
not common across a lot of boards. So fdt_decode would deal with these
few common settings and leave specific drivers to do the rest.

But if people are happy with the idea of fdt decode code bleeding more
into each driver, then fdt_decode just becomes a low-level helper
library, and does not have specific functions for decoding an i2c
node, a uart, etc. That is perhaps more pure - my main concerns with
this are uptake (too hard to swtich a board over to fdt) and
consistency (everyone will use their own way of doing the same thing -
and we have enough of that in U-Boot already :-)

Regards,
Simon

>
> Let me rework it to the appropriate place and then I'll post. ?I'll also
> remove the fdt...i2c().
>
> thx,
>
> Jason.
>

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

[Simon Glass]

On Wed, Sep 14, 2011 at 1:11 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Grant Likely,
>
> In message <20110914164528.GM3134@ponder.secretlab.ca> you wrote:
>>
>> May I suggest an alternate approach? ?Rather than hard linking the dtb
>> into the u-boot image, this would be so much more useful if the dtb
>> can be concatenated to the u-boot binary so that it can be configured
>> at install time. ?Otherwise, switching to .dtb doesn't seem to
>
> Actually the DTB address should _always_ be taken from an environment
> variable, so we have full flexibility.

Hi Wolfgang,

That is not implemented in my RFC patch but is easy enough to do. The
fdt is available very early (in board_init_f) so that it can handle
the console UART. At that point we have access to the default
environment only.

Perhaps the behaviour you refer to should be called
CONFIG_OF_ENVIRONMENT and be the default? If people want Grant's
option then they can set CONFIG_OF_SEPARATE and it will be
concatenated with U-Boot.

One reason for CONFIG_OF_SEPARATE is that some SOCs are going to
prefer the fdt to be loaded by the SOC boot ROM. For example, on
Tegra, the boot ROM loads U-Boot into SDRAM. If the fdt is somewhere
else in the flash, then U-Boot will need to load it after starting up
since the boot ROM can't find it. But that means that it is not
available very early. For example, if it is in SPI flash then the fdt
is not available until after relocation, and so you can't use the
serial console until then. Of course we could start with built-in
default fdt and move to the loaded one later, but that's not very
nice.

It also ties into the build system - e.g. does the U-Boot makefile do
the work for you, or do you need to take u-boot.bin and munge it
yourself (or flash u-boot.bin and u-boot.dtb into separate places...)

I have thought quite a bit about these issues in creating this RFC,
and these are the sorts of things we need to get right for run-time
config to work nicely.

Regards,
Simon

>
>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> In the beginning there was nothing.
> And the Lord said "Let There Be Light!"
> And still there was nothing, but at least now you could see it.
>

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

[Jason]

Simon,

On Wed, Sep 14, 2011 at 01:24:31PM -0700, Simon Glass wrote:
> On Wed, Sep 14, 2011 at 1:16 PM, Jason <u-boot@lakedaemon.net> wrote:
> > On Wed, Sep 14, 2011 at 01:05:58PM -0700, Simon Glass wrote:
...
> >> Yes, ideally I would like to keep SOC-specific things out of it at
> >> this stage, but I was asked for an example and had to choose
> >> something! My hope is that we can have the base patch and then a
> >> couple of architecture patches.
> >
> > Yes, I don't like putting fdt_decode_i2c() or fdt_decode_rtc() in
> > common/fdt_decode.c ... ?The current implementation of fdt...i2c() is
> > arch specific, and fdt...rtc() I know will only work for the simple
> > integrated rtc with two registers.
> 
> This is one of the things to resolve. I think we need an fdt_decode to
> lighten the load of finding aliases, decoding addresses and the like,
> but a bigger question is whether we want the various i2c drivers to
> share decode logic. It will help make the drivers more similar, but
> clearly means that they have to follow the lowest common denominator.
> This is a bit like CONFIG_ works at present - and we are replacing the
> CONFIG_ items. For i2c the configs might be CONFIG_SYS_I2C_SPEED and
> the controller address (and maybe CONFIG_SYS_I2C_SLAVE). But we don't
> deal with particular i2c config like pinmux settings etc. which are
> not common across a lot of boards. So fdt_decode would deal with these
> few common settings and leave specific drivers to do the rest.
> 
> But if people are happy with the idea of fdt decode code bleeding more
> into each driver, then fdt_decode just becomes a low-level helper
> library, and does not have specific functions for decoding an i2c
> node, a uart, etc.

After working with it a little, this seems more natural.  Clearly
delineated.

> That is perhaps more pure - my main concerns with
> this are uptake (too hard to swtich a board over to fdt) and
> consistency (everyone will use their own way of doing the same thing -
> and we have enough of that in U-Boot already :-)

If it becomes a problem, we can address it later.  I much prefer a clean
boundary to start with.

thx,

Jason.

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

[Grant Likely]

On Wed, Sep 14, 2011 at 2:11 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Grant Likely,
>
> In message <20110914164528.GM3134@ponder.secretlab.ca> you wrote:
>>
>> May I suggest an alternate approach? ?Rather than hard linking the dtb
>> into the u-boot image, this would be so much more useful if the dtb
>> can be concatenated to the u-boot binary so that it can be configured
>> at install time. ?Otherwise, switching to .dtb doesn't seem to
>
> Actually the DTB address should _always_ be taken from an environment
> variable, so we have full flexibility.

That doesn't work so well when u-boot needs the dtb to initialize
itself.  However, once initialized, I completely agree that the dtb
address should be in the env so that it can be changed as needed.

g.

[U-Boot] [RFC PATCH 0/4 v1] Use fdt to init mvrtc driver for dreamplug

[Jason Cooper]

Simon, All,

This patch series is my attempt to learn device tree with a driver I'm
familiar with.  Also, to create a real, working example for fdt in U-boot.

To make this work, I applied v2 of Simon Glass' patch series (what this
email is in reply to) against v2011.09-rc1.  I then cherry picked my
dreamplug board support patch.  Last, I applied the following series.

It works, although with CONFIG_OF_EMBED.  I haven't tried _SEPARATE yet.

Some thoughts:

1.) 'kirkwood-dreamplug.dts' can be renamed 'dreamplug.dts'.  Due to
    machine_is_XXX 'dreamplug' will be unique, concise, and descriptive.
    This requires changes to Simon Glass' patchset.

2.) The fdt files should be moved up one directory level, in the end, it
    should look like:

    board/Marvell/armada100-dt.c
    board/Marvell/armada100-dt.h
    board/Marvell/armada100.dtsi
    board/Marvell/dreamplug.dts
    board/Marvell/gplugd.dts      # this includes armada100.dtsi
    board/Marvell/guruplug.dts
    board/Marvell/kirkwood.dtsi
    board/Marvell/kirkwood-dt.c
    board/Marvell/kirkwood-dt.h
    board/Marvell/openrd.dts
    board/Marvell/sheevaplug.dts
    ...
    board/Marvell/<other boards>.dts

    This also requires changes to Simon's patchset.

3.) Since mach names (dreamplug) and SoC names (kirkwood) are unique,
    should we put all .dts and .dtsi files in one dir?  eg /dts ?  This
    would facilitate migration to Grant Likely's device-tree.git
    whereever and whenever it lands.  This is also similar to how Linux
    currently does it (arch/arm/boot/dts/)

Jason Cooper (4):
  fdt: remove i2c example code.
  fdt_decode: make more available.
  mvrtc: add fdt support.
  dreamplug: enable fdt

 board/Marvell/dreamplug/kirkwood-dreamplug.dts |   12 +++++
 board/Marvell/dreamplug/kirkwood.dtsi          |   25 ++++++++++
 common/fdt_decode.c                            |   26 ++--------
 drivers/rtc/mvrtc.c                            |   62 ++++++++++++++++++++++-
 drivers/rtc/mvrtc.h                            |    7 +++
 include/configs/dreamplug.h                    |    5 ++
 include/fdt_decode.h                           |   46 +++++------------
 7 files changed, 128 insertions(+), 55 deletions(-)
 create mode 100644 board/Marvell/dreamplug/kirkwood-dreamplug.dts
 create mode 100644 board/Marvell/dreamplug/kirkwood.dtsi

[U-Boot] [RFC PATCH 1/4 v1] fdt: remove i2c example code.

[Jason Cooper]

Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
---
 common/fdt_decode.c  |   14 --------------
 include/fdt_decode.h |   32 --------------------------------
 2 files changed, 0 insertions(+), 46 deletions(-)

diff --git a/common/fdt_decode.c b/common/fdt_decode.c
index 9e8cf4d..cd7a071 100644
--- a/common/fdt_decode.c
+++ b/common/fdt_decode.c
@@ -34,7 +34,6 @@
  */
 #define COMPAT(id, name) name
 static const char *compat_names[COMPAT_COUNT] = {
-	COMPAT(NVIDIA_TEGRA250_I2C, "nvidia,tegra250-i2c"),
 };
 
 /**
@@ -162,16 +161,3 @@ int fdt_decode_next_alias(const void *blob, const char *name,
 	return err ? -FDT_ERR_MISSING : node;
 }
 
-int fdt_decode_i2c(const void *blob, int node, struct fdt_i2c *config)
-{
-	config->reg = (struct i2c_ctlr *)get_addr(blob, node, "reg");
-	config->pinmux = get_int(blob, node, "pinmux", 0);
-	config->speed = get_int(blob, node, "speed", 0);
-	config->periph_id = get_int(blob, node, "periph-id", -1);
-	config->enabled = get_is_enabled(blob, node, 0);
-
-	if (config->periph_id == -1)
-		return -FDT_ERR_MISSING;
-
-	return 0;
-}
diff --git a/include/fdt_decode.h b/include/fdt_decode.h
index bdcdbba..ba3c15b 100644
--- a/include/fdt_decode.h
+++ b/include/fdt_decode.h
@@ -28,9 +28,6 @@
  * changes to support FDT are minimized.
  */
 
-#include <ns16550.h>
-#include <asm/arch/clock.h>
-
 /* A typedef for a physical address. We should move it to a generic place */
 #ifdef CONFIG_PHYS_64BIT
 typedef u64 addr_t;
@@ -55,36 +52,7 @@ struct fdt_memory {
  */
 enum fdt_compat_id {
 	COMPAT_UNKNOWN,
-	COMPAT_NVIDIA_TEGRA250_I2C,	/* Tegra 250 i2c */
-
 	COMPAT_COUNT,
 };
 
-/* Information about i2c controller */
-struct fdt_i2c {
-	struct i2c_ctlr *reg;		/* Address of controller registers */
-	int pinmux;			/* Which pin mux setting to use */
-	u32 speed;			/* Speed in KHz */
-	enum periph_id periph_id;	/* Peripheral ID for clock/pinmux */
-	int enabled;			/* 1 if enabled, 0 if disabled */
-};
 
-/**
- * Returns information from the FDT about an i2c controler. This function reads
- * out the following attributes:
- *
- *	reg
- *      enabled
- *	pinmux
- *	speed
- *	periph-id
- *
- * @param blob		FDT blob to use
- * @param node		Node to read from
- * @param config	structure to use to return information
- * @returns 0 on success, -ve on error, in which case config may or may not be
- *			unchanged. If the node is present but expected data is
- *			missing then this will generally return
- *			-FDT_ERR_MISSING.
- */
-int fdt_decode_i2c(const void *blob, int node, struct fdt_i2c *config);
-- 
1.7.0.4

[U-Boot] [RFC PATCH 2/4 v1] fdt_decode: make more available.

[Jason Cooper]

Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
---
 common/fdt_decode.c  |   11 +++++------
 include/fdt_decode.h |   13 +++++++++++++
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/common/fdt_decode.c b/common/fdt_decode.c
index cd7a071..0f13089 100644
--- a/common/fdt_decode.c
+++ b/common/fdt_decode.c
@@ -33,7 +33,7 @@
  * 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] = {
+const char *compat_names[COMPAT_COUNT] = {
 };
 
 /**
@@ -43,7 +43,7 @@ static const char *compat_names[COMPAT_COUNT] = {
  * @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)
+int find_alias_node(const void *blob, const char *name)
 {
 	const char *path;
 	int alias_node;
@@ -68,7 +68,7 @@ static int find_alias_node(const void *blob, const char *name)
  * @param prop_name	name of property to find
  * @return address, if found, or ADDR_T_NONE if not
  */
-static addr_t get_addr(const void *blob, int node, const char *prop_name)
+addr_t get_addr(const void *blob, int node, const char *prop_name)
 {
 	const addr_t *cell;
 	int len;
@@ -91,7 +91,7 @@ static addr_t get_addr(const void *blob, int node, const char *prop_name)
  * @param default_val	default value to return if the property is not found
  * @return integer value, if found, or default_val if not
  */
-static s32 get_int(const void *blob, int node, const char *prop_name,
+s32 get_int(const void *blob, int node, const char *prop_name,
 		s32 default_val)
 {
 	const s32 *cell;
@@ -115,7 +115,7 @@ static s32 get_int(const void *blob, int node, const char *prop_name,
  * @param default_val	default value to return if no 'status' property exists
  * @return integer value 0/1, if found, or default_val if not
  */
-static int get_is_enabled(const void *blob, int node, int default_val)
+int get_is_enabled(const void *blob, int node, int default_val)
 {
 	const char *cell;
 
@@ -160,4 +160,3 @@ int fdt_decode_next_alias(const void *blob, const char *name,
 		return err;
 	return err ? -FDT_ERR_MISSING : node;
 }
-
diff --git a/include/fdt_decode.h b/include/fdt_decode.h
index ba3c15b..4264e3b 100644
--- a/include/fdt_decode.h
+++ b/include/fdt_decode.h
@@ -19,6 +19,8 @@
  * MA 02111-1307 USA
  */
 
+#ifndef _FDT_DECODE_H_
+#define _FDT_DECODE_H_
 
 /*
  * This file contains convenience functions for decoding useful and
@@ -55,4 +57,15 @@ enum fdt_compat_id {
 	COMPAT_COUNT,
 };
 
+int find_alias_node(const void *blob, const char *name);
+addr_t get_addr(const void *blob, int node, const char *prop_name);
+s32 get_int(const void *blob, int node, const char *prop_name,
+	s32 default_val);
+int get_is_enabled(const void *blob, int node, int default_val);
+enum fdt_compat_id fdt_decode_lookup(const void *blob, int node);
+int fdt_decode_next_compatible(const void *blob, int node,
+		enum fdt_compat_id id);
+int fdt_decode_next_alias(const void *blob, const char *name,
+		enum fdt_compat_id id, int *upto);
 
+#endif
-- 
1.7.0.4

[U-Boot] [RFC PATCH 3/4 v1] mvrtc: add fdt support.

[Jason Cooper]

Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
---
 common/fdt_decode.c  |    1 +
 drivers/rtc/mvrtc.c  |   62 +++++++++++++++++++++++++++++++++++++++++++++++--
 drivers/rtc/mvrtc.h  |    7 +++++
 include/fdt_decode.h |    1 +
 4 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/common/fdt_decode.c b/common/fdt_decode.c
index 0f13089..1a0dcf4 100644
--- a/common/fdt_decode.c
+++ b/common/fdt_decode.c
@@ -34,6 +34,7 @@
  */
 #define COMPAT(id, name) name
 const char *compat_names[COMPAT_COUNT] = {
+	COMPAT(MARVELL_KIRKWOOD_RTC, "marvell,kirkwood-rtc"),
 };
 
 /**
diff --git a/drivers/rtc/mvrtc.c b/drivers/rtc/mvrtc.c
index ccc573a..ce2dc3d 100644
--- a/drivers/rtc/mvrtc.c
+++ b/drivers/rtc/mvrtc.c
@@ -28,18 +28,62 @@
 #include <common.h>
 #include <command.h>
 #include <rtc.h>
+#ifdef CONFIG_OF_CONTROL
+#include <fdt_decode.h>
+#endif
 #include "mvrtc.h"
 
+DECLARE_GLOBAL_DATA_PTR;
+
 /* This RTC does not support century, so we assume 20 */
 #define CENTURY 20
 
+#ifndef CONFIG_OF_CONTROL
+struct mvrtc_registers *mvrtc_get_config(void) {
+	return (struct mvrtc_registers *)KW_RTC_BASE;
+}
+
+#else
+int fdt_decode_rtc(const void *blob, int node, struct fdt_rtc *config)
+{
+	config->reg = get_addr(blob, node, "reg");
+	config->enabled = get_is_enabled(blob, node, 0);
+
+	return 0;
+}
+
+struct mvrtc_registers *mvrtc_get_config(void) {
+	const void     *blob = gd->blob;
+	struct fdt_rtc config;
+	int            node;
+	int	       index=0;
+
+	node = fdt_decode_next_alias(blob, "rtc",
+				     COMPAT_MARVELL_KIRKWOOD_RTC, &index);
+
+	if (node < 0)
+		return NULL;
+
+	if (fdt_decode_rtc(blob, node, &config))
+		return NULL;
+
+	return config.enabled ? (struct mvrtc_registers *)config.reg : NULL;
+}
+#endif
+
 int rtc_get(struct rtc_time *t)
 {
 	u32 time;
 	u32 date;
 	struct mvrtc_registers *mvrtc_regs;
 
-	mvrtc_regs = (struct mvrtc_registers *)KW_RTC_BASE;
+	mvrtc_regs = mvrtc_get_config();
+#ifdef CONFIG_OF_CONTROL
+	if (mvrtc_regs == NULL) {
+		printf("Error decoding fdt for mvrtc.\n");
+		return -1;
+	}
+#endif
 
 	/* read the time register */
 	time = readl(&mvrtc_regs->time);
@@ -79,7 +123,13 @@ int rtc_set(struct rtc_time *t)
 	u32 date = 0;
 	struct mvrtc_registers *mvrtc_regs;
 
-	mvrtc_regs = (struct mvrtc_registers *)KW_RTC_BASE;
+	mvrtc_regs = mvrtc_get_config();
+#ifdef CONFIG_OF_CONTROL
+	if (mvrtc_regs == NULL) {
+		printf("Error decoding fdt for mvrtc.\n");
+		return -1;
+	}
+#endif
 
 	/* check that this code isn't 80+ years old ;-) */
 	if ((t->tm_year / 100) != CENTURY)
@@ -111,7 +161,13 @@ void rtc_reset(void)
 	u32 sec;
 	struct mvrtc_registers *mvrtc_regs;
 
-	mvrtc_regs = (struct mvrtc_registers *)KW_RTC_BASE;
+	mvrtc_regs = mvrtc_get_config();
+#ifdef CONFIG_OF_CONTROL
+	if (mvrtc_regs == NULL) {
+		printf("Error decoding fdt for mvrtc.\n");
+		return;
+	}
+#endif
 
 	/* no init routine for this RTC needed, just check that it's working */
 	time = readl(&mvrtc_regs->time);
diff --git a/drivers/rtc/mvrtc.h b/drivers/rtc/mvrtc.h
index b9d5c6f..56b09f2 100644
--- a/drivers/rtc/mvrtc.h
+++ b/drivers/rtc/mvrtc.h
@@ -37,6 +37,13 @@ struct mvrtc_registers {
 	u32 date;
 };
 
+#ifdef CONFIG_OF_CONTROL
+struct fdt_rtc {
+	addr_t reg;  /* address of the registers */
+	int enabled; /* 1 if enabled, 0 if disabled */
+};
+#endif
+
 /* time register */
 #define MVRTC_SEC_SFT		0
 #define MVRTC_SEC_MSK		0x7f
diff --git a/include/fdt_decode.h b/include/fdt_decode.h
index 4264e3b..f236643 100644
--- a/include/fdt_decode.h
+++ b/include/fdt_decode.h
@@ -54,6 +54,7 @@ struct fdt_memory {
  */
 enum fdt_compat_id {
 	COMPAT_UNKNOWN,
+	COMPAT_MARVELL_KIRKWOOD_RTC,
 	COMPAT_COUNT,
 };
 
-- 
1.7.0.4

[U-Boot] [RFC PATCH 4/4 v1] dreamplug: enable fdt

[Jason Cooper]

Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
---
 board/Marvell/dreamplug/kirkwood-dreamplug.dts |   12 +++++++++++
 board/Marvell/dreamplug/kirkwood.dtsi          |   25 ++++++++++++++++++++++++
 include/configs/dreamplug.h                    |    5 ++++
 3 files changed, 42 insertions(+), 0 deletions(-)
 create mode 100644 board/Marvell/dreamplug/kirkwood-dreamplug.dts
 create mode 100644 board/Marvell/dreamplug/kirkwood.dtsi

diff --git a/board/Marvell/dreamplug/kirkwood-dreamplug.dts b/board/Marvell/dreamplug/kirkwood-dreamplug.dts
new file mode 100644
index 0000000..eb900c3
--- /dev/null
+++ b/board/Marvell/dreamplug/kirkwood-dreamplug.dts
@@ -0,0 +1,12 @@
+/dts-v1/;
+
+/include/ "kirkwood.dtsi"
+
+/ {
+	model = "Marvell Dreamplug";
+	compatible = "marvell,dreamplug", "marvell,kirkwood";
+
+	rtc at 0xf1010300 {
+		status = "ok";
+	};
+};
diff --git a/board/Marvell/dreamplug/kirkwood.dtsi b/board/Marvell/dreamplug/kirkwood.dtsi
new file mode 100644
index 0000000..15e52bd
--- /dev/null
+++ b/board/Marvell/dreamplug/kirkwood.dtsi
@@ -0,0 +1,25 @@
+/ {
+	model = "Marvell Kirkwood";
+	compatible = "marvell,kirkwood";
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		cpu at 0 {
+			compatible = "arm,arm926ejs";
+			reg = <0>;
+		};
+	};
+
+	rtc at 0xf1010300 {
+		compatible = "marvell,kirkwood-rtc";
+		reg = <0xf1010300 0x02>;
+		status = "disabled";
+	};
+
+	aliases {
+		rtc0 = "/rtc at 0xf1010300";
+	};
+};
diff --git a/include/configs/dreamplug.h b/include/configs/dreamplug.h
index 8d1b935..9960d35 100644
--- a/include/configs/dreamplug.h
+++ b/include/configs/dreamplug.h
@@ -50,6 +50,11 @@
 #define CONFIG_MACH_TYPE	MACH_TYPE_DREAMPLUG
 #define CONFIG_SKIP_LOWLEVEL_INIT	/* disable board lowlevel_init */
 
+#define CONFIG_OF_EMBED
+#define CONFIG_DEFAULT_DEVICE_TREE "kirkwood-dreamplug"
+#define CONFIG_OF_CONTROL
+#define CONFIG_OF_LIBFDT
+
 /*
  * Commands configuration
  */
-- 
1.7.0.4

[U-Boot] [RFC PATCH 0/4 v1] Use fdt to init mvrtc driver for dreamplug

[Simon Glass]

Hi Jason,

On Thu, Sep 15, 2011 at 6:54 AM, Jason Cooper <u-boot@lakedaemon.net> wrote:
> Simon, All,
>
> This patch series is my attempt to learn device tree with a driver I'm
> familiar with. ?Also, to create a real, working example for fdt in U-boot.

Great!
>
> To make this work, I applied v2 of Simon Glass' patch series (what this
> email is in reply to) against v2011.09-rc1. ?I then cherry picked my
> dreamplug board support patch. ?Last, I applied the following series.
>
> It works, although with CONFIG_OF_EMBED. ?I haven't tried _SEPARATE yet.

Please do, as that is what we really want people to use.

>
> Some thoughts:
>
> 1.) 'kirkwood-dreamplug.dts' can be renamed 'dreamplug.dts'. ?Due to
> ? ?machine_is_XXX 'dreamplug' will be unique, concise, and descriptive.
> ? ?This requires changes to Simon Glass' patchset.
>
> 2.) The fdt files should be moved up one directory level, in the end, it
> ? ?should look like:
>
> ? ?board/Marvell/armada100-dt.c
> ? ?board/Marvell/armada100-dt.h
> ? ?board/Marvell/armada100.dtsi
> ? ?board/Marvell/dreamplug.dts
> ? ?board/Marvell/gplugd.dts ? ? ?# this includes armada100.dtsi
> ? ?board/Marvell/guruplug.dts
> ? ?board/Marvell/kirkwood.dtsi
> ? ?board/Marvell/kirkwood-dt.c
> ? ?board/Marvell/kirkwood-dt.h
> ? ?board/Marvell/openrd.dts
> ? ?board/Marvell/sheevaplug.dts
> ? ?...
> ? ?board/Marvell/<other boards>.dts
>
> ? ?This also requires changes to Simon's patchset.

Hmm I'm not sure about that - perhaps at least a dts subdirectory?

>
> 3.) Since mach names (dreamplug) and SoC names (kirkwood) are unique,
> ? ?should we put all .dts and .dtsi files in one dir? ?eg /dts ? ?This
> ? ?would facilitate migration to Grant Likely's device-tree.git
> ? ?whereever and whenever it lands. ?This is also similar to how Linux
> ? ?currently does it (arch/arm/boot/dts/)

It does have advantages, although (hopefully) the number of files will
grow quite large. But bear in mind that people will create their own
.dtsi files, so we will need a strong naming convention there also
(always use the board prefix perhaps).

If we want to be similar to Linux they should go in arch/<arch>/dts,
but then we are putting board description files outside the board/
structure, which doesn't seem right. The arch/arm/ subdir is supposed
to be for arch-specific code which is generic across boards. Remember
that the dts will include board things, not just SOC things.

Clearly we don't want .dts files in boards/ and even boards/dts seems
odd since all the other files are in board/<vendor>. Hmm I quite like
having them in board/<vendor>/dts.

Regards,
Simon

>
> Jason Cooper (4):
> ?fdt: remove i2c example code.
> ?fdt_decode: make more available.
> ?mvrtc: add fdt support.
> ?dreamplug: enable fdt
>
> ?board/Marvell/dreamplug/kirkwood-dreamplug.dts | ? 12 +++++
> ?board/Marvell/dreamplug/kirkwood.dtsi ? ? ? ? ?| ? 25 ++++++++++
> ?common/fdt_decode.c ? ? ? ? ? ? ? ? ? ? ? ? ? ?| ? 26 ++--------
> ?drivers/rtc/mvrtc.c ? ? ? ? ? ? ? ? ? ? ? ? ? ?| ? 62 ++++++++++++++++++++++-
> ?drivers/rtc/mvrtc.h ? ? ? ? ? ? ? ? ? ? ? ? ? ?| ? ?7 +++
> ?include/configs/dreamplug.h ? ? ? ? ? ? ? ? ? ?| ? ?5 ++
> ?include/fdt_decode.h ? ? ? ? ? ? ? ? ? ? ? ? ? | ? 46 +++++------------
> ?7 files changed, 128 insertions(+), 55 deletions(-)
> ?create mode 100644 board/Marvell/dreamplug/kirkwood-dreamplug.dts
> ?create mode 100644 board/Marvell/dreamplug/kirkwood.dtsi
>
>

[U-Boot] [RFC PATCH 2/4 v1] fdt_decode: make more available.

[Simon Glass]

Hi Jason,

On Thu, Sep 15, 2011 at 6:54 AM, Jason Cooper <u-boot@lakedaemon.net> wrote:
>
> Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
> ---
> ?common/fdt_decode.c ?| ? 11 +++++------
> ?include/fdt_decode.h | ? 13 +++++++++++++
> ?2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/common/fdt_decode.c b/common/fdt_decode.c
> index cd7a071..0f13089 100644
> --- a/common/fdt_decode.c
> +++ b/common/fdt_decode.c
> @@ -33,7 +33,7 @@
> ?* 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] = {
> +const char *compat_names[COMPAT_COUNT] = {
> ?};
>
> ?/**
> @@ -43,7 +43,7 @@ static const char *compat_names[COMPAT_COUNT] = {
> ?* @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)
> +int find_alias_node(const void *blob, const char *name)

If we are going to export these (which I agree we must if we are to
move this code into drivers), then it should have the fdt_decode_
prefix.

Perhaps this is too verbose? I would be happy with
fdtdec_find_alias_node() if that sounds better?

> ?{
> ? ? ? ?const char *path;
> ? ? ? ?int alias_node;
> @@ -68,7 +68,7 @@ static int find_alias_node(const void *blob, const char *name)
> ?* @param prop_name ? ?name of property to find
> ?* @return address, if found, or ADDR_T_NONE if not
> ?*/
> -static addr_t get_addr(const void *blob, int node, const char *prop_name)
> +addr_t get_addr(const void *blob, int node, const char *prop_name)
> ?{
> ? ? ? ?const addr_t *cell;
> ? ? ? ?int len;
> @@ -91,7 +91,7 @@ static addr_t get_addr(const void *blob, int node, const char *prop_name)
> ?* @param default_val ?default value to return if the property is not found
> ?* @return integer value, if found, or default_val if not
> ?*/
> -static s32 get_int(const void *blob, int node, const char *prop_name,
> +s32 get_int(const void *blob, int node, const char *prop_name,
> ? ? ? ? ? ? ? ?s32 default_val)
> ?{
> ? ? ? ?const s32 *cell;
> @@ -115,7 +115,7 @@ static s32 get_int(const void *blob, int node, const char *prop_name,
> ?* @param default_val ?default value to return if no 'status' property exists
> ?* @return integer value 0/1, if found, or default_val if not
> ?*/
> -static int get_is_enabled(const void *blob, int node, int default_val)
> +int get_is_enabled(const void *blob, int node, int default_val)
> ?{
> ? ? ? ?const char *cell;
>
> @@ -160,4 +160,3 @@ int fdt_decode_next_alias(const void *blob, const char *name,
> ? ? ? ? ? ? ? ?return err;
> ? ? ? ?return err ? -FDT_ERR_MISSING : node;
> ?}
> -
> diff --git a/include/fdt_decode.h b/include/fdt_decode.h
> index ba3c15b..4264e3b 100644
> --- a/include/fdt_decode.h
> +++ b/include/fdt_decode.h
> @@ -19,6 +19,8 @@
> ?* MA 02111-1307 USA
> ?*/
>
> +#ifndef _FDT_DECODE_H_
> +#define _FDT_DECODE_H_
>
> ?/*
> ?* This file contains convenience functions for decoding useful and
> @@ -55,4 +57,15 @@ enum fdt_compat_id {
> ? ? ? ?COMPAT_COUNT,
> ?};
>
> +int find_alias_node(const void *blob, const char *name);
> +addr_t get_addr(const void *blob, int node, const char *prop_name);
> +s32 get_int(const void *blob, int node, const char *prop_name,
> + ? ? ? s32 default_val);
> +int get_is_enabled(const void *blob, int node, int default_val);
> +enum fdt_compat_id fdt_decode_lookup(const void *blob, int node);
> +int fdt_decode_next_compatible(const void *blob, int node,
> + ? ? ? ? ? ? ? enum fdt_compat_id id);
> +int fdt_decode_next_alias(const void *blob, const char *name,
> + ? ? ? ? ? ? ? enum fdt_compat_id id, int *upto);
>
> +#endif
> --
> 1.7.0.4
>
>

[U-Boot] [RFC PATCH 3/4 v1] mvrtc: add fdt support.

[Simon Glass]

Hi Jason,

On Thu, Sep 15, 2011 at 6:54 AM, Jason Cooper <u-boot@lakedaemon.net> wrote:
>
> Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
> ---
> ?common/fdt_decode.c ?| ? ?1 +
> ?drivers/rtc/mvrtc.c ?| ? 62 +++++++++++++++++++++++++++++++++++++++++++++++--
> ?drivers/rtc/mvrtc.h ?| ? ?7 +++++
> ?include/fdt_decode.h | ? ?1 +
> ?4 files changed, 68 insertions(+), 3 deletions(-)
>
> diff --git a/common/fdt_decode.c b/common/fdt_decode.c
> index 0f13089..1a0dcf4 100644
> --- a/common/fdt_decode.c
> +++ b/common/fdt_decode.c
> @@ -34,6 +34,7 @@
> ?*/
> ?#define COMPAT(id, name) name
> ?const char *compat_names[COMPAT_COUNT] = {
> + ? ? ? COMPAT(MARVELL_KIRKWOOD_RTC, "marvell,kirkwood-rtc"),
> ?};
>
> ?/**
> diff --git a/drivers/rtc/mvrtc.c b/drivers/rtc/mvrtc.c
> index ccc573a..ce2dc3d 100644
> --- a/drivers/rtc/mvrtc.c
> +++ b/drivers/rtc/mvrtc.c
> @@ -28,18 +28,62 @@
> ?#include <common.h>
> ?#include <command.h>
> ?#include <rtc.h>
> +#ifdef CONFIG_OF_CONTROL
> +#include <fdt_decode.h>
> +#endif
> ?#include "mvrtc.h"
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
> ?/* This RTC does not support century, so we assume 20 */
> ?#define CENTURY 20
>
> +#ifndef CONFIG_OF_CONTROL

Perhaps #ifdef and put the fdt code first?

> +struct mvrtc_registers *mvrtc_get_config(void) {
> + ? ? ? return (struct mvrtc_registers *)KW_RTC_BASE;
> +}
> +
> +#else
> +int fdt_decode_rtc(const void *blob, int node, struct fdt_rtc *config)
> +{
> + ? ? ? config->reg = get_addr(blob, node, "reg");
> + ? ? ? config->enabled = get_is_enabled(blob, node, 0);
> +
> + ? ? ? return 0;
> +}
> +
> +struct mvrtc_registers *mvrtc_get_config(void) {
> + ? ? ? const void ? ? *blob = gd->blob;
> + ? ? ? struct fdt_rtc config;
> + ? ? ? int ? ? ? ? ? ?node;
> + ? ? ? int ? ? ? ? ? ?index=0;
> +
> + ? ? ? node = fdt_decode_next_alias(blob, "rtc",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?COMPAT_MARVELL_KIRKWOOD_RTC, &index);
> +
> + ? ? ? if (node < 0)
> + ? ? ? ? ? ? ? return NULL;
> +
> + ? ? ? if (fdt_decode_rtc(blob, node, &config))
> + ? ? ? ? ? ? ? return NULL;
> +
> + ? ? ? return config.enabled ? (struct mvrtc_registers *)config.reg : NULL;
> +}
> +#endif
> +
> ?int rtc_get(struct rtc_time *t)
> ?{
> ? ? ? ?u32 time;
> ? ? ? ?u32 date;
> ? ? ? ?struct mvrtc_registers *mvrtc_regs;
>
> - ? ? ? mvrtc_regs = (struct mvrtc_registers *)KW_RTC_BASE;
> + ? ? ? mvrtc_regs = mvrtc_get_config();
> +#ifdef CONFIG_OF_CONTROL
> + ? ? ? if (mvrtc_regs == NULL) {
> + ? ? ? ? ? ? ? printf("Error decoding fdt for mvrtc.\n");
> + ? ? ? ? ? ? ? return -1;
> + ? ? ? }
> +#endif
>
> ? ? ? ?/* read the time register */
> ? ? ? ?time = readl(&mvrtc_regs->time);
> @@ -79,7 +123,13 @@ int rtc_set(struct rtc_time *t)
> ? ? ? ?u32 date = 0;
> ? ? ? ?struct mvrtc_registers *mvrtc_regs;
>
> - ? ? ? mvrtc_regs = (struct mvrtc_registers *)KW_RTC_BASE;
> + ? ? ? mvrtc_regs = mvrtc_get_config();
> +#ifdef CONFIG_OF_CONTROL
> + ? ? ? if (mvrtc_regs == NULL) {
> + ? ? ? ? ? ? ? printf("Error decoding fdt for mvrtc.\n");
> + ? ? ? ? ? ? ? return -1;
> + ? ? ? }
> +#endif
>
> ? ? ? ?/* check that this code isn't 80+ years old ;-) */
> ? ? ? ?if ((t->tm_year / 100) != CENTURY)
> @@ -111,7 +161,13 @@ void rtc_reset(void)
> ? ? ? ?u32 sec;
> ? ? ? ?struct mvrtc_registers *mvrtc_regs;
>
> - ? ? ? mvrtc_regs = (struct mvrtc_registers *)KW_RTC_BASE;
> + ? ? ? mvrtc_regs = mvrtc_get_config();
> +#ifdef CONFIG_OF_CONTROL
> + ? ? ? if (mvrtc_regs == NULL) {
> + ? ? ? ? ? ? ? printf("Error decoding fdt for mvrtc.\n");
> + ? ? ? ? ? ? ? return;
> + ? ? ? }
> +#endif
>
> ? ? ? ?/* no init routine for this RTC needed, just check that it's working */

Hmm I think it would be better to decode the fdt once in init, and
store it in your structure. It seems like you are doing it each time
the driver is called.

> ? ? ? ?time = readl(&mvrtc_regs->time);
> diff --git a/drivers/rtc/mvrtc.h b/drivers/rtc/mvrtc.h
> index b9d5c6f..56b09f2 100644
> --- a/drivers/rtc/mvrtc.h
> +++ b/drivers/rtc/mvrtc.h
> @@ -37,6 +37,13 @@ struct mvrtc_registers {
> ? ? ? ?u32 date;
> ?};
>
> +#ifdef CONFIG_OF_CONTROL
> +struct fdt_rtc {
> + ? ? ? addr_t reg; ?/* address of the registers */
> + ? ? ? int enabled; /* 1 if enabled, 0 if disabled */
> +};

It seems to be that this structure should generally only be needed in
the C file, so should perhaps go there.

> +#endif
> +
> ?/* time register */
> ?#define MVRTC_SEC_SFT ? ? ? ? ?0
> ?#define MVRTC_SEC_MSK ? ? ? ? ?0x7f
> diff --git a/include/fdt_decode.h b/include/fdt_decode.h
> index 4264e3b..f236643 100644
> --- a/include/fdt_decode.h
> +++ b/include/fdt_decode.h
> @@ -54,6 +54,7 @@ struct fdt_memory {
> ?*/
> ?enum fdt_compat_id {
> ? ? ? ?COMPAT_UNKNOWN,
> + ? ? ? COMPAT_MARVELL_KIRKWOOD_RTC,

My cunning plan is that this can be some sort of driver ID and could
serve as a lead in to a unified device model (it provides a simple
enumeration of available drivers). However, perhaps we should discuss
this separately.

Regards,
Simon

> ? ? ? ?COMPAT_COUNT,
> ?};
>
> --
> 1.7.0.4
>
>

[U-Boot] [RFC PATCH 4/4 v1] dreamplug: enable fdt

[Simon Glass]

Hi Jason,

On Thu, Sep 15, 2011 at 6:54 AM, Jason Cooper <u-boot@lakedaemon.net> wrote:
>
> Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
> ---
> ?board/Marvell/dreamplug/kirkwood-dreamplug.dts | ? 12 +++++++++++
> ?board/Marvell/dreamplug/kirkwood.dtsi ? ? ? ? ?| ? 25 ++++++++++++++++++++++++
> ?include/configs/dreamplug.h ? ? ? ? ? ? ? ? ? ?| ? ?5 ++++
> ?3 files changed, 42 insertions(+), 0 deletions(-)
> ?create mode 100644 board/Marvell/dreamplug/kirkwood-dreamplug.dts
> ?create mode 100644 board/Marvell/dreamplug/kirkwood.dtsi
>
> diff --git a/board/Marvell/dreamplug/kirkwood-dreamplug.dts b/board/Marvell/dreamplug/kirkwood-dreamplug.dts
> new file mode 100644
> index 0000000..eb900c3
> --- /dev/null
> +++ b/board/Marvell/dreamplug/kirkwood-dreamplug.dts
> @@ -0,0 +1,12 @@
> +/dts-v1/;
> +
> +/include/ "kirkwood.dtsi"
> +
> +/ {
> + ? ? ? model = "Marvell Dreamplug";
> + ? ? ? compatible = "marvell,dreamplug", "marvell,kirkwood";
> +
> + ? ? ? rtc at 0xf1010300 {
> + ? ? ? ? ? ? ? status = "ok";
> + ? ? ? };
> +};
> diff --git a/board/Marvell/dreamplug/kirkwood.dtsi b/board/Marvell/dreamplug/kirkwood.dtsi
> new file mode 100644
> index 0000000..15e52bd
> --- /dev/null
> +++ b/board/Marvell/dreamplug/kirkwood.dtsi
> @@ -0,0 +1,25 @@
> +/ {
> + ? ? ? model = "Marvell Kirkwood";
> + ? ? ? compatible = "marvell,kirkwood";
> + ? ? ? #address-cells = <1>;
> + ? ? ? #size-cells = <1>;
> +
> + ? ? ? cpus {
> + ? ? ? ? ? ? ? #address-cells = <1>;
> + ? ? ? ? ? ? ? #size-cells = <0>;
> + ? ? ? ? ? ? ? cpu at 0 {
> + ? ? ? ? ? ? ? ? ? ? ? compatible = "arm,arm926ejs";
> + ? ? ? ? ? ? ? ? ? ? ? reg = <0>;
> + ? ? ? ? ? ? ? };
> + ? ? ? };
> +
> + ? ? ? rtc at 0xf1010300 {
> + ? ? ? ? ? ? ? compatible = "marvell,kirkwood-rtc";
> + ? ? ? ? ? ? ? reg = <0xf1010300 0x02>;
> + ? ? ? ? ? ? ? status = "disabled";
> + ? ? ? };
> +
> + ? ? ? aliases {
> + ? ? ? ? ? ? ? rtc0 = "/rtc at 0xf1010300";
> + ? ? ? };
> +};
> diff --git a/include/configs/dreamplug.h b/include/configs/dreamplug.h
> index 8d1b935..9960d35 100644
> --- a/include/configs/dreamplug.h
> +++ b/include/configs/dreamplug.h
> @@ -50,6 +50,11 @@
> ?#define CONFIG_MACH_TYPE ? ? ? MACH_TYPE_DREAMPLUG
> ?#define CONFIG_SKIP_LOWLEVEL_INIT ? ? ?/* disable board lowlevel_init */
>
> +#define CONFIG_OF_EMBED
> +#define CONFIG_DEFAULT_DEVICE_TREE "kirkwood-dreamplug"

One of my experiments was to create this automatically from
<vendor>-<board>.dts, so that this isn't needed explicitly. Is it
better to require a CONFIG for this, or just use the expected name?

Regards,
Simon

> +#define CONFIG_OF_CONTROL
> +#define CONFIG_OF_LIBFDT
> +
> ?/*
> ?* Commands configuration
> ?*/
> --
> 1.7.0.4
>
>

[U-Boot] [RFC PATCH 0/4 v1] Use fdt to init mvrtc driver for dreamplug

[Jason]

On Thu, Sep 15, 2011 at 12:16:11PM -0700, Simon Glass wrote:
> On Thu, Sep 15, 2011 at 6:54 AM, Jason Cooper <u-boot@lakedaemon.net> wrote:
> > To make this work, I applied v2 of Simon Glass' patch series (what this
> > email is in reply to) against v2011.09-rc1. ?I then cherry picked my
> > dreamplug board support patch. ?Last, I applied the following series.
> >
> > It works, although with CONFIG_OF_EMBED. ?I haven't tried _SEPARATE yet.
> 
> Please do, as that is what we really want people to use.

Ok.
 
> > Some thoughts:
...
> > 3.) Since mach names (dreamplug) and SoC names (kirkwood) are unique,
> > ? ?should we put all .dts and .dtsi files in one dir? ?eg /dts ? ?This
> > ? ?would facilitate migration to Grant Likely's device-tree.git
> > ? ?whereever and whenever it lands. ?This is also similar to how Linux
> > ? ?currently does it (arch/arm/boot/dts/)
> 
> It does have advantages, although (hopefully) the number of files will
> grow quite large. But bear in mind that people will create their own
> .dtsi files, so we will need a strong naming convention there also
> (always use the board prefix perhaps).

Wouldn't they be creating their own board .dts files, that /include/s
the SoC .dtsi?

> If we want to be similar to Linux they should go in arch/<arch>/dts,
> but then we are putting board description files outside the board/
> structure, which doesn't seem right. The arch/arm/ subdir is supposed
> to be for arch-specific code which is generic across boards. Remember
> that the dts will include board things, not just SOC things.
> 
> Clearly we don't want .dts files in boards/ and even boards/dts seems
> odd since all the other files are in board/<vendor>. Hmm I quite like
> having them in board/<vendor>/dts.

Funny, that's exactly what I did at first (board/Marvell/dts/) when
creating this patchset.  It failed to build, so I conformed just to get
it working.  So, board/<vendor>/dts/ feels natural.  Must be right,
then. ;-)

I'm currently working on my second driver (mvgbe ethernet) and having a
good time trying to find the interrupt controller for kirkwood.  I'll
try _SEPARATE after I get something testable for it.

thx,

Jason.

[U-Boot] [RFC PATCH 2/4 v1] fdt_decode: make more available.

[Jason]

On Thu, Sep 15, 2011 at 12:18:00PM -0700, Simon Glass wrote:
> On Thu, Sep 15, 2011 at 6:54 AM, Jason Cooper <u-boot@lakedaemon.net> wrote:
> >
> > Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
> > ---
> > ?common/fdt_decode.c ?| ? 11 +++++------
> > ?include/fdt_decode.h | ? 13 +++++++++++++
> > ?2 files changed, 18 insertions(+), 6 deletions(-)
> >
> > diff --git a/common/fdt_decode.c b/common/fdt_decode.c
> > index cd7a071..0f13089 100644
> > --- a/common/fdt_decode.c
> > +++ b/common/fdt_decode.c
> > @@ -33,7 +33,7 @@
> > ?* 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] = {
> > +const char *compat_names[COMPAT_COUNT] = {
> > ?};
> >
> > ?/**
> > @@ -43,7 +43,7 @@ static const char *compat_names[COMPAT_COUNT] = {
> > ?* @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)
> > +int find_alias_node(const void *blob, const char *name)
> 
> If we are going to export these (which I agree we must if we are to
> move this code into drivers), then it should have the fdt_decode_
> prefix.
> 
> Perhaps this is too verbose? I would be happy with
> fdtdec_find_alias_node() if that sounds better?

I like this.

> > ?{
> > ? ? ? ?const char *path;
> > ? ? ? ?int alias_node;
> > @@ -68,7 +68,7 @@ static int find_alias_node(const void *blob, const char *name)
> > ?* @param prop_name ? ?name of property to find
> > ?* @return address, if found, or ADDR_T_NONE if not
> > ?*/
> > -static addr_t get_addr(const void *blob, int node, const char *prop_name)
> > +addr_t get_addr(const void *blob, int node, const char *prop_name)
> > ?{
> > ? ? ? ?const addr_t *cell;
> > ? ? ? ?int len;
> > @@ -91,7 +91,7 @@ static addr_t get_addr(const void *blob, int node, const char *prop_name)
> > ?* @param default_val ?default value to return if the property is not found
> > ?* @return integer value, if found, or default_val if not
> > ?*/
> > -static s32 get_int(const void *blob, int node, const char *prop_name,
> > +s32 get_int(const void *blob, int node, const char *prop_name,
> > ? ? ? ? ? ? ? ?s32 default_val)
> > ?{
> > ? ? ? ?const s32 *cell;
> > @@ -115,7 +115,7 @@ static s32 get_int(const void *blob, int node, const char *prop_name,
> > ?* @param default_val ?default value to return if no 'status' property exists
> > ?* @return integer value 0/1, if found, or default_val if not
> > ?*/
> > -static int get_is_enabled(const void *blob, int node, int default_val)
> > +int get_is_enabled(const void *blob, int node, int default_val)
> > ?{
> > ? ? ? ?const char *cell;
> >
> > @@ -160,4 +160,3 @@ int fdt_decode_next_alias(const void *blob, const char *name,
> > ? ? ? ? ? ? ? ?return err;
> > ? ? ? ?return err ? -FDT_ERR_MISSING : node;
> > ?}
> > -
> > diff --git a/include/fdt_decode.h b/include/fdt_decode.h
> > index ba3c15b..4264e3b 100644
> > --- a/include/fdt_decode.h
> > +++ b/include/fdt_decode.h
> > @@ -19,6 +19,8 @@
> > ?* MA 02111-1307 USA
> > ?*/
> >
> > +#ifndef _FDT_DECODE_H_
> > +#define _FDT_DECODE_H_
> >
> > ?/*
> > ?* This file contains convenience functions for decoding useful and
> > @@ -55,4 +57,15 @@ enum fdt_compat_id {
> > ? ? ? ?COMPAT_COUNT,
> > ?};
> >
> > +int find_alias_node(const void *blob, const char *name);
> > +addr_t get_addr(const void *blob, int node, const char *prop_name);
> > +s32 get_int(const void *blob, int node, const char *prop_name,
> > + ? ? ? s32 default_val);
> > +int get_is_enabled(const void *blob, int node, int default_val);
> > +enum fdt_compat_id fdt_decode_lookup(const void *blob, int node);
> > +int fdt_decode_next_compatible(const void *blob, int node,
> > + ? ? ? ? ? ? ? enum fdt_compat_id id);
> > +int fdt_decode_next_alias(const void *blob, const char *name,
> > + ? ? ? ? ? ? ? enum fdt_compat_id id, int *upto);
> >
> > +#endif
> > --
> > 1.7.0.4
> >
> >

[U-Boot] [RFC PATCH 4/4 v1] dreamplug: enable fdt

[Jason]

On Thu, Sep 15, 2011 at 12:25:36PM -0700, Simon Glass wrote:
> On Thu, Sep 15, 2011 at 6:54 AM, Jason Cooper <u-boot@lakedaemon.net> wrote:
> >
> > Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
> > ---
> > ?board/Marvell/dreamplug/kirkwood-dreamplug.dts | ? 12 +++++++++++
> > ?board/Marvell/dreamplug/kirkwood.dtsi ? ? ? ? ?| ? 25 ++++++++++++++++++++++++
> > ?include/configs/dreamplug.h ? ? ? ? ? ? ? ? ? ?| ? ?5 ++++
> > ?3 files changed, 42 insertions(+), 0 deletions(-)
> > ?create mode 100644 board/Marvell/dreamplug/kirkwood-dreamplug.dts
> > ?create mode 100644 board/Marvell/dreamplug/kirkwood.dtsi
> >
> > diff --git a/board/Marvell/dreamplug/kirkwood-dreamplug.dts b/board/Marvell/dreamplug/kirkwood-dreamplug.dts
> > new file mode 100644
> > index 0000000..eb900c3
> > --- /dev/null
> > +++ b/board/Marvell/dreamplug/kirkwood-dreamplug.dts
> > @@ -0,0 +1,12 @@
> > +/dts-v1/;
> > +
> > +/include/ "kirkwood.dtsi"
> > +
> > +/ {
> > + ? ? ? model = "Marvell Dreamplug";
> > + ? ? ? compatible = "marvell,dreamplug", "marvell,kirkwood";
> > +
> > + ? ? ? rtc at 0xf1010300 {
> > + ? ? ? ? ? ? ? status = "ok";
> > + ? ? ? };
> > +};
> > diff --git a/board/Marvell/dreamplug/kirkwood.dtsi b/board/Marvell/dreamplug/kirkwood.dtsi
> > new file mode 100644
> > index 0000000..15e52bd
> > --- /dev/null
> > +++ b/board/Marvell/dreamplug/kirkwood.dtsi
> > @@ -0,0 +1,25 @@
> > +/ {
> > + ? ? ? model = "Marvell Kirkwood";
> > + ? ? ? compatible = "marvell,kirkwood";
> > + ? ? ? #address-cells = <1>;
> > + ? ? ? #size-cells = <1>;
> > +
> > + ? ? ? cpus {
> > + ? ? ? ? ? ? ? #address-cells = <1>;
> > + ? ? ? ? ? ? ? #size-cells = <0>;
> > + ? ? ? ? ? ? ? cpu at 0 {
> > + ? ? ? ? ? ? ? ? ? ? ? compatible = "arm,arm926ejs";
> > + ? ? ? ? ? ? ? ? ? ? ? reg = <0>;
> > + ? ? ? ? ? ? ? };
> > + ? ? ? };
> > +
> > + ? ? ? rtc at 0xf1010300 {
> > + ? ? ? ? ? ? ? compatible = "marvell,kirkwood-rtc";
> > + ? ? ? ? ? ? ? reg = <0xf1010300 0x02>;
> > + ? ? ? ? ? ? ? status = "disabled";
> > + ? ? ? };
> > +
> > + ? ? ? aliases {
> > + ? ? ? ? ? ? ? rtc0 = "/rtc at 0xf1010300";
> > + ? ? ? };
> > +};
> > diff --git a/include/configs/dreamplug.h b/include/configs/dreamplug.h
> > index 8d1b935..9960d35 100644
> > --- a/include/configs/dreamplug.h
> > +++ b/include/configs/dreamplug.h
> > @@ -50,6 +50,11 @@
> > ?#define CONFIG_MACH_TYPE ? ? ? MACH_TYPE_DREAMPLUG
> > ?#define CONFIG_SKIP_LOWLEVEL_INIT ? ? ?/* disable board lowlevel_init */
> >
> > +#define CONFIG_OF_EMBED
> > +#define CONFIG_DEFAULT_DEVICE_TREE "kirkwood-dreamplug"
> 
> One of my experiments was to create this automatically from
> <vendor>-<board>.dts, so that this isn't needed explicitly. Is it
> better to require a CONFIG for this, or just use the expected name?

There is a large probability that users may roll their own dts files.
Until device-tree.git is a reality, I'd say keep it so users can specify
which dts file they want to build.

thx,

Jason.

[U-Boot] [RFC PATCH 3/4 v1] mvrtc: add fdt support.

[Jason]

On Thu, Sep 15, 2011 at 12:23:52PM -0700, Simon Glass wrote:
> On Thu, Sep 15, 2011 at 6:54 AM, Jason Cooper <u-boot@lakedaemon.net> wrote:
> >
> > Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
> > ---
> > ?common/fdt_decode.c ?| ? ?1 +
> > ?drivers/rtc/mvrtc.c ?| ? 62 +++++++++++++++++++++++++++++++++++++++++++++++--
> > ?drivers/rtc/mvrtc.h ?| ? ?7 +++++
> > ?include/fdt_decode.h | ? ?1 +
> > ?4 files changed, 68 insertions(+), 3 deletions(-)
> >
> > diff --git a/common/fdt_decode.c b/common/fdt_decode.c
> > index 0f13089..1a0dcf4 100644
> > --- a/common/fdt_decode.c
> > +++ b/common/fdt_decode.c
> > @@ -34,6 +34,7 @@
> > ?*/
> > ?#define COMPAT(id, name) name
> > ?const char *compat_names[COMPAT_COUNT] = {
> > + ? ? ? COMPAT(MARVELL_KIRKWOOD_RTC, "marvell,kirkwood-rtc"),
> > ?};
> >
> > ?/**
> > diff --git a/drivers/rtc/mvrtc.c b/drivers/rtc/mvrtc.c
> > index ccc573a..ce2dc3d 100644
> > --- a/drivers/rtc/mvrtc.c
> > +++ b/drivers/rtc/mvrtc.c
> > @@ -28,18 +28,62 @@
> > ?#include <common.h>
> > ?#include <command.h>
> > ?#include <rtc.h>
> > +#ifdef CONFIG_OF_CONTROL
> > +#include <fdt_decode.h>
> > +#endif
> > ?#include "mvrtc.h"
> >
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > ?/* This RTC does not support century, so we assume 20 */
> > ?#define CENTURY 20
> >
> > +#ifndef CONFIG_OF_CONTROL
> 
> Perhaps #ifdef and put the fdt code first?

Sure.  For larger drivers, I'd prefer to see a separate file.  eg:

mvgbe.c
mvgbe-dt.c

with mvgbe.h selecting _init() or _init_fdt() on CONFIG_OF_CONTROL.  Not
sure, gotta think about it more.
 
> > +struct mvrtc_registers *mvrtc_get_config(void) {
> > + ? ? ? return (struct mvrtc_registers *)KW_RTC_BASE;
> > +}
> > +
> > +#else
> > +int fdt_decode_rtc(const void *blob, int node, struct fdt_rtc *config)
> > +{
> > + ? ? ? config->reg = get_addr(blob, node, "reg");
> > + ? ? ? config->enabled = get_is_enabled(blob, node, 0);
> > +
> > + ? ? ? return 0;
> > +}
> > +
> > +struct mvrtc_registers *mvrtc_get_config(void) {
> > + ? ? ? const void ? ? *blob = gd->blob;
> > + ? ? ? struct fdt_rtc config;
> > + ? ? ? int ? ? ? ? ? ?node;
> > + ? ? ? int ? ? ? ? ? ?index=0;
> > +
> > + ? ? ? node = fdt_decode_next_alias(blob, "rtc",
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?COMPAT_MARVELL_KIRKWOOD_RTC, &index);
> > +
> > + ? ? ? if (node < 0)
> > + ? ? ? ? ? ? ? return NULL;
> > +
> > + ? ? ? if (fdt_decode_rtc(blob, node, &config))
> > + ? ? ? ? ? ? ? return NULL;
> > +
> > + ? ? ? return config.enabled ? (struct mvrtc_registers *)config.reg : NULL;
> > +}
> > +#endif
> > +
> > ?int rtc_get(struct rtc_time *t)
> > ?{
> > ? ? ? ?u32 time;
> > ? ? ? ?u32 date;
> > ? ? ? ?struct mvrtc_registers *mvrtc_regs;
> >
> > - ? ? ? mvrtc_regs = (struct mvrtc_registers *)KW_RTC_BASE;
> > + ? ? ? mvrtc_regs = mvrtc_get_config();
> > +#ifdef CONFIG_OF_CONTROL
> > + ? ? ? if (mvrtc_regs == NULL) {
> > + ? ? ? ? ? ? ? printf("Error decoding fdt for mvrtc.\n");
> > + ? ? ? ? ? ? ? return -1;
> > + ? ? ? }
> > +#endif
> >
> > ? ? ? ?/* read the time register */
> > ? ? ? ?time = readl(&mvrtc_regs->time);
> > @@ -79,7 +123,13 @@ int rtc_set(struct rtc_time *t)
> > ? ? ? ?u32 date = 0;
> > ? ? ? ?struct mvrtc_registers *mvrtc_regs;
> >
> > - ? ? ? mvrtc_regs = (struct mvrtc_registers *)KW_RTC_BASE;
> > + ? ? ? mvrtc_regs = mvrtc_get_config();
> > +#ifdef CONFIG_OF_CONTROL
> > + ? ? ? if (mvrtc_regs == NULL) {
> > + ? ? ? ? ? ? ? printf("Error decoding fdt for mvrtc.\n");
> > + ? ? ? ? ? ? ? return -1;
> > + ? ? ? }
> > +#endif
> >
> > ? ? ? ?/* check that this code isn't 80+ years old ;-) */
> > ? ? ? ?if ((t->tm_year / 100) != CENTURY)
> > @@ -111,7 +161,13 @@ void rtc_reset(void)
> > ? ? ? ?u32 sec;
> > ? ? ? ?struct mvrtc_registers *mvrtc_regs;
> >
> > - ? ? ? mvrtc_regs = (struct mvrtc_registers *)KW_RTC_BASE;
> > + ? ? ? mvrtc_regs = mvrtc_get_config();
> > +#ifdef CONFIG_OF_CONTROL
> > + ? ? ? if (mvrtc_regs == NULL) {
> > + ? ? ? ? ? ? ? printf("Error decoding fdt for mvrtc.\n");
> > + ? ? ? ? ? ? ? return;
> > + ? ? ? }
> > +#endif
> >
> > ? ? ? ?/* no init routine for this RTC needed, just check that it's working */
> 
> Hmm I think it would be better to decode the fdt once in init, and
> store it in your structure. It seems like you are doing it each time
> the driver is called.

Yes, it's a stupid/simple driver.  There is definitely room for
optimization.

> > ? ? ? ?time = readl(&mvrtc_regs->time);
> > diff --git a/drivers/rtc/mvrtc.h b/drivers/rtc/mvrtc.h
> > index b9d5c6f..56b09f2 100644
> > --- a/drivers/rtc/mvrtc.h
> > +++ b/drivers/rtc/mvrtc.h
> > @@ -37,6 +37,13 @@ struct mvrtc_registers {
> > ? ? ? ?u32 date;
> > ?};
> >
> > +#ifdef CONFIG_OF_CONTROL
> > +struct fdt_rtc {
> > + ? ? ? addr_t reg; ?/* address of the registers */
> > + ? ? ? int enabled; /* 1 if enabled, 0 if disabled */
> > +};
> 
> It seems to be that this structure should generally only be needed in
> the C file, so should perhaps go there.

true.

> > +#endif
> > +
> > ?/* time register */
> > ?#define MVRTC_SEC_SFT ? ? ? ? ?0
> > ?#define MVRTC_SEC_MSK ? ? ? ? ?0x7f
> > diff --git a/include/fdt_decode.h b/include/fdt_decode.h
> > index 4264e3b..f236643 100644
> > --- a/include/fdt_decode.h
> > +++ b/include/fdt_decode.h
> > @@ -54,6 +54,7 @@ struct fdt_memory {
> > ?*/
> > ?enum fdt_compat_id {
> > ? ? ? ?COMPAT_UNKNOWN,
> > + ? ? ? COMPAT_MARVELL_KIRKWOOD_RTC,
> 
> My cunning plan is that this can be some sort of driver ID and could
> serve as a lead in to a unified device model (it provides a simple
> enumeration of available drivers).

That's on the list of words that scare me.  ;-)  Right up there with
perfect, minimized, maximized, secure, and user-friendly.

> However, perhaps we should discuss this separately.

Definitely.

thx,

Jason.

[U-Boot] [RFC PATCH 1/4 v1] fdt: remove i2c example code.

[Kumar Gala]

On Sep 15, 2011, at 8:54 AM, Jason Cooper wrote:

> 
> Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
> ---
> common/fdt_decode.c  |   14 --------------
> include/fdt_decode.h |   32 --------------------------------
> 2 files changed, 0 insertions(+), 46 deletions(-)

Did I miss where these files were added to u-boot?

Probably good in general to have some commit message so someone in the future has some idea why this change was made.

- k

[U-Boot] [RFC PATCH 1/4 v1] fdt: remove i2c example code.

[Jason]

Kumar,

On Fri, Sep 16, 2011 at 02:31:32AM -0500, Kumar Gala wrote:
> On Sep 15, 2011, at 8:54 AM, Jason Cooper wrote:
> > 
> > Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
> > ---
> > common/fdt_decode.c  |   14 --------------
> > include/fdt_decode.h |   32 --------------------------------
> > 2 files changed, 0 insertions(+), 46 deletions(-)
> 
> Did I miss where these files were added to u-boot?

No, I sent this series in-reply-to Simon Glass' RFC series adding
fdt_decode.{c,h}.  My cover letter details how I got to here.

> Probably good in general to have some commit message so someone in the
> future has some idea why this change was made.

Very true, this code is _way_ too early to consider committing (both
Simon's and mine).  Once we remove RFC, I'll make sure to have full
commit logs.  This series will most likely get squashed into one patch
with one or two of them being merged into Simon's series.

hth,

Jason.

[U-Boot] [RFC PATCH 3/4 v1] mvrtc: add fdt support.

[Wolfgang Denk]

Dear Jason Cooper,

In message <6d40c2a90839e1f8dae389a562ea20ed2bd15083.1316092940.git.u-boot@lakedaemon.net> you wrote:
> 
> Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
> ---
>  common/fdt_decode.c  |    1 +
>  drivers/rtc/mvrtc.c  |   62 +++++++++++++++++++++++++++++++++++++++++++++++--
>  drivers/rtc/mvrtc.h  |    7 +++++
>  include/fdt_decode.h |    1 +
>  4 files changed, 68 insertions(+), 3 deletions(-)


Checkpatch says:

total: 3 errors, 0 warnings, 118 lines checked

Please clean up and resubmit.  Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The Buddha, the Godhead, resides quite as comfortably in the circuits
of a digital computer or the gears of a cycle transmission as he does
at the top of a mountain or in the petals of a flower.
            - R.  Pirsig, "Zen and the Art of Motorcycle Maintenance"

[U-Boot] [RFC PATCH 3/4 v1] mvrtc: add fdt support.

[Simon Glass]

Hi Wolfgang,

On Thu, Oct 6, 2011 at 2:31 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Jason Cooper,
>
> In message <6d40c2a90839e1f8dae389a562ea20ed2bd15083.1316092940.git.u-boot@lakedaemon.net> you wrote:
>>
>> Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
>> ---
>> ?common/fdt_decode.c ?| ? ?1 +
>> ?drivers/rtc/mvrtc.c ?| ? 62 +++++++++++++++++++++++++++++++++++++++++++++++--
>> ?drivers/rtc/mvrtc.h ?| ? ?7 +++++
>> ?include/fdt_decode.h | ? ?1 +
>> ?4 files changed, 68 insertions(+), 3 deletions(-)
>

FYI I plan to do a new version of the base fdt patch in the next few
days which addresses comments, etc.

Regards,
Simon

>
> Checkpatch says:
>
> total: 3 errors, 0 warnings, 118 lines checked
>
> Please clean up and resubmit. ?Thanks.
>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> The Buddha, the Godhead, resides quite as comfortably in the circuits
> of a digital computer or the gears of a cycle transmission as he does
> at the top of a mountain or in the petals of a flower.
> ? ? ? ? ? ?- R. ?Pirsig, "Zen and the Art of Motorcycle Maintenance"
>

[U-Boot] [RFC PATCH 3/4 v1] mvrtc: add fdt support.

[Simon Glass]

Hi Jason,

On Thu, Oct 6, 2011 at 2:42 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Wolfgang,
>
> On Thu, Oct 6, 2011 at 2:31 PM, Wolfgang Denk <wd@denx.de> wrote:
>> Dear Jason Cooper,
>>
>> In message <6d40c2a90839e1f8dae389a562ea20ed2bd15083.1316092940.git.u-boot@lakedaemon.net> you wrote:
>>>
>>> Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
>>> ---
>>> ?common/fdt_decode.c ?| ? ?1 +
>>> ?drivers/rtc/mvrtc.c ?| ? 62 +++++++++++++++++++++++++++++++++++++++++++++++--
>>> ?drivers/rtc/mvrtc.h ?| ? ?7 +++++
>>> ?include/fdt_decode.h | ? ?1 +
>>> ?4 files changed, 68 insertions(+), 3 deletions(-)
>>
>
> FYI I plan to do a new version of the base fdt patch in the next few
> days which addresses comments, etc.
>

I have done this now - if you have time please rebase and try again.

Regards,
Simon

>>
>> Checkpatch says:
>>
>> total: 3 errors, 0 warnings, 118 lines checked
>>
>> Please clean up and resubmit. ?Thanks.
>>
>> Best regards,
>>
>> Wolfgang Denk
>>
>> --
>> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
>> The Buddha, the Godhead, resides quite as comfortably in the circuits
>> of a digital computer or the gears of a cycle transmission as he does
>> at the top of a mountain or in the petals of a flower.
>> ? ? ? ? ? ?- R. ?Pirsig, "Zen and the Art of Motorcycle Maintenance"
>>
>