Fix uImage.FIT support for little endian target and non PPC, M68K and SPARC target
Hi,
The following path will fix the uImage.FIT support for little endian target and non PPC, M68K and SPARC target
Tested on different ARM board
Best Regards,
J.
PATCH 1/2 bootm: fix BOOTM_STATE_FDT subcommand usage
boot_relocate_fdt can only be used on PPC, M68K and SPARC
--- common/cmd_bootm.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
common/cmd_bootm.c +2 -0
@@ -499,6 +499,7 @@ int do_bootm_subcommand (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])break ;#endif #ifdef CONFIG_OF_LIBFDT +#if defined(CONFIG_PPC) || defined(CONFIG_M68K) || defined(CONFIG_SPARC) case BOOTM_STATE_FDT: { ulong bootmap_base = getenv_bootm_low();
@@ -507,6 +508,7 @@ int do_bootm_subcommand (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])break ; }#endif +#endif case BOOTM_STATE_OS_CMDLINE : ret =boot_fn (BOOTM_STATE_OS_CMDLINE , argc, argv, &images);if (ret)
Signature · 1 line
1.5.6.5
PATCH 2/2 fit: Fix little endian target support
bswap_32 and bswap_64 is for host on target we use cpu_to_be32 and cpu_to_be64
--- include/libfdt_env.h | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
include/libfdt_env.h +7 -0
@@ -36,11 +36,18 @@extern struct fdt_header *working_fdt;/* Pointer to the working fdt */ #if __BYTE_ORDER == __LITTLE_ENDIAN +#ifdef USE_HOSTCC #define fdt32_to_cpu (x) bswap_32(x)#define cpu_to_fdt32 (x) bswap_32(x)#define fdt64_to_cpu (x) bswap_64(x)#define cpu_to_fdt64 (x) bswap_64(x)#else +#define fdt32_to_cpu (x) cpu_to_be32(x) +#define cpu_to_fdt32 (x) cpu_to_be32(x) +#define fdt64_to_cpu (x) cpu_to_be64(x) +#define cpu_to_fdt64 (x) cpu_to_be64(x) +#endif /* USE_HOSTCC */ +#else #define fdt32_to_cpu (x) (x)#define cpu_to_fdt32 (x) (x)#define fdt64_to_cpu (x) (x)
Signature · 1 line
1.5.6.5
PATCH 2/2 fit: Fix little endian target support
Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 2 lines · In message <[email protected]> you wrote:
bswap_32andbswap_64is for host on target we usecpu_to_be32andcpu_to_be64
Hm.... do you consider this a style cleanup, or is there any real problem that needs fixing?
include/libfdt_env.h line 49
Quoted 12 lines
#if
__BYTE_ORDER==__LITTLE_ENDIANinclude/libfdt_env.h +6
-0+#ifdef USE_HOSTCC #define fdt32_to_cpu (x) bswap_32(x)#define cpu_to_fdt32 (x) bswap_32(x)#define fdt64_to_cpu (x) bswap_64(x)#define cpu_to_fdt64 (x) bswap_64(x)#else +#define fdt32_to_cpu (x) cpu_to_be32(x) +#define cpu_to_fdt32 (x) cpu_to_be32(x) +#define fdt64_to_cpu (x) cpu_to_be64(x) +#define cpu_to_fdt64 (x) cpu_to_be64(x) +#endif /* USE_HOSTCC */
This is IMO wrong. If we implement such a change, then fdt32_to_cpu should translate to be32_to_cpu, and fdt64_to_cpu should translate to be64_to_cpu.
But actually I do not see the need for this change.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Real computer scientists don't comment their code. The identifiers are so long they can't afford the disk space.
PATCH 2/2 fit: Fix little endian target support
Quoted 8 lines · Wolfgang Denk
Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 2 lines · In message <[email protected]> you wrote:
bswap_32andbswap_64is for host on target we usecpu_to_be32andcpu_to_be64Hm.... do you consider this a style cleanup, or is there any real problem that needs fixing?
on little endian it does not compile
Quoted 18 lines
include/libfdt_env.h line 49
Quoted 13 lines
#if
__BYTE_ORDER==__LITTLE_ENDIANinclude/libfdt_env.h +6
-0+#ifdef USE_HOSTCC #define fdt32_to_cpu (x) bswap_32(x)#define cpu_to_fdt32 (x) bswap_32(x)#define fdt64_to_cpu (x) bswap_64(x)#define cpu_to_fdt64 (x) bswap_64(x)#else +#define fdt32_to_cpu (x) cpu_to_be32(x) +#define cpu_to_fdt32 (x) cpu_to_be32(x) +#define fdt64_to_cpu (x) cpu_to_be64(x) +#define cpu_to_fdt64 (x) cpu_to_be64(x) +#endif /* USE_HOSTCC */ This is IMO wrong. If we implement such a change, then
fdt32_to_cpushould translate tobe32_to_cpu, andfdt64_to_cpushould translate tobe64_to_cpu.
Quoted 1 line
But actually I do not see the need for this change.
I've try some arm little endian platfrom without it it does not compile at all
Best Regards,
J.
PATCH 2/2 fit: Fix little endian target support
Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 4 lines · In message <[email protected]> you wrote:
Quoted 3 lines
Hm.... do you consider this a style cleanup, or is there any real problem that needs fixing?
on little endian it does not compile
What do you mena - on little endian? I didn't see any build problems for any of the ARM or le MIPS boards?
Quoted 17 lines
Quoted 17 lines
include/libfdt_env.h line 49
Quoted 12 lines
#if
__BYTE_ORDER==__LITTLE_ENDIANinclude/libfdt_env.h +6
-0+#ifdef USE_HOSTCC #define fdt32_to_cpu (x) bswap_32(x)#define cpu_to_fdt32 (x) bswap_32(x)#define fdt64_to_cpu (x) bswap_64(x)#define cpu_to_fdt64 (x) bswap_64(x)#else +#define fdt32_to_cpu (x) cpu_to_be32(x) +#define cpu_to_fdt32 (x) cpu_to_be32(x) +#define fdt64_to_cpu (x) cpu_to_be64(x) +#define cpu_to_fdt64 (x) cpu_to_be64(x) +#endif /* USE_HOSTCC */ This is IMO wrong. If we implement such a change, then
fdt32_to_cpushould translate tobe32_to_cpu, andfdt64_to_cpushould translate tobe64_to_cpu.
Quoted 2 lines
Quoted 1 line
But actually I do not see the need for this change.
I've try some arm little endian platfrom without it it does not compile at all
What exactly is the error?
The suggested patch is wrong in any case.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Just because your doctor has a name for your condition doesn't mean he knows what it is.
PATCH 2/2 fit: Fix little endian target support
Quoted 28 lines · Wolfgang Denk
Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 4 lines · In message <[email protected]> you wrote:
Quoted 3 lines
Hm.... do you consider this a style cleanup, or is there any real problem that needs fixing?
on little endian it does not compile
What do you mena - on little endian? I didn't see any build problems for any of the ARM or le MIPS boards?
Quoted 17 lines
Quoted 17 lines
include/libfdt_env.h line 49
Quoted 12 lines
#if
__BYTE_ORDER==__LITTLE_ENDIANinclude/libfdt_env.h +6
-0+#ifdef USE_HOSTCC #define fdt32_to_cpu (x) bswap_32(x)#define cpu_to_fdt32 (x) bswap_32(x)#define fdt64_to_cpu (x) bswap_64(x)#define cpu_to_fdt64 (x) bswap_64(x)#else +#define fdt32_to_cpu (x) cpu_to_be32(x) +#define cpu_to_fdt32 (x) cpu_to_be32(x) +#define fdt64_to_cpu (x) cpu_to_be64(x) +#define cpu_to_fdt64 (x) cpu_to_be64(x) +#endif /* USE_HOSTCC */ This is IMO wrong. If we implement such a change, then
fdt32_to_cpushould translate tobe32_to_cpu, andfdt64_to_cpushould translate tobe64_to_cpu.
Quoted 4 lines
Quoted 2 lines
Quoted 1 line
But actually I do not see the need for this change.
I've try some arm little endian platfrom without it it does not compile at all
What exactly is the error?
bswap_32 and bswap_64 not found when compilling u-boot itself
Best Regards,
J.
PATCH 1/2 bootm: fix BOOTM_STATE_FDT subcommand usage
In reply to Jean-Christophe PLAGNIOL-VILLARDDear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 12 lines · In message <[email protected]> you wrote:
boot_relocate_fdtcan only be used on PPC, M68K and SPARCSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>--- common/cmd_bootm.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)common/cmd_bootm.c +0
-0
common/cmd_bootm.c int do_bootm_subcommand (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) · line 502
Quoted 4 lines
common/cmd_bootm.c +1
-0@@ -499,6 +499,7 @@ int do_bootm_subcommand (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])break ;#endif #ifdef CONFIG_OF_LIBFDT +#if defined(CONFIG_PPC ) || defined(CONFIG_M68K ) || defined(CONFIG_SPARC )
This change seems wrong to me. There are PPC systems which do not use the device tree - why should we enable this code for these?
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Living on Earth may be expensive, but it includes an annual free trip around the Sun.
[U-Boot] Address Space in Uboot MPC85xx
Hi,
I am going through the u-boot code for MPC85xx while I see some code as follows.
So what is this AS (Address Space) required and what does it do? What is happening when we switch from AS=1 to AS=0.
Can someone explain me what is going on?
/* create a temp mapping in AS=1 to the boot window */
lis r6,FSL_BOOKE_MAS0(1, 15, 0)@h ori r6,r6,FSL_BOOKE_MAS0(1, 15, 0)@l . . . .
/* create a temp mapping in AS=1 to the stack */
lis r6,FSL_BOOKE_MAS0(1, 14, 0)@h ori r6,r6,FSL_BOOKE_MAS0(1, 14, 0)@l . . . . /* switch back to AS = 0 */ lis r3,(MSR_CE|MSR_ME|MSR_DE)@h ori r3,r3,(MSR_CE|MSR_ME|MSR_DE)@l
Regards,
Vignesh Kumar B
Member Technical Staff
HCL TECHNOLOGIES LTD.,
Mob: +91-9600015988
www.hcl.in
Disclaimer:
This message and any attachment(s) contained here are information that is confidential, proprietary to HCL Technologies and its customers. Contents may be privileged or otherwise protected by law. The information is solely intended for the individual or the entity it is addressed to. If you are not the intended recipient of this message, you are not authorized to read, forward, print, retain, copy or disseminate this message or any part of it. If you have received this e-mail in error, please notify the sender immediately by return e-mail and delete it from your computer.
DISCLAIMER:
-----------------------------------------------------------------------------------------------------------------------
The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. It shall not attach any liability on the originator or HCL or its affiliates. Any views or opinions presented in this email are solely those of the author and may not necessarily reflect the opinions of HCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of the author of this e-mail is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. Before opening any mail and attachments please check them for viruses and defect.
-----------------------------------------------------------------------------------------------------------------------
[U-Boot] Address Space in Uboot MPC85xx
Hi,
I am going through the u-boot code for MPC85xx while I see some code as follows.
So what is this AS (Address Space) required and what does it do? What is happening when we switch from AS=1 to AS=0.
Can someone explain me what is going on?
/* create a temp mapping in AS=1 to the boot window */
lis r6,FSL_BOOKE_MAS0(1, 15, 0)@h ori r6,r6,FSL_BOOKE_MAS0(1, 15, 0)@l . . . .
/* create a temp mapping in AS=1 to the stack */
lis r6,FSL_BOOKE_MAS0(1, 14, 0)@h ori r6,r6,FSL_BOOKE_MAS0(1, 14, 0)@l . . . . /* switch back to AS = 0 */ lis r3,(MSR_CE|MSR_ME|MSR_DE)@h ori r3,r3,(MSR_CE|MSR_ME|MSR_DE)@l
Regards,
Vignesh Kumar B
Member Technical Staff
HCL TECHNOLOGIES LTD.,
Mob: +91-9600015988
www.hcl.in
Disclaimer:
This message and any attachment(s) contained here are information that is confidential, proprietary to HCL Technologies and its customers. Contents may be privileged or otherwise protected by law. The information is solely intended for the individual or the entity it is addressed to. If you are not the intended recipient of this message, you are not authorized to read, forward, print, retain, copy or disseminate this message or any part of it. If you have received this e-mail in error, please notify the sender immediately by return e-mail and delete it from your computer.
DISCLAIMER:
------------------------------------------------------------------------
-----------------------------------------------
The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only.
It shall not attach any liability on the originator or HCL or its affiliates. Any views or opinions presented in
this email are solely those of the author and may not necessarily reflect the opinions of HCL or its affiliates.
Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of
this message without the prior written consent of the author of this e-mail is strictly prohibited. If you have
received this email in error please delete it and notify the sender immediately. Before opening any mail and
attachments please check them for viruses and defect.
------------------------------------------------------------------------
-----------------------------------------------
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] Address Space in Uboot MPC85xx
Quoted 21 lines · Vignesh Kumar B
Hi,
I am going through the u-boot code for MPC85xx while I see some code as
follows.
So what is this AS (Address Space) required and what does it do? What is
happening when we switch from AS=1 to AS=0.Can someone explain me what is going on?
/* create a temp mapping in AS=1 to the boot window */
lis r6,FSL_BOOKE_MAS0(1, 15, 0)@h ori r6,r6,FSL_BOOKE_MAS0(1, 15, 0)@l . . . ./* create a temp mapping in AS=1 to the stack */
Quoted 9 lines
lis r6,FSL_BOOKE_MAS0(1, 14, 0)@h ori r6,r6,FSL_BOOKE_MAS0(1, 14, 0)@l . . . . /* switch back to AS = 0 */ lis r3,(MSR_CE|MSR_ME|MSR_DE)@h ori r3,r3,(MSR_CE|MSR_ME|MSR_DE)@l
we switch address spaces to ensure that the TLBs we are going to use temporarily will not conflict.
- k
PATCH 1/2 bootm: fix BOOTM_STATE_FDT subcommand usage
In reply to Wolfgang DenkQuoted 10 lines · Wolfgang Denk
Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 7 lines · In message <[email protected]> you wrote:
boot_relocate_fdtcan only be used on PPC, M68K and SPARCSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>--- common/cmd_bootm.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
Quoted 12 lines
common/cmd_bootm.c int do_bootm_subcommand (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) · line 502
Quoted 9 lines
common/cmd_bootm.c +1
-0@@ -499,6 +499,7 @@ int do_bootm_subcommand (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])break ;#endif #ifdef CONFIG_OF_LIBFDT +#if defined(CONFIG_PPC ) || defined(CONFIG_M68K ) || defined(CONFIG_SPARC )This change seems wrong to me. There are PPC systems which do not use the device tree - why should we enable this code for these?
thats already the case by the ifdef CONFIG_OF_LIBFDT
Best Regards,
J.
PATCH 1/2 bootm: fix BOOTM_STATE_FDT subcommand usage
Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 7 lines · In message <[email protected]> you wrote:
Quoted 6 lines
Quoted 3 lines
#ifdef
CONFIG_OF_LIBFDT
+#ifdefined(CONFIG_PPC)||defined(CONFIG_M68K)||defined(CONFIG_SPARC)This change seems wrong to me. There are PPC systems which do not use the device tree - why should we enable this code for these?
thats already the case by the ifdef
CONFIG_OF_LIBFDT
Sorry for not being clear (or correct).
What I mean is: if this code should not be enabled on some systems, even though CONFIG_OF_LIBFDT is set, then we should make it depend on some property which only gets set for these boards, but not globally for the whole architecture.
I still do not understand what such a property would be in addition to LIBFDT support?
Best regards,
Wolfgang Denk
Signature · 7 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Misquotation is, in fact, the pride and privilege of the learned. A
widely-read man never quotes accurately, for the rather obvious
reason that he has read too widely.
- Hesketh Pearson _Common Misquotations_ introductionPATCH 1/2 bootm: fix BOOTM_STATE_FDT subcommand usage
Quoted 15 lines · Wolfgang Denk
Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 9 lines · In message <[email protected]> you wrote:
Quoted 8 lines
Quoted 4 lines
#ifdef
CONFIG_OF_LIBFDT
+#ifdefined(CONFIG_PPC)||defined(CONFIG_M68K)||defined(CONFIG_SPARC)This change seems wrong to me. There are PPC systems which do not use
the device tree - why should we enable this code for these?thats already the case by the ifdef
CONFIG_OF_LIBFDTSorry for not being clear (or correct).
Quoted 7 lines
What I mean is: if this code should not be enabled on some systems, even though
CONFIG_OF_LIBFDTis set, then we should make it depend on some property which only gets set for these boards, but not globally for the whole architecture.I still do not understand what such a property would be in addition to LIBFDT support?
I think the commit message needs more explanation about use of LIBFDT for FIT vs use of LIBFDT for actual system boot device tree usage.
- k
PATCH 1/2 bootm: fix BOOTM_STATE_FDT subcommand usage
Dear Kumar Gala,
Quoted 4 lines · In message <[email protected]> you wrote:
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
#ifdef
CONFIG_OF_LIBFDT
+#ifdefined(CONFIG_PPC)||defined(CONFIG_M68K)||defined(CONFIG_SPARC)
...
Quoted 5 lines
Quoted 2 lines
I still do not understand what such a property would be in addition to LIBFDT support?
I think the commit message needs more explanation about use of LIBFDT for FIT vs use of LIBFDT for actual system boot device tree usage.
Agreed. And any such dependencies should be coded for CONFIG_FIT, not for "CONFIG_PPC || CONFIG_M68K || CONFIG_SPARC" - this has nothing to do with any specific processor architectures. [Existing code that already uses such broken code should be cleaned up, too.]
Best regards,
Wolfgang Denk
Signature · 7 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
: 1. What is the possibility of this being added in the future?
In the near future, the probability is close to zero. In the distant
future, I'll be dead, and posterity can do whatever they like... :-)
- lwallPATCH 1/1 Fix FDT support to use fdt blob and FIT together or not
In reply to Jean-Christophe PLAGNIOL-VILLARDFDT support is used for both FIT style images and architectures (ppc, m68k, sparc) that can pass a fdt blob to an OS..
The BOOTM_STATE_FDT support should only exist for the case that we are passing a blob to an OS.
---
replace my precedent patch
[PATCH 1/2] bootm: fix BOOTM_STATE_FDT subcommand usage
Best Regards,
J.
common/image.c | 4 +++- include/image.h | 4 ---- libfdt/Makefile | 6 +++++- 3 files changed, 8 insertions(+), 6 deletions(-)
common/image.c +3 -1
@@ -1071,8 +1071,9 @@ int boot_ramdisk_high (struct lmb *lmb, ulong rd_data, ulong rd_len, error:return -1 ; } +#endif /* defined(CONFIG_PPC) || defined(CONFIG_M68K) || defined(CONFIG_SPARC) */ -#ifdef CONFIG_OF_LIBFDT +#if defined(CONFIG_OF_LIBFDT )static void fdt_error (const char *msg) {puts ("ERROR: " );
@@ -1575,6 +1576,7 @@ error: }#endif /* CONFIG_OF_LIBFDT */ +#if defined(CONFIG_PPC ) || defined(CONFIG_M68K )/** * boot_get_cmdline - allocate and initialize kernel cmdline * @lmb: pointer to lmb handle, will be used for memory mgmt
include/image.h +0 -4
@@ -50,10 +50,6 @@#endif /* USE_HOSTCC */ -#if defined(CONFIG_FIT ) && !defined(CONFIG_OF_LIBFDT ) -#error "CONFIG_OF_LIBFDT not enabled, required by CONFIG_FIT!" -#endif -#include <command.h> #if defined(CONFIG_FIT )
libfdt/Makefile +5 -1
@@ -27,7 +27,11 @@ LIB = $(obj)libfdt.a SOBJS = -COBJS-$(CONFIG_OF_LIBFDT) += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o +COBJS-libfdt += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o + +COBJS-$(CONFIG_OF_LIBFDT) += $(COBJS-libfdt) +COBJS-$(CONFIG_FIT) += $(COBJS-libfdt) + COBJS := $(COBJS-y) SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c)
Signature · 1 line
1.5.6.5
PATCH 1/1 Fix FIT and fdt blob support to have CONFIG_OF_LIBFDT and CONFIG_FIT independant
FDT support is used for both FIT style images and architectures (ppc, m68k, sparc) that can pass a fdt blob to an OS..
For other arch and board which do not pass a fdt blob to an OS but want to use the new uImage format, we just need FIT support (ex : ARM).
Now we can have the 3 following configurations :
1) FIT only CONFIG_FIT
2) fdt blob only CONFIG_OF_LIBFDT
3) both CONFIG_OF_LIBFDT & CONFIG_FIT
---
V3 sorry this one is the good one
Best Regards,
J.
common/image.c | 4 +++- include/image.h | 4 ---- libfdt/Makefile | 8 ++++++-- 3 files changed, 9 insertions(+), 7 deletions(-)
common/image.c +3 -1
@@ -1071,8 +1071,9 @@ int boot_ramdisk_high (struct lmb *lmb, ulong rd_data, ulong rd_len, error:return -1 ; } +#endif /* defined(CONFIG_PPC) || defined(CONFIG_M68K) || defined(CONFIG_SPARC) */ -#ifdef CONFIG_OF_LIBFDT +#if defined(CONFIG_OF_LIBFDT )static void fdt_error (const char *msg) {puts ("ERROR: " );
@@ -1575,6 +1576,7 @@ error: }#endif /* CONFIG_OF_LIBFDT */ +#if defined(CONFIG_PPC ) || defined(CONFIG_M68K )/** * boot_get_cmdline - allocate and initialize kernel cmdline * @lmb: pointer to lmb handle, will be used for memory mgmt
include/image.h +0 -4
@@ -50,10 +50,6 @@#endif /* USE_HOSTCC */ -#if defined(CONFIG_FIT ) && !defined(CONFIG_OF_LIBFDT ) -#error "CONFIG_OF_LIBFDT not enabled, required by CONFIG_FIT!" -#endif -#include <command.h> #if defined(CONFIG_FIT )
libfdt/Makefile +6 -2
@@ -27,9 +27,13 @@ LIB = $(obj)libfdt.a SOBJS = -COBJS-$(CONFIG_OF_LIBFDT) += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o +COBJS-libfdt += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o -COBJS := $(COBJS-y) +COBJS-$(CONFIG_OF_LIBFDT) += $(COBJS-libfdt) +COBJS-$(CONFIG_FIT) += $(COBJS-libfdt) + + +COBJS := $(sort $(COBJS-y)) SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS))
Signature · 1 line
1.5.6.5
PATCH 1/1 Fix FIT and fdt blob support to have CONFIG_OF_LIBFDT and CONFIG_FIT independant
Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 5 lines · In message <[email protected]> you wrote:
FDT support is used for both FIT style images and architectures (ppc, m68k, sparc) that can pass a fdt blob to an OS..
For other arch and board which do not pass a fdt blob to an OS but want to use the new uImage format, we just need FIT support (ex : ARM).
Please see comments to previous patch.
Best regards,
Wolfgang Denk
Signature · 7 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Carelessly planned projects take three times longer to complete than expected. Carefully planned projects take four times longer to complete than expected, mostly because the planners expect their planning to reduce the time it takes.
PATCH v2 1/1 Fix FIT and fdt blob support to have CONFIG_OF_LIBFDT and CONFIG_FIT independant
FDT support is used for both FIT style images and architectures (ppc, m68k, sparc) that can pass a fdt blob to an OS..
For other arch and board which do not pass a fdt blob to an OS but want to use the new uImage format, we just need FIT support (ex : ARM).
Now we can have the 4 following configurations :
1) FIT only CONFIG_FIT
2) fdt blob only CONFIG_OF_LIBFDT
3) both CONFIG_OF_LIBFDT & CONFIG_FIT
4) none none
And remove arch macro ifdef and use 2 new config for the functionnality to enable
so we introduce these :
CONFIG_BOOT_INIT_RAMDISK
Define this if you want support for relocating init ramdisk
CONFIG_BOOT_INIT_RAMDISK
Define this if you want support for allocate and initialize kernel information for booting
and create a new arch default config where these new are manage for ppc, m68k, sparc actually via include/configs/{arch}/default.h
--- README | 9 +++++++++ common/image.c | 6 ++++-- include/configs/arm/default.h | 29 +++++++++++++++++++++++++++++ include/configs/avr32/default.h | 29 +++++++++++++++++++++++++++++ include/configs/blackfin/default.h | 29 +++++++++++++++++++++++++++++ include/configs/i386/default.h | 29 +++++++++++++++++++++++++++++ include/configs/m68k/default.h | 32 ++++++++++++++++++++++++++++++++ include/configs/microblaze/default.h | 29 +++++++++++++++++++++++++++++ include/configs/mips/default.h | 29 +++++++++++++++++++++++++++++ include/configs/nios/default.h | 29 +++++++++++++++++++++++++++++ include/configs/nios2/default.h | 29 +++++++++++++++++++++++++++++ include/configs/ppc/default.h | 32 ++++++++++++++++++++++++++++++++ include/configs/sh/default.h | 29 +++++++++++++++++++++++++++++ include/configs/sparc/default.h | 31 +++++++++++++++++++++++++++++++ include/image.h | 4 ---- libfdt/Makefile | 7 +++++-- mkconfig | 1 + 17 files changed, 375 insertions(+), 8 deletions(-) create mode 100644 include/configs/arm/default.h create mode 100644 include/configs/avr32/default.h create mode 100644 include/configs/blackfin/default.h create mode 100644 include/configs/i386/default.h create mode 100644 include/configs/m68k/default.h create mode 100644 include/configs/microblaze/default.h create mode 100644 include/configs/mips/default.h create mode 100644 include/configs/nios/default.h create mode 100644 include/configs/nios2/default.h create mode 100644 include/configs/ppc/default.h create mode 100644 include/configs/sh/default.h create mode 100644 include/configs/sparc/default.h
README +9 -0
@@ -381,6 +381,15 @@ The following options need to be configured: This define fills in the correct boot CPU in the boot param header, the default value is zero if undefined. + CONFIG_BOOT_INIT_RAMDISK + + Define this if you want support for relocating init ramdisk + + CONFIG_BOOT_INIT_KERNEL_INFO + + Define this if you want support for allocate and initialize kernel + information for booting + - Serial Ports: CONFIG_PL010_SERIAL
common/image.c +4 -2
@@ -982,7 +982,7 @@ int boot_get_ramdisk (int argc, char *argv[], bootm_headers_t *images,return 0 ; } -#if defined(CONFIG_PPC ) || defined(CONFIG_M68K ) || defined(CONFIG_SPARC ) +#if defined(CONFIG_BOOT_INIT_RAMDISK )/** * boot_ramdisk_high - relocate init ramdisk * @lmb: pointer to lmb handle, will be used for memory mgmt
@@ -1071,6 +1071,7 @@ int boot_ramdisk_high (struct lmb *lmb, ulong rd_data, ulong rd_len, error: return -1; } +#endif /* CONFIG_BOOT_INIT_RAMDISK */ #ifdef CONFIG_OF_LIBFDT static void fdt_error (const char *msg)
@@ -1575,6 +1576,7 @@ error: }#endif /* CONFIG_OF_LIBFDT */ +#if defined(CONFIG_BOOT_INIT_KERNEL_INFO )/** * boot_get_cmdline - allocate and initialize kernel cmdline * @lmb: pointer to lmb handle, will be used for memory mgmt
@@ -1649,7 +1651,7 @@ int boot_get_kbd (struct lmb *lmb, bd_t **kbd, ulong bootmap_base)return 0 ; } -#endif /* CONFIG_PPC || CONFIG_M68K */ +#endif /* CONFIG_BOOT_INIT_KERNEL_INFO */ #endif /* !USE_HOSTCC */ #if defined(CONFIG_FIT )
include/configs/arm/default.h new file +29 -0
@@ -0,0 +1,29 @@ +/* +* (C) Copyright 2008 +* Jean-Christophe PLAGNIOL-VILLARD <[email protected]> +* +* See file CREDITS for list of people who contributed to this +* project. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 of +* the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, +* MA 02111-1307 USA +*/ + +#ifndef _CONFIG_ARM_DEFAULT_H_ +#define _CONFIG_ARM_DEFAULT_H_ + +/* define hear always active CONFIG_ for arm */ + +#endif /* _CONFIG_ARM_DEFAULT_H_ */
include/configs/avr32/default.h new file +29 -0
@@ -0,0 +1,29 @@ +/* +* (C) Copyright 2008 +* Jean-Christophe PLAGNIOL-VILLARD <[email protected]> +* +* See file CREDITS for list of people who contributed to this +* project. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 of +* the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, +* MA 02111-1307 USA +*/ + +#ifndef _CONFIG_AVR32_DEFAULT_H_ +#define _CONFIG_AVR32_DEFAULT_H_ + +/* define hear always active CONFIG_ for avr32 */ + +#endif /* _CONFIG_AVR32_DEFAULT_H_ */
include/configs/blackfin/default.h new file +29 -0
@@ -0,0 +1,29 @@ +/* +* (C) Copyright 2008 +* Jean-Christophe PLAGNIOL-VILLARD <[email protected]> +* +* See file CREDITS for list of people who contributed to this +* project. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 of +* the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, +* MA 02111-1307 USA +*/ + +#ifndef _CONFIG_BLACKFIN_DEFAULT_H_ +#define _CONFIG_BLACKFIN_DEFAULT_H_ + +/* define hear always active CONFIG_ for blackfin */ + +#endif /* _CONFIG_BLACKFIN_DEFAULT_H_ */
include/configs/i386/default.h new file +29 -0
@@ -0,0 +1,29 @@ +/* +* (C) Copyright 2008 +* Jean-Christophe PLAGNIOL-VILLARD <[email protected]> +* +* See file CREDITS for list of people who contributed to this +* project. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 of +* the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, +* MA 02111-1307 USA +*/ + +#ifndef _CONFIG_I386_DEFAULT_H_ +#define _CONFIG_I386_DEFAULT_H_ + +/* define hear always active CONFIG_ for i386 */ + +#endif /* _CONFIG_I386_DEFAULT_H_ */
include/configs/m68k/default.h new file +32 -0
@@ -0,0 +1,32 @@ +/* +* (C) Copyright 2008 +* Jean-Christophe PLAGNIOL-VILLARD <[email protected]> +* +* See file CREDITS for list of people who contributed to this +* project. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 of +* the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, +* MA 02111-1307 USA +*/ + +#ifndef _CONFIG_M68K_DEFAULT_H_ +#define _CONFIG_M68K_DEFAULT_H_ + +/* define hear always active CONFIG_ for m68k */ + +#define CONFIG_BOOT_INIT_KERNEL_INFO /* init kernel information for booting */ +#define CONFIG_BOOT_INIT_RAMDISK /* init init ramdisk for booting */ + +#endif /* _CONFIG_M68K_DEFAULT_H_ */
include/configs/microblaze/default.h new file +29 -0
@@ -0,0 +1,29 @@ +/* +* (C) Copyright 2008 +* Jean-Christophe PLAGNIOL-VILLARD <[email protected]> +* +* See file CREDITS for list of people who contributed to this +* project. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 of +* the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, +* MA 02111-1307 USA +*/ + +#ifndef _CONFIG_MICROBLAZE_DEFAULT_H_ +#define _CONFIG_MICROBLAZE_DEFAULT_H_ + +/* define hear always active CONFIG_ for microblaze */ + +#endif /* _CONFIG_MICROBLAZE_DEFAULT_H_ */
include/configs/mips/default.h new file +29 -0
@@ -0,0 +1,29 @@ +/* +* (C) Copyright 2008 +* Jean-Christophe PLAGNIOL-VILLARD <[email protected]> +* +* See file CREDITS for list of people who contributed to this +* project. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 of +* the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, +* MA 02111-1307 USA +*/ + +#ifndef _CONFIG_MIPS_DEFAULT_H_ +#define _CONFIG_MIPS_DEFAULT_H_ + +/* define hear always active CONFIG_ for mips */ + +#endif /* _CONFIG_MIPS_DEFAULT_H_ */
include/configs/nios/default.h new file +29 -0
@@ -0,0 +1,29 @@ +/* +* (C) Copyright 2008 +* Jean-Christophe PLAGNIOL-VILLARD <[email protected]> +* +* See file CREDITS for list of people who contributed to this +* project. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 of +* the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, +* MA 02111-1307 USA +*/ + +#ifndef _CONFIG_NIOS_DEFAULT_H_ +#define _CONFIG_NIOS_DEFAULT_H_ + +/* define hear always active CONFIG_ for nios */ + +#endif /* _CONFIG_NIOS_DEFAULT_H_ */
include/configs/nios2/default.h new file +29 -0
@@ -0,0 +1,29 @@ +/* +* (C) Copyright 2008 +* Jean-Christophe PLAGNIOL-VILLARD <[email protected]> +* +* See file CREDITS for list of people who contributed to this +* project. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 of +* the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, +* MA 02111-1307 USA +*/ + +#ifndef _CONFIG_NIOS2_DEFAULT_H_ +#define _CONFIG_NIOS2_DEFAULT_H_ + +/* define hear always active CONFIG_ for nios2 */ + +#endif /* _CONFIG_NIOS2_DEFAULT_H_ */
include/configs/ppc/default.h new file +32 -0
@@ -0,0 +1,32 @@ +/* +* (C) Copyright 2008 +* Jean-Christophe PLAGNIOL-VILLARD <[email protected]> +* +* See file CREDITS for list of people who contributed to this +* project. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 of +* the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, +* MA 02111-1307 USA +*/ + +#ifndef _CONFIG_PPC_DEFAULT_H_ +#define _CONFIG_PPC_DEFAULT_H_ + +/* define hear always active CONFIG_ for ppc */ + +#define CONFIG_BOOT_INIT_KERNEL_INFO /* init kernel information for booting */ +#define CONFIG_BOOT_INIT_RAMDISK /* init init ramdisk for booting */ + +#endif /* _CONFIG_PPC_DEFAULT_H_ */
include/configs/sh/default.h new file +29 -0
@@ -0,0 +1,29 @@ +/* +* (C) Copyright 2008 +* Jean-Christophe PLAGNIOL-VILLARD <[email protected]> +* +* See file CREDITS for list of people who contributed to this +* project. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 of +* the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, +* MA 02111-1307 USA +*/ + +#ifndef _CONFIG_SH_DEFAULT_H_ +#define _CONFIG_SH_DEFAULT_H_ + +/* define hear always active CONFIG_ for sh */ + +#endif /* _CONFIG_SH_DEFAULT_H_ */
include/configs/sparc/default.h new file +31 -0
@@ -0,0 +1,31 @@ +/* +* (C) Copyright 2008 +* Jean-Christophe PLAGNIOL-VILLARD <[email protected]> +* +* See file CREDITS for list of people who contributed to this +* project. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 of +* the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, +* MA 02111-1307 USA +*/ + +#ifndef _CONFIG_SPARC_DEFAULT_H_ +#define _CONFIG_SPARC_DEFAULT_H_ + +/* define hear always active CONFIG_ for sparc */ + +#define CONFIG_BOOT_INIT_RAMDISK /* init init ramdisk for booting */ + +#endif /* _CONFIG_SPARC_DEFAULT_H_ */
include/image.h +0 -4
@@ -50,10 +50,6 @@#endif /* USE_HOSTCC */ -#if defined(CONFIG_FIT ) && !defined(CONFIG_OF_LIBFDT ) -#error "CONFIG_OF_LIBFDT not enabled, required by CONFIG_FIT!" -#endif -#include <command.h> #if defined(CONFIG_FIT )
libfdt/Makefile +5 -2
@@ -27,9 +27,12 @@ LIB = $(obj)libfdt.a SOBJS = -COBJS-$(CONFIG_OF_LIBFDT) += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o +COBJS-libfdt += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o -COBJS := $(COBJS-y) +COBJS-$(CONFIG_OF_LIBFDT) += $(COBJS-libfdt) +COBJS-$(CONFIG_FIT) += $(COBJS-libfdt) + +COBJS := $(sort $(COBJS-y)) SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS))
mkconfig +1 -0
@@ -82,6 +82,7 @@ else > config.h # Create new config file fi echo "/* Automatically generated - do not edit */" >>config.h +echo "#include <configs/$2/default.h>" >>config.h echo "#include <configs/$1.h>" >>config.h exit 0
Signature · 1 line
1.5.6.5
PATCH v2 1/1 Fix FIT and fdt blob support to have CONFIG_OF_LIBFDT and CONFIG_FIT independant
Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 20 lines · In message <[email protected]> you wrote:
FDT support is used for both FIT style images and architectures (ppc, m68k, sparc) that can pass a fdt blob to an OS..
For other arch and board which do not pass a fdt blob to an OS but want to use the new uImage format, we just need FIT support (ex : ARM).
Now we can have the 4 following configurations :
1) FIT only
CONFIG_FIT
2) fdt blob onlyCONFIG_OF_LIBFDT
3) bothCONFIG_OF_LIBFDT&CONFIG_FIT
4) none noneAnd remove arch macro ifdef and use 2 new config for the functionnality to enable
so we introduce these :
CONFIG_BOOT_INIT_RAMDISKDefine this if you want support for relocating init ramdisk
Quoted 7 lines
CONFIG_BOOT_INIT_RAMDISKDefine this if you want support for allocate and initialize kernel information for booting
and create a new arch default config where these new are manage for ppc, m68k, sparc actually via
include/configs/{arch}/default.h
I reject this patch. It does not solve any problem, instead it only obfuscates the code.
The starting point of this discussion was that we wanted to get rid of architecture specific #defines like "#if defined(PPC)" and use feature-specific defines instead. Now you create such new defines, and move them to architecture specific files, i. e. we leave every- thing as is (architecture dependent), just much more complicated and information distributed over tens of files.
I cannot accept this.
Best regards,
Wolfgang Denk
Signature · 3 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
PATCH 1/1 Fix FDT support to use fdt blob and FIT together or not
In reply to Jean-Christophe PLAGNIOL-VILLARDDear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 2 lines · In message <[email protected]> you wrote:
FDT support is used for both FIT style images and architectures (ppc, m68k, sparc) that can pass a fdt blob to an OS..
We should not fix this on any architectures. They have nothing to do with this.
Please write instead something like this:
FDT support is needed both for passing a FDT blob to an OS and for FIT style images.
Quoted 2 lines
The
BOOTM_STATE_FDTsupport should only exist for the case that we are passing a blob to an OS.
I don't see how this statement is related to your patch?
common/image.c int boot_ramdisk_high (struct lmb *lmb, ulong rd_data, ulong rd_len, · line 1076
Quoted 12 lines
common/image.c +2
-1@@ -1071,8 +1071,9 @@ int boot_ramdisk_high (struct lmb *lmb, ulong rd_data, ulong rd_len, error: return -1; } +#endif /* defined(CONFIG_PPC) || defined(CONFIG_M68K) || defined(CONFIG_SPARC) */ -#ifdef CONFIG_OF_LIBFDT +#if defined(CONFIG_OF_LIBFDT )
This is a NO-OP and could be omitted.
common/image.c error: · line 1579
Quoted 8 lines
common/image.c +1
-0static void fdt_error (const char *msg) {puts ("ERROR: " );@@ -1575,6 +1576,7 @@ error: }#endif /* CONFIG_OF_LIBFDT */ +#if defined(CONFIG_PPC ) || defined(CONFIG_M68K )
Maybe "|| defined(CONFIG_SPARC)" is missing here?
Quoted 5 lines
libfdt/Makefile +0
-0
libfdt/Makefile LIB = $(obj)libfdt.a · line 34
Quoted 9 lines
libfdt/Makefile +5
-1@@ -27,7 +27,11 @@ LIB = $(obj)libfdt.a SOBJS = -COBJS-$(CONFIG_OF_LIBFDT) += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o +COBJS-libfdt += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o + +COBJS-$(CONFIG_OF_LIBFDT) += $(COBJS-libfdt) +COBJS-$(CONFIG_FIT) += $(COBJS-libfdt) +
Would it not be easier to just add a line
COBJS-$(CONFIG_FIT) += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o
?
This would also allow to omit files that are not needed for FIT images (if there are any).
Note: You must also change the Makefile to use
COBJS := $(sort $(COBJS-y))
or we will get errors because of duplicated opjects.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Just about every computer on the market today runs Unix, except the Mac (and nobody cares about it). - Bill Joy 6/21/85
PATCH 1/1 Fix FDT support to use fdt blob and FIT together or not
Quoted 11 lines · Wolfgang Denk
Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 2 lines · In message <[email protected]> you wrote:
FDT support is used for both FIT style images and architectures (ppc, m68k, sparc) that can pass a fdt blob to an OS..
We should not fix this on any architectures. They have nothing to do with this.
Please write instead something like this:
Quoted 7 lines
FDT support is needed both for passing a FDT blob to an OS and for FIT style images.
Quoted 2 lines
The
BOOTM_STATE_FDTsupport should only exist for the case that we are passing a blob to an OS.I don't see how this statement is related to your patch?
I've put the wrong comment please see the last patch
Quoted 23 lines
common/image.c int boot_ramdisk_high (struct lmb *lmb, ulong rd_data, ulong rd_len, · line 1076
Quoted 13 lines
common/image.c +2
-1@@ -1071,8 +1071,9 @@ int boot_ramdisk_high (struct lmb *lmb, ulong rd_data, ulong rd_len, error: return -1; } +#endif /* defined(CONFIG_PPC) || defined(CONFIG_M68K) || defined(CONFIG_SPARC) */ -#ifdef CONFIG_OF_LIBFDT +#if defined(CONFIG_OF_LIBFDT )This is a NO-OP and could be omitted.
Quoted 7 lines
common/image.c +0
-0static void fdt_error (const char *msg) {puts ("ERROR: " );@@ -1575,6 +1576,7 @@ error: }#endif /* CONFIG_OF_LIBFDT */
Quoted 3 lines
common/image.c error: · line 1579
Quoted 1 line
common/image.c +1
-0+#if defined(CONFIG_PPC ) || defined(CONFIG_M68K )Maybe "||
defined(CONFIG_SPARC)" is missing here?
no this function is only use on PPC and M68K bootm
Quoted 18 lines
Quoted 6 lines
libfdt/Makefile +0
-0libfdt/Makefile LIB = $(obj)libfdt.a · line 34
Quoted 9 lines
libfdt/Makefile +5
-1@@ -27,7 +27,11 @@ LIB = $(obj)libfdt.a SOBJS = -COBJS-$(CONFIG_OF_LIBFDT) += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o +COBJS-libfdt += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o + +COBJS-$(CONFIG_OF_LIBFDT) += $(COBJS-libfdt) +COBJS-$(CONFIG_FIT) += $(COBJS-libfdt) +Would it not be easier to just add a line
Quoted 3 lines
COBJS-$(CONFIG_FIT) += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o
?
no because we may do not have the CONFIG_OF_LIBFDT and CONFIG_FIT at the same time
Quoted 10 lines
This would also allow to omit files that are not needed for FIT images (if there are any).
Note: You must also change the
Makefileto useCOBJS := $(sort $(COBJS-y))or we will get errors because of duplicated opjects.
I've it in the last patch
please ignore this one
Best Regards,
J.
PATCH 1/1 Fix FDT support to use fdt blob and FIT together or not
Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 4 lines
Quoted 2 lines
+#if
defined(CONFIG_PPC)||defined(CONFIG_M68K)Maybe "||
defined(CONFIG_SPARC)" is missing here?no this function is only use on PPC and M68K bootm
Then, again, we should add a feature-specific #define here, but not an architecture specific one.
Quoted 9 lines
Quoted 9 lines
libfdt/Makefile LIB = $(obj)libfdt.a · line 34
Quoted 6 lines
libfdt/Makefile +5
-1-COBJS-$(CONFIG_OF_LIBFDT) += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o +COBJS-libfdt += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o + +COBJS-$(CONFIG_OF_LIBFDT) += $(COBJS-libfdt) +COBJS-$(CONFIG_FIT) += $(COBJS-libfdt) +Would it not be easier to just add a line
Quoted 5 lines
Quoted 3 lines
COBJS-$(CONFIG_FIT) += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o
?
no because we may do not have the
CONFIG_OF_LIBFDTandCONFIG_FITat the same time
Yes, of course we may have both at the same time.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Just about every computer on the market today runs Unix, except the Mac (and nobody cares about it). - Bill Joy 6/21/85
PATCH 1/1 Fix FDT support to use fdt blob and FIT together or not
Quoted 11 lines · Wolfgang Denk
Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 4 lines
Quoted 2 lines
+#if
defined(CONFIG_PPC)||defined(CONFIG_M68K)Maybe "||
defined(CONFIG_SPARC)" is missing here?no this function is only use on PPC and M68K bootm
Then, again, we should add a feature-specific #define here, but not an architecture specific one.
I'll think of 2 new CONFIG so
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
libfdt/Makefile LIB = $(obj)libfdt.a · line 34
Quoted 7 lines
libfdt/Makefile +5
-1-COBJS-$(CONFIG_OF_LIBFDT) += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o +COBJS-libfdt += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o + +COBJS-$(CONFIG_OF_LIBFDT) += $(COBJS-libfdt) +COBJS-$(CONFIG_FIT) += $(COBJS-libfdt) +Would it not be easier to just add a line
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
COBJS-$(CONFIG_FIT) += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o
?
no because we may do not have the
CONFIG_OF_LIBFDTandCONFIG_FITat the same timeYes, of course we may have both at the same time.
with my modification
I'd like to support the 3 following configurations :
1) FIT only CONFIG_FIT
2) fdt blob only CONFIG_OF_LIBFDT
3) both CONFIG_OF_LIBFDT & CONFIG_FIT
so CONFIG_FIT could have CONFIG_FIT without CONFIG_OF_LIBFDT
Best Regards,
J.
PATCH 1/1 Fix FDT support to use fdt blob and FIT together or not
Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 2 lines
Yes, of course we may have both at the same time.
with my modification
Quoted 7 lines
I'd like to support the 3 following configurations :
1) FIT only
CONFIG_FIT
2) fdt blob onlyCONFIG_OF_LIBFDT
3) bothCONFIG_OF_LIBFDT&CONFIG_FITso
CONFIG_FITcould haveCONFIG_FITwithoutCONFIG_OF_LIBFDT
Yes, that's what we've been discussing all the time - but let's not forget that there is obviously also case # 4: having none of both.
Best regards,
Wolfgang Denk
Signature · 6 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Quotation, n. The act of repeating erroneously the words of another.
The words erroneously repeated.
- Ambrose Bierce _The Devil's Dictionary_PATCH 1/1 Fix FDT support to use fdt blob and FIT together or not
Quoted 10 lines · Wolfgang Denk
Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 7 lines · In message <[email protected]> you wrote:
Quoted 2 lines
Yes, of course we may have both at the same time.
with my modification
I'd like to support the 3 following configurations :
Quoted 8 lines
Quoted 5 lines
1) FIT only
CONFIG_FIT
2) fdt blob onlyCONFIG_OF_LIBFDT
3) bothCONFIG_OF_LIBFDT&CONFIG_FITso
CONFIG_FITcould haveCONFIG_FITwithoutCONFIG_OF_LIBFDTYes, that's what we've been discussing all the time - but let's not forget that there is obviously also case # 4: having none of both.
yes I'll update the commit message
so
4) none none
Best Regards,
J.