All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] Build system: Don't check for CONFIG_SYS_TEXT_BASE being set
Date: Mon, 12 Feb 2018 16:21:09 +0000	[thread overview]
Message-ID: <1518452468.4154.10.camel@synopsys.com> (raw)
In-Reply-To: <20180212142359.GM3061@bill-the-cat>

On Mon, 2018-02-12 at 09:23 -0500, Tom Rini wrote:
> On Mon, Feb 12, 2018 at 01:27:30PM +0000, Alexey Brodkin wrote:
> > Hi Tom, Simon,
> > 
> > On Sun, 2018-02-11 at 15:47 -0500, Tom Rini wrote:
> > > On Tue, Jan 30, 2018 at 06:23:13PM +0300, Alexey Brodkin wrote:
> > > 
> > > > CONFIG_SYS_TEXT_BASE must be set anyways and then it is used in many
> > > > places in the same Makefile without any checks so there's no point in
> > > > keeping this check araound just in one place.
> > > > 
> > > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> > > > Cc: Tom Rini <trini@konsulko.com>
> > > > Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > > > ---
> > > >  Makefile | 2 --
> > > >  1 file changed, 2 deletions(-)
> > > > 
> > > > diff --git a/Makefile b/Makefile
> > > > index ab3453dcebdc..6f15612b4d07 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -820,9 +820,7 @@ LDFLAGS_u-boot += $(LDFLAGS_FINAL)
> > > >  # Avoid 'Not enough room for program headers' error on binutils 2.28 onwards.
> > > >  LDFLAGS_u-boot += $(call ld-option, --no-dynamic-linker)
> > > >  
> > > > -ifneq ($(CONFIG_SYS_TEXT_BASE),)
> > > >  LDFLAGS_u-boot += -Ttext $(CONFIG_SYS_TEXT_BASE)
> > > > -endif
> > > 
> > > This then causes xtensa to fail to build as it does not set
> > > CONFIG_SYS_TEXT_BASE.
> > 
> > And that also obviously breaks "efi-x86" target as well because CONFIG_SYS_TEXT_BASE
> > seems to not be defined for EFI and then LD gets a string like "-Ttext  -o u-boot"
> > where CONFIG_SYS_TEXT_BASE is supposed to be used as some value.

[snip]

> > So if "-Ttext CONFIG_SYS_TEXT_BASE" is not used for each and every board I may use
> > CONFIG_SYS_TEXT_BASE directly in linker just as we have it now, see
> > https://urldefense.proofpoint.com/v2/url?u=http-3A__git.denx.de_-3Fp-3Du-2Dboot.git-3Ba-3Dblob-3Bf-3Darch_arc_cpu_u-2Dboot.lds-23l14&d=DwIBAg&c=DP
> > L6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I&m=3oKsgMcKriZJ81auV4XDwr4ovIElxycDMpRlTqS7nhc&s=Yq0YW-
> > MQvofxwQ961IRHzIS5xWYtHHzxMp_Fwn91h5s&e=
> > and then there might be any section like .ivt, .text, .myfunkysection etc.
> > 
> > In fact in the Linux kernel "-Ttext XXX" is not used for everybody - some
> > arches like MIPS and PPC indeed set it but others do other things.
> > 
> > The simplest thing might be is to add another #ifdef for ARC and X86 which both
> > use CONFIG_SYS_TEXT_BASE directly in their linker scripts like that:
> > ------------------->8-----------------
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -820,9 +820,11 @@ LDFLAGS_u-boot += $(LDFLAGS_FINAL)
> >  # Avoid 'Not enough room for program headers' error on binutils 2.28 onwards.
> >  LDFLAGS_u-boot += $(call ld-option, --no-dynamic-linker)
> >  
> > +ifeq($(CONFIG_ARC)$(CONFIG_X86),)
> >  LDFLAGS_u-boot += -Ttext $(CONFIG_SYS_TEXT_BASE)
> > +endif
> >  
> >  # Normally we fill empty space with 0xff
> >  quiet_cmd_objcopy = OBJCOPY $@
> > ------------------->8-----------------
> > 
> > Any thoughts?
> 
> I'm largely ok with the above, but:
> - You need to exclude CONFIG_NIOS2 in the above as well, or convert the
>   usage of CONFIG_SYS_MONITOR_BASE to CONFIG_SYS_TEXT_BASE (Thomas?)

Looks like NIOS2 builds are not yet supported in TravisCI which is
quite unfortunate... are there any instructions on how to build for Nios2
so that I may test my changes before posting them?

-Alexey

  parent reply	other threads:[~2018-02-12 16:21 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-30 15:23 [U-Boot] [PATCH] Build system: Don't check for CONFIG_SYS_TEXT_BASE being set Alexey Brodkin
2018-01-30 16:23 ` Masahiro Yamada
2018-01-30 16:28   ` Alexey Brodkin
2018-01-31 15:18     ` Masahiro Yamada
2018-01-31 15:21       ` Alexey Brodkin
2018-02-02 15:01         ` Masahiro Yamada
2018-02-11 20:47 ` [U-Boot] " Tom Rini
2018-02-12 13:27   ` Alexey Brodkin
2018-02-12 14:23     ` Tom Rini
2018-02-12 14:46       ` Marek Vasut
2018-02-12 16:21       ` Alexey Brodkin [this message]
2018-02-12 16:29         ` Tom Rini
2018-02-12 20:48       ` Max Filippov
2018-02-12 21:03         ` Tom Rini
2018-02-12 21:48           ` Max Filippov

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=1518452468.4154.10.camel@synopsys.com \
    --to=alexey.brodkin@synopsys.com \
    --cc=u-boot@lists.denx.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.