[PATCH] ARM: Versatile Express: Add default memory layout

[PATCH] ARM: Versatile Express: Add default memory layout

[Catalin Marinas]

Since this platform is known to have 1GB of RAM at 0x60000000, add a
.fixup function to set the defaults in case no "mem=" option is passed
on the kernel command line.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Russell King <rmk@arm.linux.org.uk>
---
 arch/arm/mach-vexpress/ct-ca9x4.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-vexpress/ct-ca9x4.c b/arch/arm/mach-vexpress/ct-ca9x4.c
index 6353459..3556284 100644
--- a/arch/arm/mach-vexpress/ct-ca9x4.c
+++ b/arch/arm/mach-vexpress/ct-ca9x4.c
@@ -16,6 +16,7 @@
 #include <asm/hardware/gic.h>
 #include <asm/mach-types.h>
 #include <asm/pmu.h>
+#include <asm/setup.h>
 
 #include <mach/clkdev.h>
 #include <mach/ct-ca9x4.h>
@@ -32,6 +33,17 @@
 
 #define V2M_PA_CS7	0x10000000
 
+void ct_ca9x4_fixup(struct machine_desc *mdesc, struct tag *tags, char **from,
+		    struct meminfo *meminfo)
+{
+	/*
+	 * 1GB at 0x60000000.
+	 */
+	meminfo->bank[0].start = 0x60000000;
+	meminfo->bank[0].size = SZ_1G;
+	meminfo->nr_banks = 1;
+}
+
 static struct map_desc ct_ca9x4_io_desc[] __initdata = {
 	{
 		.virtual	= __MMIO_P2V(CT_CA9X4_MPIC),
@@ -240,6 +252,7 @@ MACHINE_START(VEXPRESS, "ARM-Versatile Express CA9x4")
 	.phys_io	= V2M_UART0 & SECTION_MASK,
 	.io_pg_offst	= (__MMIO_P2V(V2M_UART0) >> 18) & 0xfffc,
 	.boot_params	= PHYS_OFFSET + 0x00000100,
+	.fixup		= ct_ca9x4_fixup,
 	.map_io		= ct_ca9x4_map_io,
 	.init_irq	= ct_ca9x4_init_irq,
 #if 0

[PATCH] ARM: Versatile Express: Add default memory layout

[Russell King - ARM Linux]

On Wed, Jul 21, 2010 at 05:55:09PM +0100, Catalin Marinas wrote:
> Since this platform is known to have 1GB of RAM at 0x60000000, add a
> .fixup function to set the defaults in case no "mem=" option is passed
> on the kernel command line.

And the reason why uboot can't pass the RAM information to the kernel is...

[PATCH] ARM: Versatile Express: Add default memory layout

[Catalin Marinas]

On Thu, 2010-07-22 at 11:30 +0100, Russell King - ARM Linux wrote:
> On Wed, Jul 21, 2010 at 05:55:09PM +0100, Catalin Marinas wrote:
> > Since this platform is known to have 1GB of RAM at 0x60000000, add a
> > .fixup function to set the defaults in case no "mem=" option is passed
> > on the kernel command line.
> 
> And the reason why uboot can't pass the RAM information to the kernel
> is...

It needs implementing, or ask the user to pass it on the kernel command
line. For the latter, I think it would be nicer if the kernel has some
sane defaults.

But anyway I'm proposing this patch for similarities to mach-realview.

-- 
Catalin

[PATCH] ARM: Versatile Express: Add default memory layout

[Russell King - ARM Linux]

On Thu, Jul 22, 2010 at 11:54:29AM +0100, Catalin Marinas wrote:
> On Thu, 2010-07-22 at 11:30 +0100, Russell King - ARM Linux wrote:
> > On Wed, Jul 21, 2010 at 05:55:09PM +0100, Catalin Marinas wrote:
> > > Since this platform is known to have 1GB of RAM at 0x60000000, add a
> > > .fixup function to set the defaults in case no "mem=" option is passed
> > > on the kernel command line.
> > 
> > And the reason why uboot can't pass the RAM information to the kernel
> > is...
> 
> It needs implementing, or ask the user to pass it on the kernel command
> line. For the latter, I think it would be nicer if the kernel has some
> sane defaults.

The kernel does have some sane _safe_ defaults - it assumes that there's
at least 16M of RAM.

> But anyway I'm proposing this patch for similarities to mach-realview.

I don't think that's a good enough justification.  We have the fixup for
realview because of the dual RAM mappings in physical space, and the
boot loader doesn't know which one we're using.  That problem is unique
to the Realview platforms, and doesn't exist on any of the other ARM
development platforms.

[PATCH] ARM: Versatile Express: Add default memory layout

[Catalin Marinas]

On Thu, 2010-07-22 at 11:57 +0100, Russell King - ARM Linux wrote:
> On Thu, Jul 22, 2010 at 11:54:29AM +0100, Catalin Marinas wrote:
> > On Thu, 2010-07-22 at 11:30 +0100, Russell King - ARM Linux wrote:
> > > On Wed, Jul 21, 2010 at 05:55:09PM +0100, Catalin Marinas wrote:
> > > > Since this platform is known to have 1GB of RAM at 0x60000000, add a
> > > > .fixup function to set the defaults in case no "mem=" option is passed
> > > > on the kernel command line.
> > >
> > > And the reason why uboot can't pass the RAM information to the kernel
> > > is...
> >
> > It needs implementing, or ask the user to pass it on the kernel command
> > line. For the latter, I think it would be nicer if the kernel has some
> > sane defaults.
> 
> The kernel does have some sane _safe_ defaults - it assumes that there's
> at least 16M of RAM.

That's only during the initial MMU setup. Afterwards it cannot allocate
any memory without a proper mem= parameter or ATAG.

> > But anyway I'm proposing this patch for similarities to mach-realview.
> 
> I don't think that's a good enough justification.  We have the fixup for
> realview because of the dual RAM mappings in physical space, and the
> boot loader doesn't know which one we're using.  That problem is unique
> to the Realview platforms, and doesn't exist on any of the other ARM
> development platforms.

I think this goes down to personal preference - pushing more
configuration towards the bootloader or having some correct defaults in
the kernel. If you prefer the former, that's fine as well. At some point
I hope the device tree would solve all this anyway.

-- 
Catalin

[PATCH] ARM: Versatile Express: Add default memory layout

[Russell King - ARM Linux]

On Thu, Jul 22, 2010 at 12:11:29PM +0100, Catalin Marinas wrote:
> On Thu, 2010-07-22 at 11:57 +0100, Russell King - ARM Linux wrote:
> > On Thu, Jul 22, 2010 at 11:54:29AM +0100, Catalin Marinas wrote:
> > > On Thu, 2010-07-22 at 11:30 +0100, Russell King - ARM Linux wrote:
> > > > On Wed, Jul 21, 2010 at 05:55:09PM +0100, Catalin Marinas wrote:
> > > > > Since this platform is known to have 1GB of RAM at 0x60000000, add a
> > > > > .fixup function to set the defaults in case no "mem=" option is passed
> > > > > on the kernel command line.
> > > >
> > > > And the reason why uboot can't pass the RAM information to the kernel
> > > > is...
> > >
> > > It needs implementing, or ask the user to pass it on the kernel command
> > > line. For the latter, I think it would be nicer if the kernel has some
> > > sane defaults.
> > 
> > The kernel does have some sane _safe_ defaults - it assumes that there's
> > at least 16M of RAM.
> 
> That's only during the initial MMU setup. Afterwards it cannot allocate
> any memory without a proper mem= parameter or ATAG.

That's because uboot passes a tagged list without any ATAG_MEM entries.
If it passed no tagged list, then the defaults will be used.  If it
passed the ATAG_MEM entry, it'd also work.

But as it does neither, it falls into the hole.

The kernel boot requirements document has for the last _8_ years said:

   The boot loader must pass at a minimum the size and location of the system
   memory, and root filesystem location. Therefore, the minimum tagged list
   should look:

           +-----------+
   base -> | ATAG_CORE |  |
           +-----------+  |
           | ATAG_MEM  |  | increasing address
           +-----------+  |
           | ATAG_NONE |  |
           +-----------+  v

and still u-boot gets it wrong...  Now tell me why I hate boot loaders
soo much.

[PATCH] ARM: Versatile Express: Add default memory layout

[Catalin Marinas]

On Thu, 2010-07-22 at 12:16 +0100, Russell King - ARM Linux wrote:
> The kernel boot requirements document has for the last _8_ years said:
> 
>    The boot loader must pass at a minimum the size and location of the system
>    memory, and root filesystem location. Therefore, the minimum tagged list
>    should look:
> 
>            +-----------+
>    base -> | ATAG_CORE |  |
>            +-----------+  |
>            | ATAG_MEM  |  | increasing address
>            +-----------+  |
>            | ATAG_NONE |  |
>            +-----------+  v
> 
> and still u-boot gets it wrong...  Now tell me why I hate boot loaders
> soo much.

Maybe it's time to ignore all boot loaders and do everything in the
kernel :)

-- 
Catalin

[PATCH] ARM: Versatile Express: Add default memory layout

[Russell King - ARM Linux]

On Thu, Jul 22, 2010 at 12:19:56PM +0100, Catalin Marinas wrote:
> On Thu, 2010-07-22 at 12:16 +0100, Russell King - ARM Linux wrote:
> > The kernel boot requirements document has for the last _8_ years said:
> > 
> >    The boot loader must pass at a minimum the size and location of the system
> >    memory, and root filesystem location. Therefore, the minimum tagged list
> >    should look:
> > 
> >            +-----------+
> >    base -> | ATAG_CORE |  |
> >            +-----------+  |
> >            | ATAG_MEM  |  | increasing address
> >            +-----------+  |
> >            | ATAG_NONE |  |
> >            +-----------+  v
> > 
> > and still u-boot gets it wrong...  Now tell me why I hate boot loaders
> > soo much.
> 
> Maybe it's time to ignore all boot loaders and do everything in the
> kernel :)

