[v2,0/2] Add iterator for an entity's data links

There are a bunch of places in the kernel where code iterates over an entity's
links to perform some action. Those almost invariably have the implicit
assumption that those links are data links, which might not be true following
the introduction of ancillary links. Add a dedicated iterator that skips any
non data links for use instead, which will allow that assumption to hold true.

Daniel Scally (2):
  media: entity: Add iterator for entity data links
  media: entity: Use dedicated data link iterator

 drivers/media/mc/mc-entity.c | 22 +++++++++++++++++++---
 include/media/media-entity.h | 29 +++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 3 deletions(-)

Hi Daniel,

Thank you for the patch.

On Thu, Jul 07, 2022 at 11:47:32PM +0100, Daniel Scally wrote:
> Iterating over the links for an entity is a somewhat common need
> through the media subsystem, but generally the assumption is that
> they will all be data links. To meet that assumption add a new macro
> that iterates through an entity's links and skips non-data links.
> 
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> Changes since v1 (suggested by Jacopo and Laurent):
> 
> 	- Simplified __media_entity_next_link()
> 	- Added the link_type param to __media_entity_next_link()
> 	- Moved __media_entity_next_link() to mc-entity.c rather than
> 	  media-entity.h
> 
>  drivers/media/mc/mc-entity.c | 16 ++++++++++++++++
>  include/media/media-entity.h | 29 +++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index 11f5207f73aa..a247d5679930 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
> @@ -9,6 +9,7 @@
>   */
>  
>  #include <linux/bitmap.h>
> +#include <linux/list.h>
>  #include <linux/property.h>
>  #include <linux/slab.h>
>  #include <media/media-entity.h>
> @@ -1051,3 +1052,18 @@ struct media_link *media_create_ancillary_link(struct media_entity *primary,
>  	return link;
>  }
>  EXPORT_SYMBOL_GPL(media_create_ancillary_link);
> +
> +struct media_link *__media_entity_next_link(struct media_entity *entity,
> +					    struct media_link *link,
> +					    unsigned long link_type)
> +{
> +	link = link ? list_next_entry(link, list)
> +		    : list_first_entry(&entity->links, typeof(*link), list);
> +
> +	list_for_each_entry_from(link, &entity->links, list)
> +		if ((link->flags & MEDIA_LNK_FL_LINK_TYPE) == link_type)
> +			return link;
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(__media_entity_next_link);
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index a9a1c0ec5d1c..903c9368c50d 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -1140,4 +1140,33 @@ struct media_link *
>  media_create_ancillary_link(struct media_entity *primary,
>  			    struct media_entity *ancillary);
>  
> +/**
> + * __media_entity_next_link() - iterate through a &media_entity's links,

s/iterate/Iterate/
s/,$//

> + *
> + * @entity:	pointer to the &media_entity
> + * @link:	pointer to a &media_link to hold the iterated values
> + * @link_type:	one of the MEDIA_LNK_FL_LINK_TYPE flags
> + *
> + * Return the next link against an entity matching a specific link type. This
> + * allows iteration through an entity's links whilst guaranteeing all of the
> + * returned links are of the given type.
> + */
> +struct media_link *__media_entity_next_link(struct media_entity *entity,
> +					    struct media_link *link,
> +					    unsigned long link_type);
> +
> +/**
> + * for_each_media_entity_data_link() - Iterate through an entity's data links
> + *
> + * @entity:	pointer to the &media_entity
> + * @link:	pointer to a &media_link to hold the iterated values
> + *
> + * Iterate over a &media_entity's data links
> + */
> +#define for_each_media_entity_data_link(entity, link)			\
> +	for (link = __media_entity_next_link(entity, NULL,		\
> +					     MEDIA_LNK_FL_DATA_LINK);	\
> +	     link;							\
> +	     link = __media_entity_next_link(entity, link,		\
> +					     MEDIA_LNK_FL_DATA_LINK))

Missing one blank line here.

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

>  #endif

Hi Laurent, Daniel,

On Mon, Jul 11, 2022 at 07:17:34PM +0300, Laurent Pinchart wrote:
> Hi Daniel,
> 
> Thank you for the patch.
> 
> On Thu, Jul 07, 2022 at 11:47:32PM +0100, Daniel Scally wrote:
> > Iterating over the links for an entity is a somewhat common need
> > through the media subsystem, but generally the assumption is that
> > they will all be data links. To meet that assumption add a new macro
> > that iterates through an entity's links and skips non-data links.
> > 
> > Signed-off-by: Daniel Scally <djrscally@gmail.com>
> > ---
> > Changes since v1 (suggested by Jacopo and Laurent):
> > 
> > 	- Simplified __media_entity_next_link()
> > 	- Added the link_type param to __media_entity_next_link()
> > 	- Moved __media_entity_next_link() to mc-entity.c rather than
> > 	  media-entity.h
> > 
> >  drivers/media/mc/mc-entity.c | 16 ++++++++++++++++
> >  include/media/media-entity.h | 29 +++++++++++++++++++++++++++++
> >  2 files changed, 45 insertions(+)
> > 
> > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> > index 11f5207f73aa..a247d5679930 100644
> > --- a/drivers/media/mc/mc-entity.c
> > +++ b/drivers/media/mc/mc-entity.c
> > @@ -9,6 +9,7 @@
> >   */
> >  
> >  #include <linux/bitmap.h>
> > +#include <linux/list.h>
> >  #include <linux/property.h>
> >  #include <linux/slab.h>
> >  #include <media/media-entity.h>
> > @@ -1051,3 +1052,18 @@ struct media_link *media_create_ancillary_link(struct media_entity *primary,
> >  	return link;
> >  }
> >  EXPORT_SYMBOL_GPL(media_create_ancillary_link);
> > +
> > +struct media_link *__media_entity_next_link(struct media_entity *entity,
> > +					    struct media_link *link,
> > +					    unsigned long link_type)
> > +{
> > +	link = link ? list_next_entry(link, list)
> > +		    : list_first_entry(&entity->links, typeof(*link), list);
> > +
> > +	list_for_each_entry_from(link, &entity->links, list)
> > +		if ((link->flags & MEDIA_LNK_FL_LINK_TYPE) == link_type)
> > +			return link;
> > +
> > +	return NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(__media_entity_next_link);
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index a9a1c0ec5d1c..903c9368c50d 100644
> > --- a/include/media/media-entity.h
> > +++ b/include/media/media-entity.h
> > @@ -1140,4 +1140,33 @@ struct media_link *
> >  media_create_ancillary_link(struct media_entity *primary,
> >  			    struct media_entity *ancillary);
> >  
> > +/**
> > + * __media_entity_next_link() - iterate through a &media_entity's links,
> 
> s/iterate/Iterate/
> s/,$//
> 
> > + *
> > + * @entity:	pointer to the &media_entity
> > + * @link:	pointer to a &media_link to hold the iterated values
> > + * @link_type:	one of the MEDIA_LNK_FL_LINK_TYPE flags
> > + *
> > + * Return the next link against an entity matching a specific link type. This
> > + * allows iteration through an entity's links whilst guaranteeing all of the
> > + * returned links are of the given type.
> > + */
> > +struct media_link *__media_entity_next_link(struct media_entity *entity,
> > +					    struct media_link *link,
> > +					    unsigned long link_type);
> > +
> > +/**
> > + * for_each_media_entity_data_link() - Iterate through an entity's data links
> > + *
> > + * @entity:	pointer to the &media_entity
> > + * @link:	pointer to a &media_link to hold the iterated values
> > + *
> > + * Iterate over a &media_entity's data links
> > + */
> > +#define for_each_media_entity_data_link(entity, link)			\
> > +	for (link = __media_entity_next_link(entity, NULL,		\
> > +					     MEDIA_LNK_FL_DATA_LINK);	\
> > +	     link;							\
> > +	     link = __media_entity_next_link(entity, link,		\
> > +					     MEDIA_LNK_FL_DATA_LINK))
> 
> Missing one blank line here.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Added with these addressed.