[dunfell/master PATCH] ti-sci-fw: k3-image-gen: Revert SPL address change for mainline

[dunfell/master PATCH] ti-sci-fw: k3-image-gen: Revert SPL address change for mainline

[Nishanth Menon]

Commit 02a2433c ("ti-sci-fw: Update image gen to latest") switched to a
newer commit in k3-image-gen that also picked up TISCI firmware release,
However, does include a change where the SPL address changed.

This switch in address has been queued for upstream in a future release
(v5.14 + u-boot 2021.10) and current mainline formal tags do not support
this address change. As a result, we have a broken AM64x mainline
support.

Revert this specific change from k3-image-gen for mainline alone for
now.

NOTE: This uses ARAGO_BRAND=mainline which is NOT accurate for meta-ti
which is distro independent, hence without appropriate changes while
using alternate distros, this change will not take effect.

Reported-by: Tom Rini <trini@konsulko.com>
Suggested-by: Denys Dmytriyenko <denys@konsulko.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---

Example: u-boot 2021.07-rc5 + v5.13 kernel: https://pastebin.ubuntu.com/p/Xxvz5YkCvY/

 ...-Makefile-Move-the-SPL-Load-address-.patch | 36 +++++++++++++++++++
 recipes-bsp/ti-sci-fw/ti-sci-fw.inc           |  8 +++++
 2 files changed, 44 insertions(+)
 create mode 100644 recipes-bsp/ti-sci-fw/files/0001-Revert-soc-am64x-Makefile-Move-the-SPL-Load-address-.patch

diff --git a/recipes-bsp/ti-sci-fw/files/0001-Revert-soc-am64x-Makefile-Move-the-SPL-Load-address-.patch b/recipes-bsp/ti-sci-fw/files/0001-Revert-soc-am64x-Makefile-Move-the-SPL-Load-address-.patch
new file mode 100644
index 000000000000..63a5796d6b82
--- /dev/null
+++ b/recipes-bsp/ti-sci-fw/files/0001-Revert-soc-am64x-Makefile-Move-the-SPL-Load-address-.patch
@@ -0,0 +1,36 @@
+From 83d29d5e346083254e71fbe483cb14f6657b9965 Mon Sep 17 00:00:00 2001
+From: Nishanth Menon <nm@ti.com>
+Date: Tue, 29 Jun 2021 12:03:29 -0500
+Subject: [PATCH] Revert "soc: am64x: Makefile: Move the SPL Load address to
+ 0x70000000"
+
+This reverts commit e411dc16862b5bb2d8befb12496b03d7b01c87b2.
+
+Mainline is NOT ready for the change yet as of u-boot 2021.07 + v5.13,
+So, this patch reverts the SPL address change.
+
+This patch needs to be dropped once mainline u-boot and kernel have
+migrated over to newer releases.
+
+Upstream-Status: Not applicable
+Signed-off-by: Nishanth Menon <nm@ti.com>
+---
+ soc/am64x/Makefile | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/soc/am64x/Makefile b/soc/am64x/Makefile
+index 45425428f9c6..e8747376f153 100644
+--- a/soc/am64x/Makefile
++++ b/soc/am64x/Makefile
+@@ -30,7 +30,7 @@
+ #  OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ #
+ 
+-SBL_LOADADDDR ?= 0x70000000
++SBL_LOADADDDR ?= 0x70020000
+ COMBINED_SYSFW_BRDCFG_LOADADDR ?= 0x70000
+ LOADADDR  ?= 0x44000
+ SCIFS = sci
+-- 
+2.32.0
+
diff --git a/recipes-bsp/ti-sci-fw/ti-sci-fw.inc b/recipes-bsp/ti-sci-fw/ti-sci-fw.inc
index 4004d93f47c9..503cd5ceadc6 100644
--- a/recipes-bsp/ti-sci-fw/ti-sci-fw.inc
+++ b/recipes-bsp/ti-sci-fw/ti-sci-fw.inc
@@ -15,4 +15,12 @@ SRC_URI = " \
 	git://git.ti.com/k3-image-gen/k3-image-gen.git;protocol=git;branch=master;destsuffix=imggen;name=imggen \
 "
 