Maybe, but then we can never do things like pass command line arguments
to the kernel, or discover where system RAM is, etc.

It's also an argument for avoiding DT!  If boot loaders can't even get
the most basic of information right...

[PATCH] ARM: Versatile Express: Add default memory layout

[Catalin Marinas]

On Thu, 2010-07-22 at 12:30 +0100, Russell King - ARM Linux wrote:
> On Thu, Jul 22, 2010 at 12:19:56PM +0100, Catalin Marinas wrote:
> > On Thu, 2010-07-22 at 12:16 +0100, Russell King - ARM Linux wrote:
> > > The kernel boot requirements document has for the last _8_ years said:
> > >
> > >    The boot loader must pass at a minimum the size and location of the system
> > >    memory, and root filesystem location. Therefore, the minimum tagged list
> > >    should look:
> > >
> > >            +-----------+
> > >    base -> | ATAG_CORE |  |
> > >            +-----------+  |
> > >            | ATAG_MEM  |  | increasing address
> > >            +-----------+  |
> > >            | ATAG_NONE |  |
> > >            +-----------+  v
> > >
> > > and still u-boot gets it wrong...  Now tell me why I hate boot loaders
> > > soo much.
> >
> > Maybe it's time to ignore all boot loaders and do everything in the
> > kernel :)
> 
> Maybe, but then we can never do things like pass command line arguments
> to the kernel, or discover where system RAM is, etc.

