[PATCH 1/1] media: Correctly notify about the failed pipeline validation

[PATCH 1/1] media: Correctly notify about the failed pipeline validation

[Sakari Ailus]

On the place of the source entity name, the sink entity name was printed.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-entity.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index defe4ac..d894481 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -283,9 +283,9 @@ __must_check int media_entity_pipeline_start(struct media_entity *entity,
 			if (ret < 0 && ret != -ENOIOCTLCMD) {
 				dev_dbg(entity->parent->dev,
 					"link validation failed for \"%s\":%u -> \"%s\":%u, error %d\n",
-					entity->name, link->source->index,
-					link->sink->entity->name,
-					link->sink->index, ret);
+					link->source->entity->name,
+					link->source->index,
+					entity->name, link->sink->index, ret);
 				goto error;
 			}
 		}
-- 
2.1.0.231.g7484e3b

Re: [PATCH 1/1] media: Correctly notify about the failed pipeline validation

[Mauro Carvalho Chehab]

Em Thu, 12 Feb 2015 15:43:11 +0200
Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:

> On the place of the source entity name, the sink entity name was printed.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/media-entity.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index defe4ac..d894481 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -283,9 +283,9 @@ __must_check int media_entity_pipeline_start(struct media_entity *entity,
>  			if (ret < 0 && ret != -ENOIOCTLCMD) {
>  				dev_dbg(entity->parent->dev,
>  					"link validation failed for \"%s\":%u -> \"%s\":%u, error %d\n",
> -					entity->name, link->source->index,
> -					link->sink->entity->name,
> -					link->sink->index, ret);
> +					link->source->entity->name,
> +					link->source->index,
> +					entity->name, link->sink->index, ret);

This should likely be reviewed by Laurent, but the above code
seems weird to me...

1) Why should it print the link source, instead of the sink?
I suspect that the code here should take into account the chosen
pad:

                        struct media_pad *pad = link->sink->entity == entity
                                                ? link->sink : link->source;

2) Assuming that your patch is right, why are you printing the
link->sink->index, instead of link->source->index?

Regards,
Mauro

Re: [PATCH 1/1] media: Correctly notify about the failed pipeline validation

[Sakari Ailus]

Hi Mauro,

Mauro Carvalho Chehab wrote:
> Em Thu, 12 Feb 2015 15:43:11 +0200
> Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:
> 
>> On the place of the source entity name, the sink entity name was printed.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> ---
>>  drivers/media/media-entity.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
>> index defe4ac..d894481 100644
>> --- a/drivers/media/media-entity.c
>> +++ b/drivers/media/media-entity.c
>> @@ -283,9 +283,9 @@ __must_check int media_entity_pipeline_start(struct media_entity *entity,
>>  			if (ret < 0 && ret != -ENOIOCTLCMD) {
>>  				dev_dbg(entity->parent->dev,
>>  					"link validation failed for \"%s\":%u -> \"%s\":%u, error %d\n",
>> -					entity->name, link->source->index,
>> -					link->sink->entity->name,
>> -					link->sink->index, ret);
>> +					link->source->entity->name,
>> +					link->source->index,
>> +					entity->name, link->sink->index, ret);
> 
> This should likely be reviewed by Laurent, but the above code
> seems weird to me...
> 
> 1) Why should it print the link source, instead of the sink?
> I suspect that the code here should take into account the chosen
> pad:
> 
>                         struct media_pad *pad = link->sink->entity == entity
>                                                 ? link->sink : link->source;

Link validation is only performed on sink pads. This is checked a few
lines above this, so the pad here is always the sink pad. Instead of
link->sink->index I could have used pad->index but the pad and thus the
integer value is the same.

> 
> 2) Assuming that your patch is right, why are you printing the
> link->sink->index, instead of link->source->index?

The source pad index is prited as well. The end result is, after the patch:

	source entity:source pad -> sink entity:sink pad

Before it was:

	sink entity:source pad -> sink entity:sink pad

Which indeed was wrong.

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

Re: [PATCH 1/1] media: Correctly notify about the failed pipeline validation

[Laurent Pinchart]

Hello Sakari,

Thank you for the patch.

On Thursday 12 February 2015 15:43:11 Sakari Ailus wrote:
> On the place of the source entity name, the sink entity name was printed.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

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

and applied to my tree. It's a bit late for v4.1, can it wait for v4.2 ?

> ---
>  drivers/media/media-entity.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index defe4ac..d894481 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -283,9 +283,9 @@ __must_check int media_entity_pipeline_start(struct
> media_entity *entity, if (ret < 0 && ret != -ENOIOCTLCMD) {
>  				dev_dbg(entity->parent->dev,
>  					"link validation failed for \"%s\":%u -> \"%s\":%u, error 
%d\n",
> -					entity->name, link->source->index,
> -					link->sink->entity->name,
> -					link->sink->index, ret);
> +					link->source->entity->name,
> +					link->source->index,
> +					entity->name, link->sink->index, ret);
>  				goto error;
>  			}
>  		}

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/1] media: Correctly notify about the failed pipeline validation

[Sakari Ailus]

Hi Laurent,

Laurent Pinchart wrote:
> Hello Sakari,
>
> Thank you for the patch.
>
> On Thursday 12 February 2015 15:43:11 Sakari Ailus wrote:
>> On the place of the source entity name, the sink entity name was printed.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> and applied to my tree. It's a bit late for v4.1, can it wait for v4.2 ?

Thanks!

v4.2 is fine. This is just a bug fix in a debug print. I wouldn't bother 
with stable or v4.1.

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