Re: [PATCH 15/18] [media] uapi/media.h: Rename entities types to functions

[Mauro Carvalho Chehab]

Em Fri, 11 Sep 2015 17:36:42 +0200
Hans Verkuil  escreveu:

> On 09/06/2015 07:30 PM, Mauro Carvalho Chehab wrote:
> > Rename the userspace types from MEDIA_ENT_T_ to MEDIA_ENT_F_
> > and add the backward compatibility bits.
> > 
> > The changes at the .c files was generated by the following
> > coccinelle script:
> > 
> 
> 
> > @@
> > -MEDIA_ENT_T_DVB_DEMUX
> > +MEDIA_ENT_F_MPEG_TS_DEMUX
> 
> I'm not sure about the 'MPEG_' part here. I think that in general a transport 
> stream
> can contain non-MPEG streams as well. Why not just say _F_TS_DEMUX?

Changed.

> 
> > @@
> > @@
> > -MEDIA_ENT_T_DVB_TSOUT
> > +MEDIA_ENT_F_DTV_TSOUT
> 
> Shouldn't this be MEDIA_ENT_F_IO?

As we've discussed on IRC, per Shuah's request, I ended by keeping
one different I/O entity per API type:

#define MEDIA_ENT_F_IO_DTV  (MEDIA_ENT_F_BASE + 31)
#define MEDIA_ENT_F_IO_VBI  (MEDIA_ENT_F_BASE + 32)
#define MEDIA_ENT_F_IO_SWRADIO  (MEDIA_ENT_F_BASE + 33)
#define MEDIA_ENT_F_IO_V4L  (MEDIA_ENT_F_OLD_BASE + 1)

> 
> > @@
> > @@
> > -MEDIA_ENT_T_DVB_CA
> > +MEDIA_ENT_F_DTV_CA
> > @@
> > @@
> > -MEDIA_ENT_T_DVB_NET_DECAP
> > +MEDIA_ENT_F_DTV_NET_DECAP
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > 
> > diff --git a/drivers/media/dvb-core/dvbdev.c 
> > b/drivers/media/dvb-core/dvbdev.c
> > index e925909bc99e..8527fc40e6a0 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->function = MEDIA_ENT_T_DVB_TSOUT;
> > +   entity->function = MEDIA_ENT_F_IO;
> > pads->flags = MEDIA_PAD_FL_SINK;
> >  
> > ret = media_entity_init(entity, 1, pads);
> > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > index d232cc680c67..90e90a6e62bf 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -46,87 +46,86 @@ struct media_device_info {
> >   * Initial value to be used when a new entity is created
> >   * Drivers should change it to something useful
> >   */
> > -#define MEDIA_ENT_T_UNKNOWN0x
> > +#define MEDIA_ENT_F_UNKNOWN0x
> >  
> >  /*
> > - * Base numbers for entity types
> > + * Base number ranges for entity functions
> >   *
> > - * Please notice that the huge gap of 16 bits for each base is overkill!
> > - * 8 bits is more than enough to avoid starving entity types for each
> > - * subsystem.
> > - *
> > - * However, It is kept this way just to avoid binary breakages with the
> > - * namespace provided on legacy versions of this header.
> > + * NOTE: those ranges and entity function number are spased just to
> 
> s/spased/phased/
> 
> > + * make easier to maintain this file. Userspace should not rely on
> 
> s/make/make it/
> 
> > + * the ranges to identify a group of function types, as newer
> > + * functions can be added with any name within the full u32 range.
> >   */
> > -#define MEDIA_ENT_T_DVB_BASE   0x
> > -#define MEDIA_ENT_T_V4L2_BASE  0x0001
> > -#define MEDIA_ENT_T_V4L2_SUBDEV_BASE   0x0002
> > -#define MEDIA_ENT_T_CONNECTOR_BASE 0x0003
> > +#define MEDIA_ENT_F_BASE   0x
> > +#define MEDIA_ENT_F_OLD_BASE   0x0001
> > +#define MEDIA_ENT_F_OLD_SUBDEV_BASE0x0002
> >  
> >  /*
> > - * V4L2 entities - Those are used for DMA (mmap/DMABUF) and
> > - * read()/write() data I/O associated with the V4L2 devnodes.
> > + * DVB entities
> >   */
> > -#define MEDIA_ENT_T_V4L2_VIDEO (MEDIA_ENT_T_V4L2_BASE + 1)
> > -   /*
> > -* Please notice that numbers between MEDIA_ENT_T_V4L2_BASE + 2 and
> > -* MEDIA_ENT_T_V4L2_BASE + 4 can't be used, as those values used
> > -* to be declared for FB, ALSA and DVB entities.
> > -* As those values were never actually used in practice, we're just
> > -* adding them as backward compatibility macros and keeping the
> > -* numberspace clean here. This way, we avoid breaking compilation,
> > -* in the case of having some userspace application using the old
> > -* symbols.
> > -*/
> > -#define MEDIA_ENT_T_V4L2_VBI   (MEDIA_ENT_T_V4L2_BASE + 5)
> > -#define MEDIA_ENT_T_V4L2_SWRADIO   (MEDIA_ENT_T_V4L2_BASE + 6)
> > -
> > -/* V4L2 Sub-device entities */
> > -
> > -   /*
> > -* Subdevs are initialized with MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN,
> > -* in order to preserve backward compatibility.
> > -* Drivers should change to the proper subdev type before
> > -* registering the entity.
> > -*/
> > -#define MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWNMEDIA_ENT_T_V4L2_SUBDEV_BASE
> > -
> > -#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 
> > 1)
> > -#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH  

