Em Wed, 6 Jan 2016 17:37:26 -0600
Benoit Parrot <bparrot@xxxxxx> escreveu:
> The Camera Adaptation Layer (CAL) is a block which consists of a dual
> port CSI2/MIPI camera capture engine.
> Port #0 can handle CSI2 camera connected to up to 4 data lanes.
> Port #1 can handle CSI2 camera connected to up to 2 data lanes.
> The driver implements the required API/ioctls to be V4L2 compliant.
> Driver supports the following:
> - V4L2 API using DMABUF/MMAP buffer access based on videobuf2 api
> - Asynchronous sensor sub device registration
> - DT support
>
> Signed-off-by: Benoit Parrot <bparrot@xxxxxx>
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> ---
...
> +/* timeperframe is arbitrary and continuous */
> +static int cal_enum_frameintervals(struct file *file, void *priv,
> + struct v4l2_frmivalenum *fival)
> +{
> + struct cal_ctx *ctx = video_drvdata(file);
> + const struct cal_fmt *fmt;
> + struct v4l2_subdev_frame_size_enum fse;
> + int ret;
> +
> + if (fival->index)
> + return -EINVAL;
> +
> + fmt = find_format_by_pix(ctx, fival->pixel_format);
> + if (!fmt)
> + return -EINVAL;
> +
> + /* check for valid width/height */
> + ret = 0;
> + fse.pad = 0;
> + fse.code = fmt->code;
> + fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> + for (fse.index = 0; ; fse.index++) {
> + ret = v4l2_subdev_call(ctx->sensor, pad, enum_frame_size,
> + NULL, &fse);
> + if (ret)
> + return -EINVAL;
> +
> + if ((fival->width == fse.max_width) &&
> + (fival->height == fse.max_height))
> + break;
> + else if ((fival->width >= fse.min_width) &&
> + (fival->width <= fse.max_width) &&
> + (fival->height >= fse.min_height) &&
> + (fival->height <= fse.max_height))
> + break;
> +
> + return -EINVAL;
> + }
> +
> + fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
> + fival->discrete.numerator = 1;
> + fival->discrete.denominator = 30;
> +
> + return 0;
> +}
The above routine is too complex and sounds wrong. Why do you
need a loop there, if the loop will either return -EINVAL or
be aborted the first time it runs?
The way it is, it is just confusing and produces a smatch error:
drivers/media/platform/ti-vpe/cal.c:1219 cal_enum_frameintervals() info: ignoring unreachable code.
If all you want here is to run the loop once, this patch would do the
same, with a clearer logic.
ti-vpe/cal: Simplify the logic to avoid confusing smatch
drivers/media/platform/ti-vpe/cal.c:1219 cal_enum_frameintervals() info: ignoring unreachable code.
This is caused by a very confusing logic that looks like a loop, but
it runs only once.
Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx>
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 35fa1071c5b2..62721ee7b9bc 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1216,29 +1216,28 @@ static int cal_enum_frameintervals(struct file *file, void *priv,
fse.pad = 0;
fse.code = fmt->code;
fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;
- for (fse.index = 0; ; fse.index++) {
- ret = v4l2_subdev_call(ctx->sensor, pad, enum_frame_size,
- NULL, &fse);
- if (ret)
- return -EINVAL;
-
- if ((fival->width == fse.max_width) &&
- (fival->height == fse.max_height))
- break;
- else if ((fival->width >= fse.min_width) &&
- (fival->width <= fse.max_width) &&
- (fival->height >= fse.min_height) &&
- (fival->height <= fse.max_height))
- break;
+ fse.index = 0;
+ ret = v4l2_subdev_call(ctx->sensor, pad, enum_frame_size,
+ NULL, &fse);
+ if (ret)
return -EINVAL;
- }
fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
fival->discrete.numerator = 1;
fival->discrete.denominator = 30;
- return 0;
+ if ((fival->width == fse.max_width) &&
+ (fival->height == fse.max_height))
+ return 0;
+
+ if ((fival->width >= fse.min_width) &&
+ (fival->width <= fse.max_width) &&
+ (fival->height >= fse.min_height) &&
+ (fival->height <= fse.max_height))
+ return 0;
+
+ return -EINVAL;
}
/*
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
