diff mbox series

[v4l-utils,1/8] utils: media-ctl: Print MUST_CONNECT pad flags

Message ID 20240402000033.4007-2-laurent.pinchart@ideasonboard.com (mailing list archive)
State New
Headers show
Series Support for the generic line-based metadata support | expand

Commit Message

Laurent Pinchart April 2, 2024, midnight UTC 
From: Sakari Ailus <sakari.ailus@linux.intel.com>

Print the MUST_CONNECT pad flag for each pad.

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

Comments

Laurent Pinchart April 3, 2024, 8:40 a.m. UTC | #1 
Hi Sakari,

Thank you for the patch.

On Tue, Apr 02, 2024 at 03:00:26AM +0300, Laurent Pinchart wrote:
> From: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> Print the MUST_CONNECT pad flag for each pad.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  utils/media-ctl/media-ctl.c | 50 +++++++++++++++++++++----------------
>  1 file changed, 28 insertions(+), 22 deletions(-)
> 
> diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
> index 2081f111f2db..1b40552253f1 100644
> --- a/utils/media-ctl/media-ctl.c
> +++ b/utils/media-ctl/media-ctl.c
> @@ -368,26 +368,6 @@ static const char *media_entity_subtype_to_string(unsigned type)
>  	}
>  }
>  
> -static const char *media_pad_type_to_string(unsigned flag)
> -{
> -	static const struct {
> -		__u32 flag;
> -		const char *name;
> -	} flags[] = {
> -		{ MEDIA_PAD_FL_SINK, "Sink" },
> -		{ MEDIA_PAD_FL_SOURCE, "Source" },
> -	};
> -
> -	unsigned int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(flags); i++) {
> -		if (flags[i].flag & flag)
> -			return flags[i].name;
> -	}
> -
> -	return "Unknown";
> -}
> -
>  static void media_print_topology_dot(struct media_device *media)
>  {
>  	unsigned int nents = media_get_entities_count(media);
> @@ -525,6 +505,25 @@ static void media_print_pad_text(struct media_entity *entity,
>  		v4l2_subdev_print_subdev_dv(entity);
>  }
>  
> +static unsigned int weight(uint32_t word)
> +{
> +	unsigned int w = 0, i;
> +
> +	for (i = 0; i < sizeof(word) << 3; i++, word >>= 1)
> +		w += word & 1U;
> +
> +	return w;
> +}
> +
> +static const char *comma(uint32_t flag, uint32_t prev_flags, uint32_t flags)
> +{
> +	static const char *empty = "", *comma = ", ";
> +	if (!(flag & flags))
> +		return empty;
> +
> +	return weight(prev_flags & flags) ? comma : empty;

Unless I'm mistaken, we can write this

	return prev_flags & flags ? comma : empty;

and drop the weight function.

> +}
> +
>  static void media_print_topology_text_entity(struct media_device *media,
>  					     struct media_entity *entity)
>  {
> @@ -567,8 +566,15 @@ static void media_print_topology_text_entity(struct media_device *media,
>  	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%s%s%s%s\n", j,
> +		       pad->flags & MEDIA_PAD_FL_SINK ? "Sink" : "",
> +		       comma(MEDIA_PAD_FL_SOURCE, MEDIA_PAD_FL_SINK,
> +			     pad->flags),
> +		       pad->flags & MEDIA_PAD_FL_SOURCE ? "Source" : "",
> +		       comma(MEDIA_PAD_FL_MUST_CONNECT,
> +			     MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE,
> +			     pad->flags),
> +		       pad->flags & MEDIA_PAD_FL_MUST_CONNECT ? "Must connect" : "");

To be honest, this looks overly complicated. How about printing the
flags with a loop ?

>  		media_print_pad_text(entity, pad, routes, num_routes);
>  
>  		for (k = 0; k < num_links; k++) {
Tomi Valkeinen April 3, 2024, 8:43 a.m. UTC | #2 
On 03/04/2024 11:40, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Tue, Apr 02, 2024 at 03:00:26AM +0300, Laurent Pinchart wrote:
>> From: Sakari Ailus <sakari.ailus@linux.intel.com>
>>
>> Print the MUST_CONNECT pad flag for each pad.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> ---
>>   utils/media-ctl/media-ctl.c | 50 +++++++++++++++++++++----------------
>>   1 file changed, 28 insertions(+), 22 deletions(-)
>>
>> diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
>> index 2081f111f2db..1b40552253f1 100644
>> --- a/utils/media-ctl/media-ctl.c
>> +++ b/utils/media-ctl/media-ctl.c
>> @@ -368,26 +368,6 @@ static const char *media_entity_subtype_to_string(unsigned type)
>>   	}
>>   }
>>   
>> -static const char *media_pad_type_to_string(unsigned flag)
>> -{
>> -	static const struct {
>> -		__u32 flag;
>> -		const char *name;
>> -	} flags[] = {
>> -		{ MEDIA_PAD_FL_SINK, "Sink" },
>> -		{ MEDIA_PAD_FL_SOURCE, "Source" },
>> -	};
>> -
>> -	unsigned int i;
>> -
>> -	for (i = 0; i < ARRAY_SIZE(flags); i++) {
>> -		if (flags[i].flag & flag)
>> -			return flags[i].name;
>> -	}
>> -
>> -	return "Unknown";
>> -}
>> -
>>   static void media_print_topology_dot(struct media_device *media)
>>   {
>>   	unsigned int nents = media_get_entities_count(media);
>> @@ -525,6 +505,25 @@ static void media_print_pad_text(struct media_entity *entity,
>>   		v4l2_subdev_print_subdev_dv(entity);
>>   }
>>   
>> +static unsigned int weight(uint32_t word)
>> +{
>> +	unsigned int w = 0, i;
>> +
>> +	for (i = 0; i < sizeof(word) << 3; i++, word >>= 1)
>> +		w += word & 1U;
>> +
>> +	return w;
>> +}
>> +
>> +static const char *comma(uint32_t flag, uint32_t prev_flags, uint32_t flags)
>> +{
>> +	static const char *empty = "", *comma = ", ";
>> +	if (!(flag & flags))
>> +		return empty;
>> +
>> +	return weight(prev_flags & flags) ? comma : empty;
> 
> Unless I'm mistaken, we can write this
> 
> 	return prev_flags & flags ? comma : empty;
> 
> and drop the weight function.
> 
>> +}
>> +
>>   static void media_print_topology_text_entity(struct media_device *media,
>>   					     struct media_entity *entity)
>>   {
>> @@ -567,8 +566,15 @@ static void media_print_topology_text_entity(struct media_device *media,
>>   	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%s%s%s%s\n", j,
>> +		       pad->flags & MEDIA_PAD_FL_SINK ? "Sink" : "",
>> +		       comma(MEDIA_PAD_FL_SOURCE, MEDIA_PAD_FL_SINK,
>> +			     pad->flags),
>> +		       pad->flags & MEDIA_PAD_FL_SOURCE ? "Source" : "",
>> +		       comma(MEDIA_PAD_FL_MUST_CONNECT,
>> +			     MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE,
>> +			     pad->flags),
>> +		       pad->flags & MEDIA_PAD_FL_MUST_CONNECT ? "Must connect" : "");
> 
> To be honest, this looks overly complicated. How about printing the
> flags with a loop ?

I was just about to reply that this looks a bit too "smart" to me... I'd 
prefer just a simple loop here for readability's and maintainability's 
sake, even if it's not as optimal.

  Tomi
Hans Verkuil April 3, 2024, 8:56 a.m. UTC | #3 
On 03/04/2024 10:43, Tomi Valkeinen wrote:
> On 03/04/2024 11:40, Laurent Pinchart wrote:
>> Hi Sakari,
>>
>> Thank you for the patch.
>>
>> On Tue, Apr 02, 2024 at 03:00:26AM +0300, Laurent Pinchart wrote:
>>> From: Sakari Ailus <sakari.ailus@linux.intel.com>
>>>
>>> Print the MUST_CONNECT pad flag for each pad.
>>>
>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>> ---
>>>   utils/media-ctl/media-ctl.c | 50 +++++++++++++++++++++----------------
>>>   1 file changed, 28 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
>>> index 2081f111f2db..1b40552253f1 100644
>>> --- a/utils/media-ctl/media-ctl.c
>>> +++ b/utils/media-ctl/media-ctl.c
>>> @@ -368,26 +368,6 @@ static const char *media_entity_subtype_to_string(unsigned type)
>>>       }
>>>   }
>>>   -static const char *media_pad_type_to_string(unsigned flag)
>>> -{
>>> -    static const struct {
>>> -        __u32 flag;
>>> -        const char *name;
>>> -    } flags[] = {
>>> -        { MEDIA_PAD_FL_SINK, "Sink" },
>>> -        { MEDIA_PAD_FL_SOURCE, "Source" },
>>> -    };
>>> -
>>> -    unsigned int i;
>>> -
>>> -    for (i = 0; i < ARRAY_SIZE(flags); i++) {
>>> -        if (flags[i].flag & flag)
>>> -            return flags[i].name;
>>> -    }
>>> -
>>> -    return "Unknown";
>>> -}
>>> -
>>>   static void media_print_topology_dot(struct media_device *media)
>>>   {
>>>       unsigned int nents = media_get_entities_count(media);
>>> @@ -525,6 +505,25 @@ static void media_print_pad_text(struct media_entity *entity,
>>>           v4l2_subdev_print_subdev_dv(entity);
>>>   }
>>>   +static unsigned int weight(uint32_t word)
>>> +{
>>> +    unsigned int w = 0, i;
>>> +
>>> +    for (i = 0; i < sizeof(word) << 3; i++, word >>= 1)
>>> +        w += word & 1U;
>>> +
>>> +    return w;
>>> +}
>>> +
>>> +static const char *comma(uint32_t flag, uint32_t prev_flags, uint32_t flags)
>>> +{
>>> +    static const char *empty = "", *comma = ", ";
>>> +    if (!(flag & flags))
>>> +        return empty;
>>> +
>>> +    return weight(prev_flags & flags) ? comma : empty;
>>
>> Unless I'm mistaken, we can write this
>>
>>     return prev_flags & flags ? comma : empty;
>>
>> and drop the weight function.
>>
>>> +}
>>> +
>>>   static void media_print_topology_text_entity(struct media_device *media,
>>>                            struct media_entity *entity)
>>>   {
>>> @@ -567,8 +566,15 @@ static void media_print_topology_text_entity(struct media_device *media,
>>>       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%s%s%s%s\n", j,
>>> +               pad->flags & MEDIA_PAD_FL_SINK ? "Sink" : "",
>>> +               comma(MEDIA_PAD_FL_SOURCE, MEDIA_PAD_FL_SINK,
>>> +                 pad->flags),
>>> +               pad->flags & MEDIA_PAD_FL_SOURCE ? "Source" : "",
>>> +               comma(MEDIA_PAD_FL_MUST_CONNECT,
>>> +                 MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE,
>>> +                 pad->flags),
>>> +               pad->flags & MEDIA_PAD_FL_MUST_CONNECT ? "Must connect" : "");
>>
>> To be honest, this looks overly complicated. How about printing the
>> flags with a loop ?
> 
> I was just about to reply that this looks a bit too "smart" to me... I'd prefer just a simple loop here for readability's and maintainability's sake, even if it's not as optimal.

Same comment from me :-)

Regards,

	Hans
Sakari Ailus April 4, 2024, 8:17 p.m. UTC | #4 
Hi Hans, Tomi,

On Wed, Apr 03, 2024 at 10:56:55AM +0200, Hans Verkuil wrote:
> On 03/04/2024 10:43, Tomi Valkeinen wrote:
> > On 03/04/2024 11:40, Laurent Pinchart wrote:
> >> Hi Sakari,
> >>
> >> Thank you for the patch.
> >>
> >> On Tue, Apr 02, 2024 at 03:00:26AM +0300, Laurent Pinchart wrote:
> >>> From: Sakari Ailus <sakari.ailus@linux.intel.com>
> >>>
> >>> Print the MUST_CONNECT pad flag for each pad.
> >>>
> >>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >>> ---
> >>>   utils/media-ctl/media-ctl.c | 50 +++++++++++++++++++++----------------
> >>>   1 file changed, 28 insertions(+), 22 deletions(-)
> >>>
> >>> diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
> >>> index 2081f111f2db..1b40552253f1 100644
> >>> --- a/utils/media-ctl/media-ctl.c
> >>> +++ b/utils/media-ctl/media-ctl.c
> >>> @@ -368,26 +368,6 @@ static const char *media_entity_subtype_to_string(unsigned type)
> >>>       }
> >>>   }
> >>>   -static const char *media_pad_type_to_string(unsigned flag)
> >>> -{
> >>> -    static const struct {
> >>> -        __u32 flag;
> >>> -        const char *name;
> >>> -    } flags[] = {
> >>> -        { MEDIA_PAD_FL_SINK, "Sink" },
> >>> -        { MEDIA_PAD_FL_SOURCE, "Source" },
> >>> -    };
> >>> -
> >>> -    unsigned int i;
> >>> -
> >>> -    for (i = 0; i < ARRAY_SIZE(flags); i++) {
> >>> -        if (flags[i].flag & flag)
> >>> -            return flags[i].name;
> >>> -    }
> >>> -
> >>> -    return "Unknown";
> >>> -}
> >>> -
> >>>   static void media_print_topology_dot(struct media_device *media)
> >>>   {
> >>>       unsigned int nents = media_get_entities_count(media);
> >>> @@ -525,6 +505,25 @@ static void media_print_pad_text(struct media_entity *entity,
> >>>           v4l2_subdev_print_subdev_dv(entity);
> >>>   }
> >>>   +static unsigned int weight(uint32_t word)
> >>> +{
> >>> +    unsigned int w = 0, i;
> >>> +
> >>> +    for (i = 0; i < sizeof(word) << 3; i++, word >>= 1)
> >>> +        w += word & 1U;
> >>> +
> >>> +    return w;
> >>> +}
> >>> +
> >>> +static const char *comma(uint32_t flag, uint32_t prev_flags, uint32_t flags)
> >>> +{
> >>> +    static const char *empty = "", *comma = ", ";
> >>> +    if (!(flag & flags))
> >>> +        return empty;
> >>> +
> >>> +    return weight(prev_flags & flags) ? comma : empty;
> >>
> >> Unless I'm mistaken, we can write this
> >>
> >>     return prev_flags & flags ? comma : empty;
> >>
> >> and drop the weight function.

Correct. An earlier version of the patch used it and I forgot to remove it.

It should be possible to write this as:

static const char *comma(uint32_t flag, uint32_t prev_flags, uint32_t flags)
{
	return flag & flags && prev_flags & flags ? ", " : "";
}

This nicely demonstrates C operator precedence.

> >>
> >>> +}
> >>> +
> >>>   static void media_print_topology_text_entity(struct media_device *media,
> >>>                            struct media_entity *entity)
> >>>   {
> >>> @@ -567,8 +566,15 @@ static void media_print_topology_text_entity(struct media_device *media,
> >>>       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%s%s%s%s\n", j,
> >>> +               pad->flags & MEDIA_PAD_FL_SINK ? "Sink" : "",
> >>> +               comma(MEDIA_PAD_FL_SOURCE, MEDIA_PAD_FL_SINK,
> >>> +                 pad->flags),
> >>> +               pad->flags & MEDIA_PAD_FL_SOURCE ? "Source" : "",
> >>> +               comma(MEDIA_PAD_FL_MUST_CONNECT,
> >>> +                 MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE,
> >>> +                 pad->flags),
> >>> +               pad->flags & MEDIA_PAD_FL_MUST_CONNECT ? "Must connect" : "");
> >>
> >> To be honest, this looks overly complicated. How about printing the
> >> flags with a loop ?
> > 
> > I was just about to reply that this looks a bit too "smart" to me... I'd prefer just a simple loop here for readability's and maintainability's sake, even if it's not as optimal.
> 
> Same comment from me :-)

The above gets it done as a single printf call. Perhaps it doesn't matter
much if it doesn't though, this isn't printk. Still do note that checking
whether the commas will be printed isn't trivial so replacing this with a
loop isn't necessarily making the code notably simpler.
Laurent Pinchart April 4, 2024, 10:04 p.m. UTC | #5 
On Thu, Apr 04, 2024 at 08:17:09PM +0000, Sakari Ailus wrote:
> On Wed, Apr 03, 2024 at 10:56:55AM +0200, Hans Verkuil wrote:
> > On 03/04/2024 10:43, Tomi Valkeinen wrote:
> > > On 03/04/2024 11:40, Laurent Pinchart wrote:
> > >> On Tue, Apr 02, 2024 at 03:00:26AM +0300, Laurent Pinchart wrote:
> > >>> From: Sakari Ailus <sakari.ailus@linux.intel.com>
> > >>>
> > >>> Print the MUST_CONNECT pad flag for each pad.
> > >>>
> > >>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > >>> ---
> > >>>   utils/media-ctl/media-ctl.c | 50 +++++++++++++++++++++----------------
> > >>>   1 file changed, 28 insertions(+), 22 deletions(-)
> > >>>
> > >>> diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
> > >>> index 2081f111f2db..1b40552253f1 100644
> > >>> --- a/utils/media-ctl/media-ctl.c
> > >>> +++ b/utils/media-ctl/media-ctl.c
> > >>> @@ -368,26 +368,6 @@ static const char *media_entity_subtype_to_string(unsigned type)
> > >>>       }
> > >>>   }
> > >>>   -static const char *media_pad_type_to_string(unsigned flag)
> > >>> -{
> > >>> -    static const struct {
> > >>> -        __u32 flag;
> > >>> -        const char *name;
> > >>> -    } flags[] = {
> > >>> -        { MEDIA_PAD_FL_SINK, "Sink" },
> > >>> -        { MEDIA_PAD_FL_SOURCE, "Source" },
> > >>> -    };
> > >>> -
> > >>> -    unsigned int i;
> > >>> -
> > >>> -    for (i = 0; i < ARRAY_SIZE(flags); i++) {
> > >>> -        if (flags[i].flag & flag)
> > >>> -            return flags[i].name;
> > >>> -    }
> > >>> -
> > >>> -    return "Unknown";
> > >>> -}
> > >>> -
> > >>>   static void media_print_topology_dot(struct media_device *media)
> > >>>   {
> > >>>       unsigned int nents = media_get_entities_count(media);
> > >>> @@ -525,6 +505,25 @@ static void media_print_pad_text(struct media_entity *entity,
> > >>>           v4l2_subdev_print_subdev_dv(entity);
> > >>>   }
> > >>>   +static unsigned int weight(uint32_t word)
> > >>> +{
> > >>> +    unsigned int w = 0, i;
> > >>> +
> > >>> +    for (i = 0; i < sizeof(word) << 3; i++, word >>= 1)
> > >>> +        w += word & 1U;
> > >>> +
> > >>> +    return w;
> > >>> +}
> > >>> +
> > >>> +static const char *comma(uint32_t flag, uint32_t prev_flags, uint32_t flags)
> > >>> +{
> > >>> +    static const char *empty = "", *comma = ", ";
> > >>> +    if (!(flag & flags))
> > >>> +        return empty;
> > >>> +
> > >>> +    return weight(prev_flags & flags) ? comma : empty;
> > >>
> > >> Unless I'm mistaken, we can write this
> > >>
> > >>     return prev_flags & flags ? comma : empty;
> > >>
> > >> and drop the weight function.
> 
> Correct. An earlier version of the patch used it and I forgot to remove it.
> 
> It should be possible to write this as:
> 
> static const char *comma(uint32_t flag, uint32_t prev_flags, uint32_t flags)
> {
> 	return flag & flags && prev_flags & flags ? ", " : "";
> }
> 
> This nicely demonstrates C operator precedence.
> 
> > >>
> > >>> +}
> > >>> +
> > >>>   static void media_print_topology_text_entity(struct media_device *media,
> > >>>                            struct media_entity *entity)
> > >>>   {
> > >>> @@ -567,8 +566,15 @@ static void media_print_topology_text_entity(struct media_device *media,
> > >>>       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%s%s%s%s\n", j,
> > >>> +               pad->flags & MEDIA_PAD_FL_SINK ? "Sink" : "",
> > >>> +               comma(MEDIA_PAD_FL_SOURCE, MEDIA_PAD_FL_SINK,
> > >>> +                 pad->flags),
> > >>> +               pad->flags & MEDIA_PAD_FL_SOURCE ? "Source" : "",
> > >>> +               comma(MEDIA_PAD_FL_MUST_CONNECT,
> > >>> +                 MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE,
> > >>> +                 pad->flags),
> > >>> +               pad->flags & MEDIA_PAD_FL_MUST_CONNECT ? "Must connect" : "");
> > >>
> > >> To be honest, this looks overly complicated. How about printing the
> > >> flags with a loop ?
> > > 
> > > I was just about to reply that this looks a bit too "smart" to me... I'd prefer just a simple loop here for readability's and maintainability's sake, even if it's not as optimal.
> > 
> > Same comment from me :-)
> 
> The above gets it done as a single printf call. Perhaps it doesn't matter
> much if it doesn't though, this isn't printk. Still do note that checking
> whether the commas will be printed isn't trivial so replacing this with a
> loop isn't necessarily making the code notably simpler.

See https://lore.kernel.org/linux-media/20240404220312.8019-1-laurent.pinchart@ideasonboard.com
diff mbox series

Patch

diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
index 2081f111f2db..1b40552253f1 100644
--- a/utils/media-ctl/media-ctl.c
+++ b/utils/media-ctl/media-ctl.c
@@ -368,26 +368,6 @@  static const char *media_entity_subtype_to_string(unsigned type)
 	}
 }
 
