[PATCH] drivers/of: add option to load a default Device Tree

[PATCH] drivers/of: add option to load a default Device Tree

[Franck Jullien]

Even if a platform doesn't use a device tree during its
boot process it can be useful to enable CONFIG_OF and get
an empty device tree.

Then, devices can use device tree overlays to populate this
default tree.

Signed-off-by: Franck Jullien <franck.jullien-EduPiq9onwdphFt5fpzH3laPQRlvutdw@public.gmane.org>
---
 drivers/of/Kconfig     |  9 +++++++++
 drivers/of/Makefile    |  3 +++
 drivers/of/base.c      | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/of/default.dts |  4 ++++
 drivers/of/unittest.c  | 33 +++++----------------------------
 include/linux/of.h     |  2 ++
 6 files changed, 73 insertions(+), 28 deletions(-)
 create mode 100644 drivers/of/default.dts

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index b3bec3a..ed60af7 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -11,6 +11,15 @@ menuconfig OF
 
 if OF
 
+config OF_DEFAULT_DTB
+	bool "Load default empty Device Tree"
+	depends on OF_IRQ
+	select OF_EARLY_FLATTREE
+	select OF_RESOLVE
+	help
+	  This option builds in an empty device tree.
+	  If unsure, say N here, but this option is safe to enable.
+
 config OF_UNITTEST
 	bool "Device Tree runtime unit tests"
 	depends on OF_IRQ
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index d7efd9d..e611dc0 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -15,4 +15,7 @@ obj-$(CONFIG_OF_RESOLVE)  += resolver.o
 obj-$(CONFIG_OF_OVERLAY) += overlay.o
 obj-$(CONFIG_OF_NUMA) += of_numa.o
 
+obj-y += default.dtb.o
+targets += default.dtb default.dtb.S
+
 obj-$(CONFIG_OF_UNITTEST) += unittest-data/
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ebf84e3..43ebf5f 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_graph.h>
+#include <linux/of_fdt.h>
 #include <linux/spinlock.h>
 #include <linux/slab.h>
 #include <linux/string.h>
@@ -189,10 +190,55 @@ int __of_attach_node_sysfs(struct device_node *np)
 	return 0;
 }
 
+int of_get_builtin_dtb(uint8_t *dtb_begin, uint8_t *dtb_end, struct device_node **dtb_node)
+{
+	const int size = dtb_end - dtb_begin;
+	void *dtb;
+	struct device_node *node;
+	int rc;
+
+	if (!size) {
+		pr_warn("%s: No DTB to attach\n", __func__);
+		return -ENODATA;
+	}
+
+	dtb = kmemdup(dtb_begin, size, GFP_KERNEL);
+
+	if (!dtb_begin) {
+		pr_warn("%s: Failed to allocate memory for built-in dtb\n", __func__);
+		return -ENOMEM;
+	}
+
+	of_fdt_unflatten_tree(dtb, NULL, &node);
+	if (!node) {
+		pr_warn("%s: No tree to attach\n", __func__);
+		return -ENODATA;
+	}
+
+	of_node_set_flag(node, OF_DETACHED);
+	rc = of_resolve_phandles(node);
+	if (rc) {
+		pr_err("%s: Failed to resolve phandles (rc=%i)\n", __func__, rc);
+		return -EINVAL;
+	}
+
+	*dtb_node = node;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_builtin_dtb);
+
 void __init of_core_init(void)
 {
 	struct device_node *np;
 
+	/*
+	 * __dtb_default_begin[] and __dtb_default_end[] are magically
+	 * created by cmd_dt_S_dtb in scripts/Makefile.lib
+	 */
+	extern uint8_t __dtb_default_begin[];
+	extern uint8_t __dtb_default_end[];
+
 	/* Create the kset, and register existing nodes */
 	mutex_lock(&of_mutex);
 	of_kset = kset_create_and_add("devicetree", NULL, firmware_kobj);
@@ -201,6 +247,10 @@ void __init of_core_init(void)
 		pr_err("devicetree: failed to register existing nodes\n");
 		return;
 	}
+
+	if (!of_root && IS_ENABLED(CONFIG_OF_DEFAULT_DTB))
+		of_get_builtin_dtb(__dtb_default_begin, __dtb_default_end, &of_root);
+
 	for_each_of_allnodes(np)
 		__of_attach_node_sysfs(np);
 	mutex_unlock(&of_mutex);
