diff mbox series

[v5,05/24] media: entity: Add iterator helper for entity pads

Message ID 20210415130450.421168-6-tomi.valkeinen@ideasonboard.com
State Superseded
Headers show
Series v4l: subdev internal routing | expand

Commit Message

Tomi Valkeinen April 15, 2021, 1:04 p.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>
---
 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

Laurent Pinchart April 18, 2021, 5:52 p.m. UTC | #1
Hi Tomi and Jacopo,

Thank you for the patch.

On Thu, Apr 15, 2021 at 04:04:31PM +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>

> ---

>  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 9e56d2ad6b94..704ef1360eba 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);

> @@ -617,7 +617,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 ||

> @@ -646,9 +646,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 401fddf320e7..830841e0cd28 100644

> --- a/drivers/media/mc/mc-entity.c

> +++ b/drivers/media/mc/mc-entity.c

> @@ -198,7 +198,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;

> @@ -209,12 +210,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 2d45344ca527..52b1a1cab57a 100644

> --- a/include/media/media-entity.h

> +++ b/include/media/media-entity.h

> @@ -1102,3 +1102,15 @@ void media_remove_intf_links(struct media_interface *intf);

>  	 (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)

>  

>  #endif

> +

> +/**

> + * media_entity_for_each_pad - Iterate on all pads in an entity

> + * @entity: The entity the pads belong to

> + * @iter: The iterator pad


I would name the variable pad, here and in the code above.

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


> + *

> + * 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)


-- 
Regards,

Laurent Pinchart
Tomi Valkeinen April 22, 2021, 12:04 p.m. UTC | #2
On 18/04/2021 20:52, Laurent Pinchart wrote:
> Hi Tomi and Jacopo,

> 

> Thank you for the patch.

> 

> On Thu, Apr 15, 2021 at 04:04:31PM +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>

>> ---

>>   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 9e56d2ad6b94..704ef1360eba 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);

>> @@ -617,7 +617,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 ||

>> @@ -646,9 +646,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 401fddf320e7..830841e0cd28 100644

>> --- a/drivers/media/mc/mc-entity.c

>> +++ b/drivers/media/mc/mc-entity.c

>> @@ -198,7 +198,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;

>> @@ -209,12 +210,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 2d45344ca527..52b1a1cab57a 100644

>> --- a/include/media/media-entity.h

>> +++ b/include/media/media-entity.h

>> @@ -1102,3 +1102,15 @@ void media_remove_intf_links(struct media_interface *intf);

>>   	 (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)

>>   

>>   #endif

>> +

>> +/**

>> + * media_entity_for_each_pad - Iterate on all pads in an entity

>> + * @entity: The entity the pads belong to

>> + * @iter: The iterator pad

> 

> I would name the variable pad, here and in the code above.


'iter' name is used in other similar functions/macros here, so I'd 
rather keep this the same.

  Tomi
diff mbox series

Patch

diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
index 9e56d2ad6b94..704ef1360eba 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);
@@ -617,7 +617,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 ||
@@ -646,9 +646,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 401fddf320e7..830841e0cd28 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -198,7 +198,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;
@@ -209,12 +210,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 2d45344ca527..52b1a1cab57a 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -1102,3 +1102,15 @@  void media_remove_intf_links(struct media_interface *intf);
 	 (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)
 
 #endif
+
+/**
+ * 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)