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

From: "Clément Léger" <clement.leger@bootlin.com>
To: Frank Rowand <frowand.list@gmail.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: Wed, 18 May 2022 12:03:40 +0200	[thread overview]
Message-ID: <20220518120340.61e8e11a@fixe.home> (raw)
In-Reply-To: <30bb7968-06fb-308c-a5f0-df316dfb240e@gmail.com>

Le Tue, 17 May 2022 11:03:41 -0400,
Frank Rowand <frowand.list@gmail.com> a écrit :

> 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.

If you want to implement this, feel free to do so, I'll (at least) be
able to test it.

> 
> > 
> > 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.

Acked, makes sense.

Clément

> 
> -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
> >>  
> > 
> >   
> 

-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com