[U-Boot] ARM CONFIG_OF_CONTROL status

[Simon Glass <sjg@chromium.org>]

Hi,

On Tue, Jul 3, 2012 at 1:22 PM, Stephan Linz <linz@li-pro.net> wrote:

> Am Dienstag, den 03.07.2012, 12:21 -0700 schrieb Simon Glass:
> > Hi,
> >
> > On Sun, Jul 1, 2012 at 10:43 PM, Michal Simek <monstr@monstr.eu> wrote:
> >
> > > 2012/6/29 Stephan Linz <linz@li-pro.net>:
> > > > Am Freitag, den 29.06.2012, 10:18 +0200 schrieb Michal Simek:
> > > >> On 06/29/2012 04:32 AM, Simon Glass wrote:
> > > >> > Hi,
> > > >> >
> > > >> > --snip--
> > > >>
> > > >> I have sent support for Microblaze. Currently without dts because I
> > > want to clear this part a little bit.
> > > >
> > > > Hi Michal,
> > > >
> > > > looks good, I've been waiting a long time on the FDT support in
> U-Boot
> > > > for Microblaze -- great -- PS: see my comment on patch 5 ...
> > > >
> > > >>
> > > >> Tegra is using ./arch/arm/dts/tegra20.dtsi and
> > > board/nvidia/dts/tegra2-seaboard.dts
> > > >> and they are composed together in dts/Makefile by calling
> preprocessor.
> > > >> Microblaze will be totally different case because every Microblaze
> hw
> > > design is different.
> > > >
> > > > Yes, that's right. We will never be in the position to define a
> skeleton
> > > > or a basic platform configuration.
> > > >
> > > >> We can use two main buses (little and big endian) and cpu is also
> > > configurable.
> > > >> Based on this for Microblaze is the best solution directly to use
> dts.
> > > >> (DTS for Microblaze is also generated directly from design tool).
> > > >
> > > > ... directly in the context of a board, not arch/cpu, right?
> > >
> > > yes.
> > >
> > > >
> > > >>
> > > >>
> > > >> Anyway - here is the bug message I am getting if I use full dts in
> > > board/<name>/dts/microblaze.dts
> > > >> and empty arch/microblaze/dts/microblaze.dtsi
> > > >>
> > > >> <stdin>:34:3: error: invalid preprocessing directive #address
> > > >> <stdin>:35:3: error: invalid preprocessing directive #size
> > > >> <stdin>:52:4: error: invalid preprocessing directive #address
> > > >> <stdin>:53:4: error: invalid preprocessing directive #cpus
> > > >> <stdin>:54:4: error: invalid preprocessing directive #size
> > > >> <stdin>:155:4: error: invalid preprocessing directive #address
> > > >> <stdin>:156:4: error: invalid preprocessing directive #size
> > > >> <stdin>:160:5: error: invalid preprocessing directive #gpio
> > > >> <stdin>:192:5: error: invalid preprocessing directive #gpio
> > > >> <stdin>:209:5: error: invalid preprocessing directive #gpio
> > > >> <stdin>:241:5: error: invalid preprocessing directive #gpio
> > > >> <stdin>:267:5: error: invalid preprocessing directive #address
> > > >> <stdin>:268:5: error: invalid preprocessing directive #size
> > > >> <stdin>:394:5: error: invalid preprocessing directive #interrupt
> > > >>
> > > >> This is error for opposite case - empty microblaze.dts and full
> > > microblaze.dtsi.
> > > >
> > > > That are CPP errors, because the auto generated xilinx.dts is full of
> > > > CPP pragma like syntax (#something) that are wrong (invalid).
> > >
> > > I know what it is.
> > >
> > > >
> > > >>
> > > >> make[1]: Entering directory `/mnt/projects/u-boot/dts'
> > > >> rc=$( cat /mnt/projects/u-boot/board/petalogix/dts/microblaze.dts |
> > > microblaze-unknown-linux-gnu-gcc -E
> > > >> -P
> > >
> -DARCH_CPU_DTS=\"/mnt/projects/u-boot/arch/microblaze/dts/microblaze.dtsi\"
> > > - | { { dtc -R 4 -p 0x1000
> > > >> -O dtb -o dt.dtb - 2>&1 ; echo $? >&3 ; } | grep -v '^DTC: dts->dtb
>  on
> > > file' ; } 3>&1 ) ; \
> > > >>       exit $rc
> > > >> /bin/sh: line 1: exit: too many arguments
> > > >> make[1]: *** [dt.dtb] Error 1
> > > >> make[1]: Leaving directory `/mnt/projects/u-boot/dts'
> > > >>
> > > >>
> > > >> I have just tried to fix it by introducing new CONFIG option for
> > > skipping that preprocessor
> > > >> part.
> > > >
> > > > Instead of disable / skipp the CPP step you can hide the auto
> generated
> > > > xilinx.dts with a second include stage, for example:
> > > >
> > > > board/microblaze/dts/microblaze.dts looks like:
> > > >
> > > > /include/ ARCH_CPU_DTS
> > > > /include/ BOARD_DTS
> > > >
> > > >
> > > > Right, only two lines.   The arch/microblaze/dts/microblaze.dtsi
> remains
> > > > empty as you have said above. Just new is BOARD_DTS -- with the
> attached
> > > > patch for dts/Makefile you can copy the auto generated xilinx.dts
> into
> > > > the specific board directory and the CPP step substitute the right
> place
> > > > to board/microblaze/microblaze-generic/dts/microblaze.dts
> > > >
> > > > I think there are no side effects with other ports like the tegra2.
> > > >
> > > > If you want you can omit the ARCH_CPU_DTS inclusion. The
> architectural
> > > > microblaze.dtsi file is empty and (!!) have to be empty, because the
> DTC
> > > > will break with an error on multiple "/dts-v1/;" lines!
> > > >
> > > > Here is the patch:
> > > >
> > > > diff --git a/dts/Makefile b/dts/Makefile
> > > > index 914e479..b1f47a1 100644
> > > > --- a/dts/Makefile
> > > > +++ b/dts/Makefile
> > > > @@ -36,7 +36,8 @@ $(error Your architecture does not have device tree
> > > > support enabled. \
> > > > Please define CONFIG_ARCH_DEVICE_TREE))
> > > >
> > > > # We preprocess the device tree file provide a useful define
> > > > -DTS_CPPFLAGS := -DARCH_CPU_DTS=
> > > > \"$(SRCTREE)/arch/$(ARCH)/dts/$(CONFIG_ARCH_DEVICE_TREE).dtsi\"
> > > > +DTS_CPPFLAGS := -DARCH_CPU_DTS=
> > > > \"$(SRCTREE)/arch/$(ARCH)/dts/$(CONFIG_ARCH_DEVICE_TREE).dtsi\" \
> > > > +               -DBOARD_DTS=
> > > > \"$(SRCTREE)/board/$(VENDOR)/$(BOARD)/dts/$(DEVICE_TREE).dts\"
> > > >
> > > > all:   $(obj).depend $(LIB)
> > >
> > > Not sure if using another dts file will be the best approach.
> > > From my point of view will be the best to support only one dts file
> > > (without dtsi)
> > > because it is much cleaner then using 3 dts files.
> > >
> >
> > Well there is no inherent problem with having multiple include files,
> > except that it is hard to support with the old dtc when there are in
> > different subdirs.
> >
> > As a workaround, how about putting the include files in the
> > board/vendor/dts subdir as well for now?
>
> Hi,
>
> good idea -- but they cannot be used directly. The substitution variable
> ARCH_CPU_DTS is already reserved for dtsi in arch/cpu. The Microblaze
> architecture needs a board specific dts onyl. That's why I think the new
> substitution variable BOARD_DTS can be a option to solve the CPP problem
> today and handle the dtc -i in the future.
>
> BOARD_DTS can point to anything below board/vendor and perhaps with a
> new configuration option similar to CONFIG_DEFAULT_DEVICE_TREE the
> substitution could be affected with freely selectable file name instead
> of DEVICE_TREE only.
>

Just in case there is any confusion here...

The device tree file is not necessarily intended to be built with/by the
U-Boot Makefile. Yes it is convenient to do that, but where you have
multiple board variants it is actually best to have the Makefile build
U-Boot without a device tree, i.e. no need to select the particular board
variant.

Then, in a separate step:

for board in ${list_of_available_boards}; do
   dtc ... ${board}.dts
   cat u-boot.bin ${board}.dtb >u-boot-${board}.bin
done

I mention this because if we make U-Boot build the particular board
variant, then have we actually achieved the goal of a single U-Boot image
that supports multiple boards?

So IMO the infrastructure to support the post-processing of U-Boot binaries
and device trees may not in fact belong in the U-Boot Makefile. It is
convenient to be able to specify a device tree for U-Boot to pick up and
build, but I don't think it should come from the boards.cfg file - after
all the whole point is that we support a number of build variants. The
board name in boards.cfg will be something generic, like microblaze-dt, or
similar.

I hope that makes sense.

Regards,
Simon


>
>
> br,
> Stephan
>
>
> >
> > Regards,
> > Simon
> >
> >
> > >
> > > Thanks,
> > > Michal
> > >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
>
>
>
>
>