[PATCH 0/4 v2] of/overlay: sysfs based ABI for dt overlays

[PATCH 0/4 v2] of/overlay: sysfs based ABI for dt overlays

[Heinrich Schuchardt]

Currently the kernel only supplies an internal API for creating
and destroying device tree overlays.

For some boards vendor specific kernel modules exist for
managing device tree overlays but they have not been
upstreamed or upstreaming stalled.
https://lkml.org/lkml/2015/6/12/624
https://lkml.org/lkml/2013/1/7/366

This patch series provides a sysfs based ABI for creation and
destruction of dt overlays in /sys/firmware/devicetree/overlays.

The following files are provided:

load:   This is a write only file.
        A string written to it is interpreted as the path to a
        flattened device tree overlay file. It is used to create
        and apply the contained overlays.

loaded: This is a read only file.
        It provides the count of loaded overlays as a decimal
        number.

unload: This is a write only file.
        If a positive number n is wrtten to this file the n
        most recent overlays are destroyed.
        If a negative number is written to this file all
        overlays are destroyed.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

version 2:
	change sysfs path to
	/sys/firmware/devicetree/overlays

	Fix errors indicated by kbuild robot:
	Add missing inline attribute to of_overlay_count
	in patch 1.
	Add 'select CONFIG_OF_EARLY_FLATTREE' to Kconfig
	in patch 2.

	Change unit test cases to check new functions
	of_overlay_count and of_overlay_destroy_last.

Heinrich Schuchardt (4):
  of/overlay: add API function to count and pop last
  of/overlay: sysfs based ABI for dt overlays
  of/overlay: documentation for sysfs ABI 
  of/overlay: test count and destroy_last

 .../ABI/testing/sysfs-firmware-devicetree-overlays |  24 +++ 
 Documentation/devicetree/overlay-notes.txt         |   7 +-
 drivers/of/Kconfig                                 |  15 ++
 drivers/of/Makefile                                |   2 + 
 drivers/of/base.c                                  |   1 + 
 drivers/of/ov_sysfs.c                              | 223 +++++++++++++++++++++
 drivers/of/overlay.c                               |  50 +++++
 drivers/of/unittest.c                              |  15 +-
 include/linux/of.h                                 |  12 ++
 9 files changed, 346 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
 create mode 100644 drivers/of/ov_sysfs.c

-- 
2.11.0

[PATCH 1/4 v2] of/overlay: add API function to count and pop last

[Heinrich Schuchardt]

To allow building interfaces which are not id based
two new functions are added to the device tree
overlay API:

of_overlay_count        - counts the loaded overlays
of_overlay_destroy_last - removes the last overlay loaded

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---

v2:
	Add missing inline attribute for ov_overlay_count.

 Documentation/devicetree/overlay-notes.txt |  7 +++--
 drivers/of/overlay.c                       | 50 ++++++++++++++++++++++++++++++
 include/linux/of.h                         | 12 +++++++
 3 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/overlay-notes.txt b/Documentation/devicetree/overlay-notes.txt
index d418a6c..7c8099c 100644
--- a/Documentation/devicetree/overlay-notes.txt
+++ b/Documentation/devicetree/overlay-notes.txt
@@ -89,17 +89,20 @@ Overlay in-kernel API
 
 The API is quite easy to use.
 
-1. Call of_overlay_create() to create and apply an overlay. The return value
+Call of_overlay_create() to create and apply an overlay. The return value
 is a cookie identifying this overlay.
 
-2. Call of_overlay_destroy() to remove and cleanup the overlay previously
+Call of_overlay_destroy() to remove and cleanup the overlay previously
 created via the call to of_overlay_create(). Removal of an overlay that
 is stacked by another will not be permitted.
+Or call of_overlay_destroy_last() to remove the most recent overlay.
 
 Finally, if you need to remove all overlays in one-go, just call
 of_overlay_destroy_all() which will remove every single one in the correct
 order.
 
