Message ID | 20190603235718.28381-2-dalon.westergreen@linux.intel.com |
---|---|
State | Superseded |
Delegated to: | Marek Vasut |
Headers | show |
Series | [U-Boot,v2,1/2] Makefile: Add target to generate hex output for combined spl and dtb | expand |
On Tue, Jun 4, 2019 at 1:57 AM Dalon Westergreen <dalon.westergreen@linux.intel.com> wrote: > > From: Dalon Westergreen <dalon.westergreen@intel.com> > > CONFIG_OF_EMBED was primarily enabled to support the stratix10 > spl hex file requirements. Since this option now produces a > warning during build, and the spl hex can be created using > alternate methods, CONFIG_OF_EMBED is no longer needed. > > Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com> > > --- > Changes in v2: > -> Change CONFIG_SPL_TARGET back to u-boot-spl.hex > --- > configs/socfpga_stratix10_defconfig | 1 - > include/configs/socfpga_stratix10_socdk.h | 2 +- > 2 files changed, 1 insertion(+), 2 deletions(-) > > diff --git a/configs/socfpga_stratix10_defconfig b/configs/socfpga_stratix10_defconfig > index fbab388b43..f27180385d 100644 > --- a/configs/socfpga_stratix10_defconfig > +++ b/configs/socfpga_stratix10_defconfig > @@ -26,7 +26,6 @@ CONFIG_CMD_CACHE=y > CONFIG_CMD_EXT4=y > CONFIG_CMD_FAT=y > CONFIG_CMD_FS_GENERIC=y > -CONFIG_OF_EMBED=y > CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk" > CONFIG_ENV_IS_IN_MMC=y > CONFIG_NET_RANDOM_ETHADDR=y > diff --git a/include/configs/socfpga_stratix10_socdk.h b/include/configs/socfpga_stratix10_socdk.h > index 39d757d737..66855ff0d8 100644 > --- a/include/configs/socfpga_stratix10_socdk.h > +++ b/include/configs/socfpga_stratix10_socdk.h > @@ -210,6 +210,6 @@ unsigned int cm_get_l4_sys_free_clk_hz(void); > > /* SPL SDMMC boot support */ > #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1 > -#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot.img" > +#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot-dtb.img" Is that really necessary? I don't have the aarch64 compiler at hand, but when compiling a gen5 board, "u-boot.img" and "u-boot-dtb.img" are the same. Changing to "u-boot-dtb.img" here only complicates things for the user, I think. Regards, Simon > > #endif /* __CONFIG_H */ > -- > 2.21.0 >
On Tue, 2019-06-04 at 07:13 +0200, Simon Goldschmidt wrote: > On Tue, Jun 4, 2019 at 1:57 AM Dalon Westergreen > <dalon.westergreen@linux.intel.com> wrote: > > > > > > From: Dalon Westergreen <dalon.westergreen@intel.com> > > > > CONFIG_OF_EMBED was primarily enabled to support the stratix10 > > spl hex file requirements. Since this option now produces a > > warning during build, and the spl hex can be created using > > alternate methods, CONFIG_OF_EMBED is no longer needed. > > > > Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com> > > > > --- > > Changes in v2: > > -> Change CONFIG_SPL_TARGET back to u-boot-spl.hex > > --- > > configs/socfpga_stratix10_defconfig | 1 - > > include/configs/socfpga_stratix10_socdk.h | 2 +- > > 2 files changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/configs/socfpga_stratix10_defconfig > > b/configs/socfpga_stratix10_defconfig > > index fbab388b43..f27180385d 100644 > > --- a/configs/socfpga_stratix10_defconfig > > +++ b/configs/socfpga_stratix10_defconfig > > @@ -26,7 +26,6 @@ CONFIG_CMD_CACHE=y > > CONFIG_CMD_EXT4=y > > CONFIG_CMD_FAT=y > > CONFIG_CMD_FS_GENERIC=y > > -CONFIG_OF_EMBED=y > > CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk" > > CONFIG_ENV_IS_IN_MMC=y > > CONFIG_NET_RANDOM_ETHADDR=y > > diff --git a/include/configs/socfpga_stratix10_socdk.h > > b/include/configs/socfpga_stratix10_socdk.h > > index 39d757d737..66855ff0d8 100644 > > --- a/include/configs/socfpga_stratix10_socdk.h > > +++ b/include/configs/socfpga_stratix10_socdk.h > > @@ -210,6 +210,6 @@ unsigned int cm_get_l4_sys_free_clk_hz(void); > > > > /* SPL SDMMC boot support */ > > #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1 > > -#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u- > > boot.img" > > +#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot- > > dtb.img" > Is that really necessary? I don't have the aarch64 compiler at hand, > but when compiling a gen5 board, "u-boot.img" and "u-boot-dtb.img" > are the same. Changing to "u-boot-dtb.img" here only complicates > things for the user, I think. I would agree with Dalon since we want to make sure we use same name as socfpga_common.h, which is for CV, A10 SoCs. This would help to standardize our internal test infra. Thanks Chin Liang > > Regards, > Simon > > > > > > > #endif /* __CONFIG_H */ > > -- > > 2.21.0 > >
On Tue, Jun 4, 2019 at 7:58 AM See, Chin Liang <chin.liang.see@intel.com> wrote: > > On Tue, 2019-06-04 at 07:13 +0200, Simon Goldschmidt wrote: > > On Tue, Jun 4, 2019 at 1:57 AM Dalon Westergreen > > <dalon.westergreen@linux.intel.com> wrote: > > > > > > > > > From: Dalon Westergreen <dalon.westergreen@intel.com> > > > > > > CONFIG_OF_EMBED was primarily enabled to support the stratix10 > > > spl hex file requirements. Since this option now produces a > > > warning during build, and the spl hex can be created using > > > alternate methods, CONFIG_OF_EMBED is no longer needed. > > > > > > Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com> > > > > > > --- > > > Changes in v2: > > > -> Change CONFIG_SPL_TARGET back to u-boot-spl.hex > > > --- > > > configs/socfpga_stratix10_defconfig | 1 - > > > include/configs/socfpga_stratix10_socdk.h | 2 +- > > > 2 files changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/configs/socfpga_stratix10_defconfig > > > b/configs/socfpga_stratix10_defconfig > > > index fbab388b43..f27180385d 100644 > > > --- a/configs/socfpga_stratix10_defconfig > > > +++ b/configs/socfpga_stratix10_defconfig > > > @@ -26,7 +26,6 @@ CONFIG_CMD_CACHE=y > > > CONFIG_CMD_EXT4=y > > > CONFIG_CMD_FAT=y > > > CONFIG_CMD_FS_GENERIC=y > > > -CONFIG_OF_EMBED=y > > > CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk" > > > CONFIG_ENV_IS_IN_MMC=y > > > CONFIG_NET_RANDOM_ETHADDR=y > > > diff --git a/include/configs/socfpga_stratix10_socdk.h > > > b/include/configs/socfpga_stratix10_socdk.h > > > index 39d757d737..66855ff0d8 100644 > > > --- a/include/configs/socfpga_stratix10_socdk.h > > > +++ b/include/configs/socfpga_stratix10_socdk.h > > > @@ -210,6 +210,6 @@ unsigned int cm_get_l4_sys_free_clk_hz(void); > > > > > > /* SPL SDMMC boot support */ > > > #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1 > > > -#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u- > > > boot.img" > > > +#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot- > > > dtb.img" > > Is that really necessary? I don't have the aarch64 compiler at hand, > > but when compiling a gen5 board, "u-boot.img" and "u-boot-dtb.img" > > are the same. Changing to "u-boot-dtb.img" here only complicates > > things for the user, I think. > > I would agree with Dalon since we want to make sure we use same name as > socfpga_common.h, which is for CV, A10 SoCs. This would help to > standardize our internal test infra. But that 'dtb' thing is an implementation detail. Who of the testers cares whether the devicetree is embedded or not? "u-boot.img" exists with OF_EMBED and without it, or doesn't it? Regards, Simon > > Thanks > Chin Liang > > > > > Regards, > > Simon > > > > > > > > > > > #endif /* __CONFIG_H */ > > > -- > > > 2.21.0 > > >
On Tue, 2019-06-04 at 08:12 +0200, Simon Goldschmidt wrote: > On Tue, Jun 4, 2019 at 7:58 AM See, Chin Liang <chin.liang.see@intel.com> > wrote: > > On Tue, 2019-06-04 at 07:13 +0200, Simon Goldschmidt wrote: > > > On Tue, Jun 4, 2019 at 1:57 AM Dalon Westergreen< > > > dalon.westergreen@linux.intel.com> wrote: > > > > From: Dalon Westergreen <dalon.westergreen@intel.com> > > > > CONFIG_OF_EMBED was primarily enabled to support the stratix10spl hex > > > > file requirements. Since this option now produces awarning during > > > > build, and the spl hex can be created usingalternate methods, > > > > CONFIG_OF_EMBED is no longer needed. > > > > Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com> > > > > ---Changes in v2: -> Change CONFIG_SPL_TARGET back to u-boot-spl.hex--- > > > > configs/socfpga_stratix10_defconfig | 1 - > > > > include/configs/socfpga_stratix10_socdk.h | 2 +- 2 files changed, 1 > > > > insertion(+), 2 deletions(-) > > > > diff --git > > > > a/configs/socfpga_stratix10_defconfigb/configs/socfpga_stratix10_defconf > > > > igindex fbab388b43..f27180385d 100644--- > > > > a/configs/socfpga_stratix10_defconfig+++ > > > > b/configs/socfpga_stratix10_defconfig@@ -26,7 +26,6 @@ > > > > CONFIG_CMD_CACHE=y CONFIG_CMD_EXT4=y CONFIG_CMD_FAT=y > > > > CONFIG_CMD_FS_GENERIC=y-CONFIG_OF_EMBED=y > > > > CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk" > > > > CONFIG_ENV_IS_IN_MMC=y CONFIG_NET_RANDOM_ETHADDR=ydiff --git > > > > a/include/configs/socfpga_stratix10_socdk.hb/include/configs/socfpga_str > > > > atix10_socdk.hindex 39d757d737..66855ff0d8 100644--- > > > > a/include/configs/socfpga_stratix10_socdk.h+++ > > > > b/include/configs/socfpga_stratix10_socdk.h@@ -210,6 +210,6 @@ unsigned > > > > int cm_get_l4_sys_free_clk_hz(void); > > > > /* SPL SDMMC boot support */ #define > > > > CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1-#define > > > > CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot.img"+#define > > > > CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot-dtb.img" > > > Is that really necessary? I don't have the aarch64 compiler at hand,but > > > when compiling a gen5 board, "u-boot.img" and "u-boot-dtb.img"are the > > > same. Changing to "u-boot-dtb.img" here only complicatesthings for the > > > user, I think. > > > > I would agree with Dalon since we want to make sure we use same name > > assocfpga_common.h, which is for CV, A10 SoCs. This would help tostandardize > > our internal test infra. > > But that 'dtb' thing is an implementation detail. Who of the testers > careswhether the devicetree is embedded or not? "u-boot.img" exists > withOF_EMBED and without it, or doesn't it? > Regards,Simon Yes, it exists either way. I have no issue leaving it as u-boot.img but i do agree that it should bethe same across the socfpga family. As u-boot.img exists regardless of CONFIG_OF_EMBEDi would suggest just using u-boot.img, agreed? I will submit another patch to changesocfpga_common.h? --dalon > > ThanksChin Liang > > > Regards,Simon > > > > #endif /* __CONFIG_H */--2.21.0
On Tue, Jun 04, 2019 at 09:10:27AM -0700, Dalon L Westergreen wrote: > On Tue, 2019-06-04 at 08:12 +0200, Simon Goldschmidt wrote: > > On Tue, Jun 4, 2019 at 7:58 AM See, Chin Liang <chin.liang.see@intel.com> > > wrote: > > > On Tue, 2019-06-04 at 07:13 +0200, Simon Goldschmidt wrote: > > > > On Tue, Jun 4, 2019 at 1:57 AM Dalon Westergreen< > > > > dalon.westergreen@linux.intel.com> wrote: > > > > > From: Dalon Westergreen <dalon.westergreen@intel.com> > > > > > CONFIG_OF_EMBED was primarily enabled to support the stratix10spl hex > > > > > file requirements. Since this option now produces awarning during > > > > > build, and the spl hex can be created usingalternate methods, > > > > > CONFIG_OF_EMBED is no longer needed. > > > > > Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com> > > > > > ---Changes in v2: -> Change CONFIG_SPL_TARGET back to u-boot-spl.hex--- > > > > > configs/socfpga_stratix10_defconfig | 1 - > > > > > include/configs/socfpga_stratix10_socdk.h | 2 +- 2 files changed, 1 > > > > > insertion(+), 2 deletions(-) > > > > > diff --git > > > > > a/configs/socfpga_stratix10_defconfigb/configs/socfpga_stratix10_defconf > > > > > igindex fbab388b43..f27180385d 100644--- > > > > > a/configs/socfpga_stratix10_defconfig+++ > > > > > b/configs/socfpga_stratix10_defconfig@@ -26,7 +26,6 @@ > > > > > CONFIG_CMD_CACHE=y CONFIG_CMD_EXT4=y CONFIG_CMD_FAT=y > > > > > CONFIG_CMD_FS_GENERIC=y-CONFIG_OF_EMBED=y > > > > > CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk" > > > > > CONFIG_ENV_IS_IN_MMC=y CONFIG_NET_RANDOM_ETHADDR=ydiff --git > > > > > a/include/configs/socfpga_stratix10_socdk.hb/include/configs/socfpga_str > > > > > atix10_socdk.hindex 39d757d737..66855ff0d8 100644--- > > > > > a/include/configs/socfpga_stratix10_socdk.h+++ > > > > > b/include/configs/socfpga_stratix10_socdk.h@@ -210,6 +210,6 @@ unsigned > > > > > int cm_get_l4_sys_free_clk_hz(void); > > > > > /* SPL SDMMC boot support */ #define > > > > > CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1-#define > > > > > CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot.img"+#define > > > > > CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot-dtb.img" > > > > Is that really necessary? I don't have the aarch64 compiler at hand,but > > > > when compiling a gen5 board, "u-boot.img" and "u-boot-dtb.img"are the > > > > same. Changing to "u-boot-dtb.img" here only complicatesthings for the > > > > user, I think. > > > > > > I would agree with Dalon since we want to make sure we use same name > > > assocfpga_common.h, which is for CV, A10 SoCs. This would help tostandardize > > > our internal test infra. > > > > But that 'dtb' thing is an implementation detail. Who of the testers > > careswhether the devicetree is embedded or not? "u-boot.img" exists > > withOF_EMBED and without it, or doesn't it? > > Regards,Simon > > Yes, it exists either way. I have no issue leaving it as u-boot.img but i do > agree that it should bethe same across the socfpga family. As u-boot.img exists > regardless of CONFIG_OF_EMBEDi would suggest just using u-boot.img, agreed? I > will submit another patch to changesocfpga_common.h? Using "u-boot.img" is the right choice, it's what's used on all other platforms for "u-boot with dtb" and we have logic in the Makefile to do the right thing so that users can upgrade from a version of U-Boot where the dtb wasn't in use to one where it is without their scripts breaking.
Am 04.06.2019 um 20:15 schrieb Tom Rini: > On Tue, Jun 04, 2019 at 09:10:27AM -0700, Dalon L Westergreen wrote: >> On Tue, 2019-06-04 at 08:12 +0200, Simon Goldschmidt wrote: >>> On Tue, Jun 4, 2019 at 7:58 AM See, Chin Liang <chin.liang.see@intel.com> >>> wrote: >>>> On Tue, 2019-06-04 at 07:13 +0200, Simon Goldschmidt wrote: >>>>> On Tue, Jun 4, 2019 at 1:57 AM Dalon Westergreen< >>>>> dalon.westergreen@linux.intel.com> wrote: >>>>>> From: Dalon Westergreen <dalon.westergreen@intel.com> >>>>>> CONFIG_OF_EMBED was primarily enabled to support the stratix10spl hex >>>>>> file requirements. Since this option now produces awarning during >>>>>> build, and the spl hex can be created usingalternate methods, >>>>>> CONFIG_OF_EMBED is no longer needed. >>>>>> Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com> >>>>>> ---Changes in v2: -> Change CONFIG_SPL_TARGET back to u-boot-spl.hex--- >>>>>> configs/socfpga_stratix10_defconfig | 1 - >>>>>> include/configs/socfpga_stratix10_socdk.h | 2 +- 2 files changed, 1 >>>>>> insertion(+), 2 deletions(-) >>>>>> diff --git >>>>>> a/configs/socfpga_stratix10_defconfigb/configs/socfpga_stratix10_defconf >>>>>> igindex fbab388b43..f27180385d 100644--- >>>>>> a/configs/socfpga_stratix10_defconfig+++ >>>>>> b/configs/socfpga_stratix10_defconfig@@ -26,7 +26,6 @@ >>>>>> CONFIG_CMD_CACHE=y CONFIG_CMD_EXT4=y CONFIG_CMD_FAT=y >>>>>> CONFIG_CMD_FS_GENERIC=y-CONFIG_OF_EMBED=y >>>>>> CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk" >>>>>> CONFIG_ENV_IS_IN_MMC=y CONFIG_NET_RANDOM_ETHADDR=ydiff --git >>>>>> a/include/configs/socfpga_stratix10_socdk.hb/include/configs/socfpga_str >>>>>> atix10_socdk.hindex 39d757d737..66855ff0d8 100644--- >>>>>> a/include/configs/socfpga_stratix10_socdk.h+++ >>>>>> b/include/configs/socfpga_stratix10_socdk.h@@ -210,6 +210,6 @@ unsigned >>>>>> int cm_get_l4_sys_free_clk_hz(void); >>>>>> /* SPL SDMMC boot support */ #define >>>>>> CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1-#define >>>>>> CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot.img"+#define >>>>>> CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot-dtb.img" >>>>> Is that really necessary? I don't have the aarch64 compiler at hand,but >>>>> when compiling a gen5 board, "u-boot.img" and "u-boot-dtb.img"are the >>>>> same. Changing to "u-boot-dtb.img" here only complicatesthings for the >>>>> user, I think. >>>> >>>> I would agree with Dalon since we want to make sure we use same name >>>> assocfpga_common.h, which is for CV, A10 SoCs. This would help tostandardize >>>> our internal test infra. >>> >>> But that 'dtb' thing is an implementation detail. Who of the testers >>> careswhether the devicetree is embedded or not? "u-boot.img" exists >>> withOF_EMBED and without it, or doesn't it? >>> Regards,Simon >> >> Yes, it exists either way. I have no issue leaving it as u-boot.img but i do >> agree that it should bethe same across the socfpga family. As u-boot.img exists >> regardless of CONFIG_OF_EMBEDi would suggest just using u-boot.img, agreed? I >> will submit another patch to changesocfpga_common.h? > > Using "u-boot.img" is the right choice, it's what's used on all other > platforms for "u-boot with dtb" and we have logic in the Makefile to do > the right thing so that users can upgrade from a version of U-Boot where > the dtb wasn't in use to one where it is without their scripts breaking. That's what I thought. Thanks for clarifying this, Tom. Regards, Simon
diff --git a/configs/socfpga_stratix10_defconfig b/configs/socfpga_stratix10_defconfig index fbab388b43..f27180385d 100644 --- a/configs/socfpga_stratix10_defconfig +++ b/configs/socfpga_stratix10_defconfig @@ -26,7 +26,6 @@ CONFIG_CMD_CACHE=y CONFIG_CMD_EXT4=y CONFIG_CMD_FAT=y CONFIG_CMD_FS_GENERIC=y -CONFIG_OF_EMBED=y CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk" CONFIG_ENV_IS_IN_MMC=y CONFIG_NET_RANDOM_ETHADDR=y diff --git a/include/configs/socfpga_stratix10_socdk.h b/include/configs/socfpga_stratix10_socdk.h index 39d757d737..66855ff0d8 100644 --- a/include/configs/socfpga_stratix10_socdk.h +++ b/include/configs/socfpga_stratix10_socdk.h @@ -210,6 +210,6 @@ unsigned int cm_get_l4_sys_free_clk_hz(void); /* SPL SDMMC boot support */ #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1 -#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot.img" +#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot-dtb.img" #endif /* __CONFIG_H */