Discussion:
[ORLinux] [PATCH 02/28] openrisc: use boot_command_line instead of private cmd_line
Rob Herring
2013-09-16 23:08:58 UTC
Permalink
From: Rob Herring <rob.herring at calxeda.com>

Save some pointless copying of the kernel command line and just use
boot_command_line instead. The DT code already handles CONFIG_CMDLINE,
so a separate copy is not needed.

Signed-off-by: Rob Herring <rob.herring at calxeda.com>
Cc: Jonas Bonn <jonas at southpole.se>
Cc: linux at lists.openrisc.net
---
arch/openrisc/kernel/prom.c | 7 +------
arch/openrisc/kernel/setup.c | 4 +---
2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/arch/openrisc/kernel/prom.c b/arch/openrisc/kernel/prom.c
index a63e768..bf3fd05 100644
--- a/arch/openrisc/kernel/prom.c
+++ b/arch/openrisc/kernel/prom.c
@@ -47,8 +47,6 @@
#include <asm/sections.h>
#include <asm/setup.h>

-extern char cmd_line[COMMAND_LINE_SIZE];
-
void __init early_init_dt_add_memory_arch(u64 base, u64 size)
{
size &= PAGE_MASK;
@@ -67,15 +65,12 @@ void __init early_init_devtree(void *params)
* device-tree, including the platform type, initrd location and
* size, TCE reserve, and more ...
*/
- of_scan_flat_dt(early_init_dt_scan_chosen, cmd_line);
+ of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);

/* Scan memory nodes and rebuild MEMBLOCKs */
of_scan_flat_dt(early_init_dt_scan_root, NULL);
of_scan_flat_dt(early_init_dt_scan_memory, NULL);

- /* Save command line for /proc/cmdline and then parse parameters */
- strlcpy(boot_command_line, cmd_line, COMMAND_LINE_SIZE);
-
memblock_allow_resize();

/* We must copy the flattend device tree from init memory to regular
diff --git a/arch/openrisc/kernel/setup.c b/arch/openrisc/kernel/setup.c
index d7359ff..719c5c8 100644
--- a/arch/openrisc/kernel/setup.c
+++ b/arch/openrisc/kernel/setup.c
@@ -50,8 +50,6 @@

#include "vmlinux.h"

-char __initdata cmd_line[COMMAND_LINE_SIZE] = CONFIG_CMDLINE;
-
static unsigned long __init setup_memory(void)
{
unsigned long bootmap_size;
@@ -316,7 +314,7 @@ void __init setup_arch(char **cmdline_p)
conswitchp = &dummy_con;
#endif

- *cmdline_p = cmd_line;
+ *cmdline_p = boot_command_line;

printk(KERN_INFO "OpenRISC Linux -- http://openrisc.net\n");
}
--
1.8.1.2
Rob Herring
2013-09-16 23:09:01 UTC
Permalink
From: Rob Herring <rob.herring at calxeda.com>

Use the common unflatten_and_copy_device_tree to copy the built-in FDT
out of init section. This moves the copy later in the boot, but there
do not appear to be any references to strings in the FDT before the copy.

Signed-off-by: Rob Herring <rob.herring at calxeda.com>
Cc: Jonas Bonn <jonas at southpole.se>
Cc: linux at lists.openrisc.net
---
arch/openrisc/kernel/prom.c | 13 -------------
arch/openrisc/kernel/setup.c | 2 +-
2 files changed, 1 insertion(+), 14 deletions(-)

diff --git a/arch/openrisc/kernel/prom.c b/arch/openrisc/kernel/prom.c
index bf3fd05..3b94972 100644
--- a/arch/openrisc/kernel/prom.c
+++ b/arch/openrisc/kernel/prom.c
@@ -55,8 +55,6 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)

void __init early_init_devtree(void *params)
{
- void *alloc;
-
/* Setup flat device-tree pointer */
initial_boot_params = params;

@@ -72,17 +70,6 @@ void __init early_init_devtree(void *params)
of_scan_flat_dt(early_init_dt_scan_memory, NULL);

memblock_allow_resize();
-
- /* We must copy the flattend device tree from init memory to regular
- * memory because the device tree references the strings in it
- * directly.
- */
-
- alloc = __va(memblock_alloc(initial_boot_params->totalsize, PAGE_SIZE));
-
- memcpy(alloc, initial_boot_params, initial_boot_params->totalsize);
-
- initial_boot_params = alloc;
}

#ifdef CONFIG_BLK_DEV_INITRD
diff --git a/arch/openrisc/kernel/setup.c b/arch/openrisc/kernel/setup.c
index 719c5c8..09a769b 100644
--- a/arch/openrisc/kernel/setup.c
+++ b/arch/openrisc/kernel/setup.c
@@ -283,7 +283,7 @@ void __init setup_arch(char **cmdline_p)
{
unsigned long max_low_pfn;

- unflatten_device_tree();
+ unflatten_and_copy_device_tree();

setup_cpuinfo();
--
1.8.1.2
Rob Herring
2013-09-16 23:09:12 UTC
Permalink
From: Rob Herring <rob.herring at calxeda.com>

Convert openrisc to use new early_init_dt_scan function.

Signed-off-by: Rob Herring <rob.herring at calxeda.com>
Cc: Jonas Bonn <jonas at southpole.se>
Cc: linux at lists.openrisc.net
---
arch/openrisc/kernel/prom.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/arch/openrisc/kernel/prom.c b/arch/openrisc/kernel/prom.c
index 3b94972..fbed459 100644
--- a/arch/openrisc/kernel/prom.c
+++ b/arch/openrisc/kernel/prom.c
@@ -55,20 +55,7 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)

void __init early_init_devtree(void *params)
{
- /* Setup flat device-tree pointer */
- initial_boot_params = params;
-
-
- /* Retrieve various informations from the /chosen node of the
- * device-tree, including the platform type, initrd location and
- * size, TCE reserve, and more ...
- */
- of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);
-
- /* Scan memory nodes and rebuild MEMBLOCKs */
- of_scan_flat_dt(early_init_dt_scan_root, NULL);
- of_scan_flat_dt(early_init_dt_scan_memory, NULL);
-
+ early_init_dt_scan(params);
memblock_allow_resize();
}
--
1.8.1.2
Rob Herring
2013-09-16 23:09:14 UTC
Permalink
From: Rob Herring <rob.herring at calxeda.com>

Create a weak version of early_init_dt_add_memory_arch which uses
memblock or is an empty function when memblock is not enabled. This
will unify all architectures except ones with custom memory bank
structs.

Signed-off-by: Rob Herring <rob.herring at calxeda.com>
Cc: Catalin Marinas <catalin.marinas at arm.com>
Cc: Will Deacon <will.deacon at arm.com>
Cc: James Hogan <james.hogan at imgtec.com>
Cc: Michal Simek <monstr at monstr.eu>
Cc: Jonas Bonn <jonas at southpole.se>
Cc: Thomas Gleixner <tglx at linutronix.de>
Cc: Ingo Molnar <mingo at redhat.com>
Cc: "H. Peter Anvin" <hpa at zytor.com>
Cc: x86 at kernel.org
Cc: Grant Likely <grant.likely at linaro.org>
Cc: linux-arm-kernel at lists.infradead.org
Cc: microblaze-uclinux at itee.uq.edu.au
Cc: linux at lists.openrisc.net
Cc: devicetree at vger.kernel.org
---
arch/arm64/kernel/setup.c | 18 ------------------
arch/metag/kernel/devtree.c | 6 ------
arch/microblaze/kernel/prom.c | 5 -----
arch/openrisc/kernel/prom.c | 6 ------
arch/x86/kernel/devicetree.c | 10 ----------
drivers/of/fdt.c | 11 +++++++++++
6 files changed, 11 insertions(+), 45 deletions(-)

diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index b4461e1..3790004 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -147,24 +147,6 @@ static void __init setup_machine_fdt(phys_addr_t dt_phys)
pr_info("Machine: %s\n", machine_name);
}

