* [U-Boot] [PATCH] lib: Add CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE
@ 2014-04-07 4:56 Nobuhiro Iwamatsu
2014-04-07 6:53 ` Wolfgang Denk
0 siblings, 1 reply; 10+ messages in thread
From: Nobuhiro Iwamatsu @ 2014-04-07 4:56 UTC (permalink / raw)
To: u-boot
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 */
--
1.8.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] lib: Add CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE
2014-04-07 4:56 [U-Boot] [PATCH] lib: Add CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE Nobuhiro Iwamatsu
@ 2014-04-07 6:53 ` Wolfgang Denk
2014-04-07 17:41 ` Tom Rini
2014-04-08 1:50 ` Nobuhiro Iwamatsu
0 siblings, 2 replies; 10+ messages in thread
From: Wolfgang Denk @ 2014-04-07 6:53 UTC (permalink / raw)
To: u-boot
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
--
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
To be a winner, all you need to give is all you have.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] lib: Add CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE
2014-04-07 6:53 ` Wolfgang Denk
@ 2014-04-07 17:41 ` Tom Rini
2014-04-08 1:54 ` Nobuhiro Iwamatsu
2014-04-08 1:50 ` Nobuhiro Iwamatsu
1 sibling, 1 reply; 10+ messages in thread
From: Tom Rini @ 2014-04-07 17:41 UTC (permalink / raw)
To: u-boot
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.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140407/0ae7f440/attachment.pgp>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] lib: Add CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE
2014-04-07 6:53 ` Wolfgang Denk
2014-04-07 17:41 ` Tom Rini
@ 2014-04-08 1:50 ` Nobuhiro Iwamatsu
2014-04-08 13:05 ` Wolfgang Denk
1 sibling, 1 reply; 10+ messages in thread
From: Nobuhiro Iwamatsu @ 2014-04-08 1:50 UTC (permalink / raw)
To: 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 at denx.de
> To be a winner, all you need to give is all you have.
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
--
Nobuhiro Iwamatsu
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] lib: Add CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE
2014-04-07 17:41 ` Tom Rini
@ 2014-04-08 1:54 ` Nobuhiro Iwamatsu
0 siblings, 0 replies; 10+ messages in thread
From: Nobuhiro Iwamatsu @ 2014-04-08 1:54 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
--
Nobuhiro Iwamatsu
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] lib: Add CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE
2014-04-08 1:50 ` Nobuhiro Iwamatsu
@ 2014-04-08 13:05 ` Wolfgang Denk
2014-04-08 16:17 ` Tom Rini
0 siblings, 1 reply; 10+ messages in thread
From: Wolfgang Denk @ 2014-04-08 13:05 UTC (permalink / raw)
To: 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
--
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
This restaurant was advertising breakfast any time. So I ordered
french toast in the renaissance. - Steven Wright, comedian
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] lib: Add CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE
2014-04-08 13:05 ` Wolfgang Denk
@ 2014-04-08 16:17 ` Tom Rini
2014-04-08 16:39 ` thomas.langer at lantiq.com
2014-04-09 3:20 ` Masahiro Yamada
0 siblings, 2 replies; 10+ messages in thread
From: Tom Rini @ 2014-04-08 16:17 UTC (permalink / raw)
To: u-boot
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.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140408/71b974f4/attachment.pgp>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] lib: Add CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE
2014-04-08 16:17 ` Tom Rini
@ 2014-04-08 16:39 ` thomas.langer at lantiq.com
2014-04-09 3:20 ` Masahiro Yamada
1 sibling, 0 replies; 10+ messages in thread
From: thomas.langer at lantiq.com @ 2014-04-08 16:39 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] lib: Add CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE
2014-04-08 16:17 ` Tom Rini
2014-04-08 16:39 ` thomas.langer at lantiq.com
@ 2014-04-09 3:20 ` Masahiro Yamada
2014-04-09 12:27 ` Tom Rini
1 sibling, 1 reply; 10+ messages in thread
From: Masahiro Yamada @ 2014-04-09 3:20 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] lib: Add CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE
2014-04-09 3:20 ` Masahiro Yamada
@ 2014-04-09 12:27 ` Tom Rini
0 siblings, 0 replies; 10+ messages in thread
From: Tom Rini @ 2014-04-09 12:27 UTC (permalink / raw)
To: u-boot
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!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140409/937df1b2/attachment.pgp>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-04-09 12:27 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-07 4:56 [U-Boot] [PATCH] lib: Add CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE Nobuhiro Iwamatsu
2014-04-07 6:53 ` Wolfgang Denk
2014-04-07 17:41 ` Tom Rini
2014-04-08 1:54 ` Nobuhiro Iwamatsu
2014-04-08 1:50 ` Nobuhiro Iwamatsu
2014-04-08 13:05 ` Wolfgang Denk
2014-04-08 16:17 ` Tom Rini
2014-04-08 16:39 ` thomas.langer at lantiq.com
2014-04-09 3:20 ` Masahiro Yamada
2014-04-09 12:27 ` Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.