[PATCH 0/9] dm: core: Support collecting and reporting stats
Driver model can use a lot of memory, as it is the core of all drivers and devices in U-Boot. Add a command to show how much is in use, along with the sizes of various data structures.
This patch can be used to analyse the impact of various potential changes to driver model for SPL, none of which has been implemented. Most involve shrinking the size of struct udevice, which is a particular problem on 64-bit machines since their pointers are so unnecessarily large in SPL.
To try this out, enable SPL_DM_STATS and then build and run on your board. You should see output for SPL and U-Boot proper, like this:
Struct sizes: udevice 90, driver 78, uclass 30, uc_driver 78 Memory: device 11:990, device names 111, uclass a:1e0
Attached type Count Size Cur Tags Save --------------- ----- ----- ----- ----- ----- plat 3 e0 990 914 7c (124) parent_plat 2 40 990 910 80 (128) uclass_plat 1 10 990 90c 84 (132) priv 6 13d 990 920 70 (112) parent_priv 0 0 990 908 88 (136) uclass_priv 3 38 990 914 7c (124) driver_data 0 0 990 908 88 (136) uclass 0 0 Attached total f 2a5 37c (892) tags 0 0
Total size: e15 (3605)
With tags: a99 (2713) - singly-linked: 901 (2305) - driver index: 88a (2186) - uclass index: 813 (2067) Drop device name (not SRAM): 111 (273)
Simon Glass (9): dm: core: Rename dm_dump_all() dm: core: Sort dm subcommands dm: core: Fix addresses in the dm static command dm: core: Add documentation for the dm command dm: core: Switch the testbus driver to use a new struct dm: core: Support accessing core tags dm: core: Add a way to collect memory usage dm: core: Add a command to show driver model statistics dm: spl: Allow SPL to show memory usage
cmd/dm.c | 69 ++++-- common/spl/spl.c | 9 + doc/usage/cmd/dm.rst | 487 ++++++++++++++++++++++++++++++++++++++++ doc/usage/index.rst | 1 + drivers/core/Kconfig | 21 ++ drivers/core/device.c | 65 ++++++ drivers/core/dump.c | 79 ++++++- drivers/core/root.c | 53 +++++ drivers/core/tag.c | 29 +++ drivers/misc/test_drv.c | 6 +- include/dm/device.h | 25 +++ include/dm/root.h | 45 ++++ include/dm/tag.h | 32 ++- include/dm/test.h | 7 + include/dm/util.h | 11 +- test/dm/core.c | 91 ++++++++ tools/dtoc/test_dtoc.py | 6 +- 17 files changed, 1004 insertions(+), 32 deletions(-) create mode 100644 doc/usage/cmd/dm.rst
This is not a good name anymore as it does not dump everything. Rename it to dm_dump_tree() to avoid confusion.
Signed-off-by: Simon Glass sjg@chromium.org ---
cmd/dm.c | 8 ++++---- drivers/core/dump.c | 2 +- include/dm/util.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/cmd/dm.c b/cmd/dm.c index 1dd19fe45b5..cb4c358e942 100644 --- a/cmd/dm.c +++ b/cmd/dm.c @@ -16,10 +16,10 @@ #include <dm/root.h> #include <dm/util.h>
-static int do_dm_dump_all(struct cmd_tbl *cmdtp, int flag, int argc, - char *const argv[]) +static int do_dm_dump_tree(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) { - dm_dump_all(); + dm_dump_tree();
return 0; } @@ -65,7 +65,7 @@ static int do_dm_dump_static_driver_info(struct cmd_tbl *cmdtp, int flag, int ar }
static struct cmd_tbl test_commands[] = { - U_BOOT_CMD_MKENT(tree, 0, 1, do_dm_dump_all, "", ""), + U_BOOT_CMD_MKENT(tree, 0, 1, do_dm_dump_tree, "", ""), U_BOOT_CMD_MKENT(uclass, 1, 1, do_dm_dump_uclass, "", ""), U_BOOT_CMD_MKENT(devres, 1, 1, do_dm_dump_devres, "", ""), U_BOOT_CMD_MKENT(drivers, 1, 1, do_dm_dump_drivers, "", ""), diff --git a/drivers/core/dump.c b/drivers/core/dump.c index f2f9cacc56c..606883ac941 100644 --- a/drivers/core/dump.c +++ b/drivers/core/dump.c @@ -45,7 +45,7 @@ static void show_devices(struct udevice *dev, int depth, int last_flag) } }
-void dm_dump_all(void) +void dm_dump_tree(void) { struct udevice *root;
diff --git a/include/dm/util.h b/include/dm/util.h index 4428f045b72..c52daa87ef3 100644 --- a/include/dm/util.h +++ b/include/dm/util.h @@ -25,7 +25,7 @@ struct list_head; int list_count_items(struct list_head *head);
/* Dump out a tree of all devices */ -void dm_dump_all(void); +void dm_dump_tree(void);
/* Dump out a list of uclasses and their devices */ void dm_dump_uclass(void);
This is not a good name anymore as it does not dump everything. Rename it to dm_dump_tree() to avoid confusion.
Signed-off-by: Simon Glass sjg@chromium.org ---
cmd/dm.c | 8 ++++---- drivers/core/dump.c | 2 +- include/dm/util.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-)
Applied to u-boot-dm, thanks!
Put these in alphabetic order, both in the help and in the implementation, as there are quite a few subcommands now. Tweak the help for 'dm tree' to better explain what it does.
Signed-off-by: Simon Glass sjg@chromium.org ---
cmd/dm.c | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/cmd/dm.c b/cmd/dm.c index cb4c358e942..4c0a8a53ced 100644 --- a/cmd/dm.c +++ b/cmd/dm.c @@ -16,18 +16,10 @@ #include <dm/root.h> #include <dm/util.h>
-static int do_dm_dump_tree(struct cmd_tbl *cmdtp, int flag, int argc, - char *const argv[]) -{ - dm_dump_tree(); - - return 0; -} - -static int do_dm_dump_uclass(struct cmd_tbl *cmdtp, int flag, int argc, - char *const argv[]) +static int do_dm_dump_driver_compat(struct cmd_tbl *cmdtp, int flag, int argc, + char * const argv[]) { - dm_dump_uclass(); + dm_dump_driver_compat();
return 0; } @@ -48,29 +40,37 @@ static int do_dm_dump_drivers(struct cmd_tbl *cmdtp, int flag, int argc, return 0; }
-static int do_dm_dump_driver_compat(struct cmd_tbl *cmdtp, int flag, int argc, - char * const argv[]) +static int do_dm_dump_static_driver_info(struct cmd_tbl *cmdtp, int flag, + int argc, char * const argv[]) { - dm_dump_driver_compat(); + dm_dump_static_driver_info();
return 0; }
-static int do_dm_dump_static_driver_info(struct cmd_tbl *cmdtp, int flag, int argc, - char * const argv[]) +static int do_dm_dump_tree(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) { - dm_dump_static_driver_info(); + dm_dump_tree(); + + return 0; +} + +static int do_dm_dump_uclass(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + dm_dump_uclass();
return 0; }
static struct cmd_tbl test_commands[] = { - U_BOOT_CMD_MKENT(tree, 0, 1, do_dm_dump_tree, "", ""), - U_BOOT_CMD_MKENT(uclass, 1, 1, do_dm_dump_uclass, "", ""), + U_BOOT_CMD_MKENT(compat, 1, 1, do_dm_dump_driver_compat, "", ""), U_BOOT_CMD_MKENT(devres, 1, 1, do_dm_dump_devres, "", ""), U_BOOT_CMD_MKENT(drivers, 1, 1, do_dm_dump_drivers, "", ""), - U_BOOT_CMD_MKENT(compat, 1, 1, do_dm_dump_driver_compat, "", ""), U_BOOT_CMD_MKENT(static, 1, 1, do_dm_dump_static_driver_info, "", ""), + U_BOOT_CMD_MKENT(tree, 0, 1, do_dm_dump_tree, "", ""), + U_BOOT_CMD_MKENT(uclass, 1, 1, do_dm_dump_uclass, "", ""), };
static __maybe_unused void dm_reloc(void) @@ -109,10 +109,10 @@ static int do_dm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) U_BOOT_CMD( dm, 3, 1, do_dm, "Driver model low level access", - "tree Dump driver model tree ('*' = activated)\n" - "dm uclass Dump list of instances for each uclass\n" + "compat Dump list of drivers with compatibility strings\n" "dm devres Dump list of device resources for each device\n" "dm drivers Dump list of drivers with uclass and instances\n" - "dm compat Dump list of drivers with compatibility strings\n" - "dm static Dump list of drivers with static platform data" + "dm static Dump list of drivers with static platform data\n" + "dm tree Dump tree of driver model devices ('*' = activated)\n" + "dm uclass Dump list of instances for each uclass" );
Put these in alphabetic order, both in the help and in the implementation, as there are quite a few subcommands now. Tweak the help for 'dm tree' to better explain what it does.
Signed-off-by: Simon Glass sjg@chromium.org ---
cmd/dm.c | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-)
Applied to u-boot-dm, thanks!
This command converts pointers to addresses, but the pointers being converted are in the image's rodata region. For sandbox this means it is not in DRAM so it does not make sense to do this conversion.
Fix this by showing a simple pointer instead. Drop the unnecessary @ and hex prefixes.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/dump.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/core/dump.c b/drivers/core/dump.c index 606883ac941..ce679e26290 100644 --- a/drivers/core/dump.c +++ b/drivers/core/dump.c @@ -171,8 +171,6 @@ void dm_dump_static_driver_info(void)
puts("Driver Address\n"); puts("---------------------------------\n"); - for (entry = drv; entry != drv + n_ents; entry++) { - printf("%-25.25s @%08lx\n", entry->name, - (ulong)map_to_sysmem(entry->plat)); - } + for (entry = drv; entry != drv + n_ents; entry++) + printf("%-25.25s %p\n", entry->name, entry->plat); }
This command converts pointers to addresses, but the pointers being converted are in the image's rodata region. For sandbox this means it is not in DRAM so it does not make sense to do this conversion.
Fix this by showing a simple pointer instead. Drop the unnecessary @ and hex prefixes.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/dump.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
Applied to u-boot-dm, thanks!
Add a description and examples for the dm subcommands.
Signed-off-by: Simon Glass sjg@chromium.org ---
doc/usage/cmd/dm.rst | 487 +++++++++++++++++++++++++++++++++++++++++++ doc/usage/index.rst | 1 + 2 files changed, 488 insertions(+) create mode 100644 doc/usage/cmd/dm.rst
diff --git a/doc/usage/cmd/dm.rst b/doc/usage/cmd/dm.rst new file mode 100644 index 00000000000..7bc1962a754 --- /dev/null +++ b/doc/usage/cmd/dm.rst @@ -0,0 +1,487 @@ +.. SPDX-License-Identifier: GPL-2.0+: + +dm command +========== + +Synopis +------- + +:: + + dm compat + dm devres + dm drivers + dm static + dm tree + dm uclass + +Description +----------- + +The *dm* command allows viewing information about driver model, including the +tree of devices and list of available uclasses. + + +dm compat +~~~~~~~~~ + +This shows the compatible strings associated with each driver. Often there +is only one, but multiple strings are shown on their own line. These strings +can be looked up in the device tree files for each board, to see which driver is +used for each node. + +dm devres +~~~~~~~~~ + +This shows a list of a `devres` (device resource) records for a device. Some +drivers use the devres API to allocate memory, so that it can be freed +automatically (without any code needed in the driver's remove() method) when the +device is removed. + +This feature is controlled by CONFIG_DEVRES so no useful output is obtained if +this option is disabled. + +dm drivers +~~~~~~~~~~ + +This shows all the available drivers, their uclass and a list of devices that +use that driver, each on its own line. Drivers with no devices are shown with +`<none>` as the driver name. + + +dm mem +~~~~~~ + +This subcommand is really just for debugging and exploration. It can be enabled +with the `CONFIG_DM_STATS` option. + +All output is in hex except that in brackets which is decimal. + +The output consists of a header shows the size of the main device model +structures (struct udevice, struct driver, struct uclass and struct uc_driver) +and the count and memory used by each (number of devices, memory used by +devices, memory used by device names, number of uclasses, memory used by +uclasses). + +After that is a table of information about each type of data that can be +attached to a device, showing the number that have non-null data for that type, +the total size of all that data, the amount of memory used in total, the +amount that would be used if this type uses tags instead and the amount that +would be thus saved. + +The `driver_data` line shows the number of devices which have non-NULL driver +data. + +The `tags` line shows the number of tags and the memory used by those. + +At the bottom is an indication of the total memory usage obtained by undertaking +various changes, none of which is currently implemented in U-Boot: + +With tags + Using tags instead of all attached types + +Singly linked + Using a singly linked list + +driver index + Using a driver index instead of a pointer + +uclass index + Using a uclass index instead of a pointer + +Drop device name + Using empty device names + + +dm static +~~~~~~~~~ + +This shows devices bound by platform data, i.e. not from the device tree. There +are normally none of these, but some boards may use static devices for space +reasons. + + +dm tree +~~~~~~~ + +This shows the full tree of devices including the following fields: + +uclass + Shows the name of the uclass for the device + +Index + Shows the index number of the device, within the uclass. This shows the + ordering within the uclass, but not the sequence number. + +Probed + Shows `+` if the device is active + +Driver + Shows the name of the driver that this device uses + +Name + Shows the device name as well as the tree structure, since child devices are + shown attached to their parent. + + +dm uclass +~~~~~~~~~ + +This shows each uclass along with a list of devices in that uclass. The uclass +ID is shown (e.g. uclass 7) and its name. + +For each device, the format is:: + + n name @ a, seq s + +where `n` is the index within the uclass, `a` is the address of the device in +memory and `s` is the sequence number of the device. + + +Examples +-------- + +dm compat +~~~~~~~~~ + +This example shows an abridged version of the sandbox output:: + + => dm compat + Driver Compatible + -------------------------------- + act8846_reg + sandbox_adder sandbox,adder + axi_sandbox_bus sandbox,axi + blk_partition + bootcount-rtc u-boot,bootcount-rtc + ... + rockchip_rk805 rockchip,rk805 + rockchip,rk808 + rockchip,rk809 + rockchip,rk816 + rockchip,rk817 + rockchip,rk818 + root_driver + rtc-rv8803 microcrystal,rv8803 + epson,rx8803 + epson,rx8900 + ... + wdt_gpio linux,wdt-gpio + wdt_sandbox sandbox,wdt + + +dm devres +~~~~~~~~~ + +This example shows an abridged version of the sandbox test output (running +U-Boot with the -T flag):: + + => dm devres + - root_driver + - demo_shape_drv + - demo_simple_drv + - demo_shape_drv + ... + - h-test + - devres-test + 00000000130194e0 (100 byte) devm_kmalloc_release BIND + - another-test + ... + - syscon@3 + - a-mux-controller + 0000000013025e60 (96 byte) devm_kmalloc_release PROBE + 0000000013025f00 (24 byte) devm_kmalloc_release PROBE + 0000000013026010 (24 byte) devm_kmalloc_release PROBE + 0000000013026070 (24 byte) devm_kmalloc_release PROBE + 00000000130260d0 (24 byte) devm_kmalloc_release PROBE + - syscon@3 + - a-mux-controller + 0000000013026150 (96 byte) devm_kmalloc_release PROBE + 00000000130261f0 (24 byte) devm_kmalloc_release PROBE + 0000000013026300 (24 byte) devm_kmalloc_release PROBE + 0000000013026360 (24 byte) devm_kmalloc_release PROBE + 00000000130263c0 (24 byte) devm_kmalloc_release PROBE + - emul-mux-controller + 0000000013025fa0 (32 byte) devm_kmalloc_release PROBE + - testfdtm0 + - testfdtm1 + ... + - pinmux_spi0_pins + - pinmux_uart0_pins + - pinctrl-single-bits + 0000000013229180 (320 byte) devm_kmalloc_release PROBE + 0000000013229300 (40 byte) devm_kmalloc_release PROBE + 0000000013229370 (160 byte) devm_kmalloc_release PROBE + 000000001322c190 (40 byte) devm_kmalloc_release PROBE + 000000001322c200 (32 byte) devm_kmalloc_release PROBE + - pinmux_i2c0_pins + ... + - reg@0 + - reg@1 + + +dm drivers +~~~~~~~~~~ + +This example shows an abridged version of the sandbox output:: + + => dm drivers + Driver uid uclass Devices + ---------------------------------------------------------- + act8846_reg 087 regulator <none> + sandbox_adder 021 axi adder + adder + axi_sandbox_bus 021 axi axi@0 + ... + da7219 061 misc <none> + demo_shape_drv 001 demo demo_shape_drv + demo_shape_drv + demo_shape_drv + demo_simple_drv 001 demo demo_simple_drv + demo_simple_drv + testfdt_drv 003 testfdt a-test + b-test + d-test + e-test + f-test + g-test + another-test + chosen-test + testbus_drv 005 testbus some-bus + mmio-bus@0 + mmio-bus@1 + dsa-port 039 ethernet lan0 + lan1 + dsa_sandbox 035 dsa dsa-test + eep_sandbox 121 w1_eeprom <none> + ... + pfuze100_regulator 087 regulator <none> + phy_sandbox 077 phy bind-test-child1 + gen_phy@0 + gen_phy@1 + gen_phy@2 + pinconfig 078 pinconfig gpios + gpio0 + gpio1 + gpio2 + gpio3 + i2c + groups + pins + i2s + spi + cs + pinmux_pwm_pins + pinmux_spi0_pins + pinmux_uart0_pins + pinmux_i2c0_pins + pinmux_lcd_pins + pmc_sandbox 017 power-mgr pci@1e,0 + act8846 pmic 080 pmic <none> + max77686_pmic 080 pmic <none> + mc34708_pmic 080 pmic pmic@41 + ... + wdt_gpio 122 watchdog gpio-wdt + wdt_sandbox 122 watchdog wdt@0 + => + + +dm mem +~~~~~~ + +This example shows the sandbox output:: + + > dm mem + Struct sizes: udevice b0, driver 80, uclass 30, uc_driver 78 + Memory: device fe:aea0, device names a16, uclass 5e:11a0 + + Attached type Count Size Cur Tags Save + --------------- ----- ----- ----- ----- ----- + plat 45 a8f aea0 a7c4 6dc (1756) + parent_plat 1a 3b8 aea0 a718 788 (1928) + uclass_plat 3d 6b4 aea0 a7a4 6fc (1788) + priv 8a 68f3 aea0 a8d8 5c8 (1480) + parent_priv 8 38a0 aea0 a6d0 7d0 (2000) + uclass_priv 4e 14a6 aea0 a7e8 6b8 (1720) + driver_data f 0 aea0 a6ec 7b4 (1972) + uclass 6 20 + Attached total 191 cb54 3164 (12644) + tags 0 0 + + Total size: 18b94 (101268) + + With tags: 15a30 (88624) + - singly-linked: 14260 (82528) + - driver index: 13b6e (80750) + - uclass index: 1347c (78972) + Drop device name (not SRAM): a16 (2582) + => + + +dm static +~~~~~~~~~ + +This example shows the sandbox output:: + + => dm static + Driver Address + --------------------------------- + demo_shape_drv 0000562edab8dca0 + demo_simple_drv 0000562edab8dca0 + demo_shape_drv 0000562edab8dc90 + demo_simple_drv 0000562edab8dc80 + demo_shape_drv 0000562edab8dc80 + test_drv 0000562edaae8840 + test_drv 0000562edaae8848 + test_drv 0000562edaae8850 + sandbox_gpio 0000000000000000 + mod_exp_sw 0000000000000000 + sandbox_test_proc 0000562edabb5330 + qfw_sandbox 0000000000000000 + sandbox_timer 0000000000000000 + sandbox_serial 0000562edaa8ed00 + sysreset_sandbox 0000000000000000 + + +dm tree +------- + +This example shows the abridged sandbox output:: + + => dm tree + Class Index Probed Driver Name + ----------------------------------------------------------- + root 0 [ + ] root_driver root_driver + demo 0 [ ] demo_shape_drv |-- demo_shape_drv + demo 1 [ ] demo_simple_drv |-- demo_simple_drv + demo 2 [ ] demo_shape_drv |-- demo_shape_drv + demo 3 [ ] demo_simple_drv |-- demo_simple_drv + demo 4 [ ] demo_shape_drv |-- demo_shape_drv + test 0 [ ] test_drv |-- test_drv + test 1 [ ] test_drv |-- test_drv + test 2 [ ] test_drv |-- test_drv + .. + sysreset 0 [ ] sysreset_sandbox |-- sysreset_sandbox + bootstd 0 [ ] bootstd_drv |-- bootstd + bootmeth 0 [ ] bootmeth_distro | |-- syslinux + bootmeth 1 [ ] bootmeth_efi | `-- efi + reboot-mod 0 [ ] reboot-mode-gpio |-- reboot-mode0 + reboot-mod 1 [ ] reboot-mode-rtc |-- reboot-mode@14 + ... + ethernet 7 [ + ] dsa-port | `-- lan1 + pinctrl 0 [ + ] sandbox_pinctrl_gpio |-- pinctrl-gpio + gpio 1 [ + ] sandbox_gpio | |-- base-gpios + nop 0 [ + ] gpio_hog | | |-- hog_input_active_low + nop 1 [ + ] gpio_hog | | |-- hog_input_active_high + nop 2 [ + ] gpio_hog | | |-- hog_output_low + nop 3 [ + ] gpio_hog | | `-- hog_output_high + gpio 2 [ ] sandbox_gpio | |-- extra-gpios + gpio 3 [ ] sandbox_gpio | `-- pinmux-gpios + i2c 0 [ + ] sandbox_i2c |-- i2c@0 + i2c_eeprom 0 [ ] i2c_eeprom | |-- eeprom@2c + i2c_eeprom 1 [ ] i2c_eeprom_partition | | `-- bootcount@10 + rtc 0 [ ] sandbox_rtc | |-- rtc@43 + rtc 1 [ + ] sandbox_rtc | |-- rtc@61 + i2c_emul_p 0 [ + ] sandbox_i2c_emul_par | |-- emul + i2c_emul 0 [ ] sandbox_i2c_eeprom_e | | |-- emul-eeprom + i2c_emul 1 [ ] sandbox_i2c_rtc_emul | | |-- emul0 + i2c_emul 2 [ + ] sandbox_i2c_rtc_emul | | |-- emull + i2c_emul 3 [ ] sandbox_i2c_pmic_emu | | |-- pmic-emul0 + i2c_emul 4 [ ] sandbox_i2c_pmic_emu | | `-- pmic-emul1 + pmic 0 [ ] sandbox_pmic | |-- sandbox_pmic + regulator 0 [ ] sandbox_buck | | |-- buck1 + regulator 1 [ ] sandbox_buck | | |-- buck2 + regulator 2 [ ] sandbox_ldo | | |-- ldo1 + regulator 3 [ ] sandbox_ldo | | |-- ldo2 + regulator 4 [ ] sandbox_buck | | `-- no_match_by_nodename + pmic 1 [ ] mc34708_pmic | `-- pmic@41 + bootcount 0 [ + ] bootcount-rtc |-- bootcount@0 + bootcount 1 [ ] bootcount-i2c-eeprom |-- bootcount + ... + clk 4 [ ] fixed_clock |-- osc + firmware 0 [ ] sandbox_firmware |-- sandbox-firmware + scmi_agent 0 [ ] sandbox-scmi_agent `-- scmi + clk 5 [ ] scmi_clk |-- protocol@14 + reset 2 [ ] scmi_reset_domain |-- protocol@16 + nop 8 [ ] scmi_voltage_domain `-- regulators + regulator 5 [ ] scmi_regulator |-- reg@0 + regulator 6 [ ] scmi_regulator `-- reg@1 + => + + +dm uclass +~~~~~~~~~ + +This example shows the abridged sandbox output:: + + => dm uclass + uclass 0: root + 0 * root_driver @ 03015460, seq 0 + + uclass 1: demo + 0 demo_shape_drv @ 03015560, seq 0 + 1 demo_simple_drv @ 03015620, seq 1 + 2 demo_shape_drv @ 030156e0, seq 2 + 3 demo_simple_drv @ 030157a0, seq 3 + 4 demo_shape_drv @ 03015860, seq 4 + + uclass 2: test + 0 test_drv @ 03015980, seq 0 + 1 test_drv @ 03015a60, seq 1 + 2 test_drv @ 03015b40, seq 2 + ... + uclass 20: audio-codec + 0 audio-codec @ 030168e0, seq 0 + + uclass 21: axi + 0 adder @ 0301db60, seq 1 + 1 adder @ 0301dc40, seq 2 + 2 axi@0 @ 030217d0, seq 0 + + uclass 22: blk + 0 mmc2.blk @ 0301ca00, seq 0 + 1 mmc1.blk @ 0301cee0, seq 1 + 2 mmc0.blk @ 0301d380, seq 2 + + uclass 23: bootcount + 0 * bootcount@0 @ 0301b3f0, seq 0 + 1 bootcount @ 0301b4b0, seq 1 + 2 bootcount_4@0 @ 0301b570, seq 2 + 3 bootcount_2@0 @ 0301b630, seq 3 + + uclass 24: bootdev + 0 mmc2.bootdev @ 0301cbb0, seq 0 + 1 mmc1.bootdev @ 0301d050, seq 1 + 2 mmc0.bootdev @ 0301d4f0, seq 2 + + ... + uclass 78: pinconfig + 0 gpios @ 03022410, seq 0 + 1 gpio0 @ 030224d0, seq 1 + 2 gpio1 @ 03022590, seq 2 + 3 gpio2 @ 03022650, seq 3 + 4 gpio3 @ 03022710, seq 4 + 5 i2c @ 030227d0, seq 5 + 6 groups @ 03022890, seq 6 + 7 pins @ 03022950, seq 7 + 8 i2s @ 03022a10, seq 8 + 9 spi @ 03022ad0, seq 9 + 10 cs @ 03022b90, seq 10 + 11 pinmux_pwm_pins @ 03022e10, seq 11 + 12 pinmux_spi0_pins @ 03022ed0, seq 12 + 13 pinmux_uart0_pins @ 03022f90, seq 13 + 14 * pinmux_i2c0_pins @ 03023130, seq 14 + 15 * pinmux_lcd_pins @ 030231f0, seq 15 + + ... + uclass 119: virtio + 0 sandbox_virtio1 @ 030220d0, seq 0 + 1 sandbox_virtio2 @ 03022190, seq 1 + + uclass 120: w1 + uclass 121: w1_eeprom + uclass 122: watchdog + 0 * gpio-wdt @ 0301c070, seq 0 + 1 * wdt@0 @ 03021710, seq 1 + + => diff --git a/doc/usage/index.rst b/doc/usage/index.rst index c03f4aef9eb..d3255275c77 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -32,6 +32,7 @@ Shell commands cmd/button cmd/cbsysinfo cmd/conitrace + cmd/dm cmd/echo cmd/env cmd/event
Add a description and examples for the dm subcommands.
Signed-off-by: Simon Glass sjg@chromium.org ---
doc/usage/cmd/dm.rst | 487 +++++++++++++++++++++++++++++++++++++++++++ doc/usage/index.rst | 1 + 2 files changed, 488 insertions(+) create mode 100644 doc/usage/cmd/dm.rst
Applied to u-boot-dm, thanks!
At present this driver uses 'priv' struct to hold 'plat' data, which is confusing. The contents of the strct don't matter, since only dtoc is using it. Create a new struct with the correct name.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/misc/test_drv.c | 2 +- include/dm/test.h | 7 +++++++ tools/dtoc/test_dtoc.py | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/test_drv.c b/drivers/misc/test_drv.c index 5d72982f258..b6df1189032 100644 --- a/drivers/misc/test_drv.c +++ b/drivers/misc/test_drv.c @@ -109,7 +109,7 @@ UCLASS_DRIVER(testbus) = { .child_post_probe = testbus_child_post_probe_uclass,
/* This is for dtoc testing only */ - .per_device_plat_auto = sizeof(struct dm_test_uclass_priv), + .per_device_plat_auto = sizeof(struct dm_test_uclass_plat), };
static int testfdt_drv_ping(struct udevice *dev, int pingval, int *pingret) diff --git a/include/dm/test.h b/include/dm/test.h index 4919064cc02..b5937509212 100644 --- a/include/dm/test.h +++ b/include/dm/test.h @@ -92,6 +92,13 @@ struct dm_test_uclass_priv { int total_add; };
+/** + * struct dm_test_uclass_plat - private plat data for test uclass + */ +struct dm_test_uclass_plat { + char dummy[32]; +}; + /** * struct dm_test_parent_data - parent's information on each child * diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index c81bcc9c32f..8bac2076214 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -616,7 +616,7 @@ struct dm_test_pdata __attribute__ ((section (".priv_data"))) u8 _denx_u_boot_test_bus_priv_some_bus[sizeof(struct dm_test_priv)] \t__attribute__ ((section (".priv_data"))); #include <dm/test.h> -u8 _denx_u_boot_test_bus_ucplat_some_bus[sizeof(struct dm_test_uclass_priv)] +u8 _denx_u_boot_test_bus_ucplat_some_bus[sizeof(struct dm_test_uclass_plat)] \t__attribute__ ((section (".priv_data"))); #include <test.h>
At present this driver uses 'priv' struct to hold 'plat' data, which is confusing. The contents of the strct don't matter, since only dtoc is using it. Create a new struct with the correct name.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/misc/test_drv.c | 2 +- include/dm/test.h | 7 +++++++ tools/dtoc/test_dtoc.py | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
At present tag numbers are only allocated for non-core data, meaning that the 'core' data, like priv and plat, are accessed through dedicated functions.
For debugging and consistency it is convenient to use tags for this 'core' data too. Add support for this, with new tag numbers and functions to access the pointer and size for each.
Update one of the test drivers so that the uclass-private data can be tested here.
There is some code duplication with functions like device_alloc_priv() but this is not addressed for now. At some point, some rationalisation may help to reduce code size, but more thought it needed on that.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device.c | 65 +++++++++++++++++++++++++++++++++ drivers/misc/test_drv.c | 4 ++- include/dm/device.h | 25 +++++++++++++ include/dm/tag.h | 13 ++++++- test/dm/core.c | 80 +++++++++++++++++++++++++++++++++++++++++ tools/dtoc/test_dtoc.py | 4 +++ 6 files changed, 189 insertions(+), 2 deletions(-)
diff --git a/drivers/core/device.c b/drivers/core/device.c index 3ab2583df38..d7936a46732 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -680,6 +680,71 @@ void *dev_get_parent_priv(const struct udevice *dev) return dm_priv_to_rw(dev->parent_priv_); }
+void *dev_get_attach_ptr(const struct udevice *dev, enum dm_tag_t tag) +{ + switch (tag) { + case DM_TAG_PLAT: + return dev_get_plat(dev); + case DM_TAG_PARENT_PLAT: + return dev_get_parent_plat(dev); + case DM_TAG_UC_PLAT: + return dev_get_uclass_plat(dev); + case DM_TAG_PRIV: + return dev_get_priv(dev); + case DM_TAG_PARENT_PRIV: + return dev_get_parent_priv(dev); + case DM_TAG_UC_PRIV: + return dev_get_uclass_priv(dev); + default: + return NULL; + } +} + +int dev_get_attach_size(const struct udevice *dev, enum dm_tag_t tag) +{ + const struct udevice *parent = dev_get_parent(dev); + const struct uclass *uc = dev->uclass; + const struct uclass_driver *uc_drv = uc->uc_drv; + const struct driver *parent_drv = NULL; + int size = 0; + + if (parent) + parent_drv = parent->driver; + + switch (tag) { + case DM_TAG_PLAT: + size = dev->driver->plat_auto; + break; + case DM_TAG_PARENT_PLAT: + if (parent) { + size = parent_drv->per_child_plat_auto; + if (!size) + size = parent->uclass->uc_drv->per_child_plat_auto; + } + break; + case DM_TAG_UC_PLAT: + size = uc_drv->per_device_plat_auto; + break; + case DM_TAG_PRIV: + size = dev->driver->priv_auto; + break; + case DM_TAG_PARENT_PRIV: + if (parent) { + size = parent_drv->per_child_auto; + if (!size) + size = parent->uclass->uc_drv->per_child_auto; + } + break; + case DM_TAG_UC_PRIV: + size = uc_drv->per_device_auto; + break; + default: + break; + } + + return size; +} + static int device_get_device_tail(struct udevice *dev, int ret, struct udevice **devp) { diff --git a/drivers/misc/test_drv.c b/drivers/misc/test_drv.c index b6df1189032..927618256f0 100644 --- a/drivers/misc/test_drv.c +++ b/drivers/misc/test_drv.c @@ -108,7 +108,9 @@ UCLASS_DRIVER(testbus) = { .child_pre_probe = testbus_child_pre_probe_uclass, .child_post_probe = testbus_child_post_probe_uclass,
- /* This is for dtoc testing only */ + .per_device_auto = sizeof(struct dm_test_uclass_priv), + + /* Note: this is for dtoc testing as well as tags*/ .per_device_plat_auto = sizeof(struct dm_test_uclass_plat), };
diff --git a/include/dm/device.h b/include/dm/device.h index 5bdb10653f8..12c6ba37ff3 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -11,6 +11,7 @@ #define _DM_DEVICE_H
#include <dm/ofnode.h> +#include <dm/tag.h> #include <dm/uclass-id.h> #include <fdtdec.h> #include <linker_lists.h> @@ -546,6 +547,30 @@ void *dev_get_parent_priv(const struct udevice *dev); */ void *dev_get_uclass_priv(const struct udevice *dev);
+/** + * dev_get_attach_ptr() - Get the value of an attached pointed tag + * + * The tag is assumed to hold a pointer, if it exists + * + * @dev: Device to look at + * @tag: Tag to access + * @return value of tag, or NULL if there is no tag of this type + */ +void *dev_get_attach_ptr(const struct udevice *dev, enum dm_tag_t tag); + +/** + * dev_get_attach_size() - Get the size of an attached tag + * + * Core tags have an automatic-allocation mechanism where the allocated size is + * defined by the device, parent or uclass. This returns the size associated + * with a particular tag + * + * @dev: Device to look at + * @tag: Tag to access + * @return size of auto-allocated data, 0 if none + */ +int dev_get_attach_size(const struct udevice *dev, enum dm_tag_t tag); + /** * dev_get_parent() - Get the parent of a device * diff --git a/include/dm/tag.h b/include/dm/tag.h index 54fc31eb153..9cb5d68f0a3 100644 --- a/include/dm/tag.h +++ b/include/dm/tag.h @@ -13,8 +13,19 @@ struct udevice;
enum dm_tag_t { + /* Types of core tags that can be attached to devices */ + DM_TAG_PLAT, + DM_TAG_PARENT_PLAT, + DM_TAG_UC_PLAT, + + DM_TAG_PRIV, + DM_TAG_PARENT_PRIV, + DM_TAG_UC_PRIV, + DM_TAG_DRIVER_DATA, + DM_TAG_ATTACH_COUNT, + /* EFI_LOADER */ - DM_TAG_EFI = 0, + DM_TAG_EFI = DM_TAG_ATTACH_COUNT,
DM_TAG_COUNT, }; diff --git a/test/dm/core.c b/test/dm/core.c index ebd504427d1..26e2fd56619 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -1275,3 +1275,83 @@ static int dm_test_uclass_find_device(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_uclass_find_device, UT_TESTF_SCAN_FDT); + +/* Test getting information about tags attached to devices */ +static int dm_test_dev_get_attach(struct unit_test_state *uts) +{ + struct udevice *dev; + + ut_assertok(uclass_first_device_err(UCLASS_TEST_FDT, &dev)); + ut_asserteq_str("a-test", dev->name); + + ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PLAT)); + ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PRIV)); + ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_UC_PRIV)); + ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_UC_PLAT)); + ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PLAT)); + ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PRIV)); + + ut_asserteq(sizeof(struct dm_test_pdata), + dev_get_attach_size(dev, DM_TAG_PLAT)); + ut_asserteq(sizeof(struct dm_test_priv), + dev_get_attach_size(dev, DM_TAG_PRIV)); + ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_UC_PRIV)); + ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_UC_PLAT)); + ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PLAT)); + ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PRIV)); + + return 0; +} +DM_TEST(dm_test_dev_get_attach, UT_TESTF_SCAN_FDT); + +/* Test getting information about tags attached to bus devices */ +static int dm_test_dev_get_attach_bus(struct unit_test_state *uts) +{ + struct udevice *dev, *child; + + ut_assertok(uclass_first_device_err(UCLASS_TEST_BUS, &dev)); + ut_asserteq_str("some-bus", dev->name); + + ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PLAT)); + ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PRIV)); + ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_UC_PRIV)); + ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_UC_PLAT)); + ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PLAT)); + ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PRIV)); + + ut_asserteq(sizeof(struct dm_test_pdata), + dev_get_attach_size(dev, DM_TAG_PLAT)); + ut_asserteq(sizeof(struct dm_test_priv), + dev_get_attach_size(dev, DM_TAG_PRIV)); + ut_asserteq(sizeof(struct dm_test_uclass_priv), + dev_get_attach_size(dev, DM_TAG_UC_PRIV)); + ut_asserteq(sizeof(struct dm_test_uclass_plat), + dev_get_attach_size(dev, DM_TAG_UC_PLAT)); + ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PLAT)); + ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PRIV)); + + /* Now try the child of the bus */ + ut_assertok(device_first_child_err(dev, &child)); + ut_asserteq_str("c-test@5", child->name); + + ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PLAT)); + ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PRIV)); + ut_assertnull(dev_get_attach_ptr(child, DM_TAG_UC_PRIV)); + ut_assertnull(dev_get_attach_ptr(child, DM_TAG_UC_PLAT)); + ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PARENT_PLAT)); + ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PARENT_PRIV)); + + ut_asserteq(sizeof(struct dm_test_pdata), + dev_get_attach_size(child, DM_TAG_PLAT)); + ut_asserteq(sizeof(struct dm_test_priv), + dev_get_attach_size(child, DM_TAG_PRIV)); + ut_asserteq(0, dev_get_attach_size(child, DM_TAG_UC_PRIV)); + ut_asserteq(0, dev_get_attach_size(child, DM_TAG_UC_PLAT)); + ut_asserteq(sizeof(struct dm_test_parent_plat), + dev_get_attach_size(child, DM_TAG_PARENT_PLAT)); + ut_asserteq(sizeof(struct dm_test_parent_data), + dev_get_attach_size(child, DM_TAG_PARENT_PRIV)); + + return 0; +} +DM_TEST(dm_test_dev_get_attach_bus, UT_TESTF_SCAN_FDT); diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 8bac2076214..879ca2ab2bf 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -618,6 +618,9 @@ u8 _denx_u_boot_test_bus_priv_some_bus[sizeof(struct dm_test_priv)] #include <dm/test.h> u8 _denx_u_boot_test_bus_ucplat_some_bus[sizeof(struct dm_test_uclass_plat)] \t__attribute__ ((section (".priv_data"))); +#include <dm/test.h> +u8 _denx_u_boot_test_bus_uc_priv_some_bus[sizeof(struct dm_test_uclass_priv)] + __attribute__ ((section (".priv_data"))); #include <test.h>
DM_DEVICE_INST(some_bus) = { @@ -628,6 +631,7 @@ DM_DEVICE_INST(some_bus) = { \t.driver_data\t= DM_TEST_TYPE_FIRST, \t.priv_\t\t= _denx_u_boot_test_bus_priv_some_bus, \t.uclass\t\t= DM_UCLASS_REF(testbus), +\t.uclass_priv_ = _denx_u_boot_test_bus_uc_priv_some_bus, \t.uclass_node\t= { \t\t.prev = &DM_UCLASS_REF(testbus)->dev_head, \t\t.next = &DM_UCLASS_REF(testbus)->dev_head,
Hi Simon,
On Sun, May 08, 2022 at 04:39:24AM -0600, Simon Glass wrote:
At present tag numbers are only allocated for non-core data, meaning that the 'core' data, like priv and plat, are accessed through dedicated functions.
For debugging and consistency it is convenient to use tags for this 'core' data too. Add support for this, with new tag numbers and functions to access the pointer and size for each.
Update one of the test drivers so that the uclass-private data can be tested here.
There is some code duplication with functions like device_alloc_priv() but this is not addressed for now. At some point, some rationalisation may help to reduce code size, but more thought it needed on that.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/device.c | 65 +++++++++++++++++++++++++++++++++ drivers/misc/test_drv.c | 4 ++- include/dm/device.h | 25 +++++++++++++ include/dm/tag.h | 13 ++++++- test/dm/core.c | 80 +++++++++++++++++++++++++++++++++++++++++ tools/dtoc/test_dtoc.py | 4 +++ 6 files changed, 189 insertions(+), 2 deletions(-)
diff --git a/drivers/core/device.c b/drivers/core/device.c index 3ab2583df38..d7936a46732 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -680,6 +680,71 @@ void *dev_get_parent_priv(const struct udevice *dev) return dm_priv_to_rw(dev->parent_priv_); }
+void *dev_get_attach_ptr(const struct udevice *dev, enum dm_tag_t tag)
Why not (enhance and) re-use dev_tag_get_ptr()? Both functions, at least, share the syntax and even the semantics from user's viewpoint.
+{
- switch (tag) {
- case DM_TAG_PLAT:
return dev_get_plat(dev);- case DM_TAG_PARENT_PLAT:
return dev_get_parent_plat(dev);- case DM_TAG_UC_PLAT:
return dev_get_uclass_plat(dev);- case DM_TAG_PRIV:
return dev_get_priv(dev);- case DM_TAG_PARENT_PRIV:
return dev_get_parent_priv(dev);- case DM_TAG_UC_PRIV:
return dev_get_uclass_priv(dev);- default:
return NULL;- }
+}
+int dev_get_attach_size(const struct udevice *dev, enum dm_tag_t tag) +{
- const struct udevice *parent = dev_get_parent(dev);
- const struct uclass *uc = dev->uclass;
- const struct uclass_driver *uc_drv = uc->uc_drv;
- const struct driver *parent_drv = NULL;
- int size = 0;
- if (parent)
parent_drv = parent->driver;- switch (tag) {
- case DM_TAG_PLAT:
size = dev->driver->plat_auto;break;- case DM_TAG_PARENT_PLAT:
if (parent) {size = parent_drv->per_child_plat_auto;if (!size)size = parent->uclass->uc_drv->per_child_plat_auto;}break;- case DM_TAG_UC_PLAT:
size = uc_drv->per_device_plat_auto;break;- case DM_TAG_PRIV:
size = dev->driver->priv_auto;break;- case DM_TAG_PARENT_PRIV:
if (parent) {size = parent_drv->per_child_auto;if (!size)size = parent->uclass->uc_drv->per_child_auto;}break;- case DM_TAG_UC_PRIV:
size = uc_drv->per_device_auto;break;- default:
break;- }
- return size;
+}
static int device_get_device_tail(struct udevice *dev, int ret, struct udevice **devp) { diff --git a/drivers/misc/test_drv.c b/drivers/misc/test_drv.c index b6df1189032..927618256f0 100644 --- a/drivers/misc/test_drv.c +++ b/drivers/misc/test_drv.c @@ -108,7 +108,9 @@ UCLASS_DRIVER(testbus) = { .child_pre_probe = testbus_child_pre_probe_uclass, .child_post_probe = testbus_child_post_probe_uclass,
- /* This is for dtoc testing only */
- .per_device_auto = sizeof(struct dm_test_uclass_priv),
- /* Note: this is for dtoc testing as well as tags*/ .per_device_plat_auto = sizeof(struct dm_test_uclass_plat),
};
diff --git a/include/dm/device.h b/include/dm/device.h index 5bdb10653f8..12c6ba37ff3 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -11,6 +11,7 @@ #define _DM_DEVICE_H
#include <dm/ofnode.h> +#include <dm/tag.h> #include <dm/uclass-id.h> #include <fdtdec.h> #include <linker_lists.h> @@ -546,6 +547,30 @@ void *dev_get_parent_priv(const struct udevice *dev); */ void *dev_get_uclass_priv(const struct udevice *dev);
+/**
- dev_get_attach_ptr() - Get the value of an attached pointed tag
- The tag is assumed to hold a pointer, if it exists
- @dev: Device to look at
- @tag: Tag to access
- @return value of tag, or NULL if there is no tag of this type
- */
+void *dev_get_attach_ptr(const struct udevice *dev, enum dm_tag_t tag);
+/**
- dev_get_attach_size() - Get the size of an attached tag
- Core tags have an automatic-allocation mechanism where the allocated size is
- defined by the device, parent or uclass. This returns the size associated
- with a particular tag
- @dev: Device to look at
- @tag: Tag to access
- @return size of auto-allocated data, 0 if none
- */
+int dev_get_attach_size(const struct udevice *dev, enum dm_tag_t tag);
/**
- dev_get_parent() - Get the parent of a device
diff --git a/include/dm/tag.h b/include/dm/tag.h index 54fc31eb153..9cb5d68f0a3 100644 --- a/include/dm/tag.h +++ b/include/dm/tag.h @@ -13,8 +13,19 @@ struct udevice;
enum dm_tag_t {
- /* Types of core tags that can be attached to devices */
- DM_TAG_PLAT,
- DM_TAG_PARENT_PLAT,
- DM_TAG_UC_PLAT,
- DM_TAG_PRIV,
- DM_TAG_PARENT_PRIV,
- DM_TAG_UC_PRIV,
- DM_TAG_DRIVER_DATA,
- DM_TAG_ATTACH_COUNT,
- /* EFI_LOADER */
- DM_TAG_EFI = 0,
- DM_TAG_EFI = DM_TAG_ATTACH_COUNT,
What does DM_TAG_ATTACH_COUNT mean?
-Takahiro Akashi
DM_TAG_COUNT, }; diff --git a/test/dm/core.c b/test/dm/core.c index ebd504427d1..26e2fd56619 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -1275,3 +1275,83 @@ static int dm_test_uclass_find_device(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_uclass_find_device, UT_TESTF_SCAN_FDT);
+/* Test getting information about tags attached to devices */ +static int dm_test_dev_get_attach(struct unit_test_state *uts) +{
- struct udevice *dev;
- ut_assertok(uclass_first_device_err(UCLASS_TEST_FDT, &dev));
- ut_asserteq_str("a-test", dev->name);
- ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PLAT));
- ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PRIV));
- ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_UC_PRIV));
- ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_UC_PLAT));
- ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PLAT));
- ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PRIV));
- ut_asserteq(sizeof(struct dm_test_pdata),
dev_get_attach_size(dev, DM_TAG_PLAT));- ut_asserteq(sizeof(struct dm_test_priv),
dev_get_attach_size(dev, DM_TAG_PRIV));- ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_UC_PRIV));
- ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_UC_PLAT));
- ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PLAT));
- ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PRIV));
- return 0;
+} +DM_TEST(dm_test_dev_get_attach, UT_TESTF_SCAN_FDT);
+/* Test getting information about tags attached to bus devices */ +static int dm_test_dev_get_attach_bus(struct unit_test_state *uts) +{
- struct udevice *dev, *child;
- ut_assertok(uclass_first_device_err(UCLASS_TEST_BUS, &dev));
- ut_asserteq_str("some-bus", dev->name);
- ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PLAT));
- ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PRIV));
- ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_UC_PRIV));
- ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_UC_PLAT));
- ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PLAT));
- ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PRIV));
- ut_asserteq(sizeof(struct dm_test_pdata),
dev_get_attach_size(dev, DM_TAG_PLAT));- ut_asserteq(sizeof(struct dm_test_priv),
dev_get_attach_size(dev, DM_TAG_PRIV));- ut_asserteq(sizeof(struct dm_test_uclass_priv),
dev_get_attach_size(dev, DM_TAG_UC_PRIV));- ut_asserteq(sizeof(struct dm_test_uclass_plat),
dev_get_attach_size(dev, DM_TAG_UC_PLAT));- ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PLAT));
- ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PRIV));
- /* Now try the child of the bus */
- ut_assertok(device_first_child_err(dev, &child));
- ut_asserteq_str("c-test@5", child->name);
- ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PLAT));
- ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PRIV));
- ut_assertnull(dev_get_attach_ptr(child, DM_TAG_UC_PRIV));
- ut_assertnull(dev_get_attach_ptr(child, DM_TAG_UC_PLAT));
- ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PARENT_PLAT));
- ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PARENT_PRIV));
- ut_asserteq(sizeof(struct dm_test_pdata),
dev_get_attach_size(child, DM_TAG_PLAT));- ut_asserteq(sizeof(struct dm_test_priv),
dev_get_attach_size(child, DM_TAG_PRIV));- ut_asserteq(0, dev_get_attach_size(child, DM_TAG_UC_PRIV));
- ut_asserteq(0, dev_get_attach_size(child, DM_TAG_UC_PLAT));
- ut_asserteq(sizeof(struct dm_test_parent_plat),
dev_get_attach_size(child, DM_TAG_PARENT_PLAT));- ut_asserteq(sizeof(struct dm_test_parent_data),
dev_get_attach_size(child, DM_TAG_PARENT_PRIV));- return 0;
+} +DM_TEST(dm_test_dev_get_attach_bus, UT_TESTF_SCAN_FDT); diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 8bac2076214..879ca2ab2bf 100755 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -618,6 +618,9 @@ u8 _denx_u_boot_test_bus_priv_some_bus[sizeof(struct dm_test_priv)] #include <dm/test.h> u8 _denx_u_boot_test_bus_ucplat_some_bus[sizeof(struct dm_test_uclass_plat)] \t__attribute__ ((section (".priv_data"))); +#include <dm/test.h> +u8 _denx_u_boot_test_bus_uc_priv_some_bus[sizeof(struct dm_test_uclass_priv)]
- __attribute__ ((section (".priv_data")));
#include <test.h>
DM_DEVICE_INST(some_bus) = { @@ -628,6 +631,7 @@ DM_DEVICE_INST(some_bus) = { \t.driver_data\t= DM_TEST_TYPE_FIRST, \t.priv_\t\t= _denx_u_boot_test_bus_priv_some_bus, \t.uclass\t\t= DM_UCLASS_REF(testbus), +\t.uclass_priv_ = _denx_u_boot_test_bus_uc_priv_some_bus, \t.uclass_node\t= { \t\t.prev = &DM_UCLASS_REF(testbus)->dev_head, \t\t.next = &DM_UCLASS_REF(testbus)->dev_head, -- 2.36.0.512.ge40c2bad7a-goog
Hi Simon,
On Sun, May 08, 2022 at 04:39:24AM -0600, Simon Glass wrote:
At present tag numbers are only allocated for non-core data, meaning that the 'core' data, like priv and plat, are accessed through dedicated functions.
For debugging and consistency it is convenient to use tags for this 'core' data too. Add support for this, with new tag numbers and functions to access the pointer and size for each.
Update one of the test drivers so that the uclass-private data can be tested here.
There is some code duplication with functions like device_alloc_priv() but this is not addressed for now. At some point, some rationalisation may help to reduce code size, but more thought it needed on that.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/device.c | 65 +++++++++++++++++++++++++++++++++ drivers/misc/test_drv.c | 4 ++- include/dm/device.h | 25 +++++++++++++ include/dm/tag.h | 13 ++++++- test/dm/core.c | 80 +++++++++++++++++++++++++++++++++++++++++ tools/dtoc/test_dtoc.py | 4 +++ 6 files changed, 189 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
On Tue, Jun 28, 2022 at 09:37:56AM -0400, Simon Glass wrote:
Hi Simon,
On Sun, May 08, 2022 at 04:39:24AM -0600, Simon Glass wrote:
At present tag numbers are only allocated for non-core data, meaning that the 'core' data, like priv and plat, are accessed through dedicated functions.
For debugging and consistency it is convenient to use tags for this 'core' data too. Add support for this, with new tag numbers and functions to access the pointer and size for each.
Update one of the test drivers so that the uclass-private data can be tested here.
There is some code duplication with functions like device_alloc_priv() but this is not addressed for now. At some point, some rationalisation may help to reduce code size, but more thought it needed on that.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/device.c | 65 +++++++++++++++++++++++++++++++++ drivers/misc/test_drv.c | 4 ++- include/dm/device.h | 25 +++++++++++++ include/dm/tag.h | 13 ++++++- test/dm/core.c | 80 +++++++++++++++++++++++++++++++++++++++++ tools/dtoc/test_dtoc.py | 4 +++ 6 files changed, 189 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
I expect you to reply to my comments: https://lists.denx.de/pipermail/u-boot/2022-May/483606.html
-Takahiro Akashi
On Tue, Jun 28, 2022 at 11:18:51PM +0900, AKASHI Takahiro wrote:
On Tue, Jun 28, 2022 at 09:37:56AM -0400, Simon Glass wrote:
Hi Simon,
On Sun, May 08, 2022 at 04:39:24AM -0600, Simon Glass wrote:
At present tag numbers are only allocated for non-core data, meaning that the 'core' data, like priv and plat, are accessed through dedicated functions.
For debugging and consistency it is convenient to use tags for this 'core' data too. Add support for this, with new tag numbers and functions to access the pointer and size for each.
Update one of the test drivers so that the uclass-private data can be tested here.
There is some code duplication with functions like device_alloc_priv() but this is not addressed for now. At some point, some rationalisation may help to reduce code size, but more thought it needed on that.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/device.c | 65 +++++++++++++++++++++++++++++++++ drivers/misc/test_drv.c | 4 ++- include/dm/device.h | 25 +++++++++++++ include/dm/tag.h | 13 ++++++- test/dm/core.c | 80 +++++++++++++++++++++++++++++++++++++++++ tools/dtoc/test_dtoc.py | 4 +++ 6 files changed, 189 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
I expect you to reply to my comments: https://lists.denx.de/pipermail/u-boot/2022-May/483606.html
Simon? This is why I haven't applied the PR for -next yet, I was waiting for your comments here, thanks.
Hi,
On Tue, 5 Jul 2022 at 13:39, Tom Rini trini@konsulko.com wrote:
On Tue, Jun 28, 2022 at 11:18:51PM +0900, AKASHI Takahiro wrote:
On Tue, Jun 28, 2022 at 09:37:56AM -0400, Simon Glass wrote:
Hi Simon,
On Sun, May 08, 2022 at 04:39:24AM -0600, Simon Glass wrote:
At present tag numbers are only allocated for non-core data, meaning that the 'core' data, like priv and plat, are accessed through dedicated functions.
For debugging and consistency it is convenient to use tags for this 'core' data too. Add support for this, with new tag numbers and functions to access the pointer and size for each.
Update one of the test drivers so that the uclass-private data can be tested here.
There is some code duplication with functions like device_alloc_priv() but this is not addressed for now. At some point, some rationalisation may help to reduce code size, but more thought it needed on that.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/device.c | 65 +++++++++++++++++++++++++++++++++ drivers/misc/test_drv.c | 4 ++- include/dm/device.h | 25 +++++++++++++ include/dm/tag.h | 13 ++++++- test/dm/core.c | 80 +++++++++++++++++++++++++++++++++++++++++ tools/dtoc/test_dtoc.py | 4 +++ 6 files changed, 189 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
I expect you to reply to my comments: https://lists.denx.de/pipermail/u-boot/2022-May/483606.html
Oh I missed this as I have not been reading email for some months and did not notice this in patchwork.
Simon? This is why I haven't applied the PR for -next yet, I was waiting for your comments here, thanks.
The reason is that the tag functionality is optional and is not used for most boards. The new API is a core function.
I do expect to be able to rationalise things at some point once we have a bit more code in there, but perhaps in the opposite direction.
Regards, Simon
Hi Simon,
On Tue, Jul 05, 2022 at 02:27:54PM +0100, Simon Glass wrote:
Hi,
On Tue, 5 Jul 2022 at 13:39, Tom Rini trini@konsulko.com wrote:
On Tue, Jun 28, 2022 at 11:18:51PM +0900, AKASHI Takahiro wrote:
On Tue, Jun 28, 2022 at 09:37:56AM -0400, Simon Glass wrote:
Hi Simon,
On Sun, May 08, 2022 at 04:39:24AM -0600, Simon Glass wrote:
At present tag numbers are only allocated for non-core data, meaning that the 'core' data, like priv and plat, are accessed through dedicated functions.
For debugging and consistency it is convenient to use tags for this 'core' data too. Add support for this, with new tag numbers and functions to access the pointer and size for each.
Update one of the test drivers so that the uclass-private data can be tested here.
There is some code duplication with functions like device_alloc_priv() but this is not addressed for now. At some point, some rationalisation may help to reduce code size, but more thought it needed on that.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/device.c | 65 +++++++++++++++++++++++++++++++++ drivers/misc/test_drv.c | 4 ++- include/dm/device.h | 25 +++++++++++++ include/dm/tag.h | 13 ++++++- test/dm/core.c | 80 +++++++++++++++++++++++++++++++++++++++++ tools/dtoc/test_dtoc.py | 4 +++ 6 files changed, 189 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
I expect you to reply to my comments: https://lists.denx.de/pipermail/u-boot/2022-May/483606.html
Oh I missed this as I have not been reading email for some months and did not notice this in patchwork.
Simon? This is why I haven't applied the PR for -next yet, I was waiting for your comments here, thanks.
The reason is that the tag functionality is optional and is not used for most boards. The new API is a core function.
Please let me make sure your intension: Is this your answer to my question: "Why not (enhance and) re-use dev_tag_get_ptr()? Both functions, at least, share the syntax and even the semantics from user's viewpoint."
I do expect to be able to rationalise things at some point once we have a bit more code in there, but perhaps in the opposite direction.
So you have a concern that this kind of API (i.e. tag) might disappear or be changed and diverge in incompatible(?) direction in the future and you think that it's the best for now to have separate APIs for different subsystems/users?
-Takahiro Akashi
Regards, Simon
Hi Takahiro,
On Tue, 5 Jul 2022 at 19:27, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
Hi Simon,
On Tue, Jul 05, 2022 at 02:27:54PM +0100, Simon Glass wrote:
Hi,
On Tue, 5 Jul 2022 at 13:39, Tom Rini trini@konsulko.com wrote:
On Tue, Jun 28, 2022 at 11:18:51PM +0900, AKASHI Takahiro wrote:
On Tue, Jun 28, 2022 at 09:37:56AM -0400, Simon Glass wrote:
Hi Simon,
On Sun, May 08, 2022 at 04:39:24AM -0600, Simon Glass wrote:
At present tag numbers are only allocated for non-core data, meaning that the 'core' data, like priv and plat, are accessed through dedicated functions.
For debugging and consistency it is convenient to use tags for this 'core' data too. Add support for this, with new tag numbers and functions to access the pointer and size for each.
Update one of the test drivers so that the uclass-private data can be tested here.
There is some code duplication with functions like device_alloc_priv() but this is not addressed for now. At some point, some rationalisation may help to reduce code size, but more thought it needed on that.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/device.c | 65 +++++++++++++++++++++++++++++++++ drivers/misc/test_drv.c | 4 ++- include/dm/device.h | 25 +++++++++++++ include/dm/tag.h | 13 ++++++- test/dm/core.c | 80 +++++++++++++++++++++++++++++++++++++++++ tools/dtoc/test_dtoc.py | 4 +++ 6 files changed, 189 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
I expect you to reply to my comments: https://lists.denx.de/pipermail/u-boot/2022-May/483606.html
Oh I missed this as I have not been reading email for some months and did not notice this in patchwork.
Simon? This is why I haven't applied the PR for -next yet, I was waiting for your comments here, thanks.
The reason is that the tag functionality is optional and is not used for most boards. The new API is a core function.
Please let me make sure your intension: Is this your answer to my question: "Why not (enhance and) re-use dev_tag_get_ptr()? Both functions, at least, share the syntax and even the semantics from user's viewpoint."
I do expect to be able to rationalise things at some point once we have a bit more code in there, but perhaps in the opposite direction.
So you have a concern that this kind of API (i.e. tag) might disappear or be changed and diverge in incompatible(?) direction in the future and you think that it's the best for now to have separate APIs for different subsystems/users?
It is more the other way around. The tag API is not used by most boards and is not (yet) a 'required' part of driver model. I have yet to invest enough time to figure out how this should all pan out, but my feeling is that, by making the private-data pointers into tags, we can save data space, if not code space. I am leaning towards joining tags up more so that we can use the 'tag' concept for private-data pointers, with the ability to add other tags as a separate feature which can be enabled or disabled. If we go that way, then dev_tag_get_ptr() will call the new function I've added for 'core' tags, otherwise look up its list.
The goal of this patch (and series) is to provide stats to inform the future direction.
Regards, Simon
Add a function for collecting the amount of memory used by driver model, including devices, uclasses and attached data and tags.
This information can provide insights into how to reduce the memory required by driver model. Future work may look at execution speed also.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/root.c | 53 +++++++++++++++++++++++++++++++++++++++++++++ drivers/core/tag.c | 11 ++++++++++ include/dm/root.h | 45 ++++++++++++++++++++++++++++++++++++++ include/dm/tag.h | 11 ++++++++++ test/dm/core.c | 11 ++++++++++ 5 files changed, 131 insertions(+)
diff --git a/drivers/core/root.c b/drivers/core/root.c index 17dd1205a32..f24ddfa5218 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -449,6 +449,59 @@ void dm_get_stats(int *device_countp, int *uclass_countp) *uclass_countp = uclass_get_count(); }
+void dev_collect_stats(struct dm_stats *stats, const struct udevice *parent) +{ + const struct udevice *dev; + int i; + + stats->dev_count++; + stats->dev_size += sizeof(struct udevice); + stats->dev_name_size += strlen(parent->name) + 1; + for (i = 0; i < DM_TAG_ATTACH_COUNT; i++) { + int size = dev_get_attach_size(parent, i); + + if (size || + (i == DM_TAG_DRIVER_DATA && parent->driver_data)) { + stats->attach_count[i]++; + stats->attach_size[i] += size; + stats->attach_count_total++; + stats->attach_size_total += size; + } + } + + list_for_each_entry(dev, &parent->child_head, sibling_node) + dev_collect_stats(stats, dev); +} + +void uclass_collect_stats(struct dm_stats *stats) +{ + struct uclass *uc; + + list_for_each_entry(uc, gd->uclass_root, sibling_node) { + int size; + + stats->uc_count++; + stats->uc_size += sizeof(struct uclass); + size = uc->uc_drv->priv_auto; + if (size) { + stats->uc_attach_count++; + stats->uc_attach_size += size; + } + } +} + +void dm_get_mem(struct dm_stats *stats) +{ + memset(stats, '\0', sizeof(*stats)); + dev_collect_stats(stats, gd->dm_root); + uclass_collect_stats(stats); + dev_tag_collect_stats(stats); + + stats->total_size = stats->dev_size + stats->uc_size + + stats->attach_size_total + stats->uc_attach_size + + stats->tag_size; +} + #ifdef CONFIG_ACPIGEN static int root_acpi_get_name(const struct udevice *dev, char *out_name) { diff --git a/drivers/core/tag.c b/drivers/core/tag.c index 22999193a5a..2961725b658 100644 --- a/drivers/core/tag.c +++ b/drivers/core/tag.c @@ -6,6 +6,7 @@
#include <malloc.h> #include <asm/global_data.h> +#include <dm/root.h> #include <dm/tag.h> #include <linux/err.h> #include <linux/list.h> @@ -137,3 +138,13 @@ int dev_tag_del_all(struct udevice *dev)
return -ENOENT; } + +void dev_tag_collect_stats(struct dm_stats *stats) +{ + struct dmtag_node *node; + + list_for_each_entry(node, &gd->dmtag_list, sibling) { + stats->tag_count++; + stats->tag_size += sizeof(struct dmtag_node); + } +} diff --git a/include/dm/root.h b/include/dm/root.h index e888fb993c0..382f83c7f5b 100644 --- a/include/dm/root.h +++ b/include/dm/root.h @@ -9,11 +9,49 @@ #ifndef _DM_ROOT_H_ #define _DM_ROOT_H_
+#include <dm/tag.h> + struct udevice;
/* Head of the uclass list if CONFIG_OF_PLATDATA_INST is enabled */ extern struct list_head uclass_head;
+/** + * struct dm_stats - Information about driver model memory usage + * + * @total_size: All data + * @dev_count: Number of devices + * @dev_size: Size of all devices (just the struct udevice) + * @dev_name_size: Bytes used by device names + * @uc_count: Number of uclasses + * @uc_size: Size of all uclasses (just the struct uclass) + * @tag_count: Number of tags + * @tag_size: Bytes used by all tags + * @uc_attach_count: Number of uclasses with attached data (priv) + * @uc_attach_size: Total size of that attached data + * @attach_count_total: Total number of attached data items for all udevices and + * uclasses + * @attach_size_total: Total number of bytes of attached data + * @attach_count: Number of devices with attached, for each type + * @attach_size: Total number of bytes of attached data, for each type + */ +struct dm_stats { + int total_size; + int dev_count; + int dev_size; + int dev_name_size; + int uc_count; + int uc_size; + int tag_count; + int tag_size; + int uc_attach_count; + int uc_attach_size; + int attach_count_total; + int attach_size_total; + int attach_count[DM_TAG_ATTACH_COUNT]; + int attach_size[DM_TAG_ATTACH_COUNT]; +}; + /** * dm_root() - Return pointer to the top of the driver tree * @@ -141,4 +179,11 @@ static inline int dm_remove_devices_flags(uint flags) { return 0; } */ void dm_get_stats(int *device_countp, int *uclass_countp);
+/** + * dm_get_mem() - Get stats on memory usage in driver model + * + * @mem: Place to put the information + */ +void dm_get_mem(struct dm_stats *stats); + #endif diff --git a/include/dm/tag.h b/include/dm/tag.h index 9cb5d68f0a3..1ea3c9f7af3 100644 --- a/include/dm/tag.h +++ b/include/dm/tag.h @@ -10,6 +10,7 @@ #include <linux/list.h> #include <linux/types.h>
+struct dm_stats; struct udevice;
enum dm_tag_t { @@ -118,4 +119,14 @@ int dev_tag_del(struct udevice *dev, enum dm_tag_t tag); */ int dev_tag_del_all(struct udevice *dev);
+/** + * dev_tag_collect_stats() - Collect information on driver model performance + * + * This collects information on how driver model is performing. For now it only + * includes memory usage + * + * @stats: Place to put the collected information + */ +void dev_tag_collect_stats(struct dm_stats *stats); + #endif /* _DM_TAG_H */ diff --git a/test/dm/core.c b/test/dm/core.c index 26e2fd56619..fd4d7569728 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -1355,3 +1355,14 @@ static int dm_test_dev_get_attach_bus(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_dev_get_attach_bus, UT_TESTF_SCAN_FDT); + +/* Test getting information about tags attached to bus devices */ +static int dm_test_dev_get_mem(struct unit_test_state *uts) +{ + struct dm_stats stats; + + dm_get_mem(&stats); + + return 0; +} +DM_TEST(dm_test_dev_get_mem, UT_TESTF_SCAN_FDT);
Add a function for collecting the amount of memory used by driver model, including devices, uclasses and attached data and tags.
This information can provide insights into how to reduce the memory required by driver model. Future work may look at execution speed also.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/root.c | 53 +++++++++++++++++++++++++++++++++++++++++++++ drivers/core/tag.c | 11 ++++++++++ include/dm/root.h | 45 ++++++++++++++++++++++++++++++++++++++ include/dm/tag.h | 11 ++++++++++ test/dm/core.c | 11 ++++++++++ 5 files changed, 131 insertions(+)
Applied to u-boot-dm, thanks!
This command shows the memory used by driver model along with various hints as to what it might be if some 'core' tags were moved to use the tag list instead of a core (i.e. always-there) pointer.
This may help with future work to reduce memory usage.
Signed-off-by: Simon Glass sjg@chromium.org ---
cmd/dm.c | 23 ++++++++++++++ drivers/core/Kconfig | 11 +++++++ drivers/core/dump.c | 73 ++++++++++++++++++++++++++++++++++++++++++++ drivers/core/tag.c | 18 +++++++++++ include/dm/root.h | 2 +- include/dm/tag.h | 8 +++++ include/dm/util.h | 9 ++++++ 7 files changed, 143 insertions(+), 1 deletion(-)
diff --git a/cmd/dm.c b/cmd/dm.c index 4c0a8a53ced..3814dcd20f1 100644 --- a/cmd/dm.c +++ b/cmd/dm.c @@ -40,6 +40,19 @@ static int do_dm_dump_drivers(struct cmd_tbl *cmdtp, int flag, int argc, return 0; }
+#if CONFIG_IS_ENABLED(DM_STATS) +static int do_dm_dump_mem(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct dm_stats mem; + + dm_get_mem(&mem); + dm_dump_mem(&mem); + + return 0; +} +#endif /* DM_STATS */ + static int do_dm_dump_static_driver_info(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) { @@ -68,6 +81,9 @@ static struct cmd_tbl test_commands[] = { U_BOOT_CMD_MKENT(compat, 1, 1, do_dm_dump_driver_compat, "", ""), U_BOOT_CMD_MKENT(devres, 1, 1, do_dm_dump_devres, "", ""), U_BOOT_CMD_MKENT(drivers, 1, 1, do_dm_dump_drivers, "", ""), +#if CONFIG_IS_ENABLED(DM_STATS) + U_BOOT_CMD_MKENT(mem, 1, 1, do_dm_dump_mem, "", ""), +#endif U_BOOT_CMD_MKENT(static, 1, 1, do_dm_dump_static_driver_info, "", ""), U_BOOT_CMD_MKENT(tree, 0, 1, do_dm_dump_tree, "", ""), U_BOOT_CMD_MKENT(uclass, 1, 1, do_dm_dump_uclass, "", ""), @@ -106,12 +122,19 @@ static int do_dm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return cmd_process_error(test_cmd, ret); }
+#if CONFIG_IS_ENABLED(DM_STATS) +#define DM_MEM_HELP "dm mem Provide a summary of memory usage\n" +#else +#define DM_MEM_HELP +#endif + U_BOOT_CMD( dm, 3, 1, do_dm, "Driver model low level access", "compat Dump list of drivers with compatibility strings\n" "dm devres Dump list of device resources for each device\n" "dm drivers Dump list of drivers with uclass and instances\n" + DM_MEM_HELP "dm static Dump list of drivers with static platform data\n" "dm tree Dump tree of driver model devices ('*' = activated)\n" "dm uclass Dump list of instances for each uclass" diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig index 408a8d8e28b..fa2811af83c 100644 --- a/drivers/core/Kconfig +++ b/drivers/core/Kconfig @@ -75,6 +75,17 @@ config DM_DEBUG help Say Y here if you want to compile in debug messages in DM core.
+config DM_STATS + bool "Collect and show driver model stats" + depends on DM + default y if SANDBOX + help + Enable this to collect and display memory statistics about driver + model. This can help to figure out where all the memory is going and + to find optimisations. + + To display the memory stats, use the 'dm mem' command. + config DM_DEVICE_REMOVE bool "Support device removal" depends on DM diff --git a/drivers/core/dump.c b/drivers/core/dump.c index ce679e26290..43d1349635b 100644 --- a/drivers/core/dump.c +++ b/drivers/core/dump.c @@ -174,3 +174,76 @@ void dm_dump_static_driver_info(void) for (entry = drv; entry != drv + n_ents; entry++) printf("%-25.25s %p\n", entry->name, entry->plat); } + +void dm_dump_mem(struct dm_stats *stats) +{ + int total, total_delta; + int i; + + /* Support SPL printf() */ + printf("Struct sizes: udevice %x, driver %x, uclass %x, uc_driver %x\n", + (int)sizeof(struct udevice), (int)sizeof(struct driver), + (int)sizeof(struct uclass), (int)sizeof(struct uclass_driver)); + printf("Memory: device %x:%x, device names %x, uclass %x:%x\n", + stats->dev_count, stats->dev_size, stats->dev_name_size, + stats->uc_count, stats->uc_size); + printf("\n"); + printf("%-15s %5s %5s %5s %5s %5s\n", "Attached type", "Count", + "Size", "Cur", "Tags", "Save"); + printf("%-15s %5s %5s %5s %5s %5s\n", "---------------", "-----", + "-----", "-----", "-----", "-----"); + total_delta = 0; + for (i = 0; i < DM_TAG_ATTACH_COUNT; i++) { + int cur_size, new_size, delta; + + cur_size = stats->dev_count * sizeof(struct udevice); + new_size = stats->dev_count * (sizeof(struct udevice) - + sizeof(void *)); + /* + * Let's assume we can fit each dmtag_node into 32 bits. We can + * limit the 'tiny tags' feature to SPL with + * CONFIG_SPL_SYS_MALLOC_F_LEN <= 64KB, so needing 14 bits to + * point to anything in that region (with 4-byte alignment). + * So: + * 4 bits for tag + * 14 bits for offset of dev + * 14 bits for offset of data + */ + new_size += stats->attach_count[i] * sizeof(u32); + delta = cur_size - new_size; + total_delta += delta; + printf("%-16s %5x %6x %6x %6x %6x (%d)\n", tag_get_name(i), + stats->attach_count[i], stats->attach_size[i], + cur_size, new_size, delta > 0 ? delta : 0, delta); + } + printf("%-16s %5x %6x\n", "uclass", stats->uc_attach_count, + stats->uc_attach_size); + printf("%-16s %5x %6x %5s %5s %6x (%d)\n", "Attached total", + stats->attach_count_total + stats->uc_attach_count, + stats->attach_size_total + stats->uc_attach_size, "", "", + total_delta > 0 ? total_delta : 0, total_delta); + printf("%-16s %5x %6x\n", "tags", stats->tag_count, stats->tag_size); + printf("\n"); + printf("Total size: %x (%d)\n", stats->total_size, stats->total_size); + printf("\n"); + + total = stats->total_size; + total -= total_delta; + printf("With tags: %x (%d)\n", total, total); + + /* Use singly linked lists in struct udevice (3 nodes in each) */ + total -= sizeof(void *) * 3 * stats->dev_count; + printf("- singly-linked: %x (%d)\n", total, total); + + /* Use an index into the struct_driver list instead of a pointer */ + total = total + stats->dev_count * (1 - sizeof(void *)); + printf("- driver index: %x (%d)\n", total, total); + + /* Same with the uclass */ + total = total + stats->dev_count * (1 - sizeof(void *)); + printf("- uclass index: %x (%d)\n", total, total); + + /* Drop the device name */ + printf("Drop device name (not SRAM): %x (%d)\n", stats->dev_name_size, + stats->dev_name_size); +} diff --git a/drivers/core/tag.c b/drivers/core/tag.c index 2961725b658..a3c5cb7e57c 100644 --- a/drivers/core/tag.c +++ b/drivers/core/tag.c @@ -16,6 +16,24 @@ struct udevice;
DECLARE_GLOBAL_DATA_PTR;
+static const char *const tag_name[] = { + [DM_TAG_PLAT] = "plat", + [DM_TAG_PARENT_PLAT] = "parent_plat", + [DM_TAG_UC_PLAT] = "uclass_plat", + + [DM_TAG_PRIV] = "priv", + [DM_TAG_PARENT_PRIV] = "parent_priv", + [DM_TAG_UC_PRIV] = "uclass_priv", + [DM_TAG_DRIVER_DATA] = "driver_data", + + [DM_TAG_EFI] = "efi", +}; + +const char *tag_get_name(enum dm_tag_t tag) +{ + return tag_name[tag]; +} + int dev_tag_set_ptr(struct udevice *dev, enum dm_tag_t tag, void *ptr) { struct dmtag_node *node; diff --git a/include/dm/root.h b/include/dm/root.h index 382f83c7f5b..b2f30a842f5 100644 --- a/include/dm/root.h +++ b/include/dm/root.h @@ -182,7 +182,7 @@ void dm_get_stats(int *device_countp, int *uclass_countp); /** * dm_get_mem() - Get stats on memory usage in driver model * - * @mem: Place to put the information + * @stats: Place to put the information */ void dm_get_mem(struct dm_stats *stats);
diff --git a/include/dm/tag.h b/include/dm/tag.h index 1ea3c9f7af3..745088ffcff 100644 --- a/include/dm/tag.h +++ b/include/dm/tag.h @@ -129,4 +129,12 @@ int dev_tag_del_all(struct udevice *dev); */ void dev_tag_collect_stats(struct dm_stats *stats);
+/** + * tag_get_name() - Get the name of a tag + * + * @tag: Tag to look up, which must be valid + * Returns: Name of tag + */ +const char *tag_get_name(enum dm_tag_t tag); + #endif /* _DM_TAG_H */ diff --git a/include/dm/util.h b/include/dm/util.h index c52daa87ef3..e10c6060ce0 100644 --- a/include/dm/util.h +++ b/include/dm/util.h @@ -6,6 +6,8 @@ #ifndef __DM_UTIL_H #define __DM_UTIL_H
+struct dm_stats; + #if CONFIG_IS_ENABLED(DM_WARN) #define dm_warn(fmt...) log(LOGC_DM, LOGL_WARNING, ##fmt) #else @@ -48,6 +50,13 @@ void dm_dump_driver_compat(void); /* Dump out a list of drivers with static platform data */ void dm_dump_static_driver_info(void);
+/** + * dm_dump_mem() - Dump stats on memory usage in driver model + * + * @mem: Stats to dump + */ +void dm_dump_mem(struct dm_stats *stats); + #if CONFIG_IS_ENABLED(OF_PLATDATA_INST) && CONFIG_IS_ENABLED(READ_ONLY) void *dm_priv_to_rw(void *priv); #else
This command shows the memory used by driver model along with various hints as to what it might be if some 'core' tags were moved to use the tag list instead of a core (i.e. always-there) pointer.
This may help with future work to reduce memory usage.
Signed-off-by: Simon Glass sjg@chromium.org ---
cmd/dm.c | 23 ++++++++++++++ drivers/core/Kconfig | 11 +++++++ drivers/core/dump.c | 73 ++++++++++++++++++++++++++++++++++++++++++++ drivers/core/tag.c | 18 +++++++++++ include/dm/root.h | 2 +- include/dm/tag.h | 8 +++++ include/dm/util.h | 9 ++++++ 7 files changed, 143 insertions(+), 1 deletion(-)
Applied to u-boot-dm, thanks!
Add an option to tell SPL to show memory usage for driver model just before it boots into the next phase.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/spl/spl.c | 9 +++++++++ drivers/core/Kconfig | 10 ++++++++++ 2 files changed, 19 insertions(+)
diff --git a/common/spl/spl.c b/common/spl/spl.c index c8c463f80bd..540e1925577 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -33,6 +33,7 @@ #include <malloc.h> #include <mapmem.h> #include <dm/root.h> +#include <dm/util.h> #include <linux/compiler.h> #include <fdt_support.h> #include <bootcount.h> @@ -780,6 +781,14 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
bootcount_inc();
+ /* Dump driver model states to aid analysis */ + if (CONFIG_IS_ENABLED(DM_STATS)) { + struct dm_stats mem; + + dm_get_mem(&mem); + dm_dump_mem(&mem); + } + memset(&spl_image, '\0', sizeof(spl_image)); #ifdef CONFIG_SYS_SPL_ARGS_ADDR spl_image.arg = (void *)CONFIG_SYS_SPL_ARGS_ADDR; diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig index fa2811af83c..5c35914d30b 100644 --- a/drivers/core/Kconfig +++ b/drivers/core/Kconfig @@ -86,6 +86,16 @@ config DM_STATS
To display the memory stats, use the 'dm mem' command.
+config SPL_DM_STATS + bool "Collect and show driver model stats in SPL" + depends on DM_SPL + help + Enable this to collect and display memory statistics about driver + model. This can help to figure out where all the memory is going and + to find optimisations. + + The stats are displayed just before SPL boots to the next phase. + config DM_DEVICE_REMOVE bool "Support device removal" depends on DM
Add an option to tell SPL to show memory usage for driver model just before it boots into the next phase.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/spl/spl.c | 9 +++++++++ drivers/core/Kconfig | 10 ++++++++++ 2 files changed, 19 insertions(+)
Applied to u-boot-dm, thanks!
participants (3)
-
AKASHI Takahiro -
Simon Glass -
Tom Rini