All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments
@ 2017-10-10  5:34 John Stultz
  2017-10-10  5:34 ` [RFC][PATCH 1/3] of: overlay_mgr: Add overlay manager driver John Stultz
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: John Stultz @ 2017-10-10  5:34 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Rob Herring, Mark Rutland, Frank Rowand,
	Dmitry Shmidt, devicetree

In working with the HiKey and HiKey960 as targets for AOSP,
Dmitry developed the following overlay manager driver, which
allows a number of pre-determined DT overlay configurations to
be defined, and the configurations to be enabled at boot time
via a kernel boot argument.

This has been submitted before, but while the earlier discussion
didn't really resolve to any sort of actionable direction, this
issue cropped up again and was a major discussion topic at the
Linux Plumbers Conference Android Microconference, so I suspect
it is worth revisiting this solution again.

The overall use case is being able to configure devboards that
support a number of different mezzanine peripherals which
unfortunately cannot be probed. Some example mezzanines are
LCD panels or sensor hubs, as well as other options.

The new functionality this driver provides is a mechanism to
specify multiple pre-determined dt overlay fragments in a
dtb file, and providing a way to select which dt fragments
should be applied via the kernel boot argument.

The desire to use a kernel boot-argument as the selection
mechanism, comes from the Android Boot Image format not handling
dtbs independently. Usually with Android, the dtb is appended
to the kernel image, and modifying that is much more difficult
then changing the boot argugments. There is also a usability
argument that using a kernel command option to select
pre-defined entries is simpler for users to navigate.

Also, since the mezzanines are unable to be probed, we cannot
use other solutions, like having the bootloader specify
additional dtb overlays to the kernel.

Obviously the earlier objections to this approach likely still
apply, but we wanted to resubmit it for feedback in order to
restart the discussion to find an actionable direction as to
what sort of usable and more general approach could be found.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Dmitry Shmidt <dimitrysh@google.com>
Cc: devicetree@vger.kernel.org

Dmitry Shmidt (3):
  of: overlay_mgr: Add overlay manager driver
  of: overlay_mgr: Add ability to apply through sysfs entry
  of: overlay_mgr: Add ability to apply several hardware configurations

 .../devicetree/bindings/of/overlay_mgr.txt         |  32 +++++
 drivers/of/Kconfig                                 |  10 ++
 drivers/of/Makefile                                |   1 +
 drivers/of/overlay_mgr.c                           | 152 +++++++++++++++++++++
 4 files changed, 195 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/of/overlay_mgr.txt
 create mode 100644 drivers/of/overlay_mgr.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC][PATCH 1/3] of: overlay_mgr: Add overlay manager driver
  2017-10-10  5:34 [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments John Stultz
@ 2017-10-10  5:34 ` John Stultz
  2017-10-10  5:34 ` [RFC][PATCH 2/3] of: overlay_mgr: Add ability to apply through sysfs entry John Stultz
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2017-10-10  5:34 UTC (permalink / raw)
  To: lkml
  Cc: Dmitry Shmidt, Rob Herring, Mark Rutland, Frank Rowand,
	devicetree, John Stultz

From: Dmitry Shmidt <dimitrysh@google.com>

This patch adds a driver to manage applying pre-defined DT
overlay fragments at bootup.

The pre-defined DT fragments are specified via the main
overlay_mng entry which includes all the possible HW config setups.

Then kernel command line option is used to choose between them.

These entries are specified on kernel boot arguments as follows:
   overlay_mgr.overlay_dt_entry=hardware_cfg_0

Example DT entry:
    overlay_mgr {
        compatible = "linux,overlay_manager";
        hardware_cfg_0 {
            overlay@0 {
                    fragment@0 {
                            __overlay__ {
                            };
                    };
            };
            overlay@1 {
                    fragment@0 {
                            __overlay__ {
                            };
                    };
            };
        };
    };

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Dmitry Shmidt <dimitrysh@google.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Dmitry Shmidt <dimitrysh@google.com>
[jstultz: Reworded commit message]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 .../devicetree/bindings/of/overlay_mgr.txt         | 32 ++++++++
 drivers/of/Kconfig                                 | 10 +++
 drivers/of/Makefile                                |  1 +
 drivers/of/overlay_mgr.c                           | 90 ++++++++++++++++++++++
 4 files changed, 133 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/of/overlay_mgr.txt
 create mode 100644 drivers/of/overlay_mgr.c

diff --git a/Documentation/devicetree/bindings/of/overlay_mgr.txt b/Documentation/devicetree/bindings/of/overlay_mgr.txt
new file mode 100644
index 0000000..5f3ce4c
--- /dev/null
+++ b/Documentation/devicetree/bindings/of/overlay_mgr.txt
@@ -0,0 +1,32 @@
+overlay_mgr
+
+Required properties:
+- compatible: "linux,overlay_manager";
+
+Optional properties:
+- starts from the word "hardware": hardware_cfg_0
+
+These properties can be chosen from kernel command line:
+overlay_mgr.overlay_dt_entry=hardware_cfg_0
+DT contains main overlay_mng entry with all possible
+HW config setups. And then kernel command line option
+will allow to choose between them.
+
+Example:
+    overlay_mgr {
+        compatible = "linux,overlay_manager";
+        hardware_cfg_0 {
+            overlay@0 {
+                    fragment@0 {
+                            __overlay__ {
+                            };
+                    };
+            };
+            overlay@1 {
+                    fragment@0 {
+                            __overlay__ {
+                            };
+                    };
+            };
+        };
+    };
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index ba7b034..5132e41 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -112,4 +112,14 @@ config OF_OVERLAY
 config OF_NUMA
 	bool
 
+config OF_OVERLAY_MGR
+	bool "Enable Overlay manager"
+	default n
+	depends on OF_OVERLAY
+	help
+	  Enables Overlay manager - it accepts DT entry from command line
+	  overlay_mgr.overlay_dt_entry=<name> and applies all overlays in
+	  it to current DT. It is also possible to apply predefined DT
+	  entry on the fly by writing it to 'current_overlay' sysfs entry.
+
 endif # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index 97dc01c..e2b8afa 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -14,5 +14,6 @@ obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
 obj-$(CONFIG_OF_RESOLVE)  += resolver.o
 obj-$(CONFIG_OF_OVERLAY) += overlay.o
 obj-$(CONFIG_OF_NUMA) += of_numa.o
+obj-$(CONFIG_OF_OVERLAY_MGR) += overlay_mgr.o
 
 obj-$(CONFIG_OF_UNITTEST) += unittest-data/
diff --git a/drivers/of/overlay_mgr.c b/drivers/of/overlay_mgr.c
new file mode 100644
index 0000000..1fdeb0a
--- /dev/null
+++ b/drivers/of/overlay_mgr.c
@@ -0,0 +1,90 @@
+/*
+ * Overlay manager that allows to apply list of overlays from DT entry
+ *
+ * Copyright (C) 2016 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+static char *of_overlay_dt_entry;
+module_param_named(overlay_dt_entry, of_overlay_dt_entry, charp, 0644);
+
+static int of_overlay_mgr_apply_overlay(struct device_node *onp)
+{
+	int ret;
+
+	ret = of_overlay_create(onp);
+	if (ret < 0) {
+		pr_err("overlay_mgr: fail to create overlay: %d\n", ret);
+		of_node_put(onp);
+		return ret;
+	}
+	pr_info("overlay_mgr: %s overlay applied\n", onp->name);
+	return 0;
+}
+
+static int of_overlay_mgr_apply_dt(struct device *dev, char *dt_entry)
+{
+	struct device_node *enp = dev->of_node;
+	struct device_node *next;
+	struct device_node *prev = NULL;
+
+	if (!enp) {
+		pr_err("overlay_mgr: no dt entry\n");
+		return -ENODEV;
+	}
+
+	enp = of_get_child_by_name(enp, dt_entry);
+	if (!enp) {
+		pr_err("overlay_mgr: dt entry %s not found\n", dt_entry);
+		return -ENODEV;
+	}
+	pr_info("overlay_mgr: apply %s dt entry\n", enp->name);
+	while ((next = of_get_next_available_child(enp, prev)) != NULL) {
+		if (strncmp(next->name, "overlay", 7) == 0)
+			of_overlay_mgr_apply_overlay(next);
+		prev = next;
+	}
+	return 0;
+}
+
+static int of_overlay_mgr_probe(struct platform_device *pdev)
+{
+	if (!of_overlay_dt_entry)
+		return 0;
+	of_overlay_mgr_apply_dt(&pdev->dev, of_overlay_dt_entry);
+	return 0;
+}
+
+static const struct of_device_id of_overlay_mgr_match[] = {
+	{ .compatible = "linux,overlay_manager", },
+	{}
+};
+
+static struct platform_driver of_overlay_mgr_driver = {
+	.probe	= of_overlay_mgr_probe,
+	.driver	= {
+		.name = "overlay_manager",
+		.of_match_table = of_match_ptr(of_overlay_mgr_match),
+	},
+};
+
+static int __init of_overlay_mgr_init(void)
+{
+	return platform_driver_register(&of_overlay_mgr_driver);
+}
+
+postcore_initcall(of_overlay_mgr_init);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC][PATCH 2/3] of: overlay_mgr: Add ability to apply through sysfs entry
  2017-10-10  5:34 [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments John Stultz
  2017-10-10  5:34 ` [RFC][PATCH 1/3] of: overlay_mgr: Add overlay manager driver John Stultz
