[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