We can use extern instead, so let's drop these macros. It adds one more thing to learn about and doesn't make the code any clearer.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: - Add new patch to drop DECL() macros from dtoc (to squash in)

tools/dtoc/dtb_platdata.py | 8 ++--- tools/dtoc/test_dtoc.py | 64 +++++++++++++++++++------------------- 2 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index f5b5ad5f71a..b5c449ebb47 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -902,14 +902,14 @@ class DtbPlatdata(): self.buf( '/* driver declarations - these allow DM_DRIVER_GET() to be used */\n') for node in nodes_to_output: - self.buf('DM_DRIVER_DECL(%s);\n' % node.struct_name); + self.buf('extern U_BOOT_DRIVER(%s);\n' % node.struct_name); self.buf('\n')

if self._instantiate: self.buf( '/* device declarations - these allow DM_DEVICE_REF() to be used */\n') for node in nodes_to_output: - self.buf('DM_DEVICE_DECL(%s);\n' % node.var_name) + self.buf('extern DM_DEVICE_INST(%s);\n' % node.var_name) self.buf('\n')

uclass_list = self._valid_uclasses @@ -917,13 +917,13 @@ class DtbPlatdata(): self.buf( '/* uclass driver declarations - needed for DM_UCLASS_DRIVER_REF() */\n') for uclass in uclass_list: - self.buf('DM_UCLASS_DRIVER_DECL(%s);\n' % uclass.name) + self.buf('extern UCLASS_DRIVER(%s);\n' % uclass.name)

if self._instantiate: self.buf('\n') self.buf('/* uclass declarations - needed for DM_UCLASS_REF() */\n') for uclass in uclass_list: - self.buf('DM_UCLASS_DECL(%s);\n' % uclass.name) + self.buf('extern DM_UCLASS_INST(%s);\n' % uclass.name) self.out(''.join(self.get_buf()))

def assign_seqs(self): diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index bb689f3b6e9..1912a8723f2 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -241,52 +241,52 @@ class TestDtoc(unittest.TestCase): #include <dm/uclass-internal.h>

/* driver declarations - these allow DM_DRIVER_GET() to be used */ -DM_DRIVER_DECL(sandbox_i2c); -DM_DRIVER_DECL(sandbox_pmic); -DM_DRIVER_DECL(sandbox_spl_test); -DM_DRIVER_DECL(sandbox_spl_test); -DM_DRIVER_DECL(sandbox_spl_test); +extern U_BOOT_DRIVER(sandbox_i2c); +extern U_BOOT_DRIVER(sandbox_pmic); +extern U_BOOT_DRIVER(sandbox_spl_test); +extern U_BOOT_DRIVER(sandbox_spl_test); +extern U_BOOT_DRIVER(sandbox_spl_test);

/* uclass driver declarations - needed for DM_UCLASS_DRIVER_REF() */ -DM_UCLASS_DRIVER_DECL(i2c); -DM_UCLASS_DRIVER_DECL(misc); -DM_UCLASS_DRIVER_DECL(pmic); +extern UCLASS_DRIVER(i2c); +extern UCLASS_DRIVER(misc); +extern UCLASS_DRIVER(pmic); ''' decl_text_inst = DECL_HEADER + ''' #include <dm/device-internal.h> #include <dm/uclass-internal.h>

/* driver declarations - these allow DM_DRIVER_GET() to be used */ -DM_DRIVER_DECL(sandbox_i2c); -DM_DRIVER_DECL(root_driver); -DM_DRIVER_DECL(denx_u_boot_test_bus); -DM_DRIVER_DECL(sandbox_spl_test); -DM_DRIVER_DECL(sandbox_spl_test); -DM_DRIVER_DECL(denx_u_boot_fdt_test); -DM_DRIVER_DECL(denx_u_boot_fdt_test); +extern U_BOOT_DRIVER(sandbox_i2c); +extern U_BOOT_DRIVER(root_driver); +extern U_BOOT_DRIVER(denx_u_boot_test_bus); +extern U_BOOT_DRIVER(sandbox_spl_test); +extern U_BOOT_DRIVER(sandbox_spl_test); +extern U_BOOT_DRIVER(denx_u_boot_fdt_test); +extern U_BOOT_DRIVER(denx_u_boot_fdt_test);

/* device declarations - these allow DM_DEVICE_REF() to be used */ -DM_DEVICE_DECL(i2c); -DM_DEVICE_DECL(root); -DM_DEVICE_DECL(some_bus); -DM_DEVICE_DECL(spl_test); -DM_DEVICE_DECL(spl_test3); -DM_DEVICE_DECL(test); -DM_DEVICE_DECL(test0); +extern DM_DEVICE_INST(i2c); +extern DM_DEVICE_INST(root); +extern DM_DEVICE_INST(some_bus); +extern DM_DEVICE_INST(spl_test); +extern DM_DEVICE_INST(spl_test3); +extern DM_DEVICE_INST(test); +extern DM_DEVICE_INST(test0);

/* uclass driver declarations - needed for DM_UCLASS_DRIVER_REF() */ -DM_UCLASS_DRIVER_DECL(i2c); -DM_UCLASS_DRIVER_DECL(misc); -DM_UCLASS_DRIVER_DECL(root); -DM_UCLASS_DRIVER_DECL(testbus); -DM_UCLASS_DRIVER_DECL(testfdt); +extern UCLASS_DRIVER(i2c); +extern UCLASS_DRIVER(misc); +extern UCLASS_DRIVER(root); +extern UCLASS_DRIVER(testbus); +extern UCLASS_DRIVER(testfdt);

/* uclass declarations - needed for DM_UCLASS_REF() */ -DM_UCLASS_DECL(i2c); -DM_UCLASS_DECL(misc); -DM_UCLASS_DECL(root); -DM_UCLASS_DECL(testbus); -DM_UCLASS_DECL(testfdt); +extern DM_UCLASS_INST(i2c); +extern DM_UCLASS_INST(misc); +extern DM_UCLASS_INST(root); +extern DM_UCLASS_INST(testbus); +extern DM_UCLASS_INST(testfdt); ''' struct_text = HEADER + ''' struct dtd_sandbox_i2c {

At present linker lists are designed for use in code. They make use of statements within expressions ({...}), for example.

It is possible to generate a reference to a linker_list entry that can be used in data structures, where such features are not permitted. It requires that the reference first be declared as extern. In other words the existing macro needs to be split into two parts.

Add new macros to support this.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: - Drop the decl() variant since it is just as easy to use extern

include/linker_lists.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)

diff --git a/include/linker_lists.h b/include/linker_lists.h index fd98ecd297c..0ca30da4172 100644 --- a/include/linker_lists.h +++ b/include/linker_lists.h @@ -211,6 +211,18 @@ _ll_result; \ })

+/** + * ll_entry_ref() - Get a reference to a linker-generated array entry + * + * Once ll_entry_decl() has been used to declare the reference, this macro + * allows the entry to be accessed. + * + * This is like ll_entry_get(), but without the extra code, so it is suitable + * for putting into data structures. + */ +#define ll_entry_ref(_type, _name, _list) \ + ((_type *)&_u_boot_list_2_##_list##_2_##_name) + /** * ll_start() - Point to first entry of first linker-generated array * @_type: Data type of the entry

With OF_PLATDATA_INST devices are bound at build time. We should not need binding of devices at runtime in most cases. However it is inflexible to absolutely prohibit it, so add an option to control this.

Update the driver model core so that it does not bind devices. Update device_bind() to return an error if called.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: - Update the commit message to explain OF_PLATDATA_NO_BIND better - Update OF_PLATDATA_NO_BIND help as well

drivers/core/device.c | 42 +++++++++++++++++++++++++----------------- dts/Kconfig | 18 ++++++++++++++++++ 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/drivers/core/device.c b/drivers/core/device.c index c0547b75c76..b75a87e60a4 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -44,6 +44,9 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, bool auto_seq = true; void *ptr;

+ if (CONFIG_IS_ENABLED(OF_PLATDATA_NO_BIND)) + return -ENOSYS; + if (devp) *devp = NULL; if (!name) @@ -394,26 +397,31 @@ int device_of_to_plat(struct udevice *dev) if (dev_get_flags(dev) & DM_FLAG_PLATDATA_VALID) return 0;

