Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <56822a64-f289-3b7d-2175-3669194587c9@gmail.com>
Date:   Thu, 23 Jun 2022 20:01:14 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH v1 1/2] of: base: populate of_root node if not set
Content-Language: en-US
From:   Frank Rowand <frowand.list@gmail.com>
To:     =?UTF-8?B?Q2zDqW1lbnQgTMOpZ2Vy?= <clement.leger@bootlin.com>,
        Rob Herring <robh+dt@kernel.org>
Cc:     linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        Lizhi Hou <lizhi.hou@xilinx.com>,
        Allan Nielsen <allan.nielsen@microchip.com>,
        Horatiu Vultur <horatiu.vultur@microchip.com>,
        Steen Hegelund <steen.hegelund@microchip.com>,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
        Rob Herring <robh@kernel.org>
References: <20220623105044.152832-1-clement.leger@bootlin.com>
 <20220623105044.152832-2-clement.leger@bootlin.com>
 <ecf13de1-531f-b73b-b325-44ef78a3f818@gmail.com>
In-Reply-To: <ecf13de1-531f-b73b-b325-44ef78a3f818@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 6/23/22 19:15, Frank Rowand wrote:
> Hi Clement,
> 
> I said that I would send a patch to do this, but have failed to follow
> through.  Sorry about that.
> 
> 
> On 6/23/22 06:50, 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. Create this root node
>> by unflattening an empty builtin dtb with of_fdt_unflatten().
>>
>> Co-developed-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
>> ---
>>  drivers/of/Makefile       |  2 +-
>>  drivers/of/base.c         | 18 ++++++++++++++++--
>>  drivers/of/empty_root.dts |  6 ++++++
>>  3 files changed, 23 insertions(+), 3 deletions(-)
>>  create mode 100644 drivers/of/empty_root.dts
>>
>> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
>> index e0360a44306e..ce56c8b95c83 100644
>> --- a/drivers/of/Makefile
>> +++ b/drivers/of/Makefile
>> @@ -1,5 +1,5 @@
>>  # SPDX-License-Identifier: GPL-2.0
>> -obj-y = base.o device.o platform.o property.o
>> +obj-y = base.o empty_root.dtb.o device.o platform.o property.o
>>  obj-$(CONFIG_OF_KOBJ) += kobj.o
>>  obj-$(CONFIG_OF_DYNAMIC) += dynamic.o
>>  obj-$(CONFIG_OF_FLATTREE) += fdt.o
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index d4f98c8469ed..43e0f027a49c 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -22,6 +22,7 @@
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/of_device.h>
>> +#include <linux/of_fdt.h>
>>  #include <linux/of_graph.h>
>>  #include <linux/spinlock.h>
>>  #include <linux/slab.h>
>> @@ -163,6 +164,8 @@ void __of_phandle_cache_inv_entry(phandle handle)
>>  		phandle_cache[handle_hash] = NULL;
>>  }
>>  
>> +extern const char __dtb_empty_root_begin[];
>> +
>>  void __init of_core_init(void)
>>  {
>>  	struct device_node *np;
>> @@ -176,6 +179,18 @@ void __init of_core_init(void)
>>  		pr_err("failed to register existing nodes\n");
>>  		return;
>>  	}
>> +
>> +	if (!of_root) {
>> +		void *dt;
>> +		const unsigned long *fdt = (const unsigned long *)
>> +							__dtb_empty_root_begin;
>> +		dt = of_fdt_unflatten_tree(fdt, NULL, &of_root);
>> +		if (!dt) {
>> +			pr_err("Failed to setup empty root dt\n");
>> +			return;
>> +		}
>> +	}
>> +
>>  	for_each_of_allnodes(np) {
>>  		__of_attach_node_sysfs(np);
>>  		if (np->phandle && !phandle_cache[of_phandle_cache_hash(np->phandle)])
>> @@ -184,8 +199,7 @@ void __init of_core_init(void)
>>  	mutex_unlock(&of_mutex);
>>  
>>  	/* Symlink in /proc as required by userspace ABI */
>> -	if (of_root)
>> -		proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
>> +	proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
>>  }
> 
> This approach is adding an additional method of unflattening the tree.
> I would prefer to consolidate in a single location.
> 
> I have leveraged this patch series into a different patch series to
> accomplish that.  I have boot tested with one configuration, but want
> to test two more configurations before sending the new series.  It
> should only take "a few minutes".

"a few minutes" is taking longer than expected.  One of the devicetree
interrupt unittests is not giving the expected result for one of the
configurations I am checking.

-Frank

> 
> -Frank
> 
>>  
>>  static struct property *__of_find_property(const struct device_node *np,
>> diff --git a/drivers/of/empty_root.dts b/drivers/of/empty_root.dts
>> new file mode 100644
>> index 000000000000..cf9e97a60f48
>> --- /dev/null
>> +++ b/drivers/of/empty_root.dts
>> @@ -0,0 +1,6 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/dts-v1/;
>> +
>> +/ {
>> +
>> +};
>