From: Tom Rini <trini@konsulko.com>
To: Jesse T <mr.bossman075@gmail.com>
Cc: "Simon Glass" <sjg@chromium.org>,
u-boot@lists.denx.de, "Andrew F . Davis" <afd@ti.com>,
"Giulio Benetti" <giulio.benetti@benettiengineering.com>,
"Stefan Roese" <sr@denx.de>,
"Weijie Gao" <weijie.gao@mediatek.com>,
GSS_MTK_Uboot_upstream <GSS_MTK_Uboot_upstream@mediatek.com>,
"Mingming lee" <mingming.lee@mediatek.com>,
"Mauro Condarelli" <mc5686@mclink.it>,
"Kever Yang" <kever.yang@rock-chips.com>,
"Quentin Schulz" <quentin.schulz@theobroma-systems.com>,
"Marek Behún" <kabel@kernel.org>,
"Nikhil M Jain" <n-jain1@ti.com>,
"Michal Simek" <michal.simek@amd.com>,
"Patrick Delaunay" <patrick.delaunay@foss.st.com>,
"Heinrich Schuchardt" <xypron.glpk@gmx.de>,
"Nathan Barrett-Morrison" <nathan.morrison@timesys.com>,
"Stefan Herbrechtsmeier" <stefan.herbrechtsmeier@weidmueller.com>,
"Marek Vasut" <marex@denx.de>,
"Oleksandr Suvorov" <oleksandr.suvorov@foundries.io>,
"Mayuresh Chitale" <mchitale@ventanamicro.com>,
"Roger Quadros" <rogerq@kernel.org>,
"Marek Vasut" <marek.vasut+renesas@gmail.com>
Subject: Re: [PATCH] Convert CFG_SYS_UBOOT_START to Kconfig
Date: Thu, 24 Aug 2023 18:45:35 -0400 [thread overview]
Message-ID: <20230824224535.GG3953269@bill-the-cat> (raw)
In-Reply-To: <CAJFTR8S6Poz=CSK-Cb_k2ZVTH8bfQFFNeyH56d4CCeq80Shqtw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 8355 bytes --]
On Thu, Aug 24, 2023 at 06:40:09PM -0400, Jesse T wrote:
> On Thu, Aug 24, 2023 at 4:01 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Jesse,
> >
> > On Wed, 23 Aug 2023 at 20:30, Jesse Taube <mr.bossman075@gmail.com> wrote:
> > >
> > > Commit 65cc0e2a65d2 ("global: Move remaining CONFIG_SYS_* to CFG_SYS_*")
> > > renamed CONFIG_SYS_UBOOT_START to CFG_SYS_UBOOT_START. Unfortunately,
> > > this meant that the value was no longer available to the Makefile. This
> > > caused imxrt to fail to boot. All the other boards that used this
> > > variable were unaffected because they were using the default value
> > > which is CONFIG_TEXT_BASE.
> > >
> > > This commit converts CFG_SYS_UBOOT_START to Kconfig and sets the default
> > > value to CONFIG_TEXT_BASE.
> > >
> > > Suggested-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> > > Suggested-by: Tom Rini <trini@konsulko.com>
> > > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> > > ---
> > > Makefile | 16 ++++------------
> > > arch/arm/mach-k3/config_secure.mk | 2 +-
> > > arch/arm/mach-omap2/config_secure.mk | 2 +-
> > > boot/Kconfig | 16 ++++++++++++++++
> > > common/spl/spl.c | 6 +-----
> > > common/spl/spl_fit.c | 2 +-
> > > configs/imxrt1020-evk_defconfig | 2 ++
> > > configs/imxrt1050-evk_defconfig | 2 ++
> > > configs/imxrt1170-evk_defconfig | 2 ++
> > > include/configs/gardena-smart-gateway-mt7688.h | 4 ----
> > > include/configs/imxrt1020-evk.h | 6 ------
> > > include/configs/imxrt1050-evk.h | 6 ------
> > > include/configs/imxrt1170-evk.h | 3 ---
> > > include/configs/linkit-smart-7688.h | 4 ----
> > > include/configs/mt7620.h | 3 ---
> > > include/configs/mt7628.h | 3 ---
> > > include/configs/mt8512.h | 2 --
> > > include/configs/vocore2.h | 4 ----
> > > include/spl.h | 2 +-
> > > 19 files changed, 31 insertions(+), 56 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 9b90204bfe..13d4c63439 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1364,14 +1364,6 @@ OBJCOPYFLAGS_u-boot.ldr.srec := -I binary -O srec
> > > u-boot.ldr.hex u-boot.ldr.srec: u-boot.ldr FORCE
> > > $(call if_changed,objcopy)
> > >
> > > -#
> > > -# U-Boot entry point, needed for booting of full-blown U-Boot
> > > -# from the SPL U-Boot version.
> > > -#
> > > -ifndef CFG_SYS_UBOOT_START
> > > -CFG_SYS_UBOOT_START := $(CONFIG_TEXT_BASE)
> > > -endif
> > > -
> > > # Boards with more complex image requirements can provide an .its source file
> > > # or a generator script
> > > # NOTE: Please do not use this. We are migrating away from Makefile rules to use
> > > @@ -1391,7 +1383,7 @@ endif
> > >
> > > ifdef CONFIG_SPL_LOAD_FIT
> > > MKIMAGEFLAGS_u-boot.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
> > > - -a $(CONFIG_TEXT_BASE) -e $(CFG_SYS_UBOOT_START) \
> > > + -a $(CONFIG_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \
> > > -p $(CONFIG_FIT_EXTERNAL_OFFSET) \
> > > -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \
> > > $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(DEVICE_TREE))) \
> > > @@ -1399,10 +1391,10 @@ MKIMAGEFLAGS_u-boot.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
> > > $(patsubst %,-b arch/$(ARCH)/dts/%.dtbo,$(subst ",,$(CONFIG_OF_OVERLAY_LIST)))
> > > else
> > > MKIMAGEFLAGS_u-boot.img = -A $(ARCH) -T firmware -C none -O u-boot \
> > > - -a $(CONFIG_TEXT_BASE) -e $(CFG_SYS_UBOOT_START) \
> > > + -a $(CONFIG_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \
> > > -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board"
> > > MKIMAGEFLAGS_u-boot-ivt.img = -A $(ARCH) -T firmware_ivt -C none -O u-boot \
> > > - -a $(CONFIG_TEXT_BASE) -e $(CFG_SYS_UBOOT_START) \
> > > + -a $(CONFIG_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \
> > > -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board"
> > > u-boot-ivt.img: MKIMAGEOUTPUT = u-boot-ivt.img.log
> > > endif
> > > @@ -1433,7 +1425,7 @@ MKIMAGEFLAGS_u-boot.pbl = -n $(srctree)/$(CONFIG_SYS_FSL_PBL_RCW:"%"=%) \
> > > UBOOT_BIN := u-boot.bin
> > >
> > > MKIMAGEFLAGS_u-boot-lzma.img = -A $(ARCH) -T standalone -C lzma -O u-boot \
> > > - -a $(CONFIG_TEXT_BASE) -e $(CFG_SYS_UBOOT_START) \
> > > + -a $(CONFIG_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \
> > > -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board"
> > >
> > > u-boot.bin.lzma: u-boot.bin FORCE
> > > diff --git a/arch/arm/mach-k3/config_secure.mk b/arch/arm/mach-k3/config_secure.mk
> > > index 7bc8af813a..9cc1f9eb24 100644
> > > --- a/arch/arm/mach-k3/config_secure.mk
> > > +++ b/arch/arm/mach-k3/config_secure.mk
> > > @@ -30,7 +30,7 @@ tispl.bin_HS: $(obj)/u-boot-spl-nodtb.bin_HS $(patsubst %,$(obj)/dts/%.dtb_HS,$(
> > > $(call if_changed,mkfitimage)
> > >
> > > MKIMAGEFLAGS_u-boot.img_HS = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
> > > - -a $(CONFIG_TEXT_BASE) -e $(CFG_SYS_UBOOT_START) \
> > > + -a $(CONFIG_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \
> > > -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \
> > > $(patsubst %,-b arch/$(ARCH)/dts/%.dtb_HS,$(subst ",,$(CONFIG_OF_LIST)))
> > >
> > > diff --git a/arch/arm/mach-omap2/config_secure.mk b/arch/arm/mach-omap2/config_secure.mk
> > > index 24ddcdb961..f76262bb0c 100644
> > > --- a/arch/arm/mach-omap2/config_secure.mk
> > > +++ b/arch/arm/mach-omap2/config_secure.mk
> > > @@ -102,7 +102,7 @@ u-boot_HS_XIP_X-LOADER: $(obj)/u-boot.bin FORCE
> > > ifdef CONFIG_SPL_LOAD_FIT
> > >
> > > MKIMAGEFLAGS_u-boot_HS.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
> > > - -a $(CONFIG_TEXT_BASE) -e $(CFG_SYS_UBOOT_START) \
> > > + -a $(CONFIG_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \
> > > -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \
> > > $(patsubst %,-b arch/$(ARCH)/dts/%.dtb_HS,$(subst ",,$(CONFIG_OF_LIST)))
> > >
> > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > index 5e2d4286ae..f2f7821cc1 100644
> > > --- a/boot/Kconfig
> > > +++ b/boot/Kconfig
> > > @@ -737,6 +737,22 @@ config TEXT_BASE
> > > default 0x4a000000 if SUNXI_MINIMUM_DRAM_MB >= 256
> > > default 0x42e00000 if SUNXI_MINIMUM_DRAM_MB >= 64
> > > hex "Text Base"
> > > + help
> > > + The address in memory that U-Boot will be copied from, initially.
> > > +
> > > +config HAVE_SYS_UBOOT_START
> > > + bool
> > > + depends on HAVE_TEXT_BASE
> > > + prompt "Use custom U-Boot Start"
> > > + help
> > > + By default, the address in memory that U-Boot will be copied from(TEXT_BASE) and the
> > > + entry point are the same. Select this to set a different U-Boot start address.
> >
> > space after 'from'
> >
> > Also this is a little unclear. START means the start address that
> > U-Boot will run from. If it is different from TEXT_BASE, then
> > something loads it here
> >
> > This could do with a bit more detail.
>
> Yes, it is and why I edited the TEXT_BASE help prompt.
> I'm not sure how to word that this is where the SPL will start
> execution after the jump.
> TEXT_BASE is where the spl will copy the binary to.
>
> >
> > > +
> > > +config SYS_UBOOT_START
> > > + hex
> > > + depends on HAVE_TEXT_BASE
> > > + default TEXT_BASE
> > > + prompt "U-Boot entry" if HAVE_SYS_UBOOT_START
> > Tom:
> > This is clever, and can be kept. I didn't catch at first that this will
> > do what I was hoping of setting a default, but then allowing for it to
> > be changed.
>
> Was there another way of doing this? I looked at the docs for a while
> to figure this out.
This part here is mechanically correct. Just need a slight wording
change to the help to better explain what / why one wants to adjust the
address. Why are these platforms needing start != text?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
prev parent reply other threads:[~2023-08-24 22:45 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-24 2:30 [PATCH] Convert CFG_SYS_UBOOT_START to Kconfig Jesse Taube
2023-08-24 16:54 ` Tom Rini
2023-08-24 20:01 ` Simon Glass
2023-08-24 22:40 ` Jesse T
2023-08-24 22:45 ` Tom Rini [this message]
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=20230824224535.GG3953269@bill-the-cat \
--to=trini@konsulko.com \
--cc=GSS_MTK_Uboot_upstream@mediatek.com \
--cc=afd@ti.com \
--cc=giulio.benetti@benettiengineering.com \
--cc=kabel@kernel.org \
--cc=kever.yang@rock-chips.com \
--cc=marek.vasut+renesas@gmail.com \
--cc=marex@denx.de \
--cc=mc5686@mclink.it \
--cc=mchitale@ventanamicro.com \
--cc=michal.simek@amd.com \
--cc=mingming.lee@mediatek.com \
--cc=mr.bossman075@gmail.com \
--cc=n-jain1@ti.com \
--cc=nathan.morrison@timesys.com \
--cc=oleksandr.suvorov@foundries.io \
--cc=patrick.delaunay@foss.st.com \
--cc=quentin.schulz@theobroma-systems.com \
--cc=rogerq@kernel.org \
--cc=sjg@chromium.org \
--cc=sr@denx.de \
--cc=stefan.herbrechtsmeier@weidmueller.com \
--cc=u-boot@lists.denx.de \
--cc=weijie.gao@mediatek.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