u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] dm: core: Support collecting and reporting stats
@ 2022-05-08 10:39 Simon Glass
  2022-05-08 10:39 ` [PATCH 1/9] dm: core: Rename dm_dump_all() Simon Glass
                   ` (16 more replies)
  0 siblings, 17 replies; 25+ messages in thread
From: Simon Glass @ 2022-05-08 10:39 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, U-Boot Custodians, Simon Glass, Bin Meng,
	Heinrich Schuchardt, Marek Behún, Marek Vasut,
	Ovidiu Panait, Pali Rohár, Patrick Delaunay, Pavel Herrmann,
	Ricardo Salveti, Stefan Roese

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

-- 
2.36.0.512.ge40c2bad7a-goog


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/9] dm: core: Rename dm_dump_all()
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
@ 2022-05-08 10:39 ` Simon Glass
  2022-05-08 10:39 ` [PATCH 2/9] dm: core: Sort dm subcommands Simon Glass
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-05-08 10:39 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, U-Boot Custodians, Simon Glass, Marek Vasut, Pavel Herrmann

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);
-- 
2.36.0.512.ge40c2bad7a-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/9] dm: core: Sort dm subcommands
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
  2022-05-08 10:39 ` [PATCH 1/9] dm: core: Rename dm_dump_all() Simon Glass
@ 2022-05-08 10:39 ` Simon Glass
  2022-05-08 10:39 ` [PATCH 3/9] dm: core: Fix addresses in the dm static command Simon Glass
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-05-08 10:39 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, U-Boot Custodians, Simon Glass, Marek Vasut, Pavel Herrmann

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"
 );
-- 
2.36.0.512.ge40c2bad7a-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 3/9] dm: core: Fix addresses in the dm static command
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
  2022-05-08 10:39 ` [PATCH 1/9] dm: core: Rename dm_dump_all() Simon Glass
  2022-05-08 10:39 ` [PATCH 2/9] dm: core: Sort dm subcommands Simon Glass
@ 2022-05-08 10:39 ` Simon Glass
  2022-05-08 10:39 ` [PATCH 4/9] dm: core: Add documentation for the dm command Simon Glass
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-05-08 10:39 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, U-Boot Custodians, Simon Glass, Marek Vasut, Pavel Herrmann

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);
 }
-- 
2.36.0.512.ge40c2bad7a-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 4/9] dm: core: Add documentation for the dm command
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
                   ` (2 preceding siblings ...)
  2022-05-08 10:39 ` [PATCH 3/9] dm: core: Fix addresses in the dm static command Simon Glass
@ 2022-05-08 10:39 ` Simon Glass
  2022-05-08 10:39 ` [PATCH 5/9] dm: core: Switch the testbus driver to use a new struct Simon Glass
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-05-08 10:39 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, U-Boot Custodians, Simon Glass, Bin Meng,
	Heinrich Schuchardt, Marek Behún, Marek Vasut,
	Patrick Delaunay, Pavel Herrmann

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
-- 
2.36.0.512.ge40c2bad7a-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 5/9] dm: core: Switch the testbus driver to use a new struct
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
                   ` (3 preceding siblings ...)
  2022-05-08 10:39 ` [PATCH 4/9] dm: core: Add documentation for the dm command Simon Glass
@ 2022-05-08 10:39 ` Simon Glass
  2022-05-08 10:39 ` [PATCH 6/9] dm: core: Support accessing core tags Simon Glass
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-05-08 10:39 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, U-Boot Custodians, Simon Glass, Marek Vasut, Pavel Herrmann

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>
 
-- 
2.36.0.512.ge40c2bad7a-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 6/9] dm: core: Support accessing core tags
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
                   ` (4 preceding siblings ...)
  2022-05-08 10:39 ` [PATCH 5/9] dm: core: Switch the testbus driver to use a new struct Simon Glass
@ 2022-05-08 10:39 ` Simon Glass
  2022-05-09  4:52   ` AKASHI Takahiro
  2022-06-28 13:37   ` Simon Glass
  2022-05-08 10:39 ` [PATCH 7/9] dm: core: Add a way to collect memory usage Simon Glass
                   ` (10 subsequent siblings)
  16 siblings, 2 replies; 25+ messages in thread