-void __init early_init_dt_add_memory_arch(u64 base, u64 size)
-{
- base &= PAGE_MASK;
- size &= PAGE_MASK;
- if (base + size < PHYS_OFFSET) {
- pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
- base, base + size);
- return;
- }
- if (base < PHYS_OFFSET) {
- pr_warning("Ignoring memory range 0x%llx - 0x%llx\n",
- base, PHYS_OFFSET);
- size -= PHYS_OFFSET - base;
- base = PHYS_OFFSET;
- }
- memblock_add(base, size);
-}
-
/*
* Limit the memory size that was specified via FDT.
*/
diff --git a/arch/metag/kernel/devtree.c b/arch/metag/kernel/devtree.c
index 049af56..2c6ee6d 100644
--- a/arch/metag/kernel/devtree.c
+++ b/arch/metag/kernel/devtree.c
@@ -23,12 +23,6 @@
#include <asm/page.h>
#include <asm/mach/arch.h>

-void __init early_init_dt_add_memory_arch(u64 base, u64 size)
-{
- pr_err("%s(%llx, %llx)\n",
- __func__, base, size);
-}
-
void * __init early_init_dt_alloc_memory_arch(u64 size, u64 align)
{
return alloc_bootmem_align(size, align);
diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
index e13686e..951e4d6 100644
--- a/arch/microblaze/kernel/prom.c
+++ b/arch/microblaze/kernel/prom.c
@@ -41,11 +41,6 @@
#include <asm/sections.h>
#include <asm/pci-bridge.h>

-void __init early_init_dt_add_memory_arch(u64 base, u64 size)
-{
- memblock_add(base, size);
-}
-
#ifdef CONFIG_EARLY_PRINTK
static char *stdout;

diff --git a/arch/openrisc/kernel/prom.c b/arch/openrisc/kernel/prom.c
index fbed459..6dbcaa8 100644
--- a/arch/openrisc/kernel/prom.c
+++ b/arch/openrisc/kernel/prom.c
@@ -47,12 +47,6 @@
#include <asm/sections.h>
#include <asm/setup.h>

-void __init early_init_dt_add_memory_arch(u64 base, u64 size)
-{
- size &= PAGE_MASK;
- memblock_add(base, size);
-}
-
void __init early_init_devtree(void *params)
{
early_init_dt_scan(params);
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 0db805c..2f5cb37 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -36,16 +36,6 @@ unsigned long pci_address_to_pio(phys_addr_t address)
}
EXPORT_SYMBOL_GPL(pci_address_to_pio);

-void __init early_init_dt_scan_chosen_arch(unsigned long node)
-{
- BUG();
-}
-
-void __init early_init_dt_add_memory_arch(u64 base, u64 size)
-{
- BUG();
-}
-
void * __init early_init_dt_alloc_memory_arch(u64 size, u64 align)
{
return __alloc_bootmem(size, align, __pa(MAX_DMA_ADDRESS));
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 0714dd4..a9dce7a 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -688,6 +688,17 @@ u64 __init dt_mem_next_cell(int s, __be32 **cellp)
return of_read_number(p, s);
}

+void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
+{
+#ifdef CONFIG_HAVE_MEMBLOCK
+ base &= PAGE_MASK;
+ size &= PAGE_MASK;
+ memblock_add(base, size);
+#else
+ pr_err("%s: ignoring memory (%llx, %llx)\n", __func__, base, size);
+#endif
+}
+
/**
* early_init_dt_scan_memory - Look for an parse memory nodes
*/
--
1.8.1.2
Catalin Marinas
2013-09-17 08:46:52 UTC
Permalink
Post by Rob Herring
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -147,24 +147,6 @@ static void __init setup_machine_fdt(phys_addr_t dt_phys)
pr_info("Machine: %s\n", machine_name);
}
-void __init early_init_dt_add_memory_arch(u64 base, u64 size)
-{
- base &= PAGE_MASK;
- size &= PAGE_MASK;
- if (base + size < PHYS_OFFSET) {
- pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
- base, base + size);
- return;
- }
- if (base < PHYS_OFFSET) {
- pr_warning("Ignoring memory range 0x%llx - 0x%llx\n",
- base, PHYS_OFFSET);
- size -= PHYS_OFFSET - base;
- base = PHYS_OFFSET;
- }
- memblock_add(base, size);
-}
-
/*
* Limit the memory size that was specified via FDT.
*/
...
Post by Rob Herring
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -688,6 +688,17 @@ u64 __init dt_mem_next_cell(int s, __be32 **cellp)
return of_read_number(p, s);
}
+void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
+{
+#ifdef CONFIG_HAVE_MEMBLOCK
+ base &= PAGE_MASK;
+ size &= PAGE_MASK;
+ memblock_add(base, size);
+#else
+ pr_err("%s: ignoring memory (%llx, %llx)\n", __func__, base, size);
+#endif
+}
Are the arm64 changes equivalent here? There are some safety checks to
cope with the kernel being loaded at a higher offset than the
recommended one (PHYS_OFFSET calculated automatically).

