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@xxxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
