| Message ID | 20210112132339.5621-13-ezequiel@collabora.com |
|---|---|
| State | Superseded |
| 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 -- Regards, Laurent Pinchart
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. -- Regards, Sakari Ailus
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(-)