From: Simon Glass @ 2022-05-08 10:39 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, U-Boot Custodians, Simon Glass, Marek Vasut, Pavel Herrmann

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,
-- 
2.36.0.512.ge40c2bad7a-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 7/9] dm: core: Add a way to collect memory usage
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
                   ` (5 preceding siblings ...)
  2022-05-08 10:39 ` [PATCH 6/9] dm: core: Support accessing core tags Simon Glass
@ 2022-05-08 10:39 ` Simon Glass
  2022-05-08 10:39 ` [PATCH 8/9] dm: core: Add a command to show driver model statistics Simon Glass
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-05-08 10:39 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, U-Boot Custodians, Simon Glass, Marek Vasut, Pavel Herrmann

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);
-- 
2.36.0.512.ge40c2bad7a-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 8/9] dm: core: Add a command to show driver model statistics
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
                   ` (6 preceding siblings ...)
  2022-05-08 10:39 ` [PATCH 7/9] dm: core: Add a way to collect memory usage Simon Glass
@ 2022-05-08 10:39 ` Simon Glass
  2022-05-08 10:39 ` [PATCH 9/9] dm: spl: Allow SPL to show memory usage Simon Glass
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-05-08 10:39 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, U-Boot Custodians, Simon Glass, Marek Vasut, Pavel Herrmann

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
-- 
2.36.0.512.ge40c2bad7a-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 9/9] dm: spl: Allow SPL to show memory usage
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
                   ` (7 preceding siblings ...)
  2022-05-08 10:39 ` [PATCH 8/9] dm: core: Add a command to show driver model statistics Simon Glass