+# Mainline u_boot 2021.07 and upstream kernel v5.13 tag cannot use new SPL address
+# in K3-Imagegen
+SRC_URI += "\
+	${@oe.utils.conditional('ARAGO_BRAND', 'mainline', '\
+	file://0001-Revert-soc-am64x-Makefile-Move-the-SPL-Load-address-.patch;patchdir=../imggen \
+	', '', d)} \
+"
+
 S = "${WORKDIR}/git"
-- 
2.32.0

Re: [meta-ti] [dunfell/master PATCH] ti-sci-fw: k3-image-gen: Revert SPL address change for mainline

[Denys Dmytriyenko]

On Tue, Jun 29, 2021 at 03:36:30PM -0500, Nishanth Menon via lists.yoctoproject.org wrote:
> Commit 02a2433c ("ti-sci-fw: Update image gen to latest") switched to a
> newer commit in k3-image-gen that also picked up TISCI firmware release,
> However, does include a change where the SPL address changed.
> 
> This switch in address has been queued for upstream in a future release
> (v5.14 + u-boot 2021.10) and current mainline formal tags do not support
> this address change. As a result, we have a broken AM64x mainline
> support.
> 
> Revert this specific change from k3-image-gen for mainline alone for
> now.
> 
> NOTE: This uses ARAGO_BRAND=mainline which is NOT accurate for meta-ti
> which is distro independent, hence without appropriate changes while
> using alternate distros, this change will not take effect.

Right, this flag is Arago Distro specific and it won't do anything for 
people using meta-ti directly. Hopefully this can be fixed properly very 
soon and this hack removed.

One more cleaner approach would be to have a separate "mainline" branch 
for k3-image-gen repo, where you have the correct address and mainline 
recipe using it - that way it will work for meta-ti by itself...

Another cleaner solution could be to pass SBL_LOADADDR from the recipe 
itself to override the one weakly-assigned in the Makefile...

Meanwhile, I would suggest weakly-definint ARAGO_BRAND in the recipe, 
as otherwise the variable will be undefined and may break parsing, i.e. 
"undefined behavior":
ARAGO_BRAND ?= ""



> Reported-by: Tom Rini <trini@konsulko.com>
> Suggested-by: Denys Dmytriyenko <denys@konsulko.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> 
> Example: u-boot 2021.07-rc5 + v5.13 kernel: https://pastebin.ubuntu.com/p/Xxvz5YkCvY/
> 
>  ...-Makefile-Move-the-SPL-Load-address-.patch | 36 +++++++++++++++++++
>  recipes-bsp/ti-sci-fw/ti-sci-fw.inc           |  8 +++++
>  2 files changed, 44 insertions(+)
>  create mode 100644 recipes-bsp/ti-sci-fw/files/0001-Revert-soc-am64x-Makefile-Move-the-SPL-Load-address-.patch
> 
> diff --git a/recipes-bsp/ti-sci-fw/files/0001-Revert-soc-am64x-Makefile-Move-the-SPL-Load-address-.patch b/recipes-bsp/ti-sci-fw/files/0001-Revert-soc-am64x-Makefile-Move-the-SPL-Load-address-.patch
> new file mode 100644
> index 000000000000..63a5796d6b82
> --- /dev/null
> +++ b/recipes-bsp/ti-sci-fw/files/0001-Revert-soc-am64x-Makefile-Move-the-SPL-Load-address-.patch
> @@ -0,0 +1,36 @@
> +From 83d29d5e346083254e71fbe483cb14f6657b9965 Mon Sep 17 00:00:00 2001
> +From: Nishanth Menon <nm@ti.com>
> +Date: Tue, 29 Jun 2021 12:03:29 -0500
> +Subject: [PATCH] Revert "soc: am64x: Makefile: Move the SPL Load address to
> + 0x70000000"
> +
> +This reverts commit e411dc16862b5bb2d8befb12496b03d7b01c87b2.
> +
> +Mainline is NOT ready for the change yet as of u-boot 2021.07 + v5.13,
> +So, this patch reverts the SPL address change.
> +
> +This patch needs to be dropped once mainline u-boot and kernel have
> +migrated over to newer releases.
> +
> +Upstream-Status: Not applicable

Extra thanks for this! ^


