diff mbox

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

Message ID 1315865067-1443-3-git-send-email-sjg@chromium.org
State New, archived
Headers show

Commit Message

Simon Glass Sept. 12, 2011, 10:04 p.m. UTC
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

Comments

u-boot@lakedaemon.net Sept. 12, 2011, 11:37 p.m. UTC | #1
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.
Simon Glass Sept. 13, 2011, 12:12 a.m. UTC | #2
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@lakedaemon.net Sept. 13, 2011, 2:37 p.m. UTC | #3
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.
Simon Glass Sept. 13, 2011, 9:06 p.m. UTC | #4
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@lakedaemon.net Sept. 14, 2011, 1:47 p.m. UTC | #5
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.
Simon Glass Sept. 14, 2011, 3:47 p.m. UTC | #6
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@lakedaemon.net Sept. 14, 2011, 4:11 p.m. UTC | #7
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@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@0 {
                        compatible = "arm,arm926ejs";
                        reg = <0>;
                };
        };

        rtc@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.
Grant Likely Sept. 14, 2011, 4:45 p.m. UTC | #8
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
>
Simon Glass Sept. 14, 2011, 5:45 p.m. UTC | #9
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@0x7000d000";
		i2c1 = "/i2c@0x7000c000";
		i2c2 = "/i2c@0x7000c400";
		i2c3 = "/i2c@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@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@0 {
>                        compatible = "arm,arm926ejs";
>                        reg = <0>;
>                };
>        };
>
>        rtc@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.
>
Simon Glass Sept. 14, 2011, 6:03 p.m. UTC | #10
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]
Grant Likely Sept. 14, 2011, 7:17 p.m. UTC | #11
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.
Simon Glass Sept. 14, 2011, 7:22 p.m. UTC | #12
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@lakedaemon.net Sept. 14, 2011, 7:50 p.m. UTC | #13
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@0x7000d000";
> 		i2c1 = "/i2c@0x7000c000";
> 		i2c2 = "/i2c@0x7000c400";
> 		i2c3 = "/i2c@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@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@0 {
> >                        compatible = "arm,arm926ejs";
> >                        reg = <0>;
> >                };
> >        };
> >
> >        rtc@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.
Simon Glass Sept. 14, 2011, 8:05 p.m. UTC | #14
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@0x7000d000";
>>               i2c1 = "/i2c@0x7000c000";
>>               i2c2 = "/i2c@0x7000c400";
>>               i2c3 = "/i2c@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.
>
Wolfgang Denk Sept. 14, 2011, 8:11 p.m. UTC | #15
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
u-boot@lakedaemon.net Sept. 14, 2011, 8:16 p.m. UTC | #16
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@0x7000d000";
> >>               i2c1 = "/i2c@0x7000c000";
> >>               i2c2 = "/i2c@0x7000c400";
> >>               i2c3 = "/i2c@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.
Simon Glass Sept. 14, 2011, 8:24 p.m. UTC | #17
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@0x7000d000";
>> >>               i2c1 = "/i2c@0x7000c000";
>> >>               i2c2 = "/i2c@0x7000c400";
>> >>               i2c3 = "/i2c@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.
>
Simon Glass Sept. 14, 2011, 8:32 p.m. UTC | #18
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@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@lakedaemon.net Sept. 14, 2011, 8:35 p.m. UTC | #19
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.
Grant Likely Sept. 14, 2011, 9:09 p.m. UTC | #20
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.
diff mbox

Patch

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);