@ 2017-10-10  5:34 ` John Stultz
  2017-10-10  5:34 ` [RFC][PATCH 3/3] of: overlay_mgr: Add ability to apply several hardware configurations John Stultz
  2017-10-13 21:41   ` Rob Herring
  3 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2017-10-10  5:34 UTC (permalink / raw)
  To: lkml
  Cc: Dmitry Shmidt, Rob Herring, Mark Rutland, Frank Rowand,
	devicetree, John Stultz

From: Dmitry Shmidt <dimitrysh@google.com>

Allow pre-defined overlay_mgr DT fragments to be enabled on the
fly by writing to:
  /sys/devices/platform/overlay_mgr/current_overlay

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Dmitry Shmidt <dimitrysh@google.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Dmitry Shmidt <dimitrysh@google.com>
[jstultz: Slightly reworded commit message]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/of/overlay_mgr.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/of/overlay_mgr.c b/drivers/of/overlay_mgr.c
index 1fdeb0a..5a082be 100644
--- a/drivers/of/overlay_mgr.c
+++ b/drivers/of/overlay_mgr.c
@@ -18,10 +18,14 @@
 #include <linux/kernel.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/slab.h>
 
 static char *of_overlay_dt_entry;
 module_param_named(overlay_dt_entry, of_overlay_dt_entry, charp, 0644);
 
+static char *of_overlay_dt_apply;
+DEFINE_MUTEX(of_overlay_mgr_mutex);
+
 static int of_overlay_mgr_apply_overlay(struct device_node *onp)
 {
 	int ret;
@@ -61,8 +65,56 @@ static int of_overlay_mgr_apply_dt(struct device *dev, char *dt_entry)
 	return 0;
 }
 
+static ssize_t current_overlay_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	size_t len;
+
+	mutex_lock(&of_overlay_mgr_mutex);
+	if (!of_overlay_dt_apply) {
+		mutex_unlock(&of_overlay_mgr_mutex);
+		return 0;
+	}
+	len = strlen(of_overlay_dt_apply);
+	if (len >= PAGE_SIZE)
+		len = PAGE_SIZE - 1;
+	memcpy(buf, of_overlay_dt_apply, len + 1);
+	mutex_unlock(&of_overlay_mgr_mutex);
+	return len;
+}
+
+static ssize_t current_overlay_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t size)
+{
+	mutex_lock(&of_overlay_mgr_mutex);
+	kfree(of_overlay_dt_apply);
+	of_overlay_dt_apply = kmalloc(size, GFP_KERNEL);
+	if (!of_overlay_dt_apply) {
+		pr_err("overlay_mgr: fail to allocate memory\n");
+		mutex_unlock(&of_overlay_mgr_mutex);
+		return 0;
+	}
+	memcpy(of_overlay_dt_apply, buf, size);
+	of_overlay_dt_apply[size - 1] = '\0';
+
+	if (of_overlay_mgr_apply_dt(dev, of_overlay_dt_apply)) {
+		kfree(of_overlay_dt_apply);
+		of_overlay_dt_apply = NULL;
+		size = 0;
+	}
+	mutex_unlock(&of_overlay_mgr_mutex);
+	return size;
+}
+
+static DEVICE_ATTR(current_overlay, 0644, current_overlay_show,
+		   current_overlay_store);
+
 static int of_overlay_mgr_probe(struct platform_device *pdev)
 {
+	if (device_create_file(&pdev->dev, &dev_attr_current_overlay))
+		pr_err("overlay_mgr: fail to register apply entry\n");
+
 	if (!of_overlay_dt_entry)
 		return 0;
 	of_overlay_mgr_apply_dt(&pdev->dev, of_overlay_dt_entry);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC][PATCH 3/3] of: overlay_mgr: Add ability to apply several hardware configurations
  2017-10-10  5:34 [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments John Stultz
  2017-10-10  5:34 ` [RFC][PATCH 1/3] of: overlay_mgr: Add overlay manager driver John Stultz
  2017-10-10  5:34 ` [RFC][PATCH 2/3] of: overlay_mgr: Add ability to apply through sysfs entry John Stultz
@ 2017-10-10  5:34 ` John Stultz
  2017-10-13 21:41   ` Rob Herring
  3 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2017-10-10  5:34 UTC (permalink / raw)
  To: lkml
  Cc: Dmitry Shmidt, Rob Herring, Mark Rutland, Frank Rowand,
	devicetree, John Stultz

From: Dmitry Shmidt <dimitrysh@google.com>

This adds the ability to specify multiple DT overlay
configurations, which are predefined in the overlay_mgr entry,
at once via the boot arguments.

Example:
  overlay_mgr.overlay_dt_entry=hardware_cfg_0,hardware_cfg_1,...

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Dmitry Shmidt <dimitrysh@google.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Dmitry Shmidt <dimitrysh@google.com>
[jstultz: reworded commit message]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/of/overlay_mgr.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/of/overlay_mgr.c b/drivers/of/overlay_mgr.c
index 5a082be..6b15f7c 100644
--- a/drivers/of/overlay_mgr.c
+++ b/drivers/of/overlay_mgr.c
@@ -112,12 +112,22 @@ static DEVICE_ATTR(current_overlay, 0644, current_overlay_show,
 
 static int of_overlay_mgr_probe(struct platform_device *pdev)
 {
+	char *cur_entry;
+	char *next_entry;
+
 	if (device_create_file(&pdev->dev, &dev_attr_current_overlay))
 		pr_err("overlay_mgr: fail to register apply entry\n");
 
 	if (!of_overlay_dt_entry)
 		return 0;
-	of_overlay_mgr_apply_dt(&pdev->dev, of_overlay_dt_entry);
+	next_entry = of_overlay_dt_entry;
+	do {
+		cur_entry = next_entry;
+		next_entry = strchr(cur_entry, ',');
+		if (next_entry)
+			*next_entry++ = '\0';
+		of_overlay_mgr_apply_dt(&pdev->dev, cur_entry);
+	} while (next_entry);
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments
@ 2017-10-13 21:41   ` Rob Herring
  0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2017-10-13 21:41 UTC (permalink / raw)
  To: John Stultz; +Cc: lkml, Mark Rutland, Frank Rowand, Dmitry Shmidt, devicetree

