On Tue, Dec 14, 2021 at 05:01:23PM +0200, Sakari Ailus wrote:
> On Mon, Dec 13, 2021 at 11:28:47PM +0000, Daniel Scally wrote:
> > When iterating over the media graph, don't follow links that are not
> > pad-to-pad links.
> >
> > Signed-off-by: Daniel Scally <djrscally@xxxxxxxxx>
> > ---
> > Changes since the rfc:
> >
> > - new patch
> >
> > drivers/media/mc/mc-entity.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> > index d79eb88bc167..aeddc3f6310e 100644
> > --- a/drivers/media/mc/mc-entity.c
> > +++ b/drivers/media/mc/mc-entity.c
> > @@ -325,6 +325,14 @@ static void media_graph_walk_iter(struct media_graph *graph)
> >
> > link = list_entry(link_top(graph), typeof(*link), list);
> >
> > + /* If the link is not a pad-to-pad link, don't follow it */
>
> This comment should mention data links, not pad-to-pad links.
>
> Seems fine apart from this.
>
> > + if ((link->flags & MEDIA_LNK_FL_LINK_TYPE) != MEDIA_LNK_FL_DATA_LINK) {
> > + link_top(graph) = link_top(graph)->next;
> > + dev_dbg(entity->graph_obj.mdev->dev, "walk: skipping %s link\n",
> > + link_type(link));
I would drop the debug message. The other messages in this function can
be useful to figure out why graph walk doesn't behave like expected
(reporting, for instance, that a disabled link is not traversed), but I
don't think there's much value in indicating we're skipping non-data
links.
With these issues addressed,
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > + return;
> > + }
> > +
> > /* The link is not enabled so we do not follow. */
> > if (!(link->flags & MEDIA_LNK_FL_ENABLED)) {
> > link_top(graph) = link_top(graph)->next;
--
Regards,
Laurent Pinchart
