[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