+Call of_overlay_count() to determine the number of loaded overlays.
+
 Overlay DTS Format
 ------------------
 
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 0d4cda7..bd30253 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -587,3 +587,53 @@ int of_overlay_destroy_all(void)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(of_overlay_destroy_all);
+
+/**
+ * of_overlay_destroy_last() - Removes the last overlay from the system
+ *
+ * It is allways possible to delete the last overlay.
+ *
+ * Returns 0 on success, or a negative error number
+ */
+int of_overlay_destroy_last(void)
+{
+	struct of_overlay *ov, *ovn;
+	int id;
+
+	mutex_lock(&of_mutex);
+
+	list_for_each_entry_safe_reverse(ov, ovn, &ov_list, node) {
+		id = ov->id;
+		mutex_unlock(&of_mutex);
+		return of_overlay_destroy(id);
+	}
+
+	mutex_unlock(&of_mutex);
+
+	pr_info("destroy: No overlay to destroy");
+
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(of_overlay_destroy_last);
+
+/**
+ * of_overlay_count - Counts number of loaded overlays
+ *
+ * Returns number of loaded overlays
+ */
+int of_overlay_count(void)
+{
+	struct of_overlay *ov, *ovn;
+	int count = 0;
+
+	mutex_lock(&of_mutex);
+
+	list_for_each_entry_safe(ov, ovn, &ov_list, node) {
+		++count;
+	}
+
+	mutex_unlock(&of_mutex);
+
+	return count;
+}
+EXPORT_SYMBOL_GPL(of_overlay_count);
diff --git a/include/linux/of.h b/include/linux/of.h
index d72f010..5ca3523 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1281,15 +1281,22 @@ struct of_overlay_notify_data {
 #ifdef CONFIG_OF_OVERLAY
 
 /* ID based overlays; the API for external users */
+int of_overlay_count(void);
 int of_overlay_create(struct device_node *tree);
 int of_overlay_destroy(int id);
 int of_overlay_destroy_all(void);
+int of_overlay_destroy_last(void);
 
 int of_overlay_notifier_register(struct notifier_block *nb);
 int of_overlay_notifier_unregister(struct notifier_block *nb);
 
 #else
 
+static inline int of_overlay_count(void)
+{
+	return -ENOTSUPP;
+}
+
 static inline int of_overlay_create(struct device_node *tree)
 {
 	return -ENOTSUPP;
@@ -1305,6 +1312,11 @@ static inline int of_overlay_destroy_all(void)
 	return -ENOTSUPP;
 }
 
+static inline int of_overlay_destroy_last(void)
+{
+	return -ENOTSUPP;
+}
+
 static inline int of_overlay_notifier_register(struct notifier_block *nb)
 {
 	return 0;
-- 
2.10.2

[PATCH 2/4 v2] of/overlay: sysfs based ABI for dt overlays

[Heinrich Schuchardt]

Currently the kernel only supplies an internal API for creating
and destroying device tree overlays.

For some boards vendor specific kernel modules exist for
managing device tree overlays but the have not been
upstreamed.

This patch provides a sysfs based ABI for creation and destruction
of dt overlays in /sys/firmware/devicetree-overlay.

The following files are provided:

load:   This is a write only file.
        A string written to it is interpreted as the path to a
        flattened device tree overlay file. It is used to create
        and apply the contained overlays.

loaded: This is a read only file.
        It provides the count of loaded overlays as a decimal
        number.

unload: This is a write only file.
        If a positive number n is wrtten to this file the n
        most recent overlays are destroyed.
        If a negative number is written to this file all
        overlays are destroyed.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---

v2:
	Change sysfs path to
	/sys/firmware/devicetree/overlays.
	Add 'select CONFIG_OF_EARLY_FLATTREE' to Kconfig.

 drivers/of/Kconfig    |  15 ++++
 drivers/of/Makefile   |   2 +
 drivers/of/base.c     |   1 +
 drivers/of/ov_sysfs.c | 223 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 241 insertions(+)
 create mode 100644 drivers/of/ov_sysfs.c

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index ba7b034..09ff057 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -109,6 +109,21 @@ config OF_OVERLAY
 	  While this option is selected automatically when needed, you can
 	  enable it manually to improve device tree unit test coverage.
 
+if OF_OVERLAY
+
+config OF_OVERLAY_SYSFS
+
+	tristate "Sysfs support for device tree overlays"
+	default m
+	depends on SYSFS
+	select OF_EARLY_FLATTREE
+	help
+	  This module provides a sysfs based ABI to manage device tree
+	  overlays. You can use it to create overlays based on flattened
+	  device tree overlay files and to destroy them.
+
+endif # OF_OVERLAY
+
 config OF_NUMA
 	bool
 
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index d7efd9d..7026de4 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -16,3 +16,5 @@ obj-$(CONFIG_OF_OVERLAY) += overlay.o
 obj-$(CONFIG_OF_NUMA) += of_numa.o
 
 obj-$(CONFIG_OF_UNITTEST) += unittest-data/
+
+obj-$(CONFIG_OF_OVERLAY_SYSFS) += ov_sysfs.o
diff --git a/drivers/of/base.c b/drivers/of/base.c
index d4bea3c..8cffe38 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -43,6 +43,7 @@ struct device_node *of_stdout;
 static const char *of_stdout_options;
 
 struct kset *of_kset;
+EXPORT_SYMBOL(of_kset);
 
 /*
  * Used to protect the of_aliases, to hold off addition of nodes to sysfs.
diff --git a/drivers/of/ov_sysfs.c b/drivers/of/ov_sysfs.c
new file mode 100644
index 0000000..f3fcb70
--- /dev/null
+++ b/drivers/of/ov_sysfs.c
@@ -0,0 +1,223 @@
+/*
+ * Sysfs ABI for device tree overlays
+ *
+ * Copyright (C) 2016  Heinrich Schuchardt <xypron.glpk@gmx.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License,
+ * version 2 as published by the Free Software Foundation.
+ */
+
+#include <linux/fcntl.h>
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/kernel.h>
+#include <linux/libfdt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+#include <linux/uaccess.h>
+
+#include "of_private.h"
+
+static int of_create_overlay_from_file(const char *path)
+{
+	struct file *filp = NULL;
+	mm_segment_t fs;
+	int ret = 0;
+	loff_t size;
+	char *buffer = NULL;
+	ssize_t bytes_read;
+	loff_t offset = 0;
+	struct device_node *overlay = NULL;
+
+	fs = get_fs();
+	set_fs(get_ds());
+	filp = filp_open(path, O_RDONLY | O_LARGEFILE, 0);
+	if (IS_ERR(filp)) {
+		ret = PTR_ERR(filp);
+		goto err_file_open;
+	}
+
+	if (!S_ISREG(filp->f_inode->i_mode)) {
+		ret = -EISDIR;
+		goto err_file_read;
+	}
+	size = i_size_read(filp->f_inode);
+	buffer = vmalloc(size);
+	if (buffer == NULL) {
+		ret = -ENOMEM;
+		goto err_malloc;
+	}
+	for (; size > 0; ) {
+		bytes_read = vfs_read(filp, buffer, size, &offset);
+		if (bytes_read == 0)
+			break;
+		if (bytes_read < 0) {
+			ret = bytes_read;
+			goto err_file_read;
+		}
+		size -= bytes_read;
+	}
+	if (offset < sizeof(struct fdt_header) ||
+	    offset < fdt_totalsize(buffer)) {
+		pr_err("OF: Size of %s does not match header information\n",
+		       path);
+		ret = -EINVAL;
+		goto err_file_read;
+	}
+	overlay = of_fdt_unflatten_tree((unsigned long *) buffer, NULL, NULL);
+	if (overlay == NULL) {
+		pr_err("OF: Cannot unflatten %s\n", path);
+		ret = -EINVAL;
+		goto err_file_read;
+	}
+	of_node_set_flag(overlay, OF_DETACHED);
+	ret = of_resolve_phandles(overlay);
+	if (ret < 0) {
+		pr_err("OF: Failed to resolve phandles for %s\n", path);
+		goto err_overlay;
+	}
+	ret = of_overlay_create(overlay);
+	if (ret < 0) {
+		pr_err("OF: Cannot create overlay from %s\n", path);
+	} else {
+		pr_info("OF: Overlay %d created from %s\n", ret, path);
+		ret = 0;
+	}
+err_overlay:
+	of_node_put(overlay);
+err_file_read:
+	vfree(buffer);
+err_malloc:
+	fput(filp);
+err_file_open:
+	set_fs(fs);
+	return ret;
+}
+
+static ssize_t attribute_read(struct kobject *kobj,
+			      struct kobj_attribute *attr,
+			      char *buf)
+{
+	int ret;
+
+	if (strcmp(attr->attr.name, "loaded") == 0)
+		ret = sprintf(buf, "%d\n", of_overlay_count());
+	else
+		ret = -ENOENT;
+
+	return ret;
+}
+
+static ssize_t attribute_write(struct kobject *kobj,
+			       struct kobj_attribute *attr,
+			       const char *buf, size_t size)
+{
+	char *parameter;
+	int ret;
+	long count;
+
+	if (size > PATH_MAX)
+		return -ENAMETOOLONG;
+
+	/* The parameter has to be terminated either by LF or \0. */
+
+	switch (buf[size - 1]) {
+	case 0x00:
+	case 0x0a:
+		break;
+	default:
+		return -ENOENT;
+	}
+	parameter = vmalloc(size);
+	if (!parameter)
+		return -ENOMEM;
+	memcpy(parameter, buf, size);
+	parameter[size - 1] = 0x00;
+
+	if (strcmp(attr->attr.name, "load") == 0) {
+		ret = of_create_overlay_from_file(parameter);
+		if (!ret)
+			ret = size;
+	} else if (strcmp(attr->attr.name, "unload") == 0) {
+		ret = kstrtol(parameter, 0, &count);
+		if (ret)
+			goto out;
+		if (count < 0)
+			ret = of_overlay_destroy_all();
+		else
+			for (; count > 0; --count) {
+				ret = of_overlay_destroy_last();
+				if (ret)
+					goto out;
+			}
+		ret = size;
+	} else
+		ret = -ENOENT;
+out:
+	vfree(parameter);
+
+	return ret;
+}
+
+static struct kobject *kobj;
+
+static struct kobj_attribute load_attribute =
+	__ATTR(load, 0200, NULL, attribute_write);
+static struct kobj_attribute loaded_attribute =
+	__ATTR(loaded, 0444, attribute_read, NULL);
+static struct kobj_attribute unload_attribute =
+	__ATTR(unload, 0200, NULL, attribute_write);
+static struct attribute *attrs[] = {
+	&load_attribute.attr,
+	&loaded_attribute.attr,
+	&unload_attribute.attr,
+	NULL
+};
+static struct attribute_group attr_group = {
+	.attrs = attrs,
+};
+
+static int __init ov_sysfs_init(void)
+{
+	int ret;
+
+	if (!of_kset) {
+		pr_err("OF: failed to register overlays\n");
+		return -ENODEV;
+	}
+
+	kobj = kobject_create_and_add("overlays", &of_kset->kobj);
+	if (kobj == 0) {
+		pr_err("OF: failed to register overlays\n");
+		return -ENOMEM;
+	}
+	ret = sysfs_create_group(kobj, &attr_group);
+	if (ret) {
+		kobject_put(kobj);
+		return ret;
+	}
+
+	/*
+	 * It is not possible to ensure that no sysfs io is started while
+	 * module_exit is called. So disable unloading.
+	 */
+	__module_get(THIS_MODULE);
+
+	return 0;
+}
+
+static void __exit ov_sysfs_exit(void)
+{
+	kobject_put(kobj);
+}
+
+module_init(ov_sysfs_init);
+module_exit(ov_sysfs_exit);
+
+MODULE_AUTHOR("Heinrich Schuchardt <xypron.glpk@gmx.de>");
+MODULE_DESCRIPTION("Sysfs ABI for device tree overlays");
+MODULE_LICENSE("GPL v2");
-- 
2.10.2

[PATCH 3/4 v2] of/overlay: documentation for sysfs ABI

[Heinrich Schuchardt]

The sysfs filesystem ABI to load and unload device tree
overlays is decribed.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---

v2:
	Change sysfs path to
	/sys/firmware/devicetree/overlays

 .../ABI/testing/sysfs-firmware-devicetree-overlays | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-firmware-devicetree-overlays

diff --git a/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays b/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
new file mode 100644
index 0000000..bda0001
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
@@ -0,0 +1,24 @@
+What:		/sys/firmware/devicetree/overlays
+Date:		Dec 2016
+KernelVersion:	4.11
+Contact:	Heinrich Schuchardt <xypron.glpk@gmx.de>
+Description:	Devicetree overlays can be used to update the devicetree
+		while the system is running. For details see
+		Documentation/devicetree/overlay-notes.txt.
+
+		The following attributes are provided:
+
+		load:	This is a write only file.
+			A string written to it is interpreted as the path to a
+			flattened device tree overlay file. It is used to create
+			and apply the contained overlays.
+
+		loaded: This is a read only file.
+			It provides the count of loaded overlays as a decimal
+			number.
+
+		unload: This is a write only file.
+		        If a positive number n is wrtten to this file the n
+			most recent overlays are destroyed.
+			If a negative number is written to this file all
+			overlays are destroyed.
-- 
2.10.2

[PATCH 4/4 v2] of/overlay: test count and destroy_last

[Heinrich Schuchardt]

In the unit tests check that the functions
of_overlay_destroy_last() and of_overlay_count()
work as expected.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---

v2:
	Added this patch

 drivers/of/unittest.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 53c83d6..e3fbf0b 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -1466,6 +1466,7 @@ static void of_unittest_overlay_8(void)
 	struct device_node *np;
 	int ret, i, ov_id[2];
 	int overlay_nr = 8, unittest_nr = 8;
+	int count_before = of_overlay_count();
 
 	/* we don't care about device state in this test */
 
@@ -1485,6 +1486,12 @@ static void of_unittest_overlay_8(void)
 					overlay_path(overlay_nr + i));
 			return;
 		}
+
+		if (count_before + i + 1 != of_overlay_count()) {
+			unittest(0, "count does not increment by 1\n");
+			return;
+		}
+
 		ov_id[i] = ret;
 		of_unittest_track_overlay(ov_id[i]);
 	}
