Re: [PATCH v2] media: i2c: imx471: Fix uninitialized error value in imx471_set_ctrl()

[Kate Hsuan]

Hi David,

Thank you for your work.

On Tue, Jul 28, 2026 at 5:24 PM David Carlier <devnexen@xxxxxxxxx> wrote:
>
> The exposure and vertical blanking writes pass the address of the local
> ret variable to cci_write() as its error pointer, but there is no earlier
> error to propagate: each case is a single standalone write, like the other
> controls in the same switch that already pass NULL. In the exposure case
> ret is still uninitialized, so a non-zero stack value makes cci_write()
> return early without programming the register, and the control write
> reports a bogus status. The vertical blanking case is benign today because
> ret is zero there, but the construct is equally wrong.
>
> Pass NULL as the error pointer in both cases.
>
> Fixes: be1589e567ae ("media: i2c: imx471: Add Sony IMX471 image sensor driver")
> Suggested-by: Kate Hsuan <hpa@xxxxxxxxxx>
> Signed-off-by: David Carlier <devnexen@xxxxxxxxx>
> ---
> Changes in v2:
> - Fix the root cause by passing NULL as the cci_write() error pointer for
>   the exposure and vertical blanking writes, instead of initializing ret
>   to zero (Kate Hsuan).
>
> v1: https://lore.kernel.org/linux-media/20260724041417.9921-1-devnexen@xxxxxxxxx/
>
>  drivers/media/i2c/imx471.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/imx471.c b/drivers/media/i2c/imx471.c
> index 6d358b11e96d..4053aed84340 100644
> --- a/drivers/media/i2c/imx471.c
> +++ b/drivers/media/i2c/imx471.c
> @@ -334,12 +334,12 @@ static int imx471_set_ctrl(struct v4l2_ctrl *ctrl)
>                 break;
>         case V4L2_CID_EXPOSURE:
>                 ret = cci_write(sensor->regmap, IMX471_REG_EXPOSURE,
> -                               ctrl->val, &ret);
> +                               ctrl->val, NULL);
>                 break;
>         case V4L2_CID_VBLANK:
>                 /* Update FLL that meets expected vertical blanking */
>                 ret = cci_write(sensor->regmap, IMX471_REG_FLL,
> -                               format->height + ctrl->val, &ret);
> +                               format->height + ctrl->val, NULL);
>                 break;
>         case V4L2_CID_TEST_PATTERN:
>                 ret = cci_write(sensor->regmap, IMX471_REG_TEST_PATTERN,
> --
> 2.53.0
>

It is good for me.

Reviewed-by: Kate Hsuan <hpa@xxxxxxxxxx>

-- 
BR,
Kate