- /* Ensure all parents have ofdata */ - if (dev->parent) { - ret = device_of_to_plat(dev->parent); + /* + * This is not needed if binding is disabled, since data is allocated + * at build time. + */ + if (!CONFIG_IS_ENABLED(OF_PLATDATA_NO_BIND)) { + /* Ensure all parents have ofdata */ + if (dev->parent) { + ret = device_of_to_plat(dev->parent); + if (ret) + goto fail; + + /* + * The device might have already been probed during + * the call to device_probe() on its parent device + * (e.g. PCI bridge devices). Test the flags again + * so that we don't mess up the device. + */ + if (dev_get_flags(dev) & DM_FLAG_PLATDATA_VALID) + return 0; + } + + ret = device_alloc_priv(dev); if (ret) goto fail; - - /* - * The device might have already been probed during - * the call to device_probe() on its parent device - * (e.g. PCI bridge devices). Test the flags again - * so that we don't mess up the device. - */ - if (dev_get_flags(dev) & DM_FLAG_PLATDATA_VALID) - return 0; } - - ret = device_alloc_priv(dev); - if (ret) - goto fail; - drv = dev->driver; assert(drv);

diff --git a/dts/Kconfig b/dts/Kconfig index c39cc368881..d289752a139 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -371,6 +371,15 @@ config SPL_OF_PLATDATA_INST Declare devices as udevice instances so that they do not need to be bound when U-Boot starts. This can save time and code space.

+config SPL_OF_PLATDATA_NO_BIND + bool "Don't allow run-time binding of devices" + depends on SPL_OF_PLATDATA_INST + default y + help + This removes the ability to bind devices at run time, thus saving + some code space in U-Boot. This can be disabled if binding is needed, + at the code of some code size increase. + endif

config TPL_OF_PLATDATA @@ -411,6 +420,15 @@ config TPL_OF_PLATDATA_INST Declare devices as udevice instances so that they do not need to be bound when U-Boot starts. This can save time and code space.

+config TPL_OF_PLATDATA_NO_BIND + bool "Don't allow run-time binding of devices" + depends on TPL_OF_PLATDATA_INST + default y + help + This removes the ability to bind devices at run time, thus saving + some code space in U-Boot. This can be disabled if binding is needed, + at the code of some code size increase. + endif

endmenu

With this we don't need to scan and bind drivers, not even the root device. We just need to locate the root device that was set up at build time, then set our root in global_data to point to it.

Update the code to handle this case.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

drivers/core/root.c | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/core/root.c b/drivers/core/root.c index 8769289b26e..b06e508689a 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -128,6 +128,13 @@ void fix_devices(void) } }

+static int dm_setup_inst(void) +{ + DM_ROOT_NON_CONST = DM_DEVICE_GET(root); + + return 0; +} + int dm_init(bool of_live) { int ret; @@ -152,14 +159,23 @@ int dm_init(bool of_live) fix_devices(); }

- ret = device_bind_by_name(NULL, false, &root_info, &DM_ROOT_NON_CONST); - if (ret) - return ret; - if (CONFIG_IS_ENABLED(OF_CONTROL)) - dev_set_ofnode(DM_ROOT_NON_CONST, ofnode_root()); - ret = device_probe(DM_ROOT_NON_CONST); - if (ret) - return ret; + if (CONFIG_IS_ENABLED(OF_PLATDATA_INST)) { + ret = dm_setup_inst(); + if (ret) { + log_debug("dm_setup_inst() failed: %d\n", ret); + return ret; + } + } else { + ret = device_bind_by_name(NULL, false, &root_info, + &DM_ROOT_NON_CONST); + if (ret) + return ret; + if (CONFIG_IS_ENABLED(OF_CONTROL)) + dev_set_ofnode(DM_ROOT_NON_CONST, ofnode_root()); + ret = device_probe(DM_ROOT_NON_CONST); + if (ret) + return ret; + }

return 0; } @@ -350,10 +366,12 @@ int dm_init_and_scan(bool pre_reloc_only) debug("dm_init() failed: %d\n", ret); return ret; } - ret = dm_scan(pre_reloc_only); - if (ret) { - log_debug("dm_scan() failed: %d\n", ret); - return ret; + if (!CONFIG_IS_ENABLED(OF_PLATDATA_INST)) { + ret = dm_scan(pre_reloc_only); + if (ret) { + log_debug("dm_scan() failed: %d\n", ret); + return ret; + } }

return 0;

Now that dtoc generates some new C files, add these to the build so that the instantiated devices and uclasses can be used.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

scripts/Makefile.spl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 52609112ecd..fe9b616dcc4 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -120,7 +120,8 @@ endif u-boot-spl-init := $(head-y) u-boot-spl-main := $(libs-y) ifdef CONFIG_$(SPL_TPL_)OF_PLATDATA -u-boot-spl-platdata := $(obj)/dts/dt-plat.o +u-boot-spl-platdata := $(obj)/dts/dt-plat.o $(obj)/dts/dt-uclass.o \ + $(obj)/dts/dt-device.o u-boot-spl-platdata_c := $(patsubst %.o,%.c,$(u-boot-spl-platdata)) endif

It is not possible to destroy the uclasses when they are created at build time. Skip this step so that SPL test can complete successfully.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v2)

Changes in v2: - Update to deal with test refactoring series

