[U-Boot] [PATCH 6/8] dm: core: look up drivers more efficiently

Simon Glass sjg at chromium.org
Mon Nov 17 10:22:19 CET 2014


HI Masahiro,

On 17 November 2014 08:19, Masahiro Yamada <yamada.m at jp.panasonic.com> wrote:
> The DM initialization function, dm_init_and_scan() is called twice,
> before and after relocation.
>
> In the first DM scan, only some of drivers are bound for the use in
> pre-reloc code (mostly UART drivers).  In the second scan, all the
> drivers are bound.
>
> In the current implementation, all the drivers are collected into a
> single array.  Every driver is searched through the array and then
> the DM_FLAG_PRE_RELOC flag is checked.  In the pre-reloc DM scan,
> drivers without DM_FLAG_PRE_RELOC are ignored.  This algorithm works
> enough, but is not very efficient.
>
> This commit aims to split drivers into two contiguous arrays,
> pre-reloc drivers and post-reloc drivers.  The drivers in the former
> group are declared with U_BOOT_DRIVER_F.  The others are declared with
> U_BOOT_DRIVER.
>
>                                                     pre      post
>                             _____________________  reloc     reloc
>   .u_boot_list_2_driver1_1  |                   |    ||       ||
>                             |                   |    ||       ||
>                             |                   |    ||       ||
>                             |  U_BOOT_DRIVER_F  |    ||       ||
>                             |     (driver1)     |    ||       ||
>                             |                   |   _||_      ||
>                             |                   |   \  /      ||
>   .u_boot_list_2_driver1_3  |___________________|    \/       ||
>   .u_boot_list_2_driver2_1  |                   |             ||
>                             |                   |             ||
>                             |                   |             ||
>                             |   U_BOOT_DRIVER   |             ||
>                             |     (driver2)     |             ||
>                             |                   |            _||_
>                             |                   |            \  /
>   .u_boot_list_2_driver2_3  |___________________|             \/
>
> The pre-reloc DM scan searches drivers from .u_boot_list_2_driver1_1
> to .u_boot_list_2_driver1_3.  The post-reloc scan searches ones from
> .u_boot_list_2_driver1_1 and .u_boot_list_2_driver2_3.
>
> Signed-off-by: Masahiro Yamada <yamada.m at jp.panasonic.com>
> ---
>
>  doc/driver-model/README.txt |  2 +-
>  drivers/core/device.c       |  4 +---
>  drivers/core/lists.c        | 22 +++++++++++++---------
>  drivers/core/root.c         |  3 ++-
>  include/dm/device.h         |  8 ++++++--
>  include/dm/lists.h          | 16 ++++++++++++----
>  include/dm/root.h           | 16 ++++++++--------
>  7 files changed, 43 insertions(+), 28 deletions(-)
>
> diff --git a/doc/driver-model/README.txt b/doc/driver-model/README.txt
> index 0278dda..e4114d5 100644
> --- a/doc/driver-model/README.txt
> +++ b/doc/driver-model/README.txt
> @@ -739,7 +739,7 @@ Pre-Relocation Support
>  ----------------------
>
>  For pre-relocation we simply call the driver model init function. Only
> -drivers marked with DM_FLAG_PRE_RELOC or the device tree
> +drivers declared with U_BOOT_DRIVER_F or the device tree
>  'u-boot,dm-pre-reloc' flag are initialised prior to relocation. This helps
>  to reduce the driver model overhead.
>
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 49faa29..6fd2f07 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -157,11 +157,9 @@ int device_bind_by_name(struct udevice *parent, bool pre_reloc_only,
>  {
>         struct driver *drv;
>
> -       drv = lists_driver_lookup_name(info->name);
> +       drv = __lists_driver_lookup_name(info->name, pre_reloc_only);

This patch looks good, except that I would prefer
lists_driver_lookup_name_prereloc() to __lists_driver_lookup_name().
The __ seems like an internal compiler name to me. So can you rename
it?

>         if (!drv)
>                 return -ENOENT;
> -       if (pre_reloc_only && !(drv->flags & DM_FLAG_PRE_RELOC))
> -               return -EPERM;
>
>         return device_bind(parent, drv, info->name, (void *)info->platdata,
>                            -1, devp);
> diff --git a/drivers/core/lists.c b/drivers/core/lists.c
> index 1476715..5b61ab5 100644
> --- a/drivers/core/lists.c
> +++ b/drivers/core/lists.c
> @@ -18,10 +18,13 @@
>  #include <fdtdec.h>
>  #include <linux/compiler.h>
>
> -struct driver *lists_driver_lookup_name(const char *name)
> +#define driver_entry_end(p) (p) ? ll_entry_end(struct driver, driver1) : \
> +                                 ll_entry_end(struct driver, driver2);
> +
> +struct driver *__lists_driver_lookup_name(const char *name, bool pre_reloc_only)
>  {
> -       struct driver *entry = ll_entry_start(struct driver, driver);
> -       struct driver *end = ll_entry_end(struct driver, driver);
> +       struct driver *entry = ll_entry_start(struct driver, driver1);
> +       struct driver *end = driver_entry_end(pre_reloc_only);
>
>         for (; entry < end; entry++) {
>                 if (!strcmp(name, entry->name))
> @@ -58,11 +61,12 @@ int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only)
>
>         for (; entry < end; entry++) {
>                 ret = device_bind_by_name(parent, pre_reloc_only, entry, &dev);
> -               if (ret && ret != -EPERM) {
> +
> +               if (!pre_reloc_only && ret == -ENOENT)
>                         dm_warn("No match for driver '%s'\n", entry->name);
> -                       if (!result || ret != -ENOENT)
> -                               result = ret;
> -               }
> +
> +               if (!result || ret != -ENOENT)
> +                       result = ret;
>         }
>
>         return result;
> @@ -105,8 +109,8 @@ static int driver_check_compatible(const void *blob, int offset,
>  int lists_bind_fdt(struct udevice *parent, const void *blob, int offset,
>                    struct udevice **devp)
>  {
> -       struct driver *entry = ll_entry_start(struct driver, driver);
> -       struct driver *end = ll_entry_end(struct driver, driver);
> +       struct driver *entry = ll_entry_start(struct driver, driver1);
> +       struct driver *end = ll_entry_end(struct driver, driver2);
>         struct udevice *dev;
>         bool found = false;
>         const char *name;
> diff --git a/drivers/core/root.c b/drivers/core/root.c
> index 47b3acf..02970c8 100644
> --- a/drivers/core/root.c
> +++ b/drivers/core/root.c
> @@ -70,7 +70,8 @@ int dm_scan_platdata(bool pre_reloc_only)
>
>         ret = lists_bind_drivers(DM_ROOT_NON_CONST, pre_reloc_only);
>         if (ret == -ENOENT) {
> -               dm_warn("Some drivers were not found\n");
> +               if (!pre_reloc_only)
> +                       dm_warn("Some drivers were not found\n");
>                 ret = 0;
>         }
>
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 9ce95a8..a2fd7b6 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -167,9 +167,13 @@ struct driver {
>         uint32_t flags;
>  };
>
> -/* Declare a new U-Boot driver */
> +/* Drivers bound before and after relocation */
> +#define U_BOOT_DRIVER_F(__name)                                                \
> +       ll_entry_declare(struct driver, __name, driver1)
> +
> +/* Drivers bound only after relocation */
>  #define U_BOOT_DRIVER(__name)                                          \
> -       ll_entry_declare(struct driver, __name, driver)
> +       ll_entry_declare(struct driver, __name, driver2)
>
>  /**
>   * dev_get_platdata() - Get the platform data for a device
> diff --git a/include/dm/lists.h b/include/dm/lists.h
> index 704e33e..fbd428d 100644
> --- a/include/dm/lists.h
> +++ b/include/dm/lists.h
> @@ -13,15 +13,23 @@
>  #include <dm/uclass-id.h>
>
>  /**
> - * lists_driver_lookup_name() - Return u_boot_driver corresponding to name
> + * __lists_driver_lookup_name() - Return u_boot_driver corresponding to name
>   *
>   * This function returns a pointer to a driver given its name. This is used
>   * for binding a driver given its name and platdata.
>   *
>   * @name: Name of driver to look up
> + * @pre_reloc_only: If true, searches in drivers declared with U_BOOT_DRIVER_F.
> + * If false searches in all drivers.
>   * @return pointer to driver, or NULL if not found
>   */
> -struct driver *lists_driver_lookup_name(const char *name);
> +struct driver *__lists_driver_lookup_name(const char *name,
> +                                         bool pre_reloc_only);
> +
> +static inline struct driver *lists_driver_lookup_name(const char *name)
> +{
> +       return __lists_driver_lookup_name(name, false);
> +}
>
>  /**
>   * lists_uclass_lookup() - Return uclass_driver based on ID of the class
> @@ -39,8 +47,8 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id);
>   * each one. The devices will have @parent as their parent.
>   *
>   * @parent: parent device (root)
> - * @early_only: If true, bind only drivers with the DM_INIT_F flag. If false
> - * bind all drivers.
> + * @pre_reloc_only: If true, bind only drivers declared with U_BOOT_DRIVER_F.
> + * If false bind all drivers.
>   */
>  int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only);
>
> diff --git a/include/dm/root.h b/include/dm/root.h
> index c7f0c1d..61d1cd8 100644
> --- a/include/dm/root.h
> +++ b/include/dm/root.h
> @@ -26,8 +26,8 @@ struct udevice *dm_root(void);
>   *
>   * This scans all available platdata and creates drivers for each
>   *
> - * @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
> - * flag. If false bind all drivers.
> + * @pre_reloc_only: If true, bind only drivers declared with U_BOOT_DRIVER_F.
> + * If false bind all drivers.
>   * @return 0 if OK, -ve on error
>   */
>  int dm_scan_platdata(bool pre_reloc_only);
> @@ -54,8 +54,8 @@ int dm_scan_fdt(const void *blob, bool pre_reloc_only);
>   * @parent: Parent device for the devices that will be created
>   * @blob: Pointer to device tree blob
>   * @offset: Offset of node to scan
> - * @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
> - * flag. If false bind all drivers.
> + * @pre_reloc_only: If true, bind only drivers declared with U_BOOT_DRIVER_F.
> + * If false bind all drivers.
>   * @return 0 if OK, -ve on error
>   */
>  int dm_scan_fdt_node(struct udevice *parent, const void *blob, int offset,
> @@ -69,8 +69,8 @@ int dm_scan_fdt_node(struct udevice *parent, const void *blob, int offset,
>   * programmaticaly. They should do this by calling device_bind() on each
>   * device.
>   *
> - * @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
> - * flag. If false bind all drivers.
> + * @pre_reloc_only: If true, bind only drivers declared with U_BOOT_DRIVER_F.
> + * If false bind all drivers.
>   */
>  int dm_scan_other(bool pre_reloc_only);
>
> @@ -81,8 +81,8 @@ int dm_scan_other(bool pre_reloc_only);
>   * then scans and binds available devices from platform data and the FDT.
>   * This calls dm_init() to set up Driver Model structures.
>   *
> - * @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
> - * flag. If false bind all drivers.
> + * @pre_reloc_only: If true, bind only drivers declared with U_BOOT_DRIVER_F.
> + * If false bind all drivers.
>   * @return 0 if OK, -ve on error
>   */
>  int dm_init_and_scan(bool pre_reloc_only);
> --
> 1.9.1
>

Regards,
Simon


More information about the U-Boot mailing list