diff mbox series

[RFC,11/12] platform/x86: skeleton for oftree based board device initialization

Message ID 20210208222203.22335-12-info@metux.net
State New
Headers show
Series [RFC,01/12] of: base: improve error message in of_phandle_iterator_next() | expand

Commit Message

Enrico Weigelt, metux IT consult Feb. 8, 2021, 10:22 p.m. UTC
Lots of boards have extra devices that can't be fully probed via bus'es
(like PCI) or generic firmware mechanisms like ACPI. Often those capabilities
are just partial or even highly depend on firmware version.

Instead of hand-writing board specific drivers merely for the correct
initialization / parameterization of generic drivers, hereby introducing
a generic mechanism, using the already well supported oftree.

These oftrees are compiled into the driver, which first tries to match
machine identifications (eg. DMI strings) against rules defined in the
individual oftrees, and on success, probes the devices that are defined
by them.

For the time being, we just support matching on DMI_BOARD_NAME and
DMI_SYS_VENDOR - other criteria, even bus- or ACPI-id's can be added later,
when needed.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/platform/Kconfig      |   2 +
 drivers/platform/Makefile     |   1 +
 drivers/platform/of/Kconfig   |  41 ++++++++++++++
 drivers/platform/of/Makefile  |   5 ++
 drivers/platform/of/drv.c     | 123 +++++++++++++++++++++++++++++++++++++++++
 drivers/platform/of/init.c    | 126 ++++++++++++++++++++++++++++++++++++++++++
 drivers/platform/of/ofboard.h |   8 +++
 7 files changed, 306 insertions(+)
 create mode 100644 drivers/platform/of/Kconfig
 create mode 100644 drivers/platform/of/Makefile
 create mode 100644 drivers/platform/of/drv.c
 create mode 100644 drivers/platform/of/init.c
 create mode 100644 drivers/platform/of/ofboard.h

Comments

Andy Shevchenko Feb. 10, 2021, 10:32 a.m. UTC | #1
On Tue, Feb 9, 2021 at 12:27 AM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
>
> Lots of boards have extra devices that can't be fully probed via bus'es
> (like PCI) or generic firmware mechanisms like ACPI. Often those capabilities
> are just partial or even highly depend on firmware version.
>
> Instead of hand-writing board specific drivers merely for the correct
> initialization / parameterization of generic drivers, hereby introducing
> a generic mechanism, using the already well supported oftree.
>
> These oftrees are compiled into the driver, which first tries to match
> machine identifications (eg. DMI strings) against rules defined in the
> individual oftrees, and on success, probes the devices that are defined
> by them.
>
> For the time being, we just support matching on DMI_BOARD_NAME and
> DMI_SYS_VENDOR - other criteria, even bus- or ACPI-id's can be added later,
> when needed.

This is weird and seems it missed the fact of the DMI based modprobe
mechanism that the kernel has for ages.
Linus Walleij Feb. 12, 2021, 9:58 a.m. UTC | #2
On Mon, Feb 8, 2021 at 11:22 PM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:

> Lots of boards have extra devices that can't be fully probed via bus'es
> (like PCI) or generic firmware mechanisms like ACPI. Often those capabilities
> are just partial or even highly depend on firmware version.

I think Intel people often take the stance that the ACPI DSDT (or whatever)
needs to be fixed.

If the usecase is to explicitly work around deployed firmware that cannot
and will not be upgraded/fixed by describing the hardware using DT
instead, based on just the DMI ID then we should spell that out
explicitly.

It feels a bit like fixing a problem using a different hardware description
just because we can. Look in drivers/gpio/gpiolib-acpi.c
table gpiolib_acpi_quirks[]. It's just an example how this is fixed using
fine granular ACPI-specific mechanisms at several places in the kernel
instead of just tossing out the whole description and redoing it in
device tree.

I haven't worked with this in practice so I suppose it is a bit up to
the people who end up having to fix this kind of stuff, Hans de Goede
and Andy are fixing this kind of stuff all the time so their buy-in is
important, we need to see that this is a real, useful tool for people
like them, not just nice to have.

Yours,
Linus Walleij
Enrico Weigelt, metux IT consult Feb. 12, 2021, 11:54 a.m. UTC | #3
On 12.02.21 10:58, Linus Walleij wrote:

