| Message ID | 1484834566-19845-1-git-send-email-fabio.estevam@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jan 19, 2017 at 12:02:46PM -0200, Fabio Estevam wrote: > 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. > > Also pass the 'memory' node in order to fix boot issues on the SolidRun > iMX6 platforms. > > Fixes: 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi") > Reported-by: kernelci.org bot <bot@kernelci.org> > Reported-by: Russell King <rmk+kernel@armlinux.org.uk> > Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com> > --- > Changes since v1: > - Also pass the memory node. > > arch/arm/boot/dts/imx1.dtsi | 2 ++ > arch/arm/boot/dts/imx23.dtsi | 2 ++ > arch/arm/boot/dts/imx25.dtsi | 2 ++ > arch/arm/boot/dts/imx27.dtsi | 2 ++ > arch/arm/boot/dts/imx28.dtsi | 2 ++ > arch/arm/boot/dts/imx31.dtsi | 2 ++ > arch/arm/boot/dts/imx35.dtsi | 2 ++ > arch/arm/boot/dts/imx50.dtsi | 2 ++ > arch/arm/boot/dts/imx51.dtsi | 2 ++ > arch/arm/boot/dts/imx53.dtsi | 2 ++ > arch/arm/boot/dts/imx6qdl.dtsi | 2 ++ > arch/arm/boot/dts/imx6sl.dtsi | 2 ++ > arch/arm/boot/dts/imx6sx.dtsi | 2 ++ > arch/arm/boot/dts/imx6ul.dtsi | 2 ++ > arch/arm/boot/dts/imx7s.dtsi | 2 ++ > 15 files changed, 30 insertions(+) > > diff --git a/arch/arm/boot/dts/imx1.dtsi b/arch/arm/boot/dts/imx1.dtsi > index dd3de38..7ef0d36 100644 > --- a/arch/arm/boot/dts/imx1.dtsi > +++ b/arch/arm/boot/dts/imx1.dtsi > @@ -18,6 +18,8 @@ > / { > #address-cells = <1>; > #size-cells = <1>; > + chosen {}; > + memory { device_type = "memory"; reg = <0 0>; }; Would it be nice to add a comment about why this was added? Something to prevent a cleanup like "remove empty nodes and invalid memory configurations". Best regards Uwe
Hi Uwe, On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > Would it be nice to add a comment about why this was added? Something to > prevent a cleanup like "remove empty nodes and invalid memory > configurations". Do you mean something like this? /* "chosen" and "memory" nodes are mandatory */ chosen {}; memory { device_type = "memory"; reg = <0 0>; };
On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote: > Hi Uwe, > > On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> wrote: > > > Would it be nice to add a comment about why this was added? Something to > > prevent a cleanup like "remove empty nodes and invalid memory > > configurations". > > Do you mean something like this? > > /* "chosen" and "memory" nodes are mandatory */ > chosen {}; > memory { device_type = "memory"; reg = <0 0>; }; Not very helpful comment. Something like: /* * The decompressor relies on a pre-existing chosen node to be * available to insert the command line and merge other ATAGS * info. */ Is it difficult to fix the decompressor? I didn't understood the breakage regarding the memory node good enough to suggest a comment for that. Best regards Uwe
On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-König wrote: > On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote: > > Hi Uwe, > > > > On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-König > > <u.kleine-koenig@pengutronix.de> wrote: > > > > > Would it be nice to add a comment about why this was added? Something to > > > prevent a cleanup like "remove empty nodes and invalid memory > > > configurations". > > > > Do you mean something like this? > > > > /* "chosen" and "memory" nodes are mandatory */ > > chosen {}; > > memory { device_type = "memory"; reg = <0 0>; }; > > Not very helpful comment. Something like: > > /* > * The decompressor relies on a pre-existing chosen node to be > * available to insert the command line and merge other ATAGS > * info. > */ > > Is it difficult to fix the decompressor? ... and that comment would be wrong. Yes, the decompressor relies on it, as do some uboot versions. > I didn't understood the breakage regarding the memory node good enough > to suggest a comment for that. A missing memory node appears to prevent some uboot versions supplying any kind of memory layout to the kernel, which then causes the kernel to crash very early during boot. Again, this is not using appended DTB - this is using a separately loaded DTB in uboot. uboot fails to update the dtb if these nodes are missing. Frankly, I think the original change (removing the skeleton.dtsi include) was misguided and needs to be reverted - the change is imho built upon an incorrect assumption that nothing in skeleton.dtsi is required. That's clearly false.
On Thu, Jan 19, 2017 at 12:56 PM, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > Frankly, I think the original change (removing the skeleton.dtsi include) > was misguided and needs to be reverted - the change is imho built upon an > incorrect assumption that nothing in skeleton.dtsi is required. That's > clearly false. Wouldn't this v2 patch restore things back to normal?
On Thu, Jan 19, 2017 at 02:56:42PM +0000, Russell King - ARM Linux wrote: > On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-König wrote: > > On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote: > > > Hi Uwe, > > > > > > On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-König > > > <u.kleine-koenig@pengutronix.de> wrote: > > > > > > > Would it be nice to add a comment about why this was added? Something to > > > > prevent a cleanup like "remove empty nodes and invalid memory > > > > configurations". > > > > > > Do you mean something like this? > > > > > > /* "chosen" and "memory" nodes are mandatory */ > > > chosen {}; > > > memory { device_type = "memory"; reg = <0 0>; }; > > > > Not very helpful comment. Something like: > > > > /* > > * The decompressor relies on a pre-existing chosen node to be > > * available to insert the command line and merge other ATAGS > > * info. > > */ > > > > Is it difficult to fix the decompressor? > > ... and that comment would be wrong. Yes, the decompressor relies on it, > as do some uboot versions. Good that we talked about this, otherwise I would have removed that once the decompressor is fixed. So the comment must be /* * The decompressor and also some versions of U-Boot rely on a * pre-existing /chosen node to be available to insert the * command line and merge other ATAGS info. * Also for U-Boot there must be a pre-existing /memory node. */ > > I didn't understood the breakage regarding the memory node good enough > > to suggest a comment for that. > > A missing memory node appears to prevent some uboot versions supplying > any kind of memory layout to the kernel, which then causes the kernel to > crash very early during boot. > > Again, this is not using appended DTB - this is using a separately loaded > DTB in uboot. uboot fails to update the dtb if these nodes are missing. > > Frankly, I think the original change (removing the skeleton.dtsi include) > was misguided and needs to be reverted - the change is imho built upon an > incorrect assumption that nothing in skeleton.dtsi is required. That's > clearly false. I tend to agree as I assume it's not only U-Boot on i.MX but on all ARM platforms. Best regards Uwe
On 01/19/17 08:14, Uwe Kleine-König wrote: > On Thu, Jan 19, 2017 at 02:56:42PM +0000, Russell King - ARM Linux wrote: >> On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-König wrote: >>> On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote: >>>> Hi Uwe, >>>> >>>> On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-König >>>> <u.kleine-koenig@pengutronix.de> wrote: >>>> >>>>> Would it be nice to add a comment about why this was added? Something to >>>>> prevent a cleanup like "remove empty nodes and invalid memory >>>>> configurations". >>>> >>>> Do you mean something like this? >>>> >>>> /* "chosen" and "memory" nodes are mandatory */ >>>> chosen {}; >>>> memory { device_type = "memory"; reg = <0 0>; }; >>> >>> Not very helpful comment. Something like: >>> >>> /* >>> * The decompressor relies on a pre-existing chosen node to be >>> * available to insert the command line and merge other ATAGS >>> * info. >>> */ >>> >>> Is it difficult to fix the decompressor? >> >> ... and that comment would be wrong. Yes, the decompressor relies on it, >> as do some uboot versions. > > Good that we talked about this, otherwise I would have removed that once > the decompressor is fixed. So the comment must be > > /* > * The decompressor and also some versions of U-Boot rely on a > * pre-existing /chosen node to be available to insert the > * command line and merge other ATAGS info. > * Also for U-Boot there must be a pre-existing /memory node. > */ > >>> I didn't understood the breakage regarding the memory node good enough >>> to suggest a comment for that. >> >> A missing memory node appears to prevent some uboot versions supplying >> any kind of memory layout to the kernel, which then causes the kernel to >> crash very early during boot. >> >> Again, this is not using appended DTB - this is using a separately loaded >> DTB in uboot. uboot fails to update the dtb if these nodes are missing. >> >> Frankly, I think the original change (removing the skeleton.dtsi include) >> was misguided and needs to be reverted - the change is imho built upon an >> incorrect assumption that nothing in skeleton.dtsi is required. That's >> clearly false. > > I tend to agree as I assume it's not only U-Boot on i.MX but on all ARM > platforms. > > Best regards > Uwe > Adding back the chosen nodes is a bandaid that papers over the actual bug in the decompressor. A comment about Fabio's attempt to fix the decompressor noted a possible issue with the method taken to fix the problem, but then the discussion of fixing the decompressor was dropped and v3 of the patch to add chosen into a bunch of .dtsi files was applied by Shawn. Can someone please help Fabio to create a correct patch to fix the decompressor? Note that the ePAPR v1.1 does require at least one memory node in section 3.1, so the portion of v3 of the patch that adds a memory node is valid in terms of following the standard, not just a U-boot requirement. Thanks, Frank
On Wed, Jan 25, 2017 at 3:13 PM, Frank Rowand <frowand.list@gmail.com> wrote: > On 01/19/17 08:14, Uwe Kleine-König wrote: >> On Thu, Jan 19, 2017 at 02:56:42PM +0000, Russell King - ARM Linux wrote: >>> On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-König wrote: >>>> On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote: >>>>> Hi Uwe, >>>>> >>>>> On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-König >>>>> <u.kleine-koenig@pengutronix.de> wrote: >>>>> >>>>>> Would it be nice to add a comment about why this was added? Something to >>>>>> prevent a cleanup like "remove empty nodes and invalid memory >>>>>> configurations". >>>>> >>>>> Do you mean something like this? >>>>> >>>>> /* "chosen" and "memory" nodes are mandatory */ >>>>> chosen {}; >>>>> memory { device_type = "memory"; reg = <0 0>; }; >>>> >>>> Not very helpful comment. Something like: >>>> >>>> /* >>>> * The decompressor relies on a pre-existing chosen node to be >>>> * available to insert the command line and merge other ATAGS >>>> * info. >>>> */ >>>> >>>> Is it difficult to fix the decompressor? >>> >>> ... and that comment would be wrong. Yes, the decompressor relies on it, >>> as do some uboot versions. >> >> Good that we talked about this, otherwise I would have removed that once >> the decompressor is fixed. So the comment must be >> >> /* >> * The decompressor and also some versions of U-Boot rely on a >> * pre-existing /chosen node to be available to insert the >> * command line and merge other ATAGS info. >> * Also for U-Boot there must be a pre-existing /memory node. >> */ >> >>>> I didn't understood the breakage regarding the memory node good enough >>>> to suggest a comment for that. >>> >>> A missing memory node appears to prevent some uboot versions supplying >>> any kind of memory layout to the kernel, which then causes the kernel to >>> crash very early during boot. >>> >>> Again, this is not using appended DTB - this is using a separately loaded >>> DTB in uboot. uboot fails to update the dtb if these nodes are missing. >>> >>> Frankly, I think the original change (removing the skeleton.dtsi include) >>> was misguided and needs to be reverted - the change is imho built upon an >>> incorrect assumption that nothing in skeleton.dtsi is required. That's >>> clearly false. >> >> I tend to agree as I assume it's not only U-Boot on i.MX but on all ARM >> platforms. >> >> Best regards >> Uwe >> > > Adding back the chosen nodes is a bandaid that papers over the actual bug > in the decompressor. > > A comment about Fabio's attempt to fix the decompressor noted a possible > issue with the method taken to fix the problem, but then the discussion > of fixing the decompressor was dropped and v3 of the patch to add chosen > into a bunch of .dtsi files was applied by Shawn. > > Can someone please help Fabio to create a correct patch to fix the > decompressor? > > Note that the ePAPR v1.1 does require at least one memory node in We have a DT spec now, let's stop pointing people to ePAPR. > section 3.1, so the portion of v3 of the patch that adds a memory > node is valid in terms of following the standard, not just a U-boot > requirement. I'd have no issue making chosen be required. Any platform with a serial console should be setting stdout-path anyway. Just add it to the spec. :) We should also add a dtc check as well if we do that. Rob
On 01/25/17 14:46, Rob Herring wrote: > On Wed, Jan 25, 2017 at 3:13 PM, Frank Rowand <frowand.list@gmail.com> wrote: >> On 01/19/17 08:14, Uwe Kleine-König wrote: >>> On Thu, Jan 19, 2017 at 02:56:42PM +0000, Russell King - ARM Linux wrote: >>>> On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-König wrote: >>>>> On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote: >>>>>> Hi Uwe, >>>>>> >>>>>> On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-König >>>>>> <u.kleine-koenig@pengutronix.de> wrote: >>>>>> >>>>>>> Would it be nice to add a comment about why this was added? Something to >>>>>>> prevent a cleanup like "remove empty nodes and invalid memory >>>>>>> configurations". >>>>>> >>>>>> Do you mean something like this? >>>>>> >>>>>> /* "chosen" and "memory" nodes are mandatory */ >>>>>> chosen {}; >>>>>> memory { device_type = "memory"; reg = <0 0>; }; >>>>> >>>>> Not very helpful comment. Something like: >>>>> >>>>> /* >>>>> * The decompressor relies on a pre-existing chosen node to be >>>>> * available to insert the command line and merge other ATAGS >>>>> * info. >>>>> */ >>>>> >>>>> Is it difficult to fix the decompressor? >>>> >>>> ... and that comment would be wrong. Yes, the decompressor relies on it, >>>> as do some uboot versions. >>> >>> Good that we talked about this, otherwise I would have removed that once >>> the decompressor is fixed. So the comment must be >>> >>> /* >>> * The decompressor and also some versions of U-Boot rely on a >>> * pre-existing /chosen node to be available to insert the >>> * command line and merge other ATAGS info. >>> * Also for U-Boot there must be a pre-existing /memory node. >>> */ >>> >>>>> I didn't understood the breakage regarding the memory node good enough >>>>> to suggest a comment for that. >>>> >>>> A missing memory node appears to prevent some uboot versions supplying >>>> any kind of memory layout to the kernel, which then causes the kernel to >>>> crash very early during boot. >>>> >>>> Again, this is not using appended DTB - this is using a separately loaded >>>> DTB in uboot. uboot fails to update the dtb if these nodes are missing. >>>> >>>> Frankly, I think the original change (removing the skeleton.dtsi include) >>>> was misguided and needs to be reverted - the change is imho built upon an >>>> incorrect assumption that nothing in skeleton.dtsi is required. That's >>>> clearly false. >>> >>> I tend to agree as I assume it's not only U-Boot on i.MX but on all ARM >>> platforms. >>> >>> Best regards >>> Uwe >>> >> >> Adding back the chosen nodes is a bandaid that papers over the actual bug >> in the decompressor. >> >> A comment about Fabio's attempt to fix the decompressor noted a possible >> issue with the method taken to fix the problem, but then the discussion >> of fixing the decompressor was dropped and v3 of the patch to add chosen >> into a bunch of .dtsi files was applied by Shawn. >> >> Can someone please help Fabio to create a correct patch to fix the >> decompressor? >> >> Note that the ePAPR v1.1 does require at least one memory node in > > We have a DT spec now, let's stop pointing people to ePAPR. > >> section 3.1, so the portion of v3 of the patch that adds a memory >> node is valid in terms of following the standard, not just a U-boot >> requirement. > > I'd have no issue making chosen be required. Any platform with a > serial console should be setting stdout-path anyway. Just add it to > the spec. :) We should also add a dtc check as well if we do that. > > Rob > Sounds good to me. -Frank
On Wed, Jan 25, 2017 at 01:13:23PM -0800, Frank Rowand wrote: > Adding back the chosen nodes is a bandaid that papers over the actual bug > in the decompressor. > > A comment about Fabio's attempt to fix the decompressor noted a possible > issue with the method taken to fix the problem, but then the discussion > of fixing the decompressor was dropped and v3 of the patch to add chosen > into a bunch of .dtsi files was applied by Shawn. > > Can someone please help Fabio to create a correct patch to fix the > decompressor? I refuse to assist, see below. The reason this has come up is that this problem has _nothing_ to do with the decompressor - see recent context on the linux-arm-kernel list. The removal of skeleton.dtsi from iMX6 DTS files has caused a regression in the current -rc kernels when an iMX6 board is booted with uboot using a _separated_ device tree file. In that situation, the decompressor has _nothing_ at all to do with modifying the DT binary file. The DT binary file ends up being passed to the kernel from the boot loader unmodified by the decompressor. Remember that the decompressor's modification of the *appended* DT file is a band-aid to allow an *ATAG* based system to continue working. That doesn't apply to iMX6. The lack of the memory node means that uboot fails to add a memory node to the DT binary file before passing control to the decompressor, which in turn causes the kernel to be booted with a DT binary file which says that the system has no memory. That causes the kernel to crash in the very early stages of boot, before the console is up and running. So it's a nasty silent crash. So, while it may be desirable to fix the decompressor, it's completely pointless as far as whether Fabio's patch gets merged, because this _regression_ has nothing to do with the decompressor. We need this regression fixed before 4.10 comes out so systems don't break when 4.10 comes out.
On Mon, Jan 30, 2017 at 4:54 PM, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > We need this regression fixed before 4.10 comes out so systems don't > break when 4.10 comes out. It will be fixed in the next rc with the following commit: https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9
Hello, On Mon, Jan 30, 2017 at 06:54:44PM +0000, Russell King - ARM Linux wrote: > On Wed, Jan 25, 2017 at 01:13:23PM -0800, Frank Rowand wrote: > > Adding back the chosen nodes is a bandaid that papers over the actual bug > > in the decompressor. > > > > A comment about Fabio's attempt to fix the decompressor noted a possible > > issue with the method taken to fix the problem, but then the discussion > > of fixing the decompressor was dropped and v3 of the patch to add chosen > > into a bunch of .dtsi files was applied by Shawn. > > > > Can someone please help Fabio to create a correct patch to fix the > > decompressor? > > I refuse to assist, see below. > > The reason this has come up is that this problem has _nothing_ to do > with the decompressor - see recent context on the linux-arm-kernel > list. Several people reported problems with the patch that removes skeleton.dtsi. There are at least: - Russell, who saw U-Boot failing to specify the amount of RAM; - somebody (I forgot and I'm lazy so I didn't look up the name) who noticed some i.MX6 boards fail in kernelci with barebox because barebox assumed /chosen to be present; and - somebody else (...) who reported the ATAGS_DTB_COMPAT failure (also wrongly called "decompressor failure" in this thread). I agree that for each single of the above issues it is worth to readd the /chosen and /memory nodes for now. Still all three are IMHO worth to be fixed. barebox is already fixed in v2017.01.0 (commit f49b415b9231 (of: base: add chosen node if it does not exist when adding initrd), https://git.pengutronix.de/cgit/barebox/commit/?id=f49b415b9231). Looking at the ATAGS_DTB_COMPAT problem: The code in question is (if I understood correctly) merge_fdt_bootargs in arch/arm/boot/compressed/atags_to_fdt.c: setprop_string(fdt, "/chosen", "bootargs", cmdline); looking further: static int node_offset(void *fdt, const char *node_path) { int offset = fdt_path_offset(fdt, node_path); if (offset == -FDT_ERR_NOTFOUND) offset = fdt_add_subnode(fdt, 0, node_path); return offset; } static int setprop_string(void *fdt, const char *node_path, const char *property, const char *string) { int offset = node_offset(fdt, node_path); if (offset < 0) return offset; return fdt_setprop_string(fdt, offset, property, string); } So it seems "/chosen" not being there should be handled. I don't have a reproducer for this problem, but it doesn't look that hard to fix with DEBUG_LL and a system that shows the problem. Best regards Uwe
Hi, > On Mon, Jan 30, 2017 at 4:54 PM, Russell King - ARM Linux > <linux@xxxxxxxxxxxxxxx> wrote: > > >/We need this regression fixed before 4.10 comes out so systems don't/ > >/break when 4.10 comes out./ > > It will be fixed in the next rc with the following commit: > https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9 <https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9> i want to report that the combination i.MX28 + imx-bootlets (requires CONFIG_ARM_APPENDED_DTB since bootlets can't handle DTBs) was also affected before and fixed by this patch. Btw are Arnd and Olof informed about this urgent patch?
Am 01.02.2017 um 14:30 schrieb Stefan Wahren: > Hi, > >> On Mon, Jan 30, 2017 at 4:54 PM, Russell King - ARM Linux >> <linux@xxxxxxxxxxxxxxx> wrote: >> >>> /We need this regression fixed before 4.10 comes out so systems don't/ >>> /break when 4.10 comes out./ >> It will be fixed in the next rc with the following commit: >> > https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9 > <https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9> > > > i want to report that the combination i.MX28 + imx-bootlets (requires > CONFIG_ARM_APPENDED_DTB since bootlets can't handle DTBs) was also > affected before and fixed by this patch. > > Btw are Arnd and Olof informed about this urgent patch? @Shawn Okay, 4.10-rc7 is out but this fix isn't applied :-( Is there a chance for 4.10 or do we have to wait for 4.10.1? Maybe the subject of the patch is to harmless.
Hi Stefan, On Mon, Feb 6, 2017 at 8:49 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >> Btw are Arnd and Olof informed about this urgent patch? > > @Shawn > > Okay, 4.10-rc7 is out but this fix isn't applied :-( > > Is there a chance for 4.10 or do we have to wait for 4.10.1? > > Maybe the subject of the patch is to harmless. The patch is in the arm-soc 'fixes' branch: https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/?h=fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9 Olof/Arnd, Will this branch be sent to Linus so that it lands at 4.10 final? Thanks
On Monday, February 6, 2017 9:05:26 AM CET Fabio Estevam wrote: > > Maybe the subject of the patch is to harmless. > > The patch is in the arm-soc 'fixes' branch: > https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/?h=fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9 > > Olof/Arnd, > > Will this branch be sent to Linus so that it lands at 4.10 final? > Yes, that is the plan. Arnd
diff --git a/arch/arm/boot/dts/imx1.dtsi b/arch/arm/boot/dts/imx1.dtsi index dd3de38..7ef0d36 100644 --- a/arch/arm/boot/dts/imx1.dtsi +++ b/arch/arm/boot/dts/imx1.dtsi @@ -18,6 +18,8 @@ / { #address-cells = <1>; #size-cells = <1>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { gpio0 = &gpio1; diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi index 96eae64..7bb662c 100644 --- a/arch/arm/boot/dts/imx23.dtsi +++ b/arch/arm/boot/dts/imx23.dtsi @@ -16,6 +16,8 @@ #size-cells = <1>; interrupt-parent = <&icoll>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { gpio0 = &gpio0; diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi index 213d86e..f328ecc 100644 --- a/arch/arm/boot/dts/imx25.dtsi +++ b/arch/arm/boot/dts/imx25.dtsi @@ -14,6 +14,8 @@ / { #address-cells = <1>; #size-cells = <1>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { ethernet0 = &fec; diff --git a/arch/arm/boot/dts/imx27.dtsi b/arch/arm/boot/dts/imx27.dtsi index 6a7cb9e..3271ec49 100644 --- a/arch/arm/boot/dts/imx27.dtsi +++ b/arch/arm/boot/dts/imx27.dtsi @@ -19,6 +19,8 @@ / { #address-cells = <1>; #size-cells = <1>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { ethernet0 = &fec; diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi index 905bdb5..5f47558 100644 --- a/arch/arm/boot/dts/imx28.dtsi +++ b/arch/arm/boot/dts/imx28.dtsi @@ -17,6 +17,8 @@ #size-cells = <1>; interrupt-parent = <&icoll>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { ethernet0 = &mac0; diff --git a/arch/arm/boot/dts/imx31.dtsi b/arch/arm/boot/dts/imx31.dtsi index c0a5d5f..afd4106 100644 --- a/arch/arm/boot/dts/imx31.dtsi +++ b/arch/arm/boot/dts/imx31.dtsi @@ -12,6 +12,8 @@ / { #address-cells = <1>; #size-cells = <1>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { serial0 = &uart1; diff --git a/arch/arm/boot/dts/imx35.dtsi b/arch/arm/boot/dts/imx35.dtsi index 6f7b943..e08e574 100644 --- a/arch/arm/boot/dts/imx35.dtsi +++ b/arch/arm/boot/dts/imx35.dtsi @@ -13,6 +13,8 @@ / { #address-cells = <1>; #size-cells = <1>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { ethernet0 = &fec; diff --git a/arch/arm/boot/dts/imx50.dtsi b/arch/arm/boot/dts/imx50.dtsi index fe0221e..46329ff 100644 --- a/arch/arm/boot/dts/imx50.dtsi +++ b/arch/arm/boot/dts/imx50.dtsi @@ -17,6 +17,8 @@ / { #address-cells = <1>; #size-cells = <1>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { ethernet0 = &fec; diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi index 33526ca..c6ffc7b 100644 --- a/arch/arm/boot/dts/imx51.dtsi +++ b/arch/arm/boot/dts/imx51.dtsi @@ -19,6 +19,8 @@ / { #address-cells = <1>; #size-cells = <1>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { ethernet0 = &fec; diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi index ca51dc0..2d56f26 100644 --- a/arch/arm/boot/dts/imx53.dtsi +++ b/arch/arm/boot/dts/imx53.dtsi @@ -19,6 +19,8 @@ / { #address-cells = <1>; #size-cells = <1>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { ethernet0 = &fec; diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi index 89b834f..c88a373 100644 --- a/arch/arm/boot/dts/imx6qdl.dtsi +++ b/arch/arm/boot/dts/imx6qdl.dtsi @@ -16,6 +16,8 @@ / { #address-cells = <1>; #size-cells = <1>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { ethernet0 = &fec; diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi index 19cbd87..89e8315 100644 --- a/arch/arm/boot/dts/imx6sl.dtsi +++ b/arch/arm/boot/dts/imx6sl.dtsi @@ -14,6 +14,8 @@ / { #address-cells = <1>; #size-cells = <1>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { ethernet0 = &fec; diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi index 10f3330..f52d017 100644 --- a/arch/arm/boot/dts/imx6sx.dtsi +++ b/arch/arm/boot/dts/imx6sx.dtsi @@ -15,6 +15,8 @@ / { #address-cells = <1>; #size-cells = <1>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { can0 = &flexcan1; diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi index 0f69a3f..1d93601 100644 --- a/arch/arm/boot/dts/imx6ul.dtsi +++ b/arch/arm/boot/dts/imx6ul.dtsi @@ -15,6 +15,8 @@ / { #address-cells = <1>; #size-cells = <1>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { ethernet0 = &fec1; diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi index 8db1eb9..1757789 100644 --- a/arch/arm/boot/dts/imx7s.dtsi +++ b/arch/arm/boot/dts/imx7s.dtsi @@ -50,6 +50,8 @@ / { #address-cells = <1>; #size-cells = <1>; + chosen {}; + memory { device_type = "memory"; reg = <0 0>; }; aliases { gpio0 = &gpio1;
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. Also pass the 'memory' node in order to fix boot issues on the SolidRun iMX6 platforms. Fixes: 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi") Reported-by: kernelci.org bot <bot@kernelci.org> Reported-by: Russell King <rmk+kernel@armlinux.org.uk> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com> --- Changes since v1: - Also pass the memory node. arch/arm/boot/dts/imx1.dtsi | 2 ++ arch/arm/boot/dts/imx23.dtsi | 2 ++ arch/arm/boot/dts/imx25.dtsi | 2 ++ arch/arm/boot/dts/imx27.dtsi | 2 ++ arch/arm/boot/dts/imx28.dtsi | 2 ++ arch/arm/boot/dts/imx31.dtsi | 2 ++ arch/arm/boot/dts/imx35.dtsi | 2 ++ arch/arm/boot/dts/imx50.dtsi | 2 ++ arch/arm/boot/dts/imx51.dtsi | 2 ++ arch/arm/boot/dts/imx53.dtsi | 2 ++ arch/arm/boot/dts/imx6qdl.dtsi | 2 ++ arch/arm/boot/dts/imx6sl.dtsi | 2 ++ arch/arm/boot/dts/imx6sx.dtsi | 2 ++ arch/arm/boot/dts/imx6ul.dtsi | 2 ++ arch/arm/boot/dts/imx7s.dtsi | 2 ++ 15 files changed, 30 insertions(+)