Re: [PATCH v1 1/2] media: i2c: imx219: Propagate errors from control range updates

[Tarang Raval]

Hi Dave,

> On Fri, 31 Oct 2025 at 11:49, Tarang Raval
> <tarang.raval@xxxxxxxxxxxxxxxxx> wrote:
> >
> > Propagate return values from __v4l2_ctrl_modify_range() and
> > __v4l2_ctrl_s_ctrl() in imx219_set_ctrl() and imx219_set_pad_format().
> > This ensures proper error handling instead of ignoring possible
> > failures. Also return the result of imx219_set_pad_format() from
> > imx219_init_state().
> 
> It should be impossible for a number of these to fail unless someone
> has messed up in updating the driver, but it does little harm in
> returning the error code back out.
> 
> The slight hesitation would be that in imx219_set_pad_format you could
> have updated the ranges/values of one or more controls, and then fail
> leaving a partially implemented mode change. It has returned an error,
> but an application would be reasonable in thinking that the previous
> working state has been retained when it hasn't.
> As long as it would only trigger due to a driver bug rather than user
> interaction, I would *not* propose that all values have to be saved
> and have to be restored on failure. It just gets too ugly.

Thank you for the detailed feedback and insights. I completely agree with 
your assessment regarding the potential for partial updates in the 
imx219_set_pad_format() function.

As you mentioned, such failures should only occur due to a driver bug, so 
this change mainly serves as protection against that scenario. Returning 
the error codes ensures better visibility of unexpected issues and makes 
debugging easier if a regression ever occurs. 

> > Signed-off-by: Tarang Raval <tarang.raval@xxxxxxxxxxxxxxxxx>
> 
> Reviewed-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>

Thanks!

Best Regards,
Tarang