On Mon, Oct 09, 2017 at 10:34:17PM -0700, John Stultz wrote:
> In working with the HiKey and HiKey960 as targets for AOSP,
> Dmitry developed the following overlay manager driver, which
> allows a number of pre-determined DT overlay configurations to
> be defined, and the configurations to be enabled at boot time
> via a kernel boot argument.
> 
> This has been submitted before, but while the earlier discussion
> didn't really resolve to any sort of actionable direction, this
> issue cropped up again and was a major discussion topic at the
> Linux Plumbers Conference Android Microconference, so I suspect
> it is worth revisiting this solution again.

The Treble way to handle this is it is the bootloader's problem, right?


> The overall use case is being able to configure devboards that
> support a number of different mezzanine peripherals which
> unfortunately cannot be probed. Some example mezzanines are
> LCD panels or sensor hubs, as well as other options.

There's probably some usecases for putting the overlay into the base 
DT, but I don't think mezzanine boards is one of them especially for dev 
boards where we may want to share overlays.

I could see it in cases where you have 2nd source components for a 
product and want to select one.

> The new functionality this driver provides is a mechanism to
> specify multiple pre-determined dt overlay fragments in a
> dtb file, and providing a way to select which dt fragments
> should be applied via the kernel boot argument.
> 
> The desire to use a kernel boot-argument as the selection
> mechanism, comes from the Android Boot Image format not handling
> dtbs independently. Usually with Android, the dtb is appended
> to the kernel image, and modifying that is much more difficult
> then changing the boot argugments. There is also a usability
> argument that using a kernel command option to select
> pre-defined entries is simpler for users to navigate.

Doesn't Treble address the handling of overlays? Of course, none of what 
has been outlined for Treble has been reviewed upstream either... The 
concept seems fine, it's the vendor implementations that worry me.

As far as usability, a RPi user would probably say listing 
overlays in a text file is easiest as that's how the RPi firmware works.

> Also, since the mezzanines are unable to be probed, we cannot
> use other solutions, like having the bootloader specify
> additional dtb overlays to the kernel.

Not sure I follow.

Someone has to decide what to put on the kernel command line. If you are 
setting the overlay in the command line at bootimage build time or in 
the bootloader, then you could just apply the overlay at that point in 
time. Maybe it's slightly easier to change the kernel command line than 
change the dtb. 

Rob

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments
@ 2017-10-13 21:41   ` Rob Herring
  0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2017-10-13 21:41 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Mark Rutland, Frank Rowand, Dmitry Shmidt,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On Mon, Oct 09, 2017 at 10:34:17PM -0700, John Stultz wrote:
> In working with the HiKey and HiKey960 as targets for AOSP,
> Dmitry developed the following overlay manager driver, which
> allows a number of pre-determined DT overlay configurations to
> be defined, and the configurations to be enabled at boot time
> via a kernel boot argument.
> 
> This has been submitted before, but while the earlier discussion
> didn't really resolve to any sort of actionable direction, this
> issue cropped up again and was a major discussion topic at the
> Linux Plumbers Conference Android Microconference, so I suspect
> it is worth revisiting this solution again.

The Treble way to handle this is it is the bootloader's problem, right?


> The overall use case is being able to configure devboards that
> support a number of different mezzanine peripherals which
> unfortunately cannot be probed. Some example mezzanines are
> LCD panels or sensor hubs, as well as other options.

There's probably some usecases for putting the overlay into the base 
DT, but I don't think mezzanine boards is one of them especially for dev 
boards where we may want to share overlays.

I could see it in cases where you have 2nd source components for a 
product and want to select one.

> The new functionality this driver provides is a mechanism to
> specify multiple pre-determined dt overlay fragments in a
> dtb file, and providing a way to select which dt fragments
> should be applied via the kernel boot argument.
> 
> The desire to use a kernel boot-argument as the selection
> mechanism, comes from the Android Boot Image format not handling
> dtbs independently. Usually with Android, the dtb is appended
> to the kernel image, and modifying that is much more difficult
> then changing the boot argugments. There is also a usability
> argument that using a kernel command option to select
> pre-defined entries is simpler for users to navigate.

Doesn't Treble address the handling of overlays? Of course, none of what 
has been outlined for Treble has been reviewed upstream either... The 
concept seems fine, it's the vendor implementations that worry me.

As far as usability, a RPi user would probably say listing 
overlays in a text file is easiest as that's how the RPi firmware works.

> Also, since the mezzanines are unable to be probed, we cannot
> use other solutions, like having the bootloader specify
> additional dtb overlays to the kernel.

Not sure I follow.

Someone has to decide what to put on the kernel command line. If you are 
setting the overlay in the command line at bootimage build time or in 
the bootloader, then you could just apply the overlay at that point in 
time. Maybe it's slightly easier to change the kernel command line than 
change the dtb. 

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments
  2017-10-13 21:41   ` Rob Herring
