Re: [PATCH v3 05/17] media: i2c: os05b10: add V4L2 digital gain control

[Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>]

On 7/18/26 23:08, Tarang Raval wrote:
> Stop programming digital gain in the common register sequence and expose it as
> a V4L2_CID_DIGITAL_GAIN control. Initialize the new control and handle writes
> in the ctrl callback.
> 
> Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
> Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
> ---
>   drivers/media/i2c/os05b10.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/os05b10.c b/drivers/media/i2c/os05b10.c
> index 12c4e2610d6f..abf3d2970900 100644
> --- a/drivers/media/i2c/os05b10.c
> +++ b/drivers/media/i2c/os05b10.c
> @@ -138,7 +138,6 @@ static const struct cci_reg_sequence os05b10_common_regs[] = {
>   	{ CCI_REG8(0x3503), 0x88 },
>   	{ OS05B10_REG_DIGITAL_GAIN, 0x0400 },
>   	{ OS05B10_REG_ANALOG_GAIN_SHORT, 0x0080 },
> -	{ OS05B10_REG_DIGITAL_GAIN_SHORT, OS05B10_DIGITAL_GAIN_DEFAULT },
>   	{ OS05B10_REG_EXPOSURE_SHORT, 0x000020 },
>   	{ CCI_REG8(0x3600), 0x4d },
>   	{ CCI_REG8(0x3601), 0x08 },
> @@ -549,6 +548,10 @@ static int os05b10_set_ctrl(struct v4l2_ctrl *ctrl)
>   		ret = cci_write(os05b10->cci, OS05B10_REG_ANALOG_GAIN,
>   				ctrl->val, NULL);
>   		break;
> +	case V4L2_CID_DIGITAL_GAIN:
> +		ret = cci_write(os05b10->cci, OS05B10_REG_DIGITAL_GAIN,
> +				ctrl->val, NULL);

OS05B10_REG_DIGITAL_GAIN_SHORT control is removed, but
OS05B10_REG_DIGITAL_GAIN is left in the long sequence and rewritten here.

If OS05B10_REG_DIGITAL_GAIN_SHORT control is just not needed, please
remove it from the long sequence in advance, and then this change will
touch the remaining OS05B10_REG_DIGITAL_GAIN control only.

> +		break;
>   	case V4L2_CID_EXPOSURE:
>   		ret = cci_write(os05b10->cci, OS05B10_REG_EXPOSURE,
>   				ctrl->val, NULL);
> @@ -925,7 +928,7 @@ static int os05b10_init_controls(struct os05b10 *os05b10)
>   	int ret;
>   
>   	ctrl_hdlr = &os05b10->handler;
> -	v4l2_ctrl_handler_init(ctrl_hdlr, 8);
> +	v4l2_ctrl_handler_init(ctrl_hdlr, 9);
>   
>   	pixel_rate = os05b10_pixel_rate(os05b10, mode);
>   	v4l2_ctrl_new_std(ctrl_hdlr, &os05b10_ctrl_ops, V4L2_CID_PIXEL_RATE,
> @@ -967,6 +970,10 @@ static int os05b10_init_controls(struct os05b10 *os05b10)
>   					  OS05B10_ANALOG_GAIN_STEP,
>   					  OS05B10_ANALOG_GAIN_DEFAULT);
>   
> +	v4l2_ctrl_new_std(ctrl_hdlr, &os05b10_ctrl_ops, V4L2_CID_DIGITAL_GAIN,
> +			  OS05B10_DIGITAL_GAIN_MIN, OS05B10_DIGITAL_GAIN_MAX,
> +			  OS05B10_DIGITAL_GAIN_STEP, OS05B10_DIGITAL_GAIN_DEFAULT);
> +
>   	if (ctrl_hdlr->error) {
>   		ret = ctrl_hdlr->error;
>   		dev_err(os05b10->dev, "control init failed (%d)\n", ret);

-- 
Best wishes,
Vladimir