I should have rephrased/clarified 'ignore'. It's more like ignoring
incomplete information coming from the bootloader and using sane
defaults in the kernel.

Anyway, I don't think it's worth arguing much here, we've been living
with this issues for many years.

> It's also an argument for avoiding DT!  If boot loaders can't even get
> the most basic of information right...

With DT I see it as slightly different. The DT information is provided
by the board manufacturer and the boot loader doesn't need to do much
discovery. It can be a pretty dumb boot loader just passing the relevant
DT address to the kernel. But that's another thread of discussion (which
unfortunately I didn't have time to follow properly).

-- 
Catalin

[PATCH] ARM: Versatile Express: Add default memory layout

[Russell King - ARM Linux]

On Thu, Jul 22, 2010 at 12:40:23PM +0100, Catalin Marinas wrote:
> On Thu, 2010-07-22 at 12:30 +0100, Russell King - ARM Linux wrote:
> > It's also an argument for avoiding DT!  If boot loaders can't even get
> > the most basic of information right...
> 
> With DT I see it as slightly different. The DT information is provided
> by the board manufacturer and the boot loader doesn't need to do much
> discovery. It can be a pretty dumb boot loader just passing the relevant
> DT address to the kernel. But that's another thread of discussion (which
> unfortunately I didn't have time to follow properly).

The boot loader is also provided by the board manufacturer...  I see it
as being not much different.