@ 2017-10-13 23:51     ` John Stultz
  -1 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2017-10-13 23:51 UTC (permalink / raw)
  To: Rob Herring
  Cc: lkml, Mark Rutland, Frank Rowand, Dmitry Shmidt,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Fri, Oct 13, 2017 at 2:41 PM, Rob Herring <robh@kernel.org> wrote:
> On Mon, Oct 09, 2017 at 10:34:17PM -0700, John Stultz wrote:
>> In working with the HiKey and HiKey960 as targets for AOSP,
>> Dmitry developed the following overlay manager driver, which
>> allows a number of pre-determined DT overlay configurations to
>> be defined, and the configurations to be enabled at boot time
>> via a kernel boot argument.
>>
>> This has been submitted before, but while the earlier discussion
>> didn't really resolve to any sort of actionable direction, this
>> issue cropped up again and was a major discussion topic at the
>> Linux Plumbers Conference Android Microconference, so I suspect
>> it is worth revisiting this solution again.
>
> The Treble way to handle this is it is the bootloader's problem, right?

I believe so, though with the treble approach, the device
configuration is still somewhat fixed. Just that configuration is
spread out between multiple partitions. Its a bit of a separate issue.

>> The overall use case is being able to configure devboards that
>> support a number of different mezzanine peripherals which
>> unfortunately cannot be probed. Some example mezzanines are
>> LCD panels or sensor hubs, as well as other options.
>
> There's probably some usecases for putting the overlay into the base
> DT, but I don't think mezzanine boards is one of them especially for dev
> boards where we may want to share overlays.
>
> I could see it in cases where you have 2nd source components for a
> product and want to select one.
>
>> The new functionality this driver provides is a mechanism to
>> specify multiple pre-determined dt overlay fragments in a
>> dtb file, and providing a way to select which dt fragments
>> should be applied via the kernel boot argument.
>>
>> The desire to use a kernel boot-argument as the selection
>> mechanism, comes from the Android Boot Image format not handling
>> dtbs independently. Usually with Android, the dtb is appended
>> to the kernel image, and modifying that is much more difficult
>> then changing the boot argugments. There is also a usability
>> argument that using a kernel command option to select
>> pre-defined entries is simpler for users to navigate.
>
> Doesn't Treble address the handling of overlays? Of course, none of what
> has been outlined for Treble has been reviewed upstream either... The
> concept seems fine, it's the vendor implementations that worry me.

Again, I'm not sure how that connects to the proposal here. Having
treble's multiple overlays being picked up in a standard fashion is
nice, but how does that translate into a user booting a board with one
mezzanine and then change the mezzanine and be able to get the new
mezzanine to work with a minimal amount of fuss.

> As far as usability, a RPi user would probably say listing
> overlays in a text file is easiest as that's how the RPi firmware works.

Right, and I agree that usability wise, its very similar to the
cmdline arguments. But that requires firmware that keeps track of
various overlays and has some small filesystem where that config file
can be stored and easily updated.

>> Also, since the mezzanines are unable to be probed, we cannot
>> use other solutions, like having the bootloader specify
>> additional dtb overlays to the kernel.
>
> Not sure I follow.
>
> Someone has to decide what to put on the kernel command line. If you are
> setting the overlay in the command line at bootimage build time or in
> the bootloader, then you could just apply the overlay at that point in
> time. Maybe it's slightly easier to change the kernel command line than
> change the dtb.

Not, its not slightly easier, its much *much* easier to tweak the boot
arguments. Since the dtb is appended to the kernel image w/ the
abootimg format, its not simple at all to cut one out and replace it.

While the cmdline is trivial to do with the abootimg command:
abootimg -x boot.img
vi bootimg.cfg
abootimg -u boot.img -f ./bootimg.cfg

and re-flash.  This doesn't require access to the kernel source or
rebuilding anything.

It seems you're suggesting that there be some sort of special overlay
partition which users have to flash with pre-built images containing
the appropriate overlay dtbs, so that something like the treble
overlay-per-partition approach could be used.

And in the case with hikey/hikey960, this may be doable, as we can
modify the UEFI source, but it would also require users to have all
the various overlay images to flash as well, which adds complexity.
Additionally on other boards which don't have open firmware, such a
solution may not be feasible. The benefit to this approach is it
doesn't require such standardized firmware behavior.

I really do think there is value in the simplicity of this approach.

thanks
-john

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments
@ 2017-10-13 23:51     ` John Stultz
  0 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2017-10-13 23:51 UTC (permalink / raw)
  To: Rob Herring
  Cc: lkml, Mark Rutland, Frank Rowand, Dmitry Shmidt,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Fri, Oct 13, 2017 at 2:41 PM, Rob Herring <robh@kernel.org> wrote:
> On Mon, Oct 09, 2017 at 10:34:17PM -0700, John Stultz wrote:
>> In working with the HiKey and HiKey960 as targets for AOSP,
>> Dmitry developed the following overlay manager driver, which
>> allows a number of pre-determined DT overlay configurations to
>> be defined, and the configurations to be enabled at boot time
>> via a kernel boot argument.
>>
>> This has been submitted before, but while the earlier discussion
>> didn't really resolve to any sort of actionable direction, this
>> issue cropped up again and was a major discussion topic at the
>> Linux Plumbers Conference Android Microconference, so I suspect
>> it is worth revisiting this solution again.
>
> The Treble way to handle this is it is the bootloader's problem, right?

I believe so, though with the treble approach, the device
configuration is still somewhat fixed. Just that configuration is
spread out between multiple partitions. Its a bit of a separate issue.

>> The overall use case is being able to configure devboards that
>> support a number of different mezzanine peripherals which
>> unfortunately cannot be probed. Some example mezzanines are
>> LCD panels or sensor hubs, as well as other options.
>
> There's probably some usecases for putting the overlay into the base
> DT, but I don't think mezzanine boards is one of them especially for dev
> boards where we may want to share overlays.
>
> I could see it in cases where you have 2nd source components for a
> product and want to select one.
>
>> The new functionality this driver provides is a mechanism to
>> specify multiple pre-determined dt overlay fragments in a
>> dtb file, and providing a way to select which dt fragments
>> should be applied via the kernel boot argument.
>>
>> The desire to use a kernel boot-argument as the selection
>> mechanism, comes from the Android Boot Image format not handling
>> dtbs independently. Usually with Android, the dtb is appended
>> to the kernel image, and modifying that is much more difficult
>> then changing the boot argugments. There is also a usability
>> argument that using a kernel command option to select
>> pre-defined entries is simpler for users to navigate.
>
> Doesn't Treble address the handling of overlays? Of course, none of what
> has been outlined for Treble has been reviewed upstream either... The
> concept seems fine, it's the vendor implementations that worry me.

Again, I'm not sure how that connects to the proposal here. Having
treble's multiple overlays being picked up in a standard fashion is
nice, but how does that translate into a user booting a board with one
mezzanine and then change the mezzanine and be able to get the new
mezzanine to work with a minimal amount of fuss.

> As far as usability, a RPi user would probably say listing
> overlays in a text file is easiest as that's how the RPi firmware works.

Right, and I agree that usability wise, its very similar to the
cmdline arguments. But that requires firmware that keeps track of
various overlays and has some small filesystem where that config file
can be stored and easily updated.