@@ -1501,7 +1508,7 @@ static void of_unittest_overlay_8(void)
 
 	/* removing them in order should work */
 	for (i = 1; i >= 0; i--) {
-		ret = of_overlay_destroy(ov_id[i]);
+		ret = of_overlay_destroy_last();
 		if (ret != 0) {
 			unittest(0, "overlay @\"%s\" not destroyed @\"%s\"\n",
 					overlay_path(overlay_nr + i),
@@ -1509,6 +1516,12 @@ static void of_unittest_overlay_8(void)
 						PDEV_OVERLAY));
 			return;
 		}
+
+		if (count_before + i != of_overlay_count()) {
+			unittest(0, "count does not decrement by 1\n");
+			return;
+		}
+
 		of_unittest_untrack_overlay(ov_id[i]);
 	}
 
-- 
2.10.2

Re: [PATCH 0/4 v2] of/overlay: sysfs based ABI for dt overlays

[Frank Rowand]

Hi Heinrich,

On 12/20/16 11:04, Heinrich Schuchardt wrote:
> Currently the kernel only supplies an internal API for creating
> and destroying device tree overlays.
> 
> For some boards vendor specific kernel modules exist for
> managing device tree overlays but they have not been
> upstreamed or upstreaming stalled.
> https://lkml.org/lkml/2015/6/12/624
> https://lkml.org/lkml/2013/1/7/366
> 
> This patch series provides a sysfs based ABI for creation and
> destruction of dt overlays in /sys/firmware/devicetree/overlays.
> 
> The following files are provided:
> 
> load:   This is a write only file.
>         A string written to it is interpreted as the path to a
>         flattened device tree overlay file. It is used to create
>         and apply the contained overlays.
> 
> loaded: This is a read only file.
>         It provides the count of loaded overlays as a decimal
>         number.
> 
> unload: This is a write only file.
>         If a positive number n is wrtten to this file the n
>         most recent overlays are destroyed.
>         If a negative number is written to this file all
>         overlays are destroyed.

