On 01/22/2018 11:37 AM, Dan Carpenter wrote:
> The while loop is a post op, "while (i-- >= 0)" so the last iteration
> will read camif_mbus_formats[-1] and then the loop will exit with "i"
> set to -2 and so we do: "mf->code = camif_mbus_formats[-2];".
>
> I've changed it to a pre-op, I've added a check to ensure we found the
> right format and I've removed the "mf->code = camif_mbus_formats[i];"
> because that is a no-op anyway.
>
> Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series camera interface")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
>
> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
> index 437395a61065..012f4b389c55 100644
> --- a/drivers/media/platform/s3c-camif/camif-capture.c
> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> @@ -1261,11 +1261,11 @@ static void __camif_subdev_try_format(struct camif_dev *camif,
> /* FIXME: constraints against codec or preview path ? */
> pix_lim = &variant->vp_pix_limits[VP_CODEC];
>
> - while (i-- >= 0)
> + while (--i >= 0)
> if (camif_mbus_formats[i] == mf->code)
> break;
> -
> - mf->code = camif_mbus_formats[i];
> + if (i < 0)
> + return;
Thanks for the patch Dan. mf->width needs to be aligned by this try_format
function so we shouldn't return here. Also it needs to be ensured mf->code
is set to one of the supported values when this function returns. Sorry,
the current code really doesn't give a clue what was intended.
There is already queued a patch from Arnd [1] addressing the issues you
have found.
> if (pad == CAMIF_SD_PAD_SINK) {
> v4l_bound_align_image(&mf->width, 8, CAMIF_MAX_PIX_WIDTH,
>
[1] https://patchwork.linuxtv.org/patch/46508
--
Regards,
Sylwester