>> Also, since the mezzanines are unable to be probed, we cannot
>> use other solutions, like having the bootloader specify
>> additional dtb overlays to the kernel.
>
> Not sure I follow.
>
> Someone has to decide what to put on the kernel command line. If you are
> setting the overlay in the command line at bootimage build time or in
> the bootloader, then you could just apply the overlay at that point in
> time. Maybe it's slightly easier to change the kernel command line than
> change the dtb.

Not, its not slightly easier, its much *much* easier to tweak the boot
arguments. Since the dtb is appended to the kernel image w/ the
abootimg format, its not simple at all to cut one out and replace it.

While the cmdline is trivial to do with the abootimg command:
abootimg -x boot.img
vi bootimg.cfg
abootimg -u boot.img -f ./bootimg.cfg

and re-flash.  This doesn't require access to the kernel source or
rebuilding anything.

It seems you're suggesting that there be some sort of special overlay
partition which users have to flash with pre-built images containing
the appropriate overlay dtbs, so that something like the treble
overlay-per-partition approach could be used.

And in the case with hikey/hikey960, this may be doable, as we can
modify the UEFI source, but it would also require users to have all
the various overlay images to flash as well, which adds complexity.
Additionally on other boards which don't have open firmware, such a
solution may not be feasible. The benefit to this approach is it
doesn't require such standardized firmware behavior.

I really do think there is value in the simplicity of this approach.

thanks
-john

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments
  2017-10-13 23:51     ` John Stultz
@ 2017-10-17 19:46       ` Rob Herring
  -1 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2017-10-17 19:46 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Mark Rutland, Frank Rowand, Dmitry Shmidt,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Fri, Oct 13, 2017 at 04:51:03PM -0700, John Stultz wrote:
> On Fri, Oct 13, 2017 at 2:41 PM, Rob Herring <robh@kernel.org> wrote:
> > On Mon, Oct 09, 2017 at 10:34:17PM -0700, John Stultz wrote:
> >> In working with the HiKey and HiKey960 as targets for AOSP,
> >> Dmitry developed the following overlay manager driver, which
> >> allows a number of pre-determined DT overlay configurations to
> >> be defined, and the configurations to be enabled at boot time
> >> via a kernel boot argument.
> >>
> >> This has been submitted before, but while the earlier discussion
> >> didn't really resolve to any sort of actionable direction, this
> >> issue cropped up again and was a major discussion topic at the
> >> Linux Plumbers Conference Android Microconference, so I suspect
> >> it is worth revisiting this solution again.
> >
> > The Treble way to handle this is it is the bootloader's problem, right?
> 
> I believe so, though with the treble approach, the device
> configuration is still somewhat fixed. Just that configuration is
> spread out between multiple partitions. Its a bit of a separate issue.
> 
> >> The overall use case is being able to configure devboards that
> >> support a number of different mezzanine peripherals which
> >> unfortunately cannot be probed. Some example mezzanines are
> >> LCD panels or sensor hubs, as well as other options.
> >
> > There's probably some usecases for putting the overlay into the base
> > DT, but I don't think mezzanine boards is one of them especially for dev
> > boards where we may want to share overlays.
> >
> > I could see it in cases where you have 2nd source components for a
> > product and want to select one.
> >
> >> The new functionality this driver provides is a mechanism to
> >> specify multiple pre-determined dt overlay fragments in a
> >> dtb file, and providing a way to select which dt fragments
> >> should be applied via the kernel boot argument.
> >>
> >> The desire to use a kernel boot-argument as the selection
> >> mechanism, comes from the Android Boot Image format not handling
> >> dtbs independently. Usually with Android, the dtb is appended
> >> to the kernel image, and modifying that is much more difficult
> >> then changing the boot argugments. There is also a usability
> >> argument that using a kernel command option to select
> >> pre-defined entries is simpler for users to navigate.
> >
> > Doesn't Treble address the handling of overlays? Of course, none of what
> > has been outlined for Treble has been reviewed upstream either... The
> > concept seems fine, it's the vendor implementations that worry me.
> 
> Again, I'm not sure how that connects to the proposal here. Having
> treble's multiple overlays being picked up in a standard fashion is
> nice, but how does that translate into a user booting a board with one
> mezzanine and then change the mezzanine and be able to get the new
> mezzanine to work with a minimal amount of fuss.

Perhaps I need to study how exactly treble systems will select all their 
overlays. Supporting different panels or other 2nd source components is 
a pretty common problem in production devices. Those aren't probe-able 
either and folks don't want to flash different images for each 
combination of h/w.

> > As far as usability, a RPi user would probably say listing
> > overlays in a text file is easiest as that's how the RPi firmware works.
> 
> Right, and I agree that usability wise, its very similar to the
> cmdline arguments. But that requires firmware that keeps track of
> various overlays and has some small filesystem where that config file
> can be stored and easily updated.
> 
> >> Also, since the mezzanines are unable to be probed, we cannot
> >> use other solutions, like having the bootloader specify
> >> additional dtb overlays to the kernel.
> >
> > Not sure I follow.
> >
> > Someone has to decide what to put on the kernel command line. If you are
> > setting the overlay in the command line at bootimage build time or in
> > the bootloader, then you could just apply the overlay at that point in
> > time. Maybe it's slightly easier to change the kernel command line than
> > change the dtb.
> 
> Not, its not slightly easier, its much *much* easier to tweak the boot
> arguments. Since the dtb is appended to the kernel image w/ the
> abootimg format, its not simple at all to cut one out and replace it.

Appended dtb was to support legacy non-DT aware bootloaders. It never 
should have been carried over to arm64.

> While the cmdline is trivial to do with the abootimg command:
> abootimg -x boot.img
> vi bootimg.cfg
> abootimg -u boot.img -f ./bootimg.cfg

It's easy because there is a tool to do so, not because a the kernel 
command line is inheritly easier to modify than a dtb.

It would be trivial to write a tool or add to this one the code to find 
the dtb within the image and modify it. That's what every bootloader can 
do.

Of course, abootimg is a horrible creation IMO. Why Android can't use 
something called a filesystem for the boot partition is beyond me. Then 
modifying it would not require special tools that could only modify 
whatever someone had the itch to modify. And combining the kernel, dtb, 
and overlays into a single file would not be necessary.

> and re-flash.  This doesn't require access to the kernel source or
> rebuilding anything.
> 
> It seems you're suggesting that there be some sort of special overlay
> partition which users have to flash with pre-built images containing
> the appropriate overlay dtbs, so that something like the treble
> overlay-per-partition approach could be used.

I'm not really suggesting anything. I'm not going to take something that 
*only* solves your usecase of apply an overlay embedded in a base dtb 
based on the kernel command line. I have no issue really with either one 
of those. What I don't want is another "overlay manager" for each 
usecase. And sorry, you don't win for being first (you're not really).
All this I said before.

> And in the case with hikey/hikey960, this may be doable, as we can
> modify the UEFI source, but it would also require users to have all
> the various overlay images to flash as well, which adds complexity.
> Additionally on other boards which don't have open firmware, such a
> solution may not be feasible. The benefit to this approach is it
> doesn't require such standardized firmware behavior.

