Re: [PATCH 38/57] media: atomisp: ov2680: Fix ov2680_enum_frame_interval()

Andy Shevchenko
2023-01-24T10:42:21Z

On Mon, Jan 23, 2023 at 01:51:46PM +0100, Hans de Goede wrote:
> Fix and simplify ov2680_enum_frame_interval(), the index is not
> an index into ov2680_res_preview[], so using N_PREVIEW is wrong.
>
> Instead it is an index indexing the different framerates for
> the resolution specified in fie->width, fie->height.
>
> Since the ov2680 code only supports a single fixed 30 fps,
> index must always be 0 and we don't need to check the other
> fie input values.

Reviewed-by: Andy Shevchenko <andy@kernel.org>

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> .../staging/media/atomisp/i2c/atomisp-ov2680.c | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> index 09c260ac93bf..75d09c44202c 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> @@ -718,19 +718,12 @@ static int ov2680_enum_frame_interval(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_frame_interval_enum *fie)
> {
> - struct v4l2_fract fract;
> -
> - if (fie->index >= N_RES_PREVIEW ||
> - fie->width > ov2680_res_preview[0].width ||
> - fie->height > ov2680_res_preview[0].height ||
> - fie->which > V4L2_SUBDEV_FORMAT_ACTIVE)
> + /* Only 1 framerate */
> + if (fie->index)
> return -EINVAL;
>
> - fract.numerator = 1;
> - fract.denominator = OV2680_FPS;
> -
> - fie->interval = fract;
> -
> + fie->interval.numerator = 1;
> + fie->interval.denominator = OV2680_FPS;
> return 0;
> }
>
> --
> 2.39.0
>

--
With Best Regards,
Andy Shevchenko