| 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 |
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. -- With Best Regards, Andy Shevchenko
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
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
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 >
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
On Fri, Feb 12, 2021 at 12:54 PM Enrico Weigelt, metux IT consult <lkml@metux.net> wrote: > On 12.02.21 10:58, Linus Walleij wrote: > > 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). I understand. I have had the same problem with trying to fix 96boards mezzanines. I also tried to sidestep the DT overlays, and it was generally disliked. The DT people have made up their mind that overlays is what they want to use for this type of stuff. Yours, Linus Walleij
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
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