Unless there's some great, overwhelming demand to support such devices 
with closed firmware, there's no benefit to me to care. What board do we 
have with closed firmware that needs this?

Rob

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments
@ 2017-10-17 19:46       ` Rob Herring
  0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2017-10-17 19:46 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Mark Rutland, Frank Rowand, Dmitry Shmidt,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Fri, Oct 13, 2017 at 04:51:03PM -0700, John Stultz wrote:
> On Fri, Oct 13, 2017 at 2:41 PM, Rob Herring <robh@kernel.org> wrote:
> > On Mon, Oct 09, 2017 at 10:34:17PM -0700, John Stultz wrote:
> >> In working with the HiKey and HiKey960 as targets for AOSP,
> >> Dmitry developed the following overlay manager driver, which
> >> allows a number of pre-determined DT overlay configurations to
> >> be defined, and the configurations to be enabled at boot time
> >> via a kernel boot argument.
> >>
> >> This has been submitted before, but while the earlier discussion
> >> didn't really resolve to any sort of actionable direction, this
> >> issue cropped up again and was a major discussion topic at the
> >> Linux Plumbers Conference Android Microconference, so I suspect
> >> it is worth revisiting this solution again.
> >
> > The Treble way to handle this is it is the bootloader's problem, right?
> 
> I believe so, though with the treble approach, the device
> configuration is still somewhat fixed. Just that configuration is
> spread out between multiple partitions. Its a bit of a separate issue.
> 
> >> The overall use case is being able to configure devboards that
> >> support a number of different mezzanine peripherals which
> >> unfortunately cannot be probed. Some example mezzanines are
> >> LCD panels or sensor hubs, as well as other options.
> >
> > There's probably some usecases for putting the overlay into the base
> > DT, but I don't think mezzanine boards is one of them especially for dev
> > boards where we may want to share overlays.
> >
> > I could see it in cases where you have 2nd source components for a
> > product and want to select one.
> >
> >> The new functionality this driver provides is a mechanism to
> >> specify multiple pre-determined dt overlay fragments in a
> >> dtb file, and providing a way to select which dt fragments
> >> should be applied via the kernel boot argument.
> >>
> >> The desire to use a kernel boot-argument as the selection
> >> mechanism, comes from the Android Boot Image format not handling
> >> dtbs independently. Usually with Android, the dtb is appended
> >> to the kernel image, and modifying that is much more difficult
> >> then changing the boot argugments. There is also a usability
> >> argument that using a kernel command option to select
> >> pre-defined entries is simpler for users to navigate.
> >
> > Doesn't Treble address the handling of overlays? Of course, none of what
> > has been outlined for Treble has been reviewed upstream either... The
> > concept seems fine, it's the vendor implementations that worry me.
> 
> Again, I'm not sure how that connects to the proposal here. Having
> treble's multiple overlays being picked up in a standard fashion is
> nice, but how does that translate into a user booting a board with one
> mezzanine and then change the mezzanine and be able to get the new
> mezzanine to work with a minimal amount of fuss.

Perhaps I need to study how exactly treble systems will select all their 
overlays. Supporting different panels or other 2nd source components is 
a pretty common problem in production devices. Those aren't probe-able 
either and folks don't want to flash different images for each 
combination of h/w.

> > As far as usability, a RPi user would probably say listing
> > overlays in a text file is easiest as that's how the RPi firmware works.
> 
> Right, and I agree that usability wise, its very similar to the
> cmdline arguments. But that requires firmware that keeps track of
> various overlays and has some small filesystem where that config file
> can be stored and easily updated.
> 
> >> Also, since the mezzanines are unable to be probed, we cannot
> >> use other solutions, like having the bootloader specify
> >> additional dtb overlays to the kernel.
> >
> > Not sure I follow.
> >
> > Someone has to decide what to put on the kernel command line. If you are
> > setting the overlay in the command line at bootimage build time or in
> > the bootloader, then you could just apply the overlay at that point in
> > time. Maybe it's slightly easier to change the kernel command line than
> > change the dtb.
> 
> Not, its not slightly easier, its much *much* easier to tweak the boot
> arguments. Since the dtb is appended to the kernel image w/ the
> abootimg format, its not simple at all to cut one out and replace it.

Appended dtb was to support legacy non-DT aware bootloaders. It never 
should have been carried over to arm64.

> While the cmdline is trivial to do with the abootimg command:
> abootimg -x boot.img
> vi bootimg.cfg
> abootimg -u boot.img -f ./bootimg.cfg

It's easy because there is a tool to do so, not because a the kernel 
command line is inheritly easier to modify than a dtb.

It would be trivial to write a tool or add to this one the code to find 
the dtb within the image and modify it. That's what every bootloader can 
do.

Of course, abootimg is a horrible creation IMO. Why Android can't use 
something called a filesystem for the boot partition is beyond me. Then 
modifying it would not require special tools that could only modify 
whatever someone had the itch to modify. And combining the kernel, dtb, 
and overlays into a single file would not be necessary.

> and re-flash.  This doesn't require access to the kernel source or
> rebuilding anything.
> 
> It seems you're suggesting that there be some sort of special overlay
> partition which users have to flash with pre-built images containing
> the appropriate overlay dtbs, so that something like the treble
> overlay-per-partition approach could be used.

I'm not really suggesting anything. I'm not going to take something that 
*only* solves your usecase of apply an overlay embedded in a base dtb 
based on the kernel command line. I have no issue really with either one 
of those. What I don't want is another "overlay manager" for each 
usecase. And sorry, you don't win for being first (you're not really).
All this I said before.

> And in the case with hikey/hikey960, this may be doable, as we can
> modify the UEFI source, but it would also require users to have all
> the various overlay images to flash as well, which adds complexity.
> Additionally on other boards which don't have open firmware, such a
> solution may not be feasible. The benefit to this approach is it
> doesn't require such standardized firmware behavior.

Unless there's some great, overwhelming demand to support such devices 
with closed firmware, there's no benefit to me to care. What board do we 
have with closed firmware that needs this?

Rob

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments
  2017-10-17 19:46       ` Rob Herring
