Re: [PATCH 13/18] [media] media-entity.h: rename entity.type to entity.function

[Mauro Carvalho Chehab]

Em Mon, 23 Nov 2015 19:51:41 +0200
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Sunday 06 September 2015 14:30:56 Mauro Carvalho Chehab wrote:
> > Entities should have one or more functions. Calling it as a
> > type proofed to not be correct, as an entity could eventually
> 
> s/proofed/proved/
> 
> > have more than one type.
> > 
> > So, rename the field as function.
> 
> s/as/to/
> 
> > Please notice that this patch doesn't extend support for
> > multiple function entities. Such change will happen when
> > we have real case drivers using it.
> > 
> > No functional changes.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> 
> [snip]
> 
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index 8bdc10dcc5e7..10f7d5f0eb66 100644
> > --- a/include/media/media-entity.h
> > +++ b/include/media/media-entity.h
> > @@ -152,7 +152,8 @@ struct media_entity_operations {
> >   *
> >   * @graph_obj: Embedded structure containing the media object common 
> > data.
> >   * @name:  Entity name.
> > - * @type:  Entity type, as defined at uapi/media.h (MEDIA_ENT_T_*)
> > + * @function:  Entity main function, as defined at uapi/media.h
> > + * (MEDIA_ENT_F_*)
> 
> I would squash this patch with "uapi/media.h: Rename entities types to 
> functions" as they essentially touch the same lines. 

They touch at the same lines, but the rationale is different. Keeping
it separate is, IMHO, better, as one logical change per patch is preferred.

> If you want to keep them 
> separate I would use MEDIA_ENT_T_* here and rename it to MEDIA_ENT_F_* in 
> "uapi/media.h: Rename entities types to functions".

IMHO, that would be confusing, as this patch would be incomplete, and
the other one would be renaming from MEDIA_ENT_T_* to MEDIA_ENT_F_*
due to a reason that it is on a different patch.

> 
> >   * @revision:  Entity revision - OBSOLETE - should be removed soon.
> >   * @flags: Entity flags, as defined at uapi/media.h (MEDIA_ENT_FL_*)
> >   * @group_id:  Entity group ID - OBSOLETE - should be removed soon.
> 
> [snip]
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/18] [media] media-entity.h: rename entity.type to entity.function

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 06 September 2015 14:30:56 Mauro Carvalho Chehab wrote:
> Entities should have one or more functions. Calling it as a
> type proofed to not be correct, as an entity could eventually

s/proofed/proved/

> have more than one type.
> 
> So, rename the field as function.

s/as/to/

> Please notice that this patch doesn't extend support for
> multiple function entities. Such change will happen when
> we have real case drivers using it.
> 
> No functional changes.
> 
> Signed-off-by: Mauro Carvalho Chehab 

[snip]

> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 8bdc10dcc5e7..10f7d5f0eb66 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -152,7 +152,8 @@ struct media_entity_operations {
>   *
>   * @graph_obj:   Embedded structure containing the media object common 
> data.
>   * @name:Entity name.
> - * @type:Entity type, as defined at uapi/media.h (MEDIA_ENT_T_*)
> + * @function:Entity main function, as defined at uapi/media.h
> + *   (MEDIA_ENT_F_*)

I would squash this patch with "uapi/media.h: Rename entities types to 
functions" as they essentially touch the same lines. If you want to keep them 
separate I would use MEDIA_ENT_T_* here and rename it to MEDIA_ENT_F_* in 
"uapi/media.h: Rename entities types to functions".

>   * @revision:Entity revision - OBSOLETE - should be removed soon.
>   * @flags:   Entity flags, as defined at uapi/media.h (MEDIA_ENT_FL_*)
>   * @group_id:Entity group ID - OBSOLETE - should be removed soon.

[snip]

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/18] [media] media-entity.h: rename entity.type to entity.function

[Hans Verkuil]

On 09/06/2015 07:30 PM, Mauro Carvalho Chehab wrote:
> Entities should have one or more functions. Calling it as a
> type proofed to not be correct, as an entity could eventually
> have more than one type.
> 
> So, rename the field as function.
> 
> Please notice that this patch doesn't extend support for
> multiple function entities. Such change will happen when
> we have real case drivers using it.
> 
> No functional changes.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

