fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined

PATCH Superseded Suriyan Ramasami 4 participants 5 messages Nov 20, 2014 – Nov 27, 2014

Share & tools

Suriyan Ramasami

author

2014-11-20 23:55 UTC #0 lore raw

PATCH 1/1 fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined

The boot commands - bootz/bootm mandate a third argument which is the address to the FDT blob. In cases where this argument is not specified, boot fails with a message indicating a missing FDT.

This causes non-FDT kernels to fail to boot. This patch allows both FDT and non-FDT kernels to boot by making the third parameter to the bootm/bootz optional.

Signed-off-by Suriyan Ramasami <[email protected]>

---

Changes in v1:
- First try

 common/image-fdt.c | 4 ++++
 1 file changed, 4 insertions(+)

common/image-fdt.c +4 -0

index a39ae1b..1a02166 100644
@@ -430,6 +430,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
 error:
 	*of_flat_tree = NULL;
 	*of_size = 0;
+	if (argc <= 2) {
+		debug("Continuing to boot without FDT\n");
+		return 0;
+	}
 	return 1;
 }

Signature · 1 line

1.8.3.1

Heiko Schocher

2014-11-21 08:35 UTC #1 lore raw

PATCH 1/1 fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined

Hello Suriyan Ramasami,

Am 21.11.2014 00:55, schrieb Suriyan Ramasami:

Quoted 8 lines

The boot commands - bootz/bootm mandate a third argument which is the address to the FDT blob. In cases where this argument is not specified, boot fails with a message indicating a missing FDT.
This causes non-FDT kernels to fail to boot. This patch allows both FDT and non-FDT kernels to boot by making the third parameter to the bootm/bootz optional.

Quoted 8 lines

Signed-off-by Suriyan Ramasami <[email protected]>
---
Changes in v1:
- First try
  common/image-fdt.c | 4 ++++
  1 file changed, 4 insertions(+)

Thanks!

Acked-by Heiko Schocher <[email protected]>

tested on an at91sam9g20 based board (not mainlined yet), so:

Tested-by Heiko Schocher <[email protected]>

bye,
Heiko

Signature · 2 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Hans de Goede

2014-11-21 14:24 UTC #2 lore raw

PATCH 1/1 fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined

In reply to Suriyan Ramasami

Hi,

Quoted 9 lines · Suriyan Ramasami

The boot commands - bootz/bootm mandate a third argument which is the address to the FDT blob. In cases where this argument is not specified, boot fails with a message indicating a missing FDT.
This causes non-FDT kernels to fail to boot. This patch allows both FDT and non-FDT kernels to boot by making the third parameter to the bootm/bootz optional.
Signed-off-by Suriyan Ramasami <[email protected]>

Looks good, and works for my case (booting old linux-sunxi 3.4 kernels) too) :

Tested-by Hans de Goede <[email protected]>

Acked-by Hans de Goede <[email protected]>

Thanks & Regards,

Hans

Quoted 24 lines

---

Changes in v1:
- First try

 common/image-fdt.c | 4 ++++
 1 file changed, 4 insertions(+)

common/image-fdt.c +4 -0

index a39ae1b..1a02166 100644
@@ -430,6 +430,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
 error:
 	*of_flat_tree = NULL;
 	*of_size = 0;
+	if (argc <= 2) {
+		debug("Continuing to boot without FDT\n");
+		return 0;
+	}
 	return 1;
 }

Simon Glass

2014-11-27 15:58 UTC #3 lore raw

PATCH 1/1 fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined

In reply to Suriyan Ramasami

Hi Suriyan,

Quoted 22 lines · Suriyan Ramasami

The boot commands - bootz/bootm mandate a third argument which is the address to the FDT blob. In cases where this argument is not specified, boot fails with a message indicating a missing FDT.
This causes non-FDT kernels to fail to boot. This patch allows both FDT and non-FDT kernels to boot by making the third parameter to the bootm/bootz optional.
Signed-off-by Suriyan Ramasami <[email protected]>
---
Changes in v1:
- First try
 common/image-fdt.c | 4 ++++
 1 file changed, 4 insertions(+)
common/image-fdt.c +0 -0
index a39ae1b..1a02166 100644

common/image-fdt.c int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, · line 433

Quoted 4 lines

common/image-fdt.c +1 -0

@@ -430,6 +430,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
 error:
        *of_flat_tree = NULL;
        *of_size = 0;
+       if (argc <= 2) {

if (!select)

is better I think since that holds the selected device tree. If it NULL when there is none.

common/image-fdt.c int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, · line 438

Quoted 5 lines

common/image-fdt.c +3 -0

+               debug("Continuing to boot without FDT\n");
+               return 0;
+       }
        return 1;
 }

I think everyone is happy with the approach so I'd like to merge this. But I'm not keen on the error handling. Some of the cases are genuine errors, viz.:

if ((load < image_end) && (load_end > image_start)) { fdt_error("fdt overwritten");
goto error;
}

if (fdt_check_header(fdt_blob) != 0) {
fdt_error("image is not a fdt");
goto error;
}

if (fdt_totalsize(fdt_blob) != fdt_len) {
fdt_error("fdt size != image size");
goto error;
}

So how about leaving error: alone and adding a new label above it like no_fdt:

Then you can change the other goto statements to 'goto no_fdt' which can do your check and either return, or fall through to errror:.

Regards,
Simon

Suriyan Ramasami

author

2014-11-27 21:25 UTC #4 lore raw

PATCH 1/1 fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined

Hello Simon,
Thanks for the review! and happy Thanksgiving :-)

Quoted 60 lines · Simon Glass

Hi Suriyan,
Quoted 22 lines · Suriyan Ramasami
The boot commands - bootz/bootm mandate a third argument which is the address to the FDT blob. In cases where this argument is not specified, boot fails with a message indicating a missing FDT.
This causes non-FDT kernels to fail to boot. This patch allows both FDT and non-FDT kernels to boot by making the third parameter to the bootm/bootz optional.
Signed-off-by Suriyan Ramasami <[email protected]>
---
Changes in v1:
- First try
 common/image-fdt.c | 4 ++++
 1 file changed, 4 insertions(+)
common/image-fdt.c +0 -0
index a39ae1b..1a02166 100644
common/image-fdt.c int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, · line 433
Quoted 4 lines
common/image-fdt.c +1 -0
@@ -430,6 +430,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
 error:
        *of_flat_tree = NULL;
        *of_size = 0;
+       if (argc <= 2) {
if (!select)
is better I think since that holds the selected device tree. If it NULL when there is none.
common/image-fdt.c int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, · line 438
Quoted 5 lines
common/image-fdt.c +3 -0
+               debug("Continuing to boot without FDT\n");
+               return 0;
+       }
        return 1;
 }
I think everyone is happy with the approach so I'd like to merge this. But I'm not keen on the error handling. Some of the cases are genuine errors, viz.:
if ((load < image_end) && (load_end > image_start)) { fdt_error("fdt overwritten");
goto error;
}
if (fdt_check_header(fdt_blob) != 0) {
fdt_error("image is not a fdt");
goto error;
}
if (fdt_totalsize(fdt_blob) != fdt_len) {
fdt_error("fdt size != image size");
goto error;
}

Quoted 5 lines

So how about leaving error: alone and adding a new label above it like no_fdt:
Then you can change the other goto statements to 'goto no_fdt' which can do your check and either return, or fall through to errror:.

I shall incorporate your suggestions in my next patch. Thanks
- Suriyan

Quoted 2 lines

Regards,
Simon