This patch series follows a _somewhat_ similar approach to what
was first proposed two years ago, and does not address the
issues that were brought up at that time.  See:

  From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
  Date: Wed,  3 Dec 2014 13:23:28 +0200
  Subject: [PATCH] OF: DT-Overlay configfs interface (v3)

But just responding directly to the two year old issues would not
be a productive approach, since there has been a lot of subsequent
discussion on how to load overlays (you point to two of the many
threads above).  The latest discussions are based on the concept
of describing the overlay attachment points as connectors.

Please join in pushing the connectors discussion along to make
sure that it meets your needs.

-Frank


> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> 
> version 2:
> 	change sysfs path to
> 	/sys/firmware/devicetree/overlays
> 
> 	Fix errors indicated by kbuild robot:
> 	Add missing inline attribute to of_overlay_count
> 	in patch 1.
> 	Add 'select CONFIG_OF_EARLY_FLATTREE' to Kconfig
> 	in patch 2.
> 
> 	Change unit test cases to check new functions
> 	of_overlay_count and of_overlay_destroy_last.
> 
> Heinrich Schuchardt (4):
>   of/overlay: add API function to count and pop last
>   of/overlay: sysfs based ABI for dt overlays
>   of/overlay: documentation for sysfs ABI 
>   of/overlay: test count and destroy_last
> 
>  .../ABI/testing/sysfs-firmware-devicetree-overlays |  24 +++ 
>  Documentation/devicetree/overlay-notes.txt         |   7 +-
>  drivers/of/Kconfig                                 |  15 ++
>  drivers/of/Makefile                                |   2 + 
>  drivers/of/base.c                                  |   1 + 
>  drivers/of/ov_sysfs.c                              | 223 +++++++++++++++++++++
>  drivers/of/overlay.c                               |  50 +++++
>  drivers/of/unittest.c                              |  15 +-
>  include/linux/of.h                                 |  12 ++
>  9 files changed, 346 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
>  create mode 100644 drivers/of/ov_sysfs.c
> 

