u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
To: Simon Glass <sjg@chromium.org>
Cc: "Tom Rini" <trini@konsulko.com>, "Pali Rohár" <pali@kernel.org>,
	"Marek Behún" <marek.behun@nic.cz>,
	"Bin Meng" <bmeng.cn@gmail.com>,
	"Chee Hong Ang" <chee.hong.ang@intel.com>,
	"Heinrich Schuchardt" <xypron.glpk@gmx.de>,
	"Sean Anderson" <seanga2@gmail.com>,
	"Patrick Delaunay" <patrick.delaunay@foss.st.com>,
	"Joe Hershberger" <joe.hershberger@ni.com>,
	"Nicolas Saenz Julienne" <nsaenzjulienne@suse.de>,
	"Wasim Khan" <wasim.khan@nxp.com>,
	"AKASHI Takahiro" <takahiro.akashi@linaro.org>,
	"Etienne Carriere" <etienne.carriere@linaro.org>,
	"Alexandru Gagniuc" <mr.nuke.me@gmail.com>,
	"Nandor Han" <nandor.han@vaisala.com>,
	"Steffen Jaeckel" <jaeckel-floss@eyet-services.de>,
	"Heiko Schocher" <hs@denx.de>,
	"Asherah Connor" <ashe@kivikakk.ee>,
	"Andre Przywara" <andre.przywara@arm.com>,
	"U-Boot Mailing List" <u-boot@lists.denx.de>
Subject: Re: [PATCH] sandbox: Remove OF_HOSTFILE
Date: Wed, 29 Sep 2021 08:39:15 +0300	[thread overview]
Message-ID: <YVP8AzIlqqftDsoe@apalos.home> (raw)
In-Reply-To: <CAPnjgZ1eiR8=jnSHw7-9LgyLz-kcyo5UiHqhPmCC31BmUYo37g@mail.gmail.com>

Hi Simon, 

[...]
> > -INPUTS-$(CONFIG_OF_HOSTFILE) += u-boot.dtb
> > +INPUTS-$(CONFIG_SANDBOX) += u-boot.dtb
> >  ifneq ($(CONFIG_SPL_TARGET),)
> >  INPUTS-$(CONFIG_SPL) += $(CONFIG_SPL_TARGET:"%"=%)
> >  endif
> > @@ -1423,7 +1423,7 @@ u-boot-lzma.img: u-boot.bin.lzma FORCE
> >
> >  u-boot-dtb.img u-boot.img u-boot.kwb u-boot.pbl u-boot-ivt.img: \
> >                 $(if $(CONFIG_SPL_LOAD_FIT),u-boot-nodtb.bin \
> > -                       $(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE)$(CONFIG_BINMAN_STANDALONE_FDT),dts/dt.dtb) \
> > +                       $(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_SANDBOX)$(CONFIG_BINMAN_STANDALONE_FDT),dts/dt.dtb) \
> >                 ,$(UBOOT_BIN)) FORCE
> >         $(call if_changed,mkimage)
> >         $(BOARD_SIZE_CHECK)
> > @@ -1437,7 +1437,7 @@ MKIMAGEFLAGS_u-boot.itb += -B 0x8
> >
> >  ifdef U_BOOT_ITS
> >  u-boot.itb: u-boot-nodtb.bin \
> > -               $(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),dts/dt.dtb) \
> > +               $(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_SANDBOX),dts/dt.dtb) \
> >                 $(U_BOOT_ITS) FORCE
> >         $(call if_changed,mkfitimage)
> >         $(BOARD_SIZE_CHECK)
> > diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
> > index 48636ab63919..bc67a5a5a10b 100644
> > --- a/arch/sandbox/cpu/cpu.c
> > +++ b/arch/sandbox/cpu/cpu.c
> > @@ -291,44 +291,47 @@ void invalidate_dcache_range(unsigned long start, unsigned long stop)
> >  {
> >  }
> >
> > -int sandbox_read_fdt_from_file(void)
> > +void *board_fdt_blob_setup(void)
> 
> Can you instead keep the function and call it from your new
> board_fdt_blob_setup() function? That way the error path goes through
> one place and we don't need to add goto.