Hi,

> I think Intel people often take the stance that the ACPI DSDT (or whatever)
> needs to be fixed.

It should, actually board/firmware vendors should think more carefully
and do it right in the first place. But reality is different. And
firmware upgrade often is anything but easy (as soon as we leave the
field of average Joh Doe's home PC)

> If the usecase is to explicitly work around deployed firmware that cannot
> and will not be upgraded/fixed by describing the hardware using DT
> instead, based on just the DMI ID then we should spell that out
> explicitly.

Okay, maybe I should have stated this more clearly.

OTOH, the scope is also a little bit greater: certain external cards
that don't need much special handling for the card itself, just
enumerate devices (and connections between them) using existing drivers.

That's a pretty common scenario in industrial backplane systems, where
we have lots of different (even application specific) cards, usually
composed of standard chips, that can be identified by some ID, but
cannot describe themselves. We have to write lots of specific drivers
for them, usually just for instantiating existing drivers. (we rarely
see such code going towards mainline).

A similar case (mainlined) seems to be the RCAR display unit - they're
using dt overlays that are built into the driver and applied by it
based on the detected DU at runtime. RCAR seems to be a pure DT
platform, so that's an obvious move. APU2/3/4 is ACPI based, so I went
in a different direction - but I'm now investigating how to make DT
overlays work on an ACPI platform (eg. needs some initial nodes, ...)
In case that's successful, I'll rework my RFC to use overlays, and
it will become much smaller (my oftree core changes then won't be
necessary anymore).

> It feels a bit like fixing a problem using a different hardware description
> just because we can. Look in drivers/gpio/gpiolib-acpi.c
> table gpiolib_acpi_quirks[]. It's just an example how this is fixed using
> fine granular ACPI-specific mechanisms at several places in the kernel
> instead of just tossing out the whole description and redoing it in
> device tree.

I'm quite reluctant to put everything in there. Theoretically, for apu
case, I could prevent enumerating the incomplete gpios there, but the
actual driver setup still remains (certainly don't wanna put that into
such a global place). But the original problem of having to write so
much code for just instantiating generic drivers remains. And
distributing knowledge of certain devices over several places doesn't
feel like a good idea to me.


--mtx
Frank Rowand Feb. 15, 2021, 1:18 a.m. UTC | #4
On 2/12/21 5:54 AM, Enrico Weigelt, metux IT consult wrote:
> On 12.02.21 10:58, Linus Walleij wrote:
> 
> Hi,
> 
>> I think Intel people often take the stance that the ACPI DSDT (or whatever)
>> needs to be fixed.
> 
> It should, actually board/firmware vendors should think more carefully
> and do it right in the first place. But reality is different. And
> firmware upgrade often is anything but easy (as soon as we leave the
> field of average Joh Doe's home PC)
> 
>> If the usecase is to explicitly work around deployed firmware that cannot
>> and will not be upgraded/fixed by describing the hardware using DT
>> instead, based on just the DMI ID then we should spell that out
>> explicitly.
> 
> Okay, maybe I should have stated this more clearly.
> 
> OTOH, the scope is also a little bit greater: certain external cards
> that don't need much special handling for the card itself, just
> enumerate devices (and connections between them) using existing drivers.
> 
> That's a pretty common scenario in industrial backplane systems, where
> we have lots of different (even application specific) cards, usually
> composed of standard chips, that can be identified by some ID, but
> cannot describe themselves. We have to write lots of specific drivers
> for them, usually just for instantiating existing drivers. (we rarely
> see such code going towards mainline).
> 
> A similar case (mainlined) seems to be the RCAR display unit - they're
> using dt overlays that are built into the driver and applied by it
> based on the detected DU at runtime. RCAR seems to be a pure DT

The RCAR use of overlays that are built into the driver are a known
pattern that is explicitly not to be repeated.  The driver has been
granted a grandfathered in status, thus an exception as long as
needed.

-Frank

> platform, so that's an obvious move. APU2/3/4 is ACPI based, so I went
> in a different direction - but I'm now investigating how to make DT
> overlays work on an ACPI platform (eg. needs some initial nodes, ...)
> In case that's successful, I'll rework my RFC to use overlays, and
> it will become much smaller (my oftree core changes then won't be
> necessary anymore).
> 
>> It feels a bit like fixing a problem using a different hardware description
>> just because we can. Look in drivers/gpio/gpiolib-acpi.c
>> table gpiolib_acpi_quirks[]. It's just an example how this is fixed using
>> fine granular ACPI-specific mechanisms at several places in the kernel
>> instead of just tossing out the whole description and redoing it in
>> device tree.
> 
> I'm quite reluctant to put everything in there. Theoretically, for apu
> case, I could prevent enumerating the incomplete gpios there, but the
> actual driver setup still remains (certainly don't wanna put that into
> such a global place). But the original problem of having to write so
> much code for just instantiating generic drivers remains. And
> distributing knowledge of certain devices over several places doesn't
> feel like a good idea to me.
> 
> 
> --mtx
>
Enrico Weigelt, metux IT consult Feb. 23, 2021, 8:41 p.m. UTC | #5
On 15.02.21 02:18, Frank Rowand wrote:

> The RCAR use of overlays that are built into the driver are a known
> pattern that is explicitly not to be repeated. 

Well, that driver indeed looks quite complex - if belive unnecessarily.
But can't judge on these devices, don't have one of them.

In my case, I believe it's a simple and straightforward approach,
instead of writing a whole driver, that just consists of a bunch
of tables and some trivial setup calls. DT seems to be a perfect
choice for that, since it's a very short and precise language for
describing hw layout, w/o any piece of imperative code.

The only point where I'm still not satisfied with: module auto-loading
requires the match data in the kernel module. But i'd like to have
everything in one source file and not having to write individual
modules for invididual boards anymore. Finally, there should be one
dts per board and really minimal effort adding another dts.
(hmm, maybe I should try generating glue code from dt ?)

BTW: I've already rewritten much of it, using overlay instead of an
completely own detached tree (so, some of the prev patches will fall
off the queue).


--mtx
diff mbox series

Patch

diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
index 18fc6a08569e..9ac6d4e2a762 100644
--- a/drivers/platform/Kconfig
+++ b/drivers/platform/Kconfig
@@ -15,3 +15,5 @@  source "drivers/platform/mellanox/Kconfig"
 source "drivers/platform/olpc/Kconfig"
 
 source "drivers/platform/surface/Kconfig"
+
+source "drivers/platform/of/Kconfig"
diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
index 4de08ef4ec9d..ca4d74701fd7 100644
--- a/drivers/platform/Makefile
+++ b/drivers/platform/Makefile
@@ -10,3 +10,4 @@  obj-$(CONFIG_OLPC_EC)		+= olpc/
 obj-$(CONFIG_GOLDFISH)		+= goldfish/
 obj-$(CONFIG_CHROME_PLATFORMS)	+= chrome/
 obj-$(CONFIG_SURFACE_PLATFORMS)	+= surface/
+obj-$(CONFIG_PLATFORM_OF_DRV)	+= of/
diff --git a/drivers/platform/of/Kconfig b/drivers/platform/of/Kconfig
new file mode 100644
index 000000000000..a0b5a641a7c6
--- /dev/null
+++ b/drivers/platform/of/Kconfig
@@ -0,0 +1,41 @@ 
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# X86 Platform Specific Drivers
+#
+
+menuconfig PLATFORM_OF_DRV
+	tristate "Platform support via device tree"
+	select OF
+	select OF_FLATTREE
+	help
+	  Say Y here to get to see options for board support that's initialized
+	  via compiled-in flattened device trees.
+
+	  This is entirely independent from traditional DT-based booting (or DT
+	  overlays) and meant for additional devices on non-OF-based (eg. APCI)
+	  boards or composite devices behind probing-capable busses (eg. PCI).
+
+	  Instead of writing individual drivers for just the initialization of
+	  subdevices, this option provides a generic mechanism for describing
+	  these devices via device tree.
+
+	  If you say N, all options in this submenu will be skipped and disabled.
+
+if PLATFORM_OF_DRV
+
+config PLATFORM_OF_DRV_SYSFS_DTB
+	bool "Expose device tree binaries in sysfs"
+	default y
+	depends on SYSFS
+	help
+	  Say Y here to enable exposing device tree binaries at /sys/firmware.
+
+config PLATFORM_OF_DRV_SYSFS_DT
+	bool "Expose parsed device tree in sysfs"
+	default y
+	depends on SYSFS
+	help
+	  Say Y here to enable exposing device tree nodes at
+	  /sys/firmware/devicetree.
+
+endif # PLATFORM_OF_DRV
diff --git a/drivers/platform/of/Makefile b/drivers/platform/of/Makefile
new file mode 100644
index 000000000000..84cf3003c500
--- /dev/null
+++ b/drivers/platform/of/Makefile
@@ -0,0 +1,5 @@ 
+# SPDX-License-Identifier: GPL-2.0
+
+ofboard-y := init.o drv.o
+
+obj-$(CONFIG_PLATFORM_OF_DRV) += ofboard.o
diff --git a/drivers/platform/of/drv.c b/drivers/platform/of/drv.c
new file mode 100644
index 000000000000..ff7006c24cf7
--- /dev/null
+++ b/drivers/platform/of/drv.c
@@ -0,0 +1,123 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Copyright (C) 2021 metux IT consult
+ * Author: Enrico Weigelt <info@metux.net>
+ */
+
+#include <linux/dmi.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/slab.h>
+
+#include "ofboard.h"
+
+static bool __init ofboard_match_dmi(struct device *dev)
+{
+#ifdef CONFIG_DMI
+	const char *board = dmi_get_system_info(DMI_BOARD_NAME);
+	const char *vendor = dmi_get_system_info(DMI_SYS_VENDOR);
+	const struct device_node *node = dev->of_node;
+
+	if (!of_match_string(node, "dmi-sys-vendor", vendor))
+		return false;
+
+	if (!of_match_string(node, "dmi-board-name", board))
+		return false;
+
+	dev_info(dev, "matched dmi: vendor=\"%s\" board=\"%s\"\n", vendor,
+		 board);
+
+	return true;
+#else
+	return false;
+#endif
+}
+
+static void __init ofboard_kick_devs(struct device *dev,
+				     struct device_node *np,
+				     const char *bus_name)
+{
+	struct property *prop;
+	const char *walk;
+	struct bus_type *bus;
+	int ret;
+
+	if (strcmp(bus_name, "name")==0)
+		return;
+
+	bus = find_bus(bus_name);
+	if (!bus) {
+		dev_warn(dev, "cant find bus \"%s\"\n", bus_name);
+		return;
+	}
+
+	of_property_for_each_string(np, bus_name, prop, walk) {
+		ret = bus_unregister_device_by_name(bus, walk);
+		if (ret)
+			dev_warn(dev, "failed removing device \"%s\" on bus "
+				 "\"%s\": %d\n", walk, bus_name, ret);
+		else
+			dev_info(dev, "removed device \"%s\" from bus "
+				 "\"%s\"\n", walk, bus_name);
+	}
+
+	bus_put(bus);
+}
+
+static void __init ofboard_unbind(struct device *dev)
+{
+	struct property *pr;
+	struct device_node *np = of_get_child_by_name(dev->of_node, "unbind");
+
+	if (!IS_ERR_OR_NULL(np))
+		for_each_property_of_node(np, pr)
+			ofboard_kick_devs(dev, np, pr->name);
+}
+
+static int ofboard_populate(struct device *dev)
+{
+	int ret;
+	struct device_node *of_node = dev->of_node;
+	struct device_node *np = of_get_child_by_name(of_node, "devices");
+
+	if (IS_ERR_OR_NULL(np)) {
+		dev_info(dev, "board oftree has no devices\n");
+		return -ENOENT;
+	}
+
+	ret = of_platform_populate(np, NULL, NULL, dev);
+	if (ret) {
+		dev_err(dev, "failed probing of childs: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ofboard_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+
+	if (!ofboard_match_dmi(dev))
+		return -EINVAL;
+
+	ofboard_unbind(&pdev->dev);
+
+	return ofboard_populate(dev);
+}
+
+static const struct of_device_id ofboard_of_match[] = {
+	{ .compatible = "virtual,dmi-board" },
+	{}
+};
+
+struct platform_driver ofboard_driver = {
+	.driver = {
+		.name = "ofboard",
+		.of_match_table = ofboard_of_match,
+	},
+	.probe = ofboard_probe,
+};
diff --git a/drivers/platform/of/init.c b/drivers/platform/of/init.c
new file mode 100644
index 000000000000..3b8373cda77a
--- /dev/null
+++ b/drivers/platform/of/init.c
@@ -0,0 +1,126 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Copyright (C) 2021 metux IT consult
+ * Author: Enrico Weigelt <info@metux.net>
+ */
+
+#define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
+
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_fdt.h>
+#include <linux/libfdt.h>
+#include <linux/of_platform.h>
+#include <linux/slab.h>
+#include "ofboard.h"
+
+#define DECLARE_FDT_EXTERN(n) \
+	extern char __dtb_##n##_begin[]; \
+	static struct bin_attribute __dtb_##n##_attr = { \
+		.attr = { .name = "fdt-" #n, .mode = S_IRUSR }, \
+		.private = __dtb_##n##_begin, \
+		.read = fdt_image_raw_read, \
+	};
+
+struct fdt_image {
+	char *begin;
+	size_t size;
+	char *basename;
+	struct bin_attribute *bin_attr;
+	struct device_node *root;
+};
+
+#define FDT_IMAGE_ENT(n)			\
+	{					\
+		.begin = __dtb_##n##_begin,	\
+		.bin_attr = &__dtb_##n##_attr,	\
+		.basename = "ofboard-" #n	\
+	}
+
+static ssize_t fdt_image_raw_read(struct file *filep, struct kobject *kobj,
+				  struct bin_attribute *bin_attr, char *buf,
+				  loff_t off, size_t count)
+{
+	memcpy(buf, bin_attr->private + off, count);
+	return count;
+}
+
+static struct fdt_image fdt[] = {
+};
+
+static int __init ofdrv_init_sysfs(struct fdt_image *image)
+{
+	image->bin_attr->size = image->size;
+	image->bin_attr->private = image->begin;
+
+	if (sysfs_create_bin_file(firmware_kobj, image->bin_attr))
+		pr_warn("failed creating sysfs bin_file\n");
+
+	of_attach_tree_sysfs(image->root, image->basename);
+
+	return 0;
+}
+
+static int __init ofdrv_parse_image(struct fdt_image *image)
+{
+	struct device_node* root;
+	void *new_fdt;
+
+	image->size = fdt_totalsize(image->begin);
+	new_fdt = kmemdup(image->begin, image->size, GFP_KERNEL);
+	if (!new_fdt)
+		return -ENOMEM;
+
+	image->begin = new_fdt;
+	of_fdt_unflatten_tree(new_fdt, NULL, &root);
+
+	if (IS_ERR_OR_NULL(root))
+		return PTR_ERR(root);
+
+	image->root = root;
+
+	return 0;
+}
+
+static int __init ofdrv_init_image(struct fdt_image *image)
+{
+	struct device_node *np;
+	int ret;
+
+	ret = ofdrv_parse_image(image);
+	if (ret)
+		return ret;
+
+	ofdrv_init_sysfs(image);
+
+	for_each_child_of_node(image->root, np) {
+		struct platform_device_info pdevinfo = {
+			.name = np->name,
+			.fwnode = &np->fwnode,
+			.id = PLATFORM_DEVID_NONE,
+		};
+		platform_device_register_full(&pdevinfo);
+	}
+
+	return 0;
+}
+
+static int __init ofdrv_init(void)
+{
+	int x;
+
+	platform_driver_register(&ofboard_driver);
+
+	for (x=0; x<ARRAY_SIZE(fdt); x++)
+		ofdrv_init_image(&fdt[x]);
+
+	return 0;
+}
+
+module_init(ofdrv_init);
+
+MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
+MODULE_DESCRIPTION("Generic oftree based initialization of custom board devices");
+MODULE_LICENSE("GPL");
diff --git a/drivers/platform/of/ofboard.h b/drivers/platform/of/ofboard.h
new file mode 100644
index 000000000000..7516e5df4f18
--- /dev/null
+++ b/drivers/platform/of/ofboard.h
@@ -0,0 +1,8 @@ 
+#ifndef __DRIVERS_PLATFORM_OFBOARD_H
+#define __DRIVERS_PLATFORM_OFBOARD_H
+
+#include <linux/platform_device.h>
+
+extern struct platform_driver ofboard_driver;
+
+#endif