Hi Mauro, On Thursday 09 September 2010 03:25:38 Mauro Carvalho Chehab wrote: > Em 20-08-2010 12:29, Laurent Pinchart escreveu: > > V4L2 subdevices are media entities. As such they need to inherit from > > (include) the media_entity structure. > > > > When registering/unregistering the subdevice, the media entity is > > automatically registered/unregistered. The entity is acquired on device > > open and released on device close. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx> > > --- > > > > Documentation/video4linux/v4l2-framework.txt | 23 ++++++++++++++++++ > > drivers/media/video/v4l2-device.c | 32 > > +++++++++++++++++++++----- drivers/media/video/v4l2-subdev.c > > | 27 +++++++++++++++++++++- include/media/v4l2-subdev.h > > | 7 +++++ > > 4 files changed, 82 insertions(+), 7 deletions(-) > > > > diff --git a/Documentation/video4linux/v4l2-framework.txt > > b/Documentation/video4linux/v4l2-framework.txt index 7ff4016..3416d93 > > 100644 > > --- a/Documentation/video4linux/v4l2-framework.txt > > +++ b/Documentation/video4linux/v4l2-framework.txt > > > > @@ -263,6 +263,26 @@ A sub-device driver initializes the v4l2_subdev struct using: > > Afterwards you need to initialize subdev->name with a unique name and > > set the module owner. This is done for you if you use the i2c helper > > functions. > > > > +If integration with the media framework is needed, you must initialize > > the +media_entity struct embedded in the v4l2_subdev struct (entity > > field) by +calling media_entity_init(): > > + > > + struct media_pad *pads = &my_sd->pads; > > + int err; > > + > > + err = media_entity_init(&sd->entity, npads, pads, 0); > > + > > +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. > > + > > +A reference to the entity will be automatically acquired/released when > > the +subdev device node (if any) is opened/closed. > > + > > +Don't forget to cleanup the media entity before the sub-device is > > destroyed: + > > + media_entity_cleanup(&sd->entity); > > + > > > > A device (bridge) driver needs to register the v4l2_subdev with the > > > > v4l2_device: > > @@ -272,6 +292,9 @@ This can fail if the subdev module disappeared before > > it could be registered. > > > > After this function was called successfully the subdev->dev field points > > to the v4l2_device. > > > > +If the v4l2_device parent device has a non-NULL mdev field, the > > sub-device +entity will be automatically registered with the media > > device. > > + > > > > You can unregister a sub-device using: > > v4l2_device_unregister_subdev(sd); > > > > diff --git a/drivers/media/video/v4l2-device.c > > b/drivers/media/video/v4l2-device.c index 91452cd..4f74d01 100644 > > --- a/drivers/media/video/v4l2-device.c > > +++ b/drivers/media/video/v4l2-device.c > > @@ -114,10 +114,11 @@ void v4l2_device_unregister(struct v4l2_device > > *v4l2_dev) > > > > EXPORT_SYMBOL_GPL(v4l2_device_unregister); > > > > int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev, > > > > - struct v4l2_subdev *sd) > > + struct v4l2_subdev *sd) > > > > { > > > > + struct media_entity *entity = &sd->entity; > > > > struct video_device *vdev; > > > > - int ret = 0; > > + int ret; > > > > /* Check for valid input */ > > if (v4l2_dev == NULL || sd == NULL || !sd->name[0]) > > > > @@ -129,6 +130,15 @@ int v4l2_device_register_subdev(struct v4l2_device > > *v4l2_dev, > > > > if (!try_module_get(sd->owner)) > > > > return -ENODEV; > > > > + /* Register the entity. */ > > + if (v4l2_dev->mdev) { > > + ret = media_device_register_entity(v4l2_dev->mdev, entity); > > + if (ret < 0) { > > + module_put(sd->owner); > > + return ret; > > + } > > + } > > + > > > > sd->v4l2_dev = v4l2_dev; > > spin_lock(&v4l2_dev->lock); > > list_add_tail(&sd->list, &v4l2_dev->subdevs); > > > > @@ -143,26 +153,36 @@ int v4l2_device_register_subdev(struct v4l2_device > > *v4l2_dev, > > > > if (sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE) { > > > > ret = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1, > > > > sd->owner); > > > > - if (ret < 0) > > + if (ret < 0) { > > > > v4l2_device_unregister_subdev(sd); > > > > + return ret; > > + } > > > > } > > > > - return ret; > > + entity->v4l.major = VIDEO_MAJOR; > > + entity->v4l.minor = vdev->minor; > > Hmm... it needs to check if entity is not null. Entity is set to struct media_entity *entity = &sd->entity; It can't be NULL. > > + return 0; > > > > } > > EXPORT_SYMBOL_GPL(v4l2_device_register_subdev); > > > > void v4l2_device_unregister_subdev(struct v4l2_subdev *sd) > > { > > > > + struct v4l2_device *v4l2_dev; > > + > > > > /* return if it isn't registered */ > > if (sd == NULL || sd->v4l2_dev == NULL) > > > > return; > > > > - spin_lock(&sd->v4l2_dev->lock); > > + v4l2_dev = sd->v4l2_dev; > > + > > + spin_lock(&v4l2_dev->lock); > > > > list_del(&sd->list); > > > > - spin_unlock(&sd->v4l2_dev->lock); > > + spin_unlock(&v4l2_dev->lock); > > > > sd->v4l2_dev = NULL; > > > > module_put(sd->owner); > > > > + if (v4l2_dev->mdev) > > + media_device_unregister_entity(&sd->entity); > > > > video_unregister_device(&sd->devnode); > > > > } > > EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev); > > > > diff --git a/drivers/media/video/v4l2-subdev.c > > b/drivers/media/video/v4l2-subdev.c index b063195..1efa267 100644 > > --- a/drivers/media/video/v4l2-subdev.c > > +++ b/drivers/media/video/v4l2-subdev.c > > @@ -32,7 +32,8 @@ static int subdev_open(struct file *file) > > > > { > > > > struct video_device *vdev = video_devdata(file); > > struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev); > > > > - struct v4l2_fh *vfh; > > + struct media_entity *entity; > > + struct v4l2_fh *vfh = NULL; > > > > int ret; > > > > if (!sd->initialized) > > > > @@ -59,10 +60,17 @@ static int subdev_open(struct file *file) > > > > file->private_data = vfh; > > > > } > > > > + entity = media_entity_get(&sd->entity); > > + if (!entity) { > > + ret = -EBUSY; > > + goto err; > > + } > > + > > It needs to check if v4l2_dev->mdev is not null, as it makes no sense to > get an entity if the driver is not using the media entity. OK. > > return 0; > > > > err: > > if (vfh != NULL) { > > > > + v4l2_fh_del(vfh); > > > > v4l2_fh_exit(vfh); > > kfree(vfh); > > > > } > > > > @@ -72,8 +80,12 @@ err: > > static int subdev_close(struct file *file) > > { > > > > + struct video_device *vdev = video_devdata(file); > > + struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev); > > > > struct v4l2_fh *vfh = file->private_data; > > > > + media_entity_put(&sd->entity); > > Same here: don't put it, if v4l2_dev->mdev = NULL (I think that you're > already checking for it at media_entity_put). > > > + > > > > if (vfh != NULL) { > > > > v4l2_fh_del(vfh); > > v4l2_fh_exit(vfh); > > > > @@ -172,5 +184,18 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const > > struct v4l2_subdev_ops *ops) > > > > sd->grp_id = 0; > > sd->priv = NULL; > > sd->initialized = 1; > > > > + sd->entity.name = sd->name; > > + sd->entity.type = MEDIA_ENTITY_TYPE_SUBDEV; > > > > } > > EXPORT_SYMBOL(v4l2_subdev_init); > > > > + > > +int v4l2_subdev_set_power(struct media_entity *entity, int power) > > +{ > > + struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity); > > + > > + dev_dbg(entity->parent->dev, > > + "%s power%s\n", entity->name, power ? "on" : "off"); > > + > > + return v4l2_subdev_call(sd, core, s_power, power); > > +} > > +EXPORT_SYMBOL_GPL(v4l2_subdev_set_power); > > Also, should do nothing if media entity is null. > > PS.: Not sure where this function is called, as the caller is not on this > patch series. The better would be to add this together with the patch > calling v4l2_subdev_set_power(). v4l2_subdev_set_power() is a helper meant to be assigned to struct media_entity_operations::set_power in subdev drivers. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html