Em Tue, 2 Feb 2021 15:56:11 +0200
Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> escreveu:
> From: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
>
> 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.
>
> Document functions that drivers should use, and their purpose.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> .../driver-api/media/v4l2-subdev.rst | 48 +++++++++++++++----
> include/media/v4l2-async.h | 15 ++++--
> 2 files changed, 50 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst
> index 0e82c77cf3e2..8b53da2f9c74 100644
> --- a/Documentation/driver-api/media/v4l2-subdev.rst
> +++ b/Documentation/driver-api/media/v4l2-subdev.rst
> @@ -197,15 +197,45 @@ unregister the notifier the driver has to call
> takes two arguments: a pointer to struct :c:type:`v4l2_device` and a
> pointer to struct :c:type:`v4l2_async_notifier`.
>
> -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`.
> +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. Several functions are available
> +to add subdevice descriptors to a notifier, depending on the type of device and
> +the needs of the driver.
> +
> +:c:func:`v4l2_async_notifier_add_fwnode_remote_subdev` and
> +:c:func:`v4l2_async_notifier_add_i2c_subdev` are for bridge and ISP drivers for
> +registering their async sub-devices with the notifier.
> +
> +:c:func:`v4l2_async_register_subdev_sensor_common` is a helper function for
> +sensor drivers registering their own async sub-device, but it also registers a
> +notifier and further registers async sub-devices for lens and flash devices
> +found in firmware. The notifier for the sub-device is unregistered with the
> +async sub-device.
> +
> +These functions allocate an async sub-device descriptor which is of type struct
> +:c:type:`v4l2_async_subdev` embedded in a driver-specific struct. The &struct
> +:c:type:`v4l2_async_subdev` shall be the first member of this struct:
There's absolutely no need anymore to use:
struct :c:type:`v4l2_async_subdev`
or
:c:func:`v4l2_async_notifier_add_fwnode_remote_subdev`
In a matter of fact, this can even cause troubles with newer versions of
Sphinx, as, after version 3.0, structs should be declared as:
:c:struct:`foo`
Our building system has gained a few years ago a Sphinx extension that
will automatically use the right markup, if all structs are declared
as:
struct foo
and all functions as:
bar()
So, the last two paragraphs could be simply:
v4l2_async_register_subdev_sensor_common() is a helper function for
sensor drivers registering their own async sub-device, but it also registers a
notifier and further registers async sub-devices for lens and flash devices
found in firmware. The notifier for the sub-device is unregistered with the
async sub-device.
These functions allocate an async sub-device descriptor which is of type
struct v4l2_async_subdev embedded in a driver-specific struct. The
struct v4l2_async_subdev shall be the first member of this struct:
PS.: I guess the automarkup.py would accept having something like:
very big line here with lots of words... struct
foo
IMHO, for people reading the text files, it is a lot easier to keep
"struct foo" at the same line, like:
very big line here with lots of words...
struct foo
> +
> +.. code-block:: c
> +
> + struct my_async_subdev {
> + struct v4l2_async_subdev asd;
> + ...
> + };
> +
> + struct my_async_subdev *my_asd;
> + struct fwnode_handle *ep;
> +
> + ...
> +
> + my_asd = v4l2_async_notifier_add_fwnode_remote_subdev(¬ifier, ep,
> + struct my_async_subdev);
> + fwnode_handle_put(ep);
> +
> + if (IS_ERR(asd))
> + return PTR_ERR(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 192a11bdc4ad..6dac6cb6290f 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -128,7 +128,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_remote_subdev,
> + * @v4l2_async_notifier_add_fwnode_subdev,
> + * @v4l2_async_notifier_add_i2c_subdev,
> + * @v4l2_async_notifier_add_subdev or
> + * @v4l2_async_notifier_parse_fwnode_endpoints.
The markups here are wrong:
'@foo' is to be used for literal blocks. It won't produce
any cross-references. The right way to describe functions is to
write it as:
foo()
> */
> void v4l2_async_notifier_init(struct v4l2_async_notifier *notifier);
>
> @@ -262,9 +267,11 @@ 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,
> - * @v4l2_async_notifier_parse_fwnode_endpoints or
> - * @v4l2_fwnode_reference_parse_sensor_common.
> + * @v4l2_async_notifier_add_fwnode_remote_subdev,
> + * @v4l2_async_notifier_add_fwnode_subdev,
> + * @v4l2_async_notifier_add_i2c_subdev,
> + * @v4l2_async_notifier_add_subdev or
> + * @v4l2_async_notifier_parse_fwnode_endpoints.
> *
> * There is no harm from calling v4l2_async_notifier_cleanup in other
> * cases as long as its memory has been zeroed after it has been
Please send a followup patch.
Thanks,
Mauro