Catalin
Rob Herring
2013-09-17 13:01:36 UTC
Permalink
On Tue, Sep 17, 2013 at 3:46 AM, Catalin Marinas
Post by Catalin Marinas
Post by Rob Herring
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -147,24 +147,6 @@ static void __init setup_machine_fdt(phys_addr_t dt_phys)
pr_info("Machine: %s\n", machine_name);
}
-void __init early_init_dt_add_memory_arch(u64 base, u64 size)
-{
- base &= PAGE_MASK;
- size &= PAGE_MASK;
- if (base + size < PHYS_OFFSET) {
- pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
- base, base + size);
- return;
- }
- if (base < PHYS_OFFSET) {
- pr_warning("Ignoring memory range 0x%llx - 0x%llx\n",
- base, PHYS_OFFSET);
- size -= PHYS_OFFSET - base;
- base = PHYS_OFFSET;
- }
- memblock_add(base, size);
-}
-
/*
* Limit the memory size that was specified via FDT.
*/
...
Post by Rob Herring
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -688,6 +688,17 @@ u64 __init dt_mem_next_cell(int s, __be32 **cellp)
return of_read_number(p, s);
}
+void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
+{
+#ifdef CONFIG_HAVE_MEMBLOCK
+ base &= PAGE_MASK;
+ size &= PAGE_MASK;
+ memblock_add(base, size);
+#else
+ pr_err("%s: ignoring memory (%llx, %llx)\n", __func__, base, size);
+#endif
+}
Are the arm64 changes equivalent here? There are some safety checks to
cope with the kernel being loaded at a higher offset than the
recommended one (PHYS_OFFSET calculated automatically).
I tried to keep that, but PHYS_OFFSET is not universally defined. My
reasoning is this range checking is hardly specific to an
architecture. Perhaps if memory always starts at 0 you don't need it.
If arm64 really needs these checks, then all architectures do.

Perhaps "__virt_to_phys(PAGE_OFFSET)" instead of PHYS_OFFSET would work for all?

