Re: [PATCH v2 1/1] media: Print information on failed link validation

2014-10-28 Thread Sakari Ailus
Hi Laurent,

On Wed, Oct 29, 2014 at 01:20:06AM +0200, Laurent Pinchart wrote:
> > ret = -EPIPE;
> > +   dev_dbg(entity->parent->dev,
> > +   "\"%s\":%u must be connected by an enabled 
> > link, error %d\n",
> > +   entity->name,
> > +   find_first_zero_bit(active, entity->num_pads),
> > +   ret);
> 
> Given that ret is always set to -EPIPE, I wouldn't print ", error %d".
> 
> Apart from that,
> 
> Acked-by: Laurent Pinchart 

Thanks for the ack. Good point as well, I'll fix that for v3.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
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


Re: [PATCH v2 1/1] media: Print information on failed link validation

2014-10-28 Thread Laurent Pinchart
Hi Sakari,

Thank you for the patch.

On Wednesday 29 October 2014 01:10:04 Sakari Ailus wrote:
> From: Sakari Ailus 
> 
> The Media controller doesn't tell much to the user in cases such as pipeline
> startup failure. The link validation is the most common media graph (or in
> V4L2's case, format) related reason for the failure. In more complex
> pipelines the reason may not always be obvious to the user, so point them
> to look at the right direction.
> 
> Signed-off-by: Sakari Ailus 
> ---
> since v1:
> - Fix language in the second message.
> 
>  drivers/media/media-entity.c |   13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 37c334e..a4030c3 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -279,8 +279,14 @@ __must_check int media_entity_pipeline_start(struct
> media_entity *entity, continue;
> 
>   ret = entity->ops->link_validate(link);
> - if (ret < 0 && ret != -ENOIOCTLCMD)
> + 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);
>   goto error;
> + }
>   }
> 
>   /* Either no links or validated links are fine. */
> @@ -288,6 +294,11 @@ __must_check int media_entity_pipeline_start(struct
> media_entity *entity,
> 
>   if (!bitmap_full(active, entity->num_pads)) {
>   ret = -EPIPE;
> + dev_dbg(entity->parent->dev,
> + "\"%s\":%u must be connected by an enabled 
> link, error %d\n",
> + entity->name,
> + find_first_zero_bit(active, entity->num_pads),
> + ret);

Given that ret is always set to -EPIPE, I wouldn't print ", error %d".

Apart from that,

Acked-by: Laurent Pinchart 

>   goto error;
>   }
>   }

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