| Message ID | 20210112132339.5621-13-ezequiel@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | V4L2 Async notifier API cleanup | expand |
Hi Ezequiel, Thank you for the patch. On Tue, Jan 12, 2021 at 10:23:38AM -0300, Ezequiel Garcia wrote: > Now that most users of v4l2_async_notifier_add_subdev have > been converted, let's fix the documentation so it's more clear > how the v4l2-async API should be used. > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > --- > .../driver-api/media/v4l2-subdev.rst | 38 ++++++++++++++++--- > include/media/v4l2-async.h | 12 +++++- > 2 files changed, 43 insertions(+), 7 deletions(-) > > diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst > index bb5b1a7cdfd9..5ddf9de4fcf7 100644 > --- a/Documentation/driver-api/media/v4l2-subdev.rst > +++ b/Documentation/driver-api/media/v4l2-subdev.rst > @@ -204,11 +204,39 @@ Before registering the notifier, bridge drivers must do two things: > first, the notifier must be initialized using the > :c:func:`v4l2_async_notifier_init`. Second, bridge drivers can then > begin to form a list of subdevice descriptors that the bridge device > -needs for its operation. Subdevice descriptors are added to the notifier > -using the :c:func:`v4l2_async_notifier_add_subdev` call. This function > -takes two arguments: a pointer to struct :c:type:`v4l2_async_notifier`, > -and a pointer to the subdevice descripter, which is of type struct > -:c:type:`v4l2_async_subdev`. > +needs for its operation. Several functions are available, to > +add subdevice descriptors to a notifier, depending on the type of device: You could reflow this to needs for its operation. Several functions are available, to add subdevice descriptors to a notifier, depending on the type of device: > +:c:func:`v4l2_async_notifier_add_devname_subdev`, > +:c:func:`v4l2_async_notifier_add_fwnode_subdev` or > +:c:func:`v4l2_async_notifier_add_i2c_subdev`. Should you also list v4l2_async_notifier_add_fwnode_remote_subdev() (and possibly v4l2_async_notifier_parse_fwnode_endpoints()) here ? Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > +These functions allocate a subdevice descriptor, which is of > +type struct :c:type:`v4l2_async_subdev`, and take a size argument > +which can be used to embed the descriptor in a driver-specific > +async subdevice struct. The &struct :c:type:`v4l2_async_subdev` > +shall be the first member of this struct: > + > +.. code-block:: c > + > + struct my_async_subdev { > + struct v4l2_async_subdev asd; > + ... > + }; > + > + struct my_async_subdev *my_asd; > + struct v4l2_async_subdev *asd; > + struct fwnode_handle *ep; > + > + ... > + > + asd = v4l2_async_notifier_add_fwnode_subdev( > + ¬ifier, ep, sizeof(*my_asd)); > + fwnode_handle_put(ep); > + > + if (IS_ERR(asd)) > + return PTR_ERR(asd); > + > + my_asd = container_of(asd, struct my_async_subdev, asd); > > The V4L2 core will then use these descriptors to match asynchronously > registered subdevices to them. If a match is detected the ``.bound()`` > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h > index 2429ac55be1c..1278f98355a7 100644 > --- a/include/media/v4l2-async.h > +++ b/include/media/v4l2-async.h > @@ -151,7 +151,12 @@ void v4l2_async_debug_init(struct dentry *debugfs_dir); > * @notifier: pointer to &struct v4l2_async_notifier > * > * This function initializes the notifier @asd_list. It must be called > - * before the first call to @v4l2_async_notifier_add_subdev. > + * before adding a subdevice to a notifier, using one of: > + * @v4l2_async_notifier_add_fwnode_subdev, > + * @v4l2_async_notifier_add_fwnode_remote_subdev, > + * @v4l2_async_notifier_add_i2c_subdev, > + * @v4l2_async_notifier_add_devname_subdev or > + * @v4l2_async_notifier_parse_fwnode_endpoints. > */ > void v4l2_async_notifier_init(struct v4l2_async_notifier *notifier); > > @@ -290,7 +295,10 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier); > * sub-devices allocated for the purposes of the notifier but not the notifier > * itself. The user is responsible for calling this function to clean up the > * notifier after calling > - * @v4l2_async_notifier_add_subdev or > + * @v4l2_async_notifier_add_fwnode_subdev, > + * @v4l2_async_notifier_add_fwnode_remote_subdev, > + * @v4l2_async_notifier_add_i2c_subdev, > + * @v4l2_async_notifier_add_devname_subdev or > * @v4l2_async_notifier_parse_fwnode_endpoints. > * > * There is no harm from calling v4l2_async_notifier_cleanup in other
On Thu, 2021-01-14 at 04:21 +0200, Laurent Pinchart wrote: > Hi Ezequiel, > > Thank you for the patch. > > On Tue, Jan 12, 2021 at 10:23:38AM -0300, Ezequiel Garcia wrote: > > Now that most users of v4l2_async_notifier_add_subdev have > > been converted, let's fix the documentation so it's more clear > > how the v4l2-async API should be used. > > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > --- > > .../driver-api/media/v4l2-subdev.rst | 38 ++++++++++++++++--- > > include/media/v4l2-async.h | 12 +++++- > > 2 files changed, 43 insertions(+), 7 deletions(-) > > > > diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst > > index bb5b1a7cdfd9..5ddf9de4fcf7 100644 > > --- a/Documentation/driver-api/media/v4l2-subdev.rst > > +++ b/Documentation/driver-api/media/v4l2-subdev.rst > > @@ -204,11 +204,39 @@ Before registering the notifier, bridge drivers must do two things: > > first, the notifier must be initialized using the > > :c:func:`v4l2_async_notifier_init`. Second, bridge drivers can then > > begin to form a list of subdevice descriptors that the bridge device > > -needs for its operation. Subdevice descriptors are added to the notifier > > -using the :c:func:`v4l2_async_notifier_add_subdev` call. This function > > -takes two arguments: a pointer to struct :c:type:`v4l2_async_notifier`, > > -and a pointer to the subdevice descripter, which is of type struct > > -:c:type:`v4l2_async_subdev`. > > +needs for its operation. Several functions are available, to > > +add subdevice descriptors to a notifier, depending on the type of device: > > You could reflow this to > > needs for its operation. Several functions are available, to add subdevice > descriptors to a notifier, depending on the type of device: > > > +:c:func:`v4l2_async_notifier_add_devname_subdev`, > > +:c:func:`v4l2_async_notifier_add_fwnode_subdev` or > > +:c:func:`v4l2_async_notifier_add_i2c_subdev`. > > Should you also list v4l2_async_notifier_add_fwnode_remote_subdev() (and Yes. > possibly v4l2_async_notifier_parse_fwnode_endpoints()) here ? > Unsure. I'd rather not document this one, as it's deprecated and we want to remove it. > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Thanks! Ezequiel
On Thu, Jan 14, 2021 at 10:39:33AM -0300, Ezequiel Garcia wrote: > On Thu, 2021-01-14 at 04:21 +0200, Laurent Pinchart wrote: > > Hi Ezequiel, > > > > Thank you for the patch. > > > > On Tue, Jan 12, 2021 at 10:23:38AM -0300, Ezequiel Garcia wrote: > > > Now that most users of v4l2_async_notifier_add_subdev have > > > been converted, let's fix the documentation so it's more clear > > > how the v4l2-async API should be used. > > > > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > > --- > > > .../driver-api/media/v4l2-subdev.rst | 38 ++++++++++++++++--- > > > include/media/v4l2-async.h | 12 +++++- > > > 2 files changed, 43 insertions(+), 7 deletions(-) > > > > > > diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst > > > index bb5b1a7cdfd9..5ddf9de4fcf7 100644 > > > --- a/Documentation/driver-api/media/v4l2-subdev.rst > > > +++ b/Documentation/driver-api/media/v4l2-subdev.rst > > > @@ -204,11 +204,39 @@ Before registering the notifier, bridge drivers must do two things: > > > first, the notifier must be initialized using the > > > :c:func:`v4l2_async_notifier_init`. Second, bridge drivers can then > > > begin to form a list of subdevice descriptors that the bridge device > > > -needs for its operation. Subdevice descriptors are added to the notifier > > > -using the :c:func:`v4l2_async_notifier_add_subdev` call. This function > > > -takes two arguments: a pointer to struct :c:type:`v4l2_async_notifier`, > > > -and a pointer to the subdevice descripter, which is of type struct > > > -:c:type:`v4l2_async_subdev`. > > > +needs for its operation. Several functions are available, to > > > +add subdevice descriptors to a notifier, depending on the type of device: > > > > You could reflow this to > > > > needs for its operation. Several functions are available, to add subdevice > > descriptors to a notifier, depending on the type of device: > > > > > +:c:func:`v4l2_async_notifier_add_devname_subdev`, > > > +:c:func:`v4l2_async_notifier_add_fwnode_subdev` or > > > +:c:func:`v4l2_async_notifier_add_i2c_subdev`. > > > > Should you also list v4l2_async_notifier_add_fwnode_remote_subdev() (and > > Yes. > > > possibly v4l2_async_notifier_parse_fwnode_endpoints()) here ? > > > > Unsure. I'd rather not document this one, as it's deprecated > and we want to remove it. This document is here to guide people to use the right functions and that isn't one of them. So it shouldn't be added here.
diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst index bb5b1a7cdfd9..5ddf9de4fcf7 100644 --- a/Documentation/driver-api/media/v4l2-subdev.rst +++ b/Documentation/driver-api/media/v4l2-subdev.rst @@ -204,11 +204,39 @@ Before registering the notifier, bridge drivers must do two things: first, the notifier must be initialized using the :c:func:`v4l2_async_notifier_init`. Second, bridge drivers can then begin to form a list of subdevice descriptors that the bridge device -needs for its operation. Subdevice descriptors are added to the notifier -using the :c:func:`v4l2_async_notifier_add_subdev` call. This function -takes two arguments: a pointer to struct :c:type:`v4l2_async_notifier`, -and a pointer to the subdevice descripter, which is of type struct -:c:type:`v4l2_async_subdev`. +needs for its operation. Several functions are available, to +add subdevice descriptors to a notifier, depending on the type of device: +:c:func:`v4l2_async_notifier_add_devname_subdev`, +:c:func:`v4l2_async_notifier_add_fwnode_subdev` or +:c:func:`v4l2_async_notifier_add_i2c_subdev`. + +These functions allocate a subdevice descriptor, which is of +type struct :c:type:`v4l2_async_subdev`, and take a size argument +which can be used to embed the descriptor in a driver-specific +async subdevice struct. The &struct :c:type:`v4l2_async_subdev` +shall be the first member of this struct: + +.. code-block:: c + + struct my_async_subdev { + struct v4l2_async_subdev asd; + ... + }; + + struct my_async_subdev *my_asd; + struct v4l2_async_subdev *asd; + struct fwnode_handle *ep; + + ... + + asd = v4l2_async_notifier_add_fwnode_subdev( + ¬ifier, ep, sizeof(*my_asd)); + fwnode_handle_put(ep); + + if (IS_ERR(asd)) + return PTR_ERR(asd); + + my_asd = container_of(asd, struct my_async_subdev, asd); The V4L2 core will then use these descriptors to match asynchronously registered subdevices to them. If a match is detected the ``.bound()`` diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h index 2429ac55be1c..1278f98355a7 100644 --- a/include/media/v4l2-async.h +++ b/include/media/v4l2-async.h @@ -151,7 +151,12 @@ void v4l2_async_debug_init(struct dentry *debugfs_dir); * @notifier: pointer to &struct v4l2_async_notifier * * This function initializes the notifier @asd_list. It must be called - * before the first call to @v4l2_async_notifier_add_subdev. + * before adding a subdevice to a notifier, using one of: + * @v4l2_async_notifier_add_fwnode_subdev, + * @v4l2_async_notifier_add_fwnode_remote_subdev, + * @v4l2_async_notifier_add_i2c_subdev, + * @v4l2_async_notifier_add_devname_subdev or + * @v4l2_async_notifier_parse_fwnode_endpoints. */ void v4l2_async_notifier_init(struct v4l2_async_notifier *notifier); @@ -290,7 +295,10 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier); * sub-devices allocated for the purposes of the notifier but not the notifier * itself. The user is responsible for calling this function to clean up the * notifier after calling - * @v4l2_async_notifier_add_subdev or + * @v4l2_async_notifier_add_fwnode_subdev, + * @v4l2_async_notifier_add_fwnode_remote_subdev, + * @v4l2_async_notifier_add_i2c_subdev, + * @v4l2_async_notifier_add_devname_subdev or * @v4l2_async_notifier_parse_fwnode_endpoints. * * There is no harm from calling v4l2_async_notifier_cleanup in other
Now that most users of v4l2_async_notifier_add_subdev have been converted, let's fix the documentation so it's more clear how the v4l2-async API should be used. Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> --- .../driver-api/media/v4l2-subdev.rst | 38 ++++++++++++++++--- include/media/v4l2-async.h | 12 +++++- 2 files changed, 43 insertions(+), 7 deletions(-)