[v4l-utils PATCH 0/2] Print information on given entity only

[v4l-utils PATCH 0/2] Print information on given entity only

[Sakari Ailus]

Hi,

These patches add the ability to media-ctl to print information on a given
entity only. That's sometimes handy.

-- 
Regards,
Sakari

[v4l-utils PATCH 1/2] media-ctl: Split off printing information related to a single entity

[Sakari Ailus]

As a result, a function that can be used to print information on a given
entity only is provided.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 utils/media-ctl/media-ctl.c | 93 ++++++++++++++++++++++++---------------------
 1 file changed, 49 insertions(+), 44 deletions(-)

diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
index 2f049c6..0499008 100644
--- a/utils/media-ctl/media-ctl.c
+++ b/utils/media-ctl/media-ctl.c
@@ -436,67 +436,72 @@ static void media_print_pad_text(struct media_entity *entity,
 		v4l2_subdev_print_subdev_dv(entity);
 }
 
-static void media_print_topology_text(struct media_device *media)
+static void media_print_topology_text_entity(struct media_device *media,
+					     struct media_entity *entity)
 {
 	static const struct flag_name link_flags[] = {
 		{ MEDIA_LNK_FL_ENABLED, "ENABLED" },
 		{ MEDIA_LNK_FL_IMMUTABLE, "IMMUTABLE" },
 		{ MEDIA_LNK_FL_DYNAMIC, "DYNAMIC" },
 	};
-
-	unsigned int nents = media_get_entities_count(media);
-	unsigned int i, j, k;
+	const struct media_entity_desc *info = media_entity_get_info(entity);
+	const char *devname = media_entity_get_devname(entity);
+	unsigned int num_links = media_entity_get_links_count(entity);
+	unsigned int j, k;
 	unsigned int padding;
 
-	printf("Device topology\n");
-
-	for (i = 0; i < nents; ++i) {
-		struct media_entity *entity = media_get_entity(media, i);
-		const struct media_entity_desc *info = media_entity_get_info(entity);
-		const char *devname = media_entity_get_devname(entity);
-		unsigned int num_links = media_entity_get_links_count(entity);
-
-		padding = printf("- entity %u: ", info->id);
-		printf("%s (%u pad%s, %u link%s)\n", info->name,
-			info->pads, info->pads > 1 ? "s" : "",
-			num_links, num_links > 1 ? "s" : "");
-		printf("%*ctype %s subtype %s flags %x\n", padding, ' ',
-			media_entity_type_to_string(info->type),
-			media_entity_subtype_to_string(info->type),
-			info->flags);
-		if (devname)
-			printf("%*cdevice node name %s\n", padding, ' ', devname);
+	padding = printf("- entity %u: ", info->id);
+	printf("%s (%u pad%s, %u link%s)\n", info->name,
+	       info->pads, info->pads > 1 ? "s" : "",
+	       num_links, num_links > 1 ? "s" : "");
+	printf("%*ctype %s subtype %s flags %x\n", padding, ' ',
+	       media_entity_type_to_string(info->type),
+	       media_entity_subtype_to_string(info->type),
+	       info->flags);
+	if (devname)
+		printf("%*cdevice node name %s\n", padding, ' ', devname);
 
-		for (j = 0; j < info->pads; j++) {
-			const struct media_pad *pad = media_entity_get_pad(entity, j);
+	for (j = 0; j < info->pads; j++) {
+		const struct media_pad *pad = media_entity_get_pad(entity, j);
 
-			printf("\tpad%u: %s\n", j, media_pad_type_to_string(pad->flags));
+		printf("\tpad%u: %s\n", j, media_pad_type_to_string(pad->flags));
 
-			media_print_pad_text(entity, pad);
+		media_print_pad_text(entity, pad);
 
-			for (k = 0; k < num_links; k++) {
-				const struct media_link *link = media_entity_get_link(entity, k);
-				const struct media_pad *source = link->source;
-				const struct media_pad *sink = link->sink;
+		for (k = 0; k < num_links; k++) {
+			const struct media_link *link = media_entity_get_link(entity, k);
+			const struct media_pad *source = link->source;
+			const struct media_pad *sink = link->sink;
 
-				if (source->entity == entity && source->index == j)
-					printf("\t\t-> \"%s\":%u [",
-						media_entity_get_info(sink->entity)->name,
-						sink->index);
-				else if (sink->entity == entity && sink->index == j)
-					printf("\t\t<- \"%s\":%u [",
-						media_entity_get_info(source->entity)->name,
-						source->index);
-				else
-					continue;
+			if (source->entity == entity && source->index == j)
+				printf("\t\t-> \"%s\":%u [",
+				       media_entity_get_info(sink->entity)->name,
+				       sink->index);
+			else if (sink->entity == entity && sink->index == j)
+				printf("\t\t<- \"%s\":%u [",
+				       media_entity_get_info(source->entity)->name,
+				       source->index);
+			else
+				continue;
 
-				print_flags(link_flags, ARRAY_SIZE(link_flags), link->flags);
+			print_flags(link_flags, ARRAY_SIZE(link_flags), link->flags);
 
-				printf("]\n");
-			}
+			printf("]\n");
 		}
-		printf("\n");
 	}
+	printf("\n");
+}
+
+static void media_print_topology_text(struct media_device *media)
+{
+	unsigned int nents = media_get_entities_count(media);
+	unsigned int i;
+
+	printf("Device topology\n");
+
+	for (i = 0; i < nents; ++i)
+		media_print_topology_text_entity(
+			media, media_get_entity(media, i));
 }
 
 void media_print_topology(struct media_device *media, int dot)
-- 
2.7.4

Re: [v4l-utils PATCH 1/2] media-ctl: Split off printing information related to a single entity

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Tuesday 13 Sep 2016 11:28:15 Sakari Ailus wrote:
> As a result, a function that can be used to print information on a given
> entity only is provided.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  utils/media-ctl/media-ctl.c | 93 +++++++++++++++++++++---------------------
>  1 file changed, 49 insertions(+), 44 deletions(-)

-- 
Regards,

Laurent Pinchart

[v4l-utils PATCH 2/2] media-ctl: Print information related to a single entity

[Sakari Ailus]

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.

Signed-off-by: Sakari Ailus <sakari.ailus@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");
 	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;
-- 
2.7.4

Re: [v4l-utils PATCH 2/2] media-ctl: Print information related to a single entity

[Laurent Pinchart]

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@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

[v4l-utils PATCH v1.1 2/2] media-ctl: Print information related to a single entity

[Sakari Ailus]

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.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 utils/media-ctl/media-ctl.c | 33 +++++++++++++++------------------
 utils/media-ctl/options.c   |  2 ++
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
index 0499008..109cc11 100644
--- a/utils/media-ctl/media-ctl.c
+++ b/utils/media-ctl/media-ctl.c
@@ -504,19 +504,11 @@ 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;
+	struct media_entity *entity = NULL;
 	int ret = -1;
-	const char *devname;
 
 	if (parse_cmdline(argc, argv))
 		return EXIT_FAILURE;
@@ -562,17 +554,11 @@ int main(int argc, char **argv)
 	}
 
 	if (media_opts.entity) {
-		struct media_entity *entity;
-
 		entity = media_get_entity_by_name(media, media_opts.entity);
 		if (entity == NULL) {
 			printf("Entity '%s' not found\n", media_opts.entity);
 			goto out;
 		}
-
-		devname = media_entity_get_devname(entity);
-		if (devname)
-			printf("%s\n", devname);
 	}
 
 	if (media_opts.fmt_pad) {
@@ -611,9 +597,20 @@ 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) {
+		if (entity) {
+			media_print_topology_text_entity(media, entity);
+		} else {
+			media_print_topology_text(media);
+		}
+	} else if (entity) {
+		const char *devname;
+
+		devname = media_entity_get_devname(entity);
+		if (devname)
+			printf("%s\n", devname);
 	}
 
 	if (media_opts.reset) {
diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
index a288a1b..304a86c 100644
--- a/utils/media-ctl/options.c
+++ b/utils/media-ctl/options.c
@@ -52,6 +52,8 @@ static void usage(const char *argv0)
 	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("			If entity name is specified using -e option, information\n");
+	printf("			related to that entity only is printed.\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");
-- 
2.7.4

[v4l-utils PATCH v1.2 2/2] media-ctl: Print information related to a single entity

[Sakari Ailus]

Add a possibility to printing all information related to a given entity by
using both -p and -e options. This may be handy sometimes if only a single
entity is of interest and there are many entities.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
Fixed the commit message as well.

 utils/media-ctl/media-ctl.c | 33 +++++++++++++++------------------
 utils/media-ctl/options.c   |  2 ++
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
index 0499008..109cc11 100644
--- a/utils/media-ctl/media-ctl.c
+++ b/utils/media-ctl/media-ctl.c
@@ -504,19 +504,11 @@ 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;
+	struct media_entity *entity = NULL;
 	int ret = -1;
-	const char *devname;
 
 	if (parse_cmdline(argc, argv))
 		return EXIT_FAILURE;
@@ -562,17 +554,11 @@ int main(int argc, char **argv)
 	}
 
 	if (media_opts.entity) {
-		struct media_entity *entity;
-
 		entity = media_get_entity_by_name(media, media_opts.entity);
 		if (entity == NULL) {
 			printf("Entity '%s' not found\n", media_opts.entity);
 			goto out;
 		}
-
-		devname = media_entity_get_devname(entity);
-		if (devname)
-			printf("%s\n", devname);
 	}
 
 	if (media_opts.fmt_pad) {
@@ -611,9 +597,20 @@ 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) {
+		if (entity) {
+			media_print_topology_text_entity(media, entity);
+		} else {
+			media_print_topology_text(media);
+		}
+	} else if (entity) {
+		const char *devname;
+
+		devname = media_entity_get_devname(entity);
+		if (devname)
+			printf("%s\n", devname);
 	}
 
 	if (media_opts.reset) {
diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
index a288a1b..304a86c 100644
--- a/utils/media-ctl/options.c
+++ b/utils/media-ctl/options.c
@@ -52,6 +52,8 @@ static void usage(const char *argv0)
 	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("			If entity name is specified using -e option, information\n");
+	printf("			related to that entity only is printed.\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");
-- 
2.7.4

Re: [v4l-utils PATCH v1.2 2/2] media-ctl: Print information related to a single entity

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Wednesday 14 Sep 2016 17:29:39 Sakari Ailus wrote:
> Add a possibility to printing all information related to a given entity by
> using both -p and -e options. This may be handy sometimes if only a single
> entity is of interest and there are many entities.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> Fixed the commit message as well.
> 
>  utils/media-ctl/media-ctl.c | 33 +++++++++++++++------------------
>  utils/media-ctl/options.c   |  2 ++
>  2 files changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
> index 0499008..109cc11 100644
> --- a/utils/media-ctl/media-ctl.c
> +++ b/utils/media-ctl/media-ctl.c
> @@ -504,19 +504,11 @@ 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;
> +	struct media_entity *entity = NULL;
>  	int ret = -1;
> -	const char *devname;
> 
>  	if (parse_cmdline(argc, argv))
>  		return EXIT_FAILURE;
> @@ -562,17 +554,11 @@ int main(int argc, char **argv)
>  	}
> 
>  	if (media_opts.entity) {
> -		struct media_entity *entity;
> -
>  		entity = media_get_entity_by_name(media, media_opts.entity);
>  		if (entity == NULL) {
>  			printf("Entity '%s' not found\n", media_opts.entity);
>  			goto out;
>  		}
> -
> -		devname = media_entity_get_devname(entity);
> -		if (devname)
> -			printf("%s\n", devname);
>  	}
> 
>  	if (media_opts.fmt_pad) {
> @@ -611,9 +597,20 @@ 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) {
> +		if (entity) {
> +			media_print_topology_text_entity(media, entity);
> +		} else {
> +			media_print_topology_text(media);
> +		}

You could remove the curly braces here.

> +	} else if (entity) {
> +		const char *devname;
> +
> +		devname = media_entity_get_devname(entity);
> +		if (devname)
> +			printf("%s\n", devname);
>  	}
> 
>  	if (media_opts.reset) {
> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
> index a288a1b..304a86c 100644
> --- a/utils/media-ctl/options.c
> +++ b/utils/media-ctl/options.c
> @@ -52,6 +52,8 @@ static void usage(const char *argv0)
>  	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("			If entity name is specified using -e 
option, information\n");
> +	printf("			related to that entity only is 
printed.\n");

Nitpicking, was anything wrong with

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);

? I think the help text looks more natural when using articles :-)

>  	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");

-- 
Regards,

Laurent Pinchart

Re: [v4l-utils PATCH v1.2 2/2] media-ctl: Print information related to a single entity

[Sakari Ailus]

On 09/15/16 01:05, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Wednesday 14 Sep 2016 17:29:39 Sakari Ailus wrote:
>> Add a possibility to printing all information related to a given entity by
>> using both -p and -e options. This may be handy sometimes if only a single
>> entity is of interest and there are many entities.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> ---
>> Fixed the commit message as well.
>>
>>  utils/media-ctl/media-ctl.c | 33 +++++++++++++++------------------
>>  utils/media-ctl/options.c   |  2 ++
>>  2 files changed, 17 insertions(+), 18 deletions(-)
>>
>> diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
>> index 0499008..109cc11 100644
>> --- a/utils/media-ctl/media-ctl.c
>> +++ b/utils/media-ctl/media-ctl.c
>> @@ -504,19 +504,11 @@ 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;
>> +	struct media_entity *entity = NULL;
>>  	int ret = -1;
>> -	const char *devname;
>>
>>  	if (parse_cmdline(argc, argv))
>>  		return EXIT_FAILURE;
>> @@ -562,17 +554,11 @@ int main(int argc, char **argv)
>>  	}
>>
>>  	if (media_opts.entity) {
>> -		struct media_entity *entity;
>> -
>>  		entity = media_get_entity_by_name(media, media_opts.entity);
>>  		if (entity == NULL) {
>>  			printf("Entity '%s' not found\n", media_opts.entity);
>>  			goto out;
>>  		}
>> -
>> -		devname = media_entity_get_devname(entity);
>> -		if (devname)
>> -			printf("%s\n", devname);
>>  	}
>>
>>  	if (media_opts.fmt_pad) {
>> @@ -611,9 +597,20 @@ 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) {
>> +		if (entity) {
>> +			media_print_topology_text_entity(media, entity);
>> +		} else {
>> +			media_print_topology_text(media);
>> +		}
> 
> You could remove the curly braces here.

Will fix.

> 
>> +	} else if (entity) {
>> +		const char *devname;
>> +
>> +		devname = media_entity_get_devname(entity);
>> +		if (devname)
>> +			printf("%s\n", devname);
>>  	}
>>
>>  	if (media_opts.reset) {
>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
>> index a288a1b..304a86c 100644
>> --- a/utils/media-ctl/options.c
>> +++ b/utils/media-ctl/options.c
>> @@ -52,6 +52,8 @@ static void usage(const char *argv0)
>>  	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("			If entity name is specified using -e 
> option, information\n");
>> +	printf("			related to that entity only is 
> printed.\n");
> 
> Nitpicking, was anything wrong with
> 
> 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);

Not necessarily, but I missed that part of your reply. I'll replace it.

> 
> ? I think the help text looks more natural when using articles :-)

The message was still clear, wasn't it? :-D

> 
>>  	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");
> 


-- 
Cheers,

Sakari Ailus
sakari.ailus@linux.intel.com

[v4l-utils PATCH v1.3 2/2] media-ctl: Print information related to a single entity

[Sakari Ailus]

Add a possibility to printing all information related to a given entity by
using both -p and -e options. This may be handy sometimes if only a single
entity is of interest and there are many entities.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 utils/media-ctl/media-ctl.c | 32 ++++++++++++++------------------
 utils/media-ctl/options.c   |  4 +++-
 2 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
index 0499008..b60d297 100644
--- a/utils/media-ctl/media-ctl.c
+++ b/utils/media-ctl/media-ctl.c
@@ -504,19 +504,11 @@ 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;
+	struct media_entity *entity = NULL;
 	int ret = -1;
-	const char *devname;
 
 	if (parse_cmdline(argc, argv))
 		return EXIT_FAILURE;
@@ -562,17 +554,11 @@ int main(int argc, char **argv)
 	}
 
 	if (media_opts.entity) {
-		struct media_entity *entity;
-
 		entity = media_get_entity_by_name(media, media_opts.entity);
 		if (entity == NULL) {
 			printf("Entity '%s' not found\n", media_opts.entity);
 			goto out;
 		}
-
-		devname = media_entity_get_devname(entity);
-		if (devname)
-			printf("%s\n", devname);
 	}
 
 	if (media_opts.fmt_pad) {
@@ -611,9 +597,19 @@ 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) {
+		if (entity)
+			media_print_topology_text_entity(media, entity);
+		else
+			media_print_topology_text(media);
+	} else if (entity) {
+		const char *devname;
+
+		devname = media_entity_get_devname(entity);
+		if (devname)
+			printf("%s\n", devname);
 	}
 
 	if (media_opts.reset) {
diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
index a288a1b..77d1a51 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	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");
-- 
2.7.4

Re: [v4l-utils PATCH v1.3 2/2] media-ctl: Print information related to a single entity

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Thursday 15 Sep 2016 09:40:39 Sakari Ailus wrote:
> Add a possibility to printing all information related to a given entity by
> using both -p and -e options. This may be handy sometimes if only a single
> entity is of interest and there are many entities.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  utils/media-ctl/media-ctl.c | 32 ++++++++++++++------------------
>  utils/media-ctl/options.c   |  4 +++-
>  2 files changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
> index 0499008..b60d297 100644
> --- a/utils/media-ctl/media-ctl.c
> +++ b/utils/media-ctl/media-ctl.c
> @@ -504,19 +504,11 @@ 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;
> +	struct media_entity *entity = NULL;
>  	int ret = -1;
> -	const char *devname;
> 
>  	if (parse_cmdline(argc, argv))
>  		return EXIT_FAILURE;
> @@ -562,17 +554,11 @@ int main(int argc, char **argv)
>  	}
> 
>  	if (media_opts.entity) {
> -		struct media_entity *entity;
> -
>  		entity = media_get_entity_by_name(media, media_opts.entity);
>  		if (entity == NULL) {
>  			printf("Entity '%s' not found\n", media_opts.entity);
>  			goto out;
>  		}
> -
> -		devname = media_entity_get_devname(entity);
> -		if (devname)
> -			printf("%s\n", devname);
>  	}
> 
>  	if (media_opts.fmt_pad) {
> @@ -611,9 +597,19 @@ 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) {
> +		if (entity)
> +			media_print_topology_text_entity(media, entity);
> +		else
> +			media_print_topology_text(media);
> +	} else if (entity) {
> +		const char *devname;
> +
> +		devname = media_entity_get_devname(entity);
> +		if (devname)
> +			printf("%s\n", devname);
>  	}
> 
>  	if (media_opts.reset) {
> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
> index a288a1b..77d1a51 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	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");

-- 
Regards,

Laurent Pinchart

Re: [v4l-utils PATCH v1.3 2/2] media-ctl: Print information related to a single entity

[Sakari Ailus]

On 09/15/16 11:10, Laurent Pinchart wrote:
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

pushed to master.

-- 
Sakari Ailus
sakari.ailus@linux.intel.com