Re: [PATCH v2 09/11] media: ov2740: add manual white balance controls

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Thu, Aug 27, 2026 at 08:17:54PM +0200, Maurizio Casciano wrote:
> The sensor has separate red, green and blue manual white-balance gain
> registers, but the driver currently writes the same digital-gain value
> to all three channels. This prevents userspace from correcting the
> strong color cast of raw Bayer capture.
> 
> Expose red- and blue-balance controls relative to the digital gain,
> update all three channels under group hold, and always release and
> launch the group even when a channel write fails.
> 
> Tested on the Yoga Book OV2740 with live gain changes and continuous raw
> capture.

...

> -static int ov2740_update_digital_gain(struct ov2740 *ov2740, u32 d_gain)
> +static int ov2740_update_mwb_gains(struct ov2740 *ov2740)
>  {
> -	int ret;
> +	u32 blue_gain, green_gain, red_gain;
> +	int end_ret, launch_ret, ret;
> +
> +	/* Balance controls use 1024 as unity relative to the digital gain. */
> +	green_gain = ov2740->digital_gain->val;
> +	red_gain = DIV_ROUND_CLOSEST(green_gain * ov2740->red_balance->val,
> +				     OV2740_DGTL_GAIN_DEFAULT);
> +	red_gain = min(red_gain, OV2740_DGTL_GAIN_MAX);
> +	blue_gain = DIV_ROUND_CLOSEST(green_gain * ov2740->blue_balance->val,
> +				      OV2740_DGTL_GAIN_DEFAULT);
> +	blue_gain = min(blue_gain, OV2740_DGTL_GAIN_MAX);

Move each of them closer to their first user(s) respectively.

>  	ret = ov2740_write_reg(ov2740, OV2740_REG_GROUP_ACCESS, 1,
>  			       OV2740_GROUP_HOLD_START);
>  	if (ret)
>  		return ret;

red is here

	red_gain = DIV_ROUND_CLOSEST(d_gain * ov2740->red_balance->val,
				     OV2740_DGTL_GAIN_DEFAULT);
	red_gain = min(red_gain, OV2740_DGTL_GAIN_MAX);

> -	ret = ov2740_write_reg(ov2740, OV2740_REG_MWB_R_GAIN, 2, d_gain);
> +	ret = ov2740_write_reg(ov2740, OV2740_REG_MWB_R_GAIN, 2, red_gain);
>  	if (ret)
> -		return ret;
> +		goto release_group;

green is here

Perhaps leave d_gain as local variable and assign it with
ov2740->digital_gain->val.

	/* Balance controls use 1024 as unity relative to the digital gain. */
	u32 d_gain = ov2740->digital_gain->val;
	u32 blue_gain, green_gain, red_gain;
	int end_ret, launch_ret, ret;


> -	ret = ov2740_write_reg(ov2740, OV2740_REG_MWB_G_GAIN, 2, d_gain);
> +	ret = ov2740_write_reg(ov2740, OV2740_REG_MWB_G_GAIN, 2, green_gain);
>  	if (ret)
> -		return ret;
> +		goto release_group;

blue is here

> -	ret = ov2740_write_reg(ov2740, OV2740_REG_MWB_B_GAIN, 2, d_gain);
> -	if (ret)
> -		return ret;
> +	ret = ov2740_write_reg(ov2740, OV2740_REG_MWB_B_GAIN, 2, blue_gain);
>  
> -	ret = ov2740_write_reg(ov2740, OV2740_REG_GROUP_ACCESS, 1,
> -			       OV2740_GROUP_HOLD_END);
> -	if (ret)
> -		return ret;
> +release_group:
> +	end_ret = ov2740_write_reg(ov2740, OV2740_REG_GROUP_ACCESS, 1,
> +				   OV2740_GROUP_HOLD_END);
> +	launch_ret = ov2740_write_reg(ov2740, OV2740_REG_GROUP_ACCESS, 1,
> +				      OV2740_GROUP_HOLD_LAUNCH);
>  
> -	ret = ov2740_write_reg(ov2740, OV2740_REG_GROUP_ACCESS, 1,
> -			       OV2740_GROUP_HOLD_LAUNCH);
> -	return ret;
> +	return ret ?: end_ret ?: launch_ret;
>  }

-- 
With Best Regards,
Andy Shevchenko