Hi Sakari,
Thank you for the patch. This is a feature I've often thought would be useful.
On Tuesday 13 Sep 2016 11:28:16 Sakari Ailus wrote:
> Add an optional argument to the -p option that allows printing all
> information related to a given entity. This may be handy sometimes if only
> a single entity is of interest and there are many entities.
Would it make sense to instead reuse the -e argument ? If both -p and -e are
specified, print entity information for the entity referenced by -e. If only -
p or -e are specified, operate as we do today.
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> utils/media-ctl/media-ctl.c | 26 +++++++++++++++-----------
> utils/media-ctl/options.c | 9 ++++++---
> utils/media-ctl/options.h | 1 +
> 3 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
> index 0499008..fdd2449 100644
> --- a/utils/media-ctl/media-ctl.c
> +++ b/utils/media-ctl/media-ctl.c
> @@ -504,14 +504,6 @@ static void media_print_topology_text(struct
> media_device *media) media, media_get_entity(media, i));
> }
>
> -void media_print_topology(struct media_device *media, int dot)
> -{
> - if (dot)
> - media_print_topology_dot(media);
> - else
> - media_print_topology_text(media);
> -}
> -
> int main(int argc, char **argv)
> {
> struct media_device *media;
> @@ -611,9 +603,21 @@ int main(int argc, char **argv)
> }
> }
>
> - if (media_opts.print || media_opts.print_dot) {
> - media_print_topology(media, media_opts.print_dot);
> - printf("\n");
> + if (media_opts.print_dot) {
> + media_print_topology_dot(media);
> + } else if (media_opts.print_entity) {
> + struct media_entity *entity = NULL;
> +
> + entity = media_get_entity_by_name(media,
> + media_opts.print_entity);
> + if (entity == NULL) {
> + printf("Entity '%s' not found\n",
> + media_opts.print_entity);
> + goto out;
> + }
> + media_print_topology_text_entity(media, entity);
> + } else if (media_opts.print) {
> + media_print_topology_text(media);
> }
>
> if (media_opts.reset) {
> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
> index a288a1b..3352626 100644
> --- a/utils/media-ctl/options.c
> +++ b/utils/media-ctl/options.c
> @@ -51,7 +51,9 @@ static void usage(const char *argv0)
> printf("-i, --interactive Modify links interactively\n");
> printf("-l, --links links Comma-separated list of link
> descriptors to setup\n");
> printf(" --known-mbus-fmts List known media bus formats and
> their numeric values\n");
> - printf("-p, --print-topology Print the device topology\n");
> + printf("-p, --print-topology [name] Print the device topology\n");
> + printf(" If entity name is specified,
> information to that entity\n");
> + printf(" only is printed.\n");
I'd wrap this to make lines a bit shorter, and perhaps write it as
printf("-p, --print-topology [name]\n);
printf(" Print the device topology. If name\n");
printf(" is specified, print information for\n");
printf(" the named entity only.\n);
Or if you go by my proposal above,
printf("-p, --print-topology Print the device topology. If an entity\n");
printf(" is specified through the -e option, print\n");
printf(" information for that entity only.\n);
> printf(" --print-dot Print the device topology as a dot
graph\n");
> printf("-r, --reset Reset all links to inactive\n");
> printf("-v, --verbose Be verbose\n");
> @@ -109,7 +111,7 @@ static struct option opts[] = {
> {"links", 1, 0, 'l'},
> {"known-mbus-fmts", 0, 0, OPT_LIST_KNOWN_MBUS_FMTS},
> {"print-dot", 0, 0, OPT_PRINT_DOT},
> - {"print-topology", 0, 0, 'p'},
> + {"print-topology", 2, 0, 'p'},
> {"reset", 0, 0, 'r'},
> {"verbose", 0, 0, 'v'},
> { },
> @@ -146,7 +148,7 @@ int parse_cmdline(int argc, char **argv)
> }
>
> /* parse options */
> - while ((opt = getopt_long(argc, argv, "d:e:f:hil:prvV:",
> + while ((opt = getopt_long(argc, argv, "d:e:f:hil:p::rvV:",
> opts, NULL)) != -1) {
> switch (opt) {
> case 'd':
> @@ -182,6 +184,7 @@ int parse_cmdline(int argc, char **argv)
>
> case 'p':
> media_opts.print = 1;
> + media_opts.print_entity = optarg;
> break;
>
> case 'r':
> diff --git a/utils/media-ctl/options.h b/utils/media-ctl/options.h
> index 9b5f314..ff9dfdf 100644
> --- a/utils/media-ctl/options.h
> +++ b/utils/media-ctl/options.h
> @@ -30,6 +30,7 @@ struct media_options
> print_dot:1,
> reset:1,
> verbose:1;
> + const char *print_entity;
> const char *entity;
> const char *formats;
> const char *links;
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