@ 2022-05-08 10:39 ` Simon Glass
  2022-06-28 13:37 ` Simon Glass
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-05-08 10:39 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, U-Boot Custodians, Simon Glass, Heinrich Schuchardt,
	Marek Behún, Marek Vasut, Ovidiu Panait, Pali Rohár,
	Patrick Delaunay, Pavel Herrmann, Ricardo Salveti, Stefan Roese

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
-- 
2.36.0.512.ge40c2bad7a-goog


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 6/9] dm: core: Support accessing core tags
  2022-05-08 10:39 ` [PATCH 6/9] dm: core: Support accessing core tags Simon Glass
@ 2022-05-09  4:52   ` AKASHI Takahiro
  2022-06-28 13:37   ` Simon Glass
  1 sibling, 0 replies; 25+ messages in thread
From: AKASHI Takahiro @ 2022-05-09  4:52 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Tom Rini, U-Boot Custodians, Marek Vasut,
	Pavel Herrmann

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
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 9/9] dm: spl: Allow SPL to show memory usage
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
                   ` (8 preceding siblings ...)
  2022-05-08 10:39 ` [PATCH 9/9] dm: spl: Allow SPL to show memory usage Simon Glass
@ 2022-06-28 13:37 ` Simon Glass
  2022-06-28 13:37 ` [PATCH 8/9] dm: core: Add a command to show driver model statistics Simon Glass
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-06-28 13:37 UTC (permalink / raw)
  To: Simon Glass
  Cc: Tom Rini, U-Boot Custodians, Heinrich Schuchardt,
	Marek Behún, Marek Vasut, Ovidiu Panait, Pali Rohár,
	Patrick Delaunay, Pavel Herrmann, Ricardo Salveti, Stefan Roese,
	U-Boot Mailing List

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!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 8/9] dm: core: Add a command to show driver model statistics
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
                   ` (9 preceding siblings ...)
  2022-06-28 13:37 ` Simon Glass
@ 2022-06-28 13:37 ` Simon Glass
  2022-06-28 13:37 ` [PATCH 7/9] dm: core: Add a way to collect memory usage Simon Glass
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-06-28 13:37 UTC (permalink / raw)
  To: Simon Glass
  Cc: Tom Rini, U-Boot Custodians, Marek Vasut, Pavel Herrmann,
	U-Boot Mailing List

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!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 7/9] dm: core: Add a way to collect memory usage
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
                   ` (10 preceding siblings ...)
  2022-06-28 13:37 ` [PATCH 8/9] dm: core: Add a command to show driver model statistics Simon Glass
@ 2022-06-28 13:37 ` Simon Glass
  2022-06-28 13:37 ` [PATCH 5/9] dm: core: Switch the testbus driver to use a new struct Simon Glass
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-06-28 13:37 UTC (permalink / raw)
  To: Simon Glass
  Cc: Tom Rini, U-Boot Custodians, Marek Vasut, Pavel Herrmann,
	U-Boot Mailing List

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!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 6/9] dm: core: Support accessing core tags
  2022-05-08 10:39 ` [PATCH 6/9] dm: core: Support accessing core tags Simon Glass
  2022-05-09  4:52   ` AKASHI Takahiro
@ 2022-06-28 13:37   ` Simon Glass
  2022-06-28 14:18     ` AKASHI Takahiro
  1 sibling, 1 reply; 25+ messages in thread
From: Simon Glass @ 2022-06-28 13:37 UTC (permalink / raw)
  To: AKASHI Takahiro
  Cc: U-Boot Mailing List, Tom Rini, U-Boot Custodians, Marek Vasut,
	Pavel Herrmann, Simon Glass

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!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 5/9] dm: core: Switch the testbus driver to use a new struct
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
                   ` (11 preceding siblings ...)
  2022-06-28 13:37 ` [PATCH 7/9] dm: core: Add a way to collect memory usage Simon Glass
@ 2022-06-28 13:37 ` Simon Glass
  2022-06-28 13:37 ` [PATCH 4/9] dm: core: Add documentation for the dm command Simon Glass
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-06-28 13:37 UTC (permalink / raw)
  To: Simon Glass
  Cc: Tom Rini, U-Boot Custodians, Marek Vasut, Pavel Herrmann,
	U-Boot Mailing List

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!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 4/9] dm: core: Add documentation for the dm command
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
                   ` (12 preceding siblings ...)
  2022-06-28 13:37 ` [PATCH 5/9] dm: core: Switch the testbus driver to use a new struct Simon Glass
@ 2022-06-28 13:37 ` Simon Glass
  2022-06-28 13:37 ` [PATCH 3/9] dm: core: Fix addresses in the dm static command Simon Glass
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-06-28 13:37 UTC (permalink / raw)
  To: Simon Glass
  Cc: Tom Rini, U-Boot Custodians, Bin Meng, Heinrich Schuchardt,
	Marek Behún, Marek Vasut, Patrick Delaunay, Pavel Herrmann,
	U-Boot Mailing List

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!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/9] dm: core: Fix addresses in the dm static command
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
                   ` (13 preceding siblings ...)
  2022-06-28 13:37 ` [PATCH 4/9] dm: core: Add documentation for the dm command Simon Glass
@ 2022-06-28 13:37 ` Simon Glass
  2022-06-28 13:38 ` [PATCH 2/9] dm: core: Sort dm subcommands Simon Glass
  2022-06-28 13:38 ` [PATCH 1/9] dm: core: Rename dm_dump_all() Simon Glass
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-06-28 13:37 UTC (permalink / raw)
  To: Simon Glass
  Cc: Tom Rini, U-Boot Custodians, Marek Vasut, Pavel Herrmann,
	U-Boot Mailing List

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!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/9] dm: core: Sort dm subcommands
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
                   ` (14 preceding siblings ...)
  2022-06-28 13:37 ` [PATCH 3/9] dm: core: Fix addresses in the dm static command Simon Glass
@ 2022-06-28 13:38 ` Simon Glass
  2022-06-28 13:38 ` [PATCH 1/9] dm: core: Rename dm_dump_all() Simon Glass
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-06-28 13:38 UTC (permalink / raw)
  To: Simon Glass
  Cc: Tom Rini, U-Boot Custodians, Marek Vasut, Pavel Herrmann,
	U-Boot Mailing List

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!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/9] dm: core: Rename dm_dump_all()
  2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
                   ` (15 preceding siblings ...)
  2022-06-28 13:38 ` [PATCH 2/9] dm: core: Sort dm subcommands Simon Glass
@ 2022-06-28 13:38 ` Simon Glass
  16 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-06-28 13:38 UTC (permalink / raw)
  To: Simon Glass
  Cc: Tom Rini, U-Boot Custodians, Marek Vasut, Pavel Herrmann,
	U-Boot Mailing List

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!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 6/9] dm: core: Support accessing core tags
  2022-06-28 13:37   ` Simon Glass
@ 2022-06-28 14:18     ` AKASHI Takahiro
  2022-07-05 12:39       ` Tom Rini
  0 siblings, 1 reply; 25+ messages in thread