Re: [PATCH 0/4 v2] of/overlay: sysfs based ABI for dt overlays

[Pantelis Antoniou]

Hi Frank, Heinrich,

> On Dec 22, 2016, at 21:00 , Frank Rowand <frowand.list@gmail.com> wrote:
> 
> Hi Heinrich,
> 
> On 12/20/16 11:04, Heinrich Schuchardt wrote:
>> Currently the kernel only supplies an internal API for creating
>> and destroying device tree overlays.
>> 
>> For some boards vendor specific kernel modules exist for
>> managing device tree overlays but they have not been
>> upstreamed or upstreaming stalled.
>> https://lkml.org/lkml/2015/6/12/624
>> https://lkml.org/lkml/2013/1/7/366
>> 
>> This patch series provides a sysfs based ABI for creation and
>> destruction of dt overlays in /sys/firmware/devicetree/overlays.
>> 
>> The following files are provided:
>> 
>> load:   This is a write only file.
>>        A string written to it is interpreted as the path to a
>>        flattened device tree overlay file. It is used to create
>>        and apply the contained overlays.
>> 
>> loaded: This is a read only file.
>>        It provides the count of loaded overlays as a decimal
>>        number.
>> 
>> unload: This is a write only file.
>>        If a positive number n is wrtten to this file the n
>>        most recent overlays are destroyed.
>>        If a negative number is written to this file all
>>        overlays are destroyed.
> 
> This patch series follows a _somewhat_ similar approach to what
> was first proposed two years ago, and does not address the
> issues that were brought up at that time.  See:
> 
>  From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>  Date: Wed,  3 Dec 2014 13:23:28 +0200
>  Subject: [PATCH] OF: DT-Overlay configfs interface (v3)
> 
> But just responding directly to the two year old issues would not
> be a productive approach, since there has been a lot of subsequent
> discussion on how to load overlays (you point to two of the many
> threads above).  The latest discussions are based on the concept
> of describing the overlay attachment points as connectors.
> 
> Please join in pushing the connectors discussion along to make
> sure that it meets your needs.
> 

