Hi Masahiro, On 23 July 2015 at 00:17, Masahiro Yamada <yamada.masah...@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.masah...@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.masah...@socionext.com> > + * > + * Based on the original work in Linux by > + * Copyright (c) 2006 SUSE Linux Products GmbH > + * Copyright (c) 2006 Tejun Heo <te...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot