| Message ID | 20201218202646.1060123-3-hi@senzilla.io |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Introduce EDK2 firmware package | expand |
Dick, All, On 2020-12-18 20:27 +0000, Dick Olsson via buildroot spake thusly: > Rebase on the mv-ddr-devel branch as the release branches are no longer > maintained. > > Signed-off-by: Dick Olsson <hi@senzilla.io> Applied to master, thanks. Regards, Yann E. MORIN. > --- > ...raining_leveling.c-uninitialized-var.patch | 31 ------------------- > boot/mv-ddr-marvell/mv-ddr-marvell.hash | 2 +- > boot/mv-ddr-marvell/mv-ddr-marvell.mk | 4 +-- > 3 files changed, 3 insertions(+), 34 deletions(-) > delete mode 100644 boot/mv-ddr-marvell/0001-mv_ddr-mv_ddr4_training_leveling.c-uninitialized-var.patch > > diff --git a/boot/mv-ddr-marvell/0001-mv_ddr-mv_ddr4_training_leveling.c-uninitialized-var.patch b/boot/mv-ddr-marvell/0001-mv_ddr-mv_ddr4_training_leveling.c-uninitialized-var.patch > deleted file mode 100644 > index eada18b473..0000000000 > --- a/boot/mv-ddr-marvell/0001-mv_ddr-mv_ddr4_training_leveling.c-uninitialized-var.patch > +++ /dev/null > @@ -1,31 +0,0 @@ > -From 5867fcad6e88af3d843bfa831648d84a53732d57 Mon Sep 17 00:00:00 2001 > -From: Heinrich Schuchardt <xypron.glpk@gmx.de> > -Date: Wed, 19 Dec 2018 17:17:13 +0100 > -Subject: [PATCH] mv_ddr: mv_ddr4_training_leveling.c: uninitialized variable > - > -With GCC 8.2 uninitialized variables lead to a build error. > - > -Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > -Signed-off-by: Baruch Siach <baruch@tkos.co.il> > ---- > -Upstream status: commit 5867fcad6e8 > - > - mv_ddr4_training_leveling.c | 2 +- > - 1 file changed, 1 insertion(+), 1 deletion(-) > - > -diff --git a/mv_ddr4_training_leveling.c b/mv_ddr4_training_leveling.c > -index 144e21a03c01..cb95389f5466 100644 > ---- a/mv_ddr4_training_leveling.c > -+++ b/mv_ddr4_training_leveling.c > -@@ -368,7 +368,7 @@ static int mv_ddr4_dynamic_pb_wl_supp(u32 dev_num, enum mv_wl_supp_mode ecc_mode > - u32 subphy_num = ddr3_tip_dev_attr_get(dev_num, MV_ATTR_OCTET_PER_INTERFACE); > - u8 compare_result = 0; > - u32 orig_phase; > -- u32 rd_data, wr_data; > -+ u32 rd_data, wr_data = 0; > - u32 flag, step; > - struct mv_ddr_topology_map *tm = mv_ddr_topology_map_get(); > - u32 ecc_phy_access_id; > --- > -2.20.1 > - > diff --git a/boot/mv-ddr-marvell/mv-ddr-marvell.hash b/boot/mv-ddr-marvell/mv-ddr-marvell.hash > index ec2fe6ed37..e1a86008db 100644 > --- a/boot/mv-ddr-marvell/mv-ddr-marvell.hash > +++ b/boot/mv-ddr-marvell/mv-ddr-marvell.hash > @@ -1,3 +1,3 @@ > # Locally calculated > -sha256 39dcc8baccb82cbc746d8f82ce7f673e1b1236e8aee0d09e7ab12c27eeb6ecda mv-ddr-marvell-618dadd1491eb2f7b2fd74313c04f7accddae475.tar.gz > +sha256 bfab74a625d65238c569b9df282b55c0fc9a1e2d3decedcf194d44774df2ede4 mv-ddr-marvell-305d923e6bc4236cd3b902f6679b0aef9e5fa52d.tar.gz > sha256 69208236fc322026920b92d1d839ebdc521ca65379bfdb3368a24945e794fc78 ddr3_init.c > diff --git a/boot/mv-ddr-marvell/mv-ddr-marvell.mk b/boot/mv-ddr-marvell/mv-ddr-marvell.mk > index a4e0c0467d..442b6aed53 100644 > --- a/boot/mv-ddr-marvell/mv-ddr-marvell.mk > +++ b/boot/mv-ddr-marvell/mv-ddr-marvell.mk > @@ -4,8 +4,8 @@ > # > ################################################################################ > > -# This is the commit for mv_ddr-armada-18.12.0 > -MV_DDR_MARVELL_VERSION = 618dadd1491eb2f7b2fd74313c04f7accddae475 > +# This is the latest commit on mv-ddr-devel as of 20201207 > +MV_DDR_MARVELL_VERSION = 305d923e6bc4236cd3b902f6679b0aef9e5fa52d > MV_DDR_MARVELL_SITE = $(call github,MarvellEmbeddedProcessors,mv-ddr-marvell,$(MV_DDR_MARVELL_VERSION)) > MV_DDR_MARVELL_LICENSE = GPL-2.0+ or LGPL-2.1 with freertos-exception-2.0, BSD-3-Clause, Marvell Commercial > MV_DDR_MARVELL_LICENSE_FILES = ddr3_init.c > -- > 2.25.1 > > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
On Fri, Dec 18, 2020 at 08:27:15PM +0000, Dick Olsson via buildroot wrote: > Rebase on the mv-ddr-devel branch as the release branches are no longer > maintained. > > Signed-off-by: Dick Olsson <hi@senzilla.io> > --- > ...raining_leveling.c-uninitialized-var.patch | 31 ------------------- > boot/mv-ddr-marvell/mv-ddr-marvell.hash | 2 +- > boot/mv-ddr-marvell/mv-ddr-marvell.mk | 4 +-- > 3 files changed, 3 insertions(+), 34 deletions(-) > delete mode 100644 boot/mv-ddr-marvell/0001-mv_ddr-mv_ddr4_training_leveling.c-uninitialized-var.patch Hello Dick, My apologies for the late feedback. This change breaks the build for MacchiatoBin board. I guess it also breaks the build for ClearFrog GT 8k board. The reason is straightforward: both boards make use of Marvell ATF v18.12.x which is compatible with mv-ddr-marvell release devel-18.12.x. Your change bumps mv-ddr-marvell version, however the new version is not compatible with older Marvell ATF release since mv_ddr_topology_map structure has been changed. Could you please clarify the reason behind this change ? There are no users for mv-ddr-marvell other than Marvell ATF that is used only for MacchiatoBin and ClearFrog GT 8k boards. So on the first glance it looks like this change has been introduced as an accompanying update/cleanup during your work on adding EDK2 firmware. If so, then I would suggest to revert this particular commit. Let me know if you have any objections for this change. Regards, Sergey
Hi Sergey, On Saturday, January 9, 2021 1:32 PM, Sergey Matyukevich <geomatsi@gmail.com> wrote: > My apologies for the late feedback. This change breaks the > build for MacchiatoBin board. I guess it also breaks the > build for ClearFrog GT 8k board. The reason is straightforward: > both boards make use of Marvell ATF v18.12.x which is > compatible with mv-ddr-marvell release devel-18.12.x. > Your change bumps mv-ddr-marvell version, however the new > version is not compatible with older Marvell ATF release since > mv_ddr_topology_map structure has been changed. I'm very sorry for breaking this! I thought I tested a build with the original MacchiatoBin defconfig, but clearly my testing was flawed! > Could you please clarify the reason behind this change ? There > are no users for mv-ddr-marvell other than Marvell ATF that is > used only for MacchiatoBin and ClearFrog GT 8k boards. So on > the first glance it looks like this change has been introduced > as an accompanying update/cleanup during your work on adding > EDK2 firmware. Bumping to a later version of mv-ddr-marvell was required in order to successfully built with mainline ATF, which in turn is required in order to build well with the latest release of EDK2. Based on my brief analysis, most of the patches in Marvell's ATF has made it upstream to ATF v2.4 (or earlier). So I thought the best and most secure approach would be to use mainline. > If so, then I would suggest to revert this particular commit. > Let me know if you have any objections for this change. May I propose that we "roll-forward" instead, and update all Marvell defconfigs to use mainline ATF instead? I believe this would add the most value for Buildroot users, having access to the latest and most stable fixes. What do you think? I'll submit a patch for comments in the next day or so! Cheers Dick Olsson PGP: 8204A8CD
Hi Dick, Sergey, On Sun, Jan 10 2021, D. Olsson via buildroot wrote: > On Saturday, January 9, 2021 1:32 PM, Sergey Matyukevich <geomatsi@gmail.com> wrote: >> If so, then I would suggest to revert this particular commit. >> Let me know if you have any objections for this change. > > May I propose that we "roll-forward" instead, and update all > Marvell defconfigs to use mainline ATF instead? I believe this > would add the most value for Buildroot users, having access to > the latest and most stable fixes. What do you think? > > I'll submit a patch for comments in the next day or so! Note that commit 6f0d85fe7e6e6 ("configs/solidrun_clearfog_gt_8k: bump BSP components") fixed the Clearfog GT-8K defconfig. baruch
Hello Dick, > > My apologies for the late feedback. This change breaks the > > build for MacchiatoBin board. I guess it also breaks the > > build for ClearFrog GT 8k board. The reason is straightforward: > both boards make use of Marvell ATF v18.12.x which is > > compatible with mv-ddr-marvell release devel-18.12.x. > > Your change bumps mv-ddr-marvell version, however the new > > version is not compatible with older Marvell ATF release since > > mv_ddr_topology_map structure has been changed. > > I'm very sorry for breaking this! I thought I tested a build with the original MacchiatoBin defconfig, but clearly my testing was flawed! > > > Could you please clarify the reason behind this change ? There > > are no users for mv-ddr-marvell other than Marvell ATF that is > > used only for MacchiatoBin and ClearFrog GT 8k boards. So on > > the first glance it looks like this change has been introduced > > as an accompanying update/cleanup during your work on adding > > EDK2 firmware. > > Bumping to a later version of mv-ddr-marvell was required in > order to successfully built with mainline ATF, which in turn is > required in order to build well with the latest release of EDK2. > Based on my brief analysis, most of the patches in Marvell's ATF > has made it upstream to ATF v2.4 (or earlier). So I thought the > best and most secure approach would be to use mainline. As I mentioned, so far there were just two users of mv-ddr-marvell: defconfigs for MacchiatoBin and ClearFrog GT 8k. And both of them used Marvell ATF. It looks like test_atf.py still uses Marvell ATF as well. So could you please clarify this point ? Am I correct assuming that you have some defconfigs that are not yet in mainline buildroot, enabling both EDK2 and mv-ddr-marvell ? > > If so, then I would suggest to revert this particular commit. > > Let me know if you have any objections for this change. > > May I propose that we "roll-forward" instead, and update all > Marvell defconfigs to use mainline ATF instead? I believe this > would add the most value for Buildroot users, having access to > the latest and most stable fixes. What do you think? I see that Baruch has already updated ClearFrog GT 8k to use mainline ATF. So I will just do the same for MacchiatoBin. Though I guess there might be some issues with PCIe support for MacchiatoBin board since upstream ATF does not yet include Marvell PCIe drivers. Regards, Sergey
diff --git a/boot/mv-ddr-marvell/0001-mv_ddr-mv_ddr4_training_leveling.c-uninitialized-var.patch b/boot/mv-ddr-marvell/0001-mv_ddr-mv_ddr4_training_leveling.c-uninitialized-var.patch deleted file mode 100644 index eada18b473..0000000000 --- a/boot/mv-ddr-marvell/0001-mv_ddr-mv_ddr4_training_leveling.c-uninitialized-var.patch +++ /dev/null @@ -1,31 +0,0 @@ -From 5867fcad6e88af3d843bfa831648d84a53732d57 Mon Sep 17 00:00:00 2001 -From: Heinrich Schuchardt <xypron.glpk@gmx.de> -Date: Wed, 19 Dec 2018 17:17:13 +0100 -Subject: [PATCH] mv_ddr: mv_ddr4_training_leveling.c: uninitialized variable - -With GCC 8.2 uninitialized variables lead to a build error. - -Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> -Signed-off-by: Baruch Siach <baruch@tkos.co.il> ---- -Upstream status: commit 5867fcad6e8 - - mv_ddr4_training_leveling.c | 2 +- - 1 file changed, 1 insertion(+), 1 deletion(-) - -diff --git a/mv_ddr4_training_leveling.c b/mv_ddr4_training_leveling.c -index 144e21a03c01..cb95389f5466 100644 ---- a/mv_ddr4_training_leveling.c -+++ b/mv_ddr4_training_leveling.c -@@ -368,7 +368,7 @@ static int mv_ddr4_dynamic_pb_wl_supp(u32 dev_num, enum mv_wl_supp_mode ecc_mode - u32 subphy_num = ddr3_tip_dev_attr_get(dev_num, MV_ATTR_OCTET_PER_INTERFACE); - u8 compare_result = 0; - u32 orig_phase; -- u32 rd_data, wr_data; -+ u32 rd_data, wr_data = 0; - u32 flag, step; - struct mv_ddr_topology_map *tm = mv_ddr_topology_map_get(); - u32 ecc_phy_access_id; --- -2.20.1 - diff --git a/boot/mv-ddr-marvell/mv-ddr-marvell.hash b/boot/mv-ddr-marvell/mv-ddr-marvell.hash index ec2fe6ed37..e1a86008db 100644 --- a/boot/mv-ddr-marvell/mv-ddr-marvell.hash +++ b/boot/mv-ddr-marvell/mv-ddr-marvell.hash @@ -1,3 +1,3 @@ # Locally calculated -sha256 39dcc8baccb82cbc746d8f82ce7f673e1b1236e8aee0d09e7ab12c27eeb6ecda mv-ddr-marvell-618dadd1491eb2f7b2fd74313c04f7accddae475.tar.gz +sha256 bfab74a625d65238c569b9df282b55c0fc9a1e2d3decedcf194d44774df2ede4 mv-ddr-marvell-305d923e6bc4236cd3b902f6679b0aef9e5fa52d.tar.gz sha256 69208236fc322026920b92d1d839ebdc521ca65379bfdb3368a24945e794fc78 ddr3_init.c diff --git a/boot/mv-ddr-marvell/mv-ddr-marvell.mk b/boot/mv-ddr-marvell/mv-ddr-marvell.mk index a4e0c0467d..442b6aed53 100644 --- a/boot/mv-ddr-marvell/mv-ddr-marvell.mk +++ b/boot/mv-ddr-marvell/mv-ddr-marvell.mk @@ -4,8 +4,8 @@ # ################################################################################ -# This is the commit for mv_ddr-armada-18.12.0 -MV_DDR_MARVELL_VERSION = 618dadd1491eb2f7b2fd74313c04f7accddae475 +# This is the latest commit on mv-ddr-devel as of 20201207 +MV_DDR_MARVELL_VERSION = 305d923e6bc4236cd3b902f6679b0aef9e5fa52d MV_DDR_MARVELL_SITE = $(call github,MarvellEmbeddedProcessors,mv-ddr-marvell,$(MV_DDR_MARVELL_VERSION)) MV_DDR_MARVELL_LICENSE = GPL-2.0+ or LGPL-2.1 with freertos-exception-2.0, BSD-3-Clause, Marvell Commercial MV_DDR_MARVELL_LICENSE_FILES = ddr3_init.c
Rebase on the mv-ddr-devel branch as the release branches are no longer maintained. Signed-off-by: Dick Olsson <hi@senzilla.io> --- ...raining_leveling.c-uninitialized-var.patch | 31 ------------------- boot/mv-ddr-marvell/mv-ddr-marvell.hash | 2 +- boot/mv-ddr-marvell/mv-ddr-marvell.mk | 4 +-- 3 files changed, 3 insertions(+), 34 deletions(-) delete mode 100644 boot/mv-ddr-marvell/0001-mv_ddr-mv_ddr4_training_leveling.c-uninitialized-var.patch