Hi Jon,
On 27 May 2014 09:36, Jon Loeliger <loeli...@gmail.com> wrote:
>> 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.
Yes this always comes up. Another approach would be to declare that a
serial driver must be available and just use the console alias and
assume that the device must be pre-reloc. But I worry that with I2C,
SPI , etc. potentially wanting pre-reloc init this might be a losing
battle.
This way, board authors have complete flexibility, and the file is in
after all the U-Boot source tree.
Regards,
Simon
>
> HTH,
> jdl
>
> On Sat, May 24, 2014 at 4:21 PM, Simon Glass <s...@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 <s...@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@lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
