[PATCH 00/20] dm: Enhance of-platdata to support parent devices
At present dev_get_parent() always returns the root device with of-platdata. This means that it is not possible to find the I2C bus for an I2C device easily. In many cases this does not cause problems as there is only a single I2C bus, but it is still inconsistent with U-Boot proper.
Worse is that because devices are not bound to the correct parent, they will not have parent_platdata allocated correctly. Manual fix-ups are required in the code.
This series adds support for parent devices with of-platdata.
Recent improvements to of-platadata have provided a function to obtain a device based on its driver_info struct. This requires in generated C code in dm_populate_phandle_data() which writes the udevice pointer into each phandle value, since this cannot be known at build time.
This works well and fixes a big hole in of-platdata. But the implementation has a few drawbacks: the need for generated code adds (very slightly) to code size and it means that the SPL must sit in writable memory. The latter can be considered a security risk and is not actually supported on Intel Apollo Lake.
Another point is that in future of-platadata may support instantiation of devices at build time. Pointer fix-ups could become quite taxing.
So this series adjusts the approach, storing an index (idx) to the driver_info struct in the linker list instead. That index can be figured out in dtoc and the value placed directly in the phandle struct. For 64-bit machines it is also smaller than a pointer. It requires a new table (driver_rt) to be set up at runtime.
This series adds more SPL tests written in C, using the infrastructure in the previous series (u-boot-dm/tes-working). A few clean-ups and a bug fix resulting from these are included.
Finally, x86 is updated to use the parent support. For now, parent support is behind a Kconfig option, but I expect this will become the default, assuming that no major problems are found.
SPL size impact is fairly neural, slightly positive on chromebook_jerry (Thumb-2) and slightly negative on rock64-rk3328 (aarch64):
aarch64: spl/u-boot-spl:all +39.0 spl/u-boot-spl:rodata -5.0 spl/u-boot-spl:text +44.0 arm: spl/u-boot-spl:all -49.0 spl/u-boot-spl:data -80.0 spl/u-boot-spl:text +36.0 spl/u-boot-spl:rodata -5.0
The increase in code size is due to lists_bind_drivers() having to do multiple passes to ensure that parents are processed before children. This could possible be reduced with a more complex linker list ordering mechanism, but it would be a bit messy and that idea is not explored in this series.
Simon Glass (20): sandbox: Drop ad-hoc device declarations in SPL dtoc: Document the return value of scan_structs() dtoc: Order the structures internally by name dm: core: Allow dm_warn() to be used in SPL dtoc: Fix widening of int to bytes dm: Add a C test for of-platdata properties sandbox: Allow selection of SPL unit tests dm: test: Drop of-platdata pytest dm: test: Add a check that all devices have a dev value dm: test: Add a test for of-platdata phandles dm: Use an allocated array for run-time device info sandbox: Fix up building for of-platdata dm: Support parent devices with of-platdata dm: Add a test for of-platdata parent information dm: core: Convert #ifdef to if() in root.c x86: apl: Enable SPI flash in TPL with APL_SPI_FLASH_BOOT x86: apl: Take advantage of the of-platdata parent support dm: Use driver_info index instead of pointer dm: Don't allow U_BOOT_DEVICE() when of-platdata is used dm: doc: Update the of-platadata documentation
arch/sandbox/cpu/spl.c | 14 +- arch/sandbox/cpu/start.c | 16 +-- arch/sandbox/dts/sandbox.dts | 1 + arch/sandbox/dts/sandbox.dtsi | 27 ++++ arch/sandbox/include/asm/state.h | 2 +- arch/x86/cpu/apollolake/Kconfig | 2 + arch/x86/cpu/apollolake/spl.c | 3 +- board/sandbox/sandbox.c | 2 + configs/sandbox_spl_defconfig | 5 +- doc/driver-model/of-plat.rst | 42 +++--- drivers/clk/clk-uclass.c | 3 +- drivers/clk/clk_fixed_rate.c | 4 +- drivers/clk/clk_sandbox.c | 4 +- drivers/core/Kconfig | 18 ++- drivers/core/device.c | 23 +++- drivers/core/lists.c | 70 +++++++++- drivers/core/root.c | 29 ++-- drivers/core/util.c | 2 +- drivers/i2c/Makefile | 2 +- drivers/i2c/i2c-emul-uclass.c | 2 + drivers/i2c/sandbox_i2c.c | 4 +- drivers/misc/irq-uclass.c | 2 +- drivers/misc/p2sb-uclass.c | 27 ++-- drivers/misc/spltest_sandbox.c | 35 ----- drivers/mmc/fsl_esdhc_imx.c | 7 +- drivers/rtc/rtc-uclass.c | 2 + drivers/rtc/sandbox_rtc.c | 4 +- drivers/serial/sandbox.c | 3 + drivers/spi/ich.c | 4 +- drivers/sysreset/sysreset_sandbox.c | 2 + dts/Kconfig | 18 +++ include/asm-generic/global_data.h | 13 ++ include/config_uncmd_spl.h | 1 - include/dm/device-internal.h | 2 +- include/dm/device.h | 14 ++ include/dm/platdata.h | 32 ++++- include/dm/util.h | 2 +- include/dt-structs.h | 6 +- test/dm/of_platdata.c | 203 ++++++++++++++++++++++++++++ test/dm/test-main.c | 24 +++- test/py/tests/test_ofplatdata.py | 47 ------- test/py/tests/test_spl.py | 5 +- tools/dtoc/dtb_platdata.py | 63 +++++---- tools/dtoc/dtoc_test_simple.dts | 1 + tools/dtoc/fdt.py | 9 ++ tools/dtoc/test_dtoc.py | 178 ++++++++++++++++-------- tools/dtoc/test_fdt.py | 10 ++ 47 files changed, 705 insertions(+), 284 deletions(-)
Since sandbox's SPL is build with of-platadata, we should not use U_BOOT_DEVICE() declarations as well. Drop them.
Signed-off-by: Simon Glass sjg@chromium.org ---
board/sandbox/sandbox.c | 2 ++ drivers/serial/sandbox.c | 3 +++ drivers/sysreset/sysreset_sandbox.c | 2 ++ 3 files changed, 7 insertions(+)
diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c index 937ce284111..18a605de026 100644 --- a/board/sandbox/sandbox.c +++ b/board/sandbox/sandbox.c @@ -21,10 +21,12 @@ */ gd_t *gd;
+#if !CONFIG_IS_ENABLED(OF_PLATDATA) /* Add a simple GPIO device */ U_BOOT_DEVICE(gpio_sandbox) = { .name = "sandbox_gpio", }; +#endif
void flush_cache(unsigned long start, unsigned long size) { diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c index f09d291e043..db2fbac6295 100644 --- a/drivers/serial/sandbox.c +++ b/drivers/serial/sandbox.c @@ -267,6 +267,7 @@ U_BOOT_DRIVER(sandbox_serial) = { .flags = DM_FLAG_PRE_RELOC, };
+#if !CONFIG_IS_ENABLED(OF_PLATDATA) static const struct sandbox_serial_platdata platdata_non_fdt = { .colour = -1, }; @@ -275,4 +276,6 @@ U_BOOT_DEVICE(serial_sandbox_non_fdt) = { .name = "sandbox_serial", .platdata = &platdata_non_fdt, }; +#endif + #endif /* CONFIG_IS_ENABLED(OF_CONTROL) */ diff --git a/drivers/sysreset/sysreset_sandbox.c b/drivers/sysreset/sysreset_sandbox.c index 69c22a70008..71cabd19568 100644 --- a/drivers/sysreset/sysreset_sandbox.c +++ b/drivers/sysreset/sysreset_sandbox.c @@ -130,7 +130,9 @@ U_BOOT_DRIVER(warm_sysreset_sandbox) = { .ops = &sandbox_warm_sysreset_ops, };
+#if !CONFIG_IS_ENABLED(OF_PLATDATA) /* This is here in case we don't have a device tree */ U_BOOT_DEVICE(sysreset_sandbox_non_fdt) = { .name = "sysreset_sandbox", }; +#endif
Since sandbox's SPL is build with of-platadata, we should not use U_BOOT_DEVICE() declarations as well. Drop them.
Signed-off-by: Simon Glass sjg@chromium.org ---
board/sandbox/sandbox.c | 2 ++ drivers/serial/sandbox.c | 3 +++ drivers/sysreset/sysreset_sandbox.c | 2 ++ 3 files changed, 7 insertions(+)
Applied to u-boot-dm, thanks!
Add documentation to this function as well as generate_structs(), where the return value is ultimately passed in.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/dtoc/dtb_platdata.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index 1b52198a943..72725bb6fa1 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -466,6 +466,13 @@ class DtbPlatdata(object):
Once the widest property is determined, all other properties are updated to match that width. + + Returns: + dict containing structures: + key (str): Node name, as a C identifier + value: dict containing structure fields: + key (str): Field name + value: Prop object with field information """ structs = {} for node in self._valid_nodes: @@ -536,6 +543,14 @@ class DtbPlatdata(object): This writes out the body of a header file consisting of structure definitions for node in self._valid_nodes. See the documentation in doc/driver-model/of-plat.rst for more information. + + Args: + structs: dict containing structures: + key (str): Node name, as a C identifier + value: dict containing structure fields: + key (str): Field name + value: Prop object with field information + """ self.out_header() self.out('#include <stdbool.h>\n')
Add documentation to this function as well as generate_structs(), where the return value is ultimately passed in.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/dtoc/dtb_platdata.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
Applied to u-boot-dm, thanks!
At present the structures are written in name order, but parents have to be written before their children, so the file does not end up being in order. The order of nodes in _valid_nodes matches the order of the devicetree.
Update the code so that _valid_nodes is in sorted order, by C name of the structure. This allows us to assign a sequential ordering to each U_BOOT_DEVICE() declaration.
U-Boot's linker lists are also ordered alphabetically, which means that the order in the driver_info list will match the order used by dtoc. This defines an index ('idx') for the U_BOOT_DEVICE declarations. They appear in alphabetical order, numbered from 0 in _valid_nodes and in the driver_info linker list.
Add a comment against each declaration, showing the idx value.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/dtoc/dtb_platdata.py | 21 +++++--- tools/dtoc/test_dtoc.py | 105 ++++++++++++++++++++++++------------- 2 files changed, 83 insertions(+), 43 deletions(-)
diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index 72725bb6fa1..31a9b3877ea 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -143,7 +143,8 @@ class DtbPlatdata(object): Properties: _fdt: Fdt object, referencing the device tree _dtb_fname: Filename of the input device tree binary file - _valid_nodes: A list of Node object with compatible strings + _valid_nodes: A list of Node object with compatible strings. The list + is ordered by conv_name_to_c(node.name) _include_disabled: true to include nodes marked status = "disabled" _outfile: The current output file (sys.stdout or a real file) _warning_disabled: true to disable warnings about driver names not found @@ -367,23 +368,24 @@ class DtbPlatdata(object): """ self._fdt = fdt.FdtScan(self._dtb_fname)
- def scan_node(self, root): + def scan_node(self, root, valid_nodes): """Scan a node and subnodes to build a tree of node and phandle info
This adds each node to self._valid_nodes.
Args: root: Root node for scan + valid_nodes: List of Node objects to add to """ for node in root.subnodes: if 'compatible' in node.props: status = node.props.get('status') if (not self._include_disabled and not status or status.value != 'disabled'): - self._valid_nodes.append(node) + valid_nodes.append(node)
# recurse to handle any subnodes - self.scan_node(node) + self.scan_node(node, valid_nodes)
def scan_tree(self): """Scan the device tree for useful information @@ -392,8 +394,12 @@ class DtbPlatdata(object): _valid_nodes: A list of nodes we wish to consider include in the platform data """ - self._valid_nodes = [] - return self.scan_node(self._fdt.GetRoot()) + valid_nodes = [] + self.scan_node(self._fdt.GetRoot(), valid_nodes) + self._valid_nodes = sorted(valid_nodes, + key=lambda x: conv_name_to_c(x.name)) + for idx, node in enumerate(self._valid_nodes): + node.idx = idx
@staticmethod def get_num_cells(node): @@ -474,7 +480,7 @@ class DtbPlatdata(object): key (str): Field name value: Prop object with field information """ - structs = {} + structs = collections.OrderedDict() for node in self._valid_nodes: node_name, _ = self.get_normalized_compat_name(node) fields = {} @@ -633,6 +639,7 @@ class DtbPlatdata(object):
struct_name, _ = self.get_normalized_compat_name(node) var_name = conv_name_to_c(node.name) + self.buf('/* Node %s index %d */\n' % (node.path, node.idx)) self.buf('static struct %s%s %s%s = {\n' % (STRUCT_PREFIX, struct_name, VAL_PREFIX, var_name)) for pname in sorted(node.props): diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index c2ff267de70..857a98e435f 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -209,6 +209,27 @@ struct dtd_sandbox_spl_test_2 { with open(output) as infile: data = infile.read() self._CheckStrings(C_HEADER + ''' +/* Node /i2c@0 index 0 */ +static struct dtd_sandbox_i2c_test dtv_i2c_at_0 = { +}; +U_BOOT_DEVICE(i2c_at_0) = { +\t.name\t\t= "sandbox_i2c_test", +\t.platdata\t= &dtv_i2c_at_0, +\t.platdata_size\t= sizeof(dtv_i2c_at_0), +}; + +/* Node /i2c@0/pmic@9 index 1 */ +static struct dtd_sandbox_pmic_test dtv_pmic_at_9 = { +\t.low_power\t\t= true, +\t.reg\t\t\t= {0x9, 0x0}, +}; +U_BOOT_DEVICE(pmic_at_9) = { +\t.name\t\t= "sandbox_pmic_test", +\t.platdata\t= &dtv_pmic_at_9, +\t.platdata_size\t= sizeof(dtv_pmic_at_9), +}; + +/* Node /spl-test index 2 */ static struct dtd_sandbox_spl_test dtv_spl_test = { \t.boolval\t\t= true, \t.bytearray\t\t= {0x6, 0x0, 0x0}, @@ -227,6 +248,7 @@ U_BOOT_DEVICE(spl_test) = { \t.platdata_size\t= sizeof(dtv_spl_test), };
+/* Node /spl-test2 index 3 */ static struct dtd_sandbox_spl_test dtv_spl_test2 = { \t.acpi_name\t\t= "\\_SB.GPO0", \t.bytearray\t\t= {0x1, 0x23, 0x34}, @@ -244,6 +266,7 @@ U_BOOT_DEVICE(spl_test2) = { \t.platdata_size\t= sizeof(dtv_spl_test2), };
+/* Node /spl-test3 index 4 */ static struct dtd_sandbox_spl_test dtv_spl_test3 = { \t.stringarray\t\t= {"one", "", ""}, }; @@ -253,6 +276,7 @@ U_BOOT_DEVICE(spl_test3) = { \t.platdata_size\t= sizeof(dtv_spl_test3), };
+/* Node /spl-test4 index 5 */ static struct dtd_sandbox_spl_test_2 dtv_spl_test4 = { }; U_BOOT_DEVICE(spl_test4) = { @@ -261,24 +285,6 @@ U_BOOT_DEVICE(spl_test4) = { \t.platdata_size\t= sizeof(dtv_spl_test4), };
-static struct dtd_sandbox_i2c_test dtv_i2c_at_0 = { -}; -U_BOOT_DEVICE(i2c_at_0) = { -\t.name\t\t= "sandbox_i2c_test", -\t.platdata\t= &dtv_i2c_at_0, -\t.platdata_size\t= sizeof(dtv_i2c_at_0), -}; - -static struct dtd_sandbox_pmic_test dtv_pmic_at_9 = { -\t.low_power\t\t= true, -\t.reg\t\t\t= {0x9, 0x0}, -}; -U_BOOT_DEVICE(pmic_at_9) = { -\t.name\t\t= "sandbox_pmic_test", -\t.platdata\t= &dtv_pmic_at_9, -\t.platdata_size\t= sizeof(dtv_pmic_at_9), -}; - ''' + C_EMPTY_POPULATE_PHANDLE_DATA, data)
def test_driver_alias(self): @@ -300,6 +306,7 @@ struct dtd_sandbox_gpio { with open(output) as infile: data = infile.read() self._CheckStrings(C_HEADER + ''' +/* Node /gpios@0 index 0 */ static struct dtd_sandbox_gpio dtv_gpios_at_0 = { \t.gpio_bank_name\t\t= "a", \t.gpio_controller\t= true, @@ -333,6 +340,7 @@ struct dtd_invalid { with open(output) as infile: data = infile.read() self._CheckStrings(C_HEADER + ''' +/* Node /spl-test index 0 */ static struct dtd_invalid dtv_spl_test = { }; U_BOOT_DEVICE(spl_test) = { @@ -365,15 +373,7 @@ struct dtd_target { with open(output) as infile: data = infile.read() self._CheckStrings(C_HEADER + ''' -static struct dtd_target dtv_phandle_target = { -\t.intval\t\t\t= 0x0, -}; -U_BOOT_DEVICE(phandle_target) = { -\t.name\t\t= "target", -\t.platdata\t= &dtv_phandle_target, -\t.platdata_size\t= sizeof(dtv_phandle_target), -}; - +/* Node /phandle2-target index 0 */ static struct dtd_target dtv_phandle2_target = { \t.intval\t\t\t= 0x1, }; @@ -383,6 +383,7 @@ U_BOOT_DEVICE(phandle2_target) = { \t.platdata_size\t= sizeof(dtv_phandle2_target), };
+/* Node /phandle3-target index 1 */ static struct dtd_target dtv_phandle3_target = { \t.intval\t\t\t= 0x2, }; @@ -392,6 +393,17 @@ U_BOOT_DEVICE(phandle3_target) = { \t.platdata_size\t= sizeof(dtv_phandle3_target), };
+/* Node /phandle-target index 4 */ +static struct dtd_target dtv_phandle_target = { +\t.intval\t\t\t= 0x0, +}; +U_BOOT_DEVICE(phandle_target) = { +\t.name\t\t= "target", +\t.platdata\t= &dtv_phandle_target, +\t.platdata_size\t= sizeof(dtv_phandle_target), +}; + +/* Node /phandle-source index 2 */ static struct dtd_source dtv_phandle_source = { \t.clocks\t\t\t= { \t\t\t{NULL, {}}, @@ -405,6 +417,7 @@ U_BOOT_DEVICE(phandle_source) = { \t.platdata_size\t= sizeof(dtv_phandle_source), };
+/* Node /phandle-source2 index 3 */ static struct dtd_source dtv_phandle_source2 = { \t.clocks\t\t\t= { \t\t\t{NULL, {}},}, @@ -448,6 +461,7 @@ struct dtd_target { with open(output) as infile: data = infile.read() self._CheckStrings(C_HEADER + ''' +/* Node /phandle-target index 1 */ static struct dtd_target dtv_phandle_target = { }; U_BOOT_DEVICE(phandle_target) = { @@ -456,6 +470,7 @@ U_BOOT_DEVICE(phandle_target) = { \t.platdata_size\t= sizeof(dtv_phandle_target), };
+/* Node /phandle-source2 index 0 */ static struct dtd_source dtv_phandle_source2 = { \t.clocks\t\t\t= { \t\t\t{NULL, {}},}, @@ -479,15 +494,7 @@ void dm_populate_phandle_data(void) { with open(output) as infile: data = infile.read() self._CheckStrings(C_HEADER + ''' -static struct dtd_target dtv_phandle_target = { -\t.intval\t\t\t= 0x0, -}; -U_BOOT_DEVICE(phandle_target) = { -\t.name\t\t= "target", -\t.platdata\t= &dtv_phandle_target, -\t.platdata_size\t= sizeof(dtv_phandle_target), -}; - +/* Node /phandle2-target index 0 */ static struct dtd_target dtv_phandle2_target = { \t.intval\t\t\t= 0x1, }; @@ -497,6 +504,7 @@ U_BOOT_DEVICE(phandle2_target) = { \t.platdata_size\t= sizeof(dtv_phandle2_target), };
+/* Node /phandle3-target index 1 */ static struct dtd_target dtv_phandle3_target = { \t.intval\t\t\t= 0x2, }; @@ -506,6 +514,17 @@ U_BOOT_DEVICE(phandle3_target) = { \t.platdata_size\t= sizeof(dtv_phandle3_target), };
+/* Node /phandle-target index 4 */ +static struct dtd_target dtv_phandle_target = { +\t.intval\t\t\t= 0x0, +}; +U_BOOT_DEVICE(phandle_target) = { +\t.name\t\t= "target", +\t.platdata\t= &dtv_phandle_target, +\t.platdata_size\t= sizeof(dtv_phandle_target), +}; + +/* Node /phandle-source index 2 */ static struct dtd_source dtv_phandle_source = { \t.cd_gpios\t\t= { \t\t\t{NULL, {}}, @@ -519,6 +538,7 @@ U_BOOT_DEVICE(phandle_source) = { \t.platdata_size\t= sizeof(dtv_phandle_source), };
+/* Node /phandle-source2 index 3 */ static struct dtd_source dtv_phandle_source2 = { \t.cd_gpios\t\t= { \t\t\t{NULL, {}},}, @@ -581,6 +601,7 @@ struct dtd_test3 { with open(output) as infile: data = infile.read() self._CheckStrings(C_HEADER + ''' +/* Node /test1 index 0 */ static struct dtd_test1 dtv_test1 = { \t.reg\t\t\t= {0x1234, 0x5678}, }; @@ -590,6 +611,7 @@ U_BOOT_DEVICE(test1) = { \t.platdata_size\t= sizeof(dtv_test1), };
+/* Node /test2 index 1 */ static struct dtd_test2 dtv_test2 = { \t.reg\t\t\t= {0x1234567890123456, 0x9876543210987654}, }; @@ -599,6 +621,7 @@ U_BOOT_DEVICE(test2) = { \t.platdata_size\t= sizeof(dtv_test2), };
+/* Node /test3 index 2 */ static struct dtd_test3 dtv_test3 = { \t.reg\t\t\t= {0x1234567890123456, 0x9876543210987654, 0x2, 0x3}, }; @@ -630,6 +653,7 @@ struct dtd_test2 { with open(output) as infile: data = infile.read() self._CheckStrings(C_HEADER + ''' +/* Node /test1 index 0 */ static struct dtd_test1 dtv_test1 = { \t.reg\t\t\t= {0x1234, 0x5678}, }; @@ -639,6 +663,7 @@ U_BOOT_DEVICE(test1) = { \t.platdata_size\t= sizeof(dtv_test1), };
+/* Node /test2 index 1 */ static struct dtd_test2 dtv_test2 = { \t.reg\t\t\t= {0x12345678, 0x98765432, 0x2, 0x3}, }; @@ -673,6 +698,7 @@ struct dtd_test3 { with open(output) as infile: data = infile.read() self._CheckStrings(C_HEADER + ''' +/* Node /test1 index 0 */ static struct dtd_test1 dtv_test1 = { \t.reg\t\t\t= {0x123400000000, 0x5678}, }; @@ -682,6 +708,7 @@ U_BOOT_DEVICE(test1) = { \t.platdata_size\t= sizeof(dtv_test1), };
+/* Node /test2 index 1 */ static struct dtd_test2 dtv_test2 = { \t.reg\t\t\t= {0x1234567890123456, 0x98765432}, }; @@ -691,6 +718,7 @@ U_BOOT_DEVICE(test2) = { \t.platdata_size\t= sizeof(dtv_test2), };
+/* Node /test3 index 2 */ static struct dtd_test3 dtv_test3 = { \t.reg\t\t\t= {0x1234567890123456, 0x98765432, 0x2, 0x3}, }; @@ -725,6 +753,7 @@ struct dtd_test3 { with open(output) as infile: data = infile.read() self._CheckStrings(C_HEADER + ''' +/* Node /test1 index 0 */ static struct dtd_test1 dtv_test1 = { \t.reg\t\t\t= {0x1234, 0x567800000000}, }; @@ -734,6 +763,7 @@ U_BOOT_DEVICE(test1) = { \t.platdata_size\t= sizeof(dtv_test1), };
+/* Node /test2 index 1 */ static struct dtd_test2 dtv_test2 = { \t.reg\t\t\t= {0x12345678, 0x9876543210987654}, }; @@ -743,6 +773,7 @@ U_BOOT_DEVICE(test2) = { \t.platdata_size\t= sizeof(dtv_test2), };
+/* Node /test3 index 2 */ static struct dtd_test3 dtv_test3 = { \t.reg\t\t\t= {0x12345678, 0x9876543210987654, 0x2, 0x3}, }; @@ -792,6 +823,7 @@ struct dtd_sandbox_spl_test { with open(output) as infile: data = infile.read() self._CheckStrings(C_HEADER + ''' +/* Node /spl-test index 0 */ static struct dtd_sandbox_spl_test dtv_spl_test = { \t.intval\t\t\t= 0x1, }; @@ -801,6 +833,7 @@ U_BOOT_DEVICE(spl_test) = { \t.platdata_size\t= sizeof(dtv_spl_test), };
+/* Node /spl-test2 index 1 */ static struct dtd_sandbox_spl_test dtv_spl_test2 = { \t.intarray\t\t= 0x5, };
At present the structures are written in name order, but parents have to be written before their children, so the file does not end up being in order. The order of nodes in _valid_nodes matches the order of the devicetree.
Update the code so that _valid_nodes is in sorted order, by C name of the structure. This allows us to assign a sequential ordering to each U_BOOT_DEVICE() declaration.
U-Boot's linker lists are also ordered alphabetically, which means that the order in the driver_info list will match the order used by dtoc. This defines an index ('idx') for the U_BOOT_DEVICE declarations. They appear in alphabetical order, numbered from 0 in _valid_nodes and in the driver_info linker list.
Add a comment against each declaration, showing the idx value.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/dtoc/dtb_platdata.py | 21 +++++--- tools/dtoc/test_dtoc.py | 105 ++++++++++++++++++++++++------------- 2 files changed, 83 insertions(+), 43 deletions(-)
Applied to u-boot-dm, thanks!
At present this option is disabled in SPL, meaning that warnings are not displayed. It is sometimes useful to see warnings in SPL for debugging purposes.
Add a new Kconfig option to permit this.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/Kconfig | 18 ++++++++++++++++-- drivers/core/util.c | 2 +- include/config_uncmd_spl.h | 1 - include/dm/util.h | 2 +- 4 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig index 1ca5d66141b..603a1d27c0c 100644 --- a/drivers/core/Kconfig +++ b/drivers/core/Kconfig @@ -40,10 +40,24 @@ config DM_WARN depends on DM default y help + Enable this to see warnings related to driver model. + + Warnings may help with debugging, such as when expected devices do + not bind correctly. If the option is disabled, dm_warn() is compiled + out - it will do nothing when called. + +config SPL_DM_WARN + bool "Enable warnings in driver model wuth SPL" + depends on SPL_DM + help + Enable this to see warnings related to driver model in SPL + The dm_warn() function can use up quite a bit of space for its strings. By default this is disabled for SPL builds to save space. - This will cause dm_warn() to be compiled out - it will do nothing - when called. + + Warnings may help with debugging, such as when expected devices do + not bind correctly. If the option is disabled, dm_warn() is compiled + out - it will do nothing when called.
config DM_DEBUG bool "Enable debug messages in driver model core" diff --git a/drivers/core/util.c b/drivers/core/util.c index 25b0d76f430..91e93b0cf14 100644 --- a/drivers/core/util.c +++ b/drivers/core/util.c @@ -11,7 +11,7 @@ #include <linux/libfdt.h> #include <vsprintf.h>
-#ifdef CONFIG_DM_WARN +#if CONFIG_IS_ENABLED(DM_WARN) void dm_warn(const char *fmt, ...) { va_list args; diff --git a/include/config_uncmd_spl.h b/include/config_uncmd_spl.h index 31da6215b3a..af7e3e49fdd 100644 --- a/include/config_uncmd_spl.h +++ b/include/config_uncmd_spl.h @@ -16,7 +16,6 @@ #undef CONFIG_DM_SPI #endif
-#undef CONFIG_DM_WARN #undef CONFIG_DM_STDIO
#endif /* CONFIG_SPL_BUILD */ diff --git a/include/dm/util.h b/include/dm/util.h index 9773db6de17..01a044992f2 100644 --- a/include/dm/util.h +++ b/include/dm/util.h @@ -6,7 +6,7 @@ #ifndef __DM_UTIL_H #define __DM_UTIL_H
-#ifdef CONFIG_DM_WARN +#if CONFIG_IS_ENABLED(DM_WARN) void dm_warn(const char *fmt, ...); #else static inline void dm_warn(const char *fmt, ...)
At present this option is disabled in SPL, meaning that warnings are not displayed. It is sometimes useful to see warnings in SPL for debugging purposes.
Add a new Kconfig option to permit this.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/Kconfig | 18 ++++++++++++++++-- drivers/core/util.c | 2 +- include/config_uncmd_spl.h | 1 - include/dm/util.h | 2 +- 4 files changed, 18 insertions(+), 5 deletions(-)
Applied to u-boot-dm, thanks!
At present an integer is converted to bytes incorrectly. The whole 32-bit integer is inserted as the first element of the byte array, and the other three bytes are skipped. This was not noticed because the unit test did not check it, and the functional test was checking for wrong values.
Update the code to handle this as a special case. Add one more test to cover all code paths.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/py/tests/test_ofplatdata.py | 2 +- tools/dtoc/dtoc_test_simple.dts | 1 + tools/dtoc/fdt.py | 9 +++++++++ tools/dtoc/test_dtoc.py | 4 +++- tools/dtoc/test_fdt.py | 10 ++++++++++ 5 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/test/py/tests/test_ofplatdata.py b/test/py/tests/test_ofplatdata.py index 263334b0743..13154935b9b 100644 --- a/test/py/tests/test_ofplatdata.py +++ b/test/py/tests/test_ofplatdata.py @@ -20,7 +20,7 @@ byte 08 bytearray 01 23 34 int 3 intarray 5 0 0 0 -longbytearray 09 00 00 00 00 00 00 00 00 +longbytearray 09 0a 0b 0c 00 00 00 00 00 string message2 stringarray "another" "multi-word" "message" of-platdata probe: diff --git a/tools/dtoc/dtoc_test_simple.dts b/tools/dtoc/dtoc_test_simple.dts index 11bfc4c47aa..fd168cb5931 100644 --- a/tools/dtoc/dtoc_test_simple.dts +++ b/tools/dtoc/dtoc_test_simple.dts @@ -41,6 +41,7 @@ u-boot,dm-pre-reloc; compatible = "sandbox,spl-test"; stringarray = "one"; + longbytearray = [09 0a 0b 0c 0d 0e 0f 10]; };
spl-test4 { diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index d058c59e927..03b86773d5f 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -129,6 +129,15 @@ class Prop: specific. """ if newprop.type < self.type: + # Special handling to convert an int into bytes + if self.type == TYPE_INT and newprop.type == TYPE_BYTE: + if type(self.value) == list: + new_value = [] + for val in self.value: + new_value += [tools.ToChar(by) for by in val] + else: + new_value = [tools.ToChar(by) for by in self.value] + self.value = new_value self.type = newprop.type
if type(newprop.value) == list and type(self.value) != list: diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 857a98e435f..8dcac91ee70 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -255,7 +255,7 @@ static struct dtd_sandbox_spl_test dtv_spl_test2 = { \t.byteval\t\t= 0x8, \t.intarray\t\t= {0x5, 0x0, 0x0, 0x0}, \t.intval\t\t\t= 0x3, -\t.longbytearray\t\t= {0x9, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, +\t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0x0, 0x0, 0x0, 0x0, \t\t0x0}, \t.stringarray\t\t= {"another", "multi-word", "message"}, \t.stringval\t\t= "message2", @@ -268,6 +268,8 @@ U_BOOT_DEVICE(spl_test2) = {
/* Node /spl-test3 index 4 */ static struct dtd_sandbox_spl_test dtv_spl_test3 = { +\t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0x10, +\t\t0x0}, \t.stringarray\t\t= {"one", "", ""}, }; U_BOOT_DEVICE(spl_test3) = { diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index b4f9b7f498f..cfe3e04c7ad 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -298,6 +298,7 @@ class TestProp(unittest.TestCase): def testWiden(self): """Test widening of values""" node2 = self.dtb.GetNode('/spl-test2') + node3 = self.dtb.GetNode('/spl-test3') prop = self.node.props['intval']
# No action @@ -316,11 +317,20 @@ class TestProp(unittest.TestCase): # byte array, it should turn into an array. prop = self.node.props['longbytearray'] prop2 = node2.props['longbytearray'] + prop3 = node3.props['longbytearray'] self.assertFalse(isinstance(prop2.value, list)) self.assertEqual(4, len(prop2.value)) + self.assertEqual(b'\x09\x0a\x0b\x0c', prop2.value) prop2.Widen(prop) self.assertTrue(isinstance(prop2.value, list)) self.assertEqual(9, len(prop2.value)) + self.assertEqual(['\x09', '\x0a', '\x0b', '\x0c', '\0', + '\0', '\0', '\0', '\0'], prop2.value) + prop3.Widen(prop) + self.assertTrue(isinstance(prop3.value, list)) + self.assertEqual(9, len(prop3.value)) + self.assertEqual(['\x09', '\x0a', '\x0b', '\x0c', '\x0d', + '\x0e', '\x0f', '\x10', '\0'], prop3.value)
# Similarly for a string array prop = self.node.props['stringval']
At present an integer is converted to bytes incorrectly. The whole 32-bit integer is inserted as the first element of the byte array, and the other three bytes are skipped. This was not noticed because the unit test did not check it, and the functional test was checking for wrong values.
Update the code to handle this as a special case. Add one more test to cover all code paths.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/py/tests/test_ofplatdata.py | 2 +- tools/dtoc/dtoc_test_simple.dts | 1 + tools/dtoc/fdt.py | 9 +++++++++ tools/dtoc/test_dtoc.py | 4 +++- tools/dtoc/test_fdt.py | 10 ++++++++++ 5 files changed, 24 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
At present properties are tested in a roundabout way. The driver's probe() method writes out the values of the properties and the Python test checks the output from U-Boot SPL.
Add a C test which checks these values more directly.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/dm/of_platdata.c | 69 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+)
diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c index 7a864eb0fb3..8763005ea9a 100644 --- a/test/dm/of_platdata.c +++ b/test/dm/of_platdata.c @@ -2,6 +2,7 @@
#include <common.h> #include <dm.h> +#include <dt-structs.h> #include <dm/test.h> #include <test/test.h> #include <test/ut.h> @@ -17,3 +18,71 @@ static int dm_test_of_platdata_base(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_of_platdata_base, UT_TESTF_SCAN_PDATA); + +/* Test that we can read properties from a device */ +static int dm_test_of_platdata_props(struct unit_test_state *uts) +{ + struct dtd_sandbox_spl_test *plat; + struct udevice *dev; + int i; + + ut_assertok(uclass_first_device_err(UCLASS_MISC, &dev)); + plat = dev_get_platdata(dev); + ut_assert(plat->boolval); + ut_asserteq(1, plat->intval); + ut_asserteq(4, ARRAY_SIZE(plat->intarray)); + ut_asserteq(2, plat->intarray[0]); + ut_asserteq(3, plat->intarray[1]); + ut_asserteq(4, plat->intarray[2]); + ut_asserteq(0, plat->intarray[3]); + ut_asserteq(5, plat->byteval); + ut_asserteq(3, ARRAY_SIZE(plat->bytearray)); + ut_asserteq(6, plat->bytearray[0]); + ut_asserteq(0, plat->bytearray[1]); + ut_asserteq(0, plat->bytearray[2]); + ut_asserteq(9, ARRAY_SIZE(plat->longbytearray)); + for (i = 0; i < ARRAY_SIZE(plat->longbytearray); i++) + ut_asserteq(9 + i, plat->longbytearray[i]); + ut_asserteq_str("message", plat->stringval); + ut_asserteq(3, ARRAY_SIZE(plat->stringarray)); + ut_asserteq_str("multi-word", plat->stringarray[0]); + ut_asserteq_str("message", plat->stringarray[1]); + ut_asserteq_str("", plat->stringarray[2]); + + ut_assertok(uclass_next_device_err(&dev)); + plat = dev_get_platdata(dev); + ut_assert(!plat->boolval); + ut_asserteq(3, plat->intval); + ut_asserteq(5, plat->intarray[0]); + ut_asserteq(0, plat->intarray[1]); + ut_asserteq(0, plat->intarray[2]); + ut_asserteq(0, plat->intarray[3]); + ut_asserteq(8, plat->byteval); + ut_asserteq(3, ARRAY_SIZE(plat->bytearray)); + ut_asserteq(1, plat->bytearray[0]); + ut_asserteq(0x23, plat->bytearray[1]); + ut_asserteq(0x34, plat->bytearray[2]); + for (i = 0; i < ARRAY_SIZE(plat->longbytearray); i++) + ut_asserteq(i < 4 ? 9 + i : 0, plat->longbytearray[i]); + ut_asserteq_str("message2", plat->stringval); + ut_asserteq_str("another", plat->stringarray[0]); + ut_asserteq_str("multi-word", plat->stringarray[1]); + ut_asserteq_str("message", plat->stringarray[2]); + + ut_assertok(uclass_next_device_err(&dev)); + plat = dev_get_platdata(dev); + ut_assert(!plat->boolval); + ut_asserteq_str("one", plat->stringarray[0]); + ut_asserteq_str("", plat->stringarray[1]); + ut_asserteq_str("", plat->stringarray[2]); + + ut_assertok(uclass_next_device_err(&dev)); + plat = dev_get_platdata(dev); + ut_assert(!plat->boolval); + ut_asserteq_str("spl", plat->stringarray[0]); + + ut_asserteq(-ENODEV, uclass_next_device_err(&dev)); + + return 0; +} +DM_TEST(dm_test_of_platdata_props, UT_TESTF_SCAN_PDATA);
At present properties are tested in a roundabout way. The driver's probe() method writes out the values of the properties and the Python test checks the output from U-Boot SPL.
Add a C test which checks these values more directly.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/dm/of_platdata.c | 69 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+)
Applied to u-boot-dm, thanks!
Now that we have more than one test, add a way to select the test to run.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/cpu/spl.c | 2 +- arch/sandbox/cpu/start.c | 9 +++++++++ arch/sandbox/include/asm/state.h | 1 + 3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c index 48fd1265afe..81b217a1d70 100644 --- a/arch/sandbox/cpu/spl.c +++ b/arch/sandbox/cpu/spl.c @@ -72,7 +72,7 @@ void spl_board_init(void) if (state->run_unittests) { int ret;
- ret = dm_test_main(NULL); + ret = dm_test_main(state->select_unittests); /* continue execution into U-Boot */ } } diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c index f5e104b127b..569dafbcfee 100644 --- a/arch/sandbox/cpu/start.c +++ b/arch/sandbox/cpu/start.c @@ -383,6 +383,15 @@ static int sandbox_cmdline_cb_unittests(struct sandbox_state *state, } SANDBOX_CMDLINE_OPT_SHORT(unittests, 'u', 0, "Run unit tests");
+static int sandbox_cmdline_cb_select_unittests(struct sandbox_state *state, + const char *arg) +{ + state->select_unittests = arg; + + return 0; +} +SANDBOX_CMDLINE_OPT_SHORT(select_unittests, 'k', 1, "Select unit tests to run"); + static void setup_ram_buf(struct sandbox_state *state) { /* Zero the RAM buffer if we didn't read it, to keep valgrind happy */ diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h index f828d9d2447..7547602dd1c 100644 --- a/arch/sandbox/include/asm/state.h +++ b/arch/sandbox/include/asm/state.h @@ -93,6 +93,7 @@ struct sandbox_state { bool show_of_platdata; /* Show of-platdata in SPL */ bool ram_buf_read; /* true if we read the RAM buffer */ bool run_unittests; /* Run unit tests */ + const char *select_unittests; /* Unit test to run */
/* Pointer to information for each SPI bus/cs */ struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS]
Now that we have more than one test, add a way to select the test to run.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/cpu/spl.c | 2 +- arch/sandbox/cpu/start.c | 9 +++++++++ arch/sandbox/include/asm/state.h | 1 + 3 files changed, 11 insertions(+), 1 deletion(-)
Applied to u-boot-dm, thanks!
Now that we have a C version of this test, drop the Python implementation.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/cpu/spl.c | 12 -------- arch/sandbox/cpu/start.c | 9 ------ arch/sandbox/include/asm/state.h | 1 - drivers/misc/spltest_sandbox.c | 35 ------------------------ test/py/tests/test_ofplatdata.py | 47 -------------------------------- 5 files changed, 104 deletions(-)
diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c index 81b217a1d70..9a77da15619 100644 --- a/arch/sandbox/cpu/spl.c +++ b/arch/sandbox/cpu/spl.c @@ -54,20 +54,8 @@ SPL_LOAD_IMAGE_METHOD("sandbox", 9, BOOT_DEVICE_BOARD, spl_board_load_image); void spl_board_init(void) { struct sandbox_state *state = state_get_current(); - struct udevice *dev;
preloader_console_init(); - if (state->show_of_platdata) { - /* - * Scan all the devices so that we can output their platform - * data. See sandbox_spl_probe(). - */ - printf("Scanning misc devices\n"); - for (uclass_first_device(UCLASS_MISC, &dev); - dev; - uclass_next_device(&dev)) - ; - }
if (state->run_unittests) { int ret; diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c index 569dafbcfee..58ada13fba5 100644 --- a/arch/sandbox/cpu/start.c +++ b/arch/sandbox/cpu/start.c @@ -365,15 +365,6 @@ static int sandbox_cmdline_cb_log_level(struct sandbox_state *state, SANDBOX_CMDLINE_OPT_SHORT(log_level, 'L', 1, "Set log level (0=panic, 7=debug)");
-static int sandbox_cmdline_cb_show_of_platdata(struct sandbox_state *state, - const char *arg) -{ - state->show_of_platdata = true; - - return 0; -} -SANDBOX_CMDLINE_OPT(show_of_platdata, 0, "Show of-platdata in SPL"); - static int sandbox_cmdline_cb_unittests(struct sandbox_state *state, const char *arg) { diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h index 7547602dd1c..bca13069824 100644 --- a/arch/sandbox/include/asm/state.h +++ b/arch/sandbox/include/asm/state.h @@ -90,7 +90,6 @@ struct sandbox_state { bool skip_delays; /* Ignore any time delays (for test) */ bool show_test_output; /* Don't suppress stdout in tests */ int default_log_level; /* Default log level for sandbox */ - bool show_of_platdata; /* Show of-platdata in SPL */ bool ram_buf_read; /* true if we read the RAM buffer */ bool run_unittests; /* Run unit tests */ const char *select_unittests; /* Unit test to run */ diff --git a/drivers/misc/spltest_sandbox.c b/drivers/misc/spltest_sandbox.c index 999031625b5..3ae6707593e 100644 --- a/drivers/misc/spltest_sandbox.c +++ b/drivers/misc/spltest_sandbox.c @@ -8,43 +8,8 @@ #include <dm.h> #include <dt-structs.h>
-static int sandbox_spl_probe(struct udevice *dev) -{ - struct dtd_sandbox_spl_test *plat = dev_get_platdata(dev); - int i; - - printf("of-platdata probe:\n"); - printf("bool %d\n", plat->boolval); - - printf("byte %02x\n", plat->byteval); - printf("bytearray"); - for (i = 0; i < sizeof(plat->bytearray); i++) - printf(" %02x", plat->bytearray[i]); - printf("\n"); - - printf("int %d\n", plat->intval); - printf("intarray"); - for (i = 0; i < ARRAY_SIZE(plat->intarray); i++) - printf(" %d", plat->intarray[i]); - printf("\n"); - - printf("longbytearray"); - for (i = 0; i < sizeof(plat->longbytearray); i++) - printf(" %02x", plat->longbytearray[i]); - printf("\n"); - - printf("string %s\n", plat->stringval); - printf("stringarray"); - for (i = 0; i < ARRAY_SIZE(plat->stringarray); i++) - printf(" "%s"", plat->stringarray[i]); - printf("\n"); - - return 0; -} - U_BOOT_DRIVER(sandbox_spl_test) = { .name = "sandbox_spl_test", .id = UCLASS_MISC, .flags = DM_FLAG_PRE_RELOC, - .probe = sandbox_spl_probe, }; diff --git a/test/py/tests/test_ofplatdata.py b/test/py/tests/test_ofplatdata.py index 13154935b9b..78837a3c008 100644 --- a/test/py/tests/test_ofplatdata.py +++ b/test/py/tests/test_ofplatdata.py @@ -4,53 +4,6 @@ import pytest import u_boot_utils as util
-OF_PLATDATA_OUTPUT = ''' -of-platdata probe: -bool 1 -byte 05 -bytearray 06 00 00 -int 1 -intarray 2 3 4 0 -longbytearray 09 0a 0b 0c 0d 0e 0f 10 11 -string message -stringarray "multi-word" "message" "" -of-platdata probe: -bool 0 -byte 08 -bytearray 01 23 34 -int 3 -intarray 5 0 0 0 -longbytearray 09 0a 0b 0c 00 00 00 00 00 -string message2 -stringarray "another" "multi-word" "message" -of-platdata probe: -bool 0 -byte 00 -bytearray 00 00 00 -int 0 -intarray 0 0 0 0 -longbytearray 00 00 00 00 00 00 00 00 00 -string <NULL> -stringarray "one" "" "" -of-platdata probe: -bool 0 -byte 00 -bytearray 00 00 00 -int 0 -intarray 0 0 0 0 -longbytearray 00 00 00 00 00 00 00 00 00 -string <NULL> -stringarray "spl" "" "" -''' - -@pytest.mark.buildconfigspec('spl_of_platdata') -def test_ofplatdata(u_boot_console): - """Test that of-platdata can be generated and used in sandbox""" - cons = u_boot_console - cons.restart_uboot_with_flags(['--show_of_platdata']) - output = cons.get_spawn_output().replace('\r', '') - assert OF_PLATDATA_OUTPUT in output - @pytest.mark.buildconfigspec('spl_of_platdata') def test_spl_devicetree(u_boot_console): """Test content of spl device-tree"""
With of-platdata, the driver_info struct is updated with the device pointer when it is bound. This makes it easy for a device to be found by its driver info with the device_get_by_driver_info() function.
Add a test that all devices (except the root device) have such an entry. Fix a bug that the function does not set *devp to NULL on failure, which the documentation asserts.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device.c | 1 + test/dm/of_platdata.c | 81 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+)
diff --git a/drivers/core/device.c b/drivers/core/device.c index 355dbd147a9..aacc6adf02d 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -745,6 +745,7 @@ int device_get_by_driver_info(const struct driver_info *info, struct udevice *dev;
dev = info->dev; + *devp = NULL;
return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp); } diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c index 8763005ea9a..80900e416ba 100644 --- a/test/dm/of_platdata.c +++ b/test/dm/of_platdata.c @@ -86,3 +86,84 @@ static int dm_test_of_platdata_props(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_of_platdata_props, UT_TESTF_SCAN_PDATA); + +/* + * find_driver_info - recursively find the driver_info for a device + * + * This sets found[idx] to true when it finds the driver_info record for a + * device, where idx is the index in the driver_info linker list. + * + * @uts: Test state + * @parent: Parent to search + * @found: bool array to update + * @return 0 if OK, non-zero on error + */ +static int find_driver_info(struct unit_test_state *uts, struct udevice *parent, + bool found[]) +{ + struct udevice *dev; + + /* If not the root device, find the entry that caused it to be bound */ + if (parent->parent) { + 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); + const struct driver_info *entry; + int idx = -1; + + for (entry = info; entry != info + n_ents; entry++) { + if (entry->dev == parent) { + idx = entry - info; + found[idx] = true; + break; + } + } + + ut_assert(idx != -1); + } + + device_foreach_child(dev, parent) { + int ret; + + ret = find_driver_info(uts, dev, found); + if (ret < 0) + return ret; + } + + return 0; +} + +/* Check that every device is recorded in its driver_info struct */ +static int dm_test_of_platdata_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; + + /* Record the indexes that are found */ + memset(found, '\0', sizeof(found)); + ut_assertok(find_driver_info(uts, gd->dm_root, found)); + + /* Make sure that the driver entries without devices have no ->dev */ + for (i = 0; i < n_ents; i++) { + const struct driver_info *entry = info + i; + struct udevice *dev; + + if (found[i]) { + /* Make sure we can find it */ + ut_assertnonnull(entry->dev); + ut_assertok(device_get_by_driver_info(entry, &dev)); + ut_asserteq_ptr(dev, entry->dev); + } else { + ut_assertnull(entry->dev); + ut_asserteq(-ENOENT, + device_get_by_driver_info(entry, &dev)); + } + } + + return 0; +} +DM_TEST(dm_test_of_platdata_dev, UT_TESTF_SCAN_PDATA);
With of-platdata, the driver_info struct is updated with the device pointer when it is bound. This makes it easy for a device to be found by its driver info with the device_get_by_driver_info() function.
Add a test that all devices (except the root device) have such an entry. Fix a bug that the function does not set *devp to NULL on failure, which the documentation asserts.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device.c | 1 + test/dm/of_platdata.c | 81 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+)
Applied to u-boot-dm, thanks!
We have a test in dtoc for this feature, but not one in U-Boot itself. Add a simple test that checks that the information comes through correctly.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/dts/sandbox.dtsi | 26 ++++++++++++++++++++++++ configs/sandbox_spl_defconfig | 1 + drivers/clk/clk_fixed_rate.c | 4 ++-- drivers/clk/clk_sandbox.c | 4 ++-- test/dm/of_platdata.c | 37 +++++++++++++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 4 deletions(-)
diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi index c76ecc013c9..ca7049fc974 100644 --- a/arch/sandbox/dts/sandbox.dtsi +++ b/arch/sandbox/dts/sandbox.dtsi @@ -29,6 +29,32 @@ }; };
+ clk_fixed: clk-fixed { + u-boot,dm-pre-reloc; + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <1234>; + }; + + clk_sandbox: clk-sbox { + u-boot,dm-pre-reloc; + compatible = "sandbox,clk"; + #clock-cells = <1>; + assigned-clocks = <&clk_sandbox 3>; + assigned-clock-rates = <321>; + }; + + clk-test { + u-boot,dm-pre-reloc; + compatible = "sandbox,clk-test"; + clocks = <&clk_fixed>, + <&clk_sandbox 1>, + <&clk_sandbox 0>, + <&clk_sandbox 3>, + <&clk_sandbox 2>; + clock-names = "fixed", "i2c", "spi", "uart2", "uart1"; + }; + gpio_a: gpios@0 { u-boot,dm-pre-reloc; gpio-controller; diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 49060cab7a3..91215d23a0f 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -104,6 +104,7 @@ CONFIG_ADC_SANDBOX=y CONFIG_AXI=y CONFIG_AXI_SANDBOX=y CONFIG_CLK=y +CONFIG_SPL_CLK=y CONFIG_CPU=y CONFIG_DM_DEMO=y CONFIG_DM_DEMO_SIMPLE=y diff --git a/drivers/clk/clk_fixed_rate.c b/drivers/clk/clk_fixed_rate.c index 55e1f8caa52..f86b4a0e924 100644 --- a/drivers/clk/clk_fixed_rate.c +++ b/drivers/clk/clk_fixed_rate.c @@ -46,8 +46,8 @@ static const struct udevice_id clk_fixed_rate_match[] = { { /* sentinel */ } };
-U_BOOT_DRIVER(clk_fixed_rate) = { - .name = "fixed_rate_clock", +U_BOOT_DRIVER(fixed_clock) = { + .name = "fixed_clock", .id = UCLASS_CLK, .of_match = clk_fixed_rate_match, .ofdata_to_platdata = clk_fixed_rate_ofdata_to_platdata, diff --git a/drivers/clk/clk_sandbox.c b/drivers/clk/clk_sandbox.c index 768fbb7c520..0ff1b496338 100644 --- a/drivers/clk/clk_sandbox.c +++ b/drivers/clk/clk_sandbox.c @@ -124,8 +124,8 @@ static const struct udevice_id sandbox_clk_ids[] = { { } };
-U_BOOT_DRIVER(clk_sandbox) = { - .name = "clk_sandbox", +U_BOOT_DRIVER(sandbox_clk) = { + .name = "sandbox_clk", .id = UCLASS_CLK, .of_match = sandbox_clk_ids, .ops = &sandbox_clk_ops, diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c index 80900e416ba..57f903611a6 100644 --- a/test/dm/of_platdata.c +++ b/test/dm/of_platdata.c @@ -26,7 +26,11 @@ static int dm_test_of_platdata_props(struct unit_test_state *uts) struct udevice *dev; int i;
+ /* Skip the clock */ ut_assertok(uclass_first_device_err(UCLASS_MISC, &dev)); + ut_asserteq_str("sandbox_clk_test", dev->name); + + ut_assertok(uclass_next_device_err(&dev)); plat = dev_get_platdata(dev); ut_assert(plat->boolval); ut_asserteq(1, plat->intval); @@ -167,3 +171,36 @@ static int dm_test_of_platdata_dev(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_of_platdata_dev, UT_TESTF_SCAN_PDATA); + +/* Test handling of phandles that point to other devices */ +static int dm_test_of_platdata_phandle(struct unit_test_state *uts) +{ + struct dtd_sandbox_clk_test *plat; + struct udevice *dev, *clk; + + ut_assertok(uclass_first_device_err(UCLASS_MISC, &dev)); + ut_asserteq_str("sandbox_clk_test", dev->name); + plat = dev_get_platdata(dev); + + ut_assertok(device_get_by_driver_info(plat->clocks[0].node, &clk)); + ut_asserteq_str("fixed_clock", clk->name); + + ut_assertok(device_get_by_driver_info(plat->clocks[1].node, &clk)); + ut_asserteq_str("sandbox_clk", clk->name); + ut_asserteq(1, plat->clocks[1].arg[0]); + + ut_assertok(device_get_by_driver_info(plat->clocks[2].node, &clk)); + ut_asserteq_str("sandbox_clk", clk->name); + ut_asserteq(0, plat->clocks[2].arg[0]); + + ut_assertok(device_get_by_driver_info(plat->clocks[3].node, &clk)); + ut_asserteq_str("sandbox_clk", clk->name); + ut_asserteq(3, plat->clocks[3].arg[0]); + + ut_assertok(device_get_by_driver_info(plat->clocks[4].node, &clk)); + ut_asserteq_str("sandbox_clk", clk->name); + ut_asserteq(2, plat->clocks[4].arg[0]); + + return 0; +} +DM_TEST(dm_test_of_platdata_phandle, UT_TESTF_SCAN_PDATA);
We have a test in dtoc for this feature, but not one in U-Boot itself. Add a simple test that checks that the information comes through correctly.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/dts/sandbox.dtsi | 26 ++++++++++++++++++++++++ configs/sandbox_spl_defconfig | 1 + drivers/clk/clk_fixed_rate.c | 4 ++-- drivers/clk/clk_sandbox.c | 4 ++-- test/dm/of_platdata.c | 37 +++++++++++++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 4 deletions(-)
Applied to u-boot-dm, thanks!
At present we update the driver_info struct with a pointer to the device that it created (i.e. caused to be bound). This works fine when U-Boot SPL is stored in read-write memory. But on some platforms, such as Intel Apollo Lake, it is not possible to update the data memory.
In any case, it is bad form to put this information in a structure that is in the data region, since it expands the size of the binary.
Create a new driver_rt structure which holds runtime information about drivers. Update the code to store the device pointer in this instead. Also update the test check that this works.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device.c | 11 ++++++----- drivers/core/lists.c | 16 +++++++++++----- drivers/core/root.c | 11 +++++++++++ include/asm-generic/global_data.h | 13 +++++++++++++ include/dm/device-internal.h | 2 +- include/dm/platdata.h | 14 ++++++++++++-- test/dm/of_platdata.c | 19 ++++++++++--------- 7 files changed, 64 insertions(+), 22 deletions(-)
diff --git a/drivers/core/device.c b/drivers/core/device.c index aacc6adf02d..c3338efc4fa 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -249,7 +249,7 @@ int device_bind_ofnode(struct udevice *parent, const struct driver *drv, }
int device_bind_by_name(struct udevice *parent, bool pre_reloc_only, - struct driver_info *info, struct udevice **devp) + const struct driver_info *info, struct udevice **devp) { struct driver *drv; uint platdata_size = 0; @@ -269,9 +269,6 @@ int device_bind_by_name(struct udevice *parent, bool pre_reloc_only, platdata_size, devp); if (ret) return ret; -#if CONFIG_IS_ENABLED(OF_PLATDATA) - info->dev = *devp; -#endif
return ret; } @@ -742,9 +739,13 @@ int device_get_global_by_ofnode(ofnode ofnode, struct udevice **devp) 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 = info->dev; + dev = drt->dev; *devp = NULL;
return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp); diff --git a/drivers/core/lists.c b/drivers/core/lists.c index 5beba9181cc..2e6bd5006ce 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -56,14 +56,20 @@ int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only) struct driver_info *info = ll_entry_start(struct driver_info, driver_info); const int n_ents = ll_entry_count(struct driver_info, driver_info); - struct driver_info *entry; - struct udevice *dev; int result = 0; - int ret; + uint idx; + + for (idx = 0; idx < n_ents; idx++) { + const struct driver_info *entry = info + idx; + struct driver_rt *drt = gd_dm_driver_rt() + idx; + struct udevice *dev; + int ret;
- for (entry = info; entry != info + n_ents; entry++) { ret = device_bind_by_name(parent, pre_reloc_only, entry, &dev); - if (ret && ret != -EPERM) { + if (!ret) { + if (CONFIG_IS_ENABLED(OF_PLATDATA)) + drt->dev = dev; + } else if (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 de23161cff8..e8df5aebe84 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -185,6 +185,17 @@ int dm_scan_platdata(bool pre_reloc_only) { int ret;
+ if (CONFIG_IS_ENABLED(OF_PLATDATA)) { + struct driver_rt *dyn; + int n_ents; + + n_ents = ll_entry_count(struct driver_info, driver_info); + dyn = calloc(n_ents, sizeof(struct driver_rt)); + if (!dyn) + return -ENOMEM; + gd_set_dm_driver_rt(dyn); + } + ret = lists_bind_drivers(DM_ROOT_NON_CONST, pre_reloc_only); if (ret == -ENOENT) { dm_warn("Some drivers were not found\n"); diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index d6f562d90d4..7fc53e8deee 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -24,6 +24,8 @@ #include <membuff.h> #include <linux/list.h>
+struct driver_rt; + typedef struct global_data { struct bd_info *bd; unsigned long flags; @@ -67,6 +69,9 @@ typedef struct global_data { struct udevice *dm_root; /* Root instance for Driver Model */ struct udevice *dm_root_f; /* Pre-relocation root instance */ struct list_head uclass_root; /* Head of core tree */ +# if CONFIG_IS_ENABLED(OF_PLATDATA) + struct driver_rt *dm_driver_rt; /* Dynamic info about the driver */ +# endif #endif #ifdef CONFIG_TIMER struct udevice *timer; /* Timer instance for Driver Model */ @@ -157,6 +162,14 @@ typedef struct global_data { #define gd_set_of_root(_root) #endif
+#if CONFIG_IS_ENABLED(OF_PLATDATA) +#define gd_set_dm_driver_rt(dyn) gd->dm_driver_rt = dyn +#define gd_dm_driver_rt() gd->dm_driver_rt +#else +#define gd_set_dm_driver_rt(dyn) +#define gd_dm_driver_rt() NULL +#endif + /* * Global Data Flags */ diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index 5145fb4e145..294d6c18105 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -81,7 +81,7 @@ int device_bind_with_driver_data(struct udevice *parent, * @return 0 if OK, -ve on error */ int device_bind_by_name(struct udevice *parent, bool pre_reloc_only, - struct driver_info *info, struct udevice **devp); + const struct driver_info *info, struct udevice **devp);
/** * device_ofdata_to_platdata() - Read platform data for a device diff --git a/include/dm/platdata.h b/include/dm/platdata.h index 25479b03d22..2c3cc90c291 100644 --- a/include/dm/platdata.h +++ b/include/dm/platdata.h @@ -22,17 +22,27 @@ * @name: Driver name * @platdata: Driver-specific platform data * @platdata_size: Size of platform data structure - * @dev: Device created from this structure data */ struct driver_info { const char *name; const void *platdata; #if CONFIG_IS_ENABLED(OF_PLATDATA) uint platdata_size; - struct udevice *dev; #endif };
+/** + * driver_rt - runtime information set up by U-Boot + * + * There is one of these for every driver_info in the linker list, indexed by + * the driver_info idx value. + * + * @dev: Device created from this idx + */ +struct driver_rt { + struct udevice *dev; +}; + /** * NOTE: Avoid using these except in extreme circumstances, where device tree * is not feasible (e.g. serial driver in SPL where <8KB of SRAM is diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c index 57f903611a6..e827d45ffb7 100644 --- a/test/dm/of_platdata.c +++ b/test/dm/of_platdata.c @@ -109,16 +109,16 @@ static int find_driver_info(struct unit_test_state *uts, struct udevice *parent,
/* If not the root device, find the entry that caused it to be bound */ if (parent->parent) { - 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); - const struct driver_info *entry; int idx = -1; + int i;
- for (entry = info; entry != info + n_ents; entry++) { - if (entry->dev == parent) { - idx = entry - info; + for (i = 0; i < n_ents; i++) { + const struct driver_rt *drt = gd_dm_driver_rt() + i; + + if (drt->dev == parent) { + idx = i; found[idx] = true; break; } @@ -153,16 +153,17 @@ static int dm_test_of_platdata_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(entry->dev); + ut_assertnonnull(drt->dev); ut_assertok(device_get_by_driver_info(entry, &dev)); - ut_asserteq_ptr(dev, entry->dev); + ut_asserteq_ptr(dev, drt->dev); } else { - ut_assertnull(entry->dev); + ut_assertnull(drt->dev); ut_asserteq(-ENOENT, device_get_by_driver_info(entry, &dev)); }
At present we update the driver_info struct with a pointer to the device that it created (i.e. caused to be bound). This works fine when U-Boot SPL is stored in read-write memory. But on some platforms, such as Intel Apollo Lake, it is not possible to update the data memory.
In any case, it is bad form to put this information in a structure that is in the data region, since it expands the size of the binary.
Create a new driver_rt structure which holds runtime information about drivers. Update the code to store the device pointer in this instead. Also update the test check that this works.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device.c | 11 ++++++----- drivers/core/lists.c | 16 +++++++++++----- drivers/core/root.c | 11 +++++++++++ include/asm-generic/global_data.h | 13 +++++++++++++ include/dm/device-internal.h | 2 +- include/dm/platdata.h | 14 ++++++++++++-- test/dm/of_platdata.c | 19 ++++++++++--------- 7 files changed, 64 insertions(+), 22 deletions(-)
Applied to u-boot-dm, thanks!
There is no devicetree with of-platdata. Update a few uclasses to allow them to be built for sandbox_spl. Also drop the i2c-gpio from SPL to avoid build errors, since it does not support of-platdata.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/i2c/Makefile | 2 +- drivers/i2c/i2c-emul-uclass.c | 2 ++ drivers/rtc/rtc-uclass.c | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile index bd248cbf52b..b37198036c0 100644 --- a/drivers/i2c/Makefile +++ b/drivers/i2c/Makefile @@ -6,7 +6,7 @@ obj-$(CONFIG_DM_I2C) += i2c-uclass.o ifdef CONFIG_ACPIGEN obj-$(CONFIG_DM_I2C) += acpi_i2c.o endif -obj-$(CONFIG_DM_I2C_GPIO) += i2c-gpio.o +obj-$(CONFIG_$(SPL_)DM_I2C_GPIO) += i2c-gpio.o obj-$(CONFIG_$(SPL_)I2C_CROS_EC_TUNNEL) += cros_ec_tunnel.o obj-$(CONFIG_$(SPL_)I2C_CROS_EC_LDO) += cros_ec_ldo.o
diff --git a/drivers/i2c/i2c-emul-uclass.c b/drivers/i2c/i2c-emul-uclass.c index 1b70e140545..84b6a219d19 100644 --- a/drivers/i2c/i2c-emul-uclass.c +++ b/drivers/i2c/i2c-emul-uclass.c @@ -76,7 +76,9 @@ UCLASS_DRIVER(i2c_emul) = { UCLASS_DRIVER(i2c_emul_parent) = { .id = UCLASS_I2C_EMUL_PARENT, .name = "i2c_emul_parent", +#if !CONFIG_IS_ENABLED(OF_PLATDATA) .post_bind = dm_scan_fdt_dev, +#endif };
static const struct udevice_id i2c_emul_parent_ids[] = { diff --git a/drivers/rtc/rtc-uclass.c b/drivers/rtc/rtc-uclass.c index 8035f7fe9cc..b406bab32d1 100644 --- a/drivers/rtc/rtc-uclass.c +++ b/drivers/rtc/rtc-uclass.c @@ -174,5 +174,7 @@ int rtc_write32(struct udevice *dev, unsigned int reg, u32 value) UCLASS_DRIVER(rtc) = { .name = "rtc", .id = UCLASS_RTC, +#if !CONFIG_IS_ENABLED(OF_PLATDATA) .post_bind = dm_scan_fdt_dev, +#endif };
There is no devicetree with of-platdata. Update a few uclasses to allow them to be built for sandbox_spl. Also drop the i2c-gpio from SPL to avoid build errors, since it does not support of-platdata.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/i2c/Makefile | 2 +- drivers/i2c/i2c-emul-uclass.c | 2 ++ drivers/rtc/rtc-uclass.c | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-)
Applied to u-boot-dm, thanks!
At present of-platdata does not provide parent information. But this is useful for I2C devices, for example, since it allows them to determine which bus they are on.
Add support for setting the parent correctly, by storing the parent driver_info index in dtoc and reading this in lists_bind_drivers(). This needs multiple passes since we must process children after their parents already have been bound.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/lists.c | 54 ++++++++++++++++++++++++++++++++++++-- dts/Kconfig | 18 +++++++++++++ include/dm/platdata.h | 10 ++++++- tools/dtoc/dtb_platdata.py | 4 +++ tools/dtoc/test_dtoc.py | 33 +++++++++++++++++++++++ 5 files changed, 116 insertions(+), 3 deletions(-)
diff --git a/drivers/core/lists.c b/drivers/core/lists.c index 2e6bd5006ce..b23ee3030e5 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -51,21 +51,48 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id) return NULL; }
-int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only) +static int bind_drivers_pass(struct udevice *parent, bool pre_reloc_only) { 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 missing_parent = false; int result = 0; uint idx;
+ /* + * Do one iteration through the driver_info records. For of-platdata, + * bind only devices whose parent is already bound. If we find any + * device we can't bind, set missing_parent to true, which will cause + * this function to be called again. + */ for (idx = 0; idx < n_ents; idx++) { + struct udevice *par = parent; const struct driver_info *entry = info + idx; struct driver_rt *drt = gd_dm_driver_rt() + idx; struct udevice *dev; int ret;
- ret = device_bind_by_name(parent, pre_reloc_only, entry, &dev); + if (CONFIG_IS_ENABLED(OF_PLATDATA)) { + int parent_idx = driver_info_parent_id(entry); + + if (drt->dev) + continue; + + if (CONFIG_IS_ENABLED(OF_PLATDATA_PARENT) && + parent_idx != -1) { + struct driver_rt *parent_drt; + + parent_drt = gd_dm_driver_rt() + parent_idx; + if (!parent_drt->dev) { + missing_parent = true; + continue; + } + + par = parent_drt->dev; + } + } + ret = device_bind_by_name(par, pre_reloc_only, entry, &dev); if (!ret) { if (CONFIG_IS_ENABLED(OF_PLATDATA)) drt->dev = dev; @@ -76,6 +103,29 @@ int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only) } }
+ return result ? result : missing_parent ? -EAGAIN : 0; +} + +int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only) +{ + int result = 0; + int pass; + + /* + * 10 passes is 10 levels deep in the devicetree, which is plenty. If + * OF_PLATDATA_PARENT is not enabled, then bind_drivers_pass() will + * always succeed on the first pass. + */ + for (pass = 0; pass < 10; pass++) { + int ret; + + ret = bind_drivers_pass(parent, pre_reloc_only); + if (!ret) + break; + if (ret != -EAGAIN && !result) + result = ret; + } + return result; }
diff --git a/dts/Kconfig b/dts/Kconfig index 046a54a1736..616f8484280 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -355,6 +355,15 @@ config SPL_OF_PLATDATA compatible string, then adding platform data and U_BOOT_DEVICE declarations for each node. See of-plat.txt for more information.
+config SPL_OF_PLATDATA_PARENT + bool "Support parent information in devices" + depends on SPL_OF_PLATDATA + default y + help + Generally it is useful to be able to access the parent of a device + with of-platdata. To save space this can be disabled, but in that + case dev_get_parent() will always return NULL; + config TPL_OF_PLATDATA bool "Generate platform data for use in TPL" depends on TPL_OF_CONTROL @@ -376,6 +385,15 @@ config TPL_OF_PLATDATA compatible string, then adding platform data and U_BOOT_DEVICE declarations for each node. See of-plat.txt for more information.
+config TPL_OF_PLATDATA_PARENT + bool "Support parent information in devices" + depends on TPL_OF_PLATDATA + default y + help + Generally it is useful to be able to access the parent of a device + with of-platdata. To save space this can be disabled, but in that + case dev_get_parent() will always return NULL; + endmenu
config MKIMAGE_DTC_PATH diff --git a/include/dm/platdata.h b/include/dm/platdata.h index 2c3cc90c291..f800a866dda 100644 --- a/include/dm/platdata.h +++ b/include/dm/platdata.h @@ -22,15 +22,23 @@ * @name: Driver name * @platdata: Driver-specific platform data * @platdata_size: Size of platform data structure + * @parent_idx: Index of the parent driver_info structure */ struct driver_info { const char *name; const void *platdata; #if CONFIG_IS_ENABLED(OF_PLATDATA) - uint platdata_size; + unsigned short platdata_size; + short parent_idx; #endif };
+#if CONFIG_IS_ENABLED(OF_PLATDATA) +#define driver_info_parent_id(driver_info) driver_info->parent_idx +#else +#define driver_info_parent_id(driver_info) (-1) +#endif + /** * driver_rt - runtime information set up by U-Boot * diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index 31a9b3877ea..8832e6ebecb 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -662,6 +662,10 @@ class DtbPlatdata(object): self.buf('\t.name\t\t= "%s",\n' % struct_name) self.buf('\t.platdata\t= &%s%s,\n' % (VAL_PREFIX, var_name)) self.buf('\t.platdata_size\t= sizeof(%s%s),\n' % (VAL_PREFIX, var_name)) + idx = -1 + if node.parent and node.parent in self._valid_nodes: + idx = node.parent.idx + self.buf('\t.parent_idx\t= %d,\n' % idx) self.buf('};\n') self.buf('\n')
diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 8dcac91ee70..fee9853d034 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -216,6 +216,7 @@ U_BOOT_DEVICE(i2c_at_0) = { \t.name\t\t= "sandbox_i2c_test", \t.platdata\t= &dtv_i2c_at_0, \t.platdata_size\t= sizeof(dtv_i2c_at_0), +\t.parent_idx\t= -1, };
/* Node /i2c@0/pmic@9 index 1 */ @@ -227,6 +228,7 @@ U_BOOT_DEVICE(pmic_at_9) = { \t.name\t\t= "sandbox_pmic_test", \t.platdata\t= &dtv_pmic_at_9, \t.platdata_size\t= sizeof(dtv_pmic_at_9), +\t.parent_idx\t= 0, };
/* Node /spl-test index 2 */ @@ -246,6 +248,7 @@ U_BOOT_DEVICE(spl_test) = { \t.name\t\t= "sandbox_spl_test", \t.platdata\t= &dtv_spl_test, \t.platdata_size\t= sizeof(dtv_spl_test), +\t.parent_idx\t= -1, };
/* Node /spl-test2 index 3 */ @@ -264,6 +267,7 @@ U_BOOT_DEVICE(spl_test2) = { \t.name\t\t= "sandbox_spl_test", \t.platdata\t= &dtv_spl_test2, \t.platdata_size\t= sizeof(dtv_spl_test2), +\t.parent_idx\t= -1, };
/* Node /spl-test3 index 4 */ @@ -276,6 +280,7 @@ U_BOOT_DEVICE(spl_test3) = { \t.name\t\t= "sandbox_spl_test", \t.platdata\t= &dtv_spl_test3, \t.platdata_size\t= sizeof(dtv_spl_test3), +\t.parent_idx\t= -1, };
/* Node /spl-test4 index 5 */ @@ -285,6 +290,7 @@ U_BOOT_DEVICE(spl_test4) = { \t.name\t\t= "sandbox_spl_test_2", \t.platdata\t= &dtv_spl_test4, \t.platdata_size\t= sizeof(dtv_spl_test4), +\t.parent_idx\t= -1, };
''' + C_EMPTY_POPULATE_PHANDLE_DATA, data) @@ -318,6 +324,7 @@ U_BOOT_DEVICE(gpios_at_0) = { \t.name\t\t= "sandbox_gpio", \t.platdata\t= &dtv_gpios_at_0, \t.platdata_size\t= sizeof(dtv_gpios_at_0), +\t.parent_idx\t= -1, };
void dm_populate_phandle_data(void) { @@ -349,6 +356,7 @@ U_BOOT_DEVICE(spl_test) = { \t.name\t\t= "invalid", \t.platdata\t= &dtv_spl_test, \t.platdata_size\t= sizeof(dtv_spl_test), +\t.parent_idx\t= -1, };
void dm_populate_phandle_data(void) { @@ -383,6 +391,7 @@ U_BOOT_DEVICE(phandle2_target) = { \t.name\t\t= "target", \t.platdata\t= &dtv_phandle2_target, \t.platdata_size\t= sizeof(dtv_phandle2_target), +\t.parent_idx\t= -1, };
/* Node /phandle3-target index 1 */ @@ -393,6 +402,7 @@ U_BOOT_DEVICE(phandle3_target) = { \t.name\t\t= "target", \t.platdata\t= &dtv_phandle3_target, \t.platdata_size\t= sizeof(dtv_phandle3_target), +\t.parent_idx\t= -1, };
/* Node /phandle-target index 4 */ @@ -403,6 +413,7 @@ U_BOOT_DEVICE(phandle_target) = { \t.name\t\t= "target", \t.platdata\t= &dtv_phandle_target, \t.platdata_size\t= sizeof(dtv_phandle_target), +\t.parent_idx\t= -1, };
/* Node /phandle-source index 2 */ @@ -417,6 +428,7 @@ U_BOOT_DEVICE(phandle_source) = { \t.name\t\t= "source", \t.platdata\t= &dtv_phandle_source, \t.platdata_size\t= sizeof(dtv_phandle_source), +\t.parent_idx\t= -1, };
/* Node /phandle-source2 index 3 */ @@ -428,6 +440,7 @@ U_BOOT_DEVICE(phandle_source2) = { \t.name\t\t= "source", \t.platdata\t= &dtv_phandle_source2, \t.platdata_size\t= sizeof(dtv_phandle_source2), +\t.parent_idx\t= -1, };
void dm_populate_phandle_data(void) { @@ -470,6 +483,7 @@ U_BOOT_DEVICE(phandle_target) = { \t.name\t\t= "target", \t.platdata\t= &dtv_phandle_target, \t.platdata_size\t= sizeof(dtv_phandle_target), +\t.parent_idx\t= -1, };
/* Node /phandle-source2 index 0 */ @@ -481,6 +495,7 @@ U_BOOT_DEVICE(phandle_source2) = { \t.name\t\t= "source", \t.platdata\t= &dtv_phandle_source2, \t.platdata_size\t= sizeof(dtv_phandle_source2), +\t.parent_idx\t= -1, };
void dm_populate_phandle_data(void) { @@ -504,6 +519,7 @@ U_BOOT_DEVICE(phandle2_target) = { \t.name\t\t= "target", \t.platdata\t= &dtv_phandle2_target, \t.platdata_size\t= sizeof(dtv_phandle2_target), +\t.parent_idx\t= -1, };
/* Node /phandle3-target index 1 */ @@ -514,6 +530,7 @@ U_BOOT_DEVICE(phandle3_target) = { \t.name\t\t= "target", \t.platdata\t= &dtv_phandle3_target, \t.platdata_size\t= sizeof(dtv_phandle3_target), +\t.parent_idx\t= -1, };
/* Node /phandle-target index 4 */ @@ -524,6 +541,7 @@ U_BOOT_DEVICE(phandle_target) = { \t.name\t\t= "target", \t.platdata\t= &dtv_phandle_target, \t.platdata_size\t= sizeof(dtv_phandle_target), +\t.parent_idx\t= -1, };
/* Node /phandle-source index 2 */ @@ -538,6 +556,7 @@ U_BOOT_DEVICE(phandle_source) = { \t.name\t\t= "source", \t.platdata\t= &dtv_phandle_source, \t.platdata_size\t= sizeof(dtv_phandle_source), +\t.parent_idx\t= -1, };
/* Node /phandle-source2 index 3 */ @@ -549,6 +568,7 @@ U_BOOT_DEVICE(phandle_source2) = { \t.name\t\t= "source", \t.platdata\t= &dtv_phandle_source2, \t.platdata_size\t= sizeof(dtv_phandle_source2), +\t.parent_idx\t= -1, };
void dm_populate_phandle_data(void) { @@ -611,6 +631,7 @@ U_BOOT_DEVICE(test1) = { \t.name\t\t= "test1", \t.platdata\t= &dtv_test1, \t.platdata_size\t= sizeof(dtv_test1), +\t.parent_idx\t= -1, };
/* Node /test2 index 1 */ @@ -621,6 +642,7 @@ U_BOOT_DEVICE(test2) = { \t.name\t\t= "test2", \t.platdata\t= &dtv_test2, \t.platdata_size\t= sizeof(dtv_test2), +\t.parent_idx\t= -1, };
/* Node /test3 index 2 */ @@ -631,6 +653,7 @@ U_BOOT_DEVICE(test3) = { \t.name\t\t= "test3", \t.platdata\t= &dtv_test3, \t.platdata_size\t= sizeof(dtv_test3), +\t.parent_idx\t= -1, };
''' + C_EMPTY_POPULATE_PHANDLE_DATA, data) @@ -663,6 +686,7 @@ U_BOOT_DEVICE(test1) = { \t.name\t\t= "test1", \t.platdata\t= &dtv_test1, \t.platdata_size\t= sizeof(dtv_test1), +\t.parent_idx\t= -1, };
/* Node /test2 index 1 */ @@ -673,6 +697,7 @@ U_BOOT_DEVICE(test2) = { \t.name\t\t= "test2", \t.platdata\t= &dtv_test2, \t.platdata_size\t= sizeof(dtv_test2), +\t.parent_idx\t= -1, };
''' + C_EMPTY_POPULATE_PHANDLE_DATA, data) @@ -708,6 +733,7 @@ U_BOOT_DEVICE(test1) = { \t.name\t\t= "test1", \t.platdata\t= &dtv_test1, \t.platdata_size\t= sizeof(dtv_test1), +\t.parent_idx\t= -1, };
/* Node /test2 index 1 */ @@ -718,6 +744,7 @@ U_BOOT_DEVICE(test2) = { \t.name\t\t= "test2", \t.platdata\t= &dtv_test2, \t.platdata_size\t= sizeof(dtv_test2), +\t.parent_idx\t= -1, };
/* Node /test3 index 2 */ @@ -728,6 +755,7 @@ U_BOOT_DEVICE(test3) = { \t.name\t\t= "test3", \t.platdata\t= &dtv_test3, \t.platdata_size\t= sizeof(dtv_test3), +\t.parent_idx\t= -1, };
''' + C_EMPTY_POPULATE_PHANDLE_DATA, data) @@ -763,6 +791,7 @@ U_BOOT_DEVICE(test1) = { \t.name\t\t= "test1", \t.platdata\t= &dtv_test1, \t.platdata_size\t= sizeof(dtv_test1), +\t.parent_idx\t= -1, };
/* Node /test2 index 1 */ @@ -773,6 +802,7 @@ U_BOOT_DEVICE(test2) = { \t.name\t\t= "test2", \t.platdata\t= &dtv_test2, \t.platdata_size\t= sizeof(dtv_test2), +\t.parent_idx\t= -1, };
/* Node /test3 index 2 */ @@ -783,6 +813,7 @@ U_BOOT_DEVICE(test3) = { \t.name\t\t= "test3", \t.platdata\t= &dtv_test3, \t.platdata_size\t= sizeof(dtv_test3), +\t.parent_idx\t= -1, };
''' + C_EMPTY_POPULATE_PHANDLE_DATA, data) @@ -833,6 +864,7 @@ U_BOOT_DEVICE(spl_test) = { \t.name\t\t= "sandbox_spl_test", \t.platdata\t= &dtv_spl_test, \t.platdata_size\t= sizeof(dtv_spl_test), +\t.parent_idx\t= -1, };
/* Node /spl-test2 index 1 */ @@ -843,6 +875,7 @@ U_BOOT_DEVICE(spl_test2) = { \t.name\t\t= "sandbox_spl_test", \t.platdata\t= &dtv_spl_test2, \t.platdata_size\t= sizeof(dtv_spl_test2), +\t.parent_idx\t= -1, };
''' + C_EMPTY_POPULATE_PHANDLE_DATA, data)
At present of-platdata does not provide parent information. But this is useful for I2C devices, for example, since it allows them to determine which bus they are on.
Add support for setting the parent correctly, by storing the parent driver_info index in dtoc and reading this in lists_bind_drivers(). This needs multiple passes since we must process children after their parents already have been bound.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/lists.c | 54 ++++++++++++++++++++++++++++++++++++-- dts/Kconfig | 18 +++++++++++++ include/dm/platdata.h | 10 ++++++- tools/dtoc/dtb_platdata.py | 4 +++ tools/dtoc/test_dtoc.py | 33 +++++++++++++++++++++++ 5 files changed, 116 insertions(+), 3 deletions(-)
Applied to u-boot-dm, thanks!
Add a simple test that we can obtain the correct parent for an I2C device. This requires updating the driver names to match the compatible strings, adding them to the devicetree and enabling a few options.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/dts/sandbox.dts | 1 + arch/sandbox/dts/sandbox.dtsi | 1 + configs/sandbox_spl_defconfig | 4 +++- drivers/i2c/sandbox_i2c.c | 4 ++-- drivers/rtc/sandbox_rtc.c | 4 ++-- test/dm/of_platdata.c | 15 +++++++++++++++ 6 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts index 20f68938297..8b50a402898 100644 --- a/arch/sandbox/dts/sandbox.dts +++ b/arch/sandbox/dts/sandbox.dts @@ -69,6 +69,7 @@ clock-frequency = <400000>; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_i2c0>; + u-boot,dm-pre-reloc; };
pcic: pci@0 { diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi index ca7049fc974..2ec43b009bc 100644 --- a/arch/sandbox/dts/sandbox.dtsi +++ b/arch/sandbox/dts/sandbox.dtsi @@ -90,6 +90,7 @@ reg = <0x43>; compatible = "sandbox-rtc"; sandbox,emul = <&emul0>; + u-boot,dm-pre-reloc; }; sandbox_pmic: sandbox_pmic { reg = <0x40>; diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 91215d23a0f..0cf8cf76577 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -26,6 +26,8 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_HANDOFF=y CONFIG_SPL_BOARD_INIT=y CONFIG_SPL_ENV_SUPPORT=y +CONFIG_SPL_I2C_SUPPORT=y +CONFIG_SPL_RTC_SUPPORT=y CONFIG_CMD_CPU=y CONFIG_CMD_LICENSE=y CONFIG_CMD_BOOTZ=y @@ -120,7 +122,6 @@ CONFIG_I2C_CROS_EC_LDO=y CONFIG_DM_I2C_GPIO=y CONFIG_SYS_I2C_SANDBOX=y CONFIG_I2C_MUX=y -CONFIG_SPL_I2C_MUX=y CONFIG_I2C_ARB_GPIO_CHALLENGE=y CONFIG_CROS_EC_KEYB=y CONFIG_I8042_KEYB=y @@ -187,6 +188,7 @@ CONFIG_REMOTEPROC_SANDBOX=y CONFIG_DM_RESET=y CONFIG_SANDBOX_RESET=y CONFIG_DM_RTC=y +CONFIG_SPL_DM_RTC=y CONFIG_SANDBOX_SERIAL=y CONFIG_SOUND=y CONFIG_SOUND_SANDBOX=y diff --git a/drivers/i2c/sandbox_i2c.c b/drivers/i2c/sandbox_i2c.c index 57b1c60fde6..2cbdaf9cc73 100644 --- a/drivers/i2c/sandbox_i2c.c +++ b/drivers/i2c/sandbox_i2c.c @@ -93,8 +93,8 @@ static const struct udevice_id sandbox_i2c_ids[] = { { } };
-U_BOOT_DRIVER(i2c_sandbox) = { - .name = "i2c_sandbox", +U_BOOT_DRIVER(sandbox_i2c) = { + .name = "sandbox_i2c", .id = UCLASS_I2C, .of_match = sandbox_i2c_ids, .ops = &sandbox_i2c_ops, diff --git a/drivers/rtc/sandbox_rtc.c b/drivers/rtc/sandbox_rtc.c index 852770a49cf..d0864b1df97 100644 --- a/drivers/rtc/sandbox_rtc.c +++ b/drivers/rtc/sandbox_rtc.c @@ -92,8 +92,8 @@ static const struct udevice_id sandbox_rtc_ids[] = { { } };
-U_BOOT_DRIVER(rtc_sandbox) = { - .name = "rtc-sandbox", +U_BOOT_DRIVER(sandbox_rtc) = { + .name = "sandbox_rtc", .id = UCLASS_RTC, .of_match = sandbox_rtc_ids, .ops = &sandbox_rtc_ops, diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c index e827d45ffb7..bad733fbee0 100644 --- a/test/dm/of_platdata.c +++ b/test/dm/of_platdata.c @@ -205,3 +205,18 @@ static int dm_test_of_platdata_phandle(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_of_platdata_phandle, UT_TESTF_SCAN_PDATA); + +#if CONFIG_IS_ENABLED(OF_PLATDATA_PARENT) +/* Test that device parents are correctly set up */ +static int dm_test_of_platdata_parent(struct unit_test_state *uts) +{ + struct udevice *rtc, *i2c; + + 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; +} +DM_TEST(dm_test_of_platdata_parent, UT_TESTF_SCAN_PDATA); +#endif
Add a simple test that we can obtain the correct parent for an I2C device. This requires updating the driver names to match the compatible strings, adding them to the devicetree and enabling a few options.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/dts/sandbox.dts | 1 + arch/sandbox/dts/sandbox.dtsi | 1 + configs/sandbox_spl_defconfig | 4 +++- drivers/i2c/sandbox_i2c.c | 4 ++-- drivers/rtc/sandbox_rtc.c | 4 ++-- test/dm/of_platdata.c | 15 +++++++++++++++ 6 files changed, 24 insertions(+), 5 deletions(-)
Applied to u-boot-dm, thanks!
Convert a few conditions to use compile-time checks to reduce the number of build paths.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/root.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/core/root.c b/drivers/core/root.c index e8df5aebe84..5f10d7a39c7 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -50,7 +50,6 @@ void dm_fixup_for_gd_move(struct global_data *new_gd) } }
-#if defined(CONFIG_NEEDS_MANUAL_RELOC) void fix_drivers(void) { struct driver *drv = @@ -129,8 +128,6 @@ void fix_devices(void) } }
-#endif - int dm_init(bool of_live) { int ret; @@ -141,11 +138,11 @@ int dm_init(bool of_live) } INIT_LIST_HEAD(&DM_UCLASS_ROOT_NON_CONST);
-#if defined(CONFIG_NEEDS_MANUAL_RELOC) - fix_drivers(); - fix_uclass(); - fix_devices(); -#endif + if (IS_ENABLED(CONFIG_NEEDS_MANUAL_RELOC)) { + fix_drivers(); + fix_uclass(); + fix_devices(); + }
ret = device_bind_by_name(NULL, false, &root_info, &DM_ROOT_NON_CONST); if (ret) @@ -350,9 +347,8 @@ int dm_init_and_scan(bool pre_reloc_only) { int ret;
-#if CONFIG_IS_ENABLED(OF_PLATDATA) - dm_populate_phandle_data(); -#endif + if (CONFIG_IS_ENABLED(OF_PLATDATA)) + dm_populate_phandle_data();
ret = dm_init(CONFIG_IS_ENABLED(OF_LIVE)); if (ret) {
Convert a few conditions to use compile-time checks to reduce the number of build paths.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/root.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
Applied to u-boot-dm, thanks!
Hi Simon,
so 3. 10. 2020 v 19:35 odesÃlatel Simon Glass sjg@chromium.org napsal:
Convert a few conditions to use compile-time checks to reduce the number of build paths.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/root.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/core/root.c b/drivers/core/root.c index e8df5aebe84..5f10d7a39c7 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -50,7 +50,6 @@ void dm_fixup_for_gd_move(struct global_data *new_gd) } }
-#if defined(CONFIG_NEEDS_MANUAL_RELOC) void fix_drivers(void) { struct driver *drv = @@ -129,8 +128,6 @@ void fix_devices(void) } }
-#endif
int dm_init(bool of_live) { int ret; @@ -141,11 +138,11 @@ int dm_init(bool of_live) } INIT_LIST_HEAD(&DM_UCLASS_ROOT_NON_CONST);
-#if defined(CONFIG_NEEDS_MANUAL_RELOC)
fix_drivers();fix_uclass();fix_devices();-#endif
if (IS_ENABLED(CONFIG_NEEDS_MANUAL_RELOC)) {fix_drivers();fix_uclass();fix_devices();}
This change is causing a hang on the Microblaze system. CONFIG_NEEDS_MANUAL_RELOC is out of Kconfig that's why likely this condition is not handled properly.
U-Boot 2021.01-rc1-00211-g63677927bf26-dirty (Nov 04 2020 - 13:46:33 +0100)
Model: Xilinx MicroBlaze DRAM: 1 GiB Error binding driver 'axi_emac': -12 Error binding driver 'xlnx_gpio': -12 Error binding driver 'xlnx_gpio': -12 Error binding driver 'xlnx_gpio': -12 Error binding driver 'xlnx_gpio': -12 Error binding driver 'xlnx_gpio': -12 Error binding driver 'ns16550_serial': -12 Error binding driver 'gpio_restart': -12 Some drivers failed to bind Error binding driver 'simple_bus': -12 Some drivers failed to bind initcall sequence bffeec9c failed at call 85016230 (err=-12) ### ERROR ### Please RESET the board ###
Thanks, Michal
On Wed, Nov 04, 2020 at 01:55:50PM +0100, Michal Simek wrote:
Hi Simon,
so 3. 10. 2020 v 19:35 odesÃlatel Simon Glass sjg@chromium.org napsal:
Convert a few conditions to use compile-time checks to reduce the number of build paths.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/root.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/core/root.c b/drivers/core/root.c index e8df5aebe84..5f10d7a39c7 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -50,7 +50,6 @@ void dm_fixup_for_gd_move(struct global_data *new_gd) } }
-#if defined(CONFIG_NEEDS_MANUAL_RELOC) void fix_drivers(void) { struct driver *drv = @@ -129,8 +128,6 @@ void fix_devices(void) } }
-#endif
int dm_init(bool of_live) { int ret; @@ -141,11 +138,11 @@ int dm_init(bool of_live) } INIT_LIST_HEAD(&DM_UCLASS_ROOT_NON_CONST);
-#if defined(CONFIG_NEEDS_MANUAL_RELOC)
fix_drivers();fix_uclass();fix_devices();-#endif
if (IS_ENABLED(CONFIG_NEEDS_MANUAL_RELOC)) {fix_drivers();fix_uclass();fix_devices();}This change is causing a hang on the Microblaze system. CONFIG_NEEDS_MANUAL_RELOC is out of Kconfig that's why likely this condition is not handled properly.
Can you do the migration or should I? It looks like just adding to the top-level Kconfig and select'ing for microblaze/m68k.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hi,
On 04. 11. 20 14:59, Tom Rini wrote:
On Wed, Nov 04, 2020 at 01:55:50PM +0100, Michal Simek wrote:
Hi Simon,
so 3. 10. 2020 v 19:35 odesÃlatel Simon Glass sjg@chromium.org napsal:
Convert a few conditions to use compile-time checks to reduce the number of build paths.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/root.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/core/root.c b/drivers/core/root.c index e8df5aebe84..5f10d7a39c7 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -50,7 +50,6 @@ void dm_fixup_for_gd_move(struct global_data *new_gd) } }
-#if defined(CONFIG_NEEDS_MANUAL_RELOC) void fix_drivers(void) { struct driver *drv = @@ -129,8 +128,6 @@ void fix_devices(void) } }
-#endif - int dm_init(bool of_live) { int ret; @@ -141,11 +138,11 @@ int dm_init(bool of_live) } INIT_LIST_HEAD(&DM_UCLASS_ROOT_NON_CONST);
-#if defined(CONFIG_NEEDS_MANUAL_RELOC) - fix_drivers();
fix_uclass(); - fix_devices(); -#endif + if(IS_ENABLED(CONFIG_NEEDS_MANUAL_RELOC)) { + fix_drivers(); + fix_uclass(); + fix_devices(); + }
This change is causing a hang on the Microblaze system. CONFIG_NEEDS_MANUAL_RELOC is out of Kconfig that's why likely this condition is not handled properly.
Can you do the migration or should I? It looks like just adding to the top-level Kconfig and select'ing for microblaze/m68k.
Send a patch your way. Please review.
Thanks, Michal
At present, enabling CONFIG_APL_SPI_FLASH_BOOT does not build since SPI and SPI flash are not enabled for TPL. Add a condition to fix this and tidy up a build warning in the SPI-flash driver.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/cpu/apollolake/Kconfig | 2 ++ drivers/spi/ich.c | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/apollolake/Kconfig b/arch/x86/cpu/apollolake/Kconfig index 35a425cd1bc..c6c1350f4f0 100644 --- a/arch/x86/cpu/apollolake/Kconfig +++ b/arch/x86/cpu/apollolake/Kconfig @@ -85,6 +85,8 @@ config APL_SPI_FLASH_BOOT bool "Support booting with SPI-flash driver instead memory-mapped SPI" select TPL_SPI_FLASH_SUPPORT select TPL_SPI_SUPPORT + select TPL_DM_SPI + select TPL_DM_SPI_FLASH help This enables SPI and SPI flash in TPL. Without the this only available boot method is to use memory-mapped SPI. Since this is diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c index e1336b89c5a..a91cb785680 100644 --- a/drivers/spi/ich.c +++ b/drivers/spi/ich.c @@ -615,6 +615,7 @@ static int ich_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) return ret; }
+#if !CONFIG_IS_ENABLED(OF_PLATDATA) /** * ich_spi_get_basics() - Get basic information about the ICH device * @@ -657,6 +658,7 @@ static int ich_spi_get_basics(struct udevice *bus, bool can_probe,
return ret; } +#endif
/** * ich_get_mmap_bus() - Handle the get_mmap() method for a bus @@ -946,10 +948,10 @@ static int ich_spi_child_pre_probe(struct udevice *dev) static int ich_spi_ofdata_to_platdata(struct udevice *dev) { struct ich_spi_platdata *plat = dev_get_platdata(dev); - int ret;
#if !CONFIG_IS_ENABLED(OF_PLATDATA) struct ich_spi_priv *priv = dev_get_priv(dev); + int ret;
ret = ich_spi_get_basics(dev, true, &priv->pch, &plat->ich_version, &plat->mmio_base);
At present, enabling CONFIG_APL_SPI_FLASH_BOOT does not build since SPI and SPI flash are not enabled for TPL. Add a condition to fix this and tidy up a build warning in the SPI-flash driver.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/cpu/apollolake/Kconfig | 2 ++ drivers/spi/ich.c | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-)
Applied to u-boot-dm, thanks!
Now that parent devices are supported with of-platadata, we don't need the messy code to fix up the parent pointers and allocations on Apollo Lake. Put the code behind a condition.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/cpu/apollolake/spl.c | 3 ++- drivers/misc/p2sb-uclass.c | 27 ++++++++++++++------------- 2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/arch/x86/cpu/apollolake/spl.c b/arch/x86/cpu/apollolake/spl.c index 5a53831dc6a..089b37c59f8 100644 --- a/arch/x86/cpu/apollolake/spl.c +++ b/arch/x86/cpu/apollolake/spl.c @@ -90,7 +90,8 @@ static int apl_flash_probe(struct udevice *dev) */ static int apl_flash_bind(struct udevice *dev) { - if (CONFIG_IS_ENABLED(OF_PLATDATA)) { + if (CONFIG_IS_ENABLED(OF_PLATDATA) && + !CONFIG_IS_ENABLED(OF_PLATDATA_PARENT)) { struct dm_spi_slave_platdata *plat; struct udevice *spi; int ret; diff --git a/drivers/misc/p2sb-uclass.c b/drivers/misc/p2sb-uclass.c index b5219df46be..12abcff2da4 100644 --- a/drivers/misc/p2sb-uclass.c +++ b/drivers/misc/p2sb-uclass.c @@ -174,19 +174,20 @@ int p2sb_set_port_id(struct udevice *dev, int portid) if (!CONFIG_IS_ENABLED(OF_PLATDATA)) return -ENOSYS;
- uclass_find_first_device(UCLASS_P2SB, &ps2b); - if (!ps2b) - return -EDEADLK; - dev->parent = ps2b; - - /* - * We must allocate this, since when the device was bound it did not - * have a parent. - * TODO(sjg@chromium.org): Add a parent pointer to child devices in dtoc - */ - dev->parent_platdata = malloc(sizeof(*pplat)); - if (!dev->parent_platdata) - return -ENOMEM; + if (!CONFIG_IS_ENABLED(OF_PLATDATA_PARENT)) { + uclass_find_first_device(UCLASS_P2SB, &ps2b); + if (!ps2b) + return -EDEADLK; + dev->parent = ps2b; + + /* + * We must allocate this, since when the device was bound it did + * not have a parent. + */ + dev->parent_platdata = malloc(sizeof(*pplat)); + if (!dev->parent_platdata) + return -ENOMEM; + } pplat = dev_get_parent_platdata(dev); pplat->pid = portid;
Now that parent devices are supported with of-platadata, we don't need the messy code to fix up the parent pointers and allocations on Apollo Lake. Put the code behind a condition.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/cpu/apollolake/spl.c | 3 ++- drivers/misc/p2sb-uclass.c | 27 ++++++++++++++------------- 2 files changed, 16 insertions(+), 14 deletions(-)
Applied to u-boot-dm, thanks!
At present we use a 'node' pointer in the of-platadata phandle_n_arg structs. This is a pointer to the struct driver_info for a particular device, and we can use it to obtain the struct udevice pointer itself.
Since we don't know the struct udevice pointer until it is allocated in memory, we have to fix up the phandle_n_arg.node at runtime. This is annoying since it requires that SPL's data is writable and adds a small amount of extra (generated) code in the dm_populate_phandle_data() function.
Now that we can find a driver_info by its index, it is easier to put the index in the phandle_n_arg structures.
Update dtoc to do this, add a new device_get_by_driver_info_idx() to look up a device by drive_info index and update the tests to match.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/clk/clk-uclass.c | 3 +-- drivers/core/device.c | 11 +++++++++++ drivers/misc/irq-uclass.c | 2 +- drivers/mmc/fsl_esdhc_imx.c | 7 ++----- include/dm/device.h | 14 ++++++++++++++ include/dt-structs.h | 6 +++--- test/dm/of_platdata.c | 10 +++++----- test/dm/test-main.c | 24 ++++++++++++++++++++---- test/py/tests/test_spl.py | 5 ++--- tools/dtoc/dtb_platdata.py | 20 ++++---------------- tools/dtoc/test_dtoc.py | 33 +++++++++++---------------------- 11 files changed, 74 insertions(+), 61 deletions(-)
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 934cd5787a5..c56bc71e387 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -37,8 +37,7 @@ int clk_get_by_driver_info(struct udevice *dev, struct phandle_1_arg *cells, { int ret;
- ret = device_get_by_driver_info((struct driver_info *)cells->node, - &clk->dev); + ret = device_get_by_driver_info_idx(cells->idx, &clk->dev); if (ret) return ret; clk->id = cells->arg[0]; diff --git a/drivers/core/device.c b/drivers/core/device.c index c3338efc4fa..522d2ffe7e3 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -750,6 +750,17 @@ int device_get_by_driver_info(const struct driver_info *info,
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; + struct udevice *dev; + + dev = drt->dev; + *devp = NULL; + + return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp); +} #endif
int device_find_first_child(const struct udevice *parent, struct udevice **devp) diff --git a/drivers/misc/irq-uclass.c b/drivers/misc/irq-uclass.c index 94fa233f193..24b27962a7d 100644 --- a/drivers/misc/irq-uclass.c +++ b/drivers/misc/irq-uclass.c @@ -69,7 +69,7 @@ int irq_get_by_driver_info(struct udevice *dev, { int ret;
- ret = device_get_by_driver_info(cells->node, &irq->dev); + ret = device_get_by_driver_info_idx(cells->idx, &irq->dev); if (ret) return ret; irq->id = cells->arg[0]; diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index 0c866b168f9..ed04d1f964c 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -1511,12 +1511,9 @@ static int fsl_esdhc_probe(struct udevice *dev)
if (CONFIG_IS_ENABLED(DM_GPIO) && !priv->non_removable) { struct udevice *gpiodev; - struct driver_info *info; - - info = (struct driver_info *)dtplat->cd_gpios->node; - - ret = device_get_by_driver_info(info, &gpiodev);
+ ret = device_get_by_driver_info(dtplat->cd_gpios->idx, + &gpiodev); if (ret) return ret;
diff --git a/include/dm/device.h b/include/dm/device.h index ac3b6c1b8a0..c00bee1ccb0 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -553,6 +553,20 @@ int device_get_global_by_ofnode(ofnode node, struct udevice **devp); 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 + * + * Locates a device by its struct driver_info, by using its index number which + * is written into the idx field of struct phandle_1_arg, etc. + * + * The device is probed to activate it ready for use. + * + * @idx: Index number of the driver_info structure (0=first) + * @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_idx(uint idx, struct udevice **devp); + /** * device_find_first_child() - Find the first child of a device * diff --git a/include/dt-structs.h b/include/dt-structs.h index eed8273d18e..f0e1c9cb901 100644 --- a/include/dt-structs.h +++ b/include/dt-structs.h @@ -11,17 +11,17 @@ struct driver_info;
struct phandle_0_arg { - const struct driver_info *node; + uint idx; int arg[0]; };
struct phandle_1_arg { - const struct driver_info *node; + uint idx; int arg[1]; };
struct phandle_2_arg { - const struct driver_info *node; + uint idx; int arg[2]; }; #include <generated/dt-structs-gen.h> diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c index bad733fbee0..4f3cc159d03 100644 --- a/test/dm/of_platdata.c +++ b/test/dm/of_platdata.c @@ -183,22 +183,22 @@ static int dm_test_of_platdata_phandle(struct unit_test_state *uts) ut_asserteq_str("sandbox_clk_test", dev->name); plat = dev_get_platdata(dev);
- ut_assertok(device_get_by_driver_info(plat->clocks[0].node, &clk)); + ut_assertok(device_get_by_driver_info_idx(plat->clocks[0].idx, &clk)); ut_asserteq_str("fixed_clock", clk->name);
- ut_assertok(device_get_by_driver_info(plat->clocks[1].node, &clk)); + ut_assertok(device_get_by_driver_info_idx(plat->clocks[1].idx, &clk)); ut_asserteq_str("sandbox_clk", clk->name); ut_asserteq(1, plat->clocks[1].arg[0]);
- ut_assertok(device_get_by_driver_info(plat->clocks[2].node, &clk)); + ut_assertok(device_get_by_driver_info_idx(plat->clocks[2].idx, &clk)); ut_asserteq_str("sandbox_clk", clk->name); ut_asserteq(0, plat->clocks[2].arg[0]);
- ut_assertok(device_get_by_driver_info(plat->clocks[3].node, &clk)); + ut_assertok(device_get_by_driver_info_idx(plat->clocks[3].idx, &clk)); ut_asserteq_str("sandbox_clk", clk->name); ut_asserteq(3, plat->clocks[3].arg[0]);
- ut_assertok(device_get_by_driver_info(plat->clocks[4].node, &clk)); + ut_assertok(device_get_by_driver_info_idx(plat->clocks[4].idx, &clk)); ut_asserteq_str("sandbox_clk", clk->name); ut_asserteq(2, plat->clocks[4].arg[0]);
diff --git a/test/dm/test-main.c b/test/dm/test-main.c index 9d22df8c4dc..fd24635006c 100644 --- a/test/dm/test-main.c +++ b/test/dm/test-main.c @@ -127,6 +127,24 @@ static bool dm_test_run_on_flattree(struct unit_test *test) return !strstr(fname, "video") || strstr(test->name, "video_base"); }
+static bool test_matches(const char *test_name, const char *find_name) +{ + if (!find_name) + return true; + + if (!strcmp(test_name, find_name)) + return true; + + /* All tests have this prefix */ + if (!strncmp(test_name, "dm_test_", 8)) + test_name += 8; + + if (!strcmp(test_name, find_name)) + return true; + + return false; +} + int dm_test_main(const char *test_name) { struct unit_test *tests = ll_entry_start(struct unit_test, dm_test); @@ -152,6 +170,7 @@ int dm_test_main(const char *test_name)
if (!test_name) printf("Running %d driver model tests\n", n_ents); + else
found = 0; uts->of_root = gd_of_root(); @@ -159,10 +178,7 @@ int dm_test_main(const char *test_name) const char *name = test->name; int runs;
- /* All tests have this prefix */ - if (!strncmp(name, "dm_test_", 8)) - name += 8; - if (test_name && strcmp(test_name, name)) + if (!test_matches(name, test_name)) continue;
/* Run with the live tree if possible */ diff --git a/test/py/tests/test_spl.py b/test/py/tests/test_spl.py index 58a851e5ec8..d2b99027ce6 100644 --- a/test/py/tests/test_spl.py +++ b/test/py/tests/test_spl.py @@ -20,10 +20,9 @@ def test_spl(u_boot_console, ut_spl_subtest):
Args: u_boot_console (ConsoleBase): U-Boot console - ut_subtest (str): test to be executed via command ut, e.g 'foo bar' to - execute command 'ut foo bar' + ut_subtest (str): SPL test to be executed (e.g. 'dm platdata_phandle') """ cons = u_boot_console - cons.restart_uboot_with_flags(['-u', ut_spl_subtest]) + cons.restart_uboot_with_flags(['-u', '-k', ut_spl_subtest.split()[1]]) output = cons.get_spawn_output().replace('\r', '') assert 'Failures: 0' in output diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index 8832e6ebecb..2be11fff6c2 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -154,8 +154,6 @@ class DtbPlatdata(object): key: Driver alias declared with U_BOOT_DRIVER_ALIAS(driver_alias, driver_name) value: Driver name declared with U_BOOT_DRIVER(driver_name) - _links: List of links to be included in dm_populate_phandle_data(), - each a PhandleLink _drivers_additional: List of additional drivers to use during scanning """ def __init__(self, dtb_fname, include_disabled, warning_disabled, @@ -169,7 +167,6 @@ class DtbPlatdata(object): self._lines = [] self._drivers = [] self._driver_aliases = {} - self._links = [] self._drivers_additional = drivers_additional
def get_normalized_compat_name(self, node): @@ -612,17 +609,11 @@ class DtbPlatdata(object): name = conv_name_to_c(target_node.name) arg_values = [] for i in range(args): - arg_values.append(str(fdt_util.fdt32_to_cpu(prop.value[pos + 1 + i]))) + arg_values.append( + str(fdt_util.fdt32_to_cpu(prop.value[pos + 1 + i]))) pos += 1 + args - # node member is filled with NULL as the real value - # will be update at run-time during dm_init_and_scan() - # by dm_populate_phandle_data() - vals.append('\t{NULL, {%s}}' % (', '.join(arg_values))) - var_node = '%s%s.%s[%d].node' % \ - (VAL_PREFIX, var_name, member_name, item) - # Save the the link information to be use to define - # dm_populate_phandle_data() - self._links.append(PhandleLink(var_node, name)) + vals.append('\t{%d, {%s}}' % (target_node.idx, + ', '.join(arg_values))) item += 1 for val in vals: self.buf('\n\t\t%s,' % val) @@ -703,9 +694,6 @@ class DtbPlatdata(object): # nodes using DM_GET_DEVICE # dtv_dmc_at_xxx.clocks[0].node = DM_GET_DEVICE(clock_controller_at_xxx) self.buf('void dm_populate_phandle_data(void) {\n') - for link in self._links: - self.buf('\t%s = DM_GET_DEVICE(%s);\n' % - (link.var_node, link.dev_name)) self.buf('}\n')
self.out(''.join(self.get_buf())) diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index fee9853d034..8e16dc0f0fa 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -419,10 +419,10 @@ U_BOOT_DEVICE(phandle_target) = { /* Node /phandle-source index 2 */ static struct dtd_source dtv_phandle_source = { \t.clocks\t\t\t= { -\t\t\t{NULL, {}}, -\t\t\t{NULL, {11}}, -\t\t\t{NULL, {12, 13}}, -\t\t\t{NULL, {}},}, +\t\t\t{4, {}}, +\t\t\t{0, {11}}, +\t\t\t{1, {12, 13}}, +\t\t\t{4, {}},}, }; U_BOOT_DEVICE(phandle_source) = { \t.name\t\t= "source", @@ -434,7 +434,7 @@ U_BOOT_DEVICE(phandle_source) = { /* Node /phandle-source2 index 3 */ static struct dtd_source dtv_phandle_source2 = { \t.clocks\t\t\t= { -\t\t\t{NULL, {}},}, +\t\t\t{4, {}},}, }; U_BOOT_DEVICE(phandle_source2) = { \t.name\t\t= "source", @@ -444,11 +444,6 @@ U_BOOT_DEVICE(phandle_source2) = { };
void dm_populate_phandle_data(void) { -\tdtv_phandle_source.clocks[0].node = DM_GET_DEVICE(phandle_target); -\tdtv_phandle_source.clocks[1].node = DM_GET_DEVICE(phandle2_target); -\tdtv_phandle_source.clocks[2].node = DM_GET_DEVICE(phandle3_target); -\tdtv_phandle_source.clocks[3].node = DM_GET_DEVICE(phandle_target); -\tdtv_phandle_source2.clocks[0].node = DM_GET_DEVICE(phandle_target); } ''', data)
@@ -489,7 +484,7 @@ U_BOOT_DEVICE(phandle_target) = { /* Node /phandle-source2 index 0 */ static struct dtd_source dtv_phandle_source2 = { \t.clocks\t\t\t= { -\t\t\t{NULL, {}},}, +\t\t\t{1, {}},}, }; U_BOOT_DEVICE(phandle_source2) = { \t.name\t\t= "source", @@ -499,7 +494,6 @@ U_BOOT_DEVICE(phandle_source2) = { };
void dm_populate_phandle_data(void) { -\tdtv_phandle_source2.clocks[0].node = DM_GET_DEVICE(phandle_target); } ''', data)
@@ -547,10 +541,10 @@ U_BOOT_DEVICE(phandle_target) = { /* Node /phandle-source index 2 */ static struct dtd_source dtv_phandle_source = { \t.cd_gpios\t\t= { -\t\t\t{NULL, {}}, -\t\t\t{NULL, {11}}, -\t\t\t{NULL, {12, 13}}, -\t\t\t{NULL, {}},}, +\t\t\t{4, {}}, +\t\t\t{0, {11}}, +\t\t\t{1, {12, 13}}, +\t\t\t{4, {}},}, }; U_BOOT_DEVICE(phandle_source) = { \t.name\t\t= "source", @@ -562,7 +556,7 @@ U_BOOT_DEVICE(phandle_source) = { /* Node /phandle-source2 index 3 */ static struct dtd_source dtv_phandle_source2 = { \t.cd_gpios\t\t= { -\t\t\t{NULL, {}},}, +\t\t\t{4, {}},}, }; U_BOOT_DEVICE(phandle_source2) = { \t.name\t\t= "source", @@ -572,11 +566,6 @@ U_BOOT_DEVICE(phandle_source2) = { };
void dm_populate_phandle_data(void) { -\tdtv_phandle_source.cd_gpios[0].node = DM_GET_DEVICE(phandle_target); -\tdtv_phandle_source.cd_gpios[1].node = DM_GET_DEVICE(phandle2_target); -\tdtv_phandle_source.cd_gpios[2].node = DM_GET_DEVICE(phandle3_target); -\tdtv_phandle_source.cd_gpios[3].node = DM_GET_DEVICE(phandle_target); -\tdtv_phandle_source2.cd_gpios[0].node = DM_GET_DEVICE(phandle_target); } ''', data)
At present we use a 'node' pointer in the of-platadata phandle_n_arg structs. This is a pointer to the struct driver_info for a particular device, and we can use it to obtain the struct udevice pointer itself.
Since we don't know the struct udevice pointer until it is allocated in memory, we have to fix up the phandle_n_arg.node at runtime. This is annoying since it requires that SPL's data is writable and adds a small amount of extra (generated) code in the dm_populate_phandle_data() function.
Now that we can find a driver_info by its index, it is easier to put the index in the phandle_n_arg structures.
Update dtoc to do this, add a new device_get_by_driver_info_idx() to look up a device by drive_info index and update the tests to match.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/clk/clk-uclass.c | 3 +-- drivers/core/device.c | 11 +++++++++++ drivers/misc/irq-uclass.c | 2 +- drivers/mmc/fsl_esdhc_imx.c | 7 ++----- include/dm/device.h | 14 ++++++++++++++ include/dt-structs.h | 6 +++--- test/dm/of_platdata.c | 10 +++++----- test/dm/test-main.c | 24 ++++++++++++++++++++---- test/py/tests/test_spl.py | 5 ++--- tools/dtoc/dtb_platdata.py | 20 ++++---------------- tools/dtoc/test_dtoc.py | 33 +++++++++++---------------------- 11 files changed, 74 insertions(+), 61 deletions(-)
Applied to u-boot-dm, thanks!
With of-platdata, the devicetree is supposed to specify all the devices in the system. So far this hasn't really mattered since of-platdata still works correctly.
However, new of-platdata features rely on numbering the devices in a particular order so that they can be referenced by a single integer. It is tricky to implement this efficiently when other devices are present in the build.
To address this, disable use of U_BOOT_DEVICE() when of-platdata is enabled. This seems acceptable as it is not supposed to be used at all, except in SPL/TPL, where of-platdata is the recommended approach.
This breaks one non-compliant boards at present: mx6cuboxi
Signed-off-by: Simon Glass sjg@chromium.org ---
include/dm/platdata.h | 8 ++++++++ tools/dtoc/dtb_platdata.py | 3 +++ tools/dtoc/test_dtoc.py | 3 +++ 3 files changed, 14 insertions(+)
diff --git a/include/dm/platdata.h b/include/dm/platdata.h index f800a866dda..216efa8ef77 100644 --- a/include/dm/platdata.h +++ b/include/dm/platdata.h @@ -55,9 +55,17 @@ struct driver_rt { * NOTE: Avoid using these except in extreme circumstances, where device tree * is not feasible (e.g. serial driver in SPL where <8KB of SRAM is * available). U-Boot's driver model uses device tree for configuration. + * + * When of-platdata is in use, U_BOOT_DEVICE() cannot be used outside of the + * dt-platdata.c file created by dtoc */ +#if CONFIG_IS_ENABLED(OF_PLATDATA) && !defined(DT_PLATDATA_C) +#define U_BOOT_DEVICE(__name) _Static_assert(false, \ + "Cannot use U_BOOT_DEVICE with of-platdata. Please use devicetree instead") +#else #define U_BOOT_DEVICE(__name) \ ll_entry_declare(struct driver_info, __name, driver_info) +#endif
/* Declare a list of devices. The argument is a driver_info[] array */ #define U_BOOT_DEVICES(__name) \ diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index 2be11fff6c2..9b27aecc140 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -673,6 +673,9 @@ class DtbPlatdata(object): information. """ self.out_header() + self.out('/* Allow use of U_BOOT_DEVICE() in this file */\n') + self.out('#define DT_PLATDATA_C\n') + self.out('\n') self.out('#include <common.h>\n') self.out('#include <dm.h>\n') self.out('#include <dt-structs.h>\n') diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 8e16dc0f0fa..a5836e04b7a 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -44,6 +44,9 @@ C_HEADER = '''/* * This file was generated by dtoc from a .dtb (device tree binary) file. */
+/* Allow use of U_BOOT_DEVICE() in this file */ +#define DT_PLATDATA_C + #include <common.h> #include <dm.h> #include <dt-structs.h>
With of-platdata, the devicetree is supposed to specify all the devices in the system. So far this hasn't really mattered since of-platdata still works correctly.
However, new of-platdata features rely on numbering the devices in a particular order so that they can be referenced by a single integer. It is tricky to implement this efficiently when other devices are present in the build.
To address this, disable use of U_BOOT_DEVICE() when of-platdata is enabled. This seems acceptable as it is not supposed to be used at all, except in SPL/TPL, where of-platdata is the recommended approach.
This breaks one non-compliant boards at present: mx6cuboxi
Signed-off-by: Simon Glass sjg@chromium.org ---
include/dm/platdata.h | 8 ++++++++ tools/dtoc/dtb_platdata.py | 3 +++ tools/dtoc/test_dtoc.py | 3 +++ 3 files changed, 14 insertions(+)
Applied to u-boot-dm, thanks!
Update this to incorporate the parent feature as well as other recent developments.
Signed-off-by: Simon Glass sjg@chromium.org ---
doc/driver-model/of-plat.rst | 42 ++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 23 deletions(-)
diff --git a/doc/driver-model/of-plat.rst b/doc/driver-model/of-plat.rst index 1e3fad137be..58481665cec 100644 --- a/doc/driver-model/of-plat.rst +++ b/doc/driver-model/of-plat.rst @@ -66,12 +66,6 @@ strictly necessary. Notable problems include: normally also supports device tree it must use #ifdef to separate out this code, since the structures are only available in SPL.
- - Correct relations between nodes are not implemented. This means that - parent/child relations (like bus device iteration) do not work yet. - Some phandles (those that are recognised as such) are converted into - a pointer to struct driver_info. This pointer can be used to access - the referenced device. -
How it works ------------ @@ -134,10 +128,14 @@ the following C struct declaration: fdt32_t vmmc_supply; };
-and the following device declaration: +and the following device declarations:
.. code-block:: c
+ /* Node /clock-controller@ff760000 index 0 */ + ... + + /* Node /dwmmc@ff0c0000 index 2 */ static struct dtd_rockchip_rk3288_dw_mshc dtv_dwmmc_at_ff0c0000 = { .fifo_depth = 0x100, .cap_sd_highspeed = true, @@ -145,10 +143,10 @@ and the following device declaration: .clock_freq_min_max = {0x61a80, 0x8f0d180}, .vmmc_supply = 0xb, .num_slots = 0x1, - .clocks = {{NULL, 456}, - {NULL, 68}, - {NULL, 114}, - {NULL, 118}}, + .clocks = {{0, 456}, + {0, 68}, + {0, 114}, + {0, 118}}, .cap_mmc_highspeed = true, .disable_wp = true, .bus_width = 0x4, @@ -161,13 +159,10 @@ and the following device declaration: .name = "rockchip_rk3288_dw_mshc", .platdata = &dtv_dwmmc_at_ff0c0000, .platdata_size = sizeof(dtv_dwmmc_at_ff0c0000), + .parent_idx = -1, };
void dm_populate_phandle_data(void) { - dtv_dwmmc_at_ff0c0000.clocks[0].node = DM_GET_DEVICE(clock_controller_at_ff760000); - dtv_dwmmc_at_ff0c0000.clocks[1].node = DM_GET_DEVICE(clock_controller_at_ff760000); - dtv_dwmmc_at_ff0c0000.clocks[2].node = DM_GET_DEVICE(clock_controller_at_ff760000); - dtv_dwmmc_at_ff0c0000.clocks[3].node = DM_GET_DEVICE(clock_controller_at_ff760000); }
The device is then instantiated at run-time and the platform data can be @@ -193,6 +188,13 @@ In order to make this a bit more flexible U_BOOT_DRIVER_ALIAS macro can be used to declare an alias for a driver name, typically a 'compatible' string. This macro produces no code, but it is by dtoc tool.
+The parent_idx is the index of the parent driver_info structure within its +linker list (instantiated by the U_BOOT_DEVICE() macro). This is used to support +dev_get_parent(). The dm_populate_phandle_data() is included to allow for +fix-ups required by dtoc. It is not currently used. The values in 'clocks' are +the index of the driver_info for the target device followed by any phandle +arguments. This is used to support device_get_by_driver_info_idx(). + During the build process dtoc parses both U_BOOT_DRIVER and U_BOOT_DRIVER_ALIAS to build a list of valid driver names and driver aliases. If the 'compatible' string used for a device does not not match a valid driver name, it will be @@ -339,12 +341,7 @@ spl/dt-platdata.c. It additionally contains the definition of dm_populate_phandle_data() which is responsible of filling the phandle information by adding references to U_BOOT_DEVICE by using DM_GET_DEVICE
-The beginnings of a libfdt Python module are provided. So far this only -implements a subset of the features. - -The 'swig' tool is needed to build the libfdt Python module. If this is not -found then the Python model is not used and a fallback is used instead, which -makes use of fdtget. +The pylibfdt Python module is used to access the devicetree.
Credits @@ -357,11 +354,10 @@ Future work ----------- - Consider programmatically reading binding files instead of device tree contents -- Complete the phandle feature -- Move to using a full Python libfdt module
.. Simon Glass sjg@chromium.org .. Google, Inc .. 6/6/16 .. Updated Independence Day 2016 +.. Updated 1st October 2020
Update this to incorporate the parent feature as well as other recent developments.
Signed-off-by: Simon Glass sjg@chromium.org ---
doc/driver-model/of-plat.rst | 42 ++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 23 deletions(-)
Applied to u-boot-dm, thanks!
participants (4)
-
Michal Simek -
Michal Simek -
Simon Glass -
Tom Rini