[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y4DmQ8DftK9/WLXM@valkosipuli.retiisi.eu>
Date: Fri, 25 Nov 2022 17:58:59 +0200
From: Sakari Ailus <sakari.ailus@....fi>
To: shravan kumar <shravan.chippa@...rochip.com>
Cc: paul.j.murphy@...el.com, daniele.alessandrelli@...el.com,
mchehab@...nel.org, linux-media@...r.kernel.org,
linux-kernel@...r.kernel.org, Jacopo Mondi <jacopo@...ndi.org>
Subject: Re: [PATCH v5 3/6] media: i2c: imx334: hblank set function modify
Hi Shravan,
Thanks for the patch.
On Fri, Nov 25, 2022 at 10:38:04AM +0530, shravan kumar wrote:
> From: Shravan Chippa <shravan.chippa@...rochip.com>
>
> -If we one modes hblank will not change and it is readonly
> -If we have multipull modes and if we do mode switch, hblank value will
> change and __v4l2_ctrl_s_ctrl() returns error, so modified
> function from __v4l2_ctrl_s_ctrl() to __v4l2_ctrl_modify_range()
> will updated all values max, min default it is not showing any error
> while changing the mode and update value
I think the commit message needs some more work.
>
> Suggested-by: Jacopo Mondi <jacopo@...ndi.org>
> Signed-off-by: Shravan Chippa <shravan.chippa@...rochip.com>
> ---
> drivers/media/i2c/imx334.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
> index acc9f9f15e47..d3bb62c162b3 100644
> --- a/drivers/media/i2c/imx334.c
> +++ b/drivers/media/i2c/imx334.c
> @@ -382,7 +382,8 @@ static int imx334_update_controls(struct imx334 *imx334,
> if (ret)
> return ret;
>
> - ret = __v4l2_ctrl_s_ctrl(imx334->hblank_ctrl, mode->hblank);
> + ret = __v4l2_ctrl_modify_range(imx334->hblank_ctrl, IMX334_REG_MIN,
> + IMX334_REG_MAX, 1, mode->hblank);
I'd use mode->hblank for all the three values. This won't be settable by
the user anyway.
I wonder if IMX334_REG_MAX is correct as it would appear to require 20
bits, not 16, to store the value.
> if (ret)
> return ret;
>
--
Kind regards,
Sakari Ailus
Powered by blists - more mailing lists