diff mbox series

[v12,02/30] media: mc: entity: Add iterator helper for entity pads

Message ID 20220727103639.581567-3-tomi.valkeinen@ideasonboard.com (mailing list archive)
State Superseded
Headers
Series v4l: routing and streams support |

Commit Message

Tomi Valkeinen July 27, 2022, 10:36 a.m. UTC 
  From: Jacopo Mondi <jacopo+renesas@jmondi.org>

Add an iterator helper to easily cycle through all pads in an entity and
use it in media-entity and media-device code where appropriate.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/media/mc/mc-device.c | 13 ++++++-------
 drivers/media/mc/mc-entity.c | 11 ++++++-----
 include/media/media-entity.h | 12 ++++++++++++
 3 files changed, 24 insertions(+), 12 deletions(-)

Comments

Sakari Ailus July 30, 2022, 11:11 a.m. UTC | #1 
Moi,

Thanks for the update!

On Wed, Jul 27, 2022 at 01:36:11PM +0300, Tomi Valkeinen wrote:
> From: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> Add an iterator helper to easily cycle through all pads in an entity and
> use it in media-entity and media-device code where appropriate.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/media/mc/mc-device.c | 13 ++++++-------
>  drivers/media/mc/mc-entity.c | 11 ++++++-----
>  include/media/media-entity.h | 12 ++++++++++++
>  3 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
> index b8176a3b76d3..25020d58eb06 100644
> --- a/drivers/media/mc/mc-device.c
> +++ b/drivers/media/mc/mc-device.c
> @@ -581,7 +581,7 @@ static void __media_device_unregister_entity(struct media_entity *entity)
>  	struct media_device *mdev = entity->graph_obj.mdev;
>  	struct media_link *link, *tmp;
>  	struct media_interface *intf;
> -	unsigned int i;
> +	struct media_pad *iter;
>  
>  	ida_free(&mdev->entity_internal_idx, entity->internal_idx);
>  
> @@ -597,8 +597,8 @@ static void __media_device_unregister_entity(struct media_entity *entity)
>  	__media_entity_remove_links(entity);
>  
>  	/* Remove all pads that belong to this entity */
> -	for (i = 0; i < entity->num_pads; i++)
> -		media_gobj_destroy(&entity->pads[i].graph_obj);
> +	media_entity_for_each_pad(entity, iter)
> +		media_gobj_destroy(&iter->graph_obj);
>  
>  	/* Remove the entity */
>  	media_gobj_destroy(&entity->graph_obj);
> @@ -610,7 +610,7 @@ int __must_check media_device_register_entity(struct media_device *mdev,
>  					      struct media_entity *entity)
>  {
>  	struct media_entity_notify *notify, *next;
> -	unsigned int i;
> +	struct media_pad *iter;
>  	int ret;
>  
>  	if (entity->function == MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN ||
> @@ -639,9 +639,8 @@ int __must_check media_device_register_entity(struct media_device *mdev,
>  	media_gobj_create(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
>  
>  	/* Initialize objects at the pads */
> -	for (i = 0; i < entity->num_pads; i++)
> -		media_gobj_create(mdev, MEDIA_GRAPH_PAD,
> -			       &entity->pads[i].graph_obj);
> +	media_entity_for_each_pad(entity, iter)
> +		media_gobj_create(mdev, MEDIA_GRAPH_PAD, &iter->graph_obj);
>  
>  	/* invoke entity_notify callbacks */
>  	list_for_each_entry_safe(notify, next, &mdev->entity_notify, list)
> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index 11f5207f73aa..35a1aa218c51 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
> @@ -192,7 +192,8 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
>  			   struct media_pad *pads)
>  {
>  	struct media_device *mdev = entity->graph_obj.mdev;
> -	unsigned int i;
> +	struct media_pad *iter;
> +	unsigned int i = 0;
>  
>  	if (num_pads >= MEDIA_ENTITY_MAX_PADS)
>  		return -E2BIG;
> @@ -203,12 +204,12 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
>  	if (mdev)
>  		mutex_lock(&mdev->graph_mutex);
>  
> -	for (i = 0; i < num_pads; i++) {
> -		pads[i].entity = entity;
> -		pads[i].index = i;
> +	media_entity_for_each_pad(entity, iter) {
> +		iter->entity = entity;
> +		iter->index = i++;
>  		if (mdev)
>  			media_gobj_create(mdev, MEDIA_GRAPH_PAD,
> -					&entity->pads[i].graph_obj);
> +					&iter->graph_obj);

I know it was wrongly aligned but fixing it would be nice here.

>  	}
>  
>  	if (mdev)
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index a9a1c0ec5d1c..6b6fea6a713e 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -315,6 +315,18 @@ struct media_entity {
>  	} info;
>  };
>  
> +/**
> + * media_entity_for_each_pad - Iterate on all pads in an entity
> + * @entity: The entity the pads belong to
> + * @iter: The iterator pad
> + *
> + * Iterate on all pads in a media entity.
> + */
> +#define media_entity_for_each_pad(entity, iter)			\
> +	for (iter = (entity)->pads;				\
> +	     iter < &(entity)->pads[(entity)->num_pads];	\
> +	     ++iter)

It'd be nice to align the backslashes.

> +
>  /**
>   * struct media_interface - A media interface graph object.
>   *
Tomi Valkeinen Aug. 1, 2022, 5:58 a.m. UTC | #2 
On 30/07/2022 14:11, Sakari Ailus wrote:

>> @@ -203,12 +204,12 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
>>   	if (mdev)
>>   		mutex_lock(&mdev->graph_mutex);
>>   
>> -	for (i = 0; i < num_pads; i++) {
>> -		pads[i].entity = entity;
>> -		pads[i].index = i;
>> +	media_entity_for_each_pad(entity, iter) {
>> +		iter->entity = entity;
>> +		iter->index = i++;
>>   		if (mdev)
>>   			media_gobj_create(mdev, MEDIA_GRAPH_PAD,
>> -					&entity->pads[i].graph_obj);
>> +					&iter->graph_obj);
> 
> I know it was wrongly aligned but fixing it would be nice here.

Ok.

>>   	}
>>   
>>   	if (mdev)
>> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
>> index a9a1c0ec5d1c..6b6fea6a713e 100644
>> --- a/include/media/media-entity.h
>> +++ b/include/media/media-entity.h
>> @@ -315,6 +315,18 @@ struct media_entity {
>>   	} info;
>>   };
>>   
>> +/**
>> + * media_entity_for_each_pad - Iterate on all pads in an entity
>> + * @entity: The entity the pads belong to
>> + * @iter: The iterator pad
>> + *
>> + * Iterate on all pads in a media entity.
>> + */
>> +#define media_entity_for_each_pad(entity, iter)			\
>> +	for (iter = (entity)->pads;				\
>> +	     iter < &(entity)->pads[(entity)->num_pads];	\
>> +	     ++iter)
> 
> It'd be nice to align the backslashes.

They are aligned. But aligning with tabs can get messed up visually in 
patches as there's a +/- at the front of the line.

  Tomi
diff mbox series

Patch

diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
index b8176a3b76d3..25020d58eb06 100644
--- a/drivers/media/mc/mc-device.c
+++ b/drivers/media/mc/mc-device.c
@@ -581,7 +581,7 @@  static void __media_device_unregister_entity(struct media_entity *entity)
 	struct media_device *mdev = entity->graph_obj.mdev;
 	struct media_link *link, *tmp;
 	struct media_interface *intf;
-	unsigned int i;
+	struct media_pad *iter;
 
 	ida_free(&mdev->entity_internal_idx, entity->internal_idx);
 
@@ -597,8 +597,8 @@  static void __media_device_unregister_entity(struct media_entity *entity)
 	__media_entity_remove_links(entity);
 
 	/* Remove all pads that belong to this entity */
-	for (i = 0; i < entity->num_pads; i++)
-		media_gobj_destroy(&entity->pads[i].graph_obj);
+	media_entity_for_each_pad(entity, iter)
+		media_gobj_destroy(&iter->graph_obj);
 
 	/* Remove the entity */
 	media_gobj_destroy(&entity->graph_obj);
@@ -610,7 +610,7 @@  int __must_check media_device_register_entity(struct media_device *mdev,
 					      struct media_entity *entity)
 {
 	struct media_entity_notify *notify, *next;
-	unsigned int i;
+	struct media_pad *iter;
 	int ret;
 
 	if (entity->function == MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN ||
@@ -639,9 +639,8 @@  int __must_check media_device_register_entity(struct media_device *mdev,
 	media_gobj_create(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
 
 	/* Initialize objects at the pads */
-	for (i = 0; i < entity->num_pads; i++)
-		media_gobj_create(mdev, MEDIA_GRAPH_PAD,
-			       &entity->pads[i].graph_obj);
+	media_entity_for_each_pad(entity, iter)
+		media_gobj_create(mdev, MEDIA_GRAPH_PAD, &iter->graph_obj);
 
 	/* invoke entity_notify callbacks */
 	list_for_each_entry_safe(notify, next, &mdev->entity_notify, list)
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 11f5207f73aa..35a1aa218c51 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -192,7 +192,8 @@  int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
 			   struct media_pad *pads)
 {
 	struct media_device *mdev = entity->graph_obj.mdev;
-	unsigned int i;
+	struct media_pad *iter;
+	unsigned int i = 0;
 
 	if (num_pads >= MEDIA_ENTITY_MAX_PADS)
 		return -E2BIG;
@@ -203,12 +204,12 @@  int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
 	if (mdev)
 		mutex_lock(&mdev->graph_mutex);
 
-	for (i = 0; i < num_pads; i++) {
-		pads[i].entity = entity;
-		pads[i].index = i;
+	media_entity_for_each_pad(entity, iter) {
+		iter->entity = entity;
+		iter->index = i++;
 		if (mdev)
 			media_gobj_create(mdev, MEDIA_GRAPH_PAD,
-					&entity->pads[i].graph_obj);
+					&iter->graph_obj);
 	}
 
 	if (mdev)
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index a9a1c0ec5d1c..6b6fea6a713e 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -315,6 +315,18 @@  struct media_entity {
 	} info;
 };
 
+/**
+ * media_entity_for_each_pad - Iterate on all pads in an entity
+ * @entity: The entity the pads belong to
+ * @iter: The iterator pad
+ *
+ * Iterate on all pads in a media entity.
+ */
+#define media_entity_for_each_pad(entity, iter)			\
+	for (iter = (entity)->pads;				\
+	     iter < &(entity)->pads[(entity)->num_pads];	\
+	     ++iter)
+
 /**
  * struct media_interface - A media interface graph object.
  *