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.ai...@linux.intel.com>
> ---
>  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to