From: AKASHI Takahiro @ 2022-06-28 14:18 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Tom Rini, U-Boot Custodians, Marek Vasut,
	Pavel Herrmann

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 6/9] dm: core: Support accessing core tags
  2022-06-28 14:18     ` AKASHI Takahiro
@ 2022-07-05 12:39       ` Tom Rini
  2022-07-05 13:27         ` Simon Glass
  0 siblings, 1 reply; 25+ messages in thread
From: Tom Rini @ 2022-07-05 12:39 UTC (permalink / raw)
  To: AKASHI Takahiro, Simon Glass, U-Boot Mailing List,
	U-Boot Custodians, Marek Vasut, Pavel Herrmann

[-- Attachment #1: Type: text/plain, Size: 1679 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 6/9] dm: core: Support accessing core tags
  2022-07-05 12:39       ` Tom Rini
@ 2022-07-05 13:27         ` Simon Glass
  2022-07-06  1:27           ` AKASHI Takahiro
  0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2022-07-05 13:27 UTC (permalink / raw)
  To: Tom Rini
  Cc: AKASHI Takahiro, U-Boot Mailing List, U-Boot Custodians,
	Marek Vasut, Pavel Herrmann

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 6/9] dm: core: Support accessing core tags
  2022-07-05 13:27         ` Simon Glass
@ 2022-07-06  1:27           ` AKASHI Takahiro
  2022-07-06  8:38             ` Simon Glass
  0 siblings, 1 reply; 25+ messages in thread
From: AKASHI Takahiro @ 2022-07-06  1:27 UTC (permalink / raw)
  To: Simon Glass
  Cc: Tom Rini, U-Boot Mailing List, U-Boot Custodians, Marek Vasut,
	Pavel Herrmann

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 6/9] dm: core: Support accessing core tags
  2022-07-06  1:27           ` AKASHI Takahiro
@ 2022-07-06  8:38             ` Simon Glass
  0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-07-06  8:38 UTC (permalink / raw)
  To: AKASHI Takahiro, Simon Glass, Tom Rini, U-Boot Mailing List,
	U-Boot Custodians, Marek Vasut, Pavel Herrmann

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2022-07-06  8:38 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
2022-05-08 10:39 ` [PATCH 1/9] dm: core: Rename dm_dump_all() Simon Glass
2022-05-08 10:39 ` [PATCH 2/9] dm: core: Sort dm subcommands Simon Glass
2022-05-08 10:39 ` [PATCH 3/9] dm: core: Fix addresses in the dm static command Simon Glass
2022-05-08 10:39 ` [PATCH 4/9] dm: core: Add documentation for the dm command Simon Glass
2022-05-08 10:39 ` [PATCH 5/9] dm: core: Switch the testbus driver to use a new struct Simon Glass
2022-05-08 10:39 ` [PATCH 6/9] dm: core: Support accessing core tags Simon Glass
2022-05-09  4:52   ` AKASHI Takahiro
2022-06-28 13:37   ` Simon Glass
2022-06-28 14:18     ` AKASHI Takahiro
2022-07-05 12:39       ` Tom Rini
2022-07-05 13:27         ` Simon Glass
2022-07-06  1:27           ` AKASHI Takahiro
2022-07-06  8:38             ` Simon Glass
2022-05-08 10:39 ` [PATCH 7/9] dm: core: Add a way to collect memory usage Simon Glass
2022-05-08 10:39 ` [PATCH 8/9] dm: core: Add a command to show driver model statistics Simon Glass
2022-05-08 10:39 ` [PATCH 9/9] dm: spl: Allow SPL to show memory usage Simon Glass
2022-06-28 13:37 ` Simon Glass
2022-06-28 13:37 ` [PATCH 8/9] dm: core: Add a command to show driver model statistics Simon Glass
2022-06-28 13:37 ` [PATCH 7/9] dm: core: Add a way to collect memory usage Simon Glass
2022-06-28 13:37 ` [PATCH 5/9] dm: core: Switch the testbus driver to use a new struct Simon Glass
2022-06-28 13:37 ` [PATCH 4/9] dm: core: Add documentation for the dm command Simon Glass
2022-06-28 13:37 ` [PATCH 3/9] dm: core: Fix addresses in the dm static command Simon Glass
2022-06-28 13:38 ` [PATCH 2/9] dm: core: Sort dm subcommands Simon Glass
2022-06-28 13:38 ` [PATCH 1/9] dm: core: Rename dm_dump_all() Simon Glass

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).