Re: [PATCH] fixes! [max9286]: Validate link formats

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

 



Hi Kieran,

On Mon, May 18, 2020 at 05:11:59PM +0100, Kieran Bingham wrote:
> Disallow setting a format on the source link, but enable link validation
> by returning the format of the first bound source when getting the
> format of the source pad.
>
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
> ---
>  drivers/media/i2c/max9286.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> index ef824d2b26b8..6c01595936d7 100644
> --- a/drivers/media/i2c/max9286.c
> +++ b/drivers/media/i2c/max9286.c
> @@ -711,7 +711,10 @@ static int max9286_set_fmt(struct v4l2_subdev *sd,
>  	struct max9286_priv *priv = sd_to_max9286(sd);
>  	struct v4l2_mbus_framefmt *cfg_fmt;
>
> -	if (format->pad >= MAX9286_SRC_PAD)
> +	/* \todo: Multiplexed streams support

I know where \todo comes from, but it's usually TODO.
Also
        /*
         * TODO:

if the comment is multiline

> +	 * Prevent setting the format on the shared multiplexed bus.
> +	 */
> +	if (format->pad == MAX9286_SRC_PAD)
>  		return -EINVAL;
>
>  	/* Refuse non YUV422 formats as we hardcode DT to 8 bit YUV422 */
> @@ -743,11 +746,17 @@ static int max9286_get_fmt(struct v4l2_subdev *sd,
>  {
>  	struct max9286_priv *priv = sd_to_max9286(sd);
>  	struct v4l2_mbus_framefmt *cfg_fmt;
> +	unsigned int pad = format->pad;
>
> -	if (format->pad >= MAX9286_SRC_PAD)
> -		return -EINVAL;
> +	/* \todo: Multiplexed Stream Support
> +	 * Support link validation by returning the format of the first bound
> +	 * link. All links must have the same format, as we do not support
> +	 * mixing, and matching of cameras connected to the max9286.
> +	 */
> +	if (format->pad == MAX9286_SRC_PAD)

You can use pad

> +		pad = ffs(priv->bound_sources);
>
> -	cfg_fmt = max9286_get_pad_format(priv, cfg, format->pad, format->which);
> +	cfg_fmt = max9286_get_pad_format(priv, cfg, pad, format->which);

Or you could add one entry to struct max9286_priv.fmt for the source
pad format, set the format there when is set on one sink, and just
drop the check on the pad index in get_fmt. Up to you.

Please squash in v10

Thanks
  j

>  	if (!cfg_fmt)
>  		return -EINVAL;
>
> --
> 2.25.1
>



[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