Rob
Catalin Marinas
2013-09-17 15:28:30 UTC
Permalink
Post by Rob Herring
On Tue, Sep 17, 2013 at 3:46 AM, Catalin Marinas
Post by Catalin Marinas
Post by Rob Herring
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -147,24 +147,6 @@ static void __init setup_machine_fdt(phys_addr_t dt_phys)
pr_info("Machine: %s\n", machine_name);
}
-void __init early_init_dt_add_memory_arch(u64 base, u64 size)
-{
- base &= PAGE_MASK;
- size &= PAGE_MASK;
- if (base + size < PHYS_OFFSET) {
- pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
- base, base + size);
- return;
- }
- if (base < PHYS_OFFSET) {
- pr_warning("Ignoring memory range 0x%llx - 0x%llx\n",
- base, PHYS_OFFSET);
- size -= PHYS_OFFSET - base;
- base = PHYS_OFFSET;
- }
- memblock_add(base, size);
-}
-
/*
* Limit the memory size that was specified via FDT.
*/
...
Post by Rob Herring
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -688,6 +688,17 @@ u64 __init dt_mem_next_cell(int s, __be32 **cellp)
return of_read_number(p, s);
}
+void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
+{
+#ifdef CONFIG_HAVE_MEMBLOCK
+ base &= PAGE_MASK;
+ size &= PAGE_MASK;
+ memblock_add(base, size);
+#else
+ pr_err("%s: ignoring memory (%llx, %llx)\n", __func__, base, size);
+#endif
+}
Are the arm64 changes equivalent here? There are some safety checks to
cope with the kernel being loaded at a higher offset than the
recommended one (PHYS_OFFSET calculated automatically).
I tried to keep that, but PHYS_OFFSET is not universally defined. My
reasoning is this range checking is hardly specific to an
architecture. Perhaps if memory always starts at 0 you don't need it.
If arm64 really needs these checks, then all architectures do.
Perhaps "__virt_to_phys(PAGE_OFFSET)" instead of PHYS_OFFSET would work for all?
I think virt_to_phys() or __pa() should work, the __virt_to_phys() is
only defined by a few architectures.
--
Catalin
Rob Herring
2013-09-18 15:09:40 UTC
Permalink
Post by Rob Herring
From: Rob Herring <rob.herring at calxeda.com>
Create a weak version of early_init_dt_add_memory_arch which uses
memblock or is an empty function when memblock is not enabled. This
will unify all architectures except ones with custom memory bank
structs.
Acked-by: Grant Likely <grant.likely at linaro.org>
Post by Rob Herring
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 0714dd4..a9dce7a 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -688,6 +688,17 @@ u64 __init dt_mem_next_cell(int s, __be32 **cellp)
return of_read_number(p, s);
}
+void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
+{
+#ifdef CONFIG_HAVE_MEMBLOCK
+ base &= PAGE_MASK;
+ size &= PAGE_MASK;
+ memblock_add(base, size);
+#else
+ pr_err("%s: ignoring memory (%llx, %llx)\n", __func__, base, size);
+#endif
+}
+
#ifdef CONFIG_HAVE_MEMBLOCK
void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
{
base &= PAGE_MASK;
size &= PAGE_MASK;
memblock_add(base, size);
}
#endif
If the platform doesn't provide an early_init_dt_add_memory_arch()
function and it doesn't have a memblock implementation, then the build
should outright fail. I don't see a scenario where we would want to
successfully build the kernel without a working add memory function.
metag and x86 both have empty functions. I guess they get memory from
a different boot interface.

Rob
Also, can you group this function with the common __weak
early_init_dt_alloc_memory_arch() implementation? It would be good to
group all the memblock specific functions together.
g.
Rob Herring
2013-09-16 23:09:18 UTC
Permalink
From: Rob Herring <rob.herring at calxeda.com>

It appears openrisc prom.c was just copied from another arch with a
bunch of unnecessary includes. Remove all the unecessary ones.

Signed-off-by: Rob Herring <rob.herring at calxeda.com>
Cc: Jonas Bonn <jonas at southpole.se>
Cc: linux at lists.openrisc.net
---
arch/openrisc/kernel/prom.c | 22 ----------------------
1 file changed, 22 deletions(-)

diff --git a/arch/openrisc/kernel/prom.c b/arch/openrisc/kernel/prom.c
index 2aae474..6a44340 100644
--- a/arch/openrisc/kernel/prom.c
+++ b/arch/openrisc/kernel/prom.c
@@ -18,34 +18,12 @@
*
*/

-#include <stdarg.h>
-#include <linux/kernel.h>
-#include <linux/string.h>
#include <linux/init.h>
-#include <linux/threads.h>
-#include <linux/spinlock.h>
#include <linux/types.h>
-#include <linux/pci.h>
-#include <linux/stringify.h>
-#include <linux/delay.h>
-#include <linux/initrd.h>
-#include <linux/bitops.h>
-#include <linux/module.h>
-#include <linux/kexec.h>
-#include <linux/debugfs.h>
-#include <linux/irq.h>
#include <linux/memblock.h>
#include <linux/of_fdt.h>

-#include <asm/prom.h>
#include <asm/page.h>
-#include <asm/processor.h>
-#include <asm/irq.h>
-#include <linux/io.h>
-#include <asm/mmu.h>
-#include <asm/pgtable.h>
-#include <asm/sections.h>
-#include <asm/setup.h>

void __init early_init_devtree(void *params)
{
--
1.8.1.2
Loading...