diff --git a/drivers/of/default.dts b/drivers/of/default.dts
new file mode 100644
index 0000000..fc387fb
--- /dev/null
+++ b/drivers/of/default.dts
@@ -0,0 +1,4 @@
+/dts-v1/;
+/ {
+
+};
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index f34ed93..2400e78 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -895,42 +895,19 @@ static int attach_node_and_children(struct device_node *np)
  */
 static int __init unittest_data_add(void)
 {
-	void *unittest_data;
 	struct device_node *unittest_data_node, *np;
+	int rc;
+
 	/*
 	 * __dtb_testcases_begin[] and __dtb_testcases_end[] are magically
 	 * created by cmd_dt_S_dtb in scripts/Makefile.lib
 	 */
 	extern uint8_t __dtb_testcases_begin[];
 	extern uint8_t __dtb_testcases_end[];
-	const int size = __dtb_testcases_end - __dtb_testcases_begin;
-	int rc;
-
-	if (!size) {
-		pr_warn("%s: No testcase data to attach; not running tests\n",
-			__func__);
-		return -ENODATA;
-	}
-
-	/* creating copy */
-	unittest_data = kmemdup(__dtb_testcases_begin, size, GFP_KERNEL);
 
-	if (!unittest_data) {
-		pr_warn("%s: Failed to allocate memory for unittest_data; "
-			"not running tests\n", __func__);
-		return -ENOMEM;
-	}
-	of_fdt_unflatten_tree(unittest_data, NULL, &unittest_data_node);
-	if (!unittest_data_node) {
-		pr_warn("%s: No tree to attach; not running tests\n", __func__);
-		return -ENODATA;
-	}
-	of_node_set_flag(unittest_data_node, OF_DETACHED);
-	rc = of_resolve_phandles(unittest_data_node);
-	if (rc) {
-		pr_err("%s: Failed to resolve phandles (rc=%i)\n", __func__, rc);
-		return -EINVAL;
-	}
+	rc = of_get_builtin_dtb(__dtb_testcases_begin, __dtb_testcases_end, &unittest_data_node);
+	if (rc)
+		return rc;
 
 	if (!of_root) {
 		of_root = unittest_data_node;
diff --git a/include/linux/of.h b/include/linux/of.h
index 74eb28c..5cab2b5 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -406,6 +406,8 @@ const char *of_prop_next_string(struct property *prop, const char *cur);
 
 bool of_console_check(struct device_node *dn, char *name, int index);
 
+int of_get_builtin_dtb(uint8_t *dtb_begin, uint8_t *dtb_end, struct device_node **dtb_node);
+
 #else /* CONFIG_OF */
 
 static inline void of_core_init(void)
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/of: add option to load a default Device Tree

[Frank Rowand]

On 06/22/16 08:05, Franck Jullien wrote:
> Even if a platform doesn't use a device tree during its
> boot process it can be useful to enable CONFIG_OF and get
> an empty device tree.
> 
> Then, devices can use device tree overlays to populate this
> default tree.


Hi Franck,

Can you elaborate on the problem that you are trying to
solve and the use cases that you are envisioning?

Thanks,

Frank

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/of: add option to load a default Device Tree

[Franck Jullien]

Le 23/06/2016 à 20:27, Frank Rowand a écrit :
> On 06/22/16 08:05, Franck Jullien wrote:
>> Even if a platform doesn't use a device tree during its
>> boot process it can be useful to enable CONFIG_OF and get
>> an empty device tree.
>>
>> Then, devices can use device tree overlays to populate this
>> default tree.
> 
> 
> Hi Franck,
> 
> Can you elaborate on the problem that you are trying to
> solve and the use cases that you are envisioning?
> 
> Thanks,
> 
> Frank
> 

Hi,

The idea behind this patch is to describe devices that are in a FPGA
which is itself connected to the platform through PCIe. The platform
might not use a devicetree for its own configuration. That's why we need
this patch.

There is an ongoing  discussion about this subject [1].

I have a working setup (on a x86_64) using this principle. I have
created a pci resources proxy module which is parent of all devices on
the FPGA. This is an example of binding:

	proxy: pci1337_0001 {
		#address-cells = <1>;
		#size-cells = <1>;
		#interrupt-cells = <1>;
		vendor-id = <0x00001337>;
		device-id = <0x00000001>;
		compatible = "pci1337,0001",
			     "generic,pci-proxy";
		interrupt-controller;
		/* Ranges will be overwritten during probe */
		ranges = <0 0 0xA0000000 0x100000     /* BAR 0 */
			  1 0 0xB0000000 0x100000     /* BAR 1 */
			  2 0 0xC0000000 0x100000>;   /* BAR 2 */
	};

Ranges are dynamically updated on probe with regards to FPGA bars.
This node is attached to the tree (an empty tree in my case) from the
client module.

Then FPGA devices nodes are added using overlay. This is an example:

/dts-v1/;
/ {
	#address-cells = <1>;
	#size-cells = <1>;

	fragment@0 {
		target-path = "/pci1337_0001";
		__overlay__ {

			#address-cells = <2>;
			#size-cells = <1>;
			interrupt-parent = <&proxy>;

			osc: oscillator {
				compatible = "fixed-clock";
				#clock-cells = <1>;
				clock-frequency  = <148500000>;
				clock-output-names = "osc";
			};
			enet0: ethoc {
				compatible = "opencores,ethoc";
				reg = <1 0x50000 0x100>;
				interrupts = <0>;
			};
			gpio0: gpio {
				#gpio-cells = <2>;
				compatible = "xlnx,xps-gpio-1.00.a";
				reg = <0 0x30000 0x10000>;
				gpio-controller;
			};
		};
	};


};

Interrupts property are dynamically modified before the overlay is
applied. By the way, I had to do a little hack here in the pci-proxy.
I did not declare a new interrupt domain. What I did is to get the PCI
interrupt domain (IO-APIC on my platform):

irq_data = irq_get_irq_data(priv->pci_dev->irq);
domain_parent = irq_data->domain;

and then set fwnode of IO-APIC to pci-proxy fwnode so
irq_create_of_mapping get IO-APIC's domain:

domain_parent->fwnode = &np->fwnode;

There might be a better way to make things work.

Beside this patch I needed to export of_attach_node and of_detach_node.
This will be another patch.

Thank you for your interest.

Franck.

[1] http://www.spinics.net/lists/dmaengine/msg09869.html

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/of: add option to load a default Device Tree

[Jose Abreu]

Hi Franck,

++ Carlos

On 24-06-2016 09:15, Franck Jullien wrote:
> Le 23/06/2016 à 20:27, Frank Rowand a écrit :
>> On 06/22/16 08:05, Franck Jullien wrote:
>>> Even if a platform doesn't use a device tree during its
>>> boot process it can be useful to enable CONFIG_OF and get
>>> an empty device tree.
>>>
>>> Then, devices can use device tree overlays to populate this
>>> default tree.
>>
>> Hi Franck,
>>
>> Can you elaborate on the problem that you are trying to
>> solve and the use cases that you are envisioning?
>>
>> Thanks,
>>
>> Frank
>>
> Hi,
>
> The idea behind this patch is to describe devices that are in a FPGA
> which is itself connected to the platform through PCIe. The platform
> might not use a devicetree for its own configuration. That's why we need
> this patch.

This sure looks interesting to us because we are using the
exactly same setup (x86_64 + FPGA through PCIe). As I said before
in the DMA mailing list our approach was different: we use
platform data in each driver and we register them using a PCI
driver which passes all the required parameters. Still, we were
wondering if your approach can save us development time in the
future. So, I have a few questions that I was hoping you could
answer.

>
> There is an ongoing  discussion about this subject [1].
>
> I have a working setup (on a x86_64) using this principle. I have
> created a pci resources proxy module which is parent of all devices on
> the FPGA. This is an example of binding:
>
> 	proxy: pci1337_0001 {
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		#interrupt-cells = <1>;
> 		vendor-id = <0x00001337>;
> 		device-id = <0x00000001>;
> 		compatible = "pci1337,0001",
> 			     "generic,pci-proxy";
> 		interrupt-controller;
> 		/* Ranges will be overwritten during probe */
> 		ranges = <0 0 0xA0000000 0x100000     /* BAR 0 */
> 			  1 0 0xB0000000 0x100000     /* BAR 1 */
> 			  2 0 0xC0000000 0x100000>;   /* BAR 2 */
> 	};
>
> Ranges are dynamically updated on probe with regards to FPGA bars.
> This node is attached to the tree (an empty tree in my case) from the
> client module.

I am not fully understanding where does this binding is added.
You use your patches to create an empty device tree or a device
tree where the FPGA is declared?

>
> Then FPGA devices nodes are added using overlay. This is an example:
>
> /dts-v1/;
> / {
> 	#address-cells = <1>;
> 	#size-cells = <1>;
>
> 	fragment@0 {
> 		target-path = "/pci1337_0001";
> 		__overlay__ {
>
> 			#address-cells = <2>;
> 			#size-cells = <1>;
> 			interrupt-parent = <&proxy>;
>
> 			osc: oscillator {
> 				compatible = "fixed-clock";
> 				#clock-cells = <1>;
> 				clock-frequency  = <148500000>;
> 				clock-output-names = "osc";
> 			};
> 			enet0: ethoc {
> 				compatible = "opencores,ethoc";
> 				reg = <1 0x50000 0x100>;
> 				interrupts = <0>;
> 			};
> 			gpio0: gpio {
> 				#gpio-cells = <2>;
> 				compatible = "xlnx,xps-gpio-1.00.a";
> 				reg = <0 0x30000 0x10000>;
> 				gpio-controller;
> 			};
> 		};
> 	};
>
>
> };
>
> Interrupts property are dynamically modified before the overlay is
> applied. By the way, I had to do a little hack here in the pci-proxy.
> I did not declare a new interrupt domain. What I did is to get the PCI
> interrupt domain (IO-APIC on my platform):
>
> irq_data = irq_get_irq_data(priv->pci_dev->irq);
> domain_parent = irq_data->domain;
>
> and then set fwnode of IO-APIC to pci-proxy fwnode so
> irq_create_of_mapping get IO-APIC's domain:
>
> domain_parent->fwnode = &np->fwnode;

In my case we have an interrupt controller driver which is
embedded into the PCI driver so I also can't use the generic PCI
proxy without modifying it. Can this controller be declared in
the overlay device tree and then make the remaining drivers use
the domain created?

>
> There might be a better way to make things work.
>
> Beside this patch I needed to export of_attach_node and of_detach_node.
> This will be another patch.

There is still one more thing: By default CONFIG_OF is not
enabled in x86_64 so you need to enable this, right? Shouldn't a
"select OF" be added to your kconfig entry?

>
> Thank you for your interest.
>
> Franck.
>
> [1] http://www.spinics.net/lists/dmaengine/msg09869.html
>

Best regards,
Jose Miguel Abreu
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/of: add option to load a default Device Tree

[Frank Rowand]

Hi Franck,

(This could be confusing, with Franck and Frank -- feel free to
address me as rowand in this thread if it is less confusing.)

On 06/22/16 08:05, Franck Jullien wrote:
> Even if a platform doesn't use a device tree during its
> boot process it can be useful to enable CONFIG_OF and get
> an empty device tree.
> 
> Then, devices can use device tree overlays to populate this
> default tree.
> 
> Signed-off-by: Franck Jullien <franck.jullien-EduPiq9onwdphFt5fpzH3laPQRlvutdw@public.gmane.org>
> ---
>  drivers/of/Kconfig     |  9 +++++++++
>  drivers/of/Makefile    |  3 +++
>  drivers/of/base.c      | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/of/default.dts |  4 ++++
>  drivers/of/unittest.c  | 33 +++++----------------------------
>  include/linux/of.h     |  2 ++
>  6 files changed, 73 insertions(+), 28 deletions(-)
>  create mode 100644 drivers/of/default.dts

< snip >

For context, in a later reply, you mention that this is for x86_64.

My current inclination is to prefer not to solve the problem this way.
I was going to ask why you didn't just add the compiled default.dts
to the kernel as an appended device tree blob, until I found out
this was for x86_64.

Can you add unflatten_devicetree() to x86_64, then just use the
appended device tree blob method?

-Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/of: add option to load a default Device Tree

[Franck Jullien]

2016-06-30 19:45 GMT+02:00 Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> Hi Franck,
>
> (This could be confusing, with Franck and Frank -- feel free to
> address me as rowand in this thread if it is less confusing.)
>
> On 06/22/16 08:05, Franck Jullien wrote:
>> Even if a platform doesn't use a device tree during its
>> boot process it can be useful to enable CONFIG_OF and get
>> an empty device tree.
>>
>> Then, devices can use device tree overlays to populate this
>> default tree.
>>
>> Signed-off-by: Franck Jullien <franck.jullien-EduPiq9onwdphFt5fpzH3laPQRlvutdw@public.gmane.org>
>> ---
>>  drivers/of/Kconfig     |  9 +++++++++
>>  drivers/of/Makefile    |  3 +++
>>  drivers/of/base.c      | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/of/default.dts |  4 ++++
>>  drivers/of/unittest.c  | 33 +++++----------------------------
>>  include/linux/of.h     |  2 ++
>>  6 files changed, 73 insertions(+), 28 deletions(-)
>>  create mode 100644 drivers/of/default.dts
>
> < snip >
>
> For context, in a later reply, you mention that this is for x86_64.
>
> My current inclination is to prefer not to solve the problem this way.
> I was going to ask why you didn't just add the compiled default.dts
> to the kernel as an appended device tree blob, until I found out
> this was for x86_64.
>
> Can you add unflatten_devicetree() to x86_64, then just use the
> appended device tree blob method?
>

You're saying I should implement the ability for x86_64 to detect and
load a devicetree that is appended to the binary image like ARM does [1] ?

Would you keep CONFIG_DEFAULT_DTB at all ? Or appending an
empty (or not) devicetree would be after the binary image is built ?

I like the idea. I think I could remove CONFIG_DEFAULT_DTB and
let the user append a blob to the image.

By the way, why having an empty devicetree built-in is not your
preferred choice ?

Franck.

[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e2a6a3aafa9862c4a4b59f2a59b8f923d64a680e
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/of: add option to load a default Device Tree

[Pantelis Antoniou]

Hi Franck,

> On Jun 30, 2016, at 23:44 , Franck Jullien <franck.jullien-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
> 2016-06-30 19:45 GMT+02:00 Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> Hi Franck,
>> 
>> (This could be confusing, with Franck and Frank -- feel free to
>> address me as rowand in this thread if it is less confusing.)
>> 
>> On 06/22/16 08:05, Franck Jullien wrote:
>>> Even if a platform doesn't use a device tree during its
>>> boot process it can be useful to enable CONFIG_OF and get
>>> an empty device tree.
>>> 
>>> Then, devices can use device tree overlays to populate this
>>> default tree.
>>> 
>>> Signed-off-by: Franck Jullien <franck.jullien-EduPiq9onwdphFt5fpzH3laPQRlvutdw@public.gmane.org>
>>> ---
>>> drivers/of/Kconfig     |  9 +++++++++
>>> drivers/of/Makefile    |  3 +++
>>> drivers/of/base.c      | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>> drivers/of/default.dts |  4 ++++
>>> drivers/of/unittest.c  | 33 +++++----------------------------
>>> include/linux/of.h     |  2 ++
>>> 6 files changed, 73 insertions(+), 28 deletions(-)
>>> create mode 100644 drivers/of/default.dts
>> 
>> < snip >
>> 
>> For context, in a later reply, you mention that this is for x86_64.
>> 
>> My current inclination is to prefer not to solve the problem this way.
>> I was going to ask why you didn't just add the compiled default.dts
>> to the kernel as an appended device tree blob, until I found out
>> this was for x86_64.
>> 
>> Can you add unflatten_devicetree() to x86_64, then just use the
>> appended device tree blob method?
>> 
> 
> You're saying I should implement the ability for x86_64 to detect and
> load a devicetree that is appended to the binary image like ARM does [1] ?
> 
> Would you keep CONFIG_DEFAULT_DTB at all ? Or appending an
> empty (or not) devicetree would be after the binary image is built ?
> 
> I like the idea. I think I could remove CONFIG_DEFAULT_DTB and
> let the user append a blob to the image.
> 
> By the way, why having an empty devicetree built-in is not your
> preferred choice ?
> 

Already have a patch that does that but it needs some cleanup. I haven’t bothered
with the i386 version either.

Sigh, -ENOTIME..

> Franck.
> 

Regards

— Pantelis

> [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e2a6a3aafa9862c4a4b59f2a59b8f923d64a680e

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/of: add option to load a default Device Tree

[Franck Jullien]

Hi,

2016-06-30 12:53 GMT+02:00 Jose Abreu <Jose.Abreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>:
> Hi Franck,
>
> ++ Carlos
>
> On 24-06-2016 09:15, Franck Jullien wrote:
>> Le 23/06/2016 à 20:27, Frank Rowand a écrit :
>>> On 06/22/16 08:05, Franck Jullien wrote:
>>>> Even if a platform doesn't use a device tree during its
>>>> boot process it can be useful to enable CONFIG_OF and get
>>>> an empty device tree.
>>>>
>>>> Then, devices can use device tree overlays to populate this
>>>> default tree.
>>>
>>> Hi Franck,
>>>
>>> Can you elaborate on the problem that you are trying to
>>> solve and the use cases that you are envisioning?
>>>
>>> Thanks,
>>>
>>> Frank
>>>
>> Hi,
>>
>> The idea behind this patch is to describe devices that are in a FPGA
>> which is itself connected to the platform through PCIe. The platform
>> might not use a devicetree for its own configuration. That's why we need
>> this patch.
>
> This sure looks interesting to us because we are using the
> exactly same setup (x86_64 + FPGA through PCIe). As I said before
> in the DMA mailing list our approach was different: we use
> platform data in each driver and we register them using a PCI
> driver which passes all the required parameters. Still, we were
> wondering if your approach can save us development time in the
> future. So, I have a few questions that I was hoping you could
> answer.
>
>>
>> There is an ongoing  discussion about this subject [1].
>>
>> I have a working setup (on a x86_64) using this principle. I have
>> created a pci resources proxy module which is parent of all devices on
>> the FPGA. This is an example of binding:
>>
>>       proxy: pci1337_0001 {
>>               #address-cells = <1>;
>>               #size-cells = <1>;
>>               #interrupt-cells = <1>;
>>               vendor-id = <0x00001337>;
>>               device-id = <0x00000001>;
>>               compatible = "pci1337,0001",
>>                            "generic,pci-proxy";
>>               interrupt-controller;
>>               /* Ranges will be overwritten during probe */
>>               ranges = <0 0 0xA0000000 0x100000     /* BAR 0 */
>>                         1 0 0xB0000000 0x100000     /* BAR 1 */
>>                         2 0 0xC0000000 0x100000>;   /* BAR 2 */
>>       };
>>
>> Ranges are dynamically updated on probe with regards to FPGA bars.
>> This node is attached to the tree (an empty tree in my case) from the
>> client module.
>
> I am not fully understanding where does this binding is added.
> You use your patches to create an empty device tree or a device
> tree where the FPGA is declared?

The patch creates an empty devicetree. In my setup, I have two modules:

- pci-proxy, which is independant of the functionnal part of the board.

- my_module, where the node above is attached to the root node. This
module also creates the overlay.

>
>>
>> Then FPGA devices nodes are added using overlay. This is an example:
>>
>> /dts-v1/;
>> / {
>>       #address-cells = <1>;
>>       #size-cells = <1>;
>>
>>       fragment@0 {
>>               target-path = "/pci1337_0001";
>>               __overlay__ {
>>
>>                       #address-cells = <2>;
>>                       #size-cells = <1>;
>>                       interrupt-parent = <&proxy>;
>>
>>                       osc: oscillator {
>>                               compatible = "fixed-clock";
>>                               #clock-cells = <1>;
>>                               clock-frequency  = <148500000>;
>>                               clock-output-names = "osc";
>>                       };
>>                       enet0: ethoc {
>>                               compatible = "opencores,ethoc";
>>                               reg = <1 0x50000 0x100>;
>>                               interrupts = <0>;
>>                       };
>>                       gpio0: gpio {
>>                               #gpio-cells = <2>;
>>                               compatible = "xlnx,xps-gpio-1.00.a";
>>                               reg = <0 0x30000 0x10000>;
>>                               gpio-controller;
>>                       };
>>               };
>>       };
>>
>>
>> };
>>
>> Interrupts property are dynamically modified before the overlay is
>> applied. By the way, I had to do a little hack here in the pci-proxy.
>> I did not declare a new interrupt domain. What I did is to get the PCI
>> interrupt domain (IO-APIC on my platform):
>>
>> irq_data = irq_get_irq_data(priv->pci_dev->irq);
>> domain_parent = irq_data->domain;
>>
>> and then set fwnode of IO-APIC to pci-proxy fwnode so
>> irq_create_of_mapping get IO-APIC's domain:
>>
>> domain_parent->fwnode = &np->fwnode;
>
> In my case we have an interrupt controller driver which is
> embedded into the PCI driver so I also can't use the generic PCI
> proxy without modifying it. Can this controller be declared in
> the overlay device tree and then make the remaining drivers use
> the domain created?

Yes you can. In my current work, pci-proxy is not an interrupt controller
anymore. I created a io-apic node (which is here just to get a fwnode
reference) which is interrupt parent of other nodes in the overlay.

>
>>
>> There might be a better way to make things work.
>>
>> Beside this patch I needed to export of_attach_node and of_detach_node.
>> This will be another patch.
>
> There is still one more thing: By default CONFIG_OF is not
> enabled in x86_64 so you need to enable this, right? Shouldn't a
> "select OF" be added to your kconfig entry?
>

You're right. CONFIG_OF still need to be enabled.

Franck.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/of: add option to load a default Device Tree

[Frank Rowand]

On 06/30/16 13:44, Franck Jullien wrote:
> 2016-06-30 19:45 GMT+02:00 Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> Hi Franck,
>>
>> (This could be confusing, with Franck and Frank -- feel free to
>> address me as rowand in this thread if it is less confusing.)
>>
>> On 06/22/16 08:05, Franck Jullien wrote:
>>> Even if a platform doesn't use a device tree during its
>>> boot process it can be useful to enable CONFIG_OF and get
>>> an empty device tree.
>>>
>>> Then, devices can use device tree overlays to populate this
>>> default tree.
>>>
>>> Signed-off-by: Franck Jullien <franck.jullien-EduPiq9onwdphFt5fpzH3laPQRlvutdw@public.gmane.org>
>>> ---
>>>  drivers/of/Kconfig     |  9 +++++++++
>>>  drivers/of/Makefile    |  3 +++
>>>  drivers/of/base.c      | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  drivers/of/default.dts |  4 ++++
>>>  drivers/of/unittest.c  | 33 +++++----------------------------
>>>  include/linux/of.h     |  2 ++
>>>  6 files changed, 73 insertions(+), 28 deletions(-)
>>>  create mode 100644 drivers/of/default.dts
>>
>> < snip >
>>
>> For context, in a later reply, you mention that this is for x86_64.
>>
>> My current inclination is to prefer not to solve the problem this way.
>> I was going to ask why you didn't just add the compiled default.dts
>> to the kernel as an appended device tree blob, until I found out
>> this was for x86_64.
>>
>> Can you add unflatten_devicetree() to x86_64, then just use the
>> appended device tree blob method?
>>
> 
> You're saying I should implement the ability for x86_64 to detect and
> load a devicetree that is appended to the binary image like ARM does [1] ?

Yes, that is what I was referring to.


> Would you keep CONFIG_DEFAULT_DTB at all ? Or appending an
> empty (or not) devicetree would be after the binary image is built ?
> 
> I like the idea. I think I could remove CONFIG_DEFAULT_DTB and
> let the user append a blob to the image.

There should be a config option to add the Makefile entry for the
default.dts so that default.dtb is created.

And yes, the .dtb would be appended to the kernel image after the
kernel image was built.

One description of this is at:

http://community.cadence.com/cadence_blogs_8/b/sd/archive/2013/10/09/combining-the-linux-device-tree-and-kernel-image-for-arm


> 
> By the way, why having an empty devicetree built-in is not your
> preferred choice ?

The issue is not whether the .dtb is build into the kernel image or
appended to the kernel image.  The issue is whether the normal
unflatten_devicetree()  path is used to load the .dtb or whether
an additional method of creating the root device tree was added.


> Franck.
> 
> [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e2a6a3aafa9862c4a4b59f2a59b8f923d64a680e
> .
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html