Not without changing board_fdt_blob_setup() itself.  The function returns a
ptr and not an int.  We had a different #ifdef up to now, so the existing
function was returning an int and was setting the gd->fdt_blob internally.
In any case the goto either remains of gets converted to 'return NULL'.

> 
> The other problem is that we need to know whether a DT is required
> (i.e. -d, -D or -T). So it must fail (and exit) if it is required but
> cannot be loaded.

So there's two things we could do here.  Either explicitly check for NULL
if sandbox is enabled or change board_fdt_blob_setup and put an error
return code in an argument.  I'll go with the return code in v2

[...]
> > - *
> > - * Read a device tree file from a host file and set it up for use as the
> > - * control FDT.
> > - */
> > -int sandbox_read_fdt_from_file(void);
> > -
> >  /**
> >   * sandbox_reset() - reset sandbox
> >   *
> > diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> > index f7098b496983..358a6c168259 100644
> > --- a/configs/sandbox64_defconfig
> > +++ b/configs/sandbox64_defconfig
> > @@ -86,7 +86,7 @@ CONFIG_MAC_PARTITION=y
> >  CONFIG_AMIGA_PARTITION=y
> >  CONFIG_OF_CONTROL=y
> >  CONFIG_OF_LIVE=y
> > -CONFIG_OF_HOSTFILE=y
> > +CONFIG_OF_BOARD=y
> 
> Can we put this in Kconfig instead, so it is enabled for all sandbox boards?

Sure

[...]
> > --- a/scripts/Makefile.spl
> > +++ b/scripts/Makefile.spl
> > @@ -298,11 +298,11 @@ endif
> >  # Build the .dtb file if:
> >  #   - we are not using OF_PLATDATA
> >  #   - we are using OF_CONTROL
> > -#   - we have either OF_SEPARATE or OF_HOSTFILE
> > +#   - we have either OF_SEPARATE or we are compiling for sandbox
> >  build_dtb :=
> >  ifeq ($(CONFIG_$(SPL_TPL_)OF_PLATDATA),)
> >  ifneq ($(CONFIG_$(SPL_TPL_)OF_CONTROL),)
> > -ifeq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_HOSTFILE),y)
> > +ifeq ($(CONFIG_OF_SEPARATE)$(CONFIG_SANDBOX_SPL),y)
> 
> Do you mean CONFIG_SANDBOX ?

I thought we could use either.  I assumed since this is an SPL makefile
CONFIG_SANDBOX_SPL would be defined.  If it's not I can switch to CONFIG_SANDBOX

> 
> >  build_dtb := y
> >  endif
> >  endif
> > --
> > 2.33.0
> >
> 
> Regards,
> Simon

Thanks
/Ilias

  reply	other threads:[~2021-09-29  5:39 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-28  9:05 [PATCH] sandbox: Remove OF_HOSTFILE Ilias Apalodimas
2021-09-28  9:05 ` Ilias Apalodimas
2021-09-28 22:46   ` Simon Glass
2021-09-29  5:39     ` Ilias Apalodimas [this message]
2021-09-29  6:25       ` Ilias Apalodimas
2021-09-29  7:44         ` Heinrich Schuchardt
2021-10-11 19:20           ` Simon Glass
2021-10-11 20:16             ` Ilias Apalodimas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YVP8AzIlqqftDsoe@apalos.home \
    --to=ilias.apalodimas@linaro.org \
    --cc=andre.przywara@arm.com \
    --cc=ashe@kivikakk.ee \
    --cc=bmeng.cn@gmail.com \
    --cc=chee.hong.ang@intel.com \
    --cc=etienne.carriere@linaro.org \
    --cc=hs@denx.de \
    --cc=jaeckel-floss@eyet-services.de \
    --cc=joe.hershberger@ni.com \
    --cc=marek.behun@nic.cz \
    --cc=mr.nuke.me@gmail.com \
    --cc=nandor.han@vaisala.com \
    --cc=nsaenzjulienne@suse.de \
    --cc=pali@kernel.org \
    --cc=patrick.delaunay@foss.st.com \
    --cc=seanga2@gmail.com \
    --cc=sjg@chromium.org \
    --cc=takahiro.akashi@linaro.org \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    --cc=wasim.khan@nxp.com \
    --cc=xypron.glpk@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).