| Message ID | 1423748591-19402-1-git-send-email-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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; > } > }
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.
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; } }
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(-)