I think it would be best if we focus on getting the configfs based loader
to work. It is pretty similar to what Heinrich is proposing.

> -Frank
> 

Regards

— Pantelis

> 
>> 
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> 
>> version 2:
>> 	change sysfs path to
>> 	/sys/firmware/devicetree/overlays
>> 
>> 	Fix errors indicated by kbuild robot:
>> 	Add missing inline attribute to of_overlay_count
>> 	in patch 1.
>> 	Add 'select CONFIG_OF_EARLY_FLATTREE' to Kconfig
>> 	in patch 2.
>> 
>> 	Change unit test cases to check new functions
>> 	of_overlay_count and of_overlay_destroy_last.
>> 
>> Heinrich Schuchardt (4):
>>  of/overlay: add API function to count and pop last
>>  of/overlay: sysfs based ABI for dt overlays
>>  of/overlay: documentation for sysfs ABI 
>>  of/overlay: test count and destroy_last
>> 
>> .../ABI/testing/sysfs-firmware-devicetree-overlays |  24 +++ 
>> Documentation/devicetree/overlay-notes.txt         |   7 +-
>> drivers/of/Kconfig                                 |  15 ++
>> drivers/of/Makefile                                |   2 + 
>> drivers/of/base.c                                  |   1 + 
>> drivers/of/ov_sysfs.c                              | 223 +++++++++++++++++++++
>> drivers/of/overlay.c                               |  50 +++++
>> drivers/of/unittest.c                              |  15 +-
>> include/linux/of.h                                 |  12 ++
>> 9 files changed, 346 insertions(+), 3 deletions(-)
>> create mode 100644 Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
>> create mode 100644 drivers/of/ov_sysfs.c