test/test-main.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/test/test-main.c b/test/test-main.c index e1b49e091ab..46a0c2ee2f6 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -62,17 +62,25 @@ static int dm_test_post_run(struct unit_test_state *uts) { int id;

- for (id = 0; id < UCLASS_COUNT; id++) { - struct uclass *uc; - - /* - * If the uclass doesn't exist we don't want to create it. So - * check that here before we call uclass_find_device(). - */ - uc = uclass_find(id); - if (!uc) - continue; - ut_assertok(uclass_destroy(uc)); + /* + * With of-platdata-inst the uclasses are created at build time. If we + * destroy them we cannot get them back since uclass_add() is not + * supported. So skip this. + */ + if (!CONFIG_IS_ENABLED(OF_PLATDATA_INST)) { + for (id = 0; id < UCLASS_COUNT; id++) { + struct uclass *uc; + + /* + * If the uclass doesn't exist we don't want to create + * it. So check that here before we call + * uclass_find_device(). + */ + uc = uclass_find(id); + if (!uc) + continue; + ut_assertok(uclass_destroy(uc)); + } }

return 0;

We need to allow SoCs to create their own drivers for this so that they can use their own of-platdata structs. To minimise code duplication, export the driver operations and the ofdata_to_plat() setup function.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

drivers/clk/clk_fixed_rate.c | 14 ++++++++++---- include/linux/clk-provider.h | 5 +++++ 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk_fixed_rate.c b/drivers/clk/clk_fixed_rate.c index 3c5a83c523c..09f9ef26a42 100644 --- a/drivers/clk/clk_fixed_rate.c +++ b/drivers/clk/clk_fixed_rate.c @@ -25,18 +25,24 @@ const struct clk_ops clk_fixed_rate_ops = { .enable = dummy_enable, };

-static int clk_fixed_rate_of_to_plat(struct udevice *dev) +void clk_fixed_rate_ofdata_to_plat_(struct udevice *dev, + struct clk_fixed_rate *plat) { - struct clk *clk = &to_clk_fixed_rate(dev)->clk; + struct clk *clk = &plat->clk; #if !CONFIG_IS_ENABLED(OF_PLATDATA) - to_clk_fixed_rate(dev)->fixed_rate = - dev_read_u32_default(dev, "clock-frequency", 0); + plat->fixed_rate = dev_read_u32_default(dev, "clock-frequency", 0); #endif /* Make fixed rate clock accessible from higher level struct clk */ /* FIXME: This is not allowed */ dev_set_uclass_priv(dev, clk); + clk->dev = dev; clk->enable_count = 0; +} + +static int clk_fixed_rate_of_to_plat(struct udevice *dev) +{ + clk_fixed_rate_ofdata_to_plat_(dev, to_clk_fixed_rate(dev));

return 0; } diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index c871ea646d3..f9ca8e433ec 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -193,6 +193,8 @@ struct clk_fixed_factor { unsigned int div; };

+extern const struct clk_ops clk_fixed_rate_ops; + #define to_clk_fixed_factor(_clk) container_of(_clk, struct clk_fixed_factor,\ clk)

@@ -203,6 +205,9 @@ struct clk_fixed_rate {

#define to_clk_fixed_rate(dev) ((struct clk_fixed_rate *)dev_get_plat(dev))

+void clk_fixed_rate_ofdata_to_plat_(struct udevice *dev, + struct clk_fixed_rate *plat); + struct clk_composite { struct clk clk; struct clk_ops ops;

This function is now only used in a test. Drop it. Also drop DM_DRVINFO_GET() which was the only purpose for having the function.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: - Drop DM_DRVINFO_GET() also

drivers/core/device.c | 15 --------------- include/dm/device.h | 15 --------------- include/dm/platdata.h | 15 --------------- test/dm/of_platdata.c | 7 ++----- 4 files changed, 2 insertions(+), 50 deletions(-)

diff --git a/drivers/core/device.c b/drivers/core/device.c index b75a87e60a4..4ca63d904e2 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -769,21 +769,6 @@ int device_get_global_by_ofnode(ofnode ofnode, struct udevice **devp) }

#if CONFIG_IS_ENABLED(OF_PLATDATA) -int device_get_by_driver_info(const struct driver_info *info, - struct udevice **devp) -{ - struct driver_info *info_base = - ll_entry_start(struct driver_info, driver_info); - int idx = info - info_base; - struct driver_rt *drt = gd_dm_driver_rt() + idx; - struct udevice *dev; - - dev = drt->dev; - *devp = NULL; - - return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp); -} - int device_get_by_driver_info_idx(uint idx, struct udevice **devp) { struct driver_rt *drt = gd_dm_driver_rt() + idx; diff --git a/include/dm/device.h b/include/dm/device.h index 42b3895d5ed..b7208ab032b 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -653,21 +653,6 @@ int device_find_global_by_ofnode(ofnode node, struct udevice **devp); */ int device_get_global_by_ofnode(ofnode node, struct udevice **devp);

-/** - * device_get_by_driver_info() - Get a device based on driver_info - * - * Locates a device by its struct driver_info, by using its reference which - * is updated during the bind process. - * - * The device is probed to activate it ready for use. - * - * @info: Struct driver_info - * @devp: Returns pointer to device if found, otherwise this is set to NULL - * @return 0 if OK, -ve on error - */ -int device_get_by_driver_info(const struct driver_info *info, - struct udevice **devp); - /** * device_get_by_driver_info_idx() - Get a device based on driver_info index * diff --git a/include/dm/platdata.h b/include/dm/platdata.h index 3821a56f2ca..4efb1dfe12d 100644 --- a/include/dm/platdata.h +++ b/include/dm/platdata.h @@ -71,19 +71,4 @@ struct driver_rt { #define U_BOOT_DRVINFOS(__name) \ ll_entry_declare_list(struct driver_info, __name, driver_info)

-/** - * Get a pointer to a given device info given its name - * - * With the declaration U_BOOT_DRVINFO(name), DM_DRVINFO_GET(name) will return a - * pointer to the struct driver_info created by that declaration. - * - * if OF_PLATDATA is enabled, from this it is possible to use the @dev member of - * struct driver_info to find the device pointer itself. - * - * @__name: Driver name (C identifier, not a string. E.g. gpio7_at_ff7e0000) - * @return struct driver_info * to the driver that created the device - */ -#define DM_DRVINFO_GET(__name) \ - ll_entry_get(struct driver_info, __name, driver_info) - #endif diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c index f93a75b4481..0de4ab5dafa 100644 --- a/test/dm/of_platdata.c +++ b/test/dm/of_platdata.c @@ -141,8 +141,6 @@ static int find_driver_info(struct unit_test_state *uts, struct udevice *parent, /* Check that every device is recorded in its driver_info struct */ static int dm_test_of_plat_dev(struct unit_test_state *uts) { - const struct driver_info *info = - ll_entry_start(struct driver_info, driver_info); const int n_ents = ll_entry_count(struct driver_info, driver_info); bool found[n_ents]; uint i; @@ -154,18 +152,17 @@ static int dm_test_of_plat_dev(struct unit_test_state *uts) /* Make sure that the driver entries without devices have no ->dev */ for (i = 0; i < n_ents; i++) { const struct driver_rt *drt = gd_dm_driver_rt() + i; - const struct driver_info *entry = info + i; struct udevice *dev;

if (found[i]) { /* Make sure we can find it */ ut_assertnonnull(drt->dev); - ut_assertok(device_get_by_driver_info(entry, &dev)); + ut_assertok(device_get_by_driver_info_idx(i, &dev)); ut_asserteq_ptr(dev, drt->dev); } else { ut_assertnull(drt->dev); ut_asserteq(-ENOENT, - device_get_by_driver_info(entry, &dev)); + device_get_by_driver_info_idx(i, &dev)); } }

At present the structs used by these drivers are declared in the C files and so are not accessible to dtoc. Move them to header files, as required.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

arch/sandbox/include/asm/i2c.h | 15 +++++++++++++++ arch/sandbox/include/asm/rtc.h | 24 ++++++++++++++++++++++++ drivers/i2c/i2c-emul-uclass.c | 13 +------------ drivers/rtc/i2c_rtc_emul.c | 19 ------------------- 4 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/arch/sandbox/include/asm/i2c.h b/arch/sandbox/include/asm/i2c.h index b482be485ca..4fc190be4bd 100644 --- a/arch/sandbox/include/asm/i2c.h +++ b/arch/sandbox/include/asm/i2c.h @@ -11,4 +11,19 @@ struct sandbox_i2c_priv { bool test_mode; };

+/** + * struct i2c_emul_uc_plat - information about the emulator for this device + * + * This is used by devices in UCLASS_I2C_EMUL to record information about the + * device being emulated. It is accessible with dev_get_uclass_plat() + * + * @dev: Device being emulated + * @idx: of-platdata index, set up by the device's bind() method if of-platdata + * is in use + */ +struct i2c_emul_uc_plat { + struct udevice *dev; + int idx; +}; + #endif /* __asn_i2c_h */ diff --git a/arch/sandbox/include/asm/rtc.h b/arch/sandbox/include/asm/rtc.h index 5bb032f59f2..025cd6c67cf 100644 --- a/arch/sandbox/include/asm/rtc.h +++ b/arch/sandbox/include/asm/rtc.h @@ -9,6 +9,8 @@ #ifndef __asm_rtc_h #define __asm_rtc_h

+#include <dt-structs.h> + /* Register numbers in the sandbox RTC */ enum { REG_SEC = 5, @@ -29,4 +31,26 @@ enum { REG_COUNT = 0x80, };

+/** + * struct sandbox_i2c_rtc_plat_data - platform data for the RTC + * + * @base_time: Base system time when RTC device was bound + * @offset: RTC offset from current system time + * @use_system_time: true to use system time, false to use @base_time + * @reg: Register values + */ +struct sandbox_i2c_rtc_plat_data { +#if CONFIG_IS_ENABLED(OF_PLATDATA) + struct dtd_sandbox_i2c_rtc_emul dtplat; +#endif + long base_time; + long offset; + bool use_system_time; + u8 reg[REG_COUNT]; +}; + +struct sandbox_i2c_rtc { + unsigned int offset_secs; +}; + #endif diff --git a/drivers/i2c/i2c-emul-uclass.c b/drivers/i2c/i2c-emul-uclass.c index 75d79883380..9ef4651fa43 100644 --- a/drivers/i2c/i2c-emul-uclass.c +++ b/drivers/i2c/i2c-emul-uclass.c @@ -7,6 +7,7 @@ #include <dm.h> #include <i2c.h> #include <log.h> +#include <asm/i2c.h> #include <dm/device-internal.h> #include <dm/uclass-internal.h>

@@ -23,18 +24,6 @@ * uclass so avoid having strange devices on the I2C bus. */

-/** - * struct i2c_emul_uc_plat - information about the emulator for this device - * - * This is used by devices in UCLASS_I2C_EMUL to record information about the - * device being emulated. It is accessible with dev_get_uclass_plat() - * - * @dev: Device being emulated - */ -struct i2c_emul_uc_plat { - struct udevice *dev; -}; - struct udevice *i2c_emul_get_device(struct udevice *emul) { struct i2c_emul_uc_plat *uc_plat = dev_get_uclass_plat(emul); diff --git a/drivers/rtc/i2c_rtc_emul.c b/drivers/rtc/i2c_rtc_emul.c index fdc885c518b..ba418c25daf 100644 --- a/drivers/rtc/i2c_rtc_emul.c +++ b/drivers/rtc/i2c_rtc_emul.c @@ -28,25 +28,6 @@ #define debug_buffer(x, ...) #endif

-/** - * struct sandbox_i2c_rtc_plat_data - platform data for the RTC - * - * @base_time: Base system time when RTC device was bound - * @offset: RTC offset from current system time - * @use_system_time: true to use system time, false to use @base_time - * @reg: Register values - */ -struct sandbox_i2c_rtc_plat_data { - long base_time; - long offset; - bool use_system_time; - u8 reg[REG_COUNT]; -}; - -struct sandbox_i2c_rtc { - unsigned int offset_secs; -}; - long sandbox_i2c_rtc_set_offset(struct udevice *dev, bool use_system_time, int offset) {

The test framework reinits driver model tests before running each test. Since malloc_simple does not support free(), this eventually runs out of memory.

Fix it for now by increasing the space to 32KB.

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de ---

(no changes since v1)

configs/sandbox_spl_defconfig | 1 + 1 file changed, 1 insertion(+)

diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 041c72ac261..223f956fb78 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -7,6 +7,7 @@ CONFIG_SYS_MEMTEST_END=0x00101000 CONFIG_ENV_SIZE=0x2000 CONFIG_SPL_SERIAL_SUPPORT=y CONFIG_SPL_DRIVERS_MISC_SUPPORT=y +CONFIG_SPL_SYS_MALLOC_F_LEN=0x8000 CONFIG_SPL=y CONFIG_BOOTSTAGE_STASH_ADDR=0x0 CONFIG_DEFAULT_DEVICE_TREE="sandbox"

With recent changes this can be supported again. Add it back.

This reverts commit d85f2c4f2970d0ec2f5f075de734afd11200d153.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

arch/sandbox/dts/sandbox.dtsi | 10 +++------- configs/sandbox_spl_defconfig | 1 + drivers/i2c/Makefile | 2 -- test/dm/of_platdata.c | 8 ++++---- 4 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi index 5bf84451c99..69d7d3019ee 100644 --- a/arch/sandbox/dts/sandbox.dtsi +++ b/arch/sandbox/dts/sandbox.dtsi @@ -264,14 +264,10 @@ stringarray = "pre-proper"; };

- test-bus { - compatible = "simple-bus"; + spl-test7 { u-boot,dm-spl; - spl-test7 { - u-boot,dm-spl; - compatible = "sandbox,spl-test"; - stringarray = "spl"; - }; + compatible = "sandbox,spl-test"; + stringarray = "spl"; };

square { diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 223f956fb78..a17f7074424 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -102,6 +102,7 @@ CONFIG_SYSCON=y CONFIG_SPL_SYSCON=y CONFIG_DEVRES=y CONFIG_DEBUG_DEVRES=y +# CONFIG_SPL_SIMPLE_BUS is not set CONFIG_ADC=y CONFIG_ADC_SANDBOX=y CONFIG_AXI=y diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile index 29aab0f9e30..1aac5c481e2 100644 --- a/drivers/i2c/Makefile +++ b/drivers/i2c/Makefile @@ -39,9 +39,7 @@ obj-$(CONFIG_SYS_I2C_RCAR_I2C) += rcar_i2c.o obj-$(CONFIG_SYS_I2C_RCAR_IIC) += rcar_iic.o obj-$(CONFIG_SYS_I2C_ROCKCHIP) += rk_i2c.o obj-$(CONFIG_SYS_I2C_S3C24X0) += s3c24x0_i2c.o exynos_hs_i2c.o -ifndef CONFIG_SPL_BUILD obj-$(CONFIG_SYS_I2C_SANDBOX) += sandbox_i2c.o i2c-emul-uclass.o -endif obj-$(CONFIG_SYS_I2C_SH) += sh_i2c.o obj-$(CONFIG_SYS_I2C_SOFT) += soft_i2c.o obj-$(CONFIG_SYS_I2C_STM32F7) += stm32f7_i2c.o diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c index 1b46bdf555a..b05d2850e9f 100644 --- a/test/dm/of_platdata.c +++ b/test/dm/of_platdata.c @@ -206,11 +206,11 @@ DM_TEST(dm_test_of_plat_phandle, UT_TESTF_SCAN_PDATA); /* Test that device parents are correctly set up */ static int dm_test_of_plat_parent(struct unit_test_state *uts) { - struct udevice *dev, *bus; + struct udevice *rtc, *i2c;

- ut_assertok(uclass_first_device_err(UCLASS_SIMPLE_BUS, &bus)); - ut_assertok(device_first_child_err(bus, &dev)); - ut_asserteq_ptr(bus, dev_get_parent(dev)); + ut_assertok(uclass_first_device_err(UCLASS_RTC, &rtc)); + ut_assertok(uclass_first_device_err(UCLASS_I2C, &i2c)); + ut_asserteq_ptr(i2c, dev_get_parent(rtc));

return 0; }

Run the tests on this build too, to prevent regressions.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

test/run | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/test/run b/test/run index 735628e7e37..869406cd8d2 100755 --- a/test/run +++ b/test/run @@ -30,6 +30,10 @@ fi run_test "sandbox_spl" ./test/py/test.py --bd sandbox_spl --build \ -k 'test_ofplatdata or test_handoff or test_spl'

+# Run the sane tests with sandbox_noinst (i.e. without OF_PLATDATA_INST) +run_test "sandbox_spl" ./test/py/test.py --bd sandbox_noinst --build \ + -k 'test_ofplatdata or test_handoff or test_spl' + if [ -z "$tools_only" ]; then # Run tests for the flat-device-tree version of sandbox. This is a special # build which does not enable CONFIG_OF_LIVE for the live device tree, so we can

Some systems (e.g. x86 APL) run SPL from read-only memory. The device instances created by dtoc are therefore not writeable. To make things work we would need to copy the devices to read/write memory.

To avoid this, add an option to use a separate runtime struct for devices, just as is done for drivers. This can be used to hold information that changes at runtime, avoiding the need for a copy.

Also add a Kconfig option for read-only SPL, which selects this feature.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

common/spl/Kconfig | 24 ++++++++++++++++++++++++ dts/Kconfig | 22 ++++++++++++++++++++++ 2 files changed, 46 insertions(+)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig index 774541c02bc..584f8483e2e 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -276,6 +276,19 @@ config SPL_SEPARATE_BSS location is used. Normally we put the device tree at the end of BSS but with this option enabled, it goes at _image_binary_end.

+config SPL_READ_ONLY + bool + depends on SPL_OF_PLATDATA + # Bind cannot be supported because the udevice structs are in read-only + # memory so we cannot update the linked lists. + select SPL_OF_PLATDATA_NO_BIND + select SPL_OF_PLATDATA_RT + help + Some platforms (e.g. x86 Apollo Lake) load SPL into a read-only + section of memory. This means that of-platdata must make a copy (in + writeable memory) of anything it wants to modify, such as + device-private data. + config SPL_BANNER_PRINT bool "Enable output of the SPL banner 'U-Boot SPL ...'" default y @@ -1440,6 +1453,17 @@ config TPL_STACK The address of the initial stack-pointer for the TPL stage. Usually this will be the (aligned) top-of-stack.

+config TPL_READ_ONLY + bool + depends on TPL_OF_PLATDATA + select TPL_OF_PLATDATA_NO_BIND + select TPL_OF_PLATDATA_RT + help + Some platforms (e.g. x86 Apollo Lake) load SPL into a read-only + section of memory. This means that of-platdata must make a copy (in + writeable memory) of anything it wants to modify, such as + device-private data. + config TPL_BOOTROM_SUPPORT bool "Support returning to the BOOTROM (from TPL)" help diff --git a/dts/Kconfig b/dts/Kconfig index d289752a139..e6bbdc596bd 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -380,6 +380,17 @@ config SPL_OF_PLATDATA_NO_BIND some code space in U-Boot. This can be disabled if binding is needed, at the code of some code size increase.

+config SPL_OF_PLATDATA_RT + bool "Use a separate struct for device runtime data" + depends on SPL_OF_PLATDATA_INST + default y + help + For systems running SPL from read-only memory it is convenient to + separate out the runtime information, so that the devices don't need + to be copied before being used. This moves the read-write parts of + struct udevice (at present just the flags) into a separate struct, + which is allocated at runtime. + endif

config TPL_OF_PLATDATA @@ -429,6 +440,17 @@ config TPL_OF_PLATDATA_NO_BIND some code space in U-Boot. This can be disabled if binding is needed, at the code of some code size increase.

+config TPL_OF_PLATDATA_RT + bool "Use a separate struct for device runtime data" + depends on TPL_OF_PLATDATA_INST + default y + help + For systems running TPL from read-only memory it is convenient to + separate out the runtime information, so that the devices don't need + to be copied before being used. This moves the read-write parts of + struct udevice (at present just the flags) into a separate struct, + which is allocated at runtime. + endif

endmenu

When of-platdata-inst is active, use the flags in the new udevice_rt table, dropping them from the main struct udevice. This ensures that the latter is not updated at runtime.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v2)

Changes in v2: - Leave the flags_ memeber in the same struct position

drivers/core/device.c | 33 +++++++++++++++++++++++++++++++++ include/dm/device.h | 8 ++++++++ 2 files changed, 41 insertions(+)

diff --git a/drivers/core/device.c b/drivers/core/device.c index 74dd131241c..9c06f43f83c 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -1094,3 +1094,36 @@ int dev_enable_by_path(const char *path) return lists_bind_fdt(parent, node, NULL, false); } #endif + +#if CONFIG_IS_ENABLED(OF_PLATDATA_RT) +static struct udevice_rt *dev_get_rt(const struct udevice *dev) +{ + struct udevice *base = ll_entry_start(struct udevice, udevice); + int idx = dev - base; + + struct udevice_rt *urt = gd_dm_udevice_rt() + idx; + + return urt; +} + +u32 dev_get_flags(const struct udevice *dev) +{ + const struct udevice_rt *urt = dev_get_rt(dev); + + return urt->flags_; +} + +void dev_or_flags(const struct udevice *dev, u32 or) +{ + struct udevice_rt *urt = dev_get_rt(dev); + + urt->flags_ |= or; +} + +void dev_bic_flags(const struct udevice *dev, u32 bic) +{ + struct udevice_rt *urt = dev_get_rt(dev); + + urt->flags_ &= ~bic; +} +#endif /* OF_PLATDATA_RT */ diff --git a/include/dm/device.h b/include/dm/device.h index d18ced35ae4..6643a1be751 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -175,7 +175,9 @@ struct udevice { struct list_head uclass_node; struct list_head child_head; struct list_head sibling_node; +#if !CONFIG_IS_ENABLED(OF_PLATDATA_RT) u32 flags_; +#endif int seq_; #if !CONFIG_IS_ENABLED(OF_PLATDATA) ofnode node_; @@ -206,6 +208,11 @@ struct udevice_rt { /* Returns the operations for a device */ #define device_get_ops(dev) (dev->driver->ops)

+#if CONFIG_IS_ENABLED(OF_PLATDATA_RT) +u32 dev_get_flags(const struct udevice *dev); +void dev_or_flags(const struct udevice *dev, u32 or); +void dev_bic_flags(const struct udevice *dev, u32 bic); +#else static inline u32 dev_get_flags(const struct udevice *dev) { return dev->flags_; @@ -220,6 +227,7 @@ static inline void dev_bic_flags(struct udevice *dev, u32 bic) { dev->flags_ &= ~bic; } +#endif /* OF_PLATDATA_RT */

/** * dev_ofnode() - get the DT node reference associated with a udevice

Collect this together in one place, so driver model can access set it up in a new place if needed.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

arch/sandbox/cpu/u-boot-spl.lds | 8 ++++++++ 1 file changed, 8 insertions(+)

diff --git a/arch/sandbox/cpu/u-boot-spl.lds b/arch/sandbox/cpu/u-boot-spl.lds index 649abeb5ee7..18160436a36 100644 --- a/arch/sandbox/cpu/u-boot-spl.lds +++ b/arch/sandbox/cpu/u-boot-spl.lds @@ -13,6 +13,14 @@ SECTIONS KEEP(*(SORT(.u_boot_list*))); }

+ /* Private data for devices with OF_PLATDATA_RT */ + . = ALIGN(4); + .priv_data : { + __priv_data_start = .; + *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.priv_data*))) + __priv_data_end = .; + } + __u_boot_sandbox_option_start = .; _u_boot_sandbox_getopt : { KEEP(*(.u_boot_sandbox_getopt)) } __u_boot_sandbox_option_end = .;

