[U-Boot] [RFC PATCH 07/22] dm: Allow drivers to be marked 'before relocation'
Jon Loeliger
loeliger at gmail.com
Tue May 27 17:36:43 CEST 2014
> Allow drivers to mark themselves as 'pre-reloc' which means that they will
> be initialised prior to relocation. This can be done either with a driver
> flag or with a 'dm,pre-reloc' device tree property.
Hmmm, "dm,pre-reloc" isn't really describing the hardware any more.
That's really a flag for the purpose of telling SW how it should work.
We really should be careful here to maintain the DTS content as a
description of the hardware.
HTH,
jdl
On Sat, May 24, 2014 at 4:21 PM, Simon Glass <sjg at chromium.org> wrote:
> Driver model currently only operates after relocation is complete. In this
> state U-Boot typically has a small amount of memory available. In adding
> support for driver model prior to relocation we must try to use as little
> memory as possible.
>
> In addition, on some machines the memory has not be inited and/or the CPU
> is not running at full speed or the data cache is off. These can reduce
> execution performance, so the less initialisation that is done before
> relocation the better.
>
> An immediately-obvious improvement is to only initialise drivers which are
> actually going to be used before relocation. On many boards the only such
> driver is a serial UART, so this provides a very large potential benefit.
>
> Allow drivers to mark themselves as 'pre-reloc' which means that they will
> be initialised prior to relocation. This can be done either with a driver
> flag or with a 'dm,pre-reloc' device tree property.
>
> To support this, the various dm scanning function now take a 'pre_reloc_only'
> parameter which indicates that only drivers marked pre-reloc should be
> bound.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> common/board_r.c | 4 ++--
> doc/driver-model/README.txt | 23 +++++++++++++++-------
> drivers/core/device.c | 6 ++++--
> drivers/core/lists.c | 6 +++---
> drivers/core/root.c | 11 +++++++----
> include/dm/device-internal.h | 6 ++++--
> include/dm/device.h | 5 +++++
> include/dm/lists.h | 2 +-
> include/dm/root.h | 8 ++++++--
> test/dm/core.c | 47 ++++++++++++++++++++++++++++++++++----------
> test/dm/test-driver.c | 11 +++++++++++
> test/dm/test-fdt.c | 20 ++++++++++++++++++-
> test/dm/test-main.c | 4 ++--
> test/dm/test.dts | 11 +++++++++++
> 14 files changed, 128 insertions(+), 36 deletions(-)
>
> diff --git a/common/board_r.c b/common/board_r.c
> index d1f0aa9..d2a59ee 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -276,13 +276,13 @@ static int initr_dm(void)
> debug("dm_init() failed: %d\n", ret);
> return ret;
> }
> - ret = dm_scan_platdata();
> + ret = dm_scan_platdata(false);
> if (ret) {
> debug("dm_scan_platdata() failed: %d\n", ret);
> return ret;
> }
> #ifdef CONFIG_OF_CONTROL
> - ret = dm_scan_fdt(gd->fdt_blob);
> + ret = dm_scan_fdt(gd->fdt_blob, false);
> if (ret) {
> debug("dm_scan_fdt() failed: %d\n", ret);
> return ret;
> diff --git a/doc/driver-model/README.txt b/doc/driver-model/README.txt
> index e0b395a..deacfe9 100644
> --- a/doc/driver-model/README.txt
> +++ b/doc/driver-model/README.txt
> @@ -332,26 +332,35 @@ dealing with this might not be worth it.
> - Implemented a GPIO system, trying to keep it simple
>
>
> +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 'dm,pre-reloc'
> +flag are initialised prior to relocation. This helps to reduce the driver
> +model overhead.
> +
> +Then post relocation we throw that away and re-init driver model again.
> +For drivers which require some sort of continuity between pre- and
> +post-relocation devices, we can provide access to the pre-relocation
> +device pointers, but this is not currently implemented (the root device
> +pointer is saved but not made available).
> +
> +
> Things to punt for later
> ------------------------
>
> - SPL support - this will have to be present before many drivers can be
> converted, but it seems like we can add it once we are happy with the
> core implementation.
> -- Pre-relocation support - similar story
>
> -That is not to say that no thinking has gone into these - in fact there
> +That is not to say that no thinking has gone into this - in fact there
> is quite a lot there. However, getting these right is non-trivial and
> there is a high cost associated with going down the wrong path.
>
> For SPL, it may be possible to fit in a simplified driver model with only
> bind and probe methods, to reduce size.
>
> -For pre-relocation we can simply call the driver model init function. Then
> -post relocation we throw that away and re-init driver model again. For drivers
> -which require some sort of continuity between pre- and post-relocation
> -devices, we can provide access to the pre-relocation device pointers.
> -
> Uclasses are statically numbered at compile time. It would be possible to
> change this to dynamic numbering, but then we would require some sort of
> lookup service, perhaps searching by name. This is slightly less efficient
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 55ba281..2c2634e 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -129,14 +129,16 @@ fail_bind:
> return ret;
> }
>
> -int device_bind_by_name(struct device *parent, const struct driver_info *info,
> - struct device **devp)
> +int device_bind_by_name(struct device *parent, bool pre_reloc_only,
> + const struct driver_info *info, struct device **devp)
> {
> struct driver *drv;
>
> drv = lists_driver_lookup_name(info->name);
> 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 6a53362..9e38993 100644
> --- a/drivers/core/lists.c
> +++ b/drivers/core/lists.c
> @@ -61,7 +61,7 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id)
> return NULL;
> }
>
> -int lists_bind_drivers(struct device *parent)
> +int lists_bind_drivers(struct device *parent, bool pre_reloc_only)
> {
> struct driver_info *info =
> ll_entry_start(struct driver_info, driver_info);
> @@ -72,8 +72,8 @@ int lists_bind_drivers(struct device *parent)
> int ret;
>
> for (entry = info; entry != info + n_ents; entry++) {
> - ret = device_bind_by_name(parent, entry, &dev);
> - if (ret) {
> + ret = device_bind_by_name(parent, pre_reloc_only, entry, &dev);
> + if (ret && ret != -EPERM) {
> dm_warn("No match for driver '%s'\n", entry->name);
> if (!result || ret != -ENOENT)
> result = ret;
> diff --git a/drivers/core/root.c b/drivers/core/root.c
> index 4873e7b..83299f7 100644
> --- a/drivers/core/root.c
> +++ b/drivers/core/root.c
> @@ -45,7 +45,7 @@ int dm_init(void)
> }
> INIT_LIST_HEAD(&DM_UCLASS_ROOT());
>
> - ret = device_bind_by_name(NULL, &root_info, &DM_ROOT());
> + ret = device_bind_by_name(NULL, false, &root_info, &DM_ROOT());
> if (ret)
> return ret;
> ret = device_probe(DM_ROOT());
> @@ -63,11 +63,11 @@ int dm_uninit(void)
> return 0;
> }
>
> -int dm_scan_platdata(void)
> +int dm_scan_platdata(bool pre_reloc_only)
> {
> int ret;
>
> - ret = lists_bind_drivers(DM_ROOT());
> + ret = lists_bind_drivers(DM_ROOT(), pre_reloc_only);
> if (ret == -ENOENT) {
> dm_warn("Some drivers were not found\n");
> ret = 0;
> @@ -79,7 +79,7 @@ int dm_scan_platdata(void)
> }
>
> #ifdef CONFIG_OF_CONTROL
> -int dm_scan_fdt(const void *blob)
> +int dm_scan_fdt(const void *blob, bool pre_reloc_only)
> {
> int offset = 0;
> int ret = 0, err;
> @@ -88,6 +88,9 @@ int dm_scan_fdt(const void *blob)
> do {
> offset = fdt_next_node(blob, offset, &depth);
> if (offset > 0 && depth == 1) {
> + if (pre_reloc_only &&
> + !fdt_getprop(blob, offset, "dm,pre-reloc", NULL))
> + continue;
> err = lists_bind_fdt(gd->dm_root, blob, offset);
> if (err && !ret)
> ret = err;
> diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
> index e95b7a9..fde67bb 100644
> --- a/include/dm/device-internal.h
> +++ b/include/dm/device-internal.h
> @@ -45,12 +45,14 @@ int device_bind(struct device *parent, struct driver *drv,
> * tree.
> *
> * @parent: Pointer to device's parent
> + * @pre_reloc_only: If true, bind the driver only if its DM_INIT_F flag is set.
> + * If false bind the driver always.
> * @info: Name and platdata for this device
> * @devp: Returns a pointer to the bound device
> * @return 0 if OK, -ve on error
> */
> -int device_bind_by_name(struct device *parent, const struct driver_info *info,
> - struct device **devp);
> +int device_bind_by_name(struct device *parent, bool pre_reloc_only,
> + const struct driver_info *info, struct device **devp);
>
> /**
> * device_probe() - Probe a device, activating it
> diff --git a/include/dm/device.h b/include/dm/device.h
> index b048b66..32650fd 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -23,6 +23,9 @@ struct driver_info;
> /* DM is responsible for allocating and freeing platdata */
> #define DM_FLAG_ALLOC_PDATA (1 << 1)
>
> +/* DM should init this device prior to relocation */
> +#define DM_FLAG_PRE_RELOC (1 << 2)
> +
> /**
> * struct device - An instance of a driver
> *
> @@ -117,6 +120,7 @@ struct device_id {
> * ops: Driver-specific operations. This is typically a list of function
> * pointers defined by the driver, to implement driver functions required by
> * the uclass.
> + * @flags: driver flags - see DM_FLAGS_...
> */
> struct driver {
> char *name;
> @@ -130,6 +134,7 @@ struct driver {
> int priv_auto_alloc_size;
> int platdata_auto_alloc_size;
> const void *ops; /* driver-specific operations */
> + uint32_t flags;
> };
>
> /* Declare a new U-Boot driver */
> diff --git a/include/dm/lists.h b/include/dm/lists.h
> index d0b720b..13e025e 100644
> --- a/include/dm/lists.h
> +++ b/include/dm/lists.h
> @@ -42,7 +42,7 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id);
> * @early_only: If true, bind only drivers with the DM_INIT_F flag. If false
> * bind all drivers.
> */
> -int lists_bind_drivers(struct device *parent);
> +int lists_bind_drivers(struct device *parent, bool pre_reloc_only);
>
> /**
> * lists_bind_fdt() - bind a device tree node
> diff --git a/include/dm/root.h b/include/dm/root.h
> index 1631d5d..30ebe6c 100644
> --- a/include/dm/root.h
> +++ b/include/dm/root.h
> @@ -26,9 +26,11 @@ struct device *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.
> * @return 0 if OK, -ve on error
> */
> -int dm_scan_platdata(void);
> +int dm_scan_platdata(bool pre_reloc_only);
>
> /**
> * dm_scan_fdt() - Scan the device tree and bind drivers
> @@ -36,9 +38,11 @@ int dm_scan_platdata(void);
> * This scans the device tree and creates a driver for each node
> *
> * @blob: Pointer to device tree blob
> + * @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
> + * flag. If false bind all drivers.
> * @return 0 if OK, -ve on error
> */
> -int dm_scan_fdt(const void *blob);
> +int dm_scan_fdt(const void *blob, bool pre_reloc_only);
>
> /**
> * dm_init() - Initialise Driver Model structures
> diff --git a/test/dm/core.c b/test/dm/core.c
> index a889fad..63bb561 100644
> --- a/test/dm/core.c
> +++ b/test/dm/core.c
> @@ -25,6 +25,7 @@ enum {
> TEST_INTVAL2 = 3,
> TEST_INTVAL3 = 6,
> TEST_INTVAL_MANUAL = 101112,
> + TEST_INTVAL_PRE_RELOC = 7,
> };
>
> static const struct dm_test_pdata test_pdata[] = {
> @@ -37,6 +38,10 @@ static const struct dm_test_pdata test_pdata_manual = {
> .ping_add = TEST_INTVAL_MANUAL,
> };
>
> +static const struct dm_test_pdata test_pdata_pre_reloc = {
> + .ping_add = TEST_INTVAL_PRE_RELOC,
> +};
> +
> U_BOOT_DEVICE(dm_test_info1) = {
> .name = "test_drv",
> .platdata = &test_pdata[0],
> @@ -57,6 +62,11 @@ static struct driver_info driver_info_manual = {
> .platdata = &test_pdata_manual,
> };
>
> +static struct driver_info driver_info_pre_reloc = {
> + .name = "test_pre_reloc_drv",
> + .platdata = &test_pdata_manual,
> +};
> +
> /* Test that binding with platdata occurs correctly */
> static int dm_test_autobind(struct dm_test_state *dms)
> {
> @@ -71,7 +81,7 @@ static int dm_test_autobind(struct dm_test_state *dms)
> ut_asserteq(0, list_count_items(&gd->dm_root->child_head));
> ut_asserteq(0, dm_testdrv_op_count[DM_TEST_OP_POST_BIND]);
>
> - ut_assertok(dm_scan_platdata());
> + ut_assertok(dm_scan_platdata(false));
>
> /* We should have our test class now at least, plus more children */
> ut_assert(1 < list_count_items(&gd->uclass_root));
> @@ -181,7 +191,7 @@ static int dm_test_lifecycle(struct dm_test_state *dms)
>
> memcpy(op_count, dm_testdrv_op_count, sizeof(op_count));
>
> - ut_assertok(device_bind_by_name(dms->root, &driver_info_manual,
> + ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual,
> &dev));
> ut_assert(dev);
> ut_assert(dm_testdrv_op_count[DM_TEST_OP_BIND]
> @@ -232,15 +242,15 @@ static int dm_test_ordering(struct dm_test_state *dms)
> struct device *dev, *dev_penultimate, *dev_last, *test_dev;
> int pingret;
>
> - ut_assertok(device_bind_by_name(dms->root, &driver_info_manual,
> + ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual,
> &dev));
> ut_assert(dev);
>
> /* Bind two new devices (numbers 4 and 5) */
> - ut_assertok(device_bind_by_name(dms->root, &driver_info_manual,
> + ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual,
> &dev_penultimate));
> ut_assert(dev_penultimate);
> - ut_assertok(device_bind_by_name(dms->root, &driver_info_manual,
> + ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual,
> &dev_last));
> ut_assert(dev_last);
>
> @@ -255,7 +265,8 @@ static int dm_test_ordering(struct dm_test_state *dms)
> ut_assert(dev_last == test_dev);
>
> /* Add back the original device 3, now in position 5 */
> - ut_assertok(device_bind_by_name(dms->root, &driver_info_manual, &dev));
> + ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual,
> + &dev));
> ut_assert(dev);
>
> /* Try ping */
> @@ -375,8 +386,8 @@ static int dm_test_leak(struct dm_test_state *dms)
> if (!start.uordblks)
> puts("Warning: Please add '#define DEBUG' to the top of common/dlmalloc.c\n");
>
> - ut_assertok(dm_scan_platdata());
> - ut_assertok(dm_scan_fdt(gd->fdt_blob));
> + ut_assertok(dm_scan_platdata(false));
> + ut_assertok(dm_scan_fdt(gd->fdt_blob, false));
>
> /* Scanning the uclass is enough to probe all the devices */
> for (id = UCLASS_ROOT; id < UCLASS_COUNT; id++) {
> @@ -444,8 +455,8 @@ static int create_children(struct dm_test_state *dms, struct device *parent,
> for (i = 0; i < count; i++) {
> struct dm_test_pdata *pdata;
>
> - ut_assertok(device_bind_by_name(parent, &driver_info_manual,
> - &dev));
> + ut_assertok(device_bind_by_name(parent, false,
> + &driver_info_manual, &dev));
> pdata = calloc(1, sizeof(*pdata));
> pdata->ping_add = key + i;
> dev->platdata = pdata;
> @@ -542,3 +553,19 @@ static int dm_test_children(struct dm_test_state *dms)
> return 0;
> }
> DM_TEST(dm_test_children, 0);
> +
> +static int dm_test_pre_reloc(struct dm_test_state *dms)
> +{
> + struct device *dev;
> +
> + /* The normal driver should refuse to bind before relocation */
> + ut_asserteq(-EPERM, device_bind_by_name(dms->root, true,
> + &driver_info_manual, &dev));
> +
> + /* But this one is marked pre-reloc */
> + ut_assertok(device_bind_by_name(dms->root, true,
> + &driver_info_pre_reloc, &dev));
> +
> + return 0;
> +}
> +DM_TEST(dm_test_pre_reloc, 0);
> diff --git a/test/dm/test-driver.c b/test/dm/test-driver.c
> index c4be8a1..e7fb199 100644
> --- a/test/dm/test-driver.c
> +++ b/test/dm/test-driver.c
> @@ -144,3 +144,14 @@ U_BOOT_DRIVER(test_manual_drv) = {
> .remove = test_manual_remove,
> .unbind = test_manual_unbind,
> };
> +
> +U_BOOT_DRIVER(test_pre_reloc_drv) = {
> + .name = "test_pre_reloc_drv",
> + .id = UCLASS_TEST,
> + .ops = &test_manual_ops,
> + .bind = test_manual_bind,
> + .probe = test_manual_probe,
> + .remove = test_manual_remove,
> + .unbind = test_manual_unbind,
> + .flags = DM_FLAG_PRE_RELOC,
> +};
> diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c
> index e1d982f..d6f5bb8 100644
> --- a/test/dm/test-fdt.c
> +++ b/test/dm/test-fdt.c
> @@ -99,7 +99,7 @@ static int dm_test_fdt(struct dm_test_state *dms)
> int ret;
> int i;
>
> - ret = dm_scan_fdt(gd->fdt_blob);
> + ret = dm_scan_fdt(gd->fdt_blob, false);
> ut_assert(!ret);
>
> ret = uclass_get(UCLASS_TEST_FDT, &uc);
> @@ -142,3 +142,21 @@ static int dm_test_fdt(struct dm_test_state *dms)
> return 0;
> }
> DM_TEST(dm_test_fdt, 0);
> +
> +static int dm_test_fdt_pre_reloc(struct dm_test_state *dms)
> +{
> + struct uclass *uc;
> + int ret;
> +
> + ret = dm_scan_fdt(gd->fdt_blob, true);
> + ut_assert(!ret);
> +
> + ret = uclass_get(UCLASS_TEST_FDT, &uc);
> + ut_assert(!ret);
> +
> + /* These is only one pre-reloc device */
> + ut_asserteq(1, list_count_items(&uc->dev_head));
> +
> + return 0;
> +}
> +DM_TEST(dm_test_fdt_pre_reloc, 0);
> diff --git a/test/dm/test-main.c b/test/dm/test-main.c
> index 828ed46..49805eb 100644
> --- a/test/dm/test-main.c
> +++ b/test/dm/test-main.c
> @@ -89,11 +89,11 @@ int dm_test_main(void)
> ut_assertok(dm_test_init(dms));
>
> if (test->flags & DM_TESTF_SCAN_PDATA)
> - ut_assertok(dm_scan_platdata());
> + ut_assertok(dm_scan_platdata(false));
> if (test->flags & DM_TESTF_PROBE_TEST)
> ut_assertok(do_autoprobe(dms));
> if (test->flags & DM_TESTF_SCAN_FDT)
> - ut_assertok(dm_scan_fdt(gd->fdt_blob));
> + ut_assertok(dm_scan_fdt(gd->fdt_blob, false));
>
> if (test->func(dms))
> break;
> diff --git a/test/dm/test.dts b/test/dm/test.dts
> index ec5364f..b2eaddd 100644
> --- a/test/dm/test.dts
> +++ b/test/dm/test.dts
> @@ -6,10 +6,21 @@
> #address-cells = <1>;
> #size-cells = <0>;
>
> + alias {
> + console = <&uart0>;
> + };
> +
> + uart0: serial {
> + compatible = "sandbox,serial";
> + dm,pre-reloc;
> + //text-colour = "cyan";
> + };
> +
> a-test {
> reg = <0>;
> compatible = "denx,u-boot-fdt-test";
> ping-add = <0>;
> + dm,pre-reloc;
> };
>
> junk {
> --
> 1.9.1.423.g4596e3a
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
More information about the U-Boot
mailing list