[U-Boot] [PATCH] lib: Add CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE
Usually, when CONFIG_OF_LIBFDT is enabled, U-Boot is set to the FDT memory information that is set in the U-Boot. This patch disables this behavior.
Signed-off-by: Nobuhiro Iwamatsu nobuhiro.iwamatsu.yj@renesas.com --- README | 8 ++++++++ arch/arm/lib/bootm-fdt.c | 2 ++ 2 files changed, 10 insertions(+)
diff --git a/README b/README index d337374..73453fe 100644 --- a/README +++ b/README @@ -650,6 +650,14 @@ The following options need to be configured: in a single configuration file and the machine type is runtime discoverable, do not have to use this setting.
+ CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE + + Usually, when CONFIG_OF_LIBFDT is enabled, U-Boot is set to + the FDT memory information that is set in the U-Boot. This will + disable this behavior. + If you do not use the memory configuration of U-Boot, you want + to set the priority of the FDT, please enable this. + - vxWorks boot parameters:
bootvx constructs a valid bootline using the following diff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c index e40691d..8da9dac 100644 --- a/arch/arm/lib/bootm-fdt.c +++ b/arch/arm/lib/bootm-fdt.c @@ -18,6 +18,7 @@ #include <common.h> #include <fdt_support.h>
+#ifndef CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE DECLARE_GLOBAL_DATA_PTR;
int arch_fixup_memory_node(void *blob) @@ -34,3 +35,4 @@ int arch_fixup_memory_node(void *blob)
return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS); } +#endif /* CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE */
Dear Nobuhiro Iwamatsu,
In message 1396846600-15386-1-git-send-email-nobuhiro.iwamatsu.yj@renesas.com you wrote:
Usually, when CONFIG_OF_LIBFDT is enabled, U-Boot is set to the FDT memory information that is set in the U-Boot. This patch disables this behavior.
Signed-off-by: Nobuhiro Iwamatsu nobuhiro.iwamatsu.yj@renesas.com
README | 8 ++++++++ arch/arm/lib/bootm-fdt.c | 2 ++ 2 files changed, 10 insertions(+)
Please explain why you would want to do this. To me it makes no sense. Either U-Boot knows the correct memory size, then it should pass it to Linux. Or it does not, then U-Boot should be fixed.
Also, I object that your implementation is ARM specific. If such a feature gets added, it should be architecture independent.
Thanks.
Wolfgang Denk
On Mon, Apr 07, 2014 at 08:53:39AM +0200, Wolfgang Denk wrote:
Dear Nobuhiro Iwamatsu,
In message 1396846600-15386-1-git-send-email-nobuhiro.iwamatsu.yj@renesas.com you wrote:
Usually, when CONFIG_OF_LIBFDT is enabled, U-Boot is set to the FDT memory information that is set in the U-Boot. This patch disables this behavior.
Signed-off-by: Nobuhiro Iwamatsu nobuhiro.iwamatsu.yj@renesas.com
README | 8 ++++++++ arch/arm/lib/bootm-fdt.c | 2 ++ 2 files changed, 10 insertions(+)
Please explain why you would want to do this. To me it makes no sense. Either U-Boot knows the correct memory size, then it should pass it to Linux. Or it does not, then U-Boot should be fixed.
Also, I object that your implementation is ARM specific. If such a feature gets added, it should be architecture independent.
This has shown up before in the context of platforms with >4GB memory and we "correct" the node by reducing the system memory, which is incorrect. I agree this needs to be done for more than just ARM however.
Hi,
2014-04-08 2:41 GMT+09:00 Tom Rini trini@ti.com:
On Mon, Apr 07, 2014 at 08:53:39AM +0200, Wolfgang Denk wrote:
Dear Nobuhiro Iwamatsu,
In message 1396846600-15386-1-git-send-email-nobuhiro.iwamatsu.yj@renesas.com you wrote:
Usually, when CONFIG_OF_LIBFDT is enabled, U-Boot is set to the FDT memory information that is set in the U-Boot. This patch disables this behavior.
Signed-off-by: Nobuhiro Iwamatsu nobuhiro.iwamatsu.yj@renesas.com
README | 8 ++++++++ arch/arm/lib/bootm-fdt.c | 2 ++ 2 files changed, 10 insertions(+)
Please explain why you would want to do this. To me it makes no sense. Either U-Boot knows the correct memory size, then it should pass it to Linux. Or it does not, then U-Boot should be fixed.
Also, I object that your implementation is ARM specific. If such a feature gets added, it should be architecture independent.
This has shown up before in the context of platforms with >4GB memory and we "correct" the node by reducing the system memory, which is incorrect. I agree this needs to be done for more than just ARM however.
It is one of the reasons of this patch that you wrote.
Thanks, Nobuhiro
-- Tom
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi,
Thanks for your comment.
2014-04-07 15:53 GMT+09:00 Wolfgang Denk wd@denx.de:
Dear Nobuhiro Iwamatsu,
In message 1396846600-15386-1-git-send-email-nobuhiro.iwamatsu.yj@renesas.com you wrote:
Usually, when CONFIG_OF_LIBFDT is enabled, U-Boot is set to the FDT memory information that is set in the U-Boot. This patch disables this behavior.
Signed-off-by: Nobuhiro Iwamatsu nobuhiro.iwamatsu.yj@renesas.com
README | 8 ++++++++ arch/arm/lib/bootm-fdt.c | 2 ++ 2 files changed, 10 insertions(+)
Please explain why you would want to do this. To me it makes no sense. Either U-Boot knows the correct memory size, then it should pass it to Linux. Or it does not, then U-Boot should be fixed.
For example, I can access the memory of all in the U-Boot, but I may want to control the highmem on Linux,I do not want to show a specific area from kernel and userland.
Also, I object that your implementation is ARM specific. If such a feature gets added, it should be architecture independent.
I see. But arch_fixup_memory_node() is used by ARM only. So, we see to be dependent on the ARM is only this.
Thanks.
Wolfgang Denk
-- 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@denx.de To be a winner, all you need to give is all you have. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Dear Nobuhiro,
In message CABMQnV+k+Rmx7E8o-nfBpYg5-nWrXi6Oz_+BCYs-vDNdv_z-rw@mail.gmail.com you wrote:
Please explain why you would want to do this. To me it makes no sense. Either U-Boot knows the correct memory size, then it should pass it to Linux. Or it does not, then U-Boot should be fixed.
For example, I can access the memory of all in the U-Boot, but I may want to control the highmem on Linux,I do not want to show a specific area from kernel and userland.
Is it not sufficient to pass some "mem=" boot argument? We even have automatic support for this in U-Boot (see the CONFIG_PRAM feature).
Also, I object that your implementation is ARM specific. If such a feature gets added, it should be architecture independent.
I see. But arch_fixup_memory_node() is used by ARM only. So, we see to be dependent on the ARM is only this.
All architectures that support the device tree update the memory size for Linux, so we should find a generic way to handle this. Actually we should always strive to reduce this arhitecture specific code.
Best regards,
Wolfgang Denk
On Tue, Apr 08, 2014 at 03:05:36PM +0200, Wolfgang Denk wrote:
Dear Nobuhiro,
In message CABMQnV+k+Rmx7E8o-nfBpYg5-nWrXi6Oz_+BCYs-vDNdv_z-rw@mail.gmail.com you wrote:
Please explain why you would want to do this. To me it makes no sense. Either U-Boot knows the correct memory size, then it should pass it to Linux. Or it does not, then U-Boot should be fixed.
For example, I can access the memory of all in the U-Boot, but I may want to control the highmem on Linux,I do not want to show a specific area from kernel and userland.
Is it not sufficient to pass some "mem=" boot argument? We even have automatic support for this in U-Boot (see the CONFIG_PRAM feature).
There's various ways to do this, yes. But it doesn't cover the >4GB case.
Also, I object that your implementation is ARM specific. If such a feature gets added, it should be architecture independent.
I see. But arch_fixup_memory_node() is used by ARM only. So, we see to be dependent on the ARM is only this.
All architectures that support the device tree update the memory size for Linux, so we should find a generic way to handle this. Actually we should always strive to reduce this arhitecture specific code.
Note that ARM provides arch_fixup_memory_node to make sure we have all of the bank information populated and then calls fdt_fixup_memory_banks, while PowerPC just calls fdt_fixup_memory which calls banks with a '1' for number of banks. MIPS (and everyone else) isn't doing anything about this atm, but probably should.
At the high level, we need to see if we _really_ do need to be using arch_fixup_memory_node at all because my gut feeling is (a) we've already always filled in the bank info and if not (b) that is a bug to correct. But I haven't dived in to the relevant code here yet.
Hello Tom,
Note that ARM provides arch_fixup_memory_node to make sure we have all of the bank information populated and then calls fdt_fixup_memory_banks, while PowerPC just calls fdt_fixup_memory which calls banks with a '1' for number of banks. MIPS (and everyone else) isn't doing anything about this atm, but probably should.
I assume the main reason this is not done for MIPS is the missing support for providing devicetrees from U-Boot.
Daniel: Do you know if there is any activity to add this?
Best Regards, Thomas
Hi Nobuhiro, Tom,
diff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c index e40691d..8da9dac 100644 --- a/arch/arm/lib/bootm-fdt.c +++ b/arch/arm/lib/bootm-fdt.c @@ -18,6 +18,7 @@ #include <common.h> #include <fdt_support.h>
+#ifndef CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE DECLARE_GLOBAL_DATA_PTR;
int arch_fixup_memory_node(void *blob) @@ -34,3 +35,4 @@ int arch_fixup_memory_node(void *blob)
return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS); } +#endif /* CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE */
I am not happy about defining CONFIG macro to disable some code.
Please do
#ifdef CONFIG_FDT_FIXUP_MEMORY_NODE ..... #endif
rather than
#ifndef CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE ..... #endif
We expect most of boards should be fixed-up by U-Boot. So, add
#define CONFIG_FDT_FIXUP_MEMORY_NODE
to include/config_defaults.h
and
#undef CONFIG_FDT_FIXUP_MEMORY_NODE
only to boards for which you want to skip memory fix-up.
Basically, we should not use CONFIG macros for negation.
CONFIG_SKIP_LOWLEVEL_INIT, CONFIG_SYS_DCACHE_OFF, are examples of bad macros.
See
ifndef CONFIG_SKIP_LOWLEVEL_INIT obj-y += lowlevel_init.o endif
everywhere in Makefiles, which suggests we had chosen a bad macro name.
Best Regards Masahiro Yamada
On Wed, Apr 09, 2014 at 12:20:43PM +0900, Masahiro Yamada wrote:
Hi Nobuhiro, Tom,
diff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c index e40691d..8da9dac 100644 --- a/arch/arm/lib/bootm-fdt.c +++ b/arch/arm/lib/bootm-fdt.c @@ -18,6 +18,7 @@ #include <common.h> #include <fdt_support.h>
+#ifndef CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE DECLARE_GLOBAL_DATA_PTR;
int arch_fixup_memory_node(void *blob) @@ -34,3 +35,4 @@ int arch_fixup_memory_node(void *blob)
return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS); } +#endif /* CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE */
I am not happy about defining CONFIG macro to disable some code.
Please do
#ifdef CONFIG_FDT_FIXUP_MEMORY_NODE ..... #endif
rather than
#ifndef CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE ..... #endif
We expect most of boards should be fixed-up by U-Boot. So, add
#define CONFIG_FDT_FIXUP_MEMORY_NODE
to include/config_defaults.h
and
#undef CONFIG_FDT_FIXUP_MEMORY_NODE
only to boards for which you want to skip memory fix-up.
Agreed.
Basically, we should not use CONFIG macros for negation.
CONFIG_SKIP_LOWLEVEL_INIT, CONFIG_SYS_DCACHE_OFF, are examples of bad macros.
Lets hold off on fixing these until we're farther along with the conversion to Kconfig. Unless it'll be really problematic not to.. Thanks!
participants (5)
-
Masahiro Yamada -
Nobuhiro Iwamatsu -
thomas.langer＠lantiq.com -
Tom Rini -
Wolfgang Denk