Add a warning to each of these functions so that people do not attempt to use them outside driver model.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: - Aew patch with warnings for private / platform setters

include/dm/device-internal.h | 18 ++++++++++++++++++ include/dm/uclass-internal.h | 3 +++ 2 files changed, 21 insertions(+)

diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index 71e5c75028d..e6b71cbfd2b 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -284,6 +284,9 @@ static inline int device_chld_remove(struct udevice *dev, struct driver *drv, * Use this function to override normal operation for special situations, such * as needing to allocate a variable amount of data. * + * If OF_PLATDATA_RT is enabled, this function cannot be used out of core driver + * model code, since the pointer must be within the gd->dm_priv_base region. + * * @dev Device to check * @priv New private-data pointer */ @@ -298,6 +301,9 @@ void dev_set_priv(struct udevice *dev, void *priv); * Use this function to override normal operation for special situations, such * as needing to allocate a variable amount of data. * + * If OF_PLATDATA_RT is enabled, this function cannot be used out of core driver + * model code, since the pointer must be within the gd->dm_priv_base region. + * * @dev: Device to update * @parent_priv: New parent-private data */ @@ -312,6 +318,9 @@ void dev_set_parent_priv(struct udevice *dev, void *parent_priv); * Use this function to override normal operation for special situations, such * as needing to allocate a variable amount of data. * + * If OF_PLATDATA_RT is enabled, this function cannot be used out of core driver + * model code, since the pointer must be within the gd->dm_priv_base region. + * * @dev: Device to update * @uclass_priv: New uclass private data */ @@ -326,6 +335,9 @@ void dev_set_uclass_priv(struct udevice *dev, void *uclass_priv); * Use this function to override normal operation for special situations, such * as needing to allocate a variable amount of data. * + * If OF_PLATDATA_RT is enabled, this function cannot be used out of core driver + * model code, since the pointer must be within the gd->dm_priv_base region. + * * @dev Device to check * @plat New platform-data pointer */ @@ -340,6 +352,9 @@ void dev_set_plat(struct udevice *dev, void *priv); * Use this function to override normal operation for special situations, such * as needing to allocate a variable amount of data. * + * If OF_PLATDATA_RT is enabled, this function cannot be used out of core driver + * model code, since the pointer must be within the gd->dm_priv_base region. + * * @dev: Device to update * @parent_plat: New parent platform data */ @@ -354,6 +369,9 @@ void dev_set_parent_plat(struct udevice *dev, void *parent_plat); * Use this function to override normal operation for special situations, such * as needing to allocate a variable amount of data. * + * If OF_PLATDATA_RT is enabled, this function cannot be used out of core driver + * model code, since the pointer must be within the gd->dm_priv_base region. + * * @dev: Device to update * @uclass_plat: New uclass platform data */ diff --git a/include/dm/uclass-internal.h b/include/dm/uclass-internal.h index f2a780682b4..57c664c6daa 100644 --- a/include/dm/uclass-internal.h +++ b/include/dm/uclass-internal.h @@ -69,6 +69,9 @@ * Use this function to override normal operation for special situations, such * as needing to allocate a variable amount of data. * + * If OF_PLATDATA_RT is enabled, this function cannot be used out of core driver + * model code, since the pointer must be within the gd->dm_priv_base region. + * * @uc Uclass to update * @priv New private-data pointer */

Add a description of the new features, along with internal technical documentation.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: - Add documentation for of-platdata-inst

doc/driver-model/of-plat.rst | 587 +++++++++++++++++++++++++++++++++++ 1 file changed, 587 insertions(+)

diff --git a/doc/driver-model/of-plat.rst b/doc/driver-model/of-plat.rst index 3c9ba01b302..74f1932473b 100644 --- a/doc/driver-model/of-plat.rst +++ b/doc/driver-model/of-plat.rst @@ -286,9 +286,394 @@ it. There would be little point in having some drivers require the device tree data, since then libfdt would still be needed for those drivers and there would be no code-size benefit.

+ +Build-time instantiation +------------------------ + +Even with of-platdata there is a fair amount of code required in driver model. +It is possible to have U-Boot handle the instantiation of devices at build-time, +so avoiding the need for the `device_bind()` code and some parts of +`device_probe()`. + +The feature is enabled by CONFIG_OF_PLATDATA_INST. + +Here is an example device, as generated by dtoc:: + + /* + * Node /serial index 6 + * driver sandbox_serial parent root_driver + */ + + #include <asm/serial.h> + struct sandbox_serial_plat __attribute__ ((section (".priv_data"))) + _sandbox_serial_plat_serial = { + .dtplat = { + .sandbox_text_colour = "cyan", + }, + }; + #include <asm/serial.h> + u8 _sandbox_serial_priv_serial[sizeof(struct sandbox_serial_priv)] + __attribute__ ((section (".priv_data"))); + #include <serial.h> + u8 _sandbox_serial_uc_priv_serial[sizeof(struct serial_dev_priv)] + __attribute__ ((section (".priv_data"))); + + DM_DEVICE_INST(serial) = { + .driver = DM_DRIVER_REF(sandbox_serial), + .name = "sandbox_serial", + .plat_ = &_sandbox_serial_plat_serial, + .priv_ = _sandbox_serial_priv_serial, + .uclass = DM_UCLASS_REF(serial), + .uclass_priv_ = _sandbox_serial_uc_priv_serial, + .uclass_node = { + .prev = &DM_UCLASS_REF(serial)->dev_head, + .next = &DM_UCLASS_REF(serial)->dev_head, + }, + .child_head = { + .prev = &DM_DEVICE_REF(serial)->child_head, + .next = &DM_DEVICE_REF(serial)->child_head, + }, + .sibling_node = { + .prev = &DM_DEVICE_REF(i2c_at_0)->sibling_node, + .next = &DM_DEVICE_REF(spl_test)->sibling_node, + }, + .seq_ = 0, + }; + +Here is part of the driver, for reference:: + + static const struct udevice_id sandbox_serial_ids[] = { + { .compatible = "sandbox,serial" }, + { } + }; + + U_BOOT_DRIVER(sandbox_serial) = { + .name = "sandbox_serial", + .id = UCLASS_SERIAL, + .of_match = sandbox_serial_ids, + .of_to_plat = sandbox_serial_of_to_plat, + .plat_auto = sizeof(struct sandbox_serial_plat), + .priv_auto = sizeof(struct sandbox_serial_priv), + .probe = sandbox_serial_probe, + .remove = sandbox_serial_remove, + .ops = &sandbox_serial_ops, + .flags = DM_FLAG_PRE_RELOC, + }; + + +The `DM_DEVICE_INST()` macro declares a struct udevice so you can see that the +members are from that struct. The private data is declared immediately above, +as `_sandbox_serial_priv_serial`, so there is no need for run-time memory +allocation. The #include lines are generated as well, since dtoc searches the +U-Boot source code for the definition of `struct sandbox_serial_priv` and adds +the relevant header so that the code will compile without errors. + +The `plat_` member is set to the dtv data which is declared immediately above +the device. This is similar to how it would look without of-platdata-inst, but +node that the `dtplat` member inside is part of the wider +`_sandbox_serial_plat_serial` struct. This is because the driver declares its +own platform data, and the part generated by dtoc can only be a portion of it. +The `dtplat` part is always first in the struct. If the device has no +`.plat_auto` field, then a simple dtv struct can be used as with this example:: + + static struct dtd_sandbox_clk dtv_clk_sbox = { + .assigned_clock_rates = 0x141, + .assigned_clocks = {0x7, 0x3}, + }; + + #include <asm/clk.h> + u8 _sandbox_clk_priv_clk_sbox[sizeof(struct sandbox_clk_priv)] + __attribute__ ((section (".priv_data"))); + + DM_DEVICE_INST(clk_sbox) = { + .driver = DM_DRIVER_REF(sandbox_clk), + .name = "sandbox_clk", + .plat_ = &dtv_clk_sbox, + +Here is part of the driver, for reference:: + + static const struct udevice_id sandbox_clk_ids[] = { + { .compatible = "sandbox,clk" }, + { } + }; + + U_BOOT_DRIVER(sandbox_clk) = { + .name = "sandbox_clk", + .id = UCLASS_CLK, + .of_match = sandbox_clk_ids, + .ops = &sandbox_clk_ops, + .probe = sandbox_clk_probe, + .priv_auto = sizeof(struct sandbox_clk_priv), + }; + + +You can see that `dtv_clk_sbox` just has the devicetree contents and there is +no need for the `dtplat` separation, since the driver has no platform data of +its own, besides that provided by the devicetree (i.e. no `.plat_auto` field). + +The doubly linked lists are handled by explicitly declaring the value of each +node, as you can see with the `.prev` and `.next` values in the example above. +Since dtoc knows the order of devices it can link them into the appropriate +lists correctly. + +One of the features of driver model is the ability for a uclass to have a +small amount of private data for each device in that uclass. This is used to +provide a generic data structure that the uclass can use for all devices, thus +allowing generic features to be implemented in common code. An example is I2C, +which stores the bus speed there. + +Similarly, parent devices can have data associated with each of their children. +This is used to provide information common to all children of a particular bus. +For an I2C bus, this is used to store the I2C address of each child on the bus. + +This is all handled automatically by dtoc:: + + #include <asm/i2c.h> + u8 _sandbox_i2c_priv_i2c_at_0[sizeof(struct sandbox_i2c_priv)] + __attribute__ ((section (".priv_data"))); + #include <i2c.h> + u8 _sandbox_i2c_uc_priv_i2c_at_0[sizeof(struct dm_i2c_bus)] + __attribute__ ((section (".priv_data"))); + + DM_DEVICE_INST(i2c_at_0) = { + .driver = DM_DRIVER_REF(sandbox_i2c), + .name = "sandbox_i2c", + .plat_ = &dtv_i2c_at_0, + .priv_ = _sandbox_i2c_priv_i2c_at_0, + .uclass = DM_UCLASS_REF(i2c), + .uclass_priv_ = _sandbox_i2c_uc_priv_i2c_at_0, + ... + +Part of driver, for reference:: + + static const struct udevice_id sandbox_i2c_ids[] = { + { .compatible = "sandbox,i2c" }, + { } + }; + + U_BOOT_DRIVER(sandbox_i2c) = { + .name = "sandbox_i2c", + .id = UCLASS_I2C, + .of_match = sandbox_i2c_ids, + .ops = &sandbox_i2c_ops, + .priv_auto = sizeof(struct sandbox_i2c_priv), + }; + +Part of I2C uclass, for reference:: + + UCLASS_DRIVER(i2c) = { + .id = UCLASS_I2C, + .name = "i2c", + .flags = DM_UC_FLAG_SEQ_ALIAS, + .post_bind = i2c_post_bind, + .pre_probe = i2c_pre_probe, + .post_probe = i2c_post_probe, + .per_device_auto = sizeof(struct dm_i2c_bus), + .per_child_plat_auto = sizeof(struct dm_i2c_chip), + .child_post_bind = i2c_child_post_bind, + }; + +Here, `_sandbox_i2c_uc_priv_i2c_at_0` is required by the uclass but is declared +in the device, as required by driver model. The required header file is included +so that the code will compile without errors. A similar mechanism is used for +child devices, but is not shown by this example. + +It would not be that useful to avoid binding devices but still need to allocate +uclasses at runtime. So dtoc generates uclass instances as well:: + + struct list_head uclass_head = { + .prev = &DM_UCLASS_REF(serial)->sibling_node, + .next = &DM_UCLASS_REF(clk)->sibling_node, + }; + + DM_UCLASS_INST(clk) = { + .uc_drv = DM_UCLASS_DRIVER_REF(clk), + .sibling_node = { + .prev = &uclass_head, + .next = &DM_UCLASS_REF(i2c)->sibling_node, + }, + .dev_head = { + .prev = &DM_DEVICE_REF(clk_sbox)->uclass_node, + .next = &DM_DEVICE_REF(clk_fixed)->uclass_node, + }, + }; + +At the top is the list head. Driver model uses this on start-up, instead of +creating its own. + +Below that are a set of `DM_UCLASS_INST()` macros, each declaring a +`struct uclass`. The doubly linked lists work as for devices. + +All private data is placed into a `.priv_data` section so that it is contiguous +in the resulting output binary. + + +Indexes +------- + +U-Boot stores drivers, devices and many other things in linker_list structures. +These are sorted by name, so dtoc knows the order that they will appear when +the linker runs. Each driver_info / udevice is referenced by its index in the +linker_list array, called 'idx' in the code. + +When CONFIG_OF_PLATDATA_INST is enabled, idx is the udevice index, otherwise it +is the driver_info index. In either case, indexes are used to reference devices +using device_get_by_ofplat_idx(). This allows phandles to work as expected. + + +Phases +------ + +U-Boot operates in several phases, typically TPL, SPL and U-Boot proper. +The latter does not use dtoc. + +In some rare cases different drivers are used for two phases. For example, +in TPL it may not be necessary to use the full PCI subsystem, so a simple +driver can be used instead. + +This works in the build system simply by compiling in one driver or the +other (e.g. PCI driver + uclass for SPL; simple_bus for TPL). But dtoc has +no way of knowing which code is compiled in for which phase, since it does +not inspect Makefiles or dependency graphs. + +So to make this work for dtoc, we need to be able to explicitly mark +drivers with their phase. This is done by adding a macro to the driver:: + + /* code in tpl.c only compiled into TPL */ + U_BOOT_DRIVER(pci_x86) = { + .name = "pci_x86", + .id = UCLASS_SIMPLE_BUS, + .of_match = of_match_ptr(tpl_fake_pci_ids), + DM_PHASE(tpl) + }; + + + /* code in pci_x86.c compiled into SPL and U-Boot proper */ + U_BOOT_DRIVER(pci_x86) = { + .name = "pci_x86", + .id = UCLASS_PCI, + .of_match = pci_x86_ids, + .ops = &pci_x86_ops, + }; + + +Notice that the second driver has the same name but no DM_PHASE(), so it will be +used for SPL and U-Boot. + +Note also that this only affects the code generated by dtoc. You still need to +make sure that only the required driver is build into each phase. + + +Header files +------------ + +With OF_PLATDATA_INST, dtoc must include the correct header file in the +generated code for any structs that are used, so that the code will compile. +For example, if `struct ns16550_plat` is used, the code must include the +`ns16550.h` header file. + +Typically dtoc can detect the header file needed for a driver by looking +for the structs that it uses. For example, if a driver as a `.priv_auto` +that uses `struct ns16550_plat`, then dtoc can search header files for the +definition of that struct and use the file. + +In some cases, enums are used in drivers, typically with the `.data` field +of `struct udevice_id`. Since dtoc does not support searching for these, +you must use the `DM_HDR()` macro to tell dtoc which header to use. This works +as a macro included in the driver definition:: + + static const struct udevice_id apl_syscon_ids[] = { + { .compatible = "intel,apl-punit", .data = X86_SYSCON_PUNIT }, + { } + }; + + U_BOOT_DRIVER(intel_apl_punit) = { + .name = "intel_apl_punit", + .id = UCLASS_SYSCON, + .of_match = apl_syscon_ids, + .probe = apl_punit_probe, + DM_HEADER(<asm/cpu.h>) /* for X86_SYSCON_PUNIT */ + }; + + + +Caveats +------- + +There are various complications with this feature which mean it should only +be used when strictly necessary, i.e. in SPL with limited memory. Notable +caveats include: + + - Device tree does not describe data types. But the C code must define a + type for each property. These are guessed using heuristics which + are wrong in several fairly common cases. For example an 8-byte value + is considered to be a 2-item integer array, and is byte-swapped. A + boolean value that is not present means 'false', but cannot be + included in the structures since there is generally no mention of it + in the devicetree file. + + - Naming of nodes and properties is automatic. This means that they follow + the naming in the devicetree, which may result in C identifiers that + look a bit strange. + + - It is not possible to find a value given a property name. Code must use + the associated C member variable directly in the code. This makes + the code less robust in the face of devicetree changes. To avoid having + a second struct with similar members and names you need to explicitly + declare it as an alias with `DM_DRIVER_ALIAS()`. + + - The platform data is provided to drivers as a C structure. The driver + must use the same structure to access the data. Since a driver + normally also supports devicetree it must use `#ifdef` to separate + out this code, since the structures are only available in SPL. This could + be fixed fairly easily by making the structs available outside SPL, so + that `IS_ENABLED()` could be used. + + - With CONFIG_OF_PLATDATA_INST all binding happens at build-time, meaning + that (by default) it is not possible to call `device_bind()` from C code. + This means that all devices must have an associated devicetree node and + compatible string. For example if a GPIO device currently creates child + devices in its `bind()` method, it will not work with + CONFIG_OF_PLATDATA_INST. Arguably this is bad practice anyway and the + devicetree binding should be updated to declare compatible strings for + the child devices. It is possible to disable OF_PLATDATA_NO_BIND but this + is not recommended since it increases code size. + + Internals ---------

+Generated files +``````````````` + +When enabled, dtoc generates the following five files: + +include/generated/dt-decl.h (OF_PLATDATA_INST only) + Contains declarations for all drivers, devices and uclasses. This allows + any `struct udevice`, `struct driver` or `struct uclass` to be located by its + name + +include/generated/dt-structs-gen.h + Contains the struct definitions for the devicetree nodes that are used. This + is the same as without OF_PLATDATA_INST + +spl/dts/dt-plat.c (only with !OF_PLATDATA_INST) + Contains the `U_BOOT_DRVINFO()` declarations that U-Boot uses to bind devices + at start-up. See above for an example + +spl/dts/dt-device.c (only with OF_PLATDATA_INST) + Contains `DM_DEVICE_INST()` declarations for each device that can be used at + run-time. These are declared in the file along with any private/platform data + that they use. Every device has an idx, as above. Since each device must be + part of a double-linked list, the nodes are declared in the code as well. + +spl/dts/dt-uclass.c (only with OF_PLATDATA_INST) + Contains `DM_UCLASS_INST()` declarations for each uclass that can be used at + run-time. These are declared in the file along with any private data + associated with the uclass itself (the `.priv_auto` member). Since each + uclass must be part of a double-linked list, the nodes are declared in the + code as well. + The dt-structs.h file includes the generated file `(include/generated/dt-structs.h`) if CONFIG_SPL_OF_PLATDATA is enabled. Otherwise (such as in U-Boot proper) these structs are not available. This @@ -298,6 +683,208 @@ prevents them being used inadvertently. All usage must be bracketed with The dt-plat.c file contains the device declarations and is is built in spl/dt-plat.c.

+ +CONFIG options +`````````````` + +Several CONFIG options are used to control the behaviour of of-platdata, all +available for both SPL and TPL: + +OF_PLATDATA + This is the main option which enables the of-platdata feature + +OF_PLATDATA_PARENT + This allows `device_get_parent()` to work. Without this, all devices exist as + direct children of the root node. This option is highly desirable (if not + always absolutely essential) for buses such as I2C. + +OF_PLATDATA_INST + This controls the instantiation of devices at build time. With it disabled, + only `U_BOOT_DRVINFO()` records are created, with U-Boot handling the binding + in `device_bind()` on start-up. With it enabled, only `DM_DEVICE_INST()` and + `DM_UCLASS_INST()` records are created, and `device_bind()` is not needed at + runtime. + +OF_PLATDATA_NO_BIND + This controls whether `device_bind()` is supported. It is enabled by default + with OF_PLATDATA_INST since code-size reduction is really the main point of + the feature. It can be disabled if needed but is not likely to be supported + in the long term. + +OF_PLATDATA_DRIVER_RT + This controls whether the `struct driver_rt` records are used by U-Boot. + Normally when a device is bound, U-Boot stores the device pointer in one of + these records. There is one for every `struct driver_info` in the system, + i.e. one for every device that is bound from those records. It provides a + way to locate a device in the code and is used by + `device_get_by_ofplat_idx()`. This option is always enabled with of-platdata, + provided OF_PLATDATA_INST is not. In that case the records are useless since + we don't have any `struct driver_info` records. + +OF_PLATDATA_RT + This controls whether the `struct udevice_rt` records are used by U-Boot. + It moves the updatable fields from `struct udevice` (currently only `flags`) + into a separate structure, allowing the records to be kept in read-only + memory. It is generally enabled if OF_PLATDATA_INST is enabled. This option + also controls whether the private data is used in situ, or first copied into + an allocated region. Again this is to allow the private data declared by + dtoc-generated code to be in read-only memory. Note that access to private + data must be done via accessor functions, such as `dev_get_priv()`, so that + the relocation is handled. + +READ_ONLY + This indicates that the data generated by dtoc should not be modified. Only + a few fields actually do get changed in U-Boot, such as device flags. This + option causes those to move into an allocated space (see OF_PLATDATA_RT). + Also, since updating doubly linked lists is generally impossible when some of + the nodes cannot be updated, OF_PLATDATA_NO_BIND is enabled. + +Data structures +``````````````` + +A few extra data structures are used with of-platdata: + +`struct udevice_rt` + Run-time information for devices. When OF_PLATDATA_RT is enabled, this holds + the flags for each device, so that `struct udevice` can remain unchanged by + U-Boot, and potentially reside in read-only memory. Access to flags is then + via functions like `dev_get_flags()` and `dev_or_flags()`. This data + structure is allocated on start-up, where the private data is also copied. + All flags values start at 0 and any changes are handled by `dev_or_flags()` + and `dev_bic_flags()`. It would be more correct for the flags to be set to + `DM_FLAG_BOUND`, or perhaps `DM_FLAG_BOUND | DM_FLAG_ALLOC_PDATA`, but since + there is no code to bind/unbind devices and no code to allocate/free + private data / platform data, it doesn't matter. + +`struct driver_rt` + Run-time information for `struct driver_info` records. When + OF_PLATDATA_DRIVER_RT is enabled, this holds a pointer to the device + created by each record. This is needed so that is it possible to locate a + device from C code. Specifically, the code can use `DM_DRVINFO_GET(name)` to + get a reference to a particular `struct driver_info`, with `name` being the + name of the devicetree node. This is very convenient. It is also fast, since + no searching or string comparison is needed. This data structure is + allocated on start-up, filled out by `device_bind()` and used by + `device_get_by_ofplat_idx()`. + +Other changes +````````````` + +Some other changes are made with of-platdata: + +Accessor functions + Accessing private / platform data via functions such as `dev_get_priv()` has + always been encouraged. With OF_PLATDATA_RT this is essential, since the + `priv_` and `plat_` (etc.) values point to the data generated by dtoc, not + the read-write copy that is sometimes made on start-up. Changing the + private / platform data pointers has always been discouraged (the API is + marked internal) but with OF_PLATDATA_RT this is not currently supported in + general, since it assumes that all such pointers point to the relocated data. + Note also that the renaming of struct members to have a trailing underscore + was partly done to make people aware that they should not be accessed + directly. + +`gd->uclass_root_s` + Normally U-Boot sets up the head of the uclass list here and makes + `gd->uclass_root` point to it. With OF_PLATDATA_INST, dtoc generates a + declaration of `uclass_head` in `dt-uclass.c` since it needs to link the + head node into the list. In that case, `gd->uclass_root_s` is not used and + U-Boot just makes `gd->uclass_root` point to `uclass_head`. + +`gd->dm_driver_rt` + This holds a pointer to a list of `struct driver_rt` records, one for each + `struct driver_info`. The list is in alphabetical order by the name used + in `U_BOOT_DRVINFO(name)` and indexed by idx, with the first record having + an index of 0. It is only used if OF_PLATDATA_INST is not enabled. This is + accessed via macros so that it can be used inside IS_ENABLED(), rather than + requiring #ifdefs in the C code when it is not present. + +`gd->dm_udevice_rt` + This holds a pointer to a list of `struct udevice_rt` records, one for each + `struct udevice`. The list is in alphabetical order by the name used + in `DM_DEVICE_INST(name)` (a C version of the devicetree node) and indexed by + idx, with the first record having an index of 0. It is only used if + OF_PLATDATA_INST is enabled. This is accessed via macros so that it can be + used inside `IS_ENABLED()`, rather than requiring #ifdefs in the C code when + it is not present. + +`gd->dm_priv_base` + When OF_PLATDATA_RT is enabled, the private/platform data for each device is + copied into an allocated region by U-Boot on start-up. This points to that + region. All calls to accessor functions (e.g. `dev_get_priv()`) then + translate from the pointer provided by the caller (assumed to lie between + `__priv_data_start` and `__priv_data_end`) to the new allocated region. This + member is accessed via macros so that it can be used inside IS_ENABLED(), + rather than required #ifdefs in the C code when it is not present. + +`struct udevice->flags_` + When OF_PLATDATA_RT is enabled, device flags are no-longer part of + `struct udevice`, but are instead kept in `struct udevice_rt`, as described + above. Flags are accessed via functions, such as `dev_get_flags()` and + `dev_or_flags()`. + +`struct udevice->node_` + When OF_PLATDATA is enabled, there is no devicetree at runtime, so no need + for this field. It is removed, just to save space. + +`DM_PHASE` + This macro is used to indicate which phase of U-Boot a driver is intended + for. See above for details. + +`DM_HDR` + This macro is used to indicate which header file dtoc should use to allow + a driver declaration to compile correctly. See above for details. + +`device_get_by_ofplat_idx()` + There used to be a function called `device_get_by_driver_info()` which + looked up a `struct driver_info` pointer and returned the `struct udevice` + that was created from it. It was only available for use with of-platdata. + This has been removed in favour of `device_get_by_ofplat_idx()` which uses + `idx`, the index of the `struct driver_info` or `struct udevice` in the + linker_list. Similarly, the `struct phandle_0_arg` (etc.) structs have been + updated to use this index instead of a pointer to `struct driver_info`. + +`DM_DRVINFO_GET` + This has been removed since we now use indexes to obtain a driver from + `struct phandle_0_arg` and the like. + +Two-pass binding + The original of-platdata tried to order `U_BOOT_DRVINFO()` in the generated + files so as to have parents declared ahead of children. This was convenient + as it avoided any special code in U-Boot. With OF_PLATDATA_INST this does + not work as the idx value relies on using alphabetical order for everything, + so that dtoc and U-Boot's linker_lists agree on the idx value. Devices are + then bound in order of idx, having no regard to parent/child relationships. + For this reason, device binding now hapens in multiple passes, with parents + being bound before their children. This is important so that children can + find their parents in the bind() method if needed. + +Root device + The root device is generally bound by U-Boot but with OF_PLATDATA_INST it + cannot be, since binding needs to be done at build time. So in this case + dtoc sets up a root device using `DM_DEVICE_INST()` in `dt-device.c` and + U-Boot makes use of that. When OF_PLATDATA_INST is not enabled, U-Boot + generally ignores the root node and does not create a `U_BOOT_DRVINFO()` + record for it. This means that the idx numbers used by `struct driver_info` + (when OF_PLATDATA_INST is disabled) and the idx numbers used by + `struct udevice` (when OF_PLATDATA_INST is enabled) differ, since one has a + root node and the other does not. This does not actually matter, since only + one of them is actually used for any particular build, but it is worth + keeping in mind if comparing index values and switching OF_PLATDATA_INST on + and off. + +`__priv_data_start` and `__priv_data_end` + The private/platform data declared by dtoc is all collected together in + a linker section and these symbols mark the start and end of it. This allows + U-Boot to relocate the area to a new location if needed (with + OF_PLATDATA_RT) + +`dm_priv_to_rw()` + This function converts a private- or platform-data pointer value generated by + dtoc into one that can be used by U-Boot. It is a NOP unless OF_PLATDATA_RT + is enabled, in which case it translates the address to the relocated + region. See above for more information. + The dm_populate_phandle_data() function that was previous needed has now been removed, since dtoc can address the drivers directly from dt-plat.c and does not need to fix up things at runtime.

The dm.h header should come first. In fact it needs to, since otherwise the driver model definitions are not available to dt-structs.h

Fix this, since it causes problems with OF_PLATDATA_INST.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

arch/x86/cpu/apollolake/pmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/cpu/apollolake/pmc.c b/arch/x86/cpu/apollolake/pmc.c index e23d38ea072..1d21187c96d 100644 --- a/arch/x86/cpu/apollolake/pmc.c +++ b/arch/x86/cpu/apollolake/pmc.c @@ -9,8 +9,8 @@ #define LOG_CATEGORY UCLASS_ACPI_PMC

#include <common.h> -#include <dt-structs.h> #include <dm.h> +#include <dt-structs.h> #include <log.h> #include <spl.h> #include <acpi/acpi_s3.h>

With the standard of-platdata we must fix up driver_data manually. With of-platadata-inst this is not necessary, since it is added to the device by dtoc.

Update the code to handle this.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

arch/x86/cpu/intel_common/itss.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/cpu/intel_common/itss.c b/arch/x86/cpu/intel_common/itss.c index ae4de4ca8c6..b258970e2ee 100644 --- a/arch/x86/cpu/intel_common/itss.c +++ b/arch/x86/cpu/intel_common/itss.c @@ -152,8 +152,9 @@ static int route_pmc_gpio_gpe(struct udevice *dev, uint pmc_gpe_num)

static int itss_bind(struct udevice *dev) { - /* This is not set with of-platdata, so set it manually */ - if (CONFIG_IS_ENABLED(OF_PLATDATA)) + /* This is not set with basic of-platdata, so set it manually */ + if (CONFIG_IS_ENABLED(OF_PLATDATA) && + !CONFIG_IS_ENABLED(OF_PLATDATA_INST)) dev->driver_data = X86_IRQT_ITSS;

return 0;

We don't normally need this driver in TPL/SPL, so drop it for now. It can be enabled by individual boards if needed.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

arch/x86/dts/reset.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/dts/reset.dtsi b/arch/x86/dts/reset.dtsi index 555d0dd9608..f2ba2fb5e84 100644 --- a/arch/x86/dts/reset.dtsi +++ b/arch/x86/dts/reset.dtsi @@ -1,6 +1,6 @@ / { reset: reset { compatible = "x86,reset"; - u-boot,dm-pre-reloc; + u-boot,dm-pre-proper; }; };

These devices are not actually built in TPL but are currently active in the TPL devicetree. For of-platdata-inst this means that we will try to generate devices for them, which fails.

Update them to be active only in U-Boot proper.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: - Add new patch to drop coral's TPM and ACPI interrupts from TPL - Use 'u-boot,dm-pre-proper' so these are not present in TPL/SPL

arch/x86/dts/chromebook_coral.dts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/dts/chromebook_coral.dts b/arch/x86/dts/chromebook_coral.dts index 2ffe3b423c3..d109a387597 100644 --- a/arch/x86/dts/chromebook_coral.dts +++ b/arch/x86/dts/chromebook_coral.dts @@ -100,7 +100,7 @@ clk: clock { compatible = "intel,apl-clk"; #clock-cells = <1>; - u-boot,dm-pre-reloc; + u-boot,dm-pre-proper; };

cpus { @@ -141,7 +141,7 @@ };

acpi_gpe: general-purpose-events { - u-boot,dm-pre-reloc; + u-boot,dm-pre-proper; reg = <IOMAP_ACPI_BASE IOMAP_ACPI_SIZE>; compatible = "intel,acpi-gpe"; interrupt-controller; @@ -423,7 +423,7 @@ compatible = "intel,apl-i2c", "snps,designware-i2c-pci"; reg = <0x0200b210 0 0 0 0>; early-regs = <IOMAP_I2C2_BASE 0x1000>; - u-boot,dm-pre-reloc; + u-boot,dm-pre-proper; #address-cells = <1>; #size-cells = <0>; clock-frequency = <400000>; @@ -434,7 +434,7 @@ tpm: tpm@50 { reg = <0x50>; compatible = "google,cr50"; - u-boot,dm-pre-reloc; + u-boot,dm-pre-proper; u-boot,i2c-offset-len = <0>; ready-gpios = <&gpio_n 28 GPIO_ACTIVE_LOW>; interrupts-extended = <&acpi_gpe GPIO_28_IRQ @@ -1233,5 +1233,5 @@ &rtc { #address-cells = <1>; #size-cells = <0>; - u-boot,dm-pre-reloc; + u-boot,dm-pre-proper; };