[PATCH 3/3] cmd: dm: Fixed/Added DM driver listing subcommands

[Sean Anderson]

On 3/17/20 10:09 AM, Niel Fourie wrote:
> Renamed dm "drivers" subcommand to "compat" (as it listed
> compatibility strings) and prevent it from segfaulting when
> drivers have no of_match populated.
> 
> Added a new "drivers" subcommand to dump a list of all known DM
> drivers and for each, their uclass id, uclass driver and names of
> attached devices.
> 
> Added a new "static" subcommand to dump a list of DM drivers with
> statically defined platform data.
> 
> Signed-off-by: Niel Fourie <lusus at denx.de>
> CC: Simon Glass <sjg at chromium.org>
> ---
>  cmd/dm.c            | 24 ++++++++++++++++--
>  drivers/core/dump.c | 60 ++++++++++++++++++++++++++++++++++++++++++++-
>  include/dm/util.h   |  6 +++++
>  3 files changed, 87 insertions(+), 3 deletions(-)
> 
> diff --git a/cmd/dm.c b/cmd/dm.c
> index 108707c298..a17ef6a1bb 100644
> --- a/cmd/dm.c
> +++ b/cmd/dm.c
> @@ -48,11 +48,29 @@ static int do_dm_dump_drivers(cmd_tbl_t *cmdtp, int flag, int argc,
>  	return 0;
>  }
>  
> +static int do_dm_dump_driver_compat(cmd_tbl_t *cmdtp, int flag, int argc,
> +				    char * const argv[])
> +{
> +	dm_dump_driver_compat();
> +
> +	return 0;
> +}
> +
> +static int do_dm_dump_static_driver_info(cmd_tbl_t *cmdtp, int flag, int argc,
> +					 char * const argv[])
> +{
> +	dm_dump_static_driver_info();
> +
> +	return 0;
> +}
> +
>  static cmd_tbl_t test_commands[] = {
>  	U_BOOT_CMD_MKENT(tree, 0, 1, do_dm_dump_all, "", ""),
>  	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, "", ""),
> +	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, "", ""),
>  };
>  
>  static __maybe_unused void dm_reloc(void)
> @@ -91,8 +109,10 @@ static int do_dm(cmd_tbl_t *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 tree          Dump driver model tree ('*' = activated)\n"
>  	"dm uclass        Dump list of instances for each uclass\n"
>  	"dm devres        Dump list of device resources for each device\n"
> -	"dm drivers       Dump list of drivers and their compatible strings\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\n"
>  );
> diff --git a/drivers/core/dump.c b/drivers/core/dump.c
> index e73ebeabcc..a42bfb577f 100644
> --- a/drivers/core/dump.c
> +++ b/drivers/core/dump.c
> @@ -97,7 +97,7 @@ void dm_dump_uclass(void)
>  	}
>  }
>  
> -void dm_dump_drivers(void)
> +void dm_dump_driver_compat(void)
>  {
>  	struct driver *d = ll_entry_start(struct driver, driver);
>  	const int n_ents = ll_entry_count(struct driver, driver);
> @@ -107,6 +107,9 @@ void dm_dump_drivers(void)
>  	puts("Driver                Compatible\n");
>  	puts("--------------------------------\n");
>  	for (entry = d; entry < d + n_ents; entry++) {
> +		if (!entry->of_match) {
> +			continue;
> +		}

This should have been fixed in version 2 of the patch [1].

[1] https://patchwork.ozlabs.org/patch/1234460/

>  		for (match = entry->of_match; match->compatible; match++)
>  			printf("%-20.20s  %s\n",
>  			       match == entry->of_match ? entry->name : "",
> @@ -115,3 +118,58 @@ void dm_dump_drivers(void)
>  			printf("%-20.20s\n", entry->name);
>  	}
>  }
> +
> +void dm_dump_drivers(void)
> +{
> +	struct driver *d = ll_entry_start(struct driver, driver);
> +	const int n_ents = ll_entry_count(struct driver, driver);
> +	struct driver *entry;
> +	struct udevice *udev;
> +	struct uclass *uc;
> +	int i;
> +
> +	puts("Driver                    uid uclass               Devices\n");
> +	for (i = 0; i < 77; i++)
> +		putc('-');

Can you print these dashes in a way which makes it obvious that they are
the correct length? E.g. do something like

puts("Driver                    uid uclass               Devices\n");
puts("----------------------------------------------------------\n");

or

char header[] = "Driver                    uid uclass               Devices";
puts("%s\n", header);
for (i = 0; i < sizeof(header) - 1; i++)
	putc('-');

> +	putc('\n');
> +
> +	for (entry = d; entry < d + n_ents; entry++) {
> +		uclass_get(entry->id, &uc);
> +
> +		printf("%-25.25s %-3.3d %-20.20s ", entry->name, entry->id,
> +		       uc ? uc->uc_drv->name : "<no uclass>");
> +
> +		if (!uc) {
> +			puts("\n");
> +			continue;
> +		}
> +
> +		i = 0;
> +		uclass_foreach_dev(udev, uc) {
> +			if (udev->driver != entry)
> +				continue;
> +			if (i)
> +				printf("%-51.51s", "");
> +
> +			printf("%-25.25s\n", udev->name);
> +			i++;
> +		}
> +		if (!i)
> +			puts("<none>\n");
> +	}
> +}
> +
> +void dm_dump_static_driver_info(void)
> +{
> +	struct driver_info *drv = ll_entry_start(struct driver_info,
> +						 driver_info);
> +	const int n_ents = ll_entry_count(struct driver_info, driver_info);
> +	struct driver_info *entry;
> +
> +	puts("Driver               Address\n");
> +	puts("------------------------------\n");
> +	for (entry = drv; entry != drv + n_ents; entry++) {
> +		printf("%-20.20s @%08lx\n", entry->name,
> +		       (ulong)map_to_sysmem(entry->platdata));
> +	}
> +}

Just curious, what were you using this for?

> diff --git a/include/dm/util.h b/include/dm/util.h
> index 0ccb3fbadf..974347ce0b 100644
> --- a/include/dm/util.h
> +++ b/include/dm/util.h
> @@ -42,6 +42,12 @@ static inline void dm_dump_devres(void)
>  /* Dump out a list of drivers */
>  void dm_dump_drivers(void);
>  
> +/* Dump out a list with each driver's compatibility strings */
> +void dm_dump_driver_compat(void);
> +
> +/* Dump out a list of drivers with static platform data */
> +void dm_dump_static_driver_info(void);
> +
>  /**
>   * Check if an of node should be or was bound before relocation.
>   *
> 

--Sean