I like this!

> 
> diff --git a/Documentation/video4linux/v4l2-framework.txt 
> b/Documentation/video4linux/v4l2-framework.txt
> index 109cc3792534..2e0fc28fa12f 100644
> --- a/Documentation/video4linux/v4l2-framework.txt
> +++ b/Documentation/video4linux/v4l2-framework.txt
> @@ -303,8 +303,8 @@ calling media_entity_init():
>   err = media_entity_init(>entity, npads, pads);
>  
>  The pads array must have been previously initialized. There is no need to
> -manually set the struct media_entity type and name fields, but the revision
> -field must be initialized if needed.
> +manually set the struct media_entity function and name fields, but the
> +revision field must be initialized if needed.
>  
>  A reference to the entity will be automatically acquired/released when the
>  subdev device node (if any) is opened/closed.
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index df2fe4cc2d47..e925909bc99e 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -229,7 +229,7 @@ static int dvb_create_tsout_entity(struct dvb_device 
> *dvbdev,
>   if (!entity->name)
>   return ret;
>  
> - entity->type = MEDIA_ENT_T_DVB_TSOUT;
> + entity->function = MEDIA_ENT_T_DVB_TSOUT;
>   pads->flags = MEDIA_PAD_FL_SINK;
>  
>   ret = media_entity_init(entity, 1, pads);
> @@ -302,18 +302,18 @@ static int dvb_create_media_entity(struct dvb_device 
> *dvbdev,
>  
>   switch (type) {
>   case DVB_DEVICE_FRONTEND:
> - dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMOD;
> + dvbdev->entity->function = MEDIA_ENT_T_DVB_DEMOD;
>   dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
>   dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
>   break;
>   case DVB_DEVICE_DEMUX:
> - dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMUX;
> + dvbdev->entity->function = MEDIA_ENT_T_DVB_DEMUX;
>   dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
>   for (i = 1; i < npads; i++)
>   dvbdev->pads[i].flags = MEDIA_PAD_FL_SOURCE;
>   break;
>   case DVB_DEVICE_CA:
> - dvbdev->entity->type = MEDIA_ENT_T_DVB_CA;
> + dvbdev->entity->function = MEDIA_ENT_T_DVB_CA;
>   dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
>   dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
>   break;
> @@ -537,7 +537,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap)
>   return 0;
>  
>   media_device_for_each_entity(entity, mdev) {
> - switch (entity->type) {
> + switch (entity->function) {
>   case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
>   tuner = entity;
>   break;
> @@ -576,7 +576,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap)
>   /* Create demux links for each ringbuffer/pad */
>   if (demux) {
>   media_device_for_each_entity(entity, mdev) {
> - if (entity->type == MEDIA_ENT_T_DVB_TSOUT) {
> + if (entity->function == MEDIA_ENT_T_DVB_TSOUT) {
>   if (!strncmp(entity->name, DVR_TSOUT,
>   strlen(DVR_TSOUT))) {
>   ret = media_create_pad_link(demux,
> @@ -621,7 +621,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap)
>   }
>  
>   media_device_for_each_entity(entity, mdev) {
> - if (entity->type == MEDIA_ENT_T_DVB_TSOUT) {
> + if (entity->function == MEDIA_ENT_T_DVB_TSOUT) {
>   if (!strcmp(entity->name, DVR_TSOUT)) {
>   link = media_create_intf_link(entity,
>   intf,
> diff --git a/drivers/media/dvb-frontends/au8522_decoder.c 
> b/drivers/media/dvb-frontends/au8522_decoder.c
> index 55cd42a584a5..a6fbe78a70e3 100644
> --- a/drivers/media/dvb-frontends/au8522_decoder.c
> +++ b/drivers/media/dvb-frontends/au8522_decoder.c
> @@ -775,7 +775,7 @@ static int au8522_probe(struct i2c_client *client,
>   state->pads[AU8522_PAD_INPUT].flags = MEDIA_PAD_FL_SINK;
>   state->pads[AU8522_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
>   state->pads[AU8522_PAD_VBI_OUT].flags = 

[PATCH 13/18] [media] media-entity.h: rename entity.type to entity.function

[Mauro Carvalho Chehab]

Entities should have one or more functions. Calling it as a
type proofed to not be correct, as an entity could eventually
have more than one type.

So, rename the field as function.

Please notice that this patch doesn't extend support for
multiple function entities. Such change will happen when
we have real case drivers using it.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab 

diff --git a/Documentation/video4linux/v4l2-framework.txt 
b/Documentation/video4linux/v4l2-framework.txt
index 109cc3792534..2e0fc28fa12f 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -303,8 +303,8 @@ calling media_entity_init():
err = media_entity_init(>entity, npads, pads);
 
 The pads array must have been previously initialized. There is no need to
-manually set the struct media_entity type and name fields, but the revision
-field must be initialized if needed.
+manually set the struct media_entity function and name fields, but the
+revision field must be initialized if needed.
 
 A reference to the entity will be automatically acquired/released when the
 subdev device node (if any) is opened/closed.
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index df2fe4cc2d47..e925909bc99e 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -229,7 +229,7 @@ static int dvb_create_tsout_entity(struct dvb_device 
*dvbdev,
if (!entity->name)
return ret;
 
-   entity->type = MEDIA_ENT_T_DVB_TSOUT;
+   entity->function = MEDIA_ENT_T_DVB_TSOUT;
pads->flags = MEDIA_PAD_FL_SINK;
 
ret = media_entity_init(entity, 1, pads);
@@ -302,18 +302,18 @@ static int dvb_create_media_entity(struct dvb_device 
*dvbdev,
 
switch (type) {
case DVB_DEVICE_FRONTEND:
-   dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMOD;
+   dvbdev->entity->function = MEDIA_ENT_T_DVB_DEMOD;
dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
break;
case DVB_DEVICE_DEMUX:
-   dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMUX;
+   dvbdev->entity->function = MEDIA_ENT_T_DVB_DEMUX;
dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
for (i = 1; i < npads; i++)
dvbdev->pads[i].flags = MEDIA_PAD_FL_SOURCE;
break;
case DVB_DEVICE_CA:
-   dvbdev->entity->type = MEDIA_ENT_T_DVB_CA;
+   dvbdev->entity->function = MEDIA_ENT_T_DVB_CA;
dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
break;
@@ -537,7 +537,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap)
return 0;
 
media_device_for_each_entity(entity, mdev) {
-   switch (entity->type) {
+   switch (entity->function) {
case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
tuner = entity;
break;
@@ -576,7 +576,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap)
/* Create demux links for each ringbuffer/pad */
if (demux) {
media_device_for_each_entity(entity, mdev) {
-   if (entity->type == MEDIA_ENT_T_DVB_TSOUT) {
+   if (entity->function == MEDIA_ENT_T_DVB_TSOUT) {
if (!strncmp(entity->name, DVR_TSOUT,
strlen(DVR_TSOUT))) {
ret = media_create_pad_link(demux,
@@ -621,7 +621,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap)
}
 
media_device_for_each_entity(entity, mdev) {
-   if (entity->type == MEDIA_ENT_T_DVB_TSOUT) {
+   if (entity->function == MEDIA_ENT_T_DVB_TSOUT) {
if (!strcmp(entity->name, DVR_TSOUT)) {
link = media_create_intf_link(entity,
intf,
diff --git a/drivers/media/dvb-frontends/au8522_decoder.c 
b/drivers/media/dvb-frontends/au8522_decoder.c
index 55cd42a584a5..a6fbe78a70e3 100644
--- a/drivers/media/dvb-frontends/au8522_decoder.c
+++ b/drivers/media/dvb-frontends/au8522_decoder.c
@@ -775,7 +775,7 @@ static int au8522_probe(struct i2c_client *client,
state->pads[AU8522_PAD_INPUT].flags = MEDIA_PAD_FL_SINK;
state->pads[AU8522_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
state->pads[AU8522_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE;
-   sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
+   sd->entity.function = MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
 
ret = media_entity_init(>entity, ARRAY_SIZE(state->pads),