Re: [PATCH 15/18] [media] uapi/media.h: Rename entities types to functions

[Hans Verkuil]

On 09/06/2015 07:30 PM, Mauro Carvalho Chehab wrote:
> Rename the userspace types from MEDIA_ENT_T_ to MEDIA_ENT_F_
> and add the backward compatibility bits.
> 
> The changes at the .c files was generated by the following
> coccinelle script:
> 


> @@
> -MEDIA_ENT_T_DVB_DEMUX
> +MEDIA_ENT_F_MPEG_TS_DEMUX

I'm not sure about the 'MPEG_' part here. I think that in general a transport 
stream
can contain non-MPEG streams as well. Why not just say _F_TS_DEMUX?

> @@
> @@
> -MEDIA_ENT_T_DVB_TSOUT
> +MEDIA_ENT_F_DTV_TSOUT

Shouldn't this be MEDIA_ENT_F_IO?

> @@
> @@
> -MEDIA_ENT_T_DVB_CA
> +MEDIA_ENT_F_DTV_CA
> @@
> @@
> -MEDIA_ENT_T_DVB_NET_DECAP
> +MEDIA_ENT_F_DTV_NET_DECAP
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index e925909bc99e..8527fc40e6a0 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->function = MEDIA_ENT_T_DVB_TSOUT;
> + entity->function = MEDIA_ENT_F_IO;
>   pads->flags = MEDIA_PAD_FL_SINK;
>  
>   ret = media_entity_init(entity, 1, pads);
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index d232cc680c67..90e90a6e62bf 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -46,87 +46,86 @@ struct media_device_info {
>   * Initial value to be used when a new entity is created
>   * Drivers should change it to something useful
>   */
> -#define MEDIA_ENT_T_UNKNOWN  0x
> +#define MEDIA_ENT_F_UNKNOWN  0x
>  
>  /*
> - * Base numbers for entity types
> + * Base number ranges for entity functions
>   *
> - * Please notice that the huge gap of 16 bits for each base is overkill!
> - * 8 bits is more than enough to avoid starving entity types for each
> - * subsystem.
> - *
> - * However, It is kept this way just to avoid binary breakages with the
> - * namespace provided on legacy versions of this header.
> + * NOTE: those ranges and entity function number are spased just to

s/spased/phased/

> + * make easier to maintain this file. Userspace should not rely on

s/make/make it/

> + * the ranges to identify a group of function types, as newer
> + * functions can be added with any name within the full u32 range.
>   */
> -#define MEDIA_ENT_T_DVB_BASE 0x
> -#define MEDIA_ENT_T_V4L2_BASE0x0001
> -#define MEDIA_ENT_T_V4L2_SUBDEV_BASE 0x0002
> -#define MEDIA_ENT_T_CONNECTOR_BASE   0x0003
> +#define MEDIA_ENT_F_BASE 0x
> +#define MEDIA_ENT_F_OLD_BASE 0x0001
> +#define MEDIA_ENT_F_OLD_SUBDEV_BASE  0x0002
>  
>  /*
> - * V4L2 entities - Those are used for DMA (mmap/DMABUF) and
> - *   read()/write() data I/O associated with the V4L2 devnodes.
> + * DVB entities
>   */
> -#define MEDIA_ENT_T_V4L2_VIDEO   (MEDIA_ENT_T_V4L2_BASE + 1)
> - /*
> -  * Please notice that numbers between MEDIA_ENT_T_V4L2_BASE + 2 and
> -  * MEDIA_ENT_T_V4L2_BASE + 4 can't be used, as those values used
> -  * to be declared for FB, ALSA and DVB entities.
> -  * As those values were never actually used in practice, we're just
> -  * adding them as backward compatibility macros and keeping the
> -  * numberspace clean here. This way, we avoid breaking compilation,
> -  * in the case of having some userspace application using the old
> -  * symbols.
> -  */
> -#define MEDIA_ENT_T_V4L2_VBI (MEDIA_ENT_T_V4L2_BASE + 5)
> -#define MEDIA_ENT_T_V4L2_SWRADIO (MEDIA_ENT_T_V4L2_BASE + 6)
> -
> -/* V4L2 Sub-device entities */
> -
> - /*
> -  * Subdevs are initialized with MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN,
> -  * in order to preserve backward compatibility.
> -  * Drivers should change to the proper subdev type before
> -  * registering the entity.
> -  */
> -#define MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN  MEDIA_ENT_T_V4L2_SUBDEV_BASE
> -
> -#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR   (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 
> 1)
> -#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 
> 2)
> -#define MEDIA_ENT_T_V4L2_SUBDEV_LENS (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 3)
> - /* A converter of analogue video to its digital representation. */
> -#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER  (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 
> 4)
> - /* Tuner entity is actually both V4L2 and DVB subdev */
> -#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 
> 5)
> +#define MEDIA_ENT_F_DTV_DEMOD(MEDIA_ENT_F_BASE + 1)
> +#define MEDIA_ENT_F_MPEG_TS_DEMUX(MEDIA_ENT_F_BASE + 2)
> +#define MEDIA_ENT_F_DTV_CA   (MEDIA_ENT_F_BASE + 3)
> +#define MEDIA_ENT_F_DTV_NET_DECAP(MEDIA_ENT_F_BASE + 4)
>  
> -/* DVB