Hi Philipp,
(CC'ing Sakari, Jacopo, Kieran and Niklas)
Thank you for the patch.
On Tue, Sep 24, 2019 at 01:49:53PM +0200, Philipp Zabel wrote:
> Add a subdevice video operation that allows to query the number
> of data lanes a MIPI CSI-2 TX is actively transmitting on.
>
> Suggested-by: Hans Verkuil <hverk...@xs4all.nl>
> Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
> ---
> New in v4.
> ---
> include/media/v4l2-subdev.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 71f1f2f0da53..bb71eedc38f6 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -411,6 +411,8 @@ struct v4l2_mbus_frame_desc {
> * @s_rx_buffer: set a host allocated memory buffer for the subdev. The
> subdev
> * can adjust @size to a lower value and must not write more data to the
> * buffer starting at @data than the original value of @size.
> + *
> + * @g_csi_active_lanes: Get number of currently active MIPI CSI-2 data lanes.
> */
> struct v4l2_subdev_video_ops {
> int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32
> config);
> @@ -441,6 +443,7 @@ struct v4l2_subdev_video_ops {
> const struct v4l2_mbus_config *cfg);
> int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
> unsigned int *size);
> + int (*g_csi_active_lanes)(struct v4l2_subdev *sd, u32 *lanes);
This shouldn't be a video operation but a pad operation, as a subdev
could have multiple CSI-2 pads.
Furthermore, you need to define the semantics of this operation more
precisely. When can it be called, when is the information valid ? Can
the subdev change the number of lanes it supports at runtime ? If so,
how are race conditions avoided ? All this needs to be documented.
Finally, the number of lanes is far from being the only information
about a physical bus that could be interesting for a remote subdev. I
would much prefer a more generic operation to retrieve bus
information/configuration, with a data structure that we will be able to
extend later.
> };
>
> /**
--
Regards,
Laurent Pinchart
