Hi Hans,
On Sun, 3 May 2015, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>
> Simplify ov2640_set_params and ov2640_set_fmt.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> Reported-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> ---
> drivers/media/i2c/soc_camera/ov2640.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/i2c/soc_camera/ov2640.c b/drivers/media/i2c/soc_camera/ov2640.c
> index 9b4f5de..5dcaf24 100644
> --- a/drivers/media/i2c/soc_camera/ov2640.c
> +++ b/drivers/media/i2c/soc_camera/ov2640.c
> @@ -769,15 +769,15 @@ static const struct ov2640_win_size *ov2640_select_win(u32 *width, u32 *height)
> return &ov2640_supported_win_sizes[default_size];
> }
>
> -static int ov2640_set_params(struct i2c_client *client, u32 *width, u32 *height,
> - u32 code)
> +static int ov2640_set_params(struct i2c_client *client,
> + const struct ov2640_win_size *win, u32 code)
> {
> struct ov2640_priv *priv = to_ov2640(client);
> const struct regval_list *selected_cfmt_regs;
> int ret;
>
> /* select win */
> - priv->win = ov2640_select_win(width, height);
> + priv->win = win;
>
> /* select format */
> priv->cfmt_code = 0;
> @@ -798,6 +798,7 @@ static int ov2640_set_params(struct i2c_client *client, u32 *width, u32 *height,
> case MEDIA_BUS_FMT_UYVY8_2X8:
> dev_dbg(&client->dev, "%s: Selected cfmt UYVY", __func__);
> selected_cfmt_regs = ov2640_uyvy_regs;
> + break;
Hm, IIRC, some versions of gcc complain like "break at the end of a switch
statement is deprecated." Why did you add this at two locations? Are you
seeing a warning? If not, maybe better not do that.
Otherwise
Acked-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
Thanks
Guennadi
> }
>
> /* reset hardware */
> @@ -832,8 +833,6 @@ static int ov2640_set_params(struct i2c_client *client, u32 *width, u32 *height,
> goto err;
>
> priv->cfmt_code = code;
> - *width = priv->win->width;
> - *height = priv->win->height;
>
> return 0;
>
> @@ -887,14 +886,13 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
> {
> struct v4l2_mbus_framefmt *mf = &format->format;
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> + const struct ov2640_win_size *win;
>
> if (format->pad)
> return -EINVAL;
>
> - /*
> - * select suitable win, but don't store it
> - */
> - ov2640_select_win(&mf->width, &mf->height);
> + /* select suitable win */
> + win = ov2640_select_win(&mf->width, &mf->height);
>
> mf->field = V4L2_FIELD_NONE;
>
> @@ -905,14 +903,15 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
> break;
> default:
> mf->code = MEDIA_BUS_FMT_UYVY8_2X8;
> + /* fall through */
> case MEDIA_BUS_FMT_YUYV8_2X8:
> case MEDIA_BUS_FMT_UYVY8_2X8:
> mf->colorspace = V4L2_COLORSPACE_JPEG;
> + break;
> }
>
> if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> - return ov2640_set_params(client, &mf->width,
> - &mf->height, mf->code);
> + return ov2640_set_params(client, win, mf->code);
> cfg->try_fmt = *mf;
> return 0;
> }
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
