Re: [PATCH v6 15/24] v4l: Add bus type to frame descriptors

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

Thank you for the patch.

Authorship lost here too it seems.

On Tue, Apr 27, 2021 at 03:45:14PM +0300, Tomi Valkeinen wrote:
> Add the media bus type to the frame descriptor. CSI-2 specific
> information will be added in next patch to the frame descriptor.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> - Make the bus type a named enum
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  include/media/v4l2-subdev.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index d0e9a5bdb08b..ac531b752028 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -340,12 +340,31 @@ struct v4l2_mbus_frame_desc_entry {
>  
>  #define V4L2_FRAME_DESC_ENTRY_MAX	4
>  
> +/**
> + * enum v4l2_mbus_frame_desc_type - media bus frame description type
> + *
> + * @V4L2_MBUS_FRAME_DESC_TYPE_PLATFORM:
> + *	Platform specific frame desc type for backwards compatibility.

I'd have named it UNKNOWN, or possibly LEGACY or something similar, to
convey the fact that it shouldn't be used, but is only there so that
drivers that don't set the field can be told apart. Maybe the
documentation could capture this more clearly.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> + * @V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL:
> + *	Parallel media bus.
> + * @V4L2_MBUS_FRAME_DESC_TYPE_CSI2:
> + *	CSI-2 media bus. Frame desc parameters must be set in
> + *	&struct v4l2_mbus_frame_desc_entry->csi2.
> + */
> +enum v4l2_mbus_frame_desc_type {
> +	V4L2_MBUS_FRAME_DESC_TYPE_PLATFORM,
> +	V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL,
> +	V4L2_MBUS_FRAME_DESC_TYPE_CSI2,
> +};
> +
>  /**
>   * struct v4l2_mbus_frame_desc - media bus data frame description
> + * @type: type of the bus (enum v4l2_mbus_frame_desc_type)
>   * @entry: frame descriptors array
>   * @num_entries: number of entries in @entry array
>   */
>  struct v4l2_mbus_frame_desc {
> +	enum v4l2_mbus_frame_desc_type type;
>  	struct v4l2_mbus_frame_desc_entry entry[V4L2_FRAME_DESC_ENTRY_MAX];
>  	unsigned short num_entries;
>  };

-- 
Regards,

Laurent Pinchart