Re: [PATCH 0/4 v2] of/overlay: sysfs based ABI for dt overlays

[Heinrich Schuchardt]

On 01/03/2017 01:11 PM, Pantelis Antoniou wrote:
> Hi Frank, Heinrich,
> 
>> On Dec 22, 2016, at 21:00 , Frank Rowand <frowand.list@gmail.com> wrote:
>>
>> Hi Heinrich,
>>
>> On 12/20/16 11:04, Heinrich Schuchardt wrote:
>>> Currently the kernel only supplies an internal API for creating
>>> and destroying device tree overlays.
>>>
>>> For some boards vendor specific kernel modules exist for
>>> managing device tree overlays but they have not been
>>> upstreamed or upstreaming stalled.
>>> https://lkml.org/lkml/2015/6/12/624
>>> https://lkml.org/lkml/2013/1/7/366
>>>
>>> This patch series provides a sysfs based ABI for creation and
>>> destruction of dt overlays in /sys/firmware/devicetree/overlays.
>>>
>>> The following files are provided:
>>>
>>> load:   This is a write only file.
>>>        A string written to it is interpreted as the path to a
>>>        flattened device tree overlay file. It is used to create
>>>        and apply the contained overlays.
>>>
>>> loaded: This is a read only file.
>>>        It provides the count of loaded overlays as a decimal
>>>        number.
>>>
>>> unload: This is a write only file.
>>>        If a positive number n is wrtten to this file the n
>>>        most recent overlays are destroyed.
>>>        If a negative number is written to this file all
>>>        overlays are destroyed.
>>
>> This patch series follows a _somewhat_ similar approach to what
>> was first proposed two years ago, and does not address the
>> issues that were brought up at that time.  See:
>>
>>  From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>>  Date: Wed,  3 Dec 2014 13:23:28 +0200
>>  Subject: [PATCH] OF: DT-Overlay configfs interface (v3)
>>
>> But just responding directly to the two year old issues would not
>> be a productive approach, since there has been a lot of subsequent
>> discussion on how to load overlays (you point to two of the many
>> threads above).  The latest discussions are based on the concept
>> of describing the overlay attachment points as connectors.
>>
>> Please join in pushing the connectors discussion along to make
>> sure that it meets your needs.
>>
> 
> I think it would be best if we focus on getting the configfs based loader
> to work. It is pretty similar to what Heinrich is proposing.
> 
>> -Frank
>>
> 
> Regards
> 
> — Pantelis
> 

Reading Documentation/filesystems/configfs/configfs.txt it is not
evident to me that using configfs would be more appropriate than using
sysfs.

But I think before going into implementation details it is necessary to
define the scope of the development.

Up to now I have seen the following possible usages of device tree overlays:

1) Manage virtual devices
e.g. hot plugging CPUs and memory of a virtual machine

2) Activating optional hardware without discovery mechanism,
e.g an RTC on the I2C bus, or an I2C Grove sensor. This may include
changing the GPIO multiplexing.

3) Activating optional hardware with a discovery mechanism,
e.g. Beaglebone Capes and Raspberry hats with an EEPROM that can be read
over the I2C bus.

For scenario 3 elaborate kernel plugins have been written that try to
handle the complete complexity in kernel code, e.g. the Beaglebone cape
manager.

The discussion about connectors is also putting a lot of complexity into
possible kernel code, cf.
https://lkml.org/lkml/2016/6/30/734

In my view the kernel ABI for loading and removing overlays should
follow the KISS principle.

In scenario 3 a possible alternative design would be to put most of the
complexity into the userland and only supply the reading of the EEPROMs
via I2C and the loading mechanism for overlays as kernel modules.

In my patch series I hence only provided three interface functions:
Push an overlay on the stack of overlays.
Pop an overlay from the stack.
Count the overlays.

I prefer pop to delete by id because it will always succeed. But of
cause delete by id could be added to the ABI.

When applying overlays to change the same property again and again the
overlay stack can be considered a memory leak. So we might want to add a
merge/commit function. That function would empty the overlay stack
without undoing the changes.

Best regards

Heinrich Schuchardt