ARM: dts: imx: Pass an empty 'chosen' node

Commit 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi") causes boot issues when the bootloader does not create a 'chosen' node if such node is not present in the dtb.

The reason for the boot failure is well explained by Javier Martinez
Canillas: "the decompressor relies on a pre-existing chosen node to be available to insert the command line and merge other ATAGS info."

, so pass an empty 'chosen' node to fix the boot problem.

This issue has been seen in the kernelci reports with Barebox as bootloader.

Fixes 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi")

Reported-by kernelci.org bot <[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---

Commit 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi") causes boot issues when the bootloader does not create a 'chosen' node if such node is not present in the dtb.

The reason for the boot failure is well explained by Javier Martinez
Canillas: "the decompressor relies on a pre-existing chosen node to be available to insert the command line and merge other ATAGS info."

, so pass an empty 'chosen' node to fix the boot problem.

This issue has been seen in the kernelci reports with Barebox as bootloader.

Fixes 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi")

Reported-by kernelci.org bot <[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

Commit 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi") causes boot issues when the bootloader does not create a 'chosen' node if such node is not present in the dtb.

The reason for the boot failure is well explained by Javier Martinez
Canillas: "the decompressor relies on a pre-existing chosen node to be available to insert the command line and merge other ATAGS info."

, so pass an empty 'chosen' node to fix the boot problem.

This issue has been seen in the kernelci reports with Barebox as bootloader.

wouldn't it be better to fix the decompressor code to eventually create the /chosen node when it doesn't exist?

Hi Uwe,

On Mon, Dec 19, 2016 at 4:41 PM, Uwe Kleine-K?nig
<[email protected]> wrote:

Quoted 2 lines

wouldn't it be better to fix the decompressor code to eventually create the /chosen node when it doesn't exist?

Thanks for the suggestion.

I don't have a Barebox setup handy, but would the following change fix it as you propose?

arch/arm/boot/compressed/atags_to_fdt.c +2 -1

@@ -66,13 +66,21 @@ static uint32_t get_cell_size(const void *fdt)
        return cell_size;
 }
 
-static void merge_fdt_bootargs(void *fdt, const char *fdt_cmdline)
+static int merge_fdt_bootargs(void *fdt, const char *fdt_cmdline)
 {
        char cmdline[COMMAND_LINE_SIZE];
        const char *fdt_bootargs;
        char *ptr = cmdline;
+       int chosen_off;
        int len = 0;
 

[adding Pali who first reported this issue]

Hello Pali,

Quoted 4 lines · Fabio Estevam

Hi Uwe,

On Mon, Dec 19, 2016 at 4:41 PM, Uwe Kleine-K?nig

Quoted 8 lines

Quoted 2 lines · <[email protected]> wrote:

wouldn't it be better to fix the decompressor code to eventually create the /chosen node when it doesn't exist?

Thanks for the suggestion.

I don't have a Barebox setup handy, but would the following change fix it as you propose?

Could you test Fabio's patch with NoLo to see if it fixes your issue?

I also think it makes more sense for Linux to create the "chosen" node if not present since this is a requirement due a Linux implementation detail.

Forcing to have a "chosen" node in dts leaks this implementation detail since the ePAPR document says that the "chosen" node should be optional.

Quoted 29 lines

arch/arm/boot/compressed/atags_to_fdt.c +9 -1

@@ -66,13 +66,21 @@ static uint32_t get_cell_size(const void *fdt)
 
        return cell_size;
 
 }
 
-static void merge_fdt_bootargs(void *fdt, const char *fdt_cmdline)
+static int merge_fdt_bootargs(void *fdt, const char *fdt_cmdline)
 
 {
 
        char cmdline[COMMAND_LINE_SIZE];
        const char *fdt_bootargs;
        char *ptr = cmdline;
 
+       int chosen_off;
 
        int len = 0;
 
+       /* find or add chosen node */
+       chosen_off = fdt_path_offset(fdt, "/chosen");
+       if (chosen_off == -FDT_ERR_NOTFOUND)
+               chosen_off = fdt_add_subnode(fdt, 0, "chosen");
+       if (chosen_off < 0)
+               return chosen_off;
+

Quoted 3 lines

        /* copy the fdt command line into the buffer */
        fdt_bootargs = getprop(fdt, "/chosen", "bootargs", &len);
        if (fdt_bootargs)

Hi Uwe,

Quoted 2 lines · Uwe Kleine-K?nig

wouldn't it be better to fix the decompressor code to eventually create the /chosen node when it doesn't exist?

Thanks for the suggestion.

I don't have a Barebox setup handy, but would the following change fix it as you propose?

arch/arm/boot/compressed/atags_to_fdt.c +1 -1

@@ -66,13 +66,21 @@ static uint32_t get_cell_size(const void *fdt)
        return cell_size;
 }
 
-static void merge_fdt_bootargs(void *fdt, const char *fdt_cmdline)
+static int merge_fdt_bootargs(void *fdt, const char *fdt_cmdline)
 {
        char cmdline[COMMAND_LINE_SIZE];
        const char *fdt_bootargs;