[U-Boot] CONFIG_OF_EMBED

[Dalon L Westergreen]

On Fri, 2019-03-22 at 10:11 +0800, Simon Glass wrote:
> Hi,
> 
> On Fri, 22 Mar 2019 at 05:37, Marek Vasut <marex at denx.de> wrote:
> > On 3/21/19 5:37 PM, Dalon L Westergreen wrote:
> > > On Thu, 2019-03-21 at 16:48 +0100, Marek Vasut wrote:
> > > > On 3/21/19 3:33 PM, Dalon L Westergreen wrote:
> > > > > On Thu, 2019-03-21 at 03:30 +0100, Marek Vasut wrote:
> > > > > > On 3/20/19 9:24 PM, Dalon L Westergreen wrote:
> > > > > > > On Wed, 2019-03-20 at 19:37 +0100, Marek Vasut wrote:
> > > > > > > > On 3/20/19 6:28 PM, Dalon L Westergreen wrote:
> > > > > > > > > Hey Marek,
> > > > > > > > 
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > > A while ago, when adding the hex output required for
> > > > > > > > > stratix10, I
> > > > > > > > > enabled
> > > > > > > > > CONFIG_OF_EMBED so that the spl elf included the dtb.  This
> > > > > > > > > avoided
> > > > > > > > > the
> > > > > > > > > --
> > > > > > > > > change-address option when using objcopy to convert the u-
> > > > > > > > > boot-spl-
> > > > > > > > > dtb.bin
> > > > > > > > > into
> > > > > > > > > a hex file with the correct address as required by the quartus
> > > > > > > > > tools.
> > > > > > > > > 
> > > > > > > > > In any case, there is now a warning that CONFIG_OF_EMBED
> > > > > > > > > should only
> > > > > > > > > be
> > > > > > > > > used
> > > > > > > > > for
> > > > > > > > > development and debug.  I am wondering how best to resolve
> > > > > > > > > this
> > > > > > > > > issue,
> > > > > > > > > should i
> > > > > > > > > revert back to using objcopy with the u-boot-spl-dtb.bin, is
> > > > > > > > > there a
> > > > > > > > > way
> > > > > > > > > to
> > > > > > > > > have
> > > > > > > > > CONFIG_OF_EMBED only apply to SPL?
> > > > > > > > 
> > > > > > > > CONFIG_OF_SEPARATE is what you want.
> > > > > > > 
> > > > > > > I dont think this is what i want exactly.  I was thinking of this.
> > > > > > > From 82c1d1bc6092ce40f33a04f2bc7713b0143e30a5 Mon Sep 17 00:00:00
> > > > > > > 2001
> > > > > > > From: Dalon Westergreen <dalon.westergreen at intel.com>
> > > > > > > Date: Wed, 20 Mar 2019 11:21:20 -0700
> > > > > > > Subject: [PATCH 1/2] Makefile: Add target to generate hex output
> > > > > > > for
> > > > > > > combined
> > > > > > >  spl and dtb
> > > > > > > 
> > > > > > > Some architectures, Stratix10, require a hex formatted spl that
> > > > > > > combines
> > > > > > > the spl image and dtb.  This adds a target to create said hex file
> > > > > > > with
> > > > > > > and offset of SPL_TEXT_BASE.
> 
> I don't really follow this thread, but you should not use
> CONFIG_OF_EMBED. If there is a problem with setting up the image with
> CONFIG_OF_SEPARATE, we should discuss it.
> 
> - Simon

Simon,

Did you see my patches following this email?  They resolve the issues with S10
around CONFIG_OF_EMBED.

--dalon

> 
> > > > > > The CONFIG_OF_SEPARATE doesn't generate a combined image anymore ?
> > > > > > Seems
> > > > > > like it does at least on the renesas platforms (cfr e.g.
> > > > > > stout_defconfig, where I use exactly that).
> > > > > 
> > > > > It generates a combined binary, u-boot-spl-dtb.bin, as far as i know.
> > > > > The elf is not combined, so using objcopy on the elf results in a hex
> > > > > file without the dtb.  Stratix 10 needs a combined hex file that
> > > > > starts
> > > > > at CONFIG_SPL_TEXT_BASE.  This file is then combined by quartus with
> > > > > the fpga image.  The SDM loads the spl image into the cpu's onchip ram
> > > > > and releases it from reset.
> > > > > 
> > > > > So what i am trying to do is get a hex file generated that is
> > > > > appriopriate
> > > > > for the quartus tools.  With CONFIG_OF_EMBED, this was easy, as you
> > > > > just
> > > > > run objcopy on the elf to create the hex output.  With
> > > > > CONFIG_OF_SEPARATE
> > > > > you need the combined binary to do the same.  hence,
> > > > > 
> > > > > +OBJCOPYFLAGS_u-boot-spl-dtb.hex := -I binary -O ihex --change-
> > > > > > > address=$(CONFIG_SPL_TEXT_BASE)
> > > > > > > +
> > > > > > > +spl/u-boot-spl-dtb.hex: spl/u-boot-spl-dtb.bin FORCE
> > > > > > > +       $(call if_changed,objcopy)
> > > > Aha, right. CCing Simon, maybe he has some better idea.
> > > > 
> > > > Otherwise, let's add the custom target. Maybe we can add it to
> > > > arch/arm/mach-socfpga instead though.
> > > 
> > > After a quick test to add the target to mach-socfpga instead of the root
> > > Makefile, it seems i would still need to modify the root Makefile to call
> > > the mach-socfpga makefile for the target in any case.  If acceptable, i
> > > think it cleaner to just add the target to the root Makefile.
> > 
> > I think that's fine. The root Makefile is growing like dough. that needs
> > to stop.
> > 
> > --
> > Best regards,
> > Marek Vasut