[U-Boot] [PATCH v3 2/5] devres: introduce Devres (Managed Device Resource) framework
Simon Glass
sjg at chromium.org
Fri Jul 24 01:20:48 CEST 2015
Hi Masahiro,
On 23 July 2015 at 00:17, Masahiro Yamada <yamada.masahiro at socionext.com> wrote:
> In U-Boot's driver model, memory is basically allocated and freed
> in the core framework. So, low level drivers generally only have
> to specify the size of needed memory with .priv_auto_alloc_size,
> .platdata_auto_alloc_size, etc. Nevertheless, some drivers still
> need to allocate/free memory on their own in case they cannot
> statically know the necessary memory size. So, I believe it is
> reasonable enough to port Devres into U-boot.
>
> Devres, which originates in Linux, manages device resources for each
> device and automatically releases them on driver detach. With devres,
> device resources are guaranteed to be freed whether initialization
> fails half-way or the device gets detached.
>
> The basic idea is totally the same to that of Linux, but I tweaked
> it a bit so that it fits in U-Boot's driver model.
>
> In U-Boot, drivers are activated in two steps: binding and probing.
> Binding puts a driver and a device together. It is just data
> manipulation on the system memory, so nothing has happened on the
> hardware device at this moment. When the device is really used, it
> is probed. Probing initializes the real hardware device to make it
> really ready for use.
>
> So, the resources acquired during the probing process must be freed
> when the device is removed. Likewise, what has been allocated in
> binding should be released when the device is unbound. The struct
> devres has a member "probe" to remember when the resource was
> allocated.
>
> CONFIG_DEBUG_DEVRES is also supported for easier debugging.
> If enabled, debug messages are printed each time a resource is
> allocated/freed.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro at socionext.com>
> ---
>
> Changes in v3:
> - Update git-description. Do not mention about the DM core part.
>
> Changes in v2:
> - Add more APIs: _free, _find, _get, _remove, _destroy, _release
> - Move devres_release_probe() and devres_release_all() decrlarations
> to dm/device-internal.h
> - Move comments to headers
>
> drivers/core/Kconfig | 10 +++
> drivers/core/Makefile | 2 +-
> drivers/core/device-remove.c | 5 ++
> drivers/core/device.c | 3 +
> drivers/core/devres.c | 187 +++++++++++++++++++++++++++++++++++++++++++
> include/dm/device-internal.h | 19 +++++
> include/dm/device.h | 140 ++++++++++++++++++++++++++++++++
> 7 files changed, 365 insertions(+), 1 deletion(-)
> create mode 100644 drivers/core/devres.c
>
> diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> index e40372d..6889025 100644
> --- a/drivers/core/Kconfig
> +++ b/drivers/core/Kconfig
> @@ -59,3 +59,13 @@ config DM_SEQ_ALIAS
> Most boards will have a '/aliases' node containing the path to
> numbered devices (e.g. serial0 = &serial0). This feature can be
> disabled if it is not required, to save code space in SPL.
> +
> +config DEBUG_DEVRES
> + bool "Managed device resources verbose debug messages"
> + depends on DM
> + help
> + If this option is enabled, devres debug messages are printed.
> + Select this if you are having a problem with devres or want to
> + debug resource management for a managed device.
> +
> + If you are unsure about this, Say N here.
> diff --git a/drivers/core/Makefile b/drivers/core/Makefile
> index 5c2ead8..d2cf2ea 100644
> --- a/drivers/core/Makefile
> +++ b/drivers/core/Makefile
> @@ -4,7 +4,7 @@
> # SPDX-License-Identifier: GPL-2.0+
> #
>
> -obj-y += device.o lists.o root.o uclass.o util.o
> +obj-y += device.o lists.o root.o uclass.o util.o devres.o
> ifndef CONFIG_SPL_BUILD
> obj-$(CONFIG_OF_CONTROL) += simple-bus.o
> endif
> diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
> index 45d6543..bd6d406 100644
> --- a/drivers/core/device-remove.c
> +++ b/drivers/core/device-remove.c
> @@ -95,6 +95,9 @@ int device_unbind(struct udevice *dev)
>
> if (dev->parent)
> list_del(&dev->sibling_node);
> +
> + devres_release_all(dev);
> +
> free(dev);
>
> return 0;
> @@ -128,6 +131,8 @@ void device_free(struct udevice *dev)
> dev->parent_priv = NULL;
> }
> }
> +
> + devres_release_probe(dev);
> }
>
> int device_remove(struct udevice *dev)
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 0333889..a2e384c 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -47,6 +47,7 @@ int device_bind(struct udevice *parent, const struct driver *drv,
> INIT_LIST_HEAD(&dev->sibling_node);
> INIT_LIST_HEAD(&dev->child_head);
> INIT_LIST_HEAD(&dev->uclass_node);
> + INIT_LIST_HEAD(&dev->devres_head);
> dev->platdata = platdata;
> dev->name = name;
> dev->of_offset = of_offset;
> @@ -170,6 +171,8 @@ fail_alloc2:
> dev->platdata = NULL;
> }
> fail_alloc1:
> + devres_release_all(dev);
> +
> free(dev);
>
> return ret;
> diff --git a/drivers/core/devres.c b/drivers/core/devres.c
> new file mode 100644
> index 0000000..e7330b3
> --- /dev/null
> +++ b/drivers/core/devres.c
> @@ -0,0 +1,187 @@
> +/*
> + * Copyright (C) 2015 Masahiro Yamada <yamada.masahiro at socionext.com>
> + *
> + * Based on the original work in Linux by
> + * Copyright (c) 2006 SUSE Linux Products GmbH
> + * Copyright (c) 2006 Tejun Heo <teheo at suse.de>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <linux/compat.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <dm/device.h>
> +
Please can you add comments for these fields? I'm not sure what
dr_release_t is for, for exable.
> +struct devres {
> + struct list_head entry;
> + dr_release_t release;
> + bool probe;
> +#ifdef CONFIG_DEBUG_DEVRES
> + const char *name;
> + size_t size;
> +#endif
> + unsigned long long data[];
> +};
> +
> +#ifdef CONFIG_DEBUG_DEVRES
> +
> +static void set_node_dbginfo(struct devres *dr, const char *name, size_t size)
> +{
> + dr->name = name;
> + dr->size = size;
> +}
> +
> +static void devres_log(struct udevice *dev, struct devres *dr,
> + const char *op)
> +{
> + printf("%s: DEVRES %3s %p %s (%lu bytes)\n",
> + dev->name, op, dr, dr->name, (unsigned long)dr->size);
> +}
> +#else /* CONFIG_DEBUG_DEVRES */
> +#define set_node_dbginfo(dr, n, s) do {} while (0)
> +#define devres_log(dev, dr, op) do {} while (0)
> +#endif
> +
> +#if CONFIG_DEBUG_DEVRES
> +void *__devres_alloc(dr_release_t release, size_t size, gfp_t gfp,
> + const char *name)
> +#else
> +void *_devres_alloc(dr_release_t release, size_t size, gfp_t gfp)
> +#endif
> +{
> + size_t tot_size = sizeof(struct devres) + size;
> + struct devres *dr;
> +
> + dr = kmalloc(tot_size, gfp);
> + if (unlikely(!dr))
> + return NULL;
> +
> + INIT_LIST_HEAD(&dr->entry);
> + dr->release = release;
> + set_node_dbginfo(dr, name, size);
> +
> + return dr->data;
> +}
> +
> +void devres_free(void *res)
> +{
> + if (res) {
> + struct devres *dr = container_of(res, struct devres, data);
> +
> + BUG_ON(!list_empty(&dr->entry));
> + kfree(dr);
> + }
> +}
> +
> +void devres_add(struct udevice *dev, void *res)
> +{
> + struct devres *dr = container_of(res, struct devres, data);
> +
> + devres_log(dev, dr, "ADD");
> + BUG_ON(!list_empty(&dr->entry));
> + dr->probe = dev->flags & DM_FLAG_BOUND ? true : false;
> + list_add_tail(&dr->entry, &dev->devres_head);
> +}
> +
> +void *devres_find(struct udevice *dev, dr_release_t release,
> + dr_match_t match, void *match_data)
> +{
> + struct devres *dr;
> +
> + list_for_each_entry_reverse(dr, &dev->devres_head, entry) {
> + if (dr->release != release)
> + continue;
> + if (match && !match(dev, dr->data, match_data))
> + continue;
> + return dr->data;
> + }
> +
> + return NULL;
> +}
> +
> +void *devres_get(struct udevice *dev, void *new_res,
> + dr_match_t match, void *match_data)
> +{
> + struct devres *new_dr = container_of(new_res, struct devres, data);
> + void *res;
> +
> + res = devres_find(dev, new_dr->release, match, match_data);
> + if (!res) {
> + devres_add(dev, new_res);
> + res = new_res;
> + new_res = NULL;
> + }
> + devres_free(new_res);
> +
> + return res;
> +}
> +
> +void *devres_remove(struct udevice *dev, dr_release_t release,
> + dr_match_t match, void *match_data)
> +{
> + void *res;
> +
> + res = devres_find(dev, release, match, match_data);
> + if (res) {
> + struct devres *dr = container_of(res, struct devres, data);
> +
> + list_del_init(&dr->entry);
> + devres_log(dev, dr, "REM");
> + }
> +
> + return res;
> +}
> +
> +int devres_destroy(struct udevice *dev, dr_release_t release,
> + dr_match_t match, void *match_data)
> +{
> + void *res;
> +
> + res = devres_remove(dev, release, match, match_data);
> + if (unlikely(!res))
> + return -ENOENT;
> +
> + devres_free(res);
> + return 0;
> +}
> +
> +int devres_release(struct udevice *dev, dr_release_t release,
> + dr_match_t match, void *match_data)
> +{
> + void *res;
> +
> + res = devres_remove(dev, release, match, match_data);
> + if (unlikely(!res))
> + return -ENOENT;
> +
> + (*release)(dev, res);
> + devres_free(res);
> + return 0;
> +}
> +
> +static void release_nodes(struct udevice *dev, struct list_head *head,
> + bool probe_only)
> +{
> + struct devres *dr, *tmp;
> +
> + list_for_each_entry_safe_reverse(dr, tmp, head, entry) {
> + if (probe_only && !dr->probe)
> + break;
> + devres_log(dev, dr, "REL");
> + dr->release(dev, dr->data);
Somewhere in the header file can you please explain the use case for
the release() method?
> + list_del(&dr->entry);
> + kfree(dr);
> + }
> +}
> +
> +void devres_release_probe(struct udevice *dev)
> +{
> + release_nodes(dev, &dev->devres_head, true);
> +}
> +
> +void devres_release_all(struct udevice *dev)
> +{
> + release_nodes(dev, &dev->devres_head, false);
> +}
> diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
> index 402304f..ee3b00d 100644
> --- a/include/dm/device-internal.h
> +++ b/include/dm/device-internal.h
> @@ -143,4 +143,23 @@ static inline void device_free(struct udevice *dev) {}
> #define DM_ROOT_NON_CONST (((gd_t *)gd)->dm_root)
> #define DM_UCLASS_ROOT_NON_CONST (((gd_t *)gd)->uclass_root)
>
> +/* device resource management */
> +/**
> + * devres_release_probe - Release managed resources allocated after probing
> + * @dev: Device to release resources for
> + *
> + * Release all resources allocated for @dev when it was probed or later.
> + * This function is called on driver removal.
> + */
> +void devres_release_probe(struct udevice *dev);
> +
> +/**
> + * devres_release_all - Release all managed resources
> + * @dev: Device to release resources for
> + *
> + * Release all resources associated with @dev. This function is
> + * called on driver unbinding.
> + */
> +void devres_release_all(struct udevice *dev);
> +
> #endif
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 43004ac..b909b9a 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -96,6 +96,7 @@ struct udevice {
> uint32_t flags;
> int req_seq;
> int seq;
> + struct list_head devres_head;
> };
>
> /* Maximum sequence number supported */
> @@ -463,4 +464,143 @@ bool device_has_active_children(struct udevice *dev);
> */
> bool device_is_last_sibling(struct udevice *dev);
>
> +/* device resource management */
> +typedef void (*dr_release_t)(struct udevice *dev, void *res);
> +typedef int (*dr_match_t)(struct udevice *dev, void *res, void *match_data);
> +
> +#ifdef CONFIG_DEBUG_DEVRES
> +void *__devres_alloc(dr_release_t release, size_t size, gfp_t gfp,
> + const char *name);
> +#define _devres_alloc(release, size, gfp) \
> + __devres_alloc(release, size, gfp, #release)
> +#else
> +void *_devres_alloc(dr_release_t release, size_t size, gfp_t gfp);
> +#endif
> +
> +/**
> + * devres_alloc - Allocate device resource data
> + * @release: Release function devres will be associated with
> + * @size: Allocation size
> + * @gfp: Allocation flags
> + *
> + * Allocate devres of @size bytes. The allocated area is associated
> + * with @release. The returned pointer can be passed to
> + * other devres_*() functions.
> + *
> + * RETURNS:
> + * Pointer to allocated devres on success, NULL on failure.
> + */
> +#define devres_alloc(release, size, gfp) \
> + _devres_alloc(release, size, gfp | __GFP_ZERO)
> +
> +/**
> + * devres_free - Free device resource data
> + * @res: Pointer to devres data to free
> + *
> + * Free devres created with devres_alloc().
> + */
> +void devres_free(void *res);
> +
> +/**
> + * devres_add - Register device resource
> + * @dev: Device to add resource to
> + * @res: Resource to register
> + *
> + * Register devres @res to @dev. @res should have been allocated
> + * using devres_alloc(). On driver detach, the associated release
> + * function will be invoked and devres will be freed automatically.
> + */
> +void devres_add(struct udevice *dev, void *res);
> +
> +/**
> + * devres_find - Find device resource
> + * @dev: Device to lookup resource from
> + * @release: Look for resources associated with this release function
> + * @match: Match function (optional)
> + * @match_data: Data for the match function
> + *
> + * Find the latest devres of @dev which is associated with @release
> + * and for which @match returns 1. If @match is NULL, it's considered
> + * to match all.
> + *
> + * RETURNS:
> + * Pointer to found devres, NULL if not found.
> + */
> +void *devres_find(struct udevice *dev, dr_release_t release,
> + dr_match_t match, void *match_data);
> +
> +/**
> + * devres_get - Find devres, if non-existent, add one atomically
> + * @dev: Device to lookup or add devres for
> + * @new_res: Pointer to new initialized devres to add if not found
> + * @match: Match function (optional)
> + * @match_data: Data for the match function
> + *
> + * Find the latest devres of @dev which has the same release function
> + * as @new_res and for which @match return 1. If found, @new_res is
> + * freed; otherwise, @new_res is added atomically.
> + *
> + * RETURNS:
> + * Pointer to found or added devres.
> + */
> +void *devres_get(struct udevice *dev, void *new_res,
> + dr_match_t match, void *match_data);
> +
> +/**
> + * devres_remove - Find a device resource and remove it
> + * @dev: Device to find resource from
> + * @release: Look for resources associated with this release function
> + * @match: Match function (optional)
> + * @match_data: Data for the match function
> + *
> + * Find the latest devres of @dev associated with @release and for
> + * which @match returns 1. If @match is NULL, it's considered to
> + * match all. If found, the resource is removed atomically and
> + * returned.
> + *
> + * RETURNS:
> + * Pointer to removed devres on success, NULL if not found.
> + */
> +void *devres_remove(struct udevice *dev, dr_release_t release,
> + dr_match_t match, void *match_data);
> +
> +/**
> + * devres_destroy - Find a device resource and destroy it
> + * @dev: Device to find resource from
> + * @release: Look for resources associated with this release function
> + * @match: Match function (optional)
> + * @match_data: Data for the match function
> + *
> + * Find the latest devres of @dev associated with @release and for
> + * which @match returns 1. If @match is NULL, it's considered to
> + * match all. If found, the resource is removed atomically and freed.
> + *
> + * Note that the release function for the resource will not be called,
> + * only the devres-allocated data will be freed. The caller becomes
> + * responsible for freeing any other data.
> + *
> + * RETURNS:
> + * 0 if devres is found and freed, -ENOENT if not found.
> + */
> +int devres_destroy(struct udevice *dev, dr_release_t release,
> + dr_match_t match, void *match_data);
> +
> +/**
> + * devres_release - Find a device resource and destroy it, calling release
> + * @dev: Device to find resource from
> + * @release: Look for resources associated with this release function
> + * @match: Match function (optional)
> + * @match_data: Data for the match function
> + *
> + * Find the latest devres of @dev associated with @release and for
> + * which @match returns 1. If @match is NULL, it's considered to
> + * match all. If found, the resource is removed atomically, the
> + * release function called and the resource freed.
> + *
> + * RETURNS:
> + * 0 if devres is found and freed, -ENOENT if not found.
> + */
> +int devres_release(struct udevice *dev, dr_release_t release,
> + dr_match_t match, void *match_data);
> +
> #endif
> --
> 1.9.1
>
Regards,
Simon
More information about the U-Boot
mailing list