[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Rob Herring]

On Wed, Jan 25, 2017 at 3:13 PM, Frank Rowand <frowand.list at 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 at 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