-static const char *media_pad_type_to_string(unsigned flag)
-{
-	static const struct {
-		__u32 flag;
-		const char *name;
-	} flags[] = {
-		{ MEDIA_PAD_FL_SINK, "Sink" },
-		{ MEDIA_PAD_FL_SOURCE, "Source" },
-	};
-
-	unsigned int i;
-
-	for (i = 0; i < ARRAY_SIZE(flags); i++) {
-		if (flags[i].flag & flag)
-			return flags[i].name;
-	}
-
-	return "Unknown";
-}
-
 static void media_print_topology_dot(struct media_device *media)
 {
 	unsigned int nents = media_get_entities_count(media);
@@ -525,6 +505,25 @@  static void media_print_pad_text(struct media_entity *entity,
 		v4l2_subdev_print_subdev_dv(entity);
 }
 
+static unsigned int weight(uint32_t word)
+{
+	unsigned int w = 0, i;
+
+	for (i = 0; i < sizeof(word) << 3; i++, word >>= 1)
+		w += word & 1U;
+
+	return w;
+}
+
+static const char *comma(uint32_t flag, uint32_t prev_flags, uint32_t flags)
+{
+	static const char *empty = "", *comma = ", ";
+	if (!(flag & flags))
+		return empty;
+
+	return weight(prev_flags & flags) ? comma : empty;
+}
+
 static void media_print_topology_text_entity(struct media_device *media,
 					     struct media_entity *entity)
 {
@@ -567,8 +566,15 @@  static void media_print_topology_text_entity(struct media_device *media,
 	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%s%s%s%s\n", j,
+		       pad->flags & MEDIA_PAD_FL_SINK ? "Sink" : "",
+		       comma(MEDIA_PAD_FL_SOURCE, MEDIA_PAD_FL_SINK,
+			     pad->flags),
+		       pad->flags & MEDIA_PAD_FL_SOURCE ? "Source" : "",
+		       comma(MEDIA_PAD_FL_MUST_CONNECT,
+			     MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_SOURCE,
+			     pad->flags),
+		       pad->flags & MEDIA_PAD_FL_MUST_CONNECT ? "Must connect" : "");
 		media_print_pad_text(entity, pad, routes, num_routes);
 
 		for (k = 0; k < num_links; k++) {