> +Signed-off-by: Nishanth Menon <nm@ti.com>
> +---
> + soc/am64x/Makefile | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/soc/am64x/Makefile b/soc/am64x/Makefile
> +index 45425428f9c6..e8747376f153 100644
> +--- a/soc/am64x/Makefile
> ++++ b/soc/am64x/Makefile
> +@@ -30,7 +30,7 @@
> + #  OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + #
> + 
> +-SBL_LOADADDDR ?= 0x70000000
> ++SBL_LOADADDDR ?= 0x70020000
> + COMBINED_SYSFW_BRDCFG_LOADADDR ?= 0x70000
> + LOADADDR  ?= 0x44000
> + SCIFS = sci
> +-- 
> +2.32.0
> +
> diff --git a/recipes-bsp/ti-sci-fw/ti-sci-fw.inc b/recipes-bsp/ti-sci-fw/ti-sci-fw.inc
> index 4004d93f47c9..503cd5ceadc6 100644
> --- a/recipes-bsp/ti-sci-fw/ti-sci-fw.inc
> +++ b/recipes-bsp/ti-sci-fw/ti-sci-fw.inc
> @@ -15,4 +15,12 @@ SRC_URI = " \
>  	git://git.ti.com/k3-image-gen/k3-image-gen.git;protocol=git;branch=master;destsuffix=imggen;name=imggen \
>  "
>  
> +# Mainline u_boot 2021.07 and upstream kernel v5.13 tag cannot use new SPL address
> +# in K3-Imagegen
> +SRC_URI += "\
> +	${@oe.utils.conditional('ARAGO_BRAND', 'mainline', '\
> +	file://0001-Revert-soc-am64x-Makefile-Move-the-SPL-Load-address-.patch;patchdir=../imggen \
> +	', '', d)} \
> +"
> +
>  S = "${WORKDIR}/git"
> -- 
> 2.32.0
> 

-- 
Regards,
Denys Dmytriyenko <denis@denix.org>
PGP: 0x420902729A92C964 - https://denix.org/0x420902729A92C964
Fingerprint: 25FC E4A5 8A72 2F69 1186  6D76 4209 0272 9A92 C964

Re: [meta-ti] [dunfell/master PATCH] ti-sci-fw: k3-image-gen: Revert SPL address change for mainline

[Nishanth Menon]

On 20:09-20210629, Denys Dmytriyenko wrote:
> On Tue, Jun 29, 2021 at 03:36:30PM -0500, Nishanth Menon via lists.yoctoproject.org wrote:
> > Commit 02a2433c ("ti-sci-fw: Update image gen to latest") switched to a
> > newer commit in k3-image-gen that also picked up TISCI firmware release,
> > However, does include a change where the SPL address changed.
> > 
> > This switch in address has been queued for upstream in a future release
> > (v5.14 + u-boot 2021.10) and current mainline formal tags do not support
> > this address change. As a result, we have a broken AM64x mainline
> > support.
> > 
> > Revert this specific change from k3-image-gen for mainline alone for
> > now.
> > 
> > NOTE: This uses ARAGO_BRAND=mainline which is NOT accurate for meta-ti
> > which is distro independent, hence without appropriate changes while
> > using alternate distros, this change will not take effect.
> 
> Right, this flag is Arago Distro specific and it won't do anything for 
> people using meta-ti directly. Hopefully this can be fixed properly very 
> soon and this hack removed.

Yes, this is temporary - at least for one u-boot and kernel window :(

> One more cleaner approach would be to have a separate "mainline" branch 
> for k3-image-gen repo, where you have the correct address and mainline 
> recipe using it - that way it will work for meta-ti by itself...

> Another cleaner solution could be to pass SBL_LOADADDR from the recipe 
> itself to override the one weakly-assigned in the Makefile...

Honestly, I am really hoping this entire k3-image-gen stuff gets
into u-boot as buildman based solution and we should get rid of this
recipe. But, I dont have a clear timeframe for that yet.

I am hoping this is an aberration and should'nt be made as a standard
/ legitimate option by making a valid -mainline recipe or formalize a
framework to provide specific override option- hence the attempt to
engineer it as little as possible.

Let me know if we have a strong opinion on this, and I can try to
respin this.

> Meanwhile, I would suggest weakly-definint ARAGO_BRAND in the recipe, 
> as otherwise the variable will be undefined and may break parsing, i.e. 
> "undefined behavior":
> ARAGO_BRAND ?= ""

Yes, this definitely makes sense.

[...]

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D)/Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D