Em Mon, 26 Jan 2015 14:11:50 +0100 Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: > On 01/26/2015 01:47 PM, Mauro Carvalho Chehab wrote: > > The previous provision for DVB media controller support were to > > define an ID (likely meaning the adapter number) for the DVB > > devnodes. > > > > This is just plain wrong. Just like V4L, DVB devices (and ALSA, > > or whatever) are identified via a (major, minor) tuple. > > > > This is enough to uniquely identify a devnode, no matter what > > API it implements. > > > > So, before we go too far, let's mark the old v4l, dvb and alsa > > "devnode" info as deprecated, and just call it as "dev". > > > > As we don't want to break compilation on already existing apps, > > let's just keep the old definitions as-is, adding a note that > > those are deprecated at media-entity.h. > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx> > > > > diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c > > index 86bb93fd7db8..d89d5cb465d9 100644 > > --- a/drivers/media/v4l2-core/v4l2-dev.c > > +++ b/drivers/media/v4l2-core/v4l2-dev.c > > @@ -943,8 +943,8 @@ int __video_register_device(struct video_device *vdev, int type, int nr, > > vdev->vfl_type != VFL_TYPE_SUBDEV) { > > vdev->entity.type = MEDIA_ENT_T_DEVNODE_V4L; > > vdev->entity.name = vdev->name; > > - vdev->entity.info.v4l.major = VIDEO_MAJOR; > > - vdev->entity.info.v4l.minor = vdev->minor; > > + vdev->entity.info.dev.major = VIDEO_MAJOR; > > + vdev->entity.info.dev.minor = vdev->minor; > > ret = media_device_register_entity(vdev->v4l2_dev->mdev, > > &vdev->entity); > > if (ret < 0) > > diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c > > index 015f92aab44a..204cc67c84e8 100644 > > --- a/drivers/media/v4l2-core/v4l2-device.c > > +++ b/drivers/media/v4l2-core/v4l2-device.c > > @@ -248,8 +248,8 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev) > > goto clean_up; > > } > > #if defined(CONFIG_MEDIA_CONTROLLER) > > - sd->entity.info.v4l.major = VIDEO_MAJOR; > > - sd->entity.info.v4l.minor = vdev->minor; > > + sd->entity.info.dev.major = VIDEO_MAJOR; > > + sd->entity.info.dev.minor = vdev->minor; > > #endif > > sd->devnode = vdev; > > } > > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > > index e00459185d20..d6d74bcfe183 100644 > > --- a/include/media/media-entity.h > > +++ b/include/media/media-entity.h > > @@ -87,17 +87,7 @@ struct media_entity { > > struct { > > u32 major; > > u32 minor; > > - } v4l; > > - struct { > > - u32 major; > > - u32 minor; > > - } fb; > > - struct { > > - u32 card; > > - u32 device; > > - u32 subdevice; > > - } alsa; > > I don't think the alsa entity information can be replaced by major/minor. > In particular you will loose the subdevice information which you need as > well. In addition, alsa devices are almost never referenced via major and > minor numbers, but always by card/device/subdevice numbers. For media-ctl, it is easier to handle major/minor, in order to identify the associated devnode name. Btw, media-ctl currently assumes that all devnode devices are specified by v4l.major/v4l.minor. Ok, maybe for alsa we'll need also card/device/subdevice, but I think this should be mapped elsewhere, if this can't be retrieved via its sysfs/udev interface (with seems to be doubtful). > > > - int dvb; > > + } dev; > > > > /* Sub-device specifications */ > > /* Nothing needed yet */ > > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h > > index d847c760e8f0..418f4fec391a 100644 > > --- a/include/uapi/linux/media.h > > +++ b/include/uapi/linux/media.h > > @@ -78,6 +78,20 @@ struct media_entity_desc { > > struct { > > __u32 major; > > __u32 minor; > > + } dev; > > + > > +#if 1 > > + /* > > + * DEPRECATED: previous node specifications. Kept just to > > + * avoid breaking compilation, but media_entity_desc.dev > > + * should be used instead. In particular, alsa and dvb > > + * fields below are wrong: for all devnodes, there should > > + * be just major/minor inside the struct, as this is enough > > + * to represent any devnode, no matter what type. > > + */ > > + struct { > > + __u32 major; > > + __u32 minor; > > } v4l; > > struct { > > __u32 major; > > @@ -89,6 +103,7 @@ struct media_entity_desc { > > __u32 subdevice; > > } alsa; > > int dvb; > > I wouldn't merge all the v4l/fb/etc. structs into one struct. That will make it > difficult in the future if you need to add a field for e.g. v4l entities. No. You could just create another union for the API-specific bits, using the reserved bytes. > So I would keep the v4l, fb and alsa structs, and just add a new struct for > dvb. I wonder if the dvb field can't just be replaced since I doubt anyone is > using it. And even if someone does, then it can't be right since a single > int isn't enough and never worked anyway. All devnodes have major/minor. Making it standard for all devices makes easy for userspace to properly get the data it requires to work. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html