[PATCH 14/17] dt: Consolidate __dtb_start declarations in <linux/of_fdt.h>

[PATCH 14/17] dt: Consolidate __dtb_start declarations in <linux/of_fdt.h>

[Geert Uytterhoeven]

The different architectures used their own (and different) declarations:

    extern struct boot_param_header __dtb_start;
    extern u32 __dtb_start[];
    extern char __dtb_start[];

Consolidate them using the first variant in <linux/of_fdt.h>.
This requires adding a few "address of" operators on architectures where
__dtb_start was an array before.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Jonas Bonn <jonas@southpole.se>
CC: Chris Zankel <chris@zankel.net>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: devicetree@vger.kernel.org
---
 arch/arc/include/asm/sections.h             |    1 -
 arch/arc/kernel/setup.c                     |    2 +-
 arch/metag/kernel/setup.c                   |    6 +-----
 arch/mips/include/asm/mips-boards/generic.h |    4 ----
 arch/mips/lantiq/prom.h                     |    2 --
 arch/mips/netlogic/xlp/dt.c                 |    4 ++--
 arch/mips/ralink/of.c                       |    2 --
 arch/openrisc/kernel/setup.c                |    2 +-
 arch/openrisc/kernel/vmlinux.h              |    2 --
 arch/xtensa/kernel/setup.c                  |    3 +--
 include/linux/of_fdt.h                      |    3 +++
 11 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/arch/arc/include/asm/sections.h b/arch/arc/include/asm/sections.h
index 764f1e3ba752..09db952e14bd 100644
--- a/arch/arc/include/asm/sections.h
+++ b/arch/arc/include/asm/sections.h
@@ -12,6 +12,5 @@
 #include <asm-generic/sections.h>
 
 extern char __arc_dccm_base[];
-extern char __dtb_start[];
 
 #endif
diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
index 643eae4436e0..ef705d0f50f5 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -314,7 +314,7 @@ void setup_processor(void)
 void __init setup_arch(char **cmdline_p)
 {
 	/* This also populates @boot_command_line from /bootargs */
-	machine_desc = setup_machine_fdt(__dtb_start);
+	machine_desc = setup_machine_fdt(&__dtb_start);
 	if (!machine_desc)
 		panic("Embedded DT invalid\n");
 
diff --git a/arch/metag/kernel/setup.c b/arch/metag/kernel/setup.c
index 129c7cdda1ce..919c782f5479 100644
--- a/arch/metag/kernel/setup.c
+++ b/arch/metag/kernel/setup.c
@@ -105,10 +105,6 @@
 
 extern char _heap_start[];
 
-#ifdef CONFIG_METAG_BUILTIN_DTB
-extern u32 __dtb_start[];
-#endif
-
 #ifdef CONFIG_DA_CONSOLE
 /* Our early channel based console driver */
 extern struct console dash_console;
@@ -251,7 +247,7 @@ void __init setup_arch(char **cmdline_p)
 	if (!machine_desc) {
 #ifdef CONFIG_METAG_BUILTIN_DTB
 		/* try the embedded device tree */
-		machine_desc = setup_machine_fdt(__dtb_start);
+		machine_desc = setup_machine_fdt(&__dtb_start);
 		if (!machine_desc)
 			panic("Invalid embedded device tree.");
 #else
diff --git a/arch/mips/include/asm/mips-boards/generic.h b/arch/mips/include/asm/mips-boards/generic.h
index 48616816bcbc..c904c24550f6 100644
--- a/arch/mips/include/asm/mips-boards/generic.h
+++ b/arch/mips/include/asm/mips-boards/generic.h
@@ -67,10 +67,6 @@
 
 extern int mips_revision_sconid;
 
-#ifdef CONFIG_OF
-extern struct boot_param_header __dtb_start;
-#endif
-
 #ifdef CONFIG_PCI
 extern void mips_pcibios_init(void);
 #else
diff --git a/arch/mips/lantiq/prom.h b/arch/mips/lantiq/prom.h
index 8e07b5f28ef1..bfd2d58c1d69 100644
--- a/arch/mips/lantiq/prom.h
+++ b/arch/mips/lantiq/prom.h
@@ -26,6 +26,4 @@ struct ltq_soc_info {
 extern void ltq_soc_detect(struct ltq_soc_info *i);
 extern void ltq_soc_init(void);
 
-extern struct boot_param_header __dtb_start;
-
 #endif
diff --git a/arch/mips/netlogic/xlp/dt.c b/arch/mips/netlogic/xlp/dt.c
index 8316d5454b17..87250f378386 100644
--- a/arch/mips/netlogic/xlp/dt.c
+++ b/arch/mips/netlogic/xlp/dt.c
@@ -42,7 +42,7 @@
 #include <asm/prom.h>
 
 extern u32 __dtb_xlp_evp_begin[], __dtb_xlp_svp_begin[],
-	__dtb_xlp_fvp_begin[], __dtb_start[];
+	__dtb_xlp_fvp_begin[];
 static void *xlp_fdt_blob;
 
 void __init *xlp_dt_init(void *fdtp)
@@ -66,7 +66,7 @@ void __init *xlp_dt_init(void *fdtp)
 #endif
 		default:
 			/* Pick a built-in if any, and hope for the best */
-			fdtp = __dtb_start;
+			fdtp = &__dtb_start;
 			break;
 		}
 	}
diff --git a/arch/mips/ralink/of.c b/arch/mips/ralink/of.c
index eccc5526155e..615603bd8063 100644
--- a/arch/mips/ralink/of.c
+++ b/arch/mips/ralink/of.c
@@ -28,8 +28,6 @@
 __iomem void *rt_sysc_membase;
 __iomem void *rt_memc_membase;
 
-extern struct boot_param_header __dtb_start;
-
 __iomem void *plat_of_remap_node(const char *node)
 {
 	struct resource res;
diff --git a/arch/openrisc/kernel/setup.c b/arch/openrisc/kernel/setup.c
index 4fc7ccc0a2cf..c9b880d64bb8 100644
--- a/arch/openrisc/kernel/setup.c
+++ b/arch/openrisc/kernel/setup.c
@@ -213,7 +213,7 @@ void __init or32_early_setup(void *fdt)
 	if (fdt)
 		pr_info("FDT at %p\n", fdt);
 	else {
-		fdt = __dtb_start;
+		fdt = &__dtb_start;
 		pr_info("Compiled-in FDT at %p\n", fdt);
 	}
 	early_init_devtree(fdt);
diff --git a/arch/openrisc/kernel/vmlinux.h b/arch/openrisc/kernel/vmlinux.h
index 70b9ce41835c..bbcdf21b0b35 100644
--- a/arch/openrisc/kernel/vmlinux.h
+++ b/arch/openrisc/kernel/vmlinux.h
@@ -5,6 +5,4 @@
 extern char __initrd_start, __initrd_end;
 #endif
 
-extern u32 __dtb_start[];
-
 #endif
diff --git a/arch/xtensa/kernel/setup.c b/arch/xtensa/kernel/setup.c
index 6e2b6638122d..d96cf8f3fe70 100644
--- a/arch/xtensa/kernel/setup.c
+++ b/arch/xtensa/kernel/setup.c
@@ -68,8 +68,7 @@ extern int initrd_below_start_ok;
 #endif
 
 #ifdef CONFIG_OF
-extern u32 __dtb_start[];
-void *dtb_start = __dtb_start;
+void *dtb_start = &__dtb_start;
 #endif
 
 unsigned char aux_device_present;
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 0beaee9dac1f..aabc49f3e403 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -57,6 +57,9 @@ struct boot_param_header {
 	__be32	dt_struct_size;		/* size of the DT structure block */
 };
 
+/* For kernels with a built-in device tree */
+extern struct boot_param_header __dtb_start;
+
 #if defined(CONFIG_OF_FLATTREE)
 
 struct device_node;
-- 
1.7.9.5

Re: [PATCH 14/17] dt: Consolidate __dtb_start declarations in <linux/of_fdt.h>

[Vineet Gupta]

On 11/13/2013 01:12 AM, Geert Uytterhoeven wrote:
> The different architectures used their own (and different) declarations:
> 
>     extern struct boot_param_header __dtb_start;
>     extern u32 __dtb_start[];
>     extern char __dtb_start[];
> 
> Consolidate them using the first variant in <linux/of_fdt.h>.
> This requires adding a few "address of" operators on architectures where
> __dtb_start was an array before.
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Vineet Gupta <vgupta@synopsys.com>
> Cc: James Hogan <james.hogan@imgtec.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Jonas Bonn <jonas@southpole.se>
> CC: Chris Zankel <chris@zankel.net>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: devicetree@vger.kernel.org
> ---
>  arch/arc/include/asm/sections.h             |    1 -
>  arch/arc/kernel/setup.c                     |    2 +-
>  arch/metag/kernel/setup.c                   |    6 +-----
>  arch/mips/include/asm/mips-boards/generic.h |    4 ----
>  arch/mips/lantiq/prom.h                     |    2 --
>  arch/mips/netlogic/xlp/dt.c                 |    4 ++--
>  arch/mips/ralink/of.c                       |    2 --
>  arch/openrisc/kernel/setup.c                |    2 +-
>  arch/openrisc/kernel/vmlinux.h              |    2 --
>  arch/xtensa/kernel/setup.c                  |    3 +--
>  include/linux/of_fdt.h                      |    3 +++
>  11 files changed, 9 insertions(+), 22 deletions(-)


Acked-by: Vineet Gupta <vgupta@synopsys.com>

Thx,
-Vineet

Re: [PATCH 14/17] dt: Consolidate __dtb_start declarations in <linux/of_fdt.h>

[Rob Herring]

On Tue, Nov 12, 2013 at 1:42 PM, Geert Uytterhoeven
<geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> The different architectures used their own (and different) declarations:
>
>     extern struct boot_param_header __dtb_start;
>     extern u32 __dtb_start[];
>     extern char __dtb_start[];
>
> Consolidate them using the first variant in <linux/of_fdt.h>.
> This requires adding a few "address of" operators on architectures where
> __dtb_start was an array before.
>
> Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> Cc: Vineet Gupta <vgupta-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> Cc: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> Cc: Ralf Baechle <ralf-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
> Cc: Jonas Bonn <jonas-A9uVI2HLR7kOP4wsBPIw7w@public.gmane.org>
> CC: Chris Zankel <chris-YvXeqwSYzG2sTnJN9+BGXg@public.gmane.org>
> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  arch/arc/include/asm/sections.h             |    1 -
>  arch/arc/kernel/setup.c                     |    2 +-
>  arch/metag/kernel/setup.c                   |    6 +-----
>  arch/mips/include/asm/mips-boards/generic.h |    4 ----
>  arch/mips/lantiq/prom.h                     |    2 --
>  arch/mips/netlogic/xlp/dt.c                 |    4 ++--
>  arch/mips/ralink/of.c                       |    2 --
>  arch/openrisc/kernel/setup.c                |    2 +-
>  arch/openrisc/kernel/vmlinux.h              |    2 --
>  arch/xtensa/kernel/setup.c                  |    3 +--
>  include/linux/of_fdt.h                      |    3 +++
>  11 files changed, 9 insertions(+), 22 deletions(-)

This is great, but I would like to see this taken one step further and
move the selection logic into the core code. We can try to use the
built-in dtb if early_init_dt_scan is passed a NULL or invalid dtb. This
should work for most arches I think and still allows an arch to do a
different selection algorithm if desired.

Are the DT related patches dependent on the rest of the series?

Something like this is what I have in mind:

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 5c47910..723f854d 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -869,14 +869,16 @@ void * __init __weak
early_init_dt_alloc_memory_arch(u64 size, u64 align)

 bool __init early_init_dt_scan(void *params)
 {
-	if (!params)
-		return false;
-
 	/* Setup flat device-tree pointer */
 	initial_boot_params = params;

+	if (!initial_boot_params ||
+	    (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER))
+		initial_boot_params = &__dtb_start;
+
 	/* check device tree validity */
-	if (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER) {
+	if (!initial_boot_params ||
+	    (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER)) {
 		initial_boot_params = NULL;
 		return false;
 	}
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/17] dt: Consolidate __dtb_start declarations in <linux/of_fdt.h>

[Geert Uytterhoeven]

On Wed, Nov 13, 2013 at 4:51 PM, Rob Herring <robherring2@gmail.com> wrote:
> On Tue, Nov 12, 2013 at 1:42 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> The different architectures used their own (and different) declarations:
>>
>>     extern struct boot_param_header __dtb_start;
>>     extern u32 __dtb_start[];
>>     extern char __dtb_start[];
>>
>> Consolidate them using the first variant in <linux/of_fdt.h>.
>> This requires adding a few "address of" operators on architectures where
>> __dtb_start was an array before.
>>
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Vineet Gupta <vgupta@synopsys.com>
>> Cc: James Hogan <james.hogan@imgtec.com>
>> Cc: Ralf Baechle <ralf@linux-mips.org>
>> Cc: Jonas Bonn <jonas@southpole.se>
>> CC: Chris Zankel <chris@zankel.net>
>> Cc: Rob Herring <rob.herring@calxeda.com>
>> Cc: devicetree@vger.kernel.org
>> ---
>>  arch/arc/include/asm/sections.h             |    1 -
>>  arch/arc/kernel/setup.c                     |    2 +-
>>  arch/metag/kernel/setup.c                   |    6 +-----
>>  arch/mips/include/asm/mips-boards/generic.h |    4 ----
>>  arch/mips/lantiq/prom.h                     |    2 --
>>  arch/mips/netlogic/xlp/dt.c                 |    4 ++--
>>  arch/mips/ralink/of.c                       |    2 --
>>  arch/openrisc/kernel/setup.c                |    2 +-
>>  arch/openrisc/kernel/vmlinux.h              |    2 --
>>  arch/xtensa/kernel/setup.c                  |    3 +--
>>  include/linux/of_fdt.h                      |    3 +++
>>  11 files changed, 9 insertions(+), 22 deletions(-)
>
> This is great, but I would like to see this taken one step further and
> move the selection logic into the core code. We can try to use the
> built-in dtb if early_init_dt_scan is passed a NULL or invalid dtb. This
> should work for most arches I think and still allows an arch to do a
> different selection algorithm if desired.

Yes, that makes sense.

> Are the DT related patches dependent on the rest of the series?

Not really. There may be small rejects if you apply it out-of-order,
but nothing serious.

> Something like this is what I have in mind:
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 5c47910..723f854d 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -869,14 +869,16 @@ void * __init __weak
> early_init_dt_alloc_memory_arch(u64 size, u64 align)
>
>  bool __init early_init_dt_scan(void *params)
>  {
> -       if (!params)
> -               return false;
> -
>         /* Setup flat device-tree pointer */
>         initial_boot_params = params;
>
> +       if (!initial_boot_params ||
> +           (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER))
> +               initial_boot_params = &__dtb_start;
> +
>         /* check device tree validity */
> -       if (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER) {
> +       if (!initial_boot_params ||

initial_boot_params cannot be NULL here, so no need to check.

> +           (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER)) {

However, if __dtb_end == __dtb_start, you may be reading random
data here from the next section. The OF_DT_HEADER check should cover
this, but better safe than sorry?

>                 initial_boot_params = NULL;
>                 return false;
>         }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 14/17] dt: Consolidate __dtb_start declarations in <linux/of_fdt.h>

[Rob Herring]

On 11/13/2013 11:20 AM, Geert Uytterhoeven wrote:
> On Wed, Nov 13, 2013 at 4:51 PM, Rob Herring <robherring2@gmail.com> wrote:
>> On Tue, Nov 12, 2013 at 1:42 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> The different architectures used their own (and different) declarations:
>>>
>>>     extern struct boot_param_header __dtb_start;
>>>     extern u32 __dtb_start[];
>>>     extern char __dtb_start[];
>>>
>>> Consolidate them using the first variant in <linux/of_fdt.h>.
>>> This requires adding a few "address of" operators on architectures where
>>> __dtb_start was an array before.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>> Cc: Vineet Gupta <vgupta@synopsys.com>
>>> Cc: James Hogan <james.hogan@imgtec.com>
>>> Cc: Ralf Baechle <ralf@linux-mips.org>
>>> Cc: Jonas Bonn <jonas@southpole.se>
>>> CC: Chris Zankel <chris@zankel.net>
>>> Cc: Rob Herring <rob.herring@calxeda.com>
>>> Cc: devicetree@vger.kernel.org
>>> ---
>>>  arch/arc/include/asm/sections.h             |    1 -
>>>  arch/arc/kernel/setup.c                     |    2 +-
>>>  arch/metag/kernel/setup.c                   |    6 +-----
>>>  arch/mips/include/asm/mips-boards/generic.h |    4 ----
>>>  arch/mips/lantiq/prom.h                     |    2 --
>>>  arch/mips/netlogic/xlp/dt.c                 |    4 ++--
>>>  arch/mips/ralink/of.c                       |    2 --
>>>  arch/openrisc/kernel/setup.c                |    2 +-
>>>  arch/openrisc/kernel/vmlinux.h              |    2 --
>>>  arch/xtensa/kernel/setup.c                  |    3 +--
>>>  include/linux/of_fdt.h                      |    3 +++
>>>  11 files changed, 9 insertions(+), 22 deletions(-)
>>
>> This is great, but I would like to see this taken one step further and
>> move the selection logic into the core code. We can try to use the
>> built-in dtb if early_init_dt_scan is passed a NULL or invalid dtb. This
>> should work for most arches I think and still allows an arch to do a
>> different selection algorithm if desired.
> 
> Yes, that makes sense.
> 
>> Are the DT related patches dependent on the rest of the series?
> 
> Not really. There may be small rejects if you apply it out-of-order,
> but nothing serious.
> 
>> Something like this is what I have in mind:
>>
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index 5c47910..723f854d 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -869,14 +869,16 @@ void * __init __weak
>> early_init_dt_alloc_memory_arch(u64 size, u64 align)
>>
>>  bool __init early_init_dt_scan(void *params)
>>  {
>> -       if (!params)
>> -               return false;
>> -
>>         /* Setup flat device-tree pointer */
>>         initial_boot_params = params;
>>
>> +       if (!initial_boot_params ||
>> +           (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER))
>> +               initial_boot_params = &__dtb_start;
>> +
>>         /* check device tree validity */
>> -       if (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER) {
>> +       if (!initial_boot_params ||
> 
> initial_boot_params cannot be NULL here, so no need to check.

What about the case of no built-in dtb like on arm?

> 
>> +           (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER)) {
> 
> However, if __dtb_end == __dtb_start, you may be reading random
> data here from the next section. The OF_DT_HEADER check should cover
> this, but better safe than sorry?

Then we should also check that (__dtb_end != __dtb_start).

Rob

> 
>>                 initial_boot_params = NULL;
>>                 return false;
>>         }
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

Re: [PATCH 14/17] dt: Consolidate __dtb_start declarations in <linux/of_fdt.h>

[Geert Uytterhoeven]

On Wed, Nov 13, 2013 at 6:34 PM, Rob Herring <robherring2@gmail.com> wrote:
>>> +       if (!initial_boot_params ||
>>> +           (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER))
>>> +               initial_boot_params = &__dtb_start;
>>> +
>>>         /* check device tree validity */
>>> -       if (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER) {
>>> +       if (!initial_boot_params ||
>>
>> initial_boot_params cannot be NULL here, so no need to check.
>
> What about the case of no built-in dtb like on arm?

Even then the symbol exists, as all architectures put INIT_DATA or
INIT_DATA_SECTION in their vmlinux.lds.S.
But the section will be empty, cfr. the case __dtb_end == __dtb_start below.

>>
>>> +           (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER)) {
>>
>> However, if __dtb_end == __dtb_start, you may be reading random
>> data here from the next section. The OF_DT_HEADER check should cover
>> this, but better safe than sorry?
>
> Then we should also check that (__dtb_end != __dtb_start).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 1/9] dt: Handle passed/built-in DT selection in early_init_dt_scan()

[Geert Uytterhoeven]

Let early_init_dt_scan() fall-back to the built-in DT if no DT was passed,
or if it's invalid, so architectures don't have to duplicate this logic.

Suggested-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
---
 drivers/of/fdt.c       |   24 +++++++++++++++++-------
 include/linux/of_fdt.h |    1 +
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 2fa024b97c43..a797cd43bc8b 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -868,18 +868,28 @@ void * __init __weak early_init_dt_alloc_memory_arch(u64 size, u64 align)
 
 bool __init early_init_dt_scan(void *params)
 {
-	if (!params)
-		return false;
-
 	/* Setup flat device-tree pointer */
 	initial_boot_params = params;
 
-	/* check device tree validity */
-	if (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER) {
-		initial_boot_params = NULL;
-		return false;
+	/* check passed device tree validity */
+	if (initial_boot_params &&
+	    be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
+		pr_info("FDT at %p\n", initial_boot_params);
+		goto found;
+	}
+
+	/* check built-in device tree validity */
+	initial_boot_params = &__dtb_start;
+	if (__dtb_end != (void *)&__dtb_start &&
+	    be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
+		pr_info("Compiled-in FDT at %p\n", initial_boot_params);
+		goto found;
 	}
 
+	initial_boot_params = NULL;
+	return false;
+
+found:
 	/* Retrieve various information from the /chosen node */
 	of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);
 
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index aabc49f3e403..6c4a60454854 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -59,6 +59,7 @@ struct boot_param_header {
 
 /* For kernels with a built-in device tree */
 extern struct boot_param_header __dtb_start;
+extern char __dtb_end[];
 
 #if defined(CONFIG_OF_FLATTREE)
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/9] arc: Use NULL as the default DTB

[Geert Uytterhoeven]

early_init_dt_scan() now takes care of falling back to the built-in DTB.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Vineet Gupta <vgupta@synopsys.com>
---
 arch/arc/kernel/setup.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
index ef705d0f50f5..481e11240e12 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -314,7 +314,7 @@ void setup_processor(void)
 void __init setup_arch(char **cmdline_p)
 {
 	/* This also populates @boot_command_line from /bootargs */
-	machine_desc = setup_machine_fdt(&__dtb_start);
+	machine_desc = setup_machine_fdt(NULL);
 	if (!machine_desc)
 		panic("Embedded DT invalid\n");
 
-- 
1.7.9.5

Re: [PATCH 2/9] arc: Use NULL as the default DTB

[Vineet Gupta]

On 11/19/2013 04:42 PM, Geert Uytterhoeven wrote:
> early_init_dt_scan() now takes care of falling back to the built-in DTB.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Vineet Gupta <vgupta@synopsys.com>

Acked-by: Vineet Gupta <vgupta@synopsys.com>

Thx,
-Vineet

[PATCH 3/9] c6x: Remove duplicate DT selection logic

[Geert Uytterhoeven]

early_init_dt_scan() now takes care of falling back to the built-in DTB.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Mark Salter <msalter@redhat.com>
Cc: Aurelien Jacquiot <a-jacquiot@ti.com>
Cc: linux-c6x-dev@linux-c6x.org
---
 arch/c6x/kernel/setup.c |   10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/arch/c6x/kernel/setup.c b/arch/c6x/kernel/setup.c
index 414353027ba9..ad7c5d73bb3c 100644
--- a/arch/c6x/kernel/setup.c
+++ b/arch/c6x/kernel/setup.c
@@ -266,7 +266,6 @@ int __init c6x_add_memory(phys_addr_t start, unsigned long size)
 notrace void __init machine_init(unsigned long dt_ptr)
 {
 	struct boot_param_header *dtb = __va(dt_ptr);
-	struct boot_param_header *fdt = &__dtb_start;
 
 	/* interrupts must be masked */
 	set_creg(IER, 2);
@@ -279,15 +278,8 @@ notrace void __init machine_init(unsigned long dt_ptr)
 
 	lockdep_init();
 
-	/*
-	 * dtb is passed in from bootloader.
-	 * fdt is linked in blob.
-	 */
-	if (dtb && dtb != fdt)
-		fdt = dtb;
-
 	/* Do some early initialization based on the flat device tree */
-	early_init_dt_scan(fdt);
+	early_init_dt_scan(dtb);
 
 	parse_early_param();
 }
-- 
1.7.9.5

[PATCH 4/9] metag: Remove duplicate DT selection logic

[Geert Uytterhoeven]

early_init_dt_scan() now takes care of falling back to the built-in DTB.
Note that we now have to check ourselves whether original_cmd_line is
a DTB or a command line, to keep command line append working.

Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
Cc: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Cc: linux-metag-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
Untested due to lack of cross-compiler.

 arch/metag/kernel/setup.c |   22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/metag/kernel/setup.c b/arch/metag/kernel/setup.c
index 919c782f5479..e3ebfc5f5d77 100644
--- a/arch/metag/kernel/setup.c
+++ b/arch/metag/kernel/setup.c
@@ -221,6 +221,7 @@ static struct notifier_block metag_panic_block = {
 
 void __init setup_arch(char **cmdline_p)
 {
+	void *dt = NULL;
 	unsigned long start_pfn;
 	unsigned long text_start = (unsigned long)(&_stext);
 	unsigned long cpu = smp_processor_id();
@@ -242,26 +243,27 @@ void __init setup_arch(char **cmdline_p)
 #endif
 
 	/* try interpreting the argument as a device tree */
-	machine_desc = setup_machine_fdt(original_cmd_line);
-	/* if it doesn't look like a device tree it must be a command line */
+	if (original_cmd_line &&
+	    be32_to_cpup((__be32 *)original_cmd_line) == OF_DT_HEADER) {
+		dt = original_cmd_line;
+		original_cmd_line = NULL;
+	}
+
+	machine_desc = setup_machine_fdt(dt);
 	if (!machine_desc) {
-#ifdef CONFIG_METAG_BUILTIN_DTB
-		/* try the embedded device tree */
-		machine_desc = setup_machine_fdt(&__dtb_start);
-		if (!machine_desc)
-			panic("Invalid embedded device tree.");
-#else
 		/* use the default machine description */
 		machine_desc = default_machine_desc();
-#endif
+	}
+
 #ifndef CONFIG_CMDLINE_FORCE
+	if (original_cmd_line) {
 		/* append the bootloader cmdline to any builtin fdt cmdline */
 		if (boot_command_line[0] && original_cmd_line[0])
 			strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
 		strlcat(boot_command_line, original_cmd_line,
 			COMMAND_LINE_SIZE);
-#endif
 	}
+#endif
 	setup_meta_clocks(machine_desc->clocks);
 
 	*cmdline_p = boot_command_line;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/9] metag: Remove duplicate DT selection logic

[James Hogan]

On 19/11/13 11:12, Geert Uytterhoeven wrote:
> early_init_dt_scan() now takes care of falling back to the built-in DTB.
> Note that we now have to check ourselves whether original_cmd_line is
> a DTB or a command line, to keep command line append working.
> 
> Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> Cc: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> Cc: linux-metag-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Looks good and seems to work fine.
Acked-by: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

Thanks
James

--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/9] microblaze: Use NULL as the default DTB

[Geert Uytterhoeven]

early_init_dt_scan() now takes care of falling back to the built-in DTB,
and printing what it did.

Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
Cc: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
Cc: microblaze-uclinux-rVRm/Wmeqae7NGdpmJTKYQ@public.gmane.org
---
Note: I did not remove the now-unused fdt parameter from
      machine_early_init(), as this would require quite some changes
      to head.S. BTW, cmdline is also unused.

 arch/microblaze/kernel/setup.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/microblaze/kernel/setup.c b/arch/microblaze/kernel/setup.c
index a4534f584be0..a9a9f8baa94a 100644
--- a/arch/microblaze/kernel/setup.c
+++ b/arch/microblaze/kernel/setup.c
@@ -136,7 +136,7 @@ void __init machine_early_init(const char *cmdline, unsigned int ram,
 	lockdep_init();
 
 /* initialize device tree for usage in early_printk */
-	early_init_devtree(&__dtb_start);
+	early_init_devtree(NULL);
 
 #ifdef CONFIG_EARLY_PRINTK
 	setup_early_printk(NULL);
@@ -149,10 +149,6 @@ void __init machine_early_init(const char *cmdline, unsigned int ram,
 							tlb1, kernel_tlb); */
 
 	pr_info("Ramdisk addr 0x%08x, ", ram);
-	if (fdt)
-		pr_info("FDT at 0x%08x\n", fdt);
-	else
-		pr_info("Compiled-in FDT at %p\n", &__dtb_start);
 
 #ifdef CONFIG_MTD_UCLINUX
 	pr_info("Found romfs @ 0x%08x (0x%08x)\n",
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/9] mips: Remove unused dt_setup_arch()

[Geert Uytterhoeven]

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: John Crispin <blogic@openwrt.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/include/asm/prom.h |    7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/mips/include/asm/prom.h b/arch/mips/include/asm/prom.h
index ccd2b75f152c..7e2ee8301c06 100644
--- a/arch/mips/include/asm/prom.h
+++ b/arch/mips/include/asm/prom.h
@@ -23,13 +23,6 @@ struct boot_param_header;
 
 extern void __dt_setup_arch(struct boot_param_header *bph);
 
-#define dt_setup_arch(sym)						\
-({									\
-	extern struct boot_param_header __dtb_##sym##_begin;		\
-									\
-	__dt_setup_arch(&__dtb_##sym##_begin);				\
-})
-
 #else /* CONFIG_OF */
 static inline void device_tree_init(void) { }
 #endif /* CONFIG_OF */
-- 
1.7.9.5

[PATCH 7/9] mips: Use NULL as the default DTB

[Geert Uytterhoeven]

early_init_dt_scan() now takes care of falling back to the built-in DTB.

On Netlogic XLP, use initial_boot_params in device_tree_init(), like
the other MIPS sub-platforms do, as xlp_fdt_blob will now be NULL when
using the default built-in DTB.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/lantiq/prom.c           |    2 +-
 arch/mips/mti-sead3/sead3-setup.c |    2 +-
 arch/mips/netlogic/xlp/dt.c       |    8 +++-----
 arch/mips/ralink/of.c             |    2 +-
 4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/mips/lantiq/prom.c b/arch/mips/lantiq/prom.c
index 19686c5bc5ed..582f6e4a5f6d 100644
--- a/arch/mips/lantiq/prom.c
+++ b/arch/mips/lantiq/prom.c
@@ -71,7 +71,7 @@ void __init plat_mem_setup(void)
 	 * Load the builtin devicetree. This causes the chosen node to be
 	 * parsed resulting in our memory appearing
 	 */
-	__dt_setup_arch(&__dtb_start);
+	__dt_setup_arch(NULL);
 }
 
 void __init device_tree_init(void)
diff --git a/arch/mips/mti-sead3/sead3-setup.c b/arch/mips/mti-sead3/sead3-setup.c
index 928ba84c8a78..3afec4507ea5 100644
--- a/arch/mips/mti-sead3/sead3-setup.c
+++ b/arch/mips/mti-sead3/sead3-setup.c
@@ -25,7 +25,7 @@ void __init plat_mem_setup(void)
 	 * Load the builtin devicetree. This causes the chosen node to be
 	 * parsed resulting in our memory appearing
 	 */
-	__dt_setup_arch(&__dtb_start);
+	__dt_setup_arch(NULL);
 }
 
 void __init device_tree_init(void)
diff --git a/arch/mips/netlogic/xlp/dt.c b/arch/mips/netlogic/xlp/dt.c
index 87250f378386..e52281b9214d 100644
--- a/arch/mips/netlogic/xlp/dt.c
+++ b/arch/mips/netlogic/xlp/dt.c
@@ -66,7 +66,6 @@ void __init *xlp_dt_init(void *fdtp)
 #endif
 		default:
 			/* Pick a built-in if any, and hope for the best */
-			fdtp = &__dtb_start;
 			break;
 		}
 	}
@@ -83,13 +82,12 @@ void __init xlp_early_init_devtree(void)
 void __init device_tree_init(void)
 {
 	unsigned long base, size;
-	struct boot_param_header *fdtp = xlp_fdt_blob;
 
-	if (!fdtp)
+	if (!initial_boot_params)
 		return;
 
-	base = virt_to_phys(fdtp);
-	size = be32_to_cpu(fdtp->totalsize);
+	base = virt_to_phys(initial_boot_params);
+	size = be32_to_cpu(initial_boot_params->totalsize);
 
 	/* Before we do anything, lets reserve the dt blob */
 	reserve_bootmem(base, size, BOOTMEM_DEFAULT);
diff --git a/arch/mips/ralink/of.c b/arch/mips/ralink/of.c
index 615603bd8063..d72a5bc1efc2 100644
--- a/arch/mips/ralink/of.c
+++ b/arch/mips/ralink/of.c
@@ -84,7 +84,7 @@ void __init plat_mem_setup(void)
 	 * Load the builtin devicetree. This causes the chosen node to be
 	 * parsed resulting in our memory appearing
 	 */
-	__dt_setup_arch(&__dtb_start);
+	__dt_setup_arch(NULL);
 
 	if (soc_info.mem_size)
 		add_memory_region(soc_info.mem_base, soc_info.mem_size * SZ_1M,
-- 
1.7.9.5

[PATCH 8/9] openrisc: Remove duplicate DT selection logic

[Geert Uytterhoeven]

early_init_dt_scan() now takes care of falling back to the built-in DTB,
and printing what it did.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: linux@lists.openrisc.net
---
 arch/openrisc/kernel/setup.c |    6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/openrisc/kernel/setup.c b/arch/openrisc/kernel/setup.c
index 3ca640cd42be..4ec6b07f02b3 100644
--- a/arch/openrisc/kernel/setup.c
+++ b/arch/openrisc/kernel/setup.c
@@ -212,12 +212,6 @@ void __init setup_cpuinfo(void)
 
 void __init or32_early_setup(void *fdt)
 {
-	if (fdt)
-		pr_info("FDT at %p\n", fdt);
-	else {
-		fdt = &__dtb_start;
-		pr_info("Compiled-in FDT at %p\n", fdt);
-	}
 	early_init_devtree(fdt);
 }
 
-- 
1.7.9.5

[PATCH 9/9] xtensa: Use NULL as the default DTB

[Geert Uytterhoeven]

early_init_dt_scan() now takes care of falling back to the built-in DTB.
Also make dtb_start static while we're at it.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Chris Zankel <chris@zankel.net>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: linux-xtensa@linux-xtensa.org
---
 arch/xtensa/kernel/setup.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/xtensa/kernel/setup.c b/arch/xtensa/kernel/setup.c
index d96cf8f3fe70..dd440deb137d 100644
--- a/arch/xtensa/kernel/setup.c
+++ b/arch/xtensa/kernel/setup.c
@@ -68,7 +68,7 @@ extern int initrd_below_start_ok;
 #endif
 
 #ifdef CONFIG_OF
-void *dtb_start = &__dtb_start;
+static void *dtb_start;
 #endif
 
 unsigned char aux_device_present;
-- 
1.7.9.5

Re: [PATCH 1/9] dt: Handle passed/built-in DT selection in early_init_dt_scan()

[James Hogan]

On 19/11/13 11:12, Geert Uytterhoeven wrote:
> Let early_init_dt_scan() fall-back to the built-in DT if no DT was passed,
> or if it's invalid, so architectures don't have to duplicate this logic.
> 
> Suggested-by: Rob Herring <rob.herring@calxeda.com>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

Reviewed-by: James Hogan <james.hogan@imgtec.com>

Thanks
James

Re: [PATCH 1/9] dt: Handle passed/built-in DT selection in early_init_dt_scan()

[Rob Herring]

On Tue, Nov 19, 2013 at 5:12 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Let early_init_dt_scan() fall-back to the built-in DT if no DT was passed,
> or if it's invalid, so architectures don't have to duplicate this logic.
>
> Suggested-by: Rob Herring <rob.herring@calxeda.com>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---

Looks good to me. I can take this series if you want to send me a pull
request. In any case, for the series:

Reviewed-by: Rob Herring <rob.herring@calxeda.com>

Rob

>  drivers/of/fdt.c       |   24 +++++++++++++++++-------
>  include/linux/of_fdt.h |    1 +
>  2 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 2fa024b97c43..a797cd43bc8b 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -868,18 +868,28 @@ void * __init __weak early_init_dt_alloc_memory_arch(u64 size, u64 align)
>
>  bool __init early_init_dt_scan(void *params)
>  {
> -       if (!params)
> -               return false;
> -
>         /* Setup flat device-tree pointer */
>         initial_boot_params = params;
>
> -       /* check device tree validity */
> -       if (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER) {
> -               initial_boot_params = NULL;
> -               return false;
> +       /* check passed device tree validity */
> +       if (initial_boot_params &&
> +           be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
> +               pr_info("FDT at %p\n", initial_boot_params);
> +               goto found;
> +       }
> +
> +       /* check built-in device tree validity */
> +       initial_boot_params = &__dtb_start;
> +       if (__dtb_end != (void *)&__dtb_start &&
> +           be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
> +               pr_info("Compiled-in FDT at %p\n", initial_boot_params);
> +               goto found;
>         }
>
> +       initial_boot_params = NULL;
> +       return false;
> +
> +found:
>         /* Retrieve various information from the /chosen node */
>         of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);
>
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index aabc49f3e403..6c4a60454854 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -59,6 +59,7 @@ struct boot_param_header {
>
>  /* For kernels with a built-in device tree */
>  extern struct boot_param_header __dtb_start;
> +extern char __dtb_end[];
>
>  #if defined(CONFIG_OF_FLATTREE)
>
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/9] dt: Handle passed/built-in DT selection in early_init_dt_scan()

[Grant Likely]

On Tue, 19 Nov 2013 12:12:26 +0100, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Let early_init_dt_scan() fall-back to the built-in DT if no DT was passed,
> or if it's invalid, so architectures don't have to duplicate this logic.
> 
> Suggested-by: Rob Herring <rob.herring@calxeda.com>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>  drivers/of/fdt.c       |   24 +++++++++++++++++-------
>  include/linux/of_fdt.h |    1 +
>  2 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 2fa024b97c43..a797cd43bc8b 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -868,18 +868,28 @@ void * __init __weak early_init_dt_alloc_memory_arch(u64 size, u64 align)
>  
>  bool __init early_init_dt_scan(void *params)
>  {
> -	if (!params)
> -		return false;
> -
>  	/* Setup flat device-tree pointer */
>  	initial_boot_params = params;
>  
> -	/* check device tree validity */
> -	if (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER) {
> -		initial_boot_params = NULL;
> -		return false;
> +	/* check passed device tree validity */
> +	if (initial_boot_params &&
> +	    be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
> +		pr_info("FDT at %p\n", initial_boot_params);
> +		goto found;
> +	}
> +
> +	/* check built-in device tree validity */
> +	initial_boot_params = &__dtb_start;
> +	if (__dtb_end != (void *)&__dtb_start &&
> +	    be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
> +		pr_info("Compiled-in FDT at %p\n", initial_boot_params);
> +		goto found;

__dtb_start is in an init section which is discarded after initcalls.
For this to work, the dtb needs to be copied into a region of allocated
memory. The dtb section can also potentially contain multiple .dtb
blobs. In the use case that you care about you are probably only
thinking about one, but it is entirely possible for device drivers to
have a dtb linked in which may break this if it gets linked in a
different order. The specific example I'm thinking about is I want to
have the DT selftest code load an overlay to get testcase data from a
dtb blob.

The other concern I have here is that I don't really want this to be the
default on a lot of platforms. ARM and PowerPC for instance should only
get the default dtb from the boot wrapper. It needs to be configurable
in some way.

Instead of using __dtb_start, it would probably solve both of the above
problems if the build-in DTB has a specific label identifying it as the
default DT.

g.

Re: [PATCH 1/9] dt: Handle passed/built-in DT selection in early_init_dt_scan()

[Grant Likely]

On Thu, Nov 21, 2013 at 12:21 PM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> On Tue, 19 Nov 2013 12:12:26 +0100, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> Let early_init_dt_scan() fall-back to the built-in DT if no DT was passed,
>> or if it's invalid, so architectures don't have to duplicate this logic.
>>
>> Suggested-by: Rob Herring <rob.herring@calxeda.com>
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>>  drivers/of/fdt.c       |   24 +++++++++++++++++-------
>>  include/linux/of_fdt.h |    1 +
>>  2 files changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index 2fa024b97c43..a797cd43bc8b 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -868,18 +868,28 @@ void * __init __weak early_init_dt_alloc_memory_arch(u64 size, u64 align)
>>
>>  bool __init early_init_dt_scan(void *params)
>>  {
>> -     if (!params)
>> -             return false;
>> -
>>       /* Setup flat device-tree pointer */
>>       initial_boot_params = params;
>>
>> -     /* check device tree validity */
>> -     if (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER) {
>> -             initial_boot_params = NULL;
>> -             return false;
>> +     /* check passed device tree validity */
>> +     if (initial_boot_params &&
>> +         be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
>> +             pr_info("FDT at %p\n", initial_boot_params);
>> +             goto found;
>> +     }
>> +
>> +     /* check built-in device tree validity */
>> +     initial_boot_params = &__dtb_start;
>> +     if (__dtb_end != (void *)&__dtb_start &&
>> +         be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
>> +             pr_info("Compiled-in FDT at %p\n", initial_boot_params);
>> +             goto found;
>
> __dtb_start is in an init section which is discarded after initcalls.
> For this to work, the dtb needs to be copied into a region of allocated
> memory. The dtb section can also potentially contain multiple .dtb
> blobs. In the use case that you care about you are probably only
> thinking about one, but it is entirely possible for device drivers to
> have a dtb linked in which may break this if it gets linked in a
> different order. The specific example I'm thinking about is I want to
> have the DT selftest code load an overlay to get testcase data from a
> dtb blob.
>
> The other concern I have here is that I don't really want this to be the
> default on a lot of platforms. ARM and PowerPC for instance should only
> get the default dtb from the boot wrapper. It needs to be configurable
> in some way.
>
> Instead of using __dtb_start, it would probably solve both of the above
> problems if the build-in DTB has a specific label identifying it as the
> default DT.

Er, not quite. It still doesn't solve the init section discard
problem. That still needs solving.

g.

Re: [PATCH 1/9] dt: Handle passed/built-in DT selection in early_init_dt_scan()

[Grant Likely]

On Thu, Nov 21, 2013 at 12:23 PM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> On Thu, Nov 21, 2013 at 12:21 PM, Grant Likely
> <grant.likely@secretlab.ca> wrote:
>> On Tue, 19 Nov 2013 12:12:26 +0100, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>> Let early_init_dt_scan() fall-back to the built-in DT if no DT was passed,
>>> or if it's invalid, so architectures don't have to duplicate this logic.
>>>
>>> Suggested-by: Rob Herring <rob.herring@calxeda.com>
>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>> ---
>>>  drivers/of/fdt.c       |   24 +++++++++++++++++-------
>>>  include/linux/of_fdt.h |    1 +
>>>  2 files changed, 18 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>> index 2fa024b97c43..a797cd43bc8b 100644
>>> --- a/drivers/of/fdt.c
>>> +++ b/drivers/of/fdt.c
>>> @@ -868,18 +868,28 @@ void * __init __weak early_init_dt_alloc_memory_arch(u64 size, u64 align)
>>>
>>>  bool __init early_init_dt_scan(void *params)
>>>  {
>>> -     if (!params)
>>> -             return false;
>>> -
>>>       /* Setup flat device-tree pointer */
>>>       initial_boot_params = params;
>>>
>>> -     /* check device tree validity */
>>> -     if (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER) {
>>> -             initial_boot_params = NULL;
>>> -             return false;
>>> +     /* check passed device tree validity */
>>> +     if (initial_boot_params &&
>>> +         be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
>>> +             pr_info("FDT at %p\n", initial_boot_params);
>>> +             goto found;
>>> +     }
>>> +
>>> +     /* check built-in device tree validity */
>>> +     initial_boot_params = &__dtb_start;
>>> +     if (__dtb_end != (void *)&__dtb_start &&
>>> +         be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
>>> +             pr_info("Compiled-in FDT at %p\n", initial_boot_params);
>>> +             goto found;
>>
>> __dtb_start is in an init section which is discarded after initcalls.
>> For this to work, the dtb needs to be copied into a region of allocated
>> memory. The dtb section can also potentially contain multiple .dtb
>> blobs. In the use case that you care about you are probably only
>> thinking about one, but it is entirely possible for device drivers to
>> have a dtb linked in which may break this if it gets linked in a
>> different order. The specific example I'm thinking about is I want to
>> have the DT selftest code load an overlay to get testcase data from a
>> dtb blob.
>>
>> The other concern I have here is that I don't really want this to be the
>> default on a lot of platforms. ARM and PowerPC for instance should only
>> get the default dtb from the boot wrapper. It needs to be configurable
>> in some way.
>>
>> Instead of using __dtb_start, it would probably solve both of the above
>> problems if the build-in DTB has a specific label identifying it as the
>> default DT.
>
> Er, not quite. It still doesn't solve the init section discard
> problem. That still needs solving.

Okay, I've got one more comment on this. I know that some
architectures use unflatten_and_copy_devicetree() to sort out the dtb
copy problem. My point here is that for this to become generic code
then early_init_dt_scan() needs to grow the ability to differentiate
between a DT that needs to be copied (built in) vs. a DT that is
already in safe memory. The contents of
unflatten_and_copy_devicetree() could even be rolled into
unflatten_devicetree() if there is a common way to indicate whether or
not the DT needs to be copied that early_init_dt_scan() can
manipulate. ie. a global flag.

g.

Re: [PATCH 1/9] dt: Handle passed/built-in DT selection in early_init_dt_scan()

[Geert Uytterhoeven]

On Thu, Nov 21, 2013 at 1:21 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
>>  bool __init early_init_dt_scan(void *params)
>>  {
>> -     if (!params)
>> -             return false;
>> -
>>       /* Setup flat device-tree pointer */
>>       initial_boot_params = params;
>>
>> -     /* check device tree validity */
>> -     if (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER) {
>> -             initial_boot_params = NULL;
>> -             return false;
>> +     /* check passed device tree validity */
>> +     if (initial_boot_params &&
>> +         be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
>> +             pr_info("FDT at %p\n", initial_boot_params);
>> +             goto found;
>> +     }
>> +
>> +     /* check built-in device tree validity */
>> +     initial_boot_params = &__dtb_start;
>> +     if (__dtb_end != (void *)&__dtb_start &&
>> +         be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
>> +             pr_info("Compiled-in FDT at %p\n", initial_boot_params);
>> +             goto found;
>
> __dtb_start is in an init section which is discarded after initcalls.
> For this to work, the dtb needs to be copied into a region of allocated

My changes don't change the current behavior much: currently
early_init_dt_scan() is already called with &__dtb_start in several places.
If this is broken, it's already broken.

> memory. The dtb section can also potentially contain multiple .dtb
> blobs. In the use case that you care about you are probably only

Multiple dtb blobs are currently handled in platform-specific code, which
passes the right dtb to early_init_dt_scan().

> thinking about one, but it is entirely possible for device drivers to
> have a dtb linked in which may break this if it gets linked in a
> different order. The specific example I'm thinking about is I want to
> have the DT selftest code load an overlay to get testcase data from a
> dtb blob.
>
> The other concern I have here is that I don't really want this to be the
> default on a lot of platforms. ARM and PowerPC for instance should only
> get the default dtb from the boot wrapper. It needs to be configurable
> in some way.

On ARM and PowerPC, the section is empty, hence &__dbt_start ==
&__dtb_end.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/9] dt: Handle passed/built-in DT selection in early_init_dt_scan()

[Grant Likely]

On Thu, 21 Nov 2013 14:42:27 +0100, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Thu, Nov 21, 2013 at 1:21 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> >>  bool __init early_init_dt_scan(void *params)
> >>  {
> >> -     if (!params)
> >> -             return false;
> >> -
> >>       /* Setup flat device-tree pointer */
> >>       initial_boot_params = params;
> >>
> >> -     /* check device tree validity */
> >> -     if (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER) {
> >> -             initial_boot_params = NULL;
> >> -             return false;
> >> +     /* check passed device tree validity */
> >> +     if (initial_boot_params &&
> >> +         be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
> >> +             pr_info("FDT at %p\n", initial_boot_params);
> >> +             goto found;
> >> +     }
> >> +
> >> +     /* check built-in device tree validity */
> >> +     initial_boot_params = &__dtb_start;
> >> +     if (__dtb_end != (void *)&__dtb_start &&
> >> +         be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
> >> +             pr_info("Compiled-in FDT at %p\n", initial_boot_params);
> >> +             goto found;
> >
> > __dtb_start is in an init section which is discarded after initcalls.
> > For this to work, the dtb needs to be copied into a region of allocated
> 
> My changes don't change the current behavior much: currently
> early_init_dt_scan() is already called with &__dtb_start in several places.
> If this is broken, it's already broken.

Yes, but it is called on platforms that already make that assumption
that __dtb_start must be copied and therefore call
unflatten_and_copy_devicetree(). This change makes *all* platforms do
that. That breaks arm, arm64, c6x, microblaze, some mips platforms, and
powerpc!

> > memory. The dtb section can also potentially contain multiple .dtb
> > blobs. In the use case that you care about you are probably only
> 
> Multiple dtb blobs are currently handled in platform-specific code, which
> passes the right dtb to early_init_dt_scan().

The problem is that it makes the default dt completely random because
the generic code still deferrences __dtb_start.

> > thinking about one, but it is entirely possible for device drivers to
> > have a dtb linked in which may break this if it gets linked in a
> > different order. The specific example I'm thinking about is I want to
> > have the DT selftest code load an overlay to get testcase data from a
> > dtb blob.
> >
> > The other concern I have here is that I don't really want this to be the
> > default on a lot of platforms. ARM and PowerPC for instance should only
> > get the default dtb from the boot wrapper. It needs to be configurable
> > in some way.
> 
> On ARM and PowerPC, the section is empty, hence &__dbt_start ==
> &__dtb_end.

There is no guarantee that that will always be so. The patch makes some
poor assumtions, but they shouldn't be difficult to fix.

g.

Re: [PATCH 1/9] dt: Handle passed/built-in DT selection in early_init_dt_scan()

[Geert Uytterhoeven]

On Thu, Nov 21, 2013 at 4:53 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
>> My changes don't change the current behavior much: currently
>> early_init_dt_scan() is already called with &__dtb_start in several places.
>> If this is broken, it's already broken.
>
> Yes, but it is called on platforms that already make that assumption
> that __dtb_start must be copied and therefore call
> unflatten_and_copy_devicetree(). This change makes *all* platforms do
> that. That breaks arm, arm64, c6x, microblaze, some mips platforms, and
> powerpc!

arm, arm64, and powerpc don't have builtin DTBs, hence no change.
microblaze and c6x already did "early_init_devtree(&__dtb_start)" before.

That leaves mips, which has DT handling in too many variants, which I
didn't all verify.

>> > memory. The dtb section can also potentially contain multiple .dtb
>> > blobs. In the use case that you care about you are probably only
>>
>> Multiple dtb blobs are currently handled in platform-specific code, which
>> passes the right dtb to early_init_dt_scan().
>
> The problem is that it makes the default dt completely random because
> the generic code still deferrences __dtb_start.

Only if no DT was passed by the bootloader. And only if there really
is something at __dtb_start. Before it would fail if no DTB was passed
by the bootloader.

>> > thinking about one, but it is entirely possible for device drivers to
>> > have a dtb linked in which may break this if it gets linked in a
>> > different order. The specific example I'm thinking about is I want to
>> > have the DT selftest code load an overlay to get testcase data from a
>> > dtb blob.
>> >
>> > The other concern I have here is that I don't really want this to be the
>> > default on a lot of platforms. ARM and PowerPC for instance should only
>> > get the default dtb from the boot wrapper. It needs to be configurable
>> > in some way.
>>
>> On ARM and PowerPC, the section is empty, hence &__dbt_start ==
>> &__dtb_end.
>
> There is no guarantee that that will always be so. The patch makes some
> poor assumtions, but they shouldn't be difficult to fix.

OK, how to proceed?

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/9] dt: Handle passed/built-in DT selection in early_init_dt_scan()

[Rob Herring]

On Thu, Nov 21, 2013 at 7:42 AM, Geert Uytterhoeven
<geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> On Thu, Nov 21, 2013 at 1:21 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>>>  bool __init early_init_dt_scan(void *params)
>>>  {
>>> -     if (!params)
>>> -             return false;
>>> -
>>>       /* Setup flat device-tree pointer */
>>>       initial_boot_params = params;
>>>
>>> -     /* check device tree validity */
>>> -     if (be32_to_cpu(initial_boot_params->magic) != OF_DT_HEADER) {
>>> -             initial_boot_params = NULL;
>>> -             return false;
>>> +     /* check passed device tree validity */
>>> +     if (initial_boot_params &&
>>> +         be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
>>> +             pr_info("FDT at %p\n", initial_boot_params);
>>> +             goto found;
>>> +     }
>>> +
>>> +     /* check built-in device tree validity */
>>> +     initial_boot_params = &__dtb_start;
>>> +     if (__dtb_end != (void *)&__dtb_start &&
>>> +         be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER) {
>>> +             pr_info("Compiled-in FDT at %p\n", initial_boot_params);
>>> +             goto found;
>>
>> __dtb_start is in an init section which is discarded after initcalls.
>> For this to work, the dtb needs to be copied into a region of allocated
>
> My changes don't change the current behavior much: currently
> early_init_dt_scan() is already called with &__dtb_start in several places.
> If this is broken, it's already broken.

Agreed. The early scan is already done in the init section.

>> memory. The dtb section can also potentially contain multiple .dtb
>> blobs. In the use case that you care about you are probably only
>
> Multiple dtb blobs are currently handled in platform-specific code, which
> passes the right dtb to early_init_dt_scan().
>

Agreed. Arches can still do however complicated dtb search they want.
This handles the common, default case.

>> thinking about one, but it is entirely possible for device drivers to
>> have a dtb linked in which may break this if it gets linked in a
>> different order. The specific example I'm thinking about is I want to
>> have the DT selftest code load an overlay to get testcase data from a
>> dtb blob.
>>
>> The other concern I have here is that I don't really want this to be the
>> default on a lot of platforms. ARM and PowerPC for instance should only
>> get the default dtb from the boot wrapper. It needs to be configurable
>> in some way.
>
> On ARM and PowerPC, the section is empty, hence &__dbt_start ==
> &__dtb_end.

The build rules for built-in dtbs are architecture specific already,
so I don't see a need to make this code configurable at this point.

I really don't like saying one arch can have built-in dtb and another
one can't. Either we allow it or we don't. If this is a bad feature,
then why did we let multiple arches implement it.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/9] dt: Handle passed/built-in DT selection in early_init_dt_scan()

[Geert Uytterhoeven]

On Thu, Nov 21, 2013 at 6:38 PM, Rob Herring <robherring2@gmail.com> wrote:
> I really don't like saying one arch can have built-in dtb and another
> one can't. Either we allow it or we don't. If this is a bad feature,
> then why did we let multiple arches implement it.

... and did we put it in asm-generic, so now _every_ arch has __dtb_start.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/9] dt: Handle passed/built-in DT selection in early_init_dt_scan()

[Grant Likely]

On Thu, 21 Nov 2013 18:42:58 +0100, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Thu, Nov 21, 2013 at 6:38 PM, Rob Herring <robherring2@gmail.com> wrote:
> > I really don't like saying one arch can have built-in dtb and another
> > one can't. Either we allow it or we don't. If this is a bad feature,
> > then why did we let multiple arches implement it.
> 
> ... and did we put it in asm-generic, so now _every_ arch has __dtb_start.
> 
> Gr{oetje,eeting}s,

Some platforms need it because they either don't have bootloader
support, they couldn't change the boot interface, or they don't have a
bootwrapper they can put it in. I'd rather have it in common code than a
different implementation on each architecture.

For the copy flag, it would be fine to put an "if (IS_CONFIGURED())"
test around the copy block so that it never happens on some
architectures.

g.

> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH 1/9] dt: Handle passed/built-in DT selection in early_init_dt_scan()

[Grant Likely]

On Thu, 21 Nov 2013 18:49:08 +0100, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> On Thu, Nov 21, 2013 at 4:53 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> >> My changes don't change the current behavior much: currently
> >> early_init_dt_scan() is already called with &__dtb_start in several places.
> >> If this is broken, it's already broken.
> >
> > Yes, but it is called on platforms that already make that assumption
> > that __dtb_start must be copied and therefore call
> > unflatten_and_copy_devicetree(). This change makes *all* platforms do
> > that. That breaks arm, arm64, c6x, microblaze, some mips platforms, and
> > powerpc!
> 
> arm, arm64, and powerpc don't have builtin DTBs, hence no change.
> microblaze and c6x already did "early_init_devtree(&__dtb_start)" before.
> 
> That leaves mips, which has DT handling in too many variants, which I
> didn't all verify.
> 
> >> > memory. The dtb section can also potentially contain multiple .dtb
> >> > blobs. In the use case that you care about you are probably only
> >>
> >> Multiple dtb blobs are currently handled in platform-specific code, which
> >> passes the right dtb to early_init_dt_scan().
> >
> > The problem is that it makes the default dt completely random because
> > the generic code still deferrences __dtb_start.
> 
> Only if no DT was passed by the bootloader. And only if there really
> is something at __dtb_start. Before it would fail if no DTB was passed
> by the bootloader.
> 
> >> > thinking about one, but it is entirely possible for device drivers to
> >> > have a dtb linked in which may break this if it gets linked in a
> >> > different order. The specific example I'm thinking about is I want to
> >> > have the DT selftest code load an overlay to get testcase data from a
> >> > dtb blob.
> >> >
> >> > The other concern I have here is that I don't really want this to be the
> >> > default on a lot of platforms. ARM and PowerPC for instance should only
> >> > get the default dtb from the boot wrapper. It needs to be configurable
> >> > in some way.
> >>
> >> On ARM and PowerPC, the section is empty, hence &__dbt_start ==
> >> &__dtb_end.
> >
> > There is no guarantee that that will always be so. The patch makes some
> > poor assumtions, but they shouldn't be difficult to fix.
> 
> OK, how to proceed?

Add a global flag, bool __init of_dtb_needs_copy, and make it default to
false. On platforms that need it, change it to true. Make
unflatten_device_tree() check the flag and copy the tree if necessary.
Make early_init_dt_scan() set the flag if it choses to use a default
built-in tree. Then get rid of unflatten_and_copy_devicetree() because
it has become redundant.

Finally, make an explicit backup __dtb pointer.  Don't assume that
__dtb_start == __dtb_end is the correct test. We never want to have that
behavour on arm/powerpc/microblaze, but that doesn't mean there won't be
compiled in dtbs for other reasons. Create a __dtb_default symbol
require platforms to do something specific (ie. select a config symbol)
to set it to __dtb_start. It's not perfect, but at least it acknowledges
the problem and can be refined later.

g.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/17] dt: Consolidate __dtb_start declarations in <linux/of_fdt.h>

[James Hogan]

On 12/11/13 19:42, Geert Uytterhoeven wrote:
> The different architectures used their own (and different) declarations:
> 
>     extern struct boot_param_header __dtb_start;
>     extern u32 __dtb_start[];
>     extern char __dtb_start[];
> 
> Consolidate them using the first variant in <linux/of_fdt.h>.
> This requires adding a few "address of" operators on architectures where
> __dtb_start was an array before.
> 
> Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> Cc: Vineet Gupta <vgupta-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> Cc: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> Cc: Ralf Baechle <ralf-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
> Cc: Jonas Bonn <jonas-A9uVI2HLR7kOP4wsBPIw7w@public.gmane.org>
> CC: Chris Zankel <chris-YvXeqwSYzG2sTnJN9+BGXg@public.gmane.org>
> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Acked-by: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

Thanks
James

> ---
>  arch/arc/include/asm/sections.h             |    1 -
>  arch/arc/kernel/setup.c                     |    2 +-
>  arch/metag/kernel/setup.c                   |    6 +-----
>  arch/mips/include/asm/mips-boards/generic.h |    4 ----
>  arch/mips/lantiq/prom.h                     |    2 --
>  arch/mips/netlogic/xlp/dt.c                 |    4 ++--
>  arch/mips/ralink/of.c                       |    2 --
>  arch/openrisc/kernel/setup.c                |    2 +-
>  arch/openrisc/kernel/vmlinux.h              |    2 --
>  arch/xtensa/kernel/setup.c                  |    3 +--
>  include/linux/of_fdt.h                      |    3 +++
>  11 files changed, 9 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/arc/include/asm/sections.h b/arch/arc/include/asm/sections.h
> index 764f1e3ba752..09db952e14bd 100644
> --- a/arch/arc/include/asm/sections.h
> +++ b/arch/arc/include/asm/sections.h
> @@ -12,6 +12,5 @@
>  #include <asm-generic/sections.h>
>  
>  extern char __arc_dccm_base[];
> -extern char __dtb_start[];
>  
>  #endif
> diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
> index 643eae4436e0..ef705d0f50f5 100644
> --- a/arch/arc/kernel/setup.c
> +++ b/arch/arc/kernel/setup.c
> @@ -314,7 +314,7 @@ void setup_processor(void)
>  void __init setup_arch(char **cmdline_p)
>  {
>  	/* This also populates @boot_command_line from /bootargs */
> -	machine_desc = setup_machine_fdt(__dtb_start);
> +	machine_desc = setup_machine_fdt(&__dtb_start);
>  	if (!machine_desc)
>  		panic("Embedded DT invalid\n");
>  
> diff --git a/arch/metag/kernel/setup.c b/arch/metag/kernel/setup.c
> index 129c7cdda1ce..919c782f5479 100644
> --- a/arch/metag/kernel/setup.c
> +++ b/arch/metag/kernel/setup.c
> @@ -105,10 +105,6 @@
>  
>  extern char _heap_start[];
>  
> -#ifdef CONFIG_METAG_BUILTIN_DTB
> -extern u32 __dtb_start[];
> -#endif
> -
>  #ifdef CONFIG_DA_CONSOLE
>  /* Our early channel based console driver */
>  extern struct console dash_console;
> @@ -251,7 +247,7 @@ void __init setup_arch(char **cmdline_p)
>  	if (!machine_desc) {
>  #ifdef CONFIG_METAG_BUILTIN_DTB
>  		/* try the embedded device tree */
> -		machine_desc = setup_machine_fdt(__dtb_start);
> +		machine_desc = setup_machine_fdt(&__dtb_start);
>  		if (!machine_desc)
>  			panic("Invalid embedded device tree.");
>  #else
> diff --git a/arch/mips/include/asm/mips-boards/generic.h b/arch/mips/include/asm/mips-boards/generic.h
> index 48616816bcbc..c904c24550f6 100644
> --- a/arch/mips/include/asm/mips-boards/generic.h
> +++ b/arch/mips/include/asm/mips-boards/generic.h
> @@ -67,10 +67,6 @@
>  
>  extern int mips_revision_sconid;
>  
> -#ifdef CONFIG_OF
> -extern struct boot_param_header __dtb_start;
> -#endif
> -
>  #ifdef CONFIG_PCI
>  extern void mips_pcibios_init(void);
>  #else
> diff --git a/arch/mips/lantiq/prom.h b/arch/mips/lantiq/prom.h
> index 8e07b5f28ef1..bfd2d58c1d69 100644
> --- a/arch/mips/lantiq/prom.h
> +++ b/arch/mips/lantiq/prom.h
> @@ -26,6 +26,4 @@ struct ltq_soc_info {
>  extern void ltq_soc_detect(struct ltq_soc_info *i);
>  extern void ltq_soc_init(void);
>  
> -extern struct boot_param_header __dtb_start;
> -
>  #endif
> diff --git a/arch/mips/netlogic/xlp/dt.c b/arch/mips/netlogic/xlp/dt.c
> index 8316d5454b17..87250f378386 100644
> --- a/arch/mips/netlogic/xlp/dt.c
> +++ b/arch/mips/netlogic/xlp/dt.c
> @@ -42,7 +42,7 @@
>  #include <asm/prom.h>
>  
>  extern u32 __dtb_xlp_evp_begin[], __dtb_xlp_svp_begin[],
> -	__dtb_xlp_fvp_begin[], __dtb_start[];
> +	__dtb_xlp_fvp_begin[];
>  static void *xlp_fdt_blob;
>  
>  void __init *xlp_dt_init(void *fdtp)
> @@ -66,7 +66,7 @@ void __init *xlp_dt_init(void *fdtp)
>  #endif
>  		default:
>  			/* Pick a built-in if any, and hope for the best */
> -			fdtp = __dtb_start;
> +			fdtp = &__dtb_start;
>  			break;
>  		}
>  	}
> diff --git a/arch/mips/ralink/of.c b/arch/mips/ralink/of.c
> index eccc5526155e..615603bd8063 100644
> --- a/arch/mips/ralink/of.c
> +++ b/arch/mips/ralink/of.c
> @@ -28,8 +28,6 @@
>  __iomem void *rt_sysc_membase;
>  __iomem void *rt_memc_membase;
>  
> -extern struct boot_param_header __dtb_start;
> -
>  __iomem void *plat_of_remap_node(const char *node)
>  {
>  	struct resource res;
> diff --git a/arch/openrisc/kernel/setup.c b/arch/openrisc/kernel/setup.c
> index 4fc7ccc0a2cf..c9b880d64bb8 100644
> --- a/arch/openrisc/kernel/setup.c
> +++ b/arch/openrisc/kernel/setup.c
> @@ -213,7 +213,7 @@ void __init or32_early_setup(void *fdt)
>  	if (fdt)
>  		pr_info("FDT at %p\n", fdt);
>  	else {
> -		fdt = __dtb_start;
> +		fdt = &__dtb_start;
>  		pr_info("Compiled-in FDT at %p\n", fdt);
>  	}
>  	early_init_devtree(fdt);
> diff --git a/arch/openrisc/kernel/vmlinux.h b/arch/openrisc/kernel/vmlinux.h
> index 70b9ce41835c..bbcdf21b0b35 100644
> --- a/arch/openrisc/kernel/vmlinux.h
> +++ b/arch/openrisc/kernel/vmlinux.h
> @@ -5,6 +5,4 @@
>  extern char __initrd_start, __initrd_end;
>  #endif
>  
> -extern u32 __dtb_start[];
> -
>  #endif
> diff --git a/arch/xtensa/kernel/setup.c b/arch/xtensa/kernel/setup.c
> index 6e2b6638122d..d96cf8f3fe70 100644
> --- a/arch/xtensa/kernel/setup.c
> +++ b/arch/xtensa/kernel/setup.c
> @@ -68,8 +68,7 @@ extern int initrd_below_start_ok;
>  #endif
>  
>  #ifdef CONFIG_OF
> -extern u32 __dtb_start[];
> -void *dtb_start = __dtb_start;
> +void *dtb_start = &__dtb_start;
>  #endif
>  
>  unsigned char aux_device_present;
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index 0beaee9dac1f..aabc49f3e403 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -57,6 +57,9 @@ struct boot_param_header {
>  	__be32	dt_struct_size;		/* size of the DT structure block */
>  };
>  
> +/* For kernels with a built-in device tree */
> +extern struct boot_param_header __dtb_start;
> +
>  #if defined(CONFIG_OF_FLATTREE)
>  
>  struct device_node;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC 15/17] microblaze: Convert from _fdt_start to __dtb_start

[Geert Uytterhoeven]

Kill the microblaze-specific __fdt_blob section, and start using
.dtb.init.rodata from <asm-generic/sections.h> for built-in DTBs, like
most other DT enabled architectures.

Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
Cc: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: microblaze-uclinux-rVRm/Wmeqae7NGdpmJTKYQ@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
Untested, but the resulting vmlinux after "make simpleImage.system" looks
sane.

QUESTION: The padding of the DTB is gone, is this OK?

TODO: Start using the DTB logic in scripts/Makefile.lib instead of
      arch/microblaze/boot/dts/linked_dtb.S

 arch/microblaze/boot/Makefile          |    2 +-
 arch/microblaze/boot/dts/linked_dtb.S  |    2 +-
 arch/microblaze/include/asm/sections.h |    4 +---
 arch/microblaze/kernel/head.S          |    2 +-
 arch/microblaze/kernel/setup.c         |    4 ++--
 arch/microblaze/kernel/vmlinux.lds.S   |    8 --------
 6 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/arch/microblaze/boot/Makefile b/arch/microblaze/boot/Makefile
index 8e211cc28dac..e4a99a22dab8 100644
--- a/arch/microblaze/boot/Makefile
+++ b/arch/microblaze/boot/Makefile
@@ -23,7 +23,7 @@ quiet_cmd_cp = CP      $< $@$2
 
 quiet_cmd_strip = STRIP   $@
 	cmd_strip = $(STRIP) -K microblaze_start -K _end -K __log_buf \
-				-K _fdt_start vmlinux -o $@
+				-K __dtb_start vmlinux -o $@
 
 UIMAGE_LOADADDR = $(CONFIG_KERNEL_BASE_ADDR)
 
diff --git a/arch/microblaze/boot/dts/linked_dtb.S b/arch/microblaze/boot/dts/linked_dtb.S
index 23345af3721f..bfc53b5795aa 100644
--- a/arch/microblaze/boot/dts/linked_dtb.S
+++ b/arch/microblaze/boot/dts/linked_dtb.S
@@ -1,2 +1,2 @@
-.section __fdt_blob,"a"
+.section .dtb.init.rodata,"a"
 .incbin "arch/microblaze/boot/dts/system.dtb"
diff --git a/arch/microblaze/include/asm/sections.h b/arch/microblaze/include/asm/sections.h
index 1b281d3ea734..fb8ab0ba03fc 100644
--- a/arch/microblaze/include/asm/sections.h
+++ b/arch/microblaze/include/asm/sections.h
@@ -16,8 +16,6 @@
 # ifndef __ASSEMBLY__
 extern char _ssbss[], _esbss[];
 extern unsigned long __ivt_start[], __ivt_end[];
-
-extern u32 _fdt_start[], _fdt_end[];
-
 # endif /* !__ASSEMBLY__ */
+
 #endif /* _ASM_MICROBLAZE_SECTIONS_H */
diff --git a/arch/microblaze/kernel/head.S b/arch/microblaze/kernel/head.S
index 817b7eec95b6..6108889f0025 100644
--- a/arch/microblaze/kernel/head.S
+++ b/arch/microblaze/kernel/head.S
@@ -94,7 +94,7 @@ big_endian:
 	bnei	r11, no_fdt_arg			/* No - get out of here */
 _prepare_copy_fdt:
 	or	r11, r0, r0 /* incremment */
-	ori	r4, r0, TOPHYS(_fdt_start)
+	ori	r4, r0, TOPHYS(__dtb_start)
 	ori	r3, r0, (0x8000 - 4)
 _copy_fdt:
 	lw	r12, r7, r11 /* r12 = r7 + r11 */
diff --git a/arch/microblaze/kernel/setup.c b/arch/microblaze/kernel/setup.c
index 603e22fec6d5..a4534f584be0 100644
--- a/arch/microblaze/kernel/setup.c
+++ b/arch/microblaze/kernel/setup.c
@@ -136,7 +136,7 @@ void __init machine_early_init(const char *cmdline, unsigned int ram,
 	lockdep_init();
 
 /* initialize device tree for usage in early_printk */
-	early_init_devtree(_fdt_start);
+	early_init_devtree(&__dtb_start);
 
 #ifdef CONFIG_EARLY_PRINTK
 	setup_early_printk(NULL);
@@ -152,7 +152,7 @@ void __init machine_early_init(const char *cmdline, unsigned int ram,
 	if (fdt)
 		pr_info("FDT at 0x%08x\n", fdt);
 	else
-		pr_info("Compiled-in FDT at %p\n", _fdt_start);
+		pr_info("Compiled-in FDT at %p\n", &__dtb_start);
 
 #ifdef CONFIG_MTD_UCLINUX
 	pr_info("Found romfs @ 0x%08x (0x%08x)\n",
diff --git a/arch/microblaze/kernel/vmlinux.lds.S b/arch/microblaze/kernel/vmlinux.lds.S
index 936d01a689d7..2659e70984a3 100644
--- a/arch/microblaze/kernel/vmlinux.lds.S
+++ b/arch/microblaze/kernel/vmlinux.lds.S
@@ -40,14 +40,6 @@ SECTIONS {
 		_etext = . ;
 	}
 
-	. = ALIGN (4) ;
-	__fdt_blob : AT(ADDR(__fdt_blob) - LOAD_OFFSET) {
-		_fdt_start = . ;		/* place for fdt blob */
-		*(__fdt_blob) ;			/* Any link-placed DTB */
-	        . = _fdt_start + 0x8000;	/* Pad up to 32kbyte */
-		_fdt_end = . ;
-	}
-
 	. = ALIGN(16);
 	RODATA
 	EXCEPTION_TABLE(16)
-- 
1.7.9.5

_______________________________________________
microblaze-linux mailing list
microblaze-linux-FR6EJeJVuqfA6Z3fQjNZrN9u6TNh0Fb7@public.gmane.org
https://lists.eait.uq.edu.au/mailman/listinfo/microblaze-linux

Re: [PATCH RFC 15/17] microblaze: Convert from _fdt_start to __dtb_start

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 3702 bytes --]

On 11/12/2013 08:42 PM, Geert Uytterhoeven wrote:
> Kill the microblaze-specific __fdt_blob section, and start using
> .dtb.init.rodata from <asm-generic/sections.h> for built-in DTBs, like
> most other DT enabled architectures.
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Michal Simek <monstr@monstr.eu>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: microblaze-uclinux@itee.uq.edu.au
> Cc: devicetree@vger.kernel.org
> ---
> Untested, but the resulting vmlinux after "make simpleImage.system" looks
> sane.
> 
> QUESTION: The padding of the DTB is gone, is this OK?

Removing this kernel padding should be fine. I can't remember
why it was there from the beginning.

> 
> TODO: Start using the DTB logic in scripts/Makefile.lib instead of
>       arch/microblaze/boot/dts/linked_dtb.S
> 
>  arch/microblaze/boot/Makefile          |    2 +-
>  arch/microblaze/boot/dts/linked_dtb.S  |    2 +-
>  arch/microblaze/include/asm/sections.h |    4 +---
>  arch/microblaze/kernel/head.S          |    2 +-
>  arch/microblaze/kernel/setup.c         |    4 ++--
>  arch/microblaze/kernel/vmlinux.lds.S   |    8 --------
>  6 files changed, 6 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/microblaze/boot/Makefile b/arch/microblaze/boot/Makefile
> index 8e211cc28dac..e4a99a22dab8 100644
> --- a/arch/microblaze/boot/Makefile
> +++ b/arch/microblaze/boot/Makefile
> @@ -23,7 +23,7 @@ quiet_cmd_cp = CP      $< $@$2
>  
>  quiet_cmd_strip = STRIP   $@
>  	cmd_strip = $(STRIP) -K microblaze_start -K _end -K __log_buf \
> -				-K _fdt_start vmlinux -o $@
> +				-K __dtb_start vmlinux -o $@

I have started to discuss this change internally because our tools related
on this _fdt_start name.

>  
>  UIMAGE_LOADADDR = $(CONFIG_KERNEL_BASE_ADDR)
>  
> diff --git a/arch/microblaze/boot/dts/linked_dtb.S b/arch/microblaze/boot/dts/linked_dtb.S
> index 23345af3721f..bfc53b5795aa 100644
> --- a/arch/microblaze/boot/dts/linked_dtb.S
> +++ b/arch/microblaze/boot/dts/linked_dtb.S
> @@ -1,2 +1,2 @@
> -.section __fdt_blob,"a"
> +.section .dtb.init.rodata,"a"
>  .incbin "arch/microblaze/boot/dts/system.dtb"
> diff --git a/arch/microblaze/include/asm/sections.h b/arch/microblaze/include/asm/sections.h
> index 1b281d3ea734..fb8ab0ba03fc 100644
> --- a/arch/microblaze/include/asm/sections.h
> +++ b/arch/microblaze/include/asm/sections.h
> @@ -16,8 +16,6 @@
>  # ifndef __ASSEMBLY__
>  extern char _ssbss[], _esbss[];
>  extern unsigned long __ivt_start[], __ivt_end[];
> -
> -extern u32 _fdt_start[], _fdt_end[];
> -
>  # endif /* !__ASSEMBLY__ */
> +
>  #endif /* _ASM_MICROBLAZE_SECTIONS_H */
> diff --git a/arch/microblaze/kernel/head.S b/arch/microblaze/kernel/head.S
> index 817b7eec95b6..6108889f0025 100644
> --- a/arch/microblaze/kernel/head.S
> +++ b/arch/microblaze/kernel/head.S
> @@ -94,7 +94,7 @@ big_endian:
>  	bnei	r11, no_fdt_arg			/* No - get out of here */
>  _prepare_copy_fdt:
>  	or	r11, r0, r0 /* incremment */
> -	ori	r4, r0, TOPHYS(_fdt_start)
> +	ori	r4, r0, TOPHYS(__dtb_start)
>  	ori	r3, r0, (0x8000 - 4)

If others is fine this 0x8000 is that hardcoded pad dtb size in lds file
and needs to be changed too.

Let me discuss this with others but if changing symbol name is fine
then we can fix just this problem and your patch is fine.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH RFC 15/17] microblaze: Convert from _fdt_start to __dtb_start

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1749 bytes --]

On 11/13/2013 11:19 AM, Michal Simek wrote:
> On 11/12/2013 08:42 PM, Geert Uytterhoeven wrote:
>> Kill the microblaze-specific __fdt_blob section, and start using
>> .dtb.init.rodata from <asm-generic/sections.h> for built-in DTBs, like
>> most other DT enabled architectures.
>>
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Michal Simek <monstr@monstr.eu>
>> Cc: Rob Herring <rob.herring@calxeda.com>
>> Cc: microblaze-uclinux@itee.uq.edu.au
>> Cc: devicetree@vger.kernel.org
>> ---
>> Untested, but the resulting vmlinux after "make simpleImage.system" looks
>> sane.
>>
>> QUESTION: The padding of the DTB is gone, is this OK?
> 
> Removing this kernel padding should be fine. I can't remember
> why it was there from the beginning.

I know why is this here. The reason is process how dtb is passed
to the kernel via command line. head.S code just copy that dtb to
the same location where compiled-in dts is expected and kernel
doesn't need to care about position of dtb because this copy is
done without MMU. And the full kernel is covered by 2 tlbs and
we don't need to use another TLB for dtb mapping.

It means pad matters a lot. Because u-boot ITS format doesn't use
simpleImage target but the kernel need to have a space for copying
dtb to this kernel location.

Let me think about if there is an easy way to handle dtbs
which are passed from bootloader.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH RFC 15/17] microblaze: Convert from _fdt_start to __dtb_start

[Geert Uytterhoeven]

On Wed, Nov 13, 2013 at 2:40 PM, Michal Simek <monstr@monstr.eu> wrote:
>>> QUESTION: The padding of the DTB is gone, is this OK?
>>
>> Removing this kernel padding should be fine. I can't remember
>> why it was there from the beginning.
>
> I know why is this here. The reason is process how dtb is passed
> to the kernel via command line. head.S code just copy that dtb to
> the same location where compiled-in dts is expected and kernel
> doesn't need to care about position of dtb because this copy is
> done without MMU. And the full kernel is covered by 2 tlbs and
> we don't need to use another TLB for dtb mapping.
>
> It means pad matters a lot. Because u-boot ITS format doesn't use
> simpleImage target but the kernel need to have a space for copying
> dtb to this kernel location.
>
> Let me think about if there is an easy way to handle dtbs
> which are passed from bootloader.

You can add to asm-generic/sections.h:

#ifndef ARCH_DTB_PADDING
#define ARCH_DTB_PADDING
#endif

and change KERNEL_DTB() to:

#define KERNEL_DTB()                                                    \
       STRUCT_ALIGN();                                                 \
       VMLINUX_SYMBOL(__dtb_start) = .;                                \
       *(.dtb.init.rodata)                                             \
       ARCH_DTB_PADDING                                \
       VMLINUX_SYMBOL(__dtb_end) = .;

Then microblaze can predefine ARCH_DTB_PADDING in its
<asm/sections.h>.

Still, it will crash badly if the external DTB is larger than
ARCH_DTB_PADDING.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC 15/17] microblaze: Convert from _fdt_start to __dtb_start

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2404 bytes --]

On 11/13/2013 03:02 PM, Geert Uytterhoeven wrote:
> On Wed, Nov 13, 2013 at 2:40 PM, Michal Simek <monstr@monstr.eu> wrote:
>>>> QUESTION: The padding of the DTB is gone, is this OK?
>>>
>>> Removing this kernel padding should be fine. I can't remember
>>> why it was there from the beginning.
>>
>> I know why is this here. The reason is process how dtb is passed
>> to the kernel via command line. head.S code just copy that dtb to
>> the same location where compiled-in dts is expected and kernel
>> doesn't need to care about position of dtb because this copy is
>> done without MMU. And the full kernel is covered by 2 tlbs and
>> we don't need to use another TLB for dtb mapping.
>>
>> It means pad matters a lot. Because u-boot ITS format doesn't use
>> simpleImage target but the kernel need to have a space for copying
>> dtb to this kernel location.
>>
>> Let me think about if there is an easy way to handle dtbs
>> which are passed from bootloader.
> 
> You can add to asm-generic/sections.h:
> 
> #ifndef ARCH_DTB_PADDING
> #define ARCH_DTB_PADDING
> #endif
> 
> and change KERNEL_DTB() to:
> 
> #define KERNEL_DTB()                                                    \
>        STRUCT_ALIGN();                                                 \
>        VMLINUX_SYMBOL(__dtb_start) = .;                                \
>        *(.dtb.init.rodata)                                             \
>        ARCH_DTB_PADDING                                \
>        VMLINUX_SYMBOL(__dtb_end) = .;
> 
> Then microblaze can predefine ARCH_DTB_PADDING in its
> <asm/sections.h>.
> 
> Still, it will crash badly if the external DTB is larger than
> ARCH_DTB_PADDING.

yes but the question is if our bootflow is correct or not.
Because the problem is still with ARCH_DTB_PADDING value.
In past we have extended this padding from 16k to 32k.

We are setting up one tlb for early console and we can just do the same
for dtb and fix kernel for this cases. Then we don't need to waste
of memory for allocating space for dtb.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH RFC 15/17] microblaze: Convert from _fdt_start to __dtb_start

[Rob Herring]

On 11/13/2013 07:40 AM, Michal Simek wrote:
> On 11/13/2013 11:19 AM, Michal Simek wrote:
>> On 11/12/2013 08:42 PM, Geert Uytterhoeven wrote:
>>> Kill the microblaze-specific __fdt_blob section, and start
>>> using .dtb.init.rodata from <asm-generic/sections.h> for
>>> built-in DTBs, like most other DT enabled architectures.
>>> 
>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> Cc:
>>> Michal Simek <monstr@monstr.eu> Cc: Rob Herring
>>> <rob.herring@calxeda.com> Cc:
>>> microblaze-uclinux@itee.uq.edu.au Cc:
>>> devicetree@vger.kernel.org --- Untested, but the resulting
>>> vmlinux after "make simpleImage.system" looks sane.
>>> 
>>> QUESTION: The padding of the DTB is gone, is this OK?
>> 
>> Removing this kernel padding should be fine. I can't remember why
>> it was there from the beginning.
> 
> I know why is this here. The reason is process how dtb is passed to
> the kernel via command line. head.S code just copy that dtb to the
> same location where compiled-in dts is expected and kernel doesn't
> need to care about position of dtb because this copy is done
> without MMU. And the full kernel is covered by 2 tlbs and we don't
> need to use another TLB for dtb mapping.
> 
> It means pad matters a lot. Because u-boot ITS format doesn't use 
> simpleImage target but the kernel need to have a space for copying 
> dtb to this kernel location.
> 
> Let me think about if there is an easy way to handle dtbs which are
> passed from bootloader.

Isn't the built-in dtb in a RO init section? Seems like a bad idea to
write to it. Also, it will be a double copy because a built-in dtb has
to be copied out of init section.

Rob

Re: [PATCH RFC 15/17] microblaze: Convert from _fdt_start to __dtb_start

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2681 bytes --]

On 11/13/2013 05:14 PM, Rob Herring wrote:
> On 11/13/2013 07:40 AM, Michal Simek wrote:
>> On 11/13/2013 11:19 AM, Michal Simek wrote:
>>> On 11/12/2013 08:42 PM, Geert Uytterhoeven wrote:
>>>> Kill the microblaze-specific __fdt_blob section, and start
>>>> using .dtb.init.rodata from <asm-generic/sections.h> for
>>>> built-in DTBs, like most other DT enabled architectures.
>>>>
>>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> Cc:
>>>> Michal Simek <monstr@monstr.eu> Cc: Rob Herring
>>>> <rob.herring@calxeda.com> Cc:
>>>> microblaze-uclinux@itee.uq.edu.au Cc:
>>>> devicetree@vger.kernel.org --- Untested, but the resulting
>>>> vmlinux after "make simpleImage.system" looks sane.
>>>>
>>>> QUESTION: The padding of the DTB is gone, is this OK?
>>>
>>> Removing this kernel padding should be fine. I can't remember why
>>> it was there from the beginning.
>>
>> I know why is this here. The reason is process how dtb is passed to
>> the kernel via command line. head.S code just copy that dtb to the
>> same location where compiled-in dts is expected and kernel doesn't
>> need to care about position of dtb because this copy is done
>> without MMU. And the full kernel is covered by 2 tlbs and we don't
>> need to use another TLB for dtb mapping.
>>
>> It means pad matters a lot. Because u-boot ITS format doesn't use 
>> simpleImage target but the kernel need to have a space for copying 
>> dtb to this kernel location.
>>
>> Let me think about if there is an easy way to handle dtbs which are
>> passed from bootloader.
> 
> Isn't the built-in dtb in a RO init section? Seems like a bad idea to
> write to it. Also, it will be a double copy because a built-in dtb has
> to be copied out of init section.

Microblaze has it currently out of RO section.
This patch is adding it there.

I have looked at powerpc flow which is probably the closest to our version.

head.S
Saving dtb pointer to r31 then initial_mmu which setups 16MB tlb for kernel
then some exception and calling machine_init()->early_init_devtree()
->move_device_tree() where also change initial_boot_params location
for unflatten_device_tree() in setup_arch().

Ben: I didn't find any TLB entry for DTB itself.
Does it mean that I missed it or that powerpc can access in machine_init()
the whole memory space?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

[PATCH RFC 16/17] c6x: Convert from _fdt_start to __dtb_start

[Geert Uytterhoeven]

Kill the c6x-specific __fdt_blob section, and start using .dtb.init.rodata
from <asm-generic/sections.h> for built-in DTBs, like most other DT enabled
architectures.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Aurelien Jacquiot <a-jacquiot@ti.com>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: linux-c6x-dev@linux-c6x.org
Cc: devicetree@vger.kernel.org
---
Untested due to lack of cross-compiler, but the changes are similar to
microblaze.

QUESTION: The padding of the DTB is gone, is this OK?

TODO: Start using the DTB logic in scripts/Makefile.lib instead of
      arch/c6x/boot/dts/linked_dtb.S

 arch/c6x/boot/dts/linked_dtb.S  |    2 +-
 arch/c6x/include/asm/sections.h |    1 -
 arch/c6x/kernel/setup.c         |    2 +-
 arch/c6x/kernel/vmlinux.lds.S   |    9 ---------
 4 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/arch/c6x/boot/dts/linked_dtb.S b/arch/c6x/boot/dts/linked_dtb.S
index cf347f1d16ce..a3447423abda 100644
--- a/arch/c6x/boot/dts/linked_dtb.S
+++ b/arch/c6x/boot/dts/linked_dtb.S
@@ -1,2 +1,2 @@
-.section __fdt_blob,"a"
+.section .dtb.init.rodata,"a"
 .incbin "arch/c6x/boot/dts/builtin.dtb"
diff --git a/arch/c6x/include/asm/sections.h b/arch/c6x/include/asm/sections.h
index f703989d837a..b0eeca16824f 100644
--- a/arch/c6x/include/asm/sections.h
+++ b/arch/c6x/include/asm/sections.h
@@ -7,6 +7,5 @@ extern char _vectors_start[];
 extern char _vectors_end[];
 
 extern char _data_lma[];
-extern char _fdt_start[], _fdt_end[];
 
 #endif /* _ASM_C6X_SECTIONS_H */
diff --git a/arch/c6x/kernel/setup.c b/arch/c6x/kernel/setup.c
index 731db4b9014d..414353027ba9 100644
--- a/arch/c6x/kernel/setup.c
+++ b/arch/c6x/kernel/setup.c
@@ -266,7 +266,7 @@ int __init c6x_add_memory(phys_addr_t start, unsigned long size)
 notrace void __init machine_init(unsigned long dt_ptr)
 {
 	struct boot_param_header *dtb = __va(dt_ptr);
-	struct boot_param_header *fdt = (struct boot_param_header *)_fdt_start;
+	struct boot_param_header *fdt = &__dtb_start;
 
 	/* interrupts must be masked */
 	set_creg(IER, 2);
diff --git a/arch/c6x/kernel/vmlinux.lds.S b/arch/c6x/kernel/vmlinux.lds.S
index 5a6e141d1641..fea7f7a90f0f 100644
--- a/arch/c6x/kernel/vmlinux.lds.S
+++ b/arch/c6x/kernel/vmlinux.lds.S
@@ -88,15 +88,6 @@ SECTIONS
 	}
 
 	. = ALIGN (8) ;
-	__fdt_blob : AT(ADDR(__fdt_blob) - LOAD_OFFSET)
-	{
-		_fdt_start = . ;	/* place for fdt blob */
-		*(__fdt_blob) ;		/* Any link-placed DTB */
-		BYTE(0);		/* section always has contents */
-	        . = _fdt_start + 0x4000;	/* Pad up to 16kbyte */
-		_fdt_end = . ;
-	}
-
 	_etext = .;
 
 	/*
-- 
1.7.9.5

Re: [PATCH RFC 16/17] c6x: Convert from _fdt_start to __dtb_start

[Mark Salter]

On Tue, 2013-11-12 at 20:42 +0100, Geert Uytterhoeven wrote:
> Kill the c6x-specific __fdt_blob section, and start
> using .dtb.init.rodata
> from <asm-generic/sections.h> for built-in DTBs, like most other DT
> enabled
> architectures.
> 
> Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> Cc: Aurelien Jacquiot <a-jacquiot-l0cyMroinI0@public.gmane.org>
> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> Cc: linux-c6x-dev-jPsnJVOj+W6hPH1hqNUYSQ@public.gmane.org
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
> Untested due to lack of cross-compiler, but the changes are similar to
> microblaze.
> 
> QUESTION: The padding of the DTB is gone, is this OK?

Yes, that's fine.

Acked-by: Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html