Re: [PATCH v5 16/24] v4l: Add CSI-2 bus configuration to frame descriptors

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Cc: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com,
	Jacopo Mondi <jacopo+renesas@jmondi.org>,
	niklas.soderlund+renesas@ragnatech.se,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>
Subject: Re: [PATCH v5 16/24] v4l: Add CSI-2 bus configuration to frame descriptors
Date: Sun, 18 Apr 2021 22:24:54 +0300	[thread overview]
Message-ID: <YHyHhqk+SoZLlA3S@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20210415130450.421168-17-tomi.valkeinen@ideasonboard.com>

Hi Tomi and Sakari,

Thank you for the patch.

On Thu, Apr 15, 2021 at 04:04:42PM +0300, Tomi Valkeinen wrote:
> From: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> Add CSI-2 bus specific configuration to the frame descriptors. This allows
> obtaining the virtual channel and data type information for each stream
> the transmitter is sending.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  include/media/v4l2-subdev.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 85977abbea46..30ec011d31e3 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -308,6 +308,17 @@ struct v4l2_subdev_audio_ops {
>  	int (*s_stream)(struct v4l2_subdev *sd, int enable);
>  };
>  
> +/**
> + * struct v4l2_mbus_frame_desc_entry_csi2
> + *
> + * @channel: CSI-2 virtual channel

Maybe s/channel/virtual_channel/ ? Or vc and dt ?

> + * @data_type: CSI-2 data type ID
> + */
> +struct v4l2_mbus_frame_desc_entry_csi2 {
> +	u8 channel;
> +	u8 data_type;
> +};
> +
>  /**
>   * enum v4l2_mbus_frame_desc_flags - media bus frame description flags
>   *
> @@ -331,11 +342,16 @@ enum v4l2_mbus_frame_desc_flags {
>   *		%FRAME_DESC_FL_BLOB is not set.
>   * @length:	number of octets per frame, valid if @flags
>   *		%V4L2_MBUS_FRAME_DESC_FL_LEN_MAX is set.
> + * @bus:	Bus specific frame descriptor parameters

s/Bus specific/Bus-specific/

> + * @bus.csi2:	CSI-2 specific bus configuration

Ditto.

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

>   */
>  struct v4l2_mbus_frame_desc_entry {
>  	enum v4l2_mbus_frame_desc_flags flags;
>  	u32 pixelcode;
>  	u32 length;
> +	union {
> +		struct v4l2_mbus_frame_desc_entry_csi2 csi2;
> +	} bus;
>  };
>  
>  #define V4L2_FRAME_DESC_ENTRY_MAX	4

-- 
Regards,

Laurent Pinchart