Re: [PATCH 1/3] of: always populate a root node

From: Frank Rowand <frowand.list@gmail.com>
To: "Clément Léger" <clement.leger@bootlin.com>
Cc: Rob Herring <robh@kernel.org>,
	Pantelis Antoniou <pantelis.antoniou@konsulko.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Allan Nielsen <allan.nielsen@microchip.com>,
	Horatiu Vultur <horatiu.vultur@microchip.com>,
	Steen Hegelund <steen.hegelund@microchip.com>,
	Thomas Petazzoni <thomas.petazonni@bootlin.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Mark Brown <broonie@kernel.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Hans de Goede <hdegoede@redhat.com>, Andrew Lunn <andrew@lunn.ch>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH 1/3] of: always populate a root node
Date: Tue, 17 May 2022 11:03:41 -0400	[thread overview]
Message-ID: <30bb7968-06fb-308c-a5f0-df316dfb240e@gmail.com> (raw)
In-Reply-To: <20220517093729.214c33a3@fixe.home>

On 5/17/22 02:37, Clément Léger wrote:
> Le Mon, 16 May 2022 23:11:03 -0400,
> Frank Rowand <frowand.list@gmail.com> a écrit :
> 
>> On 5/3/22 08:45, Rob Herring wrote:
>>> On Wed, Apr 27, 2022 at 11:45:00AM +0200, Clément Léger wrote:  
>>>> When enabling CONFIG_OF on a platform where of_root is not populated by
>>>> firmware, we end up without a root node. In order to apply overlays and
>>>> create subnodes of the root node, we need one. This commit creates an
>>>> empty root node if not present.  
>>>
>>> The existing unittest essentially does the same thing for running the 
>>> tests on non-DT systems. It should be modified to use this support 
>>> instead. Maybe that's just removing the unittest code that set of_root.
>>>
>>> I expect Frank will have some comments.  
>>
>> My preference would be for unflatten_and_copy_device_tree() to
>> use a compiled in FDT that only contains a root node, in the
>> case that no valid device tree is found (in other words,
>> "if (!initial_boot_params)".
> 
> Ok, so basically, instead of creating the root node manually, you
> expect a device-tree which contains the following to be builtin the
> kernel and unflattened if needed:
> 
> / {
> 
> };

Yes.  If you agree with this I can create a patch to implement it.  I think
it is useful even stand alone from the rest of the series.

> 
> Maybe "chosen" and "aliases" nodes should also be provided as empty
> nodes since the unittest are creating them anyway and the core DT code
> also uses them.

No. Unittest does not create both of them (I'm pretty sure, but I'm not
going to double check).  If I recall correctly, unittest adds a property
in one of those two nodes, and thus implicitly creates the node if not
already present.  Unittest does populate internal pointers to those two
nodes if the nodes are present (otherwise the pointers will have the
value of null).  There is no need for the nodes to be present if empty.

-Frank

> 
> Thanks,
> 
> Clément
> 
>>
>> unflatten_and_copy_device_tree() calls unittest_unflatten_overlay_base()
>> after unflattening the device tree passed into the booting kernel.  This
>> step is needed for a specific portion of the unittests.
>>
>> I'm still looking at the bigger picture of using overlays for the PCIe
>> card, so more comments will be coming about that bigger picture.
>>
>> -Frank
>>
> 
> 