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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
> 
> 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,

-- 
Regards,

Laurent Pinchart



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux