Re: [PATCH v2 2/2] media: i2c: add os05b10 image sensor driver

[Mehdi Djait]

Hello Himanshu,

Thank you for the patch!

On Thu, Dec 11, 2025 at 04:24:07PM +0530, Himanshu Bhavani wrote:
> Add a v4l2 subdevice driver for the Omnivision OS05B10 sensor.
> 
> The Omnivision OS05B10 image sensor with an active
> array size of 2592 x 1944.
> 
> The following features are supported:
> - Manual exposure an gain control support
> - vblank/hblank control support
> - Supported resolution: 2592 x 1944 @ 60fps (SBGGR10)
> 
> Co-developed-by: Elgin Perumbilly <elgin.perumbilly@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Elgin Perumbilly <elgin.perumbilly@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Himanshu Bhavani <himanshu.bhavani@xxxxxxxxxxxxxxxxx>

[..]

> 
> +config VIDEO_OS05B10
> +        tristate "OmniVision OS05B10 sensor support"
> +        select V4L2_CCI_I2C
> +        help
> +          This is a Video4Linux2 sensor driver for Omnivision
> +          OS05B10 camera sensor.
> +
> +	  To compile this driver as a module, choose M here: the
> +          module's name is ov9734.

ov9734 ? copy-paste error ?

> +

[..]

> +static int os05b10_probe(struct i2c_client *client)
> +{
> +	struct os05b10 *os05b10;
> +	unsigned int xclk_freq;
> +	int ret;
> +
> +	os05b10 = devm_kzalloc(&client->dev, sizeof(*os05b10), GFP_KERNEL);
> +	if (!os05b10)
> +		return -ENOMEM;
> +
> +	os05b10->client = client;
> +	os05b10->dev = &client->dev;
> +
> +	v4l2_i2c_subdev_init(&os05b10->sd, client, &os05b10_subdev_ops);
> +
> +	os05b10->cci = devm_cci_regmap_init_i2c(client, 16);
> +	if (IS_ERR(os05b10->cci))
> +		return dev_err_probe(os05b10->dev, PTR_ERR(os05b10->cci),
> +				     "failed to initialize CCI\n");
> +
> +	/* Get system clock (xclk) */

I don't think this comment adds value

> +	os05b10->xclk = devm_clk_get(os05b10->dev, NULL);

Use devm_v4l2_sensor_clk_get() to get the clock

> +	if (IS_ERR(os05b10->xclk))
> +		return dev_err_probe(os05b10->dev, PTR_ERR(os05b10->xclk),
> +				     "failed to get xclk\n");
> +
> +	xclk_freq = clk_get_rate(os05b10->xclk);
> +	if (xclk_freq != OS05B10_XCLK_FREQ)
> +		return dev_err_probe(os05b10->dev, -EINVAL,
> +				     "xclk frequency not supported: %d Hz\n",
> +				     xclk_freq);
> +
> +	ret = os05b10_get_regulators(os05b10);
> +	if (ret)
> +		return dev_err_probe(os05b10->dev, ret, "failed to get regulators\n");
> +
> +	ret = os05b10_parse_endpoint(os05b10);
> +	if (ret) {
> +		dev_err_probe(os05b10->dev, ret,
> +			      "failed to parse endpoint configuration\n");
> +		return ret;

return dev_err_probe() like above.

> +	}
> +
> +	os05b10->reset_gpio = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(os05b10->reset_gpio))
> +		return dev_err_probe(os05b10->dev, PTR_ERR(os05b10->reset_gpio),
> +				     "failed to get reset GPIO\n");
> +

--
Kind Regards
Mehdi Djait