@ 2017-10-17 21:20         ` John Stultz
  -1 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2017-10-17 21:20 UTC (permalink / raw)
  To: Rob Herring
  Cc: lkml, Mark Rutland, Frank Rowand, Dmitry Shmidt,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Tue, Oct 17, 2017 at 12:46 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Oct 13, 2017 at 04:51:03PM -0700, John Stultz wrote:
>> On Fri, Oct 13, 2017 at 2:41 PM, Rob Herring <robh@kernel.org> wrote:
>> > Someone has to decide what to put on the kernel command line. If you are
>> > setting the overlay in the command line at bootimage build time or in
>> > the bootloader, then you could just apply the overlay at that point in
>> > time. Maybe it's slightly easier to change the kernel command line than
>> > change the dtb.
>>
>> Not, its not slightly easier, its much *much* easier to tweak the boot
>> arguments. Since the dtb is appended to the kernel image w/ the
>> abootimg format, its not simple at all to cut one out and replace it.
>
> Appended dtb was to support legacy non-DT aware bootloaders. It never
> should have been carried over to arm64.
>
>> While the cmdline is trivial to do with the abootimg command:
>> abootimg -x boot.img
>> vi bootimg.cfg
>> abootimg -u boot.img -f ./bootimg.cfg
>
> It's easy because there is a tool to do so, not because a the kernel
> command line is inheritly easier to modify than a dtb.

Maybe not in some theoretical sense, but practically it very much is
easier. There are packaged tools available on most distros, user
familiarity, etc.


A bit of a tangent, but part of the reason why appended dtbs (despite
being only for legacy platforms) are often used is because they make
things simpler. At least for the boards I use, dtbs aren't static
enough to be considered firmware. If I've got a new kernel to test, I
usually have to copy a dtb over too. Managing separate files that are
effectively tied together is a practical pain.  With appended dtbs I
never have to guess if some failure I'm seeing is because I forgot a
step w/ the dtb.

Now, maybe with boards like hikey and others getting upstreamed, there
will be less dts churn there and I can happily forget to build and
copy over new dtbs. But I'm not sure if that really is the common case
yet.


> It would be trivial to write a tool or add to this one the code to find
> the dtb within the image and modify it. That's what every bootloader can
> do.

But would such a tool be usable? Or capable of being shared between boards?
How does one enumerate or distribute fragments?

These are all extra complexities with the pure dtb modification
methods, which the embedded fragment + boot arg selection avoids.

> Of course, abootimg is a horrible creation IMO. Why Android can't use
> something called a filesystem for the boot partition is beyond me. Then
> modifying it would not require special tools that could only modify
> whatever someone had the itch to modify. And combining the kernel, dtb,
> and overlays into a single file would not be necessary.

While I agree the extensibility of abootimg (particularly in the dtb
era) has been restrictive, I personally find it reasonably nice to
work with.  Being able to trivially modify the kernel and cmdlines w/o
having to have root permissions to mount images is pretty nice. FAT
filesystems can be similarly tweaked w/ mtools, but then you need a
secondary bootloader, etc, and tweaking grub menus isn't as simple as
the abootimg config file.


>> and re-flash.  This doesn't require access to the kernel source or
>> rebuilding anything.
>>
>> It seems you're suggesting that there be some sort of special overlay
>> partition which users have to flash with pre-built images containing
>> the appropriate overlay dtbs, so that something like the treble
>> overlay-per-partition approach could be used.
>
> I'm not really suggesting anything. I'm not going to take something that
> *only* solves your usecase of apply an overlay embedded in a base dtb
> based on the kernel command line. I have no issue really with either one
> of those. What I don't want is another "overlay manager" for each
> usecase. And sorry, you don't win for being first (you're not really).
> All this I said before.

So, I'm not actually asking anyone to take anything here, I'm just
trying to reopen the conversation that hasn't gone very far, so we can
find some direction to take.

I've not followed the discussion super closely here, so apologies if
I'm off base here.  But it does seem there's a number of similar
efforts, and it seems no one has gotten enough momentum to make a real
push upstream. It seems its just easier for everyone to keep their own
approach in their own tree and not to bother.  At one end, sure,
keeping half baked ideas out of mainline until they really sort
themselves out is fine, but at the other extreme you risk the
half-baked ideas calcifying in their own tree and then you have more
Android (or whatever variant) specific cruft to shake your fist at.
:)

How do we get to a shared solution here where folks are engaging with upstream?

Even an "I like approach B, go make that work" would help here.

>> And in the case with hikey/hikey960, this may be doable, as we can
>> modify the UEFI source, but it would also require users to have all
>> the various overlay images to flash as well, which adds complexity.
>> Additionally on other boards which don't have open firmware, such a
>> solution may not be feasible. The benefit to this approach is it
>> doesn't require such standardized firmware behavior.
>
> Unless there's some great, overwhelming demand to support such devices
> with closed firmware, there's no benefit to me to care. What board do we
> have with closed firmware that needs this?

Just across the 96boards we don't have open firmware on all the boards.

thanks
-john

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments
@ 2017-10-17 21:20         ` John Stultz
  0 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2017-10-17 21:20 UTC (permalink / raw)
  To: Rob Herring
  Cc: lkml, Mark Rutland, Frank Rowand, Dmitry Shmidt,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Tue, Oct 17, 2017 at 12:46 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Fri, Oct 13, 2017 at 04:51:03PM -0700, John Stultz wrote:
>> On Fri, Oct 13, 2017 at 2:41 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> > Someone has to decide what to put on the kernel command line. If you are
>> > setting the overlay in the command line at bootimage build time or in
>> > the bootloader, then you could just apply the overlay at that point in
>> > time. Maybe it's slightly easier to change the kernel command line than
>> > change the dtb.
>>
>> Not, its not slightly easier, its much *much* easier to tweak the boot
>> arguments. Since the dtb is appended to the kernel image w/ the
>> abootimg format, its not simple at all to cut one out and replace it.
>
> Appended dtb was to support legacy non-DT aware bootloaders. It never
> should have been carried over to arm64.
>
>> While the cmdline is trivial to do with the abootimg command:
>> abootimg -x boot.img
>> vi bootimg.cfg
>> abootimg -u boot.img -f ./bootimg.cfg
>
> It's easy because there is a tool to do so, not because a the kernel
> command line is inheritly easier to modify than a dtb.

Maybe not in some theoretical sense, but practically it very much is
easier. There are packaged tools available on most distros, user
familiarity, etc.


A bit of a tangent, but part of the reason why appended dtbs (despite
being only for legacy platforms) are often used is because they make
things simpler. At least for the boards I use, dtbs aren't static
enough to be considered firmware. If I've got a new kernel to test, I
usually have to copy a dtb over too. Managing separate files that are
effectively tied together is a practical pain.  With appended dtbs I
never have to guess if some failure I'm seeing is because I forgot a
step w/ the dtb.

Now, maybe with boards like hikey and others getting upstreamed, there
will be less dts churn there and I can happily forget to build and
copy over new dtbs. But I'm not sure if that really is the common case
yet.


> It would be trivial to write a tool or add to this one the code to find
> the dtb within the image and modify it. That's what every bootloader can
> do.

But would such a tool be usable? Or capable of being shared between boards?
How does one enumerate or distribute fragments?

These are all extra complexities with the pure dtb modification
methods, which the embedded fragment + boot arg selection avoids.

> Of course, abootimg is a horrible creation IMO. Why Android can't use
> something called a filesystem for the boot partition is beyond me. Then
> modifying it would not require special tools that could only modify
> whatever someone had the itch to modify. And combining the kernel, dtb,
> and overlays into a single file would not be necessary.

While I agree the extensibility of abootimg (particularly in the dtb
era) has been restrictive, I personally find it reasonably nice to
work with.  Being able to trivially modify the kernel and cmdlines w/o
having to have root permissions to mount images is pretty nice. FAT
filesystems can be similarly tweaked w/ mtools, but then you need a
secondary bootloader, etc, and tweaking grub menus isn't as simple as
the abootimg config file.


