diff mbox series

[v2,3/6] media: v4l2-subdev: Expand get_mbus_config doc

Message ID 20200415105004.2497356-4-jacopo+renesas@jmondi.org (mailing list archive)
State New, archived
Headers show
Series v4l2-subdev: Introduce get_mbus_format pad op | expand

Commit Message

Jacopo Mondi April 15, 2020, 10:50 a.m. UTC
Expand documentation of the newly introduced get_mbus_config() pad
operation.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---

Providing this as separate patch to ease review/discussion.
Can be likely squashed in 1/6

---
 include/media/v4l2-subdev.h | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

--
2.26.0

Comments

Laurent Pinchart April 20, 2020, 1:52 a.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Wed, Apr 15, 2020 at 12:50:00PM +0200, Jacopo Mondi wrote:
> Expand documentation of the newly introduced get_mbus_config() pad
> operation.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> 
> Providing this as separate patch to ease review/discussion.
> Can be likely squashed in 1/6

Yes, I think it should be squashed.

> ---
>  include/media/v4l2-subdev.h | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 9bf14c41626d..e95f44e778a6 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -737,7 +737,17 @@ struct v4l2_subdev_pad_config {
>   * @set_frame_desc: set the low level media bus frame parameters, @fd array
>   *                  may be adjusted by the subdev driver to device capabilities.
>   *
> - * @get_mbus_config: get the current mbus configuration
> + * @get_mbus_config: get the current media bus configuration. This operation is
> + *		     intended to be used to synchronize the media bus
> + *		     configuration parameters between receivers and
> + *		     transmitters. The static bus configuration is usually
> + *		     received from the firmware interface, and updated
> + *		     dynamically using this operation to retrieve bus
> + *		     configuration parameters which could change at run-time.
> + *		     Callers should make sure they get the most up-to-date as
> + *		     possible configuration from the connected sub-device,
> + *		     likely calling this operation as close as possible to
> + *		     stream on time.

Much better than a single line, but still quite imprecise :-S I think we
need to describe clearly when the implementer of this operation is
allowed to change the bus configuration for instance, and we need to
think about the locking model between the subdev and the caller.

The other option is to consider that most subdev operations are
currently under-documented and keep going with the flow :-) I will thus
not block this patch series due to the documentation.

>   */
>  struct v4l2_subdev_pad_ops {
>  	int (*init_cfg)(struct v4l2_subdev *sd,
diff mbox series

Patch

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 9bf14c41626d..e95f44e778a6 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -737,7 +737,17 @@  struct v4l2_subdev_pad_config {
  * @set_frame_desc: set the low level media bus frame parameters, @fd array
  *                  may be adjusted by the subdev driver to device capabilities.
  *
- * @get_mbus_config: get the current mbus configuration
+ * @get_mbus_config: get the current media bus configuration. This operation is
+ *		     intended to be used to synchronize the media bus
+ *		     configuration parameters between receivers and
+ *		     transmitters. The static bus configuration is usually
+ *		     received from the firmware interface, and updated
+ *		     dynamically using this operation to retrieve bus
+ *		     configuration parameters which could change at run-time.
+ *		     Callers should make sure they get the most up-to-date as
+ *		     possible configuration from the connected sub-device,
+ *		     likely calling this operation as close as possible to
+ *		     stream on time.
  */
 struct v4l2_subdev_pad_ops {
 	int (*init_cfg)(struct v4l2_subdev *sd,