diff mbox series

[v5,3/6] media: i2c: imx334: hblank set function modify

Message ID 20221125050807.1857479-4-shravan.chippa@microchip.com (mailing list archive)
State New, archived
Headers show
Series media: i2c: imx334: support lower bandwidth mode | expand

Commit Message

shravan kumar Nov. 25, 2022, 5:08 a.m. UTC 
From: Shravan Chippa <shravan.chippa@microchip.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

Suggested-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Shravan Chippa <shravan.chippa@microchip.com>
---
 drivers/media/i2c/imx334.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Sakari Ailus Nov. 25, 2022, 3:58 p.m. UTC | #1 
Hi Shravan,

Thanks for the patch.

On Fri, Nov 25, 2022 at 10:38:04AM +0530, shravan kumar wrote:
> From: Shravan Chippa <shravan.chippa@microchip.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@jmondi.org>
> Signed-off-by: Shravan Chippa <shravan.chippa@microchip.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;
>
diff mbox series

Patch

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);
 	if (ret)
 		return ret;