>> and re-flash.  This doesn't require access to the kernel source or
>> rebuilding anything.
>>
>> It seems you're suggesting that there be some sort of special overlay
>> partition which users have to flash with pre-built images containing
>> the appropriate overlay dtbs, so that something like the treble
>> overlay-per-partition approach could be used.
>
> I'm not really suggesting anything. I'm not going to take something that
> *only* solves your usecase of apply an overlay embedded in a base dtb
> based on the kernel command line. I have no issue really with either one
> of those. What I don't want is another "overlay manager" for each
> usecase. And sorry, you don't win for being first (you're not really).
> All this I said before.

So, I'm not actually asking anyone to take anything here, I'm just
trying to reopen the conversation that hasn't gone very far, so we can
find some direction to take.

I've not followed the discussion super closely here, so apologies if
I'm off base here.  But it does seem there's a number of similar
efforts, and it seems no one has gotten enough momentum to make a real
push upstream. It seems its just easier for everyone to keep their own
approach in their own tree and not to bother.  At one end, sure,
keeping half baked ideas out of mainline until they really sort
themselves out is fine, but at the other extreme you risk the
half-baked ideas calcifying in their own tree and then you have more
Android (or whatever variant) specific cruft to shake your fist at.
:)

How do we get to a shared solution here where folks are engaging with upstream?

Even an "I like approach B, go make that work" would help here.

>> And in the case with hikey/hikey960, this may be doable, as we can
>> modify the UEFI source, but it would also require users to have all
>> the various overlay images to flash as well, which adds complexity.
>> Additionally on other boards which don't have open firmware, such a
>> solution may not be feasible. The benefit to this approach is it
>> doesn't require such standardized firmware behavior.
>
> Unless there's some great, overwhelming demand to support such devices
> with closed firmware, there's no benefit to me to care. What board do we
> have with closed firmware that needs this?

Just across the 96boards we don't have open firmware on all the boards.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments
@ 2017-11-07 23:10           ` John Stultz
  0 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2017-11-07 23:10 UTC (permalink / raw)
  To: Rob Herring
  Cc: lkml, Mark Rutland, Frank Rowand, Dmitry Shmidt,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Tue, Oct 17, 2017 at 2:20 PM, John Stultz <john.stultz@linaro.org> wrote:
> On Tue, Oct 17, 2017 at 12:46 PM, Rob Herring <robh@kernel.org> wrote:
>> On Fri, Oct 13, 2017 at 04:51:03PM -0700, John Stultz wrote:
>>> It seems you're suggesting that there be some sort of special overlay
>>> partition which users have to flash with pre-built images containing
>>> the appropriate overlay dtbs, so that something like the treble
>>> overlay-per-partition approach could be used.
>>
>> I'm not really suggesting anything. I'm not going to take something that
>> *only* solves your usecase of apply an overlay embedded in a base dtb
>> based on the kernel command line. I have no issue really with either one
>> of those. What I don't want is another "overlay manager" for each
>> usecase. And sorry, you don't win for being first (you're not really).
>> All this I said before.
>
> So, I'm not actually asking anyone to take anything here, I'm just
> trying to reopen the conversation that hasn't gone very far, so we can
> find some direction to take.
>
> I've not followed the discussion super closely here, so apologies if
> I'm off base here.  But it does seem there's a number of similar
> efforts, and it seems no one has gotten enough momentum to make a real
> push upstream. It seems its just easier for everyone to keep their own
> approach in their own tree and not to bother.  At one end, sure,
> keeping half baked ideas out of mainline until they really sort
> themselves out is fine, but at the other extreme you risk the
> half-baked ideas calcifying in their own tree and then you have more
> Android (or whatever variant) specific cruft to shake your fist at.
> :)
>
> How do we get to a shared solution here where folks are engaging with upstream?
>
> Even an "I like approach B, go make that work" would help here.

Nudge.  Any further thoughts on what sort of an approach might be viable here?

thanks
-john

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments
@ 2017-11-07 23:10           ` John Stultz
  0 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2017-11-07 23:10 UTC (permalink / raw)
  To: Rob Herring
  Cc: lkml, Mark Rutland, Frank Rowand, Dmitry Shmidt,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Tue, Oct 17, 2017 at 2:20 PM, John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Oct 17, 2017 at 12:46 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Fri, Oct 13, 2017 at 04:51:03PM -0700, John Stultz wrote:
>>> It seems you're suggesting that there be some sort of special overlay
>>> partition which users have to flash with pre-built images containing
>>> the appropriate overlay dtbs, so that something like the treble
>>> overlay-per-partition approach could be used.
>>
>> I'm not really suggesting anything. I'm not going to take something that
>> *only* solves your usecase of apply an overlay embedded in a base dtb
>> based on the kernel command line. I have no issue really with either one
>> of those. What I don't want is another "overlay manager" for each
>> usecase. And sorry, you don't win for being first (you're not really).
>> All this I said before.
>
> So, I'm not actually asking anyone to take anything here, I'm just
> trying to reopen the conversation that hasn't gone very far, so we can
> find some direction to take.
>
> I've not followed the discussion super closely here, so apologies if
> I'm off base here.  But it does seem there's a number of similar
> efforts, and it seems no one has gotten enough momentum to make a real
> push upstream. It seems its just easier for everyone to keep their own
> approach in their own tree and not to bother.  At one end, sure,
> keeping half baked ideas out of mainline until they really sort
> themselves out is fine, but at the other extreme you risk the
> half-baked ideas calcifying in their own tree and then you have more
> Android (or whatever variant) specific cruft to shake your fist at.
> :)
>
> How do we get to a shared solution here where folks are engaging with upstream?
>
> Even an "I like approach B, go make that work" would help here.

Nudge.  Any further thoughts on what sort of an approach might be viable here?

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-11-07 23:10 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-10  5:34 [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments John Stultz
2017-10-10  5:34 ` [RFC][PATCH 1/3] of: overlay_mgr: Add overlay manager driver John Stultz
2017-10-10  5:34 ` [RFC][PATCH 2/3] of: overlay_mgr: Add ability to apply through sysfs entry John Stultz
2017-10-10  5:34 ` [RFC][PATCH 3/3] of: overlay_mgr: Add ability to apply several hardware configurations John Stultz
2017-10-13 21:41 ` [RFC][PATCH 0/3] Overlay manager for predefined DT overlay fragments Rob Herring
2017-10-13 21:41   ` Rob Herring
2017-10-13 23:51   ` John Stultz
2017-10-13 23:51     ` John Stultz
2017-10-17 19:46     ` Rob Herring
2017-10-17 19:46       ` Rob Herring
2017-10-17 21:20       ` John Stultz
2017-10-17 21:20         ` John Stultz
2017-11-07 23:10         ` John Stultz
2017-11-07 23:10           ` John Stultz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.