Re: [PATCH] media: ov8865: Preserve hflip in ov8865_mode_binning_configure

[Hans de Goede <hansg@kernel.org>]

Hi,

On 17-Jul-25 11:07 PM, Allen Ballway wrote:
> Prevents ov8865_mode_binning_configure from overwriting the hflip
> register values. Allows programs to configure the hflip.
> 
> Signed-off-by: Allen Ballway <ballway@chromium.org>

Thank you for your patch.

> ---
>  drivers/media/i2c/ov8865.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
> index 95ffe7536aa6aba814f4e5c3d12e7279470b2f07..40a852d31f13aff960acfd09b378d71525e19332 100644
> --- a/drivers/media/i2c/ov8865.c
> +++ b/drivers/media/i2c/ov8865.c
> @@ -1746,7 +1746,13 @@ static int ov8865_mode_binning_configure(struct ov8865_sensor *sensor,
>  	if (ret)
>  		return ret;
>  
> -	value = OV8865_FORMAT2_HSYNC_EN;
> +	ret = ov8865_read(sensor, OV8865_FORMAT2_REG, &value);
> +	if (ret)
> +		return ret;
> +
> +	value &= OV8865_FORMAT2_FLIP_HORZ_ISP_EN |
> +		  OV8865_FORMAT2_FLIP_HORZ_SENSOR_EN;
> +	value |= OV8865_FORMAT2_HSYNC_EN;
>  
>  	if (mode->binning_x)
>  		value |= OV8865_FORMAT2_FST_HBIN_EN;

this change should not be necessary. Lets assume we start
with the sensor runtime-suspended, then ov8865_resume()
will call:

ov8865_sensor_power(true)
ov8865_sensor_init()
  ov8865_state_configure()
    ov8865_mode_configure()
      ov8865_mode_binning_configure()
__v4l2_ctrl_handler_setup()

Where the __v4l2_ctrl_handler_setup() call will apply
all control settings including hflip.

So unless you manage to hit a code-path where somehow
ov8865_state_configure() gets called without calling
__v4l2_ctrl_handler_setup() afterwards then this should
not be necessary.

Note the whole runtime-pm / calling of ov8865_sensor_init() /
ov8865_state_configure() code in this driver is somewhat
unusual. So it could be there is a bug there.

But I don't believe that this patch is the correct fix.

Regards,

Hans





> 
> ---
> base-commit: 6832a9317eee280117cd695fa885b2b7a7a38daf
> change-id: 20250717-su-94b187fa3d1e
> 
> Best regards,